| Message ID | 20200306155707.RFT.4.Ia348ade7c6ed1d0d952ff2245bc854e5834c8d9a@changeid (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drivers: qcom: rpmh-rsc: Cleanup / add lots of comments | expand |
Hi, On 3/7/2020 5:29 AM, Douglas Anderson wrote: > The get_tcs_of_type() function doesn't provide any value. It's not > conceptually difficult to access a value in an array, even if that > value is in a structure and we want a pointer to the value. Having > the function in there makes me feel like it's doing something fancier > like looping or searching. Remove it. > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- > > drivers/soc/qcom/rpmh-rsc.c | 13 +++---------- > 1 file changed, 3 insertions(+), 10 deletions(-) > > diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c > index 099603bf14bf..a1298035bcd2 100644 > --- a/drivers/soc/qcom/rpmh-rsc.c > +++ b/drivers/soc/qcom/rpmh-rsc.c > @@ -169,17 +169,10 @@ static bool tcs_is_free(struct rsc_drv *drv, int tcs_id) > read_tcs_reg(drv, RSC_DRV_STATUS, tcs_id); > } > > -static struct tcs_group *get_tcs_of_type(struct rsc_drv *drv, int type) > -{ > - return &drv->tcs[type]; > -} > - > static int tcs_invalidate(struct rsc_drv *drv, int type) > { > int m; > - struct tcs_group *tcs; > - > - tcs = get_tcs_of_type(drv, type); > + struct tcs_group *tcs = &drv->tcs[type]; > > spin_lock(&tcs->lock); > if (bitmap_empty(tcs->slots, MAX_TCS_SLOTS)) { > @@ -245,9 +238,9 @@ static struct tcs_group *get_tcs_for_msg(struct rsc_drv *drv, > * dedicated AMC, and therefore would invalidate the sleep and wake > * TCSes before making an active state request. > */ > - tcs = get_tcs_of_type(drv, type); > + tcs = &drv->tcs[type]; > if (msg->state == RPMH_ACTIVE_ONLY_STATE && !tcs->num_tcs) { > - tcs = get_tcs_of_type(drv, WAKE_TCS); > + tcs = &drv->tcs[WAKE_TCS]; > if (tcs->num_tcs) { > ret = rpmh_rsc_invalidate(drv); > if (ret) Reviewed-by: Maulik Shah <mkshah@codeaurora.org> Thanks, Maulik
diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c index 099603bf14bf..a1298035bcd2 100644 --- a/drivers/soc/qcom/rpmh-rsc.c +++ b/drivers/soc/qcom/rpmh-rsc.c @@ -169,17 +169,10 @@ static bool tcs_is_free(struct rsc_drv *drv, int tcs_id) read_tcs_reg(drv, RSC_DRV_STATUS, tcs_id); } -static struct tcs_group *get_tcs_of_type(struct rsc_drv *drv, int type) -{ - return &drv->tcs[type]; -} - static int tcs_invalidate(struct rsc_drv *drv, int type) { int m; - struct tcs_group *tcs; - - tcs = get_tcs_of_type(drv, type); + struct tcs_group *tcs = &drv->tcs[type]; spin_lock(&tcs->lock); if (bitmap_empty(tcs->slots, MAX_TCS_SLOTS)) { @@ -245,9 +238,9 @@ static struct tcs_group *get_tcs_for_msg(struct rsc_drv *drv, * dedicated AMC, and therefore would invalidate the sleep and wake * TCSes before making an active state request. */ - tcs = get_tcs_of_type(drv, type); + tcs = &drv->tcs[type]; if (msg->state == RPMH_ACTIVE_ONLY_STATE && !tcs->num_tcs) { - tcs = get_tcs_of_type(drv, WAKE_TCS); + tcs = &drv->tcs[WAKE_TCS]; if (tcs->num_tcs) { ret = rpmh_rsc_invalidate(drv); if (ret)
The get_tcs_of_type() function doesn't provide any value. It's not conceptually difficult to access a value in an array, even if that value is in a structure and we want a pointer to the value. Having the function in there makes me feel like it's doing something fancier like looping or searching. Remove it. Signed-off-by: Douglas Anderson <dianders@chromium.org> --- drivers/soc/qcom/rpmh-rsc.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-)