Hi, On Wed, Apr 1, 2020 at 5:40 AM Maulik Shah <mkshah@xxxxxxxxxxxxxx> wrote: > > Hi, > > On 3/12/2020 4:43 AM, Douglas Anderson wrote: > > tcs_write() is documented to only be useful for writing > > RPMH_ACTIVE_ONLY_STATE. Let's be loud if someone messes up. > > > > Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx> > > --- > > > > Changes in v2: None > > > > drivers/soc/qcom/rpmh-rsc.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c > > index 93f5d1fb71ca..ba489d18c20e 100644 > > --- a/drivers/soc/qcom/rpmh-rsc.c > > +++ b/drivers/soc/qcom/rpmh-rsc.c > > @@ -573,6 +573,12 @@ static int tcs_write(struct rsc_drv *drv, const struct tcs_request *msg) > > unsigned long flags; > > int ret; > > > > + /* > > + * It only makes sense to grab a whole TCS for ourselves if we're > > + * triggering right away, which we only do for ACTIVE_ONLY. > > + */ > > + WARN_ON(msg->state != RPMH_ACTIVE_ONLY_STATE); > > + > > Unnecessary check, we will never hit this warning. Lets not add such check. That's fine. I can drop it, especially now that comments explain that this is only for ACTIVE_ONLY. Personally I like having extra assertion failures like this that indicate a serious internal logic error in the code, but I won't push strongly for it. > Saying that you can modify this change to drop below check from > rpmh_rsc_write_ctrl_data() as that never hit. > > /* Data sent to this API will not be sent immediately */ > if (msg->state == RPMH_ACTIVE_ONLY_STATE) > return -EINVAL; > > we always call rpmh_rsc_write_ctrl_data() for RPMH_SLEEP_STATE and > RPMH_WAKE_ONLY_STATE. Sure. My preference would have been to change it to a WARN_ON() too (because it signifies an internal error within the RPMH driver, not an external error that a client of RPMH could trigger), but I can just drop it entirely. -Doug
