On Wed, Apr 25 2018 at 17:41 -0600, Matthias Kaehlcke wrote:
On Thu, Apr 19, 2018 at 04:16:34PM -0600, Lina Iyer 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>
---
Changes in v6:
- replace rpmh_client with device *
Changes in v4:
- reorganize rpmh_write_batch()
- introduce wait_count here, instead of patch#4
---
drivers/soc/qcom/rpmh.c | 155 +++++++++++++++++++++++++++++++++++++++-
include/soc/qcom/rpmh.h | 8 +++
2 files changed, 161 insertions(+), 2 deletions(-)
<snip>
+static int cache_batch(struct rpmh_ctrlr *ctrlr,
+ struct rpmh_request **rpm_msg, int count)
+{
+ unsigned long flags;
+ int ret = 0;
+ int index = 0;
+ int i;
+
+ spin_lock_irqsave(&ctrlr->lock, flags);
+ while (ctrlr->batch_cache[index])
+ index++;
This will access memory beyond 'batch_cache' when the cache is full.
Ok. Will check for index.
<snip>
+static void invalidate_batch(struct rpmh_ctrlr *ctrlr)
+{
+ unsigned long flags;
+ int index = 0;
+ int i;
+
+ spin_lock_irqsave(&ctrlr->lock, flags);
+ while (ctrlr->batch_cache[index])
+ index++;
Same as above. Also, why loop twice?
Good idea. Will fix.
+ for (i = 0; i < index; i++) {
+ kfree(ctrlr->batch_cache[i]->free);
+ ctrlr->batch_cache[i] = NULL;
+ }
<snip>
+/**
+ * rpmh_write_batch: Write multiple sets of RPMH commands and wait for the
+ * batch to finish.
+ *
+ * @dev: the device making the request
+ * @state: Active/sleep set
+ * @cmd: The payload data
+ * @n: The array of count of elements in each batch, 0 terminated.
nit: in this driver 'n' is usually associated with the command offset
within a TCS. Since it isn't an overly descriptive name it may already
cost the reader a while to commit that to his/her memory, and now
we are overloading 'n' with a different meaning (I also noticed this in
another patch of this series, but didn't comment).
Sure, will change the variable name here.
/*
* Nobody else should be calling this function other than system PM,,
~
Remove extra comma.
OK.
Thanks,
Lina
--
To unsubscribe from this list: send the line "unsubscribe linux-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
