Re: [PATCH v19 2/2] power: reset: reboot-mode: Expose sysfs for registered reboot_modes
From: Bartosz Golaszewski
Date: Wed Nov 26 2025 - 11:57:08 EST
On Wed, Nov 26, 2025 at 5:48 PM Shivendra Pratap
<shivendra.pratap@xxxxxxxxxxxxxxxx> 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