[RFC PATCH 0/4] clk: sunxi: fix DT compatibility issues

[RFC PATCH 0/4] clk: sunxi: fix DT compatibility issues

[Andre Przywara]

Commit f7d372ba54ea ("clk: sunxi: Refactor A31 PLL6 so that it can be
reused") (in -next) made the A31 PLL6 clock driver more generic, so
that it can drive the PLL8 clock in newer SoCs too.
However the patch broke compatibility with older DTs, which this
series tries to fix.
The approach chosen here is to bring back the old driver under its
old name, while letting the new driver using a different name to be
able to tell them apart.
The old driver should be somewhat deprecated and not used in new DTs
anymore.
The slight disadvantage is that there are now two drivers and two
compatible names for the same hardware (the PLL6 clock), I am not
sure if this is frowned upon or can be tolerated since the new driver
is more generic (drives PLL8 as well) and makes the old one obsolete.
We just need to keep it for compatibility.

The naming for both the functions and compatible names is probably
wrong, I am relying on more sunxi - experienced people here to
suggest better identifiers.

This is only one possible approach to fix this issues, so I am open
to any kind of discussion.

The series is made on top of Maxime's sunxi/for-next branch, so it
somehow reverts the change in question. I am happy to rebase it on
any branch people tell me.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Andre Przywara (4):
  clk: sunxi: rename new sun6i_a31_pll6 clock to sun6i_a31_pll clock
  clk: sunxi: re-add old sun6i_a31_pll6 clock
  clk: sunxi: revert .dtsi changes for DTs with a sun6i_a31_pll6 clock
  DT: Allwinner H3: fix PLL8 clock

 Documentation/devicetree/bindings/clock/sunxi.txt |  2 +-
 arch/arm/boot/dts/sun6i-a31.dtsi                  | 36 ++++++++---------
 arch/arm/boot/dts/sun8i-a23-a33.dtsi              | 25 ++++--------
 arch/arm/boot/dts/sun8i-a23.dtsi                  |  2 +-
 arch/arm/boot/dts/sun8i-a33.dtsi                  |  4 +-
 arch/arm/boot/dts/sun8i-h3.dtsi                   | 31 +++++---------
 drivers/clk/sunxi/clk-sunxi.c                     | 49 ++++++++++++++++++++---
 7 files changed, 84 insertions(+), 65 deletions(-)

-- 
2.6.4

[RFC PATCH 1/4] clk: sunxi: rename new sun6i_a31_pll6 clock to sun6i_a31_pll clock

[Andre Przywara]

In preparation for bringing back the old driver for keeping
compatibility, we rename the new A31 PLL6 clock driver. This only
affects the internal naming, the compatible name stays on pll6 for
now.

Signed-off-by: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org>
---
 drivers/clk/sunxi/clk-sunxi.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 48ed81d..b4ab84b 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -233,7 +233,7 @@ static void sun4i_get_pll5_factors(struct factors_request *req)
  * parent_rate is always 24Mhz
  */
 
-static void sun6i_a31_get_pll6_factors(struct factors_request *req)
+static void sun6i_a31_get_pll_factors(struct factors_request *req)
 {
 	u8 div;
 
@@ -542,10 +542,10 @@ static const struct factors_data sun4i_pll6_data __initconst = {
 	.name = "pll6",
 };
 
-static const struct factors_data sun6i_a31_pll6_data __initconst = {
+static const struct factors_data sun6i_a31_pll_data __initconst = {
 	.enable = 31,
 	.table = &sun6i_a31_pll6_config,
-	.getter = sun6i_a31_get_pll6_factors,
+	.getter = sun6i_a31_get_pll_factors,
 	.recalc = sun6i_a31_pll6_recalc,
 };
 
@@ -622,12 +622,12 @@ static void __init sun7i_pll4_clk_setup(struct device_node *node)
 CLK_OF_DECLARE(sun7i_pll4, "allwinner,sun7i-a20-pll4-clk",
 	       sun7i_pll4_clk_setup);
 
-static void __init sun6i_pll6_clk_setup(struct device_node *node)
+static void __init sun6i_pll_clk_setup(struct device_node *node)
 {
-	sunxi_factors_clk_setup(node, &sun6i_a31_pll6_data);
+	sunxi_factors_clk_setup(node, &sun6i_a31_pll_data);
 }
-CLK_OF_DECLARE(sun6i_pll6, "allwinner,sun6i-a31-pll6-clk",
-	       sun6i_pll6_clk_setup);
+CLK_OF_DECLARE(sun6i_pll, "allwinner,sun6i-a31-pll6-clk",
+	       sun6i_pll_clk_setup);
 
 static void __init sun5i_ahb_clk_setup(struct device_node *node)
 {
-- 
2.6.4

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

[RFC PATCH 2/4] clk: sunxi: re-add old sun6i_a31_pll6 clock

[Andre Przywara]

The older version of the A31 PLL6 clock driver was used by many DTs
which we want to stay compatible with.
As an intermediate step bring back the code for the old driver, but
don't instantiate it yet.

Signed-off-by: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org>
---
 drivers/clk/sunxi/clk-sunxi.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index b4ab84b..12131ba 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -248,6 +248,21 @@ static void sun6i_a31_get_pll_factors(struct factors_request *req)
 	req->n = DIV_ROUND_UP(div, (req->k + 1)) - 1;
 }
 
+static void sun6i_a31_get_pll6_factors(struct factors_request *req)
+{
+	u8 div;
+
+	/* Normalize value to a parent_rate multiple (24M) */
+	div = req->rate / req->parent_rate;
+	req->rate = req->parent_rate * div;
+
+	req->k = div / 32;
+	if (req->k > 3)
+		req->k = 3;
+
+	req->n = DIV_ROUND_UP(div, (req->k + 1)) - 1;
+}
+
 static void sun6i_a31_pll6_recalc(struct factors_request *req)
 {
 	req->rate = req->parent_rate;
@@ -549,6 +564,13 @@ static const struct factors_data sun6i_a31_pll_data __initconst = {
 	.recalc = sun6i_a31_pll6_recalc,
 };
 
+static const struct factors_data sun6i_a31_pll6_data __initconst = {
+	.enable = 31,
+	.table = &sun6i_a31_pll6_config,
+	.getter = sun6i_a31_get_pll6_factors,
+	.name	= "pll6x2",
+};
+
 static const struct factors_data sun5i_a13_ahb_data __initconst = {
 	.mux = 6,
 	.muxmask = BIT(1) | BIT(0),
@@ -954,6 +976,15 @@ static const struct divs_data pll6_divs_data __initconst = {
 	}
 };
 
+static const struct divs_data sun6i_a31_pll6_divs_data __initconst = {
+	.factors = &sun6i_a31_pll6_data,
+	.ndivs = 2,
+	.div = {
+		{ .fixed = 2 }, /* normal output */
+		{ .self = 1 }, /* base factor clock, 2x */
+	}
+};
+
 /**
  * sunxi_divs_clk_setup() - Setup function for leaf divisors on clocks
  *
-- 
2.6.4

[RFC PATCH 3/4] clk: sunxi: revert .dtsi changes for DTs with a sun6i_a31_pll6 clock

[Andre Przywara]

The changes made to the driver broke compatibilty with older DTs.
For most of the boards the new driver does not bring any advantage,
so for the sake of compatibility bring them back to the previous
version.
This also affects the new H3 .dtsi - at least for this patch, the
next one will fix this.

Signed-off-by: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org>
---
 Documentation/devicetree/bindings/clock/sunxi.txt |  2 +-
 arch/arm/boot/dts/sun6i-a31.dtsi                  | 36 +++++++++++-----------
 arch/arm/boot/dts/sun8i-a23-a33.dtsi              | 25 +++++----------
 arch/arm/boot/dts/sun8i-a23.dtsi                  |  2 +-
 arch/arm/boot/dts/sun8i-a33.dtsi                  |  4 +--
 arch/arm/boot/dts/sun8i-h3.dtsi                   | 37 +++++++++--------------
 drivers/clk/sunxi/clk-sunxi.c                     |  8 ++++-
 7 files changed, 51 insertions(+), 63 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
index 966dcaf..c09f59b 100644
--- a/Documentation/devicetree/bindings/clock/sunxi.txt
+++ b/Documentation/devicetree/bindings/clock/sunxi.txt
@@ -88,7 +88,7 @@ Required properties for all clocks:
 - #clock-cells : from common clock binding; shall be set to 0 except for
 	the following compatibles where it shall be set to 1:
 	"allwinner,*-gates-clk", "allwinner,sun4i-pll5-clk",
-	"allwinner,sun4i-pll6-clk",
+	"allwinner,sun4i-pll6-clk", "allwinner,sun6i-a31-pll6-clk",
 	"allwinner,*-usb-clk", "allwinner,*-mmc-clk",
 	"allwinner,*-mmc-config-clk"
 - clock-output-names : shall be the corresponding names of the outputs.
diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index 7a198dc..1867af2 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -65,7 +65,7 @@
 			compatible = "allwinner,simple-framebuffer",
 				     "simple-framebuffer";
 			allwinner,pipeline = "de_be0-lcd0-hdmi";
-			clocks = <&pll6>;
+			clocks = <&pll6 0>;
 			status = "disabled";
 		};
 
@@ -73,7 +73,7 @@
 			compatible = "allwinner,simple-framebuffer",
 				     "simple-framebuffer";
 			allwinner,pipeline = "de_be0-lcd0";
-			clocks = <&pll6>;
+			clocks = <&pll6 0>;
 			status = "disabled";
 		};
 	};
@@ -201,11 +201,11 @@
 		};
 
 		pll6: clk@01c20028 {
-			#clock-cells = <0>;
+			#clock-cells = <1>;
 			compatible = "allwinner,sun6i-a31-pll6-clk";
 			reg = <0x01c20028 0x4>;
 			clocks = <&osc24M>;
-			clock-output-names = "pll6";
+			clock-output-names = "pll6", "pll6x2";
 		};
 
 		cpu: cpu@01c20050 {
@@ -235,7 +235,7 @@
 			#clock-cells = <0>;
 			compatible = "allwinner,sun6i-a31-ahb1-clk";
 			reg = <0x01c20054 0x4>;
-			clocks = <&osc32k>, <&osc24M>, <&axi>, <&pll6>;
+			clocks = <&osc32k>, <&osc24M>, <&axi>, <&pll6 0>;
 			clock-output-names = "ahb1";
 
 			/*
@@ -244,7 +244,7 @@
 			 * controller requires AHB1 clocked from PLL6.
 			 */
 			assigned-clocks = <&ahb1>;
-			assigned-clock-parents = <&pll6>;
+			assigned-clock-parents = <&pll6 0>;
 		};
 
 		ahb1_gates: clk@01c20060 {
@@ -307,7 +307,7 @@
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-apb1-clk";
 			reg = <0x01c20058 0x4>;
-			clocks = <&osc32k>, <&osc24M>, <&pll6>, <&pll6>;
+			clocks = <&osc32k>, <&osc24M>, <&pll6 0>, <&pll6 0>;
 			clock-output-names = "apb2";
 		};
 
@@ -331,7 +331,7 @@
 			#clock-cells = <1>;
 			compatible = "allwinner,sun4i-a10-mmc-clk";
 			reg = <0x01c20088 0x4>;
-			clocks = <&osc24M>, <&pll6>;
+			clocks = <&osc24M>, <&pll6 0>;
 			clock-output-names = "mmc0",
 					     "mmc0_output",
 					     "mmc0_sample";
@@ -341,7 +341,7 @@
 			#clock-cells = <1>;
 			compatible = "allwinner,sun4i-a10-mmc-clk";
 			reg = <0x01c2008c 0x4>;
-			clocks = <&osc24M>, <&pll6>;
+			clocks = <&osc24M>, <&pll6 0>;
 			clock-output-names = "mmc1",
 					     "mmc1_output",
 					     "mmc1_sample";
@@ -351,7 +351,7 @@
 			#clock-cells = <1>;
 			compatible = "allwinner,sun4i-a10-mmc-clk";
 			reg = <0x01c20090 0x4>;
-			clocks = <&osc24M>, <&pll6>;
+			clocks = <&osc24M>, <&pll6 0>;
 			clock-output-names = "mmc2",
 					     "mmc2_output",
 					     "mmc2_sample";
@@ -361,7 +361,7 @@
 			#clock-cells = <1>;
 			compatible = "allwinner,sun4i-a10-mmc-clk";
 			reg = <0x01c20094 0x4>;
-			clocks = <&osc24M>, <&pll6>;
+			clocks = <&osc24M>, <&pll6 0>;
 			clock-output-names = "mmc3",
 					     "mmc3_output",
 					     "mmc3_sample";
@@ -371,7 +371,7 @@
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-mod0-clk";
 			reg = <0x01c2009c 0x4>;
-			clocks = <&osc24M>, <&pll6>;
+			clocks = <&osc24M>, <&pll6 0>;
 			clock-output-names = "ss";
 		};
 
@@ -379,7 +379,7 @@
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-mod0-clk";
 			reg = <0x01c200a0 0x4>;
-			clocks = <&osc24M>, <&pll6>;
+			clocks = <&osc24M>, <&pll6 0>;
 			clock-output-names = "spi0";
 		};
 
@@ -387,7 +387,7 @@
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-mod0-clk";
 			reg = <0x01c200a4 0x4>;
-			clocks = <&osc24M>, <&pll6>;
+			clocks = <&osc24M>, <&pll6 0>;
 			clock-output-names = "spi1";
 		};
 
@@ -395,7 +395,7 @@
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-mod0-clk";
 			reg = <0x01c200a8 0x4>;
-			clocks = <&osc24M>, <&pll6>;
+			clocks = <&osc24M>, <&pll6 0>;
 			clock-output-names = "spi2";
 		};
 
@@ -403,7 +403,7 @@
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-mod0-clk";
 			reg = <0x01c200ac 0x4>;
-			clocks = <&osc24M>, <&pll6>;
+			clocks = <&osc24M>, <&pll6 0>;
 			clock-output-names = "spi3";
 		};
 
@@ -1052,8 +1052,8 @@
 			ar100: ar100_clk {
 				compatible = "allwinner,sun6i-a31-ar100-clk";
 				#clock-cells = <0>;
-				clocks = <&osc32k>, <&osc24M>, <&pll6>,
-					 <&pll6>;
+				clocks = <&osc32k>, <&osc24M>, <&pll6 0>,
+					 <&pll6 0>;
 				clock-output-names = "ar100";
 			};
 
diff --git a/arch/arm/boot/dts/sun8i-a23-a33.dtsi b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
index 783b4b8..7e05e09 100644
--- a/arch/arm/boot/dts/sun8i-a23-a33.dtsi
+++ b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
@@ -60,7 +60,7 @@
 			compatible = "allwinner,simple-framebuffer",
 				     "simple-framebuffer";
 			allwinner,pipeline = "de_be0-lcd0";
-			clocks = <&pll6>;
+			clocks = <&pll6 0>;
 			status = "disabled";
 		};
 	};
@@ -129,20 +129,11 @@
 		};
 
 		pll6: clk@01c20028 {
-			#clock-cells = <0>;
+			#clock-cells = <1>;
 			compatible = "allwinner,sun6i-a31-pll6-clk";
 			reg = <0x01c20028 0x4>;
 			clocks = <&osc24M>;
-			clock-output-names = "pll6";
-		};
-
-                pll6x2: pll6x2_clk {
-			compatible = "fixed-factor-clock";
-			#clock-cells = <0>;
-			clock-div = <1>;
-			clock-mult = <2>;
-			clocks = <&pll6>;
-			clock-output-names = "pll6-2x";
+			clock-output-names = "pll6", "pll6x2";
 		};
 
 		cpu: cpu_clk@01c20050 {
@@ -172,7 +163,7 @@
 			#clock-cells = <0>;
 			compatible = "allwinner,sun6i-a31-ahb1-clk";
 			reg = <0x01c20054 0x4>;
-			clocks = <&osc32k>, <&osc24M>, <&axi>, <&pll6>;
+			clocks = <&osc32k>, <&osc24M>, <&axi>, <&pll6 0>;
 			clock-output-names = "ahb1";
 		};
 
@@ -199,7 +190,7 @@
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-apb1-clk";
 			reg = <0x01c20058 0x4>;
-			clocks = <&osc32k>, <&osc24M>, <&pll6>, <&pll6>;
+			clocks = <&osc32k>, <&osc24M>, <&pll6 0>, <&pll6 0>;
 			clock-output-names = "apb2";
 		};
 
@@ -222,7 +213,7 @@
 			#clock-cells = <1>;
 			compatible = "allwinner,sun4i-a10-mmc-clk";
 			reg = <0x01c20088 0x4>;
-			clocks = <&osc24M>, <&pll6>;
+			clocks = <&osc24M>, <&pll6 0>;
 			clock-output-names = "mmc0",
 					     "mmc0_output",
 					     "mmc0_sample";
@@ -232,7 +223,7 @@
 			#clock-cells = <1>;
 			compatible = "allwinner,sun4i-a10-mmc-clk";
 			reg = <0x01c2008c 0x4>;
-			clocks = <&osc24M>, <&pll6>;
+			clocks = <&osc24M>, <&pll6 0>;
 			clock-output-names = "mmc1",
 					     "mmc1_output",
 					     "mmc1_sample";
@@ -242,7 +233,7 @@
 			#clock-cells = <1>;
 			compatible = "allwinner,sun4i-a10-mmc-clk";
 			reg = <0x01c20090 0x4>;
-			clocks = <&osc24M>, <&pll6>;
+			clocks = <&osc24M>, <&pll6 0>;
 			clock-output-names = "mmc2",
 					     "mmc2_output",
 					     "mmc2_sample";
diff --git a/arch/arm/boot/dts/sun8i-a23.dtsi b/arch/arm/boot/dts/sun8i-a23.dtsi
index 5e589c1..92e6616 100644
--- a/arch/arm/boot/dts/sun8i-a23.dtsi
+++ b/arch/arm/boot/dts/sun8i-a23.dtsi
@@ -79,7 +79,7 @@
 			#clock-cells = <0>;
 			compatible = "allwinner,sun8i-a23-mbus-clk";
 			reg = <0x01c2015c 0x4>;
-			clocks = <&osc24M>, <&pll6x2>, <&pll5>;
+			clocks = <&osc24M>, <&pll6 1>, <&pll5>;
 			clock-output-names = "mbus";
 		};
 	};
diff --git a/arch/arm/boot/dts/sun8i-a33.dtsi b/arch/arm/boot/dts/sun8i-a33.dtsi
index f3eb618..001d840 100644
--- a/arch/arm/boot/dts/sun8i-a33.dtsi
+++ b/arch/arm/boot/dts/sun8i-a33.dtsi
@@ -103,7 +103,7 @@
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-mod0-clk";
 			reg = <0x01c2009c 0x4>;
-			clocks = <&osc24M>, <&pll6>;
+			clocks = <&osc24M>, <&pll6 0>;
 			clock-output-names = "ss";
 		};
 
@@ -111,7 +111,7 @@
 			#clock-cells = <0>;
 			compatible = "allwinner,sun8i-a23-mbus-clk";
 			reg = <0x01c2015c 0x4>;
-			clocks = <&osc24M>, <&pll6x2>, <&pll5>, <&pll11>;
+			clocks = <&osc24M>, <&pll6 1>, <&pll5>, <&pll11>;
 			clock-output-names = "mbus";
 		};
 	};
diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 6f6b4e4..1524130e 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -121,11 +121,11 @@
 		};
 
 		pll6: clk@01c20028 {
-			#clock-cells = <0>;
+			#clock-cells = <1>;
 			compatible = "allwinner,sun6i-a31-pll6-clk";
 			reg = <0x01c20028 0x4>;
 			clocks = <&osc24M>;
-			clock-output-names = "pll6";
+			clock-output-names = "pll6", "pll6x2";
 		};
 
 		pll6d2: pll6d2_clk {
@@ -133,24 +133,15 @@
 			compatible = "fixed-factor-clock";
 			clock-div = <2>;
 			clock-mult = <1>;
-			clocks = <&pll6>;
-			clock-output-names = "pll6-d2";
+			clocks = <&pll6 0>;
+			clock-output-names = "pll6d2";
 		};
 
-		pll6x2: pll6x2_clk {
+		/* dummy clock until pll6 can be reused */
+		pll8: pll8_clk {
 			#clock-cells = <0>;
-			compatible = "fixed-factor-clock";
-			clock-div = <1>;
-			clock-mult = <2>;
-			clocks = <&pll6>;
-			clock-output-names = "pll6-2x";
-		};
-
-		pll8: clk@01c20044 {
-			#clock-cells = <0>;
-			compatible = "allwinner,sun6i-a31-pll6-clk";
-			reg = <0x01c20044 0x4>;
-			clocks = <&osc24M>;
+			compatible = "fixed-clock";
+			clock-frequency = <1>;
 			clock-output-names = "pll8";
 		};
 
@@ -174,7 +165,7 @@
 			#clock-cells = <0>;
 			compatible = "allwinner,sun6i-a31-ahb1-clk";
 			reg = <0x01c20054 0x4>;
-			clocks = <&osc32k>, <&osc24M>, <&axi>, <&pll6>;
+			clocks = <&osc32k>, <&osc24M>, <&axi>, <&pll6 0>;
 			clock-output-names = "ahb1";
 		};
 
@@ -198,7 +189,7 @@
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-apb1-clk";
 			reg = <0x01c20058 0x4>;
-			clocks = <&osc32k>, <&osc24M>, <&pll6>, <&pll6>;
+			clocks = <&osc32k>, <&osc24M>, <&pll6 0>, <&pll6 0>;
 			clock-output-names = "apb2";
 		};
 
@@ -252,7 +243,7 @@
 			#clock-cells = <1>;
 			compatible = "allwinner,sun4i-a10-mmc-clk";
 			reg = <0x01c20088 0x4>;
-			clocks = <&osc24M>, <&pll6>, <&pll8>;
+			clocks = <&osc24M>, <&pll6 0>, <&pll8>;
 			clock-output-names = "mmc0",
 					     "mmc0_output",
 					     "mmc0_sample";
@@ -262,7 +253,7 @@
 			#clock-cells = <1>;
 			compatible = "allwinner,sun4i-a10-mmc-clk";
 			reg = <0x01c2008c 0x4>;
-			clocks = <&osc24M>, <&pll6>, <&pll8>;
+			clocks = <&osc24M>, <&pll6 0>, <&pll8>;
 			clock-output-names = "mmc1",
 					     "mmc1_output",
 					     "mmc1_sample";
@@ -272,7 +263,7 @@
 			#clock-cells = <1>;
 			compatible = "allwinner,sun4i-a10-mmc-clk";
 			reg = <0x01c20090 0x4>;
-			clocks = <&osc24M>, <&pll6>, <&pll8>;
+			clocks = <&osc24M>, <&pll6 0>, <&pll8>;
 			clock-output-names = "mmc2",
 					     "mmc2_output",
 					     "mmc2_sample";
@@ -282,7 +273,7 @@
 			#clock-cells = <0>;
 			compatible = "allwinner,sun8i-a23-mbus-clk";
 			reg = <0x01c2015c 0x4>;
-			clocks = <&osc24M>, <&pll6x2>, <&pll5>;
+			clocks = <&osc24M>, <&pll6 1>, <&pll5>;
 			clock-output-names = "mbus";
 		};
 	};
diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 12131ba..0ed0a8e 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -648,7 +648,7 @@ static void __init sun6i_pll_clk_setup(struct device_node *node)
 {
 	sunxi_factors_clk_setup(node, &sun6i_a31_pll_data);
 }
-CLK_OF_DECLARE(sun6i_pll, "allwinner,sun6i-a31-pll6-clk",
+CLK_OF_DECLARE(sun6i_pll, "allwinner,sun6i-a31-pll-clk",
 	       sun6i_pll_clk_setup);
 
 static void __init sun5i_ahb_clk_setup(struct device_node *node)
@@ -1152,3 +1152,9 @@ static void __init sun4i_pll6_clk_setup(struct device_node *node)
 CLK_OF_DECLARE(sun4i_pll6, "allwinner,sun4i-a10-pll6-clk",
 	       sun4i_pll6_clk_setup);
 
+static void __init sun6i_pll6_clk_setup(struct device_node *node)
+{
+	sunxi_divs_clk_setup(node, &sun6i_a31_pll6_divs_data);
+}
+CLK_OF_DECLARE(sun6i_pll6, "allwinner,sun6i-a31-pll6-clk",
+	       sun6i_pll6_clk_setup);
-- 
2.6.4

[RFC PATCH 4/4] DT: Allwinner H3: fix PLL8 clock

[Andre Przywara]

With the new generic a31-pll clock driver in place, we can use it to
properly describe the PLL8 clock.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/boot/dts/sun8i-h3.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 1524130e..1b2eb20 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -137,11 +137,11 @@
 			clock-output-names = "pll6d2";
 		};
 
-		/* dummy clock until pll6 can be reused */
-		pll8: pll8_clk {
+		pll8: clk@01c20044 {
 			#clock-cells = <0>;
-			compatible = "fixed-clock";
-			clock-frequency = <1>;
+			compatible = "allwinner,sun6i-a31-pll-clk";
+			reg = <0x01c20044 0x4>;
+			clocks = <&osc24M>;
 			clock-output-names = "pll8";
 		};
 
-- 
2.6.4

Re: [linux-sunxi] [RFC PATCH 0/4] clk: sunxi: fix DT compatibility issues

[Hans de Goede]

Hi,

So far I've stayed out of this discussion, but now I feel I have to
weigh in:

There is no need for this series, device-tree compatibility for
sunxi devices is a non issue. No devices ship with dtb files as
part of the bootloader / firmware (the vendor kernels do not
use dtb at all).

So we always are using a dtb from the kernel tree, and both Fedora
and Debian (AFAIK), the 2 distros which ship with more or less official
sunxi support package things so that each kernel version uses the dtb
files compiled from the dts shipped with that exact kernel version,
(through the ftddir directive in extlinux.conf, which is per menu
entry / kernel version) even if multiple kernel versions are installed.

So there is no reason, no reason at all to worry too much about dtb
compatibility for sunxi devices.

As for compatibility with u-boot, u-boot ships with its own embedded
dtb copy, which is based on dts files from the kernel (the u-boot copies
get synced regularly), and even if this dtb were to somehow be replaced
by a new "incompatible" dtb from a newer kernel there would still not
be a problem as u-boot does not (currently) use the clk definitions
from the dtb. Note I'm the u-boot sunxi custodian and I'm fine with
the proposed changes.

TL;DR: NACK for this series.

Regards,

Hans



On 12-02-16 18:59, Andre Przywara wrote:
> Commit f7d372ba54ea ("clk: sunxi: Refactor A31 PLL6 so that it can be
> reused") (in -next) made the A31 PLL6 clock driver more generic, so
> that it can drive the PLL8 clock in newer SoCs too.
> However the patch broke compatibility with older DTs, which this
> series tries to fix.
> The approach chosen here is to bring back the old driver under its
> old name, while letting the new driver using a different name to be
> able to tell them apart.
> The old driver should be somewhat deprecated and not used in new DTs
> anymore.
> The slight disadvantage is that there are now two drivers and two
> compatible names for the same hardware (the PLL6 clock), I am not
> sure if this is frowned upon or can be tolerated since the new driver
> is more generic (drives PLL8 as well) and makes the old one obsolete.
> We just need to keep it for compatibility.
>
> The naming for both the functions and compatible names is probably
> wrong, I am relying on more sunxi - experienced people here to
> suggest better identifiers.
>
> This is only one possible approach to fix this issues, so I am open
> to any kind of discussion.
>
> The series is made on top of Maxime's sunxi/for-next branch, so it
> somehow reverts the change in question. I am happy to rebase it on
> any branch people tell me.
>
> Signed-off-by: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org>
>
> Andre Przywara (4):
>    clk: sunxi: rename new sun6i_a31_pll6 clock to sun6i_a31_pll clock
>    clk: sunxi: re-add old sun6i_a31_pll6 clock
>    clk: sunxi: revert .dtsi changes for DTs with a sun6i_a31_pll6 clock
>    DT: Allwinner H3: fix PLL8 clock
>
>   Documentation/devicetree/bindings/clock/sunxi.txt |  2 +-
>   arch/arm/boot/dts/sun6i-a31.dtsi                  | 36 ++++++++---------
>   arch/arm/boot/dts/sun8i-a23-a33.dtsi              | 25 ++++--------
>   arch/arm/boot/dts/sun8i-a23.dtsi                  |  2 +-
>   arch/arm/boot/dts/sun8i-a33.dtsi                  |  4 +-
>   arch/arm/boot/dts/sun8i-h3.dtsi                   | 31 +++++---------
>   drivers/clk/sunxi/clk-sunxi.c                     | 49 ++++++++++++++++++++---
>   7 files changed, 84 insertions(+), 65 deletions(-)
>
--
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

(Still) breaking DT compatibility (was: [RFC PATCH 0/4] clk: sunxi: fix DT compatibility issues)

[Andre Przywara]

On 12/02/16 18:51, Hans de Goede wrote:
> Hi,
> 
> So far I've stayed out of this discussion, but now I feel I have to
> weigh in:
> 
> There is no need for this series, device-tree compatibility for
> sunxi devices is a non issue. No devices ship with dtb files as
> part of the bootloader / firmware (the vendor kernels do not
> use dtb at all).

I think you are barking in the wrong thread here.
I deliberately sent some code to bring this discussion back to a more
technical level.

But anyway...

> So we always are using a dtb from the kernel tree, and both Fedora
> and Debian (AFAIK), the 2 distros which ship with more or less official
> sunxi support package things

Actually I am trying to help that it doesn't need to stay this way. I
don't see a real reason why a distribution should support a particular
platform, they certainly don't for x86, for instance.
I think a stable DT would help to overcome this limited support that we
see here and would allow distributions to not need to care about sunxi
in particular.

There should be one good DT for all kernels - it may be updated
(especially if new features get supported), but should never break.
Also please note that the DT is not only for Linux, BSDs for instance
use it too - for instance FreeBSD on ARM64.

> so that each kernel version uses the dtb
> files compiled from the dts shipped with that exact kernel version,
> (through the ftddir directive in extlinux.conf, which is per menu
> entry / kernel version) even if multiple kernel versions are installed.

That there are technical ways to mitigate the problem doesn't count as
an excuse to break DT.

To make this clear: The original PLL6 clock _binding_ is actually fine:
it describes the clock as per the manual with two clock outputs.
It's just the driver that is ignoring the clock-output-names that causes
the issue.
So we can happily keep the binding, fix the driver and re-use that very
binding for PLL8, as well.
This fix series here was the easiest approach in terms of changed code.
I can make some patches that implement the proper solution - if you
promise to not shoot down 0/x again easily.

I see that good DT bindings are not easy to come up with. Yes, we are
not perfect and especially for sunxi with it's bad original
documentation we won't create perfect DTs from the beginning.
But I think we should at least _try_ to make a DT what's is meant:
describing the hardware, part of the firmware and OS agnostic - which
implies compatibility.

> So there is no reason, no reason at all to worry too much about dtb
> compatibility for sunxi devices.

I think this argument has been discussed quite a bit in the
other thread.
What I am really worried about is that this now creeps into the arm64 world.
Yes, I see your point that it was a no-brainer so far, but that doesn't
mean that it has to stay this way. For the A64 for instance there are
more devices with eMMC in the pipe (Remix Mini PC and the Olimex board),
so we can just put our firmware bits and the boot loader on there and
distributions don't need to ship those.

> As for compatibility with u-boot, u-boot ships with its own embedded
> dtb copy, which is based on dts files from the kernel (the u-boot copies
> get synced regularly), and even if this dtb were to somehow be replaced
> by a new "incompatible" dtb from a newer kernel there would still not
> be a problem as u-boot does not (currently) use the clk definitions
> from the dtb. Note I'm the u-boot sunxi custodian and I'm fine with
> the proposed changes.
> 
> TL;DR: NACK for this series.

Seriously?

I would prefer if you would save your NACKs for patches that break
something instead.

If you are concerned that this fix is too involved, that's mostly due to
the reason that I couldn't revert the original patch easily due to
conflicts in subsequent patches, so I did a bit more for the rollback.
The actual core fix is pretty easy.

So I hope we can come to a conclusion about this one before I prepare
the next set of patches.

Cheers,
Andre.

> 
> On 12-02-16 18:59, Andre Przywara wrote:
>> Commit f7d372ba54ea ("clk: sunxi: Refactor A31 PLL6 so that it can be
>> reused") (in -next) made the A31 PLL6 clock driver more generic, so
>> that it can drive the PLL8 clock in newer SoCs too.
>> However the patch broke compatibility with older DTs, which this
>> series tries to fix.
>> The approach chosen here is to bring back the old driver under its
>> old name, while letting the new driver using a different name to be
>> able to tell them apart.
>> The old driver should be somewhat deprecated and not used in new DTs
>> anymore.
>> The slight disadvantage is that there are now two drivers and two
>> compatible names for the same hardware (the PLL6 clock), I am not
>> sure if this is frowned upon or can be tolerated since the new driver
>> is more generic (drives PLL8 as well) and makes the old one obsolete.
>> We just need to keep it for compatibility.
>>
>> The naming for both the functions and compatible names is probably
>> wrong, I am relying on more sunxi - experienced people here to
>> suggest better identifiers.
>>
>> This is only one possible approach to fix this issues, so I am open
>> to any kind of discussion.
>>
>> The series is made on top of Maxime's sunxi/for-next branch, so it
>> somehow reverts the change in question. I am happy to rebase it on
>> any branch people tell me.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>
>> Andre Przywara (4):
>>    clk: sunxi: rename new sun6i_a31_pll6 clock to sun6i_a31_pll clock
>>    clk: sunxi: re-add old sun6i_a31_pll6 clock
>>    clk: sunxi: revert .dtsi changes for DTs with a sun6i_a31_pll6 clock
>>    DT: Allwinner H3: fix PLL8 clock
>>
>>   Documentation/devicetree/bindings/clock/sunxi.txt |  2 +-
>>   arch/arm/boot/dts/sun6i-a31.dtsi                  | 36
>> ++++++++---------
>>   arch/arm/boot/dts/sun8i-a23-a33.dtsi              | 25 ++++--------
>>   arch/arm/boot/dts/sun8i-a23.dtsi                  |  2 +-
>>   arch/arm/boot/dts/sun8i-a33.dtsi                  |  4 +-
>>   arch/arm/boot/dts/sun8i-h3.dtsi                   | 31 +++++---------
>>   drivers/clk/sunxi/clk-sunxi.c                     | 49
>> ++++++++++++++++++++---
>>   7 files changed, 84 insertions(+), 65 deletions(-)
>>
> 

Re: [linux-sunxi] (Still) breaking DT compatibility

[Hans de Goede]

Hi,

On 15-02-16 11:16, Andre Przywara wrote:
> On 12/02/16 18:51, Hans de Goede wrote:
>> Hi,
>>
>> So far I've stayed out of this discussion, but now I feel I have to
>> weigh in:
>>
>> There is no need for this series, device-tree compatibility for
>> sunxi devices is a non issue. No devices ship with dtb files as
>> part of the bootloader / firmware (the vendor kernels do not
>> use dtb at all).
>
> I think you are barking in the wrong thread here.
> I deliberately sent some code to bring this discussion back to a more
> technical level.
>
> But anyway...
>
>> So we always are using a dtb from the kernel tree, and both Fedora
>> and Debian (AFAIK), the 2 distros which ship with more or less official
>> sunxi support package things
>
> Actually I am trying to help that it doesn't need to stay this way. I
> don't see a real reason why a distribution should support a particular
> platform, they certainly don't for x86, for instance.
> I think a stable DT would help to overcome this limited support that we
> see here and would allow distributions to not need to care about sunxi
> in particular.

Only if board vendors would start to actually ship things that way.

> There should be one good DT for all kernels - it may be updated
> (especially if new features get supported), but should never break.
> Also please note that the DT is not only for Linux, BSDs for instance
> use it too - for instance FreeBSD on ARM64.
>
>> so that each kernel version uses the dtb
>> files compiled from the dts shipped with that exact kernel version,
>> (through the ftddir directive in extlinux.conf, which is per menu
>> entry / kernel version) even if multiple kernel versions are installed.
>
> That there are technical ways to mitigate the problem doesn't count as
> an excuse to break DT.
>
> To make this clear: The original PLL6 clock _binding_ is actually fine:
> it describes the clock as per the manual with two clock outputs.
> It's just the driver that is ignoring the clock-output-names that causes
> the issue.
> So we can happily keep the binding, fix the driver and re-use that very
> binding for PLL8, as well.
> This fix series here was the easiest approach in terms of changed code.
> I can make some patches that implement the proper solution - if you
> promise to not shoot down 0/x again easily.
>
> I see that good DT bindings are not easy to come up with. Yes, we are
> not perfect and especially for sunxi with it's bad original
> documentation we won't create perfect DTs from the beginning.
> But I think we should at least _try_ to make a DT what's is meant:
> describing the hardware, part of the firmware and OS agnostic - which
> implies compatibility.
>
>> So there is no reason, no reason at all to worry too much about dtb
>> compatibility for sunxi devices.
>
> I think this argument has been discussed quite a bit in the
> other thread.
> What I am really worried about is that this now creeps into the arm64 world.
> Yes, I see your point that it was a no-brainer so far, but that doesn't
> mean that it has to stay this way. For the A64 for instance there are
> more devices with eMMC in the pipe (Remix Mini PC and the Olimex board),
> so we can just put our firmware bits and the boot loader on there and
> distributions don't need to ship those.

Note that putting a proper version of uboot / the bootloader on the device
is somewhat orthogonal to the whole DT compatibility discussion, the only
thing the bootloader has to do with the dtb when using extlinux booting
is that it has coded which specific dtb file to load for the board,
currently it loads the actual dtb from ftddir/<name-coded-in-u-boot>.dtb
where ftddir gets set by the extlinux.conf entry. I realize that this
will not work when trying to boot a new board with a kernel which does
not have a dtb for this board, and in this case having some sort of
fallback ftddir with a dtb would be interesting, and yes this would
require stable bindings.

I'm not arguing that stable bindings may not be useful but currently
no-one is doing the above. So currently there is no reason to worry
too much about stability. If someone had shipped such a board I would
certainly be in favor of this series. But currently the stability
problem is a hypothetical problem and I do not think that carrying
extra code to fix a hypothetical problem is a good idea.

>> As for compatibility with u-boot, u-boot ships with its own embedded
>> dtb copy, which is based on dts files from the kernel (the u-boot copies
>> get synced regularly), and even if this dtb were to somehow be replaced
>> by a new "incompatible" dtb from a newer kernel there would still not
>> be a problem as u-boot does not (currently) use the clk definitions
>> from the dtb. Note I'm the u-boot sunxi custodian and I'm fine with
>> the proposed changes.
>>
>> TL;DR: NACK for this series.
>
> Seriously?

Yes seriously, because you're fixing a hypothetical problem. I'm all
for not randomly breaking devicetree compatibility and so far on
sunxi this has only be done 2 or 3 times over the years, but in the
end it is a balancing act between wishing to keep compatibility and
wishing to keep clean code.

As soon as we actually see boards shipping with dtb-s encoded in their
bootloader, then this balancing act stops and dt compatibility will
become a hard requirement, but IMHO for now sometimes it is better to
break compat when this helps to keep things clean.

> I would prefer if you would save your NACKs for patches that break
> something instead.
>
> If you are concerned that this fix is too involved, that's mostly due to
> the reason that I couldn't revert the original patch easily due to
> conflicts in subsequent patches, so I did a bit more for the rollback.
> The actual core fix is pretty easy.
>
> So I hope we can come to a conclusion about this one before I prepare
> the next set of patches.

I think that the conclusion is that most people are ok with the break,
as is, since it is known to not cause any real world problems.

But in the end this is Maxime's call.

Regards,

Hans

p.s.

I love the work you've been doing on the A64, I've not had a chance
to try it out yet though. Have you made any progress with getting
the mmc slot to work ?  If not maybe I can make some time I've
prior experience in bringing up the mmc slot on other Allwinner SoCs

Allwinner A64 MMC support

[Andre Przywara]

Hi Hans,

....

> 
> p.s.
> 
> I love the work you've been doing on the A64, I've not had a chance
> to try it out yet though. Have you made any progress with getting
> the mmc slot to work ?  If not maybe I can make some time I've
> prior experience in bringing up the mmc slot on other Allwinner SoCs

Thanks for that!

So we made some progress on the weekend (with the help of #linux-sunxi):
1) The regulator node was stupidly put by me into a separate child node
without putting address and size cells into. So I just moved it into
/soc directly now and that seems to fixed the missing regulator.
2) The reset node is a allwinner,sun6i-a31-ahb1-reset, which does not
get registered automatically, but by an explicit call from mach-sunxi/.
I wonder why we do this (yes, I saw that comment, but still...) and if
it would work with a normal MODULE_DEVICE_TABLE() declaration.
There is and will be no equivalent to the mach- directory on arm64.
3) As Jens pointed out, the MMC IP block isn't really compatible.
Changing the clocks is easy (done already), but we need to come up with
code to cover the new phase setting registers in the A64 MMC register block.

Interestingly somehow hacking this (pretending it is compatible) seems
to work, though I got reports from people about mysterious kernel
crashes, so I'm inclined to leave MMC out of the first patch series.

So if you could take a look at the new registers (starting at offset
0x140) and work out what we actually need to do here, that would be
great. I have no real clue about what they actually do and how they
relate to the current output and sample clock phase.
To me it looks like we might get away with just triggering the automatic
calibration and the hardware does the rest for us - keep your fingers
crossed ;-)

Cheers,
Andre.

Re: [linux-sunxi] Allwinner A64 MMC support

[Chen-Yu Tsai]

On Mon, Feb 15, 2016 at 9:59 PM, Andre Przywara <andre.przywara@arm.com> wrote:
> Hi Hans,
>
> ....
>
>>
>> p.s.
>>
>> I love the work you've been doing on the A64, I've not had a chance
>> to try it out yet though. Have you made any progress with getting
>> the mmc slot to work ?  If not maybe I can make some time I've
>> prior experience in bringing up the mmc slot on other Allwinner SoCs
>
> Thanks for that!
>
> So we made some progress on the weekend (with the help of #linux-sunxi):
> 1) The regulator node was stupidly put by me into a separate child node
> without putting address and size cells into. So I just moved it into
> /soc directly now and that seems to fixed the missing regulator.
> 2) The reset node is a allwinner,sun6i-a31-ahb1-reset, which does not
> get registered automatically, but by an explicit call from mach-sunxi/.
> I wonder why we do this (yes, I saw that comment, but still...) and if
> it would work with a normal MODULE_DEVICE_TABLE() declaration.
> There is and will be no equivalent to the mach- directory on arm64.

On sun6i, we supported the high resolution timer, which had a reset control.
Timers (or clocksources) are registered before device model kicks in, so the
reset had to be explicitly registered before the timer, or the system would
hang. I think we phased that out though. Maxime should know.

ChenYu

> 3) As Jens pointed out, the MMC IP block isn't really compatible.
> Changing the clocks is easy (done already), but we need to come up with
> code to cover the new phase setting registers in the A64 MMC register block.
>
> Interestingly somehow hacking this (pretending it is compatible) seems
> to work, though I got reports from people about mysterious kernel
> crashes, so I'm inclined to leave MMC out of the first patch series.
>
> So if you could take a look at the new registers (starting at offset
> 0x140) and work out what we actually need to do here, that would be
> great. I have no real clue about what they actually do and how they
> relate to the current output and sample clock phase.
> To me it looks like we might get away with just triggering the automatic
> calibration and the hardware does the rest for us - keep your fingers
> crossed ;-)
>
> Cheers,
> Andre.
>
> --
> 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@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Allwinner A64 MMC support

[Andre Przywara]

Hi Chen-Yu,

On 15/02/16 14:23, Chen-Yu Tsai wrote:
> On Mon, Feb 15, 2016 at 9:59 PM, Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> wrote:
>> Hi Hans,
>>
>> ....
>>
>>>
>>> p.s.
>>>
>>> I love the work you've been doing on the A64, I've not had a chance
>>> to try it out yet though. Have you made any progress with getting
>>> the mmc slot to work ?  If not maybe I can make some time I've
>>> prior experience in bringing up the mmc slot on other Allwinner SoCs
>>
>> Thanks for that!
>>
>> So we made some progress on the weekend (with the help of #linux-sunxi):
>> 1) The regulator node was stupidly put by me into a separate child node
>> without putting address and size cells into. So I just moved it into
>> /soc directly now and that seems to fixed the missing regulator.
>> 2) The reset node is a allwinner,sun6i-a31-ahb1-reset, which does not
>> get registered automatically, but by an explicit call from mach-sunxi/.
>> I wonder why we do this (yes, I saw that comment, but still...) and if
>> it would work with a normal MODULE_DEVICE_TABLE() declaration.
>> There is and will be no equivalent to the mach- directory on arm64.
> 
> On sun6i, we supported the high resolution timer, which had a reset control.
> Timers (or clocksources) are registered before device model kicks in, so the
> reset had to be explicitly registered before the timer, or the system would
> hang. I think we phased that out though. Maxime should know.

Ah, thanks for that insight!
So that sounds like we would get away with a normal registration for the
A64 - and actually many other SoCs as well?
Shall we guard the explicit sun6i_reset_init with MACH_SUN6I or
something and do the normal registration otherwise?
Or do I miss something here?

Cheers,
Andre

>> 3) As Jens pointed out, the MMC IP block isn't really compatible.
>> Changing the clocks is easy (done already), but we need to come up with
>> code to cover the new phase setting registers in the A64 MMC register block.
>>
>> Interestingly somehow hacking this (pretending it is compatible) seems
>> to work, though I got reports from people about mysterious kernel
>> crashes, so I'm inclined to leave MMC out of the first patch series.
>>
>> So if you could take a look at the new registers (starting at offset
>> 0x140) and work out what we actually need to do here, that would be
>> great. I have no real clue about what they actually do and how they
>> relate to the current output and sample clock phase.
>> To me it looks like we might get away with just triggering the automatic
>> calibration and the hardware does the rest for us - keep your fingers
>> crossed ;-)
>>
>> Cheers,
>> Andre.
>>
>> --
>> 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.
> 
--
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: Allwinner A64 MMC support

[Chen-Yu Tsai]

On Mon, Feb 15, 2016 at 10:36 PM, Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> wrote:
> Hi Chen-Yu,
>
> On 15/02/16 14:23, Chen-Yu Tsai wrote:
>> On Mon, Feb 15, 2016 at 9:59 PM, Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> wrote:
>>> Hi Hans,
>>>
>>> ....
>>>
>>>>
>>>> p.s.
>>>>
>>>> I love the work you've been doing on the A64, I've not had a chance
>>>> to try it out yet though. Have you made any progress with getting
>>>> the mmc slot to work ?  If not maybe I can make some time I've
>>>> prior experience in bringing up the mmc slot on other Allwinner SoCs
>>>
>>> Thanks for that!
>>>
>>> So we made some progress on the weekend (with the help of #linux-sunxi):
>>> 1) The regulator node was stupidly put by me into a separate child node
>>> without putting address and size cells into. So I just moved it into
>>> /soc directly now and that seems to fixed the missing regulator.
>>> 2) The reset node is a allwinner,sun6i-a31-ahb1-reset, which does not
>>> get registered automatically, but by an explicit call from mach-sunxi/.
>>> I wonder why we do this (yes, I saw that comment, but still...) and if
>>> it would work with a normal MODULE_DEVICE_TABLE() declaration.
>>> There is and will be no equivalent to the mach- directory on arm64.
>>
>> On sun6i, we supported the high resolution timer, which had a reset control.
>> Timers (or clocksources) are registered before device model kicks in, so the
>> reset had to be explicitly registered before the timer, or the system would
>> hang. I think we phased that out though. Maxime should know.
>
> Ah, thanks for that insight!
> So that sounds like we would get away with a normal registration for the
> A64 - and actually many other SoCs as well?
> Shall we guard the explicit sun6i_reset_init with MACH_SUN6I or
> something and do the normal registration otherwise?
> Or do I miss something here?

You'll notice 2 reset control compatibles, one is
"allwinner,sun6i-a31-ahb1-reset",
while the other is "allwinner,sun6i-a31-clock-reset". The latter is a
proper device
model driver. We also don't have the hstimer device node for later
SoCs. For A23, it
was because the hardware had only 1 timer, while the driver needed 2,
and I didn't
want to deal with it at the time.

As long as you use "allwinner,sun6i-a31-clock-reset", no special code is needed.

ChenYu

> Cheers,
> Andre
>
>>> 3) As Jens pointed out, the MMC IP block isn't really compatible.
>>> Changing the clocks is easy (done already), but we need to come up with
>>> code to cover the new phase setting registers in the A64 MMC register block.
>>>
>>> Interestingly somehow hacking this (pretending it is compatible) seems
>>> to work, though I got reports from people about mysterious kernel
>>> crashes, so I'm inclined to leave MMC out of the first patch series.
>>>
>>> So if you could take a look at the new registers (starting at offset
>>> 0x140) and work out what we actually need to do here, that would be
>>> great. I have no real clue about what they actually do and how they
>>> relate to the current output and sample clock phase.
>>> To me it looks like we might get away with just triggering the automatic
>>> calibration and the hardware does the rest for us - keep your fingers
>>> crossed ;-)
>>>
>>> Cheers,
>>> Andre.
>>>
>>> --
>>> 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.
>>
>
> --
> 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: [linux-sunxi] Allwinner A64 MMC support

[Andre Przywara]

Hi,

On 15/02/16 14:58, Chen-Yu Tsai wrote:
> On Mon, Feb 15, 2016 at 10:36 PM, Andre Przywara <andre.przywara@arm.com> wrote:
>> Hi Chen-Yu,
>>
>> On 15/02/16 14:23, Chen-Yu Tsai wrote:
>>> On Mon, Feb 15, 2016 at 9:59 PM, Andre Przywara <andre.przywara@arm.com> wrote:
>>>> Hi Hans,
>>>>
>>>> ....
>>>>
>>>>>
>>>>> p.s.
>>>>>
>>>>> I love the work you've been doing on the A64, I've not had a chance
>>>>> to try it out yet though. Have you made any progress with getting
>>>>> the mmc slot to work ?  If not maybe I can make some time I've
>>>>> prior experience in bringing up the mmc slot on other Allwinner SoCs
>>>>
>>>> Thanks for that!
>>>>
>>>> So we made some progress on the weekend (with the help of #linux-sunxi):
>>>> 1) The regulator node was stupidly put by me into a separate child node
>>>> without putting address and size cells into. So I just moved it into
>>>> /soc directly now and that seems to fixed the missing regulator.
>>>> 2) The reset node is a allwinner,sun6i-a31-ahb1-reset, which does not
>>>> get registered automatically, but by an explicit call from mach-sunxi/.
>>>> I wonder why we do this (yes, I saw that comment, but still...) and if
>>>> it would work with a normal MODULE_DEVICE_TABLE() declaration.
>>>> There is and will be no equivalent to the mach- directory on arm64.
>>>
>>> On sun6i, we supported the high resolution timer, which had a reset control.
>>> Timers (or clocksources) are registered before device model kicks in, so the
>>> reset had to be explicitly registered before the timer, or the system would
>>> hang. I think we phased that out though. Maxime should know.
>>
>> Ah, thanks for that insight!
>> So that sounds like we would get away with a normal registration for the
>> A64 - and actually many other SoCs as well?
>> Shall we guard the explicit sun6i_reset_init with MACH_SUN6I or
>> something and do the normal registration otherwise?
>> Or do I miss something here?
> 
> You'll notice 2 reset control compatibles, one is
> "allwinner,sun6i-a31-ahb1-reset",
> while the other is "allwinner,sun6i-a31-clock-reset". The latter is a
> proper device
> model driver. We also don't have the hstimer device node for later
> SoCs. For A23, it
> was because the hardware had only 1 timer, while the driver needed 2,
> and I didn't
> want to deal with it at the time.

Oooh, now I see that those are actually the same driver, just the
initialization part is separate. Thanks for the hint!

> As long as you use "allwinner,sun6i-a31-clock-reset", no special code is needed.

Yeah, great, I will just replace the string in the DT.

Cheers,
Andre.

>> Cheers,
>> Andre
>>
>>>> 3) As Jens pointed out, the MMC IP block isn't really compatible.
>>>> Changing the clocks is easy (done already), but we need to come up with
>>>> code to cover the new phase setting registers in the A64 MMC register block.
>>>>
>>>> Interestingly somehow hacking this (pretending it is compatible) seems
>>>> to work, though I got reports from people about mysterious kernel
>>>> crashes, so I'm inclined to leave MMC out of the first patch series.
>>>>
>>>> So if you could take a look at the new registers (starting at offset
>>>> 0x140) and work out what we actually need to do here, that would be
>>>> great. I have no real clue about what they actually do and how they
>>>> relate to the current output and sample clock phase.
>>>> To me it looks like we might get away with just triggering the automatic
>>>> calibration and the hardware does the rest for us - keep your fingers
>>>> crossed ;-)
>>>>
>>>> Cheers,
>>>> Andre.
>>>>
>>>> --
>>>> 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@googlegroups.com.
>>>> For more options, visit https://groups.google.com/d/optout.
>>>
>>
>> --
>> 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@googlegroups.com.
>> For more options, visit https://groups.google.com/d/optout.
> 

Re: [linux-sunxi] Allwinner A64 MMC support

[Hans de Goede]

Hi,

On 15-02-16 14:59, Andre Przywara wrote:
> Hi Hans,
>
> ....
>
>>
>> p.s.
>>
>> I love the work you've been doing on the A64, I've not had a chance
>> to try it out yet though. Have you made any progress with getting
>> the mmc slot to work ?  If not maybe I can make some time I've
>> prior experience in bringing up the mmc slot on other Allwinner SoCs
>
> Thanks for that!
>
> So we made some progress on the weekend (with the help of #linux-sunxi):
> 1) The regulator node was stupidly put by me into a separate child node
> without putting address and size cells into. So I just moved it into
> /soc directly now and that seems to fixed the missing regulator.
> 2) The reset node is a allwinner,sun6i-a31-ahb1-reset, which does not
> get registered automatically, but by an explicit call from mach-sunxi/.
> I wonder why we do this (yes, I saw that comment, but still...) and if
> it would work with a normal MODULE_DEVICE_TABLE() declaration.
> There is and will be no equivalent to the mach- directory on arm64.
> 3) As Jens pointed out, the MMC IP block isn't really compatible.
> Changing the clocks is easy (done already), but we need to come up with
> code to cover the new phase setting registers in the A64 MMC register block.
>
> Interestingly somehow hacking this (pretending it is compatible) seems
> to work, though I got reports from people about mysterious kernel
> crashes, so I'm inclined to leave MMC out of the first patch series.
>
> So if you could take a look at the new registers (starting at offset
> 0x140) and work out what we actually need to do here, that would be
> great. I have no real clue about what they actually do and how they
> relate to the current output and sample clock phase.
> To me it looks like we might get away with just triggering the automatic
> calibration and the hardware does the rest for us - keep your fingers
> crossed ;-)

Just triggering the automatic calibration sounds good, this likely will
result in better values then the magic values we are currently using for
older SoCs.

As for the meaning of those registers, what we've been doing with the phase
registers so far pretty much boils down to look at what Allwinner BSP kernel
sources do, and then do the same in u-boot / the mainline driver since
the mmc controller documentation is very limited.

Do you have a git tree with your wip mmc support somewhere ? Then I or
maybe Chen-Yu can take a look when we've time.

Regards,

Hans

Re: Allwinner A64 MMC support

[Maxime Ripard]

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

Hi Andre,

On Mon, Feb 15, 2016 at 01:59:28PM +0000, Andre Przywara wrote:
> 2) The reset node is a allwinner,sun6i-a31-ahb1-reset, which does not
> get registered automatically, but by an explicit call from mach-sunxi/.
> I wonder why we do this (yes, I saw that comment, but still...) and if
> it would work with a normal MODULE_DEVICE_TABLE() declaration.

Because we need to access the reset controller at the time the timer
is probed, which is way before the device model is initilized, and the
"real" reset controller driver is probed.

I'm not really fond of the solution either, but there's no other
solution, if we set aside "rework the driver / device model so that it
initializes properly".

> There is and will be no equivalent to the mach- directory on arm64.

Then you cannot use the HS timer.

Maxime

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