Hi Alexandre Belloni,
> -----Original Message-----
> From: Alexandre Belloni <alexandre.belloni@xxxxxxxxxxx>
> Subject: Re: [PATCH 3/6] rtc: da9063: Use dev_err_probe()
>
> On 01/12/2023 15:43:27+0000, Biju Das wrote:
> > Hi Alexandre Belloni,
> >
> > Thanks for the feedback.
> >
> > > -----Original Message-----
> > > Subject: Re: [PATCH 3/6] rtc: da9063: Use dev_err_probe()
> > >
> > > On 01/12/2023 13:30:05+0000, Biju Das wrote:
> > > > Hi Geert,
> > > >
> > > > Thanks for the feedback.
> > > >
> > > > > -----Original Message-----
> > > > > From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> > > > > Subject: Re: [PATCH 3/6] rtc: da9063: Use dev_err_probe()
> > > > >
> > > > > Hi Biju,
> > > > >
> > > > > On Fri, Dec 1, 2023 at 12:08 PM Biju Das
> > > > > <biju.das.jz@xxxxxxxxxxxxxx>
> > > > > wrote:
> > > > > > Replace dev_err()->dev_err_probe() to simpilfy probe().
> > > > > >
> > > > > > 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
> > > > > > @@ -488,8 +480,9 @@ static int da9063_rtc_probe(struct
> > > > > > platform_device
> > > > > *pdev)
> > > > > >
> > > > > > IRQF_TRIGGER_LOW |
> > > > > IRQF_ONESHOT,
> > > > > > "ALARM", rtc);
> > > > > > if (ret)
> > > > > > - dev_err(&pdev->dev, "Failed to request
> ALARM
> > > > > IRQ %d: %d\n",
> > > > > > - irq_alarm, ret);
> > > > > > + return dev_err_probe(&pdev->dev, ret,
> > > > > > + "Failed to
> > > > > > + request ALARM
> > > > > IRQ %d\n",
> > > > > > + irq_alarm);
> > > > >
> > > > > This changes behavior: before, this was not considered fatal.
> > > >
> > > > Agreed. Maybe a separate patch?
> > > >
> > > > if there is no irqhandler on platform with IRQ populated nothing
> > > > will work, RTC won't work as "rtc_update_irq " updated in irq
> handler.
> > > > I think it is a fatal condition.
> > >
> > > This is not true, an RTC can wake up a system without an IRQ.
> >
> > Agreed, I will restore the behaviour for this case.
> > It may not be fatal. But basic "hwclock -r" from userspace won't work
> > as " RTC_FEATURE_UPDATE_INTERRUPT" set and there is no irqhandler.
> >
>
> RTC_FEATURE_ALARM is what you should clear and you have to fallback to the
> irq is not present case.
Ok,Will update Patch#3 with clearing "RTC_FEATURE_ALARM" as the fallback for the irqhandler error case
On patch#1, I will clear both RTC_FEATURE_ALARM" and "RTC_FEATURE_UPDATE_INTERRUPT",
as with just clearing "RTC_FEATURE_ALARM", I get below error.
root@smarc-rzg2ul:~# date -s "2023-08-06 19:30:00"
Sun Aug 6 19:30:00 UTC 2023
root@smarc-rzg2ul:~# hwclock -w
root@smarc-rzg2ul:~# hwclock -r
hwclock: select() to /dev/rtc0 to wait for clock tick timed out
root@smarc-rzg2ul:~#
Cheers,
Biju
> > > > >
> > > > > >
> > > > > > ret = dev_pm_set_wake_irq(&pdev->dev,
> irq_alarm);
> > > > > > if (ret)
> > > > >
> > > > > The rest LGTM, so with the above fixed/clarified:
> > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> > > > >
> > > > > 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
> > >
> > > --
> > > Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and
> > > Kernel engineering
> > >
> https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbootlin%
> 2F&data=05%7C01%7Cbiju.das.jz%40bp.renesas.com%7C72e93f2d3b25447789c608dbf
> 285e823%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C638370429169364269%7C
> Unknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwi
> LCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=zGt9Zsk6AYZ3zwOTU6l0zmN3KF1rGqOTAe3XR
> hxPWaA%3D&reserved=0.
> > > com%2F&data=05%7C01%7Cbiju.das.jz%40bp.renesas.com%7Cb699f48656d34a9
> > > 23a640
> > > 8dbf28104af%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C63837040817
> > > 604431
> > > 5%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTi
> > > I6Ik1h
> > > aWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=E9tDi08sBRoh4tBccQB%2B8az%2
> > > BqQ4%2
> > > FtQOpFjdPgU8zQXc%3D&reserved=0
>
> --
> Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel
> engineering
> https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbootlin.
> com%2F&data=05%7C01%7Cbiju.das.jz%40bp.renesas.com%7C72e93f2d3b25447789c60
> 8dbf285e823%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C63837042916952053
> 1%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h
> aWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=WULSktePlojGqVPbQJ%2BDelJnQEOUIh%
> 2BaSJm2Ra4OsRI%3D&reserved=0
