[PATCH RFC 00/11] clk: sunxi: factors clk clean up and refactor

[PATCH RFC 00/11] clk: sunxi: factors clk clean up and refactor

[Chen-Yu Tsai]

Hi everyone,

This series cleans up and reworks parts of sunxi's factors clk. The goal
is to support non-standard formulas for clock rate calculation, such as
pre-dividers on some parents, or all power-of-2 dividers. One such clock
is the AHB1 clock on A31/A31s.

Patch 1 is Maxime's patch adding an unregister function for composite
clocks. Patches 3 and 4 use this, so it is included for completeness.

Patch 2 makes the config tables for factors clk constant. These contain
the shift and width for the factors. They are used to manipulate the
clk register values. There should be no reason to change them in-flight.

Patch 3 adds a proper error path for the factors clk register function(),
so we don't leak memory when a call fails.

Patch 4 adds an unregister function for factors clks.

Patch 5 adds an error patch to sunxi_factors_clk_setup()

Patch 6 packs the parameters passed to get_factors callbacks in a struct.
This makes it easier to extend factors clk without having to edit all
the function definitions, and also makes the lines shorter.

Patch 7 makes factors clk support custom formulas for calculating clock
rates. On the clock rounding/setting side, we only need to teach
get_factors about different parent clocks. On the recalc side, we add
support for custom .recalc callbacks for clocks that need them.

Patch 8 drops .round_rate from factors clk ops. Since only one of
.round_rate and .determine_rate is needed, and the clk core prefers the
latter, remove .round_rate.

Patch 9 rewrites sun6i-a31-ahb1-clk using factors clk with the new custom
formula support. sun6i-a31-ahb1 has a pre-divider on one of its parents.

Patch 10 rewrite sun6i-ar100 using factors clk.

Patch 11 rewrites sun8i-a23-mbus-clk using the simpler composite clk.
While this patch is doing the reverse, i.e. rewriting a factors clk into
a composite clk, it is included because some changes overlap. I'm not
sure whether this approach is worthwhile, as it actually adds more code,
though it might make it easier to understand.


Regards
ChenYu


Chen-Yu Tsai (10):
  clk: sunxi: factors: Make struct clk_factors_config table const
  clk: sunxi: factors: Add clk cleanup in sunxi_factors_register() error
    path
  clk: sunxi: factors: Add unregister function
  clk: sunxi: unmap registers in sunxi_factors_clk_setup if register
    call fails
  clk: sunxi: factors: Consolidate get_factors parameters into a struct
  clk: sunxi: factors: Support custom formulas
  clk: sunxi: factors: Drop round_rate from clk ops
  clk: sunxi: rewrite sun6i-a31-ahb1-clk using factors clk with custom
    recalc
  clk: sunxi: rewrite sun6i-ar100 using factors clk
  clk: sunxi: rewrite sun8i-a23-mbus-clk using the simpler composite clk

Maxime Ripard (1):
  clk: composite: Add unregister function

 drivers/clk/clk-composite.c         |  15 +
 drivers/clk/sunxi/clk-factors.c     | 127 ++++++---
 drivers/clk/sunxi/clk-factors.h     |  25 +-
 drivers/clk/sunxi/clk-mod0.c        |  22 +-
 drivers/clk/sunxi/clk-sun6i-ar100.c | 235 ++++------------
 drivers/clk/sunxi/clk-sun8i-mbus.c  | 126 +++++----
 drivers/clk/sunxi/clk-sun9i-core.c  |  85 ++----
 drivers/clk/sunxi/clk-sunxi.c       | 540 +++++++++++++-----------------------
 include/linux/clk-provider.h        |   1 +
 9 files changed, 492 insertions(+), 684 deletions(-)

-- 
2.7.0

[PATCH RFC 00/11] clk: sunxi: factors clk clean up and refactor

[Chen-Yu Tsai]

Hi everyone,

This series cleans up and reworks parts of sunxi's factors clk. The goal
is to support non-standard formulas for clock rate calculation, such as
pre-dividers on some parents, or all power-of-2 dividers. One such clock
is the AHB1 clock on A31/A31s.

Patch 1 is Maxime's patch adding an unregister function for composite
clocks. Patches 3 and 4 use this, so it is included for completeness.

Patch 2 makes the config tables for factors clk constant. These contain
the shift and width for the factors. They are used to manipulate the
clk register values. There should be no reason to change them in-flight.

Patch 3 adds a proper error path for the factors clk register function(),
so we don't leak memory when a call fails.

Patch 4 adds an unregister function for factors clks.

Patch 5 adds an error patch to sunxi_factors_clk_setup()

Patch 6 packs the parameters passed to get_factors callbacks in a struct.
This makes it easier to extend factors clk without having to edit all
the function definitions, and also makes the lines shorter.

Patch 7 makes factors clk support custom formulas for calculating clock
rates. On the clock rounding/setting side, we only need to teach
get_factors about different parent clocks. On the recalc side, we add
support for custom .recalc callbacks for clocks that need them.

Patch 8 drops .round_rate from factors clk ops. Since only one of
.round_rate and .determine_rate is needed, and the clk core prefers the
latter, remove .round_rate.

Patch 9 rewrites sun6i-a31-ahb1-clk using factors clk with the new custom
formula support. sun6i-a31-ahb1 has a pre-divider on one of its parents.

Patch 10 rewrite sun6i-ar100 using factors clk.

Patch 11 rewrites sun8i-a23-mbus-clk using the simpler composite clk.
While this patch is doing the reverse, i.e. rewriting a factors clk into
a composite clk, it is included because some changes overlap. I'm not
sure whether this approach is worthwhile, as it actually adds more code,
though it might make it easier to understand.


Regards
ChenYu


Chen-Yu Tsai (10):
  clk: sunxi: factors: Make struct clk_factors_config table const
  clk: sunxi: factors: Add clk cleanup in sunxi_factors_register() error
    path
  clk: sunxi: factors: Add unregister function
  clk: sunxi: unmap registers in sunxi_factors_clk_setup if register
    call fails
  clk: sunxi: factors: Consolidate get_factors parameters into a struct
  clk: sunxi: factors: Support custom formulas
  clk: sunxi: factors: Drop round_rate from clk ops
  clk: sunxi: rewrite sun6i-a31-ahb1-clk using factors clk with custom
    recalc
  clk: sunxi: rewrite sun6i-ar100 using factors clk
  clk: sunxi: rewrite sun8i-a23-mbus-clk using the simpler composite clk

Maxime Ripard (1):
  clk: composite: Add unregister function

 drivers/clk/clk-composite.c         |  15 +
 drivers/clk/sunxi/clk-factors.c     | 127 ++++++---
 drivers/clk/sunxi/clk-factors.h     |  25 +-
 drivers/clk/sunxi/clk-mod0.c        |  22 +-
 drivers/clk/sunxi/clk-sun6i-ar100.c | 235 ++++------------
 drivers/clk/sunxi/clk-sun8i-mbus.c  | 126 +++++----
 drivers/clk/sunxi/clk-sun9i-core.c  |  85 ++----
 drivers/clk/sunxi/clk-sunxi.c       | 540 +++++++++++++-----------------------
 include/linux/clk-provider.h        |   1 +
 9 files changed, 492 insertions(+), 684 deletions(-)

-- 
2.7.0

[PATCH RFC 01/11] clk: composite: Add unregister function

[Chen-Yu Tsai]

From: Maxime Ripard <maxime.ripard@free-electrons.com>

The composite clock didn't have any unregistration function, which forced
us to use clk_unregister directly on it.

While it was already not great from an API point of view, it also meant
that we were leaking the clk_composite structure allocated in
clk_register_composite.

Add a clk_unregister_composite function to fix this.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/clk-composite.c  | 15 +++++++++++++++
 include/linux/clk-provider.h |  1 +
 2 files changed, 16 insertions(+)

diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
index 4735de0660cc..f4c38f209594 100644
--- a/drivers/clk/clk-composite.c
+++ b/drivers/clk/clk-composite.c
@@ -288,3 +288,18 @@ err:
 	kfree(composite);
 	return clk;
 }
+
+void clk_unregister_composite(struct clk *clk)
+{
+	struct clk_composite *composite;
+	struct clk_hw *hw;
+
+	hw = __clk_get_hw(clk);
+	if (!hw)
+		return;
+
+	composite = to_clk_composite(hw);
+
+	clk_unregister(clk);
+	kfree(composite);
+}
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 1143e38555a4..fa3dd6ba970b 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -585,6 +585,7 @@ struct clk *clk_register_composite(struct device *dev, const char *name,
 		struct clk_hw *rate_hw, const struct clk_ops *rate_ops,
 		struct clk_hw *gate_hw, const struct clk_ops *gate_ops,
 		unsigned long flags);
+void clk_unregister_composite(struct clk *clk);
 
 /***
  * struct clk_gpio_gate - gpio gated clock
-- 
2.7.0

[PATCH RFC 01/11] clk: composite: Add unregister function

[Chen-Yu Tsai]

From: Maxime Ripard <maxime.ripard@free-electrons.com>

The composite clock didn't have any unregistration function, which forced
us to use clk_unregister directly on it.

While it was already not great from an API point of view, it also meant
that we were leaking the clk_composite structure allocated in
clk_register_composite.

Add a clk_unregister_composite function to fix this.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/clk-composite.c  | 15 +++++++++++++++
 include/linux/clk-provider.h |  1 +
 2 files changed, 16 insertions(+)

diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
index 4735de0660cc..f4c38f209594 100644
--- a/drivers/clk/clk-composite.c
+++ b/drivers/clk/clk-composite.c
@@ -288,3 +288,18 @@ err:
 	kfree(composite);
 	return clk;
 }
+
+void clk_unregister_composite(struct clk *clk)
+{
+	struct clk_composite *composite;
+	struct clk_hw *hw;
+
+	hw = __clk_get_hw(clk);
+	if (!hw)
+		return;
+
+	composite = to_clk_composite(hw);
+
+	clk_unregister(clk);
+	kfree(composite);
+}
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 1143e38555a4..fa3dd6ba970b 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -585,6 +585,7 @@ struct clk *clk_register_composite(struct device *dev, const char *name,
 		struct clk_hw *rate_hw, const struct clk_ops *rate_ops,
 		struct clk_hw *gate_hw, const struct clk_ops *gate_ops,
 		unsigned long flags);
+void clk_unregister_composite(struct clk *clk);
 
 /***
  * struct clk_gpio_gate - gpio gated clock
-- 
2.7.0

[PATCH RFC 02/11] clk: sunxi: factors: Make struct clk_factors_config table const

[Chen-Yu Tsai]

struct clk_factors_config contains shifts/widths for the factors of
the factors clk. This is used to read out the factors from the register
value. In no case is it written to, so make it const.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/clk/sunxi/clk-factors.c    |  4 ++--
 drivers/clk/sunxi/clk-factors.h    |  4 ++--
 drivers/clk/sunxi/clk-mod0.c       |  2 +-
 drivers/clk/sunxi/clk-sun9i-core.c |  8 ++++----
 drivers/clk/sunxi/clk-sunxi.c      | 16 ++++++++--------
 5 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
index 59428dbd607a..928c079da193 100644
--- a/drivers/clk/sunxi/clk-factors.c
+++ b/drivers/clk/sunxi/clk-factors.c
@@ -48,7 +48,7 @@ static unsigned long clk_factors_recalc_rate(struct clk_hw *hw,
 	u32 reg;
 	unsigned long rate;
 	struct clk_factors *factors = to_clk_factors(hw);
-	struct clk_factors_config *config = factors->config;
+	const struct clk_factors_config *config = factors->config;
 
 	/* Fetch the register value */
 	reg = readl(factors->reg);
@@ -123,7 +123,7 @@ static int clk_factors_set_rate(struct clk_hw *hw, unsigned long rate,
 	u8 n = 0, k = 0, m = 0, p = 0;
 	u32 reg;
 	struct clk_factors *factors = to_clk_factors(hw);
-	struct clk_factors_config *config = factors->config;
+	const struct clk_factors_config *config = factors->config;
 	unsigned long flags = 0;
 
 	factors->get_factors((u32 *)&rate, (u32)parent_rate, &n, &k, &m, &p);
diff --git a/drivers/clk/sunxi/clk-factors.h b/drivers/clk/sunxi/clk-factors.h
index 171085ab5513..060319be2b99 100644
--- a/drivers/clk/sunxi/clk-factors.h
+++ b/drivers/clk/sunxi/clk-factors.h
@@ -23,7 +23,7 @@ struct factors_data {
 	int enable;
 	int mux;
 	int muxmask;
-	struct clk_factors_config *table;
+	const struct clk_factors_config *table;
 	void (*getter) (u32 *rate, u32 parent_rate, u8 *n, u8 *k, u8 *m, u8 *p);
 	const char *name;
 };
@@ -31,7 +31,7 @@ struct factors_data {
 struct clk_factors {
 	struct clk_hw hw;
 	void __iomem *reg;
-	struct clk_factors_config *config;
+	const struct clk_factors_config *config;
 	void (*get_factors) (u32 *rate, u32 parent, u8 *n, u8 *k, u8 *m, u8 *p);
 	spinlock_t *lock;
 };
diff --git a/drivers/clk/sunxi/clk-mod0.c b/drivers/clk/sunxi/clk-mod0.c
index d167e1efb927..c67fea1a128d 100644
--- a/drivers/clk/sunxi/clk-mod0.c
+++ b/drivers/clk/sunxi/clk-mod0.c
@@ -62,7 +62,7 @@ static void sun4i_a10_get_mod0_factors(u32 *freq, u32 parent_rate,
 }
 
 /* user manual says "n" but it's really "p" */
-static struct clk_factors_config sun4i_a10_mod0_config = {
+static const struct clk_factors_config sun4i_a10_mod0_config = {
 	.mshift = 0,
 	.mwidth = 4,
 	.pshift = 16,
diff --git a/drivers/clk/sunxi/clk-sun9i-core.c b/drivers/clk/sunxi/clk-sun9i-core.c
index 6c4c98324d3c..37c85caaef88 100644
--- a/drivers/clk/sunxi/clk-sun9i-core.c
+++ b/drivers/clk/sunxi/clk-sun9i-core.c
@@ -71,7 +71,7 @@ static void sun9i_a80_get_pll4_factors(u32 *freq, u32 parent_rate,
 	*p_ret = p;
 }
 
-static struct clk_factors_config sun9i_a80_pll4_config = {
+static const struct clk_factors_config sun9i_a80_pll4_config = {
 	.mshift = 18,
 	.mwidth = 1,
 	.nshift = 8,
@@ -134,7 +134,7 @@ static void sun9i_a80_get_gt_factors(u32 *freq, u32 parent_rate,
 	*m = div;
 }
 
-static struct clk_factors_config sun9i_a80_gt_config = {
+static const struct clk_factors_config sun9i_a80_gt_config = {
 	.mshift = 0,
 	.mwidth = 2,
 };
@@ -199,7 +199,7 @@ static void sun9i_a80_get_ahb_factors(u32 *freq, u32 parent_rate,
 	*p = _p;
 }
 
-static struct clk_factors_config sun9i_a80_ahb_config = {
+static const struct clk_factors_config sun9i_a80_ahb_config = {
 	.pshift = 0,
 	.pwidth = 2,
 };
@@ -289,7 +289,7 @@ static void sun9i_a80_get_apb1_factors(u32 *freq, u32 parent_rate,
 	*p = calcp;
 }
 
-static struct clk_factors_config sun9i_a80_apb1_config = {
+static const struct clk_factors_config sun9i_a80_apb1_config = {
 	.mshift = 0,
 	.mwidth = 5,
 	.pshift = 16,
diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 0d4525365552..7e719d089017 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -608,7 +608,7 @@ static void sun7i_a20_get_out_factors(u32 *freq, u32 parent_rate,
  * sunxi_factors_clk_setup() - Setup function for factor clocks
  */
 
-static struct clk_factors_config sun4i_pll1_config = {
+static const struct clk_factors_config sun4i_pll1_config = {
 	.nshift = 8,
 	.nwidth = 5,
 	.kshift = 4,
@@ -619,7 +619,7 @@ static struct clk_factors_config sun4i_pll1_config = {
 	.pwidth = 2,
 };
 
-static struct clk_factors_config sun6i_a31_pll1_config = {
+static const struct clk_factors_config sun6i_a31_pll1_config = {
 	.nshift	= 8,
 	.nwidth = 5,
 	.kshift = 4,
@@ -629,7 +629,7 @@ static struct clk_factors_config sun6i_a31_pll1_config = {
 	.n_start = 1,
 };
 
-static struct clk_factors_config sun8i_a23_pll1_config = {
+static const struct clk_factors_config sun8i_a23_pll1_config = {
 	.nshift = 8,
 	.nwidth = 5,
 	.kshift = 4,
@@ -641,14 +641,14 @@ static struct clk_factors_config sun8i_a23_pll1_config = {
 	.n_start = 1,
 };
 
-static struct clk_factors_config sun4i_pll5_config = {
+static const struct clk_factors_config sun4i_pll5_config = {
 	.nshift = 8,
 	.nwidth = 5,
 	.kshift = 4,
 	.kwidth = 2,
 };
 
-static struct clk_factors_config sun6i_a31_pll6_config = {
+static const struct clk_factors_config sun6i_a31_pll6_config = {
 	.nshift	= 8,
 	.nwidth = 5,
 	.kshift = 4,
@@ -656,12 +656,12 @@ static struct clk_factors_config sun6i_a31_pll6_config = {
 	.n_start = 1,
 };
 
-static struct clk_factors_config sun5i_a13_ahb_config = {
+static const struct clk_factors_config sun5i_a13_ahb_config = {
 	.pshift = 4,
 	.pwidth = 2,
 };
 
-static struct clk_factors_config sun4i_apb1_config = {
+static const struct clk_factors_config sun4i_apb1_config = {
 	.mshift = 0,
 	.mwidth = 5,
 	.pshift = 16,
@@ -669,7 +669,7 @@ static struct clk_factors_config sun4i_apb1_config = {
 };
 
 /* user manual says "n" but it's really "p" */
-static struct clk_factors_config sun7i_a20_out_config = {
+static const struct clk_factors_config sun7i_a20_out_config = {
 	.mshift = 8,
 	.mwidth = 5,
 	.pshift = 20,
-- 
2.7.0

[PATCH RFC 02/11] clk: sunxi: factors: Make struct clk_factors_config table const

[Chen-Yu Tsai]

struct clk_factors_config contains shifts/widths for the factors of
the factors clk. This is used to read out the factors from the register
value. In no case is it written to, so make it const.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/clk/sunxi/clk-factors.c    |  4 ++--
 drivers/clk/sunxi/clk-factors.h    |  4 ++--
 drivers/clk/sunxi/clk-mod0.c       |  2 +-
 drivers/clk/sunxi/clk-sun9i-core.c |  8 ++++----
 drivers/clk/sunxi/clk-sunxi.c      | 16 ++++++++--------
 5 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
index 59428dbd607a..928c079da193 100644
--- a/drivers/clk/sunxi/clk-factors.c
+++ b/drivers/clk/sunxi/clk-factors.c
@@ -48,7 +48,7 @@ static unsigned long clk_factors_recalc_rate(struct clk_hw *hw,
 	u32 reg;
 	unsigned long rate;
 	struct clk_factors *factors = to_clk_factors(hw);
-	struct clk_factors_config *config = factors->config;
+	const struct clk_factors_config *config = factors->config;
 
 	/* Fetch the register value */
 	reg = readl(factors->reg);
@@ -123,7 +123,7 @@ static int clk_factors_set_rate(struct clk_hw *hw, unsigned long rate,
 	u8 n = 0, k = 0, m = 0, p = 0;
 	u32 reg;
 	struct clk_factors *factors = to_clk_factors(hw);
-	struct clk_factors_config *config = factors->config;
+	const struct clk_factors_config *config = factors->config;
 	unsigned long flags = 0;
 
 	factors->get_factors((u32 *)&rate, (u32)parent_rate, &n, &k, &m, &p);
diff --git a/drivers/clk/sunxi/clk-factors.h b/drivers/clk/sunxi/clk-factors.h
index 171085ab5513..060319be2b99 100644
--- a/drivers/clk/sunxi/clk-factors.h
+++ b/drivers/clk/sunxi/clk-factors.h
@@ -23,7 +23,7 @@ struct factors_data {
 	int enable;
 	int mux;
 	int muxmask;
-	struct clk_factors_config *table;
+	const struct clk_factors_config *table;
 	void (*getter) (u32 *rate, u32 parent_rate, u8 *n, u8 *k, u8 *m, u8 *p);
 	const char *name;
 };
@@ -31,7 +31,7 @@ struct factors_data {
 struct clk_factors {
 	struct clk_hw hw;
 	void __iomem *reg;
-	struct clk_factors_config *config;
+	const struct clk_factors_config *config;
 	void (*get_factors) (u32 *rate, u32 parent, u8 *n, u8 *k, u8 *m, u8 *p);
 	spinlock_t *lock;
 };
diff --git a/drivers/clk/sunxi/clk-mod0.c b/drivers/clk/sunxi/clk-mod0.c
index d167e1efb927..c67fea1a128d 100644
--- a/drivers/clk/sunxi/clk-mod0.c
+++ b/drivers/clk/sunxi/clk-mod0.c
@@ -62,7 +62,7 @@ static void sun4i_a10_get_mod0_factors(u32 *freq, u32 parent_rate,
 }
 
 /* user manual says "n" but it's really "p" */
-static struct clk_factors_config sun4i_a10_mod0_config = {
+static const struct clk_factors_config sun4i_a10_mod0_config = {
 	.mshift = 0,
 	.mwidth = 4,
 	.pshift = 16,
diff --git a/drivers/clk/sunxi/clk-sun9i-core.c b/drivers/clk/sunxi/clk-sun9i-core.c
index 6c4c98324d3c..37c85caaef88 100644
--- a/drivers/clk/sunxi/clk-sun9i-core.c
+++ b/drivers/clk/sunxi/clk-sun9i-core.c
@@ -71,7 +71,7 @@ static void sun9i_a80_get_pll4_factors(u32 *freq, u32 parent_rate,
 	*p_ret = p;
 }
 
-static struct clk_factors_config sun9i_a80_pll4_config = {
+static const struct clk_factors_config sun9i_a80_pll4_config = {
 	.mshift = 18,
 	.mwidth = 1,
 	.nshift = 8,
@@ -134,7 +134,7 @@ static void sun9i_a80_get_gt_factors(u32 *freq, u32 parent_rate,
 	*m = div;
 }
 
-static struct clk_factors_config sun9i_a80_gt_config = {
+static const struct clk_factors_config sun9i_a80_gt_config = {
 	.mshift = 0,
 	.mwidth = 2,
 };
@@ -199,7 +199,7 @@ static void sun9i_a80_get_ahb_factors(u32 *freq, u32 parent_rate,
 	*p = _p;
 }
 
-static struct clk_factors_config sun9i_a80_ahb_config = {
+static const struct clk_factors_config sun9i_a80_ahb_config = {
 	.pshift = 0,
 	.pwidth = 2,
 };
@@ -289,7 +289,7 @@ static void sun9i_a80_get_apb1_factors(u32 *freq, u32 parent_rate,
 	*p = calcp;
 }
 
-static struct clk_factors_config sun9i_a80_apb1_config = {
+static const struct clk_factors_config sun9i_a80_apb1_config = {
 	.mshift = 0,
 	.mwidth = 5,
 	.pshift = 16,
diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 0d4525365552..7e719d089017 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -608,7 +608,7 @@ static void sun7i_a20_get_out_factors(u32 *freq, u32 parent_rate,
  * sunxi_factors_clk_setup() - Setup function for factor clocks
  */
 
-static struct clk_factors_config sun4i_pll1_config = {
+static const struct clk_factors_config sun4i_pll1_config = {
 	.nshift = 8,
 	.nwidth = 5,
 	.kshift = 4,
@@ -619,7 +619,7 @@ static struct clk_factors_config sun4i_pll1_config = {
 	.pwidth = 2,
 };
 
-static struct clk_factors_config sun6i_a31_pll1_config = {
+static const struct clk_factors_config sun6i_a31_pll1_config = {
 	.nshift	= 8,
 	.nwidth = 5,
 	.kshift = 4,
@@ -629,7 +629,7 @@ static struct clk_factors_config sun6i_a31_pll1_config = {
 	.n_start = 1,
 };
 
-static struct clk_factors_config sun8i_a23_pll1_config = {
+static const struct clk_factors_config sun8i_a23_pll1_config = {
 	.nshift = 8,
 	.nwidth = 5,
 	.kshift = 4,
@@ -641,14 +641,14 @@ static struct clk_factors_config sun8i_a23_pll1_config = {
 	.n_start = 1,
 };
 
-static struct clk_factors_config sun4i_pll5_config = {
+static const struct clk_factors_config sun4i_pll5_config = {
 	.nshift = 8,
 	.nwidth = 5,
 	.kshift = 4,
 	.kwidth = 2,
 };
 
-static struct clk_factors_config sun6i_a31_pll6_config = {
+static const struct clk_factors_config sun6i_a31_pll6_config = {
 	.nshift	= 8,
 	.nwidth = 5,
 	.kshift = 4,
@@ -656,12 +656,12 @@ static struct clk_factors_config sun6i_a31_pll6_config = {
 	.n_start = 1,
 };
 
-static struct clk_factors_config sun5i_a13_ahb_config = {
+static const struct clk_factors_config sun5i_a13_ahb_config = {
 	.pshift = 4,
 	.pwidth = 2,
 };
 
-static struct clk_factors_config sun4i_apb1_config = {
+static const struct clk_factors_config sun4i_apb1_config = {
 	.mshift = 0,
 	.mwidth = 5,
 	.pshift = 16,
@@ -669,7 +669,7 @@ static struct clk_factors_config sun4i_apb1_config = {
 };
 
 /* user manual says "n" but it's really "p" */
-static struct clk_factors_config sun7i_a20_out_config = {
+static const struct clk_factors_config sun7i_a20_out_config = {
 	.mshift = 8,
 	.mwidth = 5,
 	.pshift = 20,
-- 
2.7.0

Re: [PATCH RFC 02/11] clk: sunxi: factors: Make struct clk_factors_config table const

[Maxime Ripard]

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

On Mon, Jan 25, 2016 at 09:15:38PM +0800, Chen-Yu Tsai wrote:
> struct clk_factors_config contains shifts/widths for the factors of
> the factors clk. This is used to read out the factors from the register
> value. In no case is it written to, so make it const.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Applied, thanks!

Maxime

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH RFC 02/11] clk: sunxi: factors: Make struct clk_factors_config table const

[Maxime Ripard]

On Mon, Jan 25, 2016 at 09:15:38PM +0800, Chen-Yu Tsai wrote:
> struct clk_factors_config contains shifts/widths for the factors of
> the factors clk. This is used to read out the factors from the register
> value. In no case is it written to, so make it const.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Applied, 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/20160127/971e5d64/attachment.sig>

[PATCH RFC 03/11] clk: sunxi: factors: Add clk cleanup in sunxi_factors_register() error path

[Chen-Yu Tsai]

sunxi_factors_register() does not check for failures or cleanup after
clk_register_composite() or other clk-related calls.

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

diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
index 928c079da193..1de57c22a358 100644
--- a/drivers/clk/sunxi/clk-factors.c
+++ b/drivers/clk/sunxi/clk-factors.c
@@ -172,7 +172,7 @@ struct clk *sunxi_factors_register(struct device_node *node,
 	struct clk_hw *mux_hw = NULL;
 	const char *clk_name = node->name;
 	const char *parents[FACTORS_MAX_PARENTS];
-	int i = 0;
+	int ret, i = 0;
 
 	/* if we have a mux, we will have >1 parents */
 	i = of_clk_parent_fill(node, parents, FACTORS_MAX_PARENTS);
@@ -188,7 +188,7 @@ struct clk *sunxi_factors_register(struct device_node *node,
 
 	factors = kzalloc(sizeof(struct clk_factors), GFP_KERNEL);
 	if (!factors)
-		return NULL;
+		goto err_factors;
 
 	/* set up factors properties */
 	factors->reg = reg;
@@ -199,10 +199,8 @@ struct clk *sunxi_factors_register(struct device_node *node,
 	/* Add a gate if this factor clock can be gated */
 	if (data->enable) {
 		gate = kzalloc(sizeof(struct clk_gate), GFP_KERNEL);
-		if (!gate) {
-			kfree(factors);
-			return NULL;
-		}
+		if (!gate)
+			goto err_gate;
 
 		/* set up gate properties */
 		gate->reg = reg;
@@ -214,11 +212,8 @@ struct clk *sunxi_factors_register(struct device_node *node,
 	/* Add a mux if this factor clock can be muxed */
 	if (data->mux) {
 		mux = kzalloc(sizeof(struct clk_mux), GFP_KERNEL);
-		if (!mux) {
-			kfree(factors);
-			kfree(gate);
-			return NULL;
-		}
+		if (!mux)
+			goto err_mux;
 
 		/* set up gate properties */
 		mux->reg = reg;
@@ -233,11 +228,29 @@ struct clk *sunxi_factors_register(struct device_node *node,
 			mux_hw, &clk_mux_ops,
 			&factors->hw, &clk_factors_ops,
 			gate_hw, &clk_gate_ops, 0);
+	if (IS_ERR(clk))
+		goto err_register;
 
-	if (!IS_ERR(clk)) {
-		of_clk_add_provider(node, of_clk_src_simple_get, clk);
-		clk_register_clkdev(clk, clk_name, NULL);
-	}
+	ret = of_clk_add_provider(node, of_clk_src_simple_get, clk);
+	if (ret)
+		goto err_provider;
+
+	ret = clk_register_clkdev(clk, clk_name, NULL);
+	if (ret)
+		goto err_clkdev;
 
 	return clk;
+
+err_clkdev:
+	of_clk_del_provider(node);
+err_provider:
+	clk_unregister_composite(clk);
+err_register:
+	kfree(mux);
+err_mux:
+	kfree(gate);
+err_gate:
+	kfree(factors);
+err_factors:
+	return NULL;
 }
-- 
2.7.0

[PATCH RFC 03/11] clk: sunxi: factors: Add clk cleanup in sunxi_factors_register() error path

[Chen-Yu Tsai]

sunxi_factors_register() does not check for failures or cleanup after
clk_register_composite() or other clk-related calls.

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

diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
index 928c079da193..1de57c22a358 100644
--- a/drivers/clk/sunxi/clk-factors.c
+++ b/drivers/clk/sunxi/clk-factors.c
@@ -172,7 +172,7 @@ struct clk *sunxi_factors_register(struct device_node *node,
 	struct clk_hw *mux_hw = NULL;
 	const char *clk_name = node->name;
 	const char *parents[FACTORS_MAX_PARENTS];
-	int i = 0;
+	int ret, i = 0;
 
 	/* if we have a mux, we will have >1 parents */
 	i = of_clk_parent_fill(node, parents, FACTORS_MAX_PARENTS);
@@ -188,7 +188,7 @@ struct clk *sunxi_factors_register(struct device_node *node,
 
 	factors = kzalloc(sizeof(struct clk_factors), GFP_KERNEL);
 	if (!factors)
-		return NULL;
+		goto err_factors;
 
 	/* set up factors properties */
 	factors->reg = reg;
@@ -199,10 +199,8 @@ struct clk *sunxi_factors_register(struct device_node *node,
 	/* Add a gate if this factor clock can be gated */
 	if (data->enable) {
 		gate = kzalloc(sizeof(struct clk_gate), GFP_KERNEL);
-		if (!gate) {
-			kfree(factors);
-			return NULL;
-		}
+		if (!gate)
+			goto err_gate;
 
 		/* set up gate properties */
 		gate->reg = reg;
@@ -214,11 +212,8 @@ struct clk *sunxi_factors_register(struct device_node *node,
 	/* Add a mux if this factor clock can be muxed */
 	if (data->mux) {
 		mux = kzalloc(sizeof(struct clk_mux), GFP_KERNEL);
-		if (!mux) {
-			kfree(factors);
-			kfree(gate);
-			return NULL;
-		}
+		if (!mux)
+			goto err_mux;
 
 		/* set up gate properties */
 		mux->reg = reg;
@@ -233,11 +228,29 @@ struct clk *sunxi_factors_register(struct device_node *node,
 			mux_hw, &clk_mux_ops,
 			&factors->hw, &clk_factors_ops,
 			gate_hw, &clk_gate_ops, 0);
+	if (IS_ERR(clk))
+		goto err_register;
 
-	if (!IS_ERR(clk)) {
-		of_clk_add_provider(node, of_clk_src_simple_get, clk);
-		clk_register_clkdev(clk, clk_name, NULL);
-	}
+	ret = of_clk_add_provider(node, of_clk_src_simple_get, clk);
+	if (ret)
+		goto err_provider;
+
+	ret = clk_register_clkdev(clk, clk_name, NULL);
+	if (ret)
+		goto err_clkdev;
 
 	return clk;
+
+err_clkdev:
+	of_clk_del_provider(node);
+err_provider:
+	clk_unregister_composite(clk);
+err_register:
+	kfree(mux);
+err_mux:
+	kfree(gate);
+err_gate:
+	kfree(factors);
+err_factors:
+	return NULL;
 }
-- 
2.7.0

Re: [PATCH RFC 03/11] clk: sunxi: factors: Add clk cleanup in sunxi_factors_register() error path

[Maxime Ripard]

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

On Mon, Jan 25, 2016 at 09:15:39PM +0800, Chen-Yu Tsai wrote:
> sunxi_factors_register() does not check for failures or cleanup after
> clk_register_composite() or other clk-related calls.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Applied, thanks!
Maxime

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH RFC 03/11] clk: sunxi: factors: Add clk cleanup in sunxi_factors_register() error path

[Maxime Ripard]

On Mon, Jan 25, 2016 at 09:15:39PM +0800, Chen-Yu Tsai wrote:
> sunxi_factors_register() does not check for failures or cleanup after
> clk_register_composite() or other clk-related calls.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Applied, 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/20160127/af3f5c89/attachment.sig>

[PATCH RFC 04/11] clk: sunxi: factors: Add unregister function

[Chen-Yu Tsai]

sunxi's factors clk did not have an unregister function. This means
multiple structs were leaked whenever a factors clk was unregistered.

Add an unregister function for it. Also keep pointers to the mux and
gate structs so they can be freed.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/clk/sunxi/clk-factors.c | 24 ++++++++++++++++++++++++
 drivers/clk/sunxi/clk-factors.h |  5 +++++
 2 files changed, 29 insertions(+)

diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
index 1de57c22a358..b464ca4a232e 100644
--- a/drivers/clk/sunxi/clk-factors.c
+++ b/drivers/clk/sunxi/clk-factors.c
@@ -202,6 +202,8 @@ struct clk *sunxi_factors_register(struct device_node *node,
 		if (!gate)
 			goto err_gate;
 
+		factors->gate = gate;
+
 		/* set up gate properties */
 		gate->reg = reg;
 		gate->bit_idx = data->enable;
@@ -215,6 +217,8 @@ struct clk *sunxi_factors_register(struct device_node *node,
 		if (!mux)
 			goto err_mux;
 
+		factors->mux = mux;
+
 		/* set up gate properties */
 		mux->reg = reg;
 		mux->shift = data->mux;
@@ -254,3 +258,23 @@ err_gate:
 err_factors:
 	return NULL;
 }
+
+void sunxi_factors_unregister(struct device_node *node, struct clk *clk)
+{
+	struct clk_hw *hw = __clk_get_hw(clk);
+	struct clk_factors *factors;
+	const char *name;
+
+	if (!hw)
+		return;
+
+	factors = to_clk_factors(hw);
+	name = clk_hw_get_name(hw);
+
+	/* No unregister call for clkdev_* */
+	of_clk_del_provider(node);
+	clk_unregister_composite(clk);
+	kfree(factors->mux);
+	kfree(factors->gate);
+	kfree(factors);
+}
diff --git a/drivers/clk/sunxi/clk-factors.h b/drivers/clk/sunxi/clk-factors.h
index 060319be2b99..7ea1379a7cda 100644
--- a/drivers/clk/sunxi/clk-factors.h
+++ b/drivers/clk/sunxi/clk-factors.h
@@ -34,6 +34,9 @@ struct clk_factors {
 	const struct clk_factors_config *config;
 	void (*get_factors) (u32 *rate, u32 parent, u8 *n, u8 *k, u8 *m, u8 *p);
 	spinlock_t *lock;
+	/* for cleanup */
+	struct clk_mux *mux;
+	struct clk_gate *gate;
 };
 
 struct clk *sunxi_factors_register(struct device_node *node,
@@ -41,4 +44,6 @@ struct clk *sunxi_factors_register(struct device_node *node,
 				   spinlock_t *lock,
 				   void __iomem *reg);
 
+void sunxi_factors_unregister(struct device_node *node, struct clk *clk);
+
 #endif
-- 
2.7.0

[PATCH RFC 04/11] clk: sunxi: factors: Add unregister function

[Chen-Yu Tsai]

sunxi's factors clk did not have an unregister function. This means
multiple structs were leaked whenever a factors clk was unregistered.

Add an unregister function for it. Also keep pointers to the mux and
gate structs so they can be freed.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/clk/sunxi/clk-factors.c | 24 ++++++++++++++++++++++++
 drivers/clk/sunxi/clk-factors.h |  5 +++++
 2 files changed, 29 insertions(+)

diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
index 1de57c22a358..b464ca4a232e 100644
--- a/drivers/clk/sunxi/clk-factors.c
+++ b/drivers/clk/sunxi/clk-factors.c
@@ -202,6 +202,8 @@ struct clk *sunxi_factors_register(struct device_node *node,
 		if (!gate)
 			goto err_gate;
 
+		factors->gate = gate;
+
 		/* set up gate properties */
 		gate->reg = reg;
 		gate->bit_idx = data->enable;
@@ -215,6 +217,8 @@ struct clk *sunxi_factors_register(struct device_node *node,
 		if (!mux)
 			goto err_mux;
 
+		factors->mux = mux;
+
 		/* set up gate properties */
 		mux->reg = reg;
 		mux->shift = data->mux;
@@ -254,3 +258,23 @@ err_gate:
 err_factors:
 	return NULL;
 }
+
+void sunxi_factors_unregister(struct device_node *node, struct clk *clk)
+{
+	struct clk_hw *hw = __clk_get_hw(clk);
+	struct clk_factors *factors;
+	const char *name;
+
+	if (!hw)
+		return;
+
+	factors = to_clk_factors(hw);
+	name = clk_hw_get_name(hw);
+
+	/* No unregister call for clkdev_* */
+	of_clk_del_provider(node);
+	clk_unregister_composite(clk);
+	kfree(factors->mux);
+	kfree(factors->gate);
+	kfree(factors);
+}
diff --git a/drivers/clk/sunxi/clk-factors.h b/drivers/clk/sunxi/clk-factors.h
index 060319be2b99..7ea1379a7cda 100644
--- a/drivers/clk/sunxi/clk-factors.h
+++ b/drivers/clk/sunxi/clk-factors.h
@@ -34,6 +34,9 @@ struct clk_factors {
 	const struct clk_factors_config *config;
 	void (*get_factors) (u32 *rate, u32 parent, u8 *n, u8 *k, u8 *m, u8 *p);
 	spinlock_t *lock;
+	/* for cleanup */
+	struct clk_mux *mux;
+	struct clk_gate *gate;
 };
 
 struct clk *sunxi_factors_register(struct device_node *node,
@@ -41,4 +44,6 @@ struct clk *sunxi_factors_register(struct device_node *node,
 				   spinlock_t *lock,
 				   void __iomem *reg);
 
+void sunxi_factors_unregister(struct device_node *node, struct clk *clk);
+
 #endif
-- 
2.7.0

Re: [PATCH RFC 04/11] clk: sunxi: factors: Add unregister function

[Maxime Ripard]

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

On Mon, Jan 25, 2016 at 09:15:40PM +0800, Chen-Yu Tsai wrote:
> sunxi's factors clk did not have an unregister function. This means
> multiple structs were leaked whenever a factors clk was unregistered.
> 
> Add an unregister function for it. Also keep pointers to the mux and
> gate structs so they can be freed.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Applied, thanks!
Maxime

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH RFC 04/11] clk: sunxi: factors: Add unregister function

[Maxime Ripard]

On Mon, Jan 25, 2016 at 09:15:40PM +0800, Chen-Yu Tsai wrote:
> sunxi's factors clk did not have an unregister function. This means
> multiple structs were leaked whenever a factors clk was unregistered.
> 
> Add an unregister function for it. Also keep pointers to the mux and
> gate structs so they can be freed.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Applied, 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/20160127/8e5a0488/attachment-0001.sig>

Re: [PATCH RFC 04/11] clk: sunxi: factors: Add unregister function

[Maxime Ripard]

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

On Wed, Jan 27, 2016 at 04:52:10PM +0100, Maxime Ripard wrote:
> On Mon, Jan 25, 2016 at 09:15:40PM +0800, Chen-Yu Tsai wrote:
> > sunxi's factors clk did not have an unregister function. This means
> > multiple structs were leaked whenever a factors clk was unregistered.
> > 
> > Add an unregister function for it. Also keep pointers to the mux and
> > gate structs so they can be freed.
> > 
> > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> 
> Applied, thanks!
> Maxime

So, yeah, that obviously didn't work, since clk_unregister_composite
wasn't in.

I want to give that as much linux-next coverage as possible, so I
still pulled 3, 5 and 11, replacing the call to
clk_unregister_composite by clk_unregister, adding a todo note on top.

Maxime

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH RFC 04/11] clk: sunxi: factors: Add unregister function

[Maxime Ripard]

On Wed, Jan 27, 2016 at 04:52:10PM +0100, Maxime Ripard wrote:
> On Mon, Jan 25, 2016 at 09:15:40PM +0800, Chen-Yu Tsai wrote:
> > sunxi's factors clk did not have an unregister function. This means
> > multiple structs were leaked whenever a factors clk was unregistered.
> > 
> > Add an unregister function for it. Also keep pointers to the mux and
> > gate structs so they can be freed.
> > 
> > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> 
> Applied, thanks!
> Maxime

So, yeah, that obviously didn't work, since clk_unregister_composite
wasn't in.

I want to give that as much linux-next coverage as possible, so I
still pulled 3, 5 and 11, replacing the call to
clk_unregister_composite by clk_unregister, adding a todo note on top.

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/20160127/119c44cc/attachment.sig>

[PATCH RFC 05/11] clk: sunxi: unmap registers in sunxi_factors_clk_setup if register call fails

[Chen-Yu Tsai]

sunxi_factors_clk_setup() does not unmap registers when
sunxi_factors_register() fails.

This patch adds proper error handling, and also an error message
when sunxi_factors_register() fails. Also use the full DT node name
in error messages, as the node name is often just "clk", which is
useless.

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

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 7e719d089017..5dd927859bc2 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -747,15 +747,24 @@ static struct clk * __init sunxi_factors_clk_setup(struct device_node *node,
 						   const struct factors_data *data)
 {
 	void __iomem *reg;
+	struct clk *clk;
 
 	reg = of_iomap(node, 0);
 	if (!reg) {
 		pr_err("Could not get registers for factors-clk: %s\n",
-		       node->name);
+		       of_node_full_name(node));
 		return NULL;
 	}
 
-	return sunxi_factors_register(node, data, &clk_lock, reg);
+	clk = sunxi_factors_register(node, data, &clk_lock, reg);
+
+	if (!clk) {
+		pr_err("Could not register factors-clk: %s\n",
+		       of_node_full_name(node));
+		iounmap(reg);
+	}
+
+	return clk;
 }
 
 
-- 
2.7.0

[PATCH RFC 05/11] clk: sunxi: unmap registers in sunxi_factors_clk_setup if register call fails

[Chen-Yu Tsai]

sunxi_factors_clk_setup() does not unmap registers when
sunxi_factors_register() fails.

This patch adds proper error handling, and also an error message
when sunxi_factors_register() fails. Also use the full DT node name
in error messages, as the node name is often just "clk", which is
useless.

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

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 7e719d089017..5dd927859bc2 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -747,15 +747,24 @@ static struct clk * __init sunxi_factors_clk_setup(struct device_node *node,
 						   const struct factors_data *data)
 {
 	void __iomem *reg;
+	struct clk *clk;
 
 	reg = of_iomap(node, 0);
 	if (!reg) {
 		pr_err("Could not get registers for factors-clk: %s\n",
-		       node->name);
+		       of_node_full_name(node));
 		return NULL;
 	}
 
-	return sunxi_factors_register(node, data, &clk_lock, reg);
+	clk = sunxi_factors_register(node, data, &clk_lock, reg);
+
+	if (!clk) {
+		pr_err("Could not register factors-clk: %s\n",
+		       of_node_full_name(node));
+		iounmap(reg);
+	}
+
+	return clk;
 }
 
 
-- 
2.7.0

Re: [PATCH RFC 05/11] clk: sunxi: unmap registers in sunxi_factors_clk_setup if register call fails

[Maxime Ripard]

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

On Mon, Jan 25, 2016 at 09:15:41PM +0800, Chen-Yu Tsai wrote:
> sunxi_factors_clk_setup() does not unmap registers when
> sunxi_factors_register() fails.
> 
> This patch adds proper error handling, and also an error message
> when sunxi_factors_register() fails. Also use the full DT node name
> in error messages, as the node name is often just "clk", which is
> useless.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

APplied, thanks!

Maxime

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH RFC 05/11] clk: sunxi: unmap registers in sunxi_factors_clk_setup if register call fails

[Maxime Ripard]

On Mon, Jan 25, 2016 at 09:15:41PM +0800, Chen-Yu Tsai wrote:
> sunxi_factors_clk_setup() does not unmap registers when
> sunxi_factors_register() fails.
> 
> This patch adds proper error handling, and also an error message
> when sunxi_factors_register() fails. Also use the full DT node name
> in error messages, as the node name is often just "clk", which is
> useless.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

APplied, 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/20160127/857bbdf2/attachment.sig>

[PATCH RFC 06/11] clk: sunxi: factors: Consolidate get_factors parameters into a struct

[Chen-Yu Tsai]

The .get_factors callback of factors_clk has 6 parameters. To extend
factors_clk in any way that requires adding parameters to .get_factors
would make that list even longer, not to mention changing all the
function declarations.

Do this once now and consolidate all the parameters into a struct.
Also drop the space before function pointer arguments, since checkpatch
complains.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/clk/sunxi/clk-factors.c    |  24 ++--
 drivers/clk/sunxi/clk-factors.h    |  13 ++-
 drivers/clk/sunxi/clk-mod0.c       |  20 ++--
 drivers/clk/sunxi/clk-sun8i-mbus.c |  18 +--
 drivers/clk/sunxi/clk-sun9i-core.c |  77 +++++--------
 drivers/clk/sunxi/clk-sunxi.c      | 222 +++++++++++++++----------------------
 6 files changed, 155 insertions(+), 219 deletions(-)

diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
index b464ca4a232e..47b7d918b74c 100644
--- a/drivers/clk/sunxi/clk-factors.c
+++ b/drivers/clk/sunxi/clk-factors.c
@@ -73,8 +73,13 @@ static long clk_factors_round_rate(struct clk_hw *hw, unsigned long rate,
 				   unsigned long *parent_rate)
 {
 	struct clk_factors *factors = to_clk_factors(hw);
-	factors->get_factors((u32 *)&rate, (u32)*parent_rate,
-			     NULL, NULL, NULL, NULL);
+	struct factors_request req = {
+		.rate = rate,
+		.parent_rate = *parent_rate,
+	};
+
+	factors->get_factors(&req);
+
 
 	return rate;
 }
@@ -120,13 +125,16 @@ static int clk_factors_determine_rate(struct clk_hw *hw,
 static int clk_factors_set_rate(struct clk_hw *hw, unsigned long rate,
 				unsigned long parent_rate)
 {
-	u8 n = 0, k = 0, m = 0, p = 0;
+	struct factors_request req = {
+		.rate = rate,
+		.parent_rate = parent_rate,
+	};
 	u32 reg;
 	struct clk_factors *factors = to_clk_factors(hw);
 	const struct clk_factors_config *config = factors->config;
 	unsigned long flags = 0;
 
-	factors->get_factors((u32 *)&rate, (u32)parent_rate, &n, &k, &m, &p);
+	factors->get_factors(&req);
 
 	if (factors->lock)
 		spin_lock_irqsave(factors->lock, flags);
@@ -135,10 +143,10 @@ static int clk_factors_set_rate(struct clk_hw *hw, unsigned long rate,
 	reg = readl(factors->reg);
 
 	/* Set up the new factors - macros do not do anything if width is 0 */
-	reg = FACTOR_SET(config->nshift, config->nwidth, reg, n);
-	reg = FACTOR_SET(config->kshift, config->kwidth, reg, k);
-	reg = FACTOR_SET(config->mshift, config->mwidth, reg, m);
-	reg = FACTOR_SET(config->pshift, config->pwidth, reg, p);
+	reg = FACTOR_SET(config->nshift, config->nwidth, reg, req.n);
+	reg = FACTOR_SET(config->kshift, config->kwidth, reg, req.k);
+	reg = FACTOR_SET(config->mshift, config->mwidth, reg, req.m);
+	reg = FACTOR_SET(config->pshift, config->pwidth, reg, req.p);
 
 	/* Apply them now */
 	writel(reg, factors->reg);
diff --git a/drivers/clk/sunxi/clk-factors.h b/drivers/clk/sunxi/clk-factors.h
index 7ea1379a7cda..f09d7c214533 100644
--- a/drivers/clk/sunxi/clk-factors.h
+++ b/drivers/clk/sunxi/clk-factors.h
@@ -19,12 +19,21 @@ struct clk_factors_config {
 	u8 n_start;
 };
 
+struct factors_request {
+	unsigned long rate;
+	unsigned long parent_rate;
+	u8 n;
+	u8 k;
+	u8 m;
+	u8 p;
+};
+
 struct factors_data {
 	int enable;
 	int mux;
 	int muxmask;
 	const struct clk_factors_config *table;
-	void (*getter) (u32 *rate, u32 parent_rate, u8 *n, u8 *k, u8 *m, u8 *p);
+	void (*getter)(struct factors_request *req);
 	const char *name;
 };
 
@@ -32,7 +41,7 @@ struct clk_factors {
 	struct clk_hw hw;
 	void __iomem *reg;
 	const struct clk_factors_config *config;
-	void (*get_factors) (u32 *rate, u32 parent, u8 *n, u8 *k, u8 *m, u8 *p);
+	void (*get_factors)(struct factors_request *req);
 	spinlock_t *lock;
 	/* for cleanup */
 	struct clk_mux *mux;
diff --git a/drivers/clk/sunxi/clk-mod0.c b/drivers/clk/sunxi/clk-mod0.c
index c67fea1a128d..578bff0dd251 100644
--- a/drivers/clk/sunxi/clk-mod0.c
+++ b/drivers/clk/sunxi/clk-mod0.c
@@ -28,17 +28,16 @@
  * rate = (parent_rate >> p) / (m + 1);
  */
 
-static void sun4i_a10_get_mod0_factors(u32 *freq, u32 parent_rate,
-				       u8 *n, u8 *k, u8 *m, u8 *p)
+static void sun4i_a10_get_mod0_factors(struct factors_request *req)
 {
 	u8 div, calcm, calcp;
 
 	/* These clocks can only divide, so we will never be able to achieve
 	 * frequencies higher than the parent frequency */
-	if (*freq > parent_rate)
-		*freq = parent_rate;
+	if (req->rate > req->parent_rate)
+		req->rate = req->parent_rate;
 
-	div = DIV_ROUND_UP(parent_rate, *freq);
+	div = DIV_ROUND_UP(req->parent_rate, req->rate);
 
 	if (div < 16)
 		calcp = 0;
@@ -51,14 +50,9 @@ static void sun4i_a10_get_mod0_factors(u32 *freq, u32 parent_rate,
 
 	calcm = DIV_ROUND_UP(div, 1 << calcp);
 
-	*freq = (parent_rate >> calcp) / calcm;
-
-	/* we were called to round the frequency, we can now return */
-	if (n == NULL)
-		return;
-
-	*m = calcm - 1;
-	*p = calcp;
+	req->rate = (req->parent_rate >> calcp) / calcm;
+	req->m = calcm - 1;
+	req->p = calcp;
 }
 
 /* user manual says "n" but it's really "p" */
diff --git a/drivers/clk/sunxi/clk-sun8i-mbus.c b/drivers/clk/sunxi/clk-sun8i-mbus.c
index bf117a636d23..78683f02a37d 100644
--- a/drivers/clk/sunxi/clk-sun8i-mbus.c
+++ b/drivers/clk/sunxi/clk-sun8i-mbus.c
@@ -26,8 +26,7 @@
  * rate = parent_rate / (m + 1);
  */
 
-static void sun8i_a23_get_mbus_factors(u32 *freq, u32 parent_rate,
-				       u8 *n, u8 *k, u8 *m, u8 *p)
+static void sun8i_a23_get_mbus_factors(struct factors_request *req)
 {
 	u8 div;
 
@@ -35,21 +34,16 @@ static void sun8i_a23_get_mbus_factors(u32 *freq, u32 parent_rate,
 	 * These clocks can only divide, so we will never be able to
 	 * achieve frequencies higher than the parent frequency
 	 */
-	if (*freq > parent_rate)
-		*freq = parent_rate;
+	if (req->rate > req->parent_rate)
+		req->rate = req->parent_rate;
 
-	div = DIV_ROUND_UP(parent_rate, *freq);
+	div = DIV_ROUND_UP(req->parent_rate, req->rate);
 
 	if (div > 8)
 		div = 8;
 
-	*freq = parent_rate / div;
-
-	/* we were called to round the frequency, we can now return */
-	if (m == NULL)
-		return;
-
-	*m = div - 1;
+	req->rate = req->parent_rate / div;
+	req->m = div - 1;
 }
 
 static struct clk_factors_config sun8i_a23_mbus_config = {
diff --git a/drivers/clk/sunxi/clk-sun9i-core.c b/drivers/clk/sunxi/clk-sun9i-core.c
index 37c85caaef88..a81211062ecb 100644
--- a/drivers/clk/sunxi/clk-sun9i-core.c
+++ b/drivers/clk/sunxi/clk-sun9i-core.c
@@ -32,15 +32,14 @@
  * p and m are named div1 and div2 in Allwinner's SDK
  */
 
-static void sun9i_a80_get_pll4_factors(u32 *freq, u32 parent_rate,
-				       u8 *n_ret, u8 *k, u8 *m_ret, u8 *p_ret)
+static void sun9i_a80_get_pll4_factors(struct factors_request *req)
 {
 	int n;
 	int m = 1;
 	int p = 1;
 
 	/* Normalize value to a 6 MHz multiple (24 MHz / 4) */
-	n = DIV_ROUND_UP(*freq, 6000000);
+	n = DIV_ROUND_UP(req->rate, 6000000);
 
 	/* If n is too large switch to steps of 12 MHz */
 	if (n > 255) {
@@ -60,15 +59,10 @@ static void sun9i_a80_get_pll4_factors(u32 *freq, u32 parent_rate,
 	else if (n < 12)
 		n = 12;
 
-	*freq = ((24000000 * n) >> p) / (m + 1);
-
-	/* we were called to round the frequency, we can now return */
-	if (n_ret == NULL)
-		return;
-
-	*n_ret = n;
-	*m_ret = m;
-	*p_ret = p;
+	req->rate = ((24000000 * n) >> p) / (m + 1);
+	req->n = n;
+	req->m = m;
+	req->p = p;
 }
 
 static const struct clk_factors_config sun9i_a80_pll4_config = {
@@ -111,27 +105,21 @@ CLK_OF_DECLARE(sun9i_a80_pll4, "allwinner,sun9i-a80-pll4-clk", sun9i_a80_pll4_se
  * rate = parent_rate / (m + 1);
  */
 
-static void sun9i_a80_get_gt_factors(u32 *freq, u32 parent_rate,
-				     u8 *n, u8 *k, u8 *m, u8 *p)
+static void sun9i_a80_get_gt_factors(struct factors_request *req)
 {
 	u32 div;
 
-	if (parent_rate < *freq)
-		*freq = parent_rate;
+	if (req->parent_rate < req->rate)
+		req->rate = req->parent_rate;
 
-	div = DIV_ROUND_UP(parent_rate, *freq);
+	div = DIV_ROUND_UP(req->parent_rate, req->rate);
 
 	/* maximum divider is 4 */
 	if (div > 4)
 		div = 4;
 
-	*freq = parent_rate / div;
-
-	/* we were called to round the frequency, we can now return */
-	if (!m)
-		return;
-
-	*m = div;
+	req->rate = req->parent_rate / div;
+	req->m = div;
 }
 
 static const struct clk_factors_config sun9i_a80_gt_config = {
@@ -176,27 +164,21 @@ CLK_OF_DECLARE(sun9i_a80_gt, "allwinner,sun9i-a80-gt-clk", sun9i_a80_gt_setup);
  * rate = parent_rate >> p;
  */
 
-static void sun9i_a80_get_ahb_factors(u32 *freq, u32 parent_rate,
-				      u8 *n, u8 *k, u8 *m, u8 *p)
+static void sun9i_a80_get_ahb_factors(struct factors_request *req)
 {
 	u32 _p;
 
-	if (parent_rate < *freq)
-		*freq = parent_rate;
+	if (req->parent_rate < req->rate)
+		req->rate = req->parent_rate;
 
-	_p = order_base_2(DIV_ROUND_UP(parent_rate, *freq));
+	_p = order_base_2(DIV_ROUND_UP(req->parent_rate, req->rate));
 
 	/* maximum p is 3 */
 	if (_p > 3)
 		_p = 3;
 
-	*freq = parent_rate >> _p;
-
-	/* we were called to round the frequency, we can now return */
-	if (!p)
-		return;
-
-	*p = _p;
+	req->rate = req->parent_rate >> _p;
+	req->p = _p;
 }
 
 static const struct clk_factors_config sun9i_a80_ahb_config = {
@@ -262,31 +244,22 @@ CLK_OF_DECLARE(sun9i_a80_apb0, "allwinner,sun9i-a80-apb0-clk", sun9i_a80_apb0_se
  * rate = (parent_rate >> p) / (m + 1);
  */
 
-static void sun9i_a80_get_apb1_factors(u32 *freq, u32 parent_rate,
-				       u8 *n, u8 *k, u8 *m, u8 *p)
+static void sun9i_a80_get_apb1_factors(struct factors_request *req)
 {
 	u32 div;
-	u8 calcm, calcp;
 
-	if (parent_rate < *freq)
-		*freq = parent_rate;
+	if (req->parent_rate < req->rate)
+		req->rate = req->parent_rate;
 
-	div = DIV_ROUND_UP(parent_rate, *freq);
+	div = DIV_ROUND_UP(req->parent_rate, req->rate);
 
 	/* Highest possible divider is 256 (p = 3, m = 31) */
 	if (div > 256)
 		div = 256;
 
-	calcp = order_base_2(div);
-	calcm = (parent_rate >> calcp) - 1;
-	*freq = (parent_rate >> calcp) / (calcm + 1);
-
-	/* we were called to round the frequency, we can now return */
-	if (n == NULL)
-		return;
-
-	*m = calcm;
-	*p = calcp;
+	req->p = order_base_2(div);
+	req->m = (req->parent_rate >> req->p) - 1;
+	req->rate = (req->parent_rate >> req->p) / (req->m + 1);
 }
 
 static const struct clk_factors_config sun9i_a80_apb1_config = {
diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 5dd927859bc2..56d4fdcf9c89 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -246,49 +246,45 @@ CLK_OF_DECLARE(sun6i_a31_ahb1, "allwinner,sun6i-a31-ahb1-clk", sun6i_ahb1_clk_se
  * parent_rate is always 24Mhz
  */
 
-static void sun4i_get_pll1_factors(u32 *freq, u32 parent_rate,
-				   u8 *n, u8 *k, u8 *m, u8 *p)
+static void sun4i_get_pll1_factors(struct factors_request *req)
 {
 	u8 div;
 
 	/* Normalize value to a 6M multiple */
-	div = *freq / 6000000;
-	*freq = 6000000 * div;
-
-	/* we were called to round the frequency, we can now return */
-	if (n == NULL)
-		return;
+	div = req->rate / 6000000;
+	req->rate = 6000000 * div;
 
 	/* m is always zero for pll1 */
-	*m = 0;
+	req->m = 0;
 
 	/* k is 1 only on these cases */
-	if (*freq >= 768000000 || *freq == 42000000 || *freq == 54000000)
-		*k = 1;
+	if (req->rate >= 768000000 || req->rate == 42000000 ||
+			req->rate == 54000000)
+		req->k = 1;
 	else
-		*k = 0;
+		req->k = 0;
 
 	/* p will be 3 for divs under 10 */
 	if (div < 10)
-		*p = 3;
+		req->p = 3;
 
 	/* p will be 2 for divs between 10 - 20 and odd divs under 32 */
 	else if (div < 20 || (div < 32 && (div & 1)))
-		*p = 2;
+		req->p = 2;
 
 	/* p will be 1 for even divs under 32, divs under 40 and odd pairs
 	 * of divs between 40-62 */
 	else if (div < 40 || (div < 64 && (div & 2)))
-		*p = 1;
+		req->p = 1;
 
 	/* any other entries have p = 0 */
 	else
-		*p = 0;
+		req->p = 0;
 
 	/* calculate a suitable n based on k and p */
-	div <<= *p;
-	div /= (*k + 1);
-	*n = div / 4;
+	div <<= req->p;
+	div /= (req->k + 1);
+	req->n = div / 4;
 }
 
 /**
@@ -297,15 +293,14 @@ static void sun4i_get_pll1_factors(u32 *freq, u32 parent_rate,
  * rate = parent_rate * (n + 1) * (k + 1) / (m + 1);
  * parent_rate should always be 24MHz
  */
-static void sun6i_a31_get_pll1_factors(u32 *freq, u32 parent_rate,
-				       u8 *n, u8 *k, u8 *m, u8 *p)
+static void sun6i_a31_get_pll1_factors(struct factors_request *req)
 {
 	/*
 	 * We can operate only on MHz, this will make our life easier
 	 * later.
 	 */
-	u32 freq_mhz = *freq / 1000000;
-	u32 parent_freq_mhz = parent_rate / 1000000;
+	u32 freq_mhz = req->rate / 1000000;
+	u32 parent_freq_mhz = req->parent_rate / 1000000;
 
 	/*
 	 * Round down the frequency to the closest multiple of either
@@ -319,28 +314,20 @@ static void sun6i_a31_get_pll1_factors(u32 *freq, u32 parent_rate,
 	else
 		freq_mhz = round_freq_16;
 
-	*freq = freq_mhz * 1000000;
-
-	/*
-	 * If the factors pointer are null, we were just called to
-	 * round down the frequency.
-	 * Exit.
-	 */
-	if (n == NULL)
-		return;
+	req->rate = freq_mhz * 1000000;
 
 	/* If the frequency is a multiple of 32 MHz, k is always 3 */
 	if (!(freq_mhz % 32))
-		*k = 3;
+		req->k = 3;
 	/* If the frequency is a multiple of 9 MHz, k is always 2 */
 	else if (!(freq_mhz % 9))
-		*k = 2;
+		req->k = 2;
 	/* If the frequency is a multiple of 8 MHz, k is always 1 */
 	else if (!(freq_mhz % 8))
-		*k = 1;
+		req->k = 1;
 	/* Otherwise, we don't use the k factor */
 	else
-		*k = 0;
+		req->k = 0;
 
 	/*
 	 * If the frequency is a multiple of 2 but not a multiple of
@@ -351,27 +338,28 @@ static void sun6i_a31_get_pll1_factors(u32 *freq, u32 parent_rate,
 	 * somehow relates to this frequency.
 	 */
 	if ((freq_mhz % 6) == 2 || (freq_mhz % 6) == 4)
-		*m = 2;
+		req->m = 2;
 	/*
 	 * If the frequency is a multiple of 6MHz, but the factor is
 	 * odd, m will be 3
 	 */
 	else if ((freq_mhz / 6) & 1)
-		*m = 3;
+		req->m = 3;
 	/* Otherwise, we end up with m = 1 */
 	else
-		*m = 1;
+		req->m = 1;
 
 	/* Calculate n thanks to the above factors we already got */
-	*n = freq_mhz * (*m + 1) / ((*k + 1) * parent_freq_mhz) - 1;
+	req->n = freq_mhz * (req->m + 1) / ((req->k + 1) * parent_freq_mhz)
+		 - 1;
 
 	/*
 	 * If n end up being outbound, and that we can still decrease
 	 * m, do it.
 	 */
-	if ((*n + 1) > 31 && (*m + 1) > 1) {
-		*n = (*n + 1) / 2 - 1;
-		*m = (*m + 1) / 2 - 1;
+	if ((req->n + 1) > 31 && (req->m + 1) > 1) {
+		req->n = (req->n + 1) / 2 - 1;
+		req->m = (req->m + 1) / 2 - 1;
 	}
 }
 
@@ -382,45 +370,41 @@ static void sun6i_a31_get_pll1_factors(u32 *freq, u32 parent_rate,
  * parent_rate is always 24Mhz
  */
 
-static void sun8i_a23_get_pll1_factors(u32 *freq, u32 parent_rate,
-				   u8 *n, u8 *k, u8 *m, u8 *p)
+static void sun8i_a23_get_pll1_factors(struct factors_request *req)
 {
 	u8 div;
 
 	/* Normalize value to a 6M multiple */
-	div = *freq / 6000000;
-	*freq = 6000000 * div;
-
-	/* we were called to round the frequency, we can now return */
-	if (n == NULL)
-		return;
+	div = req->rate / 6000000;
+	req->rate = 6000000 * div;
 
 	/* m is always zero for pll1 */
-	*m = 0;
+	req->m = 0;
 
 	/* k is 1 only on these cases */
-	if (*freq >= 768000000 || *freq == 42000000 || *freq == 54000000)
-		*k = 1;
+	if (req->rate >= 768000000 || req->rate == 42000000 ||
+			req->rate == 54000000)
+		req->k = 1;
 	else
-		*k = 0;
+		req->k = 0;
 
 	/* p will be 2 for divs under 20 and odd divs under 32 */
 	if (div < 20 || (div < 32 && (div & 1)))
-		*p = 2;
+		req->p = 2;
 
 	/* p will be 1 for even divs under 32, divs under 40 and odd pairs
 	 * of divs between 40-62 */
 	else if (div < 40 || (div < 64 && (div & 2)))
-		*p = 1;
+		req->p = 1;
 
 	/* any other entries have p = 0 */
 	else
-		*p = 0;
+		req->p = 0;
 
 	/* calculate a suitable n based on k and p */
-	div <<= *p;
-	div /= (*k + 1);
-	*n = div / 4 - 1;
+	div <<= req->p;
+	div /= (req->k + 1);
+	req->n = div / 4 - 1;
 }
 
 /**
@@ -430,29 +414,24 @@ static void sun8i_a23_get_pll1_factors(u32 *freq, u32 parent_rate,
  * parent_rate is always 24Mhz
  */
 
-static void sun4i_get_pll5_factors(u32 *freq, u32 parent_rate,
-				   u8 *n, u8 *k, u8 *m, u8 *p)
+static void sun4i_get_pll5_factors(struct factors_request *req)
 {
 	u8 div;
 
 	/* Normalize value to a parent_rate multiple (24M) */
-	div = *freq / parent_rate;
-	*freq = parent_rate * div;
-
-	/* we were called to round the frequency, we can now return */
-	if (n == NULL)
-		return;
+	div = req->rate / req->parent_rate;
+	req->rate = req->parent_rate * div;
 
 	if (div < 31)
-		*k = 0;
+		req->k = 0;
 	else if (div / 2 < 31)
-		*k = 1;
+		req->k = 1;
 	else if (div / 3 < 31)
-		*k = 2;
+		req->k = 2;
 	else
-		*k = 3;
+		req->k = 3;
 
-	*n = DIV_ROUND_UP(div, (*k+1));
+	req->n = DIV_ROUND_UP(div, (req->k + 1));
 }
 
 /**
@@ -462,24 +441,19 @@ static void sun4i_get_pll5_factors(u32 *freq, u32 parent_rate,
  * parent_rate is always 24Mhz
  */
 
-static void sun6i_a31_get_pll6_factors(u32 *freq, u32 parent_rate,
-				       u8 *n, u8 *k, u8 *m, u8 *p)
+static void sun6i_a31_get_pll6_factors(struct factors_request *req)
 {
 	u8 div;
 
 	/* Normalize value to a parent_rate multiple (24M) */
-	div = *freq / parent_rate;
-	*freq = parent_rate * div;
+	div = req->rate / req->parent_rate;
+	req->rate = req->parent_rate * div;
 
-	/* we were called to round the frequency, we can now return */
-	if (n == NULL)
-		return;
-
-	*k = div / 32;
-	if (*k > 3)
-		*k = 3;
+	req->k = div / 32;
+	if (req->k > 3)
+		req->k = 3;
 
-	*n = DIV_ROUND_UP(div, (*k+1)) - 1;
+	req->n = DIV_ROUND_UP(div, (req->k + 1)) - 1;
 }
 
 /**
@@ -488,37 +462,32 @@ static void sun6i_a31_get_pll6_factors(u32 *freq, u32 parent_rate,
  * rate = parent_rate >> p
  */
 
-static void sun5i_a13_get_ahb_factors(u32 *freq, u32 parent_rate,
-				       u8 *n, u8 *k, u8 *m, u8 *p)
+static void sun5i_a13_get_ahb_factors(struct factors_request *req)
 {
 	u32 div;
 
 	/* divide only */
-	if (parent_rate < *freq)
-		*freq = parent_rate;
+	if (req->parent_rate < req->rate)
+		req->rate = req->parent_rate;
 
 	/*
 	 * user manual says valid speed is 8k ~ 276M, but tests show it
 	 * can work at speeds up to 300M, just after reparenting to pll6
 	 */
-	if (*freq < 8000)
-		*freq = 8000;
-	if (*freq > 300000000)
-		*freq = 300000000;
+	if (req->rate < 8000)
+		req->rate = 8000;
+	if (req->rate > 300000000)
+		req->rate = 300000000;
 
-	div = order_base_2(DIV_ROUND_UP(parent_rate, *freq));
+	div = order_base_2(DIV_ROUND_UP(req->parent_rate, req->rate));
 
 	/* p = 0 ~ 3 */
 	if (div > 3)
 		div = 3;
 
-	*freq = parent_rate >> div;
+	req->rate = req->parent_rate >> div;
 
-	/* we were called to round the frequency, we can now return */
-	if (p == NULL)
-		return;
-
-	*p = div;
+	req->p = div;
 }
 
 /**
@@ -527,39 +496,34 @@ static void sun5i_a13_get_ahb_factors(u32 *freq, u32 parent_rate,
  * rate = (parent_rate >> p) / (m + 1);
  */
 
-static void sun4i_get_apb1_factors(u32 *freq, u32 parent_rate,
-				   u8 *n, u8 *k, u8 *m, u8 *p)
+static void sun4i_get_apb1_factors(struct factors_request *req)
 {
 	u8 calcm, calcp;
+	int div;
 
-	if (parent_rate < *freq)
-		*freq = parent_rate;
+	if (req->parent_rate < req->rate)
+		req->rate = req->parent_rate;
 
-	parent_rate = DIV_ROUND_UP(parent_rate, *freq);
+	div = DIV_ROUND_UP(req->parent_rate, req->rate);
 
 	/* Invalid rate! */
-	if (parent_rate > 32)
+	if (div > 32)
 		return;
 
-	if (parent_rate <= 4)
+	if (div <= 4)
 		calcp = 0;
-	else if (parent_rate <= 8)
+	else if (div <= 8)
 		calcp = 1;
-	else if (parent_rate <= 16)
+	else if (div <= 16)
 		calcp = 2;
 	else
 		calcp = 3;
 
-	calcm = (parent_rate >> calcp) - 1;
-
-	*freq = (parent_rate >> calcp) / (calcm + 1);
+	calcm = (req->parent_rate >> calcp) - 1;
 
-	/* we were called to round the frequency, we can now return */
-	if (n == NULL)
-		return;
-
-	*m = calcm;
-	*p = calcp;
+	req->rate = (req->parent_rate >> calcp) / (calcm + 1);
+	req->m = calcm;
+	req->p = calcp;
 }
 
 
@@ -571,17 +535,16 @@ static void sun4i_get_apb1_factors(u32 *freq, u32 parent_rate,
  * rate = (parent_rate >> p) / (m + 1);
  */
 
-static void sun7i_a20_get_out_factors(u32 *freq, u32 parent_rate,
-				      u8 *n, u8 *k, u8 *m, u8 *p)
+static void sun7i_a20_get_out_factors(struct factors_request *req)
 {
 	u8 div, calcm, calcp;
 
 	/* These clocks can only divide, so we will never be able to achieve
 	 * frequencies higher than the parent frequency */
-	if (*freq > parent_rate)
-		*freq = parent_rate;
+	if (req->rate > req->parent_rate)
+		req->rate = req->parent_rate;
 
-	div = DIV_ROUND_UP(parent_rate, *freq);
+	div = DIV_ROUND_UP(req->parent_rate, req->rate);
 
 	if (div < 32)
 		calcp = 0;
@@ -594,14 +557,9 @@ static void sun7i_a20_get_out_factors(u32 *freq, u32 parent_rate,
 
 	calcm = DIV_ROUND_UP(div, 1 << calcp);
 
-	*freq = (parent_rate >> calcp) / calcm;
-
-	/* we were called to round the frequency, we can now return */
-	if (n == NULL)
-		return;
-
-	*m = calcm - 1;
-	*p = calcp;
+	req->rate = (req->parent_rate >> calcp) / calcm;
+	req->m = calcm - 1;
+	req->p = calcp;
 }
 
 /**
-- 
2.7.0

[PATCH RFC 06/11] clk: sunxi: factors: Consolidate get_factors parameters into a struct

[Chen-Yu Tsai]

The .get_factors callback of factors_clk has 6 parameters. To extend
factors_clk in any way that requires adding parameters to .get_factors
would make that list even longer, not to mention changing all the
function declarations.

Do this once now and consolidate all the parameters into a struct.
Also drop the space before function pointer arguments, since checkpatch
complains.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/clk/sunxi/clk-factors.c    |  24 ++--
 drivers/clk/sunxi/clk-factors.h    |  13 ++-
 drivers/clk/sunxi/clk-mod0.c       |  20 ++--
 drivers/clk/sunxi/clk-sun8i-mbus.c |  18 +--
 drivers/clk/sunxi/clk-sun9i-core.c |  77 +++++--------
 drivers/clk/sunxi/clk-sunxi.c      | 222 +++++++++++++++----------------------
 6 files changed, 155 insertions(+), 219 deletions(-)

diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
index b464ca4a232e..47b7d918b74c 100644
--- a/drivers/clk/sunxi/clk-factors.c
+++ b/drivers/clk/sunxi/clk-factors.c
@@ -73,8 +73,13 @@ static long clk_factors_round_rate(struct clk_hw *hw, unsigned long rate,
 				   unsigned long *parent_rate)
 {
 	struct clk_factors *factors = to_clk_factors(hw);
-	factors->get_factors((u32 *)&rate, (u32)*parent_rate,
-			     NULL, NULL, NULL, NULL);
+	struct factors_request req = {
+		.rate = rate,
+		.parent_rate = *parent_rate,
+	};
+
+	factors->get_factors(&req);
+
 
 	return rate;
 }
@@ -120,13 +125,16 @@ static int clk_factors_determine_rate(struct clk_hw *hw,
 static int clk_factors_set_rate(struct clk_hw *hw, unsigned long rate,
 				unsigned long parent_rate)
 {
-	u8 n = 0, k = 0, m = 0, p = 0;
+	struct factors_request req = {
+		.rate = rate,
+		.parent_rate = parent_rate,
+	};
 	u32 reg;
 	struct clk_factors *factors = to_clk_factors(hw);
 	const struct clk_factors_config *config = factors->config;
 	unsigned long flags = 0;
 
-	factors->get_factors((u32 *)&rate, (u32)parent_rate, &n, &k, &m, &p);
+	factors->get_factors(&req);
 
 	if (factors->lock)
 		spin_lock_irqsave(factors->lock, flags);
@@ -135,10 +143,10 @@ static int clk_factors_set_rate(struct clk_hw *hw, unsigned long rate,
 	reg = readl(factors->reg);
 
 	/* Set up the new factors - macros do not do anything if width is 0 */
-	reg = FACTOR_SET(config->nshift, config->nwidth, reg, n);
-	reg = FACTOR_SET(config->kshift, config->kwidth, reg, k);
-	reg = FACTOR_SET(config->mshift, config->mwidth, reg, m);
-	reg = FACTOR_SET(config->pshift, config->pwidth, reg, p);
+	reg = FACTOR_SET(config->nshift, config->nwidth, reg, req.n);
+	reg = FACTOR_SET(config->kshift, config->kwidth, reg, req.k);
+	reg = FACTOR_SET(config->mshift, config->mwidth, reg, req.m);
+	reg = FACTOR_SET(config->pshift, config->pwidth, reg, req.p);
 
 	/* Apply them now */
 	writel(reg, factors->reg);
diff --git a/drivers/clk/sunxi/clk-factors.h b/drivers/clk/sunxi/clk-factors.h
index 7ea1379a7cda..f09d7c214533 100644
--- a/drivers/clk/sunxi/clk-factors.h
+++ b/drivers/clk/sunxi/clk-factors.h
@@ -19,12 +19,21 @@ struct clk_factors_config {
 	u8 n_start;
 };
 
+struct factors_request {
+	unsigned long rate;
+	unsigned long parent_rate;
+	u8 n;
+	u8 k;
+	u8 m;
+	u8 p;
+};
+
 struct factors_data {
 	int enable;
 	int mux;
 	int muxmask;
 	const struct clk_factors_config *table;
-	void (*getter) (u32 *rate, u32 parent_rate, u8 *n, u8 *k, u8 *m, u8 *p);
+	void (*getter)(struct factors_request *req);
 	const char *name;
 };
 
@@ -32,7 +41,7 @@ struct clk_factors {
 	struct clk_hw hw;
 	void __iomem *reg;
 	const struct clk_factors_config *config;
-	void (*get_factors) (u32 *rate, u32 parent, u8 *n, u8 *k, u8 *m, u8 *p);
+	void (*get_factors)(struct factors_request *req);
 	spinlock_t *lock;
 	/* for cleanup */
 	struct clk_mux *mux;
diff --git a/drivers/clk/sunxi/clk-mod0.c b/drivers/clk/sunxi/clk-mod0.c
index c67fea1a128d..578bff0dd251 100644
--- a/drivers/clk/sunxi/clk-mod0.c
+++ b/drivers/clk/sunxi/clk-mod0.c
@@ -28,17 +28,16 @@
  * rate = (parent_rate >> p) / (m + 1);
  */
 
-static void sun4i_a10_get_mod0_factors(u32 *freq, u32 parent_rate,
-				       u8 *n, u8 *k, u8 *m, u8 *p)
+static void sun4i_a10_get_mod0_factors(struct factors_request *req)
 {
 	u8 div, calcm, calcp;
 
 	/* These clocks can only divide, so we will never be able to achieve
 	 * frequencies higher than the parent frequency */
-	if (*freq > parent_rate)
-		*freq = parent_rate;
+	if (req->rate > req->parent_rate)
+		req->rate = req->parent_rate;
 
-	div = DIV_ROUND_UP(parent_rate, *freq);
+	div = DIV_ROUND_UP(req->parent_rate, req->rate);
 
 	if (div < 16)
 		calcp = 0;
@@ -51,14 +50,9 @@ static void sun4i_a10_get_mod0_factors(u32 *freq, u32 parent_rate,
 
 	calcm = DIV_ROUND_UP(div, 1 << calcp);
 
-	*freq = (parent_rate >> calcp) / calcm;
-
-	/* we were called to round the frequency, we can now return */
-	if (n == NULL)
-		return;
-
-	*m = calcm - 1;
-	*p = calcp;
+	req->rate = (req->parent_rate >> calcp) / calcm;
+	req->m = calcm - 1;
+	req->p = calcp;
 }
 
 /* user manual says "n" but it's really "p" */
diff --git a/drivers/clk/sunxi/clk-sun8i-mbus.c b/drivers/clk/sunxi/clk-sun8i-mbus.c
index bf117a636d23..78683f02a37d 100644
--- a/drivers/clk/sunxi/clk-sun8i-mbus.c
+++ b/drivers/clk/sunxi/clk-sun8i-mbus.c
@@ -26,8 +26,7 @@
  * rate = parent_rate / (m + 1);
  */
 
-static void sun8i_a23_get_mbus_factors(u32 *freq, u32 parent_rate,
-				       u8 *n, u8 *k, u8 *m, u8 *p)
+static void sun8i_a23_get_mbus_factors(struct factors_request *req)
 {
 	u8 div;
 
@@ -35,21 +34,16 @@ static void sun8i_a23_get_mbus_factors(u32 *freq, u32 parent_rate,
 	 * These clocks can only divide, so we will never be able to
 	 * achieve frequencies higher than the parent frequency
 	 */
-	if (*freq > parent_rate)
-		*freq = parent_rate;
+	if (req->rate > req->parent_rate)
+		req->rate = req->parent_rate;
 
-	div = DIV_ROUND_UP(parent_rate, *freq);
+	div = DIV_ROUND_UP(req->parent_rate, req->rate);
 
 	if (div > 8)
 		div = 8;
 
-	*freq = parent_rate / div;
-
-	/* we were called to round the frequency, we can now return */
-	if (m == NULL)
-		return;
-
-	*m = div - 1;
+	req->rate = req->parent_rate / div;
+	req->m = div - 1;
 }
 
 static struct clk_factors_config sun8i_a23_mbus_config = {
diff --git a/drivers/clk/sunxi/clk-sun9i-core.c b/drivers/clk/sunxi/clk-sun9i-core.c
index 37c85caaef88..a81211062ecb 100644
--- a/drivers/clk/sunxi/clk-sun9i-core.c
+++ b/drivers/clk/sunxi/clk-sun9i-core.c
@@ -32,15 +32,14 @@
  * p and m are named div1 and div2 in Allwinner's SDK
  */
 
-static void sun9i_a80_get_pll4_factors(u32 *freq, u32 parent_rate,
-				       u8 *n_ret, u8 *k, u8 *m_ret, u8 *p_ret)
+static void sun9i_a80_get_pll4_factors(struct factors_request *req)
 {
 	int n;
 	int m = 1;
 	int p = 1;
 
 	/* Normalize value to a 6 MHz multiple (24 MHz / 4) */
-	n = DIV_ROUND_UP(*freq, 6000000);
+	n = DIV_ROUND_UP(req->rate, 6000000);
 
 	/* If n is too large switch to steps of 12 MHz */
 	if (n > 255) {
@@ -60,15 +59,10 @@ static void sun9i_a80_get_pll4_factors(u32 *freq, u32 parent_rate,
 	else if (n < 12)
 		n = 12;
 
-	*freq = ((24000000 * n) >> p) / (m + 1);
-
-	/* we were called to round the frequency, we can now return */
-	if (n_ret == NULL)
-		return;
-
-	*n_ret = n;
-	*m_ret = m;
-	*p_ret = p;
+	req->rate = ((24000000 * n) >> p) / (m + 1);
+	req->n = n;
+	req->m = m;
+	req->p = p;
 }
 
 static const struct clk_factors_config sun9i_a80_pll4_config = {
@@ -111,27 +105,21 @@ CLK_OF_DECLARE(sun9i_a80_pll4, "allwinner,sun9i-a80-pll4-clk", sun9i_a80_pll4_se
  * rate = parent_rate / (m + 1);
  */
 
-static void sun9i_a80_get_gt_factors(u32 *freq, u32 parent_rate,
-				     u8 *n, u8 *k, u8 *m, u8 *p)
+static void sun9i_a80_get_gt_factors(struct factors_request *req)
 {
 	u32 div;
 
-	if (parent_rate < *freq)
-		*freq = parent_rate;
+	if (req->parent_rate < req->rate)
+		req->rate = req->parent_rate;
 
-	div = DIV_ROUND_UP(parent_rate, *freq);
+	div = DIV_ROUND_UP(req->parent_rate, req->rate);
 
 	/* maximum divider is 4 */
 	if (div > 4)
 		div = 4;
 
-	*freq = parent_rate / div;
-
-	/* we were called to round the frequency, we can now return */
-	if (!m)
-		return;
-
-	*m = div;
+	req->rate = req->parent_rate / div;
+	req->m = div;
 }
 
 static const struct clk_factors_config sun9i_a80_gt_config = {
@@ -176,27 +164,21 @@ CLK_OF_DECLARE(sun9i_a80_gt, "allwinner,sun9i-a80-gt-clk", sun9i_a80_gt_setup);
  * rate = parent_rate >> p;
  */
 
-static void sun9i_a80_get_ahb_factors(u32 *freq, u32 parent_rate,
-				      u8 *n, u8 *k, u8 *m, u8 *p)
+static void sun9i_a80_get_ahb_factors(struct factors_request *req)
 {
 	u32 _p;
 
-	if (parent_rate < *freq)
-		*freq = parent_rate;
+	if (req->parent_rate < req->rate)
+		req->rate = req->parent_rate;
 
-	_p = order_base_2(DIV_ROUND_UP(parent_rate, *freq));
+	_p = order_base_2(DIV_ROUND_UP(req->parent_rate, req->rate));
 
 	/* maximum p is 3 */
 	if (_p > 3)
 		_p = 3;
 
-	*freq = parent_rate >> _p;
-
-	/* we were called to round the frequency, we can now return */
-	if (!p)
-		return;
-
-	*p = _p;
+	req->rate = req->parent_rate >> _p;
+	req->p = _p;
 }
 
 static const struct clk_factors_config sun9i_a80_ahb_config = {
@@ -262,31 +244,22 @@ CLK_OF_DECLARE(sun9i_a80_apb0, "allwinner,sun9i-a80-apb0-clk", sun9i_a80_apb0_se
  * rate = (parent_rate >> p) / (m + 1);
  */
 
-static void sun9i_a80_get_apb1_factors(u32 *freq, u32 parent_rate,
-				       u8 *n, u8 *k, u8 *m, u8 *p)
+static void sun9i_a80_get_apb1_factors(struct factors_request *req)
 {
 	u32 div;
-	u8 calcm, calcp;
 
-	if (parent_rate < *freq)
-		*freq = parent_rate;
+	if (req->parent_rate < req->rate)
+		req->rate = req->parent_rate;
 
-	div = DIV_ROUND_UP(parent_rate, *freq);
+	div = DIV_ROUND_UP(req->parent_rate, req->rate);
 
 	/* Highest possible divider is 256 (p = 3, m = 31) */
 	if (div > 256)
 		div = 256;
 
-	calcp = order_base_2(div);
-	calcm = (parent_rate >> calcp) - 1;
-	*freq = (parent_rate >> calcp) / (calcm + 1);
-
-	/* we were called to round the frequency, we can now return */
-	if (n == NULL)
-		return;
-
-	*m = calcm;
-	*p = calcp;
+	req->p = order_base_2(div);
+	req->m = (req->parent_rate >> req->p) - 1;
+	req->rate = (req->parent_rate >> req->p) / (req->m + 1);
 }
 
 static const struct clk_factors_config sun9i_a80_apb1_config = {
diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 5dd927859bc2..56d4fdcf9c89 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -246,49 +246,45 @@ CLK_OF_DECLARE(sun6i_a31_ahb1, "allwinner,sun6i-a31-ahb1-clk", sun6i_ahb1_clk_se
  * parent_rate is always 24Mhz
  */
 
-static void sun4i_get_pll1_factors(u32 *freq, u32 parent_rate,
-				   u8 *n, u8 *k, u8 *m, u8 *p)
+static void sun4i_get_pll1_factors(struct factors_request *req)
 {
 	u8 div;
 
 	/* Normalize value to a 6M multiple */
-	div = *freq / 6000000;
-	*freq = 6000000 * div;
-
-	/* we were called to round the frequency, we can now return */
-	if (n == NULL)
-		return;
+	div = req->rate / 6000000;
+	req->rate = 6000000 * div;
 
 	/* m is always zero for pll1 */
-	*m = 0;
+	req->m = 0;
 
 	/* k is 1 only on these cases */
-	if (*freq >= 768000000 || *freq == 42000000 || *freq == 54000000)
-		*k = 1;
+	if (req->rate >= 768000000 || req->rate == 42000000 ||
+			req->rate == 54000000)
+		req->k = 1;
 	else
-		*k = 0;
+		req->k = 0;
 
 	/* p will be 3 for divs under 10 */
 	if (div < 10)
-		*p = 3;
+		req->p = 3;
 
 	/* p will be 2 for divs between 10 - 20 and odd divs under 32 */
 	else if (div < 20 || (div < 32 && (div & 1)))
-		*p = 2;
+		req->p = 2;
 
 	/* p will be 1 for even divs under 32, divs under 40 and odd pairs
 	 * of divs between 40-62 */
 	else if (div < 40 || (div < 64 && (div & 2)))
-		*p = 1;
+		req->p = 1;
 
 	/* any other entries have p = 0 */
 	else
-		*p = 0;
+		req->p = 0;
 
 	/* calculate a suitable n based on k and p */
-	div <<= *p;
-	div /= (*k + 1);
-	*n = div / 4;
+	div <<= req->p;
+	div /= (req->k + 1);
+	req->n = div / 4;
 }
 
 /**
@@ -297,15 +293,14 @@ static void sun4i_get_pll1_factors(u32 *freq, u32 parent_rate,
  * rate = parent_rate * (n + 1) * (k + 1) / (m + 1);
  * parent_rate should always be 24MHz
  */
-static void sun6i_a31_get_pll1_factors(u32 *freq, u32 parent_rate,
-				       u8 *n, u8 *k, u8 *m, u8 *p)
+static void sun6i_a31_get_pll1_factors(struct factors_request *req)
 {
 	/*
 	 * We can operate only on MHz, this will make our life easier
 	 * later.
 	 */
-	u32 freq_mhz = *freq / 1000000;
-	u32 parent_freq_mhz = parent_rate / 1000000;
+	u32 freq_mhz = req->rate / 1000000;
+	u32 parent_freq_mhz = req->parent_rate / 1000000;
 
 	/*
 	 * Round down the frequency to the closest multiple of either
@@ -319,28 +314,20 @@ static void sun6i_a31_get_pll1_factors(u32 *freq, u32 parent_rate,
 	else
 		freq_mhz = round_freq_16;
 
-	*freq = freq_mhz * 1000000;
-
-	/*
-	 * If the factors pointer are null, we were just called to
-	 * round down the frequency.
-	 * Exit.
-	 */
-	if (n == NULL)
-		return;
+	req->rate = freq_mhz * 1000000;
 
 	/* If the frequency is a multiple of 32 MHz, k is always 3 */
 	if (!(freq_mhz % 32))
-		*k = 3;
+		req->k = 3;
 	/* If the frequency is a multiple of 9 MHz, k is always 2 */
 	else if (!(freq_mhz % 9))
-		*k = 2;
+		req->k = 2;
 	/* If the frequency is a multiple of 8 MHz, k is always 1 */
 	else if (!(freq_mhz % 8))
-		*k = 1;
+		req->k = 1;
 	/* Otherwise, we don't use the k factor */
 	else
-		*k = 0;
+		req->k = 0;
 
 	/*
 	 * If the frequency is a multiple of 2 but not a multiple of
@@ -351,27 +338,28 @@ static void sun6i_a31_get_pll1_factors(u32 *freq, u32 parent_rate,
 	 * somehow relates to this frequency.
 	 */
 	if ((freq_mhz % 6) == 2 || (freq_mhz % 6) == 4)
-		*m = 2;
+		req->m = 2;
 	/*
 	 * If the frequency is a multiple of 6MHz, but the factor is
 	 * odd, m will be 3
 	 */
 	else if ((freq_mhz / 6) & 1)
-		*m = 3;
+		req->m = 3;
 	/* Otherwise, we end up with m = 1 */
 	else
-		*m = 1;
+		req->m = 1;
 
 	/* Calculate n thanks to the above factors we already got */
-	*n = freq_mhz * (*m + 1) / ((*k + 1) * parent_freq_mhz) - 1;
+	req->n = freq_mhz * (req->m + 1) / ((req->k + 1) * parent_freq_mhz)
+		 - 1;
 
 	/*
 	 * If n end up being outbound, and that we can still decrease
 	 * m, do it.
 	 */
-	if ((*n + 1) > 31 && (*m + 1) > 1) {
-		*n = (*n + 1) / 2 - 1;
-		*m = (*m + 1) / 2 - 1;
+	if ((req->n + 1) > 31 && (req->m + 1) > 1) {
+		req->n = (req->n + 1) / 2 - 1;
+		req->m = (req->m + 1) / 2 - 1;
 	}
 }
 
@@ -382,45 +370,41 @@ static void sun6i_a31_get_pll1_factors(u32 *freq, u32 parent_rate,
  * parent_rate is always 24Mhz
  */
 
-static void sun8i_a23_get_pll1_factors(u32 *freq, u32 parent_rate,
-				   u8 *n, u8 *k, u8 *m, u8 *p)
+static void sun8i_a23_get_pll1_factors(struct factors_request *req)
 {
 	u8 div;
 
 	/* Normalize value to a 6M multiple */
-	div = *freq / 6000000;
-	*freq = 6000000 * div;
-
-	/* we were called to round the frequency, we can now return */
-	if (n == NULL)
-		return;
+	div = req->rate / 6000000;
+	req->rate = 6000000 * div;
 
 	/* m is always zero for pll1 */
-	*m = 0;
+	req->m = 0;
 
 	/* k is 1 only on these cases */
-	if (*freq >= 768000000 || *freq == 42000000 || *freq == 54000000)
-		*k = 1;
+	if (req->rate >= 768000000 || req->rate == 42000000 ||
+			req->rate == 54000000)
+		req->k = 1;
 	else
-		*k = 0;
+		req->k = 0;
 
 	/* p will be 2 for divs under 20 and odd divs under 32 */
 	if (div < 20 || (div < 32 && (div & 1)))
-		*p = 2;
+		req->p = 2;
 
 	/* p will be 1 for even divs under 32, divs under 40 and odd pairs
 	 * of divs between 40-62 */
 	else if (div < 40 || (div < 64 && (div & 2)))
-		*p = 1;
+		req->p = 1;
 
 	/* any other entries have p = 0 */
 	else
-		*p = 0;
+		req->p = 0;
 
 	/* calculate a suitable n based on k and p */
-	div <<= *p;
-	div /= (*k + 1);
-	*n = div / 4 - 1;
+	div <<= req->p;
+	div /= (req->k + 1);
+	req->n = div / 4 - 1;
 }
 
 /**
@@ -430,29 +414,24 @@ static void sun8i_a23_get_pll1_factors(u32 *freq, u32 parent_rate,
  * parent_rate is always 24Mhz
  */
 
-static void sun4i_get_pll5_factors(u32 *freq, u32 parent_rate,
-				   u8 *n, u8 *k, u8 *m, u8 *p)
+static void sun4i_get_pll5_factors(struct factors_request *req)
 {
 	u8 div;
 
 	/* Normalize value to a parent_rate multiple (24M) */
-	div = *freq / parent_rate;
-	*freq = parent_rate * div;
-
-	/* we were called to round the frequency, we can now return */
-	if (n == NULL)
-		return;
+	div = req->rate / req->parent_rate;
+	req->rate = req->parent_rate * div;
 
 	if (div < 31)
-		*k = 0;
+		req->k = 0;
 	else if (div / 2 < 31)
-		*k = 1;
+		req->k = 1;
 	else if (div / 3 < 31)
-		*k = 2;
+		req->k = 2;
 	else
-		*k = 3;
+		req->k = 3;
 
-	*n = DIV_ROUND_UP(div, (*k+1));
+	req->n = DIV_ROUND_UP(div, (req->k + 1));
 }
 
 /**
@@ -462,24 +441,19 @@ static void sun4i_get_pll5_factors(u32 *freq, u32 parent_rate,
  * parent_rate is always 24Mhz
  */
 
-static void sun6i_a31_get_pll6_factors(u32 *freq, u32 parent_rate,
-				       u8 *n, u8 *k, u8 *m, u8 *p)
+static void sun6i_a31_get_pll6_factors(struct factors_request *req)
 {
 	u8 div;
 
 	/* Normalize value to a parent_rate multiple (24M) */
-	div = *freq / parent_rate;
-	*freq = parent_rate * div;
+	div = req->rate / req->parent_rate;
+	req->rate = req->parent_rate * div;
 
-	/* we were called to round the frequency, we can now return */
-	if (n == NULL)
-		return;
-
-	*k = div / 32;
-	if (*k > 3)
-		*k = 3;
+	req->k = div / 32;
+	if (req->k > 3)
+		req->k = 3;
 
-	*n = DIV_ROUND_UP(div, (*k+1)) - 1;
+	req->n = DIV_ROUND_UP(div, (req->k + 1)) - 1;
 }
 
 /**
@@ -488,37 +462,32 @@ static void sun6i_a31_get_pll6_factors(u32 *freq, u32 parent_rate,
  * rate = parent_rate >> p
  */
 
-static void sun5i_a13_get_ahb_factors(u32 *freq, u32 parent_rate,
-				       u8 *n, u8 *k, u8 *m, u8 *p)
+static void sun5i_a13_get_ahb_factors(struct factors_request *req)
 {
 	u32 div;
 
 	/* divide only */
-	if (parent_rate < *freq)
-		*freq = parent_rate;
+	if (req->parent_rate < req->rate)
+		req->rate = req->parent_rate;
 
 	/*
 	 * user manual says valid speed is 8k ~ 276M, but tests show it
 	 * can work at speeds up to 300M, just after reparenting to pll6
 	 */
-	if (*freq < 8000)
-		*freq = 8000;
-	if (*freq > 300000000)
-		*freq = 300000000;
+	if (req->rate < 8000)
+		req->rate = 8000;
+	if (req->rate > 300000000)
+		req->rate = 300000000;
 
-	div = order_base_2(DIV_ROUND_UP(parent_rate, *freq));
+	div = order_base_2(DIV_ROUND_UP(req->parent_rate, req->rate));
 
 	/* p = 0 ~ 3 */
 	if (div > 3)
 		div = 3;
 
-	*freq = parent_rate >> div;
+	req->rate = req->parent_rate >> div;
 
-	/* we were called to round the frequency, we can now return */
-	if (p == NULL)
-		return;
-
-	*p = div;
+	req->p = div;
 }
 
 /**
@@ -527,39 +496,34 @@ static void sun5i_a13_get_ahb_factors(u32 *freq, u32 parent_rate,
  * rate = (parent_rate >> p) / (m + 1);
  */
 
-static void sun4i_get_apb1_factors(u32 *freq, u32 parent_rate,
-				   u8 *n, u8 *k, u8 *m, u8 *p)
+static void sun4i_get_apb1_factors(struct factors_request *req)
 {
 	u8 calcm, calcp;
+	int div;
 
-	if (parent_rate < *freq)
-		*freq = parent_rate;
+	if (req->parent_rate < req->rate)
+		req->rate = req->parent_rate;
 
-	parent_rate = DIV_ROUND_UP(parent_rate, *freq);
+	div = DIV_ROUND_UP(req->parent_rate, req->rate);
 
 	/* Invalid rate! */
-	if (parent_rate > 32)
+	if (div > 32)
 		return;
 
-	if (parent_rate <= 4)
+	if (div <= 4)
 		calcp = 0;
-	else if (parent_rate <= 8)
+	else if (div <= 8)
 		calcp = 1;
-	else if (parent_rate <= 16)
+	else if (div <= 16)
 		calcp = 2;
 	else
 		calcp = 3;
 
-	calcm = (parent_rate >> calcp) - 1;
-
-	*freq = (parent_rate >> calcp) / (calcm + 1);
+	calcm = (req->parent_rate >> calcp) - 1;
 
-	/* we were called to round the frequency, we can now return */
-	if (n == NULL)
-		return;
-
-	*m = calcm;
-	*p = calcp;
+	req->rate = (req->parent_rate >> calcp) / (calcm + 1);
+	req->m = calcm;
+	req->p = calcp;
 }
 
 
@@ -571,17 +535,16 @@ static void sun4i_get_apb1_factors(u32 *freq, u32 parent_rate,
  * rate = (parent_rate >> p) / (m + 1);
  */
 
-static void sun7i_a20_get_out_factors(u32 *freq, u32 parent_rate,
-				      u8 *n, u8 *k, u8 *m, u8 *p)
+static void sun7i_a20_get_out_factors(struct factors_request *req)
 {
 	u8 div, calcm, calcp;
 
 	/* These clocks can only divide, so we will never be able to achieve
 	 * frequencies higher than the parent frequency */
-	if (*freq > parent_rate)
-		*freq = parent_rate;
+	if (req->rate > req->parent_rate)
+		req->rate = req->parent_rate;
 
-	div = DIV_ROUND_UP(parent_rate, *freq);
+	div = DIV_ROUND_UP(req->parent_rate, req->rate);
 
 	if (div < 32)
 		calcp = 0;
@@ -594,14 +557,9 @@ static void sun7i_a20_get_out_factors(u32 *freq, u32 parent_rate,
 
 	calcm = DIV_ROUND_UP(div, 1 << calcp);
 
-	*freq = (parent_rate >> calcp) / calcm;
-
-	/* we were called to round the frequency, we can now return */
-	if (n == NULL)
-		return;
-
-	*m = calcm - 1;
-	*p = calcp;
+	req->rate = (req->parent_rate >> calcp) / calcm;
+	req->m = calcm - 1;
+	req->p = calcp;
 }
 
 /**
-- 
2.7.0

Re: [PATCH RFC 06/11] clk: sunxi: factors: Consolidate get_factors parameters into a struct

[Maxime Ripard]

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

On Mon, Jan 25, 2016 at 09:15:42PM +0800, Chen-Yu Tsai wrote:
> The .get_factors callback of factors_clk has 6 parameters. To extend
> factors_clk in any way that requires adding parameters to .get_factors
> would make that list even longer, not to mention changing all the
> function declarations.
> 
> Do this once now and consolidate all the parameters into a struct.
> Also drop the space before function pointer arguments, since checkpatch
> complains.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Applied, thanks!
Maxime

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH RFC 06/11] clk: sunxi: factors: Consolidate get_factors parameters into a struct

[Maxime Ripard]

On Mon, Jan 25, 2016 at 09:15:42PM +0800, Chen-Yu Tsai wrote:
> The .get_factors callback of factors_clk has 6 parameters. To extend
> factors_clk in any way that requires adding parameters to .get_factors
> would make that list even longer, not to mention changing all the
> function declarations.
> 
> Do this once now and consolidate all the parameters into a struct.
> Also drop the space before function pointer arguments, since checkpatch
> complains.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Applied, 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/20160127/08905639/attachment.sig>

[PATCH RFC 07/11] clk: sunxi: factors: Support custom formulas

[Chen-Yu Tsai]

Some clocks cannot be modelled using the standard factors clk formula,
such as clocks with special pre-dividers on one parent, or clocks
with all power-of-two dividers.

Add support for a custom .recalc callback for factors clk. Also pass
the current parent index to the .get_factor and .recalc callbacks.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/clk/sunxi/clk-factors.c | 30 ++++++++++++++++++++++++++++--
 drivers/clk/sunxi/clk-factors.h |  3 +++
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
index 47b7d918b74c..ac259305d5d5 100644
--- a/drivers/clk/sunxi/clk-factors.c
+++ b/drivers/clk/sunxi/clk-factors.c
@@ -63,6 +63,26 @@ static unsigned long clk_factors_recalc_rate(struct clk_hw *hw,
 	if (config->pwidth != SUNXI_FACTORS_NOT_APPLICABLE)
 		p = FACTOR_GET(config->pshift, config->pwidth, reg);
 
+	if (factors->recalc) {
+		struct factors_request factors_req = {
+			.parent_rate = parent_rate,
+			.n = n,
+			.k = k,
+			.m = m,
+			.p = p,
+		};
+
+		/* get mux details from mux clk structure */
+		if (factors->mux)
+			factors_req.parent_index =
+				(reg >> factors->mux->shift) &
+				factors->mux->mask;
+
+		factors->recalc(&factors_req);
+
+		return factors_req.rate;
+	}
+
 	/* Calculate the rate */
 	rate = (parent_rate * (n + config->n_start) * (k + 1) >> p) / (m + 1);
 
@@ -87,6 +107,7 @@ static long clk_factors_round_rate(struct clk_hw *hw, unsigned long rate,
 static int clk_factors_determine_rate(struct clk_hw *hw,
 				      struct clk_rate_request *req)
 {
+	struct clk_factors *factors = to_clk_factors(hw);
 	struct clk_hw *parent, *best_parent = NULL;
 	int i, num_parents;
 	unsigned long parent_rate, best = 0, child_rate, best_child_rate = 0;
@@ -94,6 +115,10 @@ static int clk_factors_determine_rate(struct clk_hw *hw,
 	/* find the parent that can help provide the fastest rate <= rate */
 	num_parents = clk_hw_get_num_parents(hw);
 	for (i = 0; i < num_parents; i++) {
+		struct factors_request factors_req = {
+			.rate = req->rate,
+			.parent_index = i,
+		};
 		parent = clk_hw_get_parent_by_index(hw, i);
 		if (!parent)
 			continue;
@@ -102,8 +127,9 @@ static int clk_factors_determine_rate(struct clk_hw *hw,
 		else
 			parent_rate = clk_hw_get_rate(parent);
 
-		child_rate = clk_factors_round_rate(hw, req->rate,
-						    &parent_rate);
+		factors_req.parent_rate = parent_rate;
+		factors->get_factors(&factors_req);
+		child_rate = factors_req.rate;
 
 		if (child_rate <= req->rate && child_rate > best_child_rate) {
 			best_parent = parent;
diff --git a/drivers/clk/sunxi/clk-factors.h b/drivers/clk/sunxi/clk-factors.h
index f09d7c214533..a44a865a6b9e 100644
--- a/drivers/clk/sunxi/clk-factors.h
+++ b/drivers/clk/sunxi/clk-factors.h
@@ -22,6 +22,7 @@ struct clk_factors_config {
 struct factors_request {
 	unsigned long rate;
 	unsigned long parent_rate;
+	u8 parent_index;
 	u8 n;
 	u8 k;
 	u8 m;
@@ -34,6 +35,7 @@ struct factors_data {
 	int muxmask;
 	const struct clk_factors_config *table;
 	void (*getter)(struct factors_request *req);
+	void (*recalc)(struct factors_request *req);
 	const char *name;
 };
 
@@ -42,6 +44,7 @@ struct clk_factors {
 	void __iomem *reg;
 	const struct clk_factors_config *config;
 	void (*get_factors)(struct factors_request *req);
+	void (*recalc)(struct factors_request *req);
 	spinlock_t *lock;
 	/* for cleanup */
 	struct clk_mux *mux;
-- 
2.7.0

[PATCH RFC 07/11] clk: sunxi: factors: Support custom formulas

[Chen-Yu Tsai]

Some clocks cannot be modelled using the standard factors clk formula,
such as clocks with special pre-dividers on one parent, or clocks
with all power-of-two dividers.

Add support for a custom .recalc callback for factors clk. Also pass
the current parent index to the .get_factor and .recalc callbacks.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/clk/sunxi/clk-factors.c | 30 ++++++++++++++++++++++++++++--
 drivers/clk/sunxi/clk-factors.h |  3 +++
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
index 47b7d918b74c..ac259305d5d5 100644
--- a/drivers/clk/sunxi/clk-factors.c
+++ b/drivers/clk/sunxi/clk-factors.c
@@ -63,6 +63,26 @@ static unsigned long clk_factors_recalc_rate(struct clk_hw *hw,
 	if (config->pwidth != SUNXI_FACTORS_NOT_APPLICABLE)
 		p = FACTOR_GET(config->pshift, config->pwidth, reg);
 
+	if (factors->recalc) {
+		struct factors_request factors_req = {
+			.parent_rate = parent_rate,
+			.n = n,
+			.k = k,
+			.m = m,
+			.p = p,
+		};
+
+		/* get mux details from mux clk structure */
+		if (factors->mux)
+			factors_req.parent_index =
+				(reg >> factors->mux->shift) &
+				factors->mux->mask;
+
+		factors->recalc(&factors_req);
+
+		return factors_req.rate;
+	}
+
 	/* Calculate the rate */
 	rate = (parent_rate * (n + config->n_start) * (k + 1) >> p) / (m + 1);
 
@@ -87,6 +107,7 @@ static long clk_factors_round_rate(struct clk_hw *hw, unsigned long rate,
 static int clk_factors_determine_rate(struct clk_hw *hw,
 				      struct clk_rate_request *req)
 {
+	struct clk_factors *factors = to_clk_factors(hw);
 	struct clk_hw *parent, *best_parent = NULL;
 	int i, num_parents;
 	unsigned long parent_rate, best = 0, child_rate, best_child_rate = 0;
@@ -94,6 +115,10 @@ static int clk_factors_determine_rate(struct clk_hw *hw,
 	/* find the parent that can help provide the fastest rate <= rate */
 	num_parents = clk_hw_get_num_parents(hw);
 	for (i = 0; i < num_parents; i++) {
+		struct factors_request factors_req = {
+			.rate = req->rate,
+			.parent_index = i,
+		};
 		parent = clk_hw_get_parent_by_index(hw, i);
 		if (!parent)
 			continue;
@@ -102,8 +127,9 @@ static int clk_factors_determine_rate(struct clk_hw *hw,
 		else
 			parent_rate = clk_hw_get_rate(parent);
 
-		child_rate = clk_factors_round_rate(hw, req->rate,
-						    &parent_rate);
+		factors_req.parent_rate = parent_rate;
+		factors->get_factors(&factors_req);
+		child_rate = factors_req.rate;
 
 		if (child_rate <= req->rate && child_rate > best_child_rate) {
 			best_parent = parent;
diff --git a/drivers/clk/sunxi/clk-factors.h b/drivers/clk/sunxi/clk-factors.h
index f09d7c214533..a44a865a6b9e 100644
--- a/drivers/clk/sunxi/clk-factors.h
+++ b/drivers/clk/sunxi/clk-factors.h
@@ -22,6 +22,7 @@ struct clk_factors_config {
 struct factors_request {
 	unsigned long rate;
 	unsigned long parent_rate;
+	u8 parent_index;
 	u8 n;
 	u8 k;
 	u8 m;
@@ -34,6 +35,7 @@ struct factors_data {
 	int muxmask;
 	const struct clk_factors_config *table;
 	void (*getter)(struct factors_request *req);
+	void (*recalc)(struct factors_request *req);
 	const char *name;
 };
 
@@ -42,6 +44,7 @@ struct clk_factors {
 	void __iomem *reg;
 	const struct clk_factors_config *config;
 	void (*get_factors)(struct factors_request *req);
+	void (*recalc)(struct factors_request *req);
 	spinlock_t *lock;
 	/* for cleanup */
 	struct clk_mux *mux;
-- 
2.7.0

Re: [PATCH RFC 07/11] clk: sunxi: factors: Support custom formulas

[Maxime Ripard]

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

On Mon, Jan 25, 2016 at 09:15:43PM +0800, Chen-Yu Tsai wrote:
> Some clocks cannot be modelled using the standard factors clk formula,
> such as clocks with special pre-dividers on one parent, or clocks
> with all power-of-two dividers.
> 
> Add support for a custom .recalc callback for factors clk. Also pass
> the current parent index to the .get_factor and .recalc callbacks.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Applied, thanks!
Maxime

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH RFC 07/11] clk: sunxi: factors: Support custom formulas

[Maxime Ripard]

On Mon, Jan 25, 2016 at 09:15:43PM +0800, Chen-Yu Tsai wrote:
> Some clocks cannot be modelled using the standard factors clk formula,
> such as clocks with special pre-dividers on one parent, or clocks
> with all power-of-two dividers.
> 
> Add support for a custom .recalc callback for factors clk. Also pass
> the current parent index to the .get_factor and .recalc callbacks.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Applied, 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/20160127/5b2622d0/attachment.sig>

[PATCH RFC 08/11] clk: sunxi: factors: Drop round_rate from clk ops

[Chen-Yu Tsai]

The common clock framework requires either determine_rate or round_rate
to be implemented. We use determine_rate so we can pass the parent index
to the get_factors callback. This cannot be done easily with round_rate,
so just drop it.

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

diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
index ac259305d5d5..5769e6532c1d 100644
--- a/drivers/clk/sunxi/clk-factors.c
+++ b/drivers/clk/sunxi/clk-factors.c
@@ -89,21 +89,6 @@ static unsigned long clk_factors_recalc_rate(struct clk_hw *hw,
 	return rate;
 }
 
-static long clk_factors_round_rate(struct clk_hw *hw, unsigned long rate,
-				   unsigned long *parent_rate)
-{
-	struct clk_factors *factors = to_clk_factors(hw);
-	struct factors_request req = {
-		.rate = rate,
-		.parent_rate = *parent_rate,
-	};
-
-	factors->get_factors(&req);
-
-
-	return rate;
-}
-
 static int clk_factors_determine_rate(struct clk_hw *hw,
 				      struct clk_rate_request *req)
 {
@@ -189,7 +174,6 @@ static int clk_factors_set_rate(struct clk_hw *hw, unsigned long rate,
 static const struct clk_ops clk_factors_ops = {
 	.determine_rate = clk_factors_determine_rate,
 	.recalc_rate = clk_factors_recalc_rate,
-	.round_rate = clk_factors_round_rate,
 	.set_rate = clk_factors_set_rate,
 };
 
-- 
2.7.0

[PATCH RFC 08/11] clk: sunxi: factors: Drop round_rate from clk ops

[Chen-Yu Tsai]

The common clock framework requires either determine_rate or round_rate
to be implemented. We use determine_rate so we can pass the parent index
to the get_factors callback. This cannot be done easily with round_rate,
so just drop it.

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

diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
index ac259305d5d5..5769e6532c1d 100644
--- a/drivers/clk/sunxi/clk-factors.c
+++ b/drivers/clk/sunxi/clk-factors.c
@@ -89,21 +89,6 @@ static unsigned long clk_factors_recalc_rate(struct clk_hw *hw,
 	return rate;
 }
 
-static long clk_factors_round_rate(struct clk_hw *hw, unsigned long rate,
-				   unsigned long *parent_rate)
-{
-	struct clk_factors *factors = to_clk_factors(hw);
-	struct factors_request req = {
-		.rate = rate,
-		.parent_rate = *parent_rate,
-	};
-
-	factors->get_factors(&req);
-
-
-	return rate;
-}
-
 static int clk_factors_determine_rate(struct clk_hw *hw,
 				      struct clk_rate_request *req)
 {
@@ -189,7 +174,6 @@ static int clk_factors_set_rate(struct clk_hw *hw, unsigned long rate,
 static const struct clk_ops clk_factors_ops = {
 	.determine_rate = clk_factors_determine_rate,
 	.recalc_rate = clk_factors_recalc_rate,
-	.round_rate = clk_factors_round_rate,
 	.set_rate = clk_factors_set_rate,
 };
 
-- 
2.7.0

Re: [PATCH RFC 08/11] clk: sunxi: factors: Drop round_rate from clk ops

[Maxime Ripard]

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

On Mon, Jan 25, 2016 at 09:15:44PM +0800, Chen-Yu Tsai wrote:
> The common clock framework requires either determine_rate or round_rate
> to be implemented. We use determine_rate so we can pass the parent index
> to the get_factors callback. This cannot be done easily with round_rate,
> so just drop it.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Applied, thanks!
Maxime

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH RFC 08/11] clk: sunxi: factors: Drop round_rate from clk ops

[Maxime Ripard]

On Mon, Jan 25, 2016 at 09:15:44PM +0800, Chen-Yu Tsai wrote:
> The common clock framework requires either determine_rate or round_rate
> to be implemented. We use determine_rate so we can pass the parent index
> to the get_factors callback. This cannot be done easily with round_rate,
> so just drop it.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Applied, 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/20160127/d56df6d1/attachment.sig>

[PATCH RFC 09/11] clk: sunxi: rewrite sun6i-a31-ahb1-clk using factors clk with custom recalc

[Chen-Yu Tsai]

The factors clk implementation has been extended to support custom
recalc callbacks to support clocks that use one factor for certain
parents only, like a pre-divider.

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

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 56d4fdcf9c89..32d0bfa4913b 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -28,214 +28,6 @@
 
 static DEFINE_SPINLOCK(clk_lock);
 
-/**
- * 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
-/* un-shifted mask is what mux_clk expects */
-#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 << SUN6I_AHB1_DIV_SHIFT)
-#define SUN6I_AHB1_DIV_GET(reg)		((reg & SUN6I_AHB1_DIV_MASK) >> \
-						SUN6I_AHB1_DIV_SHIFT)
-#define SUN6I_AHB1_DIV_SET(reg, div)	((reg & ~SUN6I_AHB1_DIV_MASK) | \
-						(div << SUN6I_AHB1_DIV_SHIFT))
-#define SUN6I_AHB1_PLL6_DIV_SHIFT	6
-#define SUN6I_AHB1_PLL6_DIV_MASK	(0x3 << SUN6I_AHB1_PLL6_DIV_SHIFT)
-#define SUN6I_AHB1_PLL6_DIV_GET(reg)	((reg & SUN6I_AHB1_PLL6_DIV_MASK) >> \
-						SUN6I_AHB1_PLL6_DIV_SHIFT)
-#define SUN6I_AHB1_PLL6_DIV_SET(reg, div) ((reg & ~SUN6I_AHB1_PLL6_DIV_MASK) | \
-						(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;
-	}
-
-	/* we were asked to pass back divider values */
-	if (divp) {
-		*divp = calcp;
-		*pre_divp = calcm - 1;
-	}
-
-	return (parent_rate / calcm) >> calcp;
-}
-
-static int sun6i_ahb1_clk_determine_rate(struct clk_hw *hw,
-					 struct clk_rate_request *req)
-{
-	struct clk_hw *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_hw_get_num_parents(hw);
-	for (i = 0; i < num_parents; i++) {
-		parent = clk_hw_get_parent_by_index(hw, i);
-		if (!parent)
-			continue;
-		if (clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT)
-			parent_rate = clk_hw_round_rate(parent, req->rate);
-		else
-			parent_rate = clk_hw_get_rate(parent);
-
-		child_rate = sun6i_ahb1_clk_round(req->rate, NULL, NULL, i,
-						  parent_rate);
-
-		if (child_rate <= req->rate && child_rate > best_child_rate) {
-			best_parent = parent;
-			best = parent_rate;
-			best_child_rate = child_rate;
-		}
-	}
-
-	if (!best_parent)
-		return -EINVAL;
-
-	req->best_parent_hw = best_parent;
-	req->best_parent_rate = best;
-	req->rate = best_child_rate;
-
-	return 0;
-}
-
-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;
-
-	reg = of_io_request_and_map(node, 0, of_node_full_name(node));
-	if (IS_ERR(reg))
-		return;
-
-	/* we have a mux, we will have >1 parents */
-	i = of_clk_parent_fill(node, parents, SUN6I_AHB1_MAX_PARENTS);
-	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);
-
 /* Maximum number of parents our clocks have */
 #define SUNXI_MAX_PARENTS	5
 
@@ -490,6 +282,68 @@ static void sun5i_a13_get_ahb_factors(struct factors_request *req)
 	req->p = div;
 }
 
+#define SUN6I_AHB1_PARENT_PLL6	3
+
+/**
+ * sun6i_a31_get_ahb_factors() - calculates m, p factors for AHB
+ * AHB rate is calculated as follows
+ * rate = parent_rate >> p
+ *
+ * if parent is pll6, then
+ * parent_rate = pll6 rate / (m + 1)
+ */
+
+static void sun6i_get_ahb1_factors(struct factors_request *req)
+{
+	u8 div, calcp, calcm = 1;
+
+	/*
+	 * clock can only divide, so we will never be able to achieve
+	 * frequencies higher than the parent frequency
+	 */
+	if (req->parent_rate && req->rate > req->parent_rate)
+		req->rate = req->parent_rate;
+
+	div = DIV_ROUND_UP(req->parent_rate, req->rate);
+
+	/* calculate pre-divider if parent is pll6 */
+	if (req->parent_index == SUN6I_AHB1_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;
+	}
+
+	req->rate = (req->parent_rate / calcm) >> calcp;
+	req->p = calcp;
+	req->m = calcm - 1;
+}
+
+/**
+ * sun6i_ahb1_recalc() - calculates AHB clock rate from m, p factors and
+ *			 parent index
+ */
+static void sun6i_ahb1_recalc(struct factors_request *req)
+{
+	req->rate = req->parent_rate;
+
+	/* apply pre-divider first if parent is pll6 */
+	if (req->parent_index == SUN6I_AHB1_PARENT_PLL6)
+		req->rate /= req->m + 1;
+
+	/* clk divider */
+	req->rate >>= req->p;
+}
+
 /**
  * sun4i_get_apb1_factors() - calculates m, p factors for APB1
  * APB1 rate is calculated as follows
@@ -619,6 +473,13 @@ static const struct clk_factors_config sun5i_a13_ahb_config = {
 	.pwidth = 2,
 };
 
+static const struct clk_factors_config sun6i_ahb1_config = {
+	.mshift = 6,
+	.mwidth = 2,
+	.pshift = 4,
+	.pwidth = 2,
+};
+
 static const struct clk_factors_config sun4i_apb1_config = {
 	.mshift = 0,
 	.mwidth = 5,
@@ -686,6 +547,14 @@ static const struct factors_data sun5i_a13_ahb_data __initconst = {
 	.getter = sun5i_a13_get_ahb_factors,
 };
 
+static const struct factors_data sun6i_ahb1_data __initconst = {
+	.mux = 12,
+	.muxmask = BIT(1) | BIT(0),
+	.table = &sun6i_ahb1_config,
+	.getter = sun6i_get_ahb1_factors,
+	.recalc = sun6i_ahb1_recalc,
+};
+
 static const struct factors_data sun4i_apb1_data __initconst = {
 	.mux = 24,
 	.muxmask = BIT(1) | BIT(0),
@@ -725,6 +594,12 @@ static struct clk * __init sunxi_factors_clk_setup(struct device_node *node,
 	return clk;
 }
 
+static void __init sun6i_ahb1_clk_setup(struct device_node *node)
+{
+	sunxi_factors_clk_setup(node, &sun6i_ahb1_data);
+}
+CLK_OF_DECLARE(sun6i_a31_ahb1, "allwinner,sun6i-a31-ahb1-clk",
+	       sun6i_ahb1_clk_setup);
 
 
 /**
-- 
2.7.0

[PATCH RFC 09/11] clk: sunxi: rewrite sun6i-a31-ahb1-clk using factors clk with custom recalc

[Chen-Yu Tsai]

The factors clk implementation has been extended to support custom
recalc callbacks to support clocks that use one factor for certain
parents only, like a pre-divider.

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

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 56d4fdcf9c89..32d0bfa4913b 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -28,214 +28,6 @@
 
 static DEFINE_SPINLOCK(clk_lock);
 
-/**
- * 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
-/* un-shifted mask is what mux_clk expects */
-#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 << SUN6I_AHB1_DIV_SHIFT)
-#define SUN6I_AHB1_DIV_GET(reg)		((reg & SUN6I_AHB1_DIV_MASK) >> \
-						SUN6I_AHB1_DIV_SHIFT)
-#define SUN6I_AHB1_DIV_SET(reg, div)	((reg & ~SUN6I_AHB1_DIV_MASK) | \
-						(div << SUN6I_AHB1_DIV_SHIFT))
-#define SUN6I_AHB1_PLL6_DIV_SHIFT	6
-#define SUN6I_AHB1_PLL6_DIV_MASK	(0x3 << SUN6I_AHB1_PLL6_DIV_SHIFT)
-#define SUN6I_AHB1_PLL6_DIV_GET(reg)	((reg & SUN6I_AHB1_PLL6_DIV_MASK) >> \
-						SUN6I_AHB1_PLL6_DIV_SHIFT)
-#define SUN6I_AHB1_PLL6_DIV_SET(reg, div) ((reg & ~SUN6I_AHB1_PLL6_DIV_MASK) | \
-						(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;
-	}
-
-	/* we were asked to pass back divider values */
-	if (divp) {
-		*divp = calcp;
-		*pre_divp = calcm - 1;
-	}
-
-	return (parent_rate / calcm) >> calcp;
-}
-
-static int sun6i_ahb1_clk_determine_rate(struct clk_hw *hw,
-					 struct clk_rate_request *req)
-{
-	struct clk_hw *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_hw_get_num_parents(hw);
-	for (i = 0; i < num_parents; i++) {
-		parent = clk_hw_get_parent_by_index(hw, i);
-		if (!parent)
-			continue;
-		if (clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT)
-			parent_rate = clk_hw_round_rate(parent, req->rate);
-		else
-			parent_rate = clk_hw_get_rate(parent);
-
-		child_rate = sun6i_ahb1_clk_round(req->rate, NULL, NULL, i,
-						  parent_rate);
-
-		if (child_rate <= req->rate && child_rate > best_child_rate) {
-			best_parent = parent;
-			best = parent_rate;
-			best_child_rate = child_rate;
-		}
-	}
-
-	if (!best_parent)
-		return -EINVAL;
-
-	req->best_parent_hw = best_parent;
-	req->best_parent_rate = best;
-	req->rate = best_child_rate;
-
-	return 0;
-}
-
-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;
-
-	reg = of_io_request_and_map(node, 0, of_node_full_name(node));
-	if (IS_ERR(reg))
-		return;
-
-	/* we have a mux, we will have >1 parents */
-	i = of_clk_parent_fill(node, parents, SUN6I_AHB1_MAX_PARENTS);
-	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);
-
 /* Maximum number of parents our clocks have */
 #define SUNXI_MAX_PARENTS	5
 
@@ -490,6 +282,68 @@ static void sun5i_a13_get_ahb_factors(struct factors_request *req)
 	req->p = div;
 }
 
+#define SUN6I_AHB1_PARENT_PLL6	3
+
+/**
+ * sun6i_a31_get_ahb_factors() - calculates m, p factors for AHB
+ * AHB rate is calculated as follows
+ * rate = parent_rate >> p
+ *
+ * if parent is pll6, then
+ * parent_rate = pll6 rate / (m + 1)
+ */
+
+static void sun6i_get_ahb1_factors(struct factors_request *req)
+{
+	u8 div, calcp, calcm = 1;
+
+	/*
+	 * clock can only divide, so we will never be able to achieve
+	 * frequencies higher than the parent frequency
+	 */
+	if (req->parent_rate && req->rate > req->parent_rate)
+		req->rate = req->parent_rate;
+
+	div = DIV_ROUND_UP(req->parent_rate, req->rate);
+
+	/* calculate pre-divider if parent is pll6 */
+	if (req->parent_index == SUN6I_AHB1_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;
+	}
+
+	req->rate = (req->parent_rate / calcm) >> calcp;
+	req->p = calcp;
+	req->m = calcm - 1;
+}
+
+/**
+ * sun6i_ahb1_recalc() - calculates AHB clock rate from m, p factors and
+ *			 parent index
+ */
+static void sun6i_ahb1_recalc(struct factors_request *req)
+{
+	req->rate = req->parent_rate;
+
+	/* apply pre-divider first if parent is pll6 */
+	if (req->parent_index == SUN6I_AHB1_PARENT_PLL6)
+		req->rate /= req->m + 1;
+
+	/* clk divider */
+	req->rate >>= req->p;
+}
+
 /**
  * sun4i_get_apb1_factors() - calculates m, p factors for APB1
  * APB1 rate is calculated as follows
@@ -619,6 +473,13 @@ static const struct clk_factors_config sun5i_a13_ahb_config = {
 	.pwidth = 2,
 };
 
+static const struct clk_factors_config sun6i_ahb1_config = {
+	.mshift = 6,
+	.mwidth = 2,
+	.pshift = 4,
+	.pwidth = 2,
+};
+
 static const struct clk_factors_config sun4i_apb1_config = {
 	.mshift = 0,
 	.mwidth = 5,
@@ -686,6 +547,14 @@ static const struct factors_data sun5i_a13_ahb_data __initconst = {
 	.getter = sun5i_a13_get_ahb_factors,
 };
 
+static const struct factors_data sun6i_ahb1_data __initconst = {
+	.mux = 12,
+	.muxmask = BIT(1) | BIT(0),
+	.table = &sun6i_ahb1_config,
+	.getter = sun6i_get_ahb1_factors,
+	.recalc = sun6i_ahb1_recalc,
+};
+
 static const struct factors_data sun4i_apb1_data __initconst = {
 	.mux = 24,
 	.muxmask = BIT(1) | BIT(0),
@@ -725,6 +594,12 @@ static struct clk * __init sunxi_factors_clk_setup(struct device_node *node,
 	return clk;
 }
 
+static void __init sun6i_ahb1_clk_setup(struct device_node *node)
+{
+	sunxi_factors_clk_setup(node, &sun6i_ahb1_data);
+}
+CLK_OF_DECLARE(sun6i_a31_ahb1, "allwinner,sun6i-a31-ahb1-clk",
+	       sun6i_ahb1_clk_setup);
 
 
 /**
-- 
2.7.0

Re: [PATCH RFC 09/11] clk: sunxi: rewrite sun6i-a31-ahb1-clk using factors clk with custom recalc

[Maxime Ripard]

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

On Mon, Jan 25, 2016 at 09:15:45PM +0800, Chen-Yu Tsai wrote:
> The factors clk implementation has been extended to support custom
> recalc callbacks to support clocks that use one factor for certain
> parents only, like a pre-divider.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Applied, thanks!
Maxime

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH RFC 09/11] clk: sunxi: rewrite sun6i-a31-ahb1-clk using factors clk with custom recalc

[Maxime Ripard]

On Mon, Jan 25, 2016 at 09:15:45PM +0800, Chen-Yu Tsai wrote:
> The factors clk implementation has been extended to support custom
> recalc callbacks to support clocks that use one factor for certain
> parents only, like a pre-divider.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Applied, 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/20160127/2ee34ebd/attachment.sig>

[PATCH RFC 10/11] clk: sunxi: rewrite sun6i-ar100 using factors clk

[Chen-Yu Tsai]

sun6i's AR100 clock is a classic factors clk case:

AR100 = ((parent mux) >> p) / (m + 1)

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/clk/sunxi/clk-sun6i-ar100.c | 235 ++++++++++--------------------------
 1 file changed, 61 insertions(+), 174 deletions(-)

diff --git a/drivers/clk/sunxi/clk-sun6i-ar100.c b/drivers/clk/sunxi/clk-sun6i-ar100.c
index 20887686bdbe..a7f5777834eb 100644
--- a/drivers/clk/sunxi/clk-sun6i-ar100.c
+++ b/drivers/clk/sunxi/clk-sun6i-ar100.c
@@ -8,211 +8,97 @@
  *
  */
 
+#include <linux/bitops.h>
 #include <linux/clk-provider.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
+#include <linux/spinlock.h>
 
-#define SUN6I_AR100_MAX_PARENTS		4
-#define SUN6I_AR100_SHIFT_MASK		0x3
-#define SUN6I_AR100_SHIFT_MAX		SUN6I_AR100_SHIFT_MASK
-#define SUN6I_AR100_SHIFT_SHIFT		4
-#define SUN6I_AR100_DIV_MASK		0x1f
-#define SUN6I_AR100_DIV_MAX		(SUN6I_AR100_DIV_MASK + 1)
-#define SUN6I_AR100_DIV_SHIFT		8
-#define SUN6I_AR100_MUX_MASK		0x3
-#define SUN6I_AR100_MUX_SHIFT		16
-
-struct ar100_clk {
-	struct clk_hw hw;
-	void __iomem *reg;
-};
-
-static inline struct ar100_clk *to_ar100_clk(struct clk_hw *hw)
-{
-	return container_of(hw, struct ar100_clk, hw);
-}
-
-static unsigned long ar100_recalc_rate(struct clk_hw *hw,
-				       unsigned long parent_rate)
-{
-	struct ar100_clk *clk = to_ar100_clk(hw);
-	u32 val = readl(clk->reg);
-	int shift = (val >> SUN6I_AR100_SHIFT_SHIFT) & SUN6I_AR100_SHIFT_MASK;
-	int div = (val >> SUN6I_AR100_DIV_SHIFT) & SUN6I_AR100_DIV_MASK;
-
-	return (parent_rate >> shift) / (div + 1);
-}
-
-static int ar100_determine_rate(struct clk_hw *hw,
-				struct clk_rate_request *req)
-{
-	int nparents = clk_hw_get_num_parents(hw);
-	long best_rate = -EINVAL;
-	int i;
-
-	req->best_parent_hw = NULL;
-
-	for (i = 0; i < nparents; i++) {
-		unsigned long parent_rate;
-		unsigned long tmp_rate;
-		struct clk_hw *parent;
-		unsigned long div;
-		int shift;
-
-		parent = clk_hw_get_parent_by_index(hw, i);
-		parent_rate = clk_hw_get_rate(parent);
-		div = DIV_ROUND_UP(parent_rate, req->rate);
-
-		/*
-		 * The AR100 clk contains 2 divisors:
-		 * - one power of 2 divisor
-		 * - one regular divisor
-		 *
-		 * First check if we can safely shift (or divide by a power
-		 * of 2) without losing precision on the requested rate.
-		 */
-		shift = ffs(div) - 1;
-		if (shift > SUN6I_AR100_SHIFT_MAX)
-			shift = SUN6I_AR100_SHIFT_MAX;
-
-		div >>= shift;
-
-		/*
-		 * Then if the divisor is still bigger than what the HW
-		 * actually supports, use a bigger shift (or power of 2
-		 * divider) value and accept to lose some precision.
-		 */
-		while (div > SUN6I_AR100_DIV_MAX) {
-			shift++;
-			div >>= 1;
-			if (shift > SUN6I_AR100_SHIFT_MAX)
-				break;
-		}
-
-		/*
-		 * If the shift value (or power of 2 divider) is bigger
-		 * than what the HW actually support, skip this parent.
-		 */
-		if (shift > SUN6I_AR100_SHIFT_MAX)
-			continue;
-
-		tmp_rate = (parent_rate >> shift) / div;
-		if (!req->best_parent_hw || tmp_rate > best_rate) {
-			req->best_parent_hw = parent;
-			req->best_parent_rate = parent_rate;
-			best_rate = tmp_rate;
-		}
-	}
-
-	if (best_rate < 0)
-		return best_rate;
-
-	req->rate = best_rate;
-
-	return 0;
-}
-
-static int ar100_set_parent(struct clk_hw *hw, u8 index)
-{
-	struct ar100_clk *clk = to_ar100_clk(hw);
-	u32 val = readl(clk->reg);
-
-	if (index >= SUN6I_AR100_MAX_PARENTS)
-		return -EINVAL;
-
-	val &= ~(SUN6I_AR100_MUX_MASK << SUN6I_AR100_MUX_SHIFT);
-	val |= (index << SUN6I_AR100_MUX_SHIFT);
-	writel(val, clk->reg);
-
-	return 0;
-}
+#include "clk-factors.h"
 
-static u8 ar100_get_parent(struct clk_hw *hw)
-{
-	struct ar100_clk *clk = to_ar100_clk(hw);
-	return (readl(clk->reg) >> SUN6I_AR100_MUX_SHIFT) &
-	       SUN6I_AR100_MUX_MASK;
-}
-
-static int ar100_set_rate(struct clk_hw *hw, unsigned long rate,
-			  unsigned long parent_rate)
+/**
+ * sun6i_get_ar100_factors - Calculates factors p, m for AR100
+ *
+ * AR100 rate is calculated as follows
+ * rate = (parent_rate >> p) / (m + 1);
+ */
+static void sun6i_get_ar100_factors(struct factors_request *req)
 {
-	unsigned long div = parent_rate / rate;
-	struct ar100_clk *clk = to_ar100_clk(hw);
-	u32 val = readl(clk->reg);
+	unsigned long div;
 	int shift;
 
-	if (parent_rate % rate)
-		return -EINVAL;
+	/* clock only divides */
+	if (req->rate > req->parent_rate)
+		req->rate = req->parent_rate;
 
-	shift = ffs(div) - 1;
-	if (shift > SUN6I_AR100_SHIFT_MAX)
-		shift = SUN6I_AR100_SHIFT_MAX;
+	div = DIV_ROUND_UP(req->parent_rate, req->rate);
 
-	div >>= shift;
+	if (div < 32)
+		shift = 0;
+	else if (div >> 1 < 32)
+		shift = 1;
+	else if (div >> 2 < 32)
+		shift = 2;
+	else
+		shift = 3;
 
-	if (div > SUN6I_AR100_DIV_MAX)
-		return -EINVAL;
+	div >>= shift;
 
-	val &= ~((SUN6I_AR100_SHIFT_MASK << SUN6I_AR100_SHIFT_SHIFT) |
-		 (SUN6I_AR100_DIV_MASK << SUN6I_AR100_DIV_SHIFT));
-	val |= (shift << SUN6I_AR100_SHIFT_SHIFT) |
-	       (div << SUN6I_AR100_DIV_SHIFT);
-	writel(val, clk->reg);
+	if (div > 32)
+		div = 32;
 
-	return 0;
+	req->rate = (req->parent_rate >> shift) / div;
+	req->m = div - 1;
+	req->p = shift;
 }
 
-static struct clk_ops ar100_ops = {
-	.recalc_rate = ar100_recalc_rate,
-	.determine_rate = ar100_determine_rate,
-	.set_parent = ar100_set_parent,
-	.get_parent = ar100_get_parent,
-	.set_rate = ar100_set_rate,
+static const struct clk_factors_config sun6i_ar100_config = {
+	.mwidth = 5,
+	.mshift = 8,
+	.pwidth = 2,
+	.pshift = 4,
 };
 
+static const struct factors_data sun6i_ar100_data __initconst = {
+	.mux = 16,
+	.muxmask = GENMASK(1, 0),
+	.table = &sun6i_ar100_config,
+	.getter = sun6i_get_ar100_factors,
+};
+
+static DEFINE_SPINLOCK(sun6i_ar100_lock);
+
 static int sun6i_a31_ar100_clk_probe(struct platform_device *pdev)
 {
-	const char *parents[SUN6I_AR100_MAX_PARENTS];
 	struct device_node *np = pdev->dev.of_node;
-	const char *clk_name = np->name;
-	struct clk_init_data init;
-	struct ar100_clk *ar100;
 	struct resource *r;
+	void __iomem *reg;
 	struct clk *clk;
-	int nparents;
-
-	ar100 = devm_kzalloc(&pdev->dev, sizeof(*ar100), GFP_KERNEL);
-	if (!ar100)
-		return -ENOMEM;
 
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	ar100->reg = devm_ioremap_resource(&pdev->dev, r);
-	if (IS_ERR(ar100->reg))
-		return PTR_ERR(ar100->reg);
+	reg = devm_ioremap_resource(&pdev->dev, r);
+	if (IS_ERR(reg))
+		return PTR_ERR(reg);
 
-	nparents = of_clk_get_parent_count(np);
-	if (nparents > SUN6I_AR100_MAX_PARENTS)
-		nparents = SUN6I_AR100_MAX_PARENTS;
-
-	of_clk_parent_fill(np, parents, nparents);
+	clk = sunxi_factors_register(np, &sun6i_ar100_data, &sun6i_ar100_lock,
+				     reg);
+	if (!clk)
+		return -ENOMEM;
 
-	of_property_read_string(np, "clock-output-names", &clk_name);
+	platform_set_drvdata(pdev, clk);
 
-	init.name = clk_name;
-	init.ops = &ar100_ops;
-	init.parent_names = parents;
-	init.num_parents = nparents;
-	init.flags = 0;
+	return 0;
+}
 
-	ar100->hw.init = &init;
+static int sun6i_a31_ar100_clk_remove(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct clk *clk = platform_get_drvdata(pdev);
 
-	clk = clk_register(&pdev->dev, &ar100->hw);
-	if (IS_ERR(clk))
-		return PTR_ERR(clk);
+	sunxi_factors_unregister(np, clk);
 
-	return of_clk_add_provider(np, of_clk_src_simple_get, clk);
+	return 0;
 }
 
 static const struct of_device_id sun6i_a31_ar100_clk_dt_ids[] = {
@@ -227,6 +113,7 @@ static struct platform_driver sun6i_a31_ar100_clk_driver = {
 		.of_match_table = sun6i_a31_ar100_clk_dt_ids,
 	},
 	.probe = sun6i_a31_ar100_clk_probe,
+	.remove = sun6i_a31_ar100_clk_remove,
 };
 module_platform_driver(sun6i_a31_ar100_clk_driver);
 
-- 
2.7.0

[PATCH RFC 10/11] clk: sunxi: rewrite sun6i-ar100 using factors clk

[Chen-Yu Tsai]

sun6i's AR100 clock is a classic factors clk case:

AR100 = ((parent mux) >> p) / (m + 1)

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/clk/sunxi/clk-sun6i-ar100.c | 235 ++++++++++--------------------------
 1 file changed, 61 insertions(+), 174 deletions(-)

diff --git a/drivers/clk/sunxi/clk-sun6i-ar100.c b/drivers/clk/sunxi/clk-sun6i-ar100.c
index 20887686bdbe..a7f5777834eb 100644
--- a/drivers/clk/sunxi/clk-sun6i-ar100.c
+++ b/drivers/clk/sunxi/clk-sun6i-ar100.c
@@ -8,211 +8,97 @@
  *
  */
 
+#include <linux/bitops.h>
 #include <linux/clk-provider.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
+#include <linux/spinlock.h>
 
-#define SUN6I_AR100_MAX_PARENTS		4
-#define SUN6I_AR100_SHIFT_MASK		0x3
-#define SUN6I_AR100_SHIFT_MAX		SUN6I_AR100_SHIFT_MASK
-#define SUN6I_AR100_SHIFT_SHIFT		4
-#define SUN6I_AR100_DIV_MASK		0x1f
-#define SUN6I_AR100_DIV_MAX		(SUN6I_AR100_DIV_MASK + 1)
-#define SUN6I_AR100_DIV_SHIFT		8
-#define SUN6I_AR100_MUX_MASK		0x3
-#define SUN6I_AR100_MUX_SHIFT		16
-
-struct ar100_clk {
-	struct clk_hw hw;
-	void __iomem *reg;
-};
-
-static inline struct ar100_clk *to_ar100_clk(struct clk_hw *hw)
-{
-	return container_of(hw, struct ar100_clk, hw);
-}
-
-static unsigned long ar100_recalc_rate(struct clk_hw *hw,
-				       unsigned long parent_rate)
-{
-	struct ar100_clk *clk = to_ar100_clk(hw);
-	u32 val = readl(clk->reg);
-	int shift = (val >> SUN6I_AR100_SHIFT_SHIFT) & SUN6I_AR100_SHIFT_MASK;
-	int div = (val >> SUN6I_AR100_DIV_SHIFT) & SUN6I_AR100_DIV_MASK;
-
-	return (parent_rate >> shift) / (div + 1);
-}
-
-static int ar100_determine_rate(struct clk_hw *hw,
-				struct clk_rate_request *req)
-{
-	int nparents = clk_hw_get_num_parents(hw);
-	long best_rate = -EINVAL;
-	int i;
-
-	req->best_parent_hw = NULL;
-
-	for (i = 0; i < nparents; i++) {
-		unsigned long parent_rate;
-		unsigned long tmp_rate;
-		struct clk_hw *parent;
-		unsigned long div;
-		int shift;
-
-		parent = clk_hw_get_parent_by_index(hw, i);
-		parent_rate = clk_hw_get_rate(parent);
-		div = DIV_ROUND_UP(parent_rate, req->rate);
-
-		/*
-		 * The AR100 clk contains 2 divisors:
-		 * - one power of 2 divisor
-		 * - one regular divisor
-		 *
-		 * First check if we can safely shift (or divide by a power
-		 * of 2) without losing precision on the requested rate.
-		 */
-		shift = ffs(div) - 1;
-		if (shift > SUN6I_AR100_SHIFT_MAX)
-			shift = SUN6I_AR100_SHIFT_MAX;
-
-		div >>= shift;
-
-		/*
-		 * Then if the divisor is still bigger than what the HW
-		 * actually supports, use a bigger shift (or power of 2
-		 * divider) value and accept to lose some precision.
-		 */
-		while (div > SUN6I_AR100_DIV_MAX) {
-			shift++;
-			div >>= 1;
-			if (shift > SUN6I_AR100_SHIFT_MAX)
-				break;
-		}
-
-		/*
-		 * If the shift value (or power of 2 divider) is bigger
-		 * than what the HW actually support, skip this parent.
-		 */
-		if (shift > SUN6I_AR100_SHIFT_MAX)
-			continue;
-
-		tmp_rate = (parent_rate >> shift) / div;
-		if (!req->best_parent_hw || tmp_rate > best_rate) {
-			req->best_parent_hw = parent;
-			req->best_parent_rate = parent_rate;
-			best_rate = tmp_rate;
-		}
-	}
-
-	if (best_rate < 0)
-		return best_rate;
-
-	req->rate = best_rate;
-
-	return 0;
-}
-
-static int ar100_set_parent(struct clk_hw *hw, u8 index)
-{
-	struct ar100_clk *clk = to_ar100_clk(hw);
-	u32 val = readl(clk->reg);
-
-	if (index >= SUN6I_AR100_MAX_PARENTS)
-		return -EINVAL;
-
-	val &= ~(SUN6I_AR100_MUX_MASK << SUN6I_AR100_MUX_SHIFT);
-	val |= (index << SUN6I_AR100_MUX_SHIFT);
-	writel(val, clk->reg);
-
-	return 0;
-}
+#include "clk-factors.h"
 
-static u8 ar100_get_parent(struct clk_hw *hw)
-{
-	struct ar100_clk *clk = to_ar100_clk(hw);
-	return (readl(clk->reg) >> SUN6I_AR100_MUX_SHIFT) &
-	       SUN6I_AR100_MUX_MASK;
-}
-
-static int ar100_set_rate(struct clk_hw *hw, unsigned long rate,
-			  unsigned long parent_rate)
+/**
+ * sun6i_get_ar100_factors - Calculates factors p, m for AR100
+ *
+ * AR100 rate is calculated as follows
+ * rate = (parent_rate >> p) / (m + 1);
+ */
+static void sun6i_get_ar100_factors(struct factors_request *req)
 {
-	unsigned long div = parent_rate / rate;
-	struct ar100_clk *clk = to_ar100_clk(hw);
-	u32 val = readl(clk->reg);
+	unsigned long div;
 	int shift;
 
-	if (parent_rate % rate)
-		return -EINVAL;
+	/* clock only divides */
+	if (req->rate > req->parent_rate)
+		req->rate = req->parent_rate;
 
-	shift = ffs(div) - 1;
-	if (shift > SUN6I_AR100_SHIFT_MAX)
-		shift = SUN6I_AR100_SHIFT_MAX;
+	div = DIV_ROUND_UP(req->parent_rate, req->rate);
 
-	div >>= shift;
+	if (div < 32)
+		shift = 0;
+	else if (div >> 1 < 32)
+		shift = 1;
+	else if (div >> 2 < 32)
+		shift = 2;
+	else
+		shift = 3;
 
-	if (div > SUN6I_AR100_DIV_MAX)
-		return -EINVAL;
+	div >>= shift;
 
-	val &= ~((SUN6I_AR100_SHIFT_MASK << SUN6I_AR100_SHIFT_SHIFT) |
-		 (SUN6I_AR100_DIV_MASK << SUN6I_AR100_DIV_SHIFT));
-	val |= (shift << SUN6I_AR100_SHIFT_SHIFT) |
-	       (div << SUN6I_AR100_DIV_SHIFT);
-	writel(val, clk->reg);
+	if (div > 32)
+		div = 32;
 
-	return 0;
+	req->rate = (req->parent_rate >> shift) / div;
+	req->m = div - 1;
+	req->p = shift;
 }
 
-static struct clk_ops ar100_ops = {
-	.recalc_rate = ar100_recalc_rate,
-	.determine_rate = ar100_determine_rate,
-	.set_parent = ar100_set_parent,
-	.get_parent = ar100_get_parent,
-	.set_rate = ar100_set_rate,
+static const struct clk_factors_config sun6i_ar100_config = {
+	.mwidth = 5,
+	.mshift = 8,
+	.pwidth = 2,
+	.pshift = 4,
 };
 
+static const struct factors_data sun6i_ar100_data __initconst = {
+	.mux = 16,
+	.muxmask = GENMASK(1, 0),
+	.table = &sun6i_ar100_config,
+	.getter = sun6i_get_ar100_factors,
+};
+
+static DEFINE_SPINLOCK(sun6i_ar100_lock);
+
 static int sun6i_a31_ar100_clk_probe(struct platform_device *pdev)
 {
-	const char *parents[SUN6I_AR100_MAX_PARENTS];
 	struct device_node *np = pdev->dev.of_node;
-	const char *clk_name = np->name;
-	struct clk_init_data init;
-	struct ar100_clk *ar100;
 	struct resource *r;
+	void __iomem *reg;
 	struct clk *clk;
-	int nparents;
-
-	ar100 = devm_kzalloc(&pdev->dev, sizeof(*ar100), GFP_KERNEL);
-	if (!ar100)
-		return -ENOMEM;
 
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	ar100->reg = devm_ioremap_resource(&pdev->dev, r);
-	if (IS_ERR(ar100->reg))
-		return PTR_ERR(ar100->reg);
+	reg = devm_ioremap_resource(&pdev->dev, r);
+	if (IS_ERR(reg))
+		return PTR_ERR(reg);
 
-	nparents = of_clk_get_parent_count(np);
-	if (nparents > SUN6I_AR100_MAX_PARENTS)
-		nparents = SUN6I_AR100_MAX_PARENTS;
-
-	of_clk_parent_fill(np, parents, nparents);
+	clk = sunxi_factors_register(np, &sun6i_ar100_data, &sun6i_ar100_lock,
+				     reg);
+	if (!clk)
+		return -ENOMEM;
 
-	of_property_read_string(np, "clock-output-names", &clk_name);
+	platform_set_drvdata(pdev, clk);
 
-	init.name = clk_name;
-	init.ops = &ar100_ops;
-	init.parent_names = parents;
-	init.num_parents = nparents;
-	init.flags = 0;
+	return 0;
+}
 
-	ar100->hw.init = &init;
+static int sun6i_a31_ar100_clk_remove(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct clk *clk = platform_get_drvdata(pdev);
 
-	clk = clk_register(&pdev->dev, &ar100->hw);
-	if (IS_ERR(clk))
-		return PTR_ERR(clk);
+	sunxi_factors_unregister(np, clk);
 
-	return of_clk_add_provider(np, of_clk_src_simple_get, clk);
+	return 0;
 }
 
 static const struct of_device_id sun6i_a31_ar100_clk_dt_ids[] = {
@@ -227,6 +113,7 @@ static struct platform_driver sun6i_a31_ar100_clk_driver = {
 		.of_match_table = sun6i_a31_ar100_clk_dt_ids,
 	},
 	.probe = sun6i_a31_ar100_clk_probe,
+	.remove = sun6i_a31_ar100_clk_remove,
 };
 module_platform_driver(sun6i_a31_ar100_clk_driver);
 
-- 
2.7.0

Re: [PATCH RFC 10/11] clk: sunxi: rewrite sun6i-ar100 using factors clk

[Maxime Ripard]

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

On Mon, Jan 25, 2016 at 09:15:46PM +0800, Chen-Yu Tsai wrote:
> sun6i's AR100 clock is a classic factors clk case:
> 
> AR100 = ((parent mux) >> p) / (m + 1)
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Applied, thanks!
Maxime

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH RFC 10/11] clk: sunxi: rewrite sun6i-ar100 using factors clk

[Maxime Ripard]

On Mon, Jan 25, 2016 at 09:15:46PM +0800, Chen-Yu Tsai wrote:
> sun6i's AR100 clock is a classic factors clk case:
> 
> AR100 = ((parent mux) >> p) / (m + 1)
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Applied, 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/20160127/ab856279/attachment.sig>

Re: [PATCH RFC 10/11] clk: sunxi: rewrite sun6i-ar100 using factors clk

[Paul Gortmaker]

On Mon, Jan 25, 2016 at 8:15 AM, Chen-Yu Tsai <wens@csie.org> wrote:
> sun6i's AR100 clock is a classic factors clk case:
>
> AR100 = ((parent mux) >> p) / (m + 1)
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

This patch adds a ".remove" function to a driver that is controlled by
a bool Kconfig, and hence the remove code can't be called unless
one explicitly goes into sysfs and muddles with the unbind as root
(which normally has no sane use case for builtin, non-modular code).

Since I'm trying to cut back on the amount of dead code we have
in .remove functions for built in drivers, is there a valid use case
for this one here, or can I just stage a delete commit for it too?

Normally I've set the set the disallow-unbind flag when deleting
a .remove function to make it clear there is no sane use case
for wandering into sysfs and unhooking the builtin driver.

Thanks,
Paul.
--

> ---
>  drivers/clk/sunxi/clk-sun6i-ar100.c | 235 ++++++++++--------------------------
>  1 file changed, 61 insertions(+), 174 deletions(-)
>
> diff --git a/drivers/clk/sunxi/clk-sun6i-ar100.c b/drivers/clk/sunxi/clk-sun6i-ar100.c
> index 20887686bdbe..a7f5777834eb 100644
> --- a/drivers/clk/sunxi/clk-sun6i-ar100.c
> +++ b/drivers/clk/sunxi/clk-sun6i-ar100.c
> @@ -8,211 +8,97 @@
>   *
>   */
>
> +#include <linux/bitops.h>
>  #include <linux/clk-provider.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
> +#include <linux/spinlock.h>
>
> -#define SUN6I_AR100_MAX_PARENTS                4
> -#define SUN6I_AR100_SHIFT_MASK         0x3
> -#define SUN6I_AR100_SHIFT_MAX          SUN6I_AR100_SHIFT_MASK
> -#define SUN6I_AR100_SHIFT_SHIFT                4
> -#define SUN6I_AR100_DIV_MASK           0x1f
> -#define SUN6I_AR100_DIV_MAX            (SUN6I_AR100_DIV_MASK + 1)
> -#define SUN6I_AR100_DIV_SHIFT          8
> -#define SUN6I_AR100_MUX_MASK           0x3
> -#define SUN6I_AR100_MUX_SHIFT          16
> -
> -struct ar100_clk {
> -       struct clk_hw hw;
> -       void __iomem *reg;
> -};
> -
> -static inline struct ar100_clk *to_ar100_clk(struct clk_hw *hw)
> -{
> -       return container_of(hw, struct ar100_clk, hw);
> -}
> -
> -static unsigned long ar100_recalc_rate(struct clk_hw *hw,
> -                                      unsigned long parent_rate)
> -{
> -       struct ar100_clk *clk = to_ar100_clk(hw);
> -       u32 val = readl(clk->reg);
> -       int shift = (val >> SUN6I_AR100_SHIFT_SHIFT) & SUN6I_AR100_SHIFT_MASK;
> -       int div = (val >> SUN6I_AR100_DIV_SHIFT) & SUN6I_AR100_DIV_MASK;
> -
> -       return (parent_rate >> shift) / (div + 1);
> -}
> -
> -static int ar100_determine_rate(struct clk_hw *hw,
> -                               struct clk_rate_request *req)
> -{
> -       int nparents = clk_hw_get_num_parents(hw);
> -       long best_rate = -EINVAL;
> -       int i;
> -
> -       req->best_parent_hw = NULL;
> -
> -       for (i = 0; i < nparents; i++) {
> -               unsigned long parent_rate;
> -               unsigned long tmp_rate;
> -               struct clk_hw *parent;
> -               unsigned long div;
> -               int shift;
> -
> -               parent = clk_hw_get_parent_by_index(hw, i);
> -               parent_rate = clk_hw_get_rate(parent);
> -               div = DIV_ROUND_UP(parent_rate, req->rate);
> -
> -               /*
> -                * The AR100 clk contains 2 divisors:
> -                * - one power of 2 divisor
> -                * - one regular divisor
> -                *
> -                * First check if we can safely shift (or divide by a power
> -                * of 2) without losing precision on the requested rate.
> -                */
> -               shift = ffs(div) - 1;
> -               if (shift > SUN6I_AR100_SHIFT_MAX)
> -                       shift = SUN6I_AR100_SHIFT_MAX;
> -
> -               div >>= shift;
> -
> -               /*
> -                * Then if the divisor is still bigger than what the HW
> -                * actually supports, use a bigger shift (or power of 2
> -                * divider) value and accept to lose some precision.
> -                */
> -               while (div > SUN6I_AR100_DIV_MAX) {
> -                       shift++;
> -                       div >>= 1;
> -                       if (shift > SUN6I_AR100_SHIFT_MAX)
> -                               break;
> -               }
> -
> -               /*
> -                * If the shift value (or power of 2 divider) is bigger
> -                * than what the HW actually support, skip this parent.
> -                */
> -               if (shift > SUN6I_AR100_SHIFT_MAX)
> -                       continue;
> -
> -               tmp_rate = (parent_rate >> shift) / div;
> -               if (!req->best_parent_hw || tmp_rate > best_rate) {
> -                       req->best_parent_hw = parent;
> -                       req->best_parent_rate = parent_rate;
> -                       best_rate = tmp_rate;
> -               }
> -       }
> -
> -       if (best_rate < 0)
> -               return best_rate;
> -
> -       req->rate = best_rate;
> -
> -       return 0;
> -}
> -
> -static int ar100_set_parent(struct clk_hw *hw, u8 index)
> -{
> -       struct ar100_clk *clk = to_ar100_clk(hw);
> -       u32 val = readl(clk->reg);
> -
> -       if (index >= SUN6I_AR100_MAX_PARENTS)
> -               return -EINVAL;
> -
> -       val &= ~(SUN6I_AR100_MUX_MASK << SUN6I_AR100_MUX_SHIFT);
> -       val |= (index << SUN6I_AR100_MUX_SHIFT);
> -       writel(val, clk->reg);
> -
> -       return 0;
> -}
> +#include "clk-factors.h"
>
> -static u8 ar100_get_parent(struct clk_hw *hw)
> -{
> -       struct ar100_clk *clk = to_ar100_clk(hw);
> -       return (readl(clk->reg) >> SUN6I_AR100_MUX_SHIFT) &
> -              SUN6I_AR100_MUX_MASK;
> -}
> -
> -static int ar100_set_rate(struct clk_hw *hw, unsigned long rate,
> -                         unsigned long parent_rate)
> +/**
> + * sun6i_get_ar100_factors - Calculates factors p, m for AR100
> + *
> + * AR100 rate is calculated as follows
> + * rate = (parent_rate >> p) / (m + 1);
> + */
> +static void sun6i_get_ar100_factors(struct factors_request *req)
>  {
> -       unsigned long div = parent_rate / rate;
> -       struct ar100_clk *clk = to_ar100_clk(hw);
> -       u32 val = readl(clk->reg);
> +       unsigned long div;
>         int shift;
>
> -       if (parent_rate % rate)
> -               return -EINVAL;
> +       /* clock only divides */
> +       if (req->rate > req->parent_rate)
> +               req->rate = req->parent_rate;
>
> -       shift = ffs(div) - 1;
> -       if (shift > SUN6I_AR100_SHIFT_MAX)
> -               shift = SUN6I_AR100_SHIFT_MAX;
> +       div = DIV_ROUND_UP(req->parent_rate, req->rate);
>
> -       div >>= shift;
> +       if (div < 32)
> +               shift = 0;
> +       else if (div >> 1 < 32)
> +               shift = 1;
> +       else if (div >> 2 < 32)
> +               shift = 2;
> +       else
> +               shift = 3;
>
> -       if (div > SUN6I_AR100_DIV_MAX)
> -               return -EINVAL;
> +       div >>= shift;
>
> -       val &= ~((SUN6I_AR100_SHIFT_MASK << SUN6I_AR100_SHIFT_SHIFT) |
> -                (SUN6I_AR100_DIV_MASK << SUN6I_AR100_DIV_SHIFT));
> -       val |= (shift << SUN6I_AR100_SHIFT_SHIFT) |
> -              (div << SUN6I_AR100_DIV_SHIFT);
> -       writel(val, clk->reg);
> +       if (div > 32)
> +               div = 32;
>
> -       return 0;
> +       req->rate = (req->parent_rate >> shift) / div;
> +       req->m = div - 1;
> +       req->p = shift;
>  }
>
> -static struct clk_ops ar100_ops = {
> -       .recalc_rate = ar100_recalc_rate,
> -       .determine_rate = ar100_determine_rate,
> -       .set_parent = ar100_set_parent,
> -       .get_parent = ar100_get_parent,
> -       .set_rate = ar100_set_rate,
> +static const struct clk_factors_config sun6i_ar100_config = {
> +       .mwidth = 5,
> +       .mshift = 8,
> +       .pwidth = 2,
> +       .pshift = 4,
>  };
>
> +static const struct factors_data sun6i_ar100_data __initconst = {
> +       .mux = 16,
> +       .muxmask = GENMASK(1, 0),
> +       .table = &sun6i_ar100_config,
> +       .getter = sun6i_get_ar100_factors,
> +};
> +
> +static DEFINE_SPINLOCK(sun6i_ar100_lock);
> +
>  static int sun6i_a31_ar100_clk_probe(struct platform_device *pdev)
>  {
> -       const char *parents[SUN6I_AR100_MAX_PARENTS];
>         struct device_node *np = pdev->dev.of_node;
> -       const char *clk_name = np->name;
> -       struct clk_init_data init;
> -       struct ar100_clk *ar100;
>         struct resource *r;
> +       void __iomem *reg;
>         struct clk *clk;
> -       int nparents;
> -
> -       ar100 = devm_kzalloc(&pdev->dev, sizeof(*ar100), GFP_KERNEL);
> -       if (!ar100)
> -               return -ENOMEM;
>
>         r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -       ar100->reg = devm_ioremap_resource(&pdev->dev, r);
> -       if (IS_ERR(ar100->reg))
> -               return PTR_ERR(ar100->reg);
> +       reg = devm_ioremap_resource(&pdev->dev, r);
> +       if (IS_ERR(reg))
> +               return PTR_ERR(reg);
>
> -       nparents = of_clk_get_parent_count(np);
> -       if (nparents > SUN6I_AR100_MAX_PARENTS)
> -               nparents = SUN6I_AR100_MAX_PARENTS;
> -
> -       of_clk_parent_fill(np, parents, nparents);
> +       clk = sunxi_factors_register(np, &sun6i_ar100_data, &sun6i_ar100_lock,
> +                                    reg);
> +       if (!clk)
> +               return -ENOMEM;
>
> -       of_property_read_string(np, "clock-output-names", &clk_name);
> +       platform_set_drvdata(pdev, clk);
>
> -       init.name = clk_name;
> -       init.ops = &ar100_ops;
> -       init.parent_names = parents;
> -       init.num_parents = nparents;
> -       init.flags = 0;
> +       return 0;
> +}
>
> -       ar100->hw.init = &init;
> +static int sun6i_a31_ar100_clk_remove(struct platform_device *pdev)
> +{
> +       struct device_node *np = pdev->dev.of_node;
> +       struct clk *clk = platform_get_drvdata(pdev);
>
> -       clk = clk_register(&pdev->dev, &ar100->hw);
> -       if (IS_ERR(clk))
> -               return PTR_ERR(clk);
> +       sunxi_factors_unregister(np, clk);
>
> -       return of_clk_add_provider(np, of_clk_src_simple_get, clk);
> +       return 0;
>  }
>
>  static const struct of_device_id sun6i_a31_ar100_clk_dt_ids[] = {
> @@ -227,6 +113,7 @@ static struct platform_driver sun6i_a31_ar100_clk_driver = {
>                 .of_match_table = sun6i_a31_ar100_clk_dt_ids,
>         },
>         .probe = sun6i_a31_ar100_clk_probe,
> +       .remove = sun6i_a31_ar100_clk_remove,
>  };
>  module_platform_driver(sun6i_a31_ar100_clk_driver);
>
> --
> 2.7.0
>

[PATCH RFC 10/11] clk: sunxi: rewrite sun6i-ar100 using factors clk

[Paul Gortmaker]

On Mon, Jan 25, 2016 at 8:15 AM, Chen-Yu Tsai <wens@csie.org> wrote:
> sun6i's AR100 clock is a classic factors clk case:
>
> AR100 = ((parent mux) >> p) / (m + 1)
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

This patch adds a ".remove" function to a driver that is controlled by
a bool Kconfig, and hence the remove code can't be called unless
one explicitly goes into sysfs and muddles with the unbind as root
(which normally has no sane use case for builtin, non-modular code).

Since I'm trying to cut back on the amount of dead code we have
in .remove functions for built in drivers, is there a valid use case
for this one here, or can I just stage a delete commit for it too?

Normally I've set the set the disallow-unbind flag when deleting
a .remove function to make it clear there is no sane use case
for wandering into sysfs and unhooking the builtin driver.

Thanks,
Paul.
--

> ---
>  drivers/clk/sunxi/clk-sun6i-ar100.c | 235 ++++++++++--------------------------
>  1 file changed, 61 insertions(+), 174 deletions(-)
>
> diff --git a/drivers/clk/sunxi/clk-sun6i-ar100.c b/drivers/clk/sunxi/clk-sun6i-ar100.c
> index 20887686bdbe..a7f5777834eb 100644
> --- a/drivers/clk/sunxi/clk-sun6i-ar100.c
> +++ b/drivers/clk/sunxi/clk-sun6i-ar100.c
> @@ -8,211 +8,97 @@
>   *
>   */
>
> +#include <linux/bitops.h>
>  #include <linux/clk-provider.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
> +#include <linux/spinlock.h>
>
> -#define SUN6I_AR100_MAX_PARENTS                4
> -#define SUN6I_AR100_SHIFT_MASK         0x3
> -#define SUN6I_AR100_SHIFT_MAX          SUN6I_AR100_SHIFT_MASK
> -#define SUN6I_AR100_SHIFT_SHIFT                4
> -#define SUN6I_AR100_DIV_MASK           0x1f
> -#define SUN6I_AR100_DIV_MAX            (SUN6I_AR100_DIV_MASK + 1)
> -#define SUN6I_AR100_DIV_SHIFT          8
> -#define SUN6I_AR100_MUX_MASK           0x3
> -#define SUN6I_AR100_MUX_SHIFT          16
> -
> -struct ar100_clk {
> -       struct clk_hw hw;
> -       void __iomem *reg;
> -};
> -
> -static inline struct ar100_clk *to_ar100_clk(struct clk_hw *hw)
> -{
> -       return container_of(hw, struct ar100_clk, hw);
> -}
> -
> -static unsigned long ar100_recalc_rate(struct clk_hw *hw,
> -                                      unsigned long parent_rate)
> -{
> -       struct ar100_clk *clk = to_ar100_clk(hw);
> -       u32 val = readl(clk->reg);
> -       int shift = (val >> SUN6I_AR100_SHIFT_SHIFT) & SUN6I_AR100_SHIFT_MASK;
> -       int div = (val >> SUN6I_AR100_DIV_SHIFT) & SUN6I_AR100_DIV_MASK;
> -
> -       return (parent_rate >> shift) / (div + 1);
> -}
> -
> -static int ar100_determine_rate(struct clk_hw *hw,
> -                               struct clk_rate_request *req)
> -{
> -       int nparents = clk_hw_get_num_parents(hw);
> -       long best_rate = -EINVAL;
> -       int i;
> -
> -       req->best_parent_hw = NULL;
> -
> -       for (i = 0; i < nparents; i++) {
> -               unsigned long parent_rate;
> -               unsigned long tmp_rate;
> -               struct clk_hw *parent;
> -               unsigned long div;
> -               int shift;
> -
> -               parent = clk_hw_get_parent_by_index(hw, i);
> -               parent_rate = clk_hw_get_rate(parent);
> -               div = DIV_ROUND_UP(parent_rate, req->rate);
> -
> -               /*
> -                * The AR100 clk contains 2 divisors:
> -                * - one power of 2 divisor
> -                * - one regular divisor
> -                *
> -                * First check if we can safely shift (or divide by a power
> -                * of 2) without losing precision on the requested rate.
> -                */
> -               shift = ffs(div) - 1;
> -               if (shift > SUN6I_AR100_SHIFT_MAX)
> -                       shift = SUN6I_AR100_SHIFT_MAX;
> -
> -               div >>= shift;
> -
> -               /*
> -                * Then if the divisor is still bigger than what the HW
> -                * actually supports, use a bigger shift (or power of 2
> -                * divider) value and accept to lose some precision.
> -                */
> -               while (div > SUN6I_AR100_DIV_MAX) {
> -                       shift++;
> -                       div >>= 1;
> -                       if (shift > SUN6I_AR100_SHIFT_MAX)
> -                               break;
> -               }
> -
> -               /*
> -                * If the shift value (or power of 2 divider) is bigger
> -                * than what the HW actually support, skip this parent.
> -                */
> -               if (shift > SUN6I_AR100_SHIFT_MAX)
> -                       continue;
> -
> -               tmp_rate = (parent_rate >> shift) / div;
> -               if (!req->best_parent_hw || tmp_rate > best_rate) {
> -                       req->best_parent_hw = parent;
> -                       req->best_parent_rate = parent_rate;
> -                       best_rate = tmp_rate;
> -               }
> -       }
> -
> -       if (best_rate < 0)
> -               return best_rate;
> -
> -       req->rate = best_rate;
> -
> -       return 0;
> -}
> -
> -static int ar100_set_parent(struct clk_hw *hw, u8 index)
> -{
> -       struct ar100_clk *clk = to_ar100_clk(hw);
> -       u32 val = readl(clk->reg);
> -
> -       if (index >= SUN6I_AR100_MAX_PARENTS)
> -               return -EINVAL;
> -
> -       val &= ~(SUN6I_AR100_MUX_MASK << SUN6I_AR100_MUX_SHIFT);
> -       val |= (index << SUN6I_AR100_MUX_SHIFT);
> -       writel(val, clk->reg);
> -
> -       return 0;
> -}
> +#include "clk-factors.h"
>
> -static u8 ar100_get_parent(struct clk_hw *hw)
> -{
> -       struct ar100_clk *clk = to_ar100_clk(hw);
> -       return (readl(clk->reg) >> SUN6I_AR100_MUX_SHIFT) &
> -              SUN6I_AR100_MUX_MASK;
> -}
> -
> -static int ar100_set_rate(struct clk_hw *hw, unsigned long rate,
> -                         unsigned long parent_rate)
> +/**
> + * sun6i_get_ar100_factors - Calculates factors p, m for AR100
> + *
> + * AR100 rate is calculated as follows
> + * rate = (parent_rate >> p) / (m + 1);
> + */
> +static void sun6i_get_ar100_factors(struct factors_request *req)
>  {
> -       unsigned long div = parent_rate / rate;
> -       struct ar100_clk *clk = to_ar100_clk(hw);
> -       u32 val = readl(clk->reg);
> +       unsigned long div;
>         int shift;
>
> -       if (parent_rate % rate)
> -               return -EINVAL;
> +       /* clock only divides */
> +       if (req->rate > req->parent_rate)
> +               req->rate = req->parent_rate;
>
> -       shift = ffs(div) - 1;
> -       if (shift > SUN6I_AR100_SHIFT_MAX)
> -               shift = SUN6I_AR100_SHIFT_MAX;
> +       div = DIV_ROUND_UP(req->parent_rate, req->rate);
>
> -       div >>= shift;
> +       if (div < 32)
> +               shift = 0;
> +       else if (div >> 1 < 32)
> +               shift = 1;
> +       else if (div >> 2 < 32)
> +               shift = 2;
> +       else
> +               shift = 3;
>
> -       if (div > SUN6I_AR100_DIV_MAX)
> -               return -EINVAL;
> +       div >>= shift;
>
> -       val &= ~((SUN6I_AR100_SHIFT_MASK << SUN6I_AR100_SHIFT_SHIFT) |
> -                (SUN6I_AR100_DIV_MASK << SUN6I_AR100_DIV_SHIFT));
> -       val |= (shift << SUN6I_AR100_SHIFT_SHIFT) |
> -              (div << SUN6I_AR100_DIV_SHIFT);
> -       writel(val, clk->reg);
> +       if (div > 32)
> +               div = 32;
>
> -       return 0;
> +       req->rate = (req->parent_rate >> shift) / div;
> +       req->m = div - 1;
> +       req->p = shift;
>  }
>
> -static struct clk_ops ar100_ops = {
> -       .recalc_rate = ar100_recalc_rate,
> -       .determine_rate = ar100_determine_rate,
> -       .set_parent = ar100_set_parent,
> -       .get_parent = ar100_get_parent,
> -       .set_rate = ar100_set_rate,
> +static const struct clk_factors_config sun6i_ar100_config = {
> +       .mwidth = 5,
> +       .mshift = 8,
> +       .pwidth = 2,
> +       .pshift = 4,
>  };
>
> +static const struct factors_data sun6i_ar100_data __initconst = {
> +       .mux = 16,
> +       .muxmask = GENMASK(1, 0),
> +       .table = &sun6i_ar100_config,
> +       .getter = sun6i_get_ar100_factors,
> +};
> +
> +static DEFINE_SPINLOCK(sun6i_ar100_lock);
> +
>  static int sun6i_a31_ar100_clk_probe(struct platform_device *pdev)
>  {
> -       const char *parents[SUN6I_AR100_MAX_PARENTS];
>         struct device_node *np = pdev->dev.of_node;
> -       const char *clk_name = np->name;
> -       struct clk_init_data init;
> -       struct ar100_clk *ar100;
>         struct resource *r;
> +       void __iomem *reg;
>         struct clk *clk;
> -       int nparents;
> -
> -       ar100 = devm_kzalloc(&pdev->dev, sizeof(*ar100), GFP_KERNEL);
> -       if (!ar100)
> -               return -ENOMEM;
>
>         r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -       ar100->reg = devm_ioremap_resource(&pdev->dev, r);
> -       if (IS_ERR(ar100->reg))
> -               return PTR_ERR(ar100->reg);
> +       reg = devm_ioremap_resource(&pdev->dev, r);
> +       if (IS_ERR(reg))
> +               return PTR_ERR(reg);
>
> -       nparents = of_clk_get_parent_count(np);
> -       if (nparents > SUN6I_AR100_MAX_PARENTS)
> -               nparents = SUN6I_AR100_MAX_PARENTS;
> -
> -       of_clk_parent_fill(np, parents, nparents);
> +       clk = sunxi_factors_register(np, &sun6i_ar100_data, &sun6i_ar100_lock,
> +                                    reg);
> +       if (!clk)
> +               return -ENOMEM;
>
> -       of_property_read_string(np, "clock-output-names", &clk_name);
> +       platform_set_drvdata(pdev, clk);
>
> -       init.name = clk_name;
> -       init.ops = &ar100_ops;
> -       init.parent_names = parents;
> -       init.num_parents = nparents;
> -       init.flags = 0;
> +       return 0;
> +}
>
> -       ar100->hw.init = &init;
> +static int sun6i_a31_ar100_clk_remove(struct platform_device *pdev)
> +{
> +       struct device_node *np = pdev->dev.of_node;
> +       struct clk *clk = platform_get_drvdata(pdev);
>
> -       clk = clk_register(&pdev->dev, &ar100->hw);
> -       if (IS_ERR(clk))
> -               return PTR_ERR(clk);
> +       sunxi_factors_unregister(np, clk);
>
> -       return of_clk_add_provider(np, of_clk_src_simple_get, clk);
> +       return 0;
>  }
>
>  static const struct of_device_id sun6i_a31_ar100_clk_dt_ids[] = {
> @@ -227,6 +113,7 @@ static struct platform_driver sun6i_a31_ar100_clk_driver = {
>                 .of_match_table = sun6i_a31_ar100_clk_dt_ids,
>         },
>         .probe = sun6i_a31_ar100_clk_probe,
> +       .remove = sun6i_a31_ar100_clk_remove,
>  };
>  module_platform_driver(sun6i_a31_ar100_clk_driver);
>
> --
> 2.7.0
>

Re: [PATCH RFC 10/11] clk: sunxi: rewrite sun6i-ar100 using factors clk

[Paul Gortmaker]

On Mon, Jan 25, 2016 at 8:15 AM, Chen-Yu Tsai <wens@csie.org> wrote:
> sun6i's AR100 clock is a classic factors clk case:
>
> AR100 = ((parent mux) >> p) / (m + 1)
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

This patch adds a ".remove" function to a driver that is controlled by
a bool Kconfig, and hence the remove code can't be called unless
one explicitly goes into sysfs and muddles with the unbind as root
(which normally has no sane use case for builtin, non-modular code).

Since I'm trying to cut back on the amount of dead code we have
in .remove functions for built in drivers, is there a valid use case
for this one here, or can I just stage a delete commit for it too?

Normally I've set the set the disallow-unbind flag when deleting
a .remove function to make it clear there is no sane use case
for wandering into sysfs and unhooking the builtin driver.

Thanks,
Paul.
--

> ---
>  drivers/clk/sunxi/clk-sun6i-ar100.c | 235 ++++++++++--------------------------
>  1 file changed, 61 insertions(+), 174 deletions(-)
>
> diff --git a/drivers/clk/sunxi/clk-sun6i-ar100.c b/drivers/clk/sunxi/clk-sun6i-ar100.c
> index 20887686bdbe..a7f5777834eb 100644
> --- a/drivers/clk/sunxi/clk-sun6i-ar100.c
> +++ b/drivers/clk/sunxi/clk-sun6i-ar100.c
> @@ -8,211 +8,97 @@
>   *
>   */
>
> +#include <linux/bitops.h>
>  #include <linux/clk-provider.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
> +#include <linux/spinlock.h>
>
> -#define SUN6I_AR100_MAX_PARENTS                4
> -#define SUN6I_AR100_SHIFT_MASK         0x3
> -#define SUN6I_AR100_SHIFT_MAX          SUN6I_AR100_SHIFT_MASK
> -#define SUN6I_AR100_SHIFT_SHIFT                4
> -#define SUN6I_AR100_DIV_MASK           0x1f
> -#define SUN6I_AR100_DIV_MAX            (SUN6I_AR100_DIV_MASK + 1)
> -#define SUN6I_AR100_DIV_SHIFT          8
> -#define SUN6I_AR100_MUX_MASK           0x3
> -#define SUN6I_AR100_MUX_SHIFT          16
> -
> -struct ar100_clk {
> -       struct clk_hw hw;
> -       void __iomem *reg;
> -};
> -
> -static inline struct ar100_clk *to_ar100_clk(struct clk_hw *hw)
> -{
> -       return container_of(hw, struct ar100_clk, hw);
> -}
> -
> -static unsigned long ar100_recalc_rate(struct clk_hw *hw,
> -                                      unsigned long parent_rate)
> -{
> -       struct ar100_clk *clk = to_ar100_clk(hw);
> -       u32 val = readl(clk->reg);
> -       int shift = (val >> SUN6I_AR100_SHIFT_SHIFT) & SUN6I_AR100_SHIFT_MASK;
> -       int div = (val >> SUN6I_AR100_DIV_SHIFT) & SUN6I_AR100_DIV_MASK;
> -
> -       return (parent_rate >> shift) / (div + 1);
> -}
> -
> -static int ar100_determine_rate(struct clk_hw *hw,
> -                               struct clk_rate_request *req)
> -{
> -       int nparents = clk_hw_get_num_parents(hw);
> -       long best_rate = -EINVAL;
> -       int i;
> -
> -       req->best_parent_hw = NULL;
> -
> -       for (i = 0; i < nparents; i++) {
> -               unsigned long parent_rate;
> -               unsigned long tmp_rate;
> -               struct clk_hw *parent;
> -               unsigned long div;
> -               int shift;
> -
> -               parent = clk_hw_get_parent_by_index(hw, i);
> -               parent_rate = clk_hw_get_rate(parent);
> -               div = DIV_ROUND_UP(parent_rate, req->rate);
> -
> -               /*
> -                * The AR100 clk contains 2 divisors:
> -                * - one power of 2 divisor
> -                * - one regular divisor
> -                *
> -                * First check if we can safely shift (or divide by a power
> -                * of 2) without losing precision on the requested rate.
> -                */
> -               shift = ffs(div) - 1;
> -               if (shift > SUN6I_AR100_SHIFT_MAX)
> -                       shift = SUN6I_AR100_SHIFT_MAX;
> -
> -               div >>= shift;
> -
> -               /*
> -                * Then if the divisor is still bigger than what the HW
> -                * actually supports, use a bigger shift (or power of 2
> -                * divider) value and accept to lose some precision.
> -                */
> -               while (div > SUN6I_AR100_DIV_MAX) {
> -                       shift++;
> -                       div >>= 1;
> -                       if (shift > SUN6I_AR100_SHIFT_MAX)
> -                               break;
> -               }
> -
> -               /*
> -                * If the shift value (or power of 2 divider) is bigger
> -                * than what the HW actually support, skip this parent.
> -                */
> -               if (shift > SUN6I_AR100_SHIFT_MAX)
> -                       continue;
> -
> -               tmp_rate = (parent_rate >> shift) / div;
> -               if (!req->best_parent_hw || tmp_rate > best_rate) {
> -                       req->best_parent_hw = parent;
> -                       req->best_parent_rate = parent_rate;
> -                       best_rate = tmp_rate;
> -               }
> -       }
> -
> -       if (best_rate < 0)
> -               return best_rate;
> -
> -       req->rate = best_rate;
> -
> -       return 0;
> -}
> -
> -static int ar100_set_parent(struct clk_hw *hw, u8 index)
> -{
> -       struct ar100_clk *clk = to_ar100_clk(hw);
> -       u32 val = readl(clk->reg);
> -
> -       if (index >= SUN6I_AR100_MAX_PARENTS)
> -               return -EINVAL;
> -
> -       val &= ~(SUN6I_AR100_MUX_MASK << SUN6I_AR100_MUX_SHIFT);
> -       val |= (index << SUN6I_AR100_MUX_SHIFT);
> -       writel(val, clk->reg);
> -
> -       return 0;
> -}
> +#include "clk-factors.h"
>
> -static u8 ar100_get_parent(struct clk_hw *hw)
> -{
> -       struct ar100_clk *clk = to_ar100_clk(hw);
> -       return (readl(clk->reg) >> SUN6I_AR100_MUX_SHIFT) &
> -              SUN6I_AR100_MUX_MASK;
> -}
> -
> -static int ar100_set_rate(struct clk_hw *hw, unsigned long rate,
> -                         unsigned long parent_rate)
> +/**
> + * sun6i_get_ar100_factors - Calculates factors p, m for AR100
> + *
> + * AR100 rate is calculated as follows
> + * rate = (parent_rate >> p) / (m + 1);
> + */
> +static void sun6i_get_ar100_factors(struct factors_request *req)
>  {
> -       unsigned long div = parent_rate / rate;
> -       struct ar100_clk *clk = to_ar100_clk(hw);
> -       u32 val = readl(clk->reg);
> +       unsigned long div;
>         int shift;
>
> -       if (parent_rate % rate)
> -               return -EINVAL;
> +       /* clock only divides */
> +       if (req->rate > req->parent_rate)
> +               req->rate = req->parent_rate;
>
> -       shift = ffs(div) - 1;
> -       if (shift > SUN6I_AR100_SHIFT_MAX)
> -               shift = SUN6I_AR100_SHIFT_MAX;
> +       div = DIV_ROUND_UP(req->parent_rate, req->rate);
>
> -       div >>= shift;
> +       if (div < 32)
> +               shift = 0;
> +       else if (div >> 1 < 32)
> +               shift = 1;
> +       else if (div >> 2 < 32)
> +               shift = 2;
> +       else
> +               shift = 3;
>
> -       if (div > SUN6I_AR100_DIV_MAX)
> -               return -EINVAL;
> +       div >>= shift;
>
> -       val &= ~((SUN6I_AR100_SHIFT_MASK << SUN6I_AR100_SHIFT_SHIFT) |
> -                (SUN6I_AR100_DIV_MASK << SUN6I_AR100_DIV_SHIFT));
> -       val |= (shift << SUN6I_AR100_SHIFT_SHIFT) |
> -              (div << SUN6I_AR100_DIV_SHIFT);
> -       writel(val, clk->reg);
> +       if (div > 32)
> +               div = 32;
>
> -       return 0;
> +       req->rate = (req->parent_rate >> shift) / div;
> +       req->m = div - 1;
> +       req->p = shift;
>  }
>
> -static struct clk_ops ar100_ops = {
> -       .recalc_rate = ar100_recalc_rate,
> -       .determine_rate = ar100_determine_rate,
> -       .set_parent = ar100_set_parent,
> -       .get_parent = ar100_get_parent,
> -       .set_rate = ar100_set_rate,
> +static const struct clk_factors_config sun6i_ar100_config = {
> +       .mwidth = 5,
> +       .mshift = 8,
> +       .pwidth = 2,
> +       .pshift = 4,
>  };
>
> +static const struct factors_data sun6i_ar100_data __initconst = {
> +       .mux = 16,
> +       .muxmask = GENMASK(1, 0),
> +       .table = &sun6i_ar100_config,
> +       .getter = sun6i_get_ar100_factors,
> +};
> +
> +static DEFINE_SPINLOCK(sun6i_ar100_lock);
> +
>  static int sun6i_a31_ar100_clk_probe(struct platform_device *pdev)
>  {
> -       const char *parents[SUN6I_AR100_MAX_PARENTS];
>         struct device_node *np = pdev->dev.of_node;
> -       const char *clk_name = np->name;
> -       struct clk_init_data init;
> -       struct ar100_clk *ar100;
>         struct resource *r;
> +       void __iomem *reg;
>         struct clk *clk;
> -       int nparents;
> -
> -       ar100 = devm_kzalloc(&pdev->dev, sizeof(*ar100), GFP_KERNEL);
> -       if (!ar100)
> -               return -ENOMEM;
>
>         r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -       ar100->reg = devm_ioremap_resource(&pdev->dev, r);
> -       if (IS_ERR(ar100->reg))
> -               return PTR_ERR(ar100->reg);
> +       reg = devm_ioremap_resource(&pdev->dev, r);
> +       if (IS_ERR(reg))
> +               return PTR_ERR(reg);
>
> -       nparents = of_clk_get_parent_count(np);
> -       if (nparents > SUN6I_AR100_MAX_PARENTS)
> -               nparents = SUN6I_AR100_MAX_PARENTS;
> -
> -       of_clk_parent_fill(np, parents, nparents);
> +       clk = sunxi_factors_register(np, &sun6i_ar100_data, &sun6i_ar100_lock,
> +                                    reg);
> +       if (!clk)
> +               return -ENOMEM;
>
> -       of_property_read_string(np, "clock-output-names", &clk_name);
> +       platform_set_drvdata(pdev, clk);
>
> -       init.name = clk_name;
> -       init.ops = &ar100_ops;
> -       init.parent_names = parents;
> -       init.num_parents = nparents;
> -       init.flags = 0;
> +       return 0;
> +}
>
> -       ar100->hw.init = &init;
> +static int sun6i_a31_ar100_clk_remove(struct platform_device *pdev)
> +{
> +       struct device_node *np = pdev->dev.of_node;
> +       struct clk *clk = platform_get_drvdata(pdev);
>
> -       clk = clk_register(&pdev->dev, &ar100->hw);
> -       if (IS_ERR(clk))
> -               return PTR_ERR(clk);
> +       sunxi_factors_unregister(np, clk);
>
> -       return of_clk_add_provider(np, of_clk_src_simple_get, clk);
> +       return 0;
>  }
>
>  static const struct of_device_id sun6i_a31_ar100_clk_dt_ids[] = {
> @@ -227,6 +113,7 @@ static struct platform_driver sun6i_a31_ar100_clk_driver = {
>                 .of_match_table = sun6i_a31_ar100_clk_dt_ids,
>         },
>         .probe = sun6i_a31_ar100_clk_probe,
> +       .remove = sun6i_a31_ar100_clk_remove,
>  };
>  module_platform_driver(sun6i_a31_ar100_clk_driver);
>
> --
> 2.7.0
>

Re: [PATCH RFC 10/11] clk: sunxi: rewrite sun6i-ar100 using factors clk

[Chen-Yu Tsai]

On Mon, Feb 1, 2016 at 12:59 AM, Paul Gortmaker
<paul.gortmaker@windriver.com> wrote:
> On Mon, Jan 25, 2016 at 8:15 AM, Chen-Yu Tsai <wens@csie.org> wrote:
>> sun6i's AR100 clock is a classic factors clk case:
>>
>> AR100 = ((parent mux) >> p) / (m + 1)
>>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>
> This patch adds a ".remove" function to a driver that is controlled by
> a bool Kconfig, and hence the remove code can't be called unless
> one explicitly goes into sysfs and muddles with the unbind as root
> (which normally has no sane use case for builtin, non-modular code).
>
> Since I'm trying to cut back on the amount of dead code we have
> in .remove functions for built in drivers, is there a valid use case
> for this one here, or can I just stage a delete commit for it too?

It's just there to balance the probe code. I thought it was missing.
But what you said makes sense. Go ahead.

> Normally I've set the set the disallow-unbind flag when deleting
> a .remove function to make it clear there is no sane use case
> for wandering into sysfs and unhooking the builtin driver.

Cool.

Thanks
ChenYu

>
> Thanks,
> Paul.
> --
>
>> ---
>>  drivers/clk/sunxi/clk-sun6i-ar100.c | 235 ++++++++++--------------------------
>>  1 file changed, 61 insertions(+), 174 deletions(-)
>>
>> diff --git a/drivers/clk/sunxi/clk-sun6i-ar100.c b/drivers/clk/sunxi/clk-sun6i-ar100.c
>> index 20887686bdbe..a7f5777834eb 100644
>> --- a/drivers/clk/sunxi/clk-sun6i-ar100.c
>> +++ b/drivers/clk/sunxi/clk-sun6i-ar100.c
>> @@ -8,211 +8,97 @@
>>   *
>>   */
>>
>> +#include <linux/bitops.h>
>>  #include <linux/clk-provider.h>
>>  #include <linux/module.h>
>>  #include <linux/of.h>
>>  #include <linux/platform_device.h>
>> +#include <linux/spinlock.h>
>>
>> -#define SUN6I_AR100_MAX_PARENTS                4
>> -#define SUN6I_AR100_SHIFT_MASK         0x3
>> -#define SUN6I_AR100_SHIFT_MAX          SUN6I_AR100_SHIFT_MASK
>> -#define SUN6I_AR100_SHIFT_SHIFT                4
>> -#define SUN6I_AR100_DIV_MASK           0x1f
>> -#define SUN6I_AR100_DIV_MAX            (SUN6I_AR100_DIV_MASK + 1)
>> -#define SUN6I_AR100_DIV_SHIFT          8
>> -#define SUN6I_AR100_MUX_MASK           0x3
>> -#define SUN6I_AR100_MUX_SHIFT          16
>> -
>> -struct ar100_clk {
>> -       struct clk_hw hw;
>> -       void __iomem *reg;
>> -};
>> -
>> -static inline struct ar100_clk *to_ar100_clk(struct clk_hw *hw)
>> -{
>> -       return container_of(hw, struct ar100_clk, hw);
>> -}
>> -
>> -static unsigned long ar100_recalc_rate(struct clk_hw *hw,
>> -                                      unsigned long parent_rate)
>> -{
>> -       struct ar100_clk *clk = to_ar100_clk(hw);
>> -       u32 val = readl(clk->reg);
>> -       int shift = (val >> SUN6I_AR100_SHIFT_SHIFT) & SUN6I_AR100_SHIFT_MASK;
>> -       int div = (val >> SUN6I_AR100_DIV_SHIFT) & SUN6I_AR100_DIV_MASK;
>> -
>> -       return (parent_rate >> shift) / (div + 1);
>> -}
>> -
>> -static int ar100_determine_rate(struct clk_hw *hw,
>> -                               struct clk_rate_request *req)
>> -{
>> -       int nparents = clk_hw_get_num_parents(hw);
>> -       long best_rate = -EINVAL;
>> -       int i;
>> -
>> -       req->best_parent_hw = NULL;
>> -
>> -       for (i = 0; i < nparents; i++) {
>> -               unsigned long parent_rate;
>> -               unsigned long tmp_rate;
>> -               struct clk_hw *parent;
>> -               unsigned long div;
>> -               int shift;
>> -
>> -               parent = clk_hw_get_parent_by_index(hw, i);
>> -               parent_rate = clk_hw_get_rate(parent);
>> -               div = DIV_ROUND_UP(parent_rate, req->rate);
>> -
>> -               /*
>> -                * The AR100 clk contains 2 divisors:
>> -                * - one power of 2 divisor
>> -                * - one regular divisor
>> -                *
>> -                * First check if we can safely shift (or divide by a power
>> -                * of 2) without losing precision on the requested rate.
>> -                */
>> -               shift = ffs(div) - 1;
>> -               if (shift > SUN6I_AR100_SHIFT_MAX)
>> -                       shift = SUN6I_AR100_SHIFT_MAX;
>> -
>> -               div >>= shift;
>> -
>> -               /*
>> -                * Then if the divisor is still bigger than what the HW
>> -                * actually supports, use a bigger shift (or power of 2
>> -                * divider) value and accept to lose some precision.
>> -                */
>> -               while (div > SUN6I_AR100_DIV_MAX) {
>> -                       shift++;
>> -                       div >>= 1;
>> -                       if (shift > SUN6I_AR100_SHIFT_MAX)
>> -                               break;
>> -               }
>> -
>> -               /*
>> -                * If the shift value (or power of 2 divider) is bigger
>> -                * than what the HW actually support, skip this parent.
>> -                */
>> -               if (shift > SUN6I_AR100_SHIFT_MAX)
>> -                       continue;
>> -
>> -               tmp_rate = (parent_rate >> shift) / div;
>> -               if (!req->best_parent_hw || tmp_rate > best_rate) {
>> -                       req->best_parent_hw = parent;
>> -                       req->best_parent_rate = parent_rate;
>> -                       best_rate = tmp_rate;
>> -               }
>> -       }
>> -
>> -       if (best_rate < 0)
>> -               return best_rate;
>> -
>> -       req->rate = best_rate;
>> -
>> -       return 0;
>> -}
>> -
>> -static int ar100_set_parent(struct clk_hw *hw, u8 index)
>> -{
>> -       struct ar100_clk *clk = to_ar100_clk(hw);
>> -       u32 val = readl(clk->reg);
>> -
>> -       if (index >= SUN6I_AR100_MAX_PARENTS)
>> -               return -EINVAL;
>> -
>> -       val &= ~(SUN6I_AR100_MUX_MASK << SUN6I_AR100_MUX_SHIFT);
>> -       val |= (index << SUN6I_AR100_MUX_SHIFT);
>> -       writel(val, clk->reg);
>> -
>> -       return 0;
>> -}
>> +#include "clk-factors.h"
>>
>> -static u8 ar100_get_parent(struct clk_hw *hw)
>> -{
>> -       struct ar100_clk *clk = to_ar100_clk(hw);
>> -       return (readl(clk->reg) >> SUN6I_AR100_MUX_SHIFT) &
>> -              SUN6I_AR100_MUX_MASK;
>> -}
>> -
>> -static int ar100_set_rate(struct clk_hw *hw, unsigned long rate,
>> -                         unsigned long parent_rate)
>> +/**
>> + * sun6i_get_ar100_factors - Calculates factors p, m for AR100
>> + *
>> + * AR100 rate is calculated as follows
>> + * rate = (parent_rate >> p) / (m + 1);
>> + */
>> +static void sun6i_get_ar100_factors(struct factors_request *req)
>>  {
>> -       unsigned long div = parent_rate / rate;
>> -       struct ar100_clk *clk = to_ar100_clk(hw);
>> -       u32 val = readl(clk->reg);
>> +       unsigned long div;
>>         int shift;
>>
>> -       if (parent_rate % rate)
>> -               return -EINVAL;
>> +       /* clock only divides */
>> +       if (req->rate > req->parent_rate)
>> +               req->rate = req->parent_rate;
>>
>> -       shift = ffs(div) - 1;
>> -       if (shift > SUN6I_AR100_SHIFT_MAX)
>> -               shift = SUN6I_AR100_SHIFT_MAX;
>> +       div = DIV_ROUND_UP(req->parent_rate, req->rate);
>>
>> -       div >>= shift;
>> +       if (div < 32)
>> +               shift = 0;
>> +       else if (div >> 1 < 32)
>> +               shift = 1;
>> +       else if (div >> 2 < 32)
>> +               shift = 2;
>> +       else
>> +               shift = 3;
>>
>> -       if (div > SUN6I_AR100_DIV_MAX)
>> -               return -EINVAL;
>> +       div >>= shift;
>>
>> -       val &= ~((SUN6I_AR100_SHIFT_MASK << SUN6I_AR100_SHIFT_SHIFT) |
>> -                (SUN6I_AR100_DIV_MASK << SUN6I_AR100_DIV_SHIFT));
>> -       val |= (shift << SUN6I_AR100_SHIFT_SHIFT) |
>> -              (div << SUN6I_AR100_DIV_SHIFT);
>> -       writel(val, clk->reg);
>> +       if (div > 32)
>> +               div = 32;
>>
>> -       return 0;
>> +       req->rate = (req->parent_rate >> shift) / div;
>> +       req->m = div - 1;
>> +       req->p = shift;
>>  }
>>
>> -static struct clk_ops ar100_ops = {
>> -       .recalc_rate = ar100_recalc_rate,
>> -       .determine_rate = ar100_determine_rate,
>> -       .set_parent = ar100_set_parent,
>> -       .get_parent = ar100_get_parent,
>> -       .set_rate = ar100_set_rate,
>> +static const struct clk_factors_config sun6i_ar100_config = {
>> +       .mwidth = 5,
>> +       .mshift = 8,
>> +       .pwidth = 2,
>> +       .pshift = 4,
>>  };
>>
>> +static const struct factors_data sun6i_ar100_data __initconst = {
>> +       .mux = 16,
>> +       .muxmask = GENMASK(1, 0),
>> +       .table = &sun6i_ar100_config,
>> +       .getter = sun6i_get_ar100_factors,
>> +};
>> +
>> +static DEFINE_SPINLOCK(sun6i_ar100_lock);
>> +
>>  static int sun6i_a31_ar100_clk_probe(struct platform_device *pdev)
>>  {
>> -       const char *parents[SUN6I_AR100_MAX_PARENTS];
>>         struct device_node *np = pdev->dev.of_node;
>> -       const char *clk_name = np->name;
>> -       struct clk_init_data init;
>> -       struct ar100_clk *ar100;
>>         struct resource *r;
>> +       void __iomem *reg;
>>         struct clk *clk;
>> -       int nparents;
>> -
>> -       ar100 = devm_kzalloc(&pdev->dev, sizeof(*ar100), GFP_KERNEL);
>> -       if (!ar100)
>> -               return -ENOMEM;
>>
>>         r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> -       ar100->reg = devm_ioremap_resource(&pdev->dev, r);
>> -       if (IS_ERR(ar100->reg))
>> -               return PTR_ERR(ar100->reg);
>> +       reg = devm_ioremap_resource(&pdev->dev, r);
>> +       if (IS_ERR(reg))
>> +               return PTR_ERR(reg);
>>
>> -       nparents = of_clk_get_parent_count(np);
>> -       if (nparents > SUN6I_AR100_MAX_PARENTS)
>> -               nparents = SUN6I_AR100_MAX_PARENTS;
>> -
>> -       of_clk_parent_fill(np, parents, nparents);
>> +       clk = sunxi_factors_register(np, &sun6i_ar100_data, &sun6i_ar100_lock,
>> +                                    reg);
>> +       if (!clk)
>> +               return -ENOMEM;
>>
>> -       of_property_read_string(np, "clock-output-names", &clk_name);
>> +       platform_set_drvdata(pdev, clk);
>>
>> -       init.name = clk_name;
>> -       init.ops = &ar100_ops;
>> -       init.parent_names = parents;
>> -       init.num_parents = nparents;
>> -       init.flags = 0;
>> +       return 0;
>> +}
>>
>> -       ar100->hw.init = &init;
>> +static int sun6i_a31_ar100_clk_remove(struct platform_device *pdev)
>> +{
>> +       struct device_node *np = pdev->dev.of_node;
>> +       struct clk *clk = platform_get_drvdata(pdev);
>>
>> -       clk = clk_register(&pdev->dev, &ar100->hw);
>> -       if (IS_ERR(clk))
>> -               return PTR_ERR(clk);
>> +       sunxi_factors_unregister(np, clk);
>>
>> -       return of_clk_add_provider(np, of_clk_src_simple_get, clk);
>> +       return 0;
>>  }
>>
>>  static const struct of_device_id sun6i_a31_ar100_clk_dt_ids[] = {
>> @@ -227,6 +113,7 @@ static struct platform_driver sun6i_a31_ar100_clk_driver = {
>>                 .of_match_table = sun6i_a31_ar100_clk_dt_ids,
>>         },
>>         .probe = sun6i_a31_ar100_clk_probe,
>> +       .remove = sun6i_a31_ar100_clk_remove,
>>  };
>>  module_platform_driver(sun6i_a31_ar100_clk_driver);
>>
>> --
>> 2.7.0
>>

[PATCH RFC 10/11] clk: sunxi: rewrite sun6i-ar100 using factors clk

[Chen-Yu Tsai]

On Mon, Feb 1, 2016 at 12:59 AM, Paul Gortmaker
<paul.gortmaker@windriver.com> wrote:
> On Mon, Jan 25, 2016 at 8:15 AM, Chen-Yu Tsai <wens@csie.org> wrote:
>> sun6i's AR100 clock is a classic factors clk case:
>>
>> AR100 = ((parent mux) >> p) / (m + 1)
>>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>
> This patch adds a ".remove" function to a driver that is controlled by
> a bool Kconfig, and hence the remove code can't be called unless
> one explicitly goes into sysfs and muddles with the unbind as root
> (which normally has no sane use case for builtin, non-modular code).
>
> Since I'm trying to cut back on the amount of dead code we have
> in .remove functions for built in drivers, is there a valid use case
> for this one here, or can I just stage a delete commit for it too?

It's just there to balance the probe code. I thought it was missing.
But what you said makes sense. Go ahead.

> Normally I've set the set the disallow-unbind flag when deleting
> a .remove function to make it clear there is no sane use case
> for wandering into sysfs and unhooking the builtin driver.

Cool.

Thanks
ChenYu

>
> Thanks,
> Paul.
> --
>
>> ---
>>  drivers/clk/sunxi/clk-sun6i-ar100.c | 235 ++++++++++--------------------------
>>  1 file changed, 61 insertions(+), 174 deletions(-)
>>
>> diff --git a/drivers/clk/sunxi/clk-sun6i-ar100.c b/drivers/clk/sunxi/clk-sun6i-ar100.c
>> index 20887686bdbe..a7f5777834eb 100644
>> --- a/drivers/clk/sunxi/clk-sun6i-ar100.c
>> +++ b/drivers/clk/sunxi/clk-sun6i-ar100.c
>> @@ -8,211 +8,97 @@
>>   *
>>   */
>>
>> +#include <linux/bitops.h>
>>  #include <linux/clk-provider.h>
>>  #include <linux/module.h>
>>  #include <linux/of.h>
>>  #include <linux/platform_device.h>
>> +#include <linux/spinlock.h>
>>
>> -#define SUN6I_AR100_MAX_PARENTS                4
>> -#define SUN6I_AR100_SHIFT_MASK         0x3
>> -#define SUN6I_AR100_SHIFT_MAX          SUN6I_AR100_SHIFT_MASK
>> -#define SUN6I_AR100_SHIFT_SHIFT                4
>> -#define SUN6I_AR100_DIV_MASK           0x1f
>> -#define SUN6I_AR100_DIV_MAX            (SUN6I_AR100_DIV_MASK + 1)
>> -#define SUN6I_AR100_DIV_SHIFT          8
>> -#define SUN6I_AR100_MUX_MASK           0x3
>> -#define SUN6I_AR100_MUX_SHIFT          16
>> -
>> -struct ar100_clk {
>> -       struct clk_hw hw;
>> -       void __iomem *reg;
>> -};
>> -
>> -static inline struct ar100_clk *to_ar100_clk(struct clk_hw *hw)
>> -{
>> -       return container_of(hw, struct ar100_clk, hw);
>> -}
>> -
>> -static unsigned long ar100_recalc_rate(struct clk_hw *hw,
>> -                                      unsigned long parent_rate)
>> -{
>> -       struct ar100_clk *clk = to_ar100_clk(hw);
>> -       u32 val = readl(clk->reg);
>> -       int shift = (val >> SUN6I_AR100_SHIFT_SHIFT) & SUN6I_AR100_SHIFT_MASK;
>> -       int div = (val >> SUN6I_AR100_DIV_SHIFT) & SUN6I_AR100_DIV_MASK;
>> -
>> -       return (parent_rate >> shift) / (div + 1);
>> -}
>> -
>> -static int ar100_determine_rate(struct clk_hw *hw,
>> -                               struct clk_rate_request *req)
>> -{
>> -       int nparents = clk_hw_get_num_parents(hw);
>> -       long best_rate = -EINVAL;
>> -       int i;
>> -
>> -       req->best_parent_hw = NULL;
>> -
>> -       for (i = 0; i < nparents; i++) {
>> -               unsigned long parent_rate;
>> -               unsigned long tmp_rate;
>> -               struct clk_hw *parent;
>> -               unsigned long div;
>> -               int shift;
>> -
>> -               parent = clk_hw_get_parent_by_index(hw, i);
>> -               parent_rate = clk_hw_get_rate(parent);
>> -               div = DIV_ROUND_UP(parent_rate, req->rate);
>> -
>> -               /*
>> -                * The AR100 clk contains 2 divisors:
>> -                * - one power of 2 divisor
>> -                * - one regular divisor
>> -                *
>> -                * First check if we can safely shift (or divide by a power
>> -                * of 2) without losing precision on the requested rate.
>> -                */
>> -               shift = ffs(div) - 1;
>> -               if (shift > SUN6I_AR100_SHIFT_MAX)
>> -                       shift = SUN6I_AR100_SHIFT_MAX;
>> -
>> -               div >>= shift;
>> -
>> -               /*
>> -                * Then if the divisor is still bigger than what the HW
>> -                * actually supports, use a bigger shift (or power of 2
>> -                * divider) value and accept to lose some precision.
>> -                */
>> -               while (div > SUN6I_AR100_DIV_MAX) {
>> -                       shift++;
>> -                       div >>= 1;
>> -                       if (shift > SUN6I_AR100_SHIFT_MAX)
>> -                               break;
>> -               }
>> -
>> -               /*
>> -                * If the shift value (or power of 2 divider) is bigger
>> -                * than what the HW actually support, skip this parent.
>> -                */
>> -               if (shift > SUN6I_AR100_SHIFT_MAX)
>> -                       continue;
>> -
>> -               tmp_rate = (parent_rate >> shift) / div;
>> -               if (!req->best_parent_hw || tmp_rate > best_rate) {
>> -                       req->best_parent_hw = parent;
>> -                       req->best_parent_rate = parent_rate;
>> -                       best_rate = tmp_rate;
>> -               }
>> -       }
>> -
>> -       if (best_rate < 0)
>> -               return best_rate;
>> -
>> -       req->rate = best_rate;
>> -
>> -       return 0;
>> -}
>> -
>> -static int ar100_set_parent(struct clk_hw *hw, u8 index)
>> -{
>> -       struct ar100_clk *clk = to_ar100_clk(hw);
>> -       u32 val = readl(clk->reg);
>> -
>> -       if (index >= SUN6I_AR100_MAX_PARENTS)
>> -               return -EINVAL;
>> -
>> -       val &= ~(SUN6I_AR100_MUX_MASK << SUN6I_AR100_MUX_SHIFT);
>> -       val |= (index << SUN6I_AR100_MUX_SHIFT);
>> -       writel(val, clk->reg);
>> -
>> -       return 0;
>> -}
>> +#include "clk-factors.h"
>>
>> -static u8 ar100_get_parent(struct clk_hw *hw)
>> -{
>> -       struct ar100_clk *clk = to_ar100_clk(hw);
>> -       return (readl(clk->reg) >> SUN6I_AR100_MUX_SHIFT) &
>> -              SUN6I_AR100_MUX_MASK;
>> -}
>> -
>> -static int ar100_set_rate(struct clk_hw *hw, unsigned long rate,
>> -                         unsigned long parent_rate)
>> +/**
>> + * sun6i_get_ar100_factors - Calculates factors p, m for AR100
>> + *
>> + * AR100 rate is calculated as follows
>> + * rate = (parent_rate >> p) / (m + 1);
>> + */
>> +static void sun6i_get_ar100_factors(struct factors_request *req)
>>  {
>> -       unsigned long div = parent_rate / rate;
>> -       struct ar100_clk *clk = to_ar100_clk(hw);
>> -       u32 val = readl(clk->reg);
>> +       unsigned long div;
>>         int shift;
>>
>> -       if (parent_rate % rate)
>> -               return -EINVAL;
>> +       /* clock only divides */
>> +       if (req->rate > req->parent_rate)
>> +               req->rate = req->parent_rate;
>>
>> -       shift = ffs(div) - 1;
>> -       if (shift > SUN6I_AR100_SHIFT_MAX)
>> -               shift = SUN6I_AR100_SHIFT_MAX;
>> +       div = DIV_ROUND_UP(req->parent_rate, req->rate);
>>
>> -       div >>= shift;
>> +       if (div < 32)
>> +               shift = 0;
>> +       else if (div >> 1 < 32)
>> +               shift = 1;
>> +       else if (div >> 2 < 32)
>> +               shift = 2;
>> +       else
>> +               shift = 3;
>>
>> -       if (div > SUN6I_AR100_DIV_MAX)
>> -               return -EINVAL;
>> +       div >>= shift;
>>
>> -       val &= ~((SUN6I_AR100_SHIFT_MASK << SUN6I_AR100_SHIFT_SHIFT) |
>> -                (SUN6I_AR100_DIV_MASK << SUN6I_AR100_DIV_SHIFT));
>> -       val |= (shift << SUN6I_AR100_SHIFT_SHIFT) |
>> -              (div << SUN6I_AR100_DIV_SHIFT);
>> -       writel(val, clk->reg);
>> +       if (div > 32)
>> +               div = 32;
>>
>> -       return 0;
>> +       req->rate = (req->parent_rate >> shift) / div;
>> +       req->m = div - 1;
>> +       req->p = shift;
>>  }
>>
>> -static struct clk_ops ar100_ops = {
>> -       .recalc_rate = ar100_recalc_rate,
>> -       .determine_rate = ar100_determine_rate,
>> -       .set_parent = ar100_set_parent,
>> -       .get_parent = ar100_get_parent,
>> -       .set_rate = ar100_set_rate,
>> +static const struct clk_factors_config sun6i_ar100_config = {
>> +       .mwidth = 5,
>> +       .mshift = 8,
>> +       .pwidth = 2,
>> +       .pshift = 4,
>>  };
>>
>> +static const struct factors_data sun6i_ar100_data __initconst = {
>> +       .mux = 16,
>> +       .muxmask = GENMASK(1, 0),
>> +       .table = &sun6i_ar100_config,
>> +       .getter = sun6i_get_ar100_factors,
>> +};
>> +
>> +static DEFINE_SPINLOCK(sun6i_ar100_lock);
>> +
>>  static int sun6i_a31_ar100_clk_probe(struct platform_device *pdev)
>>  {
>> -       const char *parents[SUN6I_AR100_MAX_PARENTS];
>>         struct device_node *np = pdev->dev.of_node;
>> -       const char *clk_name = np->name;
>> -       struct clk_init_data init;
>> -       struct ar100_clk *ar100;
>>         struct resource *r;
>> +       void __iomem *reg;
>>         struct clk *clk;
>> -       int nparents;
>> -
>> -       ar100 = devm_kzalloc(&pdev->dev, sizeof(*ar100), GFP_KERNEL);
>> -       if (!ar100)
>> -               return -ENOMEM;
>>
>>         r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> -       ar100->reg = devm_ioremap_resource(&pdev->dev, r);
>> -       if (IS_ERR(ar100->reg))
>> -               return PTR_ERR(ar100->reg);
>> +       reg = devm_ioremap_resource(&pdev->dev, r);
>> +       if (IS_ERR(reg))
>> +               return PTR_ERR(reg);
>>
>> -       nparents = of_clk_get_parent_count(np);
>> -       if (nparents > SUN6I_AR100_MAX_PARENTS)
>> -               nparents = SUN6I_AR100_MAX_PARENTS;
>> -
>> -       of_clk_parent_fill(np, parents, nparents);
>> +       clk = sunxi_factors_register(np, &sun6i_ar100_data, &sun6i_ar100_lock,
>> +                                    reg);
>> +       if (!clk)
>> +               return -ENOMEM;
>>
>> -       of_property_read_string(np, "clock-output-names", &clk_name);
>> +       platform_set_drvdata(pdev, clk);
>>
>> -       init.name = clk_name;
>> -       init.ops = &ar100_ops;
>> -       init.parent_names = parents;
>> -       init.num_parents = nparents;
>> -       init.flags = 0;
>> +       return 0;
>> +}
>>
>> -       ar100->hw.init = &init;
>> +static int sun6i_a31_ar100_clk_remove(struct platform_device *pdev)
>> +{
>> +       struct device_node *np = pdev->dev.of_node;
>> +       struct clk *clk = platform_get_drvdata(pdev);
>>
>> -       clk = clk_register(&pdev->dev, &ar100->hw);
>> -       if (IS_ERR(clk))
>> -               return PTR_ERR(clk);
>> +       sunxi_factors_unregister(np, clk);
>>
>> -       return of_clk_add_provider(np, of_clk_src_simple_get, clk);
>> +       return 0;
>>  }
>>
>>  static const struct of_device_id sun6i_a31_ar100_clk_dt_ids[] = {
>> @@ -227,6 +113,7 @@ static struct platform_driver sun6i_a31_ar100_clk_driver = {
>>                 .of_match_table = sun6i_a31_ar100_clk_dt_ids,
>>         },
>>         .probe = sun6i_a31_ar100_clk_probe,
>> +       .remove = sun6i_a31_ar100_clk_remove,
>>  };
>>  module_platform_driver(sun6i_a31_ar100_clk_driver);
>>
>> --
>> 2.7.0
>>

[PATCH] clk: sunxi: don't mark sun6i_ar100_data __initconst

[Arnd Bergmann]

The clk-sun6i-ar100 clk driver is a platform driver that may use
deferred probing, so its probe function must not access
__init symbols. Kbuild warns about this:

WARNING: drivers/clk/sunxi/built-in.o(.text+0x15f0): Section mismatch in reference from the function sun6i_a31_ar100_clk_probe() to the (unknown reference) .init.rodata:(unknown)
The function sun6i_a31_ar100_clk_probe() references
the (unknown reference) __initconst (unknown).
This is often because sun6i_a31_ar100_clk_probe lacks a __initconst
annotation or the annotation of (unknown) is wrong.

Removing the __initconst annotation avoids the warning and makes
deferred probing work.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 3ca2377b6fed ("clk: sunxi: rewrite sun6i-ar100 using factors clk")
---
 drivers/clk/sunxi/clk-sun6i-ar100.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/sunxi/clk-sun6i-ar100.c b/drivers/clk/sunxi/clk-sun6i-ar100.c
index a7f5777834eb..84a187e55360 100644
--- a/drivers/clk/sunxi/clk-sun6i-ar100.c
+++ b/drivers/clk/sunxi/clk-sun6i-ar100.c
@@ -60,7 +60,7 @@ static const struct clk_factors_config sun6i_ar100_config = {
 	.pshift = 4,
 };
 
-static const struct factors_data sun6i_ar100_data __initconst = {
+static const struct factors_data sun6i_ar100_data = {
 	.mux = 16,
 	.muxmask = GENMASK(1, 0),
 	.table = &sun6i_ar100_config,
-- 
2.7.0

[PATCH] clk: sunxi: don't mark sun6i_ar100_data __initconst

[Arnd Bergmann]

The clk-sun6i-ar100 clk driver is a platform driver that may use
deferred probing, so its probe function must not access
__init symbols. Kbuild warns about this:

WARNING: drivers/clk/sunxi/built-in.o(.text+0x15f0): Section mismatch in reference from the function sun6i_a31_ar100_clk_probe() to the (unknown reference) .init.rodata:(unknown)
The function sun6i_a31_ar100_clk_probe() references
the (unknown reference) __initconst (unknown).
This is often because sun6i_a31_ar100_clk_probe lacks a __initconst
annotation or the annotation of (unknown) is wrong.

Removing the __initconst annotation avoids the warning and makes
deferred probing work.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 3ca2377b6fed ("clk: sunxi: rewrite sun6i-ar100 using factors clk")
---
 drivers/clk/sunxi/clk-sun6i-ar100.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/sunxi/clk-sun6i-ar100.c b/drivers/clk/sunxi/clk-sun6i-ar100.c
index a7f5777834eb..84a187e55360 100644
--- a/drivers/clk/sunxi/clk-sun6i-ar100.c
+++ b/drivers/clk/sunxi/clk-sun6i-ar100.c
@@ -60,7 +60,7 @@ static const struct clk_factors_config sun6i_ar100_config = {
 	.pshift = 4,
 };
 
-static const struct factors_data sun6i_ar100_data __initconst = {
+static const struct factors_data sun6i_ar100_data = {
 	.mux = 16,
 	.muxmask = GENMASK(1, 0),
 	.table = &sun6i_ar100_config,
-- 
2.7.0

Re: [PATCH] clk: sunxi: don't mark sun6i_ar100_data __initconst

[Maxime Ripard]

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

On Tue, Feb 02, 2016 at 04:55:30PM +0100, Arnd Bergmann wrote:
> The clk-sun6i-ar100 clk driver is a platform driver that may use
> deferred probing, so its probe function must not access
> __init symbols. Kbuild warns about this:
> 
> WARNING: drivers/clk/sunxi/built-in.o(.text+0x15f0): Section mismatch in reference from the function sun6i_a31_ar100_clk_probe() to the (unknown reference) .init.rodata:(unknown)
> The function sun6i_a31_ar100_clk_probe() references
> the (unknown reference) __initconst (unknown).
> This is often because sun6i_a31_ar100_clk_probe lacks a __initconst
> annotation or the annotation of (unknown) is wrong.
> 
> Removing the __initconst annotation avoids the warning and makes
> deferred probing work.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 3ca2377b6fed ("clk: sunxi: rewrite sun6i-ar100 using factors clk")

Applied, thanks!
Maxime

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH] clk: sunxi: don't mark sun6i_ar100_data __initconst

[Maxime Ripard]

On Tue, Feb 02, 2016 at 04:55:30PM +0100, Arnd Bergmann wrote:
> The clk-sun6i-ar100 clk driver is a platform driver that may use
> deferred probing, so its probe function must not access
> __init symbols. Kbuild warns about this:
> 
> WARNING: drivers/clk/sunxi/built-in.o(.text+0x15f0): Section mismatch in reference from the function sun6i_a31_ar100_clk_probe() to the (unknown reference) .init.rodata:(unknown)
> The function sun6i_a31_ar100_clk_probe() references
> the (unknown reference) __initconst (unknown).
> This is often because sun6i_a31_ar100_clk_probe lacks a __initconst
> annotation or the annotation of (unknown) is wrong.
> 
> Removing the __initconst annotation avoids the warning and makes
> deferred probing work.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 3ca2377b6fed ("clk: sunxi: rewrite sun6i-ar100 using factors clk")

Applied, 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/20160202/a3ac1a8e/attachment.sig>

[PATCH RFC 11/11] clk: sunxi: rewrite sun8i-a23-mbus-clk using the simpler composite clk

[Chen-Yu Tsai]

sun8i-a23-mbus-clk used sunxi's factors clk, which is nice for very
complicated clocks, but is not really needed here.

Convert sun8i-a23-mbus-clk to use clk_composite, as it is a gate + mux
+ divider. This makes the code easier to understand.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---

This patch I'm not so sure about. As stated in the cover letter,
this rewrite actually increases the LoC, but possibly making it
easier to understand.

---
 drivers/clk/sunxi/clk-sun8i-mbus.c | 120 +++++++++++++++++++++++--------------
 1 file changed, 75 insertions(+), 45 deletions(-)

diff --git a/drivers/clk/sunxi/clk-sun8i-mbus.c b/drivers/clk/sunxi/clk-sun8i-mbus.c
index 78683f02a37d..d3e385337b8a 100644
--- a/drivers/clk/sunxi/clk-sun8i-mbus.c
+++ b/drivers/clk/sunxi/clk-sun8i-mbus.c
@@ -15,68 +15,98 @@
  */
 
 #include <linux/clk.h>
+#include <linux/clkdev.h>
 #include <linux/clk-provider.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
 #include <linux/of_address.h>
 
-#include "clk-factors.h"
+#define SUN8I_MBUS_ENABLE	31
+#define SUN8I_MBUS_MUX_SHIFT	24
+#define SUN8I_MBUS_MUX_MASK	0x3
+#define SUN8I_MBUS_DIV_SHIFT	0
+#define SUN8I_MBUS_DIV_WIDTH	3
+#define SUN8I_MBUS_MAX_PARENTS	4
 
-/**
- * sun8i_a23_get_mbus_factors() - calculates m factor for MBUS clocks
- * MBUS rate is calculated as follows
- * rate = parent_rate / (m + 1);
- */
+static DEFINE_SPINLOCK(sun8i_a23_mbus_lock);
 
-static void sun8i_a23_get_mbus_factors(struct factors_request *req)
+static void __init sun8i_a23_mbus_setup(struct device_node *node)
 {
-	u8 div;
+	int num_parents = of_clk_get_parent_count(node);
+	const char *parents[num_parents];
+	const char *clk_name = node->name;
+	struct resource res;
+	struct clk_divider *div;
+	struct clk_gate *gate;
+	struct clk_mux *mux;
+	struct clk *clk;
+	void __iomem *reg;
+	int err;
 
-	/*
-	 * These clocks can only divide, so we will never be able to
-	 * achieve frequencies higher than the parent frequency
-	 */
-	if (req->rate > req->parent_rate)
-		req->rate = req->parent_rate;
+	reg = of_io_request_and_map(node, 0, of_node_full_name(node));
+	if (!reg) {
+		pr_err("Could not get registers for sun8i-mbus-clk\n");
+		return;
+	}
 
-	div = DIV_ROUND_UP(req->parent_rate, req->rate);
+	div = kzalloc(sizeof(*div), GFP_KERNEL);
+	if (!div)
+		goto err_unmap;
 
-	if (div > 8)
-		div = 8;
+	mux = kzalloc(sizeof(*mux), GFP_KERNEL);
+	if (!mux)
+		goto err_free_div;
 
-	req->rate = req->parent_rate / div;
-	req->m = div - 1;
-}
+	gate = kzalloc(sizeof(*gate), GFP_KERNEL);
+	if (!gate)
+		goto err_free_mux;
 
-static struct clk_factors_config sun8i_a23_mbus_config = {
-	.mshift = 0,
-	.mwidth = 3,
-};
+	of_property_read_string(node, "clock-output-names", &clk_name);
+	of_clk_parent_fill(node, parents, num_parents);
 
-static const struct factors_data sun8i_a23_mbus_data __initconst = {
-	.enable = 31,
-	.mux = 24,
-	.muxmask = BIT(1) | BIT(0),
-	.table = &sun8i_a23_mbus_config,
-	.getter = sun8i_a23_get_mbus_factors,
-};
+	gate->reg = reg;
+	gate->bit_idx = SUN8I_MBUS_ENABLE;
+	gate->lock = &sun8i_a23_mbus_lock;
 
-static DEFINE_SPINLOCK(sun8i_a23_mbus_lock);
+	div->reg = reg;
+	div->shift = SUN8I_MBUS_DIV_SHIFT;
+	div->width = SUN8I_MBUS_DIV_WIDTH;
+	div->lock = &sun8i_a23_mbus_lock;
 
-static void __init sun8i_a23_mbus_setup(struct device_node *node)
-{
-	struct clk *mbus;
-	void __iomem *reg;
+	mux->reg = reg;
+	mux->shift = SUN8I_MBUS_MUX_SHIFT;
+	mux->mask = SUN8I_MBUS_MUX_MASK;
+	mux->lock = &sun8i_a23_mbus_lock;
 
-	reg = of_iomap(node, 0);
-	if (!reg) {
-		pr_err("Could not get registers for a23-mbus-clk\n");
-		return;
-	}
+	clk = clk_register_composite(NULL, clk_name, parents, num_parents,
+				     &mux->hw, &clk_mux_ops,
+				     &div->hw, &clk_divider_ops,
+				     &gate->hw, &clk_gate_ops,
+				     0);
+	if (IS_ERR(clk))
+		goto err_free_gate;
 
-	mbus = sunxi_factors_register(node, &sun8i_a23_mbus_data,
-				      &sun8i_a23_mbus_lock, reg);
+	err = of_clk_add_provider(node, of_clk_src_simple_get, clk);
+	if (err)
+		goto err_unregister_clk;
 
 	/* The MBUS clocks needs to be always enabled */
-	__clk_get(mbus);
-	clk_prepare_enable(mbus);
+	__clk_get(clk);
+	clk_prepare_enable(clk);
+
+	return;
+
+err_unregister_clk:
+	clk_unregister_composite(clk);
+err_free_gate:
+	kfree(gate);
+err_free_mux:
+	kfree(mux);
+err_free_div:
+	kfree(div);
+err_unmap:
+	iounmap(reg);
+	of_address_to_resource(node, 0, &res);
+	release_mem_region(res.start, resource_size(&res));
 }
 CLK_OF_DECLARE(sun8i_a23_mbus, "allwinner,sun8i-a23-mbus-clk", sun8i_a23_mbus_setup);
-- 
2.7.0

[PATCH RFC 11/11] clk: sunxi: rewrite sun8i-a23-mbus-clk using the simpler composite clk

[Chen-Yu Tsai]

sun8i-a23-mbus-clk used sunxi's factors clk, which is nice for very
complicated clocks, but is not really needed here.

Convert sun8i-a23-mbus-clk to use clk_composite, as it is a gate + mux
+ divider. This makes the code easier to understand.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---

This patch I'm not so sure about. As stated in the cover letter,
this rewrite actually increases the LoC, but possibly making it
easier to understand.

---
 drivers/clk/sunxi/clk-sun8i-mbus.c | 120 +++++++++++++++++++++++--------------
 1 file changed, 75 insertions(+), 45 deletions(-)

diff --git a/drivers/clk/sunxi/clk-sun8i-mbus.c b/drivers/clk/sunxi/clk-sun8i-mbus.c
index 78683f02a37d..d3e385337b8a 100644
--- a/drivers/clk/sunxi/clk-sun8i-mbus.c
+++ b/drivers/clk/sunxi/clk-sun8i-mbus.c
@@ -15,68 +15,98 @@
  */
 
 #include <linux/clk.h>
+#include <linux/clkdev.h>
 #include <linux/clk-provider.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
 #include <linux/of_address.h>
 
-#include "clk-factors.h"
+#define SUN8I_MBUS_ENABLE	31
+#define SUN8I_MBUS_MUX_SHIFT	24
+#define SUN8I_MBUS_MUX_MASK	0x3
+#define SUN8I_MBUS_DIV_SHIFT	0
+#define SUN8I_MBUS_DIV_WIDTH	3
+#define SUN8I_MBUS_MAX_PARENTS	4
 
-/**
- * sun8i_a23_get_mbus_factors() - calculates m factor for MBUS clocks
- * MBUS rate is calculated as follows
- * rate = parent_rate / (m + 1);
- */
+static DEFINE_SPINLOCK(sun8i_a23_mbus_lock);
 
-static void sun8i_a23_get_mbus_factors(struct factors_request *req)
+static void __init sun8i_a23_mbus_setup(struct device_node *node)
 {
-	u8 div;
+	int num_parents = of_clk_get_parent_count(node);
+	const char *parents[num_parents];
+	const char *clk_name = node->name;
+	struct resource res;
+	struct clk_divider *div;
+	struct clk_gate *gate;
+	struct clk_mux *mux;
+	struct clk *clk;
+	void __iomem *reg;
+	int err;
 
-	/*
-	 * These clocks can only divide, so we will never be able to
-	 * achieve frequencies higher than the parent frequency
-	 */
-	if (req->rate > req->parent_rate)
-		req->rate = req->parent_rate;
+	reg = of_io_request_and_map(node, 0, of_node_full_name(node));
+	if (!reg) {
+		pr_err("Could not get registers for sun8i-mbus-clk\n");
+		return;
+	}
 
-	div = DIV_ROUND_UP(req->parent_rate, req->rate);
+	div = kzalloc(sizeof(*div), GFP_KERNEL);
+	if (!div)
+		goto err_unmap;
 
-	if (div > 8)
-		div = 8;
+	mux = kzalloc(sizeof(*mux), GFP_KERNEL);
+	if (!mux)
+		goto err_free_div;
 
-	req->rate = req->parent_rate / div;
-	req->m = div - 1;
-}
+	gate = kzalloc(sizeof(*gate), GFP_KERNEL);
+	if (!gate)
+		goto err_free_mux;
 
-static struct clk_factors_config sun8i_a23_mbus_config = {
-	.mshift = 0,
-	.mwidth = 3,
-};
+	of_property_read_string(node, "clock-output-names", &clk_name);
+	of_clk_parent_fill(node, parents, num_parents);
 
-static const struct factors_data sun8i_a23_mbus_data __initconst = {
-	.enable = 31,
-	.mux = 24,
-	.muxmask = BIT(1) | BIT(0),
-	.table = &sun8i_a23_mbus_config,
-	.getter = sun8i_a23_get_mbus_factors,
-};
+	gate->reg = reg;
+	gate->bit_idx = SUN8I_MBUS_ENABLE;
+	gate->lock = &sun8i_a23_mbus_lock;
 
-static DEFINE_SPINLOCK(sun8i_a23_mbus_lock);
+	div->reg = reg;
+	div->shift = SUN8I_MBUS_DIV_SHIFT;
+	div->width = SUN8I_MBUS_DIV_WIDTH;
+	div->lock = &sun8i_a23_mbus_lock;
 
-static void __init sun8i_a23_mbus_setup(struct device_node *node)
-{
-	struct clk *mbus;
-	void __iomem *reg;
+	mux->reg = reg;
+	mux->shift = SUN8I_MBUS_MUX_SHIFT;
+	mux->mask = SUN8I_MBUS_MUX_MASK;
+	mux->lock = &sun8i_a23_mbus_lock;
 
-	reg = of_iomap(node, 0);
-	if (!reg) {
-		pr_err("Could not get registers for a23-mbus-clk\n");
-		return;
-	}
+	clk = clk_register_composite(NULL, clk_name, parents, num_parents,
+				     &mux->hw, &clk_mux_ops,
+				     &div->hw, &clk_divider_ops,
+				     &gate->hw, &clk_gate_ops,
+				     0);
+	if (IS_ERR(clk))
+		goto err_free_gate;
 
-	mbus = sunxi_factors_register(node, &sun8i_a23_mbus_data,
-				      &sun8i_a23_mbus_lock, reg);
+	err = of_clk_add_provider(node, of_clk_src_simple_get, clk);
+	if (err)
+		goto err_unregister_clk;
 
 	/* The MBUS clocks needs to be always enabled */
-	__clk_get(mbus);
-	clk_prepare_enable(mbus);
+	__clk_get(clk);
+	clk_prepare_enable(clk);
+
+	return;
+
+err_unregister_clk:
+	clk_unregister_composite(clk);
+err_free_gate:
+	kfree(gate);
+err_free_mux:
+	kfree(mux);
+err_free_div:
+	kfree(div);
+err_unmap:
+	iounmap(reg);
+	of_address_to_resource(node, 0, &res);
+	release_mem_region(res.start, resource_size(&res));
 }
 CLK_OF_DECLARE(sun8i_a23_mbus, "allwinner,sun8i-a23-mbus-clk", sun8i_a23_mbus_setup);
-- 
2.7.0

Re: [PATCH RFC 11/11] clk: sunxi: rewrite sun8i-a23-mbus-clk using the simpler composite clk

[Maxime Ripard]

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

On Mon, Jan 25, 2016 at 09:15:47PM +0800, Chen-Yu Tsai wrote:
> sun8i-a23-mbus-clk used sunxi's factors clk, which is nice for very
> complicated clocks, but is not really needed here.
> 
> Convert sun8i-a23-mbus-clk to use clk_composite, as it is a gate + mux
> + divider. This makes the code easier to understand.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Applied, thanks!
Maxime

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH RFC 11/11] clk: sunxi: rewrite sun8i-a23-mbus-clk using the simpler composite clk

[Maxime Ripard]

On Mon, Jan 25, 2016 at 09:15:47PM +0800, Chen-Yu Tsai wrote:
> sun8i-a23-mbus-clk used sunxi's factors clk, which is nice for very
> complicated clocks, but is not really needed here.
> 
> Convert sun8i-a23-mbus-clk to use clk_composite, as it is a gate + mux
> + divider. This makes the code easier to understand.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Applied, 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/20160127/bbe0d487/attachment-0001.sig>

Re: [PATCH RFC 11/11] clk: sunxi: rewrite sun8i-a23-mbus-clk using the simpler composite clk

[Chen-Yu Tsai]

Hi,

On Thu, Jan 28, 2016 at 1:49 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Mon, Jan 25, 2016 at 09:15:47PM +0800, Chen-Yu Tsai wrote:
>> sun8i-a23-mbus-clk used sunxi's factors clk, which is nice for very
>> complicated clocks, but is not really needed here.
>>
>> Convert sun8i-a23-mbus-clk to use clk_composite, as it is a gate + mux
>> + divider. This makes the code easier to understand.
>>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>
> Applied, thanks!
> Maxime

Given your suggestion for extending and generalizing factors clk
clock rate calculations, maybe we just leave this one out and use
the new stuff later?

Thanks
ChenYu

[PATCH RFC 11/11] clk: sunxi: rewrite sun8i-a23-mbus-clk using the simpler composite clk

[Chen-Yu Tsai]

Hi,

On Thu, Jan 28, 2016 at 1:49 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Mon, Jan 25, 2016 at 09:15:47PM +0800, Chen-Yu Tsai wrote:
>> sun8i-a23-mbus-clk used sunxi's factors clk, which is nice for very
>> complicated clocks, but is not really needed here.
>>
>> Convert sun8i-a23-mbus-clk to use clk_composite, as it is a gate + mux
>> + divider. This makes the code easier to understand.
>>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>
> Applied, thanks!
> Maxime

Given your suggestion for extending and generalizing factors clk
clock rate calculations, maybe we just leave this one out and use
the new stuff later?

Thanks
ChenYu

Re: [PATCH RFC 11/11] clk: sunxi: rewrite sun8i-a23-mbus-clk using the simpler composite clk

[Maxime Ripard]

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

Hi,

On Thu, Jan 28, 2016 at 10:41:33AM +0800, Chen-Yu Tsai wrote:
> Hi,
> 
> On Thu, Jan 28, 2016 at 1:49 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Mon, Jan 25, 2016 at 09:15:47PM +0800, Chen-Yu Tsai wrote:
> >> sun8i-a23-mbus-clk used sunxi's factors clk, which is nice for very
> >> complicated clocks, but is not really needed here.
> >>
> >> Convert sun8i-a23-mbus-clk to use clk_composite, as it is a gate + mux
> >> + divider. This makes the code easier to understand.
> >>
> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> >
> > Applied, thanks!
> > Maxime
> 
> Given your suggestion for extending and generalizing factors clk
> clock rate calculations, maybe we just leave this one out and use
> the new stuff later?

My suggestion was only about the core of the clk-factors, the
"external" API would not change, so I'm guessing it's still relevant.

Thanks!
Maxime

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH RFC 11/11] clk: sunxi: rewrite sun8i-a23-mbus-clk using the simpler composite clk

[Maxime Ripard]

Hi,

On Thu, Jan 28, 2016 at 10:41:33AM +0800, Chen-Yu Tsai wrote:
> Hi,
> 
> On Thu, Jan 28, 2016 at 1:49 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Mon, Jan 25, 2016 at 09:15:47PM +0800, Chen-Yu Tsai wrote:
> >> sun8i-a23-mbus-clk used sunxi's factors clk, which is nice for very
> >> complicated clocks, but is not really needed here.
> >>
> >> Convert sun8i-a23-mbus-clk to use clk_composite, as it is a gate + mux
> >> + divider. This makes the code easier to understand.
> >>
> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> >
> > Applied, thanks!
> > Maxime
> 
> Given your suggestion for extending and generalizing factors clk
> clock rate calculations, maybe we just leave this one out and use
> the new stuff later?

My suggestion was only about the core of the clk-factors, the
"external" API would not change, so I'm guessing it's still relevant.

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/20160201/78d4db1a/attachment.sig>

Re: [PATCH RFC 00/11] clk: sunxi: factors clk clean up and refactor

[Maxime Ripard]

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

Hi Chen-Yu,

On Mon, Jan 25, 2016 at 09:15:36PM +0800, Chen-Yu Tsai wrote:
> Hi everyone,
> 
> This series cleans up and reworks parts of sunxi's factors clk. The goal
> is to support non-standard formulas for clock rate calculation, such as
> pre-dividers on some parents, or all power-of-2 dividers. One such clock
> is the AHB1 clock on A31/A31s.
> 
> Patch 1 is Maxime's patch adding an unregister function for composite
> clocks. Patches 3 and 4 use this, so it is included for completeness.
> 
> Patch 2 makes the config tables for factors clk constant. These contain
> the shift and width for the factors. They are used to manipulate the
> clk register values. There should be no reason to change them in-flight.
> 
> Patch 3 adds a proper error path for the factors clk register function(),
> so we don't leak memory when a call fails.
> 
> Patch 4 adds an unregister function for factors clks.
> 
> Patch 5 adds an error patch to sunxi_factors_clk_setup()
> 
> Patch 6 packs the parameters passed to get_factors callbacks in a struct.
> This makes it easier to extend factors clk without having to edit all
> the function definitions, and also makes the lines shorter.
> 
> Patch 7 makes factors clk support custom formulas for calculating clock
> rates. On the clock rounding/setting side, we only need to teach
> get_factors about different parent clocks. On the recalc side, we add
> support for custom .recalc callbacks for clocks that need them.
> 
> Patch 8 drops .round_rate from factors clk ops. Since only one of
> .round_rate and .determine_rate is needed, and the clk core prefers the
> latter, remove .round_rate.
> 
> Patch 9 rewrites sun6i-a31-ahb1-clk using factors clk with the new custom
> formula support. sun6i-a31-ahb1 has a pre-divider on one of its parents.
> 
> Patch 10 rewrite sun6i-ar100 using factors clk.
> 
> Patch 11 rewrites sun8i-a23-mbus-clk using the simpler composite clk.
> While this patch is doing the reverse, i.e. rewriting a factors clk into
> a composite clk, it is included because some changes overlap. I'm not
> sure whether this approach is worthwhile, as it actually adds more code,
> though it might make it easier to understand.

Thanks a lot for working on this.

I'm guessing we could even take a step further, since most of the
clocks are re-using a variation of the factor calculation code. We
roughly end up in a handful of cases (the clocks are just from a quick
look at the A10 and A31 datasheet and the source code, which might
leave a few clocks that we don't support yet in the newer SoCs)

  * A single factor:
    + These ones are trivial to handle, a simple division gives us
      directly the divisor to use.
    + Clocks in this case:
      - A13 AHB (p)
      - A80 AHB (p)
      - A10 PLL3 (m)
      - A31 AHB (m)
      - A80 GT (m)

  * Two factors:
    + These ones might be a bit more difficult to handle. One case is
      quite trivial too, it's the n and m case, where we can use
      directly rational_best_approximation() that handles this just
      fine.
      The other cases are a bit more tricky, but we can always brute
      force it, it shouldn't be very difficult to implement or very
      long to run.

    + Clocks in the (p + m) case
      - A10 APB1
      - A20 CLK OUT
      - A10 MOD0
      - A31 AR100
      - A80 APB1

    + Clocks in the (n + k) case
      - A10 PLL5
      - A31 PLL6

    + Clocks in the (n + m) case
      - A10 PLL2
      - A31 PLL3
      - A31 PLL4
      - A31 PLL8
      - A31 PLL9
      - A31 PLL10

  * Three factors
    + There's probably some consolidation that can be done here too,
      or to consider brute-forcing the whole thing again. The number
      of combinations would probably rise quite a lot, which might
      have a quite significant performance hit. I'm not really sure we
      care though.

    + Clocks in the (n, k and m) case
      - A31 PLL1
      - A31 PLL5
      - A31 MIPI PLL

    + Clocks in the (n, p and m) case
      - A31 pll2
      - A80 pll4

  * All factors (n, k, p and m)
    + I'm not sure it's worth it in this case. I'd expect the code to
      be quite complex and slow to evaluate all the cases.
    + Clocks
      - A10 PLL1
      - A10 PLL4
      - A23 PLL1
    

So, I guess we could have a default (and overridable) function that
would cover at least the cases where we have a single or two
factors. I think we already have everything we need in the clk_factors
structure, so we shouldn't need to modify each and every clocks.

What do you think about it?
Maxime

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH RFC 00/11] clk: sunxi: factors clk clean up and refactor

[Maxime Ripard]

Hi Chen-Yu,

On Mon, Jan 25, 2016 at 09:15:36PM +0800, Chen-Yu Tsai wrote:
> Hi everyone,
> 
> This series cleans up and reworks parts of sunxi's factors clk. The goal
> is to support non-standard formulas for clock rate calculation, such as
> pre-dividers on some parents, or all power-of-2 dividers. One such clock
> is the AHB1 clock on A31/A31s.
> 
> Patch 1 is Maxime's patch adding an unregister function for composite
> clocks. Patches 3 and 4 use this, so it is included for completeness.
> 
> Patch 2 makes the config tables for factors clk constant. These contain
> the shift and width for the factors. They are used to manipulate the
> clk register values. There should be no reason to change them in-flight.
> 
> Patch 3 adds a proper error path for the factors clk register function(),
> so we don't leak memory when a call fails.
> 
> Patch 4 adds an unregister function for factors clks.
> 
> Patch 5 adds an error patch to sunxi_factors_clk_setup()
> 
> Patch 6 packs the parameters passed to get_factors callbacks in a struct.
> This makes it easier to extend factors clk without having to edit all
> the function definitions, and also makes the lines shorter.
> 
> Patch 7 makes factors clk support custom formulas for calculating clock
> rates. On the clock rounding/setting side, we only need to teach
> get_factors about different parent clocks. On the recalc side, we add
> support for custom .recalc callbacks for clocks that need them.
> 
> Patch 8 drops .round_rate from factors clk ops. Since only one of
> .round_rate and .determine_rate is needed, and the clk core prefers the
> latter, remove .round_rate.
> 
> Patch 9 rewrites sun6i-a31-ahb1-clk using factors clk with the new custom
> formula support. sun6i-a31-ahb1 has a pre-divider on one of its parents.
> 
> Patch 10 rewrite sun6i-ar100 using factors clk.
> 
> Patch 11 rewrites sun8i-a23-mbus-clk using the simpler composite clk.
> While this patch is doing the reverse, i.e. rewriting a factors clk into
> a composite clk, it is included because some changes overlap. I'm not
> sure whether this approach is worthwhile, as it actually adds more code,
> though it might make it easier to understand.

Thanks a lot for working on this.

I'm guessing we could even take a step further, since most of the
clocks are re-using a variation of the factor calculation code. We
roughly end up in a handful of cases (the clocks are just from a quick
look at the A10 and A31 datasheet and the source code, which might
leave a few clocks that we don't support yet in the newer SoCs)

  * A single factor:
    + These ones are trivial to handle, a simple division gives us
      directly the divisor to use.
    + Clocks in this case:
      - A13 AHB (p)
      - A80 AHB (p)
      - A10 PLL3 (m)
      - A31 AHB (m)
      - A80 GT (m)

  * Two factors:
    + These ones might be a bit more difficult to handle. One case is
      quite trivial too, it's the n and m case, where we can use
      directly rational_best_approximation() that handles this just
      fine.
      The other cases are a bit more tricky, but we can always brute
      force it, it shouldn't be very difficult to implement or very
      long to run.

    + Clocks in the (p + m) case
      - A10 APB1
      - A20 CLK OUT
      - A10 MOD0
      - A31 AR100
      - A80 APB1

    + Clocks in the (n + k) case
      - A10 PLL5
      - A31 PLL6

    + Clocks in the (n + m) case
      - A10 PLL2
      - A31 PLL3
      - A31 PLL4
      - A31 PLL8
      - A31 PLL9
      - A31 PLL10

  * Three factors
    + There's probably some consolidation that can be done here too,
      or to consider brute-forcing the whole thing again. The number
      of combinations would probably rise quite a lot, which might
      have a quite significant performance hit. I'm not really sure we
      care though.

    + Clocks in the (n, k and m) case
      - A31 PLL1
      - A31 PLL5
      - A31 MIPI PLL

    + Clocks in the (n, p and m) case
      - A31 pll2
      - A80 pll4

  * All factors (n, k, p and m)
    + I'm not sure it's worth it in this case. I'd expect the code to
      be quite complex and slow to evaluate all the cases.
    + Clocks
      - A10 PLL1
      - A10 PLL4
      - A23 PLL1
    

So, I guess we could have a default (and overridable) function that
would cover at least the cases where we have a single or two
factors. I think we already have everything we need in the clk_factors
structure, so we shouldn't need to modify each and every clocks.

What do you think about 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/20160127/af0e51c3/attachment.sig>