* [v2,2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property @ 2019-03-14 8:39 ` Biju Das 2019-03-14 9:16 ` Yoshihiro Shimoda 0 siblings, 1 reply; 11+ messages in thread From: Biju Das @ 2019-03-14 8:39 UTC (permalink / raw) To: Rob Herring, Mark Rutland Cc: Biju Das, Greg Kroah-Hartman, Heikki Krogerus, Felipe Balbi, linux-usb, devicetree, Simon Horman, Yoshihiro Shimoda, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro, linux-renesas-soc Add support for renesas_usb3 to support dual role switch using usb role switch class framework. Signed-off-by: Biju Das <biju.das@bp.renesas.com> --- V1-->V2 * Added usb-role-switch-property * Updated the example with usb-role-switch property. --- .../devicetree/bindings/usb/renesas_usb3.txt | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt b/Documentation/devicetree/bindings/usb/renesas_usb3.txt index 35039e7..eecaf4c 100644 --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt @@ -22,6 +22,7 @@ Required properties: Optional properties: - phys: phandle + phy specifier pair - phy-names: must be "usb" + - usb-role-switch: use USB role switch to support dual-role switch Example of R-Car H3 ES1.x: usb3_peri0: usb@ee020000 { @@ -39,3 +40,24 @@ Example of R-Car H3 ES1.x: interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>; clocks = <&cpg CPG_MOD 327>; }; + +Example of RZ/G2E: + usb3_peri0: usb@ee020000 { + compatible = "renesas,r8a774c0-usb3-peri", + "renesas,rcar-gen3-usb3-peri"; + reg = <0 0xee020000 0 0x400>; + interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cpg CPG_MOD 328>; + companion = <&xhci0>; + usb-role-switch; + + port { + #address-cells = <1>; + #size-cells = <0>; + + usb3peri_role_switch: endpoint@0 { + reg = <0>; + remote-endpoint = <&hd3ss3220_ep>; + }; + }; + }; ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [v2,2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property @ 2019-03-14 9:16 ` Yoshihiro Shimoda 2019-03-14 10:14 ` Biju Das 0 siblings, 1 reply; 11+ messages in thread From: Yoshihiro Shimoda @ 2019-03-14 9:16 UTC (permalink / raw) To: Biju Das, Rob Herring, Mark Rutland Cc: Greg Kroah-Hartman, Heikki Krogerus, Felipe Balbi, linux-usb@vger.kernel.org, devicetree@vger.kernel.org, Simon Horman, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro, linux-renesas-soc@vger.kernel.org Hi Biju-san, > From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM > > Add support for renesas_usb3 to support dual role switch > using usb role switch class framework. > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > --- > V1-->V2 > * Added usb-role-switch-property > * Updated the example with usb-role-switch property. > --- > .../devicetree/bindings/usb/renesas_usb3.txt | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > index 35039e7..eecaf4c 100644 > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > @@ -22,6 +22,7 @@ Required properties: > Optional properties: > - phys: phandle + phy specifier pair > - phy-names: must be "usb" > + - usb-role-switch: use USB role switch to support dual-role switch I don't think we can add such a property. At least, we have to add "renesas," prefix. Best regards, Yoshihiro Shimoda > Example of R-Car H3 ES1.x: > usb3_peri0: usb@ee020000 { > @@ -39,3 +40,24 @@ Example of R-Car H3 ES1.x: > interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>; > clocks = <&cpg CPG_MOD 327>; > }; > + > +Example of RZ/G2E: > + usb3_peri0: usb@ee020000 { > + compatible = "renesas,r8a774c0-usb3-peri", > + "renesas,rcar-gen3-usb3-peri"; > + reg = <0 0xee020000 0 0x400>; > + interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&cpg CPG_MOD 328>; > + companion = <&xhci0>; > + usb-role-switch; > + > + port { > + #address-cells = <1>; > + #size-cells = <0>; > + > + usb3peri_role_switch: endpoint@0 { > + reg = <0>; > + remote-endpoint = <&hd3ss3220_ep>; > + }; > + }; > + }; > -- > 2.7.4 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [v2,2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property @ 2019-03-14 10:14 ` Biju Das 2019-03-14 10:53 ` Heikki Krogerus 0 siblings, 1 reply; 11+ messages in thread From: Biju Das @ 2019-03-14 10:14 UTC (permalink / raw) To: Yoshihiro Shimoda, Rob Herring, Mark Rutland Cc: Greg Kroah-Hartman, Heikki Krogerus, Felipe Balbi, linux-usb@vger.kernel.org, devicetree@vger.kernel.org, Simon Horman, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro, linux-renesas-soc@vger.kernel.org Hi Shimoda-San, Thanks for the feedback. > Subject: RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role- > switch property > > Hi Biju-san, > > > From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM > > > > Add support for renesas_usb3 to support dual role switch using usb > > role switch class framework. > > > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > > --- > > V1-->V2 > > * Added usb-role-switch-property > > * Updated the example with usb-role-switch property. > > --- > > .../devicetree/bindings/usb/renesas_usb3.txt | 22 > ++++++++++++++++++++++ > > 1 file changed, 22 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > index 35039e7..eecaf4c 100644 > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > @@ -22,6 +22,7 @@ Required properties: > > Optional properties: > > - phys: phandle + phy specifier pair > > - phy-names: must be "usb" > > + - usb-role-switch: use USB role switch to support dual-role switch > > I don't think we can add such a property. At least, we have to add "renesas," > prefix. usb_role_switch_get api uses "usb-role-switch" property to get role switch linked with the device. HD3SS3220 port controller driver gets role switch handle linked with the device using usb_role_switch_get api. That is the reason, I have added " usb-role-switch" property here. Do you have any other suggestion to get usb role switch handle? Please correct me if I am wrong. > > Example of R-Car H3 ES1.x: > > usb3_peri0: usb@ee020000 { > > @@ -39,3 +40,24 @@ Example of R-Car H3 ES1.x: > > interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>; > > clocks = <&cpg CPG_MOD 327>; > > }; > > + > > +Example of RZ/G2E: > > + usb3_peri0: usb@ee020000 { > > + compatible = "renesas,r8a774c0-usb3-peri", > > + "renesas,rcar-gen3-usb3-peri"; > > + reg = <0 0xee020000 0 0x400>; > > + interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&cpg CPG_MOD 328>; > > + companion = <&xhci0>; > > + usb-role-switch; > > + > > + port { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + usb3peri_role_switch: endpoint@0 { > > + reg = <0>; > > + remote-endpoint = <&hd3ss3220_ep>; > > + }; > > + }; > > + }; > > -- > > 2.7.4 Regards, Biju ^ permalink raw reply [flat|nested] 11+ messages in thread
* [v2,2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property @ 2019-03-14 10:53 ` Heikki Krogerus 2019-03-15 9:08 ` Biju Das 0 siblings, 1 reply; 11+ messages in thread From: Heikki Krogerus @ 2019-03-14 10:53 UTC (permalink / raw) To: Biju Das Cc: Yoshihiro Shimoda, Rob Herring, Mark Rutland, Greg Kroah-Hartman, Felipe Balbi, linux-usb@vger.kernel.org, devicetree@vger.kernel.org, Simon Horman, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro, linux-renesas-soc@vger.kernel.org On Thu, Mar 14, 2019 at 10:14:12AM +0000, Biju Das wrote: > Hi Shimoda-San, > > Thanks for the feedback. > > > Subject: RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role- > > switch property > > > > Hi Biju-san, > > > > > From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM > > > > > > Add support for renesas_usb3 to support dual role switch using usb > > > role switch class framework. > > > > > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > > > --- > > > V1-->V2 > > > * Added usb-role-switch-property > > > * Updated the example with usb-role-switch property. > > > --- > > > .../devicetree/bindings/usb/renesas_usb3.txt | 22 > > ++++++++++++++++++++++ > > > 1 file changed, 22 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > index 35039e7..eecaf4c 100644 > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > @@ -22,6 +22,7 @@ Required properties: > > > Optional properties: > > > - phys: phandle + phy specifier pair > > > - phy-names: must be "usb" > > > + - usb-role-switch: use USB role switch to support dual-role switch > > > > I don't think we can add such a property. At least, we have to add "renesas," > > prefix. > > usb_role_switch_get api uses "usb-role-switch" property to get role switch linked with the device. > > HD3SS3220 port controller driver gets role switch handle linked with the device using usb_role_switch_get api. > That is the reason, I have added " usb-role-switch" property here. > > Do you have any other suggestion to get usb role switch handle? We can still change the API. Can we use the compatible for this? thanks, ^ permalink raw reply [flat|nested] 11+ messages in thread
* [v2,2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property @ 2019-03-15 9:08 ` Biju Das 2019-03-15 10:51 ` Heikki Krogerus 0 siblings, 1 reply; 11+ messages in thread From: Biju Das @ 2019-03-15 9:08 UTC (permalink / raw) To: Heikki Krogerus Cc: Yoshihiro Shimoda, Rob Herring, Mark Rutland, Greg Kroah-Hartman, Felipe Balbi, linux-usb@vger.kernel.org, devicetree@vger.kernel.org, Simon Horman, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro, linux-renesas-soc@vger.kernel.org Hi Heikki, Thanks for the feedback. > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role- > switch property > > On Thu, Mar 14, 2019 at 10:14:12AM +0000, Biju Das wrote: > > Hi Shimoda-San, > > > > Thanks for the feedback. > > > > > Subject: RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add > > > usb-role- switch property > > > > > > Hi Biju-san, > > > > > > > From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM > > > > > > > > Add support for renesas_usb3 to support dual role switch using usb > > > > role switch class framework. > > > > > > > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > > > > --- > > > > V1-->V2 > > > > * Added usb-role-switch-property > > > > * Updated the example with usb-role-switch property. > > > > --- > > > > .../devicetree/bindings/usb/renesas_usb3.txt | 22 > > > ++++++++++++++++++++++ > > > > 1 file changed, 22 insertions(+) > > > > > > > > diff --git > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > index 35039e7..eecaf4c 100644 > > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > @@ -22,6 +22,7 @@ Required properties: > > > > Optional properties: > > > > - phys: phandle + phy specifier pair > > > > - phy-names: must be "usb" > > > > + - usb-role-switch: use USB role switch to support dual-role > > > > + switch > > > > > > I don't think we can add such a property. At least, we have to add > "renesas," > > > prefix. > > > > usb_role_switch_get api uses "usb-role-switch" property to get role > switch linked with the device. > > > > HD3SS3220 port controller driver gets role switch handle linked with the > device using usb_role_switch_get api. > > That is the reason, I have added " usb-role-switch" property here. > > > > Do you have any other suggestion to get usb role switch handle? > > We can still change the API. Can we use the compatible for this? Do you mean usb_role_switch_get API needs to handle compatible "usb-x-connector" wherex= a,b ,c ? Then it uses the graphs api to get the device linked with it and return the usb role switch handle. In that case, no need to add generic "usb-role-switch" property here. Can you please confirm my understanding is correct? Regards, Biju ^ permalink raw reply [flat|nested] 11+ messages in thread
* [v2,2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property @ 2019-03-15 10:51 ` Heikki Krogerus 2019-03-28 15:33 ` Rob Herring 0 siblings, 1 reply; 11+ messages in thread From: Heikki Krogerus @ 2019-03-15 10:51 UTC (permalink / raw) To: Biju Das, Rob Herring Cc: Yoshihiro Shimoda, Mark Rutland, Greg Kroah-Hartman, Felipe Balbi, linux-usb@vger.kernel.org, devicetree@vger.kernel.org, Simon Horman, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro, linux-renesas-soc@vger.kernel.org Thanks, On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote: > > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role- > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > > index 35039e7..eecaf4c 100644 > > > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > > @@ -22,6 +22,7 @@ Required properties: > > > > > Optional properties: > > > > > - phys: phandle + phy specifier pair > > > > > - phy-names: must be "usb" > > > > > + - usb-role-switch: use USB role switch to support dual-role > > > > > + switch > > > > > > > > I don't think we can add such a property. At least, we have to add > > "renesas," > > > > prefix. > > > > > > usb_role_switch_get api uses "usb-role-switch" property to get role > > switch linked with the device. > > > > > > HD3SS3220 port controller driver gets role switch handle linked with the > > device using usb_role_switch_get api. > > > That is the reason, I have added " usb-role-switch" property here. > > > > > > Do you have any other suggestion to get usb role switch handle? > > > > We can still change the API. Can we use the compatible for this? > > Do you mean usb_role_switch_get API needs to handle compatible "usb-x-connector" wherex= a,b ,c ? > Then it uses the graphs api to get the device linked with it and return the usb role switch handle. > In that case, no need to add generic "usb-role-switch" property here. > > Can you please confirm my understanding is correct? No, I meant the compatible property would have the value "usb-role-switch". Your compatible would probable look something like this: compatible = "renesas,r8a774c0-usb3-peri", "usb-role-switch"; So the idea would be that the device supplying USB role switch functionality, in your case the USB controller, would need to have the compatible property containing "usb-role-switch". I'm really not an expert in DT, so I don't know if that is acceptable. Rob can you comment on this? thanks, ^ permalink raw reply [flat|nested] 11+ messages in thread
* [v2,2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property @ 2019-03-28 15:33 ` Rob Herring 2019-03-28 17:48 ` Heikki Krogerus 0 siblings, 1 reply; 11+ messages in thread From: Rob Herring @ 2019-03-28 15:33 UTC (permalink / raw) To: Heikki Krogerus Cc: Biju Das, Yoshihiro Shimoda, Mark Rutland, Greg Kroah-Hartman, Felipe Balbi, linux-usb@vger.kernel.org, devicetree@vger.kernel.org, Simon Horman, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro, linux-renesas-soc@vger.kernel.org On Fri, Mar 15, 2019 at 12:51:46PM +0200, Heikki Krogerus wrote: > Thanks, > > On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote: > > > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role- > > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > > > index 35039e7..eecaf4c 100644 > > > > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > > > @@ -22,6 +22,7 @@ Required properties: > > > > > > Optional properties: > > > > > > - phys: phandle + phy specifier pair > > > > > > - phy-names: must be "usb" > > > > > > + - usb-role-switch: use USB role switch to support dual-role > > > > > > + switch > > > > > > > > > > I don't think we can add such a property. At least, we have to add > > > "renesas," > > > > > prefix. > > > > > > > > usb_role_switch_get api uses "usb-role-switch" property to get role > > > switch linked with the device. > > > > > > > > HD3SS3220 port controller driver gets role switch handle linked with the > > > device using usb_role_switch_get api. > > > > That is the reason, I have added " usb-role-switch" property here. > > > > > > > > Do you have any other suggestion to get usb role switch handle? > > > > > > We can still change the API. Can we use the compatible for this? > > > > Do you mean usb_role_switch_get API needs to handle compatible "usb-x-connector" wherex= a,b ,c ? > > Then it uses the graphs api to get the device linked with it and return the usb role switch handle. > > In that case, no need to add generic "usb-role-switch" property here. > > > > Can you please confirm my understanding is correct? > > No, I meant the compatible property would have the value > "usb-role-switch". Your compatible would probable look something > like this: > > compatible = "renesas,r8a774c0-usb3-peri", > "usb-role-switch"; > > So the idea would be that the device supplying USB role switch > functionality, in your case the USB controller, would need to have the > compatible property containing "usb-role-switch". That's not really something a driver could bind to nor provides any info as to what the h/w is (and how to interact with it). > > I'm really not an expert in DT, so I don't know if that is acceptable. > Rob can you comment on this? I would go with Biju's proposal. Rob ^ permalink raw reply [flat|nested] 11+ messages in thread
* [v2,2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property @ 2019-03-28 17:48 ` Heikki Krogerus 2019-03-29 13:57 ` Rob Herring 0 siblings, 1 reply; 11+ messages in thread From: Heikki Krogerus @ 2019-03-28 17:48 UTC (permalink / raw) To: Rob Herring Cc: Biju Das, Yoshihiro Shimoda, Mark Rutland, Greg Kroah-Hartman, Felipe Balbi, linux-usb@vger.kernel.org, devicetree@vger.kernel.org, Simon Horman, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro, linux-renesas-soc@vger.kernel.org On Thu, Mar 28, 2019 at 10:33:53AM -0500, Rob Herring wrote: > On Fri, Mar 15, 2019 at 12:51:46PM +0200, Heikki Krogerus wrote: > > Thanks, > > > > On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote: > > > > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role- > > > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > > > > index 35039e7..eecaf4c 100644 > > > > > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > > > > @@ -22,6 +22,7 @@ Required properties: > > > > > > > Optional properties: > > > > > > > - phys: phandle + phy specifier pair > > > > > > > - phy-names: must be "usb" > > > > > > > + - usb-role-switch: use USB role switch to support dual-role > > > > > > > + switch > > > > > > > > > > > > I don't think we can add such a property. At least, we have to add > > > > "renesas," > > > > > > prefix. > > > > > > > > > > usb_role_switch_get api uses "usb-role-switch" property to get role > > > > switch linked with the device. > > > > > > > > > > HD3SS3220 port controller driver gets role switch handle linked with the > > > > device using usb_role_switch_get api. > > > > > That is the reason, I have added " usb-role-switch" property here. > > > > > > > > > > Do you have any other suggestion to get usb role switch handle? > > > > > > > > We can still change the API. Can we use the compatible for this? > > > > > > Do you mean usb_role_switch_get API needs to handle compatible "usb-x-connector" wherex= a,b ,c ? > > > Then it uses the graphs api to get the device linked with it and return the usb role switch handle. > > > In that case, no need to add generic "usb-role-switch" property here. > > > > > > Can you please confirm my understanding is correct? > > > > No, I meant the compatible property would have the value > > "usb-role-switch". Your compatible would probable look something > > like this: > > > > compatible = "renesas,r8a774c0-usb3-peri", > > "usb-role-switch"; > > > > So the idea would be that the device supplying USB role switch > > functionality, in your case the USB controller, would need to have the > > compatible property containing "usb-role-switch". > > That's not really something a driver could bind to nor provides any info > as to what the h/w is (and how to interact with it). Fair enough. As I said, I don't know much about DT. The problem we are trying to solve here is, how to identify the USB role switch from graph. It's primarily the USB Type-C connector drivers that need to be able to get a handle to the role switch device so they can tell it what to do. Basically the caller of usb_role_switch_get() will have the compatible "usb-c-connector", so it's of no use to us. We need the other endpoint, the role switch. Note: The USB role switch device will be a discrete (or integrated) mux on platforms that have separate USB host and USB device controllers, and on platforms with dual-role capable USB controller (like this one) the USB controller will represent it. At the moment the function usb_role_switch_get() walks trough the graph of the caller (so the connector) and expects that the remote-endpoint representing the mux will have a boolean property "usb-role-switch". I do agree that Documentation/devicetree/bindings/usb/renesas_usb3.txt should not be the file describing that boolean property, but if we had a separate file describing the bindings for just the USB role switch devices, would using it still be OK? If not, then can you propose something else? How do we identify the USB role switch endpoint from the graph? Would it be OK to expect the the endpoint subnode to have that boolean property instead? thanks, ^ permalink raw reply [flat|nested] 11+ messages in thread
* [v2,2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property @ 2019-03-29 13:57 ` Rob Herring 2019-04-11 9:04 ` Biju Das 0 siblings, 1 reply; 11+ messages in thread From: Rob Herring @ 2019-03-29 13:57 UTC (permalink / raw) To: Heikki Krogerus Cc: Biju Das, Yoshihiro Shimoda, Mark Rutland, Greg Kroah-Hartman, Felipe Balbi, linux-usb@vger.kernel.org, devicetree@vger.kernel.org, Simon Horman, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro, linux-renesas-soc@vger.kernel.org On Thu, Mar 28, 2019 at 12:48 PM Heikki Krogerus <heikki.krogerus@linux.intel.com> wrote: > > On Thu, Mar 28, 2019 at 10:33:53AM -0500, Rob Herring wrote: > > On Fri, Mar 15, 2019 at 12:51:46PM +0200, Heikki Krogerus wrote: > > > Thanks, > > > > > > On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote: > > > > > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role- > > > > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > > > > > index 35039e7..eecaf4c 100644 > > > > > > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > > > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > > > > > @@ -22,6 +22,7 @@ Required properties: > > > > > > > > Optional properties: > > > > > > > > - phys: phandle + phy specifier pair > > > > > > > > - phy-names: must be "usb" > > > > > > > > + - usb-role-switch: use USB role switch to support dual-role > > > > > > > > + switch > > > > > > > > > > > > > > I don't think we can add such a property. At least, we have to add > > > > > "renesas," > > > > > > > prefix. > > > > > > > > > > > > usb_role_switch_get api uses "usb-role-switch" property to get role > > > > > switch linked with the device. > > > > > > > > > > > > HD3SS3220 port controller driver gets role switch handle linked with the > > > > > device using usb_role_switch_get api. > > > > > > That is the reason, I have added " usb-role-switch" property here. > > > > > > > > > > > > Do you have any other suggestion to get usb role switch handle? > > > > > > > > > > We can still change the API. Can we use the compatible for this? > > > > > > > > Do you mean usb_role_switch_get API needs to handle compatible "usb-x-connector" wherex= a,b ,c ? > > > > Then it uses the graphs api to get the device linked with it and return the usb role switch handle. > > > > In that case, no need to add generic "usb-role-switch" property here. > > > > > > > > Can you please confirm my understanding is correct? > > > > > > No, I meant the compatible property would have the value > > > "usb-role-switch". Your compatible would probable look something > > > like this: > > > > > > compatible = "renesas,r8a774c0-usb3-peri", > > > "usb-role-switch"; > > > > > > So the idea would be that the device supplying USB role switch > > > functionality, in your case the USB controller, would need to have the > > > compatible property containing "usb-role-switch". > > > > That's not really something a driver could bind to nor provides any info > > as to what the h/w is (and how to interact with it). > > Fair enough. As I said, I don't know much about DT. > > The problem we are trying to solve here is, how to identify the USB > role switch from graph. It's primarily the USB Type-C connector > drivers that need to be able to get a handle to the role switch device > so they can tell it what to do. Basically the caller of > usb_role_switch_get() will have the compatible "usb-c-connector", so > it's of no use to us. We need the other endpoint, the role switch. > > Note: The USB role switch device will be a discrete (or integrated) > mux on platforms that have separate USB host and USB device > controllers, and on platforms with dual-role capable USB controller > (like this one) the USB controller will represent it. Either way, it should just be the device connected to the connector's USB data port in the graph. Though maybe if USB2 and USB3 are different controllers that would complicate things. > At the moment the function usb_role_switch_get() walks trough the > graph of the caller (so the connector) and expects that the > remote-endpoint representing the mux will have a boolean property > "usb-role-switch". Assuming we have that I think it should be in the device (controller/mux) node rather than the endpoint node itself. It could also be outside of DT in that the controller driver will know the h/w can support role switch and can register that capability. IOW, it can be implied by the compatible string. > > I do agree that Documentation/devicetree/bindings/usb/renesas_usb3.txt > should not be the file describing that boolean property, but if we had > a separate file describing the bindings for just the USB role switch > devices, would using it still be OK? It should be described somewhere common, yes, but the property itself belongs in controller nodes. Generally, we still document where common properties are used. Though, I prefer the implied by the compatible option. This should work unless there's a strong need to find the switch in DT without the switch driver being probed or if this is board specific (I wouldn't think so). > > If not, then can you propose something else? How do we identify the > USB role switch endpoint from the graph? Would it be OK to expect the > the endpoint subnode to have that boolean property instead? > > > thanks, > > -- > heikki ^ permalink raw reply [flat|nested] 11+ messages in thread
* [v2,2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property @ 2019-04-11 9:04 ` Biju Das 2019-04-11 9:04 ` [PATCH v2 2/7] " Biju Das 0 siblings, 1 reply; 11+ messages in thread From: Biju Das @ 2019-04-11 9:04 UTC (permalink / raw) To: Rob Herring, Heikki Krogerus, Chunfeng Yun, Yoshihiro Shimoda Cc: Mark Rutland, Greg Kroah-Hartman, Felipe Balbi, linux-usb@vger.kernel.org, devicetree@vger.kernel.org, Simon Horman, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro, linux-renesas-soc@vger.kernel.org Hi All, Thanks for the feedback. > -----Original Message----- > From: Rob Herring <robh@kernel.org> > Sent: 29 March 2019 13:58 > To: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Cc: Biju Das <biju.das@bp.renesas.com>; Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com>; Mark Rutland > <mark.rutland@arm.com>; Greg Kroah-Hartman > <gregkh@linuxfoundation.org>; Felipe Balbi <balbi@kernel.org>; linux- > usb@vger.kernel.org; devicetree@vger.kernel.org; Simon Horman > <horms@verge.net.au>; Geert Uytterhoeven <geert+renesas@glider.be>; > Chris Paterson <Chris.Paterson2@renesas.com>; Fabrizio Castro > <fabrizio.castro@bp.renesas.com>; linux-renesas-soc@vger.kernel.org > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role- > switch property > > On Thu, Mar 28, 2019 at 12:48 PM Heikki Krogerus > <heikki.krogerus@linux.intel.com> wrote: > > > > On Thu, Mar 28, 2019 at 10:33:53AM -0500, Rob Herring wrote: > > > On Fri, Mar 15, 2019 at 12:51:46PM +0200, Heikki Krogerus wrote: > > > > Thanks, > > > > > > > > On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote: > > > > > > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: > > > > > > add usb-role- > > > > > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > > > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > > > > > > index 35039e7..eecaf4c 100644 > > > > > > > > > --- > > > > > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > > > > > > +++ > b/Documentation/devicetree/bindings/usb/renesas_usb3 > > > > > > > > > +++ .txt > > > > > > > > > @@ -22,6 +22,7 @@ Required properties: > > > > > > > > > Optional properties: > > > > > > > > > - phys: phandle + phy specifier pair > > > > > > > > > - phy-names: must be "usb" > > > > > > > > > + - usb-role-switch: use USB role switch to support > > > > > > > > > + dual-role switch > > > > > > > > > > > > > > > > I don't think we can add such a property. At least, we > > > > > > > > have to add > > > > > > "renesas," > > > > > > > > prefix. > > > > > > > > > > > > > > usb_role_switch_get api uses "usb-role-switch" property to get > role > > > > > > switch linked with the device. > > > > > > > > > > > > > > HD3SS3220 port controller driver gets role switch handle > > > > > > > linked with the > > > > > > device using usb_role_switch_get api. > > > > > > > That is the reason, I have added " usb-role-switch" property here. > > > > > > > > > > > > > > Do you have any other suggestion to get usb role switch handle? > > > > > > > > > > > > We can still change the API. Can we use the compatible for this? > > > > > > > > > > Do you mean usb_role_switch_get API needs to handle compatible > "usb-x-connector" wherex= a,b ,c ? > > > > > Then it uses the graphs api to get the device linked with it and return > the usb role switch handle. > > > > > In that case, no need to add generic "usb-role-switch" property > here. > > > > > > > > > > Can you please confirm my understanding is correct? > > > > > > > > No, I meant the compatible property would have the value > > > > "usb-role-switch". Your compatible would probable look something > > > > like this: > > > > > > > > compatible = "renesas,r8a774c0-usb3-peri", > > > > "usb-role-switch"; > > > > > > > > So the idea would be that the device supplying USB role switch > > > > functionality, in your case the USB controller, would need to have > > > > the compatible property containing "usb-role-switch". > > > > > > That's not really something a driver could bind to nor provides any > > > info as to what the h/w is (and how to interact with it). > > > > Fair enough. As I said, I don't know much about DT. > > > > The problem we are trying to solve here is, how to identify the USB > > role switch from graph. It's primarily the USB Type-C connector > > drivers that need to be able to get a handle to the role switch device > > so they can tell it what to do. Basically the caller of > > usb_role_switch_get() will have the compatible "usb-c-connector", so > > it's of no use to us. We need the other endpoint, the role switch. > > > > Note: The USB role switch device will be a discrete (or integrated) > > mux on platforms that have separate USB host and USB device > > controllers, and on platforms with dual-role capable USB controller > > (like this one) the USB controller will represent it. Looks like we have a solution now. Heikki already proposed a new API "add API to get usb_role_switch by node" https://patchwork.kernel.org/patch/10882555/ We can use this API to find remote endpoint of USB role switch device(renesas usb3) connected with type-c DRP port controller driver(TI HD3SS3220). I believe in this case, we don't need generic Boolean property " usb-role-switch" Apart from this, we can add Renesas specific property (renesas, usb-role-switch) to differentiate the USB role switch associated with type C DRP controller driver from others. I will send V3 based on this solution. Please let me know if you think otherwise. Regards, Biju > Either way, it should just be the device connected to the connector's USB > data port in the graph. Though maybe if USB2 and USB3 are different > controllers that would complicate things. > > > At the moment the function usb_role_switch_get() walks trough the > > graph of the caller (so the connector) and expects that the > > remote-endpoint representing the mux will have a boolean property > > "usb-role-switch". > > Assuming we have that I think it should be in the device > (controller/mux) node rather than the endpoint node itself. > > It could also be outside of DT in that the controller driver will know the h/w > can support role switch and can register that capability. IOW, it can be implied > by the compatible string. > > > > > I do agree that Documentation/devicetree/bindings/usb/renesas_usb3.txt > > should not be the file describing that boolean property, but if we had > > a separate file describing the bindings for just the USB role switch > > devices, would using it still be OK? > > It should be described somewhere common, yes, but the property itself > belongs in controller nodes. Generally, we still document where common > properties are used. > > Though, I prefer the implied by the compatible option. This should work > unless there's a strong need to find the switch in DT without the switch driver > being probed or if this is board specific (I wouldn't think so). > > > > > If not, then can you propose something else? How do we identify the > > USB role switch endpoint from the graph? Would it be OK to expect the > > the endpoint subnode to have that boolean property instead? > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property 2019-04-11 9:04 ` Biju Das @ 2019-04-11 9:04 ` Biju Das 0 siblings, 0 replies; 11+ messages in thread From: Biju Das @ 2019-04-11 9:04 UTC (permalink / raw) To: Rob Herring, Heikki Krogerus, Chunfeng Yun, Yoshihiro Shimoda Cc: Mark Rutland, Greg Kroah-Hartman, Felipe Balbi, linux-usb@vger.kernel.org, devicetree@vger.kernel.org, Simon Horman, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro, linux-renesas-soc@vger.kernel.org Hi All, Thanks for the feedback. > -----Original Message----- > From: Rob Herring <robh@kernel.org> > Sent: 29 March 2019 13:58 > To: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Cc: Biju Das <biju.das@bp.renesas.com>; Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com>; Mark Rutland > <mark.rutland@arm.com>; Greg Kroah-Hartman > <gregkh@linuxfoundation.org>; Felipe Balbi <balbi@kernel.org>; linux- > usb@vger.kernel.org; devicetree@vger.kernel.org; Simon Horman > <horms@verge.net.au>; Geert Uytterhoeven <geert+renesas@glider.be>; > Chris Paterson <Chris.Paterson2@renesas.com>; Fabrizio Castro > <fabrizio.castro@bp.renesas.com>; linux-renesas-soc@vger.kernel.org > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role- > switch property > > On Thu, Mar 28, 2019 at 12:48 PM Heikki Krogerus > <heikki.krogerus@linux.intel.com> wrote: > > > > On Thu, Mar 28, 2019 at 10:33:53AM -0500, Rob Herring wrote: > > > On Fri, Mar 15, 2019 at 12:51:46PM +0200, Heikki Krogerus wrote: > > > > Thanks, > > > > > > > > On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote: > > > > > > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: > > > > > > add usb-role- > > > > > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > > > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > > > > > > index 35039e7..eecaf4c 100644 > > > > > > > > > --- > > > > > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > > > > > > +++ > b/Documentation/devicetree/bindings/usb/renesas_usb3 > > > > > > > > > +++ .txt > > > > > > > > > @@ -22,6 +22,7 @@ Required properties: > > > > > > > > > Optional properties: > > > > > > > > > - phys: phandle + phy specifier pair > > > > > > > > > - phy-names: must be "usb" > > > > > > > > > + - usb-role-switch: use USB role switch to support > > > > > > > > > + dual-role switch > > > > > > > > > > > > > > > > I don't think we can add such a property. At least, we > > > > > > > > have to add > > > > > > "renesas," > > > > > > > > prefix. > > > > > > > > > > > > > > usb_role_switch_get api uses "usb-role-switch" property to get > role > > > > > > switch linked with the device. > > > > > > > > > > > > > > HD3SS3220 port controller driver gets role switch handle > > > > > > > linked with the > > > > > > device using usb_role_switch_get api. > > > > > > > That is the reason, I have added " usb-role-switch" property here. > > > > > > > > > > > > > > Do you have any other suggestion to get usb role switch handle? > > > > > > > > > > > > We can still change the API. Can we use the compatible for this? > > > > > > > > > > Do you mean usb_role_switch_get API needs to handle compatible > "usb-x-connector" wherex= a,b ,c ? > > > > > Then it uses the graphs api to get the device linked with it and return > the usb role switch handle. > > > > > In that case, no need to add generic "usb-role-switch" property > here. > > > > > > > > > > Can you please confirm my understanding is correct? > > > > > > > > No, I meant the compatible property would have the value > > > > "usb-role-switch". Your compatible would probable look something > > > > like this: > > > > > > > > compatible = "renesas,r8a774c0-usb3-peri", > > > > "usb-role-switch"; > > > > > > > > So the idea would be that the device supplying USB role switch > > > > functionality, in your case the USB controller, would need to have > > > > the compatible property containing "usb-role-switch". > > > > > > That's not really something a driver could bind to nor provides any > > > info as to what the h/w is (and how to interact with it). > > > > Fair enough. As I said, I don't know much about DT. > > > > The problem we are trying to solve here is, how to identify the USB > > role switch from graph. It's primarily the USB Type-C connector > > drivers that need to be able to get a handle to the role switch device > > so they can tell it what to do. Basically the caller of > > usb_role_switch_get() will have the compatible "usb-c-connector", so > > it's of no use to us. We need the other endpoint, the role switch. > > > > Note: The USB role switch device will be a discrete (or integrated) > > mux on platforms that have separate USB host and USB device > > controllers, and on platforms with dual-role capable USB controller > > (like this one) the USB controller will represent it. Looks like we have a solution now. Heikki already proposed a new API "add API to get usb_role_switch by node" https://patchwork.kernel.org/patch/10882555/ We can use this API to find remote endpoint of USB role switch device(renesas usb3) connected with type-c DRP port controller driver(TI HD3SS3220). I believe in this case, we don't need generic Boolean property " usb-role-switch" Apart from this, we can add Renesas specific property (renesas, usb-role-switch) to differentiate the USB role switch associated with type C DRP controller driver from others. I will send V3 based on this solution. Please let me know if you think otherwise. Regards, Biju > Either way, it should just be the device connected to the connector's USB > data port in the graph. Though maybe if USB2 and USB3 are different > controllers that would complicate things. > > > At the moment the function usb_role_switch_get() walks trough the > > graph of the caller (so the connector) and expects that the > > remote-endpoint representing the mux will have a boolean property > > "usb-role-switch". > > Assuming we have that I think it should be in the device > (controller/mux) node rather than the endpoint node itself. > > It could also be outside of DT in that the controller driver will know the h/w > can support role switch and can register that capability. IOW, it can be implied > by the compatible string. > > > > > I do agree that Documentation/devicetree/bindings/usb/renesas_usb3.txt > > should not be the file describing that boolean property, but if we had > > a separate file describing the bindings for just the USB role switch > > devices, would using it still be OK? > > It should be described somewhere common, yes, but the property itself > belongs in controller nodes. Generally, we still document where common > properties are used. > > Though, I prefer the implied by the compatible option. This should work > unless there's a strong need to find the switch in DT without the switch driver > being probed or if this is board specific (I wouldn't think so). > > > > > If not, then can you propose something else? How do we identify the > > USB role switch endpoint from the graph? Would it be OK to expect the > > the endpoint subnode to have that boolean property instead? > > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-04-11 9:04 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1552552775-51667-1-git-send-email-biju.das@bp.renesas.com>
2019-03-14 8:39 ` [v2,2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property Biju Das
2019-03-14 9:16 ` Yoshihiro Shimoda
2019-03-14 10:14 ` Biju Das
2019-03-14 10:53 ` Heikki Krogerus
2019-03-15 9:08 ` Biju Das
2019-03-15 10:51 ` Heikki Krogerus
2019-03-28 15:33 ` Rob Herring
2019-03-28 17:48 ` Heikki Krogerus
2019-03-29 13:57 ` Rob Herring
2019-04-11 9:04 ` Biju Das
2019-04-11 9:04 ` [PATCH v2 2/7] " Biju Das
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).