Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

From: Jens Kuske
Date: Wed May 06 2015 - 16:47:42 EST


Hi,

On 06/05/15 14:19, Maxime Ripard wrote:
> On Wed, May 06, 2015 at 11:31:32AM +0200, Jens Kuske wrote:
>> The Allwinner H3 is a home entertainment system oriented SoC with
>> four Cortex-A7 cores and a Mali-400MP2 GPU.
>>
>> Signed-off-by: Jens Kuske <jenskuske@xxxxxxxxx>
>> ---
>> arch/arm/boot/dts/sun8i-h3.dtsi | 468 ++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 468 insertions(+)
>> create mode 100644 arch/arm/boot/dts/sun8i-h3.dtsi
>>
>> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
>> new file mode 100644
>> index 0000000..53aab95
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
>> @@ -0,0 +1,468 @@
>> +/*
>> + * Copyright (C) 2015 Jens Kuske <jenskuske@xxxxxxxxx>
>> + *
>> + * This file is dual-licensed: you can use it either under the terms
>> + * of the GPL or the X11 license, at your option. Note that this dual
>> + * licensing only applies to this file, and not this project as a
>> + * whole.
>> + *
>> + * a) This file 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 file 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 file; if not, write to the Free
>> + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
>> + * MA 02110-1301 USA
>
> Could you remove that last paragraph?
> It generates a checkpatch warning.

Sure, will be removed. Just copied it from some other sunxi dtsi.

>
>> + *
>> + * Or, alternatively,
>> + *
>> + * b) Permission is hereby granted, free of charge, to any person
>> + * obtaining a copy of this software and associated documentation
>> + * files (the "Software"), to deal in the Software without
>> + * restriction, including without limitation the rights to use,
>> + * copy, modify, merge, publish, distribute, sublicense, and/or
>> + * sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following
>> + * conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be
>> + * included in all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
>> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
>> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
>> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> + * OTHER DEALINGS IN THE SOFTWARE.
>> + */
>> +
>> +#include "skeleton.dtsi"
>> +
>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>> +#include <dt-bindings/pinctrl/sun4i-a10.h>
>> +
>> +/ {
>> + interrupt-parent = <&gic>;
>> +
>> + cpus {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + cpu@0 {
>> + compatible = "arm,cortex-a7";
>> + device_type = "cpu";
>> + reg = <0>;
>> + };
>> +
>> + cpu@1 {
>> + compatible = "arm,cortex-a7";
>> + device_type = "cpu";
>> + reg = <1>;
>> + };
>> +
>> + cpu@2 {
>> + compatible = "arm,cortex-a7";
>> + device_type = "cpu";
>> + reg = <2>;
>> + };
>> +
>> + cpu@3 {
>> + compatible = "arm,cortex-a7";
>> + device_type = "cpu";
>> + reg = <3>;
>> + };
>> + };
>> +
>> + memory {
>> + reg = <0x40000000 0x80000000>;
>> + };
>> +
>> + clocks {
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + ranges;
>> +
>> + osc24M: osc24M_clk {
>> + #clock-cells = <0>;
>> + compatible = "fixed-clock";
>> + clock-frequency = <24000000>;
>> + clock-output-names = "osc24M";
>> + };
>> +
>> + osc32k: osc32k_clk {
>> + #clock-cells = <0>;
>> + compatible = "fixed-clock";
>> + clock-frequency = <32768>;
>> + clock-output-names = "osc32k";
>> + };
>> +
>> + pll1: clk@01c20000 {
>> + #clock-cells = <0>;
>> + compatible = "allwinner,sun8i-a23-pll1-clk";
>> + reg = <0x01c20000 0x4>;
>> + clocks = <&osc24M>;
>> + clock-output-names = "pll1";
>> + };
>> +
>> + pll6: clk@01c20028 {
>> + #clock-cells = <1>;
>> + compatible = "allwinner,sun8i-h3-pll6-clk";
>> + reg = <0x01c20028 0x4>;
>> + clocks = <&osc24M>;
>> + clock-output-names = "pll6", "pll6x2", "pll6d2";
>> + };
>> +
>> + pll8: clk@01c20044 {
>> + #clock-cells = <0>;
>> + compatible = "allwinner,sun8i-h3-pll8-clk";
>> + reg = <0x01c20044 0x4>;
>> + clocks = <&osc24M>;
>> + clock-output-names = "pll8";
>> + };
>> +
>> + cpu: cpu_clk@01c20050 {
>> + #clock-cells = <0>;
>> + compatible = "allwinner,sun4i-a10-cpu-clk";
>> + reg = <0x01c20050 0x4>;
>> + clocks = <&osc32k>, <&osc24M>, <&pll1>, <&pll1>;
>> + clock-output-names = "cpu";
>> + };
>> +
>> + axi: axi_clk@01c20050 {
>> + #clock-cells = <0>;
>> + compatible = "allwinner,sun4i-a10-axi-clk";
>> + reg = <0x01c20050 0x4>;
>> + clocks = <&cpu>;
>> + clock-output-names = "axi";
>> + };
>> +
>> + ahb1: ahb1_clk@01c20054 {
>> + #clock-cells = <0>;
>> + compatible = "allwinner,sun6i-a31-ahb1-clk";
>> + reg = <0x01c20054 0x4>;
>> + clocks = <&osc32k>, <&osc24M>, <&axi>, <&pll6 0>;
>> + clock-output-names = "ahb1";
>> + };
>> +
>> + ahb2: ahb2_clk@01c2005c {
>> + #clock-cells = <0>;
>> + compatible = "allwinner,sun8i-h3-ahb2-clk";
>> + reg = <0x01c2005c 0x4>;
>> + clocks = <&ahb1>, <&pll6 2>;
>> + clock-output-names = "ahb2";
>> + };
>> +
>> + apb1: apb1_clk@01c20054 {
>> + #clock-cells = <0>;
>> + compatible = "allwinner,sun4i-a10-apb0-clk";
>> + reg = <0x01c20054 0x4>;
>> + clocks = <&ahb1>;
>> + clock-output-names = "apb1";
>> + };
>> +
>> + apb2: apb2_clk@01c20058 {
>> + #clock-cells = <0>;
>> + compatible = "allwinner,sun4i-a10-apb1-clk";
>> + reg = <0x01c20058 0x4>;
>> + clocks = <&osc32k>, <&osc24M>, <&pll6 0>, <&pll6 0>;
>> + clock-output-names = "apb2";
>> + };
>> +
>> + ahb1_gates: ahb1_gates_clk@01c20060 {
>> + #clock-cells = <1>;
>> + compatible = "allwinner,sun8i-h3-ahb1-gates-clk";
>> + reg = <0x01c20060 0x14>;
>> + clocks = <&ahb1>;
>> + clock-output-names = "ahb1_ce", "ahb1_dma", "ahb1_mmc0",
>> + "ahb1_mmc1", "ahb1_mmc2", "ahb1_nand",
>> + "ahb1_sdram", "ahb1_ts", "ahb1_hstimer",
>> + "ahb1_spi0", "ahb1_spi1", "ahb1_otg",
>> + "ahb1_otg_ehci0", "ahb1_ehic1",
>> + "ahb1_ehic2", "ahb1_ehic3",
>> + "ahb1_otg_ohci0", "ahb1_ve",
>> + "ahb1_lcd0", "ahb1_lcd1", "ahb1_deint",
>> + "ahb1_csi", "ahb1_tve", "ahb1_hdmi",
>> + "ahb1_de", "ahb1_gpu", "ahb1_msgbox",
>> + "ahb1_spinlock", "ahb1_ephy", "ahb1_dbg";
>> + };
>> +
>> + ahb2_gates: ahb2_gates_clk@01c20060 {
>> + #clock-cells = <1>;
>> + compatible = "allwinner,sun8i-h3-ahb2-gates-clk";
>> + reg = <0x01c20060 0x4>;
>> + clocks = <&ahb2>;
>> + clock-output-names = "ahb2_gmac", "ahb2_ohic1",
>> + "ahb2_ohic2", "ahb2_ohic3";
>> + };
>> +
>> + apb1_gates: clk@01c20068 {
>> + #clock-cells = <1>;
>> + compatible = "allwinner,sun8i-h3-apb1-gates-clk";
>> + reg = <0x01c20068 0x4>;
>> + clocks = <&apb1>;
>> + clock-output-names = "apb1_codec", "apb1_spdif",
>> + "apb1_pio", "apb1_ths", "apb1_i2s0",
>> + "apb1_i2s1", "apb1_i2s2";
>> + };
>> +
>> + apb2_gates: clk@01c2006c {
>> + #clock-cells = <1>;
>> + compatible = "allwinner,sun8i-h3-apb2-gates-clk";
>> + reg = <0x01c2006c 0x4>;
>> + clocks = <&apb2>;
>> + clock-output-names = "apb2_i2c0", "apb2_i2c1",
>> + "apb2_i2c2", "apb2_uart0",
>> + "apb2_uart1", "apb2_uart2",
>> + "apb2_uart3", "apb2_sim";
>
> I'd prefer if the clocks on a new line were right-aligned (like you
> did for the mmc clocks just below).

Copied again, but yes, I'll fix them. And apb2_sim gets renamed to
apb2_scr as mentioned in the pinctrl thread.

>
>> + };
>> +
>> + mmc0_clk: clk@01c20088 {
>> + #clock-cells = <1>;
>> + compatible = "allwinner,sun4i-a10-mmc-clk";
>> + reg = <0x01c20088 0x4>;
>> + clocks = <&osc24M>, <&pll6 0>, <&pll8>;
>> + clock-output-names = "mmc0",
>> + "mmc0_output",
>> + "mmc0_sample";
>> + };
>> +
>> + mmc1_clk: clk@01c2008c {
>> + #clock-cells = <1>;
>> + compatible = "allwinner,sun4i-a10-mmc-clk";
>> + reg = <0x01c2008c 0x4>;
>> + clocks = <&osc24M>, <&pll6 0>, <&pll8>;
>> + clock-output-names = "mmc1",
>> + "mmc1_output",
>> + "mmc1_sample";
>> + };
>> +
>> + mmc2_clk: clk@01c20090 {
>> + #clock-cells = <1>;
>> + compatible = "allwinner,sun4i-a10-mmc-clk";
>> + reg = <0x01c20090 0x4>;
>> + clocks = <&osc24M>, <&pll6 0>, <&pll8>;
>> + clock-output-names = "mmc2",
>> + "mmc2_output",
>> + "mmc2_sample";
>> + };
>> + };
>> +
>> + soc@01c00000 {
>> + compatible = "simple-bus";
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + ranges;
>> +
>> + dma: dma-controller@01c02000 {
>> + compatible = "allwinner,sun8i-h3-dma";
>> + reg = <0x01c02000 0x1000>;
>> + interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
>> + clocks = <&ahb1_gates 6>;
>> + resets = <&ahb12_rst 6>;
>> + #dma-cells = <1>;
>> + };
>> +
>> + mmc0: mmc@01c0f000 {
>> + compatible = "allwinner,sun5i-a13-mmc";
>> + reg = <0x01c0f000 0x1000>;
>> + clocks = <&ahb1_gates 8>,
>> + <&mmc0_clk 0>,
>> + <&mmc0_clk 1>,
>> + <&mmc0_clk 2>;
>> + clock-names = "ahb",
>> + "mmc",
>> + "output",
>> + "sample";
>> + resets = <&ahb12_rst 8>;
>> + reset-names = "ahb";
>> + interrupts = <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
>> + status = "disabled";
>> + };
>> +
>> + mmc1: mmc@01c10000 {
>> + compatible = "allwinner,sun5i-a13-mmc";
>> + reg = <0x01c10000 0x1000>;
>> + clocks = <&ahb1_gates 9>,
>> + <&mmc1_clk 0>,
>> + <&mmc1_clk 1>,
>> + <&mmc1_clk 2>;
>> + clock-names = "ahb",
>> + "mmc",
>> + "output",
>> + "sample";
>> + resets = <&ahb12_rst 9>;
>> + reset-names = "ahb";
>> + interrupts = <GIC_SPI 61 IRQ_TYPE_LEVEL_HIGH>;
>> + status = "disabled";
>> + };
>> +
>> + mmc2: mmc@01c11000 {
>> + compatible = "allwinner,sun5i-a13-mmc";
>> + reg = <0x01c11000 0x1000>;
>> + clocks = <&ahb1_gates 10>,
>> + <&mmc2_clk 0>,
>> + <&mmc2_clk 1>,
>> + <&mmc2_clk 2>;
>> + clock-names = "ahb",
>> + "mmc",
>> + "output",
>> + "sample";
>> + resets = <&ahb12_rst 10>;
>> + reset-names = "ahb";
>> + interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
>> + status = "disabled";
>> + };
>> +
>> + pio: pinctrl@01c20800 {
>> + compatible = "allwinner,sun8i-h3-pinctrl";
>> + reg = <0x01c20800 0x400>;
>> + interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
>> + clocks = <&apb1_gates 5>;
>> + gpio-controller;
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + #size-cells = <0>;
>> + #gpio-cells = <3>;
>> +
>> + uart0_pins_a: uart0@0 {
>> + allwinner,pins = "PA4", "PA5";
>> + allwinner,function = "uart0";
>> + allwinner,drive = <SUN4I_PINCTRL_10_MA>;
>> + allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
>> + };
>> +
>> + mmc0_pins_a: mmc0@0 {
>> + allwinner,pins = "PF0","PF1","PF2","PF3","PF4","PF5";
>
> Could you have spaces between the commas, and wrap the line at 80
> chars?

ok
>
>> + allwinner,function = "mmc0";
>> + allwinner,drive = <SUN4I_PINCTRL_30_MA>;
>> + allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
>> + };
>> +
>> + mmc0_cd_pin: mmc0_cd_pin@0 {
>> + allwinner,pins = "PF6";
>> + allwinner,function = "gpio_in";
>> + allwinner,drive = <SUN4I_PINCTRL_10_MA>;
>> + allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
>> + };
>> +
>> + mmc1_pins_a: mmc1@0 {
>> + allwinner,pins = "PG0","PG1","PG2","PG3","PG4","PG5";
>> + allwinner,function = "mmc1";
>> + allwinner,drive = <SUN4I_PINCTRL_30_MA>;
>> + allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
>> + };
>> + };
>> +
>> + ahb12_rst: reset@01c202c0 {
>> + #reset-cells = <1>;
>> + compatible = "allwinner,sun6i-a31-clock-reset";
>> + reg = <0x01c202c0 0xc>;
>> + };
>
> This reset controller also resets the timers, it should be initialised
> much earlier.
>
> What about having an allwinner,sun8i-h3-bus-reset, and adding it to
> the list of compatibles to initialise earlier in
> drivers/reset/reset-sunxi.c?
>
> Of course, it would cover the other reset controllers that you have
> below.
>

You mean using a single bus_rst instead of the three?
Or, why not using allwinner,sun6i-a31-ahb1-reset for ahb12_rst and
adding a .init_time = sun6i_timer_init to the sun8i machine.

I'm a bit confused here now, because for A23, which is almost
identical, it got removed after your comment:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/265064.html

>
>> + apb1_rst: reset@01c202d0 {
>> + #reset-cells = <1>;
>> + compatible = "allwinner,sun6i-a31-clock-reset";
>> + reg = <0x01c202d0 0x4>;
>> + };
>> +
>> + apb2_rst: reset@01c202d8 {
>> + #reset-cells = <1>;
>> + compatible = "allwinner,sun6i-a31-clock-reset";
>> + reg = <0x01c202d8 0x4>;
>> + };
>> +
>> + timer@01c20c00 {
>> + compatible = "allwinner,sun4i-a10-timer";
>> + reg = <0x01c20c00 0xa0>;
>> + interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
>> + clocks = <&osc24M>;
>> + };
>> +
>> + wdt0: watchdog@01c20ca0 {
>> + compatible = "allwinner,sun6i-a31-wdt";
>> + reg = <0x01c20ca0 0x20>;
>> + interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
>> + };
>> +
>> + uart0: serial@01c28000 {
>> + compatible = "snps,dw-apb-uart";
>> + reg = <0x01c28000 0x400>;
>> + interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
>> + reg-shift = <2>;
>> + reg-io-width = <4>;
>> + clocks = <&apb2_gates 16>;
>> + resets = <&apb2_rst 16>;
>> + dmas = <&dma 6>, <&dma 6>;
>> + dma-names = "rx", "tx";
>> + status = "disabled";
>> + };
>> +
>> + uart1: serial@01c28400 {
>> + compatible = "snps,dw-apb-uart";
>> + reg = <0x01c28400 0x400>;
>> + interrupts = <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
>> + reg-shift = <2>;
>> + reg-io-width = <4>;
>> + clocks = <&apb2_gates 17>;
>> + resets = <&apb2_rst 17>;
>> + dmas = <&dma 7>, <&dma 7>;
>> + dma-names = "rx", "tx";
>> + status = "disabled";
>> + };
>> +
>> + uart2: serial@01c28800 {
>> + compatible = "snps,dw-apb-uart";
>> + reg = <0x01c28800 0x400>;
>> + interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
>> + reg-shift = <2>;
>> + reg-io-width = <4>;
>> + clocks = <&apb2_gates 18>;
>> + resets = <&apb2_rst 18>;
>> + dmas = <&dma 8>, <&dma 8>;
>> + dma-names = "rx", "tx";
>> + status = "disabled";
>> + };
>> +
>> + uart3: serial@01c28c00 {
>> + compatible = "snps,dw-apb-uart";
>> + reg = <0x01c28c00 0x400>;
>> + interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
>> + reg-shift = <2>;
>> + reg-io-width = <4>;
>> + clocks = <&apb2_gates 19>;
>> + resets = <&apb2_rst 19>;
>> + dmas = <&dma 9>, <&dma 9>;
>> + dma-names = "rx", "tx";
>> + status = "disabled";
>> + };
>> +
>> + gic: interrupt-controller@01c81000 {
>> + compatible = "arm,cortex-a7-gic", "arm,cortex-a15-gic";
>> + reg = <0x01c81000 0x1000>,
>> + <0x01c82000 0x1000>,
>> + <0x01c84000 0x2000>,
>> + <0x01c86000 0x2000>;
>> + interrupt-controller;
>> + #interrupt-cells = <3>;
>> + interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
>> + };
>> +
>> + rtc: rtc@01f00000 {
>> + compatible = "allwinner,sun6i-a31-rtc";
>> + reg = <0x01f00000 0x54>;
>> + interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
>> + };
>> + };
>> +};
>> --
>> 2.3.7
>>
>
> Have you tested the architected timers?

Not yet, but I'll look into it.

Jens

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/