[PATCH 1/3] common: restart: number unnamed restart handlers

mail archive of the barebox mailing list
 help / color / mirror / Atom feed

* [PATCH 1/3] common: restart: number unnamed restart handlers
@ 2020-06-02  7:57 Ahmad Fatoum
  2020-06-02  7:57 ` [PATCH 2/3] restart: give all restart handlers a descriptive name Ahmad Fatoum
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Ahmad Fatoum @ 2020-06-02  7:57 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

Follow-up commit allows referencing specific restart handler by name.
Restart handlers default to "default" as name when none is given.
Number them sequentially instead.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/restart.c  | 4 +++-
 include/restart.h | 1 -
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/common/restart.c b/common/restart.c
index b19ae54657c0..dd15c8d5c362 100644
--- a/common/restart.c
+++ b/common/restart.c
@@ -19,6 +19,7 @@
 #include <of.h>
 
 static LIST_HEAD(restart_handler_list);
+static unsigned resetidx;
 
 /**
  * restart_handler_register() - register a handler for restarting the system
@@ -31,7 +32,7 @@ static LIST_HEAD(restart_handler_list);
 int restart_handler_register(struct restart_handler *rst)
 {
 	if (!rst->name)
-		rst->name = RESTART_DEFAULT_NAME;
+		rst->name = basprintf("reset%u", resetidx);
 	if (!rst->priority)
 		rst->priority = RESTART_DEFAULT_PRIORITY;
 
@@ -40,6 +41,7 @@ int restart_handler_register(struct restart_handler *rst)
 	pr_debug("registering restart handler \"%s\" with priority %d\n",
 			rst->name, rst->priority);
 
+	resetidx++;
 	return 0;
 }
 
diff --git a/include/restart.h b/include/restart.h
index 7ec0910e9404..6880b03b936f 100644
--- a/include/restart.h
+++ b/include/restart.h
@@ -15,7 +15,6 @@ int restart_handler_register(struct restart_handler *);
 int restart_handler_register_fn(void (*restart_fn)(struct restart_handler *));
 
 #define RESTART_DEFAULT_PRIORITY 100
-#define RESTART_DEFAULT_NAME "default"
 
 unsigned int of_get_restart_priority(struct device_node *node);
 
-- 
2.27.0


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

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

* [PATCH 2/3] restart: give all restart handlers a descriptive name
  2020-06-02  7:57 [PATCH 1/3] common: restart: number unnamed restart handlers Ahmad Fatoum
@ 2020-06-02  7:57 ` Ahmad Fatoum
  2020-06-03  7:12   ` Sascha Hauer
  2020-09-15 12:05   ` [PATCH] fixup! " Ahmad Fatoum
  2020-06-02  7:57 ` [PATCH 3/3] commands: reset: allow specifying reset by name Ahmad Fatoum
  2020-06-03  7:11 ` [PATCH 1/3] common: restart: number unnamed restart handlers Sascha Hauer
  2 siblings, 2 replies; 10+ messages in thread
From: Ahmad Fatoum @ 2020-06-02  7:57 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

With incoming changes to choose a specific reset method, give all
currently unnamed "default" reset handlers a name:

  - soc     reset via SoC-specific means
  - soc-wdt reset via SoC watchdog timer
  - vector  reset via jump to reset vector
  - efi     reset via EFI firmware

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/mach-at91/at91rm9200_time.c           | 2 +-
 arch/arm/mach-at91/at91sam9260.c               | 2 +-
 arch/arm/mach-at91/at91sam9261.c               | 2 +-
 arch/arm/mach-at91/at91sam9263.c               | 2 +-
 arch/arm/mach-at91/at91sam9g45.c               | 2 +-
 arch/arm/mach-at91/at91sam9n12.c               | 2 +-
 arch/arm/mach-at91/at91sam9x5.c                | 2 +-
 arch/arm/mach-at91/sama5d3.c                   | 2 +-
 arch/arm/mach-at91/sama5d4.c                   | 2 +-
 arch/arm/mach-clps711x/reset.c                 | 2 +-
 arch/arm/mach-davinci/time.c                   | 2 +-
 arch/arm/mach-ep93xx/clocksource.c             | 2 +-
 arch/arm/mach-highbank/reset.c                 | 2 +-
 arch/arm/mach-mvebu/armada-370-xp.c            | 2 +-
 arch/arm/mach-mvebu/dove.c                     | 2 +-
 arch/arm/mach-mvebu/kirkwood.c                 | 2 +-
 arch/arm/mach-mxs/soc-imx23.c                  | 2 +-
 arch/arm/mach-mxs/soc-imx28.c                  | 2 +-
 arch/arm/mach-nomadik/reset.c                  | 2 +-
 arch/arm/mach-omap/am33xx_generic.c            | 2 +-
 arch/arm/mach-omap/omap3_generic.c             | 2 +-
 arch/arm/mach-omap/omap4_generic.c             | 2 +-
 arch/arm/mach-pxa/common.c                     | 2 +-
 arch/arm/mach-rockchip/rk3188.c                | 2 +-
 arch/arm/mach-rockchip/rk3288.c                | 2 +-
 arch/arm/mach-samsung/generic.c                | 2 +-
 arch/arm/mach-socfpga/arria10-generic.c        | 2 +-
 arch/arm/mach-socfpga/cyclone5-reset-manager.c | 2 +-
 arch/arm/mach-tegra/tegra20-pmc.c              | 2 +-
 arch/arm/mach-versatile/core.c                 | 2 +-
 arch/arm/mach-vexpress/reset.c                 | 2 +-
 arch/arm/mach-zynq/zynq.c                      | 2 +-
 arch/kvx/cpu/reset.c                           | 2 +-
 arch/mips/mach-ar231x/ar231x_reset.c           | 2 +-
 arch/mips/mach-ath79/reset.c                   | 2 +-
 arch/mips/mach-bcm47xx/reset.c                 | 2 +-
 arch/mips/mach-loongson/loongson1_reset.c      | 2 +-
 arch/mips/mach-malta/reset.c                   | 2 +-
 arch/nios2/cpu/cpu.c                           | 2 +-
 arch/openrisc/cpu/cpu.c                        | 2 +-
 arch/powerpc/mach-mpc5xxx/cpu.c                | 2 +-
 arch/powerpc/mach-mpc85xx/cpu.c                | 2 +-
 common/efi/efi.c                               | 2 +-
 common/restart.c                               | 5 ++++-
 include/restart.h                              | 3 ++-
 45 files changed, 49 insertions(+), 45 deletions(-)

diff --git a/arch/arm/mach-at91/at91rm9200_time.c b/arch/arm/mach-at91/at91rm9200_time.c
index 975cd956c986..f89983fe63c6 100644
--- a/arch/arm/mach-at91/at91rm9200_time.c
+++ b/arch/arm/mach-at91/at91rm9200_time.c
@@ -88,7 +88,7 @@ static void __noreturn at91rm9200_restart_soc(struct restart_handler *rst)
 
 static int restart_register_feature(void)
 {
-	restart_handler_register_fn(at91rm9200_restart_soc);
+	restart_handler_register_fn("soc-wdt", at91rm9200_restart_soc);
 
 	return 0;
 }
diff --git a/arch/arm/mach-at91/at91sam9260.c b/arch/arm/mach-at91/at91sam9260.c
index 56327a2c4710..fdd8ea014e9d 100644
--- a/arch/arm/mach-at91/at91sam9260.c
+++ b/arch/arm/mach-at91/at91sam9260.c
@@ -243,7 +243,7 @@ static void at91sam9260_initialize(void)
 	at91_add_pit(AT91SAM9260_BASE_PIT);
 	at91_add_sam9_smc(DEVICE_ID_SINGLE, AT91SAM9260_BASE_SMC, 0x200);
 
-	restart_handler_register_fn(at91sam9260_restart);
+	restart_handler_register_fn("soc", at91sam9260_restart);
 }
 
 static int at91sam9260_setup(void)
diff --git a/arch/arm/mach-at91/at91sam9261.c b/arch/arm/mach-at91/at91sam9261.c
index 4abc55635413..0465ed9524df 100644
--- a/arch/arm/mach-at91/at91sam9261.c
+++ b/arch/arm/mach-at91/at91sam9261.c
@@ -235,7 +235,7 @@ static void at91sam9261_initialize(void)
 	at91_add_pit(AT91SAM9261_BASE_PIT);
 	at91_add_sam9_smc(DEVICE_ID_SINGLE, AT91SAM9261_BASE_SMC, 0x200);
 
-	restart_handler_register_fn(at91sam9261_restart);
+	restart_handler_register_fn("soc", at91sam9261_restart);
 }
 
 static int at91sam9261_setup(void)
diff --git a/arch/arm/mach-at91/at91sam9263.c b/arch/arm/mach-at91/at91sam9263.c
index 690f8e06bb12..dc5dddfb646a 100644
--- a/arch/arm/mach-at91/at91sam9263.c
+++ b/arch/arm/mach-at91/at91sam9263.c
@@ -256,7 +256,7 @@ static void at91sam9263_initialize(void)
 	at91_add_sam9_smc(0, AT91SAM9263_BASE_SMC0, 0x200);
 	at91_add_sam9_smc(1, AT91SAM9263_BASE_SMC1, 0x200);
 
-	restart_handler_register_fn(at91sam9263_restart);
+	restart_handler_register_fn("soc", at91sam9263_restart);
 }
 
 static int at91sam9263_setup(void)
diff --git a/arch/arm/mach-at91/at91sam9g45.c b/arch/arm/mach-at91/at91sam9g45.c
index 569aa274fc14..affc624b1d36 100644
--- a/arch/arm/mach-at91/at91sam9g45.c
+++ b/arch/arm/mach-at91/at91sam9g45.c
@@ -270,7 +270,7 @@ static void at91sam9g45_initialize(void)
 	at91_add_pit(AT91SAM9G45_BASE_PIT);
 	at91_add_sam9_smc(DEVICE_ID_SINGLE, AT91SAM9G45_BASE_SMC, 0x200);
 
-	restart_handler_register_fn(at91sam9g45_restart);
+	restart_handler_register_fn("soc", at91sam9g45_restart);
 }
 
 static int at91sam9g45_setup(void)
diff --git a/arch/arm/mach-at91/at91sam9n12.c b/arch/arm/mach-at91/at91sam9n12.c
index 365bded56e40..850d34604a2c 100644
--- a/arch/arm/mach-at91/at91sam9n12.c
+++ b/arch/arm/mach-at91/at91sam9n12.c
@@ -226,7 +226,7 @@ static void at91sam9n12_initialize(void)
 	at91_add_pit(AT91SAM9N12_BASE_PIT);
 	at91_add_sam9_smc(DEVICE_ID_SINGLE, AT91SAM9N12_BASE_SMC, 0x200);
 
-	restart_handler_register_fn(at91sam9n12_restart);
+	restart_handler_register_fn("soc", at91sam9n12_restart);
 }
 
 static int at91sam9n12_setup(void)
diff --git a/arch/arm/mach-at91/at91sam9x5.c b/arch/arm/mach-at91/at91sam9x5.c
index 40ba9ed56e8a..086e27a79f51 100644
--- a/arch/arm/mach-at91/at91sam9x5.c
+++ b/arch/arm/mach-at91/at91sam9x5.c
@@ -13,7 +13,7 @@ static void at91sam9x5_restart(struct restart_handler *rst)
 
 static int at91sam9x5_initialize(void)
 {
-	restart_handler_register_fn(at91sam9x5_restart);
+	restart_handler_register_fn("soc", at91sam9x5_restart);
 
 	return 0;
 }
diff --git a/arch/arm/mach-at91/sama5d3.c b/arch/arm/mach-at91/sama5d3.c
index a5d464eca05f..b1e7b2c565fc 100644
--- a/arch/arm/mach-at91/sama5d3.c
+++ b/arch/arm/mach-at91/sama5d3.c
@@ -397,7 +397,7 @@ static void sama5d3_initialize(void)
 	at91_add_pit(SAMA5D3_BASE_PIT);
 	at91_add_sam9_smc(DEVICE_ID_SINGLE, SAMA5D3_BASE_HSMC + 0x600, 0xa0);
 
-	restart_handler_register_fn(sama5d3_restart);
+	restart_handler_register_fn("soc", sama5d3_restart);
 }
 
 static int sama5d3_setup(void)
diff --git a/arch/arm/mach-at91/sama5d4.c b/arch/arm/mach-at91/sama5d4.c
index ca09dfe425d5..62e466fe5168 100644
--- a/arch/arm/mach-at91/sama5d4.c
+++ b/arch/arm/mach-at91/sama5d4.c
@@ -305,7 +305,7 @@ static void sama5d4_initialize(void)
 	at91_add_pit(SAMA5D4_BASE_PIT);
 	at91_add_sam9_smc(DEVICE_ID_SINGLE, SAMA5D4_BASE_HSMC + 0x600, 0xa0);
 
-	restart_handler_register_fn(sama5d4_restart);
+	restart_handler_register_fn("soc", sama5d4_restart);
 }
 
 static int sama5d4_setup(void)
diff --git a/arch/arm/mach-clps711x/reset.c b/arch/arm/mach-clps711x/reset.c
index 03f40b73fabf..90ddb8f5d2f5 100644
--- a/arch/arm/mach-clps711x/reset.c
+++ b/arch/arm/mach-clps711x/reset.c
@@ -22,7 +22,7 @@ static void __noreturn clps711x_restart_soc(struct restart_handler *rst)
 
 static int restart_register_feature(void)
 {
-	restart_handler_register_fn(clps711x_restart_soc);
+	restart_handler_register_fn("vector", clps711x_restart_soc);
 
 	return 0;
 }
diff --git a/arch/arm/mach-davinci/time.c b/arch/arm/mach-davinci/time.c
index 4d1b570aa0d4..5b57fe61929b 100644
--- a/arch/arm/mach-davinci/time.c
+++ b/arch/arm/mach-davinci/time.c
@@ -210,7 +210,7 @@ static void __noreturn davinci_restart_soc(struct restart_handler *rst)
 
 static int restart_register_feature(void)
 {
-	restart_handler_register_fn(davinci_restart_soc);
+	restart_handler_register_fn("soc-wdt", davinci_restart_soc);
 
 	return 0;
 }
diff --git a/arch/arm/mach-ep93xx/clocksource.c b/arch/arm/mach-ep93xx/clocksource.c
index 4fdcc36b1c81..1f3ff7f8f20a 100644
--- a/arch/arm/mach-ep93xx/clocksource.c
+++ b/arch/arm/mach-ep93xx/clocksource.c
@@ -85,7 +85,7 @@ static void __noreturn ep92xx_restart_soc(struct restart_handler *rst)
 
 static int restart_register_feature(void)
 {
-	restart_handler_register_fn(ep92xx_restart_soc);
+	restart_handler_register_fn("soc", ep92xx_restart_soc);
 
 	return 0;
 }
diff --git a/arch/arm/mach-highbank/reset.c b/arch/arm/mach-highbank/reset.c
index d73a0a76a5fe..ea3908ec2b59 100644
--- a/arch/arm/mach-highbank/reset.c
+++ b/arch/arm/mach-highbank/reset.c
@@ -33,7 +33,7 @@ static void __noreturn highbank_poweroff(struct poweroff_handler *handler)
 
 static int highbank_init(void)
 {
-	restart_handler_register_fn(highbank_restart_soc);
+	restart_handler_register_fn("soc", highbank_restart_soc);
 	poweroff_handler_register_fn(highbank_poweroff);
 
 	return 0;
diff --git a/arch/arm/mach-mvebu/armada-370-xp.c b/arch/arm/mach-mvebu/armada-370-xp.c
index 2589f4fe72c3..9a35c51985eb 100644
--- a/arch/arm/mach-mvebu/armada-370-xp.c
+++ b/arch/arm/mach-mvebu/armada-370-xp.c
@@ -132,7 +132,7 @@ static int armada_370_xp_init_soc(void)
 	if (!of_machine_is_compatible("marvell,armada-370-xp"))
 		return 0;
 
-	restart_handler_register_fn(armada_370_xp_restart_soc);
+	restart_handler_register_fn("soc", armada_370_xp_restart_soc);
 
 	barebox_set_model("Marvell Armada 370/XP");
 	barebox_set_hostname("armada");
diff --git a/arch/arm/mach-mvebu/dove.c b/arch/arm/mach-mvebu/dove.c
index 37fde63f18de..3c6302dd2d12 100644
--- a/arch/arm/mach-mvebu/dove.c
+++ b/arch/arm/mach-mvebu/dove.c
@@ -36,7 +36,7 @@ static int dove_init_soc(void)
 	if (!of_machine_is_compatible("marvell,dove"))
 		return 0;
 
-	restart_handler_register_fn(dove_restart_soc);
+	restart_handler_register_fn("soc", dove_restart_soc);
 
 	barebox_set_model("Marvell Dove");
 	barebox_set_hostname("dove");
diff --git a/arch/arm/mach-mvebu/kirkwood.c b/arch/arm/mach-mvebu/kirkwood.c
index 59fb95ff4adf..e50d7501c82d 100644
--- a/arch/arm/mach-mvebu/kirkwood.c
+++ b/arch/arm/mach-mvebu/kirkwood.c
@@ -34,7 +34,7 @@ static int kirkwood_init_soc(void)
 	if (!of_machine_is_compatible("marvell,kirkwood"))
 		return 0;
 
-	restart_handler_register_fn(kirkwood_restart_soc);
+	restart_handler_register_fn("soc", kirkwood_restart_soc);
 
 	barebox_set_model("Marvell Kirkwood");
 	barebox_set_hostname("kirkwood");
diff --git a/arch/arm/mach-mxs/soc-imx23.c b/arch/arm/mach-mxs/soc-imx23.c
index f25fff18c310..8c47c766cc2b 100644
--- a/arch/arm/mach-mxs/soc-imx23.c
+++ b/arch/arm/mach-mxs/soc-imx23.c
@@ -49,7 +49,7 @@ static int imx23_devices_init(void)
 	add_generic_device("imx23-gpio", 0, NULL, IMX_IOMUXC_BASE, 0x2000, IORESOURCE_MEM, NULL);
 	add_generic_device("imx23-gpio", 1, NULL, IMX_IOMUXC_BASE, 0x2000, IORESOURCE_MEM, NULL);
 	add_generic_device("imx23-gpio", 2, NULL, IMX_IOMUXC_BASE, 0x2000, IORESOURCE_MEM, NULL);
-	restart_handler_register_fn(imx23_restart_soc);
+	restart_handler_register_fn("soc", imx23_restart_soc);
 
 	return 0;
 }
diff --git a/arch/arm/mach-mxs/soc-imx28.c b/arch/arm/mach-mxs/soc-imx28.c
index 49f870b5bf2a..a214e2b7a6d9 100644
--- a/arch/arm/mach-mxs/soc-imx28.c
+++ b/arch/arm/mach-mxs/soc-imx28.c
@@ -51,7 +51,7 @@ static int imx28_init(void)
 		HW_CLKCTRL_WDOG_POR_DISABLE;
 	writel(reg, IMX_CCM_BASE + HW_CLKCTRL_RESET);
 
-	restart_handler_register_fn(imx28_restart_soc);
+	restart_handler_register_fn("soc", imx28_restart_soc);
 
 	arm_add_mem_device("ram0", IMX_MEMORY_BASE, imx28_get_memsize());
 
diff --git a/arch/arm/mach-nomadik/reset.c b/arch/arm/mach-nomadik/reset.c
index 8bdaada8a1a5..d5266068e293 100644
--- a/arch/arm/mach-nomadik/reset.c
+++ b/arch/arm/mach-nomadik/reset.c
@@ -35,7 +35,7 @@ static void __noreturn nomadik_restart_soc(struct restart_handler *rst)
 
 static int restart_register_feature(void)
 {
-	restart_handler_register_fn(nomadik_restart_soc);
+	restart_handler_register_fn("soc", nomadik_restart_soc);
 
 	return 0;
 }
diff --git a/arch/arm/mach-omap/am33xx_generic.c b/arch/arm/mach-omap/am33xx_generic.c
index 7577df761cb7..3c5cdf065c84 100644
--- a/arch/arm/mach-omap/am33xx_generic.c
+++ b/arch/arm/mach-omap/am33xx_generic.c
@@ -244,7 +244,7 @@ int am33xx_init(void)
 {
 	omap_gpmc_base = (void *)AM33XX_GPMC_BASE;
 
-	restart_handler_register_fn(am33xx_restart_soc);
+	restart_handler_register_fn("soc", am33xx_restart_soc);
 
 	am33xx_enable_per_clocks();
 
diff --git a/arch/arm/mach-omap/omap3_generic.c b/arch/arm/mach-omap/omap3_generic.c
index cff4a4fb119f..3f6a3462779d 100644
--- a/arch/arm/mach-omap/omap3_generic.c
+++ b/arch/arm/mach-omap/omap3_generic.c
@@ -540,7 +540,7 @@ int omap3_init(void)
 {
 	omap_gpmc_base = (void *)OMAP3_GPMC_BASE;
 
-	restart_handler_register_fn(omap3_restart_soc);
+	restart_handler_register_fn("soc", omap3_restart_soc);
 
 	if (IS_ENABLED(CONFIG_RESET_SOURCE))
 		omap3_detect_reset_reason();
diff --git a/arch/arm/mach-omap/omap4_generic.c b/arch/arm/mach-omap/omap4_generic.c
index 1f711538487c..848a664064f5 100644
--- a/arch/arm/mach-omap/omap4_generic.c
+++ b/arch/arm/mach-omap/omap4_generic.c
@@ -535,7 +535,7 @@ int omap4_init(void)
 {
 	omap_gpmc_base = (void *)OMAP44XX_GPMC_BASE;
 
-	restart_handler_register_fn(omap4_restart_soc);
+	restart_handler_register_fn("soc", omap4_restart_soc);
 
 	return omap4_bootsource();
 }
diff --git a/arch/arm/mach-pxa/common.c b/arch/arm/mach-pxa/common.c
index 106ca3020eff..5b980cb81bab 100644
--- a/arch/arm/mach-pxa/common.c
+++ b/arch/arm/mach-pxa/common.c
@@ -41,7 +41,7 @@ static void __noreturn pxa_restart_soc(struct restart_handler *rst)
 
 static int restart_register_feature(void)
 {
-	restart_handler_register_fn(pxa_restart_soc);
+	restart_handler_register_fn("soc-wdt", pxa_restart_soc);
 
 	return 0;
 }
diff --git a/arch/arm/mach-rockchip/rk3188.c b/arch/arm/mach-rockchip/rk3188.c
index e7cbf364573c..572e9dc58f96 100644
--- a/arch/arm/mach-rockchip/rk3188.c
+++ b/arch/arm/mach-rockchip/rk3188.c
@@ -29,7 +29,7 @@ static void __noreturn rockchip_restart_soc(struct restart_handler *rst)
 
 static int restart_register_feature(void)
 {
-	restart_handler_register_fn(rockchip_restart_soc);
+	restart_handler_register_fn("soc", rockchip_restart_soc);
 
 	return 0;
 }
diff --git a/arch/arm/mach-rockchip/rk3288.c b/arch/arm/mach-rockchip/rk3288.c
index 4e8fb4a123ac..9076fd9227cc 100644
--- a/arch/arm/mach-rockchip/rk3288.c
+++ b/arch/arm/mach-rockchip/rk3288.c
@@ -60,7 +60,7 @@ static void rk3288_detect_reset_reason(void)
 
 static int rk3288_init(void)
 {
-	restart_handler_register_fn(rockchip_restart_soc);
+	restart_handler_register_fn("soc", rockchip_restart_soc);
 
 	if (IS_ENABLED(CONFIG_RESET_SOURCE))
 		rk3288_detect_reset_reason();
diff --git a/arch/arm/mach-samsung/generic.c b/arch/arm/mach-samsung/generic.c
index de38d47e21cb..ed3d30d9954c 100644
--- a/arch/arm/mach-samsung/generic.c
+++ b/arch/arm/mach-samsung/generic.c
@@ -44,7 +44,7 @@ static void __noreturn samsung_restart_soc(struct restart_handler *rst)
 
 static int restart_register_feature(void)
 {
-	restart_handler_register_fn(samsung_restart_soc);
+	restart_handler_register_fn("soc-wdt", samsung_restart_soc);
 
 	return 0;
 }
diff --git a/arch/arm/mach-socfpga/arria10-generic.c b/arch/arm/mach-socfpga/arria10-generic.c
index 53ec278739cc..38558309f860 100644
--- a/arch/arm/mach-socfpga/arria10-generic.c
+++ b/arch/arm/mach-socfpga/arria10-generic.c
@@ -70,7 +70,7 @@ static int arria10_generic_init(void)
 	arria10_init_emac();
 
 	pr_debug("Register restart handler\n");
-	restart_handler_register_fn(arria10_restart_soc);
+	restart_handler_register_fn("soc", arria10_restart_soc);
 
 	return 0;
 }
diff --git a/arch/arm/mach-socfpga/cyclone5-reset-manager.c b/arch/arm/mach-socfpga/cyclone5-reset-manager.c
index 86358068461d..4ee90b1bb0c7 100644
--- a/arch/arm/mach-socfpga/cyclone5-reset-manager.c
+++ b/arch/arm/mach-socfpga/cyclone5-reset-manager.c
@@ -37,7 +37,7 @@ static void __noreturn socfpga_restart_soc(struct restart_handler *rst)
 
 static int restart_register_feature(void)
 {
-	restart_handler_register_fn(socfpga_restart_soc);
+	restart_handler_register_fn("soc", socfpga_restart_soc);
 
 	return 0;
 }
diff --git a/arch/arm/mach-tegra/tegra20-pmc.c b/arch/arm/mach-tegra/tegra20-pmc.c
index f7c7ac918f98..a252c995ea02 100644
--- a/arch/arm/mach-tegra/tegra20-pmc.c
+++ b/arch/arm/mach-tegra/tegra20-pmc.c
@@ -246,7 +246,7 @@ static struct driver_d tegra20_pmc_driver = {
 
 static int tegra20_pmc_init(void)
 {
-	restart_handler_register_fn(tegra20_restart_soc);
+	restart_handler_register_fn("soc", tegra20_restart_soc);
 	return platform_driver_register(&tegra20_pmc_driver);
 }
 coredevice_initcall(tegra20_pmc_init);
diff --git a/arch/arm/mach-versatile/core.c b/arch/arm/mach-versatile/core.c
index 7c6e9523a24a..001d6712545f 100644
--- a/arch/arm/mach-versatile/core.c
+++ b/arch/arm/mach-versatile/core.c
@@ -205,7 +205,7 @@ static int versatile_init(void)
 	amba_apb_device_add(NULL, "pl061_gpio", 1, 0x101e5000, 4096, NULL, 0);
 	amba_apb_device_add(NULL, "pl061_gpio", 2, 0x101e6000, 4096, NULL, 0);
 	amba_apb_device_add(NULL, "pl061_gpio", 3, 0x101e7000, 4096, NULL, 0);
-	restart_handler_register_fn(versatile_reset_soc);
+	restart_handler_register_fn("soc", versatile_reset_soc);
 	return 0;
 }
 coredevice_initcall(versatile_init);
diff --git a/arch/arm/mach-vexpress/reset.c b/arch/arm/mach-vexpress/reset.c
index 3164ae3079c6..78e452936d2c 100644
--- a/arch/arm/mach-vexpress/reset.c
+++ b/arch/arm/mach-vexpress/reset.c
@@ -24,7 +24,7 @@ static void vexpress_reset_soc(struct restart_handler *rst)
 
 static int restart_register_feature(void)
 {
-	restart_handler_register_fn(vexpress_reset_soc);
+	restart_handler_register_fn("soc-wdt", vexpress_reset_soc);
 
 	return 0;
 }
diff --git a/arch/arm/mach-zynq/zynq.c b/arch/arm/mach-zynq/zynq.c
index 79a6b908e000..806aeb913055 100644
--- a/arch/arm/mach-zynq/zynq.c
+++ b/arch/arm/mach-zynq/zynq.c
@@ -69,7 +69,7 @@ static int zynq_init(void)
 	writel(val, 0xf8f00000);
 	dmb();
 
-	restart_handler_register_fn(zynq_restart_soc);
+	restart_handler_register_fn("soc", zynq_restart_soc);
 
 	bootsource_set(zynq_bootsource_get());
 
diff --git a/arch/kvx/cpu/reset.c b/arch/kvx/cpu/reset.c
index c7f2018e0033..df36764cb66b 100644
--- a/arch/kvx/cpu/reset.c
+++ b/arch/kvx/cpu/reset.c
@@ -60,7 +60,7 @@ static int kvx_reset_init(void)
 		break;
 	}
 
-	restart_handler_register_fn(kvx_restart_soc);
+	restart_handler_register_fn("soc", kvx_restart_soc);
 
 	return 0;
 }
diff --git a/arch/mips/mach-ar231x/ar231x_reset.c b/arch/mips/mach-ar231x/ar231x_reset.c
index f88167ba4cba..91414edd269f 100644
--- a/arch/mips/mach-ar231x/ar231x_reset.c
+++ b/arch/mips/mach-ar231x/ar231x_reset.c
@@ -68,7 +68,7 @@ static struct driver_d ar231x_reset_driver = {
 
 static int ar231x_reset_init(void)
 {
-	restart_handler_register_fn(ar2312x_restart_soc);
+	restart_handler_register_fn("soc-wdt", ar2312x_restart_soc);
 	return platform_driver_register(&ar231x_reset_driver);
 }
 coredevice_initcall(ar231x_reset_init);
diff --git a/arch/mips/mach-ath79/reset.c b/arch/mips/mach-ath79/reset.c
index b756c859d8d0..393ca00b0829 100644
--- a/arch/mips/mach-ath79/reset.c
+++ b/arch/mips/mach-ath79/reset.c
@@ -22,7 +22,7 @@ static void __noreturn ath79_restart_soc(struct restart_handler *rst)
 
 static int restart_register_feature(void)
 {
-	restart_handler_register_fn(ath79_restart_soc);
+	restart_handler_register_fn("soc", ath79_restart_soc);
 
 	return 0;
 }
diff --git a/arch/mips/mach-bcm47xx/reset.c b/arch/mips/mach-bcm47xx/reset.c
index 33dfb7b3b53f..3ab9b0ce4b53 100644
--- a/arch/mips/mach-bcm47xx/reset.c
+++ b/arch/mips/mach-bcm47xx/reset.c
@@ -19,7 +19,7 @@ static void __noreturn bcm47xx_restart_soc(struct restart_handler *rst)
 
 static int restart_register_feature(void)
 {
-	restart_handler_register_fn(bcm47xx_restart_soc);
+	restart_handler_register_fn("soc", bcm47xx_restart_soc);
 
 	return 0;
 }
diff --git a/arch/mips/mach-loongson/loongson1_reset.c b/arch/mips/mach-loongson/loongson1_reset.c
index a6c05905deac..85752f4ab825 100644
--- a/arch/mips/mach-loongson/loongson1_reset.c
+++ b/arch/mips/mach-loongson/loongson1_reset.c
@@ -20,7 +20,7 @@ static void __noreturn longhorn_restart_soc(struct restart_handler *rst)
 
 static int restart_register_feature(void)
 {
-	restart_handler_register_fn(longhorn_restart_soc);
+	restart_handler_register_fn("soc-wdt", longhorn_restart_soc);
 
 	return 0;
 }
diff --git a/arch/mips/mach-malta/reset.c b/arch/mips/mach-malta/reset.c
index df7be0ae555f..ad0de2741b05 100644
--- a/arch/mips/mach-malta/reset.c
+++ b/arch/mips/mach-malta/reset.c
@@ -24,7 +24,7 @@ static void __noreturn malta_restart_soc(struct restart_handler *rst)
 
 static int restart_register_feature(void)
 {
-	restart_handler_register_fn(malta_restart_soc);
+	restart_handler_register_fn("soc", malta_restart_soc);
 
 	return 0;
 }
diff --git a/arch/nios2/cpu/cpu.c b/arch/nios2/cpu/cpu.c
index 62bcf40f636b..9f86c911cc41 100644
--- a/arch/nios2/cpu/cpu.c
+++ b/arch/nios2/cpu/cpu.c
@@ -30,7 +30,7 @@ static void __noreturn nios2_restart_soc(struct restart_handler *rst)
 
 static int restart_register_feature(void)
 {
-	return restart_handler_register_fn(nios2_restart_soc);
+	return restart_handler_register_fn("vector", nios2_restart_soc);
 }
 coredevice_initcall(restart_register_feature);
 
diff --git a/arch/openrisc/cpu/cpu.c b/arch/openrisc/cpu/cpu.c
index 8afd22bdea29..47d8ab428845 100644
--- a/arch/openrisc/cpu/cpu.c
+++ b/arch/openrisc/cpu/cpu.c
@@ -33,6 +33,6 @@ static void __noreturn openrisc_restart_cpu(struct restart_handler *rst)
 
 static int restart_register_feature(void)
 {
-	return restart_handler_register_fn(openrisc_restart_cpu);
+	return restart_handler_register_fn("vector", openrisc_restart_cpu);
 }
 coredevice_initcall(restart_register_feature);
diff --git a/arch/powerpc/mach-mpc5xxx/cpu.c b/arch/powerpc/mach-mpc5xxx/cpu.c
index a85e1667bcb5..5cf5194aa28e 100644
--- a/arch/powerpc/mach-mpc5xxx/cpu.c
+++ b/arch/powerpc/mach-mpc5xxx/cpu.c
@@ -92,7 +92,7 @@ static void __noreturn mpc5xxx_restart_soc(struct restart_handler *rst)
 
 static int restart_register_feature(void)
 {
-	return restart_handler_register_fn(mpc5xxx_restart_soc);
+	return restart_handler_register_fn("soc-wdt", mpc5xxx_restart_soc);
 }
 coredevice_initcall(restart_register_feature);
 
diff --git a/arch/powerpc/mach-mpc85xx/cpu.c b/arch/powerpc/mach-mpc85xx/cpu.c
index 7c8a59edc9c1..efdff24e6db3 100644
--- a/arch/powerpc/mach-mpc85xx/cpu.c
+++ b/arch/powerpc/mach-mpc85xx/cpu.c
@@ -42,7 +42,7 @@ static void __noreturn mpc85xx_restart_soc(struct restart_handler *rst)
 
 static int restart_register_feature(void)
 {
-	restart_handler_register_fn(mpc85xx_restart_soc);
+	restart_handler_register_fn("soc", mpc85xx_restart_soc);
 
 	return 0;
 }
diff --git a/common/efi/efi.c b/common/efi/efi.c
index 6f55e3970ed0..01003dc00f88 100644
--- a/common/efi/efi.c
+++ b/common/efi/efi.c
@@ -292,7 +292,7 @@ static void __noreturn efi_poweroff_system(struct poweroff_handler *handler)
 
 static int restart_register_feature(void)
 {
-	restart_handler_register_fn(efi_restart_system);
+	restart_handler_register_fn("efi", efi_restart_system);
 	poweroff_handler_register_fn(efi_poweroff_system);
 
 	return 0;
diff --git a/common/restart.c b/common/restart.c
index dd15c8d5c362..66131c262938 100644
--- a/common/restart.c
+++ b/common/restart.c
@@ -47,6 +47,7 @@ int restart_handler_register(struct restart_handler *rst)
 
 /**
  * restart_handler_register_fn() - register a handler function
+ * @name:	restart method name or NULL if name should be auto-generated
  * @restart_fn:	The restart function
  *
  * convenience wrapper for restart_handler_register() to register a handler
@@ -54,13 +55,15 @@ int restart_handler_register(struct restart_handler *rst)
  *
  * return: 0 for success or negative error code
  */
-int restart_handler_register_fn(void (*restart_fn)(struct restart_handler *))
+int restart_handler_register_fn(const char *name,
+				void (*restart_fn)(struct restart_handler *))
 {
 	struct restart_handler *rst;
 	int ret;
 
 	rst = xzalloc(sizeof(*rst));
 
+	rst->name = name;
 	rst->restart = restart_fn;
 
 	ret = restart_handler_register(rst);
diff --git a/include/restart.h b/include/restart.h
index 6880b03b936f..e7dd1bd2b79c 100644
--- a/include/restart.h
+++ b/include/restart.h
@@ -12,7 +12,8 @@ struct restart_handler {
 };
 
 int restart_handler_register(struct restart_handler *);
-int restart_handler_register_fn(void (*restart_fn)(struct restart_handler *));
+int restart_handler_register_fn(const char *name,
+				void (*restart_fn)(struct restart_handler *));
 
 #define RESTART_DEFAULT_PRIORITY 100
 
-- 
2.27.0


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

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

* [PATCH 3/3] commands: reset: allow specifying reset by name
  2020-06-02  7:57 [PATCH 1/3] common: restart: number unnamed restart handlers Ahmad Fatoum
  2020-06-02  7:57 ` [PATCH 2/3] restart: give all restart handlers a descriptive name Ahmad Fatoum
@ 2020-06-02  7:57 ` Ahmad Fatoum
  2020-06-03  7:11 ` [PATCH 1/3] common: restart: number unnamed restart handlers Sascha Hauer
  2 siblings, 0 replies; 10+ messages in thread
From: Ahmad Fatoum @ 2020-06-02  7:57 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

So far, we were fine by using the highest priority restart handler
whenever more than one was available. There are reasons to want to
configure this however:

 - When communicating with BootROM, e.g. to force boot from recovery
   mode: The reset chosen must not cause the reboot mode stored to
   volatile memory to vanish
 - When testing (undocumented) reset behavior, e.g. to analyze how
   the EFI reset behaves

Extend the reset command to support this. When no extra command line
option is supplied, the old behavior is maintained.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 commands/reset.c  | 27 ++++++++++++++++++++++++---
 common/restart.c  | 28 ++++++++++++++++++++++++++--
 include/restart.h |  2 ++
 3 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/commands/reset.c b/commands/reset.c
index 2b10f1cd183a..fe54e2f9b472 100644
--- a/commands/reset.c
+++ b/commands/reset.c
@@ -11,24 +11,43 @@
 
 static int cmd_reset(int argc, char *argv[])
 {
+	struct restart_handler *rst;
 	int opt, shutdown_flag;
+	const char *name = NULL;
 
 	shutdown_flag = 1;
 
-	while ((opt = getopt(argc, argv, "f")) > 0) {
+	while ((opt = getopt(argc, argv, "flr:")) > 0) {
 		switch (opt) {
 		case 'f':
 			shutdown_flag = 0;
 			break;
+		case 'l':
+			restart_handlers_print();
+			return 0;
+		case 'r':
+			name = optarg;
+			break;
 		default:
 			return COMMAND_ERROR_USAGE;
 		}
 	}
 
+	rst = restart_handler_get_by_name(name);
+	if (!rst && name) {
+		printf("reset '%s' does not exist\n", name);
+		return COMMAND_ERROR;
+	}
+
 	if (shutdown_flag)
 		shutdown_barebox();
 
-	restart_machine();
+	if (rst) {
+		console_flush();
+		rst->restart(rst);
+	}
+
+	hang();
 
 	/* Not reached */
 	return 1;
@@ -37,12 +56,14 @@ static int cmd_reset(int argc, char *argv[])
 BAREBOX_CMD_HELP_START(reset)
 BAREBOX_CMD_HELP_TEXT("Options:")
 BAREBOX_CMD_HELP_OPT("-f",  "force RESET, don't call shutdown")
+BAREBOX_CMD_HELP_OPT("-l",  "list reset handlers")
+BAREBOX_CMD_HELP_OPT("-r RESET",  "use reset handler named RESET")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(reset)
 	.cmd		= cmd_reset,
 	BAREBOX_CMD_DESC("perform RESET of the CPU")
-	BAREBOX_CMD_OPTS("[-f]")
+	BAREBOX_CMD_OPTS("[-flr]")
 	BAREBOX_CMD_GROUP(CMD_GRP_BOOT)
 	BAREBOX_CMD_HELP(cmd_reset_help)
 	BAREBOX_CMD_COMPLETE(empty_complete)
diff --git a/common/restart.c b/common/restart.c
index 66131c262938..dcd0b2959bf4 100644
--- a/common/restart.c
+++ b/common/restart.c
@@ -75,20 +75,33 @@ int restart_handler_register_fn(const char *name,
 }
 
 /**
- * restart_machine() - reset the whole system
+ * restart_handler_get_by_name() - reset the whole system
  */
-void __noreturn restart_machine(void)
+struct restart_handler *restart_handler_get_by_name(const char *name)
 {
 	struct restart_handler *rst = NULL, *tmp;
 	unsigned int priority = 0;
 
 	list_for_each_entry(tmp, &restart_handler_list, list) {
+		if (name && tmp->name && strcmp(name, tmp->name))
+			continue;
 		if (tmp->priority > priority) {
 			priority = tmp->priority;
 			rst = tmp;
 		}
 	}
 
+	return rst;
+}
+
+/**
+ * restart_machine() - reset the whole system
+ */
+void __noreturn restart_machine(void)
+{
+	struct restart_handler *rst;
+
+	rst = restart_handler_get_by_name(NULL);
 	if (rst) {
 		pr_debug("%s: using restart handler %s\n", __func__, rst->name);
 		console_flush();
@@ -112,3 +125,14 @@ unsigned int of_get_restart_priority(struct device_node *node)
 
 	return priority;
 }
+
+/*
+ * restart_handlers_print - print informations about all restart handlers
+ */
+void restart_handlers_print(void)
+{
+	struct restart_handler *tmp;
+
+	list_for_each_entry(tmp, &restart_handler_list, list)
+		printf("%-20s %6d\n", tmp->name, tmp->priority);
+}
diff --git a/include/restart.h b/include/restart.h
index e7dd1bd2b79c..2d15c7598acc 100644
--- a/include/restart.h
+++ b/include/restart.h
@@ -2,7 +2,9 @@
 #ifndef __INCLUDE_RESTART_H
 #define __INCLUDE_RESTART_H
 
+void restart_handlers_print(void);
 void __noreturn restart_machine(void);
+struct restart_handler *restart_handler_get_by_name(const char *name);
 
 struct restart_handler {
 	void (*restart)(struct restart_handler *);
-- 
2.27.0


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

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

* Re: [PATCH 1/3] common: restart: number unnamed restart handlers
  2020-06-02  7:57 [PATCH 1/3] common: restart: number unnamed restart handlers Ahmad Fatoum
  2020-06-02  7:57 ` [PATCH 2/3] restart: give all restart handlers a descriptive name Ahmad Fatoum
  2020-06-02  7:57 ` [PATCH 3/3] commands: reset: allow specifying reset by name Ahmad Fatoum
@ 2020-06-03  7:11 ` Sascha Hauer
  2020-06-03 13:32   ` Ahmad Fatoum
  2 siblings, 1 reply; 10+ messages in thread
From: Sascha Hauer @ 2020-06-03  7:11 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On Tue, Jun 02, 2020 at 09:57:55AM +0200, Ahmad Fatoum wrote:
> Follow-up commit allows referencing specific restart handler by name.
> Restart handlers default to "default" as name when none is given.
> Number them sequentially instead.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  common/restart.c  | 4 +++-
>  include/restart.h | 1 -
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/common/restart.c b/common/restart.c
> index b19ae54657c0..dd15c8d5c362 100644
> --- a/common/restart.c
> +++ b/common/restart.c
> @@ -19,6 +19,7 @@
>  #include <of.h>
>  
>  static LIST_HEAD(restart_handler_list);
> +static unsigned resetidx;
>  
>  /**
>   * restart_handler_register() - register a handler for restarting the system
> @@ -31,7 +32,7 @@ static LIST_HEAD(restart_handler_list);
>  int restart_handler_register(struct restart_handler *rst)
>  {
>  	if (!rst->name)
> -		rst->name = RESTART_DEFAULT_NAME;
> +		rst->name = basprintf("reset%u", resetidx);

With this most existing restart handlers get a unique name, but in the
next patch you give most of them the same name. I am not sure where this
is aiming at.
With the next patch every restart handler has a name, so why is the name
still optional?

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

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

* Re: [PATCH 2/3] restart: give all restart handlers a descriptive name
  2020-06-02  7:57 ` [PATCH 2/3] restart: give all restart handlers a descriptive name Ahmad Fatoum
@ 2020-06-03  7:12   ` Sascha Hauer
  2020-09-15 12:05   ` [PATCH] fixup! " Ahmad Fatoum
  1 sibling, 0 replies; 10+ messages in thread
From: Sascha Hauer @ 2020-06-03  7:12 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On Tue, Jun 02, 2020 at 09:57:56AM +0200, Ahmad Fatoum wrote:
> diff --git a/common/restart.c b/common/restart.c
> index dd15c8d5c362..66131c262938 100644
> --- a/common/restart.c
> +++ b/common/restart.c
> @@ -47,6 +47,7 @@ int restart_handler_register(struct restart_handler *rst)
>  
>  /**
>   * restart_handler_register_fn() - register a handler function
> + * @name:	restart method name or NULL if name should be auto-generated
>   * @restart_fn:	The restart function
>   *
>   * convenience wrapper for restart_handler_register() to register a handler
> @@ -54,13 +55,15 @@ int restart_handler_register(struct restart_handler *rst)
>   *
>   * return: 0 for success or negative error code
>   */
> -int restart_handler_register_fn(void (*restart_fn)(struct restart_handler *))
> +int restart_handler_register_fn(const char *name,
> +				void (*restart_fn)(struct restart_handler *))
>  {
>  	struct restart_handler *rst;
>  	int ret;
>  
>  	rst = xzalloc(sizeof(*rst));
>  
> +	rst->name = name;

Please allocate the name in case someone passes an array from the stack
here.

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

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

* Re: [PATCH 1/3] common: restart: number unnamed restart handlers
  2020-06-03  7:11 ` [PATCH 1/3] common: restart: number unnamed restart handlers Sascha Hauer
@ 2020-06-03 13:32   ` Ahmad Fatoum
  2020-06-08  5:20     ` Sascha Hauer
  0 siblings, 1 reply; 10+ messages in thread
From: Ahmad Fatoum @ 2020-06-03 13:32 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

Hello Sascha,

On 6/3/20 9:11 AM, Sascha Hauer wrote:
> On Tue, Jun 02, 2020 at 09:57:55AM +0200, Ahmad Fatoum wrote:
>> Follow-up commit allows referencing specific restart handler by name.
>> Restart handlers default to "default" as name when none is given.
>> Number them sequentially instead.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  common/restart.c  | 4 +++-
>>  include/restart.h | 1 -
>>  2 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/common/restart.c b/common/restart.c
>> index b19ae54657c0..dd15c8d5c362 100644
>> --- a/common/restart.c
>> +++ b/common/restart.c
>> @@ -19,6 +19,7 @@
>>  #include <of.h>
>>  
>>  static LIST_HEAD(restart_handler_list);
>> +static unsigned resetidx;
>>  
>>  /**
>>   * restart_handler_register() - register a handler for restarting the system
>> @@ -31,7 +32,7 @@ static LIST_HEAD(restart_handler_list);
>>  int restart_handler_register(struct restart_handler *rst)
>>  {
>>  	if (!rst->name)
>> -		rst->name = RESTART_DEFAULT_NAME;
>> +		rst->name = basprintf("reset%u", resetidx);
> 
> With this most existing restart handlers get a unique name, but in the
> next patch you give most of them the same name. I am not sure where this
> is aiming at.

I haven't exhaustively checked, but the resets given descriptive names
in the previous commit are all singletons: There shouldn't be two of them
in the same build. If there are, the solution isn't a soc0 and soc1 reset,
but instead they need more descriptive names.

> With the next patch every restart handler has a name, so why is the name
> still optional?

I guess I can just make it mandatory and error out with a warning on
registration time?

Cheers
Ahmad


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

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

* Re: [PATCH 1/3] common: restart: number unnamed restart handlers
  2020-06-03 13:32   ` Ahmad Fatoum
@ 2020-06-08  5:20     ` Sascha Hauer
  2020-09-15  8:48       ` Ahmad Fatoum
  0 siblings, 1 reply; 10+ messages in thread
From: Sascha Hauer @ 2020-06-08  5:20 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On Wed, Jun 03, 2020 at 03:32:35PM +0200, Ahmad Fatoum wrote:
> Hello Sascha,
> 
> On 6/3/20 9:11 AM, Sascha Hauer wrote:
> > On Tue, Jun 02, 2020 at 09:57:55AM +0200, Ahmad Fatoum wrote:
> >> Follow-up commit allows referencing specific restart handler by name.
> >> Restart handlers default to "default" as name when none is given.
> >> Number them sequentially instead.
> >>
> >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >> ---
> >>  common/restart.c  | 4 +++-
> >>  include/restart.h | 1 -
> >>  2 files changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/common/restart.c b/common/restart.c
> >> index b19ae54657c0..dd15c8d5c362 100644
> >> --- a/common/restart.c
> >> +++ b/common/restart.c
> >> @@ -19,6 +19,7 @@
> >>  #include <of.h>
> >>  
> >>  static LIST_HEAD(restart_handler_list);
> >> +static unsigned resetidx;
> >>  
> >>  /**
> >>   * restart_handler_register() - register a handler for restarting the system
> >> @@ -31,7 +32,7 @@ static LIST_HEAD(restart_handler_list);
> >>  int restart_handler_register(struct restart_handler *rst)
> >>  {
> >>  	if (!rst->name)
> >> -		rst->name = RESTART_DEFAULT_NAME;
> >> +		rst->name = basprintf("reset%u", resetidx);
> > 
> > With this most existing restart handlers get a unique name, but in the
> > next patch you give most of them the same name. I am not sure where this
> > is aiming at.
> 
> I haven't exhaustively checked, but the resets given descriptive names
> in the previous commit are all singletons: There shouldn't be two of them
> in the same build. If there are, the solution isn't a soc0 and soc1 reset,
> but instead they need more descriptive names.

Ok.

> 
> > With the next patch every restart handler has a name, so why is the name
> > still optional?
> 
> I guess I can just make it mandatory and error out with a warning on
> registration time?

Yes, right

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

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

* Re: [PATCH 1/3] common: restart: number unnamed restart handlers
  2020-06-08  5:20     ` Sascha Hauer
@ 2020-09-15  8:48       ` Ahmad Fatoum
  2020-09-15 12:32         ` Sascha Hauer
  0 siblings, 1 reply; 10+ messages in thread
From: Ahmad Fatoum @ 2020-09-15  8:48 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

Hello Sascha,

On 6/8/20 7:20 AM, Sascha Hauer wrote:
> On Wed, Jun 03, 2020 at 03:32:35PM +0200, Ahmad Fatoum wrote:
>> Hello Sascha,
>>
>> On 6/3/20 9:11 AM, Sascha Hauer wrote:
>>> On Tue, Jun 02, 2020 at 09:57:55AM +0200, Ahmad Fatoum wrote:
>>>> Follow-up commit allows referencing specific restart handler by name.
>>>> Restart handlers default to "default" as name when none is given.
>>>> Number them sequentially instead.
>>>>
>>>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>>> ---
>>>>  common/restart.c  | 4 +++-
>>>>  include/restart.h | 1 -
>>>>  2 files changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/common/restart.c b/common/restart.c
>>>> index b19ae54657c0..dd15c8d5c362 100644
>>>> --- a/common/restart.c
>>>> +++ b/common/restart.c
>>>> @@ -19,6 +19,7 @@
>>>>  #include <of.h>
>>>>  
>>>>  static LIST_HEAD(restart_handler_list);
>>>> +static unsigned resetidx;
>>>>  
>>>>  /**
>>>>   * restart_handler_register() - register a handler for restarting the system
>>>> @@ -31,7 +32,7 @@ static LIST_HEAD(restart_handler_list);
>>>>  int restart_handler_register(struct restart_handler *rst)
>>>>  {
>>>>  	if (!rst->name)
>>>> -		rst->name = RESTART_DEFAULT_NAME;
>>>> +		rst->name = basprintf("reset%u", resetidx);
>>>
>>> With this most existing restart handlers get a unique name, but in the
>>> next patch you give most of them the same name. I am not sure where this
>>> is aiming at.
>>
>> I haven't exhaustively checked, but the resets given descriptive names
>> in the previous commit are all singletons: There shouldn't be two of them
>> in the same build. If there are, the solution isn't a soc0 and soc1 reset,
>> but instead they need more descriptive names.
> 
> Ok.
> 
>>
>>> With the next patch every restart handler has a name, so why is the name
>>> still optional?
>>
>> I guess I can just make it mandatory and error out with a warning on
>> registration time?
> 
> Yes, right

I took look at this and I don't really like the approach.
I am wary of changing barebox API in subtle ways that breaks external
users. The existing solution doesn't have this problem. Reset handlers
without a specific name will be called reset0, reset1.. instead
of default. Could this be merged as-is?

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

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

* [PATCH] fixup! restart: give all restart handlers a descriptive name
  2020-06-02  7:57 ` [PATCH 2/3] restart: give all restart handlers a descriptive name Ahmad Fatoum
  2020-06-03  7:12   ` Sascha Hauer
@ 2020-09-15 12:05   ` Ahmad Fatoum
  1 sibling, 0 replies; 10+ messages in thread
From: Ahmad Fatoum @ 2020-09-15 12:05 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

As suggested by Sascha, allow users to pass automatically allocated
name.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/restart.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/restart.c b/common/restart.c
index 3791337a7505..7bd39de8c318 100644
--- a/common/restart.c
+++ b/common/restart.c
@@ -64,7 +64,7 @@ int restart_handler_register_fn(const char *name,
 
 	rst = xzalloc(sizeof(*rst));
 
-	rst->name = name;
+	rst->name = xstrdup(name);
 	rst->restart = restart_fn;
 
 	ret = restart_handler_register(rst);
-- 
2.28.0


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

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

* Re: [PATCH 1/3] common: restart: number unnamed restart handlers
  2020-09-15  8:48       ` Ahmad Fatoum
@ 2020-09-15 12:32         ` Sascha Hauer
  0 siblings, 0 replies; 10+ messages in thread
From: Sascha Hauer @ 2020-09-15 12:32 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On Tue, Sep 15, 2020 at 10:48:10AM +0200, Ahmad Fatoum wrote:
> Hello Sascha,
> 
> On 6/8/20 7:20 AM, Sascha Hauer wrote:
> > On Wed, Jun 03, 2020 at 03:32:35PM +0200, Ahmad Fatoum wrote:
> >> Hello Sascha,
> >>
> >> On 6/3/20 9:11 AM, Sascha Hauer wrote:
> >>> On Tue, Jun 02, 2020 at 09:57:55AM +0200, Ahmad Fatoum wrote:
> >>>> Follow-up commit allows referencing specific restart handler by name.
> >>>> Restart handlers default to "default" as name when none is given.
> >>>> Number them sequentially instead.
> >>>>
> >>>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >>>> ---
> >>>>  common/restart.c  | 4 +++-
> >>>>  include/restart.h | 1 -
> >>>>  2 files changed, 3 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/common/restart.c b/common/restart.c
> >>>> index b19ae54657c0..dd15c8d5c362 100644
> >>>> --- a/common/restart.c
> >>>> +++ b/common/restart.c
> >>>> @@ -19,6 +19,7 @@
> >>>>  #include <of.h>
> >>>>  
> >>>>  static LIST_HEAD(restart_handler_list);
> >>>> +static unsigned resetidx;
> >>>>  
> >>>>  /**
> >>>>   * restart_handler_register() - register a handler for restarting the system
> >>>> @@ -31,7 +32,7 @@ static LIST_HEAD(restart_handler_list);
> >>>>  int restart_handler_register(struct restart_handler *rst)
> >>>>  {
> >>>>  	if (!rst->name)
> >>>> -		rst->name = RESTART_DEFAULT_NAME;
> >>>> +		rst->name = basprintf("reset%u", resetidx);
> >>>
> >>> With this most existing restart handlers get a unique name, but in the
> >>> next patch you give most of them the same name. I am not sure where this
> >>> is aiming at.
> >>
> >> I haven't exhaustively checked, but the resets given descriptive names
> >> in the previous commit are all singletons: There shouldn't be two of them
> >> in the same build. If there are, the solution isn't a soc0 and soc1 reset,
> >> but instead they need more descriptive names.
> > 
> > Ok.
> > 
> >>
> >>> With the next patch every restart handler has a name, so why is the name
> >>> still optional?
> >>
> >> I guess I can just make it mandatory and error out with a warning on
> >> registration time?
> > 
> > Yes, right
> 
> I took look at this and I don't really like the approach.
> I am wary of changing barebox API in subtle ways that breaks external
> users. The existing solution doesn't have this problem. Reset handlers
> without a specific name will be called reset0, reset1.. instead
> of default. Could this be merged as-is?

Just did that.

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

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

end of thread, other threads:[~2020-09-15 12:32 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-02  7:57 [PATCH 1/3] common: restart: number unnamed restart handlers Ahmad Fatoum
2020-06-02  7:57 ` [PATCH 2/3] restart: give all restart handlers a descriptive name Ahmad Fatoum
2020-06-03  7:12   ` Sascha Hauer
2020-09-15 12:05   ` [PATCH] fixup! " Ahmad Fatoum
2020-06-02  7:57 ` [PATCH 3/3] commands: reset: allow specifying reset by name Ahmad Fatoum
2020-06-03  7:11 ` [PATCH 1/3] common: restart: number unnamed restart handlers Sascha Hauer
2020-06-03 13:32   ` Ahmad Fatoum
2020-06-08  5:20     ` Sascha Hauer
2020-09-15  8:48       ` Ahmad Fatoum
2020-09-15 12:32         ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox