Messages in this thread | ![/](/images/icornerl.gif) | | Subject | Re: [PATCH V2 04/12] clk: mediatek: reset: Merge and revise reset register function | From | Rex-BC Chen <> | Date | Fri, 22 Apr 2022 13:01:25 +0800 |
| |
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
| ![\](/images/icornerr.gif) |