[PATCH 00/10] clk: cleanup on basic clks

[PATCH 00/10] clk: cleanup on basic clks

[Shawn Guo]

The series makes some cleanup on basic clks.  Two patches from Mark
are folded here as they are doing the same thing.

Regards,
Shawn

---
Mark Brown (2):
      clk: fixed-rate: Don't open code kstrdup()
      clk: Constify parent name arrays

Shawn Guo (8):
      clk: use kzalloc in clk_register_mux
      clk: remove unnecessary EXPORT_SYMBOL_GPL
      clk: add "const" for clk_ops of basic clks
      clk: declare clk_ops of basic clks in clk-provider.h
      clk: align error return of basic clks registeration
      clk: fixed-rate: registration error handling fixup
      clk: dynamically allocate parent_names basic clk registration
      clk: copy parent_names in clk-mux registration

 drivers/clk/clk-divider.c    |   19 +++++++++++--------
 drivers/clk/clk-fixed-rate.c |   33 +++++++++++++++++----------------
 drivers/clk/clk-gate.c       |   20 ++++++++++++--------
 drivers/clk/clk-mux.c        |   39 +++++++++++++++++++++++++++++++--------
 drivers/clk/clk.c            |    2 +-
 include/linux/clk-private.h  |   10 +---------
 include/linux/clk-provider.h |   10 ++++++----
 7 files changed, 79 insertions(+), 54 deletions(-)

[PATCH 01/10] clk: use kzalloc in clk_register_mux

[Shawn Guo]

Change clk_register_mux to use kzalloc, just like what all other basic
clk registration functions do.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/clk/clk-mux.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
index c71ad1f..50e0595 100644
--- a/drivers/clk/clk-mux.c
+++ b/drivers/clk/clk-mux.c
@@ -97,7 +97,7 @@ struct clk *clk_register_mux(struct device *dev, const char *name,
 {
 	struct clk_mux *mux;
 
-	mux = kmalloc(sizeof(struct clk_mux), GFP_KERNEL);
+	mux = kzalloc(sizeof(struct clk_mux), GFP_KERNEL);
 
 	if (!mux) {
 		pr_err("%s: could not allocate mux clk\n", __func__);
-- 
1.7.5.4

[PATCH 01/10] clk: use kzalloc in clk_register_mux

[Turquette, Mike]

On Tue, Mar 27, 2012 at 12:23 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> Change clk_register_mux to use kzalloc, just like what all other basic
> clk registration functions do.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

Thanks for the patch.  Will take into my fixes branch.

Regards,
Mike

> ---
> ?drivers/clk/clk-mux.c | ? ?2 +-
> ?1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
> index c71ad1f..50e0595 100644
> --- a/drivers/clk/clk-mux.c
> +++ b/drivers/clk/clk-mux.c
> @@ -97,7 +97,7 @@ struct clk *clk_register_mux(struct device *dev, const char *name,
> ?{
> ? ? ? ?struct clk_mux *mux;
>
> - ? ? ? mux = kmalloc(sizeof(struct clk_mux), GFP_KERNEL);
> + ? ? ? mux = kzalloc(sizeof(struct clk_mux), GFP_KERNEL);
>
> ? ? ? ?if (!mux) {
> ? ? ? ? ? ? ? ?pr_err("%s: could not allocate mux clk\n", __func__);
> --
> 1.7.5.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 02/10] clk: remove unnecessary EXPORT_SYMBOL_GPL

[Shawn Guo]

It makes no sense to have EXPORT_SYMBOL_GPL on static functions.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/clk/clk-divider.c    |    3 ---
 drivers/clk/clk-fixed-rate.c |    1 -
 drivers/clk/clk-gate.c       |    3 ---
 drivers/clk/clk-mux.c        |    2 --
 4 files changed, 0 insertions(+), 9 deletions(-)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index d5ac6a7..231cd6e 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -45,7 +45,6 @@ static unsigned long clk_divider_recalc_rate(struct clk_hw *hw,
 
 	return parent_rate / div;
 }
-EXPORT_SYMBOL_GPL(clk_divider_recalc_rate);
 
 /*
  * The reverse of DIV_ROUND_UP: The maximum number which
@@ -117,7 +116,6 @@ static long clk_divider_round_rate(struct clk_hw *hw, unsigned long rate,
 		return r / div;
 	}
 }
-EXPORT_SYMBOL_GPL(clk_divider_round_rate);
 
 static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate)
 {
@@ -147,7 +145,6 @@ static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(clk_divider_set_rate);
 
 struct clk_ops clk_divider_ops = {
 	.recalc_rate = clk_divider_recalc_rate,
diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c
index 90c79fb..651b06f 100644
--- a/drivers/clk/clk-fixed-rate.c
+++ b/drivers/clk/clk-fixed-rate.c
@@ -32,7 +32,6 @@ static unsigned long clk_fixed_rate_recalc_rate(struct clk_hw *hw,
 {
 	return to_clk_fixed_rate(hw)->fixed_rate;
 }
-EXPORT_SYMBOL_GPL(clk_fixed_rate_recalc_rate);
 
 struct clk_ops clk_fixed_rate_ops = {
 	.recalc_rate = clk_fixed_rate_recalc_rate,
diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
index b5902e2..b688f47 100644
--- a/drivers/clk/clk-gate.c
+++ b/drivers/clk/clk-gate.c
@@ -71,7 +71,6 @@ static int clk_gate_enable(struct clk_hw *hw)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(clk_gate_enable);
 
 static void clk_gate_disable(struct clk_hw *hw)
 {
@@ -82,7 +81,6 @@ static void clk_gate_disable(struct clk_hw *hw)
 	else
 		clk_gate_clear_bit(gate);
 }
-EXPORT_SYMBOL_GPL(clk_gate_disable);
 
 static int clk_gate_is_enabled(struct clk_hw *hw)
 {
@@ -99,7 +97,6 @@ static int clk_gate_is_enabled(struct clk_hw *hw)
 
 	return reg ? 1 : 0;
 }
-EXPORT_SYMBOL_GPL(clk_gate_is_enabled);
 
 struct clk_ops clk_gate_ops = {
 	.enable = clk_gate_enable,
diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
index 50e0595..45cad61 100644
--- a/drivers/clk/clk-mux.c
+++ b/drivers/clk/clk-mux.c
@@ -55,7 +55,6 @@ static u8 clk_mux_get_parent(struct clk_hw *hw)
 
 	return val;
 }
-EXPORT_SYMBOL_GPL(clk_mux_get_parent);
 
 static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
 {
@@ -82,7 +81,6 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(clk_mux_set_parent);
 
 struct clk_ops clk_mux_ops = {
 	.get_parent = clk_mux_get_parent,
-- 
1.7.5.4

[PATCH 03/10] clk: add "const" for clk_ops of basic clks

[Shawn Guo]

The clk_ops of basic clks should have "const" to match the definition
in "struct clk" and clk_register prototype.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/clk/clk-divider.c    |    2 +-
 drivers/clk/clk-fixed-rate.c |    2 +-
 drivers/clk/clk-gate.c       |    2 +-
 drivers/clk/clk-mux.c        |    2 +-
 include/linux/clk-private.h  |    8 ++++----
 5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index 231cd6e..b1c4b02 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -146,7 +146,7 @@ static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate)
 	return 0;
 }
 
-struct clk_ops clk_divider_ops = {
+const struct clk_ops clk_divider_ops = {
 	.recalc_rate = clk_divider_recalc_rate,
 	.round_rate = clk_divider_round_rate,
 	.set_rate = clk_divider_set_rate,
diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c
index 651b06f..027e477 100644
--- a/drivers/clk/clk-fixed-rate.c
+++ b/drivers/clk/clk-fixed-rate.c
@@ -33,7 +33,7 @@ static unsigned long clk_fixed_rate_recalc_rate(struct clk_hw *hw,
 	return to_clk_fixed_rate(hw)->fixed_rate;
 }
 
-struct clk_ops clk_fixed_rate_ops = {
+const struct clk_ops clk_fixed_rate_ops = {
 	.recalc_rate = clk_fixed_rate_recalc_rate,
 };
 EXPORT_SYMBOL_GPL(clk_fixed_rate_ops);
diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
index b688f47..fe2ff9e 100644
--- a/drivers/clk/clk-gate.c
+++ b/drivers/clk/clk-gate.c
@@ -98,7 +98,7 @@ static int clk_gate_is_enabled(struct clk_hw *hw)
 	return reg ? 1 : 0;
 }
 
-struct clk_ops clk_gate_ops = {
+const struct clk_ops clk_gate_ops = {
 	.enable = clk_gate_enable,
 	.disable = clk_gate_disable,
 	.is_enabled = clk_gate_is_enabled,
diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
index 45cad61..5424488 100644
--- a/drivers/clk/clk-mux.c
+++ b/drivers/clk/clk-mux.c
@@ -82,7 +82,7 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
 	return 0;
 }
 
-struct clk_ops clk_mux_ops = {
+const struct clk_ops clk_mux_ops = {
 	.get_parent = clk_mux_get_parent,
 	.set_parent = clk_mux_set_parent,
 };
diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h
index 5e4312b..5f4ccd7 100644
--- a/include/linux/clk-private.h
+++ b/include/linux/clk-private.h
@@ -55,7 +55,7 @@ struct clk {
  * alternative macro for static initialization
  */
 
-extern struct clk_ops clk_fixed_rate_ops;
+extern const struct clk_ops clk_fixed_rate_ops;
 
 #define DEFINE_CLK_FIXED_RATE(_name, _flags, _rate,		\
 				_fixed_rate_flags)		\
@@ -78,7 +78,7 @@ extern struct clk_ops clk_fixed_rate_ops;
 		.flags = _flags,				\
 	};
 
-extern struct clk_ops clk_gate_ops;
+extern const struct clk_ops clk_gate_ops;
 
 #define DEFINE_CLK_GATE(_name, _parent_name, _parent_ptr,	\
 				_flags, _reg, _bit_idx,		\
@@ -110,7 +110,7 @@ extern struct clk_ops clk_gate_ops;
 		.flags = _flags,				\
 	};
 
-extern struct clk_ops clk_divider_ops;
+extern const struct clk_ops clk_divider_ops;
 
 #define DEFINE_CLK_DIVIDER(_name, _parent_name, _parent_ptr,	\
 				_flags, _reg, _shift, _width,	\
@@ -143,7 +143,7 @@ extern struct clk_ops clk_divider_ops;
 		.flags = _flags,				\
 	};
 
-extern struct clk_ops clk_mux_ops;
+extern const struct clk_ops clk_mux_ops;
 
 #define DEFINE_CLK_MUX(_name, _parent_names, _parents, _flags,	\
 				_reg, _shift, _width,		\
-- 
1.7.5.4

[PATCH 03/10] clk: add "const" for clk_ops of basic clks

[Turquette, Mike]

On Tue, Mar 27, 2012 at 12:23 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> The clk_ops of basic clks should have "const" to match the definition
> in "struct clk" and clk_register prototype.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

Thanks for the patch.  Will take into my fixes branch.

Regards,
Mike

> ---
> ?drivers/clk/clk-divider.c ? ?| ? ?2 +-
> ?drivers/clk/clk-fixed-rate.c | ? ?2 +-
> ?drivers/clk/clk-gate.c ? ? ? | ? ?2 +-
> ?drivers/clk/clk-mux.c ? ? ? ?| ? ?2 +-
> ?include/linux/clk-private.h ?| ? ?8 ++++----
> ?5 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> index 231cd6e..b1c4b02 100644
> --- a/drivers/clk/clk-divider.c
> +++ b/drivers/clk/clk-divider.c
> @@ -146,7 +146,7 @@ static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate)
> ? ? ? ?return 0;
> ?}
>
> -struct clk_ops clk_divider_ops = {
> +const struct clk_ops clk_divider_ops = {
> ? ? ? ?.recalc_rate = clk_divider_recalc_rate,
> ? ? ? ?.round_rate = clk_divider_round_rate,
> ? ? ? ?.set_rate = clk_divider_set_rate,
> diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c
> index 651b06f..027e477 100644
> --- a/drivers/clk/clk-fixed-rate.c
> +++ b/drivers/clk/clk-fixed-rate.c
> @@ -33,7 +33,7 @@ static unsigned long clk_fixed_rate_recalc_rate(struct clk_hw *hw,
> ? ? ? ?return to_clk_fixed_rate(hw)->fixed_rate;
> ?}
>
> -struct clk_ops clk_fixed_rate_ops = {
> +const struct clk_ops clk_fixed_rate_ops = {
> ? ? ? ?.recalc_rate = clk_fixed_rate_recalc_rate,
> ?};
> ?EXPORT_SYMBOL_GPL(clk_fixed_rate_ops);
> diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
> index b688f47..fe2ff9e 100644
> --- a/drivers/clk/clk-gate.c
> +++ b/drivers/clk/clk-gate.c
> @@ -98,7 +98,7 @@ static int clk_gate_is_enabled(struct clk_hw *hw)
> ? ? ? ?return reg ? 1 : 0;
> ?}
>
> -struct clk_ops clk_gate_ops = {
> +const struct clk_ops clk_gate_ops = {
> ? ? ? ?.enable = clk_gate_enable,
> ? ? ? ?.disable = clk_gate_disable,
> ? ? ? ?.is_enabled = clk_gate_is_enabled,
> diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
> index 45cad61..5424488 100644
> --- a/drivers/clk/clk-mux.c
> +++ b/drivers/clk/clk-mux.c
> @@ -82,7 +82,7 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
> ? ? ? ?return 0;
> ?}
>
> -struct clk_ops clk_mux_ops = {
> +const struct clk_ops clk_mux_ops = {
> ? ? ? ?.get_parent = clk_mux_get_parent,
> ? ? ? ?.set_parent = clk_mux_set_parent,
> ?};
> diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h
> index 5e4312b..5f4ccd7 100644
> --- a/include/linux/clk-private.h
> +++ b/include/linux/clk-private.h
> @@ -55,7 +55,7 @@ struct clk {
> ?* alternative macro for static initialization
> ?*/
>
> -extern struct clk_ops clk_fixed_rate_ops;
> +extern const struct clk_ops clk_fixed_rate_ops;
>
> ?#define DEFINE_CLK_FIXED_RATE(_name, _flags, _rate, ? ? ? ? ? ?\
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?_fixed_rate_flags) ? ? ? ? ? ? ?\
> @@ -78,7 +78,7 @@ extern struct clk_ops clk_fixed_rate_ops;
> ? ? ? ? ? ? ? ?.flags = _flags, ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> ? ? ? ?};
>
> -extern struct clk_ops clk_gate_ops;
> +extern const struct clk_ops clk_gate_ops;
>
> ?#define DEFINE_CLK_GATE(_name, _parent_name, _parent_ptr, ? ? ?\
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?_flags, _reg, _bit_idx, ? ? ? ? \
> @@ -110,7 +110,7 @@ extern struct clk_ops clk_gate_ops;
> ? ? ? ? ? ? ? ?.flags = _flags, ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> ? ? ? ?};
>
> -extern struct clk_ops clk_divider_ops;
> +extern const struct clk_ops clk_divider_ops;
>
> ?#define DEFINE_CLK_DIVIDER(_name, _parent_name, _parent_ptr, ? \
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?_flags, _reg, _shift, _width, ? \
> @@ -143,7 +143,7 @@ extern struct clk_ops clk_divider_ops;
> ? ? ? ? ? ? ? ?.flags = _flags, ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> ? ? ? ?};
>
> -extern struct clk_ops clk_mux_ops;
> +extern const struct clk_ops clk_mux_ops;
>
> ?#define DEFINE_CLK_MUX(_name, _parent_names, _parents, _flags, \
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?_reg, _shift, _width, ? ? ? ? ? \
> --
> 1.7.5.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 04/10] clk: declare clk_ops of basic clks in clk-provider.h

[Shawn Guo]

Besides the static initialization, the clk_ops of basic clks could
also be used by particular clk type being subclass of the basic clks.

For example, clk_busy_divider has the same clk_ops as clk_divider,
except it has to wait for a busy bit before return success with
.set_rate.  clk_busy_divider will somehow reuse clk_ops of clk_divider.

Since clk-provider.h is included by clk-private.h, it's safe to move
those clk_ops declaration of basic clks form  clk-private.h into
clk-provider.h, so that implementation of clks like clk_busy_divider
above do not need to include clk-private.h to access those clk_ops.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 include/linux/clk-private.h  |    8 --------
 include/linux/clk-provider.h |    4 ++++
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h
index 5f4ccd7..f19fee0 100644
--- a/include/linux/clk-private.h
+++ b/include/linux/clk-private.h
@@ -55,8 +55,6 @@ struct clk {
  * alternative macro for static initialization
  */
 
-extern const struct clk_ops clk_fixed_rate_ops;
-
 #define DEFINE_CLK_FIXED_RATE(_name, _flags, _rate,		\
 				_fixed_rate_flags)		\
 	static struct clk _name;				\
@@ -78,8 +76,6 @@ extern const struct clk_ops clk_fixed_rate_ops;
 		.flags = _flags,				\
 	};
 
-extern const struct clk_ops clk_gate_ops;
-
 #define DEFINE_CLK_GATE(_name, _parent_name, _parent_ptr,	\
 				_flags, _reg, _bit_idx,		\
 				_gate_flags, _lock)		\
@@ -110,8 +106,6 @@ extern const struct clk_ops clk_gate_ops;
 		.flags = _flags,				\
 	};
 
-extern const struct clk_ops clk_divider_ops;
-
 #define DEFINE_CLK_DIVIDER(_name, _parent_name, _parent_ptr,	\
 				_flags, _reg, _shift, _width,	\
 				_divider_flags, _lock)		\
@@ -143,8 +137,6 @@ extern const struct clk_ops clk_divider_ops;
 		.flags = _flags,				\
 	};
 
-extern const struct clk_ops clk_mux_ops;
-
 #define DEFINE_CLK_MUX(_name, _parent_names, _parents, _flags,	\
 				_reg, _shift, _width,		\
 				_mux_flags, _lock)		\
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 5508897..6eb8e5d 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -149,6 +149,7 @@ struct clk_fixed_rate {
 	u8		flags;
 };
 
+extern const struct clk_ops clk_fixed_rate_ops;
 struct clk *clk_register_fixed_rate(struct device *dev, const char *name,
 		const char *parent_name, unsigned long flags,
 		unsigned long fixed_rate);
@@ -180,6 +181,7 @@ struct clk_gate {
 
 #define CLK_GATE_SET_TO_DISABLE		BIT(0)
 
+extern const struct clk_ops clk_gate_ops;
 struct clk *clk_register_gate(struct device *dev, const char *name,
 		const char *parent_name, unsigned long flags,
 		void __iomem *reg, u8 bit_idx,
@@ -218,6 +220,7 @@ struct clk_divider {
 #define CLK_DIVIDER_ONE_BASED		BIT(0)
 #define CLK_DIVIDER_POWER_OF_TWO	BIT(1)
 
+extern const struct clk_ops clk_divider_ops;
 struct clk *clk_register_divider(struct device *dev, const char *name,
 		const char *parent_name, unsigned long flags,
 		void __iomem *reg, u8 shift, u8 width,
@@ -252,6 +255,7 @@ struct clk_mux {
 #define CLK_MUX_INDEX_ONE		BIT(0)
 #define CLK_MUX_INDEX_BIT		BIT(1)
 
+extern const struct clk_ops clk_mux_ops;
 struct clk *clk_register_mux(struct device *dev, const char *name,
 		char **parent_names, u8 num_parents, unsigned long flags,
 		void __iomem *reg, u8 shift, u8 width,
-- 
1.7.5.4

[PATCH 04/10] clk: declare clk_ops of basic clks in clk-provider.h

[Turquette, Mike]

On Tue, Mar 27, 2012 at 12:23 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> Besides the static initialization, the clk_ops of basic clks could
> also be used by particular clk type being subclass of the basic clks.
>
> For example, clk_busy_divider has the same clk_ops as clk_divider,
> except it has to wait for a busy bit before return success with
> .set_rate. ?clk_busy_divider will somehow reuse clk_ops of clk_divider.
>
> Since clk-provider.h is included by clk-private.h, it's safe to move
> those clk_ops declaration of basic clks form ?clk-private.h into
> clk-provider.h, so that implementation of clks like clk_busy_divider
> above do not need to include clk-private.h to access those clk_ops.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

Hi Shawn,

Thanks for the patch.  After Andrew's response on using clk_ops over
the EXPORT'd functions I'll take this in my fixes branch.

Mike

> ---
> ?include/linux/clk-private.h ?| ? ?8 --------
> ?include/linux/clk-provider.h | ? ?4 ++++
> ?2 files changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h
> index 5f4ccd7..f19fee0 100644
> --- a/include/linux/clk-private.h
> +++ b/include/linux/clk-private.h
> @@ -55,8 +55,6 @@ struct clk {
> ?* alternative macro for static initialization
> ?*/
>
> -extern const struct clk_ops clk_fixed_rate_ops;
> -
> ?#define DEFINE_CLK_FIXED_RATE(_name, _flags, _rate, ? ? ? ? ? ?\
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?_fixed_rate_flags) ? ? ? ? ? ? ?\
> ? ? ? ?static struct clk _name; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> @@ -78,8 +76,6 @@ extern const struct clk_ops clk_fixed_rate_ops;
> ? ? ? ? ? ? ? ?.flags = _flags, ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> ? ? ? ?};
>
> -extern const struct clk_ops clk_gate_ops;
> -
> ?#define DEFINE_CLK_GATE(_name, _parent_name, _parent_ptr, ? ? ?\
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?_flags, _reg, _bit_idx, ? ? ? ? \
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?_gate_flags, _lock) ? ? ? ? ? ? \
> @@ -110,8 +106,6 @@ extern const struct clk_ops clk_gate_ops;
> ? ? ? ? ? ? ? ?.flags = _flags, ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> ? ? ? ?};
>
> -extern const struct clk_ops clk_divider_ops;
> -
> ?#define DEFINE_CLK_DIVIDER(_name, _parent_name, _parent_ptr, ? \
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?_flags, _reg, _shift, _width, ? \
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?_divider_flags, _lock) ? ? ? ? ?\
> @@ -143,8 +137,6 @@ extern const struct clk_ops clk_divider_ops;
> ? ? ? ? ? ? ? ?.flags = _flags, ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> ? ? ? ?};
>
> -extern const struct clk_ops clk_mux_ops;
> -
> ?#define DEFINE_CLK_MUX(_name, _parent_names, _parents, _flags, \
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?_reg, _shift, _width, ? ? ? ? ? \
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?_mux_flags, _lock) ? ? ? ? ? ? ?\
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 5508897..6eb8e5d 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -149,6 +149,7 @@ struct clk_fixed_rate {
> ? ? ? ?u8 ? ? ? ? ? ? ?flags;
> ?};
>
> +extern const struct clk_ops clk_fixed_rate_ops;
> ?struct clk *clk_register_fixed_rate(struct device *dev, const char *name,
> ? ? ? ? ? ? ? ?const char *parent_name, unsigned long flags,
> ? ? ? ? ? ? ? ?unsigned long fixed_rate);
> @@ -180,6 +181,7 @@ struct clk_gate {
>
> ?#define CLK_GATE_SET_TO_DISABLE ? ? ? ? ? ? ? ?BIT(0)
>
> +extern const struct clk_ops clk_gate_ops;
> ?struct clk *clk_register_gate(struct device *dev, const char *name,
> ? ? ? ? ? ? ? ?const char *parent_name, unsigned long flags,
> ? ? ? ? ? ? ? ?void __iomem *reg, u8 bit_idx,
> @@ -218,6 +220,7 @@ struct clk_divider {
> ?#define CLK_DIVIDER_ONE_BASED ? ? ? ? ?BIT(0)
> ?#define CLK_DIVIDER_POWER_OF_TWO ? ? ? BIT(1)
>
> +extern const struct clk_ops clk_divider_ops;
> ?struct clk *clk_register_divider(struct device *dev, const char *name,
> ? ? ? ? ? ? ? ?const char *parent_name, unsigned long flags,
> ? ? ? ? ? ? ? ?void __iomem *reg, u8 shift, u8 width,
> @@ -252,6 +255,7 @@ struct clk_mux {
> ?#define CLK_MUX_INDEX_ONE ? ? ? ? ? ? ?BIT(0)
> ?#define CLK_MUX_INDEX_BIT ? ? ? ? ? ? ?BIT(1)
>
> +extern const struct clk_ops clk_mux_ops;
> ?struct clk *clk_register_mux(struct device *dev, const char *name,
> ? ? ? ? ? ? ? ?char **parent_names, u8 num_parents, unsigned long flags,
> ? ? ? ? ? ? ? ?void __iomem *reg, u8 shift, u8 width,
> --
> 1.7.5.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 05/10] clk: align error return of basic clks registeration

[Shawn Guo]

clk_register_gate and clk_register_divider return NULL for error,
which is aligned with clk_register function.  But clk_register_mux
and clk_register_fixed_rate return ERR_PTR.  Change them to return
NULL for better aligning with other registeration functions.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/clk/clk-fixed-rate.c |    2 +-
 drivers/clk/clk-mux.c        |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c
index 027e477..d268439 100644
--- a/drivers/clk/clk-fixed-rate.c
+++ b/drivers/clk/clk-fixed-rate.c
@@ -50,7 +50,7 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name,
 
 	if (!fixed) {
 		pr_err("%s: could not allocate fixed clk\n", __func__);
-		return ERR_PTR(-ENOMEM);
+		return NULL;
 	}
 
 	/* struct clk_fixed_rate assignments */
diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
index 5424488..19f3116 100644
--- a/drivers/clk/clk-mux.c
+++ b/drivers/clk/clk-mux.c
@@ -99,7 +99,7 @@ struct clk *clk_register_mux(struct device *dev, const char *name,
 
 	if (!mux) {
 		pr_err("%s: could not allocate mux clk\n", __func__);
-		return ERR_PTR(-ENOMEM);
+		return NULL;
 	}
 
 	/* struct clk_mux assignments */
-- 
1.7.5.4

[PATCH 05/10] clk: align error return of basic clks registeration

[Turquette, Mike]

On Tue, Mar 27, 2012 at 12:23 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> clk_register_gate and clk_register_divider return NULL for error,
> which is aligned with clk_register function. ?But clk_register_mux
> and clk_register_fixed_rate return ERR_PTR. ?Change them to return
> NULL for better aligning with other registeration functions.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

Thanks for the patch Shawn, but this is actually the opposite
direction that I want to go.  In light of the other thread with
Saravana and Russell on a similar topic, I'd like to make sure that
the driver-facing functions clearly return -EERROR during failure.
I've lined up a patch for the same on my side.

Regards,
Mike

> ---
> ?drivers/clk/clk-fixed-rate.c | ? ?2 +-
> ?drivers/clk/clk-mux.c ? ? ? ?| ? ?2 +-
> ?2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c
> index 027e477..d268439 100644
> --- a/drivers/clk/clk-fixed-rate.c
> +++ b/drivers/clk/clk-fixed-rate.c
> @@ -50,7 +50,7 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name,
>
> ? ? ? ?if (!fixed) {
> ? ? ? ? ? ? ? ?pr_err("%s: could not allocate fixed clk\n", __func__);
> - ? ? ? ? ? ? ? return ERR_PTR(-ENOMEM);
> + ? ? ? ? ? ? ? return NULL;
> ? ? ? ?}
>
> ? ? ? ?/* struct clk_fixed_rate assignments */
> diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
> index 5424488..19f3116 100644
> --- a/drivers/clk/clk-mux.c
> +++ b/drivers/clk/clk-mux.c
> @@ -99,7 +99,7 @@ struct clk *clk_register_mux(struct device *dev, const char *name,
>
> ? ? ? ?if (!mux) {
> ? ? ? ? ? ? ? ?pr_err("%s: could not allocate mux clk\n", __func__);
> - ? ? ? ? ? ? ? return ERR_PTR(-ENOMEM);
> + ? ? ? ? ? ? ? return NULL;
> ? ? ? ?}
>
> ? ? ? ?/* struct clk_mux assignments */
> --
> 1.7.5.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 06/10] clk: fixed-rate: Don't open code kstrdup()

[Shawn Guo]

From: Mark Brown <broonie@opensource.wolfsonmicro.com>

Save a little code by using the library function.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/clk/clk-fixed-rate.c |    7 +------
 1 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c
index d268439..1f87a03 100644
--- a/drivers/clk/clk-fixed-rate.c
+++ b/drivers/clk/clk-fixed-rate.c
@@ -44,7 +44,6 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name,
 {
 	struct clk_fixed_rate *fixed;
 	char **parent_names = NULL;
-	u8 len;
 
 	fixed = kzalloc(sizeof(struct clk_fixed_rate), GFP_KERNEL);
 
@@ -62,14 +61,10 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name,
 		if (! parent_names)
 			goto out;
 
-		len = sizeof(char) * strlen(parent_name);
-
-		parent_names[0] = kmalloc(len, GFP_KERNEL);
+		parent_names[0] = kstrdup(parent_name, GFP_KERNEL);
 
 		if (!parent_names[0])
 			goto out;
-
-		strncpy(parent_names[0], parent_name, len);
 	}
 
 out:
-- 
1.7.5.4

[PATCH 06/10] clk: fixed-rate: Don't open code kstrdup()

[Turquette, Mike]

On Tue, Mar 27, 2012 at 12:23 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> From: Mark Brown <broonie@opensource.wolfsonmicro.com>
>
> Save a little code by using the library function.
>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Shawn,  thanks for reposting.

Mark, thanks for the patch.  I wrote a near-identical patch in my
branch, but it returns -EERROR instead of just returning NULL on
failure so I'll stick with my version.

Thanks,
Mike

> ---
> ?drivers/clk/clk-fixed-rate.c | ? ?7 +------
> ?1 files changed, 1 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c
> index d268439..1f87a03 100644
> --- a/drivers/clk/clk-fixed-rate.c
> +++ b/drivers/clk/clk-fixed-rate.c
> @@ -44,7 +44,6 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name,
> ?{
> ? ? ? ?struct clk_fixed_rate *fixed;
> ? ? ? ?char **parent_names = NULL;
> - ? ? ? u8 len;
>
> ? ? ? ?fixed = kzalloc(sizeof(struct clk_fixed_rate), GFP_KERNEL);
>
> @@ -62,14 +61,10 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name,
> ? ? ? ? ? ? ? ?if (! parent_names)
> ? ? ? ? ? ? ? ? ? ? ? ?goto out;
>
> - ? ? ? ? ? ? ? len = sizeof(char) * strlen(parent_name);
> -
> - ? ? ? ? ? ? ? parent_names[0] = kmalloc(len, GFP_KERNEL);
> + ? ? ? ? ? ? ? parent_names[0] = kstrdup(parent_name, GFP_KERNEL);
>
> ? ? ? ? ? ? ? ?if (!parent_names[0])
> ? ? ? ? ? ? ? ? ? ? ? ?goto out;
> -
> - ? ? ? ? ? ? ? strncpy(parent_names[0], parent_name, len);
> ? ? ? ?}
>
> ?out:
> --
> 1.7.5.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 07/10] clk: Constify parent name arrays

[Shawn Guo]

From: Mark Brown <broonie@opensource.wolfsonmicro.com>

Drivers should be able to declare their arrays of parent names as const
so the APIs need to accept const arguments.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/clk/clk-fixed-rate.c |    2 +-
 drivers/clk/clk-mux.c        |    2 +-
 drivers/clk/clk.c            |    2 +-
 include/linux/clk-private.h  |    2 +-
 include/linux/clk-provider.h |    6 +++---
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c
index 1f87a03..295f0d5 100644
--- a/drivers/clk/clk-fixed-rate.c
+++ b/drivers/clk/clk-fixed-rate.c
@@ -43,7 +43,7 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name,
 		unsigned long fixed_rate)
 {
 	struct clk_fixed_rate *fixed;
-	char **parent_names = NULL;
+	const char **parent_names = NULL;
 
 	fixed = kzalloc(sizeof(struct clk_fixed_rate), GFP_KERNEL);
 
diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
index 19f3116..1ff90a6 100644
--- a/drivers/clk/clk-mux.c
+++ b/drivers/clk/clk-mux.c
@@ -89,7 +89,7 @@ const struct clk_ops clk_mux_ops = {
 EXPORT_SYMBOL_GPL(clk_mux_ops);
 
 struct clk *clk_register_mux(struct device *dev, const char *name,
-		char **parent_names, u8 num_parents, unsigned long flags,
+		const char **parent_names, u8 num_parents, unsigned long flags,
 		void __iomem *reg, u8 shift, u8 width,
 		u8 clk_mux_flags, spinlock_t *lock)
 {
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 9cf6f59..33ef0df 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1322,7 +1322,7 @@ out:
  */
 struct clk *clk_register(struct device *dev, const char *name,
 		const struct clk_ops *ops, struct clk_hw *hw,
-		char **parent_names, u8 num_parents, unsigned long flags)
+		const char **parent_names, u8 num_parents, unsigned long flags)
 {
 	struct clk *clk;
 
diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h
index f19fee0..e9c8b98 100644
--- a/include/linux/clk-private.h
+++ b/include/linux/clk-private.h
@@ -30,7 +30,7 @@ struct clk {
 	const struct clk_ops	*ops;
 	struct clk_hw		*hw;
 	struct clk		*parent;
-	char			**parent_names;
+	const char		**parent_names;
 	struct clk		**parents;
 	u8			num_parents;
 	unsigned long		rate;
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 6eb8e5d..24c5013 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -214,7 +214,7 @@ struct clk_divider {
 	u8		width;
 	u8		flags;
 	spinlock_t	*lock;
-	char		*parent[1];
+	const char	*parent[1];
 };
 
 #define CLK_DIVIDER_ONE_BASED		BIT(0)
@@ -257,7 +257,7 @@ struct clk_mux {
 
 extern const struct clk_ops clk_mux_ops;
 struct clk *clk_register_mux(struct device *dev, const char *name,
-		char **parent_names, u8 num_parents, unsigned long flags,
+		const char **parent_names, u8 num_parents, unsigned long flags,
 		void __iomem *reg, u8 shift, u8 width,
 		u8 clk_mux_flags, spinlock_t *lock);
 
@@ -278,7 +278,7 @@ struct clk *clk_register_mux(struct device *dev, const char *name,
  */
 struct clk *clk_register(struct device *dev, const char *name,
 		const struct clk_ops *ops, struct clk_hw *hw,
-		char **parent_names, u8 num_parents, unsigned long flags);
+		const char **parent_names, u8 num_parents, unsigned long flags);
 
 /* helper functions */
 const char *__clk_get_name(struct clk *clk);
-- 
1.7.5.4

[PATCH 07/10] clk: Constify parent name arrays

[Turquette, Mike]

On Tue, Mar 27, 2012 at 12:23 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> From: Mark Brown <broonie@opensource.wolfsonmicro.com>
>
> Drivers should be able to declare their arrays of parent names as const
> so the APIs need to accept const arguments.
>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Thanks for reposting Shawn.

Mark, I've taken this in to my fixes branch, but I dropped the change
in drivers/clk/clk-fixed-rate.c (since that code has shuffled) and I
also marked the char *parent[1] in fixed-rate and gate clocks as
const, in addition to the divider you did in your patch.

Thanks,
Mike

[PATCH 07/10] clk: Constify parent name arrays

[Mark Brown]

On Tue, Mar 27, 2012 at 06:03:49PM -0700, Turquette, Mike wrote:

> Mark, I've taken this in to my fixes branch, but I dropped the change
> in drivers/clk/clk-fixed-rate.c (since that code has shuffled) and I
> also marked the char *parent[1] in fixed-rate and gate clocks as
> const, in addition to the divider you did in your patch.

That one's more fun as you should really be declaring them as something
like a const array of pointers to const char which is very cumbersome to
actually do and looks ugly as hell.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120328/ae45a8ac/attachment.sig>

[PATCH 08/10] clk: fixed-rate: registration error handling fixup

[Shawn Guo]

The clk_register_fixed_rate should not call clk_register on "out:"
path.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/clk/clk-fixed-rate.c |   19 +++++++++++++------
 1 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c
index 295f0d5..0d80827 100644
--- a/drivers/clk/clk-fixed-rate.c
+++ b/drivers/clk/clk-fixed-rate.c
@@ -43,6 +43,7 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name,
 		unsigned long fixed_rate)
 {
 	struct clk_fixed_rate *fixed;
+	struct clk *clk;
 	const char **parent_names = NULL;
 
 	fixed = kzalloc(sizeof(struct clk_fixed_rate), GFP_KERNEL);
@@ -56,21 +57,27 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name,
 	fixed->fixed_rate = fixed_rate;
 
 	if (parent_name) {
-		parent_names = kmalloc(sizeof(char *), GFP_KERNEL);
-
-		if (! parent_names)
+		parent_names = kzalloc(sizeof(char *), GFP_KERNEL);
+		if (!parent_names)
 			goto out;
 
 		parent_names[0] = kstrdup(parent_name, GFP_KERNEL);
-
 		if (!parent_names[0])
 			goto out;
 	}
 
-out:
-	return clk_register(dev, name,
+	clk = clk_register(dev, name,
 			&clk_fixed_rate_ops, &fixed->hw,
 			parent_names,
 			(parent_name ? 1 : 0),
 			flags);
+	if (clk)
+		return clk;
+
+out:
+	kfree(parent_names[0]);
+	kfree(parent_names);
+	kfree(fixed);
+
+	return NULL;
 }
-- 
1.7.5.4

[PATCH 08/10] clk: fixed-rate: registration error handling fixup

[Turquette, Mike]

On Tue, Mar 27, 2012 at 12:23 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> The clk_register_fixed_rate should not call clk_register on "out:"
> path.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

Hi Shawn,

I already had a patch for this sitting in my fixes branch and it
properly returns -EERROR for failures, so I'll stick with that one.

Thanks,
Mike

> ---
> ?drivers/clk/clk-fixed-rate.c | ? 19 +++++++++++++------
> ?1 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c
> index 295f0d5..0d80827 100644
> --- a/drivers/clk/clk-fixed-rate.c
> +++ b/drivers/clk/clk-fixed-rate.c
> @@ -43,6 +43,7 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name,
> ? ? ? ? ? ? ? ?unsigned long fixed_rate)
> ?{
> ? ? ? ?struct clk_fixed_rate *fixed;
> + ? ? ? struct clk *clk;
> ? ? ? ?const char **parent_names = NULL;
>
> ? ? ? ?fixed = kzalloc(sizeof(struct clk_fixed_rate), GFP_KERNEL);
> @@ -56,21 +57,27 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name,
> ? ? ? ?fixed->fixed_rate = fixed_rate;
>
> ? ? ? ?if (parent_name) {
> - ? ? ? ? ? ? ? parent_names = kmalloc(sizeof(char *), GFP_KERNEL);
> -
> - ? ? ? ? ? ? ? if (! parent_names)
> + ? ? ? ? ? ? ? parent_names = kzalloc(sizeof(char *), GFP_KERNEL);
> + ? ? ? ? ? ? ? if (!parent_names)
> ? ? ? ? ? ? ? ? ? ? ? ?goto out;
>
> ? ? ? ? ? ? ? ?parent_names[0] = kstrdup(parent_name, GFP_KERNEL);
> -
> ? ? ? ? ? ? ? ?if (!parent_names[0])
> ? ? ? ? ? ? ? ? ? ? ? ?goto out;
> ? ? ? ?}
>
> -out:
> - ? ? ? return clk_register(dev, name,
> + ? ? ? clk = clk_register(dev, name,
> ? ? ? ? ? ? ? ? ? ? ? ?&clk_fixed_rate_ops, &fixed->hw,
> ? ? ? ? ? ? ? ? ? ? ? ?parent_names,
> ? ? ? ? ? ? ? ? ? ? ? ?(parent_name ? 1 : 0),
> ? ? ? ? ? ? ? ? ? ? ? ?flags);
> + ? ? ? if (clk)
> + ? ? ? ? ? ? ? return clk;
> +
> +out:
> + ? ? ? kfree(parent_names[0]);
> + ? ? ? kfree(parent_names);
> + ? ? ? kfree(fixed);
> +
> + ? ? ? return NULL;
> ?}
> --
> 1.7.5.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 09/10] clk: dynamically allocate parent_names basic clk registration

[Shawn Guo]

It's not necessary to have "char *parent[1]" carried in struct clk_gate
and clk_divider.  Instead, it can be dynamically allocated in
registration function, just like what clk_register_fixed_rate does.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/clk/clk-divider.c    |   14 ++++++++++----
 drivers/clk/clk-gate.c       |   15 +++++++++++----
 include/linux/clk-provider.h |    2 --
 3 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index b1c4b02..df03a61 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -160,6 +160,7 @@ struct clk *clk_register_divider(struct device *dev, const char *name,
 {
 	struct clk_divider *div;
 	struct clk *clk;
+	const char **parent_names = NULL;
 
 	div = kzalloc(sizeof(struct clk_divider), GFP_KERNEL);
 
@@ -176,21 +177,26 @@ struct clk *clk_register_divider(struct device *dev, const char *name,
 	div->lock = lock;
 
 	if (parent_name) {
-		div->parent[0] = kstrdup(parent_name, GFP_KERNEL);
-		if (!div->parent[0])
+		parent_names = kzalloc(sizeof(char *), GFP_KERNEL);
+		if (!parent_names)
+			goto out;
+
+		parent_names[0] = kstrdup(parent_name, GFP_KERNEL);
+		if (!parent_names[0])
 			goto out;
 	}
 
 	clk = clk_register(dev, name,
 			&clk_divider_ops, &div->hw,
-			div->parent,
+			parent_names,
 			(parent_name ? 1 : 0),
 			flags);
 	if (clk)
 		return clk;
 
 out:
-	kfree(div->parent[0]);
+	kfree(parent_names[0]);
+	kfree(parent_names);
 	kfree(div);
 
 	return NULL;
diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
index fe2ff9e..51c56cb 100644
--- a/drivers/clk/clk-gate.c
+++ b/drivers/clk/clk-gate.c
@@ -112,6 +112,7 @@ struct clk *clk_register_gate(struct device *dev, const char *name,
 {
 	struct clk_gate *gate;
 	struct clk *clk;
+	const char **parent_names = NULL;
 
 	gate = kzalloc(sizeof(struct clk_gate), GFP_KERNEL);
 
@@ -127,20 +128,26 @@ struct clk *clk_register_gate(struct device *dev, const char *name,
 	gate->lock = lock;
 
 	if (parent_name) {
-		gate->parent[0] = kstrdup(parent_name, GFP_KERNEL);
-		if (!gate->parent[0])
+		parent_names = kzalloc(sizeof(char *), GFP_KERNEL);
+		if (!parent_names)
+			goto out;
+
+		parent_names[0] = kstrdup(parent_name, GFP_KERNEL);
+		if (parent_names[0])
 			goto out;
 	}
 
 	clk = clk_register(dev, name,
 			&clk_gate_ops, &gate->hw,
-			gate->parent,
+			parent_names,
 			(parent_name ? 1 : 0),
 			flags);
 	if (clk)
 		return clk;
+
 out:
-	kfree(gate->parent[0]);
+	kfree(parent_names[0]);
+	kfree(parent_names);
 	kfree(gate);
 
 	return NULL;
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 24c5013..7948640 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -176,7 +176,6 @@ struct clk_gate {
 	u8		bit_idx;
 	u8		flags;
 	spinlock_t	*lock;
-	char		*parent[1];
 };
 
 #define CLK_GATE_SET_TO_DISABLE		BIT(0)
@@ -214,7 +213,6 @@ struct clk_divider {
 	u8		width;
 	u8		flags;
 	spinlock_t	*lock;
-	const char	*parent[1];
 };
 
 #define CLK_DIVIDER_ONE_BASED		BIT(0)
-- 
1.7.5.4

[PATCH 09/10] clk: dynamically allocate parent_names basic clk registration

[Sascha Hauer]

On Tue, Mar 27, 2012 at 03:23:28PM +0800, Shawn Guo wrote:
> It's not necessary to have "char *parent[1]" carried in struct clk_gate
> and clk_divider.  Instead, it can be dynamically allocated in
> registration function, just like what clk_register_fixed_rate does.

I patched it the way it is to get rid of the additional malloc which
seemed unnecessary. Maybe the real question is whether the core should
duplicate the parent strings instead of the basic clock types.

Sascha

> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  drivers/clk/clk-divider.c    |   14 ++++++++++----
>  drivers/clk/clk-gate.c       |   15 +++++++++++----
>  include/linux/clk-provider.h |    2 --
>  3 files changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> index b1c4b02..df03a61 100644
> --- a/drivers/clk/clk-divider.c
> +++ b/drivers/clk/clk-divider.c
> @@ -160,6 +160,7 @@ struct clk *clk_register_divider(struct device *dev, const char *name,
>  {
>  	struct clk_divider *div;
>  	struct clk *clk;
> +	const char **parent_names = NULL;
>  
>  	div = kzalloc(sizeof(struct clk_divider), GFP_KERNEL);
>  
> @@ -176,21 +177,26 @@ struct clk *clk_register_divider(struct device *dev, const char *name,
>  	div->lock = lock;
>  
>  	if (parent_name) {
> -		div->parent[0] = kstrdup(parent_name, GFP_KERNEL);
> -		if (!div->parent[0])
> +		parent_names = kzalloc(sizeof(char *), GFP_KERNEL);
> +		if (!parent_names)
> +			goto out;
> +
> +		parent_names[0] = kstrdup(parent_name, GFP_KERNEL);
> +		if (!parent_names[0])
>  			goto out;
>  	}
>  
>  	clk = clk_register(dev, name,
>  			&clk_divider_ops, &div->hw,
> -			div->parent,
> +			parent_names,
>  			(parent_name ? 1 : 0),
>  			flags);
>  	if (clk)
>  		return clk;
>  
>  out:
> -	kfree(div->parent[0]);
> +	kfree(parent_names[0]);
> +	kfree(parent_names);
>  	kfree(div);
>  
>  	return NULL;
> diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
> index fe2ff9e..51c56cb 100644
> --- a/drivers/clk/clk-gate.c
> +++ b/drivers/clk/clk-gate.c
> @@ -112,6 +112,7 @@ struct clk *clk_register_gate(struct device *dev, const char *name,
>  {
>  	struct clk_gate *gate;
>  	struct clk *clk;
> +	const char **parent_names = NULL;
>  
>  	gate = kzalloc(sizeof(struct clk_gate), GFP_KERNEL);
>  
> @@ -127,20 +128,26 @@ struct clk *clk_register_gate(struct device *dev, const char *name,
>  	gate->lock = lock;
>  
>  	if (parent_name) {
> -		gate->parent[0] = kstrdup(parent_name, GFP_KERNEL);
> -		if (!gate->parent[0])
> +		parent_names = kzalloc(sizeof(char *), GFP_KERNEL);
> +		if (!parent_names)
> +			goto out;
> +
> +		parent_names[0] = kstrdup(parent_name, GFP_KERNEL);
> +		if (parent_names[0])
>  			goto out;
>  	}
>  
>  	clk = clk_register(dev, name,
>  			&clk_gate_ops, &gate->hw,
> -			gate->parent,
> +			parent_names,
>  			(parent_name ? 1 : 0),
>  			flags);
>  	if (clk)
>  		return clk;
> +
>  out:
> -	kfree(gate->parent[0]);
> +	kfree(parent_names[0]);
> +	kfree(parent_names);
>  	kfree(gate);
>  
>  	return NULL;
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 24c5013..7948640 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -176,7 +176,6 @@ struct clk_gate {
>  	u8		bit_idx;
>  	u8		flags;
>  	spinlock_t	*lock;
> -	char		*parent[1];
>  };
>  
>  #define CLK_GATE_SET_TO_DISABLE		BIT(0)
> @@ -214,7 +213,6 @@ struct clk_divider {
>  	u8		width;
>  	u8		flags;
>  	spinlock_t	*lock;
> -	const char	*parent[1];
>  };
>  
>  #define CLK_DIVIDER_ONE_BASED		BIT(0)
> -- 
> 1.7.5.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 09/10] clk: dynamically allocate parent_names basic clk registration

[Shawn Guo]

On Tue, Mar 27, 2012 at 10:55:14AM +0200, Sascha Hauer wrote:
> On Tue, Mar 27, 2012 at 03:23:28PM +0800, Shawn Guo wrote:
> > It's not necessary to have "char *parent[1]" carried in struct clk_gate
> > and clk_divider.  Instead, it can be dynamically allocated in
> > registration function, just like what clk_register_fixed_rate does.
> 
> I patched it the way it is to get rid of the additional malloc which
> seemed unnecessary. Maybe the real question is whether the core should
> duplicate the parent strings instead of the basic clock types.
> 
If we can ensure the parent strings will only be used during
clk_register call, we do not need to duplicate the parent strings.

But right now the parent strings is used not only in clk_register call
but also __clk_set_parent for finding parent index in case the search
on cached parent points fails.  I guess Mike has a good reason for that.

-- 
Regards,
Shawn

[PATCH 09/10] clk: dynamically allocate parent_names basic clk registration

[Sascha Hauer]

On Tue, Mar 27, 2012 at 08:51:24PM +0800, Shawn Guo wrote:
> On Tue, Mar 27, 2012 at 10:55:14AM +0200, Sascha Hauer wrote:
> > On Tue, Mar 27, 2012 at 03:23:28PM +0800, Shawn Guo wrote:
> > > It's not necessary to have "char *parent[1]" carried in struct clk_gate
> > > and clk_divider.  Instead, it can be dynamically allocated in
> > > registration function, just like what clk_register_fixed_rate does.
> > 
> > I patched it the way it is to get rid of the additional malloc which
> > seemed unnecessary. Maybe the real question is whether the core should
> > duplicate the parent strings instead of the basic clock types.
> > 
> If we can ensure the parent strings will only be used during
> clk_register call, we do not need to duplicate the parent strings.
> 
> But right now the parent strings is used not only in clk_register call
> but also __clk_set_parent for finding parent index in case the search
> on cached parent points fails.  I guess Mike has a good reason for that.

Probably. That's why I suggest to copy the strings in the framework.
Then we do not have to duplicate the code to copy the strings and also
make sure that the core works on its own strings.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 09/10] clk: dynamically allocate parent_names basic clk registration

[Turquette, Mike]

On Tue, Mar 27, 2012 at 12:40 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Tue, Mar 27, 2012 at 08:51:24PM +0800, Shawn Guo wrote:
>> On Tue, Mar 27, 2012 at 10:55:14AM +0200, Sascha Hauer wrote:
>> > On Tue, Mar 27, 2012 at 03:23:28PM +0800, Shawn Guo wrote:
>> > > It's not necessary to have "char *parent[1]" carried in struct clk_gate
>> > > and clk_divider. ?Instead, it can be dynamically allocated in
>> > > registration function, just like what clk_register_fixed_rate does.
>> >
>> > I patched it the way it is to get rid of the additional malloc which
>> > seemed unnecessary. Maybe the real question is whether the core should
>> > duplicate the parent strings instead of the basic clock types.
>> >
>> If we can ensure the parent strings will only be used during
>> clk_register call, we do not need to duplicate the parent strings.
>>
>> But right now the parent strings is used not only in clk_register call
>> but also __clk_set_parent for finding parent index in case the search
>> on cached parent points fails. ?I guess Mike has a good reason for that.

I'd like to think my reasons are good ;-)

The common clk framework pulls together clock data from many sources,
and we can never assume that the tree of struct clk's is "complete".
For example an on-SoC multiplexer clock might have one input from an
external PMIC.  It is conceivable that clock data / common clk support
for that PMIC isn't yet merged.  Should that prevent the on-SoC mux
clock from using that source via clk_set_parent?  Absolutely not.
That is why the parent_names arrays are necessary.  They represent
"complete" data even in the absence of actual clock data.  Using only
the index of the array the .set_rate callback should be able to do the
[register write | i2c command | whatever] in order to switch to the
PMIC source.  The rate will be unknown, but at least the operation is
possible.

> Probably. That's why I suggest to copy the strings in the framework.
> Then we do not have to duplicate the code to copy the strings and also
> make sure that the core works on its own strings.

Copying the strings in the fwk is sensible.  I'll throw together a
patch which takes in the const char *parent_name and passes that to
clk_register directly, which builds up the char **parent_names itself.

Regards,
Mike

[PATCH 10/10] clk: copy parent_names in clk-mux registration

[Shawn Guo]

Just like what all other basic clks do, copy parent_names in clk-mux
registration, so that clock driver can mark the parent_names
__initconst.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/clk/clk-mux.c |   29 +++++++++++++++++++++++++++--
 1 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
index 1ff90a6..353fcb7 100644
--- a/drivers/clk/clk-mux.c
+++ b/drivers/clk/clk-mux.c
@@ -94,6 +94,9 @@ struct clk *clk_register_mux(struct device *dev, const char *name,
 		u8 clk_mux_flags, spinlock_t *lock)
 {
 	struct clk_mux *mux;
+	struct clk *clk;
+	const char **parents = NULL;
+	int i = 0;
 
 	mux = kzalloc(sizeof(struct clk_mux), GFP_KERNEL);
 
@@ -109,6 +112,28 @@ struct clk *clk_register_mux(struct device *dev, const char *name,
 	mux->flags = clk_mux_flags;
 	mux->lock = lock;
 
-	return clk_register(dev, name, &clk_mux_ops, &mux->hw,
-			parent_names, num_parents, flags);
+	if (num_parents) {
+		parents = kzalloc(sizeof(char *) * num_parents, GFP_KERNEL);
+		if (!parents)
+			goto out;
+
+		for (i = 0; i < num_parents; i++) {
+			parents[i] = kstrdup(parent_names[i], GFP_KERNEL);
+			if (!parents[i])
+				goto out;
+		}
+	}
+
+	clk =  clk_register(dev, name, &clk_mux_ops, &mux->hw,
+			parents, num_parents, flags);
+	if (clk)
+		return clk;
+
+out:
+	for (i--; i >= 0; i--)
+		kfree(parents[i]);
+	kfree(parents);
+	kfree(mux);
+
+	return NULL;
 }
-- 
1.7.5.4

[PATCH 10/10] clk: copy parent_names in clk-mux registration

[Turquette, Mike]

On Tue, Mar 27, 2012 at 12:23 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> Just like what all other basic clks do, copy parent_names in clk-mux
> registration, so that clock driver can mark the parent_names
> __initconst.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

Hi Shawn,

As discussed in the other thread, I've moved the parent string copies
into clk_register; that makes this patch unnecessary (and you can
still mark the parent_names as __initdata).

I'll publish my fixes either today or tomorrow.

Regards,
Mike

> ---
> ?drivers/clk/clk-mux.c | ? 29 +++++++++++++++++++++++++++--
> ?1 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
> index 1ff90a6..353fcb7 100644
> --- a/drivers/clk/clk-mux.c
> +++ b/drivers/clk/clk-mux.c
> @@ -94,6 +94,9 @@ struct clk *clk_register_mux(struct device *dev, const char *name,
> ? ? ? ? ? ? ? ?u8 clk_mux_flags, spinlock_t *lock)
> ?{
> ? ? ? ?struct clk_mux *mux;
> + ? ? ? struct clk *clk;
> + ? ? ? const char **parents = NULL;
> + ? ? ? int i = 0;
>
> ? ? ? ?mux = kzalloc(sizeof(struct clk_mux), GFP_KERNEL);
>
> @@ -109,6 +112,28 @@ struct clk *clk_register_mux(struct device *dev, const char *name,
> ? ? ? ?mux->flags = clk_mux_flags;
> ? ? ? ?mux->lock = lock;
>
> - ? ? ? return clk_register(dev, name, &clk_mux_ops, &mux->hw,
> - ? ? ? ? ? ? ? ? ? ? ? parent_names, num_parents, flags);
> + ? ? ? if (num_parents) {
> + ? ? ? ? ? ? ? parents = kzalloc(sizeof(char *) * num_parents, GFP_KERNEL);
> + ? ? ? ? ? ? ? if (!parents)
> + ? ? ? ? ? ? ? ? ? ? ? goto out;
> +
> + ? ? ? ? ? ? ? for (i = 0; i < num_parents; i++) {
> + ? ? ? ? ? ? ? ? ? ? ? parents[i] = kstrdup(parent_names[i], GFP_KERNEL);
> + ? ? ? ? ? ? ? ? ? ? ? if (!parents[i])
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? goto out;
> + ? ? ? ? ? ? ? }
> + ? ? ? }
> +
> + ? ? ? clk = ?clk_register(dev, name, &clk_mux_ops, &mux->hw,
> + ? ? ? ? ? ? ? ? ? ? ? parents, num_parents, flags);
> + ? ? ? if (clk)
> + ? ? ? ? ? ? ? return clk;
> +
> +out:
> + ? ? ? for (i--; i >= 0; i--)
> + ? ? ? ? ? ? ? kfree(parents[i]);
> + ? ? ? kfree(parents);
> + ? ? ? kfree(mux);
> +
> + ? ? ? return NULL;
> ?}
> --
> 1.7.5.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel