Re: [PATCH v2] rtc: da9063: add as wakeup source

From: Nikita Shubin
Date: Fri Nov 26 2021 - 04:17:23 EST


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
>