Messages in this thread | | | Date | Sun, 5 Feb 2023 23:11:34 +0200 | From | Sakari Ailus <> | Subject | Re: [PATCH v11 1/9] device property: Add remote endpoint to devcon matcher |
| |
Hi Pin-yen,
On Sat, Feb 04, 2023 at 09:30:32PM +0800, Pin-yen Lin wrote: > From: Prashant Malani <pmalani@chromium.org> > > When searching the device graph for device matches, check the > remote-endpoint itself for a match. > > Some drivers register devices for individual endpoints. This allows > the matcher code to evaluate those for a match too, instead > of only looking at the remote parent devices. This is required when a > device supports two mode switches in its endpoints, so we can't simply > register the mode switch with the parent node. > > Signed-off-by: Prashant Malani <pmalani@chromium.org> > Signed-off-by: Pin-yen Lin <treapking@chromium.org> > Reviewed-by: Chen-Yu Tsai <wenst@chromium.org> > Tested-by: Chen-Yu Tsai <wenst@chromium.org>
Thanks for the update.
I intended to give my Reviewed-by: but there's something still needs to be addressed. See below.
> > --- > > Changes in v11: > - Added missing fwnode_handle_put in drivers/base/property.c > > Changes in v10: > - Collected Reviewed-by and Tested-by tags > > Changes in v6: > - New in v6 > > drivers/base/property.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/base/property.c b/drivers/base/property.c > index 2a5a37fcd998..e6f915b72eb7 100644 > --- a/drivers/base/property.c > +++ b/drivers/base/property.c > @@ -1223,6 +1223,22 @@ static unsigned int fwnode_graph_devcon_matches(struct fwnode_handle *fwnode, > break; > } > > + /* > + * Some drivers may register devices for endpoints. Check > + * the remote-endpoints for matches in addition to the remote > + * port parent. > + */ > + node = fwnode_graph_get_remote_endpoint(ep);
Here fwnode_graph_get_remote_endpoint() returns an endpoint...
> + if (fwnode_device_is_available(node)) {
and you're calling fwnode_device_is_available() on the endpoint node, which always returns true.
Shouldn't you call this on the device node instead? What about match() below?
> + ret = match(node, con_id, data); > + if (ret) { > + if (matches) > + matches[count] = ret; > + count++; > + } > + } > + fwnode_handle_put(node); > + > node = fwnode_graph_get_remote_port_parent(ep); > if (!fwnode_device_is_available(node)) { > fwnode_handle_put(node);
-- Kind regards,
Sakari Ailus
| |