| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20231023064339.xglriq6ycedjlyd4@blmsp>
Date: Mon, 23 Oct 2023 08:43:39 +0200
From: Markus Schneider-Pargmann <msp@...libre.com>
To: AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>
Cc: sboyd@...nel.org, mturquette@...libre.com, matthias.bgg@...il.com,
wenst@...omium.org, amergnat@...libre.com,
yangyingliang@...wei.com, u.kleine-koenig@...gutronix.de,
linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org, kernel@...labora.com
Subject: Re: [PATCH v2 1/2] clk: mediatek: clk-mux: Support custom parent
indices for muxes
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@...libre.com>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.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
>
Powered by blists - more mailing lists