Re: [PATCH v5 4/5] soc: qcom: rpmh-rsc: Simplify locking by eliminating the per-TCS lock

[Stephen Boyd]

Quoting Douglas Anderson (2020-04-24 09:46:56)
> The rpmh-rsc code had both a driver-level lock (sometimes referred to
> in comments as drv->lock) and a lock per-TCS.  The idea was supposed
> to be that there would be times where you could get by with just
> locking a TCS lock and therefor other RPMH users wouldn't be blocked.
> 
> The above didn't work out so well.
> 
> Looking at tcs_write() the bigger drv->lock was held for most of the
> function anyway.  Only the __tcs_buffer_write() and
> __tcs_set_trigger() calls were called without holding the drv->lock.
> It actually turns out that in tcs_write() we don't need to hold the
> drv->lock for those function calls anyway even if the per-TCS lock
> isn't there anymore.  From the newly added comments in the code, this
> is because:
> - We marked "tcs_in_use" under lock.
> - Once "tcs_in_use" has been marked nobody else could be writing
>   to these registers until the interrupt goes off.
> - The interrupt can't go off until we trigger w/ the last line
>   of __tcs_set_trigger().
> Thus, from a tcs_write() point of view, the per-TCS lock was useless.
> 
> Looking at rpmh_rsc_write_ctrl_data(), only the per-TCS lock was held.
> It turns out, though, that this function already needs to be called
> with the equivalent of the drv->lock held anyway (we either need to
> hold drv->lock as we will in a future patch or we need to know no
> other CPUs could be running as happens today).  Specifically
> rpmh_rsc_write_ctrl_data() might be writing to a TCS that has been
> borrowed for writing an active transation but it never checks this.
> 
> Let's eliminate this extra overhead and avoid possible AB BA locking
> headaches.
> 
> Suggested-by: Maulik Shah <mkshah@xxxxxxxxxxxxxx>
> Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
> ---

Reviewed-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>