linux-renesas-soc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] treewide: Fix probing of devices in DT overlays
@ 2023-03-30 13:26 Geert Uytterhoeven
  2023-03-30 19:54 ` Saravana Kannan
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2023-03-30 13:26 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Saravana Kannan
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, Wolfram Sang, Rob Herring, Frank Rowand,
	Mark Brown, Ivan Bornyakov, linux-arm-kernel, linux-i2c,
	devicetree, linux-spi, linux-renesas-soc, linux-kernel,
	Geert Uytterhoeven

When loading a DT overlay that creates a device, the device is not
probed, unless the DT overlay is unloaded and reloaded again.

After the recent refactoring to improve fw_devlink, it no longer depends
on the "compatible" property to identify which device tree nodes will
become struct devices.   fw_devlink now picks up dangling consumers
(consumers pointing to descendent device tree nodes of a device that
aren't converted to child devices) when a device is successfully bound
to a driver.  See __fw_devlink_pickup_dangling_consumers().

However, during DT overlay, a device's device tree node can have
sub-nodes added/removed without unbinding/rebinding the driver.  This
difference in behavior between the normal device instantiation and
probing flow vs. the DT overlay flow has a bunch of implications that
are pointed out elsewhere[1].  One of them is that the fw_devlink logic
to pick up dangling consumers is never exercised.

This patch solves the fw_devlink issue by marking all DT nodes added by
DT overlays with FWNODE_FLAG_NOT_DEVICE (fwnode that won't become
device), and by clearing the flag when a struct device is actually
created for the DT node.  This way, fw_devlink knows not to have
consumers waiting on these newly added DT nodes, and to propagate the
dependency to an ancestor DT node that has the corresponding struct
device.

Based on a patch by Saravana Kannan, which covered only platform and spi
devices.

[1] https://lore.kernel.org/r/CAGETcx_bkuFaLCiPrAWCPQz+w79ccDp6=9e881qmK=vx3hBMyg@mail.gmail.com

Fixes: 4a032827daa89350 ("of: property: Simplify of_link_to_phandle()")
Link: https://lore.kernel.org/r/CAGETcx_+rhHvaC_HJXGrr5_WAd2+k5f=rWYnkCZ6z5bGX-wj4w@mail.gmail.com
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Mark Brown <broonie@kernel.org>
Acked-by: Wolfram Sang <wsa@kernel.org> # for I2C
---
v3:
  - Add Acked-by,
  - s/instantiate/probe/,
  - Improve commit description,
  - Add comment before clearing FWNODE_FLAG_NOT_DEVICE,

v2:
  - Add Acked-by,
  - Drop RFC.
---
 drivers/bus/imx-weim.c    | 6 ++++++
 drivers/i2c/i2c-core-of.c | 5 +++++
 drivers/of/dynamic.c      | 1 +
 drivers/of/platform.c     | 5 +++++
 drivers/spi/spi.c         | 5 +++++
 5 files changed, 22 insertions(+)

diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
index 36d42484142aede2..cf463c1d2102c6fb 100644
--- a/drivers/bus/imx-weim.c
+++ b/drivers/bus/imx-weim.c
@@ -329,6 +329,12 @@ static int of_weim_notify(struct notifier_block *nb, unsigned long action,
 				 "Failed to setup timing for '%pOF'\n", rd->dn);
 
 		if (!of_node_check_flag(rd->dn, OF_POPULATED)) {
+			/*
+			 * Clear the flag before adding the device so that
+			 * fw_devlink doesn't skip adding consumers to this
+			 * device.
+			 */
+			rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
 			if (!of_platform_device_create(rd->dn, NULL, &pdev->dev)) {
 				dev_err(&pdev->dev,
 					"Failed to create child device '%pOF'\n",
diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c
index aa93467784c29c89..5c137638689799c8 100644
--- a/drivers/i2c/i2c-core-of.c
+++ b/drivers/i2c/i2c-core-of.c
@@ -178,6 +178,11 @@ static int of_i2c_notify(struct notifier_block *nb, unsigned long action,
 			return NOTIFY_OK;
 		}
 
+		/*
+		 * Clear the flag before adding the device so that fw_devlink
+		 * doesn't skip adding consumers to this device.
+		 */
+		rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
 		client = of_i2c_register_device(adap, rd->dn);
 		if (IS_ERR(client)) {
 			dev_err(&adap->dev, "failed to create client for '%pOF'\n",
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 07d93753b12f5f4d..e311d406b1705306 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -226,6 +226,7 @@ static void __of_attach_node(struct device_node *np)
 	np->sibling = np->parent->child;
 	np->parent->child = np;
 	of_node_clear_flag(np, OF_DETACHED);
+	np->fwnode.flags |= FWNODE_FLAG_NOT_DEVICE;
 }
 
 /**
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index b2bd2e783445dd78..78ae8418744905c9 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -737,6 +737,11 @@ static int of_platform_notify(struct notifier_block *nb,
 		if (of_node_check_flag(rd->dn, OF_POPULATED))
 			return NOTIFY_OK;
 
+		/*
+		 * Clear the flag before adding the device so that fw_devlink
+		 * doesn't skip adding consumers to this device.
+		 */
+		rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
 		/* pdev_parent may be NULL when no bus platform device */
 		pdev_parent = of_find_device_by_node(rd->dn->parent);
 		pdev = of_platform_device_create(rd->dn, NULL,
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 37a7be6c5a44c8f9..a12420e28640bbd4 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -4504,6 +4504,11 @@ static int of_spi_notify(struct notifier_block *nb, unsigned long action,
 			return NOTIFY_OK;
 		}
 
+		/*
+		 * Clear the flag before adding the device so that fw_devlink
+		 * doesn't skip adding consumers to this device.
+		 */
+		rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
 		spi = of_register_spi_device(ctlr, rd->dn);
 		put_device(&ctlr->dev);
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] treewide: Fix probing of devices in DT overlays
  2023-03-30 13:26 [PATCH v3] treewide: Fix probing of devices in DT overlays Geert Uytterhoeven
@ 2023-03-30 19:54 ` Saravana Kannan
  2023-03-31 10:18 ` Ivan Bornyakov
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Saravana Kannan @ 2023-03-30 19:54 UTC (permalink / raw)
  To: geert+renesas
  Cc: Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	Wolfram Sang, Rob Herring, Frank Rowand, Mark Brown,
	Ivan Bornyakov, linux-arm-kernel, linux-i2c, devicetree,
	linux-spi, linux-renesas-soc, linux-kernel

On Thu, Mar 30, 2023 at 6:26 AM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
>
> When loading a DT overlay that creates a device, the device is not
> probed, unless the DT overlay is unloaded and reloaded again.
>
> After the recent refactoring to improve fw_devlink, it no longer depends
> on the "compatible" property to identify which device tree nodes will
> become struct devices.   fw_devlink now picks up dangling consumers
> (consumers pointing to descendent device tree nodes of a device that
> aren't converted to child devices) when a device is successfully bound
> to a driver.  See __fw_devlink_pickup_dangling_consumers().
>
> However, during DT overlay, a device's device tree node can have
> sub-nodes added/removed without unbinding/rebinding the driver.  This
> difference in behavior between the normal device instantiation and
> probing flow vs. the DT overlay flow has a bunch of implications that
> are pointed out elsewhere[1].  One of them is that the fw_devlink logic
> to pick up dangling consumers is never exercised.
>
> This patch solves the fw_devlink issue by marking all DT nodes added by
> DT overlays with FWNODE_FLAG_NOT_DEVICE (fwnode that won't become
> device), and by clearing the flag when a struct device is actually
> created for the DT node.  This way, fw_devlink knows not to have
> consumers waiting on these newly added DT nodes, and to propagate the
> dependency to an ancestor DT node that has the corresponding struct
> device.
>
> Based on a patch by Saravana Kannan, which covered only platform and spi
> devices.

Doesn't make sense to give a Reviewed-by to something I partially wrote, so:

Acked-by: Saravana Kannan <saravanak@google.com>

Thanks for making sure this is fixed Geert!

-Saravana

>
> [1] https://lore.kernel.org/r/CAGETcx_bkuFaLCiPrAWCPQz+w79ccDp6=9e881qmK=vx3hBMyg@mail.gmail.com
>
> Fixes: 4a032827daa89350 ("of: property: Simplify of_link_to_phandle()")
> Link: https://lore.kernel.org/r/CAGETcx_+rhHvaC_HJXGrr5_WAd2+k5f=rWYnkCZ6z5bGX-wj4w@mail.gmail.com
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Acked-by: Mark Brown <broonie@kernel.org>
> Acked-by: Wolfram Sang <wsa@kernel.org> # for I2C
> ---
> v3:
>   - Add Acked-by,
>   - s/instantiate/probe/,
>   - Improve commit description,
>   - Add comment before clearing FWNODE_FLAG_NOT_DEVICE,
>
> v2:
>   - Add Acked-by,
>   - Drop RFC.
> ---
>  drivers/bus/imx-weim.c    | 6 ++++++
>  drivers/i2c/i2c-core-of.c | 5 +++++
>  drivers/of/dynamic.c      | 1 +
>  drivers/of/platform.c     | 5 +++++
>  drivers/spi/spi.c         | 5 +++++
>  5 files changed, 22 insertions(+)
>
> diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
> index 36d42484142aede2..cf463c1d2102c6fb 100644
> --- a/drivers/bus/imx-weim.c
> +++ b/drivers/bus/imx-weim.c
> @@ -329,6 +329,12 @@ static int of_weim_notify(struct notifier_block *nb, unsigned long action,
>                                  "Failed to setup timing for '%pOF'\n", rd->dn);
>
>                 if (!of_node_check_flag(rd->dn, OF_POPULATED)) {
> +                       /*
> +                        * Clear the flag before adding the device so that
> +                        * fw_devlink doesn't skip adding consumers to this
> +                        * device.
> +                        */
> +                       rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
>                         if (!of_platform_device_create(rd->dn, NULL, &pdev->dev)) {
>                                 dev_err(&pdev->dev,
>                                         "Failed to create child device '%pOF'\n",
> diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c
> index aa93467784c29c89..5c137638689799c8 100644
> --- a/drivers/i2c/i2c-core-of.c
> +++ b/drivers/i2c/i2c-core-of.c
> @@ -178,6 +178,11 @@ static int of_i2c_notify(struct notifier_block *nb, unsigned long action,
>                         return NOTIFY_OK;
>                 }
>
> +               /*
> +                * Clear the flag before adding the device so that fw_devlink
> +                * doesn't skip adding consumers to this device.
> +                */
> +               rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
>                 client = of_i2c_register_device(adap, rd->dn);
>                 if (IS_ERR(client)) {
>                         dev_err(&adap->dev, "failed to create client for '%pOF'\n",
> diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
> index 07d93753b12f5f4d..e311d406b1705306 100644
> --- a/drivers/of/dynamic.c
> +++ b/drivers/of/dynamic.c
> @@ -226,6 +226,7 @@ static void __of_attach_node(struct device_node *np)
>         np->sibling = np->parent->child;
>         np->parent->child = np;
>         of_node_clear_flag(np, OF_DETACHED);
> +       np->fwnode.flags |= FWNODE_FLAG_NOT_DEVICE;
>  }
>
>  /**
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index b2bd2e783445dd78..78ae8418744905c9 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -737,6 +737,11 @@ static int of_platform_notify(struct notifier_block *nb,
>                 if (of_node_check_flag(rd->dn, OF_POPULATED))
>                         return NOTIFY_OK;
>
> +               /*
> +                * Clear the flag before adding the device so that fw_devlink
> +                * doesn't skip adding consumers to this device.
> +                */
> +               rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
>                 /* pdev_parent may be NULL when no bus platform device */
>                 pdev_parent = of_find_device_by_node(rd->dn->parent);
>                 pdev = of_platform_device_create(rd->dn, NULL,
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 37a7be6c5a44c8f9..a12420e28640bbd4 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -4504,6 +4504,11 @@ static int of_spi_notify(struct notifier_block *nb, unsigned long action,
>                         return NOTIFY_OK;
>                 }
>
> +               /*
> +                * Clear the flag before adding the device so that fw_devlink
> +                * doesn't skip adding consumers to this device.
> +                */
> +               rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
>                 spi = of_register_spi_device(ctlr, rd->dn);
>                 put_device(&ctlr->dev);
>
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] treewide: Fix probing of devices in DT overlays
  2023-03-30 13:26 [PATCH v3] treewide: Fix probing of devices in DT overlays Geert Uytterhoeven
  2023-03-30 19:54 ` Saravana Kannan
@ 2023-03-31 10:18 ` Ivan Bornyakov
  2023-04-05 14:27 ` Shawn Guo
  2023-04-05 20:42 ` Rob Herring
  3 siblings, 0 replies; 6+ messages in thread
From: Ivan Bornyakov @ 2023-03-31 10:18 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Greg Kroah-Hartman, Saravana Kannan, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	Wolfram Sang, Rob Herring, Frank Rowand, Mark Brown,
	linux-arm-kernel, linux-i2c, devicetree, linux-spi,
	linux-renesas-soc, linux-kernel

On Thu, Mar 30, 2023 at 03:26:13PM +0200, Geert Uytterhoeven wrote:
> When loading a DT overlay that creates a device, the device is not
> probed, unless the DT overlay is unloaded and reloaded again.
> 
> After the recent refactoring to improve fw_devlink, it no longer depends
> on the "compatible" property to identify which device tree nodes will
> become struct devices.   fw_devlink now picks up dangling consumers
> (consumers pointing to descendent device tree nodes of a device that
> aren't converted to child devices) when a device is successfully bound
> to a driver.  See __fw_devlink_pickup_dangling_consumers().
> 
> However, during DT overlay, a device's device tree node can have
> sub-nodes added/removed without unbinding/rebinding the driver.  This
> difference in behavior between the normal device instantiation and
> probing flow vs. the DT overlay flow has a bunch of implications that
> are pointed out elsewhere[1].  One of them is that the fw_devlink logic
> to pick up dangling consumers is never exercised.
> 
> This patch solves the fw_devlink issue by marking all DT nodes added by
> DT overlays with FWNODE_FLAG_NOT_DEVICE (fwnode that won't become
> device), and by clearing the flag when a struct device is actually
> created for the DT node.  This way, fw_devlink knows not to have
> consumers waiting on these newly added DT nodes, and to propagate the
> dependency to an ancestor DT node that has the corresponding struct
> device.
> 
> Based on a patch by Saravana Kannan, which covered only platform and spi
> devices.
> 
> [1] https://lore.kernel.org/r/CAGETcx_bkuFaLCiPrAWCPQz+w79ccDp6=9e881qmK=vx3hBMyg@mail.gmail.com
> 
> Fixes: 4a032827daa89350 ("of: property: Simplify of_link_to_phandle()")
> Link: https://lore.kernel.org/r/CAGETcx_+rhHvaC_HJXGrr5_WAd2+k5f=rWYnkCZ6z5bGX-wj4w@mail.gmail.com
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Acked-by: Mark Brown <broonie@kernel.org>
> Acked-by: Wolfram Sang <wsa@kernel.org> # for I2C

Here is my use-case: a bunch of FPGA-based devices, which are SPI
slaves, WEIM childs and other platform devices, described in DTS with
status = "disabled"; in run-time DT-overlay with status = "okay" for
these devices is loaded via ConfigFS with help of out-of-tree patch.

Although without this patch I had no issues probing overlay-enabled
devices, it won't hurt either.

Tested-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] treewide: Fix probing of devices in DT overlays
  2023-03-30 13:26 [PATCH v3] treewide: Fix probing of devices in DT overlays Geert Uytterhoeven
  2023-03-30 19:54 ` Saravana Kannan
  2023-03-31 10:18 ` Ivan Bornyakov
@ 2023-04-05 14:27 ` Shawn Guo
  2023-04-05 20:42 ` Rob Herring
  3 siblings, 0 replies; 6+ messages in thread
From: Shawn Guo @ 2023-04-05 14:27 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Greg Kroah-Hartman, Saravana Kannan, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	Wolfram Sang, Rob Herring, Frank Rowand, Mark Brown,
	Ivan Bornyakov, linux-arm-kernel, linux-i2c, devicetree,
	linux-spi, linux-renesas-soc, linux-kernel

On Thu, Mar 30, 2023 at 03:26:13PM +0200, Geert Uytterhoeven wrote:
> When loading a DT overlay that creates a device, the device is not
> probed, unless the DT overlay is unloaded and reloaded again.
> 
> After the recent refactoring to improve fw_devlink, it no longer depends
> on the "compatible" property to identify which device tree nodes will
> become struct devices.   fw_devlink now picks up dangling consumers
> (consumers pointing to descendent device tree nodes of a device that
> aren't converted to child devices) when a device is successfully bound
> to a driver.  See __fw_devlink_pickup_dangling_consumers().
> 
> However, during DT overlay, a device's device tree node can have
> sub-nodes added/removed without unbinding/rebinding the driver.  This
> difference in behavior between the normal device instantiation and
> probing flow vs. the DT overlay flow has a bunch of implications that
> are pointed out elsewhere[1].  One of them is that the fw_devlink logic
> to pick up dangling consumers is never exercised.
> 
> This patch solves the fw_devlink issue by marking all DT nodes added by
> DT overlays with FWNODE_FLAG_NOT_DEVICE (fwnode that won't become
> device), and by clearing the flag when a struct device is actually
> created for the DT node.  This way, fw_devlink knows not to have
> consumers waiting on these newly added DT nodes, and to propagate the
> dependency to an ancestor DT node that has the corresponding struct
> device.
> 
> Based on a patch by Saravana Kannan, which covered only platform and spi
> devices.
> 
> [1] https://lore.kernel.org/r/CAGETcx_bkuFaLCiPrAWCPQz+w79ccDp6=9e881qmK=vx3hBMyg@mail.gmail.com
> 
> Fixes: 4a032827daa89350 ("of: property: Simplify of_link_to_phandle()")
> Link: https://lore.kernel.org/r/CAGETcx_+rhHvaC_HJXGrr5_WAd2+k5f=rWYnkCZ6z5bGX-wj4w@mail.gmail.com
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Acked-by: Mark Brown <broonie@kernel.org>
> Acked-by: Wolfram Sang <wsa@kernel.org> # for I2C
> ---
> v3:
>   - Add Acked-by,
>   - s/instantiate/probe/,
>   - Improve commit description,
>   - Add comment before clearing FWNODE_FLAG_NOT_DEVICE,
> 
> v2:
>   - Add Acked-by,
>   - Drop RFC.
> ---
>  drivers/bus/imx-weim.c    | 6 ++++++

Acked-by: Shawn Guo <shawnguo@kernel.org>

>  drivers/i2c/i2c-core-of.c | 5 +++++
>  drivers/of/dynamic.c      | 1 +
>  drivers/of/platform.c     | 5 +++++
>  drivers/spi/spi.c         | 5 +++++
>  5 files changed, 22 insertions(+)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] treewide: Fix probing of devices in DT overlays
  2023-03-30 13:26 [PATCH v3] treewide: Fix probing of devices in DT overlays Geert Uytterhoeven
                   ` (2 preceding siblings ...)
  2023-04-05 14:27 ` Shawn Guo
@ 2023-04-05 20:42 ` Rob Herring
  2023-04-06  5:01   ` Greg Kroah-Hartman
  3 siblings, 1 reply; 6+ messages in thread
From: Rob Herring @ 2023-04-05 20:42 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Mark Brown, devicetree, Pengutronix Kernel Team, Saravana Kannan,
	Ivan Bornyakov, Greg Kroah-Hartman, Fabio Estevam, linux-i2c,
	linux-renesas-soc, linux-arm-kernel, Frank Rowand, Shawn Guo,
	linux-kernel, linux-spi, Rob Herring, NXP Linux Team,
	Sascha Hauer, Wolfram Sang


On Thu, 30 Mar 2023 15:26:13 +0200, Geert Uytterhoeven wrote:
> When loading a DT overlay that creates a device, the device is not
> probed, unless the DT overlay is unloaded and reloaded again.
> 
> After the recent refactoring to improve fw_devlink, it no longer depends
> on the "compatible" property to identify which device tree nodes will
> become struct devices.   fw_devlink now picks up dangling consumers
> (consumers pointing to descendent device tree nodes of a device that
> aren't converted to child devices) when a device is successfully bound
> to a driver.  See __fw_devlink_pickup_dangling_consumers().
> 
> However, during DT overlay, a device's device tree node can have
> sub-nodes added/removed without unbinding/rebinding the driver.  This
> difference in behavior between the normal device instantiation and
> probing flow vs. the DT overlay flow has a bunch of implications that
> are pointed out elsewhere[1].  One of them is that the fw_devlink logic
> to pick up dangling consumers is never exercised.
> 
> This patch solves the fw_devlink issue by marking all DT nodes added by
> DT overlays with FWNODE_FLAG_NOT_DEVICE (fwnode that won't become
> device), and by clearing the flag when a struct device is actually
> created for the DT node.  This way, fw_devlink knows not to have
> consumers waiting on these newly added DT nodes, and to propagate the
> dependency to an ancestor DT node that has the corresponding struct
> device.
> 
> Based on a patch by Saravana Kannan, which covered only platform and spi
> devices.
> 
> [1] https://lore.kernel.org/r/CAGETcx_bkuFaLCiPrAWCPQz+w79ccDp6=9e881qmK=vx3hBMyg@mail.gmail.com
> 
> Fixes: 4a032827daa89350 ("of: property: Simplify of_link_to_phandle()")
> Link: https://lore.kernel.org/r/CAGETcx_+rhHvaC_HJXGrr5_WAd2+k5f=rWYnkCZ6z5bGX-wj4w@mail.gmail.com
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Acked-by: Mark Brown <broonie@kernel.org>
> Acked-by: Wolfram Sang <wsa@kernel.org> # for I2C
> ---
> v3:
>   - Add Acked-by,
>   - s/instantiate/probe/,
>   - Improve commit description,
>   - Add comment before clearing FWNODE_FLAG_NOT_DEVICE,
> 
> v2:
>   - Add Acked-by,
>   - Drop RFC.
> ---
>  drivers/bus/imx-weim.c    | 6 ++++++
>  drivers/i2c/i2c-core-of.c | 5 +++++
>  drivers/of/dynamic.c      | 1 +
>  drivers/of/platform.c     | 5 +++++
>  drivers/spi/spi.c         | 5 +++++
>  5 files changed, 22 insertions(+)
> 

Applied, thanks!


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] treewide: Fix probing of devices in DT overlays
  2023-04-05 20:42 ` Rob Herring
@ 2023-04-06  5:01   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2023-04-06  5:01 UTC (permalink / raw)
  To: Rob Herring
  Cc: Geert Uytterhoeven, Mark Brown, devicetree,
	Pengutronix Kernel Team, Saravana Kannan, Ivan Bornyakov,
	Fabio Estevam, linux-i2c, linux-renesas-soc, linux-arm-kernel,
	Frank Rowand, Shawn Guo, linux-kernel, linux-spi, Rob Herring,
	NXP Linux Team, Sascha Hauer, Wolfram Sang

On Wed, Apr 05, 2023 at 03:42:52PM -0500, Rob Herring wrote:
> 
> On Thu, 30 Mar 2023 15:26:13 +0200, Geert Uytterhoeven wrote:
> > When loading a DT overlay that creates a device, the device is not
> > probed, unless the DT overlay is unloaded and reloaded again.
> > 
> > After the recent refactoring to improve fw_devlink, it no longer depends
> > on the "compatible" property to identify which device tree nodes will
> > become struct devices.   fw_devlink now picks up dangling consumers
> > (consumers pointing to descendent device tree nodes of a device that
> > aren't converted to child devices) when a device is successfully bound
> > to a driver.  See __fw_devlink_pickup_dangling_consumers().
> > 
> > However, during DT overlay, a device's device tree node can have
> > sub-nodes added/removed without unbinding/rebinding the driver.  This
> > difference in behavior between the normal device instantiation and
> > probing flow vs. the DT overlay flow has a bunch of implications that
> > are pointed out elsewhere[1].  One of them is that the fw_devlink logic
> > to pick up dangling consumers is never exercised.
> > 
> > This patch solves the fw_devlink issue by marking all DT nodes added by
> > DT overlays with FWNODE_FLAG_NOT_DEVICE (fwnode that won't become
> > device), and by clearing the flag when a struct device is actually
> > created for the DT node.  This way, fw_devlink knows not to have
> > consumers waiting on these newly added DT nodes, and to propagate the
> > dependency to an ancestor DT node that has the corresponding struct
> > device.
> > 
> > Based on a patch by Saravana Kannan, which covered only platform and spi
> > devices.
> > 
> > [1] https://lore.kernel.org/r/CAGETcx_bkuFaLCiPrAWCPQz+w79ccDp6=9e881qmK=vx3hBMyg@mail.gmail.com
> > 
> > Fixes: 4a032827daa89350 ("of: property: Simplify of_link_to_phandle()")
> > Link: https://lore.kernel.org/r/CAGETcx_+rhHvaC_HJXGrr5_WAd2+k5f=rWYnkCZ6z5bGX-wj4w@mail.gmail.com
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > Acked-by: Mark Brown <broonie@kernel.org>
> > Acked-by: Wolfram Sang <wsa@kernel.org> # for I2C
> > ---
> > v3:
> >   - Add Acked-by,
> >   - s/instantiate/probe/,
> >   - Improve commit description,
> >   - Add comment before clearing FWNODE_FLAG_NOT_DEVICE,
> > 
> > v2:
> >   - Add Acked-by,
> >   - Drop RFC.
> > ---
> >  drivers/bus/imx-weim.c    | 6 ++++++
> >  drivers/i2c/i2c-core-of.c | 5 +++++
> >  drivers/of/dynamic.c      | 1 +
> >  drivers/of/platform.c     | 5 +++++
> >  drivers/spi/spi.c         | 5 +++++
> >  5 files changed, 22 insertions(+)
> > 
> 
> Applied, thanks!

Oops, I thought I had to take this.  I'll go drop it from my tree...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-04-06  5:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-30 13:26 [PATCH v3] treewide: Fix probing of devices in DT overlays Geert Uytterhoeven
2023-03-30 19:54 ` Saravana Kannan
2023-03-31 10:18 ` Ivan Bornyakov
2023-04-05 14:27 ` Shawn Guo
2023-04-05 20:42 ` Rob Herring
2023-04-06  5:01   ` Greg Kroah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).