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

[Shivendra Pratap]

On 11/26/2025 10:26 PM, Bartosz Golaszewski wrote:
> 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.

sure. thanks. will update this.

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