Message ID | 20191109154652.7419-1-andrej.skvortzov@gmail.com |
---|---|
State | Superseded |
Headers | show |
Series | rtc: tps65910: allow using RTC without alarm interrupt | expand |
Hi, This needs a proper commit message. On 09/11/2019 18:46:52+0300, Andrey Skvortsov wrote: > Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com> > --- > drivers/rtc/rtc-tps65910.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/drivers/rtc/rtc-tps65910.c b/drivers/rtc/rtc-tps65910.c > index 2c0467a9e717..aa3a001ef413 100644 > --- a/drivers/rtc/rtc-tps65910.c > +++ b/drivers/rtc/rtc-tps65910.c > @@ -361,6 +361,13 @@ static const struct rtc_class_ops tps65910_rtc_ops = { > .set_offset = tps65910_set_offset, > }; > > +static const struct rtc_class_ops tps65910_rtc_ops_noirq = { > + .read_time = tps65910_rtc_read_time, > + .set_time = tps65910_rtc_set_time, > + .read_offset = tps65910_read_offset, > + .set_offset = tps65910_set_offset, > +}; > + > static int tps65910_rtc_probe(struct platform_device *pdev) > { > struct tps65910 *tps65910 = NULL; > @@ -415,13 +422,17 @@ static int tps65910_rtc_probe(struct platform_device *pdev) > tps65910_rtc_interrupt, IRQF_TRIGGER_LOW, > dev_name(&pdev->dev), &pdev->dev); > if (ret < 0) { > - dev_err(&pdev->dev, "IRQ is not free.\n"); > - return ret; > + dev_err(&pdev->dev, "request IRQ:%d failed, err = %d\n", > + irq, ret); Do we actually need an error message here? > + irq = -1; > } > tps_rtc->irq = irq; > - device_set_wakeup_capable(&pdev->dev, 1); > + if (irq != -1) { > + device_set_wakeup_capable(&pdev->dev, 1); > + tps_rtc->rtc->ops = &tps65910_rtc_ops; > + } else > + tps_rtc->rtc->ops = &tps65910_rtc_ops_noirq; > > - tps_rtc->rtc->ops = &tps65910_rtc_ops; > tps_rtc->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000; > tps_rtc->rtc->range_max = RTC_TIMESTAMP_END_2099; > > -- > 2.20.1 >
On 19-11-11 18:19, Alexandre Belloni wrote: > Hi, > > This needs a proper commit message. Add in v2. > On 09/11/2019 18:46:52+0300, Andrey Skvortsov wrote: > > Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com> > > --- > > drivers/rtc/rtc-tps65910.c | 19 +++++++++++++++---- > > 1 file changed, 15 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/rtc/rtc-tps65910.c b/drivers/rtc/rtc-tps65910.c > > index 2c0467a9e717..aa3a001ef413 100644 > > --- a/drivers/rtc/rtc-tps65910.c > > +++ b/drivers/rtc/rtc-tps65910.c > > @@ -361,6 +361,13 @@ static const struct rtc_class_ops tps65910_rtc_ops = { > > .set_offset = tps65910_set_offset, > > }; > > > > +static const struct rtc_class_ops tps65910_rtc_ops_noirq = { > > + .read_time = tps65910_rtc_read_time, > > + .set_time = tps65910_rtc_set_time, > > + .read_offset = tps65910_read_offset, > > + .set_offset = tps65910_set_offset, > > +}; > > + > > static int tps65910_rtc_probe(struct platform_device *pdev) > > { > > struct tps65910 *tps65910 = NULL; > > @@ -415,13 +422,17 @@ static int tps65910_rtc_probe(struct platform_device *pdev) > > tps65910_rtc_interrupt, IRQF_TRIGGER_LOW, > > dev_name(&pdev->dev), &pdev->dev); > > if (ret < 0) { > > - dev_err(&pdev->dev, "IRQ is not free.\n"); > > - return ret; > > + dev_err(&pdev->dev, "request IRQ:%d failed, err = %d\n", > > + irq, ret); > > Do we actually need an error message here? You are right. This is definitely not an error anymore. What about dev_warn(&pdev->dev, "unable to request IRQ, alarms disabled\n"); like some other drivers do? grep -RHn 'unable to request IRQ, alarms disabled' rtc-rv8803.c:576: dev_warn(&client->dev, "unable to request IRQ, alarms disabled\n"); rtc-m41t80.c:914: dev_warn(&client->dev, "unable to request IRQ, alarms disabled\n"); rtc-rv3028.c:651: dev_warn(&client->dev, "unable to request IRQ, alarms disabled\n"); rtc-pcf85363.c:416: dev_warn(&client->dev, "unable to request IRQ, alarms disabled\n"); rtc-pcf85063.c:458: "unable to request IRQ, alarms disabled\n"); rtc-rv3029c2.c:828: dev_warn(dev, "unable to request IRQ, alarms disabled\n"); rtc-rx8025.c:540: dev_err(&client->dev, "unable to request IRQ, alarms disabled\n"); rtc-abx80x.c:839: dev_err(&client->dev, "unable to request IRQ, alarms disabled\n"); > > > + irq = -1; > > } > > tps_rtc->irq = irq; > > - device_set_wakeup_capable(&pdev->dev, 1); > > + if (irq != -1) { > > + device_set_wakeup_capable(&pdev->dev, 1); > > + tps_rtc->rtc->ops = &tps65910_rtc_ops; > > + } else > > + tps_rtc->rtc->ops = &tps65910_rtc_ops_noirq; > > > > - tps_rtc->rtc->ops = &tps65910_rtc_ops; > > tps_rtc->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000; > > tps_rtc->rtc->range_max = RTC_TIMESTAMP_END_2099; > > > > -- > > 2.20.1 > > > > -- > Alexandre Belloni, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
On 14/11/2019 02:08:05+0300, Andrey Skvortsov wrote: > > > static int tps65910_rtc_probe(struct platform_device *pdev) > > > { > > > struct tps65910 *tps65910 = NULL; > > > @@ -415,13 +422,17 @@ static int tps65910_rtc_probe(struct platform_device *pdev) > > > tps65910_rtc_interrupt, IRQF_TRIGGER_LOW, > > > dev_name(&pdev->dev), &pdev->dev); > > > if (ret < 0) { > > > - dev_err(&pdev->dev, "IRQ is not free.\n"); > > > - return ret; > > > + dev_err(&pdev->dev, "request IRQ:%d failed, err = %d\n", > > > + irq, ret); > > > > Do we actually need an error message here? > > You are right. This is definitely not an error anymore. > What about > dev_warn(&pdev->dev, "unable to request IRQ, alarms disabled\n"); > like some other drivers do? > Yes, or that can be left out completely as userspace will be able to know whether alarms are supported without this message.
diff --git a/drivers/rtc/rtc-tps65910.c b/drivers/rtc/rtc-tps65910.c index 2c0467a9e717..aa3a001ef413 100644 --- a/drivers/rtc/rtc-tps65910.c +++ b/drivers/rtc/rtc-tps65910.c @@ -361,6 +361,13 @@ static const struct rtc_class_ops tps65910_rtc_ops = { .set_offset = tps65910_set_offset, }; +static const struct rtc_class_ops tps65910_rtc_ops_noirq = { + .read_time = tps65910_rtc_read_time, + .set_time = tps65910_rtc_set_time, + .read_offset = tps65910_read_offset, + .set_offset = tps65910_set_offset, +}; + static int tps65910_rtc_probe(struct platform_device *pdev) { struct tps65910 *tps65910 = NULL; @@ -415,13 +422,17 @@ static int tps65910_rtc_probe(struct platform_device *pdev) tps65910_rtc_interrupt, IRQF_TRIGGER_LOW, dev_name(&pdev->dev), &pdev->dev); if (ret < 0) { - dev_err(&pdev->dev, "IRQ is not free.\n"); - return ret; + dev_err(&pdev->dev, "request IRQ:%d failed, err = %d\n", + irq, ret); + irq = -1; } tps_rtc->irq = irq; - device_set_wakeup_capable(&pdev->dev, 1); + if (irq != -1) { + device_set_wakeup_capable(&pdev->dev, 1); + tps_rtc->rtc->ops = &tps65910_rtc_ops; + } else + tps_rtc->rtc->ops = &tps65910_rtc_ops_noirq; - tps_rtc->rtc->ops = &tps65910_rtc_ops; tps_rtc->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000; tps_rtc->rtc->range_max = RTC_TIMESTAMP_END_2099;
Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com> --- drivers/rtc/rtc-tps65910.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)