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

[Thierry Reding <thierry.reding@gmail.com>]

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