Re: [PATCH 3/6] rtc: da9063: Use dev_err_probe()

[Alexandre Belloni <alexandre.belloni@bootlin.com>]

On 20/12/2023 00:49:57+0100, Alexandre Belloni wrote:
> On 01/12/2023 16:40:25+0000, Biju Das wrote:
> > > 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:~#
> > 
> >
> 
> I can't believe this is true because the rtc core code will take the
> same path when RTC_FEATURE_ALARM or RTC_FEATURE_UPDATE_INTERRUPT is
> cleared:
> 
> https://elixir.bootlin.com/linux/latest/source/drivers/rtc/interface.c#L574
> 
> RTC_FEATURE_UPDATE_INTERRUPT must be cleared only when RTC_FEATURE_ALARM
> is set.

Are you sure you didn't break this test:

https://elixir.bootlin.com/linux/latest/source/drivers/rtc/rtc-da9063.c#L479

> 
> 
> > 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@glider.be>
> > > > > > >
> > > > > > > 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
> 
> -- 
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com