[PATCH 1/4] reset_source: introduce reset_source_xlate to translate specific type

[PATCH 1/4] reset_source: introduce reset_source_xlate to translate specific type

[Ahmad Fatoum]

We have a few places where driver code prints reset_source_name() to
console after setting the reset source.

This working is probe order dependent, because reset_source_name()
prints the highest priority reset_source so far, which doesn't
necessarily have to be the one that was just computed.

Implement reset_source_xlate, so drivers can be migrated to use it.

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

diff --git a/common/reset_source.c b/common/reset_source.c
index 8fdf05215753..e8da8da19aee 100644
--- a/common/reset_source.c
+++ b/common/reset_source.c
@@ -42,11 +42,11 @@ enum reset_src_type reset_source_get(void)
 }
 EXPORT_SYMBOL(reset_source_get);
 
-const char *reset_source_name(void)
+const char *reset_source_xlate(enum reset_src_type st)
 {
-	return reset_src_names[reset_source];
+	return reset_src_names[st];
 }
-EXPORT_SYMBOL(reset_source_name);
+EXPORT_SYMBOL(reset_source_xlate);
 
 int reset_source_get_instance(void)
 {
diff --git a/include/reset_source.h b/include/reset_source.h
index 305dde0102d5..914b9ef38458 100644
--- a/include/reset_source.h
+++ b/include/reset_source.h
@@ -28,7 +28,7 @@ enum reset_src_type {
 #ifdef CONFIG_RESET_SOURCE
 
 enum reset_src_type reset_source_get(void);
-const char *reset_source_name(void);
+const char *reset_source_xlate(enum reset_src_type st);
 int reset_source_get_instance(void);
 struct device_d *reset_source_get_device(void);
 
@@ -45,7 +45,7 @@ static inline enum reset_src_type reset_source_get(void)
 	return RESET_UKWN;
 }
 
-static inline const char *reset_source_name(void)
+static inline const char *reset_source_xlate(enum reset_src_type st)
 {
 	return "unknown";
 }
@@ -93,4 +93,9 @@ static inline void reset_source_set(enum reset_src_type type)
 	reset_source_set_priority(type, RESET_SOURCE_DEFAULT_PRIORITY);
 }
 
+static inline const char *reset_source_name(void)
+{
+	return reset_source_xlate(reset_source_get());
+}
+
 #endif /* __INCLUDE_RESET_SOURCE_H */
-- 
2.25.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/4] watchdog: f71808e: only print reset reason if one's indicated

[Ahmad Fatoum]

On most resets, a "reset reason: unknown" would clutter the console,
because the watchdog doesn't support differentiating between POR and
RST. Reduce the clutter by only printing the message when we have
something interesting to say (i.e. watchdog reset).

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/watchdog/f71808e_wdt.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
index 5307ab0b3ead..6f2d30ec77aa 100644
--- a/drivers/watchdog/f71808e_wdt.c
+++ b/drivers/watchdog/f71808e_wdt.c
@@ -256,11 +256,11 @@ static int f71808e_wdt_init(struct f71808e_wdt *wd, struct device_d *dev)
 	wdd->set_timeout	= &f71808e_wdt_set_timeout;
 	wdd->timeout_max	= WATCHDOG_MAX_TIMEOUT;
 
-	if (wdt_conf & BIT(F71808FG_FLAG_WDTMOUT_STS))
-		reset_source_set_priority(RESET_WDG,
-					  RESET_SOURCE_DEFAULT_PRIORITY);
+	if (wdt_conf & BIT(F71808FG_FLAG_WDTMOUT_STS)) {
+		reset_source_set_priority(RESET_WDG, RESET_SOURCE_DEFAULT_PRIORITY);
+		dev_info(dev, "reset reason: WDT\n");
+	}
 
-	dev_info(dev, "reset reason: %s\n", reset_source_name());
 
 	if (test_bit(F71808FG_FLAG_WD_EN, &wdt_conf))
 		wdd->running = WDOG_HW_RUNNING;
-- 
2.25.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/4] reset_source: migrate from reset_source_name to reset_source_xlate

[Ahmad Fatoum]

Whether reset_source_name() returns the just set reset_source is
dependent on probe order and the priorities of prior reset sources
in relation to the current one. Make this more robust by using the new
reset_source_xlate.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/mach-imx/imx.c        | 2 +-
 drivers/watchdog/stm32_iwdg.c  | 2 +-
 drivers/watchdog/stpmic1_wdt.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-imx/imx.c b/arch/arm/mach-imx/imx.c
index 0942d5069562..9f91cab781fb 100644
--- a/arch/arm/mach-imx/imx.c
+++ b/arch/arm/mach-imx/imx.c
@@ -202,5 +202,5 @@ void imx_set_reset_reason(void __iomem *srsr,
 	reset_source_set_prinst(type, RESET_SOURCE_DEFAULT_PRIORITY, instance);
 
 	pr_info("i.MX reset reason %s (SRSR: 0x%08x)\n",
-		reset_source_name(), reg);
+		reset_source_xlate(type), reg);
 }
diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c
index c7a5cb9caaf4..8a28bfd8a06a 100644
--- a/drivers/watchdog/stm32_iwdg.c
+++ b/drivers/watchdog/stm32_iwdg.c
@@ -185,7 +185,7 @@ static int stm32_set_reset_reason(struct regmap *rcc)
 	reset_source_set_prinst(type, RESET_SOURCE_DEFAULT_PRIORITY, instance);
 
 	pr_info("STM32 RCC reset reason %s (MP_RSTSR: 0x%08x)\n",
-		reset_source_name(), reg);
+		reset_source_xlate(type), reg);
 
 	return 0;
 }
diff --git a/drivers/watchdog/stpmic1_wdt.c b/drivers/watchdog/stpmic1_wdt.c
index 40273ffc4c85..9d7f9361a159 100644
--- a/drivers/watchdog/stpmic1_wdt.c
+++ b/drivers/watchdog/stpmic1_wdt.c
@@ -152,7 +152,7 @@ static int stpmic1_set_reset_reason(struct regmap *map)
 	reset_source_set_prinst(type, 400, instance);
 
 	pr_info("STPMIC1 reset reason %s (RREQ_STATE_SR: 0x%08x)\n",
-		reset_source_name(), reg);
+		reset_source_xlate(type), reg);
 
 	return 0;
 }
-- 
2.25.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 4/4] reset: stm32: migrate restart reason and handler from stm32_iwdg

[Ahmad Fatoum]

The watchdog driver will remain unprobed if the driver or the OF node
is disabled, but the reset reason is useful even then.

System reset and reset source determination is achieved with the RCC
peripheral for which we have a reset controller driver.
Move the code over there, so reset reason and reset are available
always.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/reset/reset-stm32.c   | 117 ++++++++++++++++++++++++++++++++--
 drivers/watchdog/stm32_iwdg.c |  99 ----------------------------
 2 files changed, 113 insertions(+), 103 deletions(-)

diff --git a/drivers/reset/reset-stm32.c b/drivers/reset/reset-stm32.c
index 5689dfd4888a..00c4ac74c773 100644
--- a/drivers/reset/reset-stm32.c
+++ b/drivers/reset/reset-stm32.c
@@ -9,14 +9,48 @@
 #include <init.h>
 #include <linux/err.h>
 #include <linux/reset-controller.h>
+#include <restart.h>
+#include <reset_source.h>
 #include <asm/io.h>
 
 #define RCC_CL 0x4
 
+#define RCC_MP_GRSTCSETR		0x404
+#define RCC_MP_RSTSCLRR			0x408
+
+#define STM32MP_RCC_RSTF_POR		BIT(0)
+#define STM32MP_RCC_RSTF_BOR		BIT(1)
+#define STM32MP_RCC_RSTF_PAD		BIT(2)
+#define STM32MP_RCC_RSTF_HCSS		BIT(3)
+#define STM32MP_RCC_RSTF_VCORE		BIT(4)
+
+#define STM32MP_RCC_RSTF_MPSYS		BIT(6)
+#define STM32MP_RCC_RSTF_MCSYS		BIT(7)
+#define STM32MP_RCC_RSTF_IWDG1		BIT(8)
+#define STM32MP_RCC_RSTF_IWDG2		BIT(9)
+
+#define STM32MP_RCC_RSTF_STDBY		BIT(11)
+#define STM32MP_RCC_RSTF_CSTDBY		BIT(12)
+#define STM32MP_RCC_RSTF_MPUP0		BIT(13)
+#define STM32MP_RCC_RSTF_MPUP1		BIT(14)
+
+struct stm32_reset_reason {
+	uint32_t mask;
+	enum reset_src_type type;
+	int instance;
+};
+
 struct stm32_reset {
 	void __iomem *base;
 	struct reset_controller_dev rcdev;
+	struct restart_handler restart;
+	const struct stm32_reset_ops *ops;
+};
+
+struct stm32_reset_ops {
 	void (*reset)(void __iomem *reg, unsigned offset, bool assert);
+	void __noreturn (*sys_reset)(struct restart_handler *rst);
+	const struct stm32_reset_reason *reset_reasons;
 };
 
 static struct stm32_reset *to_stm32_reset(struct reset_controller_dev *rcdev)
@@ -40,12 +74,40 @@ static void stm32mcu_reset(void __iomem *reg, unsigned offset, bool assert)
 		clrbits_le32(reg, BIT(offset));
 }
 
+static u32 stm32_reset_status(struct stm32_reset *priv, unsigned long bank)
+{
+	return readl(priv->base + bank);
+}
+
 static void stm32_reset(struct stm32_reset *priv, unsigned long id, bool assert)
 {
 	int bank = (id / BITS_PER_LONG) * 4;
 	int offset = id % BITS_PER_LONG;
 
-	priv->reset(priv->base + bank, offset, assert);
+	priv->ops->reset(priv->base + bank, offset, assert);
+}
+
+static void stm32_set_reset_reason(struct stm32_reset *priv,
+				   const struct stm32_reset_reason *reasons)
+{
+	enum reset_src_type type = RESET_UKWN;
+	u32 reg;
+	int i, instance = 0;
+
+	reg = stm32_reset_status(priv, RCC_MP_RSTSCLRR);
+
+	for (i = 0; reasons[i].mask; i++) {
+		if (reg & reasons[i].mask) {
+			type     = reasons[i].type;
+			instance = reasons[i].instance;
+			break;
+		}
+	}
+
+	reset_source_set_prinst(type, RESET_SOURCE_DEFAULT_PRIORITY, instance);
+
+	pr_info("STM32 RCC reset reason %s (MP_RSTSR: 0x%08x)\n",
+		reset_source_xlate(type), reg);
 }
 
 static int stm32_reset_assert(struct reset_controller_dev *rcdev,
@@ -74,7 +136,7 @@ static int stm32_reset_probe(struct device_d *dev)
 	int ret;
 
 	priv = xzalloc(sizeof(*priv));
-	ret = dev_get_drvdata(dev, (const void **)&priv->reset);
+	ret = dev_get_drvdata(dev, (const void **)&priv->ops);
 	if (ret)
 		return ret;
 
@@ -87,12 +149,59 @@ static int stm32_reset_probe(struct device_d *dev)
 	priv->rcdev.ops = &stm32_reset_ops;
 	priv->rcdev.of_node = dev->device_node;
 
+	if (priv->ops->sys_reset) {
+		priv->restart.name = "stm32-rcc";
+		priv->restart.restart = priv->ops->sys_reset;
+		priv->restart.priority = 200;
+
+		ret = restart_handler_register(&priv->restart);
+		if (ret)
+			dev_warn(dev, "Cannot register restart handler\n");
+	}
+
+	if (priv->ops->reset_reasons)
+		stm32_set_reset_reason(priv, priv->ops->reset_reasons);
+
 	return reset_controller_register(&priv->rcdev);
 }
 
+static void __noreturn stm32mp_rcc_restart_handler(struct restart_handler *rst)
+{
+	struct stm32_reset *priv = container_of(rst, struct stm32_reset, restart);
+
+	stm32_reset(priv, RCC_MP_GRSTCSETR * BITS_PER_BYTE, true);
+
+	mdelay(1000);
+	hang();
+}
+
+static const struct stm32_reset_reason stm32mp_reset_reasons[] = {
+	{ STM32MP_RCC_RSTF_POR,		RESET_POR, 0 },
+	{ STM32MP_RCC_RSTF_BOR,		RESET_BROWNOUT, 0 },
+	{ STM32MP_RCC_RSTF_STDBY,	RESET_WKE, 0 },
+	{ STM32MP_RCC_RSTF_CSTDBY,	RESET_WKE, 1 },
+	{ STM32MP_RCC_RSTF_MPSYS,	RESET_RST, 2 },
+	{ STM32MP_RCC_RSTF_MPUP0,	RESET_RST, 0 },
+	{ STM32MP_RCC_RSTF_MPUP1,	RESET_RST, 1 },
+	{ STM32MP_RCC_RSTF_IWDG1,	RESET_WDG, 0 },
+	{ STM32MP_RCC_RSTF_IWDG2,	RESET_WDG, 1 },
+	{ STM32MP_RCC_RSTF_PAD,		RESET_EXT, 1 },
+	{ /* sentinel */ }
+};
+
+static const struct stm32_reset_ops stm32mp1_reset_ops = {
+	.reset = stm32mp_reset,
+	.sys_reset = stm32mp_rcc_restart_handler,
+	.reset_reasons = stm32mp_reset_reasons,
+};
+
+static const struct stm32_reset_ops stm32mcu_reset_ops = {
+	.reset = stm32mcu_reset,
+};
+
 static const struct of_device_id stm32_rcc_reset_dt_ids[] = {
-	{ .compatible = "st,stm32mp1-rcc", .data = stm32mp_reset },
-	{ .compatible = "st,stm32-rcc", .data = stm32mcu_reset },
+	{ .compatible = "st,stm32mp1-rcc", .data = &stm32mp1_reset_ops },
+	{ .compatible = "st,stm32-rcc", .data = &stm32mcu_reset_ops },
 	{ /* sentinel */ },
 };
 
diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c
index 8a28bfd8a06a..9e38f1a669f2 100644
--- a/drivers/watchdog/stm32_iwdg.c
+++ b/drivers/watchdog/stm32_iwdg.c
@@ -6,14 +6,11 @@
 #include <common.h>
 #include <init.h>
 #include <watchdog.h>
-#include <restart.h>
 #include <asm/io.h>
 #include <of_device.h>
 #include <linux/log2.h>
 #include <linux/iopoll.h>
 #include <linux/clk.h>
-#include <mfd/syscon.h>
-#include <reset_source.h>
 
 /* IWDG registers */
 #define IWDG_KR		0x00	/* Key register */
@@ -36,40 +33,12 @@
 #define SR_PVU	BIT(0) /* Watchdog prescaler value update */
 #define SR_RVU	BIT(1) /* Watchdog counter reload value update */
 
-#define RCC_MP_GRSTCSETR		0x404
-#define RCC_MP_RSTSCLRR			0x408
-#define RCC_MP_GRSTCSETR_MPSYSRST	BIT(0)
-
-#define STM32MP_RCC_RSTF_POR		BIT(0)
-#define STM32MP_RCC_RSTF_BOR		BIT(1)
-#define STM32MP_RCC_RSTF_PAD		BIT(2)
-#define STM32MP_RCC_RSTF_HCSS		BIT(3)
-#define STM32MP_RCC_RSTF_VCORE		BIT(4)
-
-#define STM32MP_RCC_RSTF_MPSYS		BIT(6)
-#define STM32MP_RCC_RSTF_MCSYS		BIT(7)
-#define STM32MP_RCC_RSTF_IWDG1		BIT(8)
-#define STM32MP_RCC_RSTF_IWDG2		BIT(9)
-
-#define STM32MP_RCC_RSTF_STDBY		BIT(11)
-#define STM32MP_RCC_RSTF_CSTDBY		BIT(12)
-#define STM32MP_RCC_RSTF_MPUP0		BIT(13)
-#define STM32MP_RCC_RSTF_MPUP1		BIT(14)
-
 /* set timeout to 100 ms */
 #define TIMEOUT_US	100000
 
-struct stm32_reset_reason {
-	uint32_t mask;
-	enum reset_src_type type;
-	int instance;
-};
-
 struct stm32_iwdg {
 	struct watchdog wdd;
-	struct restart_handler restart;
 	void __iomem *iwdg_base;
-	struct regmap *rcc_regmap;
 	unsigned int timeout;
 	unsigned int rate;
 };
@@ -79,17 +48,6 @@ static inline struct stm32_iwdg *to_stm32_iwdg(struct watchdog *wdd)
 	return container_of(wdd, struct stm32_iwdg, wdd);
 }
 
-static void __noreturn stm32_iwdg_restart_handler(struct restart_handler *rst)
-{
-	struct stm32_iwdg *wd = container_of(rst, struct stm32_iwdg, restart);
-
-	regmap_update_bits(wd->rcc_regmap, RCC_MP_GRSTCSETR,
-			   RCC_MP_GRSTCSETR_MPSYSRST, RCC_MP_GRSTCSETR_MPSYSRST);
-
-	mdelay(1000);
-	hang();
-}
-
 static void stm32_iwdg_ping(struct stm32_iwdg *wd)
 {
 	writel(KR_KEY_RELOAD, wd->iwdg_base + IWDG_KR);
@@ -149,47 +107,6 @@ static int stm32_iwdg_set_timeout(struct watchdog *wdd, unsigned int timeout)
 	return 0;
 }
 
-static const struct stm32_reset_reason stm32_reset_reasons[] = {
-	{ STM32MP_RCC_RSTF_POR,		RESET_POR, 0 },
-	{ STM32MP_RCC_RSTF_BOR,		RESET_BROWNOUT, 0 },
-	{ STM32MP_RCC_RSTF_STDBY,	RESET_WKE, 0 },
-	{ STM32MP_RCC_RSTF_CSTDBY,	RESET_WKE, 1 },
-	{ STM32MP_RCC_RSTF_MPSYS,	RESET_RST, 2 },
-	{ STM32MP_RCC_RSTF_MPUP0,	RESET_RST, 0 },
-	{ STM32MP_RCC_RSTF_MPUP1,	RESET_RST, 1 },
-	{ STM32MP_RCC_RSTF_IWDG1,	RESET_WDG, 0 },
-	{ STM32MP_RCC_RSTF_IWDG2,	RESET_WDG, 1 },
-	{ STM32MP_RCC_RSTF_PAD,		RESET_EXT, 1 },
-	{ /* sentinel */ }
-};
-
-static int stm32_set_reset_reason(struct regmap *rcc)
-{
-	enum reset_src_type type = RESET_UKWN;
-	u32 reg;
-	int ret;
-	int i, instance = 0;
-
-	ret = regmap_read(rcc, RCC_MP_RSTSCLRR, &reg);
-	if (ret)
-		return ret;
-
-	for (i = 0; stm32_reset_reasons[i].mask; i++) {
-		if (reg & stm32_reset_reasons[i].mask) {
-			type     = stm32_reset_reasons[i].type;
-			instance = stm32_reset_reasons[i].instance;
-			break;
-		}
-	}
-
-	reset_source_set_prinst(type, RESET_SOURCE_DEFAULT_PRIORITY, instance);
-
-	pr_info("STM32 RCC reset reason %s (MP_RSTSR: 0x%08x)\n",
-		reset_source_xlate(type), reg);
-
-	return 0;
-}
-
 struct stm32_iwdg_data {
 	bool has_pclk;
 	u32 max_prescaler;
@@ -264,22 +181,6 @@ static int stm32_iwdg_probe(struct device_d *dev)
 		return ret;
 	}
 
-	wd->restart.name = "stm32-iwdg";
-	wd->restart.restart = stm32_iwdg_restart_handler;
-	wd->restart.priority = 200;
-
-	wd->rcc_regmap = syscon_regmap_lookup_by_compatible("st,stm32mp1-rcc");
-	if (IS_ERR(wd->rcc_regmap))
-		dev_warn(dev, "Cannot register restart handler\n");
-
-	ret = restart_handler_register(&wd->restart);
-	if (ret)
-		dev_warn(dev, "Cannot register restart handler\n");
-
-	ret = stm32_set_reset_reason(wd->rcc_regmap);
-	if (ret)
-		dev_warn(dev, "Cannot determine reset reason\n");
-
 	dev_info(dev, "probed\n");
 	return 0;
 }
-- 
2.25.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/4] reset_source: introduce reset_source_xlate to translate specific type

[Sascha Hauer]

On Mon, Feb 10, 2020 at 02:09:49PM +0100, Ahmad Fatoum wrote:
> We have a few places where driver code prints reset_source_name() to
> console after setting the reset source.
> 
> This working is probe order dependent, because reset_source_name()
> prints the highest priority reset_source so far, which doesn't
> necessarily have to be the one that was just computed.
> 
> Implement reset_source_xlate, so drivers can be migrated to use it.

I renamed reset_source_xlate to reset_source_to_string which IMO makes
it more clear what this function does.

Applied, thanks

Sascha

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

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox