Quoting Vinod Koul (2019-01-28 03:53:58)
> diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
> index e5eca8a1abe4..f06783c20688 100644
> --- a/drivers/clk/qcom/clk-rcg.h
> +++ b/drivers/clk/qcom/clk-rcg.h
> @@ -140,6 +140,7 @@ extern const struct clk_ops clk_dyn_rcg_ops;
> * @parent_map: map from software's parent index to hardware's src_sel field
> * @freq_tbl: frequency table
> * @clkr: regmap clock handle
> + * @cfg_off: defines the cfg register offset from the CMD_RCGR
> *
Please remove this extra line here. Also, shouldn't it say offset from
CMD_RCGR + CFG_REG?
> */
> struct clk_rcg2 {
> @@ -150,6 +151,7 @@ struct clk_rcg2 {
> const struct parent_map *parent_map;
> const struct freq_tbl *freq_tbl;
> struct clk_regmap clkr;
> + u8 cfg_off;
> };
>
> #define to_clk_rcg2(_hw) container_of(to_clk_regmap(_hw), struct clk_rcg2, clkr)
> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
> index 6e3bd195d012..106848e3313f 100644
> --- a/drivers/clk/qcom/clk-rcg2.c
> +++ b/drivers/clk/qcom/clk-rcg2.c
> @@ -74,7 +74,8 @@ static u8 clk_rcg2_get_parent(struct clk_hw *hw)
> u32 cfg;
> int i, ret;
>
> - ret = regmap_read(rcg->clkr.regmap, rcg->cmd_rcgr + CFG_REG, &cfg);
> + ret = regmap_read(rcg->clkr.regmap,
> + rcg->cmd_rcgr + rcg->cfg_off + CFG_REG, &cfg);
Maybe we should define CFG_REG as CFG_REG(rcg) and then do the math
there?
#define CFG_REG(rcg) (rcg)->cfg_off + 0x4
> if (ret)
> goto err;
>
> @@ -263,17 +268,20 @@ static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
> if (rcg->mnd_width && f->n) {
> mask = BIT(rcg->mnd_width) - 1;
> ret = regmap_update_bits(rcg->clkr.regmap,
> - rcg->cmd_rcgr + M_REG, mask, f->m);
> + rcg->cmd_rcgr + rcg->cfg_off + M_REG,
> + mask, f->m);
> if (ret)
> return ret;
>
> ret = regmap_update_bits(rcg->clkr.regmap,
> - rcg->cmd_rcgr + N_REG, mask, ~(f->n - f->m));
> + rcg->cmd_rcgr + rcg->cfg_off + N_REG,
> + mask, ~(f->n - f->m));
> if (ret)
> return ret;
>
> ret = regmap_update_bits(rcg->clkr.regmap,
> - rcg->cmd_rcgr + D_REG, mask, ~f->n);
> + rcg->cmd_rcgr + rcg->cfg_off + D_REG,
> + mask, ~f->n);
Ah the MND registers also move. Wow that's so sad. Do a similar thing
for all these too?
#define D_REG(rcg) (rcg)->cfg_off + 0x8
etc...
All just to make things fit on the same number of lines as before! We
could also throw the rcg->cmd_rcgr part into the register named macros
to make things even shorter. It was mostly OK when it was just adding
the offset to the base, but now we have another offset so I think we can
roll it all into the macro so that we can just read "regmap_read
FOO_REG" and ignore the rest.
> if (ret)
> return ret;
> }
