[PATCH 1/4] arm64: dts: marvell: Clean up armada-3720-db

[PATCH 1/4] arm64: dts: marvell: Clean up armada-3720-db

[Andreas Färber]

Instead of duplicating the SoC's node hierarchy, including a bus node
named "internal-regs", reference the actually desired nodes by label,
like Berlin already does. Add labels where necessary.

Drop an inconsistent white line while at it.

Fixes: adbc3695d9e4 ("arm64: dts: add the Marvell Armada 3700 family and a development board")
Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 arch/arm64/boot/dts/marvell/armada-3720-db.dts | 35 +++++++++++---------------
 arch/arm64/boot/dts/marvell/armada-372x.dtsi   |  1 -
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi   |  4 +--
 3 files changed, 17 insertions(+), 23 deletions(-)

diff --git a/arch/arm64/boot/dts/marvell/armada-3720-db.dts b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
index 359050154511..48f97d14b057 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-db.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
@@ -60,27 +60,22 @@
 		device_type = "memory";
 		reg = <0x00000000 0x00000000 0x00000000 0x20000000>;
 	};
+};
 
-	soc {
-		internal-regs {
-			/*
-			* Exported on the micro USB connector CON32
-			* through an FTDI
-			*/
-			uart0: serial@12000 {
-				status = "okay";
-			};
-
-			/* CON31 */
-			usb3@58000 {
-				status = "okay";
-			};
+/* CON3 */
+&sata {
+	status = "okay";
+};
 
-			/* CON3 */
-			sata@e0000 {
-			       status = "okay";
-			};
-		};
-	};
+/*
+ * Exported on the micro USB connector CON32
+ * through an FTDI
+ */
+&uart0 {
+	status = "okay";
 };
 
+/* CON31 */
+&usb3 {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/marvell/armada-372x.dtsi b/arch/arm64/boot/dts/marvell/armada-372x.dtsi
index f292a00ce97c..5120296596c2 100644
--- a/arch/arm64/boot/dts/marvell/armada-372x.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-372x.dtsi
@@ -59,5 +59,4 @@
 			enable-method = "psci";
 		};
 	};
-
 };
diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index ba9df7ff2a72..4328c2408a8a 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -105,14 +105,14 @@
 				status = "disabled";
 			};
 
-			usb3@58000 {
+			usb3: usb3@58000 {
 				compatible = "generic-xhci";
 				reg = <0x58000 0x4000>;
 				interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
 				status = "disabled";
 			};
 
-			sata@e0000 {
+			sata: sata@e0000 {
 				compatible = "marvell,armada-3700-ahci";
 				reg = <0xe0000 0x2000>;
 				interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;
-- 
2.6.2

Re: [PATCH 1/4] arm64: dts: marvell: Clean up armada-3720-db

[Gregory CLEMENT]

Hi Andreas,
 
 On mer., mars 23 2016, Andreas Färber <afaerber@suse.de> wrote:

> Instead of duplicating the SoC's node hierarchy, including a bus node
> named "internal-regs", reference the actually desired nodes by label,
> like Berlin already does. Add labels where necessary.

We talked about this for the 3rd patch, but the conclusion will apply
here too. So we are going to use this solution for the mvebu 64bits. I
have just a minor remark.

>
> Drop an inconsistent white line while at it.
>
> Fixes: adbc3695d9e4 ("arm64: dts: add the Marvell Armada 3700 family and a development board")
> Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  arch/arm64/boot/dts/marvell/armada-3720-db.dts | 35 +++++++++++---------------
>  arch/arm64/boot/dts/marvell/armada-372x.dtsi   |  1 -
>  arch/arm64/boot/dts/marvell/armada-37xx.dtsi   |  4 +--
>  3 files changed, 17 insertions(+), 23 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-3720-db.dts b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
> index 359050154511..48f97d14b057 100644
> --- a/arch/arm64/boot/dts/marvell/armada-3720-db.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
> @@ -60,27 +60,22 @@
>  		device_type = "memory";
>  		reg = <0x00000000 0x00000000 0x00000000 0x20000000>;
>  	};
> +};
>  
> -	soc {
> -		internal-regs {
> -			/*
> -			* Exported on the micro USB connector CON32
> -			* through an FTDI
> -			*/
> -			uart0: serial@12000 {
> -				status = "okay";
> -			};
> -
> -			/* CON31 */
> -			usb3@58000 {
> -				status = "okay";
> -			};
> +/* CON3 */
> +&sata {
> +	status = "okay";
> +};
>  
> -			/* CON3 */
> -			sata@e0000 {
> -			       status = "okay";
> -			};
> -		};
> -	};
> +/*
> + * Exported on the micro USB connector CON32
> + * through an FTDI
> + */

As the comment didn't start anymore from the middle of the line, we
could have it an a single line. We don't have to split in 2 lines.

Thanks,

Gregory

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

Re: [PATCH 1/4] arm64: dts: marvell: Clean up armada-3720-db

[Andreas Färber]

Hi Gregory,

Am 24.03.2016 um 17:11 schrieb Gregory CLEMENT:
>> +/*
>> + * Exported on the micro USB connector CON32
>> + * through an FTDI
>> + */
> 
> As the comment didn't start anymore from the middle of the line, we
> could have it an a single line. We don't have to split in 2 lines.

Sure. Should I change that along with 4/4 in a v2? It'll be after the
Easter holidays.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)

Re: [PATCH 1/4] arm64: dts: marvell: Clean up armada-3720-db

[Gregory CLEMENT]

Hi Andreas,
 
 On jeu., mars 24 2016, Andreas Färber <afaerber@suse.de> wrote:

> Hi Gregory,
>
> Am 24.03.2016 um 17:11 schrieb Gregory CLEMENT:
>>> +/*
>>> + * Exported on the micro USB connector CON32
>>> + * through an FTDI
>>> + */
>> 
>> As the comment didn't start anymore from the middle of the line, we
>> could have it an a single line. We don't have to split in 2 lines.
>
> Sure. Should I change that along with 4/4 in a v2? It'll be after the
> Easter holidays.

Yes please and no problem for wiating for after the Easter holidays I
won't apply the patch on the mvebu branches before the release of the
v4.6-rc1.

Thanks,

Gregory

>
> Regards,
> Andreas
>
> -- 
> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)

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

Re: [PATCH 1/4] arm64: dts: marvell: Clean up armada-3720-db

[Gregory CLEMENT]

Hi Andreas,
 
 On ven., mars 25 2016, Gregory CLEMENT <gregory.clement@free-electrons.com> wrote:

> Hi Andreas,
>  
>  On jeu., mars 24 2016, Andreas Färber <afaerber@suse.de> wrote:
>
>> Hi Gregory,
>>
>> Am 24.03.2016 um 17:11 schrieb Gregory CLEMENT:
>>>> +/*
>>>> + * Exported on the micro USB connector CON32
>>>> + * through an FTDI
>>>> + */
>>> 
>>> As the comment didn't start anymore from the middle of the line, we
>>> could have it an a single line. We don't have to split in 2 lines.
>>
>> Sure. Should I change that along with 4/4 in a v2? It'll be after the
>> Easter holidays.
>
> Yes please and no problem for wiating for after the Easter holidays I
> won't apply the patch on the mvebu branches before the release of the
> v4.6-rc1.

Do you think you could send a v2 of the serie soon?

I would like to have it in the mvebu/dt64 tree as it will impact all the
next patches aimed for these files.

Thanks,

Gregory

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

[PATCH 2/4] arm64: dts: marvell: Rename armada-37xx USB node

[Andreas Färber]

No need to reflect the USB version in the node name.

Fixes: adbc3695d9e4 ("arm64: dts: add the Marvell Armada 3700 family and a development board")
Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index 4328c2408a8a..55a861d60fae 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -105,7 +105,7 @@
 				status = "disabled";
 			};
 
-			usb3: usb3@58000 {
+			usb3: usb@58000 {
 				compatible = "generic-xhci";
 				reg = <0x58000 0x4000>;
 				interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
-- 
2.6.2

[PATCH 3/4] arm64: dts: marvell: Clean up armada-7040-db

[Andreas Färber]

Instead of duplicating the node hierarchy, reference the nodes by label,
adding labels where necessary.

Drop some trailing or inconsistent white lines while at it.

Fixes: ec7e5a569bce ("arm64: dts: marvell: add Device Tree files for Armada 7K/8K")
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 Is it intentional that there are no aliases for the two serial nodes and no /chosen/stdout-path?
 
 arch/arm64/boot/dts/marvell/armada-7040-db.dts     | 52 ++++++++++------------
 arch/arm64/boot/dts/marvell/armada-ap806-dual.dtsi |  1 -
 arch/arm64/boot/dts/marvell/armada-ap806-quad.dtsi |  2 -
 arch/arm64/boot/dts/marvell/armada-ap806.dtsi      |  7 +--
 4 files changed, 26 insertions(+), 36 deletions(-)

diff --git a/arch/arm64/boot/dts/marvell/armada-7040-db.dts b/arch/arm64/boot/dts/marvell/armada-7040-db.dts
index 064a251346dd..ee5778395bea 100644
--- a/arch/arm64/boot/dts/marvell/armada-7040-db.dts
+++ b/arch/arm64/boot/dts/marvell/armada-7040-db.dts
@@ -55,38 +55,34 @@
 		device_type = "memory";
 		reg = <0x0 0x0 0x0 0x80000000>;
 	};
+};
 
-	ap806 {
-		config-space {
-			spi@510600 {
-				status = "okay";
-
-				spi-flash@0 {
-					#address-cells = <1>;
-					#size-cells = <1>;
-					compatible = "n25q128a13";
-					reg = <0>; /* Chip select 0 */
-					spi-max-frequency = <10000000>;
+&i2c0 {
+	status = "okay";
+	clock-frequency = <100000>;
+};
 
-					partition@0 {
-						label = "U-Boot";
-						reg = <0 0x200000>;
-					};
-					partition@400000 {
-						label = "Filesystem";
-						reg = <0x200000 0xce0000>;
-					};
-				};
-			};
+&spi0 {
+	status = "okay";
 
-			i2c@511000 {
-				status = "okay";
-				clock-frequency = <100000>;
-			};
+	spi-flash@0 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "n25q128a13";
+		reg = <0>; /* Chip select 0 */
+		spi-max-frequency = <10000000>;
 
-			serial@512000 {
-				status = "okay";
-			};
+		partition@0 {
+			label = "U-Boot";
+			reg = <0 0x200000>;
+		};
+		partition@400000 {
+			label = "Filesystem";
+			reg = <0x200000 0xce0000>;
 		};
 	};
 };
+
+&uart0 {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/marvell/armada-ap806-dual.dtsi b/arch/arm64/boot/dts/marvell/armada-ap806-dual.dtsi
index f25c5c17fad7..95a1ff60f6c1 100644
--- a/arch/arm64/boot/dts/marvell/armada-ap806-dual.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-ap806-dual.dtsi
@@ -68,4 +68,3 @@
 		};
 	};
 };
-
diff --git a/arch/arm64/boot/dts/marvell/armada-ap806-quad.dtsi b/arch/arm64/boot/dts/marvell/armada-ap806-quad.dtsi
index baa7d9a516b3..ba43a4357b89 100644
--- a/arch/arm64/boot/dts/marvell/armada-ap806-quad.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-ap806-quad.dtsi
@@ -79,6 +79,4 @@
 			enable-method = "psci";
 		};
 	};
-
 };
-
diff --git a/arch/arm64/boot/dts/marvell/armada-ap806.dtsi b/arch/arm64/boot/dts/marvell/armada-ap806.dtsi
index 556a92bcc2f6..3ecf9b1798fa 100644
--- a/arch/arm64/boot/dts/marvell/armada-ap806.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-ap806.dtsi
@@ -59,7 +59,6 @@
 		method = "smc";
 	};
 
-
 	ap806 {
 		#address-cells = <2>;
 		#size-cells = <2>;
@@ -190,7 +189,7 @@
 				status = "disabled";
 			};
 
-			serial@512000 {
+			uart0: serial@512000 {
 				compatible = "snps,dw-apb-uart";
 				reg = <0x512000 0x100>;
 				reg-shift = <2>;
@@ -200,7 +199,7 @@
 				status = "disabled";
 			};
 
-			serial@512100 {
+			uart1: serial@512100 {
 				compatible = "snps,dw-apb-uart";
 				reg = <0x512100 0x100>;
 				reg-shift = <2>;
@@ -232,6 +231,4 @@
 			};
 		};
 	};
-
 };
-
-- 
2.6.2

Re: [PATCH 3/4] arm64: dts: marvell: Clean up armada-7040-db

[Thomas Petazzoni]

Hello,

On Wed, 23 Mar 2016 23:24:20 +0100, Andreas Färber wrote:
> Instead of duplicating the node hierarchy, reference the nodes by label,
> adding labels where necessary.
> 
> Drop some trailing or inconsistent white lines while at it.
> 
> Fixes: ec7e5a569bce ("arm64: dts: marvell: add Device Tree files for Armada 7K/8K")
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>

We haven't used this solution on Marvell Armada 32-bits SoCs, so there
needs to be a discussion on whether we want to go in this direction for
the 64 bits SoCs.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 3/4] arm64: dts: marvell: Clean up armada-7040-db

[Gregory CLEMENT]

Hi Thomas,
 
 On jeu., mars 24 2016, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:

> Hello,
>
> On Wed, 23 Mar 2016 23:24:20 +0100, Andreas Färber wrote:
>> Instead of duplicating the node hierarchy, reference the nodes by label,
>> adding labels where necessary.
>> 
>> Drop some trailing or inconsistent white lines while at it.
>> 
>> Fixes: ec7e5a569bce ("arm64: dts: marvell: add Device Tree files for Armada 7K/8K")
>> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>
> We haven't used this solution on Marvell Armada 32-bits SoCs, so there
> needs to be a discussion on whether we want to go in this direction for
> the 64 bits SoCs.

At first view using the label helps to have simpler dts files.

Is there any cons by using it?

I agree that converting the Marvell Armada 32-bits SoCs would produce a
lot of churn. But if some binding are common there is no file at all are
in common, so we could use this solution for the 64 bits SoCs only.

Jason, Andrew, Sebastian, do you see any problem with it?

Thanks,

Gregory

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

Re: [PATCH 3/4] arm64: dts: marvell: Clean up armada-7040-db

[Thomas Petazzoni]

Hello,

On Thu, 24 Mar 2016 11:27:27 +0100, Gregory CLEMENT wrote:

> > We haven't used this solution on Marvell Armada 32-bits SoCs, so there
> > needs to be a discussion on whether we want to go in this direction for
> > the 64 bits SoCs.
> 
> At first view using the label helps to have simpler dts files.
> 
> Is there any cons by using it?

The only minor drawback from my point of view is that you don't see
where in the hierarchy the device you're enabling is. But it's really a
minor drawback and not everybody agrees that it is actually a drawback.

So I'm fine.

> I agree that converting the Marvell Armada 32-bits SoCs would produce a
> lot of churn. But if some binding are common there is no file at all are
> in common, so we could use this solution for the 64 bits SoCs only.

Yes, we could. I'm fine with it. I was merely pointing out that it is
moving away from our 32 bits way of doing things.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 3/4] arm64: dts: marvell: Clean up armada-7040-db

[Andrew Lunn]

> > I agree that converting the Marvell Armada 32-bits SoCs would produce a
> > lot of churn. But if some binding are common there is no file at all are
> > in common, so we could use this solution for the 64 bits SoCs only.
> 
> Yes, we could. I'm fine with it. I was merely pointing out that it is
> moving away from our 32 bits way of doing things.

Sebastian spent a while converting dove and kirkwood to this
scheme. Most boards have i2c, nand, eth, and mdio properties
represented link this.

There are no many 64 bit boards, so now is the time to make the
change...

	Andrew

Re: [PATCH 3/4] arm64: dts: marvell: Clean up armada-7040-db

[Jason Cooper]

On Thu, Mar 24, 2016 at 02:37:23PM +0100, Andrew Lunn wrote:
> > > I agree that converting the Marvell Armada 32-bits SoCs would produce a
> > > lot of churn. But if some binding are common there is no file at all are
> > > in common, so we could use this solution for the 64 bits SoCs only.
> > 
> > Yes, we could. I'm fine with it. I was merely pointing out that it is
> > moving away from our 32 bits way of doing things.
> 
> Sebastian spent a while converting dove and kirkwood to this
> scheme. Most boards have i2c, nand, eth, and mdio properties
> represented link this.
> 
> There are no many 64 bit boards, so now is the time to make the
> change...

Agreed.

thx,

Jason.

[PATCH 4/4] arm64: dts: marvell: Rename armada-ap806 XOR nodes

[Andreas Färber]

Node names should not contain an instance number, the unit address
serves to distinguish nodes of the same name. So rename the XOR nodes
to just xor@address, using xorX as label instead.

Fixes: ec7e5a569bce ("arm64: dts: marvell: add Device Tree files for Armada 7K/8K")
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 "marvell,mv-xor-v2" is not yet referenced from any driver in next-20160323.
 
 arch/arm64/boot/dts/marvell/armada-ap806.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/marvell/armada-ap806.dtsi b/arch/arm64/boot/dts/marvell/armada-ap806.dtsi
index 3ecf9b1798fa..dd6e48a952ac 100644
--- a/arch/arm64/boot/dts/marvell/armada-ap806.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-ap806.dtsi
@@ -135,7 +135,7 @@
 				marvell,spi-base = <128>, <136>, <144>, <152>;
 			};
 
-			xor0@400000 {
+			xor0: xor@400000 {
 				compatible = "marvell,mv-xor-v2";
 				reg = <0x400000 0x1000>,
 				      <0x410000 0x1000>;
@@ -143,7 +143,7 @@
 				dma-coherent;
 			};
 
-			xor1@420000 {
+			xor1: xor@420000 {
 				compatible = "marvell,mv-xor-v2";
 				reg = <0x420000 0x1000>,
 				      <0x430000 0x1000>;
@@ -151,7 +151,7 @@
 				dma-coherent;
 			};
 
-			xor2@440000 {
+			xor2: xor@440000 {
 				compatible = "marvell,mv-xor-v2";
 				reg = <0x440000 0x1000>,
 				      <0x450000 0x1000>;
@@ -159,7 +159,7 @@
 				dma-coherent;
 			};
 
-			xor3@460000 {
+			xor3: xor@460000 {
 				compatible = "marvell,mv-xor-v2";
 				reg = <0x460000 0x1000>,
 				      <0x470000 0x1000>;
-- 
2.6.2

Re: [PATCH 4/4] arm64: dts: marvell: Rename armada-ap806 XOR nodes

[Thomas Petazzoni]

Hello,

On Wed, 23 Mar 2016 23:24:21 +0100, Andreas Färber wrote:
> Node names should not contain an instance number, the unit address
> serves to distinguish nodes of the same name. So rename the XOR nodes
> to just xor@address, using xorX as label instead.
> 
> Fixes: ec7e5a569bce ("arm64: dts: marvell: add Device Tree files for Armada 7K/8K")
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>

Removing the instance number from the node name is obviously good.
However, I don't think it's really needed to add labels. The XOR
engines are purely "internal" devices, so they are always enabled at
the SoC level.

>  "marvell,mv-xor-v2" is not yet referenced from any driver in next-20160323.

Yes. The driver has been submitted, but has received some comments that
I need to address.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com