| Message ID | 20231201110840.37408-2-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Add DA9062 PMIC and built-in RTC support for RZ/G2UL SMARC EVK | expand |
Hi Biju, On Fri, Dec 1, 2023 at 12:08 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > On some platforms (eg: RZ/{G2UL,Five} SMARC EVK), there is no IRQ > populated by default. Add irq optional support. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> Thanks for your patch! > --- a/drivers/rtc/rtc-da9063.c > +++ b/drivers/rtc/rtc-da9063.c > @@ -485,25 +485,26 @@ static int da9063_rtc_probe(struct platform_device *pdev) > clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->rtc_dev->features); > } > > - irq_alarm = platform_get_irq_byname(pdev, "ALARM"); > - if (irq_alarm < 0) > - return irq_alarm; > - > - ret = devm_request_threaded_irq(&pdev->dev, irq_alarm, NULL, > - da9063_alarm_event, > - IRQF_TRIGGER_LOW | IRQF_ONESHOT, > - "ALARM", rtc); > - if (ret) > - dev_err(&pdev->dev, "Failed to request ALARM IRQ %d: %d\n", > - irq_alarm, ret); > - > - ret = dev_pm_set_wake_irq(&pdev->dev, irq_alarm); > - if (ret) > - dev_warn(&pdev->dev, > - "Failed to set IRQ %d as a wake IRQ: %d\n", > - irq_alarm, ret); > - > - device_init_wakeup(&pdev->dev, true); > + irq_alarm = platform_get_irq_byname_optional(pdev, "ALARM"); > + if (irq_alarm >= 0) { > + ret = devm_request_threaded_irq(&pdev->dev, irq_alarm, NULL, > + da9063_alarm_event, > + IRQF_TRIGGER_LOW | IRQF_ONESHOT, > + "ALARM", rtc); > + if (ret) > + dev_err(&pdev->dev, "Failed to request ALARM IRQ %d: %d\n", > + irq_alarm, ret); > + > + ret = dev_pm_set_wake_irq(&pdev->dev, irq_alarm); > + if (ret) > + dev_warn(&pdev->dev, > + "Failed to set IRQ %d as a wake IRQ: %d\n", > + irq_alarm, ret); > + > + device_init_wakeup(&pdev->dev, true); > + } else { > + clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->rtc_dev->features); This does not handle and propagate real errors (e.g. -EPROBE_DEFER). } else if (irq_alarm != -ENXIO) { return irq_alarm; } else { .... } (I think -ENXIO is the correct error to check for, platform_get_irq_byname_optional() really should start returning zero for not found) > + } > > return devm_rtc_register_device(rtc->rtc_dev); Gr{oetje,eeting}s, Geert
Hi Geert Uytterhoeven, Thanks for the feedback. > -----Original Message----- > From: Geert Uytterhoeven <geert@linux-m68k.org> > Subject: Re: [PATCH 1/6] rtc: da9063: Make IRQ as optional > > Hi Biju, > > On Fri, Dec 1, 2023 at 12:08 PM Biju Das <biju.das.jz@bp.renesas.com> > wrote: > > On some platforms (eg: RZ/{G2UL,Five} SMARC EVK), there is no IRQ > > populated by default. Add irq optional support. > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > Thanks for your patch! > > > --- a/drivers/rtc/rtc-da9063.c > > +++ b/drivers/rtc/rtc-da9063.c > > @@ -485,25 +485,26 @@ static int da9063_rtc_probe(struct platform_device > *pdev) > > clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->rtc_dev- > >features); > > } > > > > - irq_alarm = platform_get_irq_byname(pdev, "ALARM"); > > - if (irq_alarm < 0) > > - return irq_alarm; > > - > > - ret = devm_request_threaded_irq(&pdev->dev, irq_alarm, NULL, > > - da9063_alarm_event, > > - IRQF_TRIGGER_LOW | IRQF_ONESHOT, > > - "ALARM", rtc); > > - if (ret) > > - dev_err(&pdev->dev, "Failed to request ALARM > IRQ %d: %d\n", > > - irq_alarm, ret); > > - > > - ret = dev_pm_set_wake_irq(&pdev->dev, irq_alarm); > > - if (ret) > > - dev_warn(&pdev->dev, > > - "Failed to set IRQ %d as a wake IRQ: %d\n", > > - irq_alarm, ret); > > - > > - device_init_wakeup(&pdev->dev, true); > > + irq_alarm = platform_get_irq_byname_optional(pdev, "ALARM"); > > + if (irq_alarm >= 0) { > > + ret = devm_request_threaded_irq(&pdev->dev, irq_alarm, > NULL, > > + da9063_alarm_event, > > + IRQF_TRIGGER_LOW | > IRQF_ONESHOT, > > + "ALARM", rtc); > > + if (ret) > > + dev_err(&pdev->dev, "Failed to request ALARM > IRQ %d: %d\n", > > + irq_alarm, ret); > > + > > + ret = dev_pm_set_wake_irq(&pdev->dev, irq_alarm); > > + if (ret) > > + dev_warn(&pdev->dev, > > + "Failed to set IRQ %d as a wake > IRQ: %d\n", > > + irq_alarm, ret); > > + > > + device_init_wakeup(&pdev->dev, true); > > + } else { > > + clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, > > + rtc->rtc_dev->features); > > This does not handle and propagate real errors (e.g. -EPROBE_DEFER). Oops. Missed propagating errors. > > } else if (irq_alarm != -ENXIO) { > return irq_alarm; > } else { > .... > } > > (I think -ENXIO is the correct error to check for, > platform_get_irq_byname_optional() really should start returning zero > for not found) Agreed, currently, if it is -ENXIO, then not found condition. Cheers, Biju > > > + } > > > > return devm_rtc_register_device(rtc->rtc_dev); > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux- > m68k.org > > In personal conversations with technical people, I call myself a hacker. > But when I'm talking to journalists I just say "programmer" or something > like that. > -- Linus Torvalds
diff --git a/drivers/rtc/rtc-da9063.c b/drivers/rtc/rtc-da9063.c index 2f5d60622564..613ba2c5d757 100644 --- a/drivers/rtc/rtc-da9063.c +++ b/drivers/rtc/rtc-da9063.c @@ -485,25 +485,26 @@ static int da9063_rtc_probe(struct platform_device *pdev) clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->rtc_dev->features); } - irq_alarm = platform_get_irq_byname(pdev, "ALARM"); - if (irq_alarm < 0) - return irq_alarm; - - ret = devm_request_threaded_irq(&pdev->dev, irq_alarm, NULL, - da9063_alarm_event, - IRQF_TRIGGER_LOW | IRQF_ONESHOT, - "ALARM", rtc); - if (ret) - dev_err(&pdev->dev, "Failed to request ALARM IRQ %d: %d\n", - irq_alarm, ret); - - ret = dev_pm_set_wake_irq(&pdev->dev, irq_alarm); - if (ret) - dev_warn(&pdev->dev, - "Failed to set IRQ %d as a wake IRQ: %d\n", - irq_alarm, ret); - - device_init_wakeup(&pdev->dev, true); + irq_alarm = platform_get_irq_byname_optional(pdev, "ALARM"); + if (irq_alarm >= 0) { + ret = devm_request_threaded_irq(&pdev->dev, irq_alarm, NULL, + da9063_alarm_event, + IRQF_TRIGGER_LOW | IRQF_ONESHOT, + "ALARM", rtc); + if (ret) + dev_err(&pdev->dev, "Failed to request ALARM IRQ %d: %d\n", + irq_alarm, ret); + + ret = dev_pm_set_wake_irq(&pdev->dev, irq_alarm); + if (ret) + dev_warn(&pdev->dev, + "Failed to set IRQ %d as a wake IRQ: %d\n", + irq_alarm, ret); + + device_init_wakeup(&pdev->dev, true); + } else { + clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->rtc_dev->features); + } return devm_rtc_register_device(rtc->rtc_dev); }
On some platforms (eg: RZ/{G2UL,Five} SMARC EVK), there is no IRQ populated by default. Add irq optional support. Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- drivers/rtc/rtc-da9063.c | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-)