Re: [PATCH v2 09/10] drivers: qcom: rpmh: add support for batch RPMH request

From: Lina Iyer
Date: Thu Feb 22 2018 - 12:05:02 EST


On Wed, Feb 21 2018 at 23:25 +0000, Evan Green wrote:
Hello Lina,

On Thu, Feb 15, 2018 at 9:35 AM, Lina Iyer <ilina@xxxxxxxxxxxxxx> wrote:
Platform drivers need make a lot of resource state requests at the same
time, say, at the start or end of an usecase. It can be quite
inefficient to send each request separately. Instead they can give the
RPMH library a batch of requests to be sent and wait on the whole
transaction to be complete.

rpmh_write_batch() is a blocking call that can be used to send multiple
RPMH command sets. Each RPMH command set is set asynchronously and the
API blocks until all the command sets are complete and receive their
tx_done callbacks.

Signed-off-by: Lina Iyer <ilina@xxxxxxxxxxxxxx>
---
drivers/soc/qcom/rpmh.c | 150 ++++++++++++++++++++++++++++++++++++++++++++++++
include/soc/qcom/rpmh.h | 8 +++
2 files changed, 158 insertions(+)

diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
index dff4c46be3af..6f60bb9a4dfa 100644
--- a/drivers/soc/qcom/rpmh.c
+++ b/drivers/soc/qcom/rpmh.c
[...]
@@ -394,6 +537,11 @@ int rpmh_flush(struct rpmh_client *rc)
}
spin_unlock_irqrestore(&rpm->lock, flags);

+ /* First flush the cached batch requests */
+ ret = flush_batch(rc);
+ if (ret)
+ return ret;
+
/*
* Nobody else should be calling this function other than system PM,,
* hence we can run without locks.
This comment and the comment in the header of this function.

@@ -438,6 +586,8 @@ int rpmh_invalidate(struct rpmh_client *rc)
if (IS_ERR_OR_NULL(rc))
return -EINVAL;

+ invalidate_batch(rc);
+

Similarly to my comments in patch 7, aren't there races here with
adding new elements? After flush_batch, but before invalidate_batch,
somebody could call cache_batch, which would add new things on the end
of the array. These new items would be clobbered by invalidate_batch,
without having been flushed first.

Flush is a system PM function and is not called by drivers and is not
expected to run conncurrently with other threads using the RPMH library.

Thanks,
Lina