linux-kernel - Re: [PATCH v19 2/2] power: reset: reboot-mode: Expose sysfs for registered reboot

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMRc=McgiuD1D+WqmO9x2G8devztrLy6uLwxjFpxJ+LbKx2YJg@mail.gmail.com>
Date: Wed, 26 Nov 2025 17:56:54 +0100
From: Bartosz Golaszewski <brgl@...ev.pl>
To: Shivendra Pratap <shivendra.pratap@....qualcomm.com>
Cc: linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org, 
	linux-pm@...r.kernel.org, Sebastian Reichel <sre@...nel.org>, 
	Bartosz Golaszewski <bartosz.golaszewski@...aro.org>, Bjorn Andersson <andersson@...nel.org>
Subject: Re: [PATCH v19 2/2] power: reset: reboot-mode: Expose sysfs for
 registered reboot_modes

On Wed, Nov 26, 2025 at 5:48 PM Shivendra Pratap
<shivendra.pratap@....qualcomm.com> wrote:
>
> >>
> >> +static bool reboot_mode_class_registered;
> >
> > You don't need this, please see below.
>
> reboot_mode_class_registered was used for two reason.
> one is resolved: will directly call class_unregister.
>
> for second : If class_register fails, we want don't call register device
> in reboot_mode_register.
>
> at -
>         if (reboot_mode_class_registered)
>                 reboot_mode_register_device(reboot);
>

I'd just error out of the initcall if registering the class fails.
It's very unlikely anyway and points to a bigger problem.

> >> +
> >> +static void reboot_mode_register_device(struct reboot_mode_driver *reboot)
> >> +{
> >> +    reboot->reboot_mode_device.class = &reboot_mode_class;
> >> +    reboot->reboot_mode_device.release = reboot_mode_device_release;
> >> +    dev_set_name(&reboot->reboot_mode_device, reboot->driver_name);
> >> +    if (!device_register(&reboot->reboot_mode_device))
> >> +            reboot->reboot_mode_device_registered = true;
> >> +    else
> >> +            reboot->reboot_mode_device_registered = false;
> >
> > Just use device_create(). I would also suggest creating a private structure
> > that embeds the pointer to the struct device created by device_create() and
> > the pointer to the reboot_mode_driver. If you pass it as driver data to
> > device_create(), you'll be able to retrieve it with dev_get_drvdata() in
> > sysfs callbacks.
>
> Had made change to use device_create and dev_get_drvdata in below change, and have then
> changed it to above as per the reviews on the same.
> https://lore.kernel.org/all/qhlxxfsyc42xemerhi36myvil3bf45isgmpugkuqzsvgcc3ifn@njrtwuooij2q/
>
> Should we change to device_create?
>

Ah, I missed that part. My preference is for device_create() as IMO it
results in much more elegant code (especially if we don't end up
extending the public struct) and memory is cheap but I'll let Bjorn
decide.

Bart