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

[Sascha Hauer]

On Tue, May 03, 2022 at 06:42:04AM +0200, Oleksij Rempel wrote:
> Add generic function to extend/fixup machine compatible.
> 
> Signed-off-by: Oleksij Rempel <o.rempel at pengutronix.de>
> ---
>  common/Kconfig   |  5 +++++
>  common/misc.c    | 23 +++++++++++++++++++++++
>  common/oftree.c  | 43 +++++++++++++++++++++++++++++++++++++++++++
>  include/common.h |  3 +++
>  include/of.h     |  6 ++++++
>  5 files changed, 80 insertions(+)
> 
> diff --git a/common/Kconfig b/common/Kconfig
> index f7a6a96e87..7385848dfc 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

Once again: s/info/into/


> +
>  config SYSTEMD_OF_WATCHDOG
>  	bool "inform devicetree-enabled kernel of used watchdog"
>  	depends on WATCHDOG && OFTREE && FLEXIBLE_BOOTARGS
> 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 995e7201bf..5b183192bc 100644
> --- a/common/oftree.c
> +++ b/common/oftree.c
> @@ -212,6 +212,15 @@ static int of_fixup_bootargs(struct device_node *root, void *unused)
>  	if (serialno)
>  		of_property_write_string(root, "serial-number", serialno);
>  
> +	if (IS_ENABLED(CONFIG_MACHINE_FIXUP)) {

The variable itself is added unconditionally, but it's used only when
CONFIG_MACHINE_FIXUP is enabled. This seems inconsistent. I don't think
we need this option, so drop it.

> +		const char *compat;
> +
> +		compat = getenv("global.of.kernel.add_machine_compatible");

You introduced barebox_get_of_machine_compatible() with this patch.
Please use it.

> +		if (compat)
> +			of_prepend_machine_compatible(root, compat);
> +
> +	}
> +
>  	node = of_create_node(root, "/chosen");
>  	if (!node)
>  		return -ENOMEM;
> @@ -483,3 +492,37 @@ int of_autoenable_i2c_by_component(char *path)
>  
>  	return ret;
>  }
> +
> +int of_prepend_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;
> +	}
> +
> +	if (of_machine_is_compatible(compat)) {

You have to check the device tree you are adding the compatible to, not
the barebox internal device tree. These are not necessarily the same.

> +		pr_warn("Machine is already compatible to %s. Skip compatible modification\n",
> +			compat);

This might be a regular usecase, I don't think we need to warn here.

> +		return 0;
> +	}
> +
> +	curcompat = of_get_property(root, "compatible", &cclen);
> +
> +	buf = xzalloc(cclen + clen);
> +
> +	memcpy(buf, compat, clen);
> +
> +	if (curcompat)
> +		memcpy(buf + clen, curcompat, cclen);
> +
> +	of_set_property(root, "compatible", buf, cclen + clen, true);
> +
> +	free(buf);
> +
> +	return 0;
> +}

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 |