diff mbox series

[v2,1/2] clk: mediatek: clk-mux: Support custom parent indices for muxes

Message ID 20231018103546.48174-2-angelogioacchino.delregno@collabora.com
State New
Headers
Series MediaTek clocks: Support mux indices list and 8195 DP |

Commit Message

AngeloGioacchino Del Regno Oct. 18, 2023, 10:35 a.m. UTC 
  Add support for customized parent indices for MediaTek muxes: this is
necessary for the case in which we want to exclude some clocks from
a mux's parent clocks list, where the exclusions are not from the
very bottom of the list but either in the middle or the beginning.

Example:
- MUX1 (all parents)
  - parent1; idx=0
  - parent2; idx=1
  - parent3; idx=2

- MUX1 (wanted parents)
  - parent1; idx=0
  - parent3; idx=2

To achieve that add a `parent_index` array pointer to struct mtk_mux,
then in .set_parent(), .get_parent() callbacks check if this array
was populated and eventually get the index from that.

Also, to avoid updating all clock drivers for all SoCs, rename the
"main" macro to __GATE_CLR_SET_UPD_FLAGS (so, `__` was added) and
add the new member to it; furthermore, GATE_CLK_SET_UPD_FLAGS has
been reintroduced as being fully compatible with the older version.

The new parent_index can be specified with the new `_INDEXED`
variants of the MUX_GATE_CLR_SET_UPD_xxxx macros.

Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/clk/mediatek/clk-mux.c | 14 +++++++++++
 drivers/clk/mediatek/clk-mux.h | 43 ++++++++++++++++++++++++++++++----
 2 files changed, 53 insertions(+), 4 deletions(-)

Comments

Markus Schneider-Pargmann Oct. 23, 2023, 6:43 a.m. UTC | #1 
Hi Angelo,

On Wed, Oct 18, 2023 at 12:35:45PM +0200, AngeloGioacchino Del Regno wrote:
> Add support for customized parent indices for MediaTek muxes: this is
> necessary for the case in which we want to exclude some clocks from
> a mux's parent clocks list, where the exclusions are not from the
> very bottom of the list but either in the middle or the beginning.
> 
> Example:
> - MUX1 (all parents)
>   - parent1; idx=0
>   - parent2; idx=1
>   - parent3; idx=2
> 
> - MUX1 (wanted parents)
>   - parent1; idx=0
>   - parent3; idx=2
> 
> To achieve that add a `parent_index` array pointer to struct mtk_mux,
> then in .set_parent(), .get_parent() callbacks check if this array
> was populated and eventually get the index from that.
> 
> Also, to avoid updating all clock drivers for all SoCs, rename the
> "main" macro to __GATE_CLR_SET_UPD_FLAGS (so, `__` was added) and
> add the new member to it; furthermore, GATE_CLK_SET_UPD_FLAGS has
> been reintroduced as being fully compatible with the older version.
> 
> The new parent_index can be specified with the new `_INDEXED`
> variants of the MUX_GATE_CLR_SET_UPD_xxxx macros.
> 
> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  drivers/clk/mediatek/clk-mux.c | 14 +++++++++++
>  drivers/clk/mediatek/clk-mux.h | 43 ++++++++++++++++++++++++++++++----
>  2 files changed, 53 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clk/mediatek/clk-mux.c b/drivers/clk/mediatek/clk-mux.c
> index c93bc7f926e5..60990296450b 100644
> --- a/drivers/clk/mediatek/clk-mux.c
> +++ b/drivers/clk/mediatek/clk-mux.c
> @@ -89,6 +89,17 @@ static u8 mtk_clk_mux_get_parent(struct clk_hw *hw)
>  	regmap_read(mux->regmap, mux->data->mux_ofs, &val);
>  	val = (val >> mux->data->mux_shift) & mask;
>  
> +	if (mux->data->parent_index) {
> +		int i;
> +
> +		for (i = 0; i < mux->data->num_parents; i++)
> +			if (mux->data->parent_index[i] == val)
> +				return i;
> +
> +		/* Not found: return an impossible index to generate error */
> +		return mux->data->num_parents + 1;
> +	}
> +
>  	return val;
>  }
>  
> @@ -104,6 +115,9 @@ static int mtk_clk_mux_set_parent_setclr_lock(struct clk_hw *hw, u8 index)
>  	else
>  		__acquire(mux->lock);
>  
> +	if (mux->data->parent_index)
> +		index = mux->data->parent_index[index];
> +
>  	regmap_read(mux->regmap, mux->data->mux_ofs, &orig);
>  	val = (orig & ~(mask << mux->data->mux_shift))
>  			| (index << mux->data->mux_shift);
> diff --git a/drivers/clk/mediatek/clk-mux.h b/drivers/clk/mediatek/clk-mux.h
> index 7ecb963b0ec6..943ad1d7ce4b 100644
> --- a/drivers/clk/mediatek/clk-mux.h
> +++ b/drivers/clk/mediatek/clk-mux.h
> @@ -21,6 +21,7 @@ struct mtk_mux {
>  	int id;
>  	const char *name;
>  	const char * const *parent_names;
> +	const u8 *parent_index;
>  	unsigned int flags;

I think at some point it would be nice to have a documentation of these
fields.

>  
>  	u32 mux_ofs;
> @@ -37,9 +38,10 @@ struct mtk_mux {
>  	signed char num_parents;
>  };
>  
> -#define GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs,		\
> -			_mux_set_ofs, _mux_clr_ofs, _shift, _width,	\
> -			_gate, _upd_ofs, _upd, _flags, _ops) {		\
> +#define __GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _paridx,		\
> +			 _num_parents, _mux_ofs, _mux_set_ofs,		\
> +			 _mux_clr_ofs, _shift, _width, _gate, _upd_ofs,	\
> +			 _upd, _flags, _ops) {				\
>  		.id = _id,						\
>  		.name = _name,						\
>  		.mux_ofs = _mux_ofs,					\
> @@ -51,11 +53,28 @@ struct mtk_mux {
>  		.gate_shift = _gate,					\
>  		.upd_shift = _upd,					\
>  		.parent_names = _parents,				\
> -		.num_parents = ARRAY_SIZE(_parents),			\
> +		.parent_index = _paridx,				\
> +		.num_parents = _num_parents,				\

I was wondering why you moved the ARRAY_SIZE() to the outer macros and
add another argument to the already huge list of arguments? I couldn't
find a use-case for this in the patches you sent.

Best,
Markus

>  		.flags = _flags,					\
>  		.ops = &_ops,						\
>  	}
>  
> +#define GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs,		\
> +			_mux_set_ofs, _mux_clr_ofs, _shift, _width,	\
> +			_gate, _upd_ofs, _upd, _flags, _ops)		\
> +		__GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents,		\
> +			NULL, ARRAY_SIZE(_parents), _mux_ofs,		\
> +			_mux_set_ofs, _mux_clr_ofs, _shift, _width,	\
> +			_gate, _upd_ofs, _upd, _flags, _ops)		\
> +
> +#define GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents, _paridx,	\
> +			 _mux_ofs, _mux_set_ofs, _mux_clr_ofs, _shift,	\
> +			 _width, _gate, _upd_ofs, _upd, _flags, _ops)	\
> +		__GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents,		\
> +			_paridx, ARRAY_SIZE(_paridx), _mux_ofs,		\
> +			_mux_set_ofs, _mux_clr_ofs, _shift, _width,	\
> +			_gate, _upd_ofs, _upd, _flags, _ops)		\
> +
>  extern const struct clk_ops mtk_mux_clr_set_upd_ops;
>  extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
>  
> @@ -67,6 +86,14 @@ extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
>  			_gate, _upd_ofs, _upd, _flags,			\
>  			mtk_mux_gate_clr_set_upd_ops)
>  
> +#define MUX_GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents,	\
> +			_paridx, _mux_ofs, _mux_set_ofs, _mux_clr_ofs,	\
> +			_shift, _width, _gate, _upd_ofs, _upd, _flags)	\
> +		GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents,	\
> +			_paridx, _mux_ofs, _mux_set_ofs, _mux_clr_ofs,	\
> +			_shift, _width, _gate, _upd_ofs, _upd, _flags,	\
> +			mtk_mux_gate_clr_set_upd_ops)
> +
>  #define MUX_GATE_CLR_SET_UPD(_id, _name, _parents, _mux_ofs,		\
>  			_mux_set_ofs, _mux_clr_ofs, _shift, _width,	\
>  			_gate, _upd_ofs, _upd)				\
> @@ -75,6 +102,14 @@ extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
>  			_width, _gate, _upd_ofs, _upd,			\
>  			CLK_SET_RATE_PARENT)
>  
> +#define MUX_GATE_CLR_SET_UPD_INDEXED(_id, _name, _parents, _paridx,	\
> +			_mux_ofs, _mux_set_ofs, _mux_clr_ofs, _shift,	\
> +			_width, _gate, _upd_ofs, _upd)			\
> +		MUX_GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name,		\
> +			_parents, _paridx, _mux_ofs, _mux_set_ofs,	\
> +			_mux_clr_ofs, _shift, _width, _gate, _upd_ofs,	\
> +			_upd, CLK_SET_RATE_PARENT)
> +
>  #define MUX_CLR_SET_UPD(_id, _name, _parents, _mux_ofs,			\
>  			_mux_set_ofs, _mux_clr_ofs, _shift, _width,	\
>  			_upd_ofs, _upd)					\
> -- 
> 2.42.0
>
Chen-Yu Tsai Oct. 23, 2023, 6:54 a.m. UTC | #2 
On Mon, Oct 23, 2023 at 2:44 PM Markus Schneider-Pargmann
<msp@baylibre.com> wrote:
>
> Hi Angelo,
>
> On Wed, Oct 18, 2023 at 12:35:45PM +0200, AngeloGioacchino Del Regno wrote:
> > Add support for customized parent indices for MediaTek muxes: this is
> > necessary for the case in which we want to exclude some clocks from
> > a mux's parent clocks list, where the exclusions are not from the
> > very bottom of the list but either in the middle or the beginning.
> >
> > Example:
> > - MUX1 (all parents)
> >   - parent1; idx=0
> >   - parent2; idx=1
> >   - parent3; idx=2
> >
> > - MUX1 (wanted parents)
> >   - parent1; idx=0
> >   - parent3; idx=2
> >
> > To achieve that add a `parent_index` array pointer to struct mtk_mux,
> > then in .set_parent(), .get_parent() callbacks check if this array
> > was populated and eventually get the index from that.
> >
> > Also, to avoid updating all clock drivers for all SoCs, rename the
> > "main" macro to __GATE_CLR_SET_UPD_FLAGS (so, `__` was added) and
> > add the new member to it; furthermore, GATE_CLK_SET_UPD_FLAGS has
> > been reintroduced as being fully compatible with the older version.
> >
> > The new parent_index can be specified with the new `_INDEXED`
> > variants of the MUX_GATE_CLR_SET_UPD_xxxx macros.
> >
> > Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
> > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> > ---
> >  drivers/clk/mediatek/clk-mux.c | 14 +++++++++++
> >  drivers/clk/mediatek/clk-mux.h | 43 ++++++++++++++++++++++++++++++----
> >  2 files changed, 53 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/clk/mediatek/clk-mux.c b/drivers/clk/mediatek/clk-mux.c
> > index c93bc7f926e5..60990296450b 100644
> > --- a/drivers/clk/mediatek/clk-mux.c
> > +++ b/drivers/clk/mediatek/clk-mux.c
> > @@ -89,6 +89,17 @@ static u8 mtk_clk_mux_get_parent(struct clk_hw *hw)
> >       regmap_read(mux->regmap, mux->data->mux_ofs, &val);
> >       val = (val >> mux->data->mux_shift) & mask;
> >
> > +     if (mux->data->parent_index) {
> > +             int i;
> > +
> > +             for (i = 0; i < mux->data->num_parents; i++)
> > +                     if (mux->data->parent_index[i] == val)
> > +                             return i;
> > +
> > +             /* Not found: return an impossible index to generate error */
> > +             return mux->data->num_parents + 1;
> > +     }
> > +
> >       return val;
> >  }
> >
> > @@ -104,6 +115,9 @@ static int mtk_clk_mux_set_parent_setclr_lock(struct clk_hw *hw, u8 index)
> >       else
> >               __acquire(mux->lock);
> >
> > +     if (mux->data->parent_index)
> > +             index = mux->data->parent_index[index];
> > +
> >       regmap_read(mux->regmap, mux->data->mux_ofs, &orig);
> >       val = (orig & ~(mask << mux->data->mux_shift))
> >                       | (index << mux->data->mux_shift);
> > diff --git a/drivers/clk/mediatek/clk-mux.h b/drivers/clk/mediatek/clk-mux.h
> > index 7ecb963b0ec6..943ad1d7ce4b 100644
> > --- a/drivers/clk/mediatek/clk-mux.h
> > +++ b/drivers/clk/mediatek/clk-mux.h
> > @@ -21,6 +21,7 @@ struct mtk_mux {
> >       int id;
> >       const char *name;
> >       const char * const *parent_names;
> > +     const u8 *parent_index;
> >       unsigned int flags;
>
> I think at some point it would be nice to have a documentation of these
> fields.
>
> >
> >       u32 mux_ofs;
> > @@ -37,9 +38,10 @@ struct mtk_mux {
> >       signed char num_parents;
> >  };
> >
> > -#define GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs,               \
> > -                     _mux_set_ofs, _mux_clr_ofs, _shift, _width,     \
> > -                     _gate, _upd_ofs, _upd, _flags, _ops) {          \
> > +#define __GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _paridx,              \
> > +                      _num_parents, _mux_ofs, _mux_set_ofs,          \
> > +                      _mux_clr_ofs, _shift, _width, _gate, _upd_ofs, \
> > +                      _upd, _flags, _ops) {                          \
> >               .id = _id,                                              \
> >               .name = _name,                                          \
> >               .mux_ofs = _mux_ofs,                                    \
> > @@ -51,11 +53,28 @@ struct mtk_mux {
> >               .gate_shift = _gate,                                    \
> >               .upd_shift = _upd,                                      \
> >               .parent_names = _parents,                               \
> > -             .num_parents = ARRAY_SIZE(_parents),                    \
> > +             .parent_index = _paridx,                                \
> > +             .num_parents = _num_parents,                            \
>
> I was wondering why you moved the ARRAY_SIZE() to the outer macros and
> add another argument to the already huge list of arguments? I couldn't
> find a use-case for this in the patches you sent.

This is the base macro that all the other ones are based on, so it has
the complete list of arguments. The other ones use this and provide
default values for the arguments they don't need.

ChenYu

> Best,
> Markus
>
> >               .flags = _flags,                                        \
> >               .ops = &_ops,                                           \
> >       }
> >
> > +#define GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs,               \
> > +                     _mux_set_ofs, _mux_clr_ofs, _shift, _width,     \
> > +                     _gate, _upd_ofs, _upd, _flags, _ops)            \
> > +             __GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents,          \
> > +                     NULL, ARRAY_SIZE(_parents), _mux_ofs,           \
> > +                     _mux_set_ofs, _mux_clr_ofs, _shift, _width,     \
> > +                     _gate, _upd_ofs, _upd, _flags, _ops)            \
> > +
> > +#define GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents, _paridx,        \
> > +                      _mux_ofs, _mux_set_ofs, _mux_clr_ofs, _shift,  \
> > +                      _width, _gate, _upd_ofs, _upd, _flags, _ops)   \
> > +             __GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents,          \
> > +                     _paridx, ARRAY_SIZE(_paridx), _mux_ofs,         \
> > +                     _mux_set_ofs, _mux_clr_ofs, _shift, _width,     \
> > +                     _gate, _upd_ofs, _upd, _flags, _ops)            \
> > +
> >  extern const struct clk_ops mtk_mux_clr_set_upd_ops;
> >  extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
> >
> > @@ -67,6 +86,14 @@ extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
> >                       _gate, _upd_ofs, _upd, _flags,                  \
> >                       mtk_mux_gate_clr_set_upd_ops)
> >
> > +#define MUX_GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents,     \
> > +                     _paridx, _mux_ofs, _mux_set_ofs, _mux_clr_ofs,  \
> > +                     _shift, _width, _gate, _upd_ofs, _upd, _flags)  \
> > +             GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents,    \
> > +                     _paridx, _mux_ofs, _mux_set_ofs, _mux_clr_ofs,  \
> > +                     _shift, _width, _gate, _upd_ofs, _upd, _flags,  \
> > +                     mtk_mux_gate_clr_set_upd_ops)
> > +
> >  #define MUX_GATE_CLR_SET_UPD(_id, _name, _parents, _mux_ofs,         \
> >                       _mux_set_ofs, _mux_clr_ofs, _shift, _width,     \
> >                       _gate, _upd_ofs, _upd)                          \
> > @@ -75,6 +102,14 @@ extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
> >                       _width, _gate, _upd_ofs, _upd,                  \
> >                       CLK_SET_RATE_PARENT)
> >
> > +#define MUX_GATE_CLR_SET_UPD_INDEXED(_id, _name, _parents, _paridx,  \
> > +                     _mux_ofs, _mux_set_ofs, _mux_clr_ofs, _shift,   \
> > +                     _width, _gate, _upd_ofs, _upd)                  \
> > +             MUX_GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name,          \
> > +                     _parents, _paridx, _mux_ofs, _mux_set_ofs,      \
> > +                     _mux_clr_ofs, _shift, _width, _gate, _upd_ofs,  \
> > +                     _upd, CLK_SET_RATE_PARENT)
> > +
> >  #define MUX_CLR_SET_UPD(_id, _name, _parents, _mux_ofs,                      \
> >                       _mux_set_ofs, _mux_clr_ofs, _shift, _width,     \
> >                       _upd_ofs, _upd)                                 \
> > --
> > 2.42.0
> >
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Chen-Yu Tsai Oct. 23, 2023, 6:57 a.m. UTC | #3 
On Wed, Oct 18, 2023 at 6:36 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Add support for customized parent indices for MediaTek muxes: this is
> necessary for the case in which we want to exclude some clocks from
> a mux's parent clocks list, where the exclusions are not from the
> very bottom of the list but either in the middle or the beginning.
>
> Example:
> - MUX1 (all parents)
>   - parent1; idx=0
>   - parent2; idx=1
>   - parent3; idx=2
>
> - MUX1 (wanted parents)
>   - parent1; idx=0
>   - parent3; idx=2
>
> To achieve that add a `parent_index` array pointer to struct mtk_mux,
> then in .set_parent(), .get_parent() callbacks check if this array
> was populated and eventually get the index from that.
>
> Also, to avoid updating all clock drivers for all SoCs, rename the
> "main" macro to __GATE_CLR_SET_UPD_FLAGS (so, `__` was added) and
> add the new member to it; furthermore, GATE_CLK_SET_UPD_FLAGS has
> been reintroduced as being fully compatible with the older version.
>
> The new parent_index can be specified with the new `_INDEXED`
> variants of the MUX_GATE_CLR_SET_UPD_xxxx macros.
>
> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  drivers/clk/mediatek/clk-mux.c | 14 +++++++++++
>  drivers/clk/mediatek/clk-mux.h | 43 ++++++++++++++++++++++++++++++----
>  2 files changed, 53 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clk/mediatek/clk-mux.c b/drivers/clk/mediatek/clk-mux.c
> index c93bc7f926e5..60990296450b 100644
> --- a/drivers/clk/mediatek/clk-mux.c
> +++ b/drivers/clk/mediatek/clk-mux.c
> @@ -89,6 +89,17 @@ static u8 mtk_clk_mux_get_parent(struct clk_hw *hw)
>         regmap_read(mux->regmap, mux->data->mux_ofs, &val);
>         val = (val >> mux->data->mux_shift) & mask;
>
> +       if (mux->data->parent_index) {
> +               int i;
> +
> +               for (i = 0; i < mux->data->num_parents; i++)
> +                       if (mux->data->parent_index[i] == val)
> +                               return i;
> +
> +               /* Not found: return an impossible index to generate error */
> +               return mux->data->num_parents + 1;
> +       }
> +
>         return val;
>  }
>
> @@ -104,6 +115,9 @@ static int mtk_clk_mux_set_parent_setclr_lock(struct clk_hw *hw, u8 index)
>         else
>                 __acquire(mux->lock);
>
> +       if (mux->data->parent_index)
> +               index = mux->data->parent_index[index];
> +
>         regmap_read(mux->regmap, mux->data->mux_ofs, &orig);
>         val = (orig & ~(mask << mux->data->mux_shift))
>                         | (index << mux->data->mux_shift);
> diff --git a/drivers/clk/mediatek/clk-mux.h b/drivers/clk/mediatek/clk-mux.h
> index 7ecb963b0ec6..943ad1d7ce4b 100644
> --- a/drivers/clk/mediatek/clk-mux.h
> +++ b/drivers/clk/mediatek/clk-mux.h
> @@ -21,6 +21,7 @@ struct mtk_mux {
>         int id;
>         const char *name;
>         const char * const *parent_names;
> +       const u8 *parent_index;
>         unsigned int flags;
>
>         u32 mux_ofs;
> @@ -37,9 +38,10 @@ struct mtk_mux {
>         signed char num_parents;
>  };
>
> -#define GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs,         \
> -                       _mux_set_ofs, _mux_clr_ofs, _shift, _width,     \
> -                       _gate, _upd_ofs, _upd, _flags, _ops) {          \
> +#define __GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _paridx,                \
> +                        _num_parents, _mux_ofs, _mux_set_ofs,          \
> +                        _mux_clr_ofs, _shift, _width, _gate, _upd_ofs, \
> +                        _upd, _flags, _ops) {                          \

Nit: I would take this opportunity to fix up the alignment of the lines
here, as well as for the new macros. Also you needn't wrap the lines at
80 characters. Instead I would wrap where it makes more sense, like group
arguments for the same type (parents / mux / gate / update ...) on the
same line if possible.

Otherwise,

Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>

>                 .id = _id,                                              \
>                 .name = _name,                                          \
>                 .mux_ofs = _mux_ofs,                                    \
> @@ -51,11 +53,28 @@ struct mtk_mux {
>                 .gate_shift = _gate,                                    \
>                 .upd_shift = _upd,                                      \
>                 .parent_names = _parents,                               \
> -               .num_parents = ARRAY_SIZE(_parents),                    \
> +               .parent_index = _paridx,                                \
> +               .num_parents = _num_parents,                            \
>                 .flags = _flags,                                        \
>                 .ops = &_ops,                                           \
>         }
>
> +#define GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs,         \
> +                       _mux_set_ofs, _mux_clr_ofs, _shift, _width,     \
> +                       _gate, _upd_ofs, _upd, _flags, _ops)            \
> +               __GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents,          \
> +                       NULL, ARRAY_SIZE(_parents), _mux_ofs,           \
> +                       _mux_set_ofs, _mux_clr_ofs, _shift, _width,     \
> +                       _gate, _upd_ofs, _upd, _flags, _ops)            \
> +
> +#define GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents, _paridx,  \
> +                        _mux_ofs, _mux_set_ofs, _mux_clr_ofs, _shift,  \
> +                        _width, _gate, _upd_ofs, _upd, _flags, _ops)   \
> +               __GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents,          \
> +                       _paridx, ARRAY_SIZE(_paridx), _mux_ofs,         \
> +                       _mux_set_ofs, _mux_clr_ofs, _shift, _width,     \
> +                       _gate, _upd_ofs, _upd, _flags, _ops)            \
> +
>  extern const struct clk_ops mtk_mux_clr_set_upd_ops;
>  extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
>
> @@ -67,6 +86,14 @@ extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
>                         _gate, _upd_ofs, _upd, _flags,                  \
>                         mtk_mux_gate_clr_set_upd_ops)
>
> +#define MUX_GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents,       \
> +                       _paridx, _mux_ofs, _mux_set_ofs, _mux_clr_ofs,  \
> +                       _shift, _width, _gate, _upd_ofs, _upd, _flags)  \
> +               GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents,    \
> +                       _paridx, _mux_ofs, _mux_set_ofs, _mux_clr_ofs,  \
> +                       _shift, _width, _gate, _upd_ofs, _upd, _flags,  \
> +                       mtk_mux_gate_clr_set_upd_ops)
> +
>  #define MUX_GATE_CLR_SET_UPD(_id, _name, _parents, _mux_ofs,           \
>                         _mux_set_ofs, _mux_clr_ofs, _shift, _width,     \
>                         _gate, _upd_ofs, _upd)                          \
> @@ -75,6 +102,14 @@ extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
>                         _width, _gate, _upd_ofs, _upd,                  \
>                         CLK_SET_RATE_PARENT)
>
> +#define MUX_GATE_CLR_SET_UPD_INDEXED(_id, _name, _parents, _paridx,    \
> +                       _mux_ofs, _mux_set_ofs, _mux_clr_ofs, _shift,   \
> +                       _width, _gate, _upd_ofs, _upd)                  \
> +               MUX_GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name,          \
> +                       _parents, _paridx, _mux_ofs, _mux_set_ofs,      \
> +                       _mux_clr_ofs, _shift, _width, _gate, _upd_ofs,  \
> +                       _upd, CLK_SET_RATE_PARENT)
> +
>  #define MUX_CLR_SET_UPD(_id, _name, _parents, _mux_ofs,                        \
>                         _mux_set_ofs, _mux_clr_ofs, _shift, _width,     \
>                         _upd_ofs, _upd)                                 \
> --
> 2.42.0
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Chen-Yu Tsai Oct. 23, 2023, 6:58 a.m. UTC | #4 
On Mon, Oct 23, 2023 at 2:57 PM Chen-Yu Tsai <wens@kernel.org> wrote:
>
> On Wed, Oct 18, 2023 at 6:36 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
> >
> > Add support for customized parent indices for MediaTek muxes: this is
> > necessary for the case in which we want to exclude some clocks from
> > a mux's parent clocks list, where the exclusions are not from the
> > very bottom of the list but either in the middle or the beginning.
> >
> > Example:
> > - MUX1 (all parents)
> >   - parent1; idx=0
> >   - parent2; idx=1
> >   - parent3; idx=2
> >
> > - MUX1 (wanted parents)
> >   - parent1; idx=0
> >   - parent3; idx=2
> >
> > To achieve that add a `parent_index` array pointer to struct mtk_mux,
> > then in .set_parent(), .get_parent() callbacks check if this array
> > was populated and eventually get the index from that.
> >
> > Also, to avoid updating all clock drivers for all SoCs, rename the
> > "main" macro to __GATE_CLR_SET_UPD_FLAGS (so, `__` was added) and
> > add the new member to it; furthermore, GATE_CLK_SET_UPD_FLAGS has
> > been reintroduced as being fully compatible with the older version.
> >
> > The new parent_index can be specified with the new `_INDEXED`
> > variants of the MUX_GATE_CLR_SET_UPD_xxxx macros.
> >
> > Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
> > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> > ---
> >  drivers/clk/mediatek/clk-mux.c | 14 +++++++++++
> >  drivers/clk/mediatek/clk-mux.h | 43 ++++++++++++++++++++++++++++++----
> >  2 files changed, 53 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/clk/mediatek/clk-mux.c b/drivers/clk/mediatek/clk-mux.c
> > index c93bc7f926e5..60990296450b 100644
> > --- a/drivers/clk/mediatek/clk-mux.c
> > +++ b/drivers/clk/mediatek/clk-mux.c
> > @@ -89,6 +89,17 @@ static u8 mtk_clk_mux_get_parent(struct clk_hw *hw)
> >         regmap_read(mux->regmap, mux->data->mux_ofs, &val);
> >         val = (val >> mux->data->mux_shift) & mask;
> >
> > +       if (mux->data->parent_index) {
> > +               int i;
> > +
> > +               for (i = 0; i < mux->data->num_parents; i++)
> > +                       if (mux->data->parent_index[i] == val)
> > +                               return i;
> > +
> > +               /* Not found: return an impossible index to generate error */
> > +               return mux->data->num_parents + 1;
> > +       }
> > +
> >         return val;
> >  }
> >
> > @@ -104,6 +115,9 @@ static int mtk_clk_mux_set_parent_setclr_lock(struct clk_hw *hw, u8 index)
> >         else
> >                 __acquire(mux->lock);
> >
> > +       if (mux->data->parent_index)
> > +               index = mux->data->parent_index[index];
> > +
> >         regmap_read(mux->regmap, mux->data->mux_ofs, &orig);
> >         val = (orig & ~(mask << mux->data->mux_shift))
> >                         | (index << mux->data->mux_shift);
> > diff --git a/drivers/clk/mediatek/clk-mux.h b/drivers/clk/mediatek/clk-mux.h
> > index 7ecb963b0ec6..943ad1d7ce4b 100644
> > --- a/drivers/clk/mediatek/clk-mux.h
> > +++ b/drivers/clk/mediatek/clk-mux.h
> > @@ -21,6 +21,7 @@ struct mtk_mux {
> >         int id;
> >         const char *name;
> >         const char * const *parent_names;
> > +       const u8 *parent_index;
> >         unsigned int flags;
> >
> >         u32 mux_ofs;
> > @@ -37,9 +38,10 @@ struct mtk_mux {
> >         signed char num_parents;
> >  };
> >
> > -#define GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs,         \
> > -                       _mux_set_ofs, _mux_clr_ofs, _shift, _width,     \
> > -                       _gate, _upd_ofs, _upd, _flags, _ops) {          \
> > +#define __GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _paridx,                \
> > +                        _num_parents, _mux_ofs, _mux_set_ofs,          \
> > +                        _mux_clr_ofs, _shift, _width, _gate, _upd_ofs, \
> > +                        _upd, _flags, _ops) {                          \
>
> Nit: I would take this opportunity to fix up the alignment of the lines
> here, as well as for the new macros. Also you needn't wrap the lines at
> 80 characters. Instead I would wrap where it makes more sense, like group
> arguments for the same type (parents / mux / gate / update ...) on the
> same line if possible.
>
> Otherwise,
>
> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>

And of course I meant to send this from my chromium.org address...

Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>

> >                 .id = _id,                                              \
> >                 .name = _name,                                          \
> >                 .mux_ofs = _mux_ofs,                                    \
> > @@ -51,11 +53,28 @@ struct mtk_mux {
> >                 .gate_shift = _gate,                                    \
> >                 .upd_shift = _upd,                                      \
> >                 .parent_names = _parents,                               \
> > -               .num_parents = ARRAY_SIZE(_parents),                    \
> > +               .parent_index = _paridx,                                \
> > +               .num_parents = _num_parents,                            \
> >                 .flags = _flags,                                        \
> >                 .ops = &_ops,                                           \
> >         }
> >
> > +#define GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs,         \
> > +                       _mux_set_ofs, _mux_clr_ofs, _shift, _width,     \
> > +                       _gate, _upd_ofs, _upd, _flags, _ops)            \
> > +               __GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents,          \
> > +                       NULL, ARRAY_SIZE(_parents), _mux_ofs,           \
> > +                       _mux_set_ofs, _mux_clr_ofs, _shift, _width,     \
> > +                       _gate, _upd_ofs, _upd, _flags, _ops)            \
> > +
> > +#define GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents, _paridx,  \
> > +                        _mux_ofs, _mux_set_ofs, _mux_clr_ofs, _shift,  \
> > +                        _width, _gate, _upd_ofs, _upd, _flags, _ops)   \
> > +               __GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents,          \
> > +                       _paridx, ARRAY_SIZE(_paridx), _mux_ofs,         \
> > +                       _mux_set_ofs, _mux_clr_ofs, _shift, _width,     \
> > +                       _gate, _upd_ofs, _upd, _flags, _ops)            \
> > +
> >  extern const struct clk_ops mtk_mux_clr_set_upd_ops;
> >  extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
> >
> > @@ -67,6 +86,14 @@ extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
> >                         _gate, _upd_ofs, _upd, _flags,                  \
> >                         mtk_mux_gate_clr_set_upd_ops)
> >
> > +#define MUX_GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents,       \
> > +                       _paridx, _mux_ofs, _mux_set_ofs, _mux_clr_ofs,  \
> > +                       _shift, _width, _gate, _upd_ofs, _upd, _flags)  \
> > +               GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents,    \
> > +                       _paridx, _mux_ofs, _mux_set_ofs, _mux_clr_ofs,  \
> > +                       _shift, _width, _gate, _upd_ofs, _upd, _flags,  \
> > +                       mtk_mux_gate_clr_set_upd_ops)
> > +
> >  #define MUX_GATE_CLR_SET_UPD(_id, _name, _parents, _mux_ofs,           \
> >                         _mux_set_ofs, _mux_clr_ofs, _shift, _width,     \
> >                         _gate, _upd_ofs, _upd)                          \
> > @@ -75,6 +102,14 @@ extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
> >                         _width, _gate, _upd_ofs, _upd,                  \
> >                         CLK_SET_RATE_PARENT)
> >
> > +#define MUX_GATE_CLR_SET_UPD_INDEXED(_id, _name, _parents, _paridx,    \
> > +                       _mux_ofs, _mux_set_ofs, _mux_clr_ofs, _shift,   \
> > +                       _width, _gate, _upd_ofs, _upd)                  \
> > +               MUX_GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name,          \
> > +                       _parents, _paridx, _mux_ofs, _mux_set_ofs,      \
> > +                       _mux_clr_ofs, _shift, _width, _gate, _upd_ofs,  \
> > +                       _upd, CLK_SET_RATE_PARENT)
> > +
> >  #define MUX_CLR_SET_UPD(_id, _name, _parents, _mux_ofs,                        \
> >                         _mux_set_ofs, _mux_clr_ofs, _shift, _width,     \
> >                         _upd_ofs, _upd)                                 \
> > --
> > 2.42.0
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff mbox series

Patch

diff --git a/drivers/clk/mediatek/clk-mux.c b/drivers/clk/mediatek/clk-mux.c
index c93bc7f926e5..60990296450b 100644
--- a/drivers/clk/mediatek/clk-mux.c
+++ b/drivers/clk/mediatek/clk-mux.c
@@ -89,6 +89,17 @@  static u8 mtk_clk_mux_get_parent(struct clk_hw *hw)
 	regmap_read(mux->regmap, mux->data->mux_ofs, &val);
 	val = (val >> mux->data->mux_shift) & mask;
 
+	if (mux->data->parent_index) {
+		int i;
+
+		for (i = 0; i < mux->data->num_parents; i++)
+			if (mux->data->parent_index[i] == val)
+				return i;
+
+		/* Not found: return an impossible index to generate error */
+		return mux->data->num_parents + 1;
+	}
+
 	return val;
 }
 
@@ -104,6 +115,9 @@  static int mtk_clk_mux_set_parent_setclr_lock(struct clk_hw *hw, u8 index)
 	else
 		__acquire(mux->lock);
 
+	if (mux->data->parent_index)
+		index = mux->data->parent_index[index];
+
 	regmap_read(mux->regmap, mux->data->mux_ofs, &orig);
 	val = (orig & ~(mask << mux->data->mux_shift))
 			| (index << mux->data->mux_shift);
diff --git a/drivers/clk/mediatek/clk-mux.h b/drivers/clk/mediatek/clk-mux.h
index 7ecb963b0ec6..943ad1d7ce4b 100644
--- a/drivers/clk/mediatek/clk-mux.h
+++ b/drivers/clk/mediatek/clk-mux.h
@@ -21,6 +21,7 @@  struct mtk_mux {
 	int id;
 	const char *name;
 	const char * const *parent_names;
+	const u8 *parent_index;
 	unsigned int flags;
 
 	u32 mux_ofs;
@@ -37,9 +38,10 @@  struct mtk_mux {
 	signed char num_parents;
 };
 
-#define GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs,		\
-			_mux_set_ofs, _mux_clr_ofs, _shift, _width,	\
-			_gate, _upd_ofs, _upd, _flags, _ops) {		\
+#define __GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _paridx,		\
+			 _num_parents, _mux_ofs, _mux_set_ofs,		\
+			 _mux_clr_ofs, _shift, _width, _gate, _upd_ofs,	\
+			 _upd, _flags, _ops) {				\
 		.id = _id,						\
 		.name = _name,						\
 		.mux_ofs = _mux_ofs,					\
@@ -51,11 +53,28 @@  struct mtk_mux {
 		.gate_shift = _gate,					\
 		.upd_shift = _upd,					\
 		.parent_names = _parents,				\
-		.num_parents = ARRAY_SIZE(_parents),			\
+		.parent_index = _paridx,				\
+		.num_parents = _num_parents,				\
 		.flags = _flags,					\
 		.ops = &_ops,						\
 	}
 
+#define GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs,		\
+			_mux_set_ofs, _mux_clr_ofs, _shift, _width,	\
+			_gate, _upd_ofs, _upd, _flags, _ops)		\
+		__GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents,		\
+			NULL, ARRAY_SIZE(_parents), _mux_ofs,		\
+			_mux_set_ofs, _mux_clr_ofs, _shift, _width,	\
+			_gate, _upd_ofs, _upd, _flags, _ops)		\
+
+#define GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents, _paridx,	\
+			 _mux_ofs, _mux_set_ofs, _mux_clr_ofs, _shift,	\
+			 _width, _gate, _upd_ofs, _upd, _flags, _ops)	\
+		__GATE_CLR_SET_UPD_FLAGS(_id, _name, _parents,		\
+			_paridx, ARRAY_SIZE(_paridx), _mux_ofs,		\
+			_mux_set_ofs, _mux_clr_ofs, _shift, _width,	\
+			_gate, _upd_ofs, _upd, _flags, _ops)		\
+
 extern const struct clk_ops mtk_mux_clr_set_upd_ops;
 extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
 
@@ -67,6 +86,14 @@  extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
 			_gate, _upd_ofs, _upd, _flags,			\
 			mtk_mux_gate_clr_set_upd_ops)
 
+#define MUX_GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents,	\
+			_paridx, _mux_ofs, _mux_set_ofs, _mux_clr_ofs,	\
+			_shift, _width, _gate, _upd_ofs, _upd, _flags)	\
+		GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name, _parents,	\
+			_paridx, _mux_ofs, _mux_set_ofs, _mux_clr_ofs,	\
+			_shift, _width, _gate, _upd_ofs, _upd, _flags,	\
+			mtk_mux_gate_clr_set_upd_ops)
+
 #define MUX_GATE_CLR_SET_UPD(_id, _name, _parents, _mux_ofs,		\
 			_mux_set_ofs, _mux_clr_ofs, _shift, _width,	\
 			_gate, _upd_ofs, _upd)				\
@@ -75,6 +102,14 @@  extern const struct clk_ops mtk_mux_gate_clr_set_upd_ops;
 			_width, _gate, _upd_ofs, _upd,			\
 			CLK_SET_RATE_PARENT)
 
+#define MUX_GATE_CLR_SET_UPD_INDEXED(_id, _name, _parents, _paridx,	\
+			_mux_ofs, _mux_set_ofs, _mux_clr_ofs, _shift,	\
+			_width, _gate, _upd_ofs, _upd)			\
+		MUX_GATE_CLR_SET_UPD_FLAGS_INDEXED(_id, _name,		\
+			_parents, _paridx, _mux_ofs, _mux_set_ofs,	\
+			_mux_clr_ofs, _shift, _width, _gate, _upd_ofs,	\
+			_upd, CLK_SET_RATE_PARENT)
+
 #define MUX_CLR_SET_UPD(_id, _name, _parents, _mux_ofs,			\
 			_mux_set_ofs, _mux_clr_ofs, _shift, _width,	\
 			_upd_ofs, _upd)					\