Hi Geert Uytterhoeven,
Thanks for the feedback.
> -----Original Message-----
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxxx>
>
> 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
