2022-08-25 18:07:37

by Vimal Kumar

[permalink] [raw]
Subject: [PATCH v2] PM: runtime: Add support to disable wakeup sources

User could find many wakeup sources available in the bsp, which
they won't be using. Currently users can only get the status and
list of enabled wakeup sources, but users can't disable it runtime.
It's very difficult to find the driver for each wakeup sources from
where it's getting enabled and make the changes for disabling it.

This will help users to disable any wakeup sources at runtime,
avoiding any code change and re-compilation. A new class attribute
"disable_ws" will be added in the wakeup calss. If user want to disable
any wakeup sources, user need to find the wakeup dev node associated
with the particular wakeup source and write the devnode name to the
class attribute "disable_ws".

Example:
Need to disable the wakeup source '1c08000.qcom,pcie'. The dev node
associated with this wakeup source is:
cat /sys/class/wakeup3/name ==> "1c08000.qcom,pcie", then for disabling
this wakeup source :
echo wakeup3 > /sys/class/wakeup/disable_ws

Co-developed-by: Mintu Patel <[email protected]>
Signed-off-by: Mintu Patel <[email protected]>
Co-developed-by: Vishal Badole <[email protected]>
Signed-off-by: Vishal Badole <[email protected]>
Signed-off-by: Vimal Kumar <[email protected]>
---
Documentation/ABI/testing/sysfs-class-wakeup | 16 +++++
drivers/base/power/wakeup_stats.c | 65 +++++++++++++++++++-
2 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-class-wakeup b/Documentation/ABI/testing/sysfs-class-wakeup
index 754aab8b6dcd..75b9a6fe737a 100644
--- a/Documentation/ABI/testing/sysfs-class-wakeup
+++ b/Documentation/ABI/testing/sysfs-class-wakeup
@@ -74,3 +74,19 @@ Contact: Tri Vo <[email protected]>
Description:
The file contains the total amount of time this wakeup source
has been preventing autosleep, in milliseconds.
+
+What: /sys/class/wakeup/disable_ws
+Date: Aug 2022
+Contact: Vimal Kumar <[email protected]>
+Description:
+ This file can be used to disable a wakeup source at runtime.
+ If user want to disable any wakeup sources, user need to find
+ the wakeup dev node associated with the particular wakeup source
+ and write the devnode name to this file "disable_ws".
+
+ Example:
+ If user Need to disable the wakeup source '1c08000.qcom,pcie', and
+ the wakeup dev node associated with this wakeup source is:
+ cat /sys/class/wakeup3/name ==> "1c08000.qcom,pcie"
+ Then for disabling this wakeup source :
+ echo wakeup3 > /sys/class/wakeup/disable_ws
diff --git a/drivers/base/power/wakeup_stats.c b/drivers/base/power/wakeup_stats.c
index 924fac493c4f..497402a28028 100644
--- a/drivers/base/power/wakeup_stats.c
+++ b/drivers/base/power/wakeup_stats.c
@@ -15,6 +15,7 @@
#include <linux/kobject.h>
#include <linux/slab.h>
#include <linux/timekeeping.h>
+#include <linux/uaccess.h>

#include "power.h"

@@ -208,9 +209,71 @@ void wakeup_source_sysfs_remove(struct wakeup_source *ws)
device_unregister(ws->dev);
}

+static ssize_t disable_ws_store(struct class *class,
+ struct class_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct device *dev;
+ struct wakeup_source *ws;
+ char *ws_name;
+ int status;
+
+ ws_name = kzalloc(len+1, GFP_KERNEL);
+ if (!ws_name)
+ return -ENOMEM;
+
+ if (sscanf(buf, "%s", ws_name) != 1)
+ return -EFAULT;
+
+ dev = class_find_device_by_name(wakeup_class, ws_name);
+ if (!dev) {
+ pr_err("%s : wakeup device not found\n", __func__);
+ return -EINVAL;
+ }
+
+ ws = dev_get_drvdata(dev);
+ if (ws->dev->parent != NULL) {
+
+ status = device_wakeup_disable(ws->dev->parent);
+ if (status < 0) {
+ /* In case of virtual device, return code will be -EINVAL
+ * then unregister the wakeup source associated with it
+ */
+ wakeup_source_unregister(ws);
+ }
+ } else
+ /* If the parent device is NULL, just unregister the wakeup source */
+ wakeup_source_unregister(ws);
+
+ return len;
+}
+
+static CLASS_ATTR_WO(disable_ws);
+
+static struct attribute *wakeup_class_attrs[] = {
+ &class_attr_disable_ws.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(wakeup_class);
+
static int __init wakeup_sources_sysfs_init(void)
{
- wakeup_class = class_create(THIS_MODULE, "wakeup");
+ int status;
+
+ wakeup_class = kzalloc(sizeof(*wakeup_class), GFP_KERNEL);
+ if (!wakeup_class)
+ return -ENOMEM;
+
+ wakeup_class->name = "wakeup";
+ wakeup_class->owner = THIS_MODULE;
+ wakeup_class->class_groups = wakeup_class_groups;
+
+ status = class_register(wakeup_class);
+
+ if (status < 0) {
+ pr_err("%s: class register failed %d\n", __func__, status);
+ return status;
+ }

return PTR_ERR_OR_ZERO(wakeup_class);
}
--
2.25.1


2022-08-25 18:14:15

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v2] PM: runtime: Add support to disable wakeup sources

On Thu, Aug 25, 2022 at 7:35 PM Vimal Kumar <[email protected]> wrote:
>
> User could find many wakeup sources available in the bsp, which
> they won't be using. Currently users can only get the status and
> list of enabled wakeup sources, but users can't disable it runtime.

What about using the wakeup attribute for devices in sysfs? That
effectively disables/enables a wakeup source.

> It's very difficult to find the driver for each wakeup sources from
> where it's getting enabled and make the changes for disabling it.

Why do you need to enable/disable them individually, though?

> This will help users to disable any wakeup sources at runtime,
> avoiding any code change and re-compilation. A new class attribute
> "disable_ws" will be added in the wakeup calss. If user want to disable
> any wakeup sources, user need to find the wakeup dev node associated
> with the particular wakeup source and write the devnode name to the
> class attribute "disable_ws".
>
> Example:
> Need to disable the wakeup source '1c08000.qcom,pcie'. The dev node
> associated with this wakeup source is:
> cat /sys/class/wakeup3/name ==> "1c08000.qcom,pcie", then for disabling
> this wakeup source :
> echo wakeup3 > /sys/class/wakeup/disable_ws

Wouldn't it be more straightforward to add a "disable" attribute for
wakeup sources?

> Co-developed-by: Mintu Patel <[email protected]>
> Signed-off-by: Mintu Patel <[email protected]>
> Co-developed-by: Vishal Badole <[email protected]>
> Signed-off-by: Vishal Badole <[email protected]>
> Signed-off-by: Vimal Kumar <[email protected]>
> ---
> Documentation/ABI/testing/sysfs-class-wakeup | 16 +++++
> drivers/base/power/wakeup_stats.c | 65 +++++++++++++++++++-
> 2 files changed, 80 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-class-wakeup b/Documentation/ABI/testing/sysfs-class-wakeup
> index 754aab8b6dcd..75b9a6fe737a 100644
> --- a/Documentation/ABI/testing/sysfs-class-wakeup
> +++ b/Documentation/ABI/testing/sysfs-class-wakeup
> @@ -74,3 +74,19 @@ Contact: Tri Vo <[email protected]>
> Description:
> The file contains the total amount of time this wakeup source
> has been preventing autosleep, in milliseconds.
> +
> +What: /sys/class/wakeup/disable_ws
> +Date: Aug 2022
> +Contact: Vimal Kumar <[email protected]>
> +Description:
> + This file can be used to disable a wakeup source at runtime.
> + If user want to disable any wakeup sources, user need to find
> + the wakeup dev node associated with the particular wakeup source
> + and write the devnode name to this file "disable_ws".
> +
> + Example:
> + If user Need to disable the wakeup source '1c08000.qcom,pcie', and
> + the wakeup dev node associated with this wakeup source is:
> + cat /sys/class/wakeup3/name ==> "1c08000.qcom,pcie"
> + Then for disabling this wakeup source :
> + echo wakeup3 > /sys/class/wakeup/disable_ws
> diff --git a/drivers/base/power/wakeup_stats.c b/drivers/base/power/wakeup_stats.c
> index 924fac493c4f..497402a28028 100644
> --- a/drivers/base/power/wakeup_stats.c
> +++ b/drivers/base/power/wakeup_stats.c
> @@ -15,6 +15,7 @@
> #include <linux/kobject.h>
> #include <linux/slab.h>
> #include <linux/timekeeping.h>
> +#include <linux/uaccess.h>
>
> #include "power.h"
>
> @@ -208,9 +209,71 @@ void wakeup_source_sysfs_remove(struct wakeup_source *ws)
> device_unregister(ws->dev);
> }
>
> +static ssize_t disable_ws_store(struct class *class,
> + struct class_attribute *attr,
> + const char *buf, size_t len)
> +{
> + struct device *dev;
> + struct wakeup_source *ws;
> + char *ws_name;
> + int status;
> +
> + ws_name = kzalloc(len+1, GFP_KERNEL);
> + if (!ws_name)
> + return -ENOMEM;
> +
> + if (sscanf(buf, "%s", ws_name) != 1)
> + return -EFAULT;
> +
> + dev = class_find_device_by_name(wakeup_class, ws_name);
> + if (!dev) {
> + pr_err("%s : wakeup device not found\n", __func__);
> + return -EINVAL;
> + }
> +
> + ws = dev_get_drvdata(dev);
> + if (ws->dev->parent != NULL) {
> +
> + status = device_wakeup_disable(ws->dev->parent);
> + if (status < 0) {
> + /* In case of virtual device, return code will be -EINVAL
> + * then unregister the wakeup source associated with it
> + */
> + wakeup_source_unregister(ws);
> + }
> + } else
> + /* If the parent device is NULL, just unregister the wakeup source */
> + wakeup_source_unregister(ws);
> +
> + return len;
> +}
> +
> +static CLASS_ATTR_WO(disable_ws);
> +
> +static struct attribute *wakeup_class_attrs[] = {
> + &class_attr_disable_ws.attr,
> + NULL,
> +};
> +ATTRIBUTE_GROUPS(wakeup_class);
> +
> static int __init wakeup_sources_sysfs_init(void)
> {
> - wakeup_class = class_create(THIS_MODULE, "wakeup");
> + int status;
> +
> + wakeup_class = kzalloc(sizeof(*wakeup_class), GFP_KERNEL);
> + if (!wakeup_class)
> + return -ENOMEM;
> +
> + wakeup_class->name = "wakeup";
> + wakeup_class->owner = THIS_MODULE;
> + wakeup_class->class_groups = wakeup_class_groups;
> +
> + status = class_register(wakeup_class);
> +
> + if (status < 0) {
> + pr_err("%s: class register failed %d\n", __func__, status);
> + return status;
> + }
>
> return PTR_ERR_OR_ZERO(wakeup_class);
> }
> --
> 2.25.1
>

2022-08-27 08:37:31

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2] PM: runtime: Add support to disable wakeup sources

On Thu, Aug 25, 2022 at 11:04:41PM +0530, Vimal Kumar wrote:
> User could find many wakeup sources available in the bsp, which
> they won't be using. Currently users can only get the status and
> list of enabled wakeup sources, but users can't disable it runtime.
> It's very difficult to find the driver for each wakeup sources from
> where it's getting enabled and make the changes for disabling it.
>
> This will help users to disable any wakeup sources at runtime,
> avoiding any code change and re-compilation. A new class attribute
> "disable_ws" will be added in the wakeup calss. If user want to disable
> any wakeup sources, user need to find the wakeup dev node associated
> with the particular wakeup source and write the devnode name to the
> class attribute "disable_ws".
>
> Example:
> Need to disable the wakeup source '1c08000.qcom,pcie'. The dev node
> associated with this wakeup source is:
> cat /sys/class/wakeup3/name ==> "1c08000.qcom,pcie", then for disabling
> this wakeup source :
> echo wakeup3 > /sys/class/wakeup/disable_ws
>
> Co-developed-by: Mintu Patel <[email protected]>
> Signed-off-by: Mintu Patel <[email protected]>
> Co-developed-by: Vishal Badole <[email protected]>
> Signed-off-by: Vishal Badole <[email protected]>
> Signed-off-by: Vimal Kumar <[email protected]>
> ---
> Documentation/ABI/testing/sysfs-class-wakeup | 16 +++++
> drivers/base/power/wakeup_stats.c | 65 +++++++++++++++++++-
> 2 files changed, 80 insertions(+), 1 deletion(-)

Based on previous discussions on the original submission that you seem
to have taken private, sorry but no, I can't even consider this
submission from you.

Please work on other portions of the kernel first to get used to the
development process.

Rafael, please don't worry about this.

greg k-h

2022-08-27 13:04:13

by Vimal Kumar

[permalink] [raw]
Subject: Re: [PATCH v2] PM: runtime: Add support to disable wakeup sources

On Sat, Aug 27, 2022 at 10:00:21AM +0200, Greg KH wrote:
> On Thu, Aug 25, 2022 at 11:04:41PM +0530, Vimal Kumar wrote:
> > User could find many wakeup sources available in the bsp, which
> > they won't be using. Currently users can only get the status and
> > list of enabled wakeup sources, but users can't disable it runtime.
> > It's very difficult to find the driver for each wakeup sources from
> > where it's getting enabled and make the changes for disabling it.
> >
> > This will help users to disable any wakeup sources at runtime,
> > avoiding any code change and re-compilation. A new class attribute
> > "disable_ws" will be added in the wakeup calss. If user want to disable
> > any wakeup sources, user need to find the wakeup dev node associated
> > with the particular wakeup source and write the devnode name to the
> > class attribute "disable_ws".
> >
> > Example:
> > Need to disable the wakeup source '1c08000.qcom,pcie'. The dev node
> > associated with this wakeup source is:
> > cat /sys/class/wakeup3/name ==> "1c08000.qcom,pcie", then for disabling
> > this wakeup source :
> > echo wakeup3 > /sys/class/wakeup/disable_ws
> >
> > Co-developed-by: Mintu Patel <[email protected]>
> > Signed-off-by: Mintu Patel <[email protected]>
> > Co-developed-by: Vishal Badole <[email protected]>
> > Signed-off-by: Vishal Badole <[email protected]>
> > Signed-off-by: Vimal Kumar <[email protected]>
> > ---
> > Documentation/ABI/testing/sysfs-class-wakeup | 16 +++++
> > drivers/base/power/wakeup_stats.c | 65 +++++++++++++++++++-
> > 2 files changed, 80 insertions(+), 1 deletion(-)
>
> Based on previous discussions on the original submission that you seem
> to have taken private, sorry but no, I can't even consider this
> submission from you.
>
> Please work on other portions of the kernel first to get used to the
> development process.
>
> Rafael, please don't worry about this.
>
> greg k-h

Hi Greg k-h,

My sincere apologies for responding privetly on the original
submission, It was not intended to do so. There was some issue
while responding via mutt and I end up responding privetly.

I have responded publicly on original submission as well. Please
consider this second version, I have taken care of some previous
reviews.

Thanks Rafael, for the review comments as well, I will be respondig
to the queries in a seperate thread.


Warm Regards,
Vimal Kumar