Hi
> -----Original Message-----
> From: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> Sent: Monday, May 31, 2021 4:26 PM
> To: Jun Li <jun.li@xxxxxxx>; Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: Hans de Goede <hdegoede@xxxxxxxxxx>; linux-usb@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 2/2] usb: typec: mux: Remove requirement for the
> "orientation-switch" device property
>
> On Mon, May 31, 2021 at 10:57:04AM +0300, Heikki Krogerus wrote:
> > On Mon, May 31, 2021 at 10:24:35AM +0300, Heikki Krogerus wrote:
> > > On Fri, May 28, 2021 at 07:26:43AM +0000, Jun Li wrote:
> > > > Hi,
> > > > > -----Original Message-----
> > > > > From: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> > > > > Sent: Wednesday, May 26, 2021 11:36 PM
> > > > > To: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>; Hans de
> > > > > Goede <hdegoede@xxxxxxxxxx>; Jun Li <jun.li@xxxxxxx>
> > > > > Cc: linux-usb@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> > > > > Subject: [PATCH 2/2] usb: typec: mux: Remove requirement for the
> > > > > "orientation-switch" device property
> > > > >
> > > > > The additional boolean device property "orientation-switch"
> > > > > is not needed when the connection is described with device
> > > > > graph, so removing the check and the requirement for it.
> > > > >
> > > > > Signed-off-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> > > > > ---
> > > > > drivers/usb/typec/mux.c | 3 ---
> > > > > 1 file changed, 3 deletions(-)
> > > > >
> > > > > diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
> > > > > index e40a555724fb6..603f3e698cc0b 100644
> > > > > --- a/drivers/usb/typec/mux.c
> > > > > +++ b/drivers/usb/typec/mux.c
> > > > > @@ -30,9 +30,6 @@ static void *typec_switch_match(struct
> > > > > fwnode_handle *fwnode, const char *id, {
> > > > > struct device *dev;
> > > > >
> > > > > - if (id && !fwnode_property_present(fwnode, id))
> > > > > - return NULL;
> > > > > -
> > > >
> > > > May this change the result of fwnode_connection_find_match() if
> > > > there are multiple remote-endpoint node?
> > > >
> > > > After the 2 patches change, typec_switch_match() will never return
> > > > NULL, so
> > > >
> > > > 17 static void *
> > > > 18 fwnode_graph_devcon_match(struct fwnode_handle *fwnode, const
> char *con_id,
> > > > 19 void *data, devcon_match_fn_t match)
> > > > 20 {
> > > > 21 struct fwnode_handle *node;
> > > > 22 struct fwnode_handle *ep;
> > > > 23 void *ret;
> > > > 24
> > > > 25 fwnode_graph_for_each_endpoint(fwnode, ep) {
> > > > 26 node = fwnode_graph_get_remote_port_parent(ep);
> > > > 27 if (!fwnode_device_is_available(node))
> > > > 28 continue;
> > > > 29
> > > > 30 ret = match(node, con_id, data);// ret can't be
> NULL;
> > > > 31 fwnode_handle_put(node);
> > > > 32 if (ret) {
> > > > /*
> > > > * So loop will go to here and stop
> > > > * checking next ep, even this ep
> > > > * actually is not for typec_switch
> > > > */
> > > > 33 fwnode_handle_put(ep);
> > > > 34 return ret;
> > > > 35 }
> > > > 36 }
> > > > 37 return NULL;
> > > > 38 }
> > > >
> > > > fwnode_graph_devcon_match() Will return ERR_PTR(-EPROBE_DEFER)
> > > > even this ep's remote parent already probed but it's not for
> > > > typec_switch.
> > >
> > > You are correct. With device graph I guess we really always need the
> > > extra device property after all.
> > >
> > > So let's forget about this one.
> >
> > Oh no. This patch just landed into Greg's usb-next. I'll prepare the
> > revert. I'm sorry about this.
>
> Actually, if we always need that extra (boolean) device property to identify
> the device class when OF graph is used,
Looks like yes, as we need a way to know if the current fwnode
is for the target device we are looking for, to return probe
defer correctly.
> shouldn't we just do that always
> in fwnode_graph_devcon_match()?
This depends on if we want to limit this to be a boolean property
(to mark this is the target fwnode), or make it to be more generic
so user can define it in its ->match().
Now there are only 2 users of it, role switch and typec mux, both work
as a boolean property for con_id.
Li Jun
>
> diff --git a/drivers/base/property.c b/drivers/base/property.c index
> 1421e9548857b..238da64375bb1 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -1263,6 +1263,13 @@ fwnode_graph_devcon_match(struct fwnode_handle
> *fwnode, const char *con_id,
> if (!fwnode_device_is_available(node))
> continue;
>
> + /*
> + * With device graph @con_id is expected to be the name of
> the
> + * "device class" of the fwnode.
> + */
> + if (con_id && !fwnode_property_present(node, con_id))
> + continue;
> +
> ret = match(node, con_id, data);
> fwnode_handle_put(node);
> if (ret) {
>
> thanks,
>
> --
> heikki
