diff mbox series

[6/8] dt-bindings: usb: Add NVIDIA Tegra XUSB device mode controller binding

Message ID 1546509899-5071-7-git-send-email-nkristam@nvidia.com (mailing list archive)
State New, archived
Headers show
Series Tegra XUSB gadget driver support | expand

Commit Message

Nagarjuna Kristam Jan. 3, 2019, 10:04 a.m. UTC 
Add device-tree binding documentation for the XUSB device mode controller
present on tegra210 SoC. This controller supports USB 3.0 specification

Based on work by Andrew Bresticker <abrestic@chromium.org>.

Signed-off-by: Nagarjuna Kristam <nkristam@nvidia.com>
---
 .../devicetree/bindings/usb/nvidia,tegra-xudc.txt  | 67 ++++++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.txt

Comments

Thierry Reding Feb. 4, 2019, 12:39 p.m. UTC | #1 
On Thu, Jan 03, 2019 at 03:34:57PM +0530, Nagarjuna Kristam wrote:
> Add device-tree binding documentation for the XUSB device mode controller
> present on tegra210 SoC. This controller supports USB 3.0 specification
> 
> Based on work by Andrew Bresticker <abrestic@chromium.org>.
> 
> Signed-off-by: Nagarjuna Kristam <nkristam@nvidia.com>
> ---
>  .../devicetree/bindings/usb/nvidia,tegra-xudc.txt  | 67 ++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.txt b/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.txt
> new file mode 100644
> index 0000000..244044b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.txt
> @@ -0,0 +1,67 @@
> +Device tree binding for NVIDIA Tegra XUSB device mode controller (XUDC)
> +============================

Make the underline as wide as the title.

> +
> +The Tegra XUDC controller supports both USB 2.0 HighSpeed/FullSpeed and
> +USB 3.0 SuperSpeed protocols.
> +
> +Required properties:
> +--------------------
> +- compatible: For Tegra210, must contain "nvidia,tegra210-xudc".
> +- reg: Must contain the base and length of the XUSB device registers, XUSB device
> +  PCI Config registers and XUSB device controller registers.
> +- interrupts: Must contain the XUSB device interrupt
> +- clocks: Must contain an entry for ell clocks used.
> +  See ../clock/clock-bindings.txt for details.
> +- nvidia,xusb-padctl: phandle to the XUSB pad controller that is used to
> +  configure the USB pads used by the XSUB controller

XSUB -> XUSB, though perhaps this should really be XUDC? Also, do we
really need the phandle to the XUSB pad controller? I think we have a
phandle to this for XUSB, but we need it for cases where we don't have
access to a PHY.

> +- power-domains: A list of PM domain specifiers that reference each power-domain
> +  used by the XUSB device mode controller. This list must comprise of a specifier
> +  for the XUSBA and XUSBB power-domains. See ../power/power_domain.txt and
> +  ../arm/tegra/nvidia,tegra20-pmc.txt for details.
> +- power-domain-names: A list of names that represent each of the specifiers in
> +  the 'power-domains' property. Must include 'xusb_ss' and 'xusb_device'
> +
> +For Tegra210:
> +- avddio_usb-supply: PCIe/USB3 analog logic power supply. Must supply 1.05 V.

avddio-usb-supply

> +- hvdd_usb-supply: USB controller power supply. Must supply 3.3 V.

hvdd-usb-supply

> +- avdd-pll-utmip-supply: UTMI PLL power supply. Must supply 1.8 V.
> +
> +- phys: Must contain an entry for each entry in phy-names.
> +  See ../phy/phy-bindings.txt for details.
> +- extcon-cables: Must contains an extcon-cable entry which detects
> +  USB VBUS pin. See ../extcon/extcon-usb-gpio.txt for details.

I think you use "extcon" in the DT change later in this series. You also
use "extcon" in the example later in this file.

> +
> +Optional properties:
> +--------------------
> +- phy-names: Should include an entry for each PHY used by the controller.
> +  Names must be "usb2", and "usb3" if support SuperSpeed device mode.
> +  - "usb3" phy, SuperSpeed (SSTX+/SSTX-/SSRX+/SSRX-) data lines
> +  - "usb2" phy, USB 2.0 (D+/D-) data lines
> +
> +Example:
> +--------
> +	extcon_usb: extcon_vbus {
> +		compatible = "linux,extcon-usb-gpio";
> +		vbus-gpio = <&gpio TEGRA_GPIO(Z, 0) 1>;

GPIO_ACTIVE_LOW for the third cell.

> +	};
> +	xudc@700d0000 {

Space between the above. Also typically we'd have the extcon at the very
end of DT, so it may make sense to reflect that in the example, like so:

	xudc@700d0000 {
		...
	};

	...

	extcon_usb: extcon_vbus {
		...
	};

> +		compatible = "nvidia,tegra210-xudc";
> +		phys = <&{/padctl@7009f000/pads/usb2/lanes/usb2-0}>;
> +		phy-names = "usb2;
> +		power-domains = <&pd_xusbdev>, <&pd_xusbss>;
> +		power-domain-names = "xusb_device", "xusb_ss";
> +        	avddio_usb-supply = <&vdd_pex_1v05>;

There's some indentation issue here. Also: avddio-usb-supply.

> +		hvdd_usb-supply = <&vdd_3v3_sys>;

hvdd-usb-supply

> +		avdd_pll_utmip-supply = <&vdd_1v8>;

avdd-pll-utmip-supply

> +		reg = <0x0 0x700d0000 0x0 0x8000>,
> +			<0x0 0x700d8000 0x0 0x1000>,
> +			<0x0 0x700d9000 0x0 0x1000>;
> +		interrupts = <0 44 0x4>;
> +		clocks = <&tegra_car TEGRA210_CLK_XUSB_DEV>,
> +			<&tegra_car TEGRA210_CLK_XUSB_SS>,
> +			<&tegra_car TEGRA210_CLK_XUSB_SSP_SRC>,
> +			<&tegra_car TEGRA210_CLK_XUSB_HS_SRC>,
> +			<&tegra_car TEGRA210_CLK_XUSB_FS_SRC>;

It's typical for common properties to go first and have more exotic ones
later. See other device tree nodes and try to stay consistent.

Also, does this not need any resets? I know that these are probably
dealt with as part of the power domains, but the DT should still
describe all signals that go into and come out of a block. The fact that
the driver doesn't use them is on OS-specific implementation detail.

Also, there's been some discussion surrounding shared resets lately and
we might end up with a situation where both the power domains and the
driver can again request an exclusive reset and make sure they only use
it exclusively at runtime.

Thierry

> +		nvidia,xusb-padctl = <&padctl>;
> +		extcon = <&extcon_usb>;
> +	};
> -- 
> 2.7.4
Nagarjuna Kristam Feb. 14, 2019, 2:56 p.m. UTC | #2 
>> +		reg = <0x0 0x700d0000 0x0 0x8000>,
>> +			<0x0 0x700d8000 0x0 0x1000>,
>> +			<0x0 0x700d9000 0x0 0x1000>;
>> +		interrupts = <0 44 0x4>;
>> +		clocks = <&tegra_car TEGRA210_CLK_XUSB_DEV>,
>> +			<&tegra_car TEGRA210_CLK_XUSB_SS>,
>> +			<&tegra_car TEGRA210_CLK_XUSB_SSP_SRC>,
>> +			<&tegra_car TEGRA210_CLK_XUSB_HS_SRC>,
>> +			<&tegra_car TEGRA210_CLK_XUSB_FS_SRC>;
> 
> It's typical for common properties to go first and have more exotic ones
> later. See other device tree nodes and try to stay consistent.
> 
> Also, does this not need any resets? I know that these are probably
> dealt with as part of the power domains, but the DT should still
> describe all signals that go into and come out of a block. The fact that
> the driver doesn't use them is on OS-specific implementation detail.
> 
> Also, there's been some discussion surrounding shared resets lately and
> we might end up with a situation where both the power domains and the
> driver can again request an exclusive reset and make sure they only use
> it exclusively at runtime.
> 
Since Resets are part of power domains, Is it fine to add reference of its
mention by providing power domain handles here similar to extcon ?

- Nagarjuna
> Thierry
> 
>> +		nvidia,xusb-padctl = <&padctl>;
>> +		extcon = <&extcon_usb>;
>> +	};
>> -- 
>> 2.7.4
Thierry Reding Feb. 25, 2019, 11:54 a.m. UTC | #3 
On Thu, Feb 14, 2019 at 08:26:52PM +0530, Nagarjuna Kristam wrote:
> >> +		reg = <0x0 0x700d0000 0x0 0x8000>,
> >> +			<0x0 0x700d8000 0x0 0x1000>,
> >> +			<0x0 0x700d9000 0x0 0x1000>;
> >> +		interrupts = <0 44 0x4>;
> >> +		clocks = <&tegra_car TEGRA210_CLK_XUSB_DEV>,
> >> +			<&tegra_car TEGRA210_CLK_XUSB_SS>,
> >> +			<&tegra_car TEGRA210_CLK_XUSB_SSP_SRC>,
> >> +			<&tegra_car TEGRA210_CLK_XUSB_HS_SRC>,
> >> +			<&tegra_car TEGRA210_CLK_XUSB_FS_SRC>;
> > 
> > It's typical for common properties to go first and have more exotic ones
> > later. See other device tree nodes and try to stay consistent.
> > 
> > Also, does this not need any resets? I know that these are probably
> > dealt with as part of the power domains, but the DT should still
> > describe all signals that go into and come out of a block. The fact that
> > the driver doesn't use them is on OS-specific implementation detail.
> > 
> > Also, there's been some discussion surrounding shared resets lately and
> > we might end up with a situation where both the power domains and the
> > driver can again request an exclusive reset and make sure they only use
> > it exclusively at runtime.
> > 
> Since Resets are part of power domains, Is it fine to add reference of its
> mention by providing power domain handles here similar to extcon ?

Yes, I think that's fine. We should describe the hardware and its
dependencies. That the reset is typically used by the power domains is
an implementation detail.

Also, there's work in progress to allow resets to be marked temporarily
exclusive, which will allow them to be used both from the power domains
but also from drivers if they have a need to assert/deassert explicitly:

	http://patchwork.ozlabs.org/project/linux-tegra/list/?series=93420

We need this for display support, but I'm not sure if it's relevant for
XUSB. Do you know of any cases in XUSB where this might be useful? In
either case, I think it makes sense to list the resets in this node just
in case. If we don't need them in the kernel, that's fine, but we should
have them in DT in case we ever need them, or if another OS decides to
use them differently.

Thierry
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.txt b/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.txt
new file mode 100644
index 0000000..244044b
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.txt
@@ -0,0 +1,67 @@ 
+Device tree binding for NVIDIA Tegra XUSB device mode controller (XUDC)
+============================
+
+The Tegra XUDC controller supports both USB 2.0 HighSpeed/FullSpeed and
+USB 3.0 SuperSpeed protocols.
+
+Required properties:
+--------------------
+- compatible: For Tegra210, must contain "nvidia,tegra210-xudc".
+- reg: Must contain the base and length of the XUSB device registers, XUSB device
+  PCI Config registers and XUSB device controller registers.
+- interrupts: Must contain the XUSB device interrupt
+- clocks: Must contain an entry for ell clocks used.
+  See ../clock/clock-bindings.txt for details.
+- nvidia,xusb-padctl: phandle to the XUSB pad controller that is used to
+  configure the USB pads used by the XSUB controller
+- power-domains: A list of PM domain specifiers that reference each power-domain
+  used by the XUSB device mode controller. This list must comprise of a specifier
+  for the XUSBA and XUSBB power-domains. See ../power/power_domain.txt and
+  ../arm/tegra/nvidia,tegra20-pmc.txt for details.
+- power-domain-names: A list of names that represent each of the specifiers in
+  the 'power-domains' property. Must include 'xusb_ss' and 'xusb_device'
+
+For Tegra210:
+- avddio_usb-supply: PCIe/USB3 analog logic power supply. Must supply 1.05 V.
+- hvdd_usb-supply: USB controller power supply. Must supply 3.3 V.
+- avdd-pll-utmip-supply: UTMI PLL power supply. Must supply 1.8 V.
+
+- phys: Must contain an entry for each entry in phy-names.
+  See ../phy/phy-bindings.txt for details.
+- extcon-cables: Must contains an extcon-cable entry which detects
+  USB VBUS pin. See ../extcon/extcon-usb-gpio.txt for details.
+
+Optional properties:
+--------------------
+- phy-names: Should include an entry for each PHY used by the controller.
+  Names must be "usb2", and "usb3" if support SuperSpeed device mode.
+  - "usb3" phy, SuperSpeed (SSTX+/SSTX-/SSRX+/SSRX-) data lines
+  - "usb2" phy, USB 2.0 (D+/D-) data lines
+
+Example:
+--------
+	extcon_usb: extcon_vbus {
+		compatible = "linux,extcon-usb-gpio";
+		vbus-gpio = <&gpio TEGRA_GPIO(Z, 0) 1>;
+	};
+	xudc@700d0000 {
+		compatible = "nvidia,tegra210-xudc";
+		phys = <&{/padctl@7009f000/pads/usb2/lanes/usb2-0}>;
+		phy-names = "usb2;
+		power-domains = <&pd_xusbdev>, <&pd_xusbss>;
+		power-domain-names = "xusb_device", "xusb_ss";
+        	avddio_usb-supply = <&vdd_pex_1v05>;
+		hvdd_usb-supply = <&vdd_3v3_sys>;
+		avdd_pll_utmip-supply = <&vdd_1v8>;
+		reg = <0x0 0x700d0000 0x0 0x8000>,
+			<0x0 0x700d8000 0x0 0x1000>,
+			<0x0 0x700d9000 0x0 0x1000>;
+		interrupts = <0 44 0x4>;
+		clocks = <&tegra_car TEGRA210_CLK_XUSB_DEV>,
+			<&tegra_car TEGRA210_CLK_XUSB_SS>,
+			<&tegra_car TEGRA210_CLK_XUSB_SSP_SRC>,
+			<&tegra_car TEGRA210_CLK_XUSB_HS_SRC>,
+			<&tegra_car TEGRA210_CLK_XUSB_FS_SRC>;
+		nvidia,xusb-padctl = <&padctl>;
+		extcon = <&extcon_usb>;
+	};