2021-08-04 23:15:00

by Daniel Scally

[permalink] [raw]
Subject: [PATCH v2 0/2] Check for endpoints in fwnode->secondary more sensibly

Hello all

A while ago I patched fwnode_graph_get_endpoint_by_id() to check for endpoints
against fwnode->secondary if none was found against the primary. It's actually
better to do this in fwnode_graph_get_next_endpoint() instead, since that
function is called by fwnode_graph_get_endpoint_by_id() and also directly called
in a bunch of other places (primarily sensor drivers checking that they have
endpoints connected during probe). This small series just adds the equivalent
functionality to fwnode_graph_get_next_endpoint() and reverts the earlier
commit.

Thanks
Dan

Daniel Scally (2):
device property: Check fwnode->secondary in
fwnode_graph_get_next_endpoint()
Revert "media: device property: Call fwnode_graph_get_endpoint_by_id()
for fwnode->secondary"

drivers/base/property.c | 30 +++++++++++++++++++++---------
1 file changed, 21 insertions(+), 9 deletions(-)

--
2.25.1


2021-08-05 03:54:04

by Daniel Scally

[permalink] [raw]
Subject: [PATCH v2 2/2] Revert "media: device property: Call fwnode_graph_get_endpoint_by_id() for fwnode->secondary"

This reverts commit acd418bfcfc415cf5e6414b6d1c6acfec850f290. Checking for
endpoints against fwnode->secondary in fwnode_graph_get_next_endpoint() is
a better way to do this since that function is also used in a bunch of
other places, for instance sensor drivers checking that they do have an
endpoint connected during probe.

Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Daniel Scally <[email protected]>
---
drivers/base/property.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index fb0e852dad5f..c6bb3d453c57 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1234,14 +1234,7 @@ fwnode_graph_get_endpoint_by_id(const struct fwnode_handle *fwnode,
best_ep_id = fwnode_ep.id;
}

- if (best_ep)
- return best_ep;
-
- if (fwnode && !IS_ERR_OR_NULL(fwnode->secondary))
- return fwnode_graph_get_endpoint_by_id(fwnode->secondary, port,
- endpoint, flags);
-
- return NULL;
+ return best_ep;
}
EXPORT_SYMBOL_GPL(fwnode_graph_get_endpoint_by_id);

--
2.25.1

2021-08-05 03:54:23

by Daniel Scally

[permalink] [raw]
Subject: [PATCH v2 1/2] device property: Check fwnode->secondary in fwnode_graph_get_next_endpoint()

Sensor drivers often check for an endpoint to make sure that they're
connected to a consuming device like a CIO2 during .probe(). Some of
those endpoints might be in the form of software_nodes assigned as
a secondary to the device's fwnode_handle. Account for this possibility
in fwnode_graph_get_next_endpoint() to avoid having to do it in the
sensor drivers themselves.

Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Daniel Scally <[email protected]>
---
Changes in v2:

- Re-ordered the IF_ERR_OR_NULL() checks to make the logical flow a bit
more apparent (Andy)

drivers/base/property.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 1421e9548857..fb0e852dad5f 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1036,7 +1036,26 @@ struct fwnode_handle *
fwnode_graph_get_next_endpoint(const struct fwnode_handle *fwnode,
struct fwnode_handle *prev)
{
- return fwnode_call_ptr_op(fwnode, graph_get_next_endpoint, prev);
+ const struct fwnode_handle *parent;
+ struct fwnode_handle *ep;
+
+ /*
+ * If this function is in a loop and the previous iteration returned
+ * an endpoint from fwnode->secondary, then we need to use the secondary
+ * as parent rather than @fwnode.
+ */
+ if (prev)
+ parent = fwnode_graph_get_port_parent(prev);
+ else
+ parent = fwnode;
+
+ ep = fwnode_call_ptr_op(parent, graph_get_next_endpoint, prev);
+
+ if (IS_ERR_OR_NULL(ep) &&
+ !IS_ERR_OR_NULL(parent) && !IS_ERR_OR_NULL(parent->secondary))
+ ep = fwnode_graph_get_next_endpoint(parent->secondary, NULL);
+
+ return ep;
}
EXPORT_SYMBOL_GPL(fwnode_graph_get_next_endpoint);

--
2.25.1

2021-08-05 09:19:18

by Daniel Scally

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] Revert "media: device property: Call fwnode_graph_get_endpoint_by_id() for fwnode->secondary"

On 05/08/2021 09:38, Greg KH wrote:
> On Thu, Aug 05, 2021 at 12:03:13AM +0100, Daniel Scally wrote:
>> This reverts commit acd418bfcfc415cf5e6414b6d1c6acfec850f290. Checking for
>> endpoints against fwnode->secondary in fwnode_graph_get_next_endpoint() is
>> a better way to do this since that function is also used in a bunch of
>> other places, for instance sensor drivers checking that they do have an
>> endpoint connected during probe.
>>
>> Reviewed-by: Andy Shevchenko <[email protected]>
>> Signed-off-by: Daniel Scally <[email protected]>
>> ---
>> drivers/base/property.c | 9 +--------
>> 1 file changed, 1 insertion(+), 8 deletions(-)
> This can only be reverted due to your 1/2 change, right?


Yes, that's correct.

> If so, you
> might want to make that explicit here...


Fair point - thanks, I'll do that.

>
> thanks,
>
> greg k-h

2021-08-05 12:19:12

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] Revert "media: device property: Call fwnode_graph_get_endpoint_by_id() for fwnode->secondary"

On Thu, Aug 05, 2021 at 12:03:13AM +0100, Daniel Scally wrote:
> This reverts commit acd418bfcfc415cf5e6414b6d1c6acfec850f290. Checking for
> endpoints against fwnode->secondary in fwnode_graph_get_next_endpoint() is
> a better way to do this since that function is also used in a bunch of
> other places, for instance sensor drivers checking that they do have an
> endpoint connected during probe.
>
> Reviewed-by: Andy Shevchenko <[email protected]>
> Signed-off-by: Daniel Scally <[email protected]>
> ---
> drivers/base/property.c | 9 +--------
> 1 file changed, 1 insertion(+), 8 deletions(-)

This can only be reverted due to your 1/2 change, right? If so, you
might want to make that explicit here...

thanks,

greg k-h