Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1350376 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH v10 1/9] dt-bindings: phy: Add NVIDIA Tegra XUSB pad controller binding Thierry Reding <thierry.reding@gmail.com> - 2016-03-04 17:20 +0100
    [PATCH v10 9/9] usb: xhci: tegra: Add Tegra210 support Thierry Reding <thierry.reding@gmail.com> - 2016-03-04 17:30 +0100
    [PATCH v10 6/9] dt-bindings: usb: Add NVIDIA Tegra XUSB controller binding Thierry Reding <thierry.reding@gmail.com> - 2016-03-04 17:30 +0100
      Re: [PATCH v10 6/9] dt-bindings: usb: Add NVIDIA Tegra XUSB  controller binding Stephen Warren <swarren@wwwdotorg.org> - 2016-03-16 19:10 +0100
    [PATCH v10 7/9] dt-bindings: usb: xhci-tegra: Add Tegra210 XUSB controller support Thierry Reding <thierry.reding@gmail.com> - 2016-03-04 17:30 +0100
      Re: [PATCH v10 7/9] dt-bindings: usb: xhci-tegra: Add Tegra210 XUSB  controller support Rob Herring <robh@kernel.org> - 2016-03-16 07:50 +0100
      Re: [PATCH v10 7/9] dt-bindings: usb: xhci-tegra: Add Tegra210 XUSB  controller support Stephen Warren <swarren@wwwdotorg.org> - 2016-03-16 19:10 +0100
    [PATCH v10 8/9] usb: xhci: Add NVIDIA Tegra XUSB controller driver Thierry Reding <thierry.reding@gmail.com> - 2016-03-04 17:30 +0100
    [PATCH v10 2/9] dt-bindings: pinctrl: Deprecate Tegra XUSB pad controller binding Thierry Reding <thierry.reding@gmail.com> - 2016-03-04 17:30 +0100
      Re: [PATCH v10 2/9] dt-bindings: pinctrl: Deprecate Tegra XUSB pad  controller binding Rob Herring <robh@kernel.org> - 2016-03-05 05:40 +0100
      Re: [PATCH v10 2/9] dt-bindings: pinctrl: Deprecate Tegra XUSB pad  controller binding Linus Walleij <linus.walleij@linaro.org> - 2016-03-15 10:10 +0100
      Re: [PATCH v10 2/9] dt-bindings: pinctrl: Deprecate Tegra XUSB pad  controller binding Stephen Warren <swarren@wwwdotorg.org> - 2016-03-16 18:50 +0100
    [PATCH v10 3/9] dt-bindings: phy: tegra-xusb-padctl: Add Tegra210 support Thierry Reding <thierry.reding@gmail.com> - 2016-03-04 17:30 +0100
      Re: [PATCH v10 3/9] dt-bindings: phy: tegra-xusb-padctl: Add Tegra210 support Andrew Bresticker <abrestic@chromium.org> - 2016-03-04 22:50 +0100
      Re: [PATCH v10 3/9] dt-bindings: phy: tegra-xusb-padctl: Add  Tegra210 support Rob Herring <robh@kernel.org> - 2016-03-05 05:40 +0100
      Re: [PATCH v10 3/9] dt-bindings: phy: tegra-xusb-padctl: Add Tegra210 support Linus Walleij <linus.walleij@linaro.org> - 2016-03-15 10:10 +0100
      Re: [PATCH v10 3/9] dt-bindings: phy: tegra-xusb-padctl: Add Tegra210  support Stephen Warren <swarren@wwwdotorg.org> - 2016-03-16 19:00 +0100
    Re: [PATCH v10 1/9] dt-bindings: phy: Add NVIDIA Tegra XUSB pad  controller binding Andrew Bresticker <abrestic@chromium.org> - 2016-03-04 22:40 +0100
    Re: [PATCH v10 1/9] dt-bindings: phy: Add NVIDIA Tegra XUSB pad  controller binding Andrew Bresticker <abrestic@chromium.org> - 2016-03-04 22:50 +0100
    Re: [PATCH v10 1/9] dt-bindings: phy: Add NVIDIA Tegra XUSB pad  controller binding Rob Herring <robh@kernel.org> - 2016-03-05 05:40 +0100
      Re: [PATCH v10 1/9] dt-bindings: phy: Add NVIDIA Tegra XUSB pad  controller binding Thierry Reding <thierry.reding@gmail.com> - 2016-03-07 12:30 +0100
        Re: [PATCH v10 1/9] dt-bindings: phy: Add NVIDIA Tegra XUSB pad  controller binding Rob Herring <robh@kernel.org> - 2016-03-16 07:50 +0100
    Re: [PATCH v10 1/9] dt-bindings: phy: Add NVIDIA Tegra XUSB pad  controller binding Stephen Warren <swarren@wwwdotorg.org> - 2016-03-16 18:40 +0100

Page 2 of 2 — ← Prev page 1 [2]

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

[Multipart message — attachments visible in raw view] — view raw

On Fri, Mar 04, 2016 at 10:31:45PM -0600, Rob Herring wrote:
> On Fri, Mar 04, 2016 at 05:19:31PM +0100, Thierry Reding 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>
> > ---
> > Changes in v10:
> > - clarify that the hardware documentation means something different when
> >   referring to a "port" (intra-SoC connectivity)
> > 
> > Changes in v9:
> > - rename UTMI -> USB2 to match hardware documentation
> > - reword according to suggestions by Stephen Warren
> > - make Tegra132 compatible string list consistent
> > - remove mailbox support
> > 
> >  .../bindings/phy/nvidia,tegra124-xusb-padctl.txt   | 376 +++++++++++++++++++++
> >  .../pinctrl/nvidia,tegra124-xusb-padctl.txt        |   5 +
> >  2 files changed, 381 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
> 
> Without really understanding the h/w here, looks okay to me.
> 
> Acked-by: Rob Herring <robh@kernel.org>
> 
> > +SoC include:
> > +
> > +	padctl@0,7009f000 {
> 
> Drop the comma. Commas should only be used if there are distinct fields.
> 
> If I get my dtc patch done, these are going to start warning, so you 
> might want to go fix dts files (assuming that's where this is coming 
> from).

I noticed that in today's next the updated DTC already complains about a
lot of things in existing DTS files. For Tegra that's primarily the
memory node, because it has a reg property but no unit name. Any hints
as to how to solve that? I think I remember from way back that memory
was supposed to be an exception, perhaps DTC needs to be taught that?

Thierry

[toc] | [prev] | [next] | [standalone]

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

On Mon, Mar 07, 2016 at 12:24:18PM +0100, Thierry Reding wrote:
> On Fri, Mar 04, 2016 at 10:31:45PM -0600, Rob Herring wrote:
> > On Fri, Mar 04, 2016 at 05:19:31PM +0100, Thierry Reding 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>
> > > ---
> > > Changes in v10:
> > > - clarify that the hardware documentation means something different when
> > >   referring to a "port" (intra-SoC connectivity)
> > > 
> > > Changes in v9:
> > > - rename UTMI -> USB2 to match hardware documentation
> > > - reword according to suggestions by Stephen Warren
> > > - make Tegra132 compatible string list consistent
> > > - remove mailbox support
> > > 
> > >  .../bindings/phy/nvidia,tegra124-xusb-padctl.txt   | 376 +++++++++++++++++++++
> > >  .../pinctrl/nvidia,tegra124-xusb-padctl.txt        |   5 +
> > >  2 files changed, 381 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
> > 
> > Without really understanding the h/w here, looks okay to me.
> > 
> > Acked-by: Rob Herring <robh@kernel.org>
> > 
> > > +SoC include:
> > > +
> > > +	padctl@0,7009f000 {
> > 
> > Drop the comma. Commas should only be used if there are distinct fields.
> > 
> > If I get my dtc patch done, these are going to start warning, so you 
> > might want to go fix dts files (assuming that's where this is coming 
> > from).
> 
> I noticed that in today's next the updated DTC already complains about a
> lot of things in existing DTS files. For Tegra that's primarily the
> memory node, because it has a reg property but no unit name. Any hints
> as to how to solve that? I think I remember from way back that memory
> was supposed to be an exception, perhaps DTC needs to be taught that?

I remember something about the skeleton.dtsi files and needing to avoid 
having both memory and memory@x nodes, but we really should fix them and 
have a unit address.

Rob

[toc] | [prev] | [next] | [standalone]

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

On 03/04/2016 09:19 AM, Thierry Reding 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.

>   .../bindings/phy/nvidia,tegra124-xusb-padctl.txt   | 376 +++++++++++++++++++++
>   .../pinctrl/nvidia,tegra124-xusb-padctl.txt        |   5 +

It seems odd to add part of the deprecation notice in this patch and one 
more line in the second/next patch. Did an update get squashed into the 
wrong commit? I'd suggest moving the edit to existing file 
nvidia,tegra124-xusb-padctl.txt entirely into patch 2. Perhaps this 
could happen while/when the patch is applied to avoid having to post 
another version.

> diff --git a/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt b/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt

...
> +Pads will be represented as children of the top-level XUSB pad controller
 > +device tree node.

Nit: grand-children, since they're house inside pads{} first. I might 
suggest:

A "pads" node exists to represent all pads contained within the XUSB 
controller. Each pad is represented as a subnode of the pads node.

> Each lane exposed by the pad will be represented by its
> +own subnode and can be referenced by users of the lane using the standard
> +PHY bindings, as described by the phy-bindings.txt file in this directory.

"Each lane exposed by the pad will be represented as a subnode of the 
pad node ..."?

I'd suggest adding a similar paragraph describing the ports node, and 
that ports are child nodes of that. Otherwise, since the documentation 
of the nodes isn't nested in any way, it's not clear from the text 
exactly what nodes are children of what other nodes.

> +The Tegra hardware documentation refers to the connection between the XUSB
> +pad controller and the XUSB controller as "ports".

I think, being pedantic, that "port" in the TRM refers to the set of 
signals at the edge/interface-to the IO controller, not the connection 
between the IO controller and the XUSB controller. Still, the existing 
wording in this patch is fine; no need to change it.

Still, the examples do clear this up, so perhaps it's not worth another 
version of the series to fix this. Or if you do think it's worth fixing, 
I'd be perfectly happy to see that done in follow-on patches. If you 
want I can write that follow-on patch once this series is applied.

...
> +PHY nodes:
> +==========
> +
> +Each pad node has one or more children, each representing one of the lanes
> +controlled by the pad.
> +
> +Required properties:
> +--------------------
> +- status: Defines the operation status of the PHY. Valid values are:
> +  - "disabled": the PHY is disabled
> +  - "okay": the PHY is enabled

Presumably the standard semantics of a missing status property 
implicitly meaning "okay" are also intended? A similar comment applies 
to other places where status is documented. "status" is typically not a 
required property.

> +Port nodes:
> +===========

> +USB2 ports:
> +-----------

Should that say "UTMI ports"? ULPI and HSIC below are (or can be) USB2 
ports too.

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

How do these properties tie the DT-based port definition to a particular 
PHY/lane/... in HW? I don't see a property that contains any kind of HW 
ID here.

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

It's not clear what "internal" and "external" mean. Presumably it's 
on-PCB vs. physical-connector-exposed-to-the-user. It may be worth 
explicitly mentioning that.

Is there no vbus-supply for USB2/UTMI ports? I'm also not sure why 
vbus-supply is optional for ULPI and HSIC. Even if there is no SW 
/control/ over VBUS, there still must be some source of power; IIRC Mark 
Brown typically desires that to be explicitly modelled with an always-on 
regulator if there's no SW control.

> +Super-speed USB ports:
> +----------------------
> +
> +Required properties:
> +- status: Defines the operation status of the port. Valid values are:
> +  - "disabled": the port is disabled
> +  - "okay": the port is enabled
> +- nvidia,usb2-companion: A single cell that specifies the physical port number
> +  to map this super-speed USB port to. The range of valid port numbers varies
> +  with the SoC generation:
> +  - 0-2: for Tegra124 and Tegra132

How can this be used to look up the corresponding USB2 node in DT? I 
would have expected a phandle here, with the physical (HW) port ID being 
represented in the referenced node. Otherwise, I don't see how to tie 
together the USB2 and USB3 DT nodes.

> +For Tegra124 and Tegra132, the XUSB pad controller exposes the following
> +ports:
> +- 3x USB2: usb2-0, usb2-1, usb2-2
> +- 1x ULPI: ulpi-0
> +- 2x HSIC: hsic-0, hsic-1
> +- 2x super-speed USB: usb3-0, usb3-1

Oh, is the physical port ID implicit in the DT node name? It may be 
worth mentioning that when describing the properties for each type of node.

I'll assume that's how the USB2<->USB3 mapping works. All of my comments 
are mainly re: the description/documentation of the binding itself. That 
can all be enhanced later. The underlying binding itself, and the 
example, look reasonable. As such, this patch,

Acked-by: Stephen Warren <swarren@nvidia.com>

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web