2018-07-16 10:00:55

by Benjamin Gaignard

[permalink] [raw]
Subject: [PATCH v2] base: core: Remove WARN_ON from link dependencies check

In some cases the link between between customer and supplier
already exist, for example when a device use it parent as supplier [1].
Do not warn about already existing dependencies because device_link_add()
take care of this case.

[1] https://lkml.org/lkml/2018/7/9/356

Reported-by: Marek Szyprowski <[email protected]>
Reviewed-by: Mark Brown <[email protected]>
Signed-off-by: Benjamin Gaignard <[email protected]>
---
drivers/base/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index df3e1a44707a..fcdc17f0f349 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -105,7 +105,7 @@ static int device_is_dependent(struct device *dev, void *target)
struct device_link *link;
int ret;

- if (WARN_ON(dev == target))
+ if (dev == target)
return 1;

ret = device_for_each_child(dev, target, device_is_dependent);
@@ -113,7 +113,7 @@ static int device_is_dependent(struct device *dev, void *target)
return ret;

list_for_each_entry(link, &dev->links.consumers, s_node) {
- if (WARN_ON(link->consumer == target))
+ if (link->consumer == target)
return 1;

ret = device_is_dependent(link->consumer, target);
--
2.15.0



2018-07-16 10:09:45

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v2] base: core: Remove WARN_ON from link dependencies check

On Monday, July 16, 2018 11:59:52 AM CEST Benjamin Gaignard wrote:
> In some cases the link between between customer and supplier
> already exist, for example when a device use it parent as supplier [1].

I think this should be "its parent as a supplier".

> Do not warn about already existing dependencies because device_link_add()
> take care of this case.

"device_link_add() takes care of"

> [1] https://lkml.org/lkml/2018/7/9/356

There is the Link: tag for links and it is better to not use lkml.org
as it is not reliable enough. You can use lore.kernel.org/lkml/ instead.

> Reported-by: Marek Szyprowski <[email protected]>
> Reviewed-by: Mark Brown <[email protected]>
> Signed-off-by: Benjamin Gaignard <[email protected]>
> ---
> drivers/base/core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index df3e1a44707a..fcdc17f0f349 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -105,7 +105,7 @@ static int device_is_dependent(struct device *dev, void *target)
> struct device_link *link;
> int ret;
>
> - if (WARN_ON(dev == target))
> + if (dev == target)
> return 1;
>
> ret = device_for_each_child(dev, target, device_is_dependent);
> @@ -113,7 +113,7 @@ static int device_is_dependent(struct device *dev, void *target)
> return ret;
>
> list_for_each_entry(link, &dev->links.consumers, s_node) {
> - if (WARN_ON(link->consumer == target))
> + if (link->consumer == target)
> return 1;
>
> ret = device_is_dependent(link->consumer, target);
>



2018-07-16 10:28:35

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2] base: core: Remove WARN_ON from link dependencies check

On Mon, Jul 16, 2018 at 12:07:20PM +0200, Rafael J. Wysocki wrote:
> On Monday, July 16, 2018 11:59:52 AM CEST Benjamin Gaignard wrote:
> > In some cases the link between between customer and supplier
> > already exist, for example when a device use it parent as supplier [1].
>
> I think this should be "its parent as a supplier".
>
> > Do not warn about already existing dependencies because device_link_add()
> > take care of this case.
>
> "device_link_add() takes care of"
>
> > [1] https://lkml.org/lkml/2018/7/9/356
>
> There is the Link: tag for links and it is better to not use lkml.org
> as it is not reliable enough. You can use lore.kernel.org/lkml/ instead.

Even better, use:
lkml.kernel.org/r/[message-id]

thanks,

greg k-h

2018-07-16 10:42:41

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v2] base: core: Remove WARN_ON from link dependencies check

On Monday, July 16, 2018 12:27:35 PM CEST Greg KH wrote:
> On Mon, Jul 16, 2018 at 12:07:20PM +0200, Rafael J. Wysocki wrote:
> > On Monday, July 16, 2018 11:59:52 AM CEST Benjamin Gaignard wrote:
> > > In some cases the link between between customer and supplier
> > > already exist, for example when a device use it parent as supplier [1].
> >
> > I think this should be "its parent as a supplier".
> >
> > > Do not warn about already existing dependencies because device_link_add()
> > > take care of this case.
> >
> > "device_link_add() takes care of"
> >
> > > [1] https://lkml.org/lkml/2018/7/9/356
> >
> > There is the Link: tag for links and it is better to not use lkml.org
> > as it is not reliable enough. You can use lore.kernel.org/lkml/ instead.
>
> Even better, use:
> lkml.kernel.org/r/[message-id]

Where [message-id] is the value of the Message-Id: header (without the
enclosing <>) of the target e-mail message (that you can find by looking
at the raw message).

Thanks,
Rafael