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

[Alexander Graf]

On 03/14/2018 02:05 PM, Michal Simek wrote:
> 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.

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