[PATCH 0/4] Introduce class devices

[PATCH 0/4] Introduce class devices

[Sascha Hauer]

Several subsystems already have a struct device embedded in their
subsystems specific structs. Allow to register these as class devices.
This way we collect devices of the same type on a list and can show
informations about these in a generic way.

Sascha Hauer (4):
  net: use for_each_netdev()
  base: add class device support
  net: register eth class
  watchdog: register watchdog class

 commands/Kconfig                  |  6 +++++
 commands/Makefile                 |  1 +
 commands/class.c                  | 30 ++++++++++++++++++++++
 drivers/base/Makefile             |  1 +
 drivers/base/class.c              | 41 +++++++++++++++++++++++++++++++
 drivers/watchdog/wd_core.c        | 14 ++++++-----
 include/asm-generic/barebox.lds.h |  7 ++++++
 include/device.h                  | 21 ++++++++++++++++
 include/net.h                     |  4 +--
 net/eth.c                         | 12 +++++----
 net/ifup.c                        |  2 +-
 11 files changed, 125 insertions(+), 14 deletions(-)
 create mode 100644 commands/class.c
 create mode 100644 drivers/base/class.c

-- 
2.39.2

[PATCH 1/4] net: use for_each_netdev()

[Sascha Hauer]

We have a define to iterate over all network interfaces, use it on the
remaining places which still iterate over the list manually.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 net/eth.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/eth.c b/net/eth.c
index 98567d8d3f..26a36474f8 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -535,7 +535,7 @@ struct eth_device *of_find_eth_device_by_node(struct device_node *np)
 	 */
 	(void)of_device_ensure_probed(np);
 
-	list_for_each_entry(edev, &netdev_list, list)
+	for_each_netdev(edev)
 		if (edev->parent->of_node == np)
 			return edev;
 	return NULL;
@@ -546,7 +546,7 @@ void eth_open_all(void)
 {
 	struct eth_device *edev;
 
-	list_for_each_entry(edev, &netdev_list, list) {
+	for_each_netdev(edev) {
 		if (edev->global_mode == ETH_MODE_DISABLED)
 			continue;
 		eth_open(edev);
@@ -559,7 +559,7 @@ static int populate_ethaddr(void)
 	bool generated = false;
 	int ret;
 
-	list_for_each_entry(edev, &netdev_list, list) {
+	for_each_netdev(edev) {
 		if (!edev->parent || is_valid_ether_addr(edev->ethaddr))
 			continue;
 
-- 
2.39.2

[PATCH 2/4] base: add class device support

[Sascha Hauer]

This introduces the concept of class devices in barebox. Class devices
group together devices of the same type. Several subsystems like
watchdog and network devices have a struct device embedded in their
subsystem specific struct anyway, so we can use this as a class device.
As these class devices are collected on a list we can use this list to
iterate over all network/watchdog devices and thus free the subsystems
from the burden of keeping a list themselves.

There is a 'class' command added in this patch which can be used to show
all registered classes along with the devices registered for each class.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 commands/Kconfig                  |  6 +++++
 commands/Makefile                 |  1 +
 commands/class.c                  | 30 ++++++++++++++++++++++
 drivers/base/Makefile             |  1 +
 drivers/base/class.c              | 41 +++++++++++++++++++++++++++++++
 include/asm-generic/barebox.lds.h |  7 ++++++
 include/device.h                  | 21 ++++++++++++++++
 7 files changed, 107 insertions(+)
 create mode 100644 commands/class.c
 create mode 100644 drivers/base/class.c

diff --git a/commands/Kconfig b/commands/Kconfig
index 899673bfee..7831e6276d 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -68,6 +68,12 @@ config CMD_BOOTROM
 
 	  bootrom [-la]
 
+config CMD_CLASS
+	tristate
+	prompt "class"
+	help
+	  Show information about registered classes and devices
+
 config CMD_DEVINFO
 	tristate
 	default y
diff --git a/commands/Makefile b/commands/Makefile
index 30e1f8403e..f65187703f 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -115,6 +115,7 @@ obj-$(CONFIG_CMD_BAREBOX_UPDATE)+= barebox-update.o
 obj-$(CONFIG_CMD_MIITOOL)	+= miitool.o
 obj-$(CONFIG_CMD_DETECT)	+= detect.o
 obj-$(CONFIG_CMD_BOOT)		+= boot.o
+obj-$(CONFIG_CMD_CLASS)		+= class.o
 obj-$(CONFIG_CMD_DEVINFO)	+= devinfo.o
 obj-$(CONFIG_CMD_DEVUNBIND)	+= devunbind.o
 obj-$(CONFIG_CMD_DEVLOOKUP)	+= devlookup.o
diff --git a/commands/class.c b/commands/class.c
new file mode 100644
index 0000000000..f5d67b012f
--- /dev/null
+++ b/commands/class.c
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <common.h>
+#include <command.h>
+#include <complete.h>
+
+static int do_class(int argc, char *argv[])
+{
+	struct class *class;
+	struct device *dev;
+
+	class_for_each(class) {
+		printf("%s:\n", class->name);
+		class_for_each_device(class, dev) {
+			printf("    %s\n", dev_name(dev));
+		}
+	}
+
+	return 0;
+}
+
+BAREBOX_CMD_HELP_START(class)
+BAREBOX_CMD_HELP_TEXT("Show information about registered classes and their devices")
+BAREBOX_CMD_HELP_END
+
+BAREBOX_CMD_START(class)
+	.cmd = do_class,
+	BAREBOX_CMD_DESC("show information about classes")
+	BAREBOX_CMD_GROUP(CMD_GRP_INFO)
+	BAREBOX_CMD_COMPLETE(empty_complete)
+BAREBOX_CMD_END
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index acc53763da..94b677eea0 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -3,6 +3,7 @@ obj-y	+= bus.o
 obj-y	+= driver.o
 obj-y	+= platform.o
 obj-y	+= resource.o
+obj-y	+= class.o
 obj-y	+= regmap/
 
 obj-$(CONFIG_PM_GENERIC_DOMAINS) += power.o
diff --git a/drivers/base/class.c b/drivers/base/class.c
new file mode 100644
index 0000000000..6757d34140
--- /dev/null
+++ b/drivers/base/class.c
@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <device.h>
+#include <driver.h>
+#include <linux/list.h>
+
+LIST_HEAD(class_list);
+
+static int class_register(struct class *class)
+{
+	list_add_tail(&class->list, &class_list);
+
+	return 0;
+}
+
+int class_add_device(struct class *class, struct device *dev)
+{
+	list_add_tail(&dev->class_list, &class->devices);
+
+	return 0;
+}
+
+void class_remove_device(struct class *class, struct device *dev)
+{
+	list_del(&class->devices);
+}
+
+extern struct class __barebox_class_start;
+extern struct class __barebox_class_end;
+
+static int register_classes(void)
+{
+	struct class *c;
+
+	for (c = &__barebox_class_start;
+	     c != &__barebox_class_end;
+	     c++)
+		class_register(c);
+
+	return 0;
+}
+device_initcall(register_classes);
diff --git a/include/asm-generic/barebox.lds.h b/include/asm-generic/barebox.lds.h
index d3736ebaed..8bbf5907cd 100644
--- a/include/asm-generic/barebox.lds.h
+++ b/include/asm-generic/barebox.lds.h
@@ -70,6 +70,12 @@
 	KEEP(*(SORT_BY_NAME(.barebox_magicvar*)))	\
 	__barebox_magicvar_end = .;
 
+#define BAREBOX_CLASSES				\
+	STRUCT_ALIGN();				\
+	__barebox_class_start = .;		\
+	KEEP(*(SORT_BY_NAME(.barebox_class*)))	\
+	__barebox_class_end = .;
+
 #define BAREBOX_CLK_TABLE			\
 	STRUCT_ALIGN();				\
 	__clk_of_table_start = .;		\
@@ -138,6 +144,7 @@
 	BAREBOX_SYMS				\
 	KERNEL_CTORS()				\
 	BAREBOX_MAGICVARS			\
+	BAREBOX_CLASSES				\
 	BAREBOX_CLK_TABLE			\
 	BAREBOX_DTB				\
 	BAREBOX_RSA_KEYS			\
diff --git a/include/device.h b/include/device.h
index ad1bc7ca1e..38eab6037e 100644
--- a/include/device.h
+++ b/include/device.h
@@ -76,6 +76,8 @@ struct device {
 
 	struct list_head cdevs;
 
+	struct list_head class_list;
+
 	const struct platform_device_id *id_entry;
 	union {
 		struct device_node *device_node;
@@ -102,6 +104,25 @@ struct device {
 	char *deferred_probe_reason;
 };
 
+struct class {
+	const char *name;
+	struct list_head devices;
+	struct list_head list;
+};
+
+#define DECLARE_CLASS(_name, _classname)					\
+	struct class _name __ll_elem(.barebox_class_##_name) = {		\
+		.name = _classname,						\
+		.devices = LIST_HEAD_INIT(_name.devices),			\
+	}
+
+int class_add_device(struct class *class, struct device *dev);
+void class_remove_device(struct class *class, struct device *dev);
+
+extern struct list_head class_list;
+#define class_for_each_device(class, dev) list_for_each_entry(dev, &class->devices, class_list)
+#define class_for_each(class) list_for_each_entry(class, &class_list, list)
+
 struct device_alias {
 	struct device *dev;
 	struct list_head list;
-- 
2.39.2

Re: [PATCH 2/4] base: add class device support

[Ahmad Fatoum]

Hello Sascha,

On 10.06.24 10:12, Sascha Hauer wrote:
> This introduces the concept of class devices in barebox. Class devices
> group together devices of the same type. Several subsystems like
> watchdog and network devices have a struct device embedded in their
> subsystem specific struct anyway, so we can use this as a class device.
> As these class devices are collected on a list we can use this list to
> iterate over all network/watchdog devices and thus free the subsystems
> from the burden of keeping a list themselves.
> 
> There is a 'class' command added in this patch which can be used to show
> all registered classes along with the devices registered for each class.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  commands/Kconfig                  |  6 +++++
>  commands/Makefile                 |  1 +
>  commands/class.c                  | 30 ++++++++++++++++++++++
>  drivers/base/Makefile             |  1 +
>  drivers/base/class.c              | 41 +++++++++++++++++++++++++++++++
>  include/asm-generic/barebox.lds.h |  7 ++++++
>  include/device.h                  | 21 ++++++++++++++++
>  7 files changed, 107 insertions(+)
>  create mode 100644 commands/class.c
>  create mode 100644 drivers/base/class.c
> 
> diff --git a/commands/Kconfig b/commands/Kconfig
> index 899673bfee..7831e6276d 100644
> --- a/commands/Kconfig
> +++ b/commands/Kconfig
> @@ -68,6 +68,12 @@ config CMD_BOOTROM
>  
>  	  bootrom [-la]
>  
> +config CMD_CLASS
> +	tristate
> +	prompt "class"
> +	help
> +	  Show information about registered classes and devices

s/classes and devices/device classes/.

> +#define BAREBOX_CLASSES				\
> +	STRUCT_ALIGN();				\
> +	__barebox_class_start = .;		\
> +	KEEP(*(SORT_BY_NAME(.barebox_class*)))	\
> +	__barebox_class_end = .;

While I don't mind using linker lists for this, can you add a note
to the commit message why you decided against dynamic allocation?

> +struct class {
> +	const char *name;
> +	struct list_head devices;
> +	struct list_head list;
> +};
> +
> +#define DECLARE_CLASS(_name, _classname)					\

This is a definition, not ony a declaration and
Linux has DEFINE_CLASS defined in <linux/cleanup.h>.

How about DEFINE_DEV_CLASS?

> +	struct class _name __ll_elem(.barebox_class_##_name) = {		\
> +		.name = _classname,						\
> +		.devices = LIST_HEAD_INIT(_name.devices),			\
> +	}

Cheers,
Ahmad

-- 
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 2/4] base: add class device support

[Sascha Hauer]

On Mon, Jun 10, 2024 at 10:33:43AM +0200, Ahmad Fatoum wrote:
> Hello Sascha,
> 
> On 10.06.24 10:12, Sascha Hauer wrote:
> > This introduces the concept of class devices in barebox. Class devices
> > group together devices of the same type. Several subsystems like
> > watchdog and network devices have a struct device embedded in their
> > subsystem specific struct anyway, so we can use this as a class device.
> > As these class devices are collected on a list we can use this list to
> > iterate over all network/watchdog devices and thus free the subsystems
> > from the burden of keeping a list themselves.
> > 
> > There is a 'class' command added in this patch which can be used to show
> > all registered classes along with the devices registered for each class.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  commands/Kconfig                  |  6 +++++
> >  commands/Makefile                 |  1 +
> >  commands/class.c                  | 30 ++++++++++++++++++++++
> >  drivers/base/Makefile             |  1 +
> >  drivers/base/class.c              | 41 +++++++++++++++++++++++++++++++
> >  include/asm-generic/barebox.lds.h |  7 ++++++
> >  include/device.h                  | 21 ++++++++++++++++
> >  7 files changed, 107 insertions(+)
> >  create mode 100644 commands/class.c
> >  create mode 100644 drivers/base/class.c
> > 
> > diff --git a/commands/Kconfig b/commands/Kconfig
> > index 899673bfee..7831e6276d 100644
> > --- a/commands/Kconfig
> > +++ b/commands/Kconfig
> > @@ -68,6 +68,12 @@ config CMD_BOOTROM
> >  
> >  	  bootrom [-la]
> >  
> > +config CMD_CLASS
> > +	tristate
> > +	prompt "class"
> > +	help
> > +	  Show information about registered classes and devices
> 
> s/classes and devices/device classes/.
> 
> > +#define BAREBOX_CLASSES				\
> > +	STRUCT_ALIGN();				\
> > +	__barebox_class_start = .;		\
> > +	KEEP(*(SORT_BY_NAME(.barebox_class*)))	\
> > +	__barebox_class_end = .;
> 
> While I don't mind using linker lists for this, can you add a note
> to the commit message why you decided against dynamic allocation?

Sure. I decided for a linker list because otherwise we would have to add
an additional initcall to each subsystem using it. That's fine to do,
but we would also have to make sure this initcall executes before the
first user registers a device. Alternatively we could do something like
this in eth_register():

	static bool class_registered;

	if (!class_registered) {
		class_register(&eth_class);
		class_registered = true;
	}

I don't have a strong preference, but I think from these alternatives I
like linker lists best.

> 
> > +struct class {
> > +	const char *name;
> > +	struct list_head devices;
> > +	struct list_head list;
> > +};
> > +
> > +#define DECLARE_CLASS(_name, _classname)					\
> 
> This is a definition, not ony a declaration and
> Linux has DEFINE_CLASS defined in <linux/cleanup.h>.
> 
> How about DEFINE_DEV_CLASS?

Ok.

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 |

[PATCH 3/4] net: register eth class

[Sascha Hauer]

Register network devices as class devices. This allows us to implement
for_each_netdev() by iterating over the network class devices.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 include/net.h | 4 ++--
 net/eth.c     | 6 ++++--
 net/ifup.c    | 2 +-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/include/net.h b/include/net.h
index 5a6dd9ca7b..edfea0d105 100644
--- a/include/net.h
+++ b/include/net.h
@@ -599,8 +599,8 @@ void ifdown_edev(struct eth_device *edev);
 int ifdown(const char *name);
 void ifdown_all(void);
 
-extern struct list_head netdev_list;
+#define for_each_netdev(netdev) list_for_each_entry(netdev, &eth_class.devices, dev.class_list)
 
-#define for_each_netdev(netdev) list_for_each_entry(netdev, &netdev_list, list)
+extern struct class eth_class;
 
 #endif /* __NET_H__ */
diff --git a/net/eth.c b/net/eth.c
index 26a36474f8..74386d6e7b 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -21,7 +21,7 @@
 #include <linux/ctype.h>
 #include <linux/stat.h>
 
-LIST_HEAD(netdev_list);
+DECLARE_CLASS(eth_class, "eth");
 
 struct eth_ethaddr {
 	struct list_head list;
@@ -445,7 +445,7 @@ int eth_register(struct eth_device *edev)
 	if (edev->init)
 		edev->init(edev);
 
-	list_add_tail(&edev->list, &netdev_list);
+	class_add_device(&eth_class, &edev->dev);
 
 	ret = eth_get_registered_ethaddr(edev, ethaddr);
 	if (!ret)
@@ -512,6 +512,8 @@ void eth_unregister(struct eth_device *edev)
 	if (IS_ENABLED(CONFIG_OFDEVICE))
 		free(edev->nodepath);
 
+	class_remove_device(&eth_class, &edev->dev);
+
 	free(edev->devname);
 
 	unregister_device(&edev->dev);
diff --git a/net/ifup.c b/net/ifup.c
index 5b92ee794d..41c1942670 100644
--- a/net/ifup.c
+++ b/net/ifup.c
@@ -358,7 +358,7 @@ int ifup_all(unsigned flags)
 	closedir(dir);
 
 	if ((flags & IFUP_FLAG_FORCE) || net_ifup_force_detect ||
-	    list_empty(&netdev_list))
+	    list_empty(&eth_class.devices))
 		device_detect_all();
 
 	/*
-- 
2.39.2

[PATCH 4/4] watchdog: register watchdog class

[Sascha Hauer]

Register watchdog devices as class devices. This allows us to implement
for_each_watchdog() by iterating over the network class devices.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/watchdog/wd_core.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/watchdog/wd_core.c b/drivers/watchdog/wd_core.c
index 42cbd7b72c..7f3ba083e5 100644
--- a/drivers/watchdog/wd_core.c
+++ b/drivers/watchdog/wd_core.c
@@ -11,7 +11,9 @@
 #include <watchdog.h>
 #include <restart.h>
 
-static LIST_HEAD(watchdog_list);
+#define for_each_watchdog(wd) list_for_each_entry(wd, &watchdog_class.devices, dev.class_list)
+
+DECLARE_CLASS(watchdog_class, "watchdog");
 
 static const char *watchdog_name(struct watchdog *wd)
 {
@@ -267,7 +269,7 @@ int watchdog_register(struct watchdog *wd)
 			dev_warn(&wd->dev, "failed to register restart handler\n");
 	}
 
-	list_add_tail(&wd->list, &watchdog_list);
+	class_add_device(&watchdog_class, &wd->dev);
 
 	pr_debug("registering watchdog %s with priority %d\n", watchdog_name(wd),
 			wd->priority);
@@ -287,7 +289,7 @@ int watchdog_deregister(struct watchdog *wd)
 		poller_async_unregister(&wd->poller);
 	}
 
-	unregister_device(&wd->dev);
+	class_remove_device(&watchdog_class, &wd->dev);
 	list_del(&wd->list);
 
 	return 0;
@@ -299,7 +301,7 @@ struct watchdog *watchdog_get_default(void)
 	struct watchdog *tmp, *wd = NULL;
 	int priority = 0;
 
-	list_for_each_entry(tmp, &watchdog_list, list) {
+	for_each_watchdog(tmp) {
 		if (tmp->priority > priority) {
 			priority = tmp->priority;
 			wd = tmp;
@@ -336,7 +338,7 @@ struct watchdog *watchdog_get_by_name(const char *name)
 	if (!dev)
 		return NULL;
 
-	list_for_each_entry(tmp, &watchdog_list, list) {
+	for_each_watchdog(tmp) {
 		if (dev == tmp->hwdev || dev == &tmp->dev)
 			return tmp;
 	}
@@ -350,7 +352,7 @@ int watchdog_inhibit_all(void)
 	struct watchdog *wd;
 	int ret = 0;
 
-	list_for_each_entry(wd, &watchdog_list, list) {
+	for_each_watchdog(wd) {
 		int err;
 		if (!wd->priority || watchdog_hw_running(wd) == false)
 			continue;
-- 
2.39.2