[PATCH 1/4] init: mark initcalls const

[PATCH 1/4] init: mark initcalls const

[Ahmad Fatoum]

The initcalls shouldn't be modified at runtime and they are part of the
BAREBOX_RO_SECTION, so mark them const accordingly.

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

diff --git a/include/init.h b/include/init.h
index 33a76974f23d..7061f911a888 100644
--- a/include/init.h
+++ b/include/init.h
@@ -36,10 +36,10 @@ typedef void (*exitcall_t)(void);
 #ifndef __ASSEMBLY__
 
 #define __define_initcall(fn,id) \
-	static initcall_t __initcall_##fn##id __ll_elem(.initcall.##id) = fn
+	static const initcall_t __initcall_##fn##id __ll_elem(.initcall.##id) = fn
 
 #define __define_exitcall(fn,id) \
-	static exitcall_t __exitcall_##fn##id __ll_elem(.exitcall.##id) = fn
+	static const exitcall_t __exitcall_##fn##id __ll_elem(.exitcall.##id) = fn
 
 
 /*
-- 
2.39.2

[PATCH 2/4] base: use initcalls instead of linker lists to register classes

[Ahmad Fatoum]

BAREBOX_CLASSES is the only member of RO_DATA_SECTION that's indeed not
read-only. This will lead to issues when we start making to mark that
section read-only on sandbox.

Replace the linker list thus with an initcall.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/base/class.c              | 20 +-------------------
 include/asm-generic/barebox.lds.h |  7 -------
 include/device.h                  | 20 ++++++++++++++------
 3 files changed, 15 insertions(+), 32 deletions(-)

diff --git a/drivers/base/class.c b/drivers/base/class.c
index fd7d6b2d7a29..5fbbae0ff025 100644
--- a/drivers/base/class.c
+++ b/drivers/base/class.c
@@ -5,11 +5,9 @@
 
 LIST_HEAD(class_list);
 
-static int class_register(struct class *class)
+void class_register(struct class *class)
 {
 	list_add_tail(&class->list, &class_list);
-
-	return 0;
 }
 
 int class_add_device(struct class *class, struct device *dev)
@@ -23,19 +21,3 @@ void class_remove_device(struct class *class, struct device *dev)
 {
 	list_del(&dev->class_list);
 }
-
-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 8bbf5907cdfd..d3736ebaed59 100644
--- a/include/asm-generic/barebox.lds.h
+++ b/include/asm-generic/barebox.lds.h
@@ -70,12 +70,6 @@
 	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 = .;		\
@@ -144,7 +138,6 @@
 	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 8550544ca807..4c76da2f82dd 100644
--- a/include/device.h
+++ b/include/device.h
@@ -7,6 +7,8 @@
 #define DEVICE_H
 
 #include <linux/types.h>
+#include <linux/list.h>
+#include <init.h>
 
 enum dev_dma_coherence {
 	DEV_DMA_COHERENCE_DEFAULT = 0,
@@ -110,12 +112,18 @@ struct class {
 	struct list_head list;
 };
 
-#define DEFINE_DEV_CLASS(_name, _classname)					\
-	struct class _name __ll_elem(.barebox_class_##_name) 			\
-	__aligned(__alignof__(struct class)) = {				\
-		.name = _classname,						\
-		.devices = LIST_HEAD_INIT(_name.devices),			\
-	}
+void class_register(struct class *class);
+
+#define DEFINE_DEV_CLASS(_name, _classname)			\
+	struct class _name = {					\
+		.name = _classname,				\
+		.devices = LIST_HEAD_INIT(_name.devices),	\
+	};							\
+	static int register_##_name(void) {			\
+		class_register(&_name);				\
+		return 0;					\
+	}							\
+	pure_initcall(register_##_name);
 
 int class_add_device(struct class *class, struct device *dev);
 void class_remove_device(struct class *class, struct device *dev);
-- 
2.39.2

[PATCH 3/4] watchdog: remove needless error checking for device parameter

[Ahmad Fatoum]

Poller and device parameter are attached to the class device, either
directly or by name. It's thus not really possible for them to fail,
because of a naming conflict, which only leaves the -ENOMEM case as
possible failure.

So far, we failed the watchdog registration, but ultimately device
parameters are optional: If barebox can still limp along in this
low-memory situation, the user is better served by a barebox that can
control the watchdog via the wd command than one that has no watchdog at
all.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/watchdog/wd_core.c | 54 +++++++-------------------------------
 1 file changed, 10 insertions(+), 44 deletions(-)

diff --git a/drivers/watchdog/wd_core.c b/drivers/watchdog/wd_core.c
index 90ae233b0f9e..44f2b6da8cf2 100644
--- a/drivers/watchdog/wd_core.c
+++ b/drivers/watchdog/wd_core.c
@@ -111,19 +111,12 @@ static int watchdog_set_poller(struct param_d *param, void *priv)
 	return 0;
 }
 
-static int watchdog_register_poller(struct watchdog *wd)
+static void watchdog_register_poller(struct watchdog *wd)
 {
-	struct param_d *p;
-	int ret;
+	poller_async_register(&wd->poller, dev_name(&wd->dev));
 
-	ret = poller_async_register(&wd->poller, dev_name(&wd->dev));
-	if (ret)
-		return ret;
-
-	p = dev_add_param_bool(&wd->dev, "autoping", watchdog_set_poller,
-			NULL, &wd->poller_enable, wd);
-
-	return PTR_ERR_OR_ZERO(p);
+	dev_add_param_bool(&wd->dev, "autoping", watchdog_set_poller,
+			   NULL, &wd->poller_enable, wd);
 }
 
 static int watchdog_register_dev(struct watchdog *wd, const char *name, int id)
@@ -194,7 +187,6 @@ static struct restart_handler restart_handler = {
 
 int watchdog_register(struct watchdog *wd)
 {
-	struct param_d *p;
 	const char *alias = NULL;
 	int ret = 0;
 
@@ -210,57 +202,35 @@ int watchdog_register(struct watchdog *wd)
 	if (ret)
 		return ret;
 
-	p = dev_add_param_tristate_ro(&wd->dev, "running", &wd->running);
-	if (IS_ERR(p)) {
-		ret = PTR_ERR(p);
-		goto error_unregister;
-	}
+	dev_add_param_tristate_ro(&wd->dev, "running", &wd->running);
 
 	if (!wd->priority)
 		wd->priority = dev_get_watchdog_priority(wd->hwdev);
 
-	p = dev_add_param_uint32(&wd->dev, "priority",
+	dev_add_param_uint32(&wd->dev, "priority",
 				 watchdog_set_priority, NULL,
 				 &wd->priority, "%u", wd);
-	if (IS_ERR(p)) {
-		ret = PTR_ERR(p);
-		goto error_unregister;
-	}
 
 	/* set some default sane value */
 	if (!wd->timeout_max)
 		wd->timeout_max = 60 * 60 * 24;
 
-	p = dev_add_param_uint32_ro(&wd->dev, "timeout_max",
+	dev_add_param_uint32_ro(&wd->dev, "timeout_max",
 			&wd->timeout_max, "%u");
-	if (IS_ERR(p)) {
-		ret = PTR_ERR(p);
-		goto error_unregister;
-	}
 
 	if (IS_ENABLED(CONFIG_WATCHDOG_POLLER)) {
 		if (!wd->poller_timeout_cur ||
 		    wd->poller_timeout_cur > wd->timeout_max)
 			wd->poller_timeout_cur = wd->timeout_max;
 
-		p = dev_add_param_uint32(&wd->dev, "timeout_cur", watchdog_set_cur,
+		dev_add_param_uint32(&wd->dev, "timeout_cur", watchdog_set_cur,
 				NULL, &wd->poller_timeout_cur, "%u", wd);
-		if (IS_ERR(p)) {
-			ret = PTR_ERR(p);
-			goto error_unregister;
-		}
 
-		ret = watchdog_register_poller(wd);
-		if (ret)
-			goto error_unregister;
+		watchdog_register_poller(wd);
 	}
 
-	p = dev_add_param_uint32(&wd->dev, "seconds_to_expire", param_set_readonly,
+	dev_add_param_uint32(&wd->dev, "seconds_to_expire", param_set_readonly,
 			seconds_to_expire_get, &wd->seconds_to_expire, "%d", wd);
-	if (IS_ERR(p)) {
-		ret = PTR_ERR(p);
-		goto error_unregister;
-	}
 
 	if (!restart_handler.priority) {
 		restart_handler.priority = 10; /* don't override others */
@@ -275,10 +245,6 @@ int watchdog_register(struct watchdog *wd)
 			wd->priority);
 
 	return 0;
-
-error_unregister:
-	unregister_device(&wd->dev);
-	return ret;
 }
 EXPORT_SYMBOL(watchdog_register);
 
-- 
2.39.2

[PATCH 4/4] watchdog: factor out device registration into common class code

[Ahmad Fatoum]

Watchdog devices are called wdog[0-...], unless a DT alias is available
and no device had been registered with that name.

This is a useful scheme for other devices as well, so move it to a
generic place for reuse.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/base/class.c       | 38 ++++++++++++++++++++++++++++++++++++++
 drivers/watchdog/wd_core.c | 25 +++----------------------
 include/device.h           |  4 ++++
 3 files changed, 45 insertions(+), 22 deletions(-)

diff --git a/drivers/base/class.c b/drivers/base/class.c
index 5fbbae0ff025..d0765e496f9e 100644
--- a/drivers/base/class.c
+++ b/drivers/base/class.c
@@ -17,6 +17,44 @@ int class_add_device(struct class *class, struct device *dev)
 	return 0;
 }
 
+static int __class_register_device(struct device *class_dev, const char *name, int id)
+{
+	class_dev->id = id;
+	dev_set_name(class_dev, name);
+
+	return register_device(class_dev);
+}
+
+int class_register_device(struct class *class,
+			  struct device *class_dev,
+			  const char *name)
+{
+	struct device *parent = class_dev->parent;
+	const char *alias = NULL;
+	int ret = 0;
+
+	if (dev_of_node(parent))
+		alias = of_alias_get(parent->of_node);
+
+	if (alias)
+		ret = __class_register_device(class_dev, alias, DEVICE_ID_SINGLE);
+
+	if (!alias || ret)
+		ret = __class_register_device(class_dev, name, DEVICE_ID_DYNAMIC);
+
+	if (ret)
+		return ret;
+
+	class_add_device(class, class_dev);
+	return 0;
+}
+
+void class_unregister_device(struct device *class_dev)
+{
+	list_del(&class_dev->class_list);
+	unregister_device(class_dev);
+}
+
 void class_remove_device(struct class *class, struct device *dev)
 {
 	list_del(&dev->class_list);
diff --git a/drivers/watchdog/wd_core.c b/drivers/watchdog/wd_core.c
index 44f2b6da8cf2..adb4c639097e 100644
--- a/drivers/watchdog/wd_core.c
+++ b/drivers/watchdog/wd_core.c
@@ -119,15 +119,6 @@ static void watchdog_register_poller(struct watchdog *wd)
 			   NULL, &wd->poller_enable, wd);
 }
 
-static int watchdog_register_dev(struct watchdog *wd, const char *name, int id)
-{
-	wd->dev.parent = wd->hwdev;
-	wd->dev.id = id;
-	dev_set_name(&wd->dev, name);
-
-	return register_device(&wd->dev);
-}
-
 /**
  * dev_get_watchdog_priority() - get a device's desired watchdog priority
  * @dev:	The device, which device_node to read the property from
@@ -187,18 +178,10 @@ static struct restart_handler restart_handler = {
 
 int watchdog_register(struct watchdog *wd)
 {
-	const char *alias = NULL;
-	int ret = 0;
-
-	if (wd->hwdev)
-		alias = of_alias_get(wd->hwdev->of_node);
-
-	if (alias)
-		ret = watchdog_register_dev(wd, alias, DEVICE_ID_SINGLE);
-
-	if (!alias || ret)
-		ret = watchdog_register_dev(wd, "wdog", DEVICE_ID_DYNAMIC);
+	int ret;
 
+	wd->dev.parent = wd->hwdev;
+	ret = class_register_device(&watchdog_class, &wd->dev, "wdog");
 	if (ret)
 		return ret;
 
@@ -239,8 +222,6 @@ int watchdog_register(struct watchdog *wd)
 			dev_warn(&wd->dev, "failed to register restart handler\n");
 	}
 
-	class_add_device(&watchdog_class, &wd->dev);
-
 	pr_debug("registering watchdog %s with priority %d\n", watchdog_name(wd),
 			wd->priority);
 
diff --git a/include/device.h b/include/device.h
index 4c76da2f82dd..5d3923786167 100644
--- a/include/device.h
+++ b/include/device.h
@@ -128,6 +128,10 @@ void class_register(struct class *class);
 int class_add_device(struct class *class, struct device *dev);
 void class_remove_device(struct class *class, struct device *dev);
 
+int class_register_device(struct class *class, struct device *class_dev,
+			  const char *name);
+void class_unregister_device(struct device *class_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)
-- 
2.39.2

Re: [PATCH 1/4] init: mark initcalls const

[Sascha Hauer]

On Tue, 16 Jul 2024 13:57:08 +0200, Ahmad Fatoum wrote:
> The initcalls shouldn't be modified at runtime and they are part of the
> BAREBOX_RO_SECTION, so mark them const accordingly.
> 
> 

Applied, thanks!

[1/4] init: mark initcalls const
      https://git.pengutronix.de/cgit/barebox/commit/?id=7b2399aff024 (link may not be stable)
[2/4] base: use initcalls instead of linker lists to register classes
      https://git.pengutronix.de/cgit/barebox/commit/?id=ac70ae1e18fb (link may not be stable)
[3/4] watchdog: remove needless error checking for device parameter
      https://git.pengutronix.de/cgit/barebox/commit/?id=5c6531025f4c (link may not be stable)
[4/4] watchdog: factor out device registration into common class code
      https://git.pengutronix.de/cgit/barebox/commit/?id=ad90c8aef8f4 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>