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 for Android: free password hash cracker in your pocket

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

Message-ID: <164ae7a4-37b4-c8c4-3457-905c6bde52e1@oss.qualcomm.com>
Date: Wed, 26 Nov 2025 22:35:07 +0530
From: Shivendra Pratap <shivendra.pratap@....qualcomm.com>
To: Bartosz Golaszewski <brgl@...ev.pl>
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 11/26/2025 10:26 PM, Bartosz Golaszewski wrote:
> 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.

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