[PATCH V9] dt: add NVIDIA Tegra XUSB controller binding

[PATCH V9] dt: add NVIDIA Tegra XUSB controller binding

[Stephen Warren]

From: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Add device-tree binding documentation for the XUSB (xHCI) controller
present on Tegra124 and later SoCs.

Signed-off-by: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
[swarren, combined separate MFD, mailbox, XHCI bindings into one node]
Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
Cc: Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Cc: Mathias Nyman <mathias.nyman-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
---
Changes from v8:
 - Combined the separate MFD, mailbox, and XHCI bindings into one; there
   is a single HW module, and so there should be a single DT node to
   represent it. Any Linux-internal driver structure should not influence
   the binding structure. This included squashing the various reg and
   interrupt resources that were previously in the separate modules into
   one list.
 - Used lists to document the compatible, reg, and interrupts properties.
 - Renamed the primary binding from xhci to xusb to reflect the name of
   the overall module.
Changes from v7:
 - Added back non-shared reg/interrupts properties.
Changes from v6:
 - Removed XUSB_DEV related clocks/resets.  They will be consumed by
   a separate driver and binding.
 - Removed reg/interrupts properties.
No changes from v5.
Changes from v4:
 - Updated regulator names, as suggested by Thierry.
No changes from v3.
Changes from v2:
 - Added mbox-names property.
Changes from v1:
 - Updated to use common mailbox bindings.
 - Added remaining XUSB-related clocks and resets.
 - Updated list of power supplies to be more accurate wrt to the hardware.
---
 .../bindings/usb/nvidia,tegra124-xusb.txt          | 96 ++++++++++++++++++++++
 1 file changed, 96 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt

diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt b/Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt
new file mode 100644
index 000000000000..f8de8d49602e
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt
@@ -0,0 +1,96 @@
+NVIDIA Tegra XUSB (XHCI) controller
+===================================
+
+The Tegra XUSB controller supports both USB2 and USB3 interfaces exposed
+by the Tegra XUSB pad controller.
+
+Required properties:
+--------------------
+ - compatible: Valid options are:
+    - Tegra124: "nvidia,tegra124-xusb".
+    - Tegra132: "nvidia,tegra132-xusb", "nvidia,tegra124-xusb".
+ - reg: Must contain the following entries, in the following order:
+    - The xHCI host registers.
+    - The IPFS registers.
+    - The FPCI registers.
+ - interrupts: Must contain the following entries, in the following order:
+    - The xHCI host interrupt
+    - The XUSB mailbox interrupt.
+ - clocks: Must contain an entry for each entry in clock-names.
+   See ../clock/clock-bindings.txt for details.
+ - clock-names: Must include the following entries:
+    - xusb_host
+    - xusb_host_src
+    - xusb_falcon_src
+    - xusb_ss
+    - xusb_ss_src
+    - xusb_ss_div2
+    - xusb_hs_src
+    - xusb_fs_src
+    - pll_u_480m
+    - clk_m
+    - pll_e
+ - resets: Must contain an entry for each entry in reset-names.
+   See ../reset/reset.txt for details.
+ - reset-names: Must include the following entries:
+   - xusb_host
+   - xusb_ss
+   - xusb_src
+   Note that xusb_src is the shared reset for xusb_{ss,hs,fs,falcon,host}_src.
+ - avddio-pex-supply: PCIe/USB3 analog logic power supply.  Must supply 1.05V.
+ - dvddio-pex-supply: PCIe/USB3 digital logic power supply.  Must supply 1.05V.
+ - avdd-usb-supply: USB controller power supply.  Must supply 3.3V.
+ - avdd-pll-utmip-supply: UTMI PLL power supply.  Must supply 1.8V.
+ - avdd-pll-erefe-supply: PLLE reference PLL power supply.  Must supply 1.05V.
+ - avdd-usb-ss-pll-supply: PCIe/USB3 PLL power supply.  Must supply 1.05V.
+ - hvdd-usb-ss-supply: High-voltage PCIe/USB3 power supply.  Must supply 3.3V.
+ - hvdd-usb-ss-pll-e-supply: High-voltage PLLE power supply.  Must supply 3.3V.
+
+Optional properties:
+--------------------
+ - phys: Must contain an entry for each entry in phy-names.
+   See ../phy/phy-bindings.txt for details.
+ - phy-names: Should include an entry for each PHY used by the controller.
+   Names must be of the form "<type>-<number>" where <type> is one of "utmi",
+   "hsic", or "usb3" and <number> is a 0-based index.  On Tegra124, there may
+   be up to 3 UTMI, 2 HSIC, and 2 USB3 PHYs.
+
+Example:
+--------
+	usb-host@0,70090000 {
+		compatible = "nvidia,tegra124-xusb";
+		reg = <0x0 0x70090000 0x0 0x8000>,
+		      <0x0 0x70099000 0x0 0x1000>,
+		      <0x0 0x70098000 0x0 0x1000>;
+		interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&tegra_car TEGRA124_CLK_XUSB_HOST>,
+			 <&tegra_car TEGRA124_CLK_XUSB_HOST_SRC>,
+			 <&tegra_car TEGRA124_CLK_XUSB_FALCON_SRC>,
+			 <&tegra_car TEGRA124_CLK_XUSB_SS>,
+			 <&tegra_car TEGRA124_CLK_XUSB_SS_DIV2>,
+			 <&tegra_car TEGRA124_CLK_XUSB_SS_SRC>,
+			 <&tegra_car TEGRA124_CLK_XUSB_HS_SRC>,
+			 <&tegra_car TEGRA124_CLK_XUSB_FS_SRC>,
+			 <&tegra_car TEGRA124_CLK_PLL_U_480M>,
+			 <&tegra_car TEGRA124_CLK_CLK_M>,
+			 <&tegra_car TEGRA124_CLK_PLL_E>;
+		clock-names = "xusb_host", "xusb_host_src", "xusb_falcon_src",
+			      "xusb_ss", "xusb_ss_div2", "xusb_ss_src",
+			      "xusb_hs_src", "xusb_fs_src", "pll_u_480m",
+			      "clk_m", "pll_e";
+		resets = <&tegra_car 89>, <&tegra_car 156>, <&tegra_car 143>;
+		reset-names = "xusb_host", "xusb_ss", "xusb_src";
+		phys = <&padctl TEGRA_XUSB_PADCTL_UTMI_P1>, /* mini-PCIe USB */
+		       <&padctl TEGRA_XUSB_PADCTL_UTMI_P2>, /* USB A */
+		       <&padctl TEGRA_XUSB_PADCTL_USB3_P0>; /* USB A */
+		phy-names = "utmi-1", "utmi-2", "usb3-0";
+		avddio-pex-supply = <&vdd_1v05_run>;
+		dvddio-pex-supply = <&vdd_1v05_run>;
+		avdd-usb-supply = <&vdd_3v3_lp0>;
+		avdd-pll-utmip-supply = <&vddio_1v8>;
+		avdd-pll-erefe-supply = <&avdd_1v05_run>;
+		avdd-usb-ss-pll-supply = <&vdd_1v05_run>;
+		hvdd-usb-ss-supply = <&vdd_3v3_lp0>;
+		hvdd-usb-ss-pll-e-supply = <&vdd_3v3_lp0>;
+	};
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V9] dt: add NVIDIA Tegra XUSB controller binding

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

Add device-tree binding documentation for the XUSB (xHCI) controller
present on Tegra124 and later SoCs.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
[swarren, combined separate MFD, mailbox, XHCI bindings into one node]
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: Mathias Nyman <mathias.nyman@intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
Changes from v8:
 - Combined the separate MFD, mailbox, and XHCI bindings into one; there
   is a single HW module, and so there should be a single DT node to
   represent it. Any Linux-internal driver structure should not influence
   the binding structure. This included squashing the various reg and
   interrupt resources that were previously in the separate modules into
   one list.
 - Used lists to document the compatible, reg, and interrupts properties.
 - Renamed the primary binding from xhci to xusb to reflect the name of
   the overall module.
Changes from v7:
 - Added back non-shared reg/interrupts properties.
Changes from v6:
 - Removed XUSB_DEV related clocks/resets.  They will be consumed by
   a separate driver and binding.
 - Removed reg/interrupts properties.
No changes from v5.
Changes from v4:
 - Updated regulator names, as suggested by Thierry.
No changes from v3.
Changes from v2:
 - Added mbox-names property.
Changes from v1:
 - Updated to use common mailbox bindings.
 - Added remaining XUSB-related clocks and resets.
 - Updated list of power supplies to be more accurate wrt to the hardware.
---
 .../bindings/usb/nvidia,tegra124-xusb.txt          | 96 ++++++++++++++++++++++
 1 file changed, 96 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt

diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt b/Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt
new file mode 100644
index 000000000000..f8de8d49602e
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt
@@ -0,0 +1,96 @@
+NVIDIA Tegra XUSB (XHCI) controller
+===================================
+
+The Tegra XUSB controller supports both USB2 and USB3 interfaces exposed
+by the Tegra XUSB pad controller.
+
+Required properties:
+--------------------
+ - compatible: Valid options are:
+    - Tegra124: "nvidia,tegra124-xusb".
+    - Tegra132: "nvidia,tegra132-xusb", "nvidia,tegra124-xusb".
+ - reg: Must contain the following entries, in the following order:
+    - The xHCI host registers.
+    - The IPFS registers.
+    - The FPCI registers.
+ - interrupts: Must contain the following entries, in the following order:
+    - The xHCI host interrupt
+    - The XUSB mailbox interrupt.
+ - clocks: Must contain an entry for each entry in clock-names.
+   See ../clock/clock-bindings.txt for details.
+ - clock-names: Must include the following entries:
+    - xusb_host
+    - xusb_host_src
+    - xusb_falcon_src
+    - xusb_ss
+    - xusb_ss_src
+    - xusb_ss_div2
+    - xusb_hs_src
+    - xusb_fs_src
+    - pll_u_480m
+    - clk_m
+    - pll_e
+ - resets: Must contain an entry for each entry in reset-names.
+   See ../reset/reset.txt for details.
+ - reset-names: Must include the following entries:
+   - xusb_host
+   - xusb_ss
+   - xusb_src
+   Note that xusb_src is the shared reset for xusb_{ss,hs,fs,falcon,host}_src.
+ - avddio-pex-supply: PCIe/USB3 analog logic power supply.  Must supply 1.05V.
+ - dvddio-pex-supply: PCIe/USB3 digital logic power supply.  Must supply 1.05V.
+ - avdd-usb-supply: USB controller power supply.  Must supply 3.3V.
+ - avdd-pll-utmip-supply: UTMI PLL power supply.  Must supply 1.8V.
+ - avdd-pll-erefe-supply: PLLE reference PLL power supply.  Must supply 1.05V.
+ - avdd-usb-ss-pll-supply: PCIe/USB3 PLL power supply.  Must supply 1.05V.
+ - hvdd-usb-ss-supply: High-voltage PCIe/USB3 power supply.  Must supply 3.3V.
+ - hvdd-usb-ss-pll-e-supply: High-voltage PLLE power supply.  Must supply 3.3V.
+
+Optional properties:
+--------------------
+ - phys: Must contain an entry for each entry in phy-names.
+   See ../phy/phy-bindings.txt for details.
+ - phy-names: Should include an entry for each PHY used by the controller.
+   Names must be of the form "<type>-<number>" where <type> is one of "utmi",
+   "hsic", or "usb3" and <number> is a 0-based index.  On Tegra124, there may
+   be up to 3 UTMI, 2 HSIC, and 2 USB3 PHYs.
+
+Example:
+--------
+	usb-host at 0,70090000 {
+		compatible = "nvidia,tegra124-xusb";
+		reg = <0x0 0x70090000 0x0 0x8000>,
+		      <0x0 0x70099000 0x0 0x1000>,
+		      <0x0 0x70098000 0x0 0x1000>;
+		interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&tegra_car TEGRA124_CLK_XUSB_HOST>,
+			 <&tegra_car TEGRA124_CLK_XUSB_HOST_SRC>,
+			 <&tegra_car TEGRA124_CLK_XUSB_FALCON_SRC>,
+			 <&tegra_car TEGRA124_CLK_XUSB_SS>,
+			 <&tegra_car TEGRA124_CLK_XUSB_SS_DIV2>,
+			 <&tegra_car TEGRA124_CLK_XUSB_SS_SRC>,
+			 <&tegra_car TEGRA124_CLK_XUSB_HS_SRC>,
+			 <&tegra_car TEGRA124_CLK_XUSB_FS_SRC>,
+			 <&tegra_car TEGRA124_CLK_PLL_U_480M>,
+			 <&tegra_car TEGRA124_CLK_CLK_M>,
+			 <&tegra_car TEGRA124_CLK_PLL_E>;
+		clock-names = "xusb_host", "xusb_host_src", "xusb_falcon_src",
+			      "xusb_ss", "xusb_ss_div2", "xusb_ss_src",
+			      "xusb_hs_src", "xusb_fs_src", "pll_u_480m",
+			      "clk_m", "pll_e";
+		resets = <&tegra_car 89>, <&tegra_car 156>, <&tegra_car 143>;
+		reset-names = "xusb_host", "xusb_ss", "xusb_src";
+		phys = <&padctl TEGRA_XUSB_PADCTL_UTMI_P1>, /* mini-PCIe USB */
+		       <&padctl TEGRA_XUSB_PADCTL_UTMI_P2>, /* USB A */
+		       <&padctl TEGRA_XUSB_PADCTL_USB3_P0>; /* USB A */
+		phy-names = "utmi-1", "utmi-2", "usb3-0";
+		avddio-pex-supply = <&vdd_1v05_run>;
+		dvddio-pex-supply = <&vdd_1v05_run>;
+		avdd-usb-supply = <&vdd_3v3_lp0>;
+		avdd-pll-utmip-supply = <&vddio_1v8>;
+		avdd-pll-erefe-supply = <&avdd_1v05_run>;
+		avdd-usb-ss-pll-supply = <&vdd_1v05_run>;
+		hvdd-usb-ss-supply = <&vdd_3v3_lp0>;
+		hvdd-usb-ss-pll-e-supply = <&vdd_3v3_lp0>;
+	};
-- 
1.9.1

Re: [PATCH V9] dt: add NVIDIA Tegra XUSB controller binding

[Stephen Warren]

(Dropping a lot of CCs since it looks like this needs some more work)

On 10/06/2015 01:01 PM, Stephen Warren wrote:
> From: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>
> Add device-tree binding documentation for the XUSB (xHCI) controller
> present on Tegra124 and later SoCs.
>
> Signed-off-by: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> [swarren, combined separate MFD, mailbox, XHCI bindings into one node]
> Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

> diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt b/Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt

> +Optional properties:
> +--------------------
> + - phys: Must contain an entry for each entry in phy-names.
> +   See ../phy/phy-bindings.txt for details.
> + - phy-names: Should include an entry for each PHY used by the controller.
> +   Names must be of the form "<type>-<number>" where <type> is one of "utmi",
> +   "hsic", or "usb3" and <number> is a 0-based index.  On Tegra124, there may
> +   be up to 3 UTMI, 2 HSIC, and 2 USB3 PHYs.

Now that I've looked at this binding in more detail, I think this 
phy-related text is wrong.

First off, I don't think the phys are optional at all. Whenever a phy is 
used, we must include it in the phys/phy-names list. I guess what we 
need is a more precise definition of which entries are required, and 
then to make these properties mandatory.

I think the naming of the phys above may be wrong too. The text above 
sounds like it's using the names of the actual phys, rather than of the 
ports that are using them. Since there's a big mux between the ports and 
the phys, I suspect it would make more sense to identify the ports in 
phy-names, and have the values in phys reflect the configuration of the 
mux. That way no matter how the mux is configured, the names used in 
phy-names always map 1:1 to the XUSB controller ports. Does that make 
sense? For some of the ports, this makes no difference since all the mux 
does is allow XUSB vs. some other module access to the PHY. But I think 
for at least the USB3 ports, each port could be mux'd to various 
different PHYs/lanes, so this distinction does have an effect.

Question: Should the XUSB controller driver determine which of its ports 
to enable/use based solely on which phy-names are present, or should we 
have some explicit property to define which ports are in use? Downstream 
does have a property for this purpose.

Question: Each physical USB connector contains signals from both a USB2 
port and a USB3 port. The HW can be configured to associate different 
USB2 and USB3 controllers with each-other for this purpose. The 
configuration for this happens via a register in the XUSB padctl module 
in register SS_PORT_MAP. However, the XUSB padctl binding doesn't allow 
specification of that mapping yet, and the driver doesn't program that 
register. I'm not sure if we should have a similar mapping property in 
the XUSB controller's DT node, a custom API between the two drivers to 
retrieve that information, or perhaps the XUSB controller driver doesn't 
actually care, since HW implements all required interactions. Does 
anyone know? Downstream also has a property that describes this mapping.

(Although note that the downstream properties for those two purposes, or 
at least the code that handles them, seems like a mess to me so far).

Either way, I think we need to update the XUSB padctl binding to contain 
this information before we consider the XUSB controller binding to be 
fully fleshed out, so we can make sure the two bindings interact well.

> +Example:

> +		phys = <&padctl TEGRA_XUSB_PADCTL_UTMI_P1>, /* mini-PCIe USB */
> +		       <&padctl TEGRA_XUSB_PADCTL_UTMI_P2>, /* USB A */
> +		       <&padctl TEGRA_XUSB_PADCTL_USB3_P0>; /* USB A */
> +		phy-names = "utmi-1", "utmi-2", "usb3-0";

We're also missing an update to the XUSB padctl binding to define all 
those TEGRA_XUSB_PADCTL_UTMI_P* values. I think we need to complete that 
in order to make sure that both bindings are fully thought out.

Thierry, I found your WIP patches perhaps related to this at:
> https://github.com/thierryreding/linux/commits/staging/xhci

However, those top two patches there don't contain any DT binding or 
.dts file updates so it's difficult to tell where the changes are going. 
Do you have binding/.dts patches elsewhere to show the intended result?

I notice that series may also have support for configuring the per-board 
tuning parameters too. That'd be useful to get in place too. Do you have 
a source for the actual tuning values for any of the T210 boards that 
mainline U-Boot now supports?

Re: [PATCH V9] dt: add NVIDIA Tegra XUSB controller binding

[Jon Hunter]

On 14/10/15 20:20, Stephen Warren wrote:
> (Dropping a lot of CCs since it looks like this needs some more work)
> 
> On 10/06/2015 01:01 PM, Stephen Warren wrote:
>> From: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>
>> Add device-tree binding documentation for the XUSB (xHCI) controller
>> present on Tegra124 and later SoCs.
>>
>> Signed-off-by: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> [swarren, combined separate MFD, mailbox, XHCI bindings into one node]
>> Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
>> diff --git
>> a/Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt
>> b/Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt
> 
>> +Optional properties:
>> +--------------------
>> + - phys: Must contain an entry for each entry in phy-names.
>> +   See ../phy/phy-bindings.txt for details.
>> + - phy-names: Should include an entry for each PHY used by the
>> controller.
>> +   Names must be of the form "<type>-<number>" where <type> is one of
>> "utmi",
>> +   "hsic", or "usb3" and <number> is a 0-based index.  On Tegra124,
>> there may
>> +   be up to 3 UTMI, 2 HSIC, and 2 USB3 PHYs.
> 
> Now that I've looked at this binding in more detail, I think this
> phy-related text is wrong.
> 
> First off, I don't think the phys are optional at all. Whenever a phy is
> used, we must include it in the phys/phy-names list. I guess what we
> need is a more precise definition of which entries are required, and
> then to make these properties mandatory.
> 
> I think the naming of the phys above may be wrong too. The text above
> sounds like it's using the names of the actual phys, rather than of the
> ports that are using them. Since there's a big mux between the ports and
> the phys, I suspect it would make more sense to identify the ports in
> phy-names, and have the values in phys reflect the configuration of the
> mux. That way no matter how the mux is configured, the names used in
> phy-names always map 1:1 to the XUSB controller ports. Does that make
> sense? For some of the ports, this makes no difference since all the mux
> does is allow XUSB vs. some other module access to the PHY. But I think
> for at least the USB3 ports, each port could be mux'd to various
> different PHYs/lanes, so this distinction does have an effect.

Yes but please note that the documentation is a bit misleading on the
number of actual options you have. For example, usb3-port0 is only
available on pcie lane 0. I have just posted a patch to correct this [0].

> Question: Should the XUSB controller driver determine which of its ports
> to enable/use based solely on which phy-names are present, or should we
> have some explicit property to define which ports are in use? Downstream
> does have a property for this purpose.

Given that the number of port-to-phy mappings are finite and there are
not too many, could be good to define them all and have the board dts
enabled the ones it is using. Right now it is not very clear exactly
what ports are available on which lanes.

> Question: Each physical USB connector contains signals from both a USB2
> port and a USB3 port. The HW can be configured to associate different
> USB2 and USB3 controllers with each-other for this purpose. The
> configuration for this happens via a register in the XUSB padctl module
> in register SS_PORT_MAP. However, the XUSB padctl binding doesn't allow
> specification of that mapping yet, and the driver doesn't program that
> register. I'm not sure if we should have a similar mapping property in
> the XUSB controller's DT node, a custom API between the two drivers to
> retrieve that information, or perhaps the XUSB controller driver doesn't
> actually care, since HW implements all required interactions. Does
> anyone know? Downstream also has a property that describes this mapping.

My understanding is the the XUSB controller has 3 usb2 ports and 2 usb3
ports. The SS_PORT_MAP allows you to map any of the 3 usb2 ports from
the XUSB module to any of the 2 usb3 ports. Yes this mapping is not
supported currently by the padctl binding but this was in Andrew's
series for XHCI [1] and the patch you are probably interested in is
here [2].

Jon

> (Although note that the downstream properties for those two purposes, or
> at least the code that handles them, seems like a mess to me so far).
> 
> Either way, I think we need to update the XUSB padctl binding to contain
> this information before we consider the XUSB controller binding to be
> fully fleshed out, so we can make sure the two bindings interact well.
> 
>> +Example:
> 
>> +        phys = <&padctl TEGRA_XUSB_PADCTL_UTMI_P1>, /* mini-PCIe USB */
>> +               <&padctl TEGRA_XUSB_PADCTL_UTMI_P2>, /* USB A */
>> +               <&padctl TEGRA_XUSB_PADCTL_USB3_P0>; /* USB A */
>> +        phy-names = "utmi-1", "utmi-2", "usb3-0";
> 
> We're also missing an update to the XUSB padctl binding to define all
> those TEGRA_XUSB_PADCTL_UTMI_P* values. I think we need to complete that
> in order to make sure that both bindings are fully thought out.
> 
> Thierry, I found your WIP patches perhaps related to this at:
>> https://github.com/thierryreding/linux/commits/staging/xhci
> 
> However, those top two patches there don't contain any DT binding or
> .dts file updates so it's difficult to tell where the changes are going.
> Do you have binding/.dts patches elsewhere to show the intended result?
> 
> I notice that series may also have support for configuring the per-board
> tuning parameters too. That'd be useful to get in place too. Do you have
> a source for the actual tuning values for any of the T210 boards that
> mainline U-Boot now supports?

[0] http://marc.info/?l=linux-tegra&m=144498745102948&w=2
[1] https://github.com/abrestic/linux/tree/tegra-xhci-v8-test
[2]
https://github.com/abrestic/linux/commit/82281999409b6c5a6a51df449a1c45204a249df8

Re: [PATCH V9] dt: add NVIDIA Tegra XUSB controller binding

[Stephen Warren]

On 10/16/2015 03:37 AM, Jon Hunter wrote:
>
> On 14/10/15 20:20, Stephen Warren wrote:
>> (Dropping a lot of CCs since it looks like this needs some more work)
>>
>> On 10/06/2015 01:01 PM, Stephen Warren wrote:
>>> From: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>>
>>> Add device-tree binding documentation for the XUSB (xHCI) controller
>>> present on Tegra124 and later SoCs.
>>>
>>> Signed-off-by: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>>> [swarren, combined separate MFD, mailbox, XHCI bindings into one node]
>>> Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt
>>> b/Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt
>>
>>> +Optional properties:
>>> +--------------------
>>> + - phys: Must contain an entry for each entry in phy-names.
>>> +   See ../phy/phy-bindings.txt for details.
>>> + - phy-names: Should include an entry for each PHY used by the
>>> controller.
>>> +   Names must be of the form "<type>-<number>" where <type> is one of
>>> "utmi",
>>> +   "hsic", or "usb3" and <number> is a 0-based index.  On Tegra124,
>>> there may
>>> +   be up to 3 UTMI, 2 HSIC, and 2 USB3 PHYs.
>>
>> Now that I've looked at this binding in more detail, I think this
>> phy-related text is wrong.
>>
>> First off, I don't think the phys are optional at all. Whenever a phy is
>> used, we must include it in the phys/phy-names list. I guess what we
>> need is a more precise definition of which entries are required, and
>> then to make these properties mandatory.
>>
>> I think the naming of the phys above may be wrong too. The text above
>> sounds like it's using the names of the actual phys, rather than of the
>> ports that are using them. Since there's a big mux between the ports and
>> the phys, I suspect it would make more sense to identify the ports in
>> phy-names, and have the values in phys reflect the configuration of the
>> mux. That way no matter how the mux is configured, the names used in
>> phy-names always map 1:1 to the XUSB controller ports. Does that make
>> sense? For some of the ports, this makes no difference since all the mux
>> does is allow XUSB vs. some other module access to the PHY. But I think
>> for at least the USB3 ports, each port could be mux'd to various
>> different PHYs/lanes, so this distinction does have an effect.
>
> Yes but please note that the documentation is a bit misleading on the
> number of actual options you have. For example, usb3-port0 is only
> available on pcie lane 0. I have just posted a patch to correct this [0].
>
>> Question: Should the XUSB controller driver determine which of its ports
>> to enable/use based solely on which phy-names are present, or should we
>> have some explicit property to define which ports are in use? Downstream
>> does have a property for this purpose.
>
> Given that the number of port-to-phy mappings are finite and there are
> not too many, could be good to define them all and have the board dts
> enabled the ones it is using. Right now it is not very clear exactly
> what ports are available on which lanes.

I think I'd prefer a binding with something more like a node per port or 
an enable bit/flag per port. This keeps things simple and maps directly 
to the HW. If we start enumerating all the valid combinations of which 
ports are active and mapped where, then we will need (a) to maintain 
that table in the binding, (b) a table in the driver to map from the 
list of options the binding allows to the set of HW ports/registers/... 
that are enabled. That sounds like extra complexity that's not necessary.

>> Question: Each physical USB connector contains signals from both a USB2
>> port and a USB3 port. The HW can be configured to associate different
>> USB2 and USB3 controllers with each-other for this purpose. The
>> configuration for this happens via a register in the XUSB padctl module
>> in register SS_PORT_MAP. However, the XUSB padctl binding doesn't allow
>> specification of that mapping yet, and the driver doesn't program that
>> register. I'm not sure if we should have a similar mapping property in
>> the XUSB controller's DT node, a custom API between the two drivers to
>> retrieve that information, or perhaps the XUSB controller driver doesn't
>> actually care, since HW implements all required interactions. Does
>> anyone know? Downstream also has a property that describes this mapping.
>
> My understanding is the the XUSB controller has 3 usb2 ports and 2 usb3
> ports. The SS_PORT_MAP allows you to map any of the 3 usb2 ports from
> the XUSB module to any of the 2 usb3 ports. Yes this mapping is not
> supported currently by the padctl binding but this was in Andrew's
> series for XHCI [1] and the patch you are probably interested in is
> here [2].

> [2]
> https://github.com/abrestic/linux/commit/82281999409b6c5a6a51df449a1c45204a249df8

Oh, I hadn't noticed that one since it wasn't part of the posted XHCI v8 
series, but in a different branch that built on it.

The binding proposed there (i.e. the usb2-port and usb3-port properties) 
don't map correctly to the way HW represents the data. I think we need 
to tie the binding to how the HW works much more directly. In HW there's 
a single "which USB2 port is associated with this USB3 port" register. I 
think we need DT to have a single property (or a property per USB3 port 
since there's a register field per USB3 port) that represents that, 
rather than properties that represent which USB2/3 controller ports will 
be using which lanes thus requiring the driver to derive the register 
data by comparing those values.

Also, I'm not sure the set of entries in 
include/dt-bindings/pinctrl/pinctrl-tegra-xusb.h represents HW 
correctly; after the patch, some of TEGRA_XUSB_PADCTL_* (e.g. 
TEGRA_XUSB_PADCTL_PCIE) represent actual physical PHYs (which often have 
multiple lanes, each potentially used by various different PCIe/SATA/USB 
controllers), whereas some of them (e.g. TEGRA_XUSB_PADCTL_USB3_P*) 
represent the USB3 controllers that make use of PHYs rather than the 
PHYs or lanes themselves. That feels inconsistent to me.

Thierry, you mentioned you had a proposal for XUSB padctl which 
represented things using sub-nodes. Can you please post a DT binding (or 
at least example DT content) for that ASAP so that all the proposed 
options are out on the table, and we can get to work on a final binding 
proposal? Thanks.