On 04/05/2016 08:44 AM, Thierry Reding wrote:
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.
When the padctl driver registers the PHY objects with the PHY subsystem,
can it pass the lanes node as the DT node? That woulud mean each lane
/was/ a child of the node registered with the PHY subsystem.
Perhaps the PHY subsystem requires a struct device rather than a DT node
registered with it? If so, does it make sense to create a separate
struct device with the of_node pointing at lanes{}?
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.
Naively, it sounds like it'd be a good idea to fix the PHY core. It
really shouldn't care about the parent of any object registered with it;
it should only interact with the specific object it was given, and any
other data such as "ops" callbacks. Do you have any inkling how much
work that would be?
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
