linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thierry Reding <thierry.reding@gmail.com>
To: Nagarjuna Kristam <nkristam@nvidia.com>
Cc: balbi@kernel.org, gregkh@linuxfoundation.org,
	jonathanh@nvidia.com, linux-tegra@vger.kernel.org,
	linux-usb@vger.kernel.org
Subject: [V2,4/8] dt-bindings: usb: Add NVIDIA Tegra XUSB device mode controller binding
Date: Fri, 3 May 2019 16:30:07 +0200	[thread overview]
Message-ID: <20190503143007.GA20469@ulmo> (raw)

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.
> 
> > 
> > Based on work by Andrew Bresticker <abrestic@chromium.org>.
> > 
> > Signed-off-by: Nagarjuna Kristam <nkristam@nvidia.com>
> > ---
> >  .../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/
> 
> > +  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.

> 
> > +- 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

> > +
> > +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.
> 
> > +
> > +- 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".
> 
> > +  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.
> 
> > +
> > +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
> 
> > +
> > +		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
> >

WARNING: multiple messages have this Message-ID (diff)
From: Thierry Reding <thierry.reding@gmail.com>
To: Nagarjuna Kristam <nkristam@nvidia.com>
Cc: balbi@kernel.org, gregkh@linuxfoundation.org,
	jonathanh@nvidia.com, linux-tegra@vger.kernel.org,
	linux-usb@vger.kernel.org
Subject: Re: [PATCH V2 4/8] dt-bindings: usb: Add  NVIDIA Tegra XUSB device mode controller binding
Date: Fri, 3 May 2019 16:30:07 +0200	[thread overview]
Message-ID: <20190503143007.GA20469@ulmo> (raw)
Message-ID: <20190503143007.b9KsfxqdBQy7B4Yy8QVC1bDEkLjFFhqCJZ3XZqKlHKg@z> (raw)
In-Reply-To: <20190425151401.GF24213@ulmo>

[-- Attachment #1: Type: text/plain, Size: 6442 bytes --]

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.
> 
> > 
> > Based on work by Andrew Bresticker <abrestic@chromium.org>.
> > 
> > Signed-off-by: Nagarjuna Kristam <nkristam@nvidia.com>
> > ---
> >  .../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/
> 
> > +  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.

> 
> > +- 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

> > +
> > +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.
> 
> > +
> > +- 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".
> 
> > +  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.
> 
> > +
> > +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
> 
> > +
> > +		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
> > 



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

         reply	other threads:[~2019-05-03 14:30 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1552302716-18554-1-git-send-email-nkristam@nvidia.com>
2019-03-11 11:11 ` [V2,1/8] phy: tegra: xusb: t210: add XUSB dual mode support Nagarjuna Kristam
2019-04-25 14:13   ` Thierry Reding
2019-04-25 14:13     ` [PATCH V2 1/8] " Thierry Reding
2019-03-11 11:11 ` [V2,2/8] phy: tegra: xusb: t210: add usb3 port fake support Nagarjuna Kristam
2019-04-25 14:55   ` Thierry Reding
2019-04-25 14:55     ` [PATCH V2 2/8] " Thierry Reding
2019-05-08  9:35     ` Nagarjuna Kristam
2019-03-11 11:11 ` [V2,3/8] phy: tegra: xusb: t210: add vbus override support Nagarjuna Kristam
2019-04-25 15:04   ` Thierry Reding
2019-04-25 15:04     ` [PATCH V2 3/8] " Thierry Reding
2019-05-08 10:21     ` Nagarjuna Kristam
2019-03-11 11:11 ` [V2,4/8] dt-bindings: usb: Add NVIDIA Tegra XUSB device mode controller binding Nagarjuna Kristam
2019-04-25 13:57   ` Thierry Reding
2019-04-25 13:57     ` [PATCH V2 4/8] " Thierry Reding
2019-04-25 15:14   ` [V2,4/8] " Thierry Reding
2019-04-25 15:14     ` [PATCH V2 4/8] " Thierry Reding
2019-05-03 14:30     ` Thierry Reding [this message]
2019-05-03 14:30       ` Thierry Reding
2019-05-08 10:51       ` Nagarjuna Kristam
2019-03-11 11:11 ` [V2,7/8] usb: gadget: Add UDC driver for tegra XUSB device mode controller Nagarjuna Kristam
2019-04-25 13:00   ` Felipe Balbi
2019-04-25 13:00     ` [PATCH V2 7/8] " Felipe Balbi
2019-04-25 13:55     ` [V2,7/8] " Thierry Reding
2019-04-25 13:55       ` [PATCH V2 7/8] " Thierry Reding
2019-05-07 10:09       ` Nagarjuna Kristam
2019-05-03 14:22   ` [V2,7/8] " Thierry Reding
2019-05-03 14:22     ` [PATCH V2 7/8] " Thierry Reding

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190503143007.GA20469@ulmo \
    --to=thierry.reding@gmail.com \
    --cc=balbi@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jonathanh@nvidia.com \
    --cc=linux-tegra@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=nkristam@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).