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

Heiner Kallweit
2026-04-02T13:05:36Z

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