Re: [PATCH v3 3/5] of: add generic of_fixup_machine_compatible()

[Sascha Hauer <sha@pengutronix.de>]

On Wed, Apr 27, 2022 at 02:14:11PM +0200, Oleksij Rempel wrote:
> Add generic function to extend/fixup machine compatible.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  common/Kconfig   |  5 +++++
>  common/misc.c    | 23 +++++++++++++++++++++++
>  common/oftree.c  | 41 +++++++++++++++++++++++++++++++++++++++++
>  include/common.h |  3 +++
>  include/of.h     |  6 ++++++
>  5 files changed, 78 insertions(+)
> 
> diff --git a/common/Kconfig b/common/Kconfig
> index d08302a573..9b65b728c0 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -1066,6 +1066,11 @@ config MACHINE_ID
>  	  Note: if no hashable information is available no machine id will be passed
>  	  to the kernel.
>  
> +config MACHINE_FIXUP
> +	bool "fix up machine compatible"
> +	help
> +	  fixup machine compatible supplied by board code info device tree

s/info/into/

> +
>  config SERIAL_NUMBER_FIXUP
>  	bool "fix up board serial number"
>  	help
> diff --git a/common/misc.c b/common/misc.c
> index 3b3bc05bfd..0f6de3e9e5 100644
> --- a/common/misc.c
> +++ b/common/misc.c
> @@ -150,6 +150,7 @@ BAREBOX_MAGICVAR(global.model, "Product name of this hardware");
>  
>  static char *hostname;
>  static char *serial_number;
> +static char *of_machine_compatible;
>  
>  /*
>   * The hostname is supposed to be the shortname of a board. It should
> @@ -195,6 +196,28 @@ const char *barebox_get_serial_number(void)
>  
>  BAREBOX_MAGICVAR(global.serial_number, "Board serial number");
>  
> +void barebox_set_of_machine_compatible(const char *__compatible)
> +{
> +	free(of_machine_compatible);
> +	of_machine_compatible = xstrdup(__compatible);
> +}
> +
> +const char *barebox_get_of_machine_compatible(void)
> +{
> +	return of_machine_compatible;
> +}
> +
> +static int of_kernel_init(void)
> +{
> +	globalvar_add_simple_string("of.kernel.add_machine_compatible",
> +				    &of_machine_compatible);
> +
> +	return 0;
> +}
> +device_initcall(of_kernel_init);
> +
> +BAREBOX_MAGICVAR(global.of.kernel.add_machine_compatible, "Additional machine/board compatible");
> +
>  void __noreturn panic(const char *fmt, ...)
>  {
>  	va_list args;
> diff --git a/common/oftree.c b/common/oftree.c
> index c6d75055cc..76239476e0 100644
> --- a/common/oftree.c
> +++ b/common/oftree.c
> @@ -216,6 +216,15 @@ static int of_fixup_bootargs(struct device_node *root, void *unused)
>  
>  	}
>  
> +	if (IS_ENABLED(CONFIG_MACHINE_FIXUP)) {
> +		const char *compat;
> +
> +		compat = getenv("global.of.kernel.add_machine_compatible");
> +		if (compat)
> +			of_fixup_machine_compatible(root, compat);
> +
> +	}
> +
>  	node = of_create_node(root, "/chosen");
>  	if (!node)
>  		return -ENOMEM;
> @@ -487,3 +496,35 @@ int of_autoenable_i2c_by_component(char *path)
>  
>  	return ret;
>  }
> +
> +int of_fixup_machine_compatible(struct device_node *root, const char *compat)
> +{
> +	int cclen = 0, clen = strlen(compat) + 1;
> +	const char *curcompat;
> +	void *buf;
> +
> +	if (!root) {
> +		root = of_get_root_node();
> +		if (!root)
> +			return -ENODEV;
> +	}
> +
> +	curcompat = of_get_property(root, "compatible", &cclen);
> +
> +	buf = xzalloc(cclen + clen);
> +
> +	memcpy(buf, compat, clen);
> +
> +	if (curcompat)
> +		memcpy(buf + clen, curcompat, cclen);
> +
> +	/*
> +	 * Prepend the compatible from board entry to the machine compatible.
> +	 * Used to match bootspec entries against it.
> +	 */

This sentence doesn't make sense in the context this function is called
in this patch. In this patch this function is called in a of_fixup which
fixes up the kernel device tree, but the sentence above reads as it
should be called on the barebox internal device tree.

maybe of_prepend_machine_compatible() would be a better name for this
function.

Also, should we check if the passed device tree already is compatible to
@compat before we potentially add the same compatible again?

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