[v1,07/12] usb: roles: Find the usb role switch by also matching against the device node

[v1,07/12] usb: roles: Find the usb role switch by also matching against the device node

[Heikki Krogerus]

Hi,

On Mon, Dec 03, 2018 at 10:17:20AM +0200, Andy Shevchenko wrote:
> On Mon, Dec 3, 2018 at 5:45 AM Yu Chen <chenyu56@huawei.com> wrote:
> >
> > This patch adds code for supporting find usb role switch by matching against
> > the device node described using of_graph.
> 
> >  #include <linux/module.h>
> >  #include <linux/mutex.h>
> >  #include <linux/slab.h>
> > +#include <linux/of.h>
> > +#include <linux/of_graph.h>
> 
> Keep it ordered.
> 
> > +static int __switch_match_by_of_node(struct device *dev, const void *name)
> > +{
> > +       if (!dev->parent || !dev->parent->of_node)
> > +               return 0;
> > +
> > +       return of_node_name_eq(dev->parent->of_node, (const char *)name);
> > +}
> 
> I think Heikki has introduced some more generic function for this.

Yes, for this I'm proposing a new function fwnode_get_name() that
should allow us to support ACPI as well as OF platforms:
https://lkml.org/lkml/2018/11/28/784

> >         sw = device_connection_find_match(dev, "usb-role-switch", NULL,
> >                                           usb_role_switch_match);
> 
> > +       sw = of_graph_find_match_by_type(dev, "usb-role-switch");
> 
> Why this is here?! device_connection_find_match() should take care for OF case.

Exactly. device_connection_find_match() needs to parse the graph (OF
and ACPI), and I already made proposal (RFC) for that:
https://lkml.org/lkml/2018/10/24/619

I have prepared a more finalized version of those patches. I've been
waiting for that fwnode_get_name() to be accepted before sending it
out since we are going to need that function, but I'll send a new RFC
if nothing happens soon with that fwnode_get_name() series.


Br,

[v1,07/12] usb: roles: Find the usb role switch by also matching against the device node

[Heikki Krogerus]

On Mon, Jan 21, 2019 at 03:51:35PM +0200, Heikki Krogerus wrote:
> Hi,
> 
> On Mon, Jan 21, 2019 at 04:36:13PM +0800, Chen Yu wrote:
> > Have the fwnode_get_name() patches been accepted?
> 
> No, I didn't have a user for the function anymore:
> https://lkml.org/lkml/2018/12/17/284
> 
> > And I will be grateful if you can tell me your plan about the
> > https://lkml.org/lkml/2018/10/24/619 patches.
> 
> The plan is to send a new version of them together with the
> fwnode_get_name() patch. I can send them this week.

I'm about to send the graph support for the device connection API, but
just letting you know that I did not need the fwnode_get_name() in the
end in that series unfortunately.


Br,

[v1,07/12] usb: roles: Find the usb role switch by also matching against the device node

[Heikki Krogerus]

Hi,

On Mon, Jan 21, 2019 at 04:36:13PM +0800, Chen Yu wrote:
> Have the fwnode_get_name() patches been accepted?

No, I didn't have a user for the function anymore:
https://lkml.org/lkml/2018/12/17/284

> And I will be grateful if you can tell me your plan about the
> https://lkml.org/lkml/2018/10/24/619 patches.

The plan is to send a new version of them together with the
fwnode_get_name() patch. I can send them this week.


thanks,

[v1,07/12] usb: roles: Find the usb role switch by also matching against the device node

[Yu Chen]

Hi Heikki,

On 2018/12/3 20:25, Heikki Krogerus wrote:
> Hi,
> 
> On Mon, Dec 03, 2018 at 10:17:20AM +0200, Andy Shevchenko wrote:
>> On Mon, Dec 3, 2018 at 5:45 AM Yu Chen <chenyu56@huawei.com> wrote:
>>>
>>> This patch adds code for supporting find usb role switch by matching against
>>> the device node described using of_graph.
>>
>>>  #include <linux/module.h>
>>>  #include <linux/mutex.h>
>>>  #include <linux/slab.h>
>>> +#include <linux/of.h>
>>> +#include <linux/of_graph.h>
>>
>> Keep it ordered.
>>
>>> +static int __switch_match_by_of_node(struct device *dev, const void *name)
>>> +{
>>> +       if (!dev->parent || !dev->parent->of_node)
>>> +               return 0;
>>> +
>>> +       return of_node_name_eq(dev->parent->of_node, (const char *)name);
>>> +}
>>
>> I think Heikki has introduced some more generic function for this.
> 
> Yes, for this I'm proposing a new function fwnode_get_name() that
> should allow us to support ACPI as well as OF platforms:
> https://lkml.org/lkml/2018/11/28/784
> 
>>>         sw = device_connection_find_match(dev, "usb-role-switch", NULL,
>>>                                           usb_role_switch_match);
>>
>>> +       sw = of_graph_find_match_by_type(dev, "usb-role-switch");
>>
>> Why this is here?! device_connection_find_match() should take care for OF case.
> 
> Exactly. device_connection_find_match() needs to parse the graph (OF
> and ACPI), and I already made proposal (RFC) for that:
> https://lkml.org/lkml/2018/10/24/619
> 
> I have prepared a more finalized version of those patches. I've been
> waiting for that fwnode_get_name() to be accepted before sending it
> out since we are going to need that function, but I'll send a new RFC
> if nothing happens soon with that fwnode_get_name() series.
> 
> 
> Br,
> 

Have the fwnode_get_name() patches been accepted?
And I will be grateful if you can tell me your plan about the
https://lkml.org/lkml/2018/10/24/619 patches.

thanks
- Yu Chen

[v1,07/12] usb: roles: Find the usb role switch by also matching against the device node

[Andy Shevchenko]

On Mon, Dec 3, 2018 at 5:45 AM Yu Chen <chenyu56@huawei.com> wrote:
>
> This patch adds code for supporting find usb role switch by matching against
> the device node described using of_graph.

>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/of_graph.h>

Keep it ordered.

> +static int __switch_match_by_of_node(struct device *dev, const void *name)
> +{
> +       if (!dev->parent || !dev->parent->of_node)
> +               return 0;
> +
> +       return of_node_name_eq(dev->parent->of_node, (const char *)name);
> +}

I think Heikki has introduced some more generic function for this.

>         sw = device_connection_find_match(dev, "usb-role-switch", NULL,
>                                           usb_role_switch_match);

> +       sw = of_graph_find_match_by_type(dev, "usb-role-switch");

Why this is here?! device_connection_find_match() should take care for OF case.

[v1,07/12] usb: roles: Find the usb role switch by also matching against the device node

[Yu Chen]

This patch adds code for supporting find usb role switch by matching against
the device node described using of_graph.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: John Stultz <john.stultz@linaro.org>
Signed-off-by: Yu Chen <chenyu56@huawei.com>
---
 drivers/usb/common/roles.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/usb/common/roles.c b/drivers/usb/common/roles.c
index 99116af07f1d..0f48090c5c30 100644
--- a/drivers/usb/common/roles.c
+++ b/drivers/usb/common/roles.c
@@ -12,6 +12,8 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_graph.h>
 
 static struct class *role_class;
 
@@ -100,6 +102,38 @@ static void *usb_role_switch_match(struct device_connection *con, int ep,
 	return dev ? to_role_switch(dev) : ERR_PTR(-EPROBE_DEFER);
 }
 
+static int __switch_match_by_of_node(struct device *dev, const void *name)
+{
+	if (!dev->parent || !dev->parent->of_node)
+		return 0;
+
+	return of_node_name_eq(dev->parent->of_node, (const char *)name);
+}
+
+static void *of_graph_find_match_by_type(struct device *dev, const char *ep_type)
+{
+	struct device_node *ep;
+	struct device_node *remote_parent;
+	struct device *role_switch;
+
+	for_each_endpoint_of_node(dev_of_node(dev), ep) {
+		if (!ep->type || strcmp(ep->type, ep_type))
+			continue;
+
+		remote_parent = of_graph_get_remote_port_parent(ep);
+		if (!remote_parent || !remote_parent->name)
+			continue;
+
+		role_switch = class_find_device(role_class, NULL,
+				remote_parent->name, __switch_match_by_of_node);
+
+		return role_switch ? to_role_switch(role_switch) :
+			ERR_PTR(-EPROBE_DEFER);
+	}
+
+	return NULL;
+}
+
 /**
  * usb_role_switch_get - Find USB role switch linked with the caller
  * @dev: The caller device
@@ -114,6 +148,12 @@ struct usb_role_switch *usb_role_switch_get(struct device *dev)
 	sw = device_connection_find_match(dev, "usb-role-switch", NULL,
 					  usb_role_switch_match);
 
+	if (!IS_ERR_OR_NULL(sw)) {
+		WARN_ON(!try_module_get(sw->dev.parent->driver->owner));
+		return sw;
+	}
+
+	sw = of_graph_find_match_by_type(dev, "usb-role-switch");
 	if (!IS_ERR_OR_NULL(sw))
 		WARN_ON(!try_module_get(sw->dev.parent->driver->owner));