diff mbox series

[1/4] of: property: Fix create device links for all child-supplier dependencies

Message ID 20200415150550.28156-2-nsaenzjulienne@suse.de
State Superseded, archived
Headers show
Series of: property: fw_devlink misc fixes | expand

Checks

Context Check Description
robh/checkpatch warning "total: 0 errors, 1 warnings, 8 lines checked"

Commit Message

Nicolas Saenz Julienne April 15, 2020, 3:05 p.m. UTC
Upon adding a new platform device we scan its properties and its
children's properties in order to create a consumer/supplier
relationship between the device and the property supplier.

That said, it's possible for some of the node's children to be disabled,
which will create links that'll never be fulfilled.

To get around this, use the for_each_available_child_of_node() function
instead of for_each_available_node() when iterating over the node's
children.

Fixes: d4387cd11741 ("of: property: Create device links for all child-supplier depencencies")
Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 drivers/of/property.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Saravana Kannan April 15, 2020, 6:20 p.m. UTC | #1
On Wed, Apr 15, 2020 at 8:06 AM Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> Upon adding a new platform device we scan its properties and its
> children's properties in order to create a consumer/supplier
> relationship between the device and the property supplier.
>
> That said, it's possible for some of the node's children to be disabled,
> which will create links that'll never be fulfilled.
>
> To get around this, use the for_each_available_child_of_node() function
> instead of for_each_available_node() when iterating over the node's
> children.
>
> Fixes: d4387cd11741 ("of: property: Create device links for all child-supplier depencencies")
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
>  drivers/of/property.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index b4916dcc9e725..a8c2b13521b27 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -1296,7 +1296,7 @@ static int of_link_to_suppliers(struct device *dev,
>                 if (of_link_property(dev, con_np, p->name))
>                         ret = -ENODEV;
>
> -       for_each_child_of_node(con_np, child)
> +       for_each_available_child_of_node(con_np, child)
>                 if (of_link_to_suppliers(dev, child) && !ret)
>                         ret = -EAGAIN;

Thanks. Good catch. I have plenty of disabled devices in my test
platform. But I guess I never ran into this scenario.

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

-Saravana
Saravana Kannan April 15, 2020, 6:22 p.m. UTC | #2
Actually a few more nits about the commit text.

On Wed, Apr 15, 2020 at 8:06 AM Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> Upon adding a new platform device we scan its properties and its

This code runs for all devices created from a DT node. Not just
platform devices. So fix this paragraph appropriately?

Upon adding a new device from a DT node, we scan... ?

-Saravana

> children's properties in order to create a consumer/supplier
> relationship between the device and the property supplier.
>
> That said, it's possible for some of the node's children to be disabled,
> which will create links that'll never be fulfilled.
>
> To get around this, use the for_each_available_child_of_node() function
> instead of for_each_available_node() when iterating over the node's
> children.
>
> Fixes: d4387cd11741 ("of: property: Create device links for all child-supplier depencencies")
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
>  drivers/of/property.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index b4916dcc9e725..a8c2b13521b27 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -1296,7 +1296,7 @@ static int of_link_to_suppliers(struct device *dev,
>                 if (of_link_property(dev, con_np, p->name))
>                         ret = -ENODEV;
>
> -       for_each_child_of_node(con_np, child)
> +       for_each_available_child_of_node(con_np, child)
>                 if (of_link_to_suppliers(dev, child) && !ret)
>                         ret = -EAGAIN;
>
> --
> 2.26.0
>
Nicolas Saenz Julienne April 16, 2020, 11:32 a.m. UTC | #3
On Wed, 2020-04-15 at 11:22 -0700, Saravana Kannan wrote:
> Actually a few more nits about the commit text.
> 
> On Wed, Apr 15, 2020 at 8:06 AM Nicolas Saenz Julienne
> <nsaenzjulienne@suse.de> wrote:
> > Upon adding a new platform device we scan its properties and its
> 
> This code runs for all devices created from a DT node. Not just
> platform devices. So fix this paragraph appropriately?
> 
> Upon adding a new device from a DT node, we scan... ?

Noted.

Regards,
Nicolas
diff mbox series

Patch

diff --git a/drivers/of/property.c b/drivers/of/property.c
index b4916dcc9e725..a8c2b13521b27 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -1296,7 +1296,7 @@  static int of_link_to_suppliers(struct device *dev,
 		if (of_link_property(dev, con_np, p->name))
 			ret = -ENODEV;
 
-	for_each_child_of_node(con_np, child)
+	for_each_available_child_of_node(con_np, child)
 		if (of_link_to_suppliers(dev, child) && !ret)
 			ret = -EAGAIN;