Current reboot-mode supports a single 32-bit argument for any
supported mode. Some reboot-mode based drivers may require
passing two independent 32-bit arguments during a reboot
sequence, for uses-cases, where a mode requires an additional
argument. Such drivers may not be able to use the reboot-mode
driver. For example, ARM PSCI vendor-specific resets, need two
arguments for its operation – reset_type and cookie, to complete
the reset operation. If a driver wants to implement this
firmware-based reset, it cannot use reboot-mode framework.

Introduce 64-bit magic values in reboot-mode driver to
accommodate dual 32-bit arguments when specified via device tree.
In cases, where no second argument is passed from device tree,
keep the upper 32-bit of magic un-changed(0) to maintain backward
compatibility.

Update the current drivers using reboot-mode for a 64-bit magic
value.

Reviewed-by: Umang Chheda <umang.chheda@oss.qualcomm.com>
Reviewed-by: Nirmesh Kumar Singh <nirmesh.singh@oss.qualcomm.com>
Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
---
 drivers/power/reset/nvmem-reboot-mode.c  | 13 +++++++++----
 drivers/power/reset/qcom-pon.c           | 11 ++++++++---
 drivers/power/reset/reboot-mode.c        | 19 +++++++++++++------
 drivers/power/reset/syscon-reboot-mode.c | 11 ++++++++---
 include/linux/reboot-mode.h              |  3 ++-
 5 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/drivers/power/reset/nvmem-reboot-mode.c b/drivers/power/reset/nvmem-reboot-mode.c
index 41530b70cfc48c2a83fbbd96f523d5816960a0d1..5d73dde585b1fd438b1847f884feb37cd9e4dd5c 100644
--- a/drivers/power/reset/nvmem-reboot-mode.c
+++ b/drivers/power/reset/nvmem-reboot-mode.c
@@ -16,15 +16,20 @@ struct nvmem_reboot_mode {
 	struct nvmem_cell *cell;
 };
 
-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;
+
+	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;
 
+	if (magic > U32_MAX || (magic << pon->reason_shift) > U32_MAX)
+		return -EINVAL;
+
+	magic_32 = magic << pon->reason_shift;
+
 	ret = regmap_update_bits(pon->regmap,
 				 pon->baseaddr + PON_SOFT_RB_SPARE,
 				 GENMASK(7, pon->reason_shift),
-				 magic << pon->reason_shift);
+				 magic_32);
 	if (ret < 0)
 		dev_err(pon->dev, "update reboot mode bits failed\n");
 
diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
index eff60d6e04df2cb84ba59d38512654336f272f8a..873ac45cd7659b214b7c21958f580ca381e0a63d 100644
--- a/drivers/power/reset/reboot-mode.c
+++ b/drivers/power/reset/reboot-mode.c
@@ -19,12 +19,11 @@
 
 struct mode_info {
 	const char *mode;
-	u32 magic;
+	u64 magic;
 	struct list_head list;
 };
 
-static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
-					  const char *cmd)
+static u64 get_reboot_mode_magic(struct reboot_mode_driver *reboot, const char *cmd)
 {
 	const char *normal = "normal";
 	struct mode_info *info;
@@ -56,7 +55,7 @@ static int reboot_mode_notify(struct notifier_block *this,
 			      unsigned long mode, void *cmd)
 {
 	struct reboot_mode_driver *reboot;
-	unsigned int magic;
+	u64 magic;
 
 	reboot = container_of(this, struct reboot_mode_driver, reboot_notifier);
 	magic = get_reboot_mode_magic(reboot, cmd);
@@ -80,6 +79,8 @@ int reboot_mode_register(struct reboot_mode_driver *reboot, struct fwnode_handle
 	struct device_node *np;
 	struct property *prop;
 	size_t len = strlen(PREFIX);
+	u32 magic_arg1;
+	u32 magic_arg2;
 	int ret;
 
 	if (!fwnode)
@@ -101,12 +102,18 @@ int reboot_mode_register(struct reboot_mode_driver *reboot, struct fwnode_handle
 			goto error;
 		}
 
-		if (of_property_read_u32(np, prop->name, &info->magic)) {
-			pr_err("reboot mode %s without magic number\n", info->mode);
+		if (of_property_read_u32(np, prop->name, &magic_arg1)) {
+			pr_err("reboot mode without magic number\n");
 			kfree(info);
 			continue;
 		}
 
+		if (of_property_read_u32_index(np, prop->name, 1, &magic_arg2))
+			magic_arg2 = 0;
+
+		info->magic = magic_arg2;
+		info->magic = (info->magic << 32) | magic_arg1;
+
 		info->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
 		if (!info->mode) {
 			ret =  -ENOMEM;
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;
 
+	if (magic > U32_MAX)
+		return -EINVAL;
+
+	magic_32 = magic;
+
 	syscon_rbm = container_of(reboot, struct syscon_reboot_mode, reboot);
 
 	ret = regmap_update_bits(syscon_rbm->map, syscon_rbm->offset,
-				 syscon_rbm->mask, magic);
+				 syscon_rbm->mask, magic_32);
 	if (ret < 0)
 		dev_err(reboot->dev, "update reboot mode bits failed\n");
 
diff --git a/include/linux/reboot-mode.h b/include/linux/reboot-mode.h
index 22f707ade4ba93592823ea8718d467dbfc5ffd7c..e0d3e8a54050a76f26846f456120b4c7e371d284 100644
--- a/include/linux/reboot-mode.h
+++ b/include/linux/reboot-mode.h
@@ -3,11 +3,12 @@
 #define __REBOOT_MODE_H__
 
 #include <linux/fwnode.h>
+#include <linux/types.h>
 
 struct reboot_mode_driver {
 	struct device *dev;
 	struct list_head head;
-	int (*write)(struct reboot_mode_driver *reboot, unsigned int magic);
+	int (*write)(struct reboot_mode_driver *reboot, u64 magic);
 	struct notifier_block reboot_notifier;
 };
 

-- 
2.34.1

On Sun, Nov 09, 2025 at 08:07:16PM +0530, Shivendra Pratap wrote:
> Current reboot-mode supports a single 32-bit argument for any
> supported mode. Some reboot-mode based drivers may require
> passing two independent 32-bit arguments during a reboot
> sequence, for uses-cases, where a mode requires an additional
> argument. Such drivers may not be able to use the reboot-mode
> driver. For example, ARM PSCI vendor-specific resets, need two
> arguments for its operation – reset_type and cookie, to complete
> the reset operation. If a driver wants to implement this
> firmware-based reset, it cannot use reboot-mode framework.
> 
> Introduce 64-bit magic values in reboot-mode driver to
> accommodate dual 32-bit arguments when specified via device tree.
> In cases, where no second argument is passed from device tree,
> keep the upper 32-bit of magic un-changed(0) to maintain backward
> compatibility.
> 
> Update the current drivers using reboot-mode for a 64-bit magic
> value.
> 
> Reviewed-by: Umang Chheda <umang.chheda@oss.qualcomm.com>
> Reviewed-by: Nirmesh Kumar Singh <nirmesh.singh@oss.qualcomm.com>
> Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
> ---
>  drivers/power/reset/nvmem-reboot-mode.c  | 13 +++++++++----
>  drivers/power/reset/qcom-pon.c           | 11 ++++++++---
>  drivers/power/reset/reboot-mode.c        | 19 +++++++++++++------
>  drivers/power/reset/syscon-reboot-mode.c | 11 ++++++++---
>  include/linux/reboot-mode.h              |  3 ++-
>  5 files changed, 40 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/power/reset/nvmem-reboot-mode.c b/drivers/power/reset/nvmem-reboot-mode.c
> index 41530b70cfc48c2a83fbbd96f523d5816960a0d1..5d73dde585b1fd438b1847f884feb37cd9e4dd5c 100644
> --- a/drivers/power/reset/nvmem-reboot-mode.c
> +++ b/drivers/power/reset/nvmem-reboot-mode.c
> @@ -16,15 +16,20 @@ struct nvmem_reboot_mode {
>  	struct nvmem_cell *cell;
>  };
>  
> -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.. 

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

> +	if (magic > U32_MAX || (magic << pon->reason_shift) > U32_MAX)


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

> +		return -EINVAL;
> +
> +	magic_32 = magic << pon->reason_shift;
> +
>  	ret = regmap_update_bits(pon->regmap,
>  				 pon->baseaddr + PON_SOFT_RB_SPARE,
>  				 GENMASK(7, pon->reason_shift),
> -				 magic << pon->reason_shift);
> +				 magic_32);
>  	if (ret < 0)
>  		dev_err(pon->dev, "update reboot mode bits failed\n");
>  
> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
> index eff60d6e04df2cb84ba59d38512654336f272f8a..873ac45cd7659b214b7c21958f580ca381e0a63d 100644
> --- a/drivers/power/reset/reboot-mode.c
> +++ b/drivers/power/reset/reboot-mode.c
> @@ -19,12 +19,11 @@
>  
>  struct mode_info {
>  	const char *mode;
> -	u32 magic;
> +	u64 magic;
>  	struct list_head list;
>  };
>  
> -static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
> -					  const char *cmd)
> +static u64 get_reboot_mode_magic(struct reboot_mode_driver *reboot, const char *cmd)
>  {
>  	const char *normal = "normal";
>  	struct mode_info *info;
> @@ -56,7 +55,7 @@ static int reboot_mode_notify(struct notifier_block *this,
>  			      unsigned long mode, void *cmd)
>  {
>  	struct reboot_mode_driver *reboot;
> -	unsigned int magic;
> +	u64 magic;
>  
>  	reboot = container_of(this, struct reboot_mode_driver, reboot_notifier);
>  	magic = get_reboot_mode_magic(reboot, cmd);
> @@ -80,6 +79,8 @@ int reboot_mode_register(struct reboot_mode_driver *reboot, struct fwnode_handle
>  	struct device_node *np;
>  	struct property *prop;
>  	size_t len = strlen(PREFIX);
> +	u32 magic_arg1;
> +	u32 magic_arg2;
>  	int ret;
>  
>  	if (!fwnode)
> @@ -101,12 +102,18 @@ int reboot_mode_register(struct reboot_mode_driver *reboot, struct fwnode_handle
>  			goto error;
>  		}
>  
> -		if (of_property_read_u32(np, prop->name, &info->magic)) {
> -			pr_err("reboot mode %s without magic number\n", info->mode);
> +		if (of_property_read_u32(np, prop->name, &magic_arg1)) {
> +			pr_err("reboot mode without magic number\n");
>  			kfree(info);
>  			continue;
>  		}
>  
> +		if (of_property_read_u32_index(np, prop->name, 1, &magic_arg2))
> +			magic_arg2 = 0;
> +
> +		info->magic = magic_arg2;
> +		info->magic = (info->magic << 32) | magic_arg1;
> +
>  		info->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
>  		if (!info->mode) {
>  			ret =  -ENOMEM;
> 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

> +	if (magic > U32_MAX)
> +		return -EINVAL;
> +
> +	magic_32 = magic;
> +
>  	syscon_rbm = container_of(reboot, struct syscon_reboot_mode, reboot);
>  
>  	ret = regmap_update_bits(syscon_rbm->map, syscon_rbm->offset,
> -				 syscon_rbm->mask, magic);
> +				 syscon_rbm->mask, magic_32);
>  	if (ret < 0)
>  		dev_err(reboot->dev, "update reboot mode bits failed\n");
>  
> diff --git a/include/linux/reboot-mode.h b/include/linux/reboot-mode.h
> index 22f707ade4ba93592823ea8718d467dbfc5ffd7c..e0d3e8a54050a76f26846f456120b4c7e371d284 100644
> --- a/include/linux/reboot-mode.h
> +++ b/include/linux/reboot-mode.h
> @@ -3,11 +3,12 @@
>  #define __REBOOT_MODE_H__
>  
>  #include <linux/fwnode.h>
> +#include <linux/types.h>
>  
>  struct reboot_mode_driver {
>  	struct device *dev;
>  	struct list_head head;
> -	int (*write)(struct reboot_mode_driver *reboot, unsigned int magic);
> +	int (*write)(struct reboot_mode_driver *reboot, u64 magic);
>  	struct notifier_block reboot_notifier;
>  };
>  
> 
> -- 
> 2.34.1
> 

-- 
-Mukesh Ojha

On Mon, Nov 10, 2025 at 07:15:29PM +0530, 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
> > supported mode. Some reboot-mode based drivers may require
> > passing two independent 32-bit arguments during a reboot
> > sequence, for uses-cases, where a mode requires an additional
> > argument. Such drivers may not be able to use the reboot-mode
> > driver. For example, ARM PSCI vendor-specific resets, need two
> > arguments for its operation – reset_type and cookie, to complete
> > the reset operation. If a driver wants to implement this
> > firmware-based reset, it cannot use reboot-mode framework.
> > 
> > Introduce 64-bit magic values in reboot-mode driver to
> > accommodate dual 32-bit arguments when specified via device tree.
> > In cases, where no second argument is passed from device tree,
> > keep the upper 32-bit of magic un-changed(0) to maintain backward
> > compatibility.
> > 
> > Update the current drivers using reboot-mode for a 64-bit magic
> > value.
> > 
> > Reviewed-by: Umang Chheda <umang.chheda@oss.qualcomm.com>
> > Reviewed-by: Nirmesh Kumar Singh <nirmesh.singh@oss.qualcomm.com>
> > Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
> > ---
> >  drivers/power/reset/nvmem-reboot-mode.c  | 13 +++++++++----
> >  drivers/power/reset/qcom-pon.c           | 11 ++++++++---
> >  drivers/power/reset/reboot-mode.c        | 19 +++++++++++++------
> >  drivers/power/reset/syscon-reboot-mode.c | 11 ++++++++---
> >  include/linux/reboot-mode.h              |  3 ++-
> >  5 files changed, 40 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/power/reset/nvmem-reboot-mode.c b/drivers/power/reset/nvmem-reboot-mode.c
> > index 41530b70cfc48c2a83fbbd96f523d5816960a0d1..5d73dde585b1fd438b1847f884feb37cd9e4dd5c 100644
> > --- a/drivers/power/reset/nvmem-reboot-mode.c
> > +++ b/drivers/power/reset/nvmem-reboot-mode.c
> > @@ -16,15 +16,20 @@ struct nvmem_reboot_mode {
> >  	struct nvmem_cell *cell;
> >  };
> >  
> > -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.. 
> 
> > +
> > +	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.
> 
> > +	if (magic > U32_MAX || (magic << pon->reason_shift) > U32_MAX)
> 
> 
> is this (magic << pon->reason_shift) > U32_MAX really needed ..?
> 

In my eyes the introduction of a magic_32 variable seems a bit
copy-paste, and this check does tell a similar story...

We have two possible bit patterns:
  GENMASK(7, 2)
  GENMASK(7, 1)

So this means that we have either 5 or 6 bits of magic, not 32.

So far we've just trusted that the mode provided in DeviceTree would
fit, and we've silently discarded the other bits.

But if we're introducing checks here, we should do it properly, if
nothing else for the sake of making the code self-documented.


There's also no need for a new local variable for the magic, check that
we have a valid range and then just typecast it into the argument.

> > +		return -EINVAL;
> > +
> > +	magic_32 = magic << pon->reason_shift;
> > +
> >  	ret = regmap_update_bits(pon->regmap,
> >  				 pon->baseaddr + PON_SOFT_RB_SPARE,
> >  				 GENMASK(7, pon->reason_shift),
> > -				 magic << pon->reason_shift);
> > +				 magic_32);
> >  	if (ret < 0)
> >  		dev_err(pon->dev, "update reboot mode bits failed\n");
> >  
> > diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
> > index eff60d6e04df2cb84ba59d38512654336f272f8a..873ac45cd7659b214b7c21958f580ca381e0a63d 100644
> > --- a/drivers/power/reset/reboot-mode.c
> > +++ b/drivers/power/reset/reboot-mode.c
> > @@ -19,12 +19,11 @@
> >  
> >  struct mode_info {
> >  	const char *mode;
> > -	u32 magic;
> > +	u64 magic;
> >  	struct list_head list;
> >  };
> >  
> > -static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
> > -					  const char *cmd)
> > +static u64 get_reboot_mode_magic(struct reboot_mode_driver *reboot, const char *cmd)
> >  {
> >  	const char *normal = "normal";
> >  	struct mode_info *info;
> > @@ -56,7 +55,7 @@ static int reboot_mode_notify(struct notifier_block *this,
> >  			      unsigned long mode, void *cmd)
> >  {
> >  	struct reboot_mode_driver *reboot;
> > -	unsigned int magic;
> > +	u64 magic;
> >  
> >  	reboot = container_of(this, struct reboot_mode_driver, reboot_notifier);
> >  	magic = get_reboot_mode_magic(reboot, cmd);
> > @@ -80,6 +79,8 @@ int reboot_mode_register(struct reboot_mode_driver *reboot, struct fwnode_handle
> >  	struct device_node *np;
> >  	struct property *prop;
> >  	size_t len = strlen(PREFIX);
> > +	u32 magic_arg1;
> > +	u32 magic_arg2;
> >  	int ret;
> >  
> >  	if (!fwnode)
> > @@ -101,12 +102,18 @@ int reboot_mode_register(struct reboot_mode_driver *reboot, struct fwnode_handle
> >  			goto error;
> >  		}
> >  
> > -		if (of_property_read_u32(np, prop->name, &info->magic)) {
> > -			pr_err("reboot mode %s without magic number\n", info->mode);
> > +		if (of_property_read_u32(np, prop->name, &magic_arg1)) {
> > +			pr_err("reboot mode without magic number\n");
> >  			kfree(info);
> >  			continue;
> >  		}
> >  
> > +		if (of_property_read_u32_index(np, prop->name, 1, &magic_arg2))
> > +			magic_arg2 = 0;
> > +
> > +		info->magic = magic_arg2;
> > +		info->magic = (info->magic << 32) | magic_arg1;

There's no reason to assign the value and then reassign it, it just
makes the code harder to read.

If you mean "info->magic = (magic_arg2 << 32) | magic_arg1" then write
that...

> > +
> >  		info->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
> >  		if (!info->mode) {
> >  			ret =  -ENOMEM;
> > 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
> 
> > +	if (magic > U32_MAX)
> > +		return -EINVAL;
> > +
> > +	magic_32 = magic;
> > +
> >  	syscon_rbm = container_of(reboot, struct syscon_reboot_mode, reboot);
> >  
> >  	ret = regmap_update_bits(syscon_rbm->map, syscon_rbm->offset,
> > -				 syscon_rbm->mask, magic);
> > +				 syscon_rbm->mask, magic_32);

As above, if we're no longer silently discarding bits, I think we should
compare the magic against syscon_rbm->mask.

No need for a local variable, just type cast after checking the validity.

Regards,
Bjorn

> >  	if (ret < 0)
> >  		dev_err(reboot->dev, "update reboot mode bits failed\n");
> >  
> > diff --git a/include/linux/reboot-mode.h b/include/linux/reboot-mode.h
> > index 22f707ade4ba93592823ea8718d467dbfc5ffd7c..e0d3e8a54050a76f26846f456120b4c7e371d284 100644
> > --- a/include/linux/reboot-mode.h
> > +++ b/include/linux/reboot-mode.h
> > @@ -3,11 +3,12 @@
> >  #define __REBOOT_MODE_H__
> >  
> >  #include <linux/fwnode.h>
> > +#include <linux/types.h>
> >  
> >  struct reboot_mode_driver {
> >  	struct device *dev;
> >  	struct list_head head;
> > -	int (*write)(struct reboot_mode_driver *reboot, unsigned int magic);
> > +	int (*write)(struct reboot_mode_driver *reboot, u64 magic);
> >  	struct notifier_block reboot_notifier;
> >  };
> >  
> > 
> > -- 
> > 2.34.1
> > 
> 
> -- 
> -Mukesh Ojha

On 11/10/2025 10:00 PM, Bjorn Andersson wrote:
> On Mon, Nov 10, 2025 at 07:15:29PM +0530, 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
>>> supported mode. Some reboot-mode based drivers may require
>>> passing two independent 32-bit arguments during a reboot
>>> sequence, for uses-cases, where a mode requires an additional
>>> argument. Such drivers may not be able to use the reboot-mode
>>> driver. For example, ARM PSCI vendor-specific resets, need two
>>> arguments for its operation – reset_type and cookie, to complete
>>> the reset operation. If a driver wants to implement this
>>> firmware-based reset, it cannot use reboot-mode framework.
>>>
>>> Introduce 64-bit magic values in reboot-mode driver to
>>> accommodate dual 32-bit arguments when specified via device tree.
>>> In cases, where no second argument is passed from device tree,
>>> keep the upper 32-bit of magic un-changed(0) to maintain backward
>>> compatibility.
>>>
>>> Update the current drivers using reboot-mode for a 64-bit magic
>>> value.

[SNIP..]

>>> +	if (magic > U32_MAX)
>>> +		return -EINVAL;
>>> +
>>> +	magic_32 = magic;
>>> +
>>>  	syscon_rbm = container_of(reboot, struct syscon_reboot_mode, reboot);
>>>  
>>>  	ret = regmap_update_bits(syscon_rbm->map, syscon_rbm->offset,
>>> -				 syscon_rbm->mask, magic);
>>> +				 syscon_rbm->mask, magic_32);
> 
> As above, if we're no longer silently discarding bits, I think we should
> compare the magic against syscon_rbm->mask.
> 
> No need for a local variable, just type cast after checking the validity.

Trying to summarize below why we added these check-

the patch in v11 used typecasting and did not have any of these checks(link below):
https://lore.kernel.org/all/20250717-arm-psci-system_reset2-vendor-reboots-v11-2-df3e2b2183c3@oss.qualcomm.com/

As per below upstream review, type cast was removed and bound checks were added all-over patchset:
"As a general rule of thumb, code with casts is poor quality code. Try
to write the code without casts." - 
https://lore.kernel.org/all/8d4a42b6-657f-4c30-8e25-4213d8d53a89@lunn.ch/

We can revert to the typecast way. Please suggest.

thanks,
Shivendra

On Mon, Nov 10, 2025 at 11:22:40PM +0530, Shivendra Pratap wrote:
> 
> 
> On 11/10/2025 10:00 PM, Bjorn Andersson wrote:
> > On Mon, Nov 10, 2025 at 07:15:29PM +0530, 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
> >>> supported mode. Some reboot-mode based drivers may require
> >>> passing two independent 32-bit arguments during a reboot
> >>> sequence, for uses-cases, where a mode requires an additional
> >>> argument. Such drivers may not be able to use the reboot-mode
> >>> driver. For example, ARM PSCI vendor-specific resets, need two
> >>> arguments for its operation – reset_type and cookie, to complete
> >>> the reset operation. If a driver wants to implement this
> >>> firmware-based reset, it cannot use reboot-mode framework.
> >>>
> >>> Introduce 64-bit magic values in reboot-mode driver to
> >>> accommodate dual 32-bit arguments when specified via device tree.
> >>> In cases, where no second argument is passed from device tree,
> >>> keep the upper 32-bit of magic un-changed(0) to maintain backward
> >>> compatibility.
> >>>
> >>> Update the current drivers using reboot-mode for a 64-bit magic
> >>> value.
> 
> [SNIP..]
> 
> >>> +	if (magic > U32_MAX)
> >>> +		return -EINVAL;
> >>> +
> >>> +	magic_32 = magic;
> >>> +
> >>>  	syscon_rbm = container_of(reboot, struct syscon_reboot_mode, reboot);
> >>>  
> >>>  	ret = regmap_update_bits(syscon_rbm->map, syscon_rbm->offset,
> >>> -				 syscon_rbm->mask, magic);
> >>> +				 syscon_rbm->mask, magic_32);
> > 
> > As above, if we're no longer silently discarding bits, I think we should
> > compare the magic against syscon_rbm->mask.
> > 
> > No need for a local variable, just type cast after checking the validity.
> 
> Trying to summarize below why we added these check-
> 
> the patch in v11 used typecasting and did not have any of these checks(link below):
> https://lore.kernel.org/all/20250717-arm-psci-system_reset2-vendor-reboots-v11-2-df3e2b2183c3@oss.qualcomm.com/
> 
> As per below upstream review, type cast was removed and bound checks were added all-over patchset:
> "As a general rule of thumb, code with casts is poor quality code. Try
> to write the code without casts." - 
> https://lore.kernel.org/all/8d4a42b6-657f-4c30-8e25-4213d8d53a89@lunn.ch/
> 
> We can revert to the typecast way. Please suggest.
> 

Okay, I'm okay with Andrew's original request, stick to that for the
nvmem case. Although I don't fancy the name "magic_32", and would prefer
that you just call it "value" or something.


For pon and syscon however, I'm wondering why you have ignored Andrew's
other request from that same email:

"""
You might be able to go further, and validate that magic actually fits
into the field when you consider the << pon->reason_shift.
"""

Writing "if (magic > U32_MAX)" in a snippet of code where magic isn't
allowed to be more than either 32 or 64 is misleading.

For syscon, it's true that the parameter is an unsigned long, but the
actual limit better be based on syscon_rbm->mask.

Regards,
Bjorn

> thanks,
> Shivendra

On 11/11/2025 12:03 AM, Bjorn Andersson wrote:
> On Mon, Nov 10, 2025 at 11:22:40PM +0530, Shivendra Pratap wrote:
>>
>>
>> On 11/10/2025 10:00 PM, Bjorn Andersson wrote:
>>> On Mon, Nov 10, 2025 at 07:15:29PM +0530, 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
>>>>> supported mode. Some reboot-mode based drivers may require
>>>>> passing two independent 32-bit arguments during a reboot
>>>>> sequence, for uses-cases, where a mode requires an additional
>>>>> argument. Such drivers may not be able to use the reboot-mode
>>>>> driver. For example, ARM PSCI vendor-specific resets, need two
>>>>> arguments for its operation – reset_type and cookie, to complete
>>>>> the reset operation. If a driver wants to implement this
>>>>> firmware-based reset, it cannot use reboot-mode framework.
>>>>>
>>>>> Introduce 64-bit magic values in reboot-mode driver to
>>>>> accommodate dual 32-bit arguments when specified via device tree.
>>>>> In cases, where no second argument is passed from device tree,
>>>>> keep the upper 32-bit of magic un-changed(0) to maintain backward
>>>>> compatibility.
>>>>>
>>>>> Update the current drivers using reboot-mode for a 64-bit magic
>>>>> value.
>>
>> [SNIP..]
>>
>>>>> +	if (magic > U32_MAX)
>>>>> +		return -EINVAL;
>>>>> +
>>>>> +	magic_32 = magic;
>>>>> +
>>>>>  	syscon_rbm = container_of(reboot, struct syscon_reboot_mode, reboot);
>>>>>  
>>>>>  	ret = regmap_update_bits(syscon_rbm->map, syscon_rbm->offset,
>>>>> -				 syscon_rbm->mask, magic);
>>>>> +				 syscon_rbm->mask, magic_32);
>>>
>>> As above, if we're no longer silently discarding bits, I think we should
>>> compare the magic against syscon_rbm->mask.
>>>
>>> No need for a local variable, just type cast after checking the validity.
>>
>> Trying to summarize below why we added these check-
>>
>> the patch in v11 used typecasting and did not have any of these checks(link below):
>> https://lore.kernel.org/all/20250717-arm-psci-system_reset2-vendor-reboots-v11-2-df3e2b2183c3@oss.qualcomm.com/
>>
>> As per below upstream review, type cast was removed and bound checks were added all-over patchset:
>> "As a general rule of thumb, code with casts is poor quality code. Try
>> to write the code without casts." - 
>> https://lore.kernel.org/all/8d4a42b6-657f-4c30-8e25-4213d8d53a89@lunn.ch/
>>
>> We can revert to the typecast way. Please suggest.
>>
> 
> Okay, I'm okay with Andrew's original request, stick to that for the
> nvmem case. Although I don't fancy the name "magic_32", and would prefer
> that you just call it "value" or something.

sure will make it proper wherever applicable. 

> 
> 
> For pon and syscon however, I'm wondering why you have ignored Andrew's
> other request from that same email:
> 
> """
> You might be able to go further, and validate that magic actually fits
> into the field when you consider the << pon->reason_shift.
> """
> 
> Writing "if (magic > U32_MAX)" in a snippet of code where magic isn't
> allowed to be more than either 32 or 64 is misleading.
> 
> For syscon, it's true that the parameter is an unsigned long, but the
> actual limit better be based on syscon_rbm->mask.

May be i was not able to interpret it correctly. Basically tried to
make sure that magic that now coming in a "u64 magic" should be passed
ahead only it its a 32 bit value.

So should i get rid of the checks done here for syscon and pon?

thanks,
Shivendra

On Tue, Nov 11, 2025 at 08:20:43PM +0530, Shivendra Pratap wrote:
> 
> 
> On 11/11/2025 12:03 AM, Bjorn Andersson wrote:
> > On Mon, Nov 10, 2025 at 11:22:40PM +0530, Shivendra Pratap wrote:
> >>
> >>
> >> On 11/10/2025 10:00 PM, Bjorn Andersson wrote:
> >>> On Mon, Nov 10, 2025 at 07:15:29PM +0530, 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
> >>>>> supported mode. Some reboot-mode based drivers may require
> >>>>> passing two independent 32-bit arguments during a reboot
> >>>>> sequence, for uses-cases, where a mode requires an additional
> >>>>> argument. Such drivers may not be able to use the reboot-mode
> >>>>> driver. For example, ARM PSCI vendor-specific resets, need two
> >>>>> arguments for its operation – reset_type and cookie, to complete
> >>>>> the reset operation. If a driver wants to implement this
> >>>>> firmware-based reset, it cannot use reboot-mode framework.
> >>>>>
> >>>>> Introduce 64-bit magic values in reboot-mode driver to
> >>>>> accommodate dual 32-bit arguments when specified via device tree.
> >>>>> In cases, where no second argument is passed from device tree,
> >>>>> keep the upper 32-bit of magic un-changed(0) to maintain backward
> >>>>> compatibility.
> >>>>>
> >>>>> Update the current drivers using reboot-mode for a 64-bit magic
> >>>>> value.
> >>
> >> [SNIP..]
> >>
> >>>>> +	if (magic > U32_MAX)
> >>>>> +		return -EINVAL;
> >>>>> +
> >>>>> +	magic_32 = magic;
> >>>>> +
> >>>>>  	syscon_rbm = container_of(reboot, struct syscon_reboot_mode, reboot);
> >>>>>  
> >>>>>  	ret = regmap_update_bits(syscon_rbm->map, syscon_rbm->offset,
> >>>>> -				 syscon_rbm->mask, magic);
> >>>>> +				 syscon_rbm->mask, magic_32);
> >>>
> >>> As above, if we're no longer silently discarding bits, I think we should
> >>> compare the magic against syscon_rbm->mask.
> >>>
> >>> No need for a local variable, just type cast after checking the validity.
> >>
> >> Trying to summarize below why we added these check-
> >>
> >> the patch in v11 used typecasting and did not have any of these checks(link below):
> >> https://lore.kernel.org/all/20250717-arm-psci-system_reset2-vendor-reboots-v11-2-df3e2b2183c3@oss.qualcomm.com/
> >>
> >> As per below upstream review, type cast was removed and bound checks were added all-over patchset:
> >> "As a general rule of thumb, code with casts is poor quality code. Try
> >> to write the code without casts." - 
> >> https://lore.kernel.org/all/8d4a42b6-657f-4c30-8e25-4213d8d53a89@lunn.ch/
> >>
> >> We can revert to the typecast way. Please suggest.
> >>
> > 
> > Okay, I'm okay with Andrew's original request, stick to that for the
> > nvmem case. Although I don't fancy the name "magic_32", and would prefer
> > that you just call it "value" or something.
> 
> sure will make it proper wherever applicable. 
> 
> > 
> > 
> > For pon and syscon however, I'm wondering why you have ignored Andrew's
> > other request from that same email:
> > 
> > """
> > You might be able to go further, and validate that magic actually fits
> > into the field when you consider the << pon->reason_shift.
> > """
> > 
> > Writing "if (magic > U32_MAX)" in a snippet of code where magic isn't
> > allowed to be more than either 32 or 64 is misleading.
> > 
> > For syscon, it's true that the parameter is an unsigned long, but the
> > actual limit better be based on syscon_rbm->mask.
> 
> May be i was not able to interpret it correctly. Basically tried to
> make sure that magic that now coming in a "u64 magic" should be passed
> ahead only it its a 32 bit value.
> 

That is the correct interpretation of the original ask. But what I'm
saying is that if you write:

if (magic > U32_MAX)

then that means "check that magic isn't larger than 32 bits". So the
reader will see that and understand that magic can only be 32 bits.

But the actual PON magic value is 5 or 6 bits, not 32 - so you should
check that the value fits in 5 or 6 bits.

> So should i get rid of the checks done here for syscon and pon?
> 

Continuing to silently ignoring other bits would be one option, but
you've been asked to sanity check the values, so please do that. You
have the code, just compare with the correct value.

Regards,
Bjorn

> thanks,
> Shivendra

On 11/11/2025 9:55 PM, Bjorn Andersson wrote:
> On Tue, Nov 11, 2025 at 08:20:43PM +0530, Shivendra Pratap wrote:
>>
>>
>> On 11/11/2025 12:03 AM, Bjorn Andersson wrote:
>>> On Mon, Nov 10, 2025 at 11:22:40PM +0530, Shivendra Pratap wrote:
>>>>
>>>>
>>>> On 11/10/2025 10:00 PM, Bjorn Andersson wrote:
>>>>> On Mon, Nov 10, 2025 at 07:15:29PM +0530, 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
>>>>>>> supported mode. Some reboot-mode based drivers may require
>>>>>>> passing two independent 32-bit arguments during a reboot
>>>>>>> sequence, for uses-cases, where a mode requires an additional
>>>>>>> argument. Such drivers may not be able to use the reboot-mode
>>>>>>> driver. For example, ARM PSCI vendor-specific resets, need two
>>>>>>> arguments for its operation – reset_type and cookie, to complete
>>>>>>> the reset operation. If a driver wants to implement this
>>>>>>> firmware-based reset, it cannot use reboot-mode framework.
>>>>>>>
>>>>>>> Introduce 64-bit magic values in reboot-mode driver to
>>>>>>> accommodate dual 32-bit arguments when specified via device tree.
>>>>>>> In cases, where no second argument is passed from device tree,
>>>>>>> keep the upper 32-bit of magic un-changed(0) to maintain backward
>>>>>>> compatibility.
>>>>>>>
>>>>>>> Update the current drivers using reboot-mode for a 64-bit magic
>>>>>>> value.
>>>>
>>>> [SNIP..]
>>>>
>>>>>>> +	if (magic > U32_MAX)
>>>>>>> +		return -EINVAL;
>>>>>>> +
>>>>>>> +	magic_32 = magic;
>>>>>>> +
>>>>>>>  	syscon_rbm = container_of(reboot, struct syscon_reboot_mode, reboot);
>>>>>>>  
>>>>>>>  	ret = regmap_update_bits(syscon_rbm->map, syscon_rbm->offset,
>>>>>>> -				 syscon_rbm->mask, magic);
>>>>>>> +				 syscon_rbm->mask, magic_32);
>>>>>
>>>>> As above, if we're no longer silently discarding bits, I think we should
>>>>> compare the magic against syscon_rbm->mask.
>>>>>
>>>>> No need for a local variable, just type cast after checking the validity.
>>>>
>>>> Trying to summarize below why we added these check-
>>>>
>>>> the patch in v11 used typecasting and did not have any of these checks(link below):
>>>> https://lore.kernel.org/all/20250717-arm-psci-system_reset2-vendor-reboots-v11-2-df3e2b2183c3@oss.qualcomm.com/
>>>>
>>>> As per below upstream review, type cast was removed and bound checks were added all-over patchset:
>>>> "As a general rule of thumb, code with casts is poor quality code. Try
>>>> to write the code without casts." - 
>>>> https://lore.kernel.org/all/8d4a42b6-657f-4c30-8e25-4213d8d53a89@lunn.ch/
>>>>
>>>> We can revert to the typecast way. Please suggest.
>>>>
>>>
>>> Okay, I'm okay with Andrew's original request, stick to that for the
>>> nvmem case. Although I don't fancy the name "magic_32", and would prefer
>>> that you just call it "value" or something.
>>
>> sure will make it proper wherever applicable. 
>>
>>>
>>>
>>> For pon and syscon however, I'm wondering why you have ignored Andrew's
>>> other request from that same email:
>>>
>>> """
>>> You might be able to go further, and validate that magic actually fits
>>> into the field when you consider the << pon->reason_shift.
>>> """
>>>
>>> Writing "if (magic > U32_MAX)" in a snippet of code where magic isn't
>>> allowed to be more than either 32 or 64 is misleading.
>>>
>>> For syscon, it's true that the parameter is an unsigned long, but the
>>> actual limit better be based on syscon_rbm->mask.
>>
>> May be i was not able to interpret it correctly. Basically tried to
>> make sure that magic that now coming in a "u64 magic" should be passed
>> ahead only it its a 32 bit value.
>>
> 
> That is the correct interpretation of the original ask. But what I'm
> saying is that if you write:
> 
> if (magic > U32_MAX)
> 
> then that means "check that magic isn't larger than 32 bits". So the
> reader will see that and understand that magic can only be 32 bits.
> 
> But the actual PON magic value is 5 or 6 bits, not 32 - so you should
> check that the value fits in 5 or 6 bits.

sure. thanks.

> 
>> So should i get rid of the checks done here for syscon and pon?
>>
> 
> Continuing to silently ignoring other bits would be one option, but
> you've been asked to sanity check the values, so please do that. You
> have the code, just compare with the correct value.

ok. got it. thanks.

thanks,
Shivendra

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