u-boot.lists.denx.de archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/6] Improved sysreset/watchdog uclass integration
@ 2021-10-29  3:16 Samuel Holland
  2021-10-29  3:16 ` [PATCH v2 1/6] sysreset: Add uclass Kconfig dependency to drivers Samuel Holland
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: Samuel Holland @ 2021-10-29  3:16 UTC (permalink / raw)
  To: Stefan Roese, u-boot, Jagan Teki, Andre Przywara
  Cc: Bin Meng, Sean Anderson, Heinrich Schuchardt, Simon Glass,
	Samuel Holland

This series hooks up the watchdog uclass to automatically register
watchdog devices for use with sysreset, doing a bit of minor cleanup
along the way.

The goal is for this to replace the sunxi board-level non-DM reset_cpu()
function. I was surprised to find that the wdt_reboot driver requires
its own undocumented device tree node, which references the watchdog
device by phandle. This is problematic for us, because sunxi-u-boot.dtsi
file covers 20 different SoCs with varying watchdog node phandle names.
So it would have required adding a -u-boot.dtsi file for each board.

Hooking things up automatically makes sense to me; this is what Linux
does. However, I put the code behind a new option to avoid surprises for
other platforms.

Changes in v2:
 - Extend the "if SYSRESET" block to the end of the file.
 - Also make gpio_reboot_probe function static.
 - Rebase on top of 492ee6b8d0e7 (now handle all watchdogs).
 - Added patches 5-6 as an example of how the new option will be used.

Samuel Holland (6):
  sysreset: Add uclass Kconfig dependency to drivers
  sysreset: Mark driver probe functions as static
  sysreset: watchdog: Move watchdog reference to plat data
  watchdog: Automatically register device with sysreset
  sunxi: Avoid duplicate reset_cpu with SYSRESET enabled
  sunxi: Use sysreset framework for poweroff/reset

 arch/arm/Kconfig                     |  3 +++
 arch/arm/mach-sunxi/board.c          |  2 ++
 drivers/sysreset/Kconfig             | 11 ++++++--
 drivers/sysreset/sysreset_gpio.c     |  2 +-
 drivers/sysreset/sysreset_resetctl.c |  2 +-
 drivers/sysreset/sysreset_syscon.c   |  2 +-
 drivers/sysreset/sysreset_watchdog.c | 40 ++++++++++++++++++++++------
 drivers/watchdog/wdt-uclass.c        |  5 ++++
 include/sysreset.h                   | 14 ++++++++++
 9 files changed, 68 insertions(+), 13 deletions(-)

-- 
2.32.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/6] sysreset: Add uclass Kconfig dependency to drivers
  2021-10-29  3:16 [PATCH v2 0/6] Improved sysreset/watchdog uclass integration Samuel Holland
@ 2021-10-29  3:16 ` Samuel Holland
  2021-11-03 13:11   ` Heinrich Schuchardt
  2021-10-29  3:16 ` [PATCH v2 2/6] sysreset: Mark driver probe functions as static Samuel Holland
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Samuel Holland @ 2021-10-29  3:16 UTC (permalink / raw)
  To: Stefan Roese, u-boot, Jagan Teki, Andre Przywara
  Cc: Bin Meng, Sean Anderson, Heinrich Schuchardt, Simon Glass,
	Samuel Holland

None of the sysreset drivers do anything beyond providing sysreset
uclass ops. They should depend on the sysreset uclass.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

Changes in v2:
 - Extend the "if SYSRESET" block to the end of the file.

 drivers/sysreset/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
index 43a948cfcd..de75c9cccc 100644
--- a/drivers/sysreset/Kconfig
+++ b/drivers/sysreset/Kconfig
@@ -118,8 +118,6 @@ config SYSRESET_TI_SCI
 	  This enables the system reset driver support over TI System Control
 	  Interface available on some new TI's SoCs.
 
-endif
-
 config SYSRESET_SYSCON
 	bool "Enable support for mfd syscon reboot driver"
 	select REGMAP
@@ -162,4 +160,6 @@ config SYSRESET_MPC83XX
 	help
 	  Reboot support for NXP MPC83xx SoCs.
 
+endif
+
 endmenu
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/6] sysreset: Mark driver probe functions as static
  2021-10-29  3:16 [PATCH v2 0/6] Improved sysreset/watchdog uclass integration Samuel Holland
  2021-10-29  3:16 ` [PATCH v2 1/6] sysreset: Add uclass Kconfig dependency to drivers Samuel Holland
@ 2021-10-29  3:16 ` Samuel Holland
  2021-11-03 14:19   ` Heinrich Schuchardt
  2021-10-29  3:16 ` [PATCH v2 3/6] sysreset: watchdog: Move watchdog reference to plat data Samuel Holland
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Samuel Holland @ 2021-10-29  3:16 UTC (permalink / raw)
  To: Stefan Roese, u-boot, Jagan Teki, Andre Przywara
  Cc: Bin Meng, Sean Anderson, Heinrich Schuchardt, Simon Glass,
	Samuel Holland

These driver probe functions are not (and should not be) called from
outside the respective driver source files. Therefore, the functions
should be marked static.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

Changes in v2:
 - Also make gpio_reboot_probe function static.

 drivers/sysreset/sysreset_gpio.c     | 2 +-
 drivers/sysreset/sysreset_resetctl.c | 2 +-
 drivers/sysreset/sysreset_syscon.c   | 2 +-
 drivers/sysreset/sysreset_watchdog.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/sysreset/sysreset_gpio.c b/drivers/sysreset/sysreset_gpio.c
index 680b759eb3..dfca10ccc8 100644
--- a/drivers/sysreset/sysreset_gpio.c
+++ b/drivers/sysreset/sysreset_gpio.c
@@ -33,7 +33,7 @@ static struct sysreset_ops gpio_reboot_ops = {
 	.request = gpio_reboot_request,
 };
 
-int gpio_reboot_probe(struct udevice *dev)
+static int gpio_reboot_probe(struct udevice *dev)
 {
 	struct gpio_reboot_priv *priv = dev_get_priv(dev);
 
diff --git a/drivers/sysreset/sysreset_resetctl.c b/drivers/sysreset/sysreset_resetctl.c
index c039521eb4..25bd5c9a7f 100644
--- a/drivers/sysreset/sysreset_resetctl.c
+++ b/drivers/sysreset/sysreset_resetctl.c
@@ -26,7 +26,7 @@ static struct sysreset_ops resetctl_reboot_ops = {
 	.request = resetctl_reboot_request,
 };
 
-int resetctl_reboot_probe(struct udevice *dev)
+static int resetctl_reboot_probe(struct udevice *dev)
 {
 	struct resetctl_reboot_priv *priv = dev_get_priv(dev);
 
diff --git a/drivers/sysreset/sysreset_syscon.c b/drivers/sysreset/sysreset_syscon.c
index 28fdfb0978..525faf2f89 100644
--- a/drivers/sysreset/sysreset_syscon.c
+++ b/drivers/sysreset/sysreset_syscon.c
@@ -39,7 +39,7 @@ static struct sysreset_ops syscon_reboot_ops = {
 	.request = syscon_reboot_request,
 };
 
-int syscon_reboot_probe(struct udevice *dev)
+static int syscon_reboot_probe(struct udevice *dev)
 {
 	struct syscon_reboot_priv *priv = dev_get_priv(dev);
 	int err;
diff --git a/drivers/sysreset/sysreset_watchdog.c b/drivers/sysreset/sysreset_watchdog.c
index 0dc2d8b9b6..c7ae368d41 100644
--- a/drivers/sysreset/sysreset_watchdog.c
+++ b/drivers/sysreset/sysreset_watchdog.c
@@ -29,7 +29,7 @@ static struct sysreset_ops wdt_reboot_ops = {
 	.request = wdt_reboot_request,
 };
 
-int wdt_reboot_probe(struct udevice *dev)
+static int wdt_reboot_probe(struct udevice *dev)
 {
 	struct wdt_reboot_priv *priv = dev_get_priv(dev);
 	int err;
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 3/6] sysreset: watchdog: Move watchdog reference to plat data
  2021-10-29  3:16 [PATCH v2 0/6] Improved sysreset/watchdog uclass integration Samuel Holland
  2021-10-29  3:16 ` [PATCH v2 1/6] sysreset: Add uclass Kconfig dependency to drivers Samuel Holland
  2021-10-29  3:16 ` [PATCH v2 2/6] sysreset: Mark driver probe functions as static Samuel Holland
@ 2021-10-29  3:16 ` Samuel Holland
  2021-10-29  3:16 ` [PATCH v2 4/6] watchdog: Automatically register device with sysreset Samuel Holland
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Samuel Holland @ 2021-10-29  3:16 UTC (permalink / raw)
  To: Stefan Roese, u-boot, Jagan Teki, Andre Przywara
  Cc: Bin Meng, Sean Anderson, Heinrich Schuchardt, Simon Glass,
	Samuel Holland

Currently, the wdt_reboot driver always gets its watchdog device
reference from an OF node. This prevents selecting a watchdog at
runtime. Move the watchdog device reference to the plat data, so
the driver can be bound with the reference pre-provided. The
reference will still be acquired from the OF node if it is not
already provided.

Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Signed-off-by: Samuel Holland <samuel@sholland.org>
---

(no changes since v1)

 drivers/sysreset/sysreset_watchdog.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/sysreset/sysreset_watchdog.c b/drivers/sysreset/sysreset_watchdog.c
index c7ae368d41..b723f5647c 100644
--- a/drivers/sysreset/sysreset_watchdog.c
+++ b/drivers/sysreset/sysreset_watchdog.c
@@ -9,16 +9,16 @@
 #include <sysreset.h>
 #include <wdt.h>
 
-struct wdt_reboot_priv {
+struct wdt_reboot_plat {
 	struct udevice *wdt;
 };
 
 static int wdt_reboot_request(struct udevice *dev, enum sysreset_t type)
 {
-	struct wdt_reboot_priv *priv = dev_get_priv(dev);
+	struct wdt_reboot_plat *plat = dev_get_plat(dev);
 	int ret;
 
-	ret = wdt_expire_now(priv->wdt, 0);
+	ret = wdt_expire_now(plat->wdt, 0);
 	if (ret)
 		return ret;
 
@@ -29,13 +29,13 @@ static struct sysreset_ops wdt_reboot_ops = {
 	.request = wdt_reboot_request,
 };
 
-static int wdt_reboot_probe(struct udevice *dev)
+static int wdt_reboot_of_to_plat(struct udevice *dev)
 {
-	struct wdt_reboot_priv *priv = dev_get_priv(dev);
+	struct wdt_reboot_plat *plat = dev_get_plat(dev);
 	int err;
 
 	err = uclass_get_device_by_phandle(UCLASS_WDT, dev,
-					   "wdt", &priv->wdt);
+					   "wdt", &plat->wdt);
 	if (err) {
 		pr_err("unable to find wdt device\n");
 		return err;
@@ -53,7 +53,7 @@ U_BOOT_DRIVER(wdt_reboot) = {
 	.name = "wdt_reboot",
 	.id = UCLASS_SYSRESET,
 	.of_match = wdt_reboot_ids,
+	.of_to_plat	= wdt_reboot_of_to_plat,
+	.plat_auto	= sizeof(struct wdt_reboot_plat),
 	.ops = &wdt_reboot_ops,
-	.priv_auto	= sizeof(struct wdt_reboot_priv),
-	.probe = wdt_reboot_probe,
 };
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 4/6] watchdog: Automatically register device with sysreset
  2021-10-29  3:16 [PATCH v2 0/6] Improved sysreset/watchdog uclass integration Samuel Holland
                   ` (2 preceding siblings ...)
  2021-10-29  3:16 ` [PATCH v2 3/6] sysreset: watchdog: Move watchdog reference to plat data Samuel Holland
@ 2021-10-29  3:16 ` Samuel Holland
  2021-11-03 13:52   ` Heinrich Schuchardt
  2021-10-29  3:16 ` [PATCH v2 5/6] sunxi: Avoid duplicate reset_cpu with SYSRESET enabled Samuel Holland
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Samuel Holland @ 2021-10-29  3:16 UTC (permalink / raw)
  To: Stefan Roese, u-boot, Jagan Teki, Andre Przywara
  Cc: Bin Meng, Sean Anderson, Heinrich Schuchardt, Simon Glass,
	Samuel Holland

Add an option to automatically register watchdog devices with the
wdt_reboot driver for use with sysreset. This allows sysreset to be a
drop-in replacement for platform-specific watchdog reset code, without
needing any device tree changes.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

Changes in v2:
 - Rebase on top of 492ee6b8d0e7 (now handle all watchdogs).

 drivers/sysreset/Kconfig             |  7 +++++++
 drivers/sysreset/sysreset_watchdog.c | 24 ++++++++++++++++++++++++
 drivers/watchdog/wdt-uclass.c        |  5 +++++
 include/sysreset.h                   | 14 ++++++++++++++
 4 files changed, 50 insertions(+)

diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
index de75c9cccc..f6d60038b8 100644
--- a/drivers/sysreset/Kconfig
+++ b/drivers/sysreset/Kconfig
@@ -131,6 +131,13 @@ config SYSRESET_WATCHDOG
 	help
 	  Reboot support for generic watchdog reset.
 
+config SYSRESET_WATCHDOG_AUTO
+	bool "Automatically register first watchdog with sysreset"
+	depends on SYSRESET_WATCHDOG
+	help
+	  If enabled, the first watchdog (as selected by the watchdog uclass)
+	  will automatically be registered with the watchdog reboot driver.
+
 config SYSRESET_RESETCTL
 	bool "Enable support for reset controller reboot driver"
 	select DM_RESET
diff --git a/drivers/sysreset/sysreset_watchdog.c b/drivers/sysreset/sysreset_watchdog.c
index b723f5647c..35efcac59d 100644
--- a/drivers/sysreset/sysreset_watchdog.c
+++ b/drivers/sysreset/sysreset_watchdog.c
@@ -5,7 +5,9 @@
 
 #include <common.h>
 #include <dm.h>
+#include <dm/device-internal.h>
 #include <errno.h>
+#include <malloc.h>
 #include <sysreset.h>
 #include <wdt.h>
 
@@ -57,3 +59,25 @@ U_BOOT_DRIVER(wdt_reboot) = {
 	.plat_auto	= sizeof(struct wdt_reboot_plat),
 	.ops = &wdt_reboot_ops,
 };
+
+#if IS_ENABLED(CONFIG_SYSRESET_WATCHDOG_AUTO)
+int sysreset_register_wdt(struct udevice *dev)
+{
+	struct wdt_reboot_plat *plat = malloc(sizeof(*plat));
+	int ret;
+
+	if (!plat)
+		return -ENOMEM;
+
+	plat->wdt = dev;
+
+	ret = device_bind(dev, DM_DRIVER_GET(wdt_reboot),
+			  dev->name, plat, ofnode_null(), NULL);
+	if (ret) {
+		free(plat);
+		return ret;
+	}
+
+	return 0;
+}
+#endif
diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index 7570710c4d..985f114084 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -10,6 +10,7 @@
 #include <errno.h>
 #include <hang.h>
 #include <log.h>
+#include <sysreset.h>
 #include <time.h>
 #include <wdt.h>
 #include <asm/global_data.h>
@@ -44,6 +45,10 @@ static void init_watchdog_dev(struct udevice *dev)
 
 	priv = dev_get_uclass_priv(dev);
 
+	ret = sysreset_register_wdt(dev);
+	if (ret)
+		printf("WDT:   Failed to register sysreset\n");
+
 	if (!IS_ENABLED(CONFIG_WATCHDOG_AUTOSTART)) {
 		printf("WDT:   Not starting %s\n", dev->name);
 		return;
diff --git a/include/sysreset.h b/include/sysreset.h
index 9d4ed87cea..d0e63e6d2b 100644
--- a/include/sysreset.h
+++ b/include/sysreset.h
@@ -133,4 +133,18 @@ void sysreset_walk_halt(enum sysreset_t type);
  */
 void reset_cpu(void);
 
+/**
+ * sysreset_register_wdt() - register a watchdog for use with sysreset
+ *
+ * This registers the given watchdog timer to be used to reset the system.
+ *
+ * @dev:	WDT device
+ * @return:	0 if OK, -errno if error
+ */
+#if IS_ENABLED(CONFIG_SYSRESET_WATCHDOG_AUTO)
+int sysreset_register_wdt(struct udevice *dev);
+#else
+static inline int sysreset_register_wdt(struct udevice *dev) { return 0; }
+#endif
+
 #endif
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 5/6] sunxi: Avoid duplicate reset_cpu with SYSRESET enabled
  2021-10-29  3:16 [PATCH v2 0/6] Improved sysreset/watchdog uclass integration Samuel Holland
                   ` (3 preceding siblings ...)
  2021-10-29  3:16 ` [PATCH v2 4/6] watchdog: Automatically register device with sysreset Samuel Holland
@ 2021-10-29  3:16 ` Samuel Holland
  2021-10-29  3:16 ` [PATCH v2 6/6] sunxi: Use sysreset framework for poweroff/reset Samuel Holland
  2021-11-02 10:21 ` [PATCH v2 0/6] Improved sysreset/watchdog uclass integration Stefan Roese
  6 siblings, 0 replies; 11+ messages in thread
From: Samuel Holland @ 2021-10-29  3:16 UTC (permalink / raw)
  To: Stefan Roese, u-boot, Jagan Teki, Andre Przywara
  Cc: Bin Meng, Sean Anderson, Heinrich Schuchardt, Simon Glass,
	Samuel Holland

The sysreset uclass unconditionally provides a definition of the
reset_cpu() function. So does the sunxi board code. Fix the build with
SYSRESET enabled by omitting the function from the board code in that
case. The code still needs to be kept around for use in SPL.

Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Signed-off-by: Samuel Holland <samuel@sholland.org>
---

Changes in v2:
 - New patch

 arch/arm/mach-sunxi/board.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
index b4ba2a72c4..3ef179742c 100644
--- a/arch/arm/mach-sunxi/board.c
+++ b/arch/arm/mach-sunxi/board.c
@@ -346,6 +346,7 @@ void board_init_f(ulong dummy)
 }
 #endif
 
+#if !CONFIG_IS_ENABLED(SYSRESET)
 void reset_cpu(void)
 {
 #if defined(CONFIG_SUNXI_GEN_SUN4I) || defined(CONFIG_MACH_SUN8I_R40)
@@ -376,6 +377,7 @@ void reset_cpu(void)
 	while (1) { }
 #endif
 }
+#endif
 
 #if !CONFIG_IS_ENABLED(SYS_DCACHE_OFF) && !defined(CONFIG_ARM64)
 void enable_caches(void)
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 6/6] sunxi: Use sysreset framework for poweroff/reset
  2021-10-29  3:16 [PATCH v2 0/6] Improved sysreset/watchdog uclass integration Samuel Holland
                   ` (4 preceding siblings ...)
  2021-10-29  3:16 ` [PATCH v2 5/6] sunxi: Avoid duplicate reset_cpu with SYSRESET enabled Samuel Holland
@ 2021-10-29  3:16 ` Samuel Holland
  2021-11-02 10:21 ` [PATCH v2 0/6] Improved sysreset/watchdog uclass integration Stefan Roese
  6 siblings, 0 replies; 11+ messages in thread
From: Samuel Holland @ 2021-10-29  3:16 UTC (permalink / raw)
  To: Stefan Roese, u-boot, Jagan Teki, Andre Przywara
  Cc: Bin Meng, Sean Anderson, Heinrich Schuchardt, Simon Glass,
	Samuel Holland

Instead of hardcoding the watchdog for reset, and the PMIC for poweroff,
use the sysreset framework to manage the available poweroff/reset
backends. This allows (as examples) using the PMIC to do a cold reset,
and using a GPIO to power off H3/H5 boards lacking a PMIC. Furthermore,
it removes the need to hardcode watchdog MMIO addresses, since the
sysreset backends can be discovered using the device tree.

Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Signed-off-by: Samuel Holland <samuel@sholland.org>
---

Changes in v2:
 - New patch

 arch/arm/Kconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 02f8306f15..88a435796f 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1065,6 +1065,9 @@ config ARCH_SUNXI
 	imply SPL_MMC if MMC
 	imply SPL_POWER
 	imply SPL_SERIAL
+	imply SYSRESET
+	imply SYSRESET_WATCHDOG
+	imply SYSRESET_WATCHDOG_AUTO
 	imply USB_GADGET
 	imply WDT
 
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 0/6] Improved sysreset/watchdog uclass integration
  2021-10-29  3:16 [PATCH v2 0/6] Improved sysreset/watchdog uclass integration Samuel Holland
                   ` (5 preceding siblings ...)
  2021-10-29  3:16 ` [PATCH v2 6/6] sunxi: Use sysreset framework for poweroff/reset Samuel Holland
@ 2021-11-02 10:21 ` Stefan Roese
  6 siblings, 0 replies; 11+ messages in thread
From: Stefan Roese @ 2021-11-02 10:21 UTC (permalink / raw)
  To: Samuel Holland, u-boot, Jagan Teki, Andre Przywara
  Cc: Bin Meng, Sean Anderson, Heinrich Schuchardt, Simon Glass

Hi Samuel,

On 29.10.21 05:16, Samuel Holland wrote:
> This series hooks up the watchdog uclass to automatically register
> watchdog devices for use with sysreset, doing a bit of minor cleanup
> along the way.
> 
> The goal is for this to replace the sunxi board-level non-DM reset_cpu()
> function. I was surprised to find that the wdt_reboot driver requires
> its own undocumented device tree node, which references the watchdog
> device by phandle. This is problematic for us, because sunxi-u-boot.dtsi
> file covers 20 different SoCs with varying watchdog node phandle names.
> So it would have required adding a -u-boot.dtsi file for each board.
> 
> Hooking things up automatically makes sense to me; this is what Linux
> does. However, I put the code behind a new option to avoid surprises for
> other platforms.
> 
> Changes in v2:
>   - Extend the "if SYSRESET" block to the end of the file.
>   - Also make gpio_reboot_probe function static.
>   - Rebase on top of 492ee6b8d0e7 (now handle all watchdogs).
>   - Added patches 5-6 as an example of how the new option will be used.
> 
> Samuel Holland (6):
>    sysreset: Add uclass Kconfig dependency to drivers
>    sysreset: Mark driver probe functions as static
>    sysreset: watchdog: Move watchdog reference to plat data
>    watchdog: Automatically register device with sysreset
>    sunxi: Avoid duplicate reset_cpu with SYSRESET enabled
>    sunxi: Use sysreset framework for poweroff/reset
> 
>   arch/arm/Kconfig                     |  3 +++
>   arch/arm/mach-sunxi/board.c          |  2 ++
>   drivers/sysreset/Kconfig             | 11 ++++++--
>   drivers/sysreset/sysreset_gpio.c     |  2 +-
>   drivers/sysreset/sysreset_resetctl.c |  2 +-
>   drivers/sysreset/sysreset_syscon.c   |  2 +-
>   drivers/sysreset/sysreset_watchdog.c | 40 ++++++++++++++++++++++------
>   drivers/watchdog/wdt-uclass.c        |  5 ++++
>   include/sysreset.h                   | 14 ++++++++++
>   9 files changed, 68 insertions(+), 13 deletions(-)

This patchset (applied to current master) generates multiple issues when
building "world" via CI (Azure in my case). Here one of those problems:

$ make sandbox_config
$ make -s -j20
In file included from ./arch/sandbox/include/asm/state.h:10,
                  from include/test/test.h:130,
                  from include/test/env.h:10,
                  from test/env/cmd_ut_env.c:9:
include/sysreset.h: In function ‘arch_reset_for_test’:
include/sysreset.h:147:19: error: invalid storage class for function 
‘sysreset_register_wdt’
   147 | static inline int sysreset_register_wdt(struct udevice *dev) { 
return 0; }
       |                   ^~~~~~~~~~~~~~~~~~~~~
make[1]: *** [scripts/Makefile.build:253: test/env/cmd_ut_env.o] Error 1
make: *** [Makefile:1804: test/env] Error 2
make: *** Waiting for unfinished jobs....
In file included from ./arch/sandbox/include/asm/state.h:10,
                  from include/test/test.h:130,
                  from include/test/lib.h:9,
                  from test/cmd/test_echo.c:12:
include/sysreset.h: In function ‘arch_reset_for_test’:
include/sysreset.h:147:19: error: invalid storage class for function 
‘sysreset_register_wdt’
   147 | static inline int sysreset_register_wdt(struct udevice *dev) { 
return 0; }
       |                   ^~~~~~~~~~~~~~~~~~~~~
make[2]: *** [scripts/Makefile.build:253: test/cmd/test_echo.o] Error 1
make[1]: *** [scripts/Makefile.build:394: test/cmd] Error 2
make: *** [Makefile:1804: test] Error 2

Could you please take a look here?

Thanks,
Stefan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/6] sysreset: Add uclass Kconfig dependency to drivers
  2021-10-29  3:16 ` [PATCH v2 1/6] sysreset: Add uclass Kconfig dependency to drivers Samuel Holland
@ 2021-11-03 13:11   ` Heinrich Schuchardt
  0 siblings, 0 replies; 11+ messages in thread
From: Heinrich Schuchardt @ 2021-11-03 13:11 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Bin Meng, Sean Anderson, Simon Glass, Andre Przywara, u-boot,
	Jagan Teki, Stefan Roese

On 10/29/21 05:16, Samuel Holland wrote:
> None of the sysreset drivers do anything beyond providing sysreset
> uclass ops. They should depend on the sysreset uclass.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>

Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

> ---
> 
> Changes in v2:
>   - Extend the "if SYSRESET" block to the end of the file.
> 
>   drivers/sysreset/Kconfig | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
> index 43a948cfcd..de75c9cccc 100644
> --- a/drivers/sysreset/Kconfig
> +++ b/drivers/sysreset/Kconfig
> @@ -118,8 +118,6 @@ config SYSRESET_TI_SCI
>   	  This enables the system reset driver support over TI System Control
>   	  Interface available on some new TI's SoCs.
>   
> -endif
> -
>   config SYSRESET_SYSCON
>   	bool "Enable support for mfd syscon reboot driver"
>   	select REGMAP
> @@ -162,4 +160,6 @@ config SYSRESET_MPC83XX
>   	help
>   	  Reboot support for NXP MPC83xx SoCs.
>   
> +endif
> +
>   endmenu
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 4/6] watchdog: Automatically register device with sysreset
  2021-10-29  3:16 ` [PATCH v2 4/6] watchdog: Automatically register device with sysreset Samuel Holland
@ 2021-11-03 13:52   ` Heinrich Schuchardt
  0 siblings, 0 replies; 11+ messages in thread
From: Heinrich Schuchardt @ 2021-11-03 13:52 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Bin Meng, Sean Anderson, Simon Glass, Andre Przywara, Jagan Teki,
	u-boot, Stefan Roese

On 10/29/21 05:16, Samuel Holland wrote:
> Add an option to automatically register watchdog devices with the
> wdt_reboot driver for use with sysreset. This allows sysreset to be a
> drop-in replacement for platform-specific watchdog reset code, without
> needing any device tree changes.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
> 
> Changes in v2:
>   - Rebase on top of 492ee6b8d0e7 (now handle all watchdogs).
> 
>   drivers/sysreset/Kconfig             |  7 +++++++
>   drivers/sysreset/sysreset_watchdog.c | 24 ++++++++++++++++++++++++
>   drivers/watchdog/wdt-uclass.c        |  5 +++++
>   include/sysreset.h                   | 14 ++++++++++++++
>   4 files changed, 50 insertions(+)
> 
> diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
> index de75c9cccc..f6d60038b8 100644
> --- a/drivers/sysreset/Kconfig
> +++ b/drivers/sysreset/Kconfig
> @@ -131,6 +131,13 @@ config SYSRESET_WATCHDOG
>   	help
>   	  Reboot support for generic watchdog reset.
>   
> +config SYSRESET_WATCHDOG_AUTO
> +	bool "Automatically register first watchdog with sysreset"
> +	depends on SYSRESET_WATCHDOG
> +	help
> +	  If enabled, the first watchdog (as selected by the watchdog uclass)
> +	  will automatically be registered with the watchdog reboot driver.
> +
>   config SYSRESET_RESETCTL
>   	bool "Enable support for reset controller reboot driver"
>   	select DM_RESET
> diff --git a/drivers/sysreset/sysreset_watchdog.c b/drivers/sysreset/sysreset_watchdog.c
> index b723f5647c..35efcac59d 100644
> --- a/drivers/sysreset/sysreset_watchdog.c
> +++ b/drivers/sysreset/sysreset_watchdog.c
> @@ -5,7 +5,9 @@
>   
>   #include <common.h>
>   #include <dm.h>
> +#include <dm/device-internal.h>
>   #include <errno.h>
> +#include <malloc.h>
>   #include <sysreset.h>
>   #include <wdt.h>
>   
> @@ -57,3 +59,25 @@ U_BOOT_DRIVER(wdt_reboot) = {
>   	.plat_auto	= sizeof(struct wdt_reboot_plat),
>   	.ops = &wdt_reboot_ops,
>   };
> +
> +#if IS_ENABLED(CONFIG_SYSRESET_WATCHDOG_AUTO)
> +int sysreset_register_wdt(struct udevice *dev)
> +{
> +	struct wdt_reboot_plat *plat = malloc(sizeof(*plat));
> +	int ret;
> +
> +	if (!plat)
> +		return -ENOMEM;
> +
> +	plat->wdt = dev;
> +
> +	ret = device_bind(dev, DM_DRIVER_GET(wdt_reboot),
> +			  dev->name, plat, ofnode_null(), NULL);
> +	if (ret) {
> +		free(plat);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +#endif
> diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
> index 7570710c4d..985f114084 100644
> --- a/drivers/watchdog/wdt-uclass.c
> +++ b/drivers/watchdog/wdt-uclass.c
> @@ -10,6 +10,7 @@
>   #include <errno.h>
>   #include <hang.h>
>   #include <log.h>
> +#include <sysreset.h>
>   #include <time.h>
>   #include <wdt.h>
>   #include <asm/global_data.h>
> @@ -44,6 +45,10 @@ static void init_watchdog_dev(struct udevice *dev)
>   
>   	priv = dev_get_uclass_priv(dev);
>   
> +	ret = sysreset_register_wdt(dev);
> +	if (ret)
> +		printf("WDT:   Failed to register sysreset\n");
> +
>   	if (!IS_ENABLED(CONFIG_WATCHDOG_AUTOSTART)) {
>   		printf("WDT:   Not starting %s\n", dev->name);
>   		return;
> diff --git a/include/sysreset.h b/include/sysreset.h
> index 9d4ed87cea..d0e63e6d2b 100644
> --- a/include/sysreset.h
> +++ b/include/sysreset.h
> @@ -133,4 +133,18 @@ void sysreset_walk_halt(enum sysreset_t type);
>    */
>   void reset_cpu(void);
>   
> +/**
> + * sysreset_register_wdt() - register a watchdog for use with sysreset
> + *
> + * This registers the given watchdog timer to be used to reset the system.
> + *
> + * @dev:	WDT device
> + * @return:	0 if OK, -errno if error
> + */
> +#if IS_ENABLED(CONFIG_SYSRESET_WATCHDOG_AUTO)
> +int sysreset_register_wdt(struct udevice *dev);
> +#else
> +static inline int sysreset_register_wdt(struct udevice *dev) { return 0; }
> +#endif
> +
>   #endif
> 

Running Gitlab CI for the complete series
https://source.denx.de/u-boot/custodians/u-boot-efi/-/jobs/346194#L106

    sandbox:  +   sandbox_spl
+In file included from arch/sandbox/include/asm/state.h:10,
+                 from include/test/test.h:130,
+                 from include/test/lib.h:9,
+                 from test/cmd/test_echo.c:12:
+include/sysreset.h: In function ‘arch_reset_for_test’:
+include/sysreset.h:147:19: error: invalid storage class for function 
‘sysreset_register_wdt’
+  147 | static inline int sysreset_register_wdt(struct udevice *dev) { 
return 0; }
+      |                   ^~~~~~~~~~~~~~~~~~~~~
+                 from include/test/common.h:10,
+                 from test/common/cmd_ut_common.c:11:

Please, remove the static inline version of sysreset_register_wdt() and 
use an if clause in drivers/watchdog/wdt-uclass.c instead:

+       if IS_ENABLED(CONFIG_SYSRESET_WATCHDOG_AUTO) {
+               ret = sysreset_register_wdt(dev);
+               if (ret)
+                       printf("WDT:   Failed to register sysreset\n");
+       }

Best regards

Heinrich

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/6] sysreset: Mark driver probe functions as static
  2021-10-29  3:16 ` [PATCH v2 2/6] sysreset: Mark driver probe functions as static Samuel Holland
@ 2021-11-03 14:19   ` Heinrich Schuchardt
  0 siblings, 0 replies; 11+ messages in thread
From: Heinrich Schuchardt @ 2021-11-03 14:19 UTC (permalink / raw)
  To: Samuel Holland, Stefan Roese, u-boot, Jagan Teki, Andre Przywara
  Cc: Bin Meng, Sean Anderson, Simon Glass

On 10/29/21 05:16, Samuel Holland wrote:
> These driver probe functions are not (and should not be) called from
> outside the respective driver source files. Therefore, the functions
> should be marked static.
> 
> Signed-off-by: Samuel Holland<samuel@sholland.org>

Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

> ---
> 
> Changes in v2:
>   - Also make gpio_reboot_probe function static.


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-11-03 14:20 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-29  3:16 [PATCH v2 0/6] Improved sysreset/watchdog uclass integration Samuel Holland
2021-10-29  3:16 ` [PATCH v2 1/6] sysreset: Add uclass Kconfig dependency to drivers Samuel Holland
2021-11-03 13:11   ` Heinrich Schuchardt
2021-10-29  3:16 ` [PATCH v2 2/6] sysreset: Mark driver probe functions as static Samuel Holland
2021-11-03 14:19   ` Heinrich Schuchardt
2021-10-29  3:16 ` [PATCH v2 3/6] sysreset: watchdog: Move watchdog reference to plat data Samuel Holland
2021-10-29  3:16 ` [PATCH v2 4/6] watchdog: Automatically register device with sysreset Samuel Holland
2021-11-03 13:52   ` Heinrich Schuchardt
2021-10-29  3:16 ` [PATCH v2 5/6] sunxi: Avoid duplicate reset_cpu with SYSRESET enabled Samuel Holland
2021-10-29  3:16 ` [PATCH v2 6/6] sunxi: Use sysreset framework for poweroff/reset Samuel Holland
2021-11-02 10:21 ` [PATCH v2 0/6] Improved sysreset/watchdog uclass integration Stefan Roese

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).