Skip to contentSkip to conversation

Changes to Z-turn dts

PATCH Superseded tossel at gmail.com 3 participants 8 messages Mar 13, 2018 – Mar 15, 2018

Subsystems (2)
Arm (32-bit) Arm/Zynq architecture

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

tossel at gmail.com

author
#0 lore raw

cover Changes to Z-turn dts

From: Anton Gerasimov <[email protected]>

A device tree for MYIR Z-turn board was recently (almost) merged to Linux kernel and a few issues arised in the review process. The issues are fixed by the patches below. The dts is still not exactly the same as one in Linux because of 'u-boot,dm-pre-reloc' and qspi device, hope that's allright.

Anton Gerasimov (2):
ARM: dts: zynq: Update dts for Z-turn board
ARM: dts: zynq: Rename dts for Z-turn board

 arch/arm/dts/Makefile                              |  2 +-
 .../dts/{zynq-zturn-myir.dts => zynq-zturn.dts}    | 64 +++++-----------------
 2 files changed, 14 insertions(+), 52 deletions(-)
 rename arch/arm/dts/{zynq-zturn-myir.dts => zynq-zturn.dts} (53%)
Signature · 1 line
2.16.2
author
#1 lore raw

PATCH 1/2 ARM: dts: zynq: Update dts for Z-turn board

From: Anton Gerasimov <[email protected]>

Delete devices implemented in PL, stylistic changes.

Signed-off-by Anton Gerasimov <[email protected]>
---
 arch/arm/dts/zynq-zturn-myir.dts | 64 ++++++++--------------------------------
 1 file changed, 13 insertions(+), 51 deletions(-)
arch/arm/dts/zynq-zturn-myir.dts +13 -51
index a5ecfcc1d7..b6661d0205 100644
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  *  Copyright (C) 2015 Andrea Merello <[email protected]>
  *  Copyright (C) 2017 Alexander Graf <[email protected]>

@@ -6,87 +7,49 @@
  *  Copyright (C) 2011 - 2014 Xilinx
  *  Copyright (C) 2012 National Instruments Corp.
  *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
  */
+
 /dts-v1/;
 /include/ "zynq-7000.dtsi"
 
 / {
 	model = "Zynq Z-Turn MYIR Board";
-	compatible = "xlnx,zynq-7000";
+	compatible = "myir,zynq-zturn", "xlnx,zynq-7000";
 
 	aliases {
 		ethernet0 = &gem0;
 		serial0 = &uart1;
 		serial1 = &uart0;
-		spi0 = &qspi;
-		mmc0 = &sdhci0;
 	};
 
-	memory {
+	memory at 0 {
 		device_type = "memory";
 		reg = <0x0 0x40000000>;
 	};
 
 	chosen {
-		stdout-path = "serial0:115200n8";
+		bootargs = "console=ttyPS0,115200 earlyprintk root=/dev/mmcblk0p2 rootwait";
 	};
 
 	gpio-leds {
 		compatible = "gpio-leds";
-		led_r {
-			label = "led_r";
-			gpios = <&gpio0 0x72 0x1>;
-			default-state = "on";
-			linux,default-trigger = "heartbeat";
-		};
-
-		led_g {
-			label = "led_g";
-			gpios = <&gpio0 0x73 0x1>;
-			default-state = "on";
-			linux,default-trigger = "heartbeat";
-		};
-
-		led_b {
-			label = "led_b";
-			gpios = <&gpio0 0x74 0x1>;
-			default-state = "on";
-			linux,default-trigger = "heartbeat";
-		};
-
-		usr_led1 {
-			label = "usr_led1";
+		usr-led1 {
+			label = "usr-led1";
 			gpios = <&gpio0 0x0 0x1>;
 			default-state = "off";
-			linux,default-trigger = "none";
 		};
 
-		usr_led2 {
-			label = "usr_led2";
+		usr-led2 {
+			label = "usr-led2";
 			gpios = <&gpio0 0x9 0x1>;
 			default-state = "off";
-			linux,default-trigger = "none";
 		};
 	};
 
-	gpio-beep {
-		compatible = "gpio-beeper";
-		label = "pl-beep";
-		gpios = <&gpio0 0x75 0x0>;
-	};
-
 	gpio-keys {
 		compatible = "gpio-keys";
-		#address-cells = <0x1>;
-		#size-cells = <0x0>;
+		#address-cells = <1>;
+		#size-cells = <0>;
 		autorepeat;
 		K1 {
 			label = "K1";

@@ -100,7 +63,6 @@
 
 &clkc {
 	ps-clk-frequency = <33333333>;
-	fclk-enable = <0xf>;
 };
 
 &qspi {

@@ -152,8 +114,8 @@
 		reg = <0x49>;
 	};
 
-	adxl345 at 53 {
-		compatible = "adi,adxl34x", "adxl34x";
+	accelerometer at 53 {
+		compatible = "adi,adxl345", "adxl345";
 		reg = <0x53>;
 		interrupt-parent = <&intc>;
 		interrupts = <0x0 0x1e 0x4>;
Signature · 1 line
2.16.2
#2 lore raw

PATCH 1/2 ARM: dts: zynq: Update dts for Z-turn board

Quoted 29 lines · tossel at gmail.com

From: Anton Gerasimov <[email protected]>

Delete devices implemented in PL, stylistic changes.

Signed-off-by Anton Gerasimov <[email protected]>
---
 arch/arm/dts/zynq-zturn-myir.dts | 64 ++++++++--------------------------------
 1 file changed, 13 insertions(+), 51 deletions(-)
arch/arm/dts/zynq-zturn-myir.dts +1 -7
index a5ecfcc1d7..b6661d0205 100644
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  *  Copyright (C) 2015 Andrea Merello <[email protected]>
  *  Copyright (C) 2017 Alexander Graf <[email protected]>

@@ -6,87 +7,49 @@
  *  Copyright (C) 2011 - 2014 Xilinx
  *  Copyright (C) 2012 National Instruments Corp.
  *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

arch/arm/dts/zynq-zturn-myir.dts line 17

Quoted 10 lines
arch/arm/dts/zynq-zturn-myir.dts +2 -2
- * GNU General Public License for more details.
  */
+
 /dts-v1/;
 /include/ "zynq-7000.dtsi"
 
 / {
 	model = "Zynq Z-Turn MYIR Board";
-	compatible = "xlnx,zynq-7000";
+	compatible = "myir,zynq-zturn", "xlnx,zynq-7000";

ack.

arch/arm/dts/zynq-zturn-myir.dts line 25

Quoted 11 lines
arch/arm/dts/zynq-zturn-myir.dts +1 -3
 	aliases {
 		ethernet0 = &gem0;
 		serial0 = &uart1;
 		serial1 = &uart0;
-		spi0 = &qspi;
-		mmc0 = &sdhci0;
 	};
 
-	memory {
+	memory at 0 {

Why?

arch/arm/dts/zynq-zturn-myir.dts line 39

Quoted 6 lines
arch/arm/dts/zynq-zturn-myir.dts +0 -1
 		device_type = "memory";
 		reg = <0x0 0x40000000>;
 	};
 
 	chosen {
-		stdout-path = "serial0:115200n8";

Nack. By default graphical output is quite unusable on this board, so we want to output to serial.

If your Linux submitted device tree doesn't contain this part, please fix it there.

arch/arm/dts/zynq-zturn-myir.dts line 31

Quoted 1 line
arch/arm/dts/zynq-zturn-myir.dts +1 -0
+		bootargs = "console=ttyPS0,115200 earlyprintk root=/dev/mmcblk0p2 rootwait";

This is even worse. Please don't prepopulate any bootargs, otherwise people may end up assuming that they're actually getting used.

Quoted 14 lines
arch/arm/dts/zynq-zturn-myir.dts +0 -10
 	};
 
 	gpio-leds {
 		compatible = "gpio-leds";
-		led_r {
-			label = "led_r";
-			gpios = <&gpio0 0x72 0x1>;
-			default-state = "on";
-			linux,default-trigger = "heartbeat";
-		};
-
-		led_g {
-			label = "led_g";
-			gpios = <&gpio0 0x73 0x1>;

arch/arm/dts/zynq-zturn-myir.dts line 63

Quoted 10 lines
arch/arm/dts/zynq-zturn-myir.dts +0 -10
-			default-state = "on";
-			linux,default-trigger = "heartbeat";
-		};
-
-		led_b {
-			label = "led_b";
-			gpios = <&gpio0 0x74 0x1>;
-			default-state = "on";
-			linux,default-trigger = "heartbeat";
-		};

Why remove them? They're hard wired on the board, no?

Quoted 15 lines
arch/arm/dts/zynq-zturn-myir.dts +4 -6
-
-		usr_led1 {
-			label = "usr_led1";
+		usr-led1 {
+			label = "usr-led1";
 			gpios = <&gpio0 0x0 0x1>;
 			default-state = "off";
-			linux,default-trigger = "none";
 		};
 
-		usr_led2 {
-			label = "usr_led2";
+		usr-led2 {
+			label = "usr-led2";
 			gpios = <&gpio0 0x9 0x1>;

arch/arm/dts/zynq-zturn-myir.dts line 84

Quoted 10 lines
arch/arm/dts/zynq-zturn-myir.dts +0 -6
 			default-state = "off";
-			linux,default-trigger = "none";
 		};
 	};
 
-	gpio-beep {
-		compatible = "gpio-beeper";
-		label = "pl-beep";
-		gpios = <&gpio0 0x75 0x0>;
-	};

This one is in PL, so ack.

arch/arm/dts/zynq-zturn-myir.dts line 52

Quoted 7 lines
arch/arm/dts/zynq-zturn-myir.dts +2 -3
-
 	gpio-keys {
 		compatible = "gpio-keys";
-		#address-cells = <0x1>;
-		#size-cells = <0x0>;
+		#address-cells = <1>;
+		#size-cells = <0>;

ack

arch/arm/dts/zynq-zturn-myir.dts line 103

Quoted 8 lines
arch/arm/dts/zynq-zturn-myir.dts +0 -1
 		autorepeat;
 		K1 {
 			label = "K1";

@@ -100,7 +63,6 @@
 
 &clkc {
 	ps-clk-frequency = <33333333>;
-	fclk-enable = <0xf>;

Why?

arch/arm/dts/zynq-zturn-myir.dts line 118

Quoted 11 lines
arch/arm/dts/zynq-zturn-myir.dts +2 -2
 };
 
 &qspi {

@@ -152,8 +114,8 @@
 		reg = <0x49>;
 	};
 
-	adxl345 at 53 {
-		compatible = "adi,adxl34x", "adxl34x";
+	accelerometer at 53 {
+		compatible = "adi,adxl345", "adxl345";

You can't just remove compatibles. Device trees are supposed to be compatible with whatever used them before someone thought they want to prettify them, so in this case you'd have to add the concrete names in the list before the abstract ones:

  compatible = "adi,adxl345", "adxl345", "adi,adxl34x", "adxl34x";

Alex

Quoted 4 lines
arch/arm/dts/zynq-zturn-myir.dts +0 -0
 		reg = <0x53>;
 		interrupt-parent = <&intc>;
 		interrupts = <0x0 0x1e 0x4>;
#3 lore raw

PATCH 1/2 ARM: dts: zynq: Update dts for Z-turn board

Quoted 54 lines · Alexander Graf
Quoted 29 lines · tossel at gmail.com

From: Anton Gerasimov <[email protected]>

Delete devices implemented in PL, stylistic changes.

Signed-off-by Anton Gerasimov <[email protected]>
---
 arch/arm/dts/zynq-zturn-myir.dts | 64 ++++++++--------------------------------
 1 file changed, 13 insertions(+), 51 deletions(-)
arch/arm/dts/zynq-zturn-myir.dts +1 -7
index a5ecfcc1d7..b6661d0205 100644
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  *  Copyright (C) 2015 Andrea Merello <[email protected]>
  *  Copyright (C) 2017 Alexander Graf <[email protected]>

@@ -6,87 +7,49 @@
  *  Copyright (C) 2011 - 2014 Xilinx
  *  Copyright (C) 2012 National Instruments Corp.
  *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

arch/arm/dts/zynq-zturn-myir.dts line 17

Quoted 10 lines
arch/arm/dts/zynq-zturn-myir.dts +2 -2
- * GNU General Public License for more details.
  */
+
 /dts-v1/;
 /include/ "zynq-7000.dtsi"
 
 / {
 	model = "Zynq Z-Turn MYIR Board";
-	compatible = "xlnx,zynq-7000";
+	compatible = "myir,zynq-zturn", "xlnx,zynq-7000";

ack.

Quoted 9 lines
arch/arm/dts/zynq-zturn-myir.dts +0 -2
 	aliases {
 		ethernet0 = &gem0;
 		serial0 = &uart1;
 		serial1 = &uart0;
-		spi0 = &qspi;
-		mmc0 = &sdhci0;
 	};
Quoted 4 lines

arch/arm/dts/zynq-zturn-myir.dts line 25

Quoted 2 lines
arch/arm/dts/zynq-zturn-myir.dts +1 -1
-	memory {
+	memory at 0 {

Why?

node with regs should have @number. DTC is checking that W=1.

Quoted 36 lines

arch/arm/dts/zynq-zturn-myir.dts line 39

Quoted 7 lines
arch/arm/dts/zynq-zturn-myir.dts +0 -1
 		device_type = "memory";
 		reg = <0x0 0x40000000>;
 	};
 
 	chosen {
-		stdout-path = "serial0:115200n8";

Nack. By default graphical output is quite unusable on this board, so we want to output to serial.

If your Linux submitted device tree doesn't contain this part, please fix it there.

arch/arm/dts/zynq-zturn-myir.dts line 31

Quoted 1 line
arch/arm/dts/zynq-zturn-myir.dts +1 -0
+		bootargs = "console=ttyPS0,115200 earlyprintk root=/dev/mmcblk0p2 rootwait";

This is even worse. Please don't prepopulate any bootargs, otherwise people may end up assuming that they're actually getting used.

Quoted 17 lines
arch/arm/dts/zynq-zturn-myir.dts +0 -13
 	};
 
 	gpio-leds {
 		compatible = "gpio-leds";
-		led_r {
-			label = "led_r";
-			gpios = <&gpio0 0x72 0x1>;
-			default-state = "on";
-			linux,default-trigger = "heartbeat";
-		};
-
-		led_g {
-			label = "led_g";
-			gpios = <&gpio0 0x73 0x1>;
-			default-state = "on";
-			linux,default-trigger = "heartbeat";
-		};
Quoted 8 lines

arch/arm/dts/zynq-zturn-myir.dts line 63

Quoted 7 lines
arch/arm/dts/zynq-zturn-myir.dts +0 -7
-
-		led_b {
-			label = "led_b";
-			gpios = <&gpio0 0x74 0x1>;
-			default-state = "on";
-			linux,default-trigger = "heartbeat";
-		};

Why remove them? They're hard wired on the board, no?

zynq has 54 MIO and all numbers above are routed via PL. It means this depends on PL.

Quoted 44 lines
Quoted 16 lines
arch/arm/dts/zynq-zturn-myir.dts +4 -6
-
-		usr_led1 {
-			label = "usr_led1";
+		usr-led1 {
+			label = "usr-led1";
 			gpios = <&gpio0 0x0 0x1>;
 			default-state = "off";
-			linux,default-trigger = "none";
 		};
 
-		usr_led2 {
-			label = "usr_led2";
+		usr-led2 {
+			label = "usr-led2";
 			gpios = <&gpio0 0x9 0x1>;

arch/arm/dts/zynq-zturn-myir.dts line 84

Quoted 10 lines
arch/arm/dts/zynq-zturn-myir.dts +0 -6
 			default-state = "off";
-			linux,default-trigger = "none";
 		};
 	};
 
-	gpio-beep {
-		compatible = "gpio-beeper";
-		label = "pl-beep";
-		gpios = <&gpio0 0x75 0x0>;
-	};

This one is in PL, so ack.

arch/arm/dts/zynq-zturn-myir.dts line 52

Quoted 7 lines
arch/arm/dts/zynq-zturn-myir.dts +2 -3
-
 	gpio-keys {
 		compatible = "gpio-keys";
-		#address-cells = <0x1>;
-		#size-cells = <0x0>;
+		#address-cells = <1>;
+		#size-cells = <0>;

ack

Quoted 5 lines
arch/arm/dts/zynq-zturn-myir.dts +0 -0
 		autorepeat;
 		K1 {
 			label = "K1";

@@ -100,7 +63,6 @@
Quoted 5 lines

arch/arm/dts/zynq-zturn-myir.dts line 103

Quoted 3 lines
arch/arm/dts/zynq-zturn-myir.dts +0 -1
 &clkc {
 	ps-clk-frequency = <33333333>;
-	fclk-enable = <0xf>;

Why?

fclk-enable is enabling clocks for PL. It exactly means enable all 4 clocks to PL which is PL dependent that's why it shouldn't be here.

Thanks,
MIchal

#4 lore raw

PATCH 1/2 ARM: dts: zynq: Update dts for Z-turn board

Quoted 144 lines · Michal Simek
Quoted 52 lines · Alexander Graf
Quoted 29 lines · tossel at gmail.com

From: Anton Gerasimov <[email protected]>

Delete devices implemented in PL, stylistic changes.

Signed-off-by Anton Gerasimov <[email protected]>
---
  arch/arm/dts/zynq-zturn-myir.dts | 64 ++++++++--------------------------------
  1 file changed, 13 insertions(+), 51 deletions(-)
arch/arm/dts/zynq-zturn-myir.dts +1 -7
index a5ecfcc1d7..b6661d0205 100644
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
  /*
   *  Copyright (C) 2015 Andrea Merello <[email protected]>
   *  Copyright (C) 2017 Alexander Graf <[email protected]>

@@ -6,87 +7,49 @@
   *  Copyright (C) 2011 - 2014 Xilinx
   *  Copyright (C) 2012 National Instruments Corp.
   *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

arch/arm/dts/zynq-zturn-myir.dts line 17

Quoted 10 lines
arch/arm/dts/zynq-zturn-myir.dts +2 -2
- * GNU General Public License for more details.
   */
+
  /dts-v1/;
  /include/ "zynq-7000.dtsi"
 
  / {
  	model = "Zynq Z-Turn MYIR Board";
-	compatible = "xlnx,zynq-7000";
+	compatible = "myir,zynq-zturn", "xlnx,zynq-7000";

ack.

Quoted 9 lines
arch/arm/dts/zynq-zturn-myir.dts +0 -2
  	aliases {
  		ethernet0 = &gem0;
  		serial0 = &uart1;
  		serial1 = &uart0;
-		spi0 = &qspi;
-		mmc0 = &sdhci0;
  	};
Quoted 3 lines

arch/arm/dts/zynq-zturn-myir.dts line 25

Quoted 2 lines
arch/arm/dts/zynq-zturn-myir.dts +1 -1
-	memory {
+	memory at 0 {

Why?

node with regs should have @number. DTC is checking that W=1.

Quoted 33 lines

arch/arm/dts/zynq-zturn-myir.dts line 39

Quoted 6 lines
arch/arm/dts/zynq-zturn-myir.dts +0 -1
  		device_type = "memory";
  		reg = <0x0 0x40000000>;
  	};
 
  	chosen {
-		stdout-path = "serial0:115200n8";

Nack. By default graphical output is quite unusable on this board, so we want to output to serial.

If your Linux submitted device tree doesn't contain this part, please fix it there.

arch/arm/dts/zynq-zturn-myir.dts line 31

Quoted 1 line
arch/arm/dts/zynq-zturn-myir.dts +1 -0
+		bootargs = "console=ttyPS0,115200 earlyprintk root=/dev/mmcblk0p2 rootwait";

This is even worse. Please don't prepopulate any bootargs, otherwise people may end up assuming that they're actually getting used.

Quoted 17 lines
arch/arm/dts/zynq-zturn-myir.dts +0 -13
  	};
 
  	gpio-leds {
  		compatible = "gpio-leds";
-		led_r {
-			label = "led_r";
-			gpios = <&gpio0 0x72 0x1>;
-			default-state = "on";
-			linux,default-trigger = "heartbeat";
-		};
-
-		led_g {
-			label = "led_g";
-			gpios = <&gpio0 0x73 0x1>;
-			default-state = "on";
-			linux,default-trigger = "heartbeat";
-		};
Quoted 8 lines

arch/arm/dts/zynq-zturn-myir.dts line 63

Quoted 7 lines
arch/arm/dts/zynq-zturn-myir.dts +0 -7
-
-		led_b {
-			label = "led_b";
-			gpios = <&gpio0 0x74 0x1>;
-			default-state = "on";
-			linux,default-trigger = "heartbeat";
-		};

Why remove them? They're hard wired on the board, no?

zynq has 54 MIO and all numbers above are routed via PL. It means this depends on PL.

Quoted 41 lines
Quoted 15 lines
arch/arm/dts/zynq-zturn-myir.dts +4 -6
-
-		usr_led1 {
-			label = "usr_led1";
+		usr-led1 {
+			label = "usr-led1";
  			gpios = <&gpio0 0x0 0x1>;
  			default-state = "off";
-			linux,default-trigger = "none";
  		};
 
-		usr_led2 {
-			label = "usr_led2";
+		usr-led2 {
+			label = "usr-led2";
  			gpios = <&gpio0 0x9 0x1>;

arch/arm/dts/zynq-zturn-myir.dts line 84

Quoted 10 lines
arch/arm/dts/zynq-zturn-myir.dts +0 -6
  			default-state = "off";
-			linux,default-trigger = "none";
  		};
  	};
 
-	gpio-beep {
-		compatible = "gpio-beeper";
-		label = "pl-beep";
-		gpios = <&gpio0 0x75 0x0>;
-	};

This one is in PL, so ack.

arch/arm/dts/zynq-zturn-myir.dts line 52

Quoted 7 lines
arch/arm/dts/zynq-zturn-myir.dts +2 -3
-
  	gpio-keys {
  		compatible = "gpio-keys";
-		#address-cells = <0x1>;
-		#size-cells = <0x0>;
+		#address-cells = <1>;
+		#size-cells = <0>;

ack

Quoted 5 lines
arch/arm/dts/zynq-zturn-myir.dts +0 -0
  		autorepeat;
  		K1 {
  			label = "K1";

@@ -100,7 +63,6 @@
Quoted 6 lines
Quoted 4 lines

arch/arm/dts/zynq-zturn-myir.dts line 103

Quoted 3 lines
arch/arm/dts/zynq-zturn-myir.dts +0 -1
  &clkc {
  	ps-clk-frequency = <33333333>;
-	fclk-enable = <0xf>;

Why?

fclk-enable is enabling clocks for PL. It exactly means enable all 4 clocks to PL which is PL dependent that's why it shouldn't be here.

IIRC on my Z-Turn, I had to take a PL clock back into the PS as AXI reference clock. See "Connect clocks" here:

https://wiki.hackerspace.pl/projects:zturn-hackers:helloworld

So we need to have the PL clock enabled, no? Or is that only needed for PL AXI peripherals?

Alex

author
#5 lore raw

PATCH 1/2 ARM: dts: zynq: Update dts for Z-turn board

In reply to Alexander Graf

Hi Alexander,

Quoted 9 lines

arch/arm/dts/zynq-zturn-myir.dts line 39

Quoted 6 lines
  		device_type = "memory";
  		reg = <0x0 0x40000000>;
  	};
  
  	chosen {
arch/arm/dts/zynq-zturn-myir.dts +0 -1
-		stdout-path = "serial0:115200n8";

Nack. By default graphical output is quite unusable on this board, so we want to output to serial.

Quoted 6 lines

If your Linux submitted device tree doesn't contain this part, please fix it there.

arch/arm/dts/zynq-zturn-myir.dts line 31

Quoted 1 line
arch/arm/dts/zynq-zturn-myir.dts +1 -0
+		bootargs = "console=ttyPS0,115200 earlyprintk root=/dev/mmcblk0p2 rootwait";

This is even worse. Please don't prepopulate any bootargs, otherwise people may end up assuming that they're actually getting used.

The older one is much cleaner I agree, thank you. I'll just need to test with both u-boot and linux.

Quoted 17 lines
Quoted 17 lines
  	};
  
  	gpio-leds {
  		compatible = "gpio-leds";
arch/arm/dts/zynq-zturn-myir.dts +0 -13
-		led_r {
-			label = "led_r";
-			gpios = <&gpio0 0x72 0x1>;
-			default-state = "on";
-			linux,default-trigger = "heartbeat";
-		};
-
-		led_g {
-			label = "led_g";
-			gpios = <&gpio0 0x73 0x1>;
-			default-state = "on";
-			linux,default-trigger = "heartbeat";
-		};
Quoted 8 lines

arch/arm/dts/zynq-zturn-myir.dts line 63

Quoted 7 lines
arch/arm/dts/zynq-zturn-myir.dts +0 -7
-
-		led_b {
-			label = "led_b";
-			gpios = <&gpio0 0x74 0x1>;
-			default-state = "on";
-			linux,default-trigger = "heartbeat";
-		};

Why remove them? They're hard wired on the board, no?

No this RGB LED is connected to PL, that's for sure.

Quoted 9 lines

arch/arm/dts/zynq-zturn-myir.dts line 103

Quoted 2 lines
  	ps-clk-frequency = <33333333>;
arch/arm/dts/zynq-zturn-myir.dts +0 -1
-	fclk-enable = <0xf>;

Why?

IIRC on my Z-Turn, I had to take a PL clock back into the PS as AXI reference clock. See "Connect clocks" here:

Quoted 7 lines

https://wiki.hackerspace.pl/projects:zturn-hackers:helloworld

So we need to have the PL clock enabled, no? Or is that only needed for PL AXI peripherals?

As far as I understand M_AXI_GPn connects AXI slaves implemented on PL to AXI and S_AXI_HPn provides access for AXI slaves in PS to PL slaves. So both involve PL and can be disconnected if PL is not clocked.

Quoted 16 lines

arch/arm/dts/zynq-zturn-myir.dts line 118

Quoted 11 lines
  };
  
  &qspi {
arch/arm/dts/zynq-zturn-myir.dts +2 -2
@@ -152,8 +114,8 @@
  		reg = <0x49>;
  	};
 
-	adxl345 at 53 {
-		compatible = "adi,adxl34x", "adxl34x";
+	accelerometer at 53 {
+		compatible = "adi,adxl345", "adxl345";

You can't just remove compatibles. Device trees are supposed to be compatible with whatever used them before someone thought they want to prettify them, so in this case you'd have to add the concrete names in the list before the abstract ones:

Quoted 1 line
   compatible = "adi,adxl345", "adxl345", "adi,adxl34x", "adxl34x";

Yes sorry, I didn't realize that Linux kernel had both.

Best,
Anton

#6 lore raw

PATCH 1/2 ARM: dts: zynq: Update dts for Z-turn board

Quoted 61 lines · Anton Gerasimov

Hi Alexander,

Quoted 8 lines
Quoted 5 lines

          device_type = "memory";
          reg = <0x0 0x40000000>;
      };
        chosen {
-        stdout-path = "serial0:115200n8";

Nack. By default graphical output is quite unusable on this board, so we want to output to serial.

Quoted 7 lines

If your Linux submitted device tree doesn't contain this part, please fix it there.

Quoted 2 lines

+        bootargs = "console=ttyPS0,115200 earlyprintk root=/dev/mmcblk0p2 rootwait";

This is even worse. Please don't prepopulate any bootargs, otherwise people may end up assuming that they're actually getting used.

The older one is much cleaner I agree, thank you. I'll just need to test with both u-boot and linux.

Quoted 16 lines
Quoted 16 lines

      };
        gpio-leds {
          compatible = "gpio-leds";
-        led_r {
-            label = "led_r";
-            gpios = <&gpio0 0x72 0x1>;
-            default-state = "on";
-            linux,default-trigger = "heartbeat";
-        };
-
-        led_g {
-            label = "led_g";
-            gpios = <&gpio0 0x73 0x1>;
-            default-state = "on";
-            linux,default-trigger = "heartbeat";
-        };

Quoted 8 lines
Quoted 7 lines

-
-        led_b {
-            label = "led_b";
-            gpios = <&gpio0 0x74 0x1>;
-            default-state = "on";
-            linux,default-trigger = "heartbeat";
-        };

Why remove them? They're hard wired on the board, no?

No this RGB LED is connected to PL, that's for sure.

Quoted 13 lines
Quoted 2 lines

      ps-clk-frequency = <33333333>;
-    fclk-enable = <0xf>;

Why?

IIRC on my Z-Turn, I had to take a PL clock back into the PS as AXI reference clock. See "Connect clocks" here:

   https://wiki.hackerspace.pl/projects:zturn-hackers:helloworld

Quoted 7 lines
Quoted 3 lines

So we need to have the PL clock enabled, no? Or is that only needed for PL AXI peripherals?

As far as I understand M_AXI_GPn connects AXI slaves implemented on PL to AXI and S_AXI_HPn provides access for AXI slaves in PS to PL slaves. So both involve PL and can be disconnected if PL is not clocked.

As you see that connection is done via PL. I am booting zynq boards without fclk-enable and without PL and they are booting properly. I have never had a time to investing what it is broken if you don't load bitstream. Definitely feel free to test.
And as you see only one clk is used not all 4.

Quoted 15 lines
Quoted 15 lines

arch/arm/dts/zynq-zturn-myir.dts line 118

Quoted 10 lines

  };
    &qspi {

arch/arm/dts/zynq-zturn-myir.dts +2 -1
@@ -152,8 +114,8 @@
           reg = <0x49>;
       };
   -    adxl345 at 53 {
-        compatible = "adi,adxl34x", "adxl34x";
+    accelerometer at 53 {
+        compatible = "adi,adxl345", "adxl345";

You can't just remove compatibles. Device trees are supposed to be compatible with whatever used them before someone thought they want to prettify them, so in this case you'd have to add the concrete names in the list before the abstract ones:

Quoted 2 lines
Quoted 1 line

   compatible = "adi,adxl345", "adxl345", "adi,adxl34x", "adxl34x";

Yes sorry, I didn't realize that Linux kernel had both.

Ok. I think it will be good to sync these two dts with the kernel. It means can you please send new updated version for linux kernel too?

Thanks,
Michal

author
#7 lore raw

PATCH 2/2 ARM: dts: zynq: Rename dts for Z-turn board

In reply to tossel at gmail.com

From: Anton Gerasimov <[email protected]>

Makes naming in line with other Zynq boards.

Signed-off-by Anton Gerasimov <[email protected]>
---
 arch/arm/dts/Makefile                                | 2 +-
 arch/arm/dts/{zynq-zturn-myir.dts => zynq-zturn.dts} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename arch/arm/dts/{zynq-zturn-myir.dts => zynq-zturn.dts} (100%)
arch/arm/dts/Makefile +1 -1
index 20a4c37d48..83aa2f4df4 100644
@@ -143,7 +143,7 @@ dtb-$(CONFIG_ARCH_ZYNQ) += \
 	zynq-zc770-xm012.dtb \
 	zynq-zc770-xm013.dtb \
 	zynq-zed.dtb \
-	zynq-zturn-myir.dtb \
+	zynq-zturn.dtb \
 	zynq-zybo.dtb
 dtb-$(CONFIG_ARCH_ZYNQMP) += \
 	zynqmp-ep108.dtb			\
arch/arm/dts/zynq-zturn.dts renamed from arch/arm/dts/zynq-zturn-myir.dts +0 -0
similarity index 100%
rename from arch/arm/dts/zynq-zturn-myir.dts
rename to arch/arm/dts/zynq-zturn.dts
Signature · 1 line
2.16.2