Re: [PATCH] PM / wakeup: Allocate class wakeup_class statically

[Heiner Kallweit <hkallweit1@gmail.com>]

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
>>>>
>>