LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
To: Nikita Shubin <nikita.shubin@maquefel.me>,
	Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
Cc: David Abdurachmanov <david.abdurachmanov@sifive.com>,
	Support Opensource <Support.Opensource@diasemi.com>,
	Alessandro Zummo <a.zummo@towertech.it>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	"linux-rtc@vger.kernel.org" <linux-rtc@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v2] rtc: da9063: add as wakeup source
Date: Fri, 26 Nov 2021 09:50:18 +0000	[thread overview]
Message-ID: <DB9PR10MB465287595152C33A43FDBCDA80639@DB9PR10MB4652.EURPRD10.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <20211126120935.188e672a@redslave.neermore.group>

On 26 November 2021 09:10, Nikita Shubin wrote:

> > 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 ?

Thanks for the detailed response; it helped a lot. Having reviewed the core code
along with your description I know understand what's happening here. Basically
marking as 'wakeup-source' is simply a means to expose the sysfs attribute to
user-space.

Yes I think in the commit message you should be clear that there's a need to
access the sys attribute 'wakealarm' in the RTC core and clarify exactly why
there is that need. Your commit log should be good enough so that if anyone else
needs to look at this later they completely understand the intention behind the
change.

By the way, I assume the functionality you're looking for could also have been
achieved through using the /dev/rtcX instance for DA9063?

  reply	other threads:[~2021-11-26  9:52 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-23 14:06 Nikita Shubin
2021-11-23 14:37 ` Adam Thomson
2021-11-26  9:09   ` Nikita Shubin
2021-11-26  9:50     ` Adam Thomson [this message]
2021-11-26 10:23       ` Nikita Shubin
2021-11-26 11:14         ` Adam Thomson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DB9PR10MB465287595152C33A43FDBCDA80639@DB9PR10MB4652.EURPRD10.PROD.OUTLOOK.COM \
    --to=adam.thomson.opensource@diasemi.com \
    --cc=Support.Opensource@diasemi.com \
    --cc=a.zummo@towertech.it \
    --cc=alexandre.belloni@bootlin.com \
    --cc=david.abdurachmanov@sifive.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rtc@vger.kernel.org \
    --cc=nikita.shubin@maquefel.me \
    --subject='RE: [PATCH v2] rtc: da9063: add as wakeup source' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).