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 <[email protected]>
> ---
> 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 <[email protected]>
> + *
> + * 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.

> + *
> + * 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).

> + };
> +
> + 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?

> + 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.

> + 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?

Thanks,
Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Attachments:

(No filename) (14.97 kB)
signature.asc (819.00 B)
Digital signature Download all attachments

2015-05-06 12:25:07

by Maxime Ripard

[permalink] [raw]

Subject: Re: [PATCH 1/6] ARM: sunxi: Introduce Allwinner H3 support

On Wed, May 06, 2015 at 12:23:49PM +0200, Jens Kuske wrote:
> On 06/05/15 12:04, Maxime Ripard wrote:
> > Hi,
> >
> > On Wed, May 06, 2015 at 11:31:28AM +0200, Jens Kuske wrote:
> >> The Allwinner H3 is a quad-core Cortex-A7-based SoC. It is very similar
> >> to other sun8i family SoCs like the A23.
> >>
> >> Signed-off-by: Jens Kuske <[email protected]>
> >> ---
> >> Documentation/devicetree/bindings/arm/sunxi.txt | 1 +
> >> arch/arm/mach-sunxi/Kconfig | 2 +-
> >> arch/arm/mach-sunxi/sunxi.c | 3 ++-
> >> 3 files changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/arm/sunxi.txt b/Documentation/devicetree/bindings/arm/sunxi.txt
> >> index 42941fd..3cb4b94 100644
> >> --- a/Documentation/devicetree/bindings/arm/sunxi.txt
> >> +++ b/Documentation/devicetree/bindings/arm/sunxi.txt
> >> @@ -9,4 +9,5 @@ using one of the following compatible strings:
> >> allwinner,sun6i-a31
> >> allwinner,sun7i-a20
> >> allwinner,sun8i-a23
> >> + allwinner,sun8i-h3
> >> allwinner,sun9i-a80
> >> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> >> index 81502b9..47ac5b4 100644
> >> --- a/arch/arm/mach-sunxi/Kconfig
> >> +++ b/arch/arm/mach-sunxi/Kconfig
> >> @@ -35,7 +35,7 @@ config MACH_SUN7I
> >> select SUN5I_HSTIMER
> >>
> >> config MACH_SUN8I
> >> - bool "Allwinner A23 (sun8i) SoCs support"
> >> + bool "Allwinner A23 / H3 (sun8i) SoCs support"
> >
> > I'd prefer that we have something similar to sun9i, just mentionning
> > the family here, instead of an ever-growing list of SoCs.
> >
> > This used to be easy to maintain, with the new naming scheme, we
> > already have at least 4 SoCs part of sun8i.
>
> According to Allwinners new naming scheme A20/A31/A31s are also part of
> sun8i (sun8iw[1,2]), but I think we can agree to use sun8i for the new
> SoCs, starting from A23.

Yep, we can't really change the family half-way, and using sun8i for
the newer SoCs (A23, A33, A83, H3, V*, etc.) works just as well.

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Attachments:

(No filename) (2.11 kB)
signature.asc (819.00 B)
Digital signature Download all attachments

2015-05-06 12:25:10

Hi Vinod,

On Fri, May 08, 2015 at 09:14:26AM +0530, Vinod Koul wrote:
> On Wed, May 06, 2015 at 12:13:42PM +0200, Maxime Ripard wrote:
> > On Wed, May 06, 2015 at 11:31:31AM +0200, Jens Kuske wrote:
> > > The H3 SoC has the same dma engine as the A31 (sun6i), with a
> > > reduced amount of endpoints and physical channels. Add the proper
> > > config data and compatible string to support it.
> > >
> > > Signed-off-by: Jens Kuske <[email protected]>
>
> This looks fine to me, I think can be merged now. Do you guys want the
> mainatainers to pick up patches to their subsystem or merge them tgether,
> though don't see any dependency though

It can be merged through your tree, like you said there's no
particular dependencies on our side.

Thanks!
Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Attachments:

(No filename) (872.00 B)
signature.asc (819.00 B)
Digital signature Download all attachments

2015-05-08 09:25:37

by Vinod Koul

[permalink] [raw]

Subject: Re: [PATCH 4/6] dmaengine: sun6i: Add support for Allwinner H3 (sun8i) variant

2015-05-09 09:17:59

by Jens Kuske

Hi,

On 06-05-15 11:31, Jens Kuske wrote:
> Hi everyone,
>
> This patch series introduces basic kernel support for Allwinner's H3 SoC,
> mainly basic clocks and pinctrl. It also adds interrupts, timers,
> watchdog, RTC, dmaengine, MMC and UARTs, which are mostly compatible to
> those in earlier SoCs like A23 and A31, and can simply be reused.
>
> These patches are based on Maxime's sunxi/for-next branch from
> git://git.kernel.org/pub/scm/linux/kernel/git/mripard/linux.git
>
> Patch 1 adds machine support for the H3.
>
> Patch 2 adds support for the basic clocks. The AHB1/AHB2 split could
> need some discussion.
>
> Patch 3 adds the pin sets for the H3 main PIO.
>
> Patch 4 adds the H3 specific parameters to the sun6i dmaengine driver.
>
> Patch 5 adds the DTSI for the H3.
>
> Patch 6 adds a DT for the Orange Pi Plus SBC, which these patches
> were developed and tested with.

Cool stuff, thanks for your work on this. Do you have any plans to also
add support for the H3 to upstream u-boot ?

Regards,

Hans

2015-05-10 10:56:55

by Jens Kuske

[permalink] [raw]

Subject: Re: [PATCH 2/6] clk: sunxi: Add H3 clocks support

Hi,

On 09/05/15 13:27, Maxime Ripard wrote:
> On Wed, May 06, 2015 at 11:31:29AM +0200, Jens Kuske wrote:
>> The H3 clock control unit is similar to the those of other sun8i family
>> members like the A23.
>>
>> The AHB1 gates got split up into AHB1 and AHB2, with AHB2 clock source
>> being muxable between AHB1 and PLL6/2, but still sharing gate registers.
>> The documentation isn't totally clear about which devices belong to
>> AHB2 now, especially USB EHIC/OHIC, so it is mostly based on Allwinner
>> kernel source code.
>>
>> Signed-off-by: Jens Kuske <[email protected]>
>> ---
>> Documentation/devicetree/bindings/clock/sunxi.txt | 7 ++++
>> drivers/clk/sunxi/clk-sunxi.c | 46 ++++++++++++++++++++++-
>> 2 files changed, 52 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
>> index 4fa11af..4eeb893 100644
>> --- a/Documentation/devicetree/bindings/clock/sunxi.txt
>> +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
>> @@ -14,6 +14,8 @@ Required properties:
>> "allwinner,sun4i-a10-pll5-clk" - for the PLL5 clock
>> "allwinner,sun4i-a10-pll6-clk" - for the PLL6 clock
>> "allwinner,sun6i-a31-pll6-clk" - for the PLL6 clock on A31
>> + "allwinner,sun8i-h3-pll6-clk" - for the PLL6 clock on H3
>> + "allwinner,sun8i-h3-pll8-clk" - for the PLL8 clock on H3
>> "allwinner,sun9i-a80-gt-clk" - for the GT bus clock on A80
>> "allwinner,sun4i-a10-cpu-clk" - for the CPU multiplexer clock
>> "allwinner,sun4i-a10-axi-clk" - for the AXI clock
>> @@ -28,8 +30,11 @@ Required properties:
>> "allwinner,sun7i-a20-ahb-gates-clk" - for the AHB gates on A20
>> "allwinner,sun6i-a31-ar100-clk" - for the AR100 on A31
>> "allwinner,sun6i-a31-ahb1-clk" - for the AHB1 clock on A31
>> + "allwinner,sun8i-h3-ahb2-clk" - for the AHB2 clock on H3
>> "allwinner,sun6i-a31-ahb1-gates-clk" - for the AHB1 gates on A31
>> "allwinner,sun8i-a23-ahb1-gates-clk" - for the AHB1 gates on A23
>> + "allwinner,sun8i-h3-ahb1-gates-clk" - for the AHB1 gates on H3
>> + "allwinner,sun8i-h3-ahb2-gates-clk" - for the AHB2 gates on H3
>> "allwinner,sun9i-a80-ahb0-gates-clk" - for the AHB0 gates on A80
>> "allwinner,sun9i-a80-ahb1-gates-clk" - for the AHB1 gates on A80
>> "allwinner,sun9i-a80-ahb2-gates-clk" - for the AHB2 gates on A80
>> @@ -52,8 +57,10 @@ Required properties:
>> "allwinner,sun6i-a31-apb1-gates-clk" - for the APB1 gates on A31
>> "allwinner,sun7i-a20-apb1-gates-clk" - for the APB1 gates on A20
>> "allwinner,sun8i-a23-apb1-gates-clk" - for the APB1 gates on A23
>> + "allwinner,sun8i-h3-apb1-gates-clk" - for the APB1 gates on H3
>> "allwinner,sun9i-a80-apb1-gates-clk" - for the APB1 gates on A80
>> "allwinner,sun6i-a31-apb2-gates-clk" - for the APB2 gates on A31
>> + "allwinner,sun8i-h3-apb2-gates-clk" - for the APB2 gates on H3
>> "allwinner,sun8i-a23-apb2-gates-clk" - for the APB2 gates on A23
>> "allwinner,sun5i-a13-mbus-clk" - for the MBUS clock on A13
>> "allwinner,sun4i-a10-mmc-clk" - for the MMC clock
>> diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
>> index 7e1e2bd..152a1f7 100644
>> --- a/drivers/clk/sunxi/clk-sunxi.c
>> +++ b/drivers/clk/sunxi/clk-sunxi.c
>> @@ -727,6 +727,12 @@ static const struct factors_data sun5i_a13_ahb_data __initconst = {
>> .getter = sun5i_a13_get_ahb_factors,
>> };
>>
>> +static const struct factors_data sun8i_h3_pll8_data __initconst = {
>> + .enable = 31,
>> + .table = &sun6i_a31_pll6_config,
>> + .getter = sun6i_a31_get_pll6_factors,
>> +};
>
> This looks like it's just another instance of the A31 pll6.
>
> In such a case, we don't need to declare a new driver, just reuse the
> same compatible.

If I reuse pll6 for pll8 I get errors because of the .name = "pll6x2"
field, already existing clock or something like that. (And pll8 doesn't
even have a x2 version)

>
>> static const struct factors_data sun4i_apb1_data __initconst = {
>> .mux = 24,
>> .muxmask = BIT(1) | BIT(0),
>> @@ -777,6 +783,10 @@ static const struct mux_data sun6i_a31_ahb1_mux_data __initconst = {
>> .shift = 12,
>> };
>>
>> +static const struct mux_data sun8i_h3_ahb2_mux_data __initconst = {
>> + .shift = 0,
>> +};
>> +
>> static void __init sunxi_mux_clk_setup(struct device_node *node,
>> struct mux_data *data)
>> {
>> @@ -892,7 +902,7 @@ static void __init sunxi_divider_clk_setup(struct device_node *node,
>> * sunxi_gates_clk_setup() - Setup function for leaf gates on clocks
>> */
>>
>> -#define SUNXI_GATES_MAX_SIZE 64
>> +#define SUNXI_GATES_MAX_SIZE 160
>>
>> struct gates_data {
>> DECLARE_BITMAP(mask, SUNXI_GATES_MAX_SIZE);
>> @@ -926,6 +936,10 @@ static const struct gates_data sun8i_a23_ahb1_gates_data __initconst = {
>> .mask = {0x25386742, 0x2505111},
>> };
>>
>> +static const struct gates_data sun8i_h3_ahb1_gates_data __initconst = {
>> + .mask = {0x1fbc6760, 0x00701b39, 0x00000000, 0x00000000, 0x00000081},
>> +};
>> +
>
> Judging from the user manual, there's a few gates in those 0
> registers, is this normal that you don't support them?

They are holes for apb1 and apb2. Which is actually pretty ugly.

>
>> static const struct gates_data sun9i_a80_ahb0_gates_data __initconst = {
>> .mask = {0xF5F12B},
>> };
>> @@ -938,6 +952,10 @@ static const struct gates_data sun9i_a80_ahb2_gates_data __initconst = {
>> .mask = {0x9B7},
>> };
>>
>> +static const struct gates_data sun8i_h3_ahb2_gates_data __initconst = {
>> + .mask = {0xe0020000},
>> +};
>> +
>
> I don't think we should split the ahb1 and ahb2 gates here. It really
> looks like it's the same controller.
>
> The way I'm seeing it would be to have a single clock driver that
> would handle both your ahb1 and ahb2 gates.
>
> It would take two parents, ahb1 and ahb2, obviously, and would take
> register depending on the gate w'ere registering either the ahb1 or
> the ahb2 parent.
>
> It seems like there's only a handful of devices in ahb2 anyway, so
> that wouldn't make a very long list of devices to declare as childs of
> ahb2.
>

I have thought about adding a bus_gates driver for all ahb1, ahb2, apb1
and apb2 gates, as it is done in the user manual.

But it would need a pretty big parents array and result in big gate
numbers in devicetree, <&bus_gates 112> for uart0 for example.

Would this be ok?

Jens

2015-05-11 08:12:15

by Chen-Yu Tsai

[permalink] [raw]

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

On Sat, May 9, 2015 at 7:44 PM, Maxime Ripard
<[email protected]> wrote:
> On Wed, May 06, 2015 at 10:47:33PM +0200, Jens Kuske wrote:
>> >> + * 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.
>
> Yeah, I know, I'm even the one that introduced this in the first place
> :)
>
> I sent a patch earlier this week to remove it from the other DT.
>
>> >> + 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?
>
> Yes.
>
>> Or, why not using allwinner,sun6i-a31-ahb1-reset for ahb12_rst
>
> Strictly speaking, they do not control the same set of devices. I'd
> prefer to have a different compatible in case we need to setup a
> particular behaviour on a given SoC (for example, force out of reset a
> particular device, even if no driver is actually using it), without
> impacting the other.
>
>> and adding a .init_time = sun6i_timer_init to the sun8i machine.
>
> But we will need to do that yes.
>
>> 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
>
> Hmmmm, I think I somehow overlooked the fact that the timer was there,
> even though Chen-Yu said it. My bad :/

On the A23, the high speed timer block only has 1 timer. The sun5i-hrtimer
driver requires 2, and turns out we weren't using them anyway, so I just
dropped sun5i-hrtimer support on A23.

If the other sun8i SoCs have 2 or more timers, feel free to support them.

ChenYu

2015-05-12 14:45:07

On Thu, May 14, 2015 at 01:14:25PM +0800, Chen-Yu Tsai wrote:
> On Tue, May 12, 2015 at 10:44 PM, Maxime Ripard
> <[email protected]> wrote:
> > Hi,
> >
> > On Sun, May 10, 2015 at 12:54:50PM +0200, Jens Kuske wrote:
> >> On 09/05/15 13:27, Maxime Ripard wrote:
> >> > On Wed, May 06, 2015 at 11:31:29AM +0200, Jens Kuske wrote:
> >> >> The H3 clock control unit is similar to the those of other sun8i family
> >> >> members like the A23.
> >> >>
> >> >> The AHB1 gates got split up into AHB1 and AHB2, with AHB2 clock source
> >> >> being muxable between AHB1 and PLL6/2, but still sharing gate registers.
> >> >> The documentation isn't totally clear about which devices belong to
> >> >> AHB2 now, especially USB EHIC/OHIC, so it is mostly based on Allwinner
> >> >> kernel source code.
> >> >>
> >> >> Signed-off-by: Jens Kuske <[email protected]>
> >> >> ---
> >> >> Documentation/devicetree/bindings/clock/sunxi.txt | 7 ++++
> >> >> drivers/clk/sunxi/clk-sunxi.c | 46 ++++++++++++++++++++++-
> >> >> 2 files changed, 52 insertions(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
> >> >> index 4fa11af..4eeb893 100644
> >> >> --- a/Documentation/devicetree/bindings/clock/sunxi.txt
> >> >> +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
> >> >> @@ -14,6 +14,8 @@ Required properties:
> >> >> "allwinner,sun4i-a10-pll5-clk" - for the PLL5 clock
> >> >> "allwinner,sun4i-a10-pll6-clk" - for the PLL6 clock
> >> >> "allwinner,sun6i-a31-pll6-clk" - for the PLL6 clock on A31
> >> >> + "allwinner,sun8i-h3-pll6-clk" - for the PLL6 clock on H3
> >> >> + "allwinner,sun8i-h3-pll8-clk" - for the PLL8 clock on H3
> >> >> "allwinner,sun9i-a80-gt-clk" - for the GT bus clock on A80
> >> >> "allwinner,sun4i-a10-cpu-clk" - for the CPU multiplexer clock
> >> >> "allwinner,sun4i-a10-axi-clk" - for the AXI clock
> >> >> @@ -28,8 +30,11 @@ Required properties:
> >> >> "allwinner,sun7i-a20-ahb-gates-clk" - for the AHB gates on A20
> >> >> "allwinner,sun6i-a31-ar100-clk" - for the AR100 on A31
> >> >> "allwinner,sun6i-a31-ahb1-clk" - for the AHB1 clock on A31
> >> >> + "allwinner,sun8i-h3-ahb2-clk" - for the AHB2 clock on H3
> >> >> "allwinner,sun6i-a31-ahb1-gates-clk" - for the AHB1 gates on A31
> >> >> "allwinner,sun8i-a23-ahb1-gates-clk" - for the AHB1 gates on A23
> >> >> + "allwinner,sun8i-h3-ahb1-gates-clk" - for the AHB1 gates on H3
> >> >> + "allwinner,sun8i-h3-ahb2-gates-clk" - for the AHB2 gates on H3
> >> >> "allwinner,sun9i-a80-ahb0-gates-clk" - for the AHB0 gates on A80
> >> >> "allwinner,sun9i-a80-ahb1-gates-clk" - for the AHB1 gates on A80
> >> >> "allwinner,sun9i-a80-ahb2-gates-clk" - for the AHB2 gates on A80
> >> >> @@ -52,8 +57,10 @@ Required properties:
> >> >> "allwinner,sun6i-a31-apb1-gates-clk" - for the APB1 gates on A31
> >> >> "allwinner,sun7i-a20-apb1-gates-clk" - for the APB1 gates on A20
> >> >> "allwinner,sun8i-a23-apb1-gates-clk" - for the APB1 gates on A23
> >> >> + "allwinner,sun8i-h3-apb1-gates-clk" - for the APB1 gates on H3
> >> >> "allwinner,sun9i-a80-apb1-gates-clk" - for the APB1 gates on A80
> >> >> "allwinner,sun6i-a31-apb2-gates-clk" - for the APB2 gates on A31
> >> >> + "allwinner,sun8i-h3-apb2-gates-clk" - for the APB2 gates on H3
> >> >> "allwinner,sun8i-a23-apb2-gates-clk" - for the APB2 gates on A23
> >> >> "allwinner,sun5i-a13-mbus-clk" - for the MBUS clock on A13
> >> >> "allwinner,sun4i-a10-mmc-clk" - for the MMC clock
> >> >> diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
> >> >> index 7e1e2bd..152a1f7 100644
> >> >> --- a/drivers/clk/sunxi/clk-sunxi.c
> >> >> +++ b/drivers/clk/sunxi/clk-sunxi.c
> >> >> @@ -727,6 +727,12 @@ static const struct factors_data sun5i_a13_ahb_data __initconst = {
> >> >> .getter = sun5i_a13_get_ahb_factors,
> >> >> };
> >> >>
> >> >> +static const struct factors_data sun8i_h3_pll8_data __initconst = {
> >> >> + .enable = 31,
> >> >> + .table = &sun6i_a31_pll6_config,
> >> >> + .getter = sun6i_a31_get_pll6_factors,
> >> >> +};
> >> >
> >> > This looks like it's just another instance of the A31 pll6.
> >> >
> >> > In such a case, we don't need to declare a new driver, just reuse the
> >> > same compatible.
> >>
> >> If I reuse pll6 for pll8 I get errors because of the .name = "pll6x2"
> >> field, already existing clock or something like that.
> >
> > Damn. You're obviously right...
>
> I think I have a solution for this.
>
> The current divs clock setup just passes the factors_data directly to
> sunxi_factors_register(). What if it did a copy, read the _correct_
> name from the DT (since it knows the index) and put it in the copy.
>
> How does that sound?

That sounds great, but what name would you take from the DT? The first
of clock-output-names? the last one? a random one?

>
> > Could you add a TODO comment on top then? just so that we know that we
> > need to merge this clock with pll6?
> >
> >> (And pll8 doesn't even have a x2 version)
> >
> > Judging by the H3 datasheet, it does.
> >
> >> >> static const struct factors_data sun4i_apb1_data __initconst = {
> >> >> .mux = 24,
> >> >> .muxmask = BIT(1) | BIT(0),
> >> >> @@ -777,6 +783,10 @@ static const struct mux_data sun6i_a31_ahb1_mux_data __initconst = {
> >> >> .shift = 12,
> >> >> };
> >> >>
> >> >> +static const struct mux_data sun8i_h3_ahb2_mux_data __initconst = {
> >> >> + .shift = 0,
> >> >> +};
> >> >> +
> >> >> static void __init sunxi_mux_clk_setup(struct device_node *node,
> >> >> struct mux_data *data)
> >> >> {
> >> >> @@ -892,7 +902,7 @@ static void __init sunxi_divider_clk_setup(struct device_node *node,
> >> >> * sunxi_gates_clk_setup() - Setup function for leaf gates on clocks
> >> >> */
> >> >>
> >> >> -#define SUNXI_GATES_MAX_SIZE 64
> >> >> +#define SUNXI_GATES_MAX_SIZE 160
> >> >>
> >> >> struct gates_data {
> >> >> DECLARE_BITMAP(mask, SUNXI_GATES_MAX_SIZE);
> >> >> @@ -926,6 +936,10 @@ static const struct gates_data sun8i_a23_ahb1_gates_data __initconst = {
> >> >> .mask = {0x25386742, 0x2505111},
> >> >> };
> >> >>
> >> >> +static const struct gates_data sun8i_h3_ahb1_gates_data __initconst = {
> >> >> + .mask = {0x1fbc6760, 0x00701b39, 0x00000000, 0x00000000, 0x00000081},
> >> >> +};
> >> >> +
> >> >
> >> > Judging from the user manual, there's a few gates in those 0
> >> > registers, is this normal that you don't support them?
> >>
> >> They are holes for apb1 and apb2. Which is actually pretty ugly.
> >
> > Ah, right.
> >
> > So I guess it's completely related to the discussion below.
>
> If the holes are really big, I guess you could split ahb and apb?

It's actually already split. The holes in the ahb1 registers are for
the apb clocks.

Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Attachments:

(No filename) (6.87 kB)
signature.asc (819.00 B)
Digital signature Download all attachments