Re: [PATCH v9 02/15] clk: mux: Split out register accessors for reuse

[Bjorn Andersson]

On Tue 06 Mar 06:38 PST 2018, Sricharan R wrote:

> From: Stephen Boyd <sboyd@xxxxxxxxxxxxxx>
> 
> We want to reuse the logic in clk-mux.c for other clock drivers
> that don't use readl as register accessors. Fortunately, there
> really isn't much to the mux code besides the table indirection
> and quirk flags if you assume any bit shifting and masking has
> been done already. Pull that logic out into reusable functions
> that operate on an optional table and some flags so that other
> drivers can use the same logic.
> 
> [Sricharan: Rebased for mainline]
> Signed-off-by: Sricharan R <sricharan@xxxxxxxxxxxxxx>
> Signed-off-by: Stephen Boyd <sboyd@xxxxxxxxxxxxxx>

This should read as a log, where the first entry is Stephen stating that
he acquired or wrote the code and can release it according to the
license requirements. Then you state that you acquired it, changed it
and are releasing it according to the license requirements.


PS. Please expand your last name.

Regards,
Bjorn

> ---
>  drivers/clk/clk-mux.c         | 74 +++++++++++++++++++++++++++----------------
>  drivers/clk/nxp/clk-lpc32xx.c | 21 +++---------
>  include/linux/clk-provider.h  |  6 ++++
>  3 files changed, 57 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
> index 39cabe1..28223fa 100644
> --- a/drivers/clk/clk-mux.c
> +++ b/drivers/clk/clk-mux.c
> @@ -26,35 +26,25 @@
>   * parent - parent is adjustable through clk_set_parent
>   */
>  
> -static u8 clk_mux_get_parent(struct clk_hw *hw)
> +unsigned int clk_mux_get_parent(struct clk_hw *hw, unsigned int val,
> +				unsigned int *table,
> +				unsigned long flags)
>  {
> -	struct clk_mux *mux = to_clk_mux(hw);
>  	int num_parents = clk_hw_get_num_parents(hw);
> -	u32 val;
> -
> -	/*
> -	 * FIXME need a mux-specific flag to determine if val is bitwise or numeric
> -	 * e.g. sys_clkin_ck's clksel field is 3 bits wide, but ranges from 0x1
> -	 * to 0x7 (index starts at one)
> -	 * OTOH, pmd_trace_clk_mux_ck uses a separate bit for each clock, so
> -	 * val = 0x4 really means "bit 2, index starts at bit 0"
> -	 */
> -	val = clk_readl(mux->reg) >> mux->shift;
> -	val &= mux->mask;
>  
> -	if (mux->table) {
> +	if (table) {
>  		int i;
>  
>  		for (i = 0; i < num_parents; i++)
> -			if (mux->table[i] == val)
> +			if (table[i] == val)
>  				return i;
>  		return -EINVAL;
>  	}
>  
> -	if (val && (mux->flags & CLK_MUX_INDEX_BIT))
> +	if (val && (flags & CLK_MUX_INDEX_BIT))
>  		val = ffs(val) - 1;
>  
> -	if (val && (mux->flags & CLK_MUX_INDEX_ONE))
> +	if (val && (flags & CLK_MUX_INDEX_ONE))
>  		val--;
>  
>  	if (val >= num_parents)
> @@ -62,23 +52,53 @@ 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)
> +static u8 _clk_mux_get_parent(struct clk_hw *hw)
>  {
>  	struct clk_mux *mux = to_clk_mux(hw);
>  	u32 val;
> -	unsigned long flags = 0;
>  
> -	if (mux->table) {
> -		index = mux->table[index];
> +	/*
> +	 * FIXME need a mux-specific flag to determine if val is bitwise or
> +	 * numeric e.g. sys_clkin_ck's clksel field is 3 bits wide,
> +	 * but ranges from 0x1 to 0x7 (index starts at one)
> +	 * OTOH, pmd_trace_clk_mux_ck uses a separate bit for each clock, so
> +	 * val = 0x4 really means "bit 2, index starts at bit 0"
> +	 */
> +	val = clk_readl(mux->reg) >> mux->shift;
> +	val &= mux->mask;
> +
> +	return clk_mux_get_parent(hw, val, mux->table, mux->flags);
> +}
> +
> +unsigned int clk_mux_reindex(u8 index, unsigned int *table,
> +			     unsigned long flags)
> +{
> +	unsigned int val = index;
> +
> +	if (table) {
> +		val = table[val];
>  	} else {
> -		if (mux->flags & CLK_MUX_INDEX_BIT)
> -			index = 1 << index;
> +		if (flags & CLK_MUX_INDEX_BIT)
> +			val = 1 << index;
>  
> -		if (mux->flags & CLK_MUX_INDEX_ONE)
> -			index++;
> +		if (flags & CLK_MUX_INDEX_ONE)
> +			val++;
>  	}
>  
> +	return val;
> +}
> +EXPORT_SYMBOL_GPL(clk_mux_reindex);
> +
> +static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
> +{
> +	struct clk_mux *mux = to_clk_mux(hw);
> +	u32 val;
> +	unsigned long flags = 0;
> +
> +	index = clk_mux_reindex(index, mux->table, mux->flags);
> +
>  	if (mux->lock)
>  		spin_lock_irqsave(mux->lock, flags);
>  	else
> @@ -102,14 +122,14 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
>  }
>  
>  const struct clk_ops clk_mux_ops = {
> -	.get_parent = clk_mux_get_parent,
> +	.get_parent = _clk_mux_get_parent,
>  	.set_parent = clk_mux_set_parent,
>  	.determine_rate = __clk_mux_determine_rate,
>  };
>  EXPORT_SYMBOL_GPL(clk_mux_ops);
>  
>  const struct clk_ops clk_mux_ro_ops = {
> -	.get_parent = clk_mux_get_parent,
> +	.get_parent = _clk_mux_get_parent,
>  };
>  EXPORT_SYMBOL_GPL(clk_mux_ro_ops);
>  
> diff --git a/drivers/clk/nxp/clk-lpc32xx.c b/drivers/clk/nxp/clk-lpc32xx.c
> index f5d815f..9b34150 100644
> --- a/drivers/clk/nxp/clk-lpc32xx.c
> +++ b/drivers/clk/nxp/clk-lpc32xx.c
> @@ -999,29 +999,16 @@ static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
>  	.set_rate = clk_divider_set_rate,
>  };
>  
> -static u8 clk_mux_get_parent(struct clk_hw *hw)
> +static u8 _clk_mux_get_parent(struct clk_hw *hw)
>  {
>  	struct lpc32xx_clk_mux *mux = to_lpc32xx_mux(hw);
> -	u32 num_parents = clk_hw_get_num_parents(hw);
>  	u32 val;
>  
>  	regmap_read(clk_regmap, mux->reg, &val);
>  	val >>= mux->shift;
>  	val &= mux->mask;
>  
> -	if (mux->table) {
> -		u32 i;
> -
> -		for (i = 0; i < num_parents; i++)
> -			if (mux->table[i] == val)
> -				return i;
> -		return -EINVAL;
> -	}
> -
> -	if (val >= num_parents)
> -		return -EINVAL;
> -
> -	return val;
> +	return clk_mux_get_parent(hw, val, mux->table, 0);
>  }
>  
>  static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
> @@ -1036,11 +1023,11 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
>  }
>  
>  static const struct clk_ops lpc32xx_clk_mux_ro_ops = {
> -	.get_parent = clk_mux_get_parent,
> +	.get_parent = _clk_mux_get_parent,
>  };
>  
>  static const struct clk_ops lpc32xx_clk_mux_ops = {
> -	.get_parent = clk_mux_get_parent,
> +	.get_parent = _clk_mux_get_parent,
>  	.set_parent = clk_mux_set_parent,
>  	.determine_rate = __clk_mux_determine_rate,
>  };
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index f711be6..344ad92 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -488,6 +488,12 @@ struct clk_mux {
>  extern const struct clk_ops clk_mux_ops;
>  extern const struct clk_ops clk_mux_ro_ops;
>  
> +unsigned int clk_mux_get_parent(struct clk_hw *hw, unsigned int val,
> +				unsigned int *table,
> +				unsigned long flags);
> +unsigned int clk_mux_reindex(u8 index, unsigned int *table,
> +			     unsigned long flags);
> +
>  struct clk *clk_register_mux(struct device *dev, const char *name,
>  		const char * const *parent_names, u8 num_parents,
>  		unsigned long flags,
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>