Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752709AbbKANlF (ORCPT ); Sun, 1 Nov 2015 08:41:05 -0500 Received: from mail-wm0-f47.google.com ([74.125.82.47]:37696 "EHLO mail-wm0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752546AbbKANk5 (ORCPT ); Sun, 1 Nov 2015 08:40:57 -0500 Subject: Re: [PATCH v4 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI To: Chen-Yu Tsai References: <1445964626-6484-1-git-send-email-jenskuske@gmail.com> <1445964626-6484-6-git-send-email-jenskuske@gmail.com> Cc: Maxime Ripard , Michael Turquette , Linus Walleij , Rob Herring , Philipp Zabel , =?UTF-8?Q?Emilio_L=c3=b3pez?= , Vishnu Patekar , Hans de Goede , devicetree , linux-arm-kernel , linux-kernel , linux-sunxi From: Jens Kuske Message-ID: <563614A3.6060805@gmail.com> Date: Sun, 1 Nov 2015 14:33:23 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 24969 Lines: 560 Hi, On 30/10/15 08:33, Chen-Yu Tsai wrote: > Hi, > > On Wed, Oct 28, 2015 at 12:50 AM, 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 >> --- >> arch/arm/boot/dts/sun8i-h3.dtsi | 482 ++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 482 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..c18b5f7 >> --- /dev/null >> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi >> @@ -0,0 +1,482 @@ >> +/* >> + * Copyright (C) 2015 Jens Kuske >> + * >> + * 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. >> + * >> + * 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 >> +#include >> + >> +/ { >> + 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>; >> + }; >> + }; >> + >> + timer { >> + compatible = "arm,armv7-timer"; >> + interrupts = , >> + , >> + , >> + ; >> + clock-frequency = <24000000>; >> + arm,cpu-registers-not-fw-configured; >> + }; >> + >> + 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"; >> + }; >> + >> + /* dummy clock until actually implemented */ >> + pll5: pll5_clk { >> + #clock-cells = <0>; >> + compatible = "fixed-clock"; >> + clock-frequency = <0>; >> + clock-output-names = "pll5"; >> + }; >> + >> + pll6: clk@01c20028 { >> + #clock-cells = <1>; >> + compatible = "allwinner,sun6i-a31-pll6-clk"; >> + reg = <0x01c20028 0x4>; >> + clocks = <&osc24M>; >> + clock-output-names = "pll6", "pll6x2", "pll6d2"; > > What's the extra "pll6d2"? If you have an extra output, it's not compatible with > "allwinner,sun6i-a31-pll6-clk". > >> + }; >> + >> + pll8: clk@01c20044 { >> + #clock-cells = <1>; >> + compatible = "allwinner,sun6i-a31-pll6-clk"; >> + reg = <0x01c20044 0x4>; >> + clocks = <&osc24M>; >> + clock-output-names = "pll8", "pll8x2"; >> + }; >> + >> + 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>; > > And this would be wrong if you don't update the pll6 compatible or driver. > There's no output on n = 2. > > You could also chain a fixed divider directly in the driver. > >> + 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"; >> + }; >> + >> + bus_gates: clk@01c20060 { >> + #clock-cells = <1>; >> + compatible = "allwinner,sun8i-h3-bus-gates-clk"; >> + reg = <0x01c20060 0x14>; >> + clocks = <&ahb1>, <&ahb2>, <&apb1>, <&apb2>; >> + clock-names = "ahb1", "ahb2", "apb1", "apb2"; >> + clock-indices = <5>, <6>, <8>, >> + <9>, <10>, <13>, >> + <14>, <17>, <18>, >> + <19>, <20>, >> + <21>, <23>, >> + <24>, <25>, >> + <26>, <27>, >> + <28>, <29>, >> + <30>, <31>, <32>, >> + <35>, <36>, <37>, >> + <40>, <41>, <43>, >> + <44>, <52>, <53>, >> + <54>, <64>, >> + <65>, <69>, <72>, >> + <76>, <77>, <78>, >> + <96>, <97>, <98>, >> + <112>, <113>, >> + <114>, <115>, <116>, >> + <128>, <135>; >> + clock-output-names = "ahb1_ce", "ahb1_dma", "ahb1_mmc0", >> + "ahb1_mmc1", "ahb1_mmc2", "ahb1_nand", >> + "ahb1_sdram", "ahb2_gmac", "ahb1_ts", >> + "ahb1_hstimer", "ahb1_spi0", >> + "ahb1_spi1", "ahb1_otg", >> + "ahb1_otg_ehci0", "ahb1_ehic1", > > ahb1_ehci1? Same for the following 3 lines. I'll fix them... > >> + "ahb1_ehic2", "ahb1_ehic3", >> + "ahb1_otg_ohci0", "ahb2_ohic1", >> + "ahb2_ohic2", "ahb2_ohic3", "ahb1_ve", >> + "ahb1_lcd0", "ahb1_lcd1", "ahb1_deint", >> + "ahb1_csi", "ahb1_tve", "ahb1_hdmi", >> + "ahb1_de", "ahb1_gpu", "ahb1_msgbox", >> + "ahb1_spinlock", "apb1_codec", >> + "apb1_spdif", "apb1_pio", "apb1_ths", >> + "apb1_i2s0", "apb1_i2s1", "apb1_i2s2", >> + "apb2_i2c0", "apb2_i2c1", "apb2_i2c2", >> + "apb2_uart0", "apb2_uart1", >> + "apb2_uart2", "apb2_uart3", "apb2_scr", >> + "ahb1_ephy", "ahb1_dbg"; > > If it weren't for the last 2 clocks, we could cleanly split out apb1 and apb2 > gates. Having a separate AHB clock gate taking 2 addresses seems messy > as well. :( Well, maybe we still should do that, if we split the resets too at least apb[12] would line up again. I don't know what to do with these bus things any more, all variants I sent had issues somewhere... > >> + }; >> + >> + mmc0_clk: clk@01c20088 { >> + #clock-cells = <1>; >> + compatible = "allwinner,sun4i-a10-mmc-clk"; >> + reg = <0x01c20088 0x4>; >> + clocks = <&osc24M>, <&pll6 0>, <&pll8 0>; >> + 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 0>; >> + 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 0>; >> + clock-output-names = "mmc2", >> + "mmc2_output", >> + "mmc2_sample"; >> + }; >> + >> + mbus_clk: clk@01c2015c { >> + #clock-cells = <0>; >> + compatible = "allwinner,sun8i-a23-mbus-clk"; >> + reg = <0x01c2015c 0x4>; >> + clocks = <&osc24M>, <&pll6 1>, <&pll5>; >> + clock-output-names = "mbus"; >> + }; >> + }; >> + >> + soc { >> + compatible = "simple-bus"; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + ranges; >> + >> + dma: dma-controller@01c02000 { >> + compatible = "allwinner,sun8i-h3-dma"; >> + reg = <0x01c02000 0x1000>; >> + interrupts = ; >> + clocks = <&bus_gates 6>; >> + resets = <&bus_rst 6>; >> + #dma-cells = <1>; >> + }; >> + >> + mmc0: mmc@01c0f000 { >> + compatible = "allwinner,sun5i-a13-mmc"; >> + reg = <0x01c0f000 0x1000>; >> + clocks = <&bus_gates 8>, >> + <&mmc0_clk 0>, >> + <&mmc0_clk 1>, >> + <&mmc0_clk 2>; >> + clock-names = "ahb", >> + "mmc", >> + "output", >> + "sample"; >> + resets = <&bus_rst 8>; >> + reset-names = "ahb"; >> + interrupts = ; >> + status = "disabled"; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + }; >> + >> + mmc1: mmc@01c10000 { >> + compatible = "allwinner,sun5i-a13-mmc"; >> + reg = <0x01c10000 0x1000>; >> + clocks = <&bus_gates 9>, >> + <&mmc1_clk 0>, >> + <&mmc1_clk 1>, >> + <&mmc1_clk 2>; >> + clock-names = "ahb", >> + "mmc", >> + "output", >> + "sample"; >> + resets = <&bus_rst 9>; >> + reset-names = "ahb"; >> + interrupts = ; >> + status = "disabled"; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + }; >> + >> + mmc2: mmc@01c11000 { >> + compatible = "allwinner,sun5i-a13-mmc"; >> + reg = <0x01c11000 0x1000>; >> + clocks = <&bus_gates 10>, >> + <&mmc2_clk 0>, >> + <&mmc2_clk 1>, >> + <&mmc2_clk 2>; >> + clock-names = "ahb", >> + "mmc", >> + "output", >> + "sample"; >> + resets = <&bus_rst 10>; >> + reset-names = "ahb"; >> + interrupts = ; >> + status = "disabled"; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + }; >> + >> + pio: pinctrl@01c20800 { >> + compatible = "allwinner,sun8i-h3-pinctrl"; >> + reg = <0x01c20800 0x400>; >> + interrupts = , >> + ; >> + clocks = <&bus_gates 69>; >> + gpio-controller; >> + #gpio-cells = <3>; >> + interrupt-controller; >> + #interrupt-cells = <2>; >> + >> + uart0_pins_a: uart0@0 { >> + allwinner,pins = "PA4", "PA5"; >> + allwinner,function = "uart0"; >> + allwinner,drive = ; >> + allwinner,pull = ; >> + }; >> + >> + mmc0_pins_a: mmc0@0 { >> + allwinner,pins = "PF0", "PF1", "PF2", "PF3", >> + "PF4", "PF5"; >> + allwinner,function = "mmc0"; >> + allwinner,drive = ; >> + allwinner,pull = ; >> + }; >> + >> + mmc0_cd_pin: mmc0_cd_pin@0 { >> + allwinner,pins = "PF6"; >> + allwinner,function = "gpio_in"; >> + allwinner,drive = ; >> + allwinner,pull = ; >> + }; > > This should be in the board DTS, unless this is the reference design, > in which case you should name the label like "mmc0_cd_pin_reference_design". > The datasheet mentions SDC0_DET function on PF6, so I thought this is sort of fixed to this pin now. All designs I've seen use this pin. Jens >> + >> + mmc1_pins_a: mmc1@0 { >> + allwinner,pins = "PG0", "PG1", "PG2", "PG3", >> + "PG4", "PG5"; >> + allwinner,function = "mmc1"; >> + allwinner,drive = ; >> + allwinner,pull = ; >> + }; >> + }; >> + >> + bus_rst: reset@01c202c0 { >> + #reset-cells = <1>; >> + compatible = "allwinner,sun8i-h3-bus-reset"; >> + reg = <0x01c202c0 0x1c>; >> + }; >> + >> + timer@01c20c00 { >> + compatible = "allwinner,sun4i-a10-timer"; >> + reg = <0x01c20c00 0xa0>; >> + interrupts = , >> + ; >> + clocks = <&osc24M>; >> + }; >> + >> + wdt0: watchdog@01c20ca0 { >> + compatible = "allwinner,sun6i-a31-wdt"; >> + reg = <0x01c20ca0 0x20>; >> + interrupts = ; >> + }; >> + >> + uart0: serial@01c28000 { >> + compatible = "snps,dw-apb-uart"; >> + reg = <0x01c28000 0x400>; >> + interrupts = ; >> + reg-shift = <2>; >> + reg-io-width = <4>; >> + clocks = <&bus_gates 112>; >> + resets = <&bus_rst 144>; > > Aren't you handling the holes in the bus_rst driver? If so, > isn't it supposed to align both bus_gates and bus_rst indexes? > > Same for the other UARTs. > > Thanks > > Regards > ChenYu > >> + dmas = <&dma 6>, <&dma 6>; >> + dma-names = "rx", "tx"; >> + status = "disabled"; >> + }; >> + >> + uart1: serial@01c28400 { >> + compatible = "snps,dw-apb-uart"; >> + reg = <0x01c28400 0x400>; >> + interrupts = ; >> + reg-shift = <2>; >> + reg-io-width = <4>; >> + clocks = <&bus_gates 113>; >> + resets = <&bus_rst 145>; >> + dmas = <&dma 7>, <&dma 7>; >> + dma-names = "rx", "tx"; >> + status = "disabled"; >> + }; >> + >> + uart2: serial@01c28800 { >> + compatible = "snps,dw-apb-uart"; >> + reg = <0x01c28800 0x400>; >> + interrupts = ; >> + reg-shift = <2>; >> + reg-io-width = <4>; >> + clocks = <&bus_gates 114>; >> + resets = <&bus_rst 146>; >> + dmas = <&dma 8>, <&dma 8>; >> + dma-names = "rx", "tx"; >> + status = "disabled"; >> + }; >> + >> + uart3: serial@01c28c00 { >> + compatible = "snps,dw-apb-uart"; >> + reg = <0x01c28c00 0x400>; >> + interrupts = ; >> + reg-shift = <2>; >> + reg-io-width = <4>; >> + clocks = <&bus_gates 115>; >> + resets = <&bus_rst 147>; >> + 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 = ; >> + }; >> + >> + rtc: rtc@01f00000 { >> + compatible = "allwinner,sun6i-a31-rtc"; >> + reg = <0x01f00000 0x54>; >> + interrupts = , >> + ; >> + }; >> + }; >> +}; >> -- >> 2.6.2 >> > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/