[PATCH 1/4] power: reset: reboot-mode: refactor to allow other mode- prefixes

[PATCH 1/4] power: reset: reboot-mode: refactor to allow other mode- prefixes

[Ahmad Fatoum]

We will add support for one more mode- prefix in the follow-up commit,
so rewrite the code to make this addition seamless.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/power/reset/reboot-mode.c | 33 +++++++++++++++++++------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
index d9b51dfea677..ab23c32a4370 100644
--- a/drivers/power/reset/reboot-mode.c
+++ b/drivers/power/reset/reboot-mode.c
@@ -12,8 +12,6 @@
 #include <globalvar.h>
 #include <magicvar.h>
 
-#define PREFIX "mode-"
-
 static int __priority;
 static struct reboot_mode_driver *__boot_mode;
 
@@ -111,6 +109,17 @@ static void reboot_mode_print(struct reboot_mode_driver *reboot,
 	__pr_printk(7, "\n");
 }
 
+static const char *get_mode_name(const struct property *prop)
+{
+	unsigned prefix_len;
+
+	prefix_len = str_has_prefix(prop->name, "mode-");
+	if (!prefix_len)
+		return NULL;
+
+	return prop->name + prefix_len;
+}
+
 /**
  * reboot_mode_register - register a reboot mode driver
  * @reboot: reboot mode driver
@@ -123,7 +132,6 @@ int reboot_mode_register(struct reboot_mode_driver *reboot,
 {
 	struct property *prop;
 	struct device_node *np = reboot->dev->device_node;
-	size_t len = strlen(PREFIX);
 	const char *alias;
 	size_t nmodes = 0;
 	int i = 0;
@@ -132,7 +140,7 @@ int reboot_mode_register(struct reboot_mode_driver *reboot,
 	for_each_property_of_node(np, prop) {
 		u32 magic;
 
-		if (strncmp(prop->name, PREFIX, len))
+		if (!get_mode_name(prop))
 			continue;
 		if (of_property_read_u32(np, prop->name, &magic))
 			continue;
@@ -154,16 +162,9 @@ int reboot_mode_register(struct reboot_mode_driver *reboot,
 		magic = &reboot->magics[i * nelems];
 		mode = &reboot->modes[i];
 
-		if (strncmp(prop->name, PREFIX, len))
-			continue;
-
-		if (of_property_read_u32_array(np, prop->name, magic, nelems)) {
-			dev_err(reboot->dev, "reboot mode %s without magic number\n",
-				*mode);
+		*mode = get_mode_name(prop);
+		if (!*mode)
 			continue;
-		}
-
-		*mode = prop->name + len;
 		if (*mode[0] == '\0') {
 			ret = -EINVAL;
 			dev_err(reboot->dev, "invalid mode name(%s): too short!\n",
@@ -171,6 +172,12 @@ int reboot_mode_register(struct reboot_mode_driver *reboot,
 			goto error;
 		}
 
+		if (of_property_read_u32_array(np, prop->name, magic, nelems)) {
+			dev_err(reboot->dev, "reboot mode %s without magic number\n",
+				*mode);
+			continue;
+		}
+
 		reboot_mode_print(reboot, *mode, magic);
 
 		i++;
-- 
2.30.2

[PATCH 2/4] power: reset: reboot-mode: parse barebox,mode-* properties

[Ahmad Fatoum]

The same syscon may be used both to communicate with the bootrom and
with barebox. In this case, it may be required that not all boot modes
are enabled for the OS as Linux just makes them available in turn
to userspace. Teach barebox to handle the barebox,mode- prefix the
same way it handles mode-. As Linux doesn't parse this new property,
this means that it is unproblematic to fix it up into the Linux
device tree.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/power/reset/reboot-mode.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
index ab23c32a4370..9a99b4056ff1 100644
--- a/drivers/power/reset/reboot-mode.c
+++ b/drivers/power/reset/reboot-mode.c
@@ -114,6 +114,8 @@ static const char *get_mode_name(const struct property *prop)
 	unsigned prefix_len;
 
 	prefix_len = str_has_prefix(prop->name, "mode-");
+	if (!prefix_len)
+		prefix_len = str_has_prefix(prop->name, "barebox,mode-");
 	if (!prefix_len)
 		return NULL;
 
-- 
2.30.2

[PATCH 3/4] ARM: dts: stm32mp: reuse now upstream tamp node

[Ahmad Fatoum]

Since 6ea3de1b8ce7 ("dts: update to v5.11-rc1"), the upstream device
tree now has a node with simple-mfd, which we can add our reboot mode
beneath without having to replicate information, so make use of that.

No functional change.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/dts/stm32mp151.dtsi | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/arch/arm/dts/stm32mp151.dtsi b/arch/arm/dts/stm32mp151.dtsi
index 7b07aaf7cf6a..ed7cf6192bea 100644
--- a/arch/arm/dts/stm32mp151.dtsi
+++ b/arch/arm/dts/stm32mp151.dtsi
@@ -31,22 +31,19 @@
 		compatible = "st,stm32mp1-ddr";
 		reg = <0x5a003000 0x1000>;
 	};
-
-	tamp@5c00a000 {
-		compatible = "simple-bus", "syscon", "simple-mfd";
-		reg = <0x5c00a000 0x400>;
-
-		reboot_mode_tamp: reboot-mode {
-			compatible = "syscon-reboot-mode";
-			offset = <0x150>; /* reg20 */
-			mask = <0xff>;
-			mode-normal = <0>;
-			mode-loader = <0xBB>;
-			mode-recovery = <0xBC>;
-		};
-	};
 };
 
 &bsec {
 	barebox,provide-mac-address = <&ethernet0 0x39>;
 };
+
+&tamp {
+	reboot_mode_tamp: reboot-mode {
+		compatible = "syscon-reboot-mode";
+		offset = <0x150>; /* reg20 */
+		mask = <0xff>;
+		mode-normal = <0>;
+		mode-loader = <0xBB>;
+		mode-recovery = <0xBC>;
+	};
+};
-- 
2.30.2

[PATCH 4/4] ARM: dts: stm32mp: add bootrom serial (DFU) reboot mode

[Ahmad Fatoum]

The same register we use for the reboot mode communication between Linux
and barebox is also sampled by the bootrom. If the lowest 16 bits of it
are 0x00FF, the bootrom will enter DFU mode. Add the appropriate
reboot-mode, but with a barebox, prefix, so Linux doesn't automatically
provide it to userspace along with the existing loader and recovery
modes.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/dts/stm32mp151.dtsi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/dts/stm32mp151.dtsi b/arch/arm/dts/stm32mp151.dtsi
index ed7cf6192bea..ac6536a556a9 100644
--- a/arch/arm/dts/stm32mp151.dtsi
+++ b/arch/arm/dts/stm32mp151.dtsi
@@ -41,9 +41,10 @@
 	reboot_mode_tamp: reboot-mode {
 		compatible = "syscon-reboot-mode";
 		offset = <0x150>; /* reg20 */
-		mask = <0xff>;
+		mask = <0xffff>;
 		mode-normal = <0>;
 		mode-loader = <0xBB>;
 		mode-recovery = <0xBC>;
+		barebox,mode-serial = <0xFF>;
 	};
 };
-- 
2.30.2

Re: [PATCH 4/4] ARM: dts: stm32mp: add bootrom serial (DFU) reboot mode

[Sascha Hauer]

On Tue, Aug 09, 2022 at 08:29:38AM +0200, Ahmad Fatoum wrote:
> The same register we use for the reboot mode communication between Linux
> and barebox is also sampled by the bootrom. If the lowest 16 bits of it
> are 0x00FF, the bootrom will enter DFU mode. Add the appropriate
> reboot-mode, but with a barebox, prefix, so Linux doesn't automatically
> provide it to userspace along with the existing loader and recovery
> modes.

Why do we need to prevent Linux from providing dfu mode to userspace?
Isn't that a vlid usecase?

Sascha

> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  arch/arm/dts/stm32mp151.dtsi | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/dts/stm32mp151.dtsi b/arch/arm/dts/stm32mp151.dtsi
> index ed7cf6192bea..ac6536a556a9 100644
> --- a/arch/arm/dts/stm32mp151.dtsi
> +++ b/arch/arm/dts/stm32mp151.dtsi
> @@ -41,9 +41,10 @@
>  	reboot_mode_tamp: reboot-mode {
>  		compatible = "syscon-reboot-mode";
>  		offset = <0x150>; /* reg20 */
> -		mask = <0xff>;
> +		mask = <0xffff>;
>  		mode-normal = <0>;
>  		mode-loader = <0xBB>;
>  		mode-recovery = <0xBC>;
> +		barebox,mode-serial = <0xFF>;
>  	};
>  };
> -- 
> 2.30.2
> 
> 
> 

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

Re: [PATCH 4/4] ARM: dts: stm32mp: add bootrom serial (DFU) reboot mode

[Ahmad Fatoum]

Hello Sascha,

On 09.08.22 11:49, Sascha Hauer wrote:
> On Tue, Aug 09, 2022 at 08:29:38AM +0200, Ahmad Fatoum wrote:
>> The same register we use for the reboot mode communication between Linux
>> and barebox is also sampled by the bootrom. If the lowest 16 bits of it
>> are 0x00FF, the bootrom will enter DFU mode. Add the appropriate
>> reboot-mode, but with a barebox, prefix, so Linux doesn't automatically
>> provide it to userspace along with the existing loader and recovery
>> modes.
> 
> Why do we need to prevent Linux from providing dfu mode to userspace?
> Isn't that a vlid usecase?

It is, but I don't want a barebox update to add this feature by default.
So, communication with BootROM = not exported by default
Communication with Linux = exported by default

and this can be overridden on a per-board basis via device tree.
We do this likewise for the i.MX6 and i.MX8 where we have
barebox,syscon-reboot-mode which isn't processed by Linux, but we
can't use this here because the same register is used for both
modes of communication.

Cheers,
Ahmad

> 
> Sascha
> 
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  arch/arm/dts/stm32mp151.dtsi | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/dts/stm32mp151.dtsi b/arch/arm/dts/stm32mp151.dtsi
>> index ed7cf6192bea..ac6536a556a9 100644
>> --- a/arch/arm/dts/stm32mp151.dtsi
>> +++ b/arch/arm/dts/stm32mp151.dtsi
>> @@ -41,9 +41,10 @@
>>  	reboot_mode_tamp: reboot-mode {
>>  		compatible = "syscon-reboot-mode";
>>  		offset = <0x150>; /* reg20 */
>> -		mask = <0xff>;
>> +		mask = <0xffff>;
>>  		mode-normal = <0>;
>>  		mode-loader = <0xBB>;
>>  		mode-recovery = <0xBC>;
>> +		barebox,mode-serial = <0xFF>;
>>  	};
>>  };
>> -- 
>> 2.30.2
>>
>>
>>
> 


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

Re: [PATCH 1/4] power: reset: reboot-mode: refactor to allow other mode- prefixes

[Sascha Hauer]

On Tue, Aug 09, 2022 at 08:29:35AM +0200, Ahmad Fatoum wrote:
> We will add support for one more mode- prefix in the follow-up commit,
> so rewrite the code to make this addition seamless.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/power/reset/reboot-mode.c | 33 +++++++++++++++++++------------
>  1 file changed, 20 insertions(+), 13 deletions(-)

Applied, thanks

Sascha

> 
> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
> index d9b51dfea677..ab23c32a4370 100644
> --- a/drivers/power/reset/reboot-mode.c
> +++ b/drivers/power/reset/reboot-mode.c
> @@ -12,8 +12,6 @@
>  #include <globalvar.h>
>  #include <magicvar.h>
>  
> -#define PREFIX "mode-"
> -
>  static int __priority;
>  static struct reboot_mode_driver *__boot_mode;
>  
> @@ -111,6 +109,17 @@ static void reboot_mode_print(struct reboot_mode_driver *reboot,
>  	__pr_printk(7, "\n");
>  }
>  
> +static const char *get_mode_name(const struct property *prop)
> +{
> +	unsigned prefix_len;
> +
> +	prefix_len = str_has_prefix(prop->name, "mode-");
> +	if (!prefix_len)
> +		return NULL;
> +
> +	return prop->name + prefix_len;
> +}
> +
>  /**
>   * reboot_mode_register - register a reboot mode driver
>   * @reboot: reboot mode driver
> @@ -123,7 +132,6 @@ int reboot_mode_register(struct reboot_mode_driver *reboot,
>  {
>  	struct property *prop;
>  	struct device_node *np = reboot->dev->device_node;
> -	size_t len = strlen(PREFIX);
>  	const char *alias;
>  	size_t nmodes = 0;
>  	int i = 0;
> @@ -132,7 +140,7 @@ int reboot_mode_register(struct reboot_mode_driver *reboot,
>  	for_each_property_of_node(np, prop) {
>  		u32 magic;
>  
> -		if (strncmp(prop->name, PREFIX, len))
> +		if (!get_mode_name(prop))
>  			continue;
>  		if (of_property_read_u32(np, prop->name, &magic))
>  			continue;
> @@ -154,16 +162,9 @@ int reboot_mode_register(struct reboot_mode_driver *reboot,
>  		magic = &reboot->magics[i * nelems];
>  		mode = &reboot->modes[i];
>  
> -		if (strncmp(prop->name, PREFIX, len))
> -			continue;
> -
> -		if (of_property_read_u32_array(np, prop->name, magic, nelems)) {
> -			dev_err(reboot->dev, "reboot mode %s without magic number\n",
> -				*mode);
> +		*mode = get_mode_name(prop);
> +		if (!*mode)
>  			continue;
> -		}
> -
> -		*mode = prop->name + len;
>  		if (*mode[0] == '\0') {
>  			ret = -EINVAL;
>  			dev_err(reboot->dev, "invalid mode name(%s): too short!\n",
> @@ -171,6 +172,12 @@ int reboot_mode_register(struct reboot_mode_driver *reboot,
>  			goto error;
>  		}
>  
> +		if (of_property_read_u32_array(np, prop->name, magic, nelems)) {
> +			dev_err(reboot->dev, "reboot mode %s without magic number\n",
> +				*mode);
> +			continue;
> +		}
> +
>  		reboot_mode_print(reboot, *mode, magic);
>  
>  		i++;
> -- 
> 2.30.2
> 
> 
> 

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