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

[Thierry Reding]

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@xxxxxxxxxx>
> > 
> > Extend the binding to cover the set of feature found in Tegra210.
> 
> Acked-by: Stephen Warren <swarren@xxxxxxxxxx>
> 
> > 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

Attachment: signature.asc
Description: PGP signature