[PATCH master 1/3] param: drop priv from dev_add_param_string_ro

[PATCH master 1/3] param: drop priv from dev_add_param_string_ro

[Ahmad Fatoum]

dev_add_param_string_ro() is used nowhere, but when we do, having an
unused priv parameter serves no purpose, so drop it.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/param.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/param.h b/include/param.h
index 76a077fd758a..5d4f7f3db52f 100644
--- a/include/param.h
+++ b/include/param.h
@@ -334,8 +334,7 @@ static inline struct param_d *dev_add_param_bool_ro(struct device *dev,
 
 static inline struct param_d *dev_add_param_string_ro(struct device *dev,
 						      const char *name,
-						      char **value,
-						      void *priv)
+						      char **value)
 {
 	return dev_add_param_string(dev, name, param_set_readonly, NULL, value, NULL);
 }
-- 
2.39.2

[PATCH master 2/3] restart: fix restart_handler_get_by_name documentation

[Ahmad Fatoum]

restart_handler_get_by_name() does what its name suggests, so reflect
that in the documentation comment.

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 0294b36eccca..35cfb542519d 100644
--- a/common/restart.c
+++ b/common/restart.c
@@ -74,7 +74,7 @@ int restart_handler_register_fn(const char *name,
 }
 
 /**
- * restart_handler_get_by_name() - reset the whole system
+ * restart_handler_get_by_name() - get highest priority `name'
  */
 struct restart_handler *restart_handler_get_by_name(const char *name, int flags)
 {
-- 
2.39.2

[PATCH master 3/3] boards: qemu-virt: apply state/env overlay only if flash exists

[Ahmad Fatoum]

/flash@0 on ARM64 may not always exist:

  - Older Qemu versions place the flash at /soc/flash@0

  - With secure=on, /flash@0 is renamed to /secflash@0 and is
    off-limits to barebox running in the normal world

Solve both issues by only applying the overlay if the node being
partitioned actually exists and is not disabled in the DT.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/boards/qemu-virt/Makefile             |  4 ++--
 common/boards/qemu-virt/board.c              | 14 ++++++++++---
 common/boards/qemu-virt/qemu-virt-flash.dtso | 12 +++--------
 common/boards/qemu-virt/qemu-virt-flash.h    | 22 ++++++++++++++++++++
 4 files changed, 38 insertions(+), 14 deletions(-)
 create mode 100644 common/boards/qemu-virt/qemu-virt-flash.h

diff --git a/common/boards/qemu-virt/Makefile b/common/boards/qemu-virt/Makefile
index b6883ff7672e..30bf4f1955ee 100644
--- a/common/boards/qemu-virt/Makefile
+++ b/common/boards/qemu-virt/Makefile
@@ -3,10 +3,10 @@
 obj-y += board.o
 obj-y += qemu-virt-flash.dtbo.o fitimage-pubkey.dtb.o
 ifeq ($(CONFIG_RISCV),y)
-DTC_CPP_FLAGS_qemu-virt-flash.dtbo := -DRISCV_VIRT=1
+DTC_CPP_FLAGS_qemu-virt-flash.dtbo := -DCONFIG_RISCV
 endif
 ifeq ($(CONFIG_ARM),y)
-DTC_CPP_FLAGS_qemu-virt-flash.dtbo := -DARM_VIRT=1
+DTC_CPP_FLAGS_qemu-virt-flash.dtbo := -DCONFIG_ARM
 endif
 
 clean-files := *.dtb *.dtb.S .*.dtc .*.pre .*.dts *.dtb.z
diff --git a/common/boards/qemu-virt/board.c b/common/boards/qemu-virt/board.c
index 4c6df5e30252..b9df129834d9 100644
--- a/common/boards/qemu-virt/board.c
+++ b/common/boards/qemu-virt/board.c
@@ -7,6 +7,7 @@
 #include <init.h>
 #include <of.h>
 #include <deep-probe.h>
+#include "qemu-virt-flash.h"
 
 #ifdef CONFIG_64BIT
 #define MACHINE "virt64"
@@ -53,7 +54,7 @@ BAREBOX_DEEP_PROBE_ENABLE(virt_of_match);
 static int virt_board_driver_init(void)
 {
 	struct device_node *root = of_get_root_node();
-	struct device_node *overlay, *pubkey;
+	struct device_node *flash, *overlay, *pubkey;
 	const struct of_device_id *id;
 	void (*init)(void);
 
@@ -66,8 +67,15 @@ static int virt_board_driver_init(void)
 		init();
 	}
 
-	overlay = of_unflatten_dtb(__dtbo_qemu_virt_flash_start, INT_MAX);
-	of_overlay_apply_tree(root, overlay);
+	/*
+	 * Catch both old Qemu versions that place /flash in /soc and
+	 * configurations, where the first flash bank is secure-world only
+	 */
+	flash = of_find_node_by_path(PARTS_TARGET_PATH_STR);
+	if (flash && of_device_is_available(flash)) {
+		overlay = of_unflatten_dtb(__dtbo_qemu_virt_flash_start, INT_MAX);
+		of_overlay_apply_tree(root, overlay);
+	}
 
 	pubkey = of_unflatten_dtb(__dtb_fitimage_pubkey_start, INT_MAX);
 	of_merge_nodes(root, pubkey);
diff --git a/common/boards/qemu-virt/qemu-virt-flash.dtso b/common/boards/qemu-virt/qemu-virt-flash.dtso
index 16b1c7923d58..087568a26d2a 100644
--- a/common/boards/qemu-virt/qemu-virt-flash.dtso
+++ b/common/boards/qemu-virt/qemu-virt-flash.dtso
@@ -3,16 +3,10 @@
 /dts-v1/;
 /plugin/;
 
-#ifdef RISCV_VIRT
-#define PARTS_TARGET_PATH	/flash@20000000
-#define ENV_DEVICE_PATH		"/flash@20000000/partitions/partition@3c00000"
-#elif defined ARM_VIRT
-#define PARTS_TARGET_PATH	/flash@0
-#define ENV_DEVICE_PATH		"/flash@0/partitions/partition@3c00000"
-#endif
+#include "qemu-virt-flash.h"
 
 &{PARTS_TARGET_PATH} {
-#ifdef ARM_VIRT
+#ifdef CONFIG_ARM
 	virtual-reg = <0x1000>;
 #endif
 	partitions {
@@ -40,7 +34,7 @@ backend_state_flash: partition@3e00000 {
 &{/chosen} {
 	environment {
 		compatible = "barebox,environment";
-		device-path = ENV_DEVICE_PATH;
+		device-path = ENV_DEVICE_PATH_STR;
 	};
 };
 
diff --git a/common/boards/qemu-virt/qemu-virt-flash.h b/common/boards/qemu-virt/qemu-virt-flash.h
new file mode 100644
index 000000000000..85f67ff03057
--- /dev/null
+++ b/common/boards/qemu-virt/qemu-virt-flash.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef __QEMU_VIRT_FLASH_H__
+#define __QEMU_VIRT_FLASH_H__
+
+#include <linux/stringify.h>
+
+#ifdef CONFIG_RISCV
+#define PARTS_TARGET_PATH	/flash@20000000
+#define ENV_DEVICE_PATH		/flash@20000000/partitions/partition@3c00000
+#elif defined CONFIG_ARM
+#define PARTS_TARGET_PATH	/flash@0
+#define ENV_DEVICE_PATH		/flash@0/partitions/partition@3c00000
+#else
+#define PARTS_TARGET_PATH
+#define ENV_DEVICE_PATH
+#endif
+
+#define PARTS_TARGET_PATH_STR	__stringify(PARTS_TARGET_PATH)
+#define ENV_DEVICE_PATH_STR	__stringify(ENV_DEVICE_PATH)
+
+#endif
-- 
2.39.2

Re: [PATCH master 1/3] param: drop priv from dev_add_param_string_ro

[Sascha Hauer]

On Tue, Aug 22, 2023 at 09:47:36AM +0200, Ahmad Fatoum wrote:
> dev_add_param_string_ro() is used nowhere, but when we do, having an
> unused priv parameter serves no purpose, so drop it.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  include/param.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/include/param.h b/include/param.h
> index 76a077fd758a..5d4f7f3db52f 100644
> --- a/include/param.h
> +++ b/include/param.h
> @@ -334,8 +334,7 @@ static inline struct param_d *dev_add_param_bool_ro(struct device *dev,
>  
>  static inline struct param_d *dev_add_param_string_ro(struct device *dev,
>  						      const char *name,
> -						      char **value,
> -						      void *priv)
> +						      char **value)
>  {
>  	return dev_add_param_string(dev, name, param_set_readonly, NULL, value, NULL);
>  }

Wouldn't it be better to pass *priv on to dev_add_param_string() instead
of removing it?

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 |

Re: [PATCH master 1/3] param: drop priv from dev_add_param_string_ro

[Ahmad Fatoum]

On 22.08.23 16:28, Sascha Hauer wrote:
> On Tue, Aug 22, 2023 at 09:47:36AM +0200, Ahmad Fatoum wrote:
>> dev_add_param_string_ro() is used nowhere, but when we do, having an
>> unused priv parameter serves no purpose, so drop it.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  include/param.h | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/include/param.h b/include/param.h
>> index 76a077fd758a..5d4f7f3db52f 100644
>> --- a/include/param.h
>> +++ b/include/param.h
>> @@ -334,8 +334,7 @@ static inline struct param_d *dev_add_param_bool_ro(struct device *dev,
>>  
>>  static inline struct param_d *dev_add_param_string_ro(struct device *dev,
>>  						      const char *name,
>> -						      char **value,
>> -						      void *priv)
>> +						      char **value)
>>  {
>>  	return dev_add_param_string(dev, name, param_set_readonly, NULL, value, NULL);
>>  }
> 
> Wouldn't it be better to pass *priv on to dev_add_param_string() instead
> of removing it?

No, because it would be unused: The non-overridable setters and getters
won't access it.

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

Re: [PATCH master 1/3] param: drop priv from dev_add_param_string_ro

[Sascha Hauer]

On Tue, Aug 22, 2023 at 06:36:07PM +0200, Ahmad Fatoum wrote:
> On 22.08.23 16:28, Sascha Hauer wrote:
> > On Tue, Aug 22, 2023 at 09:47:36AM +0200, Ahmad Fatoum wrote:
> >> dev_add_param_string_ro() is used nowhere, but when we do, having an
> >> unused priv parameter serves no purpose, so drop it.
> >>
> >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >> ---
> >>  include/param.h | 3 +--
> >>  1 file changed, 1 insertion(+), 2 deletions(-)
> >>
> >> diff --git a/include/param.h b/include/param.h
> >> index 76a077fd758a..5d4f7f3db52f 100644
> >> --- a/include/param.h
> >> +++ b/include/param.h
> >> @@ -334,8 +334,7 @@ static inline struct param_d *dev_add_param_bool_ro(struct device *dev,
> >>  
> >>  static inline struct param_d *dev_add_param_string_ro(struct device *dev,
> >>  						      const char *name,
> >> -						      char **value,
> >> -						      void *priv)
> >> +						      char **value)
> >>  {
> >>  	return dev_add_param_string(dev, name, param_set_readonly, NULL, value, NULL);
> >>  }
> > 
> > Wouldn't it be better to pass *priv on to dev_add_param_string() instead
> > of removing it?
> 
> No, because it would be unused: The non-overridable setters and getters
> won't access it.

Ah, right. Applied then

Thanks
  Sascha

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