[v4,3/5] dt-bindings: phy: tegra: Add Tegra194 support
diff mbox series

Message ID 20191009024343.30218-4-jckuo@nvidia.com
State New
Headers show
Series
  • add Tegra194 XUSB host and pad controller support
Related show

Commit Message

JC Kuo Oct. 9, 2019, 2:43 a.m. UTC
Extend the bindings to cover the set of features found in Tegra194.
Note that, technically, there are four more supplies connected to the
XUSB pad controller (DVDD_PEX, DVDD_PEX_PLL, HVDD_PEX and HVDD_PEX_PLL)
, but the power sequencing requirements of Tegra194 require these to be
under the control of the PMIC.

Tegra194 XUSB PADCTL supports up to USB 3.1 Gen 2 speed, however, it is
possible for some platforms have long signal trace that could not
provide sufficient electrical environment for Gen 2 speed. To deal with
this, a new device node property "nvidia,disable-gen2" was added to
Tegra194 that be used to specifically disable Gen 2 speed for a
particular USB 3.0 port so that the port can be limited to Gen 1 speed
and avoid the instability.

Signed-off-by: JC Kuo <jckuo@nvidia.com>
---
Changes in v4: none
Changes in v3: none
Changes in v2:
- fix a typo

 .../bindings/phy/nvidia,tegra124-xusb-padctl.txt | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

Rob Herring Oct. 9, 2019, 11:39 p.m. UTC | #1
On Wed, Oct 09, 2019 at 10:43:41AM +0800, JC Kuo wrote:
> Extend the bindings to cover the set of features found in Tegra194.
> Note that, technically, there are four more supplies connected to the
> XUSB pad controller (DVDD_PEX, DVDD_PEX_PLL, HVDD_PEX and HVDD_PEX_PLL)
> , but the power sequencing requirements of Tegra194 require these to be
> under the control of the PMIC.
> 
> Tegra194 XUSB PADCTL supports up to USB 3.1 Gen 2 speed, however, it is
> possible for some platforms have long signal trace that could not
> provide sufficient electrical environment for Gen 2 speed. To deal with
> this, a new device node property "nvidia,disable-gen2" was added to
> Tegra194 that be used to specifically disable Gen 2 speed for a
> particular USB 3.0 port so that the port can be limited to Gen 1 speed
> and avoid the instability.

I suspect this may be a common issue and we should have a common 
property. Typically, this kind of property is in the controller though 
and supports multiple speed limits. See PCI bindings for inspiration.

Rob
Thierry Reding Oct. 14, 2019, 1:17 p.m. UTC | #2
On Wed, Oct 09, 2019 at 06:39:00PM -0500, Rob Herring wrote:
> On Wed, Oct 09, 2019 at 10:43:41AM +0800, JC Kuo wrote:
> > Extend the bindings to cover the set of features found in Tegra194.
> > Note that, technically, there are four more supplies connected to the
> > XUSB pad controller (DVDD_PEX, DVDD_PEX_PLL, HVDD_PEX and HVDD_PEX_PLL)
> > , but the power sequencing requirements of Tegra194 require these to be
> > under the control of the PMIC.
> > 
> > Tegra194 XUSB PADCTL supports up to USB 3.1 Gen 2 speed, however, it is
> > possible for some platforms have long signal trace that could not
> > provide sufficient electrical environment for Gen 2 speed. To deal with
> > this, a new device node property "nvidia,disable-gen2" was added to
> > Tegra194 that be used to specifically disable Gen 2 speed for a
> > particular USB 3.0 port so that the port can be limited to Gen 1 speed
> > and avoid the instability.
> 
> I suspect this may be a common issue and we should have a common 
> property. Typically, this kind of property is in the controller though 
> and supports multiple speed limits. See PCI bindings for inspiration.

Given that support for gen 2 speeds is dependent on signal trace length,
it doesn't really make sense to restrict the whole controller to a given
speed if only the signal trace for a single port exceeds the limit for
which gen 2 would work.

Also, the USB PHYs are in a different hardware block than the USB
controller, so this really is a property of the PHY block, not the USB
controller.

Thierry
Rob Herring Oct. 14, 2019, 1:40 p.m. UTC | #3
On Mon, Oct 14, 2019 at 8:17 AM Thierry Reding <thierry.reding@gmail.com> wrote:
>
> On Wed, Oct 09, 2019 at 06:39:00PM -0500, Rob Herring wrote:
> > On Wed, Oct 09, 2019 at 10:43:41AM +0800, JC Kuo wrote:
> > > Extend the bindings to cover the set of features found in Tegra194.
> > > Note that, technically, there are four more supplies connected to the
> > > XUSB pad controller (DVDD_PEX, DVDD_PEX_PLL, HVDD_PEX and HVDD_PEX_PLL)
> > > , but the power sequencing requirements of Tegra194 require these to be
> > > under the control of the PMIC.
> > >
> > > Tegra194 XUSB PADCTL supports up to USB 3.1 Gen 2 speed, however, it is
> > > possible for some platforms have long signal trace that could not
> > > provide sufficient electrical environment for Gen 2 speed. To deal with
> > > this, a new device node property "nvidia,disable-gen2" was added to
> > > Tegra194 that be used to specifically disable Gen 2 speed for a
> > > particular USB 3.0 port so that the port can be limited to Gen 1 speed
> > > and avoid the instability.
> >
> > I suspect this may be a common issue and we should have a common
> > property. Typically, this kind of property is in the controller though
> > and supports multiple speed limits. See PCI bindings for inspiration.
>
> Given that support for gen 2 speeds is dependent on signal trace length,
> it doesn't really make sense to restrict the whole controller to a given
> speed if only the signal trace for a single port exceeds the limit for
> which gen 2 would work.
>
> Also, the USB PHYs are in a different hardware block than the USB
> controller, so this really is a property of the PHY block, not the USB
> controller.

Okay, but still should be common for USB PHYs IMO.

Rob
JC Kuo Oct. 17, 2019, 7:48 a.m. UTC | #4
Hi Thierry, Hi Rob, Hi Kishon,
Please let me know your thoughts of the below implementation.

1. Add a "bool disable_gen2" to "phy->attrs" structure.
2. In _of_phy_get() of phy-core.c to add the follow to parse a generic property.

	phy->attrs.disable_gen2 = of_property_read_bool(args.np,
							"usb-disable-gen2");
3. In individual phy driver, to add SOC/PHY specific programming accordingly.

Thanks,
JC

On 10/14/19 9:40 PM, Rob Herring wrote:
> On Mon, Oct 14, 2019 at 8:17 AM Thierry Reding <thierry.reding@gmail.com> wrote:
>>
>> On Wed, Oct 09, 2019 at 06:39:00PM -0500, Rob Herring wrote:
>>> On Wed, Oct 09, 2019 at 10:43:41AM +0800, JC Kuo wrote:
>>>> Extend the bindings to cover the set of features found in Tegra194.
>>>> Note that, technically, there are four more supplies connected to the
>>>> XUSB pad controller (DVDD_PEX, DVDD_PEX_PLL, HVDD_PEX and HVDD_PEX_PLL)
>>>> , but the power sequencing requirements of Tegra194 require these to be
>>>> under the control of the PMIC.
>>>>
>>>> Tegra194 XUSB PADCTL supports up to USB 3.1 Gen 2 speed, however, it is
>>>> possible for some platforms have long signal trace that could not
>>>> provide sufficient electrical environment for Gen 2 speed. To deal with
>>>> this, a new device node property "nvidia,disable-gen2" was added to
>>>> Tegra194 that be used to specifically disable Gen 2 speed for a
>>>> particular USB 3.0 port so that the port can be limited to Gen 1 speed
>>>> and avoid the instability.
>>>
>>> I suspect this may be a common issue and we should have a common
>>> property. Typically, this kind of property is in the controller though
>>> and supports multiple speed limits. See PCI bindings for inspiration.
>>
>> Given that support for gen 2 speeds is dependent on signal trace length,
>> it doesn't really make sense to restrict the whole controller to a given
>> speed if only the signal trace for a single port exceeds the limit for
>> which gen 2 would work.
>>
>> Also, the USB PHYs are in a different hardware block than the USB
>> controller, so this really is a property of the PHY block, not the USB
>> controller.
> 
> Okay, but still should be common for USB PHYs IMO.
> 
> Rob
>
Thierry Reding Oct. 17, 2019, 11:44 a.m. UTC | #5
On Thu, Oct 17, 2019 at 03:48:52PM +0800, JC Kuo wrote:
> Hi Thierry, Hi Rob, Hi Kishon,
> Please let me know your thoughts of the below implementation.
> 
> 1. Add a "bool disable_gen2" to "phy->attrs" structure.

phy->attrs is pretty bus agnostic, so adding a USB-specific property
doesn't sound like the right thing to do here.

> 2. In _of_phy_get() of phy-core.c to add the follow to parse a generic property.
> 
> 	phy->attrs.disable_gen2 = of_property_read_bool(args.np,
> 							"usb-disable-gen2");
> 3. In individual phy driver, to add SOC/PHY specific programming accordingly.

Could this perhaps be done using the ->set_mode() callback? We don't
currently implement that, but we could implement it and then perhaps use
the submode parameter to distinguish between USB 3.1 Gen 1 and USB 3.1
Gen 2. Perhaps a good mapping would look like this:

	USB 3.1 Gen 1: mode = PHY_MODE_USB_HOST_SS, submode = 0x0300
	USB 3.1 Gen 2: mode = PHY_MODE_USB_HOST_SS, submode = 0x0301

The above basically reflects that USB 3.1 Gen 1 is really USB 3.0. This
would also work with other speeds:

	USB 2.0: mode = PHY_MODE_USB_HOST_HS, submode = 0x0200

etc. I suppose to make this clearer we could add defines for the various
submodes. It seems like submode may be intended to represent one of the
interface modes defined by USBPHY_INTERFACE_MODE_*, but perhaps it can
be repurposed for PHY_MODE_USB_HOST_SS?

Thierry

> 
> Thanks,
> JC
> 
> On 10/14/19 9:40 PM, Rob Herring wrote:
> > On Mon, Oct 14, 2019 at 8:17 AM Thierry Reding <thierry.reding@gmail.com> wrote:
> >>
> >> On Wed, Oct 09, 2019 at 06:39:00PM -0500, Rob Herring wrote:
> >>> On Wed, Oct 09, 2019 at 10:43:41AM +0800, JC Kuo wrote:
> >>>> Extend the bindings to cover the set of features found in Tegra194.
> >>>> Note that, technically, there are four more supplies connected to the
> >>>> XUSB pad controller (DVDD_PEX, DVDD_PEX_PLL, HVDD_PEX and HVDD_PEX_PLL)
> >>>> , but the power sequencing requirements of Tegra194 require these to be
> >>>> under the control of the PMIC.
> >>>>
> >>>> Tegra194 XUSB PADCTL supports up to USB 3.1 Gen 2 speed, however, it is
> >>>> possible for some platforms have long signal trace that could not
> >>>> provide sufficient electrical environment for Gen 2 speed. To deal with
> >>>> this, a new device node property "nvidia,disable-gen2" was added to
> >>>> Tegra194 that be used to specifically disable Gen 2 speed for a
> >>>> particular USB 3.0 port so that the port can be limited to Gen 1 speed
> >>>> and avoid the instability.
> >>>
> >>> I suspect this may be a common issue and we should have a common
> >>> property. Typically, this kind of property is in the controller though
> >>> and supports multiple speed limits. See PCI bindings for inspiration.
> >>
> >> Given that support for gen 2 speeds is dependent on signal trace length,
> >> it doesn't really make sense to restrict the whole controller to a given
> >> speed if only the signal trace for a single port exceeds the limit for
> >> which gen 2 would work.
> >>
> >> Also, the USB PHYs are in a different hardware block than the USB
> >> controller, so this really is a property of the PHY block, not the USB
> >> controller.
> > 
> > Okay, but still should be common for USB PHYs IMO.
> > 
> > Rob
> >
Thierry Reding Oct. 17, 2019, 12:01 p.m. UTC | #6
On Thu, Oct 17, 2019 at 03:48:52PM +0800, JC Kuo wrote:
> Hi Thierry, Hi Rob, Hi Kishon,
> Please let me know your thoughts of the below implementation.
> 
> 1. Add a "bool disable_gen2" to "phy->attrs" structure.
> 2. In _of_phy_get() of phy-core.c to add the follow to parse a generic property.
> 
> 	phy->attrs.disable_gen2 = of_property_read_bool(args.np,
> 							"usb-disable-gen2");

Regarding this, I'm not sure how Rob imagined the generic properties to
work. Perhaps he was thinking about something like the max-link-speed
property found in the PCI bindings.

We could have something like this:

  - max-link-speed:
      If present this property specifies the USB generation supported on
      the PHY/port. Must be:
        1: for USB 3.1 Gen 1 (a.k.a. USB 3.0)
        2: for USB 3.1 Gen 2

I'm not sure if we need to consider anything prior to USB 3.0. I suppose
we could do a similar mapping to what I proposed for the PHY ->set_mode
callback:

  - max-link-speed:
      If present this property specifies the USB generation supported on
      the PHY/port. Must be:
        0x0100: for USB 1.0 (Low-Speed)
        0x0101: for USB 1.1 (Full-Speed)
        0x0200: for USB 2.0 (Hi-Speed)
        0x0300: for USB 3.0 (SuperSpeed) (a.k.a. USB 3.1 Gen 1)
        0x0301: for USB 3.1 (SuperSpeed 10 Gbit/s) (a.k.a. USB 3.1 Gen 2)
        0x0302: for USB 3.2 (SuperSpeed 20 Gbit/s) (a.k.a. USB 3.2 Gen 2 x 2)
        ...

Or those could just be sequentially enumerated, like in the above
example.

Rob, any thoughts?

Thierry

> 3. In individual phy driver, to add SOC/PHY specific programming accordingly.
> 
> Thanks,
> JC
> 
> On 10/14/19 9:40 PM, Rob Herring wrote:
> > On Mon, Oct 14, 2019 at 8:17 AM Thierry Reding <thierry.reding@gmail.com> wrote:
> >>
> >> On Wed, Oct 09, 2019 at 06:39:00PM -0500, Rob Herring wrote:
> >>> On Wed, Oct 09, 2019 at 10:43:41AM +0800, JC Kuo wrote:
> >>>> Extend the bindings to cover the set of features found in Tegra194.
> >>>> Note that, technically, there are four more supplies connected to the
> >>>> XUSB pad controller (DVDD_PEX, DVDD_PEX_PLL, HVDD_PEX and HVDD_PEX_PLL)
> >>>> , but the power sequencing requirements of Tegra194 require these to be
> >>>> under the control of the PMIC.
> >>>>
> >>>> Tegra194 XUSB PADCTL supports up to USB 3.1 Gen 2 speed, however, it is
> >>>> possible for some platforms have long signal trace that could not
> >>>> provide sufficient electrical environment for Gen 2 speed. To deal with
> >>>> this, a new device node property "nvidia,disable-gen2" was added to
> >>>> Tegra194 that be used to specifically disable Gen 2 speed for a
> >>>> particular USB 3.0 port so that the port can be limited to Gen 1 speed
> >>>> and avoid the instability.
> >>>
> >>> I suspect this may be a common issue and we should have a common
> >>> property. Typically, this kind of property is in the controller though
> >>> and supports multiple speed limits. See PCI bindings for inspiration.
> >>
> >> Given that support for gen 2 speeds is dependent on signal trace length,
> >> it doesn't really make sense to restrict the whole controller to a given
> >> speed if only the signal trace for a single port exceeds the limit for
> >> which gen 2 would work.
> >>
> >> Also, the USB PHYs are in a different hardware block than the USB
> >> controller, so this really is a property of the PHY block, not the USB
> >> controller.
> > 
> > Okay, but still should be common for USB PHYs IMO.
> > 
> > Rob
> >

Patch
diff mbox series

diff --git a/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt b/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
index 9fb682e47c29..59d870fa42e9 100644
--- a/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
+++ b/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
@@ -37,6 +37,7 @@  Required properties:
   - Tegra132: "nvidia,tegra132-xusb-padctl", "nvidia,tegra124-xusb-padctl"
   - Tegra210: "nvidia,tegra210-xusb-padctl"
   - Tegra186: "nvidia,tegra186-xusb-padctl"
+  - Tegra194: "nvidia,tegra194-xusb-padctl"
 - reg: Physical base address and length of the controller's registers.
 - resets: Must contain an entry for each entry in reset-names.
 - reset-names: Must include the following entries:
@@ -62,6 +63,10 @@  For Tegra186:
 - vclamp-usb-supply: Bias rail for USB pad. Must supply 1.8 V.
 - vddio-hsic-supply: HSIC PHY power supply. Must supply 1.2 V.
 
+For Tegra194:
+- avdd-usb-supply: USB I/Os, VBUS, ID, REXT, D+/D- power supply. Must supply
+  3.3 V.
+- vclamp-usb-supply: Bias rail for USB pad. Must supply 1.8 V.
 
 Pad nodes:
 ==========
@@ -154,6 +159,11 @@  For Tegra210, the list of valid PHY nodes is given below:
 - sata: sata-0
   - functions: "usb3-ss", "sata"
 
+For Tegra194, the list of valid PHY nodes is given below:
+- usb2: usb2-0, usb2-1, usb2-2, usb2-3
+  - functions: "xusb"
+- usb3: usb3-0, usb3-1, usb3-2, usb3-3
+  - functions: "xusb"
 
 Port nodes:
 ===========
@@ -221,6 +231,9 @@  Optional properties:
   is internal. In the absence of this property the port is considered to be
   external.
 
+- nvidia,disable-gen2: A boolean property whose presence determines that a port
+  should be limited to USB 3.1 Gen 1. This property is only for Tegra194.
+
 For Tegra124 and Tegra132, the XUSB pad controller exposes the following
 ports:
 - 3x USB2: usb2-0, usb2-1, usb2-2
@@ -233,6 +246,9 @@  For Tegra210, the XUSB pad controller exposes the following ports:
 - 2x HSIC: hsic-0, hsic-1
 - 4x super-speed USB: usb3-0, usb3-1, usb3-2, usb3-3
 
+For Tegra194, the XUSB pad controller exposes the following ports:
+- 4x USB2: usb2-0, usb2-1, usb2-2, usb2-3
+- 4x super-speed USB: usb3-0, usb3-1, usb3-2, usb3-3
 
 Examples:
 =========