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

Rafael J. Wysocki
2026-04-01T17:33:08Z

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