Re: [PATCH v10 1/9] dt-bindings: phy: Add NVIDIA Tegra XUSB pad controller binding

Andrew Bresticker
2016-03-04T21:36:11Z

Hi Thierry,

On Fri, Mar 4, 2016 at 8:19 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> The NVIDIA Tegra XUSB pad controller provides a set of pads, each with a
> set of lanes that are used for PCIe, SATA and USB.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>

Thanks, this binding looks much better, IMO. A couple small comments below...

> +Port nodes:
> +===========
> +
> +A required child node named "ports" contains a list of all the ports exposed
> +by the XUSB pad controller. Per-port configuration is only required for USB.
> +
> +USB2 ports:
> +-----------
> +
> +Required properties:
> +- status: Defines the operation status of the port. Valid values are:
> + - "disabled": the port is disabled
> + - "okay": the port is enabled
> +- mode: A string that determines the mode in which to run the port. Valid
> + values are:
> + - "host": for USB host mode
> + - "device": for USB device mode
> + - "otg": for USB OTG mode
> +
> +Optional properties:
> +- nvidia,internal: A boolean property whose presence determines that a port
> + is internal. In the absence of this property the port is considered to be
> + external.
> +- vbus-supply: phandle to a regulator supplying the VBUS voltage.

Both Blaze and Smaug require an offset to be applied to the fused
HS_CURR_LEVEL value, so I think we need another property here for
that.

> +ULPI ports:
> +-----------
> +
> +Optional properties:
> +- status: Defines the operation status of the port. Valid values are:
> + - "disabled": the port is disabled
> + - "okay": the port is enabled
> +- nvidia,internal: A boolean property whose presence determines that a port
> + is internal. In the absence of this property the port is considered to be
> + external.
> +- vbus-supply: phandle to a regulator supplying the VBUS voltage.
> +
> +HSIC ports:
> +-----------
> +
> +Required properties:
> +- status: Defines the operation status of the port. Valid values are:
> + - "disabled": the port is disabled
> + - "okay": the port is enabled
> +
> +Optional properties:
> +- vbus-supply: phandle to a regulator supplying the VBUS voltage.

I believe this pin is named VDDIO_HSIC?

Also there are several other HSIC pad parameters (STROBE_TRIM,
DATA_TRIM, etc.) which probably should be supplied via DT.