lkml.org 
[lkml]   [2022]   [Apr]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH V2 04/12] clk: mediatek: reset: Merge and revise reset register function
From
Date
On Thu, 2022-04-21 at 11:07 +0200, AngeloGioacchino Del Regno wrote:
> Il 20/04/22 15:05, Rex-BC Chen ha scritto:
> > Merge the reset register function of simple and set_clr into one
> > function.
> > - Input the version number to determine which version we will use.
> > - Rename reset register function to "mtk_clk_register_rst_ctrl"
> >
> > Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
> > ---
> > drivers/clk/mediatek/clk-mt2701-eth.c | 2 +-
> > drivers/clk/mediatek/clk-mt2701-g3d.c | 2 +-
> > drivers/clk/mediatek/clk-mt2701-hif.c | 2 +-
> > drivers/clk/mediatek/clk-mt2701.c | 4 +--
> > drivers/clk/mediatek/clk-mt2712.c | 4 +--
> > drivers/clk/mediatek/clk-mt7622-eth.c | 2 +-
> > drivers/clk/mediatek/clk-mt7622-hif.c | 4 +--
> > drivers/clk/mediatek/clk-mt7622.c | 4 +--
> > drivers/clk/mediatek/clk-mt7629-eth.c | 2 +-
> > drivers/clk/mediatek/clk-mt7629-hif.c | 4 +--
> > drivers/clk/mediatek/clk-mt8135.c | 4 +--
> > drivers/clk/mediatek/clk-mt8173.c | 4 +--
> > drivers/clk/mediatek/clk-mt8183.c | 3 ++-
> > drivers/clk/mediatek/clk-mtk.h | 13 ++++++----
> > drivers/clk/mediatek/reset.c | 35 ++++++++++++--------
> > -------
> > 15 files changed, 44 insertions(+), 45 deletions(-)
> >
>
> ..snip..
>
> > diff --git a/drivers/clk/mediatek/reset.c
> > b/drivers/clk/mediatek/reset.c
> > index 6574b19daf0f..8e42deee80a3 100644
> > --- a/drivers/clk/mediatek/reset.c
> > +++ b/drivers/clk/mediatek/reset.c
> > @@ -65,14 +65,23 @@ static const struct reset_control_ops
> > mtk_reset_ops_set_clr = {
> > .reset = mtk_reset_set_clr,
> > };
> >
> > -static void mtk_register_reset_controller_common(struct
> > device_node *np,
> > - unsigned int num_regs, int regofs,
> > - const struct reset_control_ops *reset_ops)
> > +static const struct reset_control_ops *rst_op[MTK_RST_MAX] = {
> > + [MTK_RST_SIMPLE] = &reset_simple_ops,
> > + [MTK_RST_SET_CLR] = &mtk_reset_ops_set_clr,
> > +};
>
> I don't think that we really need this to go to .rodata to get an
> improvement
> in boot times in the order of nanoseconds....
>
> > +
> > +void mtk_clk_register_rst_ctrl(struct device_node *np,
> > + u32 reg_num, u16 reg_ofs, u8 version)
> > {
> > struct mtk_reset *data;
> > int ret;
> > struct regmap *regmap;
> >
> > + if (version >= MTK_RST_MAX) {
> > + pr_err("Error version number: %d\n", version);
> > + return;
> > + }
> > +
> > regmap = device_node_to_regmap(np);
> > if (IS_ERR(regmap)) {
> > pr_err("Cannot find regmap for %pOF: %pe\n", np,
> > regmap);
> > @@ -84,10 +93,10 @@ static void
> > mtk_register_reset_controller_common(struct device_node *np,
> > return;
> >
> > data->regmap = regmap;
> > - data->regofs = regofs;
> > + data->regofs = reg_ofs;
> > data->rcdev.owner = THIS_MODULE;
> > - data->rcdev.nr_resets = num_regs * 32;
> > - data->rcdev.ops = reset_ops;
> > + data->rcdev.nr_resets = reg_num * 32;
> > + data->rcdev.ops = rst_op[version];
> > data->rcdev.of_node = np;
>
> ...hence, I would prefer to see something like:
>
> switch (version) {
> case MTK_RST_SIMPLE:
> data->rcdev.ops = &reset_simple_ops;
> break;
> case MTK_RST_SET_CLR:
> data->rcdev.ops = &mtk_reset_ops_set_clr;
> break;
> default:
> pr_err("Unknown reset version %d\n", version);
> return;
> }
>
> Like that, you'd also replace that if branch at the beginning where
> you
> do the reset version sanity check.
> If you don't want to allocate a struct mtk_reset before running this
> switch,
> you can also declare a `struct reset_control_ops *rcops = NULL;`
> locally and
> then assign `data->rcdev.ops = rcops;` later: that would also be
> acceptable.
>
> Cheers,
> Angelo

Hello Angelo,

I will do this in next version.

BRs,
Rex

\
 
 \ /
  Last update: 2022-04-22 07:02    [W:0.056 / U:0.372 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site