2017-03-31 07:41:27

by Ralph Sennhauser

[permalink] [raw]
Subject: [PATCH] ARM: dts: armada-38x: label USB and SATA nodes

Recently most nodes got labels to make them referenceable. The USB 3.0
nodes as well as the nodes for the SATA controllers were left out,
rectify the omission.

Signed-off-by: Ralph Sennhauser <[email protected]>
---

Hi everybody

In preparation to add support for the Linksys WRT3200ACM (Rango) some
rework of the "dts-stack" is highly desirable. This patch is part of the
preparatory work but independent.

The label for the USB 2.0 port is "usb0:", so using "usb1:" and "usb2:"
would be the logical continuation. Guess you realized the catch already,
"usb2:" for an USB 3.0 controller might be very confusing.

I'm sending this patch ahead of the rest so the labels can be discussed
and finalized.

Ralph

---

arch/arm/boot/dts/armada-38x.dtsi | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
index ba27ec1..78c838a 100644
--- a/arch/arm/boot/dts/armada-38x.dtsi
+++ b/arch/arm/boot/dts/armada-38x.dtsi
@@ -530,7 +530,7 @@
interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
};

- sata@a8000 {
+ satac0: sata@a8000 {
compatible = "marvell,armada-380-ahci";
reg = <0xa8000 0x2000>;
interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
@@ -546,7 +546,7 @@
status = "disabled";
};

- sata@e0000 {
+ satac1: sata@e0000 {
compatible = "marvell,armada-380-ahci";
reg = <0xe0000 0x2000>;
interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>;
@@ -590,7 +590,7 @@
status = "disabled";
};

- usb3@f0000 {
+ usb3_0: usb3@f0000 {
compatible = "marvell,armada-380-xhci";
reg = <0xf0000 0x4000>,<0xf4000 0x4000>;
interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
@@ -598,7 +598,7 @@
status = "disabled";
};

- usb3@f8000 {
+ usb3_1: usb3@f8000 {
compatible = "marvell,armada-380-xhci";
reg = <0xf8000 0x4000>,<0xfc000 0x4000>;
interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
--
2.10.2


2017-03-31 16:50:29

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH] ARM: dts: armada-38x: label USB and SATA nodes

> - sata@a8000 {
> + satac0: sata@a8000 {

Hi Ralph

Why the c in satac0?

>
> - usb3@f0000 {
> + usb3_0: usb3@f0000 {
> compatible = "marvell,armada-380-xhci";
> reg = <0xf0000 0x4000>,<0xf4000 0x4000>;
> interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
> @@ -598,7 +598,7 @@
> status = "disabled";
> };
>
> - usb3@f8000 {
> + usb3_1: usb3@f8000 {
> compatible = "marvell,armada-380-xhci";
> reg = <0xf8000 0x4000>,<0xfc000 0x4000>;
> interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;

I can understand what you are saying. But does anybody else care? Are
there other .dtsi files differentiating between USB 1.1, 2 and 3?

Andrew

2017-03-31 17:39:26

by Ralph Sennhauser

[permalink] [raw]
Subject: Re: [PATCH] ARM: dts: armada-38x: label USB and SATA nodes

On Fri, 31 Mar 2017 18:50:15 +0200
Andrew Lunn <[email protected]> wrote:

> > - sata@a8000 {
> > + satac0: sata@a8000 {
>
> Hi Ralph
>
> Why the c in satac0?

For controller and to not conflict with a use case of sata0 for a port,
similarly to pciec and pcie1. See armada-385-synology-ds116.dts.

>
> >
> > - usb3@f0000 {
> > + usb3_0: usb3@f0000 {
> > compatible =
> > "marvell,armada-380-xhci"; reg = <0xf0000 0x4000>,<0xf4000 0x4000>;
> > interrupts = <GIC_SPI 16
> > IRQ_TYPE_LEVEL_HIGH>; @@ -598,7 +598,7 @@
> > status = "disabled";
> > };
> >
> > - usb3@f8000 {
> > + usb3_1: usb3@f8000 {
> > compatible =
> > "marvell,armada-380-xhci"; reg = <0xf8000 0x4000>,<0xfc000 0x4000>;
> > interrupts = <GIC_SPI 17
> > IRQ_TYPE_LEVEL_HIGH>;
>
> I can understand what you are saying. But does anybody else care? Are
> there other .dtsi files differentiating between USB 1.1, 2 and 3?

It's handled differently where ever I looked, some do some don't. A
case for distinguishing USB 2.0 and USB 3.0 like this is
armada-388-gp.dts.

Personally I'm only interested in there being a label, making it
"natural" is a bonus though.

Thanks
Ralph

>
> Andrew

2017-03-31 18:21:28

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH] ARM: dts: armada-38x: label USB and SATA nodes

On Fri, Mar 31, 2017 at 07:39:20PM +0200, Ralph Sennhauser wrote:
> On Fri, 31 Mar 2017 18:50:15 +0200
> Andrew Lunn <[email protected]> wrote:
>
> > > - sata@a8000 {
> > > + satac0: sata@a8000 {
> >
> > Hi Ralph
> >
> > Why the c in satac0?
>
> For controller and to not conflict with a use case of sata0 for a port,
> similarly to pciec and pcie1. See armada-385-synology-ds116.dts.

:~/linux/arch/arm/boot/dts$ ls *ds116*
ls: cannot access '*ds116*': No such file or directory

But anyway, a few boards seem to solve this by calling the controller
node ahci0: and the port sata0:

> > > - usb3@f0000 {
> > > + usb3_0: usb3@f0000 {
> > > compatible =
> > > "marvell,armada-380-xhci"; reg = <0xf0000 0x4000>,<0xf4000 0x4000>;
> > > interrupts = <GIC_SPI 16
> > > IRQ_TYPE_LEVEL_HIGH>; @@ -598,7 +598,7 @@
> > > status = "disabled";
> > > };
> > >
> > > - usb3@f8000 {
> > > + usb3_1: usb3@f8000 {
> > > compatible =
> > > "marvell,armada-380-xhci"; reg = <0xf8000 0x4000>,<0xfc000 0x4000>;
> > > interrupts = <GIC_SPI 17
> > > IRQ_TYPE_LEVEL_HIGH>;
> >
> > I can understand what you are saying. But does anybody else care? Are
> > there other .dtsi files differentiating between USB 1.1, 2 and 3?
>
> It's handled differently where ever I looked, some do some don't. A
> case for distinguishing USB 2.0 and USB 3.0 like this is
> armada-388-gp.dts.

Humm...

/* CON4 */
usb@58000 {
vcc-supply = <&reg_usb2_0_vbus>;
status = "okay";
};


/* CON5 */
usb3@f0000 {
usb-phy = <&usb2_1_phy>;
status = "okay";
};

/* CON7 */
usb3@f8000 {
usb-phy = <&usb3_phy>;
status = "okay";
};

Is this clear? Is CON5 a USB 3 host, but has a USB 2 PHY connected to
it? CON7 is the only true USB 3 port? I think some comments written in
schwiizerdütsch would be clearre.:-)

Andrew

2017-04-01 08:09:51

by Ralph Sennhauser

[permalink] [raw]
Subject: Re: [PATCH] ARM: dts: armada-38x: label USB and SATA nodes

On Fri, 31 Mar 2017 20:21:11 +0200
Andrew Lunn <[email protected]> wrote:

> On Fri, Mar 31, 2017 at 07:39:20PM +0200, Ralph Sennhauser wrote:
> > On Fri, 31 Mar 2017 18:50:15 +0200
> > Andrew Lunn <[email protected]> wrote:
> >
> > > > - sata@a8000 {
> > > > + satac0: sata@a8000 {
> > >
> > > Hi Ralph
> > >
> > > Why the c in satac0?
> >
> > For controller and to not conflict with a use case of sata0 for a
> > port, similarly to pciec and pcie1. See
> > armada-385-synology-ds116.dts.
>
> :~/linux/arch/arm/boot/dts$ ls *ds116*
> ls: cannot access '*ds116*': No such file or directory

Ah, not in mainline yet, from linux-next

commit a58d73340b0ec93fc29a826e45fbbfbc3f81b7eb
Author: Willy Tarreau <[email protected]>
Date: Sun Feb 12 10:30:35 2017 +0100

The arch/arm/boot/dts/armada-388-gp.dts from below was
meant as the example for the conflict I mentioned.

>
> But anyway, a few boards seem to solve this by calling the controller
> node ahci0: and the port sata0:

That's another option I missed.

$ git grep -n ahci.:
arch/arm/boot/dts/spear1310.dtsi:59: ahci0: ahci@b1000000 {
arch/arm/boot/dts/spear1310.dtsi:68: ahci1: ahci@b1800000 {
arch/arm/boot/dts/spear1310.dtsi:77: ahci2: ahci@b4000000 {
arch/arm/boot/dts/spear1340.dtsi:42: ahci0: ahci@b1000000 {

Not a big list which I have here and the nodes themselves are named
ahci@xxxxxxxx.

>
> > > > - usb3@f0000 {
> > > > + usb3_0: usb3@f0000 {
> > > > compatible =
> > > > "marvell,armada-380-xhci"; reg = <0xf0000 0x4000>,<0xf4000
> > > > 0x4000>; interrupts = <GIC_SPI 16
> > > > IRQ_TYPE_LEVEL_HIGH>; @@ -598,7 +598,7 @@
> > > > status = "disabled";
> > > > };
> > > >
> > > > - usb3@f8000 {
> > > > + usb3_1: usb3@f8000 {
> > > > compatible =
> > > > "marvell,armada-380-xhci"; reg = <0xf8000 0x4000>,<0xfc000
> > > > 0x4000>; interrupts = <GIC_SPI 17
> > > > IRQ_TYPE_LEVEL_HIGH>;
> > >
> > > I can understand what you are saying. But does anybody else care?
> > > Are there other .dtsi files differentiating between USB 1.1, 2
> > > and 3?
> >
> > It's handled differently where ever I looked, some do some don't. A
> > case for distinguishing USB 2.0 and USB 3.0 like this is
> > armada-388-gp.dts.
>
> Humm...
>
> /* CON4 */
> usb@58000 {
> vcc-supply = <&reg_usb2_0_vbus>;
> status = "okay";
> };
>
>
> /* CON5 */
> usb3@f0000 {
> usb-phy = <&usb2_1_phy>;
> status = "okay";
> };
>
> /* CON7 */
> usb3@f8000 {
> usb-phy = <&usb3_phy>;
> status = "okay";
> };
>
> Is this clear? Is CON5 a USB 3 host, but has a USB 2 PHY connected to
> it? CON7 is the only true USB 3 port? I think some comments written in
> schwiizerdütsch would be clearre.:-)

Did you just find a bug? :)

*ufm sprung gxi* (about to head out), sorry for the mix-up. The synology
dts would actually have worked for both usb and sata labels :)

$git grep -hn -A8 usb3_0_phy: arch/arm/boot/dts/armada-385-synology-ds116.dts
191: usb3_0_phy: usb3_0_phy {
192- compatible = "usb-nop-xceiv";
193- vcc-supply = <&reg_usb3_0_vbus>;
194- };
195-
196- usb3_1_phy: usb3_1_phy {
197- compatible = "usb-nop-xceiv";
198- vcc-supply = <&reg_usb3_1_vbus>;
199- };


---

Let's add another argument for and against usb3_x type labels:

$ git grep -hn usb arch/arm/boot/dts/armada-38x.dtsi
455: usb0: usb@58000 {
593: usb3_0: usb3@f0000 {
601: usb3_1: usb3@f8000 {

They might actually be considered different types. usb vs. usb3, though
that feels quite arbitrary.

$ git grep -hn usb3_0 Documentation/devicetree/bindings/usb/qcom,dwc3.txt
45: usb3_0: usb30@0 {

usb3_0 could be mistaken for the protocol version. A bit of a stretch as well ...

First thought was using usb0,usb1,usb2. For the individual linksys
boards this meant a potential pit-fall, namely using "usb2:" for the
only USB 3.0 port while "usb0:" for the only USB 2.0 port appears in the
armada-385-linksys.dtsi only, hence the quest for alternatives.

In the end it boils down to I couldn't make out a definitive standard and
made a pick that felt about right.

If there was an obvious choice there wouldn't have been a reason to omit
the labels this patch handles when handling the bulk. Make the bulk a none
discussion item and handle the corner cases later. Guess that's what
happened here.

Thanks
Ralph

2017-04-05 15:50:00

by Gregory CLEMENT

[permalink] [raw]
Subject: Re: [PATCH] ARM: dts: armada-38x: label USB and SATA nodes

Hi Andrew,

On ven., mars 31 2017, Andrew Lunn <[email protected]> wrote:

> On Fri, Mar 31, 2017 at 07:39:20PM +0200, Ralph Sennhauser wrote:
>> On Fri, 31 Mar 2017 18:50:15 +0200
>> Andrew Lunn <[email protected]> wrote:
>>
>> > > - sata@a8000 {
>> > > + satac0: sata@a8000 {
>> >
>> > Hi Ralph
>> >
>> > Why the c in satac0?
>>
>> For controller and to not conflict with a use case of sata0 for a port,
>> similarly to pciec and pcie1. See armada-385-synology-ds116.dts.
>
> :~/linux/arch/arm/boot/dts$ ls *ds116*
> ls: cannot access '*ds116*': No such file or directory
>
> But anyway, a few boards seem to solve this by calling the controller
> node ahci0: and the port sata0:
>
>> > > - usb3@f0000 {
>> > > + usb3_0: usb3@f0000 {
>> > > compatible =
>> > > "marvell,armada-380-xhci"; reg = <0xf0000 0x4000>,<0xf4000 0x4000>;
>> > > interrupts = <GIC_SPI 16
>> > > IRQ_TYPE_LEVEL_HIGH>; @@ -598,7 +598,7 @@
>> > > status = "disabled";
>> > > };
>> > >
>> > > - usb3@f8000 {
>> > > + usb3_1: usb3@f8000 {
>> > > compatible =
>> > > "marvell,armada-380-xhci"; reg = <0xf8000 0x4000>,<0xfc000 0x4000>;
>> > > interrupts = <GIC_SPI 17
>> > > IRQ_TYPE_LEVEL_HIGH>;
>> >
>> > I can understand what you are saying. But does anybody else care? Are
>> > there other .dtsi files differentiating between USB 1.1, 2 and 3?
>>
>> It's handled differently where ever I looked, some do some don't. A
>> case for distinguishing USB 2.0 and USB 3.0 like this is
>> armada-388-gp.dts.

Actually I care and I found confusing calling usb2 the second usb port if
it is controlled by an USB3 controller.

>
> Humm...
>
> /* CON4 */
> usb@58000 {
> vcc-supply = <&reg_usb2_0_vbus>;
> status = "okay";
> };
>
>
> /* CON5 */
> usb3@f0000 {
> usb-phy = <&usb2_1_phy>;
> status = "okay";
> };
>
> /* CON7 */
> usb3@f8000 {
> usb-phy = <&usb3_phy>;
> status = "okay";
> };
>
> Is this clear? Is CON5 a USB 3 host, but has a USB 2 PHY connected to
> it? CON7 is the only true USB 3 port? I think some comments written in

I can answer it: CON5 is indeed an USB3 host with a USB2 PHY connected
to it so we can use it only as an USB2. And indeed CON7 is the only true
USB3 port.

> schwiizerdütsch would be clearre.:-)

Actually all your assumption were correct so maybe it is not as
confusing as it looks! :) But I can add a comment if needed.

Gregory

--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com