| Message ID | 1532685889-31345-2-git-send-email-rplsssn@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Andy Gross |
| Headers | show |
| Series | drivers/qcom: add additional functionality to RPMH | expand |
Hi Raju/Lina, On Fri, Jul 27, 2018 at 03:34:44PM +0530, Raju P L S S S N wrote: > From: Lina Iyer <ilina@codeaurora.org> > > Allow the controller status be queried. The controller is busy if it is > actively processing request. > > Signed-off-by: Lina Iyer <ilina@codeaurora.org> > Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org> > --- > Changes in v2: > - Remove unnecessary EXPORT_SYMBOL > --- > drivers/soc/qcom/rpmh-internal.h | 1 + > drivers/soc/qcom/rpmh-rsc.c | 20 ++++++++++++++++++++ > 2 files changed, 21 insertions(+) > > diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h > index a7bbbb6..4ff43bf 100644 > --- a/drivers/soc/qcom/rpmh-internal.h > +++ b/drivers/soc/qcom/rpmh-internal.h > @@ -108,6 +108,7 @@ struct rsc_drv { > int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv, > const struct tcs_request *msg); > int rpmh_rsc_invalidate(struct rsc_drv *drv); > +bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv); > > void rpmh_tx_done(const struct tcs_request *msg, int r); > > diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c > index 33fe9f9..42d0041 100644 > --- a/drivers/soc/qcom/rpmh-rsc.c > +++ b/drivers/soc/qcom/rpmh-rsc.c > @@ -496,6 +496,26 @@ static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg) > } > > /** > + * rpmh_rsc_ctrlr_is_idle: Check if any of the AMCs are busy. > + * > + * @drv: The controller > + * > + * Returns true if the TCSes are engaged in handling requests. > + */ > +bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv) > +{ > + int m; > + struct tcs_group *tcs = get_tcs_of_type(drv, ACTIVE_TCS); > + > + for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) { > + if (!tcs_is_free(drv, m)) > + return false; > + } > + > + return true; > +} This looks racy, tcs_write() could be running simultaneously and use TCSes that were seen as free by _is_idle(). This could be fixed by holding tcs->lock (assuming this doesn't cause lock ordering problems). However even with this tcs_write() could run right after releasing the lock, using TCSes and the caller of _is_idle() would consider the controller to be idle.
On Tue, Sep 11 2018 at 16:39 -0600, Matthias Kaehlcke wrote: >Hi Raju/Lina, > >On Fri, Jul 27, 2018 at 03:34:44PM +0530, Raju P L S S S N wrote: >> From: Lina Iyer <ilina@codeaurora.org> >> >> Allow the controller status be queried. The controller is busy if it is >> actively processing request. >> >> Signed-off-by: Lina Iyer <ilina@codeaurora.org> >> Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org> >> --- >> Changes in v2: >> - Remove unnecessary EXPORT_SYMBOL >> --- >> drivers/soc/qcom/rpmh-internal.h | 1 + >> drivers/soc/qcom/rpmh-rsc.c | 20 ++++++++++++++++++++ >> 2 files changed, 21 insertions(+) >> >> diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h >> index a7bbbb6..4ff43bf 100644 >> --- a/drivers/soc/qcom/rpmh-internal.h >> +++ b/drivers/soc/qcom/rpmh-internal.h >> @@ -108,6 +108,7 @@ struct rsc_drv { >> int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv, >> const struct tcs_request *msg); >> int rpmh_rsc_invalidate(struct rsc_drv *drv); >> +bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv); >> >> void rpmh_tx_done(const struct tcs_request *msg, int r); >> >> diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c >> index 33fe9f9..42d0041 100644 >> --- a/drivers/soc/qcom/rpmh-rsc.c >> +++ b/drivers/soc/qcom/rpmh-rsc.c >> @@ -496,6 +496,26 @@ static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg) >> } >> >> /** >> + * rpmh_rsc_ctrlr_is_idle: Check if any of the AMCs are busy. >> + * >> + * @drv: The controller >> + * >> + * Returns true if the TCSes are engaged in handling requests. >> + */ >> +bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv) >> +{ >> + int m; >> + struct tcs_group *tcs = get_tcs_of_type(drv, ACTIVE_TCS); >> + >> + for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) { >> + if (!tcs_is_free(drv, m)) >> + return false; >> + } >> + >> + return true; >> +} > >This looks racy, tcs_write() could be running simultaneously and use >TCSes that were seen as free by _is_idle(). This could be fixed by >holding tcs->lock (assuming this doesn't cause lock ordering problems). >However even with this tcs_write() could run right after releasing the >lock, using TCSes and the caller of _is_idle() would consider the >controller to be idle. We could run this without the lock, since we are only reading a status. Generally, this function is called from the idle code of the last CPU and no CPU or active TCS request should be in progress, but if it were, then this function would let the caller know we are not ready to do idle. If there were no requests that were running at that time we read the registers, we would not be making one after, since we are already in the idle code and no requests are made there. I understand, how it might appear racy, the context of the callling function helps resolve that. -- Lina
diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h index a7bbbb6..4ff43bf 100644 --- a/drivers/soc/qcom/rpmh-internal.h +++ b/drivers/soc/qcom/rpmh-internal.h @@ -108,6 +108,7 @@ struct rsc_drv { int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv, const struct tcs_request *msg); int rpmh_rsc_invalidate(struct rsc_drv *drv); +bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv); void rpmh_tx_done(const struct tcs_request *msg, int r); diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c index 33fe9f9..42d0041 100644 --- a/drivers/soc/qcom/rpmh-rsc.c +++ b/drivers/soc/qcom/rpmh-rsc.c @@ -496,6 +496,26 @@ static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg) } /** + * rpmh_rsc_ctrlr_is_idle: Check if any of the AMCs are busy. + * + * @drv: The controller + * + * Returns true if the TCSes are engaged in handling requests. + */ +bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv) +{ + int m; + struct tcs_group *tcs = get_tcs_of_type(drv, ACTIVE_TCS); + + for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) { + if (!tcs_is_free(drv, m)) + return false; + } + + return true; +} + +/** * rpmh_rsc_write_ctrl_data: Write request to the controller * * @drv: the controller