Hello Adam! On Tue, 23 Nov 2021 14:37:42 +0000 Adam Thomson <Adam.Thomson.Opensource@xxxxxxxxxxx> wrote: > On 23 November 2021 14:06, Nikita Shubin wrote: > > > As da9063 driver refuses to load without irq, we simply add it as a > > wakeup source before registering rtc device. > > Can you please make the commit message more detailed, explaining why > you're making this change; what it adds/fixes/removes/etc.? Right now > just reading this I'm unclear as to why you're adding a call to > device_init_wakeup() here. The generic I2C client code will mark the > parent MFD device as a wake source, if the relevant boolean 'wakeup' > is defined in DT, so what does this add? Sorry for long response had to double check setting wakeup-source in case i have missed something. I2C_CLIENT_WAKE is set in of_i2c_get_board_info - the place da9063 rtc would never get to. Setting "wakeup-source" for pmic indeed marks it as wakeup source, but that's not exactly we want. What we want is "wakealarm" in RTC sysfs directory, to be able to set alarm so we can wake up from SHUTDOWN/DELIVERY/RTC mode of da9063. We do have /sys/class/rtc/rtc0/wakealarm if marking da9063-rtc as device_init_wakeup. Unfortunately marking pmic or rtc as wakeup-source in device tree gives us nothing. ls /proc/device-tree/soc/i2c\@10030000/pmic\@58/ compatible interrupt-parent name regulators wakeup-source interrupt-controller interrupts reg rtc wdt ls /proc/device-tree/soc/i2c\@10030000/pmic\@58/rtc/ compatible name wakeup-source ls /sys/class/rtc/rtc0/wakealarm ls: cannot access '/sys/class/rtc/rtc0/wakealarm': No such file or directory So i currently see that either da9063 RTC should be marked as wakeup source, or the da9063 MFD should somehow set that for RTC. And we want this even if CONFIG_PM is off. Mentioning "/sys/class/rtc/rtc0/wakealarm" in commit message would be enough ? > > > > > Signed-off-by: Nikita Shubin <nikita.shubin@xxxxxxxxxxx> > > --- > > v1->v2: > > Alexandre Belloni: > > > > Dropped everything except device_init_wakeup, as driver refuses to > > load without irq specified, we can always set it as a wakeup > > source, before calling devm_rtc_register_device. > > --- > > drivers/rtc/rtc-da9063.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/rtc/rtc-da9063.c b/drivers/rtc/rtc-da9063.c > > index d4b72a9fa2ba..b9a73356bace 100644 > > --- a/drivers/rtc/rtc-da9063.c > > +++ b/drivers/rtc/rtc-da9063.c > > @@ -494,6 +494,8 @@ static int da9063_rtc_probe(struct > > platform_device *pdev) > > dev_err(&pdev->dev, "Failed to request ALARM IRQ > > %d: %d\n", irq_alarm, ret); > > > > + device_init_wakeup(&pdev->dev, true); > > + > > return devm_rtc_register_device(rtc->rtc_dev); > > } > > > > -- > > 2.31.1 >