Re: [PATCH v6] PM / wakeup: show wakeup sources stats in sysfs

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 12:25:04 -0700	[thread overview]
Message-ID: <5d433c91.1c69fb81.93f13.febc@mx.google.com> (raw)
In-Reply-To: <CAJZ5v0hDE908XcreUyzZ8JGjFJbgVX1N1ELeMwf+n_ENxPA_DA@mail.gmail.com>

Quoting Rafael J. Wysocki (2019-08-01 10:21:44)
> On Thu, Aug 1, 2019 at 5:31 PM Stephen Boyd <swboyd@chromium.org> wrote:
> >
> > 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.
> 
> But if you go from the device, it would be good to be able to figure
> out which wakeup sources are associated with it and in the alarmtimer
> example you don't even see that it is a wakeup source without
> following the link.

Userspace shouldn't go from the device path (/sys/devices/.../rtc0 in
this example). That's incorrect. Instead, userspace should go from the
/sys/class/wakeup/... path. It should iterate over all the devices in
the class path and look at the device pointers instead.

# ls /sys/class/wakeup/*/device -l
lrwxrwxrwx. 1 root root 0 Aug  1 12:13 /sys/class/wakeup/alarmtimer/device -> ../../rtc0
lrwxrwxrwx. 1 root root 0 Aug  1 12:13 /sys/class/wakeup/wakeup0/device -> ../../../a6f8800.usb
lrwxrwxrwx. 1 root root 0 Aug  1 12:13 /sys/class/wakeup/wakeup1/device -> ../../../a8f8800.usb
lrwxrwxrwx. 1 root root 0 Aug  1 12:13 /sys/class/wakeup/wakeup2/device -> ../../../cros-ec-rtc.1.auto
lrwxrwxrwx. 1 root root 0 Aug  1 12:13 /sys/class/wakeup/wakeup3/device -> ../../sbs-16-000b
lrwxrwxrwx. 1 root root 0 Aug  1 12:13 /sys/class/wakeup/wakeup4/device -> ../../../a88000.spi:ec@0:keyboard-controller
lrwxrwxrwx. 1 root root 0 Aug  1 12:13 /sys/class/wakeup/wakeup5/device -> ../../../spi10.0
lrwxrwxrwx. 1 root root 0 Aug  1 12:13 /sys/class/wakeup/wakeup6/device -> ../../../gpio-keys
lrwxrwxrwx. 1 root root 0 Aug  1 12:13 /sys/class/wakeup/wakeup7/device -> ../../../1-1.1
lrwxrwxrwx. 1 root root 0 Aug  1 12:13 /sys/class/wakeup/wakeup8/device -> ../../../1-1.2.4.1

> 
> So the "wakeupN" virtual dev names for all wakeup source objects are
> less confusing IMO.
> 
> It would be good to avoid the glue dir creation in all cases somehow too.

I recall some differences between a bus_type and a class. Are you
suggesting to use a bus_type for the wakeup sources? I like the class
approach taken here to use different device names because it avoids the
name collisions, avoids making another attribute to express the name of
the wakeup source, and doesn't make a more heavyweight driver
abstraction on top of wakeup sources.

In fact, that ls command above pretty much sums up the wakeup source
name and the device that it's associated with. Whatever goes on inside
/sys/devices/... with respect to where the devices go and how they're
structured is not important, at least to me. Why is it important to you?