Messages in this thread | | | Subject | Re: [RFT PATCH v2 06/10] drivers: qcom: rpmh-rsc: Comment tcs_is_free() + warn if state mismatch | From | Maulik Shah <> | Date | Wed, 1 Apr 2020 17:08:36 +0530 |
| |
Hi,
On 3/12/2020 4:43 AM, Douglas Anderson wrote: > tcs_is_free() had two checks in it: does the software think that the > TCS is free and does the hardware think that the TCS is free. Let's > comment this and also add a warning in the case that software and > hardware disagree, at least for ACTIVE_ONLY TCS. > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- > > Changes in v2: > - Comment tcs_is_free() new for v2; replaces old patch 6. > > drivers/soc/qcom/rpmh-rsc.c | 23 +++++++++++++++++++++-- > 1 file changed, 21 insertions(+), 2 deletions(-) > > diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c > index 9d2669cbd994..93f5d1fb71ca 100644 > --- a/drivers/soc/qcom/rpmh-rsc.c > +++ b/drivers/soc/qcom/rpmh-rsc.c > @@ -181,8 +181,27 @@ static void write_tcs_reg_sync(struct rsc_drv *drv, int reg, int tcs_id, > */ > static bool tcs_is_free(struct rsc_drv *drv, int tcs_id) > { > - return !test_bit(tcs_id, drv->tcs_in_use) && > - read_tcs_reg(drv, RSC_DRV_STATUS, tcs_id); > + /* If software thinks it's in use then it's definitely in use */ > + if (test_bit(tcs_id, drv->tcs_in_use)) > + return false; > + > + /* If hardware agrees it's free then it's definitely free */ > + if (read_tcs_reg(drv, RSC_DRV_STATUS, tcs_id) != 0) > + return true; > + > + /* > + * If we're here then software and hardware disagree about whether > + * the TCS is free. Software thinks it is free and hardware thinks > + * it is not. > + * > + * Maybe this should be a warning in all cases, but it's almost > + * certainly a warning for the ACTIVE_TCS where nobody else should > + * be doing anything else behind our backs. For now we'll just > + * warn there and then still return that we're in use. > + */ > + WARN(drv->tcs[tcs_id].type == ACTIVE_TCS, > + "Driver thought TCS was free but HW reported busy\n"); This warning can come for borrowed WAKE_TCS as well. > + return false; > }
We have a patch on downstream variant to optimize this by only checking tcs_in_use flag (SW check) and HW check is removed.
static bool tcs_is_free(struct rsc_drv *drv, int tcs_id) { - return !test_bit(tcs_id, drv->tcs_in_use) && - read_tcs_reg(drv, RSC_DRV_STATUS, tcs_id, 0); + return !test_bit(tcs_id, drv->tcs_in_use); }
With this we are good and don't require to put above warning as well.
if you want me to upload, i can post it and then you can drop this change from your series.
Or if you want to modify it as above and keep in this series i am ok.
Thanks, Maulik
> > /**
-- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
| |