[PATCH 4/6] reset: sunxi: Add compatible for Allwinner H3 bus resets

[PATCH 4/6] reset: sunxi: Add compatible for Allwinner H3 bus resets

[Jens Kuske]

Adding a new compatible allows us to define SoC specific behaviour
if necessary, for example forcing a particular device out of reset
even if no driver is actually using it.

Signed-off-by: Jens Kuske <jenskuske-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 Documentation/devicetree/bindings/reset/allwinner,sunxi-clock-reset.txt | 1 +
 drivers/reset/reset-sunxi.c                                             | 1 +
 2 files changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/reset/allwinner,sunxi-clock-reset.txt b/Documentation/devicetree/bindings/reset/allwinner,sunxi-clock-reset.txt
index c8f7757..e11f023 100644
--- a/Documentation/devicetree/bindings/reset/allwinner,sunxi-clock-reset.txt
+++ b/Documentation/devicetree/bindings/reset/allwinner,sunxi-clock-reset.txt
@@ -8,6 +8,7 @@ Required properties:
 - compatible: Should be one of the following:
   "allwinner,sun6i-a31-ahb1-reset"
   "allwinner,sun6i-a31-clock-reset"
+  "allwinner,sun8i-h3-bus-reset"
 - reg: should be register base and length as documented in the
   datasheet
 - #reset-cells: 1, see below
diff --git a/drivers/reset/reset-sunxi.c b/drivers/reset/reset-sunxi.c
index 3d95c87..6f12b5c 100644
--- a/drivers/reset/reset-sunxi.c
+++ b/drivers/reset/reset-sunxi.c
@@ -124,6 +124,7 @@ err_alloc:
  */
 static const struct of_device_id sunxi_early_reset_dt_ids[] __initdata = {
 	{ .compatible = "allwinner,sun6i-a31-ahb1-reset", },
+	{ .compatible = "allwinner,sun8i-h3-bus-reset", },
 	{ /* sentinel */ },
 };
 
-- 
2.6.1

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

[Jens Kuske]

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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/arm/boot/dts/sun8i-h3.dtsi | 499 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 499 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..4114e17
--- /dev/null
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -0,0 +1,499 @@
+/*
+ * Copyright (C) 2015 Jens Kuske <jenskuske-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+ *
+ * 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 <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>;
+		};
+	};
+
+	timer {
+		compatible = "arm,armv7-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)>;
+		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";
+		};
+
+		pll8: clk@01c20044 {
+			#clock-cells = <0>;
+			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>;
+			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>;
+			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>;
+			clocks = <&ahb1>, <&ahb1>, <&ahb1>,
+				 <&ahb1>, <&ahb1>, <&ahb1>,
+				 <&ahb1>, <&ahb2>, <&ahb1>,
+				 <&ahb1>, <&ahb1>,
+				 <&ahb1>, <&ahb1>,
+				 <&ahb1>, <&ahb1>,
+				 <&ahb1>, <&ahb1>,
+				 <&ahb1>, <&ahb2>,
+				 <&ahb2>, <&ahb2>, <&ahb1>,
+				 <&ahb1>, <&ahb1>, <&ahb1>,
+				 <&ahb1>, <&ahb1>, <&ahb1>,
+				 <&ahb1>, <&ahb1>, <&ahb1>,
+				 <&ahb1>, <&apb1>,
+				 <&apb1>, <&apb1>, <&apb1>,
+				 <&apb1>, <&apb1>, <&apb1>,
+				 <&apb2>, <&apb2>, <&apb2>,
+				 <&apb2>, <&apb2>,
+				 <&apb2>, <&apb2>, <&apb2>,
+				 <&ahb1>, <&ahb1>;
+			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_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";
+		};
+
+		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@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 = <&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 = <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
+			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 = <GIC_SPI 61 IRQ_TYPE_LEVEL_HIGH>;
+			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 = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		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 = <&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 = <SUN4I_PINCTRL_10_MA>;
+				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+			};
+
+			mmc0_pins_a: mmc0@0 {
+				allwinner,pins = "PF0", "PF1", "PF2", "PF3",
+						 "PF4", "PF5";
+				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>;
+			};
+		};
+
+		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 = <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 = <&bus_gates 112>;
+			resets = <&bus_rst 208>;
+			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 = <&bus_gates 113>;
+			resets = <&bus_rst 209>;
+			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 = <&bus_gates 114>;
+			resets = <&bus_rst 210>;
+			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 = <&bus_gates 115>;
+			resets = <&bus_rst 211>;
+			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.6.1

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

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 10520 bytes --]

Hi,

On Wed, Oct 21, 2015 at 06:20:27PM +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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  arch/arm/boot/dts/sun8i-h3.dtsi | 499 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 499 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..4114e17
> --- /dev/null
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -0,0 +1,499 @@
> +/*
> + * Copyright (C) 2015 Jens Kuske <jenskuske-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> + *
> + * 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 <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>;
> +		};
> +	};
> +
> +	timer {
> +		compatible = "arm,armv7-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)>;
> +		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";
> +		};
> +
> +		pll8: clk@01c20044 {
> +			#clock-cells = <0>;
> +			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>;
> +			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>;
> +			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>;
> +			clocks = <&ahb1>, <&ahb1>, <&ahb1>,
> +				 <&ahb1>, <&ahb1>, <&ahb1>,
> +				 <&ahb1>, <&ahb2>, <&ahb1>,
> +				 <&ahb1>, <&ahb1>,
> +				 <&ahb1>, <&ahb1>,
> +				 <&ahb1>, <&ahb1>,
> +				 <&ahb1>, <&ahb1>,
> +				 <&ahb1>, <&ahb2>,
> +				 <&ahb2>, <&ahb2>, <&ahb1>,
> +				 <&ahb1>, <&ahb1>, <&ahb1>,
> +				 <&ahb1>, <&ahb1>, <&ahb1>,
> +				 <&ahb1>, <&ahb1>, <&ahb1>,
> +				 <&ahb1>, <&apb1>,
> +				 <&apb1>, <&apb1>, <&apb1>,
> +				 <&apb1>, <&apb1>, <&apb1>,
> +				 <&apb2>, <&apb2>, <&apb2>,
> +				 <&apb2>, <&apb2>,
> +				 <&apb2>, <&apb2>, <&apb2>,
> +				 <&ahb1>, <&ahb1>;
> +			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_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";
> +		};
> +
> +		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@01c00000 {

We had some issues with this in the past, especially since it's wrong
and the SoC registers definitions start at 0, with the SRAMs. It would
be better if you removed it entirely like we did in the A80 DTSI.

> +		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 = <&bus_gates 112>;
> +			resets = <&bus_rst 208>;

It's a bit weird that the clocks and reset indices don't match,
usually they do.

What's even weirder is that there's a 96 offset between the two (4 *
32), is this expected?

Thanks!
Maxime

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

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

[Jean-Francois Moine]

On Thu, 22 Oct 2015 10:05:08 +0200
Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:

> > +		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 = <&bus_gates 112>;
> > +			resets = <&bus_rst 208>;  
> 
> It's a bit weird that the clocks and reset indices don't match,
> usually they do.
> 
> What's even weirder is that there's a 96 offset between the two (4 *
> 32), is this expected?

Yes, this is conform to the H3 documentation.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 987 bytes --]

On Thu, Oct 22, 2015 at 10:29:59AM +0200, Jean-Francois Moine wrote:
> On Thu, 22 Oct 2015 10:05:08 +0200
> Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> 
> > > +		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 = <&bus_gates 112>;
> > > +			resets = <&bus_rst 208>;  
> > 
> > It's a bit weird that the clocks and reset indices don't match,
> > usually they do.
> > 
> > What's even weirder is that there's a 96 offset between the two (4 *
> > 32), is this expected?
> 
> Yes, this is conform to the H3 documentation.

Not really. The uart0 reset is the bit 16, in the reset register 4.

4 * 32 + 16 = 44.

Not 112, but still not 208 either.

Maxime

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

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

[Jean-Francois Moine]

On Thu, 22 Oct 2015 10:47:35 +0200
Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:

> Not really. The uart0 reset is the bit 16, in the reset register 4.
> 
> 4 * 32 + 16 = 44.
> 
> Not 112, but still not 208 either.

The registers are numbered 1..5, then

(4 - 1) * 32 + 16 = 112

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

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

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 629 bytes --]

On Thu, Oct 22, 2015 at 10:57:45AM +0200, Jean-Francois Moine wrote:
> On Thu, 22 Oct 2015 10:47:35 +0200
> Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> 
> > Not really. The uart0 reset is the bit 16, in the reset register 4.
> > 
> > 4 * 32 + 16 = 44.
> > 
> > Not 112, but still not 208 either.
> 
> The registers are numbered 1..5, then
> 
> (4 - 1) * 32 + 16 = 112

Not on my version, and even then, UARTs are on the last reset
register, which would still make 144.

Maxime

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

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

[Jens Kuske]

On 22/10/15 11:14, Maxime Ripard wrote:
> On Thu, Oct 22, 2015 at 10:57:45AM +0200, Jean-Francois Moine wrote:
>> On Thu, 22 Oct 2015 10:47:35 +0200
>> Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>>
>>> Not really. The uart0 reset is the bit 16, in the reset register 4.
>>>
>>> 4 * 32 + 16 = 44.
>>>
>>> Not 112, but still not 208 either.
>>
>> The registers are numbered 1..5, then
>>
>> (4 - 1) * 32 + 16 = 112
> 
> Not on my version, and even then, UARTs are on the last reset
> register, which would still make 144.
> 
> Maxime
> 

There are holes between reg2 and reg3 and reg4 for some reason, but even
if we would correct that with some of_xlate() function they won't
completely line up with the gates.

Jens

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

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1087 bytes --]

On Thu, Oct 22, 2015 at 01:30:42PM +0200, Jens Kuske wrote:
> On 22/10/15 11:14, Maxime Ripard wrote:
> > On Thu, Oct 22, 2015 at 10:57:45AM +0200, Jean-Francois Moine wrote:
> >> On Thu, 22 Oct 2015 10:47:35 +0200
> >> Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> >>
> >>> Not really. The uart0 reset is the bit 16, in the reset register 4.
> >>>
> >>> 4 * 32 + 16 = 44.
> >>>
> >>> Not 112, but still not 208 either.
> >>
> >> The registers are numbered 1..5, then
> >>
> >> (4 - 1) * 32 + 16 = 112
> > 
> > Not on my version, and even then, UARTs are on the last reset
> > register, which would still make 144.
> > 
> > Maxime
> > 
> 
> There are holes between reg2 and reg3 and reg4 for some reason, but even
> if we would correct that with some of_xlate() function they won't
> completely line up with the gates.

Indeed. Still, dealing with the holes and sticking to what the
datasheet says seems like the right solution.

Maxime

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

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

[Jean-Francois Moine]

On Wed, 21 Oct 2015 18:20:27 +0200
Jens Kuske <jenskuske-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  arch/arm/boot/dts/sun8i-h3.dtsi | 499 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 499 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..4114e17
> --- /dev/null
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -0,0 +1,499 @@
	[snip]
> +		pll6: clk@01c20028 {
> +			#clock-cells = <1>;
> +			compatible = "allwinner,sun6i-a31-pll6-clk";
> +			reg = <0x01c20028 0x4>;
> +			clocks = <&osc24M>;
> +			clock-output-names = "pll6", "pll6x2", "pll6d2";
> +		};
> +
> +		pll8: clk@01c20044 {
> +			#clock-cells = <0>;

Should be <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";
> +		};
	[snip]

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

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

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1788 bytes --]

On Wed, Oct 21, 2015 at 06:20:27PM +0200, Jens Kuske wrote:
> +		bus_gates: clk@01c20060 {
> +			#clock-cells = <1>;
> +			compatible = "allwinner,sun8i-h3-bus-gates-clk";
> +			reg = <0x01c20060 0x14>;
> +			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>;
> +			clocks = <&ahb1>, <&ahb1>, <&ahb1>,
> +				 <&ahb1>, <&ahb1>, <&ahb1>,
> +				 <&ahb1>, <&ahb2>, <&ahb1>,
> +				 <&ahb1>, <&ahb1>,
> +				 <&ahb1>, <&ahb1>,
> +				 <&ahb1>, <&ahb1>,
> +				 <&ahb1>, <&ahb1>,
> +				 <&ahb1>, <&ahb2>,
> +				 <&ahb2>, <&ahb2>, <&ahb1>,
> +				 <&ahb1>, <&ahb1>, <&ahb1>,
> +				 <&ahb1>, <&ahb1>, <&ahb1>,
> +				 <&ahb1>, <&ahb1>, <&ahb1>,
> +				 <&ahb1>, <&apb1>,
> +				 <&apb1>, <&apb1>, <&apb1>,
> +				 <&apb1>, <&apb1>, <&apb1>,
> +				 <&apb2>, <&apb2>, <&apb2>,
> +				 <&apb2>, <&apb2>,
> +				 <&apb2>, <&apb2>, <&apb2>,
> +				 <&ahb1>, <&ahb1>;

This is not really what I had in mind...

This IP has 2 parents, and only two parents. The mapping between the
IPs should be done in the driver itself, not in the DT where it is
very error prone and barely readable.

And note that I never have expected you to use clk-simple-gates
either. This is a complicated clock, unlike the other we've seen so
far, it definitely deserves a driver of its own.

Thanks!
Maxime

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

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

[Jean-Francois Moine]

On Fri, 23 Oct 2015 20:14:06 +0200
Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:

> On Wed, Oct 21, 2015 at 06:20:27PM +0200, Jens Kuske wrote:
> > +		bus_gates: clk@01c20060 {
> > +			#clock-cells = <1>;
> > +			compatible = "allwinner,sun8i-h3-bus-gates-clk";
> > +			reg = <0x01c20060 0x14>;
> > +			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>;
> > +			clocks = <&ahb1>, <&ahb1>, <&ahb1>,
> > +				 <&ahb1>, <&ahb1>, <&ahb1>,
> > +				 <&ahb1>, <&ahb2>, <&ahb1>,
> > +				 <&ahb1>, <&ahb1>,
> > +				 <&ahb1>, <&ahb1>,
> > +				 <&ahb1>, <&ahb1>,
> > +				 <&ahb1>, <&ahb1>,
> > +				 <&ahb1>, <&ahb2>,
> > +				 <&ahb2>, <&ahb2>, <&ahb1>,
> > +				 <&ahb1>, <&ahb1>, <&ahb1>,
> > +				 <&ahb1>, <&ahb1>, <&ahb1>,
> > +				 <&ahb1>, <&ahb1>, <&ahb1>,
> > +				 <&ahb1>, <&apb1>,
> > +				 <&apb1>, <&apb1>, <&apb1>,
> > +				 <&apb1>, <&apb1>, <&apb1>,
> > +				 <&apb2>, <&apb2>, <&apb2>,
> > +				 <&apb2>, <&apb2>,
> > +				 <&apb2>, <&apb2>, <&apb2>,
> > +				 <&ahb1>, <&ahb1>;  
> 
> This is not really what I had in mind...
> 
> This IP has 2 parents, and only two parents. The mapping between the
> IPs should be done in the driver itself, not in the DT where it is
> very error prone and barely readable.
> 
> And note that I never have expected you to use clk-simple-gates
> either. This is a complicated clock, unlike the other we've seen so
> far, it definitely deserves a driver of its own.

It seems that Allwinner puts the gate definitions anywhere in the array
of registers, so, I think that the H3 scheme will not be the last
complicated one, and if the parent clocks are in the code instead of in
the DT, we will have more and more code to develop.

An other way to describe the gates would be to add containers per parent
(with still a small patch in the clk-simple-gates):

	bus_gates: clk@01c20060 {
		#clock-cells = <1>;
		compatible = "allwinner,sun8i-h3-bus-gates-clk";
		reg = <0x01c20060 0x14>;
		ahb1_gates {
			clocks = <&ahb1>;
			clock-indices = <5>, <6>, <8>,
					<9>, <10>, <13>,
					<14>, <18>,
					<19>, <20>,
					...;
			};
			clock-output-names = "ahb1_ce", "ahb1_dma", "ahb1_mmc0",
				"ahb1_mmc1", "ahb1_mmc2", "ahb1_nand",
				"ahb1_sdram", "ahb1_ts",
				"ahb1_hstimer", "ahb1_spi0",
				...;
		};
		ahb2_gates {
			clocks = <&ahb2>;
			clock-indices = <17>, <29>,
					<30>, <31>, <32>,
					...;
			clock-output-names = "ahb2_gmac", "ahb2_ohic1",
					"ahb2_ohic2", "ahb2_ohic3", "ahb1_ve",
					...;
		};
		apb1_gates {
			...
		};
		apb2_gates {
			...
		};
	};

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

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

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3659 bytes --]

On Fri, Oct 23, 2015 at 09:20:13PM +0200, Jean-Francois Moine wrote:
> On Fri, 23 Oct 2015 20:14:06 +0200
> Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> 
> > On Wed, Oct 21, 2015 at 06:20:27PM +0200, Jens Kuske wrote:
> > > +		bus_gates: clk@01c20060 {
> > > +			#clock-cells = <1>;
> > > +			compatible = "allwinner,sun8i-h3-bus-gates-clk";
> > > +			reg = <0x01c20060 0x14>;
> > > +			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>;
> > > +			clocks = <&ahb1>, <&ahb1>, <&ahb1>,
> > > +				 <&ahb1>, <&ahb1>, <&ahb1>,
> > > +				 <&ahb1>, <&ahb2>, <&ahb1>,
> > > +				 <&ahb1>, <&ahb1>,
> > > +				 <&ahb1>, <&ahb1>,
> > > +				 <&ahb1>, <&ahb1>,
> > > +				 <&ahb1>, <&ahb1>,
> > > +				 <&ahb1>, <&ahb2>,
> > > +				 <&ahb2>, <&ahb2>, <&ahb1>,
> > > +				 <&ahb1>, <&ahb1>, <&ahb1>,
> > > +				 <&ahb1>, <&ahb1>, <&ahb1>,
> > > +				 <&ahb1>, <&ahb1>, <&ahb1>,
> > > +				 <&ahb1>, <&apb1>,
> > > +				 <&apb1>, <&apb1>, <&apb1>,
> > > +				 <&apb1>, <&apb1>, <&apb1>,
> > > +				 <&apb2>, <&apb2>, <&apb2>,
> > > +				 <&apb2>, <&apb2>,
> > > +				 <&apb2>, <&apb2>, <&apb2>,
> > > +				 <&ahb1>, <&ahb1>;  
> > 
> > This is not really what I had in mind...
> > 
> > This IP has 2 parents, and only two parents. The mapping between the
> > IPs should be done in the driver itself, not in the DT where it is
> > very error prone and barely readable.
> > 
> > And note that I never have expected you to use clk-simple-gates
> > either. This is a complicated clock, unlike the other we've seen so
> > far, it definitely deserves a driver of its own.
> 
> It seems that Allwinner puts the gate definitions anywhere in the array
> of registers, so, I think that the H3 scheme will not be the last
> complicated one,

Maybe, but that's the first one. It doesn't prevent us from reusing
the driver later if it happens.

> and if the parent clocks are in the code instead of in the DT, we
> will have more and more code to develop.

I never asked that either.

> An other way to describe the gates would be to add containers per parent
> (with still a small patch in the clk-simple-gates):
> 
> 	bus_gates: clk@01c20060 {
> 		#clock-cells = <1>;
> 		compatible = "allwinner,sun8i-h3-bus-gates-clk";
> 		reg = <0x01c20060 0x14>;
> 		ahb1_gates {
> 			clocks = <&ahb1>;
> 			clock-indices = <5>, <6>, <8>,
> 					<9>, <10>, <13>,
> 					<14>, <18>,
> 					<19>, <20>,
> 					...;
> 			};
> 			clock-output-names = "ahb1_ce", "ahb1_dma", "ahb1_mmc0",
> 				"ahb1_mmc1", "ahb1_mmc2", "ahb1_nand",
> 				"ahb1_sdram", "ahb1_ts",
> 				"ahb1_hstimer", "ahb1_spi0",
> 				...;
> 		};
> 		ahb2_gates {
> 			clocks = <&ahb2>;
> 			clock-indices = <17>, <29>,
> 					<30>, <31>, <32>,
> 					...;
> 			clock-output-names = "ahb2_gmac", "ahb2_ohic1",
> 					"ahb2_ohic2", "ahb2_ohic3", "ahb1_ve",
> 					...;
> 		};
> 		apb1_gates {
> 			...
> 		};
> 		apb2_gates {
> 			...
> 		};
> 	};

Or simply

bus_gates {
	clocks = <&ahb1>, <&ahb2>;
	clock-indices = <5>, <6>, <8>, ...
	clock-output-names = "bus_ce", "bus_dma", "bus_mmc0"
};

Maxime

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

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

[Jean-Francois Moine]

On Sat, 24 Oct 2015 09:13:28 +0200
Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:

> Or simply
> 
> bus_gates {
> 	clocks = <&ahb1>, <&ahb2>;
> 	clock-indices = <5>, <6>, <8>, ...
> 	clock-output-names = "bus_ce", "bus_dma", "bus_mmc0"
> };

I don't understand: the apb1, apb2, ahb1 and ahb2 clocks may be
programmed independently to different frequencies and you have to know
which of them is the parent of each leaf clock.

So, either you hard-code the parents as Jens did in a first proposal,
or you define the full list of parents in the DT as in the last
proposal, or you use a container per parent in the DT as I proposed.

There could be an other solution using the output clock name to define
the parent clock:

bus_gates {
	clocks = <&ahb1>, <&ahb2>, <&apb1>, <&apb2>;
	clock-indices = <5>, <6>, <8>, ...
	clock-output-names = "ahb1_ce", "ahb1_dma", "ahb1_mmc0"
};

with the documentation:

	"the clocks MUST be defined in order: ahb1, ahb2, apb1, apb2."

and the code

	if (strncmp(clock_name, "ahb1", 4) == 0)
		clk_parent = of_clk_get_parent_name(node, 0);
	else if (..)

but it seems a bit hacky.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

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

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1626 bytes --]

On Sat, Oct 24, 2015 at 10:47:49AM +0200, Jean-Francois Moine wrote:
> On Sat, 24 Oct 2015 09:13:28 +0200
> Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> 
> > Or simply
> > 
> > bus_gates {
> > 	clocks = <&ahb1>, <&ahb2>;
> > 	clock-indices = <5>, <6>, <8>, ...
> > 	clock-output-names = "bus_ce", "bus_dma", "bus_mmc0"
> > };
> 
> I don't understand: the apb1, apb2, ahb1 and ahb2 clocks may be
> programmed independently to different frequencies

I don't understand why you're talking about frequencies here.

> and you have to know which of them is the parent of each leaf clock.

Indeed, but that's also doable here. Just not in the DT.

> So, either you hard-code the parents as Jens did in a first proposal,
> or you define the full list of parents in the DT as in the last
> proposal, or you use a container per parent in the DT as I proposed.
> 
> There could be an other solution using the output clock name to define
> the parent clock:
> 
> bus_gates {
> 	clocks = <&ahb1>, <&ahb2>, <&apb1>, <&apb2>;
> 	clock-indices = <5>, <6>, <8>, ...
> 	clock-output-names = "ahb1_ce", "ahb1_dma", "ahb1_mmc0"
> };
> 
> with the documentation:
> 
> 	"the clocks MUST be defined in order: ahb1, ahb2, apb1, apb2."
> 
> and the code
> 
> 	if (strncmp(clock_name, "ahb1", 4) == 0)
> 		clk_parent = of_clk_get_parent_name(node, 0);
> 	else if (..)
> 
> but it seems a bit hacky.

It's exactly what I suggested, without the string comparison, but
relying on the ID instead.

Maxime

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

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

[Hans de Goede]

Hi,

On 26-10-15 22:06, Maxime Ripard wrote:
> On Sat, Oct 24, 2015 at 10:47:49AM +0200, Jean-Francois Moine wrote:
>> On Sat, 24 Oct 2015 09:13:28 +0200
>> Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>>
>>> Or simply
>>>
>>> bus_gates {
>>> 	clocks = <&ahb1>, <&ahb2>;
>>> 	clock-indices = <5>, <6>, <8>, ...
>>> 	clock-output-names = "bus_ce", "bus_dma", "bus_mmc0"
>>> };
>>
>> I don't understand: the apb1, apb2, ahb1 and ahb2 clocks may be
>> programmed independently to different frequencies
>
> I don't understand why you're talking about frequencies here.
>
>> and you have to know which of them is the parent of each leaf clock.
>
> Indeed, but that's also doable here. Just not in the DT.
>
>> So, either you hard-code the parents as Jens did in a first proposal,
>> or you define the full list of parents in the DT as in the last
>> proposal, or you use a container per parent in the DT as I proposed.
>>
>> There could be an other solution using the output clock name to define
>> the parent clock:
>>
>> bus_gates {
>> 	clocks = <&ahb1>, <&ahb2>, <&apb1>, <&apb2>;
>> 	clock-indices = <5>, <6>, <8>, ...
>> 	clock-output-names = "ahb1_ce", "ahb1_dma", "ahb1_mmc0"
>> };
>>
>> with the documentation:
>>
>> 	"the clocks MUST be defined in order: ahb1, ahb2, apb1, apb2."
>>
>> and the code
>>
>> 	if (strncmp(clock_name, "ahb1", 4) == 0)
>> 		clk_parent = of_clk_get_parent_name(node, 0);
>> 	else if (..)
>>
>> but it seems a bit hacky.
>
> It's exactly what I suggested, without the string comparison, but
> relying on the ID instead.

I'm not following you here, what do you mean with "the ID" ?

Regards,

Hans

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

[Hans de Goede]

Hi,

On 10/23/2015 08:14 PM, Maxime Ripard wrote:
> On Wed, Oct 21, 2015 at 06:20:27PM +0200, Jens Kuske wrote:
>> +		bus_gates: clk@01c20060 {
>> +			#clock-cells = <1>;
>> +			compatible = "allwinner,sun8i-h3-bus-gates-clk";
>> +			reg = <0x01c20060 0x14>;
>> +			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>;
>> +			clocks = <&ahb1>, <&ahb1>, <&ahb1>,
>> +				 <&ahb1>, <&ahb1>, <&ahb1>,
>> +				 <&ahb1>, <&ahb2>, <&ahb1>,
>> +				 <&ahb1>, <&ahb1>,
>> +				 <&ahb1>, <&ahb1>,
>> +				 <&ahb1>, <&ahb1>,
>> +				 <&ahb1>, <&ahb1>,
>> +				 <&ahb1>, <&ahb2>,
>> +				 <&ahb2>, <&ahb2>, <&ahb1>,
>> +				 <&ahb1>, <&ahb1>, <&ahb1>,
>> +				 <&ahb1>, <&ahb1>, <&ahb1>,
>> +				 <&ahb1>, <&ahb1>, <&ahb1>,
>> +				 <&ahb1>, <&apb1>,
>> +				 <&apb1>, <&apb1>, <&apb1>,
>> +				 <&apb1>, <&apb1>, <&apb1>,
>> +				 <&apb2>, <&apb2>, <&apb2>,
>> +				 <&apb2>, <&apb2>,
>> +				 <&apb2>, <&apb2>, <&apb2>,
>> +				 <&ahb1>, <&ahb1>;
>
> This is not really what I had in mind...

I came to the same solution independently, I took my inspiration from
the rockchip clocks driver which is dealing with this problem in the
same way, so there is precedent for doing things this way, and this
does give us lot of flexibility. Given that I expect other new allwinnner
SoCs to have the same problem I believe it is good to have that
flexibility.

> This IP has 2 parents, and only two parents.

Nope it has 4: apb1, apb2, ahb1 and ahb2.

> The mapping between the
> IPs should be done in the driver itself, not in the DT where it is
> very error prone and barely readable.

It is just as error prone and barely readable in C-code, see Jens original
patchset, he did an array of clock indices there (range 0-3 with an index
into the parent clocks array), which is arguably even more unreadable since
there is an extra level of indirection here.

The problem with the unreadability simply comes from allwinner's decision
to no longer have a gates register per bus but instead shove everything
in a single bit-array in random order, there is nothing we can do to fix
that.

Also the argument "this belongs in the driver not in the DT" is a bit
inconsistent with the moving of the mask of valid gates from the
driver into the clock-indices in devicetree. The way things are done
here actually are doing pretty much the same thing, putting info which
could be derived from the compatible string into devicetree.

Last as said already there is precedent for doing things this way
in the rockchip driver, and given that 2 people have come up
with this approach independently of of each other this clearly
seems to be the most straight-foward / logical way to deal with
this.

> And note that I never have expected you to use clk-simple-gates
> either. This is a complicated clock

No it is not complicated, have you looked at the changes to the
simple-clk-gates driver which Jean Francois Moine suggested?

Those 5 extra lines (4 new lines) are all that is needed when
going with the approach of listing a parent per gate. This is
actually still a quite simple clock, we only need to find a way
to specify a parent per gate, preferably via DT since this gives
us greater flexibility which will be quite useful when adding
support for other SoCs.

Regards,

Hans

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

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 5447 bytes --]

On Sat, Oct 24, 2015 at 10:39:49AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 10/23/2015 08:14 PM, Maxime Ripard wrote:
> >On Wed, Oct 21, 2015 at 06:20:27PM +0200, Jens Kuske wrote:
> >>+		bus_gates: clk@01c20060 {
> >>+			#clock-cells = <1>;
> >>+			compatible = "allwinner,sun8i-h3-bus-gates-clk";
> >>+			reg = <0x01c20060 0x14>;
> >>+			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>;
> >>+			clocks = <&ahb1>, <&ahb1>, <&ahb1>,
> >>+				 <&ahb1>, <&ahb1>, <&ahb1>,
> >>+				 <&ahb1>, <&ahb2>, <&ahb1>,
> >>+				 <&ahb1>, <&ahb1>,
> >>+				 <&ahb1>, <&ahb1>,
> >>+				 <&ahb1>, <&ahb1>,
> >>+				 <&ahb1>, <&ahb1>,
> >>+				 <&ahb1>, <&ahb2>,
> >>+				 <&ahb2>, <&ahb2>, <&ahb1>,
> >>+				 <&ahb1>, <&ahb1>, <&ahb1>,
> >>+				 <&ahb1>, <&ahb1>, <&ahb1>,
> >>+				 <&ahb1>, <&ahb1>, <&ahb1>,
> >>+				 <&ahb1>, <&apb1>,
> >>+				 <&apb1>, <&apb1>, <&apb1>,
> >>+				 <&apb1>, <&apb1>, <&apb1>,
> >>+				 <&apb2>, <&apb2>, <&apb2>,
> >>+				 <&apb2>, <&apb2>,
> >>+				 <&apb2>, <&apb2>, <&apb2>,
> >>+				 <&ahb1>, <&ahb1>;
> >
> >This is not really what I had in mind...
> 
> I came to the same solution independently, I took my inspiration from
> the rockchip clocks driver which is dealing with this problem in the
> same way, so there is precedent for doing things this way, and this
> does give us lot of flexibility. Given that I expect other new allwinnner
> SoCs to have the same problem I believe it is good to have that
> flexibility.

The rockchip clocks driver are very different from our own regarding
the DT bindings. Being consistent within our own platform seems to
bring much more value than getting bits and pieces here and there when
it's convenient.

Plus, you actually forgot in your argument to mention that this
binding was documented as deprecated, and not used anywhere since
3.17... So apparently, someone tried that, and finally changed its
mind.

Again, this is clearly a workaround, with no way to easily identify if
a given clock has the right parent afterwards. We can't review it
properly, and it's going to be a pain to fix after the facts.

Having some association masks in the driver itself, using the BIT
macro, will be much easier to maintain in the long run.

> >This IP has 2 parents, and only two parents.
> 
> Nope it has 4: apb1, apb2, ahb1 and ahb2.

The point still remains though.

> >The mapping between the IPs should be done in the driver itself,
> >not in the DT where it is very error prone and barely readable.
> 
> It is just as error prone and barely readable in C-code, see Jens original
> patchset, he did an array of clock indices there (range 0-3 with an index
> into the parent clocks array), which is arguably even more unreadable since
> there is an extra level of indirection here.

I agree, and it's why I suggested another approach at the time.

> The problem with the unreadability simply comes from allwinner's decision
> to no longer have a gates register per bus but instead shove everything
> in a single bit-array in random order, there is nothing we can do to fix
> that.

Indeed. Except one solution is easier to maintain than the other.

> Also the argument "this belongs in the driver not in the DT" is a
> bit inconsistent with the moving of the mask of valid gates from the
> driver into the clock-indices in devicetree.

Except I never used that argument.

> The way things are done here actually are doing pretty much the same
> thing, putting info which could be derived from the compatible
> string into devicetree.
>
> Last as said already there is precedent for doing things this way
> in the rockchip driver, and given that 2 people have come up
> with this approach independently of of each other this clearly
> seems to be the most straight-foward / logical way to deal with
> this.
> 
> >And note that I never have expected you to use clk-simple-gates
> >either. This is a complicated clock
> 
> No it is not complicated, have you looked at the changes to the
> simple-clk-gates driver which Jean Francois Moine suggested?
> 
> Those 5 extra lines (4 new lines) are all that is needed when
> going with the approach of listing a parent per gate. This is
> actually still a quite simple clock, we only need to find a way
> to specify a parent per gate, preferably via DT since this gives
> us greater flexibility which will be quite useful when adding
> support for other SoCs.

The problem boils down to this: so far, we've used one DT node per
clock controller (and we won't change that, sorry).

The clocks property is defined as "List of phandle and clock specifier
pairs, one pair for each clock input to the device.".

There's only 4 clock inputs. Really. What this clock controller
controls is whether one of these four input will be output, and that's
pretty much it. The routing between the input and output pins is
internal to the controller, just like it should be internal to the
driver.

Maxime

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

[PATCH 6/6] ARM: dts: sun8i: Add Orange Pi Plus support

[Jens Kuske]

The Orange Pi Plus is a SBC based on the Allwinner H3 SoC
with 8GB eMMC, multiple USB ports through a USB hub chip, SATA through
a USB-SATA bridge, one uSD slot, a 10/100/1000M ethernet port,
WiFi, HDMI, headphone jack, IR receiver, a microphone, a CSI connector
and a 40-pin GPIO header.

Signed-off-by: Jens Kuske <jenskuske-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/arm/boot/dts/Makefile                   |  3 +-
 arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts | 77 ++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 35a3cf4..bedf51b 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -643,7 +643,8 @@ dtb-$(CONFIG_MACH_SUN8I) += \
 	sun8i-a33-ga10h-v1.1.dtb \
 	sun8i-a33-ippo-q8h-v1.2.dtb \
 	sun8i-a33-q8-tablet.dtb \
-	sun8i-a33-sinlinx-sina33.dtb
+	sun8i-a33-sinlinx-sina33.dtb \
+	sun8i-h3-orangepi-plus.dtb
 dtb-$(CONFIG_MACH_SUN9I) += \
 	sun9i-a80-optimus.dtb \
 	sun9i-a80-cubieboard4.dtb
diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts
new file mode 100644
index 0000000..e67df59
--- /dev/null
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts
@@ -0,0 +1,77 @@
+/*
+ * Copyright (C) 2015 Jens Kuske <jenskuske-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+ *
+ * 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.
+ */
+
+/dts-v1/;
+#include "sun8i-h3.dtsi"
+#include "sunxi-common-regulators.dtsi"
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/pinctrl/sun4i-a10.h>
+
+/ {
+	model = "Xunlong Orange Pi Plus";
+	compatible = "xunlong,orangepi-plus", "allwinner,sun8i-h3";
+
+	aliases {
+		serial0 = &uart0;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+};
+
+&mmc0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&mmc0_pins_a>, <&mmc0_cd_pin>;
+	vmmc-supply = <&reg_vcc3v3>;
+	bus-width = <4>;
+	cd-gpios = <&pio 5 6 GPIO_ACTIVE_HIGH>; /* PF6 */
+	cd-inverted;
+	status = "okay";
+};
+
+&uart0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart0_pins_a>;
+	status = "okay";
+};
-- 
2.6.1

Re: [PATCH 4/6] reset: sunxi: Add compatible for Allwinner H3 bus resets

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 537 bytes --]

On Wed, Oct 21, 2015 at 06:20:26PM +0200, Jens Kuske wrote:
> Adding a new compatible allows us to define SoC specific behaviour
> if necessary, for example forcing a particular device out of reset
> even if no driver is actually using it.
> 
> Signed-off-by: Jens Kuske <jenskuske-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Acked-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Thanks!
Maxime

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

Re: [PATCH 4/6] reset: sunxi: Add compatible for Allwinner H3 bus resets

[Jens Kuske]

On 22/10/15 09:58, Maxime Ripard wrote:
> On Wed, Oct 21, 2015 at 06:20:26PM +0200, Jens Kuske wrote:
>> Adding a new compatible allows us to define SoC specific behaviour
>> if necessary, for example forcing a particular device out of reset
>> even if no driver is actually using it.
>>
>> Signed-off-by: Jens Kuske <jenskuske-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> Acked-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> 
> Thanks!
> Maxime
> 

Hi,

I've send a new version of this patch, please don't apply this version.

Jens
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html