Re: [PATCH 2/2] dt-bindings: phy: tegra-xusb-padctl: Add Tegra210 support

[Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 3958 bytes --]

On Wed, Nov 04, 2015 at 01:59:51PM -0700, Stephen Warren wrote:
> On 11/04/2015 10:11 AM, Thierry Reding wrote:
> >From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> >
> >Extend the binding to cover the set of feature found in Tegra210.
> 
> >diff --git a/Documentation/devicetree/bindings/phy/nvidia,tegra-xusb-padctl.txt b/Documentation/devicetree/bindings/phy/nvidia,tegra-xusb-padctl.txt
> 
> >+PCIe pad:
> >+---------
> >+
> >+Required properties:
> >+- clocks: Must contain an entry for each entry in clock-names.
> >+- clock-names: Must contain the following entries:
> >+  - "pll": phandle and specifier referring to the PLLE
> >+- resets: Must contain an entry for each entry in reset-names.
> >+- reset-names: Must contain the following entries:
> >+  - "phy": reset for the PCIe UPHY block
> 
> I don't recall any clocks or resets properties in the pads for Tegra124. Do
> we really not need any?

Tegra124 had two instances of what used to be called IOPHY, one for PCIe
and one for SATA. On Tegra210 these have been replaced by two instances
of what's called UPHY. The resets listed in the PCIe and SATA pad nodes
are wired to those UPHY instances, hence why they are new on Tegra210.

For Tegra124 no resets exist for the IOPHY instances.

> >+SATA pad:
> >+---------
> >+
> >+Required properties:
> >+- resets: Must contain an entry for each entry in reset-names.
> >+- reset-names: Must contain the following entries:
> >+  - "phy": reset for the SATA UPHY block
> >+
> >
> >  PHY nodes:
> 
> Nit: 2 blank lines there.

Those were intentional for additional spacing between sections.

> >+For Tegra210, the list of valid PHY nodes is given below:
> >+- utmi: utmi-0, utmi-1, utmi-2, utmi-3
> >+  - functions: "snps", "xusb", "uart"
> >+- hsic: hsic-0, hsic-1
> >+  - functions: "snps", "xusb"
> >+- pcie: pcie-0, pcie-1, pcie-2, pcie-3, pcie-4, pcie-5, pcie-6
> >+  - functions: "pcie-x1", "usb3-ss", "pcie-x4"
> >+- sata: sata-0
> >+  - functions: "usb3-ss", "sata"
> 
> usb2-bias also needs to be present.

I'm not sure about this. All of the driver code that I've looked deals
with the usb2-bias pad internally. As far as I can tell, this pad needs
to be configured to whatever any of the other pads is configured for. I
think that means if any of the UTMI pads is configured for XUSB then the
usb2-bias pad must also be configured for XUSB. Which would also imply
that if one of the UTMI pads is configured for XUSB, all of them must be
configured for XUSB.

It's also not a pad in the sense that the others are pads. It doesn't
directly connect anywhere. It's also shared by all the UTMI pads. That
said, there are two registers that allow some of the parameters of the
pad to be set, so perhaps adding the node exclusively for
configurability might make sense.

It wouldn't really be a PHY node, though, so wouldn't fit into the above
group. Perhaps something like the following could be added:

  There is an additional pad that is used to support the bias voltages
  to the USB2/UTMI pads. This is not a PHY that can be consumed by any
  I/O controller, but the device tree node can be used to specify
  parameters needed for the programming of the pad.

I think the function shouldn't necessarily be exposed as a parameter,
because all that would do is add the possibility for a conflicting set
of mux options with the USB2/UTMI pads.

> >+
> >+
> >  Port nodes:
> >  ===========
> 
> Nit: 2 blank lines there.
> 
> >+For Tegra210, the XUSB pad controller exposes the following ports:
> >+- 4x UTMI: utmi-0, utmi-1, utmi-2, utmi-3
> >+- 2x HSIC: hsic-0, hsic-1
> >+- 4x super-speed USB: usb3-0, usb3-1, usb3-2, usb3-3
> >+
> >
> >  Examples:
> >  =========
> 
> Nit: 2 blank lines there.

Again, those were intentional for readability. I can remove them if you
don't think they fulfil that purpose.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]