2022-07-15 09:20:52

by Marcin Wojtas

[permalink] [raw]
Subject: [net-next: PATCH v2 5/8] device property: introduce fwnode_dev_node_match

This patch adds a new generic routine fwnode_dev_node_match
that can be used e.g. as a callback for class_find_device().
It searches for the struct device corresponding to a
struct fwnode_handle by iterating over device and
its parents.

Signed-off-by: Marcin Wojtas <[email protected]>
---
include/linux/property.h | 2 ++
drivers/base/property.c | 22 ++++++++++++++++++++
2 files changed, 24 insertions(+)

diff --git a/include/linux/property.h b/include/linux/property.h
index 23330ae2b1fa..21b59ad08a39 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -456,6 +456,8 @@ int fwnode_connection_find_matches(struct fwnode_handle *fwnode,
devcon_match_fn_t match,
void **matches, unsigned int matches_len);

+int fwnode_dev_node_match(struct device *dev, const void *data);
+
/* -------------------------------------------------------------------------- */
/* Software fwnode support - when HW description is incomplete or missing */

diff --git a/drivers/base/property.c b/drivers/base/property.c
index ed6f449f8e5c..839e7d586129 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1344,3 +1344,25 @@ int fwnode_connection_find_matches(struct fwnode_handle *fwnode,
return count_graph + count_ref;
}
EXPORT_SYMBOL_GPL(fwnode_connection_find_matches);
+
+/*
+ * fwnode_dev_node_match - look for a device matching the struct fwnode_handle
+ * @dev: the struct device to initiate the search
+ * @data: pointer to the fwnode_handle
+ *
+ * Looks up the device structure corresponding with the fwnode by iterating
+ * over @dev and its parents.
+ * The routine can be used e.g. as a callback for class_find_device().
+ *
+ * Return: 1 - if match is found, 0 - otherwise.
+ */
+int fwnode_dev_node_match(struct device *dev, const void *data)
+{
+ for (; dev; dev = dev->parent) {
+ if (device_match_fwnode(dev, data))
+ return 1;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(fwnode_dev_node_match);
--
2.29.0


2022-07-15 19:39:30

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [net-next: PATCH v2 5/8] device property: introduce fwnode_dev_node_match

On Fri, Jul 15, 2022 at 10:50:09AM +0200, Marcin Wojtas wrote:
> This patch adds a new generic routine fwnode_dev_node_match
> that can be used e.g. as a callback for class_find_device().
> It searches for the struct device corresponding to a
> struct fwnode_handle by iterating over device and
> its parents.

Implementation
1) misses the word 'parent';
2) located outside of the group of fwnode APIs operating on parents.

I would suggest to rename to fwnode_get_next_parent_node() and place
near to fwnode_get_next_parent_dev() (either before or after, where
it makes more sense).

--
With Best Regards,
Andy Shevchenko


2022-07-15 20:00:16

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [net-next: PATCH v2 5/8] device property: introduce fwnode_dev_node_match

On Fri, Jul 15, 2022 at 10:36:29PM +0300, Andy Shevchenko wrote:
> On Fri, Jul 15, 2022 at 10:50:09AM +0200, Marcin Wojtas wrote:
> > This patch adds a new generic routine fwnode_dev_node_match
> > that can be used e.g. as a callback for class_find_device().
> > It searches for the struct device corresponding to a
> > struct fwnode_handle by iterating over device and
> > its parents.
>
> Implementation
> 1) misses the word 'parent';
> 2) located outside of the group of fwnode APIs operating on parents.
>
> I would suggest to rename to fwnode_get_next_parent_node() and place
> near to fwnode_get_next_parent_dev() (either before or after, where
> it makes more sense).

And matching function will be after that:

return fwnode_get_next_parent_node(...) != NULL;

Think about it. Maybe current solution is good enough, just needs better
naming (fwnode_match_parent_node()? Dunno).

P.S. Actually _get maybe misleading as we won't bump reference counting,
rather _find?

--
With Best Regards,
Andy Shevchenko


2022-07-15 23:46:41

by Marcin Wojtas

[permalink] [raw]
Subject: Re: [net-next: PATCH v2 5/8] device property: introduce fwnode_dev_node_match

pt., 15 lip 2022 o 21:42 Andy Shevchenko
<[email protected]> napisał(a):
>
> On Fri, Jul 15, 2022 at 10:36:29PM +0300, Andy Shevchenko wrote:
> > On Fri, Jul 15, 2022 at 10:50:09AM +0200, Marcin Wojtas wrote:
> > > This patch adds a new generic routine fwnode_dev_node_match
> > > that can be used e.g. as a callback for class_find_device().
> > > It searches for the struct device corresponding to a
> > > struct fwnode_handle by iterating over device and
> > > its parents.
> >
> > Implementation
> > 1) misses the word 'parent';

I'm not sure. We don't necessarily look for parent device(s). We start
with a struct device and if it matches the fwnode, success is returned
immediately. Only otherwise we iterate over parent devices to find a
match.

> > 2) located outside of the group of fwnode APIs operating on parents.

I can shift it right below fwnode_get_nth_parent if you prefer.

> >
> > I would suggest to rename to fwnode_get_next_parent_node() and place
> > near to fwnode_get_next_parent_dev() (either before or after, where
> > it makes more sense).
>
> And matching function will be after that:
>
> return fwnode_get_next_parent_node(...) != NULL;
>
> Think about it. Maybe current solution is good enough, just needs better
> naming (fwnode_match_parent_node()? Dunno).
>
> P.S. Actually _get maybe misleading as we won't bump reference counting,
> rather _find?
>

How about the following name:
fwnode_find_dev_match()
?

Thanks,
Marcin

2022-07-18 12:57:54

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [net-next: PATCH v2 5/8] device property: introduce fwnode_dev_node_match

On Sat, Jul 16, 2022 at 01:15:55AM +0200, Marcin Wojtas wrote:
> pt., 15 lip 2022 o 21:42 Andy Shevchenko
> <[email protected]> napisał(a):
> >
> > On Fri, Jul 15, 2022 at 10:36:29PM +0300, Andy Shevchenko wrote:
> > > On Fri, Jul 15, 2022 at 10:50:09AM +0200, Marcin Wojtas wrote:
> > > > This patch adds a new generic routine fwnode_dev_node_match
> > > > that can be used e.g. as a callback for class_find_device().
> > > > It searches for the struct device corresponding to a
> > > > struct fwnode_handle by iterating over device and
> > > > its parents.
> > >
> > > Implementation
> > > 1) misses the word 'parent';
>
> I'm not sure. We don't necessarily look for parent device(s). We start
> with a struct device and if it matches the fwnode, success is returned
> immediately. Only otherwise we iterate over parent devices to find a
> match.

Yes, you iterate over parents. 0 iterations doesn't change semantics of
all cases, right?

> > > 2) located outside of the group of fwnode APIs operating on parents.
>
> I can shift it right below fwnode_get_nth_parent if you prefer.

Yes, please do.

> > > I would suggest to rename to fwnode_get_next_parent_node() and place
> > > near to fwnode_get_next_parent_dev() (either before or after, where
> > > it makes more sense).
> >
> > And matching function will be after that:
> >
> > return fwnode_get_next_parent_node(...) != NULL;
> >
> > Think about it. Maybe current solution is good enough, just needs better
> > naming (fwnode_match_parent_node()? Dunno).
> >
> > P.S. Actually _get maybe misleading as we won't bump reference counting,
> > rather _find?
>
> How about the following name:
> fwnode_find_dev_match()
> ?

fwnode_find_parent_dev_match() LGTM, thanks!

You iterate over parents.

--
With Best Regards,
Andy Shevchenko