On 03-05-2019 20:00, Thierry Reding wrote:
> On Thu, Apr 25, 2019 at 05:14:01PM +0200, Thierry Reding wrote:
>> On Mon, Mar 11, 2019 at 04:41:52PM +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
>>
>> Tegra210, please. "... supports the USB 3.0 ...". Also end sentences
>> with a fullstop.
>>
Will update
>>>
>>> Based on work by Andrew Bresticker <abrestic@xxxxxxxxxxxx>.
>>>
>>> Signed-off-by: Nagarjuna Kristam <nkristam@xxxxxxxxxx>
>>> ---
>>> .../devicetree/bindings/usb/nvidia,tegra-xudc.txt | 105 +++++++++++++++++++++
>>> 1 file changed, 105 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..990655d
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.txt
>>> @@ -0,0 +1,105 @@
>>> +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.
>>
>> s/ell/all/
>>
will do
>>> + See ../clock/clock-bindings.txt for details.
>>> +- clock-names: Must include the following entries:
>>> + - xusb_device
>>> + - xusb_ss
>>> + - xusb_ss_src
>>> + - xusb_hs_src
>>> + - xusb_fs_src
>>
>> It'd be good to explain what each of these are.
>
> Perhaps we should also drop the xusb_ prefix here. That's already
> implied by this being the XUDC controller bindings.
>
xusb_ prefix is added to be in sync with USB host driver. considering that this file
already for XUSB hardware, it can be removed.
Will add details and remove xusb_ prefix.
>>
>>> +- nvidia,xusb-padctl: phandle to the XUSB pad controller that is used to
>>> + configure the USB pads used by the XUDC 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'
>
> Same here, I'd drop the xusb_ prefix. I think the "device" power domain
> is also usually referred to as "XUSB_DEV", so perhaps make that "dev"
> instead?
>
> Thierry
>
will update accordingly. and apply event at clocks naming for "device" to "dev".
>>> +
>>> +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.
>>
>> My understanding is that this last supply is really needed for the XUSB
>> pad controller to bring up the PLL. In fact, I've just moved the same
>> supply to the XUSB pad controller from the XUSB controller for all of
>> the supported boards because having this in the XUSB controller would
>> fail under some circumstances.
>>
Will remove avdd-pll-utmip-supply from xudc driver, as its now part of padctl
>>> +
>>> +- phys: Must contain an entry for each entry in phy-names.
>>> + See ../phy/phy-bindings.txt for details.
>>> +- extcon-usb: Must contains an extcon-usb entry which detects
>>
>> In the example below, this is simply "extcon".
>>
yes, its extcon only, will update accordingly.
>>> + 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
>>
>> Why are these optional? phys is required and references phy-names
>> explicitly, so I think that effectively makes these phy-names required
>> as well.
>>
Reason being is SS mode is optional, but not usb 2.0 or earlier and hence added in
optional, but considering phys and phy-names goes together, will move this section to
required.
>>> +
>>> +Example:
>>> +--------
>>> + pmc: pmc@7000e400 {
>>> + compatible = "nvidia,tegra210-pmc";
>>> + reg = <0x0 0x7000e400 0x0 0x400>;
>>> + clocks = <&tegra_car TEGRA210_CLK_PCLK>, <&clk32k_in>;
>>> + clock-names = "pclk", "clk32k_in";
>>> +
>>> + powergates {
>>> + pd_xusbss: xusba {
>>> + clocks = <&tegra_car TEGRA210_CLK_XUSB_SS>;
>>> + resets = <&tegra_car TEGRA210_CLK_XUSB_SS>;
>>> + #power-domain-cells = <0>;
>>> + };
>>> +
>>> + pd_xusbdev: xusbb {
>>> + clocks = <&tegra_car TEGRA210_CLK_XUSB_DEV>;
>>> + resets = <&tegra_car 95>;
>>> + #power-domain-cells = <0>;
>>> + };
>>> + };
>>> + };
>>> +
>>> + xudc@700d0000 {
>>> + compatible = "nvidia,tegra210-xudc";
>>> + reg = <0x0 0x700d0000 0x0 0x8000>,
>>> + <0x0 0x700d8000 0x0 0x1000>,
>>> + <0x0 0x700d9000 0x0 0x1000>;
>>> +
>>> + interrupts = <0 44 0x4>;
>>
>> This should use symbolic names defined in the following includes:
>>
>> dt-bindings/interrupt-controller/irq.h
>> dt-bindings/interrupt-controller/arm-gic.h
>>
>> Thierry
>>
will update with corresponding macros.
-Nagarjuna
>>> +
>>> + 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>;
>>> + clock-names = "xusb_device", "xusb_ss", "xusb_ss_src",
>>> + "xusb_hs_src", "xusb_fs_src";
>>> +
>>> + power-domains = <&pd_xusbdev>, <&pd_xusbss>;
>>> + power-domain-names = "xusb_device", "xusb_ss";
>>> +
>>> + nvidia,xusb-padctl = <&padctl>;
>>> +
>>> + phys = <&{/padctl@7009f000/pads/usb2/lanes/usb2-0}>;
>>> + phy-names = "usb2;
>>> +
>>> + avddio-usb-supply = <&vdd_pex_1v05>;
>>> + hvdd-usb-supply = <&vdd_3v3_sys>;
>>> + avdd-pll-utmip-supply = <&vdd_1v8>;
>>> +
>>> + extcon = <&extcon_usb>;
>>> + };
>>> +
>>> + extcon_usb: extcon_vbus {
>>> + compatible = "linux,extcon-usb-gpio";
>>> + vbus-gpio = <&gpio TEGRA_GPIO(Z, 0) GPIO_ACTIVE_LOW>;
>>> + };
>>> +
>>> --
>>> 2.7.4
>>>
>
>
