diff mbox series

[V2,01/18] dt-bindings: phy: tegra-xusb: Add usb-role-switch

Message ID 1576660591-10383-2-git-send-email-nkristam@nvidia.com (mailing list archive)
State Superseded
Headers show
Series Tegra XUSB OTG support | expand

Commit Message

Nagarjuna Kristam Dec. 18, 2019, 9:16 a.m. UTC
Add usb-role-switch property for Tegra210 and Tegra186 platforms. This
entry is used by XUSB pad controller driver to register for role changes
for OTG/Peripheral capable USB 2 ports.

Signed-off-by: Nagarjuna Kristam <nkristam@nvidia.com>
---
V2:
 - Moved usb-role-switch to seperate Required section as suggested by Thierry.
 - Added reference to usb/usb-conn-gpio.txt for connector subnode.
---
 .../devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt         | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Thierry Reding Dec. 19, 2019, 1:05 p.m. UTC | #1
On Wed, Dec 18, 2019 at 02:46:14PM +0530, Nagarjuna Kristam wrote:
> Add usb-role-switch property for Tegra210 and Tegra186 platforms. This
> entry is used by XUSB pad controller driver to register for role changes
> for OTG/Peripheral capable USB 2 ports.
> 
> Signed-off-by: Nagarjuna Kristam <nkristam@nvidia.com>
> ---
> V2:
>  - Moved usb-role-switch to seperate Required section as suggested by Thierry.
>  - Added reference to usb/usb-conn-gpio.txt for connector subnode.
> ---
>  .../devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt         | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt b/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
> index 9fb682e..23bf354 100644
> --- a/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
> +++ b/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
> @@ -174,6 +174,12 @@ Required properties:
>    - "device": for USB device mode
>    - "otg": for USB OTG mode
>  
> +Required properties for OTG/Peripheral capable USB2 ports:
> +- usb-role-switch: Boolean property to indicate that the port support OTG or

"supports", and also, why if it supports OTG *or* peripheral? Doesn't
OTG imply peripheral? OTG means it can be either peripheral or host,
right? So I think the end of that sentence can be just:

	"... the port supports OTG."

Otherwise:

Acked-by: Thierry Reding <treding@nvidia.com>
JC Kuo Dec. 20, 2019, 8:08 a.m. UTC | #2
On 12/19/19 9:05 PM, Thierry Reding wrote:
> On Wed, Dec 18, 2019 at 02:46:14PM +0530, Nagarjuna Kristam wrote:
>> Add usb-role-switch property for Tegra210 and Tegra186 platforms. This
>> entry is used by XUSB pad controller driver to register for role changes
>> for OTG/Peripheral capable USB 2 ports.
>>
>> Signed-off-by: Nagarjuna Kristam <nkristam@nvidia.com>
>> ---
>> V2:
>>  - Moved usb-role-switch to seperate Required section as suggested by Thierry.
>>  - Added reference to usb/usb-conn-gpio.txt for connector subnode.
>> ---
>>  .../devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt         | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt b/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
>> index 9fb682e..23bf354 100644
>> --- a/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
>> +++ b/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
>> @@ -174,6 +174,12 @@ Required properties:
>>    - "device": for USB device mode
>>    - "otg": for USB OTG mode
>>  
>> +Required properties for OTG/Peripheral capable USB2 ports:
>> +- usb-role-switch: Boolean property to indicate that the port support OTG or
> 
> "supports", and also, why if it supports OTG *or* peripheral? Doesn't
> OTG imply peripheral? OTG means it can be either peripheral or host,
> right? So I think the end of that sentence can be just:
> 
> 	"... the port supports OTG."
An USB OTG port is capable of both USB host and peripheral operations. An USB
peripheral port can only act as an USB peripheral.

The micro USB ports found on Jetson TX1/TX2 platforms are micro-AB ports which
should implement both host and peripheral capabilities. We say such ports
support OTG. The micro USB port found on Jetson Nano is a micro-B port which
should implement peripheral capability only. We say such ports support
peripheral, rather than OTG.

Thanks,
JC

> 
> Otherwise:
> 
> Acked-by: Thierry Reding <treding@nvidia.com>
>
Thierry Reding Jan. 10, 2020, 11:16 a.m. UTC | #3
On Fri, Dec 20, 2019 at 04:08:30PM +0800, JC Kuo wrote:
> 
> On 12/19/19 9:05 PM, Thierry Reding wrote:
> > On Wed, Dec 18, 2019 at 02:46:14PM +0530, Nagarjuna Kristam wrote:
> >> Add usb-role-switch property for Tegra210 and Tegra186 platforms. This
> >> entry is used by XUSB pad controller driver to register for role changes
> >> for OTG/Peripheral capable USB 2 ports.
> >>
> >> Signed-off-by: Nagarjuna Kristam <nkristam@nvidia.com>
> >> ---
> >> V2:
> >>  - Moved usb-role-switch to seperate Required section as suggested by Thierry.
> >>  - Added reference to usb/usb-conn-gpio.txt for connector subnode.
> >> ---
> >>  .../devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt         | 6 ++++++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt b/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
> >> index 9fb682e..23bf354 100644
> >> --- a/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
> >> +++ b/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
> >> @@ -174,6 +174,12 @@ Required properties:
> >>    - "device": for USB device mode
> >>    - "otg": for USB OTG mode
> >>  
> >> +Required properties for OTG/Peripheral capable USB2 ports:
> >> +- usb-role-switch: Boolean property to indicate that the port support OTG or
> > 
> > "supports", and also, why if it supports OTG *or* peripheral? Doesn't
> > OTG imply peripheral? OTG means it can be either peripheral or host,
> > right? So I think the end of that sentence can be just:
> > 
> > 	"... the port supports OTG."
> An USB OTG port is capable of both USB host and peripheral operations. An USB
> peripheral port can only act as an USB peripheral.
> 
> The micro USB ports found on Jetson TX1/TX2 platforms are micro-AB ports which
> should implement both host and peripheral capabilities. We say such ports
> support OTG. The micro USB port found on Jetson Nano is a micro-B port which
> should implement peripheral capability only. We say such ports support
> peripheral, rather than OTG.

I the port supports only peripheral mode, why do we need to have a
usb-role-switch property? Shouldn't we in that case have a mode property
with value "device"?

usb-mode-switch is only needed if mode = "otg", isn't it? In all other
cases the functionality is fixed (either host or peripheral) and the
mode cannot be switched.

Thierry
Nagarjuna Kristam Jan. 13, 2020, 4:37 a.m. UTC | #4
On 10-01-2020 16:46, Thierry Reding wrote:
> On Fri, Dec 20, 2019 at 04:08:30PM +0800, JC Kuo wrote:
>> On 12/19/19 9:05 PM, Thierry Reding wrote:
>>> On Wed, Dec 18, 2019 at 02:46:14PM +0530, Nagarjuna Kristam wrote:
>>>> Add usb-role-switch property for Tegra210 and Tegra186 platforms. This
>>>> entry is used by XUSB pad controller driver to register for role changes
>>>> for OTG/Peripheral capable USB 2 ports.
>>>>
>>>> Signed-off-by: Nagarjuna Kristam<nkristam@nvidia.com>
>>>> ---
>>>> V2:
>>>>   - Moved usb-role-switch to seperate Required section as suggested by Thierry.
>>>>   - Added reference to usb/usb-conn-gpio.txt for connector subnode.
>>>> ---
>>>>   .../devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt         | 6 ++++++
>>>>   1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt b/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
>>>> index 9fb682e..23bf354 100644
>>>> --- a/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
>>>> +++ b/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
>>>> @@ -174,6 +174,12 @@ Required properties:
>>>>     - "device": for USB device mode
>>>>     - "otg": for USB OTG mode
>>>>   
>>>> +Required properties for OTG/Peripheral capable USB2 ports:
>>>> +- usb-role-switch: Boolean property to indicate that the port support OTG or
>>> "supports", and also, why if it supports OTG*or*  peripheral? Doesn't
>>> OTG imply peripheral? OTG means it can be either peripheral or host,
>>> right? So I think the end of that sentence can be just:
>>>
>>> 	"... the port supports OTG."
>> An USB OTG port is capable of both USB host and peripheral operations. An USB
>> peripheral port can only act as an USB peripheral.
>>
>> The micro USB ports found on Jetson TX1/TX2 platforms are micro-AB ports which
>> should implement both host and peripheral capabilities. We say such ports
>> support OTG. The micro USB port found on Jetson Nano is a micro-B port which
>> should implement peripheral capability only. We say such ports support
>> peripheral, rather than OTG.
> I the port supports only peripheral mode, why do we need to have a
> usb-role-switch property? Shouldn't we in that case have a mode property
> with value "device"?
> 
> usb-mode-switch is only needed if mode = "otg", isn't it? In all other
> cases the functionality is fixed (either host or peripheral) and the
> mode cannot be switched.
> 
> Thierry

usb-role-switch is needed when mode == "otg" or "peripheral".

Device mode is detected via vbus GPIO irrespective of mode being 
peripheral only or OTG.
Host mode is detected via id-gpio, in OTG case.

When mode is peripheral, role changes happens between USB_ROLE_DEVICE 
and USB_ROLE_NONE, which are generally based on Vbus GPIO(hot plug) 
detection.

When mode is otg, role changes happens between USB_ROLE_HOST, 
USB_ROLE_DEVICE and USB_ROLE_NONE, which are detected via id-gpio, 
vbus-gpio and no detection respectively.

-Nagarjuna
Thierry Reding Jan. 13, 2020, 3:06 p.m. UTC | #5
On Mon, Jan 13, 2020 at 10:07:08AM +0530, Nagarjuna Kristam wrote:
> 
> On 10-01-2020 16:46, Thierry Reding wrote:
> > On Fri, Dec 20, 2019 at 04:08:30PM +0800, JC Kuo wrote:
> > > On 12/19/19 9:05 PM, Thierry Reding wrote:
> > > > On Wed, Dec 18, 2019 at 02:46:14PM +0530, Nagarjuna Kristam wrote:
> > > > > Add usb-role-switch property for Tegra210 and Tegra186 platforms. This
> > > > > entry is used by XUSB pad controller driver to register for role changes
> > > > > for OTG/Peripheral capable USB 2 ports.
> > > > > 
> > > > > Signed-off-by: Nagarjuna Kristam<nkristam@nvidia.com>
> > > > > ---
> > > > > V2:
> > > > >   - Moved usb-role-switch to seperate Required section as suggested by Thierry.
> > > > >   - Added reference to usb/usb-conn-gpio.txt for connector subnode.
> > > > > ---
> > > > >   .../devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt         | 6 ++++++
> > > > >   1 file changed, 6 insertions(+)
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt b/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
> > > > > index 9fb682e..23bf354 100644
> > > > > --- a/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
> > > > > +++ b/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
> > > > > @@ -174,6 +174,12 @@ Required properties:
> > > > >     - "device": for USB device mode
> > > > >     - "otg": for USB OTG mode
> > > > > +Required properties for OTG/Peripheral capable USB2 ports:
> > > > > +- usb-role-switch: Boolean property to indicate that the port support OTG or
> > > > "supports", and also, why if it supports OTG*or*  peripheral? Doesn't
> > > > OTG imply peripheral? OTG means it can be either peripheral or host,
> > > > right? So I think the end of that sentence can be just:
> > > > 
> > > > 	"... the port supports OTG."
> > > An USB OTG port is capable of both USB host and peripheral operations. An USB
> > > peripheral port can only act as an USB peripheral.
> > > 
> > > The micro USB ports found on Jetson TX1/TX2 platforms are micro-AB ports which
> > > should implement both host and peripheral capabilities. We say such ports
> > > support OTG. The micro USB port found on Jetson Nano is a micro-B port which
> > > should implement peripheral capability only. We say such ports support
> > > peripheral, rather than OTG.
> > I the port supports only peripheral mode, why do we need to have a
> > usb-role-switch property? Shouldn't we in that case have a mode property
> > with value "device"?
> > 
> > usb-mode-switch is only needed if mode = "otg", isn't it? In all other
> > cases the functionality is fixed (either host or peripheral) and the
> > mode cannot be switched.
> > 
> > Thierry
> 
> usb-role-switch is needed when mode == "otg" or "peripheral".
> 
> Device mode is detected via vbus GPIO irrespective of mode being peripheral
> only or OTG.
> Host mode is detected via id-gpio, in OTG case.
> 
> When mode is peripheral, role changes happens between USB_ROLE_DEVICE and
> USB_ROLE_NONE, which are generally based on Vbus GPIO(hot plug) detection.
> 
> When mode is otg, role changes happens between USB_ROLE_HOST,
> USB_ROLE_DEVICE and USB_ROLE_NONE, which are detected via id-gpio, vbus-gpio
> and no detection respectively.

Right, that makes perfect sense. Thanks for putting it so clearly. With
the "support" -> "supports" typo fixed, this patch:

Acked-by: Thierry Reding <treding@nvidia.com>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt b/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
index 9fb682e..23bf354 100644
--- a/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
+++ b/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
@@ -174,6 +174,12 @@  Required properties:
   - "device": for USB device mode
   - "otg": for USB OTG mode
 
+Required properties for OTG/Peripheral capable USB2 ports:
+- usb-role-switch: Boolean property to indicate that the port support OTG or
+  peripheral mode. If present, the port supports switching between USB host
+  and peripheral roles. Connector should be added as subnode.
+  See usb/usb-conn-gpio.txt.
+
 Optional properties:
 - nvidia,internal: A boolean property whose presence determines that a port
   is internal. In the absence of this property the port is considered to be