[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5d423637.1c69fb81.62114.ca6f@mx.google.com>
Date: Wed, 31 Jul 2019 17:45:42 -0700
From: Stephen Boyd <swboyd@...omium.org>
To: "Rafael J. Wysocki" <rafael@...nel.org>, Tri Vo <trong@...roid.com>
Cc: "Rafael J. Wysocki" <rjw@...ysocki.net>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Viresh Kumar <viresh.kumar@...aro.org>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Hridya Valsaraju <hridya@...gle.com>,
Sandeep Patil <sspatil@...gle.com>,
Kalesh Singh <kaleshsingh@...gle.com>,
Ravi Chandra Sadineni <ravisadineni@...omium.org>,
LKML <linux-kernel@...r.kernel.org>,
Linux PM <linux-pm@...r.kernel.org>,
"Cc: Android Kernel" <kernel-team@...roid.com>
Subject: Re: [PATCH v6] PM / wakeup: show wakeup sources stats in sysfs
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
lrwxrwxrwx. 1 root root 0 Jul 31 17:43 wakeup0 -> ../../devices/platform/soc/a6f8800.usb/wakeup/wakeup0
lrwxrwxrwx. 1 root root 0 Jul 31 17:43 wakeup1 -> ../../devices/platform/soc/a8f8800.usb/wakeup/wakeup1
lrwxrwxrwx. 1 root root 0 Jul 31 17:43 wakeup2 -> ../../devices/platform/soc/ac0000.geniqup/a88000.spi/spi_master/spi10/spi10.0/cros-ec-dev.0.auto/cros-ec-rtc.1.auto/wakeup/wakeup2
lrwxrwxrwx. 1 root root 0 Jul 31 17:43 wakeup3 -> ../../devices/platform/soc/ac0000.geniqup/a88000.spi/spi_master/spi10/spi10.0/a88000.spi:ec@0:keyboard-controller/wakeup/wakeup3
lrwxrwxrwx. 1 root root 0 Jul 31 17:43 wakeup4 -> ../../devices/platform/soc/ac0000.geniqup/a88000.spi/spi_master/spi10/spi10.0/wakeup/wakeup4
lrwxrwxrwx. 1 root root 0 Jul 31 17:43 wakeup5 -> ../../devices/platform/gpio-keys/wakeup/wakeup5
lrwxrwxrwx. 1 root root 0 Jul 31 17:43 wakeup6 -> ../../devices/platform/soc/a8f8800.usb/a800000.dwc3/xhci-hcd.7.auto/usb1/1-1/1-1.1/wakeup/wakeup6
lrwxrwxrwx. 1 root root 0 Jul 31 17:43 wakeup7 -> ../../devices/platform/soc/a8f8800.usb/a800000.dwc3/xhci-hcd.7.auto/usb1/1-1/1-1.2/1-1.2.4/1-1.2.4.1/wakeup/wakeup7
----8<----
diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index 79668b45eae6..ec414f0db0b1 100644
--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -201,7 +201,7 @@ EXPORT_SYMBOL_GPL(wakeup_source_remove);
/**
* wakeup_source_register - Create wakeup source and add it to the list.
* @dev: Device this wakeup source is associated with (or NULL if virtual).
- * @name: Name of the wakeup source to register.
+ * @name: Name of the wakeup source to register (or NULL if device wakeup).
*/
struct wakeup_source *wakeup_source_register(struct device *dev,
const char *name)
@@ -209,9 +209,9 @@ struct wakeup_source *wakeup_source_register(struct device *dev,
struct wakeup_source *ws;
int ret;
- ws = wakeup_source_create(name);
+ ws = wakeup_source_create(name ? : dev_name(dev));
if (ws) {
- ret = wakeup_source_sysfs_add(dev, ws);
+ ret = wakeup_source_sysfs_add(dev, ws, !!name);
if (ret) {
kfree_const(ws->name);
kfree(ws);
@@ -275,7 +275,7 @@ int device_wakeup_enable(struct device *dev)
if (pm_suspend_target_state != PM_SUSPEND_ON)
dev_dbg(dev, "Suspicious %s() during system transition!\n", __func__);
- ws = wakeup_source_register(dev, dev_name(dev));
+ ws = wakeup_source_register(dev, NULL);
if (!ws)
return -ENOMEM;
diff --git a/drivers/base/power/wakeup_stats.c b/drivers/base/power/wakeup_stats.c
index a26f019faca9..0f4c59b02d5d 100644
--- a/drivers/base/power/wakeup_stats.c
+++ b/drivers/base/power/wakeup_stats.c
@@ -81,15 +81,6 @@ static ssize_t last_change_ms_show(struct device *dev,
}
static DEVICE_ATTR_RO(last_change_ms);
-static ssize_t name_show(struct device *dev, struct device_attribute *attr,
- char *buf)
-{
- struct wakeup_source *ws = dev_get_drvdata(dev);
-
- return sprintf(buf, "%s\n", ws->name);
-}
-static DEVICE_ATTR_RO(name);
-
static ssize_t prevent_suspend_time_ms_show(struct device *dev,
struct device_attribute *attr,
char *buf)
@@ -106,7 +97,6 @@ static ssize_t prevent_suspend_time_ms_show(struct device *dev,
static DEVICE_ATTR_RO(prevent_suspend_time_ms);
static struct attribute *wakeup_source_attrs[] = {
- &dev_attr_name.attr,
&dev_attr_active_count.attr,
&dev_attr_event_count.attr,
&dev_attr_wakeup_count.attr,
@@ -126,22 +116,35 @@ static DEFINE_IDA(wakeup_ida);
* wakeup_source_sysfs_add - Add wakeup_source attributes to sysfs.
* @parent: Device given wakeup source is associated with (or NULL if virtual).
* @ws: Wakeup source to be added in sysfs.
+ * @use_ws_name: True to use ws->name or false to use 'wakeupN' for device name
*/
-int wakeup_source_sysfs_add(struct device *parent, struct wakeup_source *ws)
+int wakeup_source_sysfs_add(struct device *parent, struct wakeup_source *ws,
+ bool use_ws_name)
{
struct device *dev;
int id;
- id = ida_alloc(&wakeup_ida, GFP_KERNEL);
- if (id < 0)
- return id;
- ws->id = id;
+ ws->id = -1;
+ if (use_ws_name) {
+ dev = device_create_with_groups(wakeup_class, parent,
+ MKDEV(0, 0), ws,
+ wakeup_source_groups,
+ ws->name);
+ } else {
+ id = ida_alloc(&wakeup_ida, GFP_KERNEL);
+ if (id < 0)
+ return id;
+ ws->id = id;
+
+ dev = device_create_with_groups(wakeup_class, parent,
+ MKDEV(0, 0), ws,
+ wakeup_source_groups,
+ "wakeup%d", ws->id);
+ }
- dev = device_create_with_groups(wakeup_class, parent, MKDEV(0, 0), ws,
- wakeup_source_groups, "ws%d",
- ws->id);
if (IS_ERR(dev)) {
- ida_free(&wakeup_ida, ws->id);
+ if (ws->id >= 0)
+ ida_free(&wakeup_ida, ws->id);
return PTR_ERR(dev);
}
@@ -157,7 +160,8 @@ EXPORT_SYMBOL_GPL(wakeup_source_sysfs_add);
void wakeup_source_sysfs_remove(struct wakeup_source *ws)
{
device_unregister(ws->dev);
- ida_simple_remove(&wakeup_ida, ws->id);
+ if (ws->id >= 0)
+ ida_free(&wakeup_ida, ws->id);
}
EXPORT_SYMBOL_GPL(wakeup_source_sysfs_remove);
diff --git a/include/linux/pm_wakeup.h b/include/linux/pm_wakeup.h
index f39f768389c8..c9fb00fca22e 100644
--- a/include/linux/pm_wakeup.h
+++ b/include/linux/pm_wakeup.h
@@ -107,7 +107,7 @@ extern void pm_wakeup_dev_event(struct device *dev, unsigned int msec, bool hard
/* drivers/base/power/wakeup_stats.c */
extern int wakeup_source_sysfs_add(struct device *parent,
- struct wakeup_source *ws);
+ struct wakeup_source *ws, bool use_ws_name);
extern void wakeup_source_sysfs_remove(struct wakeup_source *ws);
#else /* !CONFIG_PM_SLEEP */
diff --git a/kernel/power/wakelock.c b/kernel/power/wakelock.c
index 826fcd97647a..7f2fc5f9b3b3 100644
--- a/kernel/power/wakelock.c
+++ b/kernel/power/wakelock.c
@@ -192,7 +192,7 @@ static struct wakelock *wakelock_lookup_add(const char *name, size_t len,
wl->ws.name = wl->name;
wl->ws.last_time = ktime_get();
- ret = wakeup_source_sysfs_add(NULL, &wl->ws);
+ ret = wakeup_source_sysfs_add(NULL, &wl->ws, true);
if (ret) {
kfree(wl->name);
kfree(wl);
Powered by blists - more mailing lists