Re: [PATCH v6 05/10] drivers: qcom: rpmh-rsc: write sleep/wake requests to TCS

From: Lina Iyer
Date: Fri Apr 27 2018 - 13:39:53 EST

On Wed, Apr 25 2018 at 15:41 -0600, Matthias Kaehlcke wrote:
On Thu, Apr 19, 2018 at 04:16:30PM -0600, Lina Iyer wrote:
Sleep and wake requests are sent when the application processor
subsystem of the SoC is entering deep sleep states like in suspend.
These requests help lower the system power requirements when the
resources are not in use.

Sleep and wake requests are written to the TCS slots but are not
triggered at the time of writing. The TCS are triggered by the firmware
after the last of the CPUs has executed its WFI. Since these requests
may come in different batches of requests, it is the job of this
controller driver to find and arrange the requests into the available

Signed-off-by: Lina Iyer <ilina@xxxxxxxxxxxxxx>
Reviewed-by: Evan Green <evgreen@xxxxxxxxxxxx>
drivers/soc/qcom/rpmh-internal.h | 8 +++
drivers/soc/qcom/rpmh-rsc.c | 120 +++++++++++++++++++++++++++++++
2 files changed, 128 insertions(+)

diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
index d9a21726e568..6e19fe458c31 100644
--- a/drivers/soc/qcom/rpmh-internal.h
+++ b/drivers/soc/qcom/rpmh-internal.h


+static int find_match(const struct tcs_group *tcs, const struct tcs_cmd *cmd,
+ int len)
+ int i, j;
+ /* Check for already cached commands */
+ for_each_set_bit(i, tcs->slots, MAX_TCS_SLOTS) {
+ for (j = 0; j < len; j++) {
+ if (tcs->cmd_cache[i] != cmd[0].addr) {

Shouldn't the condition be 'tcs->cmd_cache[i + j] != cmd[j].addr'?

Here, we are trying to find the first address from the request and its
position 'i' in the cmd_cache.

Otherwise the code below the following if branch will never be
executed. Either the 'tcs->cmd_cache[i] != cmd[0].addr' branch isn't
entered because the addresses match, or the addresses don't match
and the inner loop is aborted after the first iteration.

+ if (j == 0)
+ break;
+ WARN(tcs->cmd_cache[i + j] != cmd[j].addr,
+ "Message does not match previous sequence.\n");
We now check for the sequence using the iterator 'j' only after we have
found 'i' (the beginning of our request).

I hope that helps clear the concern.

-- Lina

+ return -EINVAL;
+ } else if (j == len - 1) {
+ return i;
+ }
+ }
+ }
+ return -ENODATA;