* [PATCH 0/2] usb: typec: mux: a few improvements
@ 2021-05-26 15:35 Heikki Krogerus
2021-05-26 15:35 ` [PATCH 1/2] usb: typec: mux: Use device type instead of device name for matching Heikki Krogerus
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Heikki Krogerus @ 2021-05-26 15:35 UTC (permalink / raw)
To: Greg Kroah-Hartman, Hans de Goede, Li Jun; +Cc: linux-usb, linux-kernel
Hi,
The first patch should be trivial cleanup, but in the second one I'm
removing the condition that in practice forces the orientation switch
to always have the device property named "orientation-switch". This
change only impacts connections described with device graph since only
in that case the callback function is supplied with the "id" parameter
which is the first part of the condition (if the id is not supplied,
the function also does not expect the device property to exist).
But when the connection between the connector and the switch is
described with device graph, I don't see any need for that device
property. Therefore let's just remove the condition and the
requirement for the device property with it.
thanks,
Heikki Krogerus (2):
usb: typec: mux: Use device type instead of device name for matching
usb: typec: mux: Remove requirement for the "orientation-switch"
device property
drivers/usb/typec/mux.c | 29 ++++++++++-------------------
drivers/usb/typec/mux.h | 6 ++++++
2 files changed, 16 insertions(+), 19 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/2] usb: typec: mux: Use device type instead of device name for matching 2021-05-26 15:35 [PATCH 0/2] usb: typec: mux: a few improvements Heikki Krogerus @ 2021-05-26 15:35 ` Heikki Krogerus 2021-05-31 7:30 ` Heikki Krogerus 2021-05-26 15:35 ` [PATCH 2/2] usb: typec: mux: Remove requirement for the "orientation-switch" device property Heikki Krogerus 2021-05-26 16:38 ` [PATCH 0/2] usb: typec: mux: a few improvements Hans de Goede 2 siblings, 1 reply; 12+ messages in thread From: Heikki Krogerus @ 2021-05-26 15:35 UTC (permalink / raw) To: Greg Kroah-Hartman, Hans de Goede, Li Jun; +Cc: linux-usb, linux-kernel Both the USB Type-C switch and mux have already a device type defined for them. We can use those types instead of the device name to differentiate the two. Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> --- drivers/usb/typec/mux.c | 26 ++++++++++---------------- drivers/usb/typec/mux.h | 6 ++++++ 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c index 8514bec7e1b89..e40a555724fb6 100644 --- a/drivers/usb/typec/mux.c +++ b/drivers/usb/typec/mux.c @@ -17,21 +17,12 @@ #include "class.h" #include "mux.h" -static bool dev_name_ends_with(struct device *dev, const char *suffix) -{ - const char *name = dev_name(dev); - const int name_len = strlen(name); - const int suffix_len = strlen(suffix); - - if (suffix_len > name_len) - return false; - - return strcmp(name + (name_len - suffix_len), suffix) == 0; -} - static int switch_fwnode_match(struct device *dev, const void *fwnode) { - return dev_fwnode(dev) == fwnode && dev_name_ends_with(dev, "-switch"); + if (!is_typec_switch(dev)) + return 0; + + return dev_fwnode(dev) == fwnode; } static void *typec_switch_match(struct fwnode_handle *fwnode, const char *id, @@ -90,7 +81,7 @@ static void typec_switch_release(struct device *dev) kfree(to_typec_switch(dev)); } -static const struct device_type typec_switch_dev_type = { +const struct device_type typec_switch_dev_type = { .name = "orientation_switch", .release = typec_switch_release, }; @@ -180,7 +171,10 @@ EXPORT_SYMBOL_GPL(typec_switch_get_drvdata); static int mux_fwnode_match(struct device *dev, const void *fwnode) { - return dev_fwnode(dev) == fwnode && dev_name_ends_with(dev, "-mux"); + if (!is_typec_mux(dev)) + return 0; + + return dev_fwnode(dev) == fwnode; } static void *typec_mux_match(struct fwnode_handle *fwnode, const char *id, @@ -295,7 +289,7 @@ static void typec_mux_release(struct device *dev) kfree(to_typec_mux(dev)); } -static const struct device_type typec_mux_dev_type = { +const struct device_type typec_mux_dev_type = { .name = "mode_switch", .release = typec_mux_release, }; diff --git a/drivers/usb/typec/mux.h b/drivers/usb/typec/mux.h index 4fd9426ee44f6..b1d6e837cb747 100644 --- a/drivers/usb/typec/mux.h +++ b/drivers/usb/typec/mux.h @@ -18,4 +18,10 @@ struct typec_mux { #define to_typec_switch(_dev_) container_of(_dev_, struct typec_switch, dev) #define to_typec_mux(_dev_) container_of(_dev_, struct typec_mux, dev) +extern const struct device_type typec_switch_dev_type; +extern const struct device_type typec_mux_dev_type; + +#define is_typec_switch(dev) ((dev)->type == &typec_switch_dev_type) +#define is_typec_mux(dev) ((dev)->type == &typec_mux_dev_type) + #endif /* __USB_TYPEC_MUX__ */ -- 2.30.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] usb: typec: mux: Use device type instead of device name for matching 2021-05-26 15:35 ` [PATCH 1/2] usb: typec: mux: Use device type instead of device name for matching Heikki Krogerus @ 2021-05-31 7:30 ` Heikki Krogerus 2021-05-31 8:09 ` Hans de Goede 0 siblings, 1 reply; 12+ messages in thread From: Heikki Krogerus @ 2021-05-31 7:30 UTC (permalink / raw) To: Greg Kroah-Hartman, Hans de Goede, Li Jun; +Cc: linux-usb, linux-kernel On Wed, May 26, 2021 at 06:35:47PM +0300, Heikki Krogerus wrote: > Both the USB Type-C switch and mux have already a device > type defined for them. We can use those types instead of the > device name to differentiate the two. This should still be OK, right? > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > drivers/usb/typec/mux.c | 26 ++++++++++---------------- > drivers/usb/typec/mux.h | 6 ++++++ > 2 files changed, 16 insertions(+), 16 deletions(-) > > diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c > index 8514bec7e1b89..e40a555724fb6 100644 > --- a/drivers/usb/typec/mux.c > +++ b/drivers/usb/typec/mux.c > @@ -17,21 +17,12 @@ > #include "class.h" > #include "mux.h" > > -static bool dev_name_ends_with(struct device *dev, const char *suffix) > -{ > - const char *name = dev_name(dev); > - const int name_len = strlen(name); > - const int suffix_len = strlen(suffix); > - > - if (suffix_len > name_len) > - return false; > - > - return strcmp(name + (name_len - suffix_len), suffix) == 0; > -} > - > static int switch_fwnode_match(struct device *dev, const void *fwnode) > { > - return dev_fwnode(dev) == fwnode && dev_name_ends_with(dev, "-switch"); > + if (!is_typec_switch(dev)) > + return 0; > + > + return dev_fwnode(dev) == fwnode; > } > > static void *typec_switch_match(struct fwnode_handle *fwnode, const char *id, > @@ -90,7 +81,7 @@ static void typec_switch_release(struct device *dev) > kfree(to_typec_switch(dev)); > } > > -static const struct device_type typec_switch_dev_type = { > +const struct device_type typec_switch_dev_type = { > .name = "orientation_switch", > .release = typec_switch_release, > }; > @@ -180,7 +171,10 @@ EXPORT_SYMBOL_GPL(typec_switch_get_drvdata); > > static int mux_fwnode_match(struct device *dev, const void *fwnode) > { > - return dev_fwnode(dev) == fwnode && dev_name_ends_with(dev, "-mux"); > + if (!is_typec_mux(dev)) > + return 0; > + > + return dev_fwnode(dev) == fwnode; > } > > static void *typec_mux_match(struct fwnode_handle *fwnode, const char *id, > @@ -295,7 +289,7 @@ static void typec_mux_release(struct device *dev) > kfree(to_typec_mux(dev)); > } > > -static const struct device_type typec_mux_dev_type = { > +const struct device_type typec_mux_dev_type = { > .name = "mode_switch", > .release = typec_mux_release, > }; > diff --git a/drivers/usb/typec/mux.h b/drivers/usb/typec/mux.h > index 4fd9426ee44f6..b1d6e837cb747 100644 > --- a/drivers/usb/typec/mux.h > +++ b/drivers/usb/typec/mux.h > @@ -18,4 +18,10 @@ struct typec_mux { > #define to_typec_switch(_dev_) container_of(_dev_, struct typec_switch, dev) > #define to_typec_mux(_dev_) container_of(_dev_, struct typec_mux, dev) > > +extern const struct device_type typec_switch_dev_type; > +extern const struct device_type typec_mux_dev_type; > + > +#define is_typec_switch(dev) ((dev)->type == &typec_switch_dev_type) > +#define is_typec_mux(dev) ((dev)->type == &typec_mux_dev_type) > + > #endif /* __USB_TYPEC_MUX__ */ > -- > 2.30.2 -- heikki ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] usb: typec: mux: Use device type instead of device name for matching 2021-05-31 7:30 ` Heikki Krogerus @ 2021-05-31 8:09 ` Hans de Goede 0 siblings, 0 replies; 12+ messages in thread From: Hans de Goede @ 2021-05-31 8:09 UTC (permalink / raw) To: Heikki Krogerus, Greg Kroah-Hartman, Li Jun; +Cc: linux-usb, linux-kernel Hi, On 5/31/21 9:30 AM, Heikki Krogerus wrote: > On Wed, May 26, 2021 at 06:35:47PM +0300, Heikki Krogerus wrote: >> Both the USB Type-C switch and mux have already a device >> type defined for them. We can use those types instead of the >> device name to differentiate the two. > > This should still be OK, right? I believe so and a dev_type check also seems more robust then a name-suffix check. Regards, Hans > >> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> >> --- >> drivers/usb/typec/mux.c | 26 ++++++++++---------------- >> drivers/usb/typec/mux.h | 6 ++++++ >> 2 files changed, 16 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c >> index 8514bec7e1b89..e40a555724fb6 100644 >> --- a/drivers/usb/typec/mux.c >> +++ b/drivers/usb/typec/mux.c >> @@ -17,21 +17,12 @@ >> #include "class.h" >> #include "mux.h" >> >> -static bool dev_name_ends_with(struct device *dev, const char *suffix) >> -{ >> - const char *name = dev_name(dev); >> - const int name_len = strlen(name); >> - const int suffix_len = strlen(suffix); >> - >> - if (suffix_len > name_len) >> - return false; >> - >> - return strcmp(name + (name_len - suffix_len), suffix) == 0; >> -} >> - >> static int switch_fwnode_match(struct device *dev, const void *fwnode) >> { >> - return dev_fwnode(dev) == fwnode && dev_name_ends_with(dev, "-switch"); >> + if (!is_typec_switch(dev)) >> + return 0; >> + >> + return dev_fwnode(dev) == fwnode; >> } >> >> static void *typec_switch_match(struct fwnode_handle *fwnode, const char *id, >> @@ -90,7 +81,7 @@ static void typec_switch_release(struct device *dev) >> kfree(to_typec_switch(dev)); >> } >> >> -static const struct device_type typec_switch_dev_type = { >> +const struct device_type typec_switch_dev_type = { >> .name = "orientation_switch", >> .release = typec_switch_release, >> }; >> @@ -180,7 +171,10 @@ EXPORT_SYMBOL_GPL(typec_switch_get_drvdata); >> >> static int mux_fwnode_match(struct device *dev, const void *fwnode) >> { >> - return dev_fwnode(dev) == fwnode && dev_name_ends_with(dev, "-mux"); >> + if (!is_typec_mux(dev)) >> + return 0; >> + >> + return dev_fwnode(dev) == fwnode; >> } >> >> static void *typec_mux_match(struct fwnode_handle *fwnode, const char *id, >> @@ -295,7 +289,7 @@ static void typec_mux_release(struct device *dev) >> kfree(to_typec_mux(dev)); >> } >> >> -static const struct device_type typec_mux_dev_type = { >> +const struct device_type typec_mux_dev_type = { >> .name = "mode_switch", >> .release = typec_mux_release, >> }; >> diff --git a/drivers/usb/typec/mux.h b/drivers/usb/typec/mux.h >> index 4fd9426ee44f6..b1d6e837cb747 100644 >> --- a/drivers/usb/typec/mux.h >> +++ b/drivers/usb/typec/mux.h >> @@ -18,4 +18,10 @@ struct typec_mux { >> #define to_typec_switch(_dev_) container_of(_dev_, struct typec_switch, dev) >> #define to_typec_mux(_dev_) container_of(_dev_, struct typec_mux, dev) >> >> +extern const struct device_type typec_switch_dev_type; >> +extern const struct device_type typec_mux_dev_type; >> + >> +#define is_typec_switch(dev) ((dev)->type == &typec_switch_dev_type) >> +#define is_typec_mux(dev) ((dev)->type == &typec_mux_dev_type) >> + >> #endif /* __USB_TYPEC_MUX__ */ >> -- >> 2.30.2 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] usb: typec: mux: Remove requirement for the "orientation-switch" device property 2021-05-26 15:35 [PATCH 0/2] usb: typec: mux: a few improvements Heikki Krogerus 2021-05-26 15:35 ` [PATCH 1/2] usb: typec: mux: Use device type instead of device name for matching Heikki Krogerus @ 2021-05-26 15:35 ` Heikki Krogerus 2021-05-28 7:26 ` Jun Li 2021-05-26 16:38 ` [PATCH 0/2] usb: typec: mux: a few improvements Hans de Goede 2 siblings, 1 reply; 12+ messages in thread From: Heikki Krogerus @ 2021-05-26 15:35 UTC (permalink / raw) To: Greg Kroah-Hartman, Hans de Goede, Li Jun; +Cc: linux-usb, linux-kernel 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@linux.intel.com> --- 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; - dev = class_find_device(&typec_mux_class, NULL, fwnode, switch_fwnode_match); -- 2.30.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* RE: [PATCH 2/2] usb: typec: mux: Remove requirement for the "orientation-switch" device property 2021-05-26 15:35 ` [PATCH 2/2] usb: typec: mux: Remove requirement for the "orientation-switch" device property Heikki Krogerus @ 2021-05-28 7:26 ` Jun Li 2021-05-31 7:24 ` Heikki Krogerus 0 siblings, 1 reply; 12+ messages in thread From: Jun Li @ 2021-05-28 7:26 UTC (permalink / raw) To: Heikki Krogerus, Greg Kroah-Hartman, Hans de Goede Cc: linux-usb, linux-kernel Hi, > -----Original Message----- > From: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Sent: Wednesday, May 26, 2021 11:36 PM > To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Hans de Goede > <hdegoede@redhat.com>; Jun Li <jun.li@nxp.com> > Cc: linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org > 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@linux.intel.com> > --- > 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. Li Jun > dev = class_find_device(&typec_mux_class, NULL, fwnode, > switch_fwnode_match); > > -- > 2.30.2 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] usb: typec: mux: Remove requirement for the "orientation-switch" device property 2021-05-28 7:26 ` Jun Li @ 2021-05-31 7:24 ` Heikki Krogerus 2021-05-31 7:57 ` Heikki Krogerus 0 siblings, 1 reply; 12+ messages in thread From: Heikki Krogerus @ 2021-05-31 7:24 UTC (permalink / raw) To: Jun Li, Greg Kroah-Hartman; +Cc: Hans de Goede, linux-usb, linux-kernel On Fri, May 28, 2021 at 07:26:43AM +0000, Jun Li wrote: > Hi, > > -----Original Message----- > > From: Heikki Krogerus <heikki.krogerus@linux.intel.com> > > Sent: Wednesday, May 26, 2021 11:36 PM > > To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Hans de Goede > > <hdegoede@redhat.com>; Jun Li <jun.li@nxp.com> > > Cc: linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org > > 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@linux.intel.com> > > --- > > 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. thanks, -- heikki ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] usb: typec: mux: Remove requirement for the "orientation-switch" device property 2021-05-31 7:24 ` Heikki Krogerus @ 2021-05-31 7:57 ` Heikki Krogerus 2021-05-31 8:26 ` Heikki Krogerus 0 siblings, 1 reply; 12+ messages in thread From: Heikki Krogerus @ 2021-05-31 7:57 UTC (permalink / raw) To: Jun Li, Greg Kroah-Hartman; +Cc: Hans de Goede, linux-usb, linux-kernel 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@linux.intel.com> > > > Sent: Wednesday, May 26, 2021 11:36 PM > > > To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Hans de Goede > > > <hdegoede@redhat.com>; Jun Li <jun.li@nxp.com> > > > Cc: linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org > > > 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@linux.intel.com> > > > --- > > > 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. thanks, -- heikki ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] usb: typec: mux: Remove requirement for the "orientation-switch" device property 2021-05-31 7:57 ` Heikki Krogerus @ 2021-05-31 8:26 ` Heikki Krogerus 2021-05-31 11:08 ` Jun Li 0 siblings, 1 reply; 12+ messages in thread From: Heikki Krogerus @ 2021-05-31 8:26 UTC (permalink / raw) To: Jun Li, Greg Kroah-Hartman; +Cc: Hans de Goede, linux-usb, linux-kernel 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@linux.intel.com> > > > > Sent: Wednesday, May 26, 2021 11:36 PM > > > > To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Hans de Goede > > > > <hdegoede@redhat.com>; Jun Li <jun.li@nxp.com> > > > > Cc: linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org > > > > 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@linux.intel.com> > > > > --- > > > > 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, shouldn't we just do that always in fwnode_graph_devcon_match()? 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* RE: [PATCH 2/2] usb: typec: mux: Remove requirement for the "orientation-switch" device property 2021-05-31 8:26 ` Heikki Krogerus @ 2021-05-31 11:08 ` Jun Li 2021-05-31 12:33 ` Heikki Krogerus 0 siblings, 1 reply; 12+ messages in thread From: Jun Li @ 2021-05-31 11:08 UTC (permalink / raw) To: Heikki Krogerus, Greg Kroah-Hartman Cc: Hans de Goede, linux-usb, linux-kernel Hi > -----Original Message----- > From: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Sent: Monday, May 31, 2021 4:26 PM > To: Jun Li <jun.li@nxp.com>; Greg Kroah-Hartman > <gregkh@linuxfoundation.org> > Cc: Hans de Goede <hdegoede@redhat.com>; linux-usb@vger.kernel.org; > linux-kernel@vger.kernel.org > 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@linux.intel.com> > > > > > Sent: Wednesday, May 26, 2021 11:36 PM > > > > > To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Hans de > > > > > Goede <hdegoede@redhat.com>; Jun Li <jun.li@nxp.com> > > > > > Cc: linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org > > > > > 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@linux.intel.com> > > > > > --- > > > > > 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] usb: typec: mux: Remove requirement for the "orientation-switch" device property 2021-05-31 11:08 ` Jun Li @ 2021-05-31 12:33 ` Heikki Krogerus 0 siblings, 0 replies; 12+ messages in thread From: Heikki Krogerus @ 2021-05-31 12:33 UTC (permalink / raw) To: Jun Li; +Cc: Greg Kroah-Hartman, Hans de Goede, linux-usb, linux-kernel > > > 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. OK. Let's leave to the users for now. thanks, -- heikki ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] usb: typec: mux: a few improvements 2021-05-26 15:35 [PATCH 0/2] usb: typec: mux: a few improvements Heikki Krogerus 2021-05-26 15:35 ` [PATCH 1/2] usb: typec: mux: Use device type instead of device name for matching Heikki Krogerus 2021-05-26 15:35 ` [PATCH 2/2] usb: typec: mux: Remove requirement for the "orientation-switch" device property Heikki Krogerus @ 2021-05-26 16:38 ` Hans de Goede 2 siblings, 0 replies; 12+ messages in thread From: Hans de Goede @ 2021-05-26 16:38 UTC (permalink / raw) To: Heikki Krogerus, Greg Kroah-Hartman, Li Jun; +Cc: linux-usb, linux-kernel Hi, On 5/26/21 5:35 PM, Heikki Krogerus wrote: > Hi, > > The first patch should be trivial cleanup, but in the second one I'm > removing the condition that in practice forces the orientation switch > to always have the device property named "orientation-switch". This > change only impacts connections described with device graph since only > in that case the callback function is supplied with the "id" parameter > which is the first part of the condition (if the id is not supplied, > the function also does not expect the device property to exist). > > But when the connection between the connector and the switch is > described with device graph, I don't see any need for that device > property. Therefore let's just remove the condition and the > requirement for the device property with it. > > thanks, > > Heikki Krogerus (2): > usb: typec: mux: Use device type instead of device name for matching > usb: typec: mux: Remove requirement for the "orientation-switch" > device property Thanks, the series looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> for the series. Regards, Hans ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-05-31 12:34 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-26 15:35 [PATCH 0/2] usb: typec: mux: a few improvements Heikki Krogerus 2021-05-26 15:35 ` [PATCH 1/2] usb: typec: mux: Use device type instead of device name for matching Heikki Krogerus 2021-05-31 7:30 ` Heikki Krogerus 2021-05-31 8:09 ` Hans de Goede 2021-05-26 15:35 ` [PATCH 2/2] usb: typec: mux: Remove requirement for the "orientation-switch" device property Heikki Krogerus 2021-05-28 7:26 ` Jun Li 2021-05-31 7:24 ` Heikki Krogerus 2021-05-31 7:57 ` Heikki Krogerus 2021-05-31 8:26 ` Heikki Krogerus 2021-05-31 11:08 ` Jun Li 2021-05-31 12:33 ` Heikki Krogerus 2021-05-26 16:38 ` [PATCH 0/2] usb: typec: mux: a few improvements Hans de Goede
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.