[PATCH v2 0/6] commands: implement of_compatible command

[PATCH v2 0/6] commands: implement of_compatible command

[Ahmad Fatoum]

This enables scripts to do:

  if of_compatible -k radxa,rock3a radxa,rock5a ; then
    of_property -df mmc0 sd-uhs-sdr104
  fi

instead of checking $global.model. See individual patches for changelog.

Ahmad Fatoum (6):
  treewide: don't mix goto labels and statements on same line
  treewide: drop null pointer checks around of_delete_node
  commands: of_property: use new of_read_file
  commands: of_dump: remove duplicate error message
  commands: of_dump: use of_dup instead of flattening/unflattening
  commands: implement of_compatible command

 commands/Kconfig                 |  15 ++++
 commands/Makefile                |   1 +
 commands/of_compatible.c         | 124 +++++++++++++++++++++++++++++++
 commands/of_dump.c               |  28 ++-----
 commands/of_property.c           |  16 +---
 common/blspec.c                  |   3 +-
 common/state/state.c             |  15 ++--
 common/state/state_variables.c   |   9 ++-
 drivers/usb/gadget/udc/fsl_udc.c |   3 +-
 9 files changed, 168 insertions(+), 46 deletions(-)
 create mode 100644 commands/of_compatible.c

-- 
2.39.2

[PATCH v2 1/6] treewide: don't mix goto labels and statements on same line

[Ahmad Fatoum]

Coding style across the project is not to indent goto labels and
to not mix them with code on the same line. Let's fix the few outliers
that are there.

No functional change.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - new patch
---
 common/state/state.c             | 12 ++++++++----
 common/state/state_variables.c   |  9 ++++++---
 drivers/usb/gadget/udc/fsl_udc.c |  3 ++-
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/common/state/state.c b/common/state/state.c
index cabe285fbdf3..040835702b66 100644
--- a/common/state/state.c
+++ b/common/state/state.c
@@ -333,8 +333,10 @@ static int state_convert_node_variable(struct state *state,
 		goto out;
 
 	return 0;
- out_free:free(name);
- out:	return ret;
+out_free:
+	free(name);
+out:
+	return ret;
 }
 
 struct device_node *state_to_node(struct state *state,
@@ -361,7 +363,8 @@ struct device_node *state_to_node(struct state *state,
 	}
 
 	return root;
- out:	of_delete_node(root);
+out:
+	of_delete_node(root);
 	return ERR_PTR(ret);
 }
 
@@ -561,7 +564,8 @@ static int of_state_fixup(struct device_node *root, void *ctx)
 
 	return 0;
 
- out:	of_delete_node(new_node);
+out:
+	of_delete_node(new_node);
 	return ret;
 }
 
diff --git a/common/state/state_variables.c b/common/state/state_variables.c
index cb85f3a92602..77946206cd02 100644
--- a/common/state/state_variables.c
+++ b/common/state/state_variables.c
@@ -263,7 +263,8 @@ static struct state_variable *state_enum32_create(struct state *state,
 	}
 
 	return &enum32->var;
- out:	for (i--; i >= 0; i--)
+out:
+	for (i--; i >= 0; i--)
 		free((char *)enum32->names[i]);
 	free(enum32->names);
 	free(enum32);
@@ -329,7 +330,8 @@ static struct state_variable *state_mac_create(struct state *state,
 	}
 
 	return &mac->var;
- out:	free(mac);
+out:
+	free(mac);
 	return ERR_PTR(ret);
 }
 
@@ -444,7 +446,8 @@ static struct state_variable *state_string_create(struct state *state,
 	}
 
 	return &string->var;
- out:	free(string);
+out:
+	free(string);
 	return ERR_PTR(ret);
 }
 
diff --git a/drivers/usb/gadget/udc/fsl_udc.c b/drivers/usb/gadget/udc/fsl_udc.c
index 6a1d4304df1e..81e051a073c0 100644
--- a/drivers/usb/gadget/udc/fsl_udc.c
+++ b/drivers/usb/gadget/udc/fsl_udc.c
@@ -980,7 +980,8 @@ static int fsl_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
 	done(ep, req, -ECONNRESET);
 
 	/* Enable EP */
-out:	epctrl = readl(&dr_regs->endptctrl[ep_num]);
+out:
+	epctrl = readl(&dr_regs->endptctrl[ep_num]);
 	if (ep_is_in(ep))
 		epctrl |= EPCTRL_TX_ENABLE;
 	else
-- 
2.39.2

[PATCH v2 2/6] treewide: drop null pointer checks around of_delete_node

[Ahmad Fatoum]

of_delete_node is a no-op when called on a null pointer, so remove the
useless null checks around it.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - new patch
---
 commands/of_dump.c     | 3 +--
 commands/of_property.c | 3 +--
 common/blspec.c        | 3 +--
 common/state/state.c   | 3 +--
 4 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/commands/of_dump.c b/commands/of_dump.c
index 86755ff1e445..87a0b1ba535c 100644
--- a/commands/of_dump.c
+++ b/commands/of_dump.c
@@ -120,8 +120,7 @@ static int do_of_dump(int argc, char *argv[])
 		of_print_nodes(node, 0, maxpropsize);
 
 out:
-	if (of_free)
-		of_delete_node(of_free);
+	of_delete_node(of_free);
 
 	return ret;
 }
diff --git a/commands/of_property.c b/commands/of_property.c
index 063e97775c59..b507a3541e0a 100644
--- a/commands/of_property.c
+++ b/commands/of_property.c
@@ -401,8 +401,7 @@ static int do_of_property(int argc, char *argv[])
 
 out:
 
-	if (root)
-		of_delete_node(root);
+	of_delete_node(root);
 
 	return ret;
 }
diff --git a/common/blspec.c b/common/blspec.c
index 4b4e04b03997..e95a8dba8d76 100644
--- a/common/blspec.c
+++ b/common/blspec.c
@@ -473,8 +473,7 @@ static bool entry_is_of_compatible(struct blspec_entry *entry)
 	ret = false;
 
 out:
-	if (root)
-		of_delete_node(root);
+	of_delete_node(root);
 	free(filename);
 
 	return ret;
diff --git a/common/state/state.c b/common/state/state.c
index 040835702b66..6b4acbb32bcc 100644
--- a/common/state/state.c
+++ b/common/state/state.c
@@ -559,8 +559,7 @@ static int of_state_fixup(struct device_node *root, void *ctx)
 		goto out;
 
 	/* delete existing node */
-	if (node)
-		of_delete_node(node);
+	of_delete_node(node);
 
 	return 0;
 
-- 
2.39.2

[PATCH v2 3/6] commands: of_property: use new of_read_file

[Ahmad Fatoum]

Recent addition of of_read_file also involved switching over nearly all
open coded instances to use it. of_property seems the only remaining
instance where of_read_file can be used, but isn't. Switch it over.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - new patch
---
 commands/of_property.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/commands/of_property.c b/commands/of_property.c
index b507a3541e0a..0c914c55c6bf 100644
--- a/commands/of_property.c
+++ b/commands/of_property.c
@@ -305,7 +305,6 @@ static int do_of_property(int argc, char *argv[])
 	char *path, *propname;
 	char *dtbfile = NULL;
 	int ret = 0;
-	size_t size;
 	struct fdt_header *fdt = NULL;
 	struct device_node *root = NULL;
 
@@ -340,15 +339,9 @@ static int do_of_property(int argc, char *argv[])
 		return COMMAND_ERROR_USAGE;
 
 	if (dtbfile) {
-		fdt = read_file(dtbfile, &size);
-		if (!fdt) {
-			printf("unable to read %s: %m\n", dtbfile);
-			return -errno;
-		}
-
-		root = of_unflatten_dtb(fdt, size);
-
-		free(fdt);
+		root = of_read_file(dtbfile);
+		if (IS_ERR(root))
+			return PTR_ERR(root);
 	}
 
 	if (set) {
-- 
2.39.2

[PATCH v2 4/6] commands: of_dump: remove duplicate error message

[Ahmad Fatoum]

of_read_file will already print to log on failures, so we need not print
a second message in of_dump on error.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - new patch
---
 commands/of_dump.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/commands/of_dump.c b/commands/of_dump.c
index 87a0b1ba535c..f8e7280fd9f2 100644
--- a/commands/of_dump.c
+++ b/commands/of_dump.c
@@ -72,11 +72,8 @@ static int do_of_dump(int argc, char *argv[])
 
 	if (dtbfile) {
 		root = of_read_file(dtbfile);
-		if (IS_ERR(root)) {
-			printf("Cannot open %s: %pe\n", dtbfile, root);
-			ret = PTR_ERR(root);
-			goto out;
-		}
+		if (IS_ERR(root))
+			return PTR_ERR(root);
 
 		of_free = root;
 	} else {
-- 
2.39.2

[PATCH v2 5/6] commands: of_dump: use of_dup instead of flattening/unflattening

[Ahmad Fatoum]

Flattening and unflattening is needlessly laborious. We have of_dup
for that, so let's make use of it.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - new patch
---
 commands/of_dump.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/commands/of_dump.c b/commands/of_dump.c
index f8e7280fd9f2..1d6c019bf08c 100644
--- a/commands/of_dump.c
+++ b/commands/of_dump.c
@@ -79,21 +79,9 @@ static int do_of_dump(int argc, char *argv[])
 	} else {
 		root = of_get_root_node();
 
-		if (fix) {
-			/* create a copy of internal devicetree */
-			void *fdt;
-			fdt = of_flatten_dtb(root);
-			root = of_unflatten_dtb(fdt, fdt_totalsize(fdt));
-
-			free(fdt);
-
-			if (IS_ERR(root)) {
-				ret = PTR_ERR(root);
-				goto out;
-			}
-
-			of_free = root;
-		}
+		/* copy internal device tree to apply fixups onto it */
+		if (fix)
+			root = of_free = of_dup(root);
 	}
 
 	if (fix) {
-- 
2.39.2

[PATCH v2 6/6] commands: implement of_compatible command

[Ahmad Fatoum]

Currently, the usual way within scripts to differentiate between machines
is to compare global.model or global.hostname. Both are suboptimal,
because they may change between releases or overridden by the user.

In C code, the machine compatible is used for this purpose. Add a new
of_compatible command that makes of_machine_is_compatible/
of_device_is_compatible available to scripts. Example use:

/env/init/fixups:

  #!/bin/sh

  if of_compatible -k radxa,rock3a ; then
    of_property -df mmc0 sd-uhs-sdr104
  fi

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - fix typo in Kconfig help text (Sascha)
  - use of_dup/of_read_file (Sascha)
  - fix return code if compatibles don't match
  - support multiple compatibles
  - Kconfig whitespace adjustments
---
 commands/Kconfig         |  15 +++++
 commands/Makefile        |   1 +
 commands/of_compatible.c | 124 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 140 insertions(+)
 create mode 100644 commands/of_compatible.c

diff --git a/commands/Kconfig b/commands/Kconfig
index 9f64d90a5812..4e0cd2943116 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -2201,6 +2201,21 @@ config CMD_LSMOD
 	help
 	  List loaded barebox modules.
 
+config CMD_OF_COMPATIBLE
+	tristate
+	select OFTREE
+	prompt "of_compatible"
+	help
+	  Check DT node's compatible
+
+	  Usage: [-fFnk] [COMPAT]
+
+	  Options:
+		-f dtb	work on dtb instead of internal devicetree
+		-F	apply fixups on devicetree before compare
+		-n node	node path or alias to compare its compatible (default is /)
+		-k	compare $global.of.kernel.add_machine_compatible as well
+
 config CMD_OF_DIFF
 	tristate
 	select OFTREE
diff --git a/commands/Makefile b/commands/Makefile
index e5cc21f1970a..0ac84076f83d 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -82,6 +82,7 @@ obj-$(CONFIG_CMD_USB)		+= usb.o
 obj-$(CONFIG_CMD_TIME)		+= time.o
 obj-$(CONFIG_CMD_UPTIME)	+= uptime.o
 obj-$(CONFIG_CMD_OFTREE)	+= oftree.o
+obj-$(CONFIG_CMD_OF_COMPATIBLE)	+= of_compatible.o
 obj-$(CONFIG_CMD_OF_DIFF)	+= of_diff.o
 obj-$(CONFIG_CMD_OF_PROPERTY)	+= of_property.o
 obj-$(CONFIG_CMD_OF_NODE)	+= of_node.o
diff --git a/commands/of_compatible.c b/commands/of_compatible.c
new file mode 100644
index 000000000000..b460fecd3a86
--- /dev/null
+++ b/commands/of_compatible.c
@@ -0,0 +1,124 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// SPDX-FileCopyrightText: © 2023 Ahmad Fatoum <a.fatoum@pengutronix.de>
+
+#include <common.h>
+#include <libfile.h>
+#include <fdt.h>
+#include <of.h>
+#include <command.h>
+#include <complete.h>
+#include <errno.h>
+#include <getopt.h>
+
+static int do_of_compatible(int argc, char *argv[])
+{
+	int opt;
+	int ret = 0;
+	bool fix = false, kernel_compat = false;
+	struct device_node *root = NULL, *node, *of_free = NULL;
+	char **compats, **compat, *dtbfile = NULL;
+	const char *nodename = "/";
+
+	while ((opt = getopt(argc, argv, "f:n:Fk")) > 0) {
+		switch (opt) {
+		case 'f':
+			dtbfile = optarg;
+			break;
+		case 'n':
+			nodename = optarg;
+			break;
+		case 'F':
+			fix = true;
+			break;
+		case 'k':
+			kernel_compat = true;
+			break;
+		default:
+			return COMMAND_ERROR_USAGE;
+		}
+	}
+
+	if (argc - optind < 1)
+		return COMMAND_ERROR_USAGE;
+
+	compats = &argv[optind];
+
+	if (dtbfile) {
+		root = of_read_file(dtbfile);
+		if (IS_ERR(root))
+			return PTR_ERR(root);
+
+		of_free = root;
+	} else {
+		root = of_get_root_node();
+
+		/* copy internal device tree to apply fixups onto it */
+		if (fix)
+			root = of_free = of_dup(root);
+	}
+
+	if (fix) {
+		ret = of_fix_tree(root);
+		if (ret)
+			goto out;
+	}
+
+	node = of_find_node_by_path_or_alias(root, nodename);
+	if (!node) {
+		printf("Cannot find nodepath %s\n", nodename);
+		ret = -ENOENT;
+		goto out;
+	}
+
+	ret = COMMAND_ERROR;
+
+	if (kernel_compat) {
+		const char *compat_override;
+
+		if (node->parent) {
+			printf("-k only valid for root node\n");
+			ret = COMMAND_ERROR_USAGE;
+			goto out;
+		}
+
+		compat_override = barebox_get_of_machine_compatible() ?: "";
+		for (compat = compats; *compat; compat++) {
+			if (strcmp(*compat, compat_override) == 0) {
+				ret = COMMAND_SUCCESS;
+				goto out;
+			}
+		}
+	}
+
+	for (compat = compats; *compat; compat++) {
+		int score;
+
+		score = of_device_is_compatible(node, *compat);
+		if (score > 0) {
+			ret = COMMAND_SUCCESS;
+			break;
+		}
+	}
+
+out:
+	of_delete_node(of_free);
+
+	return ret;
+}
+
+BAREBOX_CMD_HELP_START(of_compatible)
+BAREBOX_CMD_HELP_TEXT("Options:")
+BAREBOX_CMD_HELP_OPT  ("-f dtb",  "work on dtb instead of internal devicetree")
+BAREBOX_CMD_HELP_OPT  ("-F",      "apply fixups on devicetree before compare")
+BAREBOX_CMD_HELP_OPT  ("-n node", "node path or alias to compare its compatible (default is /)")
+BAREBOX_CMD_HELP_OPT  ("-k",      "compare $global.of.kernel.add_machine_compatible as well")
+BAREBOX_CMD_HELP_END
+
+BAREBOX_CMD_START(of_compatible)
+	.cmd		= do_of_compatible,
+	BAREBOX_CMD_DESC("Check DT node's compatible")
+	BAREBOX_CMD_OPTS("[-fFnk] [COMPATS..]")
+	BAREBOX_CMD_GROUP(CMD_GRP_MISC)
+	BAREBOX_CMD_COMPLETE(empty_complete)
+	BAREBOX_CMD_HELP(cmd_of_compatible_help)
+BAREBOX_CMD_END
-- 
2.39.2

Re: [PATCH v2 0/6] commands: implement of_compatible command

[Sascha Hauer]

On Fri, May 19, 2023 at 12:01:14PM +0200, Ahmad Fatoum wrote:
> This enables scripts to do:
> 
>   if of_compatible -k radxa,rock3a radxa,rock5a ; then
>     of_property -df mmc0 sd-uhs-sdr104
>   fi
> 
> instead of checking $global.model. See individual patches for changelog.
> 
> Ahmad Fatoum (6):
>   treewide: don't mix goto labels and statements on same line
>   treewide: drop null pointer checks around of_delete_node
>   commands: of_property: use new of_read_file
>   commands: of_dump: remove duplicate error message
>   commands: of_dump: use of_dup instead of flattening/unflattening
>   commands: implement of_compatible command
> 
>  commands/Kconfig                 |  15 ++++
>  commands/Makefile                |   1 +
>  commands/of_compatible.c         | 124 +++++++++++++++++++++++++++++++
>  commands/of_dump.c               |  28 ++-----
>  commands/of_property.c           |  16 +---
>  common/blspec.c                  |   3 +-
>  common/state/state.c             |  15 ++--
>  common/state/state_variables.c   |   9 ++-
>  drivers/usb/gadget/udc/fsl_udc.c |   3 +-
>  9 files changed, 168 insertions(+), 46 deletions(-)
>  create mode 100644 commands/of_compatible.c

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