| Message ID | 20200414131010.v2.1.Ic70288f256ff0be65cac6a600367212dfe39f6c9@changeid |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/2] soc: qcom: rpmh-rsc: Factor "tcs_reg_addr" and "tcs_cmd_addr" calculation | expand |
Quoting Douglas Anderson (2020-04-14 13:10:15) > We can make some of the register access functions more readable by > factoring out the calculations a little bit. > > Suggested-by: Joe Perches <joe@perches.com> > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Quoting Doug Anderson (2020-04-14 13:39:15) > Hi, > > On Tue, Apr 14, 2020 at 1:30 PM Stephen Boyd <swboyd@chromium.org> wrote: > > > > Quoting Douglas Anderson (2020-04-14 13:10:16) > > > diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c > > > index f988e9cc2c30..02fc114ffb4f 100644 > > > --- a/drivers/soc/qcom/rpmh-rsc.c > > > +++ b/drivers/soc/qcom/rpmh-rsc.c > > > @@ -174,12 +175,13 @@ static void write_tcs_reg(const struct rsc_drv *drv, int reg, int tcs_id, > > > static void write_tcs_reg_sync(const struct rsc_drv *drv, int reg, int tcs_id, > > > u32 data) > > > { > > > + u32 new_data; > > > + > > > writel(data, tcs_reg_addr(drv, reg, tcs_id)); > > > - for (;;) { > > > - if (data == readl(tcs_reg_addr(drv, reg, tcs_id))) > > > - break; > > > - udelay(1); > > > - } > > > + if (readl_poll_timeout_atomic(tcs_reg_addr(drv, reg, tcs_id), new_data, > > > + new_data == data, 1, USEC_PER_SEC)) > > > + pr_err("%s: error writing %#x to %d:%d\n", drv->name, > > > > Shouldn't the register be hex? That seems to be how the registers are > > represented. But I guess tcs_id is decimal and can't be translated to be > > meaningful enough to indicate which TCS it is like the sleep or wake > > one. > > Good point. Should I quickly spin a v3 just so this is all ready to > go, or wait to see if there is any additional feedback? > That's my only complaint, so if maintainers fix it then you can have my RB tag. Reviewed-by: Stephen Boyd <swboyd@chromium.org>
diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c index 732316bb67dc..f988e9cc2c30 100644 --- a/drivers/soc/qcom/rpmh-rsc.c +++ b/drivers/soc/qcom/rpmh-rsc.c @@ -136,36 +136,47 @@ * +---------------------------------------------------+ */ -static u32 read_tcs_cmd(struct rsc_drv *drv, int reg, int tcs_id, int cmd_id) +static inline void __iomem * +tcs_reg_addr(const struct rsc_drv *drv, int reg, int tcs_id) { - return readl_relaxed(drv->tcs_base + RSC_DRV_TCS_OFFSET * tcs_id + reg + - RSC_DRV_CMD_OFFSET * cmd_id); + return drv->tcs_base + RSC_DRV_TCS_OFFSET * tcs_id + reg; } -static u32 read_tcs_reg(struct rsc_drv *drv, int reg, int tcs_id) +static inline void __iomem * +tcs_cmd_addr(const struct rsc_drv *drv, int reg, int tcs_id, int cmd_id) { - return readl_relaxed(drv->tcs_base + RSC_DRV_TCS_OFFSET * tcs_id + reg); + return tcs_reg_addr(drv, reg, tcs_id) + RSC_DRV_CMD_OFFSET * cmd_id; } -static void write_tcs_cmd(struct rsc_drv *drv, int reg, int tcs_id, int cmd_id, - u32 data) +static u32 read_tcs_cmd(const struct rsc_drv *drv, int reg, int tcs_id, + int cmd_id) +{ + return readl_relaxed(tcs_cmd_addr(drv, reg, tcs_id, cmd_id)); +} + +static u32 read_tcs_reg(const struct rsc_drv *drv, int reg, int tcs_id) { - writel_relaxed(data, drv->tcs_base + RSC_DRV_TCS_OFFSET * tcs_id + reg + - RSC_DRV_CMD_OFFSET * cmd_id); + return readl_relaxed(tcs_reg_addr(drv, reg, tcs_id)); } -static void write_tcs_reg(struct rsc_drv *drv, int reg, int tcs_id, u32 data) +static void write_tcs_cmd(const struct rsc_drv *drv, int reg, int tcs_id, + int cmd_id, u32 data) +{ + writel_relaxed(data, tcs_cmd_addr(drv, reg, tcs_id, cmd_id)); +} + +static void write_tcs_reg(const struct rsc_drv *drv, int reg, int tcs_id, + u32 data) { - writel_relaxed(data, drv->tcs_base + RSC_DRV_TCS_OFFSET * tcs_id + reg); + writel_relaxed(data, tcs_reg_addr(drv, reg, tcs_id)); } -static void write_tcs_reg_sync(struct rsc_drv *drv, int reg, int tcs_id, +static void write_tcs_reg_sync(const struct rsc_drv *drv, int reg, int tcs_id, u32 data) { - writel(data, drv->tcs_base + RSC_DRV_TCS_OFFSET * tcs_id + reg); + writel(data, tcs_reg_addr(drv, reg, tcs_id)); for (;;) { - if (data == readl(drv->tcs_base + reg + - RSC_DRV_TCS_OFFSET * tcs_id)) + if (data == readl(tcs_reg_addr(drv, reg, tcs_id))) break; udelay(1); }
We can make some of the register access functions more readable by factoring out the calculations a little bit. Suggested-by: Joe Perches <joe@perches.com> Signed-off-by: Douglas Anderson <dianders@chromium.org> --- Changes in v2: - Make drv parameter const. drivers/soc/qcom/rpmh-rsc.c | 41 +++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 15 deletions(-)