[U-Boot] [PATCH 1/2] ARM: dts: zynq: Update dts for Z-turn board

[Michal Simek]

On 13.3.2018 22:21, Alexander Graf wrote:
> 
> 
> On 13.03.18 21:35, tossel at gmail.com wrote:
>> From: Anton Gerasimov <tossel at gmail.com>
>>
>> Delete devices implemented in PL, stylistic changes.
>>
>> Signed-off-by: Anton Gerasimov <tossel at gmail.com>
>> ---
>>  arch/arm/dts/zynq-zturn-myir.dts | 64 ++++++++--------------------------------
>>  1 file changed, 13 insertions(+), 51 deletions(-)
>>
>> diff --git a/arch/arm/dts/zynq-zturn-myir.dts b/arch/arm/dts/zynq-zturn-myir.dts
>> index a5ecfcc1d7..b6661d0205 100644
>> --- a/arch/arm/dts/zynq-zturn-myir.dts
>> +++ b/arch/arm/dts/zynq-zturn-myir.dts
>> @@ -1,3 +1,4 @@
>> +// SPDX-License-Identifier: GPL-2.0
>>  /*
>>   *  Copyright (C) 2015 Andrea Merello <adnrea.merello at gmail.com>
>>   *  Copyright (C) 2017 Alexander Graf <agraf at suse.de>
>> @@ -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";
> 
> ack.
> 
>>  
>>  	aliases {
>>  		ethernet0 = &gem0;
>>  		serial0 = &uart1;
>>  		serial1 = &uart0;
>> -		spi0 = &qspi;
>> -		mmc0 = &sdhci0;
>>  	};
>>  
>> -	memory {
>> +	memory at 0 {
> 
> Why?

node with regs should have @number. DTC is checking that W=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.
> 
>> +		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.
> 
>>  	};
>>  
>>  	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";
>> -		};
> 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.


> 
>> -
>> -		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>;
>> -	};
> 
> This one is in PL, so ack.
> 
>> -
>>  	gpio-keys {
>>  		compatible = "gpio-keys";
>> -		#address-cells = <0x1>;
>> -		#size-cells = <0x0>;
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
> 
> ack
> 
>>  		autorepeat;
>>  		K1 {
>>  			label = "K1";
>> @@ -100,7 +63,6 @@
>>  
>>  &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