From: Stephen Boyd <swboyd@chromium.org>
To: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>,
Tri Vo <trong@android.com>,
"Rafael J. Wysocki" <rjw@rjwysocki.net>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Viresh Kumar <viresh.kumar@linaro.org>,
Hridya Valsaraju <hridya@google.com>,
Sandeep Patil <sspatil@google.com>,
Kalesh Singh <kaleshsingh@google.com>,
Ravi Chandra Sadineni <ravisadineni@chromium.org>,
LKML <linux-kernel@vger.kernel.org>,
Linux PM <linux-pm@vger.kernel.org>,
"Cc: Android Kernel" <kernel-team@android.com>
Subject: Re: [PATCH v6] PM / wakeup: show wakeup sources stats in sysfs
Date: Thu, 01 Aug 2019 08:31:42 -0700 [thread overview]
Message-ID: <5d4305df.1c69fb81.c4013.1950@mx.google.com> (raw)
In-Reply-To: <CAJZ5v0jkLXwqmXwyYtdZ9X2=W2KNKS4Ok_NrDew2yvvt1=4pgQ@mail.gmail.com>
Quoting Rafael J. Wysocki (2019-08-01 01:09:22)
> On Thu, Aug 1, 2019 at 2:45 AM Stephen Boyd <swboyd@chromium.org> wrote:
> >
> > Quoting Stephen Boyd (2019-07-31 16:45:31)
> > >
> > > This approach also nicely detects duplicate wakeup source names in the
> > > case that the string passed in to wakeup_source_register() is already
> > > used on the virtual bus.
> >
> > This was clearly untested! Here's a better one. This is what I see on my
> > device with this patch squashed in:
> >
> > localhost ~ # cat /sys/kernel/debug/wakeup_sources
> > name active_count event_count wakeup_count expire_count active_since total_time max_time last_change prevent_suspend_time
> > 1-1.2.4.1 0 0 0 0 0 0 0 0 0
> > 1-1.1 0 0 0 0 0 0 0 0 0
> > gpio-keys 0 0 0 0 0 0 0 0 0
> > spi10.0 0 0 0 0 0 0 0 0 0
> > a88000.spi:ec@0:keyboard-controller 0 0 0 0 0 0 0
> > 0 0
> > alarmtimer 0 0 0 0 0 0 0 0 0
> > cros-ec-rtc.1.auto 0 0 0 0 0 0 0 0
> > 0
> > a8f8800.usb 0 0 0 0 0 0 0 0 0
> > a6f8800.usb 0 0 0 0 0 0 0 0 0
> > localhost ~ # ls -l /sys/class/wakeup/
> > total 0
> > lrwxrwxrwx. 1 root root 0 Jul 31 17:43 alarmtimer -> ../../devices/platform/soc/ac0000.geniqup/a88000.spi/spi_master/spi10/spi10.0/cros-ec-dev.0.auto/cros-ec-rtc.1.auto/rtc/rtc0/alarmtimer
>
> So why is this not "(...)rtc0/wakeup/alarmtimer" ?
>
> This particular bit looks kind of inconsistent.
I believe this is the code you're looking for in drivers/base/core.c
/*
* If we have no parent, we live in "virtual".
* Class-devices with a non class-device as parent, live
* in a "glue" directory to prevent namespace collisions.
*/
if (parent == NULL)
parent_kobj = virtual_device_parent(dev);
else if (parent->class && !dev->class->ns_type)
return &parent->kobj;
else
parent_kobj = &parent->kobj;
>
> I guess without your patch you'd see "(...)rtc0/wakeup/wakeup0" instead, right?
No, it would be rtc0/wakeup0. That's because rtc is a class, and rtc0 is
part of that class, so we don't try to make a glue directory named after
the class to avoid collisions (see class_dir_create_and_add()
implementation).
BTW, paths in /sys/devices aren't supposed to matter too much. In this
case, I'd expect to see userspace looking at the /sys/class/wakeup path
to follow the symlink to figure out what device triggered a wakeup. It
can look at the 'device' symlink inside the directory for the wakeup
device to figure out which one it is.
Final thought, might want to suppress the power directory from being
created for the wakeup class. It looks odd to have
/sys/class/wakeup/wakeup0/power when the presumably does nothing.
next prev parent reply other threads:[~2019-08-01 15:31 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-31 21:55 [PATCH v6] PM / wakeup: show wakeup sources stats in sysfs Tri Vo
2019-07-31 21:59 ` Stephen Boyd
2019-07-31 22:16 ` Tri Vo
2019-07-31 22:17 ` Rafael J. Wysocki
2019-07-31 22:31 ` Tri Vo
2019-07-31 22:42 ` Rafael J. Wysocki
2019-07-31 22:58 ` Tri Vo
2019-07-31 23:10 ` Rafael J. Wysocki
2019-07-31 23:27 ` Tri Vo
2019-07-31 23:45 ` Stephen Boyd
2019-08-01 0:45 ` Stephen Boyd
2019-08-01 8:09 ` Rafael J. Wysocki
2019-08-01 15:31 ` Stephen Boyd [this message]
2019-08-01 17:21 ` Rafael J. Wysocki
2019-08-01 19:25 ` Stephen Boyd
2019-08-01 19:36 ` Greg Kroah-Hartman
2019-08-01 19:50 ` Tri Vo
2019-08-01 20:22 ` Stephen Boyd
2019-08-01 21:44 ` Tri Vo
2019-08-01 22:10 ` Rafael J. Wysocki
2019-08-03 22:40 ` Tri Vo
2019-08-05 8:32 ` Rafael J. Wysocki
2019-08-01 22:11 ` Stephen Boyd
2019-08-01 22:44 ` Tri Vo
2019-08-01 23:37 ` Stephen Boyd
2019-08-01 22:46 ` Rafael J. Wysocki
2019-08-01 23:40 ` Stephen Boyd
2019-07-31 23:00 ` Rafael J. Wysocki
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=5d4305df.1c69fb81.c4013.1950@mx.google.com \
--to=swboyd@chromium.org \
--cc=gregkh@linuxfoundation.org \
--cc=hridya@google.com \
--cc=kaleshsingh@google.com \
--cc=kernel-team@android.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=rafael@kernel.org \
--cc=ravisadineni@chromium.org \
--cc=rjw@rjwysocki.net \
--cc=sspatil@google.com \
--cc=trong@android.com \
--cc=viresh.kumar@linaro.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).