[PATCH 1/3] reset_source: have reset_source_set_device actually set device

[PATCH 1/3] reset_source: have reset_source_set_device actually set device

[Ahmad Fatoum]

reset_source_set_device is given a device pointer as argument and it
passes it to __reset_source_set, but the latter function discards the
argument and unconditionally stores a NULL into the reset_source_device.

Fix this by actually making use of the argument.

Fixes: a0748840c104 ("reset_source: implement helper to set a device as reset source")
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/reset_source.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/reset_source.c b/common/reset_source.c
index f28be90dcbb4..495f3d527a84 100644
--- a/common/reset_source.c
+++ b/common/reset_source.c
@@ -61,7 +61,7 @@ static void __reset_source_set(struct device *dev,
 	reset_source = st;
 	reset_source_priority = priority;
 	reset_source_instance = instance;
-	reset_source_device = NULL;
+	reset_source_device = dev;
 
 	pr_debug("Setting reset source to %s with priority %d\n",
 			reset_src_names[reset_source], priority);
-- 
2.39.5

[PATCH 2/3] mfd: rn5t568: refactor to call reset_source_set_device only once

[Ahmad Fatoum]

---
 drivers/mfd/rn5t568.c | 59 +++++++++++++++++++++----------------------
 1 file changed, 29 insertions(+), 30 deletions(-)

diff --git a/drivers/mfd/rn5t568.c b/drivers/mfd/rn5t568.c
index f1e2eeb0c88b..7468338a2d53 100644
--- a/drivers/mfd/rn5t568.c
+++ b/drivers/mfd/rn5t568.c
@@ -33,6 +33,7 @@ static void rn5t568_restart(struct restart_handler *rst)
 static int rn5t568_reset_reason_detect(struct device *dev,
 				       struct regmap *regmap)
 {
+	enum reset_src_type type;
 	unsigned int reg;
 	int ret;
 
@@ -47,40 +48,38 @@ static int rn5t568_reset_reason_detect(struct device *dev,
 		return 0;
 	}
 
-	if (reg & RN5T568_PONHIS_ON_EXTINPON) {
-		reset_source_set_device(dev, RESET_POR);
-		return 0;
-	} else if (reg & RN5T568_PONHIS_ON_PWRONPON) {
-		reset_source_set_device(dev, RESET_POR);
-		return 0;
-	} else if (!(reg & RN5T568_PONHIS_ON_REPWRPON))
+	if (reg & (RN5T568_PONHIS_ON_EXTINPON | RN5T568_PONHIS_ON_PWRONPON)) {
+		type = RESET_POR;
+	} else if (!(reg & RN5T568_PONHIS_ON_REPWRPON)) {
 		return -EINVAL;
+	} else {
+		ret = regmap_read(regmap, RN5T568_POFFHIS, &reg);
+		if (ret)
+			return ret;
 
-	ret = regmap_read(regmap, RN5T568_POFFHIS, &reg);
-	if (ret)
-		return ret;
+		dev_dbg(dev, "Power-off history: %x\n", reg);
 
-	dev_dbg(dev, "Power-off history: %x\n", reg);
-
-	if (reg & RN5T568_POFFHIS_PWRONPOFF)
-		reset_source_set_device(dev, RESET_POR);
-	else if (reg & RN5T568_POFFHIS_TSHUTPOFF)
-		reset_source_set_device(dev, RESET_THERM);
-	else if (reg & RN5T568_POFFHIS_VINDETPOFF)
-		reset_source_set_device(dev, RESET_BROWNOUT);
-	else if (reg & RN5T568_POFFHIS_IODETPOFF)
-		reset_source_set_device(dev, RESET_UKWN);
-	else if (reg & RN5T568_POFFHIS_CPUPOFF)
-		reset_source_set_device(dev, RESET_RST);
-	else if (reg & RN5T568_POFFHIS_WDGPOFF)
-		reset_source_set_device(dev, RESET_WDG);
-	else if (reg & RN5T568_POFFHIS_DCLIMPOFF)
-		reset_source_set_device(dev, RESET_BROWNOUT);
-	else if (reg & RN5T568_POFFHIS_N_OEPOFF)
-		reset_source_set_device(dev, RESET_EXT);
-	else
-		return -EINVAL;
+		if (reg & RN5T568_POFFHIS_PWRONPOFF)
+			type = RESET_POR;
+		else if (reg & RN5T568_POFFHIS_TSHUTPOFF)
+			type = RESET_THERM;
+		else if (reg & RN5T568_POFFHIS_VINDETPOFF)
+			type = RESET_BROWNOUT;
+		else if (reg & RN5T568_POFFHIS_IODETPOFF)
+			type = RESET_UKWN;
+		else if (reg & RN5T568_POFFHIS_CPUPOFF)
+			type = RESET_RST;
+		else if (reg & RN5T568_POFFHIS_WDGPOFF)
+			type = RESET_WDG;
+		else if (reg & RN5T568_POFFHIS_DCLIMPOFF)
+			type = RESET_BROWNOUT;
+		else if (reg & RN5T568_POFFHIS_N_OEPOFF)
+			type = RESET_EXT;
+		else
+			return -EINVAL;
+	}
 
+	reset_source_set_device(dev, type);
 	return 0;
 }
 
-- 
2.39.5

[PATCH 3/3] reset_source: give reset_source_set_device a priority parameter

[Ahmad Fatoum]

All users of reset_source_set_device() are PMICs, which should set a
higher reset priority that the default anyway, therefore let's give the
function a priority parameter and set a default priority of 200 as
opposed to RESET_SOURCE_DEFAULT_PRIORITY, which expands to 100.

Setting a higher priority is important as PMICs tend to power cycle the
SoC, so asking the SoC about the reset source would then always return
POR.

In my particular case, the i.MX watchdog driver sees that the reset
source is WDG and then it sets POR with a priority of 101, overriding
what the PMIC determined.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/reset_source.c  | 5 ++---
 drivers/mfd/da9053.c   | 2 +-
 drivers/mfd/da9063.c   | 2 +-
 drivers/mfd/pca9450.c  | 2 +-
 include/reset_source.h | 6 ++++--
 5 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/common/reset_source.c b/common/reset_source.c
index 495f3d527a84..bd1efe073664 100644
--- a/common/reset_source.c
+++ b/common/reset_source.c
@@ -74,10 +74,9 @@ void reset_source_set_prinst(enum reset_src_type st,
 }
 EXPORT_SYMBOL(reset_source_set_prinst);
 
-void reset_source_set_device(struct device *dev, enum reset_src_type st)
+void reset_source_set_device(struct device *dev, enum reset_src_type st,
+			     unsigned int priority)
 {
-	unsigned int priority = RESET_SOURCE_DEFAULT_PRIORITY;
-
 	of_property_read_u32(dev->of_node, "reset-source-priority", &priority);
 
 	__reset_source_set(dev, st, priority, -1);
diff --git a/drivers/mfd/da9053.c b/drivers/mfd/da9053.c
index cbfb62cef9cd..c8ad8aa0eeed 100644
--- a/drivers/mfd/da9053.c
+++ b/drivers/mfd/da9053.c
@@ -224,7 +224,7 @@ static void da9053_detect_reset_source(struct da9053_priv *da9053)
 	else
 		return;
 
-	reset_source_set_device(da9053->dev, type);
+	reset_source_set_device(da9053->dev, type, 200);
 
 	ret = da9053_reg_write(da9053, DA9053_FAULTLOG_REG, val);
 	if (ret < 0)
diff --git a/drivers/mfd/da9063.c b/drivers/mfd/da9063.c
index 04bcad88040c..4842ead6f8b9 100644
--- a/drivers/mfd/da9063.c
+++ b/drivers/mfd/da9063.c
@@ -309,7 +309,7 @@ static void da9063_detect_reset_source(struct da9063 *priv)
 	else
 		return;
 
-	reset_source_set_device(priv->dev, type);
+	reset_source_set_device(priv->dev, type, 200);
 }
 
 static void da9063_restart(struct restart_handler *rst)
diff --git a/drivers/mfd/pca9450.c b/drivers/mfd/pca9450.c
index 89eca9f890e3..73b5cecbeda5 100644
--- a/drivers/mfd/pca9450.c
+++ b/drivers/mfd/pca9450.c
@@ -58,7 +58,7 @@ static int pca9450_get_reset_source(struct device *dev, struct regmap *map)
 		type = RESET_UKWN;
 	}
 
-	reset_source_set_device(dev, type);
+	reset_source_set_device(dev, type, 200);
 
 	return 0;
 };
diff --git a/include/reset_source.h b/include/reset_source.h
index 3766208b6d11..cc1fe60d4615 100644
--- a/include/reset_source.h
+++ b/include/reset_source.h
@@ -28,7 +28,8 @@ const char *reset_source_to_string(enum reset_src_type st);
 int reset_source_get_instance(void);
 struct device *reset_source_get_device(void);
 
-void reset_source_set_device(struct device *dev, enum reset_src_type st);
+void reset_source_set_device(struct device *dev, enum reset_src_type st,
+			     unsigned int priority);
 void reset_source_set_prinst(enum reset_src_type,
 			     unsigned int priority, int instance);
 
@@ -55,7 +56,8 @@ static inline struct device *reset_source_get_device(void)
 }
 
 static inline void reset_source_set_device(struct device *dev,
-					   enum reset_src_type st)
+					   enum reset_src_type st,
+					   unsigned int priority)
 {
 }
 
-- 
2.39.5

Re: [PATCH 2/3] mfd: rn5t568: refactor to call reset_source_set_device only once

[Sascha Hauer]

On Wed, Nov 20, 2024 at 02:06:39PM +0100, Ahmad Fatoum wrote:
> ---
>  drivers/mfd/rn5t568.c | 59 +++++++++++++++++++++----------------------
>  1 file changed, 29 insertions(+), 30 deletions(-)
> 

No S-o-b?

Sascha

> diff --git a/drivers/mfd/rn5t568.c b/drivers/mfd/rn5t568.c
> index f1e2eeb0c88b..7468338a2d53 100644
> --- a/drivers/mfd/rn5t568.c
> +++ b/drivers/mfd/rn5t568.c
> @@ -33,6 +33,7 @@ static void rn5t568_restart(struct restart_handler *rst)
>  static int rn5t568_reset_reason_detect(struct device *dev,
>  				       struct regmap *regmap)
>  {
> +	enum reset_src_type type;
>  	unsigned int reg;
>  	int ret;
>  
> @@ -47,40 +48,38 @@ static int rn5t568_reset_reason_detect(struct device *dev,
>  		return 0;
>  	}
>  
> -	if (reg & RN5T568_PONHIS_ON_EXTINPON) {
> -		reset_source_set_device(dev, RESET_POR);
> -		return 0;
> -	} else if (reg & RN5T568_PONHIS_ON_PWRONPON) {
> -		reset_source_set_device(dev, RESET_POR);
> -		return 0;
> -	} else if (!(reg & RN5T568_PONHIS_ON_REPWRPON))
> +	if (reg & (RN5T568_PONHIS_ON_EXTINPON | RN5T568_PONHIS_ON_PWRONPON)) {
> +		type = RESET_POR;
> +	} else if (!(reg & RN5T568_PONHIS_ON_REPWRPON)) {
>  		return -EINVAL;
> +	} else {
> +		ret = regmap_read(regmap, RN5T568_POFFHIS, &reg);
> +		if (ret)
> +			return ret;
>  
> -	ret = regmap_read(regmap, RN5T568_POFFHIS, &reg);
> -	if (ret)
> -		return ret;
> +		dev_dbg(dev, "Power-off history: %x\n", reg);
>  
> -	dev_dbg(dev, "Power-off history: %x\n", reg);
> -
> -	if (reg & RN5T568_POFFHIS_PWRONPOFF)
> -		reset_source_set_device(dev, RESET_POR);
> -	else if (reg & RN5T568_POFFHIS_TSHUTPOFF)
> -		reset_source_set_device(dev, RESET_THERM);
> -	else if (reg & RN5T568_POFFHIS_VINDETPOFF)
> -		reset_source_set_device(dev, RESET_BROWNOUT);
> -	else if (reg & RN5T568_POFFHIS_IODETPOFF)
> -		reset_source_set_device(dev, RESET_UKWN);
> -	else if (reg & RN5T568_POFFHIS_CPUPOFF)
> -		reset_source_set_device(dev, RESET_RST);
> -	else if (reg & RN5T568_POFFHIS_WDGPOFF)
> -		reset_source_set_device(dev, RESET_WDG);
> -	else if (reg & RN5T568_POFFHIS_DCLIMPOFF)
> -		reset_source_set_device(dev, RESET_BROWNOUT);
> -	else if (reg & RN5T568_POFFHIS_N_OEPOFF)
> -		reset_source_set_device(dev, RESET_EXT);
> -	else
> -		return -EINVAL;
> +		if (reg & RN5T568_POFFHIS_PWRONPOFF)
> +			type = RESET_POR;
> +		else if (reg & RN5T568_POFFHIS_TSHUTPOFF)
> +			type = RESET_THERM;
> +		else if (reg & RN5T568_POFFHIS_VINDETPOFF)
> +			type = RESET_BROWNOUT;
> +		else if (reg & RN5T568_POFFHIS_IODETPOFF)
> +			type = RESET_UKWN;
> +		else if (reg & RN5T568_POFFHIS_CPUPOFF)
> +			type = RESET_RST;
> +		else if (reg & RN5T568_POFFHIS_WDGPOFF)
> +			type = RESET_WDG;
> +		else if (reg & RN5T568_POFFHIS_DCLIMPOFF)
> +			type = RESET_BROWNOUT;
> +		else if (reg & RN5T568_POFFHIS_N_OEPOFF)
> +			type = RESET_EXT;
> +		else
> +			return -EINVAL;
> +	}
>  
> +	reset_source_set_device(dev, type);
>  	return 0;
>  }
>  
> -- 
> 2.39.5
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |