linux-kernel - Re: [PATCH v10 3/9] dt-bindings: phy: tegra-xusb-padctl: Add Tegra210 support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 5 Apr 2016 16:44:16 +0200
From:	Thierry Reding <thierry.reding@...il.com>
To:	Stephen Warren <swarren@...dotorg.org>
Cc:	Kishon Vijay Abraham I <kishon@...com>,
	Linus Walleij <linus.walleij@...aro.org>,
	Alexandre Courbot <gnurou@...il.com>,
	Andrew Bresticker <abrestic@...omium.org>,
	linux-tegra@...r.kernel.org, devicetree@...r.kernel.org,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>
Subject: Re: [PATCH v10 3/9] dt-bindings: phy: tegra-xusb-padctl: Add
 Tegra210 support

On Wed, Mar 16, 2016 at 11:59:44AM -0600, Stephen Warren wrote:
> On 03/04/2016 09:19 AM, Thierry Reding wrote:
> > From: Thierry Reding <treding@...dia.com>
> > 
> > Extend the binding to cover the set of feature found in Tegra210.
> 
> Acked-by: Stephen Warren <swarren@...dia.com>
> 
> > diff --git a/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt b/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
> 
> > +	padctl@0,7009f000 {
> ...
> > +		pads {
> ...
> > +		};
> > +
> > +		ports {
> ...
> > +		};
> 
> As a comment not affecting my ack in any way: At the top-level, we place all
> the child nodes into "array container" nodes named "pads" and "ports". This
> is nice since it separates different types of child nodes and allows easily
> adding more types of child nodes in the future without interference, and in
> a way that allows us to explicitly know what each node is without having to
> interpret its name or compatible value to do so. However, we haven't done
> this with the per-lane child nodes inside each pad. If we were to rev the
> design, I'd be tempted to suggest:
> 
> 	padctl@0,7009f000 {
> 		pads {
> 			usb2 {
> 				lanes { // This level is new
> 					usb2-0 {

I tried to make this work, but it's unfortunately not possible with the
current code. The reason is that the PHY subsystem assumes that either
the provider DT node corresponds to that of the device (the usb2 pad in
the above example) or one of its children. Hence, putting everything
into one more level further down would require some mechanism to tell
the subsystem about it so that it can be found.

Arguably the current support code isn't a good argument for designing a
binding, so perhaps it'd be useful to add this mechanism in order to get
a better binding. On the other hand, I'm not sure it's really worth it,
since we already have generic bindings that specify the layout of child
devices, and those have been agreed upon broadly (presumably).

In light of the recent discussion on DPAUX vs. I2C, I see how having the
extra level would be useful to provide additional context. If you think
it's worth it I can spend the extra time to get this implemented in the
core.

Thierry

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)