[PATCH v2 0/6] clk: sun6i: Unify AHB1 clock and fix rate calculation

[PATCH v2 0/6] clk: sun6i: Unify AHB1 clock and fix rate calculation

[Chen-Yu Tsai]

Hi everyone,

This is v2 of the sun6i AHB1 clock unification series. This series
unifies the mux and divider parts of the AHB1 clock found on sun6i
and sun8i, while also adding support for the pre-divider on the
PLL6 input.

The rate calculation logic must factor in which parent it is using to
calculate the rate, to decide whether to use the pre-divider or not.
This is beyond the original factors clk design in sunxi. To avoid
feature bloat, this is implemented as a seperate composite clk.

The new clock driver is registered with a separate OF_CLK_DECLARE.
As it shares its register with the APB1 div clock, thus shares the same
spinlock, it cannot reside in a separate file.

This series also fixes up the PLL6 clock.

Changes since v1:

  - Dropped "clk: sunxi: Add post clk divider for factor clocks"

  - Added "clk: sunxi: Specify number of child clocks for divs clocks"

  - Reworked the PLL6 clock into a divs clock with 2 outputs.
    This matches the style of PLL6 on the other sunxi platforms.

  - Dropped "dmaengine: sun6i: Remove obsolete clk muxing code".
    Already merged.

The contents of this series are as follows:

Patch 1 makes the number of outputs on divs clocks configurable.

Patch 2 changes PLL6 into a divs clock with 2 outputs, 1 the normal PLL6
and 1 at double the clock rate. This patch also fixes rate calculation
error, due to one of the factor values starting from 1, instead of 0.

Patch 3 updates the DT with the new multiple output PLL6.

Patch 4 adds the unified AHB1 clock driver.

Patch 5 and 6 unify the AHB1 clock nodes on sun6i and sun8i respectively.

Patch 7 sets the default parent and clock rate for AHB1, as required by
the DMA controller. This is included for completeness as the original
clock muxing code in the dmaengine driver was removed. Maxime will test
whether this is needed.


Cheers
ChenYu

Chen-Yu Tsai (7):
  clk: sunxi: Specify number of child clocks for divs clocks
  clk: sunxi: Implement A31 PLL6 as a divs clock for 2x output
  ARM: sun6i: DT: Add PLL6 multiple outputs
  clk: sunxi: unify sun6i AHB1 clock with proper PLL6 pre-divider
  ARM: dts: sun6i: Unify ahb1 clock nodes
  ARM: dts: sun8i: Unify ahb1 clock nodes
  ARM: dts: sun6i: Add required ahb1 clock parent and rates for dma
    controller

 Documentation/devicetree/bindings/clock/sunxi.txt |   7 +-
 arch/arm/boot/dts/sun6i-a31.dtsi                  |  41 ++--
 arch/arm/boot/dts/sun8i-a23.dtsi                  |  12 +-
 drivers/clk/sunxi/clk-sunxi.c                     | 248 ++++++++++++++++++++--
 4 files changed, 259 insertions(+), 49 deletions(-)

-- 
2.1.1

[PATCH v2 1/7] clk: sunxi: Specify number of child clocks for divs clocks

[Chen-Yu Tsai]

Currently sunxi_divs_clk_setup assumes the number of child clocks
to be the same as the number of clock-output-names, and a maximum
of SUNXI_DIVS_MAX_QTY child clocks.

On sun6i, PLL6 only has 1 child clock, but the parent would be used
as well, thereby also having it's own clock-output-names entry. This
results in an extra bogus clock being registered.

This patch adds an entry for the number of child clocks alongside
the data structures for them.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/clk/sunxi/clk-sunxi.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 2cf6581..05d9329 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -1037,6 +1037,7 @@ static void __init sunxi_gates_clk_setup(struct device_node *node,
 
 struct divs_data {
 	const struct factors_data *factors; /* data for the factor clock */
+	int ndivs; /* number of children */
 	struct {
 		u8 fixed; /* is it a fixed divisor? if not... */
 		struct clk_div_table *table; /* is it a table based divisor? */
@@ -1056,6 +1057,7 @@ static struct clk_div_table pll6_sata_tbl[] = {
 
 static const struct divs_data pll5_divs_data __initconst = {
 	.factors = &sun4i_pll5_data,
+	.ndivs = 2,
 	.div = {
 		{ .shift = 0, .pow = 0, }, /* M, DDR */
 		{ .shift = 16, .pow = 1, }, /* P, other */
@@ -1064,6 +1066,7 @@ static const struct divs_data pll5_divs_data __initconst = {
 
 static const struct divs_data pll6_divs_data __initconst = {
 	.factors = &sun4i_pll6_data,
+	.ndivs = 2,
 	.div = {
 		{ .shift = 0, .table = pll6_sata_tbl, .gate = 14 }, /* M, SATA */
 		{ .fixed = 2 }, /* P, other */
@@ -1094,7 +1097,7 @@ static void __init sunxi_divs_clk_setup(struct device_node *node,
 	struct clk_fixed_factor *fix_factor;
 	struct clk_divider *divider;
 	void __iomem *reg;
-	int i = 0;
+	int ndivs = SUNXI_DIVS_MAX_QTY, i = 0;
 	int flags, clkflags;
 
 	/* Set up factor clock that we will be dividing */
@@ -1117,7 +1120,11 @@ static void __init sunxi_divs_clk_setup(struct device_node *node,
 	 * our RAM clock! */
 	clkflags = !strcmp("pll5", parent) ? 0 : CLK_SET_RATE_PARENT;
 
-	for (i = 0; i < SUNXI_DIVS_MAX_QTY; i++) {
+	/* if number of children known, use it */
+	if (data->ndivs)
+		ndivs = data->ndivs;
+
+	for (i = 0; i < ndivs; i++) {
 		if (of_property_read_string_index(node, "clock-output-names",
 						  i, &clk_name) != 0)
 			break;
-- 
2.1.1

[PATCH v2 2/7] clk: sunxi: Implement A31 PLL6 as a divs clock for 2x output

[Chen-Yu Tsai]

Some clock modules on the A31 use PLL6x2 as one of their inputs.
This patch changes the PLL6 implementation for A31 to a divs clock,
i.e. clock with multiple outputs that have different dividers.
The first output will be the normal PLL6 output, and the second
will be PLL6x2.

This patch fixes the PLL6 N factor in the clock driver, and removes
any /2 dividers in the PLL6 factors clock part. The N factor counts
from 1 to 32, mapping to values 0 to 31, as shown in the A31 manual.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 Documentation/devicetree/bindings/clock/sunxi.txt |  5 ++--
 drivers/clk/sunxi/clk-sunxi.c                     | 28 +++++++++++++----------
 2 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
index d3a5c3c..0d84f4b 100644
--- a/Documentation/devicetree/bindings/clock/sunxi.txt
+++ b/Documentation/devicetree/bindings/clock/sunxi.txt
@@ -59,8 +59,9 @@ Required properties for all clocks:
 	multiplexed clocks, the list order must match the hardware
 	programming order.
 - #clock-cells : from common clock binding; shall be set to 0 except for
-	"allwinner,*-gates-clk", "allwinner,sun4i-pll5-clk" and
-	"allwinner,sun4i-pll6-clk" where it shall be set to 1
+	the following compatibles where it shall be set to 1:
+	"allwinner,*-gates-clk", "allwinner,sun4i-pll5-clk",
+	"allwinner,sun4i-pll6-clk", "allwinner, sun6i-a31-pll6-clk"
 - clock-output-names : shall be the corresponding names of the outputs.
 	If the clock module only has one output, the name shall be the
 	module name.
diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 05d9329..9caebff 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -244,9 +244,9 @@ static void sun4i_get_pll5_factors(u32 *freq, u32 parent_rate,
 }
 
 /**
- * sun6i_a31_get_pll6_factors() - calculates n, k factors for A31 PLL6
- * PLL6 rate is calculated as follows
- * rate = parent_rate * n * (k + 1) / 2
+ * sun6i_a31_get_pll6_factors() - calculates n, k factors for A31 PLL6x2
+ * PLL6x2 rate is calculated as follows
+ * rate = parent_rate * (n + 1) * (k + 1)
  * parent_rate is always 24Mhz
  */
 
@@ -255,13 +255,7 @@ static void sun6i_a31_get_pll6_factors(u32 *freq, u32 parent_rate,
 {
 	u8 div;
 
-	/*
-	 * We always have 24MHz / 2, so we can just say that our
-	 * parent clock is 12MHz.
-	 */
-	parent_rate = parent_rate / 2;
-
-	/* Normalize value to a parent_rate multiple (24M / 2) */
+	/* Normalize value to a parent_rate multiple (24M) */
 	div = *freq / parent_rate;
 	*freq = parent_rate * div;
 
@@ -273,7 +267,7 @@ static void sun6i_a31_get_pll6_factors(u32 *freq, u32 parent_rate,
 	if (*k > 3)
 		*k = 3;
 
-	*n = DIV_ROUND_UP(div, (*k+1));
+	*n = DIV_ROUND_UP(div, (*k+1)) - 1;
 }
 
 /**
@@ -494,6 +488,7 @@ static struct clk_factors_config sun6i_a31_pll6_config = {
 	.nwidth = 5,
 	.kshift = 4,
 	.kwidth = 2,
+	.n_start = 1,
 };
 
 static struct clk_factors_config sun4i_apb1_config = {
@@ -561,6 +556,7 @@ 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 sun4i_apb1_data __initconst = {
@@ -1073,6 +1069,14 @@ 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 = 1,
+	.div = {
+		{ .fixed = 2 }, /* normal output */
+	}
+};
+
 /**
  * sunxi_divs_clk_setup() - Setup function for leaf divisors on clocks
  *
@@ -1213,7 +1217,6 @@ static const struct of_device_id clk_factors_match[] __initconst = {
 	{.compatible = "allwinner,sun6i-a31-pll1-clk", .data = &sun6i_a31_pll1_data,},
 	{.compatible = "allwinner,sun8i-a23-pll1-clk", .data = &sun8i_a23_pll1_data,},
 	{.compatible = "allwinner,sun7i-a20-pll4-clk", .data = &sun7i_a20_pll4_data,},
-	{.compatible = "allwinner,sun6i-a31-pll6-clk", .data = &sun6i_a31_pll6_data,},
 	{.compatible = "allwinner,sun4i-a10-apb1-clk", .data = &sun4i_apb1_data,},
 	{.compatible = "allwinner,sun4i-a10-mod0-clk", .data = &sun4i_mod0_data,},
 	{.compatible = "allwinner,sun7i-a20-out-clk", .data = &sun7i_a20_out_data,},
@@ -1234,6 +1237,7 @@ static const struct of_device_id clk_div_match[] __initconst = {
 static const struct of_device_id clk_divs_match[] __initconst = {
 	{.compatible = "allwinner,sun4i-a10-pll5-clk", .data = &pll5_divs_data,},
 	{.compatible = "allwinner,sun4i-a10-pll6-clk", .data = &pll6_divs_data,},
+	{.compatible = "allwinner,sun6i-a31-pll6-clk", .data = &sun6i_a31_pll6_divs_data,},
 	{}
 };
 
-- 
2.1.1

[PATCH v2 3/7] ARM: sun6i: DT: Add PLL6 multiple outputs

[Chen-Yu Tsai]

PLL6 on sun6i has multiple outputs, just like the other sunxi platforms.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm/boot/dts/sun6i-a31.dtsi | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index 543f895..9ac1dd3 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -132,11 +132,11 @@
 		};
 
 		pll6: clk at 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 at 01c20050 {
@@ -166,7 +166,7 @@
 			#clock-cells = <0>;
 			compatible = "allwinner,sun6i-a31-ahb1-mux-clk";
 			reg = <0x01c20054 0x4>;
-			clocks = <&osc32k>, <&osc24M>, <&axi>, <&pll6>;
+			clocks = <&osc32k>, <&osc24M>, <&axi>, <&pll6 0>;
 			clock-output-names = "ahb1_mux";
 		};
 
@@ -221,7 +221,7 @@
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-apb1-mux-clk";
 			reg = <0x01c20058 0x4>;
-			clocks = <&osc32k>, <&osc24M>, <&pll6>, <&pll6>;
+			clocks = <&osc32k>, <&osc24M>, <&pll6 0>, <&pll6 0>;
 			clock-output-names = "apb2_mux";
 		};
 
@@ -248,7 +248,7 @@
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-mod0-clk";
 			reg = <0x01c20088 0x4>;
-			clocks = <&osc24M>, <&pll6>;
+			clocks = <&osc24M>, <&pll6 0>;
 			clock-output-names = "mmc0";
 		};
 
@@ -256,7 +256,7 @@
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-mod0-clk";
 			reg = <0x01c2008c 0x4>;
-			clocks = <&osc24M>, <&pll6>;
+			clocks = <&osc24M>, <&pll6 0>;
 			clock-output-names = "mmc1";
 		};
 
@@ -264,7 +264,7 @@
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-mod0-clk";
 			reg = <0x01c20090 0x4>;
-			clocks = <&osc24M>, <&pll6>;
+			clocks = <&osc24M>, <&pll6 0>;
 			clock-output-names = "mmc2";
 		};
 
@@ -272,7 +272,7 @@
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-mod0-clk";
 			reg = <0x01c20094 0x4>;
-			clocks = <&osc24M>, <&pll6>;
+			clocks = <&osc24M>, <&pll6 0>;
 			clock-output-names = "mmc3";
 		};
 
@@ -280,7 +280,7 @@
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-mod0-clk";
 			reg = <0x01c200a0 0x4>;
-			clocks = <&osc24M>, <&pll6>;
+			clocks = <&osc24M>, <&pll6 0>;
 			clock-output-names = "spi0";
 		};
 
@@ -288,7 +288,7 @@
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-mod0-clk";
 			reg = <0x01c200a4 0x4>;
-			clocks = <&osc24M>, <&pll6>;
+			clocks = <&osc24M>, <&pll6 0>;
 			clock-output-names = "spi1";
 		};
 
@@ -296,7 +296,7 @@
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-mod0-clk";
 			reg = <0x01c200a8 0x4>;
-			clocks = <&osc24M>, <&pll6>;
+			clocks = <&osc24M>, <&pll6 0>;
 			clock-output-names = "spi2";
 		};
 
@@ -304,7 +304,7 @@
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-mod0-clk";
 			reg = <0x01c200ac 0x4>;
-			clocks = <&osc24M>, <&pll6>;
+			clocks = <&osc24M>, <&pll6 0>;
 			clock-output-names = "spi3";
 		};
 
@@ -840,7 +840,7 @@
 			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";
 			};
 
-- 
2.1.1

[PATCH v2 4/7] clk: sunxi: unify sun6i AHB1 clock with proper PLL6 pre-divider

[Chen-Yu Tsai]

This patch unifies the sun6i AHB1 clock, originally supported
with separate mux and divider clks. It also adds support for
the pre-divider on the PLL6 input, thus allowing the clock to
be muxed to PLL6 with proper clock rate calculation.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 Documentation/devicetree/bindings/clock/sunxi.txt |   2 +-
 drivers/clk/sunxi/clk-sunxi.c                     | 209 ++++++++++++++++++++++
 2 files changed, 210 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
index 0d84f4b..e862818 100644
--- a/Documentation/devicetree/bindings/clock/sunxi.txt
+++ b/Documentation/devicetree/bindings/clock/sunxi.txt
@@ -23,7 +23,7 @@ Required properties:
 	"allwinner,sun5i-a10s-ahb-gates-clk" - for the AHB gates on A10s
 	"allwinner,sun7i-a20-ahb-gates-clk" - for the AHB gates on A20
 	"allwinner,sun6i-a31-ar100-clk" - for the AR100 on A31
-	"allwinner,sun6i-a31-ahb1-mux-clk" - for the AHB1 multiplexer on A31
+	"allwinner,sun6i-a31-ahb1-clk" - for the AHB1 clock on A31
 	"allwinner,sun6i-a31-ahb1-gates-clk" - for the AHB1 gates on A31
 	"allwinner,sun8i-a23-ahb1-gates-clk" - for the AHB1 gates on A23
 	"allwinner,sun4i-a10-apb0-clk" - for the APB0 clock
diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 9caebff..7151e2c 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -19,6 +19,7 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/reset-controller.h>
+#include <linux/log2.h>
 
 #include "clk-factors.h"
 
@@ -1357,3 +1358,211 @@ static void __init sun6i_init_clocks(struct device_node *node)
 }
 CLK_OF_DECLARE(sun6i_a31_clk_init, "allwinner,sun6i-a31", sun6i_init_clocks);
 CLK_OF_DECLARE(sun8i_a23_clk_init, "allwinner,sun8i-a23", sun6i_init_clocks);
+
+
+/**
+ * sun6i_a31_ahb1_clk_setup() - Setup function for a31 ahb1 composite clk
+ */
+
+#define SUN6I_AHB1_MAX_PARENTS		4
+#define SUN6I_AHB1_MUX_PARENT_PLL6	3
+#define SUN6I_AHB1_MUX_SHIFT		12
+#define SUN6I_AHB1_MUX_MASK		0x3
+#define SUN6I_AHB1_MUX_GET_PARENT(reg)	((reg >> SUN6I_AHB1_MUX_SHIFT) & \
+					 SUN6I_AHB1_MUX_MASK)
+#define SUN6I_AHB1_DIV_SHIFT		4
+#define SUN6I_AHB1_DIV_MASK		0x3
+#define SUN6I_AHB1_DIV_GET(reg)		((reg >> SUN6I_AHB1_DIV_SHIFT) & \
+					 SUN6I_AHB1_DIV_MASK)
+#define SUN6I_AHB1_DIV_SET(reg, div)	((reg & ~(SUN6I_AHB1_DIV_MASK << \
+						  SUN6I_AHB1_DIV_SHIFT)) | \
+					 (div << SUN6I_AHB1_DIV_SHIFT))
+#define SUN6I_AHB1_PLL6_DIV_SHIFT	6
+#define SUN6I_AHB1_PLL6_DIV_MASK	0x3
+#define SUN6I_AHB1_PLL6_DIV_GET(reg)	((reg >> SUN6I_AHB1_PLL6_DIV_SHIFT) & \
+					 SUN6I_AHB1_PLL6_DIV_MASK)
+#define SUN6I_AHB1_PLL6_DIV_SET(reg, div) ((reg & \
+					    ~(SUN6I_AHB1_PLL6_DIV_MASK << \
+					      SUN6I_AHB1_PLL6_DIV_SHIFT)) | \
+					   (div << SUN6I_AHB1_PLL6_DIV_SHIFT))
+
+struct sun6i_ahb1_clk {
+	struct clk_hw hw;
+	void __iomem *reg;
+};
+
+#define to_sun6i_ahb1_clk(_hw) container_of(_hw, struct sun6i_ahb1_clk, hw)
+
+static unsigned long sun6i_ahb1_clk_recalc_rate(struct clk_hw *hw,
+						unsigned long parent_rate)
+{
+	struct sun6i_ahb1_clk *ahb1 = to_sun6i_ahb1_clk(hw);
+	unsigned long rate;
+	u32 reg;
+
+	/* Fetch the register value */
+	reg = readl(ahb1->reg);
+
+	/* apply pre-divider first if parent is pll6 */
+	if (SUN6I_AHB1_MUX_GET_PARENT(reg) == SUN6I_AHB1_MUX_PARENT_PLL6)
+		parent_rate /= SUN6I_AHB1_PLL6_DIV_GET(reg) + 1;
+
+	/* clk divider */
+	rate = parent_rate >> SUN6I_AHB1_DIV_GET(reg);
+
+	return rate;
+}
+
+static long sun6i_ahb1_clk_round(unsigned long rate, u8 *divp, u8 *pre_divp,
+				 u8 parent, unsigned long parent_rate)
+{
+	u8 div, calcp, calcm = 1;
+
+	/* clock can only divide, so we will never be able to achieve
+	 * frequencies higher than the parent frequency */
+	if (parent_rate && rate > parent_rate)
+		rate = parent_rate;
+
+	div = DIV_ROUND_UP(parent_rate, rate);
+
+	/* calculate pre-divider if parent is pll6 */
+	if (parent == SUN6I_AHB1_MUX_PARENT_PLL6) {
+		if (div < 4)
+			calcp = 0;
+		else if (div / 2 < 4)
+			calcp = 1;
+		else if (div / 4 < 4)
+			calcp = 2;
+		else
+			calcp = 3;
+
+		calcm = DIV_ROUND_UP(div, 1 << calcp);
+	} else {
+		calcp = __roundup_pow_of_two(div);
+		calcp = calcp > 3 ? 3 : calcp;
+	}
+
+	if (divp) {
+		*divp = calcp;
+		*pre_divp = calcm - 1;
+	}
+
+	return (parent_rate / calcm) >> calcp;
+}
+
+static long sun6i_ahb1_clk_determine_rate(struct clk_hw *hw, unsigned long rate,
+					  unsigned long *best_parent_rate,
+					  struct clk **best_parent_clk)
+{
+	struct clk *clk = hw->clk, *parent, *best_parent = NULL;
+	int i, num_parents;
+	unsigned long parent_rate, best = 0, child_rate, best_child_rate = 0;
+
+	/* find the parent that can help provide the fastest rate <= rate */
+	num_parents = __clk_get_num_parents(clk);
+	for (i = 0; i < num_parents; i++) {
+		parent = clk_get_parent_by_index(clk, i);
+		if (!parent)
+			continue;
+		if (__clk_get_flags(clk) & CLK_SET_RATE_PARENT)
+			parent_rate = __clk_round_rate(parent, rate);
+		else
+			parent_rate = __clk_get_rate(parent);
+
+		child_rate = sun6i_ahb1_clk_round(rate, NULL, NULL, i,
+						  parent_rate);
+
+		if (child_rate <= rate && child_rate > best_child_rate) {
+			best_parent = parent;
+			best = parent_rate;
+			best_child_rate = child_rate;
+		}
+	}
+
+	if (best_parent)
+		*best_parent_clk = best_parent;
+	*best_parent_rate = best;
+
+	return best_child_rate;
+}
+
+static int sun6i_ahb1_clk_set_rate(struct clk_hw *hw, unsigned long rate,
+				   unsigned long parent_rate)
+{
+	struct sun6i_ahb1_clk *ahb1 = to_sun6i_ahb1_clk(hw);
+	unsigned long flags;
+	u8 div, pre_div, parent;
+	u32 reg;
+
+	spin_lock_irqsave(&clk_lock, flags);
+
+	reg = readl(ahb1->reg);
+
+	/* need to know which parent is used to apply pre-divider */
+	parent = SUN6I_AHB1_MUX_GET_PARENT(reg);
+	sun6i_ahb1_clk_round(rate, &div, &pre_div, parent, parent_rate);
+
+	reg = SUN6I_AHB1_DIV_SET(reg, div);
+	reg = SUN6I_AHB1_PLL6_DIV_SET(reg, pre_div);
+	writel(reg, ahb1->reg);
+
+	spin_unlock_irqrestore(&clk_lock, flags);
+
+	return 0;
+}
+
+static const struct clk_ops sun6i_ahb1_clk_ops = {
+	.determine_rate	= sun6i_ahb1_clk_determine_rate,
+	.recalc_rate	= sun6i_ahb1_clk_recalc_rate,
+	.set_rate	= sun6i_ahb1_clk_set_rate,
+};
+
+static void __init sun6i_ahb1_clk_setup(struct device_node *node)
+{
+	struct clk *clk;
+	struct sun6i_ahb1_clk *ahb1;
+	struct clk_mux *mux;
+	const char *clk_name = node->name;
+	const char *parents[SUN6I_AHB1_MAX_PARENTS];
+	void __iomem *reg;
+	int i = 0;
+
+	reg = of_iomap(node, 0);
+
+	/* we have a mux, we will have >1 parents */
+	while (i < SUN6I_AHB1_MAX_PARENTS &&
+	       (parents[i] = of_clk_get_parent_name(node, i)) != NULL)
+		i++;
+
+	of_property_read_string(node, "clock-output-names", &clk_name);
+
+	ahb1 = kzalloc(sizeof(struct sun6i_ahb1_clk), GFP_KERNEL);
+	if (!ahb1)
+		return;
+
+	mux = kzalloc(sizeof(struct clk_mux), GFP_KERNEL);
+	if (!mux) {
+		kfree(ahb1);
+		return;
+	}
+
+	/* set up clock properties */
+	mux->reg = reg;
+	mux->shift = SUN6I_AHB1_MUX_SHIFT;
+	mux->mask = SUN6I_AHB1_MUX_MASK;
+	mux->lock = &clk_lock;
+	ahb1->reg = reg;
+
+	clk = clk_register_composite(NULL, clk_name, parents, i,
+				     &mux->hw, &clk_mux_ops,
+				     &ahb1->hw, &sun6i_ahb1_clk_ops,
+				     NULL, NULL, 0);
+
+	if (!IS_ERR(clk)) {
+		of_clk_add_provider(node, of_clk_src_simple_get, clk);
+		clk_register_clkdev(clk, clk_name, NULL);
+	}
+}
+
+CLK_OF_DECLARE(sun6i_a31_ahb1, "allwinner,sun6i-a31-ahb1-clk",
+		sun6i_ahb1_clk_setup);
-- 
2.1.1

[PATCH v2 5/7] ARM: dts: sun6i: Unify ahb1 clock nodes

[Chen-Yu Tsai]

The clock driver has unified support for the ahb1 clock.
Unify the clock nodes so it works.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm/boot/dts/sun6i-a31.dtsi | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index 9ac1dd3..caf879d 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -162,19 +162,11 @@
 			clock-output-names = "axi";
 		};
 
-		ahb1_mux: ahb1_mux at 01c20054 {
-			#clock-cells = <0>;
-			compatible = "allwinner,sun6i-a31-ahb1-mux-clk";
-			reg = <0x01c20054 0x4>;
-			clocks = <&osc32k>, <&osc24M>, <&axi>, <&pll6 0>;
-			clock-output-names = "ahb1_mux";
-		};
-
 		ahb1: ahb1 at 01c20054 {
 			#clock-cells = <0>;
-			compatible = "allwinner,sun4i-a10-ahb-clk";
+			compatible = "allwinner,sun6i-a31-ahb1-clk";
 			reg = <0x01c20054 0x4>;
-			clocks = <&ahb1_mux>;
+			clocks = <&osc32k>, <&osc24M>, <&axi>, <&pll6 0>;
 			clock-output-names = "ahb1";
 		};
 
-- 
2.1.1

[PATCH v2 6/7] ARM: dts: sun8i: Unify ahb1 clock nodes

[Chen-Yu Tsai]

The clock driver has unified support for the ahb1 clock.
Unify the clock nodes so it works.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm/boot/dts/sun8i-a23.dtsi | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/arch/arm/boot/dts/sun8i-a23.dtsi b/arch/arm/boot/dts/sun8i-a23.dtsi
index 6146ef1..4ac5daf 100644
--- a/arch/arm/boot/dts/sun8i-a23.dtsi
+++ b/arch/arm/boot/dts/sun8i-a23.dtsi
@@ -140,19 +140,11 @@
 			clock-output-names = "axi";
 		};
 
-		ahb1_mux: ahb1_mux_clk at 01c20054 {
-			#clock-cells = <0>;
-			compatible = "allwinner,sun6i-a31-ahb1-mux-clk";
-			reg = <0x01c20054 0x4>;
-			clocks = <&osc32k>, <&osc24M>, <&axi>, <&pll6>;
-			clock-output-names = "ahb1_mux";
-		};
-
 		ahb1: ahb1_clk at 01c20054 {
 			#clock-cells = <0>;
-			compatible = "allwinner,sun4i-a10-ahb-clk";
+			compatible = "allwinner,sun6i-a31-ahb1-clk";
 			reg = <0x01c20054 0x4>;
-			clocks = <&ahb1_mux>;
+			clocks = <&osc32k>, <&osc24M>, <&axi>, <&pll6>;
 			clock-output-names = "ahb1";
 		};
 
-- 
2.1.1

[PATCH v2 7/7] ARM: dts: sun6i: Add required ahb1 clock parent and rates for dma controller

[Chen-Yu Tsai]

The DMA controller requires AHB1 bus clock to be clocked from PLL6.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm/boot/dts/sun6i-a31.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index caf879d..d7845d3 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -353,6 +353,11 @@
 			clocks = <&ahb1_gates 6>;
 			resets = <&ahb1_rst 6>;
 			#dma-cells = <1>;
+
+			/* DMA controller requires AHB1 clocked from PLL6 */
+			assigned-clocks = <&ahb1>;
+			assigned-clock-parents = <&pll6 0>;
+			assigned-clock-rates = <200000000>;
 		};
 
 		mmc0: mmc at 01c0f000 {
-- 
2.1.1

[PATCH v2 2/7] clk: sunxi: Implement A31 PLL6 as a divs clock for 2x output

[Maxime Ripard]

Hi,

On Sat, Sep 27, 2014 at 04:49:50PM +0800, Chen-Yu Tsai wrote:
> Some clock modules on the A31 use PLL6x2 as one of their inputs.
> This patch changes the PLL6 implementation for A31 to a divs clock,
> i.e. clock with multiple outputs that have different dividers.
> The first output will be the normal PLL6 output, and the second
> will be PLL6x2.
> 
> This patch fixes the PLL6 N factor in the clock driver, and removes
> any /2 dividers in the PLL6 factors clock part. The N factor counts
> from 1 to 32, mapping to values 0 to 31, as shown in the A31 manual.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  Documentation/devicetree/bindings/clock/sunxi.txt |  5 ++--
>  drivers/clk/sunxi/clk-sunxi.c                     | 28 +++++++++++++----------
>  2 files changed, 19 insertions(+), 14 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
> index d3a5c3c..0d84f4b 100644
> --- a/Documentation/devicetree/bindings/clock/sunxi.txt
> +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
> @@ -59,8 +59,9 @@ Required properties for all clocks:
>  	multiplexed clocks, the list order must match the hardware
>  	programming order.
>  - #clock-cells : from common clock binding; shall be set to 0 except for
> -	"allwinner,*-gates-clk", "allwinner,sun4i-pll5-clk" and
> -	"allwinner,sun4i-pll6-clk" where it shall be set to 1
> +	the following compatibles where it shall be set to 1:
> +	"allwinner,*-gates-clk", "allwinner,sun4i-pll5-clk",
> +	"allwinner,sun4i-pll6-clk", "allwinner, sun6i-a31-pll6-clk"
                                               ^ Drop this extra space

And you're still not documenting what outputs you might have on pll6,
and what the extra argument correspond to.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140930/c1ff84f6/attachment.sig>

[PATCH v2 1/7] clk: sunxi: Specify number of child clocks for divs clocks

[Maxime Ripard]

On Sat, Sep 27, 2014 at 04:49:49PM +0800, Chen-Yu Tsai wrote:
> Currently sunxi_divs_clk_setup assumes the number of child clocks
> to be the same as the number of clock-output-names, and a maximum
> of SUNXI_DIVS_MAX_QTY child clocks.
> 
> On sun6i, PLL6 only has 1 child clock, but the parent would be used
> as well, thereby also having it's own clock-output-names entry. This
> results in an extra bogus clock being registered.

Isn't PLL6 having two childs as well?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140930/54912c5c/attachment.sig>

[PATCH v2 2/7] clk: sunxi: Implement A31 PLL6 as a divs clock for 2x output

[Chen-Yu Tsai]

Hi,

On Tue, Sep 30, 2014 at 11:39 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> On Sat, Sep 27, 2014 at 04:49:50PM +0800, Chen-Yu Tsai wrote:
>> Some clock modules on the A31 use PLL6x2 as one of their inputs.
>> This patch changes the PLL6 implementation for A31 to a divs clock,
>> i.e. clock with multiple outputs that have different dividers.
>> The first output will be the normal PLL6 output, and the second
>> will be PLL6x2.
>>
>> This patch fixes the PLL6 N factor in the clock driver, and removes
>> any /2 dividers in the PLL6 factors clock part. The N factor counts
>> from 1 to 32, mapping to values 0 to 31, as shown in the A31 manual.
>>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> ---
>>  Documentation/devicetree/bindings/clock/sunxi.txt |  5 ++--
>>  drivers/clk/sunxi/clk-sunxi.c                     | 28 +++++++++++++----------
>>  2 files changed, 19 insertions(+), 14 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
>> index d3a5c3c..0d84f4b 100644
>> --- a/Documentation/devicetree/bindings/clock/sunxi.txt
>> +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
>> @@ -59,8 +59,9 @@ Required properties for all clocks:
>>       multiplexed clocks, the list order must match the hardware
>>       programming order.
>>  - #clock-cells : from common clock binding; shall be set to 0 except for
>> -     "allwinner,*-gates-clk", "allwinner,sun4i-pll5-clk" and
>> -     "allwinner,sun4i-pll6-clk" where it shall be set to 1
>> +     the following compatibles where it shall be set to 1:
>> +     "allwinner,*-gates-clk", "allwinner,sun4i-pll5-clk",
>> +     "allwinner,sun4i-pll6-clk", "allwinner, sun6i-a31-pll6-clk"
>                                                ^ Drop this extra space
>
> And you're still not documenting what outputs you might have on pll6,
> and what the extra argument correspond to.

Sorry. Missed that part.

ChenYu

[PATCH v2 4/7] clk: sunxi: unify sun6i AHB1 clock with proper PLL6 pre-divider

[Maxime Ripard]

On Sat, Sep 27, 2014 at 04:49:52PM +0800, Chen-Yu Tsai wrote:
> This patch unifies the sun6i AHB1 clock, originally supported
> with separate mux and divider clks. It also adds support for
> the pre-divider on the PLL6 input, thus allowing the clock to
> be muxed to PLL6 with proper clock rate calculation.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  Documentation/devicetree/bindings/clock/sunxi.txt |   2 +-
>  drivers/clk/sunxi/clk-sunxi.c                     | 209 ++++++++++++++++++++++
>  2 files changed, 210 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
> index 0d84f4b..e862818 100644
> --- a/Documentation/devicetree/bindings/clock/sunxi.txt
> +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
> @@ -23,7 +23,7 @@ Required properties:
>  	"allwinner,sun5i-a10s-ahb-gates-clk" - for the AHB gates on A10s
>  	"allwinner,sun7i-a20-ahb-gates-clk" - for the AHB gates on A20
>  	"allwinner,sun6i-a31-ar100-clk" - for the AR100 on A31
> -	"allwinner,sun6i-a31-ahb1-mux-clk" - for the AHB1 multiplexer on A31
> +	"allwinner,sun6i-a31-ahb1-clk" - for the AHB1 clock on A31
>  	"allwinner,sun6i-a31-ahb1-gates-clk" - for the AHB1 gates on A31
>  	"allwinner,sun8i-a23-ahb1-gates-clk" - for the AHB1 gates on A23
>  	"allwinner,sun4i-a10-apb0-clk" - for the APB0 clock
> diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
> index 9caebff..7151e2c 100644
> --- a/drivers/clk/sunxi/clk-sunxi.c
> +++ b/drivers/clk/sunxi/clk-sunxi.c
> @@ -19,6 +19,7 @@
>  #include <linux/of.h>
>  #include <linux/of_address.h>
>  #include <linux/reset-controller.h>
> +#include <linux/log2.h>
>  
>  #include "clk-factors.h"
>  
> @@ -1357,3 +1358,211 @@ static void __init sun6i_init_clocks(struct device_node *node)
>  }
>  CLK_OF_DECLARE(sun6i_a31_clk_init, "allwinner,sun6i-a31", sun6i_init_clocks);
>  CLK_OF_DECLARE(sun8i_a23_clk_init, "allwinner,sun8i-a23", sun6i_init_clocks);
> +
> +

Drop the extra newline

> +/**
> + * sun6i_a31_ahb1_clk_setup() - Setup function for a31 ahb1 composite clk
> + */
> +
> +#define SUN6I_AHB1_MAX_PARENTS		4
> +#define SUN6I_AHB1_MUX_PARENT_PLL6	3
> +#define SUN6I_AHB1_MUX_SHIFT		12
> +#define SUN6I_AHB1_MUX_MASK		0x3
> +#define SUN6I_AHB1_MUX_GET_PARENT(reg)	((reg >> SUN6I_AHB1_MUX_SHIFT) & \
> +					 SUN6I_AHB1_MUX_MASK)
> +#define SUN6I_AHB1_DIV_SHIFT		4
> +#define SUN6I_AHB1_DIV_MASK		0x3
> +#define SUN6I_AHB1_DIV_GET(reg)		((reg >> SUN6I_AHB1_DIV_SHIFT) & \
> +					 SUN6I_AHB1_DIV_MASK)
> +#define SUN6I_AHB1_DIV_SET(reg, div)	((reg & ~(SUN6I_AHB1_DIV_MASK << \
> +						  SUN6I_AHB1_DIV_SHIFT)) | \
> +					 (div << SUN6I_AHB1_DIV_SHIFT))
> +#define SUN6I_AHB1_PLL6_DIV_SHIFT	6
> +#define SUN6I_AHB1_PLL6_DIV_MASK	0x3
> +#define SUN6I_AHB1_PLL6_DIV_GET(reg)	((reg >> SUN6I_AHB1_PLL6_DIV_SHIFT) & \
> +					 SUN6I_AHB1_PLL6_DIV_MASK)
> +#define SUN6I_AHB1_PLL6_DIV_SET(reg, div) ((reg & \
> +					    ~(SUN6I_AHB1_PLL6_DIV_MASK << \
> +					      SUN6I_AHB1_PLL6_DIV_SHIFT)) | \
> +					   (div << SUN6I_AHB1_PLL6_DIV_SHIFT))

Your indentation looks really odd, and the masks you have should
really point to the actual mask, and not just the number of bits.

> +
> +struct sun6i_ahb1_clk {
> +	struct clk_hw hw;
> +	void __iomem *reg;
> +};
> +
> +#define to_sun6i_ahb1_clk(_hw) container_of(_hw, struct sun6i_ahb1_clk, hw)
> +
> +static unsigned long sun6i_ahb1_clk_recalc_rate(struct clk_hw *hw,
> +						unsigned long parent_rate)
> +{
> +	struct sun6i_ahb1_clk *ahb1 = to_sun6i_ahb1_clk(hw);
> +	unsigned long rate;
> +	u32 reg;
> +
> +	/* Fetch the register value */
> +	reg = readl(ahb1->reg);
> +
> +	/* apply pre-divider first if parent is pll6 */
> +	if (SUN6I_AHB1_MUX_GET_PARENT(reg) == SUN6I_AHB1_MUX_PARENT_PLL6)
> +		parent_rate /= SUN6I_AHB1_PLL6_DIV_GET(reg) + 1;
> +
> +	/* clk divider */
> +	rate = parent_rate >> SUN6I_AHB1_DIV_GET(reg);
> +
> +	return rate;
> +}
> +
> +static long sun6i_ahb1_clk_round(unsigned long rate, u8 *divp, u8 *pre_divp,
> +				 u8 parent, unsigned long parent_rate)
> +{
> +	u8 div, calcp, calcm = 1;
> +
> +	/* clock can only divide, so we will never be able to achieve
> +	 * frequencies higher than the parent frequency */

Wrong multiline comment style

> +	if (parent_rate && rate > parent_rate)
> +		rate = parent_rate;
> +
> +	div = DIV_ROUND_UP(parent_rate, rate);
> +
> +	/* calculate pre-divider if parent is pll6 */
> +	if (parent == SUN6I_AHB1_MUX_PARENT_PLL6) {
> +		if (div < 4)
> +			calcp = 0;
> +		else if (div / 2 < 4)
> +			calcp = 1;
> +		else if (div / 4 < 4)
> +			calcp = 2;
> +		else
> +			calcp = 3;
> +
> +		calcm = DIV_ROUND_UP(div, 1 << calcp);
> +	} else {
> +		calcp = __roundup_pow_of_two(div);
> +		calcp = calcp > 3 ? 3 : calcp;
> +	}
> +
> +	if (divp) {
> +		*divp = calcp;
> +		*pre_divp = calcm - 1;
> +	}
> +
> +	return (parent_rate / calcm) >> calcp;
> +}
> +
> +static long sun6i_ahb1_clk_determine_rate(struct clk_hw *hw, unsigned long rate,
> +					  unsigned long *best_parent_rate,
> +					  struct clk **best_parent_clk)
> +{
> +	struct clk *clk = hw->clk, *parent, *best_parent = NULL;
> +	int i, num_parents;
> +	unsigned long parent_rate, best = 0, child_rate, best_child_rate = 0;
> +
> +	/* find the parent that can help provide the fastest rate <= rate */
> +	num_parents = __clk_get_num_parents(clk);
> +	for (i = 0; i < num_parents; i++) {
> +		parent = clk_get_parent_by_index(clk, i);
> +		if (!parent)
> +			continue;
> +		if (__clk_get_flags(clk) & CLK_SET_RATE_PARENT)
> +			parent_rate = __clk_round_rate(parent, rate);
> +		else
> +			parent_rate = __clk_get_rate(parent);
> +
> +		child_rate = sun6i_ahb1_clk_round(rate, NULL, NULL, i,
> +						  parent_rate);
> +
> +		if (child_rate <= rate && child_rate > best_child_rate) {
> +			best_parent = parent;
> +			best = parent_rate;
> +			best_child_rate = child_rate;
> +		}
> +	}
> +
> +	if (best_parent)
> +		*best_parent_clk = best_parent;
> +	*best_parent_rate = best;
> +
> +	return best_child_rate;
> +}
> +
> +static int sun6i_ahb1_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> +				   unsigned long parent_rate)
> +{
> +	struct sun6i_ahb1_clk *ahb1 = to_sun6i_ahb1_clk(hw);
> +	unsigned long flags;
> +	u8 div, pre_div, parent;
> +	u32 reg;
> +
> +	spin_lock_irqsave(&clk_lock, flags);

Isn't that already taken by the composite clock?

> +
> +	reg = readl(ahb1->reg);
> +
> +	/* need to know which parent is used to apply pre-divider */
> +	parent = SUN6I_AHB1_MUX_GET_PARENT(reg);
> +	sun6i_ahb1_clk_round(rate, &div, &pre_div, parent, parent_rate);

Haven't you called that already in determine_rate?

> +
> +	reg = SUN6I_AHB1_DIV_SET(reg, div);
> +	reg = SUN6I_AHB1_PLL6_DIV_SET(reg, pre_div);
> +	writel(reg, ahb1->reg);
> +
> +	spin_unlock_irqrestore(&clk_lock, flags);
> +
> +	return 0;
> +}
> +
> +static const struct clk_ops sun6i_ahb1_clk_ops = {
> +	.determine_rate	= sun6i_ahb1_clk_determine_rate,
> +	.recalc_rate	= sun6i_ahb1_clk_recalc_rate,
> +	.set_rate	= sun6i_ahb1_clk_set_rate,
> +};
> +
> +static void __init sun6i_ahb1_clk_setup(struct device_node *node)
> +{
> +	struct clk *clk;
> +	struct sun6i_ahb1_clk *ahb1;
> +	struct clk_mux *mux;
> +	const char *clk_name = node->name;
> +	const char *parents[SUN6I_AHB1_MAX_PARENTS];
> +	void __iomem *reg;
> +	int i = 0;
> +
> +	reg = of_iomap(node, 0);

of_io_request_and_map please :)

> +
> +	/* we have a mux, we will have >1 parents */
> +	while (i < SUN6I_AHB1_MAX_PARENTS &&
> +	       (parents[i] = of_clk_get_parent_name(node, i)) != NULL)
> +		i++;
> +
> +	of_property_read_string(node, "clock-output-names", &clk_name);
> +
> +	ahb1 = kzalloc(sizeof(struct sun6i_ahb1_clk), GFP_KERNEL);
> +	if (!ahb1)
> +		return;
> +
> +	mux = kzalloc(sizeof(struct clk_mux), GFP_KERNEL);
> +	if (!mux) {
> +		kfree(ahb1);
> +		return;
> +	}
> +
> +	/* set up clock properties */
> +	mux->reg = reg;
> +	mux->shift = SUN6I_AHB1_MUX_SHIFT;
> +	mux->mask = SUN6I_AHB1_MUX_MASK;
> +	mux->lock = &clk_lock;
> +	ahb1->reg = reg;
> +
> +	clk = clk_register_composite(NULL, clk_name, parents, i,
> +				     &mux->hw, &clk_mux_ops,
> +				     &ahb1->hw, &sun6i_ahb1_clk_ops,
> +				     NULL, NULL, 0);
> +
> +	if (!IS_ERR(clk)) {
> +		of_clk_add_provider(node, of_clk_src_simple_get, clk);
> +		clk_register_clkdev(clk, clk_name, NULL);
> +	}
> +}
> +
> +CLK_OF_DECLARE(sun6i_a31_ahb1, "allwinner,sun6i-a31-ahb1-clk",
> +		sun6i_ahb1_clk_setup);
> -- 
> 2.1.1
> 

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140930/c0d611b5/attachment-0001.sig>

[PATCH v2 7/7] ARM: dts: sun6i: Add required ahb1 clock parent and rates for dma controller

[Maxime Ripard]

On Sat, Sep 27, 2014 at 04:49:55PM +0800, Chen-Yu Tsai wrote:
> The DMA controller requires AHB1 bus clock to be clocked from PLL6.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  arch/arm/boot/dts/sun6i-a31.dtsi | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
> index caf879d..d7845d3 100644
> --- a/arch/arm/boot/dts/sun6i-a31.dtsi
> +++ b/arch/arm/boot/dts/sun6i-a31.dtsi
> @@ -353,6 +353,11 @@
>  			clocks = <&ahb1_gates 6>;
>  			resets = <&ahb1_rst 6>;
>  			#dma-cells = <1>;
> +
> +			/* DMA controller requires AHB1 clocked from PLL6 */
> +			assigned-clocks = <&ahb1>;
> +			assigned-clock-parents = <&pll6 0>;
> +			assigned-clock-rates = <200000000>;

Again, I don't think we need the rate change, and that it's the right
place to put it.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140930/68cbd249/attachment.sig>

[PATCH v2 1/7] clk: sunxi: Specify number of child clocks for divs clocks

[Chen-Yu Tsai]

On Tue, Sep 30, 2014 at 11:40 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Sat, Sep 27, 2014 at 04:49:49PM +0800, Chen-Yu Tsai wrote:
>> Currently sunxi_divs_clk_setup assumes the number of child clocks
>> to be the same as the number of clock-output-names, and a maximum
>> of SUNXI_DIVS_MAX_QTY child clocks.
>>
>> On sun6i, PLL6 only has 1 child clock, but the parent would be used
>> as well, thereby also having it's own clock-output-names entry. This
>> results in an extra bogus clock being registered.
>
> Isn't PLL6 having two childs as well?

No. It has one child, the normal PLL6 output clock.

The last "clock-output-names" entry refers to the factors clock itself,
in this case, the 2x clock output.

ChenYu

[PATCH v2 1/7] clk: sunxi: Specify number of child clocks for divs clocks

[Maxime Ripard]

On Tue, Sep 30, 2014 at 11:56:24PM +0800, Chen-Yu Tsai wrote:
> On Tue, Sep 30, 2014 at 11:40 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Sat, Sep 27, 2014 at 04:49:49PM +0800, Chen-Yu Tsai wrote:
> >> Currently sunxi_divs_clk_setup assumes the number of child clocks
> >> to be the same as the number of clock-output-names, and a maximum
> >> of SUNXI_DIVS_MAX_QTY child clocks.
> >>
> >> On sun6i, PLL6 only has 1 child clock, but the parent would be used
> >> as well, thereby also having it's own clock-output-names entry. This
> >> results in an extra bogus clock being registered.
> >
> > Isn't PLL6 having two childs as well?
> 
> No. It has one child, the normal PLL6 output clock.
> 
> The last "clock-output-names" entry refers to the factors clock itself,
> in this case, the 2x clock output.

Hmmm, OK, I guess that's one way to see it too :)

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141003/dd092d92/attachment.sig>

[PATCH v2 4/7] clk: sunxi: unify sun6i AHB1 clock with proper PLL6 pre-divider

[Chen-Yu Tsai]

On Tue, Sep 30, 2014 at 11:54 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Sat, Sep 27, 2014 at 04:49:52PM +0800, Chen-Yu Tsai wrote:
>> This patch unifies the sun6i AHB1 clock, originally supported
>> with separate mux and divider clks. It also adds support for
>> the pre-divider on the PLL6 input, thus allowing the clock to
>> be muxed to PLL6 with proper clock rate calculation.
>>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> ---
>>  Documentation/devicetree/bindings/clock/sunxi.txt |   2 +-
>>  drivers/clk/sunxi/clk-sunxi.c                     | 209 ++++++++++++++++++++++
>>  2 files changed, 210 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
>> index 0d84f4b..e862818 100644
>> --- a/Documentation/devicetree/bindings/clock/sunxi.txt
>> +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
>> @@ -23,7 +23,7 @@ Required properties:
>>       "allwinner,sun5i-a10s-ahb-gates-clk" - for the AHB gates on A10s
>>       "allwinner,sun7i-a20-ahb-gates-clk" - for the AHB gates on A20
>>       "allwinner,sun6i-a31-ar100-clk" - for the AR100 on A31
>> -     "allwinner,sun6i-a31-ahb1-mux-clk" - for the AHB1 multiplexer on A31
>> +     "allwinner,sun6i-a31-ahb1-clk" - for the AHB1 clock on A31
>>       "allwinner,sun6i-a31-ahb1-gates-clk" - for the AHB1 gates on A31
>>       "allwinner,sun8i-a23-ahb1-gates-clk" - for the AHB1 gates on A23
>>       "allwinner,sun4i-a10-apb0-clk" - for the APB0 clock
>> diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
>> index 9caebff..7151e2c 100644
>> --- a/drivers/clk/sunxi/clk-sunxi.c
>> +++ b/drivers/clk/sunxi/clk-sunxi.c
>> @@ -19,6 +19,7 @@
>>  #include <linux/of.h>
>>  #include <linux/of_address.h>
>>  #include <linux/reset-controller.h>
>> +#include <linux/log2.h>
>>
>>  #include "clk-factors.h"
>>
>> @@ -1357,3 +1358,211 @@ static void __init sun6i_init_clocks(struct device_node *node)
>>  }
>>  CLK_OF_DECLARE(sun6i_a31_clk_init, "allwinner,sun6i-a31", sun6i_init_clocks);
>>  CLK_OF_DECLARE(sun8i_a23_clk_init, "allwinner,sun8i-a23", sun6i_init_clocks);
>> +
>> +
>
> Drop the extra newline

OK.

>> +/**
>> + * sun6i_a31_ahb1_clk_setup() - Setup function for a31 ahb1 composite clk
>> + */
>> +
>> +#define SUN6I_AHB1_MAX_PARENTS               4
>> +#define SUN6I_AHB1_MUX_PARENT_PLL6   3
>> +#define SUN6I_AHB1_MUX_SHIFT         12
>> +#define SUN6I_AHB1_MUX_MASK          0x3
>> +#define SUN6I_AHB1_MUX_GET_PARENT(reg)       ((reg >> SUN6I_AHB1_MUX_SHIFT) & \
>> +                                      SUN6I_AHB1_MUX_MASK)
>> +#define SUN6I_AHB1_DIV_SHIFT         4
>> +#define SUN6I_AHB1_DIV_MASK          0x3
>> +#define SUN6I_AHB1_DIV_GET(reg)              ((reg >> SUN6I_AHB1_DIV_SHIFT) & \
>> +                                      SUN6I_AHB1_DIV_MASK)
>> +#define SUN6I_AHB1_DIV_SET(reg, div) ((reg & ~(SUN6I_AHB1_DIV_MASK << \
>> +                                               SUN6I_AHB1_DIV_SHIFT)) | \
>> +                                      (div << SUN6I_AHB1_DIV_SHIFT))
>> +#define SUN6I_AHB1_PLL6_DIV_SHIFT    6
>> +#define SUN6I_AHB1_PLL6_DIV_MASK     0x3
>> +#define SUN6I_AHB1_PLL6_DIV_GET(reg) ((reg >> SUN6I_AHB1_PLL6_DIV_SHIFT) & \
>> +                                      SUN6I_AHB1_PLL6_DIV_MASK)
>> +#define SUN6I_AHB1_PLL6_DIV_SET(reg, div) ((reg & \
>> +                                         ~(SUN6I_AHB1_PLL6_DIV_MASK << \
>> +                                           SUN6I_AHB1_PLL6_DIV_SHIFT)) | \
>> +                                        (div << SUN6I_AHB1_PLL6_DIV_SHIFT))
>
> Your indentation looks really odd, and the masks you have should
> really point to the actual mask, and not just the number of bits.

It is an attempt to match open parenthesis. Guess that's not a good idea.
Will switch to normal indentation. And fix the masks.

>> +
>> +struct sun6i_ahb1_clk {
>> +     struct clk_hw hw;
>> +     void __iomem *reg;
>> +};
>> +
>> +#define to_sun6i_ahb1_clk(_hw) container_of(_hw, struct sun6i_ahb1_clk, hw)
>> +
>> +static unsigned long sun6i_ahb1_clk_recalc_rate(struct clk_hw *hw,
>> +                                             unsigned long parent_rate)
>> +{
>> +     struct sun6i_ahb1_clk *ahb1 = to_sun6i_ahb1_clk(hw);
>> +     unsigned long rate;
>> +     u32 reg;
>> +
>> +     /* Fetch the register value */
>> +     reg = readl(ahb1->reg);
>> +
>> +     /* apply pre-divider first if parent is pll6 */
>> +     if (SUN6I_AHB1_MUX_GET_PARENT(reg) == SUN6I_AHB1_MUX_PARENT_PLL6)
>> +             parent_rate /= SUN6I_AHB1_PLL6_DIV_GET(reg) + 1;
>> +
>> +     /* clk divider */
>> +     rate = parent_rate >> SUN6I_AHB1_DIV_GET(reg);
>> +
>> +     return rate;
>> +}
>> +
>> +static long sun6i_ahb1_clk_round(unsigned long rate, u8 *divp, u8 *pre_divp,
>> +                              u8 parent, unsigned long parent_rate)
>> +{
>> +     u8 div, calcp, calcm = 1;
>> +
>> +     /* clock can only divide, so we will never be able to achieve
>> +      * frequencies higher than the parent frequency */
>
> Wrong multiline comment style

I think I've done this twice already. This is from factors clk.
I'll send a patch to fix the original.

>> +     if (parent_rate && rate > parent_rate)
>> +             rate = parent_rate;
>> +
>> +     div = DIV_ROUND_UP(parent_rate, rate);
>> +
>> +     /* calculate pre-divider if parent is pll6 */
>> +     if (parent == SUN6I_AHB1_MUX_PARENT_PLL6) {
>> +             if (div < 4)
>> +                     calcp = 0;
>> +             else if (div / 2 < 4)
>> +                     calcp = 1;
>> +             else if (div / 4 < 4)
>> +                     calcp = 2;
>> +             else
>> +                     calcp = 3;
>> +
>> +             calcm = DIV_ROUND_UP(div, 1 << calcp);
>> +     } else {
>> +             calcp = __roundup_pow_of_two(div);
>> +             calcp = calcp > 3 ? 3 : calcp;
>> +     }
>> +
>> +     if (divp) {
>> +             *divp = calcp;
>> +             *pre_divp = calcm - 1;
>> +     }
>> +
>> +     return (parent_rate / calcm) >> calcp;
>> +}
>> +
>> +static long sun6i_ahb1_clk_determine_rate(struct clk_hw *hw, unsigned long rate,
>> +                                       unsigned long *best_parent_rate,
>> +                                       struct clk **best_parent_clk)
>> +{
>> +     struct clk *clk = hw->clk, *parent, *best_parent = NULL;
>> +     int i, num_parents;
>> +     unsigned long parent_rate, best = 0, child_rate, best_child_rate = 0;
>> +
>> +     /* find the parent that can help provide the fastest rate <= rate */
>> +     num_parents = __clk_get_num_parents(clk);
>> +     for (i = 0; i < num_parents; i++) {
>> +             parent = clk_get_parent_by_index(clk, i);
>> +             if (!parent)
>> +                     continue;
>> +             if (__clk_get_flags(clk) & CLK_SET_RATE_PARENT)
>> +                     parent_rate = __clk_round_rate(parent, rate);
>> +             else
>> +                     parent_rate = __clk_get_rate(parent);
>> +
>> +             child_rate = sun6i_ahb1_clk_round(rate, NULL, NULL, i,
>> +                                               parent_rate);
>> +
>> +             if (child_rate <= rate && child_rate > best_child_rate) {
>> +                     best_parent = parent;
>> +                     best = parent_rate;
>> +                     best_child_rate = child_rate;
>> +             }
>> +     }
>> +
>> +     if (best_parent)
>> +             *best_parent_clk = best_parent;
>> +     *best_parent_rate = best;
>> +
>> +     return best_child_rate;
>> +}
>> +
>> +static int sun6i_ahb1_clk_set_rate(struct clk_hw *hw, unsigned long rate,
>> +                                unsigned long parent_rate)
>> +{
>> +     struct sun6i_ahb1_clk *ahb1 = to_sun6i_ahb1_clk(hw);
>> +     unsigned long flags;
>> +     u8 div, pre_div, parent;
>> +     u32 reg;
>> +
>> +     spin_lock_irqsave(&clk_lock, flags);
>
> Isn't that already taken by the composite clock?

Last I checked, composite clock just calls the callbacks.

>> +
>> +     reg = readl(ahb1->reg);
>> +
>> +     /* need to know which parent is used to apply pre-divider */
>> +     parent = SUN6I_AHB1_MUX_GET_PARENT(reg);
>> +     sun6i_ahb1_clk_round(rate, &div, &pre_div, parent, parent_rate);
>
> Haven't you called that already in determine_rate?

The call in determine_rate only returns a rounded clock rate.
Here we actually get the divider values.
I will add a comment in sun6i_ahb1_clk_round.

>> +
>> +     reg = SUN6I_AHB1_DIV_SET(reg, div);
>> +     reg = SUN6I_AHB1_PLL6_DIV_SET(reg, pre_div);
>> +     writel(reg, ahb1->reg);
>> +
>> +     spin_unlock_irqrestore(&clk_lock, flags);
>> +
>> +     return 0;
>> +}
>> +
>> +static const struct clk_ops sun6i_ahb1_clk_ops = {
>> +     .determine_rate = sun6i_ahb1_clk_determine_rate,
>> +     .recalc_rate    = sun6i_ahb1_clk_recalc_rate,
>> +     .set_rate       = sun6i_ahb1_clk_set_rate,
>> +};
>> +
>> +static void __init sun6i_ahb1_clk_setup(struct device_node *node)
>> +{
>> +     struct clk *clk;
>> +     struct sun6i_ahb1_clk *ahb1;
>> +     struct clk_mux *mux;
>> +     const char *clk_name = node->name;
>> +     const char *parents[SUN6I_AHB1_MAX_PARENTS];
>> +     void __iomem *reg;
>> +     int i = 0;
>> +
>> +     reg = of_iomap(node, 0);
>
> of_io_request_and_map please :)

This is new. Will switch. :)

>> +
>> +     /* we have a mux, we will have >1 parents */
>> +     while (i < SUN6I_AHB1_MAX_PARENTS &&
>> +            (parents[i] = of_clk_get_parent_name(node, i)) != NULL)
>> +             i++;
>> +
>> +     of_property_read_string(node, "clock-output-names", &clk_name);
>> +
>> +     ahb1 = kzalloc(sizeof(struct sun6i_ahb1_clk), GFP_KERNEL);
>> +     if (!ahb1)
>> +             return;
>> +
>> +     mux = kzalloc(sizeof(struct clk_mux), GFP_KERNEL);
>> +     if (!mux) {
>> +             kfree(ahb1);
>> +             return;
>> +     }
>> +
>> +     /* set up clock properties */
>> +     mux->reg = reg;
>> +     mux->shift = SUN6I_AHB1_MUX_SHIFT;
>> +     mux->mask = SUN6I_AHB1_MUX_MASK;
>> +     mux->lock = &clk_lock;
>> +     ahb1->reg = reg;
>> +
>> +     clk = clk_register_composite(NULL, clk_name, parents, i,
>> +                                  &mux->hw, &clk_mux_ops,
>> +                                  &ahb1->hw, &sun6i_ahb1_clk_ops,
>> +                                  NULL, NULL, 0);
>> +
>> +     if (!IS_ERR(clk)) {
>> +             of_clk_add_provider(node, of_clk_src_simple_get, clk);
>> +             clk_register_clkdev(clk, clk_name, NULL);
>> +     }
>> +}
>> +
>> +CLK_OF_DECLARE(sun6i_a31_ahb1, "allwinner,sun6i-a31-ahb1-clk",
>> +             sun6i_ahb1_clk_setup);
>> --
>> 2.1.1

Thanks!

ChenYu

[PATCH v2 4/7] clk: sunxi: unify sun6i AHB1 clock with proper PLL6 pre-divider

[Chen-Yu Tsai]

On Mon, Oct 6, 2014 at 8:58 PM, Chen-Yu Tsai <wens@csie.org> wrote:
> On Tue, Sep 30, 2014 at 11:54 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
>> On Sat, Sep 27, 2014 at 04:49:52PM +0800, Chen-Yu Tsai wrote:
>>> This patch unifies the sun6i AHB1 clock, originally supported
>>> with separate mux and divider clks. It also adds support for
>>> the pre-divider on the PLL6 input, thus allowing the clock to
>>> be muxed to PLL6 with proper clock rate calculation.
>>>
>>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>>> ---
>>>  Documentation/devicetree/bindings/clock/sunxi.txt |   2 +-
>>>  drivers/clk/sunxi/clk-sunxi.c                     | 209 ++++++++++++++++++++++
>>>  2 files changed, 210 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
>>> index 0d84f4b..e862818 100644
>>> --- a/Documentation/devicetree/bindings/clock/sunxi.txt
>>> +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
>>> @@ -23,7 +23,7 @@ Required properties:
>>>       "allwinner,sun5i-a10s-ahb-gates-clk" - for the AHB gates on A10s
>>>       "allwinner,sun7i-a20-ahb-gates-clk" - for the AHB gates on A20
>>>       "allwinner,sun6i-a31-ar100-clk" - for the AR100 on A31
>>> -     "allwinner,sun6i-a31-ahb1-mux-clk" - for the AHB1 multiplexer on A31
>>> +     "allwinner,sun6i-a31-ahb1-clk" - for the AHB1 clock on A31
>>>       "allwinner,sun6i-a31-ahb1-gates-clk" - for the AHB1 gates on A31
>>>       "allwinner,sun8i-a23-ahb1-gates-clk" - for the AHB1 gates on A23
>>>       "allwinner,sun4i-a10-apb0-clk" - for the APB0 clock
>>> diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
>>> index 9caebff..7151e2c 100644
>>> --- a/drivers/clk/sunxi/clk-sunxi.c
>>> +++ b/drivers/clk/sunxi/clk-sunxi.c
>>> @@ -19,6 +19,7 @@
>>>  #include <linux/of.h>
>>>  #include <linux/of_address.h>
>>>  #include <linux/reset-controller.h>
>>> +#include <linux/log2.h>
>>>
>>>  #include "clk-factors.h"
>>>
>>> @@ -1357,3 +1358,211 @@ static void __init sun6i_init_clocks(struct device_node *node)
>>>  }
>>>  CLK_OF_DECLARE(sun6i_a31_clk_init, "allwinner,sun6i-a31", sun6i_init_clocks);
>>>  CLK_OF_DECLARE(sun8i_a23_clk_init, "allwinner,sun8i-a23", sun6i_init_clocks);
>>> +
>>> +
>>
>> Drop the extra newline
>
> OK.
>
>>> +/**
>>> + * sun6i_a31_ahb1_clk_setup() - Setup function for a31 ahb1 composite clk
>>> + */
>>> +
>>> +#define SUN6I_AHB1_MAX_PARENTS               4
>>> +#define SUN6I_AHB1_MUX_PARENT_PLL6   3
>>> +#define SUN6I_AHB1_MUX_SHIFT         12
>>> +#define SUN6I_AHB1_MUX_MASK          0x3
>>> +#define SUN6I_AHB1_MUX_GET_PARENT(reg)       ((reg >> SUN6I_AHB1_MUX_SHIFT) & \
>>> +                                      SUN6I_AHB1_MUX_MASK)
>>> +#define SUN6I_AHB1_DIV_SHIFT         4
>>> +#define SUN6I_AHB1_DIV_MASK          0x3
>>> +#define SUN6I_AHB1_DIV_GET(reg)              ((reg >> SUN6I_AHB1_DIV_SHIFT) & \
>>> +                                      SUN6I_AHB1_DIV_MASK)
>>> +#define SUN6I_AHB1_DIV_SET(reg, div) ((reg & ~(SUN6I_AHB1_DIV_MASK << \
>>> +                                               SUN6I_AHB1_DIV_SHIFT)) | \
>>> +                                      (div << SUN6I_AHB1_DIV_SHIFT))
>>> +#define SUN6I_AHB1_PLL6_DIV_SHIFT    6
>>> +#define SUN6I_AHB1_PLL6_DIV_MASK     0x3
>>> +#define SUN6I_AHB1_PLL6_DIV_GET(reg) ((reg >> SUN6I_AHB1_PLL6_DIV_SHIFT) & \
>>> +                                      SUN6I_AHB1_PLL6_DIV_MASK)
>>> +#define SUN6I_AHB1_PLL6_DIV_SET(reg, div) ((reg & \
>>> +                                         ~(SUN6I_AHB1_PLL6_DIV_MASK << \
>>> +                                           SUN6I_AHB1_PLL6_DIV_SHIFT)) | \
>>> +                                        (div << SUN6I_AHB1_PLL6_DIV_SHIFT))
>>
>> Your indentation looks really odd, and the masks you have should
>> really point to the actual mask, and not just the number of bits.
>
> It is an attempt to match open parenthesis. Guess that's not a good idea.
> Will switch to normal indentation. And fix the masks.

Just took a closer look at the code. The reason MUX_MASK is not shifted
is because clk_mux_ops expects them that way.

I could get rid of SUN6I_AHB1_MUX_GET_PARENT and call clk_mux_ops.get_parent,
but I don't think that's better.

*_DIV_MASK I will fix. Only issue is line length. :(

ChenYu

>>> +
>>> +struct sun6i_ahb1_clk {
>>> +     struct clk_hw hw;
>>> +     void __iomem *reg;
>>> +};
>>> +
>>> +#define to_sun6i_ahb1_clk(_hw) container_of(_hw, struct sun6i_ahb1_clk, hw)
>>> +
>>> +static unsigned long sun6i_ahb1_clk_recalc_rate(struct clk_hw *hw,
>>> +                                             unsigned long parent_rate)
>>> +{
>>> +     struct sun6i_ahb1_clk *ahb1 = to_sun6i_ahb1_clk(hw);
>>> +     unsigned long rate;
>>> +     u32 reg;
>>> +
>>> +     /* Fetch the register value */
>>> +     reg = readl(ahb1->reg);
>>> +
>>> +     /* apply pre-divider first if parent is pll6 */
>>> +     if (SUN6I_AHB1_MUX_GET_PARENT(reg) == SUN6I_AHB1_MUX_PARENT_PLL6)
>>> +             parent_rate /= SUN6I_AHB1_PLL6_DIV_GET(reg) + 1;
>>> +
>>> +     /* clk divider */
>>> +     rate = parent_rate >> SUN6I_AHB1_DIV_GET(reg);
>>> +
>>> +     return rate;
>>> +}
>>> +
>>> +static long sun6i_ahb1_clk_round(unsigned long rate, u8 *divp, u8 *pre_divp,
>>> +                              u8 parent, unsigned long parent_rate)
>>> +{
>>> +     u8 div, calcp, calcm = 1;
>>> +
>>> +     /* clock can only divide, so we will never be able to achieve
>>> +      * frequencies higher than the parent frequency */
>>
>> Wrong multiline comment style
>
> I think I've done this twice already. This is from factors clk.
> I'll send a patch to fix the original.
>
>>> +     if (parent_rate && rate > parent_rate)
>>> +             rate = parent_rate;
>>> +
>>> +     div = DIV_ROUND_UP(parent_rate, rate);
>>> +
>>> +     /* calculate pre-divider if parent is pll6 */
>>> +     if (parent == SUN6I_AHB1_MUX_PARENT_PLL6) {
>>> +             if (div < 4)
>>> +                     calcp = 0;
>>> +             else if (div / 2 < 4)
>>> +                     calcp = 1;
>>> +             else if (div / 4 < 4)
>>> +                     calcp = 2;
>>> +             else
>>> +                     calcp = 3;
>>> +
>>> +             calcm = DIV_ROUND_UP(div, 1 << calcp);
>>> +     } else {
>>> +             calcp = __roundup_pow_of_two(div);
>>> +             calcp = calcp > 3 ? 3 : calcp;
>>> +     }
>>> +
>>> +     if (divp) {
>>> +             *divp = calcp;
>>> +             *pre_divp = calcm - 1;
>>> +     }
>>> +
>>> +     return (parent_rate / calcm) >> calcp;
>>> +}
>>> +
>>> +static long sun6i_ahb1_clk_determine_rate(struct clk_hw *hw, unsigned long rate,
>>> +                                       unsigned long *best_parent_rate,
>>> +                                       struct clk **best_parent_clk)
>>> +{
>>> +     struct clk *clk = hw->clk, *parent, *best_parent = NULL;
>>> +     int i, num_parents;
>>> +     unsigned long parent_rate, best = 0, child_rate, best_child_rate = 0;
>>> +
>>> +     /* find the parent that can help provide the fastest rate <= rate */
>>> +     num_parents = __clk_get_num_parents(clk);
>>> +     for (i = 0; i < num_parents; i++) {
>>> +             parent = clk_get_parent_by_index(clk, i);
>>> +             if (!parent)
>>> +                     continue;
>>> +             if (__clk_get_flags(clk) & CLK_SET_RATE_PARENT)
>>> +                     parent_rate = __clk_round_rate(parent, rate);
>>> +             else
>>> +                     parent_rate = __clk_get_rate(parent);
>>> +
>>> +             child_rate = sun6i_ahb1_clk_round(rate, NULL, NULL, i,
>>> +                                               parent_rate);
>>> +
>>> +             if (child_rate <= rate && child_rate > best_child_rate) {
>>> +                     best_parent = parent;
>>> +                     best = parent_rate;
>>> +                     best_child_rate = child_rate;
>>> +             }
>>> +     }
>>> +
>>> +     if (best_parent)
>>> +             *best_parent_clk = best_parent;
>>> +     *best_parent_rate = best;
>>> +
>>> +     return best_child_rate;
>>> +}
>>> +
>>> +static int sun6i_ahb1_clk_set_rate(struct clk_hw *hw, unsigned long rate,
>>> +                                unsigned long parent_rate)
>>> +{
>>> +     struct sun6i_ahb1_clk *ahb1 = to_sun6i_ahb1_clk(hw);
>>> +     unsigned long flags;
>>> +     u8 div, pre_div, parent;
>>> +     u32 reg;
>>> +
>>> +     spin_lock_irqsave(&clk_lock, flags);
>>
>> Isn't that already taken by the composite clock?
>
> Last I checked, composite clock just calls the callbacks.
>
>>> +
>>> +     reg = readl(ahb1->reg);
>>> +
>>> +     /* need to know which parent is used to apply pre-divider */
>>> +     parent = SUN6I_AHB1_MUX_GET_PARENT(reg);
>>> +     sun6i_ahb1_clk_round(rate, &div, &pre_div, parent, parent_rate);
>>
>> Haven't you called that already in determine_rate?
>
> The call in determine_rate only returns a rounded clock rate.
> Here we actually get the divider values.
> I will add a comment in sun6i_ahb1_clk_round.
>
>>> +
>>> +     reg = SUN6I_AHB1_DIV_SET(reg, div);
>>> +     reg = SUN6I_AHB1_PLL6_DIV_SET(reg, pre_div);
>>> +     writel(reg, ahb1->reg);
>>> +
>>> +     spin_unlock_irqrestore(&clk_lock, flags);
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static const struct clk_ops sun6i_ahb1_clk_ops = {
>>> +     .determine_rate = sun6i_ahb1_clk_determine_rate,
>>> +     .recalc_rate    = sun6i_ahb1_clk_recalc_rate,
>>> +     .set_rate       = sun6i_ahb1_clk_set_rate,
>>> +};
>>> +
>>> +static void __init sun6i_ahb1_clk_setup(struct device_node *node)
>>> +{
>>> +     struct clk *clk;
>>> +     struct sun6i_ahb1_clk *ahb1;
>>> +     struct clk_mux *mux;
>>> +     const char *clk_name = node->name;
>>> +     const char *parents[SUN6I_AHB1_MAX_PARENTS];
>>> +     void __iomem *reg;
>>> +     int i = 0;
>>> +
>>> +     reg = of_iomap(node, 0);
>>
>> of_io_request_and_map please :)
>
> This is new. Will switch. :)
>
>>> +
>>> +     /* we have a mux, we will have >1 parents */
>>> +     while (i < SUN6I_AHB1_MAX_PARENTS &&
>>> +            (parents[i] = of_clk_get_parent_name(node, i)) != NULL)
>>> +             i++;
>>> +
>>> +     of_property_read_string(node, "clock-output-names", &clk_name);
>>> +
>>> +     ahb1 = kzalloc(sizeof(struct sun6i_ahb1_clk), GFP_KERNEL);
>>> +     if (!ahb1)
>>> +             return;
>>> +
>>> +     mux = kzalloc(sizeof(struct clk_mux), GFP_KERNEL);
>>> +     if (!mux) {
>>> +             kfree(ahb1);
>>> +             return;
>>> +     }
>>> +
>>> +     /* set up clock properties */
>>> +     mux->reg = reg;
>>> +     mux->shift = SUN6I_AHB1_MUX_SHIFT;
>>> +     mux->mask = SUN6I_AHB1_MUX_MASK;
>>> +     mux->lock = &clk_lock;
>>> +     ahb1->reg = reg;
>>> +
>>> +     clk = clk_register_composite(NULL, clk_name, parents, i,
>>> +                                  &mux->hw, &clk_mux_ops,
>>> +                                  &ahb1->hw, &sun6i_ahb1_clk_ops,
>>> +                                  NULL, NULL, 0);
>>> +
>>> +     if (!IS_ERR(clk)) {
>>> +             of_clk_add_provider(node, of_clk_src_simple_get, clk);
>>> +             clk_register_clkdev(clk, clk_name, NULL);
>>> +     }
>>> +}
>>> +
>>> +CLK_OF_DECLARE(sun6i_a31_ahb1, "allwinner,sun6i-a31-ahb1-clk",
>>> +             sun6i_ahb1_clk_setup);
>>> --
>>> 2.1.1
>
> Thanks!
>
> ChenYu

[PATCH v2 7/7] ARM: dts: sun6i: Add required ahb1 clock parent and rates for dma controller

[Chen-Yu Tsai]

On Tue, Sep 30, 2014 at 11:55 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Sat, Sep 27, 2014 at 04:49:55PM +0800, Chen-Yu Tsai wrote:
>> The DMA controller requires AHB1 bus clock to be clocked from PLL6.
>>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> ---
>>  arch/arm/boot/dts/sun6i-a31.dtsi | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
>> index caf879d..d7845d3 100644
>> --- a/arch/arm/boot/dts/sun6i-a31.dtsi
>> +++ b/arch/arm/boot/dts/sun6i-a31.dtsi
>> @@ -353,6 +353,11 @@
>>                       clocks = <&ahb1_gates 6>;
>>                       resets = <&ahb1_rst 6>;
>>                       #dma-cells = <1>;
>> +
>> +                     /* DMA controller requires AHB1 clocked from PLL6 */
>> +                     assigned-clocks = <&ahb1>;
>> +                     assigned-clock-parents = <&pll6 0>;
>> +                     assigned-clock-rates = <200000000>;
>
> Again, I don't think we need the rate change, and that it's the right
> place to put it.

I'll drop it. As you mentioned on IRC, re-parenting to pll6
is still required, so I'll keep that bit.

Hi, Mike,

Is there a recommended way for clock providers to enforce a range
on clock rates? Tomeu's patches seem to be for clock consumers.

Cheers
ChenYu

[PATCH v2 7/7] ARM: dts: sun6i: Add required ahb1 clock parent and rates for dma controller

[Maxime Ripard]

On Thu, Oct 09, 2014 at 11:01:02AM +0800, Chen-Yu Tsai wrote:
> Hi, Mike,
> 
> Is there a recommended way for clock providers to enforce a range
> on clock rates? Tomeu's patches seem to be for clock consumers.

We discussed that in the past with Mike, and his answer at the time
was that every driver should implement that, which I did some time
ago.

However, it was before Tomeu patches, that indeed enforce constraints
at the consumer level, but I'm pretty confident we could add some
provider-side constraints quite easily.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141013/6586ff00/attachment-0001.sig>