diff mbox series

[2/3] dm: core: support dtb switching without devices/drivers unloading

Message ID 20240627113546.100333-2-mikhail.kshevetskiy@iopsys.eu
State Changes Requested
Headers show
Series [1/3] fit_dtb: relocate whole fit dtb image instead of selected dtb only | expand

Commit Message

Mikhail Kshevetskiy June 27, 2024, 11:35 a.m. UTC
Consider a case when we have several similar boards but leds & buttons
connected differently. Some of leds uses gpio, other pwm, third gpio over
spi bitbang. Also different board may have different gpio hogs. And last
but not least: board type stored inside a ubi volume.

We want a single u-boot image that will support all these boards (we are
not using SPL). So we need runtime dtb switching.

We can't use MULTI_DTB_FIT and DTB_RESELECT because they tries to switch
dtb too early when flash & ubi drivers are not loaded/activated.

The following code do runtime switching

	ret = fdtdec_resetup(&rescan);
	if (!ret && rescan){
		dm_uninit();
		dm_init_and_scan(false);
	}

but it have several nasty issues
 * additional code required to reinitialize console otherwise board will hang.
 * MTD subsystem is not cleared, we need to remove mtd partition before
   switching to new dtb
 * spi-nand flash will be named "spi-nand1" instead of "spi-nand0", thus
   flash becomes unusable
 * some drivers are unable to deinitialize properly (ex: network driver will
   not release mdio interface)
 * some memory will leak, because most of drivers assumes that it will never
   be unloaded

To avoid this nasty issues the following hack was implemented
 * Full DM and console reinitialization was skipped
 * Only missed device/subdevice added from new dtb during device scan.
 * Some driver renames their devices. As result we can't use device names
   to match old device node with a new one. The special field was added to
   struct udevice for this purposes.
 * Driver must be binded to the old device one more time to add missed
   subdevices.
 * uclass must be post_bind to the old device one more time to add missed
   subdevices.
 * We can't use old dtb anymore, so replace old device node with a new one
   for existing devices.

The following restrictions are present:
 * initial dtb MUST be a subset of a new one
 * old devices will NOT be reinitialised
 * no devices will be deleted or deactivated
 * only new devices will be added

Typical usage:
	ret = fdtdec_resetup(&rescan);
	if (!ret && rescan)
		dm_rescan();

The code was tested with: gpio, gpio_hog, leds, buttons.

Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
---
 drivers/core/device.c | 31 +++++++++++++++++++++++++++++++
 drivers/core/root.c   | 15 +++++++++++++++
 dts/Kconfig           |  8 ++++++++
 include/dm/device.h   |  3 +++
 include/dm/root.h     | 23 +++++++++++++++++++++++
 5 files changed, 80 insertions(+)

Comments

Caleb Connolly June 28, 2024, 12:02 a.m. UTC | #1
On 27/06/2024 13:35, Mikhail Kshevetskiy wrote:
> Consider a case when we have several similar boards but leds & buttons
> connected differently. Some of leds uses gpio, other pwm, third gpio over
> spi bitbang. Also different board may have different gpio hogs. And last
> but not least: board type stored inside a ubi volume.
> 
> We want a single u-boot image that will support all these boards (we are
> not using SPL). So we need runtime dtb switching.
> 
> We can't use MULTI_DTB_FIT and DTB_RESELECT because they tries to switch
> dtb too early when flash & ubi drivers are not loaded/activated.
> 
> The following code do runtime switching
> 
> 	ret = fdtdec_resetup(&rescan);
> 	if (!ret && rescan){
> 		dm_uninit();
> 		dm_init_and_scan(false);
> 	}
> 
> but it have several nasty issues
>   * additional code required to reinitialize console otherwise board will hang.
>   * MTD subsystem is not cleared, we need to remove mtd partition before
>     switching to new dtb
>   * spi-nand flash will be named "spi-nand1" instead of "spi-nand0", thus
>     flash becomes unusable
>   * some drivers are unable to deinitialize properly (ex: network driver will
>     not release mdio interface)
>   * some memory will leak, because most of drivers assumes that it will never
>     be unloaded
> 
> To avoid this nasty issues the following hack was implemented
>   * Full DM and console reinitialization was skipped
>   * Only missed device/subdevice added from new dtb during device scan.
>   * Some driver renames their devices. As result we can't use device names
>     to match old device node with a new one. The special field was added to
>     struct udevice for this purposes.
>   * Driver must be binded to the old device one more time to add missed
>     subdevices.
>   * uclass must be post_bind to the old device one more time to add missed
>     subdevices.
>   * We can't use old dtb anymore, so replace old device node with a new one
>     for existing devices.
> 
> The following restrictions are present:
>   * initial dtb MUST be a subset of a new one

Surely dtbo is a better application for your usecase? Switching out the 
entire dtb and "rebooting" the entire driver model seems a bit overkill 
to add some LEDs? If you can just pick a dtbo at some point, overlay it, 
maybe re-generate livetree, and then just devices that are affected by 
the dtbo?

Maybe that's a bit of a naive idea... Perhaps it could be restricted to 
some specific situations, like only support dtbo's that either add new 
nodes or change just the status property of a node?

Though this seems like more or less what you're doing in this patch.
>   * old devices will NOT be reinitialised
>   * no devices will be deleted or deactivated
>   * only new devices will be added
> 
> Typical usage:
> 	ret = fdtdec_resetup(&rescan);
> 	if (!ret && rescan)
> 		dm_rescan();
> 
> The code was tested with: gpio, gpio_hog, leds, buttons.
> 
> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
> ---
>   drivers/core/device.c | 31 +++++++++++++++++++++++++++++++
>   drivers/core/root.c   | 15 +++++++++++++++
>   dts/Kconfig           |  8 ++++++++
>   include/dm/device.h   |  3 +++
>   include/dm/root.h     | 23 +++++++++++++++++++++++
>   5 files changed, 80 insertions(+)
> 
> diff --git a/drivers/core/device.c b/drivers/core/device.c
> index bf7f261cbce..4f575ca82e7 100644
> --- a/drivers/core/device.c
> +++ b/drivers/core/device.c
> @@ -62,6 +62,34 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv,
>   		return ret;
>   	}
>   
> +#if CONFIG_IS_ENABLED(MULTI_DTB_FIT_RESCAN)
> +	if (parent) {
> +		struct list_head	*entry;
> +		ofnode			dev_node;
> +
> +		list_for_each(entry, &parent->child_head) {
> +			dev = list_entry(entry, struct udevice, sibling_node);
> +			dev_node = dev_ofnode(dev);
> +			if ((dev->driver == drv) && (dev->uclass == uc) &&
> +			    (dev->driver_data == driver_data) &&
> +			    (strcmp(dev->node_name, ofnode_get_name(node)) == 0))
> +			{
> +				ret = 0;
> +				dev_set_ofnode(dev, node);
> +				if (drv->bind)
> +					ret = drv->bind(dev);
> +
> +				if (!ret && uc->uc_drv->post_bind)
> +					ret = uc->uc_drv->post_bind(dev);
> +
> +				if (!ret && devp)
> +					*devp = dev;
> +				return ret;
> +			}
> +		}
> +	}
> +#endif
> +
>   	dev = calloc(1, sizeof(struct udevice));
>   	if (!dev)
>   		return -ENOMEM;
> @@ -75,6 +103,9 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv,
>   	dev_set_plat(dev, plat);
>   	dev->driver_data = driver_data;
>   	dev->name = name;
> +#if CONFIG_IS_ENABLED(MULTI_DTB_FIT_RESCAN)
> +	dev->node_name = name;
> +#endif
>   	dev_set_ofnode(dev, node);
>   	dev->parent = parent;
>   	dev->driver = drv;
> diff --git a/drivers/core/root.c b/drivers/core/root.c
> index d4ae652bcfb..4bf1ff80636 100644
> --- a/drivers/core/root.c
> +++ b/drivers/core/root.c
> @@ -340,6 +340,21 @@ static int dm_scan(bool pre_reloc_only)
>   	return dm_probe_devices(gd->dm_root, pre_reloc_only);
>   }
>   
> +#if CONFIG_IS_ENABLED(MULTI_DTB_FIT_RESCAN)
> +int dm_rescan(void)
> +{
> +	int ret;
> +
> +	ret = dm_extended_scan(false);
> +	if (ret) {
> +		debug("dm_extended_scan() failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	return dm_probe_devices(gd->dm_root, false);
> +}
> +#endif
> +
>   int dm_init_and_scan(bool pre_reloc_only)
>   {
>   	int ret;
> diff --git a/dts/Kconfig b/dts/Kconfig
> index 6883a000a05..ab486b21fe2 100644
> --- a/dts/Kconfig
> +++ b/dts/Kconfig
> @@ -292,6 +292,14 @@ config DTB_RESELECT
>   	  config allows boards to implement a function at a later point
>   	  during boot to switch to the "correct" dtb.
>   
> +config MULTI_DTB_FIT_RESCAN
> +	bool "Support switching dtb without drivers unloading (experimental)"
> +	depends on MULTI_DTB_FIT
> +	help
> +	  This configs adds possibility to switch from initial (minimal) dtb
> +	  to a "correct" one without devices/drivers unloading (ex: board type
> +	  required to switch dtb stored in ubi volume)
> +
>   config MULTI_DTB_FIT
>   	bool "Support embedding several DTBs in a FIT image for u-boot"
>   	help
> diff --git a/include/dm/device.h b/include/dm/device.h
> index add67f9ec06..409b1c4861a 100644
> --- a/include/dm/device.h
> +++ b/include/dm/device.h
> @@ -170,6 +170,9 @@ enum {
>   struct udevice {
>   	const struct driver *driver;
>   	const char *name;
> +#if CONFIG_IS_ENABLED(MULTI_DTB_FIT_RESCAN)
> +	const char *node_name;
> +#endif
>   	void *plat_;
>   	void *parent_plat_;
>   	void *uclass_plat_;
> diff --git a/include/dm/root.h b/include/dm/root.h
> index b2f30a842f5..dcab28f6c47 100644
> --- a/include/dm/root.h
> +++ b/include/dm/root.h
> @@ -155,6 +155,29 @@ int dm_init(bool of_live);
>    */
>   int dm_uninit(void);
>   
> +#if CONFIG_IS_ENABLED(MULTI_DTB_FIT_RESCAN)
> +/**
> + * dm_rescan - Rescan ftd and add missed DM devices (experimental)
> + *
> + * This function can be used for switching from initial (minimal) dtb
> + * to a "correct" one without devices/drivers unloading.
> + *
> + * Restrictions:
> + *   - initial dtb MUST be a subset of a new one
> + *   - old devices will NOT be reinitialised
> + *   - no devices will be deleted or deactivated
> + *   - only new devices will be added
> + *
> + * Typical usage:
> + *	ret = fdtdec_resetup(&rescan);
> + *	if (!ret && rescan)
> + *		dm_rescan();
> + *
> + * Return: 0 if OK, -ve on error
> + */
> +int dm_rescan(void);
> +#endif
> +
>   #if CONFIG_IS_ENABLED(DM_DEVICE_REMOVE)
>   /**
>    * dm_remove_devices_flags - Call remove function of all drivers with
diff mbox series

Patch

diff --git a/drivers/core/device.c b/drivers/core/device.c
index bf7f261cbce..4f575ca82e7 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -62,6 +62,34 @@  static int device_bind_common(struct udevice *parent, const struct driver *drv,
 		return ret;
 	}
 
+#if CONFIG_IS_ENABLED(MULTI_DTB_FIT_RESCAN)
+	if (parent) {
+		struct list_head	*entry;
+		ofnode			dev_node;
+
+		list_for_each(entry, &parent->child_head) {
+			dev = list_entry(entry, struct udevice, sibling_node);
+			dev_node = dev_ofnode(dev);
+			if ((dev->driver == drv) && (dev->uclass == uc) &&
+			    (dev->driver_data == driver_data) &&
+			    (strcmp(dev->node_name, ofnode_get_name(node)) == 0))
+			{
+				ret = 0;
+				dev_set_ofnode(dev, node);
+				if (drv->bind)
+					ret = drv->bind(dev);
+
+				if (!ret && uc->uc_drv->post_bind)
+					ret = uc->uc_drv->post_bind(dev);
+
+				if (!ret && devp)
+					*devp = dev;
+				return ret;
+			}
+		}
+	}
+#endif
+
 	dev = calloc(1, sizeof(struct udevice));
 	if (!dev)
 		return -ENOMEM;
@@ -75,6 +103,9 @@  static int device_bind_common(struct udevice *parent, const struct driver *drv,
 	dev_set_plat(dev, plat);
 	dev->driver_data = driver_data;
 	dev->name = name;
+#if CONFIG_IS_ENABLED(MULTI_DTB_FIT_RESCAN)
+	dev->node_name = name;
+#endif
 	dev_set_ofnode(dev, node);
 	dev->parent = parent;
 	dev->driver = drv;
diff --git a/drivers/core/root.c b/drivers/core/root.c
index d4ae652bcfb..4bf1ff80636 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -340,6 +340,21 @@  static int dm_scan(bool pre_reloc_only)
 	return dm_probe_devices(gd->dm_root, pre_reloc_only);
 }
 
+#if CONFIG_IS_ENABLED(MULTI_DTB_FIT_RESCAN)
+int dm_rescan(void)
+{
+	int ret;
+
+	ret = dm_extended_scan(false);
+	if (ret) {
+		debug("dm_extended_scan() failed: %d\n", ret);
+		return ret;
+	}
+
+	return dm_probe_devices(gd->dm_root, false);
+}
+#endif
+
 int dm_init_and_scan(bool pre_reloc_only)
 {
 	int ret;
diff --git a/dts/Kconfig b/dts/Kconfig
index 6883a000a05..ab486b21fe2 100644
--- a/dts/Kconfig
+++ b/dts/Kconfig
@@ -292,6 +292,14 @@  config DTB_RESELECT
 	  config allows boards to implement a function at a later point
 	  during boot to switch to the "correct" dtb.
 
+config MULTI_DTB_FIT_RESCAN
+	bool "Support switching dtb without drivers unloading (experimental)"
+	depends on MULTI_DTB_FIT
+	help
+	  This configs adds possibility to switch from initial (minimal) dtb
+	  to a "correct" one without devices/drivers unloading (ex: board type
+	  required to switch dtb stored in ubi volume)
+
 config MULTI_DTB_FIT
 	bool "Support embedding several DTBs in a FIT image for u-boot"
 	help
diff --git a/include/dm/device.h b/include/dm/device.h
index add67f9ec06..409b1c4861a 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -170,6 +170,9 @@  enum {
 struct udevice {
 	const struct driver *driver;
 	const char *name;
+#if CONFIG_IS_ENABLED(MULTI_DTB_FIT_RESCAN)
+	const char *node_name;
+#endif
 	void *plat_;
 	void *parent_plat_;
 	void *uclass_plat_;
diff --git a/include/dm/root.h b/include/dm/root.h
index b2f30a842f5..dcab28f6c47 100644
--- a/include/dm/root.h
+++ b/include/dm/root.h
@@ -155,6 +155,29 @@  int dm_init(bool of_live);
  */
 int dm_uninit(void);
 
+#if CONFIG_IS_ENABLED(MULTI_DTB_FIT_RESCAN)
+/**
+ * dm_rescan - Rescan ftd and add missed DM devices (experimental)
+ *
+ * This function can be used for switching from initial (minimal) dtb
+ * to a "correct" one without devices/drivers unloading.
+ *
+ * Restrictions:
+ *   - initial dtb MUST be a subset of a new one
+ *   - old devices will NOT be reinitialised
+ *   - no devices will be deleted or deactivated
+ *   - only new devices will be added
+ *
+ * Typical usage:
+ *	ret = fdtdec_resetup(&rescan);
+ *	if (!ret && rescan)
+ *		dm_rescan();
+ *
+ * Return: 0 if OK, -ve on error
+ */
+int dm_rescan(void);
+#endif
+
 #if CONFIG_IS_ENABLED(DM_DEVICE_REMOVE)
 /**
  * dm_remove_devices_flags - Call remove function of all drivers with