[PATCH] clk: add table lookup to mux

[PATCH] clk: add table lookup to mux

[Peter De Schrijver]

Add a table lookup feature to the mux clock. Also allow arbitrary masks
instead of the width. This will be used by some clocks on Tegra114. Also
adapt the tegra periph clk because it uses struct clk_mux directly.

Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>

--
Mike,

This is the same patch I posted before which implements a table lookup
feature for the mux clock. I squashed both the changes to clk-mux.c and
tegra/clk.h together in order to make the patch bisectable.

Thanks,

Peter.
---
 drivers/clk/clk-mux.c        |   50 ++++++++++++++++++++++++++++++++----------
 drivers/clk/tegra/clk.h      |   27 ++++++++++++++++-------
 include/linux/clk-private.h  |    3 ++-
 include/linux/clk-provider.h |    9 +++++++-
 4 files changed, 68 insertions(+), 21 deletions(-)

diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
index 508c032..25b1734 100644
--- a/drivers/clk/clk-mux.c
+++ b/drivers/clk/clk-mux.c
@@ -32,6 +32,7 @@
 static u8 clk_mux_get_parent(struct clk_hw *hw)
 {
 	struct clk_mux *mux = to_clk_mux(hw);
+	int num_parents = __clk_get_num_parents(hw->clk);
 	u32 val;
 
 	/*
@@ -42,7 +43,16 @@ static u8 clk_mux_get_parent(struct clk_hw *hw)
 	 * val = 0x4 really means "bit 2, index starts at bit 0"
 	 */
 	val = readl(mux->reg) >> mux->shift;
-	val &= (1 << mux->width) - 1;
+	val &= mux->mask;
+
+	if (mux->table) {
+		int i;
+
+		for (i = 0; i < num_parents; i++)
+			if (mux->table[i] == val)
+				return i;
+		return -EINVAL;
+	}
 
 	if (val && (mux->flags & CLK_MUX_INDEX_BIT))
 		val = ffs(val) - 1;
@@ -50,7 +60,7 @@ static u8 clk_mux_get_parent(struct clk_hw *hw)
 	if (val && (mux->flags & CLK_MUX_INDEX_ONE))
 		val--;
 
-	if (val >= __clk_get_num_parents(hw->clk))
+	if (val >= num_parents)
 		return -EINVAL;
 
 	return val;
@@ -62,17 +72,22 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
 	u32 val;
 	unsigned long flags = 0;
 
-	if (mux->flags & CLK_MUX_INDEX_BIT)
-		index = (1 << ffs(index));
+	if (mux->table)
+		index = mux->table[index];
 
-	if (mux->flags & CLK_MUX_INDEX_ONE)
-		index++;
+	else {
+		if (mux->flags & CLK_MUX_INDEX_BIT)
+			index = (1 << ffs(index));
+
+		if (mux->flags & CLK_MUX_INDEX_ONE)
+			index++;
+	}
 
 	if (mux->lock)
 		spin_lock_irqsave(mux->lock, flags);
 
 	val = readl(mux->reg);
-	val &= ~(((1 << mux->width) - 1) << mux->shift);
+	val &= ~(mux->mask << mux->shift);
 	val |= index << mux->shift;
 	writel(val, mux->reg);
 
@@ -88,10 +103,10 @@ const struct clk_ops clk_mux_ops = {
 };
 EXPORT_SYMBOL_GPL(clk_mux_ops);
 
-struct clk *clk_register_mux(struct device *dev, const char *name,
+struct clk *clk_register_mux_table(struct device *dev, const char *name,
 		const char **parent_names, u8 num_parents, unsigned long flags,
-		void __iomem *reg, u8 shift, u8 width,
-		u8 clk_mux_flags, spinlock_t *lock)
+		void __iomem *reg, u8 shift, u32 mask,
+		u8 clk_mux_flags, u32 *table, spinlock_t *lock)
 {
 	struct clk_mux *mux;
 	struct clk *clk;
@@ -113,9 +128,10 @@ struct clk *clk_register_mux(struct device *dev, const char *name,
 	/* struct clk_mux assignments */
 	mux->reg = reg;
 	mux->shift = shift;
-	mux->width = width;
+	mux->mask = mask;
 	mux->flags = clk_mux_flags;
 	mux->lock = lock;
+	mux->table = table;
 	mux->hw.init = &init;
 
 	clk = clk_register(dev, &mux->hw);
@@ -125,3 +141,15 @@ struct clk *clk_register_mux(struct device *dev, const char *name,
 
 	return clk;
 }
+
+struct clk *clk_register_mux(struct device *dev, const char *name,
+		const char **parent_names, u8 num_parents, unsigned long flags,
+		void __iomem *reg, u8 shift, u8 width,
+		u8 clk_mux_flags, spinlock_t *lock)
+{
+	u32 mask = BIT(width) - 1;
+
+	return clk_register_mux_table(dev, name, parent_names, num_parents,
+				      flags, reg, shift, mask, clk_mux_flags,
+				      NULL, lock);
+}
diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
index 0744731..a09d7dc 100644
--- a/drivers/clk/tegra/clk.h
+++ b/drivers/clk/tegra/clk.h
@@ -355,15 +355,16 @@ struct clk *tegra_clk_register_periph_nodiv(const char *name,
 		struct tegra_clk_periph *periph, void __iomem *clk_base,
 		u32 offset);
 
-#define TEGRA_CLK_PERIPH(_mux_shift, _mux_width, _mux_flags,		\
+#define TEGRA_CLK_PERIPH(_mux_shift, _mux_mask, _mux_flags,		\
 			 _div_shift, _div_width, _div_frac_width,	\
 			 _div_flags, _clk_num, _enb_refcnt, _regs,	\
-			 _gate_flags)					\
+			 _gate_flags, _table)				\
 	{								\
 		.mux = {						\
 			.flags = _mux_flags,				\
 			.shift = _mux_shift,				\
-			.width = _mux_width,				\
+			.mask = _mux_mask,				\
+			.table = _table,				\
 		},							\
 		.divider = {						\
 			.flags = _div_flags,				\
@@ -393,26 +394,36 @@ struct tegra_periph_init_data {
 	const char *dev_id;
 };
 
-#define TEGRA_INIT_DATA(_name, _con_id, _dev_id, _parent_names, _offset, \
-			_mux_shift, _mux_width, _mux_flags, _div_shift,	\
+#define TEGRA_INIT_DATA_TABLE(_name, _con_id, _dev_id, _parent_names, _offset,\
+			_mux_shift, _mux_mask, _mux_flags, _div_shift,	\
 			_div_width, _div_frac_width, _div_flags, _regs,	\
-			_clk_num, _enb_refcnt, _gate_flags, _clk_id)	\
+			_clk_num, _enb_refcnt, _gate_flags, _clk_id, _table) \
 	{								\
 		.name = _name,						\
 		.clk_id = _clk_id,					\
 		.parent_names = _parent_names,				\
 		.num_parents = ARRAY_SIZE(_parent_names),		\
-		.periph = TEGRA_CLK_PERIPH(_mux_shift, _mux_width,	\
+		.periph = TEGRA_CLK_PERIPH(_mux_shift, _mux_mask,	\
 					   _mux_flags, _div_shift,	\
 					   _div_width, _div_frac_width,	\
 					   _div_flags, _clk_num,	\
 					   _enb_refcnt, _regs,		\
-					   _gate_flags),		\
+					   _gate_flags, _table),	\
 		.offset = _offset,					\
 		.con_id = _con_id,					\
 		.dev_id = _dev_id,					\
 	}
 
+#define TEGRA_INIT_DATA(_name, _con_id, _dev_id, _parent_names, _offset,\
+			_mux_shift, _mux_width, _mux_flags, _div_shift,	\
+			_div_width, _div_frac_width, _div_flags, _regs,	\
+			_clk_num, _enb_refcnt, _gate_flags, _clk_id)	\
+	TEGRA_INIT_DATA_TABLE(_name, _con_id, _dev_id, _parent_names, _offset,\
+			_mux_shift, BIT(_mux_width) - 1, _mux_flags,	\
+			_div_shift, _div_width, _div_frac_width, _div_flags, \
+			_regs, _clk_num, _enb_refcnt, _gate_flags, _clk_id,\
+			NULL)
+
 /**
  * struct clk_super_mux - super clock
  *
diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h
index 9c7f580..53d39c2 100644
--- a/include/linux/clk-private.h
+++ b/include/linux/clk-private.h
@@ -144,12 +144,13 @@ struct clk {
 
 #define DEFINE_CLK_MUX(_name, _parent_names, _parents, _flags,	\
 				_reg, _shift, _width,		\
-				_mux_flags, _lock)		\
+				_mux_flags, _table, _lock)	\
 	static struct clk _name;				\
 	static struct clk_mux _name##_hw = {			\
 		.hw = {						\
 			.clk = &_name,				\
 		},						\
+		.table = _table,				\
 		.reg = _reg,					\
 		.shift = _shift,				\
 		.width = _width,				\
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 7f197d7..fc435bb 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -287,8 +287,9 @@ struct clk *clk_register_divider_table(struct device *dev, const char *name,
 struct clk_mux {
 	struct clk_hw	hw;
 	void __iomem	*reg;
+	u32		*table;
+	u32		mask;
 	u8		shift;
-	u8		width;
 	u8		flags;
 	spinlock_t	*lock;
 };
@@ -297,11 +298,17 @@ struct clk_mux {
 #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,
 		const char **parent_names, u8 num_parents, unsigned long flags,
 		void __iomem *reg, u8 shift, u8 width,
 		u8 clk_mux_flags, spinlock_t *lock);
 
+struct clk *clk_register_mux_table(struct device *dev, const char *name,
+		const char **parent_names, u8 num_parents, unsigned long flags,
+		void __iomem *reg, u8 shift, u32 mask,
+		u8 clk_mux_flags, u32 *table, spinlock_t *lock);
+
 /**
  * struct clk_fixed_factor - fixed multiplier and divider clock
  *
-- 
1.7.10.4

Re: [PATCH] clk: add table lookup to mux

[Stephen Warren]

On 03/12/2013 12:42 PM, Peter De Schrijver wrote:
> Add a table lookup feature to the mux clock. Also allow arbitrary masks
> instead of the width. This will be used by some clocks on Tegra114. Also
> adapt the tegra periph clk because it uses struct clk_mux directly.
> 
> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> 
> --
> Mike,
> 
> This is the same patch I posted before which implements a table lookup
> feature for the mux clock. I squashed both the changes to clk-mux.c and
> tegra/clk.h together in order to make the patch bisectable.

For the record in this thread, this is a dependency of the Tegra114
clock driver. Can this patch be added to a stable branch in the clk or
arm-soc trees, which I can merge before applying the Tegra114 clock
driver? Thanks.

Re: [PATCH] clk: add table lookup to mux

[Mike Turquette]

Quoting Peter De Schrijver (2013-03-12 11:42:23)
> Add a table lookup feature to the mux clock. Also allow arbitrary masks
> instead of the width. This will be used by some clocks on Tegra114. Also
> adapt the tegra periph clk because it uses struct clk_mux directly.
> 
> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>

Do you actually need arbitrary masks instead of a continuous bitfield?
Or does this change just make it easier for you to convert existing
data?

Thanks,
Mike

> 
> --
> Mike,
> 
> This is the same patch I posted before which implements a table lookup
> feature for the mux clock. I squashed both the changes to clk-mux.c and
> tegra/clk.h together in order to make the patch bisectable.
> 
> Thanks,
> 
> Peter.
> ---
>  drivers/clk/clk-mux.c        |   50 ++++++++++++++++++++++++++++++++----------
>  drivers/clk/tegra/clk.h      |   27 ++++++++++++++++-------
>  include/linux/clk-private.h  |    3 ++-
>  include/linux/clk-provider.h |    9 +++++++-
>  4 files changed, 68 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
> index 508c032..25b1734 100644
> --- a/drivers/clk/clk-mux.c
> +++ b/drivers/clk/clk-mux.c
> @@ -32,6 +32,7 @@
>  static u8 clk_mux_get_parent(struct clk_hw *hw)
>  {
>         struct clk_mux *mux = to_clk_mux(hw);
> +       int num_parents = __clk_get_num_parents(hw->clk);
>         u32 val;
>  
>         /*
> @@ -42,7 +43,16 @@ static u8 clk_mux_get_parent(struct clk_hw *hw)
>          * val = 0x4 really means "bit 2, index starts at bit 0"
>          */
>         val = readl(mux->reg) >> mux->shift;
> -       val &= (1 << mux->width) - 1;
> +       val &= mux->mask;
> +
> +       if (mux->table) {
> +               int i;
> +
> +               for (i = 0; i < num_parents; i++)
> +                       if (mux->table[i] == val)
> +                               return i;
> +               return -EINVAL;
> +       }
>  
>         if (val && (mux->flags & CLK_MUX_INDEX_BIT))
>                 val = ffs(val) - 1;
> @@ -50,7 +60,7 @@ static u8 clk_mux_get_parent(struct clk_hw *hw)
>         if (val && (mux->flags & CLK_MUX_INDEX_ONE))
>                 val--;
>  
> -       if (val >= __clk_get_num_parents(hw->clk))
> +       if (val >= num_parents)
>                 return -EINVAL;
>  
>         return val;
> @@ -62,17 +72,22 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
>         u32 val;
>         unsigned long flags = 0;
>  
> -       if (mux->flags & CLK_MUX_INDEX_BIT)
> -               index = (1 << ffs(index));
> +       if (mux->table)
> +               index = mux->table[index];
>  
> -       if (mux->flags & CLK_MUX_INDEX_ONE)
> -               index++;
> +       else {
> +               if (mux->flags & CLK_MUX_INDEX_BIT)
> +                       index = (1 << ffs(index));
> +
> +               if (mux->flags & CLK_MUX_INDEX_ONE)
> +                       index++;
> +       }
>  
>         if (mux->lock)
>                 spin_lock_irqsave(mux->lock, flags);
>  
>         val = readl(mux->reg);
> -       val &= ~(((1 << mux->width) - 1) << mux->shift);
> +       val &= ~(mux->mask << mux->shift);
>         val |= index << mux->shift;
>         writel(val, mux->reg);
>  
> @@ -88,10 +103,10 @@ const struct clk_ops clk_mux_ops = {
>  };
>  EXPORT_SYMBOL_GPL(clk_mux_ops);
>  
> -struct clk *clk_register_mux(struct device *dev, const char *name,
> +struct clk *clk_register_mux_table(struct device *dev, const char *name,
>                 const char **parent_names, u8 num_parents, unsigned long flags,
> -               void __iomem *reg, u8 shift, u8 width,
> -               u8 clk_mux_flags, spinlock_t *lock)
> +               void __iomem *reg, u8 shift, u32 mask,
> +               u8 clk_mux_flags, u32 *table, spinlock_t *lock)
>  {
>         struct clk_mux *mux;
>         struct clk *clk;
> @@ -113,9 +128,10 @@ struct clk *clk_register_mux(struct device *dev, const char *name,
>         /* struct clk_mux assignments */
>         mux->reg = reg;
>         mux->shift = shift;
> -       mux->width = width;
> +       mux->mask = mask;
>         mux->flags = clk_mux_flags;
>         mux->lock = lock;
> +       mux->table = table;
>         mux->hw.init = &init;
>  
>         clk = clk_register(dev, &mux->hw);
> @@ -125,3 +141,15 @@ struct clk *clk_register_mux(struct device *dev, const char *name,
>  
>         return clk;
>  }
> +
> +struct clk *clk_register_mux(struct device *dev, const char *name,
> +               const char **parent_names, u8 num_parents, unsigned long flags,
> +               void __iomem *reg, u8 shift, u8 width,
> +               u8 clk_mux_flags, spinlock_t *lock)
> +{
> +       u32 mask = BIT(width) - 1;
> +
> +       return clk_register_mux_table(dev, name, parent_names, num_parents,
> +                                     flags, reg, shift, mask, clk_mux_flags,
> +                                     NULL, lock);
> +}
> diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
> index 0744731..a09d7dc 100644
> --- a/drivers/clk/tegra/clk.h
> +++ b/drivers/clk/tegra/clk.h
> @@ -355,15 +355,16 @@ struct clk *tegra_clk_register_periph_nodiv(const char *name,
>                 struct tegra_clk_periph *periph, void __iomem *clk_base,
>                 u32 offset);
>  
> -#define TEGRA_CLK_PERIPH(_mux_shift, _mux_width, _mux_flags,           \
> +#define TEGRA_CLK_PERIPH(_mux_shift, _mux_mask, _mux_flags,            \
>                          _div_shift, _div_width, _div_frac_width,       \
>                          _div_flags, _clk_num, _enb_refcnt, _regs,      \
> -                        _gate_flags)                                   \
> +                        _gate_flags, _table)                           \
>         {                                                               \
>                 .mux = {                                                \
>                         .flags = _mux_flags,                            \
>                         .shift = _mux_shift,                            \
> -                       .width = _mux_width,                            \
> +                       .mask = _mux_mask,                              \
> +                       .table = _table,                                \
>                 },                                                      \
>                 .divider = {                                            \
>                         .flags = _div_flags,                            \
> @@ -393,26 +394,36 @@ struct tegra_periph_init_data {
>         const char *dev_id;
>  };
>  
> -#define TEGRA_INIT_DATA(_name, _con_id, _dev_id, _parent_names, _offset, \
> -                       _mux_shift, _mux_width, _mux_flags, _div_shift, \
> +#define TEGRA_INIT_DATA_TABLE(_name, _con_id, _dev_id, _parent_names, _offset,\
> +                       _mux_shift, _mux_mask, _mux_flags, _div_shift,  \
>                         _div_width, _div_frac_width, _div_flags, _regs, \
> -                       _clk_num, _enb_refcnt, _gate_flags, _clk_id)    \
> +                       _clk_num, _enb_refcnt, _gate_flags, _clk_id, _table) \
>         {                                                               \
>                 .name = _name,                                          \
>                 .clk_id = _clk_id,                                      \
>                 .parent_names = _parent_names,                          \
>                 .num_parents = ARRAY_SIZE(_parent_names),               \
> -               .periph = TEGRA_CLK_PERIPH(_mux_shift, _mux_width,      \
> +               .periph = TEGRA_CLK_PERIPH(_mux_shift, _mux_mask,       \
>                                            _mux_flags, _div_shift,      \
>                                            _div_width, _div_frac_width, \
>                                            _div_flags, _clk_num,        \
>                                            _enb_refcnt, _regs,          \
> -                                          _gate_flags),                \
> +                                          _gate_flags, _table),        \
>                 .offset = _offset,                                      \
>                 .con_id = _con_id,                                      \
>                 .dev_id = _dev_id,                                      \
>         }
>  
> +#define TEGRA_INIT_DATA(_name, _con_id, _dev_id, _parent_names, _offset,\
> +                       _mux_shift, _mux_width, _mux_flags, _div_shift, \
> +                       _div_width, _div_frac_width, _div_flags, _regs, \
> +                       _clk_num, _enb_refcnt, _gate_flags, _clk_id)    \
> +       TEGRA_INIT_DATA_TABLE(_name, _con_id, _dev_id, _parent_names, _offset,\
> +                       _mux_shift, BIT(_mux_width) - 1, _mux_flags,    \
> +                       _div_shift, _div_width, _div_frac_width, _div_flags, \
> +                       _regs, _clk_num, _enb_refcnt, _gate_flags, _clk_id,\
> +                       NULL)
> +
>  /**
>   * struct clk_super_mux - super clock
>   *
> diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h
> index 9c7f580..53d39c2 100644
> --- a/include/linux/clk-private.h
> +++ b/include/linux/clk-private.h
> @@ -144,12 +144,13 @@ struct clk {
>  
>  #define DEFINE_CLK_MUX(_name, _parent_names, _parents, _flags, \
>                                 _reg, _shift, _width,           \
> -                               _mux_flags, _lock)              \
> +                               _mux_flags, _table, _lock)      \
>         static struct clk _name;                                \
>         static struct clk_mux _name##_hw = {                    \
>                 .hw = {                                         \
>                         .clk = &_name,                          \
>                 },                                              \
> +               .table = _table,                                \
>                 .reg = _reg,                                    \
>                 .shift = _shift,                                \
>                 .width = _width,                                \
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 7f197d7..fc435bb 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -287,8 +287,9 @@ struct clk *clk_register_divider_table(struct device *dev, const char *name,
>  struct clk_mux {
>         struct clk_hw   hw;
>         void __iomem    *reg;
> +       u32             *table;
> +       u32             mask;
>         u8              shift;
> -       u8              width;
>         u8              flags;
>         spinlock_t      *lock;
>  };
> @@ -297,11 +298,17 @@ struct clk_mux {
>  #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,
>                 const char **parent_names, u8 num_parents, unsigned long flags,
>                 void __iomem *reg, u8 shift, u8 width,
>                 u8 clk_mux_flags, spinlock_t *lock);
>  
> +struct clk *clk_register_mux_table(struct device *dev, const char *name,
> +               const char **parent_names, u8 num_parents, unsigned long flags,
> +               void __iomem *reg, u8 shift, u32 mask,
> +               u8 clk_mux_flags, u32 *table, spinlock_t *lock);
> +
>  /**
>   * struct clk_fixed_factor - fixed multiplier and divider clock
>   *
> -- 
> 1.7.10.4

Re: [PATCH] clk: add table lookup to mux

[Mike Turquette]

Quoting Stephen Warren (2013-03-12 11:53:46)
> On 03/12/2013 12:42 PM, Peter De Schrijver wrote:
> > Add a table lookup feature to the mux clock. Also allow arbitrary masks
> > instead of the width. This will be used by some clocks on Tegra114. Also
> > adapt the tegra periph clk because it uses struct clk_mux directly.
> > 
> > Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> > 
> > --
> > Mike,
> > 
> > This is the same patch I posted before which implements a table lookup
> > feature for the mux clock. I squashed both the changes to clk-mux.c and
> > tegra/clk.h together in order to make the patch bisectable.
> 
> For the record in this thread, this is a dependency of the Tegra114
> clock driver. Can this patch be added to a stable branch in the clk or
> arm-soc trees, which I can merge before applying the Tegra114 clock
> driver? Thanks.

Yes.  I don't have an immutable branch today, but I will have one by
-rc4.  It will be named clk-for-3.10.

I have a pending question about the ugly width/mask issue, otherwise
I'll let you know when I take this patch into clk-next.

Regards,
Mike

Re: [PATCH] clk: add table lookup to mux

[Stephen Warren]

On 03/19/2013 03:33 PM, Mike Turquette wrote:
> Quoting Peter De Schrijver (2013-03-12 11:42:23)
>> Add a table lookup feature to the mux clock. Also allow arbitrary masks
>> instead of the width. This will be used by some clocks on Tegra114. Also
>> adapt the tegra periph clk because it uses struct clk_mux directly.
>>
>> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> 
> Do you actually need arbitrary masks instead of a continuous bitfield?
> Or does this change just make it easier for you to convert existing
> data?

Yes, Peter mentioned somewhere that Tegra apparently has some registers
where the mux bits are split up into non-contiguous regions of a register.

Re: [PATCH] clk: add table lookup to mux

[Stephen Warren]

On 03/19/2013 03:37 PM, Mike Turquette wrote:
> Quoting Stephen Warren (2013-03-12 11:53:46)
>> On 03/12/2013 12:42 PM, Peter De Schrijver wrote:
>>> Add a table lookup feature to the mux clock. Also allow arbitrary masks
>>> instead of the width. This will be used by some clocks on Tegra114. Also
>>> adapt the tegra periph clk because it uses struct clk_mux directly.
>>>
>>> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
>>>
>>> --
>>> Mike,
>>>
>>> This is the same patch I posted before which implements a table lookup
>>> feature for the mux clock. I squashed both the changes to clk-mux.c and
>>> tegra/clk.h together in order to make the patch bisectable.
>>
>> For the record in this thread, this is a dependency of the Tegra114
>> clock driver. Can this patch be added to a stable branch in the clk or
>> arm-soc trees, which I can merge before applying the Tegra114 clock
>> driver? Thanks.
> 
> Yes.  I don't have an immutable branch today, but I will have one by
> -rc4.  It will be named clk-for-3.10.

Perhaps this series could be merged into an immutable topic branch of
its own. That could then be merged into your clk-for-3.10, and Tegra's
branch, and reduce the number of commits that get merged into the Tegra
tree from outside?

Re: [PATCH] clk: add table lookup to mux

[Mike Turquette]

Quoting Peter De Schrijver (2013-03-12 11:42:23)
> diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h
> index 9c7f580..53d39c2 100644
> --- a/include/linux/clk-private.h
> +++ b/include/linux/clk-private.h
> @@ -144,12 +144,13 @@ struct clk {
>  
>  #define DEFINE_CLK_MUX(_name, _parent_names, _parents, _flags, \
>                                 _reg, _shift, _width,           \
> -                               _mux_flags, _lock)              \
> +                               _mux_flags, _table, _lock)      \
>         static struct clk _name;                                \
>         static struct clk_mux _name##_hw = {                    \
>                 .hw = {                                         \
>                         .clk = &_name,                          \
>                 },                                              \
> +               .table = _table,                                \
>                 .reg = _reg,                                    \
>                 .shift = _shift,                                \
>                 .width = _width,                                \

This breaks OMAP horribly since OMAP already uses this macro.  There are
two options:

1) stop using statically initialized data and no longer use
clk-private.h macros.  I was under the impression that the tegra clock
data no longer required this?

2) if you must continue to use the clk-private.h macros (temporarily!)
then create a new one, DEFINE_CLK_MUX_TABLE.

Regards,
Mike

Re: [PATCH] clk: add table lookup to mux

[Peter De Schrijver]

On Tue, Mar 19, 2013 at 10:33:09PM +0100, Mike Turquette wrote:
> Quoting Peter De Schrijver (2013-03-12 11:42:23)
> > Add a table lookup feature to the mux clock. Also allow arbitrary masks
> > instead of the width. This will be used by some clocks on Tegra114. Also
> > adapt the tegra periph clk because it uses struct clk_mux directly.
> >
> > Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> 
> Do you actually need arbitrary masks instead of a continuous bitfield?
> Or does this change just make it easier for you to convert existing
> data?
> 

Yes we need arbitrary masks. We have some clocks where some bits of the mux
are at the bottom of a 32bit register and other bits are at the top.

Cheers,

Peter.

Re: [PATCH] clk: add table lookup to mux

[Peter De Schrijver]

On Wed, Mar 20, 2013 at 12:51:10AM +0100, Mike Turquette wrote:
> Quoting Peter De Schrijver (2013-03-12 11:42:23)
> > diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h
> > index 9c7f580..53d39c2 100644
> > --- a/include/linux/clk-private.h
> > +++ b/include/linux/clk-private.h
> > @@ -144,12 +144,13 @@ struct clk {
> >  
> >  #define DEFINE_CLK_MUX(_name, _parent_names, _parents, _flags, \
> >                                 _reg, _shift, _width,           \
> > -                               _mux_flags, _lock)              \
> > +                               _mux_flags, _table, _lock)      \
> >         static struct clk _name;                                \
> >         static struct clk_mux _name##_hw = {                    \
> >                 .hw = {                                         \
> >                         .clk = &_name,                          \
> >                 },                                              \
> > +               .table = _table,                                \
> >                 .reg = _reg,                                    \
> >                 .shift = _shift,                                \
> >                 .width = _width,                                \
> 
> This breaks OMAP horribly since OMAP already uses this macro.  There are
> two options:
> 
> 1) stop using statically initialized data and no longer use
> clk-private.h macros.  I was under the impression that the tegra clock
> data no longer required this?

Ok. We don't do that anymore. Maybe I just modified it out of habbit...

> 
> 2) if you must continue to use the clk-private.h macros (temporarily!)
> then create a new one, DEFINE_CLK_MUX_TABLE.

Hopefully not needed.

Cheers,

Peter.

Re: [PATCH] clk: add table lookup to mux

[Peter De Schrijver]

On Wed, Mar 20, 2013 at 04:51:58PM +0100, Mike Turquette wrote:
> Quoting Peter De Schrijver (2013-03-20 02:49:57)
> > On Wed, Mar 20, 2013 at 12:51:10AM +0100, Mike Turquette wrote:
> > > Quoting Peter De Schrijver (2013-03-12 11:42:23)
> > > > diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h
> > > > index 9c7f580..53d39c2 100644
> > > > --- a/include/linux/clk-private.h
> > > > +++ b/include/linux/clk-private.h
> > > > @@ -144,12 +144,13 @@ struct clk {
> > > >  
> > > >  #define DEFINE_CLK_MUX(_name, _parent_names, _parents, _flags, \
> > > >                                 _reg, _shift, _width,           \
> > > > -                               _mux_flags, _lock)              \
> > > > +                               _mux_flags, _table, _lock)      \
> > > >         static struct clk _name;                                \
> > > >         static struct clk_mux _name##_hw = {                    \
> > > >                 .hw = {                                         \
> > > >                         .clk = &_name,                          \
> > > >                 },                                              \
> > > > +               .table = _table,                                \
> > > >                 .reg = _reg,                                    \
> > > >                 .shift = _shift,                                \
> > > >                 .width = _width,                                \
> > > 
> > > This breaks OMAP horribly since OMAP already uses this macro.  There are
> > > two options:
> > > 
> > > 1) stop using statically initialized data and no longer use
> > > clk-private.h macros.  I was under the impression that the tegra clock
> > > data no longer required this?
> > 
> > Ok. We don't do that anymore. Maybe I just modified it out of habbit...
> > 
> 
> Cool.  Will you send a V2 that drops the macro changes?
> 

Yes. Coming soon :)

Cheers,

Peter.