* [PATCH] PM / wakeup: Allocate class wakeup_class statically
@ 2026-03-29 16:14 Heiner Kallweit
2026-04-01 14:19 ` Rafael J. Wysocki
0 siblings, 1 reply; 7+ messages in thread
From: Heiner Kallweit @ 2026-03-29 16:14 UTC (permalink / raw)
To: Rafael J. Wysocki, Pavel Machek, Len Brown, Greg Kroah-Hartman,
Danilo Krummrich
Cc: Linux PM, driver-core
Allocating wakeup_class statically avoids a little runtime overhead.
Define groups and device release function as part of the class, so that
we don't have to repeat this for each class device.
Whilst at it, constify wakeup_source_attrs[].
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/base/power/wakeup_stats.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/base/power/wakeup_stats.c b/drivers/base/power/wakeup_stats.c
index 308f8bde9..72beb8fce 100644
--- a/drivers/base/power/wakeup_stats.c
+++ b/drivers/base/power/wakeup_stats.c
@@ -18,8 +18,6 @@
#include "power.h"
-static struct class *wakeup_class;
-
#define wakeup_attr(_name) \
static ssize_t _name##_show(struct device *dev, \
struct device_attribute *attr, char *buf) \
@@ -114,7 +112,7 @@ 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[] = {
+static const struct attribute *const wakeup_source_attrs[] = {
&dev_attr_name.attr,
&dev_attr_active_count.attr,
&dev_attr_event_count.attr,
@@ -135,6 +133,12 @@ static void device_create_release(struct device *dev)
kfree(dev);
}
+static const struct class wakeup_class = {
+ .name = "wakeup",
+ .dev_release = device_create_release,
+ .dev_groups = wakeup_source_groups,
+};
+
static struct device *wakeup_source_device_create(struct device *parent,
struct wakeup_source *ws)
{
@@ -149,10 +153,8 @@ static struct device *wakeup_source_device_create(struct device *parent,
device_initialize(dev);
dev->devt = MKDEV(0, 0);
- dev->class = wakeup_class;
+ dev->class = &wakeup_class;
dev->parent = parent;
- dev->groups = wakeup_source_groups;
- dev->release = device_create_release;
dev_set_drvdata(dev, ws);
device_set_pm_not_required(dev);
@@ -212,8 +214,6 @@ void wakeup_source_sysfs_remove(struct wakeup_source *ws)
static int __init wakeup_sources_sysfs_init(void)
{
- wakeup_class = class_create("wakeup");
-
- return PTR_ERR_OR_ZERO(wakeup_class);
+ return class_register(&wakeup_class);
}
postcore_initcall(wakeup_sources_sysfs_init);
--
2.53.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] PM / wakeup: Allocate class wakeup_class statically
2026-03-29 16:14 [PATCH] PM / wakeup: Allocate class wakeup_class statically Heiner Kallweit
@ 2026-04-01 14:19 ` Rafael J. Wysocki
2026-04-01 15:45 ` Heiner Kallweit
0 siblings, 1 reply; 7+ messages in thread
From: Rafael J. Wysocki @ 2026-04-01 14:19 UTC (permalink / raw)
To: Heiner Kallweit
Cc: Rafael J. Wysocki, Pavel Machek, Len Brown, Greg Kroah-Hartman,
Danilo Krummrich, Linux PM, driver-core
On Sun, Mar 29, 2026 at 6:14 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> Allocating wakeup_class statically avoids a little runtime overhead.
> Define groups and device release function as part of the class, so that
> we don't have to repeat this for each class device.
> Whilst at it, constify wakeup_source_attrs[].
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Can you please have a look at this and let me know what you think:
https://sashiko.dev/#/patchset/0fe1b679-ab28-4505-b0db-14e7ac3ba749%40gmail.com
> ---
> drivers/base/power/wakeup_stats.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/base/power/wakeup_stats.c b/drivers/base/power/wakeup_stats.c
> index 308f8bde9..72beb8fce 100644
> --- a/drivers/base/power/wakeup_stats.c
> +++ b/drivers/base/power/wakeup_stats.c
> @@ -18,8 +18,6 @@
>
> #include "power.h"
>
> -static struct class *wakeup_class;
> -
> #define wakeup_attr(_name) \
> static ssize_t _name##_show(struct device *dev, \
> struct device_attribute *attr, char *buf) \
> @@ -114,7 +112,7 @@ 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[] = {
> +static const struct attribute *const wakeup_source_attrs[] = {
> &dev_attr_name.attr,
> &dev_attr_active_count.attr,
> &dev_attr_event_count.attr,
> @@ -135,6 +133,12 @@ static void device_create_release(struct device *dev)
> kfree(dev);
> }
>
> +static const struct class wakeup_class = {
> + .name = "wakeup",
> + .dev_release = device_create_release,
> + .dev_groups = wakeup_source_groups,
> +};
> +
> static struct device *wakeup_source_device_create(struct device *parent,
> struct wakeup_source *ws)
> {
> @@ -149,10 +153,8 @@ static struct device *wakeup_source_device_create(struct device *parent,
>
> device_initialize(dev);
> dev->devt = MKDEV(0, 0);
> - dev->class = wakeup_class;
> + dev->class = &wakeup_class;
> dev->parent = parent;
> - dev->groups = wakeup_source_groups;
> - dev->release = device_create_release;
> dev_set_drvdata(dev, ws);
> device_set_pm_not_required(dev);
>
> @@ -212,8 +214,6 @@ void wakeup_source_sysfs_remove(struct wakeup_source *ws)
>
> static int __init wakeup_sources_sysfs_init(void)
> {
> - wakeup_class = class_create("wakeup");
> -
> - return PTR_ERR_OR_ZERO(wakeup_class);
> + return class_register(&wakeup_class);
> }
> postcore_initcall(wakeup_sources_sysfs_init);
> --
> 2.53.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PM / wakeup: Allocate class wakeup_class statically
2026-04-01 14:19 ` Rafael J. Wysocki
@ 2026-04-01 15:45 ` Heiner Kallweit
2026-04-01 17:32 ` Rafael J. Wysocki
0 siblings, 1 reply; 7+ messages in thread
From: Heiner Kallweit @ 2026-04-01 15:45 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Pavel Machek, Len Brown, Greg Kroah-Hartman, Danilo Krummrich,
Linux PM, driver-core
On 01.04.2026 16:19, Rafael J. Wysocki wrote:
> On Sun, Mar 29, 2026 at 6:14 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>
>> Allocating wakeup_class statically avoids a little runtime overhead.
>> Define groups and device release function as part of the class, so that
>> we don't have to repeat this for each class device.
>> Whilst at it, constify wakeup_source_attrs[].
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>
> Can you please have a look at this and let me know what you think:
>
> https://sashiko.dev/#/patchset/0fe1b679-ab28-4505-b0db-14e7ac3ba749%40gmail.com
>
Interesting finding! I think the diagnosis is right.
But: I would say the current behavior isn't a nice solution as well:
wakeup_source_device_create() does: dev->class = wakeup_class;
I think no reader will expect that wakeup_class may be NULL here due to
initcall ordering. In addition this behavior results in such early
wakeup sources not being shown in sysfs.
But I'm not sure whether registering class "wakeup" (and registering
classes in general) would be possible early enough (core_initcall,
or even pure_initcall).
>> ---
>> drivers/base/power/wakeup_stats.c | 18 +++++++++---------
>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/base/power/wakeup_stats.c b/drivers/base/power/wakeup_stats.c
>> index 308f8bde9..72beb8fce 100644
>> --- a/drivers/base/power/wakeup_stats.c
>> +++ b/drivers/base/power/wakeup_stats.c
>> @@ -18,8 +18,6 @@
>>
>> #include "power.h"
>>
>> -static struct class *wakeup_class;
>> -
>> #define wakeup_attr(_name) \
>> static ssize_t _name##_show(struct device *dev, \
>> struct device_attribute *attr, char *buf) \
>> @@ -114,7 +112,7 @@ 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[] = {
>> +static const struct attribute *const wakeup_source_attrs[] = {
>> &dev_attr_name.attr,
>> &dev_attr_active_count.attr,
>> &dev_attr_event_count.attr,
>> @@ -135,6 +133,12 @@ static void device_create_release(struct device *dev)
>> kfree(dev);
>> }
>>
>> +static const struct class wakeup_class = {
>> + .name = "wakeup",
>> + .dev_release = device_create_release,
>> + .dev_groups = wakeup_source_groups,
>> +};
>> +
>> static struct device *wakeup_source_device_create(struct device *parent,
>> struct wakeup_source *ws)
>> {
>> @@ -149,10 +153,8 @@ static struct device *wakeup_source_device_create(struct device *parent,
>>
>> device_initialize(dev);
>> dev->devt = MKDEV(0, 0);
>> - dev->class = wakeup_class;
>> + dev->class = &wakeup_class;
>> dev->parent = parent;
>> - dev->groups = wakeup_source_groups;
>> - dev->release = device_create_release;
>> dev_set_drvdata(dev, ws);
>> device_set_pm_not_required(dev);
>>
>> @@ -212,8 +214,6 @@ void wakeup_source_sysfs_remove(struct wakeup_source *ws)
>>
>> static int __init wakeup_sources_sysfs_init(void)
>> {
>> - wakeup_class = class_create("wakeup");
>> -
>> - return PTR_ERR_OR_ZERO(wakeup_class);
>> + return class_register(&wakeup_class);
>> }
>> postcore_initcall(wakeup_sources_sysfs_init);
>> --
>> 2.53.0
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PM / wakeup: Allocate class wakeup_class statically
2026-04-01 15:45 ` Heiner Kallweit
@ 2026-04-01 17:32 ` Rafael J. Wysocki
2026-04-02 13:05 ` Heiner Kallweit
0 siblings, 1 reply; 7+ messages in thread
From: Rafael J. Wysocki @ 2026-04-01 17:32 UTC (permalink / raw)
To: Heiner Kallweit
Cc: Rafael J. Wysocki, Pavel Machek, Len Brown, Greg Kroah-Hartman,
Danilo Krummrich, Linux PM, driver-core
On Wed, Apr 1, 2026 at 5:45 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> On 01.04.2026 16:19, Rafael J. Wysocki wrote:
> > On Sun, Mar 29, 2026 at 6:14 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
> >>
> >> Allocating wakeup_class statically avoids a little runtime overhead.
> >> Define groups and device release function as part of the class, so that
> >> we don't have to repeat this for each class device.
> >> Whilst at it, constify wakeup_source_attrs[].
> >>
> >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> >
> > Can you please have a look at this and let me know what you think:
> >
> > https://sashiko.dev/#/patchset/0fe1b679-ab28-4505-b0db-14e7ac3ba749%40gmail.com
> >
> Interesting finding! I think the diagnosis is right.
>
> But: I would say the current behavior isn't a nice solution as well:
It is not fantastic, but it doesn't have this issue.
> wakeup_source_device_create() does: dev->class = wakeup_class;
> I think no reader will expect that wakeup_class may be NULL here due to
> initcall ordering. In addition this behavior results in such early
> wakeup sources not being shown in sysfs.
They are registered too early to show up in sysfs, but they can work regardless.
I think that it's just pointless to call device_register() for a given
wakeup source if wakeup_class has not been registered yet.
> But I'm not sure whether registering class "wakeup" (and registering
> classes in general) would be possible early enough (core_initcall,
> or even pure_initcall).
driver_init() is called before do_initcalls() is do_basic_setup(), so
class registration should work for all initcall levels AFAICS.
> >> ---
> >> drivers/base/power/wakeup_stats.c | 18 +++++++++---------
> >> 1 file changed, 9 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/base/power/wakeup_stats.c b/drivers/base/power/wakeup_stats.c
> >> index 308f8bde9..72beb8fce 100644
> >> --- a/drivers/base/power/wakeup_stats.c
> >> +++ b/drivers/base/power/wakeup_stats.c
> >> @@ -18,8 +18,6 @@
> >>
> >> #include "power.h"
> >>
> >> -static struct class *wakeup_class;
> >> -
> >> #define wakeup_attr(_name) \
> >> static ssize_t _name##_show(struct device *dev, \
> >> struct device_attribute *attr, char *buf) \
> >> @@ -114,7 +112,7 @@ 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[] = {
> >> +static const struct attribute *const wakeup_source_attrs[] = {
> >> &dev_attr_name.attr,
> >> &dev_attr_active_count.attr,
> >> &dev_attr_event_count.attr,
> >> @@ -135,6 +133,12 @@ static void device_create_release(struct device *dev)
> >> kfree(dev);
> >> }
> >>
> >> +static const struct class wakeup_class = {
> >> + .name = "wakeup",
> >> + .dev_release = device_create_release,
> >> + .dev_groups = wakeup_source_groups,
> >> +};
> >> +
> >> static struct device *wakeup_source_device_create(struct device *parent,
> >> struct wakeup_source *ws)
> >> {
> >> @@ -149,10 +153,8 @@ static struct device *wakeup_source_device_create(struct device *parent,
> >>
> >> device_initialize(dev);
> >> dev->devt = MKDEV(0, 0);
> >> - dev->class = wakeup_class;
> >> + dev->class = &wakeup_class;
> >> dev->parent = parent;
> >> - dev->groups = wakeup_source_groups;
> >> - dev->release = device_create_release;
> >> dev_set_drvdata(dev, ws);
> >> device_set_pm_not_required(dev);
> >>
> >> @@ -212,8 +214,6 @@ void wakeup_source_sysfs_remove(struct wakeup_source *ws)
> >>
> >> static int __init wakeup_sources_sysfs_init(void)
> >> {
> >> - wakeup_class = class_create("wakeup");
> >> -
> >> - return PTR_ERR_OR_ZERO(wakeup_class);
> >> + return class_register(&wakeup_class);
> >> }
> >> postcore_initcall(wakeup_sources_sysfs_init);
> >> --
> >> 2.53.0
> >>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PM / wakeup: Allocate class wakeup_class statically
2026-04-01 17:32 ` Rafael J. Wysocki
@ 2026-04-02 13:05 ` Heiner Kallweit
2026-04-03 10:45 ` Rafael J. Wysocki
0 siblings, 1 reply; 7+ messages in thread
From: Heiner Kallweit @ 2026-04-02 13:05 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Pavel Machek, Len Brown, Greg Kroah-Hartman, Danilo Krummrich,
Linux PM, driver-core
On 01.04.2026 19:32, Rafael J. Wysocki wrote:
> On Wed, Apr 1, 2026 at 5:45 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>
>> On 01.04.2026 16:19, Rafael J. Wysocki wrote:
>>> On Sun, Mar 29, 2026 at 6:14 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>>>
>>>> Allocating wakeup_class statically avoids a little runtime overhead.
>>>> Define groups and device release function as part of the class, so that
>>>> we don't have to repeat this for each class device.
>>>> Whilst at it, constify wakeup_source_attrs[].
>>>>
>>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>>
>>> Can you please have a look at this and let me know what you think:
>>>
>>> https://sashiko.dev/#/patchset/0fe1b679-ab28-4505-b0db-14e7ac3ba749%40gmail.com
>>>
>> Interesting finding! I think the diagnosis is right.
>>
>> But: I would say the current behavior isn't a nice solution as well:
>
> It is not fantastic, but it doesn't have this issue.
>
>> wakeup_source_device_create() does: dev->class = wakeup_class;
>> I think no reader will expect that wakeup_class may be NULL here due to
>> initcall ordering. In addition this behavior results in such early
>> wakeup sources not being shown in sysfs.
>
> They are registered too early to show up in sysfs, but they can work regardless.
>
> I think that it's just pointless to call device_register() for a given
> wakeup source if wakeup_class has not been registered yet.
>
>> But I'm not sure whether registering class "wakeup" (and registering
>> classes in general) would be possible early enough (core_initcall,
>> or even pure_initcall).
>
> driver_init() is called before do_initcalls() is do_basic_setup(), so
> class registration should work for all initcall levels AFAICS.
>
When testing the current code on my system, autosleep is the first wakeup
source, registered in a core_initcall, and it's not shown in sysfs.
Same result when class wakeup is registered in a core_initcall
(instead of postcore_initcall). Registering class wakeup in a pure_initcall
works and fixes the issue. So, would this be an acceptable solution for the
discussed issue?
>>>> ---
>>>> drivers/base/power/wakeup_stats.c | 18 +++++++++---------
>>>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/drivers/base/power/wakeup_stats.c b/drivers/base/power/wakeup_stats.c
>>>> index 308f8bde9..72beb8fce 100644
>>>> --- a/drivers/base/power/wakeup_stats.c
>>>> +++ b/drivers/base/power/wakeup_stats.c
>>>> @@ -18,8 +18,6 @@
>>>>
>>>> #include "power.h"
>>>>
>>>> -static struct class *wakeup_class;
>>>> -
>>>> #define wakeup_attr(_name) \
>>>> static ssize_t _name##_show(struct device *dev, \
>>>> struct device_attribute *attr, char *buf) \
>>>> @@ -114,7 +112,7 @@ 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[] = {
>>>> +static const struct attribute *const wakeup_source_attrs[] = {
>>>> &dev_attr_name.attr,
>>>> &dev_attr_active_count.attr,
>>>> &dev_attr_event_count.attr,
>>>> @@ -135,6 +133,12 @@ static void device_create_release(struct device *dev)
>>>> kfree(dev);
>>>> }
>>>>
>>>> +static const struct class wakeup_class = {
>>>> + .name = "wakeup",
>>>> + .dev_release = device_create_release,
>>>> + .dev_groups = wakeup_source_groups,
>>>> +};
>>>> +
>>>> static struct device *wakeup_source_device_create(struct device *parent,
>>>> struct wakeup_source *ws)
>>>> {
>>>> @@ -149,10 +153,8 @@ static struct device *wakeup_source_device_create(struct device *parent,
>>>>
>>>> device_initialize(dev);
>>>> dev->devt = MKDEV(0, 0);
>>>> - dev->class = wakeup_class;
>>>> + dev->class = &wakeup_class;
>>>> dev->parent = parent;
>>>> - dev->groups = wakeup_source_groups;
>>>> - dev->release = device_create_release;
>>>> dev_set_drvdata(dev, ws);
>>>> device_set_pm_not_required(dev);
>>>>
>>>> @@ -212,8 +214,6 @@ void wakeup_source_sysfs_remove(struct wakeup_source *ws)
>>>>
>>>> static int __init wakeup_sources_sysfs_init(void)
>>>> {
>>>> - wakeup_class = class_create("wakeup");
>>>> -
>>>> - return PTR_ERR_OR_ZERO(wakeup_class);
>>>> + return class_register(&wakeup_class);
>>>> }
>>>> postcore_initcall(wakeup_sources_sysfs_init);
>>>> --
>>>> 2.53.0
>>>>
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PM / wakeup: Allocate class wakeup_class statically
2026-04-02 13:05 ` Heiner Kallweit
@ 2026-04-03 10:45 ` Rafael J. Wysocki
2026-04-03 11:02 ` Heiner Kallweit
0 siblings, 1 reply; 7+ messages in thread
From: Rafael J. Wysocki @ 2026-04-03 10:45 UTC (permalink / raw)
To: Heiner Kallweit
Cc: Rafael J. Wysocki, Pavel Machek, Len Brown, Greg Kroah-Hartman,
Danilo Krummrich, Linux PM, driver-core
On Thu, Apr 2, 2026 at 3:05 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> On 01.04.2026 19:32, Rafael J. Wysocki wrote:
> > On Wed, Apr 1, 2026 at 5:45 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
> >>
> >> On 01.04.2026 16:19, Rafael J. Wysocki wrote:
> >>> On Sun, Mar 29, 2026 at 6:14 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
> >>>>
> >>>> Allocating wakeup_class statically avoids a little runtime overhead.
> >>>> Define groups and device release function as part of the class, so that
> >>>> we don't have to repeat this for each class device.
> >>>> Whilst at it, constify wakeup_source_attrs[].
> >>>>
> >>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> >>>
> >>> Can you please have a look at this and let me know what you think:
> >>>
> >>> https://sashiko.dev/#/patchset/0fe1b679-ab28-4505-b0db-14e7ac3ba749%40gmail.com
> >>>
> >> Interesting finding! I think the diagnosis is right.
> >>
> >> But: I would say the current behavior isn't a nice solution as well:
> >
> > It is not fantastic, but it doesn't have this issue.
> >
> >> wakeup_source_device_create() does: dev->class = wakeup_class;
> >> I think no reader will expect that wakeup_class may be NULL here due to
> >> initcall ordering. In addition this behavior results in such early
> >> wakeup sources not being shown in sysfs.
> >
> > They are registered too early to show up in sysfs, but they can work regardless.
> >
> > I think that it's just pointless to call device_register() for a given
> > wakeup source if wakeup_class has not been registered yet.
> >
> >> But I'm not sure whether registering class "wakeup" (and registering
> >> classes in general) would be possible early enough (core_initcall,
> >> or even pure_initcall).
> >
> > driver_init() is called before do_initcalls() is do_basic_setup(), so
> > class registration should work for all initcall levels AFAICS.
> >
> When testing the current code on my system, autosleep is the first wakeup
> source, registered in a core_initcall, and it's not shown in sysfs.
> Same result when class wakeup is registered in a core_initcall
> (instead of postcore_initcall). Registering class wakeup in a pure_initcall
> works and fixes the issue. So, would this be an acceptable solution for the
> discussed issue?
I don't see why not.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PM / wakeup: Allocate class wakeup_class statically
2026-04-03 10:45 ` Rafael J. Wysocki
@ 2026-04-03 11:02 ` Heiner Kallweit
0 siblings, 0 replies; 7+ messages in thread
From: Heiner Kallweit @ 2026-04-03 11:02 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Pavel Machek, Len Brown, Greg Kroah-Hartman, Danilo Krummrich,
Linux PM, driver-core
On 03.04.2026 12:45, Rafael J. Wysocki wrote:
> On Thu, Apr 2, 2026 at 3:05 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>
>> On 01.04.2026 19:32, Rafael J. Wysocki wrote:
>>> On Wed, Apr 1, 2026 at 5:45 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>>>
>>>> On 01.04.2026 16:19, Rafael J. Wysocki wrote:
>>>>> On Sun, Mar 29, 2026 at 6:14 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>>>>>
>>>>>> Allocating wakeup_class statically avoids a little runtime overhead.
>>>>>> Define groups and device release function as part of the class, so that
>>>>>> we don't have to repeat this for each class device.
>>>>>> Whilst at it, constify wakeup_source_attrs[].
>>>>>>
>>>>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>>>>
>>>>> Can you please have a look at this and let me know what you think:
>>>>>
>>>>> https://sashiko.dev/#/patchset/0fe1b679-ab28-4505-b0db-14e7ac3ba749%40gmail.com
>>>>>
>>>> Interesting finding! I think the diagnosis is right.
>>>>
>>>> But: I would say the current behavior isn't a nice solution as well:
>>>
>>> It is not fantastic, but it doesn't have this issue.
>>>
>>>> wakeup_source_device_create() does: dev->class = wakeup_class;
>>>> I think no reader will expect that wakeup_class may be NULL here due to
>>>> initcall ordering. In addition this behavior results in such early
>>>> wakeup sources not being shown in sysfs.
>>>
>>> They are registered too early to show up in sysfs, but they can work regardless.
>>>
>>> I think that it's just pointless to call device_register() for a given
>>> wakeup source if wakeup_class has not been registered yet.
>>>
>>>> But I'm not sure whether registering class "wakeup" (and registering
>>>> classes in general) would be possible early enough (core_initcall,
>>>> or even pure_initcall).
>>>
>>> driver_init() is called before do_initcalls() is do_basic_setup(), so
>>> class registration should work for all initcall levels AFAICS.
>>>
>> When testing the current code on my system, autosleep is the first wakeup
>> source, registered in a core_initcall, and it's not shown in sysfs.
>> Same result when class wakeup is registered in a core_initcall
>> (instead of postcore_initcall). Registering class wakeup in a pure_initcall
>> works and fixes the issue. So, would this be an acceptable solution for the
>> discussed issue?
>
> I don't see why not.
OK, then I'll send a patch for it. I was asking because comment in init.h says:
A "pure" initcall has no dependencies on anything else, and purely
*initializes variables* that couldn't be statically initialized.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-04-03 11:02 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-29 16:14 [PATCH] PM / wakeup: Allocate class wakeup_class statically Heiner Kallweit
2026-04-01 14:19 ` Rafael J. Wysocki
2026-04-01 15:45 ` Heiner Kallweit
2026-04-01 17:32 ` Rafael J. Wysocki
2026-04-02 13:05 ` Heiner Kallweit
2026-04-03 10:45 ` Rafael J. Wysocki
2026-04-03 11:02 ` Heiner Kallweit
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox