Hi Jiancheng, Am 09.02.2017 um 08:07 schrieb Jiancheng Xue: > Add basic dts files for hi3798cv200-poplar board. Poplar is the > first development board compliant with the 96Boards Enterprise > Edition TV Platform specification. The board features the > Hi3798CV200 with an integrated quad-core 64-bit ARM Cortex A53 > processor and high performance Mali T720 GPU. > > Signed-off-by: Jiancheng Xue <xuejiancheng@xxxxxxxxxxxxx> > Reviewed-by: Alex Elder <elder@xxxxxxxxxx> Thanks for this patch! Some comments below. Do you have instructions for how to test? In my tries I so far only got resets like for example: fastboot# bootm 0x01000000 - 0x20000000 ## Booting kernel from Legacy Image at 01000000 ... Image Name: linux-next Image Type: AArch64 Linux Kernel Image (uncompressed) Data Size: 8741376 Bytes = 8.3 MiB Load Address: 02000000 Entry Point: 02000000 ## Flattened Device Tree blob at 20000000 Booting using the fdt blob at 0x20000000 Loading Kernel Image from 0x16777280 to 0x33554432 ... OK OK Starting kernel ... *** irq: undefined instruction undefined instruction pc : [<600001d3>] lr : [<00c661ec>] sp : 00bfffb8 ip : 00000036 fp : 00000000 r10: 00000000 r9 : 00000000 r8 : 00000000 r7 : 00000080 r6 : 005fffc4 r5 : f36e6f75 r4 : 00000000 r3 : 0000001e r2 : 00000100 r1 : 00000000 r0 : 00000000 Flags: nzcv IRQs off FIQs off Mode UK12_32 Resetting CPU ... resetting ... Does U-Boot need to be updated for this to work? > diff --git a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt > index 7df79a7..7d90bf1 100644 > --- a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt > +++ b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt > @@ -1,5 +1,9 @@ > Hisilicon Platforms Device Tree Bindings > ---------------------------------------------------- > +Hi3798cv200 Poplar Board > +Required root node properties: > + - compatible = "hisilicon,hi3798cv200-poplar", "hisilicon,hi3798cv200"; > + Shouldn't this rather be "tocoding,poplar", "hisilicon,hi3798cv200"? linux-next.git already has Hi3660 here, so you'll need to rebase. Also, theoretically bindings documentation should be in a separate, preceding patch. > Hi4511 Board > Required root node properties: > - compatible = "hisilicon,hi3620-hi4511"; > diff --git a/arch/arm64/boot/dts/hisilicon/Makefile b/arch/arm64/boot/dts/hisilicon/Makefile > index c8b8f80..96202fe 100644 > --- a/arch/arm64/boot/dts/hisilicon/Makefile > +++ b/arch/arm64/boot/dts/hisilicon/Makefile > @@ -2,6 +2,7 @@ dtb-$(CONFIG_ARCH_HISI) += hi6220-hikey.dtb > dtb-$(CONFIG_ARCH_HISI) += hip05-d02.dtb > dtb-$(CONFIG_ARCH_HISI) += hip06-d03.dtb > dtb-$(CONFIG_ARCH_HISI) += hip07-d05.dtb > +dtb-$(CONFIG_ARCH_HISI) += hi3798cv200-poplar.dtb Please keep this sorted alphabetically. > > always := $(dtb-y) > subdir-y := $(dts-dirs) > diff --git a/arch/arm64/boot/dts/hisilicon/hi3798cv200-poplar.dts b/arch/arm64/boot/dts/hisilicon/hi3798cv200-poplar.dts > new file mode 100644 > index 0000000..4e2b1d1 > --- /dev/null > +++ b/arch/arm64/boot/dts/hisilicon/hi3798cv200-poplar.dts > @@ -0,0 +1,169 @@ > +/* > + * DTS File for HiSilicon Poplar Development Board > + * > + * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * 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. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see <http://www.gnu.org/licenses/>. > + */ > + > +/dts-v1/; > + > +#include <dt-bindings/gpio/gpio.h> > +#include "hi3798cv200.dtsi" > + > +/ { > + model = "HiSilicon Poplar Development Board"; Ditto: HiSilicon? > + compatible = "hisilicon,hi3798cv200-poplar", "hisilicon,hi3798cv200"; > + > + aliases { > + serial0 = &uart0; > + }; > + > + chosen { > + stdout-path = "serial0:115200n8"; > + }; > + > + memory { > + device_type = "memory"; > + reg = <0x000000000 0x00000000 0x00000000 0x80000000>; First one has one zero too much. > + }; > + > + soc { [...] > + }; > +}; > + > +&uart0 { > + status = "ok"; I believe the canonical form is "okay". > +}; > + > +&uart2 { > + status = "ok"; > + label = "LS-UART0"; > +}; > +/* No optional LS-UART1 on Low Speed Expansion Connector. */ > + > +&i2c0 { > + status = "ok"; > + label = "LS-I2C0"; > +}; > + > +&i2c2 { > + status = "ok"; > + label = "LS-I2C1"; > +}; > + > +&spi0 { > + status = "ok"; > + label = "LS-SPI0"; > +}; > + > +&gpio1 { > + status = "ok"; > + gpio-line-names = "LS-GPIO-E", "", > + "", "", > + "", "LS-GPIO-F", > + "", "LS-GPIO-J"; > +}; > + > +&gpio2 { > + status = "ok"; > + gpio-line-names = "LS-GPIO-H", "LS-GPIO-I", > + "LS-GPIO-L", "LS-GPIO-G", > + "LS-GPIO-K", "", > + "", ""; > +}; > + > +&gpio3 { > + status = "ok"; > + gpio-line-names = "", "", > + "", "", > + "LS-GPIO-C", "", > + "", "LS-GPIO-B"; > +}; > + > +&gpio4 { > + status = "ok"; > + gpio-line-names = "", "", > + "", "", > + "", "LS-GPIO-D", > + "", ""; > +}; > + > +&gpio5 { > + status = "ok"; > + gpio-line-names = "", "USER-LED-1", > + "USER-LED-2", "", > + "", "LS-GPIO-A", > + "", ""; > +}; > + > +&gpio6 { > + status = "ok"; > + gpio-line-names = "", "", > + "", "USER-LED-0", > + "", "", > + "", ""; > +}; > + > +&gpio10 { > + status = "ok"; > + gpio-line-names = "", "", > + "", "", > + "", "", > + "USER-LED-3", ""; > +}; > + > +&gmac0 { > + #address-cells = <1>; > + #size-cells = <0>; > + phy-handle = <ð_phy1>; > + phy-mode = "rgmii"; > + hisilicon,phy-reset-delays-us = <10000 10000 30000>; > + status = "ok"; Move this to the top for consistency? > + > + eth_phy1: phy@3{ Space before brace missing. > + reg = <3>; > + }; > +}; > diff --git a/arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi b/arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi > new file mode 100644 > index 0000000..ae3ce6d > --- /dev/null > +++ b/arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi > @@ -0,0 +1,413 @@ > +/* > + * DTS File for HiSilicon Hi3798cv200 SOC. "SoC"? > + * > + * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * 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. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see <http://www.gnu.org/licenses/>. > + */ > + > +#include <dt-bindings/interrupt-controller/arm-gic.h> > +#include <dt-bindings/clock/histb-clock.h> > +#include <dt-bindings/reset/ti-syscon.h> Sort alphabetically? > + > +/ { > + compatible = "hisilicon,hi3798cv200"; > + interrupt-parent = <&gic>; > + #address-cells = <2>; > + #size-cells = <2>; > + > + psci { > + compatible = "arm,psci-0.2"; > + method = "smc"; > + }; > + > + cpus { > + #address-cells = <2>; > + #size-cells = <0>; > + > + cpu@0 { > + compatible = "arm,cortex-a53"; > + device_type = "cpu"; Nit: Elsewhere I've seen device_type before compatible? > + reg = <0x0 0x0>; > + enable-method = "psci"; > + }; > + > + cpu@1 { > + compatible = "arm,cortex-a53"; > + device_type = "cpu"; > + reg = <0x0 0x1>; > + enable-method = "psci"; > + }; > + > + cpu@2 { > + compatible = "arm,cortex-a53"; > + device_type = "cpu"; > + reg = <0x0 0x2>; > + enable-method = "psci"; > + }; > + > + cpu@3 { > + compatible = "arm,cortex-a53"; > + device_type = "cpu"; > + reg = <0x0 0x3>; > + enable-method = "psci"; > + }; > + }; > + > + gic: interrupt-controller@f1001000 { > + compatible = "arm,gic-400"; > + reg = <0x0 0xf1001000 0x0 0x1000>, /*GICD*/ > + <0x0 0xf1002000 0x0 0x100>; /*GICC*/ Please leave spaces inside the comments for readability. Are you sure about the size of the second region? And are there really only two, not four? (1k, 2k, 2k, 2k elsewhere [0]) [0] http://lists.infradead.org/pipermail/linux-arm-kernel/2017-January/480590.html > + #address-cells = <0>; Unneeded since there are no subnodes? > + #interrupt-cells = <3>; > + interrupt-controller; No interrupts property with PPI 9 here? > + }; > + > + timer { > + compatible = "arm,armv8-timer"; > + interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, > + <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, > + <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, > + <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>; > + }; > + > + soc { > + compatible = "simple-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges = <0x0 0x00000000 0x0 0xffffffff>; ranges = <0x0 0x0 0x00000000 0xffffffff>? (addr, parent addr, size) > + > + crg: clock-reset-controller@f8a22000 { > + compatible = "hisilicon,hi3798cv200-crg", "simple-mfd"; > + reg = <0xf8a22000 0x1000>; > + #clock-cells = <1>; > + #reset-cells = <2>; > + > + gmacphyrst: reset-controller { > + compatible = "ti,syscon-reset"; > + reset-cells = <1>; > + ti,reset-bits = < > + 0xcc 12 0xcc 12 0 0 (ASSERT_CLEAR|DEASSERT_SET|STATUS_NONE) /* 0: gmac0-phy-rst */ > + 0xcc 13 0xcc 13 0 0 (ASSERT_CLEAR|DEASSERT_SET|STATUS_NONE) /* 1: gmac1-phy-rst */ > + >; Use <>, <> tuple syntax for the two lines instead? > + }; > + }; [snip] Regards, Andreas -- SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
