Re: [PATCH v17 03/12] power: reset: reboot-mode: Add support for 64 bit magic

Shivendra Pratap
2025-11-10T14:38:39Z

On 11/10/2025 7:15 PM, Mukesh Ojha wrote:
> On Sun, Nov 09, 2025 at 08:07:16PM +0530, Shivendra Pratap wrote:
>> Current reboot-mode supports a single 32-bit argument for any

[SNIP..]

>>
>> -static int nvmem_reboot_mode_write(struct reboot_mode_driver *reboot,
>> - unsigned int magic)
>> +static int nvmem_reboot_mode_write(struct reboot_mode_driver *reboot, u64 magic)
>> {
>> - int ret;
>> struct nvmem_reboot_mode *nvmem_rbm;
>> + u32 magic_32;
>> + int ret;
>> +
>> + if (magic > U32_MAX)
>> + return -EINVAL;
>
>
> I believe, we need a comment in all the client driver..

Ack. Will add a comment.

>
>> +
>> + magic_32 = magic;
>>
>> nvmem_rbm = container_of(reboot, struct nvmem_reboot_mode, reboot);
>>
>> - ret = nvmem_cell_write(nvmem_rbm->cell, &magic, sizeof(magic));
>> + ret = nvmem_cell_write(nvmem_rbm->cell, &magic_32, sizeof(magic_32));
>> if (ret < 0)
>> dev_err(reboot->dev, "update reboot mode bits failed\n");
>>
>> diff --git a/drivers/power/reset/qcom-pon.c b/drivers/power/reset/qcom-pon.c
>> index 7e108982a582e8243c5c806bd4a793646b87189f..d0ed9431a02313a7bbaa93743c16fa1ae713ddfe 100644
>> --- a/drivers/power/reset/qcom-pon.c
>> +++ b/drivers/power/reset/qcom-pon.c
>> @@ -27,17 +27,22 @@ struct qcom_pon {
>> long reason_shift;
>> };
>>
>> -static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot,
>> - unsigned int magic)
>> +static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot, u64 magic)
>> {
>> struct qcom_pon *pon = container_of
>> (reboot, struct qcom_pon, reboot_mode);
>> + u32 magic_32;
>> int ret;
>>
>
> Since we are doing this change in reboot framework, client driver should know about
> it too about this new check because of framework.

ok.

>
>> + if (magic > U32_MAX || (magic << pon->reason_shift) > U32_MAX)
>
>
> is this (magic << pon->reason_shift) > U32_MAX really needed ..?

Can be omitted as we already checked magic > U32_MAX. Will
remove it.

>

[SNIP..]

>> diff --git a/drivers/power/reset/syscon-reboot-mode.c b/drivers/power/reset/syscon-reboot-mode.c
>> index e0772c9f70f7a19cd8ec8a0b7fdbbaa7ba44afd0..3cbd000c512239b12ec51987e900d260540a9dea 100644
>> --- a/drivers/power/reset/syscon-reboot-mode.c
>> +++ b/drivers/power/reset/syscon-reboot-mode.c
>> @@ -20,16 +20,21 @@ struct syscon_reboot_mode {
>> u32 mask;
>> };
>>
>> -static int syscon_reboot_mode_write(struct reboot_mode_driver *reboot,
>> - unsigned int magic)
>> +static int syscon_reboot_mode_write(struct reboot_mode_driver *reboot, u64 magic)
>> {
>> struct syscon_reboot_mode *syscon_rbm;
>> + u32 magic_32;
>> int ret;
>>
>
> same here

will add comment here.

thanks,
Shivendra