[PATCH v2 2/4] soc: qcom: rpmh: Add support to read back resource settings

Kamal Wadhwa posted 4 patches 3 months, 2 weeks ago
[PATCH v2 2/4] soc: qcom: rpmh: Add support to read back resource settings
Posted by Kamal Wadhwa 3 months, 2 weeks ago
From: Maulik Shah <maulik.shah@oss.qualcomm.com>

All rpmh_*() APIs so far have supported placing votes for various
resource settings but the H/W also have option to read resource
settings.

This change adds a new rpmh_read() API to allow clients
to read back resource setting from H/W. This will be useful for
clients like regulators, which currently don't have a way to know
the settings applied during bootloader stage.

Link: https://lore.kernel.org/r/20250623-add-rpmh-read-support-v1-1-ae583d260195@oss.qualcomm.com
Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
Signed-off-by: Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>
---
 drivers/soc/qcom/rpmh-rsc.c | 13 +++++++++++--
 drivers/soc/qcom/rpmh.c     | 47 +++++++++++++++++++++++++++++++++++++++++----
 include/soc/qcom/rpmh.h     |  5 +++++
 include/soc/qcom/tcs.h      |  2 ++
 4 files changed, 61 insertions(+), 6 deletions(-)

diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index c6f7d5c9c493d9e06c048930b8a14a38660df4b1..ec85c457ea4527f94339c2033bfcc12346e3c443 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -443,6 +443,7 @@ static irqreturn_t tcs_tx_done(int irq, void *p)
 	int i;
 	unsigned long irq_status;
 	const struct tcs_request *req;
+	u32 reg;
 
 	irq_status = readl_relaxed(drv->tcs_base + drv->regs[RSC_DRV_IRQ_STATUS]);
 
@@ -453,6 +454,11 @@ static irqreturn_t tcs_tx_done(int irq, void *p)
 
 		trace_rpmh_tx_done(drv, i, req);
 
+		if (req->is_read) {
+			reg = drv->regs[RSC_DRV_CMD_RESP_DATA];
+			req->cmds[0].data = read_tcs_reg(drv, reg, i);
+		}
+
 		/* Clear AMC trigger & enable modes and
 		 * disable interrupt for this TCS
 		 */
@@ -493,13 +499,15 @@ static void __tcs_buffer_write(struct rsc_drv *drv, int tcs_id, int cmd_id,
 			       const struct tcs_request *msg)
 {
 	u32 msgid;
-	u32 cmd_msgid = CMD_MSGID_LEN | CMD_MSGID_WRITE;
+	u32 cmd_msgid = CMD_MSGID_LEN;
 	u32 cmd_enable = 0;
 	struct tcs_cmd *cmd;
 	int i, j;
 
 	/* Convert all commands to RR when the request has wait_for_compl set */
 	cmd_msgid |= msg->wait_for_compl ? CMD_MSGID_RESP_REQ : 0;
+	if (!msg->is_read)
+		cmd_msgid |= CMD_MSGID_WRITE;
 
 	for (i = 0, j = cmd_id; i < msg->num_cmds; i++, j++) {
 		cmd = &msg->cmds[i];
@@ -513,7 +521,8 @@ static void __tcs_buffer_write(struct rsc_drv *drv, int tcs_id, int cmd_id,
 
 		write_tcs_cmd(drv, drv->regs[RSC_DRV_CMD_MSGID], tcs_id, j, msgid);
 		write_tcs_cmd(drv, drv->regs[RSC_DRV_CMD_ADDR], tcs_id, j, cmd->addr);
-		write_tcs_cmd(drv, drv->regs[RSC_DRV_CMD_DATA], tcs_id, j, cmd->data);
+		if (!msg->is_read)
+			write_tcs_cmd(drv, drv->regs[RSC_DRV_CMD_DATA], tcs_id, j, cmd->data);
 		trace_rpmh_send_msg(drv, tcs_id, msg->state, j, msgid, cmd);
 	}
 
diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
index 8903ed956312d0a2ac7f673d86ef504947e9b119..4a611dac437ef28bac124583073c87a79e9e5cad 100644
--- a/drivers/soc/qcom/rpmh.c
+++ b/drivers/soc/qcom/rpmh.c
@@ -175,6 +175,9 @@ static int __rpmh_write(const struct device *dev, enum rpmh_state state,
 	struct cache_req *req;
 	int i;
 
+	if (rpm_msg->msg.is_read)
+		goto send_data;
+
 	/* Cache the request in our store and link the payload */
 	for (i = 0; i < rpm_msg->msg.num_cmds; i++) {
 		req = cache_rpm_request(ctrlr, state, &rpm_msg->msg.cmds[i]);
@@ -182,6 +185,7 @@ static int __rpmh_write(const struct device *dev, enum rpmh_state state,
 			return PTR_ERR(req);
 	}
 
+send_data:
 	if (state == RPMH_ACTIVE_ONLY_STATE) {
 		ret = rpmh_rsc_send_data(ctrlr_to_drv(ctrlr), &rpm_msg->msg);
 	} else {
@@ -194,7 +198,7 @@ static int __rpmh_write(const struct device *dev, enum rpmh_state state,
 }
 
 static int __fill_rpmh_msg(struct rpmh_request *req, enum rpmh_state state,
-		const struct tcs_cmd *cmd, u32 n)
+		const struct tcs_cmd *cmd, u32 n, bool is_read)
 {
 	if (!cmd || !n || n > MAX_RPMH_PAYLOAD)
 		return -EINVAL;
@@ -204,10 +208,45 @@ static int __fill_rpmh_msg(struct rpmh_request *req, enum rpmh_state state,
 	req->msg.state = state;
 	req->msg.cmds = req->cmd;
 	req->msg.num_cmds = n;
+	req->msg.is_read = is_read;
 
 	return 0;
 }
 
+/**
+ * rpmh_read: Read a resource value
+ *
+ * @dev: The device making the request
+ * @cmd: The payload having address of resource to read
+ *
+ * Reads the value for the resource address given in tcs_cmd->addr
+ * and returns the tcs_cmd->data filled with same.
+ *
+ * May sleep. Do not call from atomic contexts.
+ *
+ * Return: 0 on success, negative errno on failure
+ */
+int rpmh_read(const struct device *dev, struct tcs_cmd *cmd)
+{
+	DECLARE_COMPLETION_ONSTACK(compl);
+	DEFINE_RPMH_MSG_ONSTACK(dev, RPMH_ACTIVE_ONLY_STATE, &compl, rpm_msg);
+	int ret;
+
+	ret = __fill_rpmh_msg(&rpm_msg, RPMH_ACTIVE_ONLY_STATE, cmd, 1, true);
+	if (ret)
+		return ret;
+
+	ret = __rpmh_write(dev, RPMH_ACTIVE_ONLY_STATE, &rpm_msg);
+	if (ret)
+		return ret;
+
+	ret = wait_for_completion_timeout(&compl, RPMH_TIMEOUT_MS);
+	cmd[0].data = rpm_msg.cmd[0].data;
+
+	return (ret > 0) ? 0 : -ETIMEDOUT;
+}
+EXPORT_SYMBOL_GPL(rpmh_read);
+
 /**
  * rpmh_write_async: Write a set of RPMH commands
  *
@@ -230,7 +269,7 @@ int rpmh_write_async(const struct device *dev, enum rpmh_state state,
 		return -ENOMEM;
 	rpm_msg->needs_free = true;
 
-	ret = __fill_rpmh_msg(rpm_msg, state, cmd, n);
+	ret = __fill_rpmh_msg(rpm_msg, state, cmd, n, false);
 	if (ret) {
 		kfree(rpm_msg);
 		return ret;
@@ -257,7 +296,7 @@ int rpmh_write(const struct device *dev, enum rpmh_state state,
 	DEFINE_RPMH_MSG_ONSTACK(dev, state, &compl, rpm_msg);
 	int ret;
 
-	ret = __fill_rpmh_msg(&rpm_msg, state, cmd, n);
+	ret = __fill_rpmh_msg(&rpm_msg, state, cmd, n, false);
 	if (ret)
 		return ret;
 
@@ -352,7 +391,7 @@ int rpmh_write_batch(const struct device *dev, enum rpmh_state state,
 	rpm_msgs = req->rpm_msgs;
 
 	for (i = 0; i < count; i++) {
-		__fill_rpmh_msg(rpm_msgs + i, state, cmd, n[i]);
+		__fill_rpmh_msg(rpm_msgs + i, state, cmd, n[i], false);
 		cmd += n[i];
 	}
 
diff --git a/include/soc/qcom/rpmh.h b/include/soc/qcom/rpmh.h
index bdbee1a97d3685d8a6153d586ddf650bd3bd3dde..14ecbf242b6bd67c8167c176ed0970f42432f4f4 100644
--- a/include/soc/qcom/rpmh.h
+++ b/include/soc/qcom/rpmh.h
@@ -11,6 +11,8 @@
 
 
 #if IS_ENABLED(CONFIG_QCOM_RPMH)
+int rpmh_read(const struct device *dev, struct tcs_cmd *cmd);
+
 int rpmh_write(const struct device *dev, enum rpmh_state state,
 	       const struct tcs_cmd *cmd, u32 n);
 
@@ -24,6 +26,9 @@ void rpmh_invalidate(const struct device *dev);
 
 #else
 
+static inline int rpmh_read(const struct device *dev, struct tcs_cmd *cmd)
+{ return -ENODEV; }
+
 static inline int rpmh_write(const struct device *dev, enum rpmh_state state,
 			     const struct tcs_cmd *cmd, u32 n)
 { return -ENODEV; }
diff --git a/include/soc/qcom/tcs.h b/include/soc/qcom/tcs.h
index cff67ce25488e2d3603a7707af2ca77f8266a713..45b8513be2f9bb0957796476f6031146ee34e931 100644
--- a/include/soc/qcom/tcs.h
+++ b/include/soc/qcom/tcs.h
@@ -51,6 +51,7 @@ struct tcs_cmd {
  * struct tcs_request: A set of tcs_cmds sent together in a TCS
  *
  * @state:          state for the request.
+ * @is_read:        set for read only requests
  * @wait_for_compl: wait until we get a response from the h/w accelerator
  *                  (same as setting cmd->wait for all commands in the request)
  * @num_cmds:       the number of @cmds in this request
@@ -58,6 +59,7 @@ struct tcs_cmd {
  */
 struct tcs_request {
 	enum rpmh_state state;
+	bool is_read;
 	u32 wait_for_compl;
 	u32 num_cmds;
 	struct tcs_cmd *cmds;

-- 
2.25.1
Re: [PATCH v2 2/4] soc: qcom: rpmh: Add support to read back resource settings
Posted by Konrad Dybcio 2 months, 4 weeks ago
On 10/21/25 11:08 PM, Kamal Wadhwa wrote:
> From: Maulik Shah <maulik.shah@oss.qualcomm.com>
> 
> All rpmh_*() APIs so far have supported placing votes for various
> resource settings but the H/W also have option to read resource
> settings.
> 
> This change adds a new rpmh_read() API to allow clients
> to read back resource setting from H/W. This will be useful for
> clients like regulators, which currently don't have a way to know
> the settings applied during bootloader stage.
> 
> Link: https://lore.kernel.org/r/20250623-add-rpmh-read-support-v1-1-ae583d260195@oss.qualcomm.com
> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> Signed-off-by: Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>
> ---

[...]

> +int rpmh_read(const struct device *dev, struct tcs_cmd *cmd)
> +{
> +	DECLARE_COMPLETION_ONSTACK(compl);
> +	DEFINE_RPMH_MSG_ONSTACK(dev, RPMH_ACTIVE_ONLY_STATE, &compl, rpm_msg);
> +	int ret;
> +
> +	ret = __fill_rpmh_msg(&rpm_msg, RPMH_ACTIVE_ONLY_STATE, cmd, 1, true);
> +	if (ret)
> +		return ret;
> +
> +	ret = __rpmh_write(dev, RPMH_ACTIVE_ONLY_STATE, &rpm_msg);

Is there a reason for making this ACTIVE_ONLY?

Konrad
Re: [PATCH v2 2/4] soc: qcom: rpmh: Add support to read back resource settings
Posted by Maulik Shah (mkshah) 2 months, 3 weeks ago

On 11/12/2025 4:56 PM, Konrad Dybcio wrote:
> On 10/21/25 11:08 PM, Kamal Wadhwa wrote:
>> From: Maulik Shah <maulik.shah@oss.qualcomm.com>
>>
>> All rpmh_*() APIs so far have supported placing votes for various
>> resource settings but the H/W also have option to read resource
>> settings.
>>
>> This change adds a new rpmh_read() API to allow clients
>> to read back resource setting from H/W. This will be useful for
>> clients like regulators, which currently don't have a way to know
>> the settings applied during bootloader stage.
>>
>> Link: https://lore.kernel.org/r/20250623-add-rpmh-read-support-v1-1-ae583d260195@oss.qualcomm.com
>> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
>> Signed-off-by: Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>
>> ---
> 
> [...]
> 
>> +int rpmh_read(const struct device *dev, struct tcs_cmd *cmd)
>> +{
>> +	DECLARE_COMPLETION_ONSTACK(compl);
>> +	DEFINE_RPMH_MSG_ONSTACK(dev, RPMH_ACTIVE_ONLY_STATE, &compl, rpm_msg);
>> +	int ret;
>> +
>> +	ret = __fill_rpmh_msg(&rpm_msg, RPMH_ACTIVE_ONLY_STATE, cmd, 1, true);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = __rpmh_write(dev, RPMH_ACTIVE_ONLY_STATE, &rpm_msg);
> 
> Is there a reason for making this ACTIVE_ONLY?

Yes, using ACTIVE_ONLY makes the read request place immediately to read back the current resource setting.
Sleep/Wake are H/W based trigger and are not applicable for this API.

Thanks,
Maulik
Re: [PATCH v2 2/4] soc: qcom: rpmh: Add support to read back resource settings
Posted by Konrad Dybcio 2 months, 3 weeks ago
On 11/17/25 9:26 AM, Maulik Shah (mkshah) wrote:
> 
> 
> On 11/12/2025 4:56 PM, Konrad Dybcio wrote:
>> On 10/21/25 11:08 PM, Kamal Wadhwa wrote:
>>> From: Maulik Shah <maulik.shah@oss.qualcomm.com>
>>>
>>> All rpmh_*() APIs so far have supported placing votes for various
>>> resource settings but the H/W also have option to read resource
>>> settings.
>>>
>>> This change adds a new rpmh_read() API to allow clients
>>> to read back resource setting from H/W. This will be useful for
>>> clients like regulators, which currently don't have a way to know
>>> the settings applied during bootloader stage.
>>>
>>> Link: https://lore.kernel.org/r/20250623-add-rpmh-read-support-v1-1-ae583d260195@oss.qualcomm.com
>>> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
>>> Signed-off-by: Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>
>>> ---
>>
>> [...]
>>
>>> +int rpmh_read(const struct device *dev, struct tcs_cmd *cmd)
>>> +{
>>> +	DECLARE_COMPLETION_ONSTACK(compl);
>>> +	DEFINE_RPMH_MSG_ONSTACK(dev, RPMH_ACTIVE_ONLY_STATE, &compl, rpm_msg);
>>> +	int ret;
>>> +
>>> +	ret = __fill_rpmh_msg(&rpm_msg, RPMH_ACTIVE_ONLY_STATE, cmd, 1, true);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	ret = __rpmh_write(dev, RPMH_ACTIVE_ONLY_STATE, &rpm_msg);
>>
>> Is there a reason for making this ACTIVE_ONLY?
> 
> Yes, using ACTIVE_ONLY makes the read request place immediately to read back the current resource setting.
> Sleep/Wake are H/W based trigger and are not applicable for this API.

Huh? So if I send a read request with e.g. SLEEP_STATE, it would only
get fulfilled upon an active->sleep transition?

Konrad
Re: [PATCH v2 2/4] soc: qcom: rpmh: Add support to read back resource settings
Posted by Maulik Shah (mkshah) 2 months, 2 weeks ago

On 11/17/2025 6:04 PM, Konrad Dybcio wrote:
> On 11/17/25 9:26 AM, Maulik Shah (mkshah) wrote:
>>
>>
>> On 11/12/2025 4:56 PM, Konrad Dybcio wrote:
>>> On 10/21/25 11:08 PM, Kamal Wadhwa wrote:
>>>> From: Maulik Shah <maulik.shah@oss.qualcomm.com>
>>>>
>>>> All rpmh_*() APIs so far have supported placing votes for various
>>>> resource settings but the H/W also have option to read resource
>>>> settings.
>>>>
>>>> This change adds a new rpmh_read() API to allow clients
>>>> to read back resource setting from H/W. This will be useful for
>>>> clients like regulators, which currently don't have a way to know
>>>> the settings applied during bootloader stage.
>>>>
>>>> Link: https://lore.kernel.org/r/20250623-add-rpmh-read-support-v1-1-ae583d260195@oss.qualcomm.com
>>>> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
>>>> Signed-off-by: Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>
>>>> ---
>>>
>>> [...]
>>>
>>>> +int rpmh_read(const struct device *dev, struct tcs_cmd *cmd)
>>>> +{
>>>> +	DECLARE_COMPLETION_ONSTACK(compl);
>>>> +	DEFINE_RPMH_MSG_ONSTACK(dev, RPMH_ACTIVE_ONLY_STATE, &compl, rpm_msg);
>>>> +	int ret;
>>>> +
>>>> +	ret = __fill_rpmh_msg(&rpm_msg, RPMH_ACTIVE_ONLY_STATE, cmd, 1, true);
>>>> +	if (ret)
>>>> +		return ret;
>>>> +
>>>> +	ret = __rpmh_write(dev, RPMH_ACTIVE_ONLY_STATE, &rpm_msg);
>>>
>>> Is there a reason for making this ACTIVE_ONLY?
>>
>> Yes, using ACTIVE_ONLY makes the read request place immediately to read back the current resource setting.
>> Sleep/Wake are H/W based trigger and are not applicable for this API.
> 
> Huh? So if I send a read request with e.g. SLEEP_STATE, it would only
> get fulfilled upon an active->sleep transition?
> 

Read requests will get fulfilled immediately with the return of the current resource setting,
there is no separate active/sleep/wake vote values that can be read, put it other way the
rpmh_read() API do not take any "enum rpmh_state state" argument like various rpmh_write_*() APIs.

Thanks,
Maulik
Re: [PATCH v2 2/4] soc: qcom: rpmh: Add support to read back resource settings
Posted by Konrad Dybcio 1 month, 3 weeks ago
On 11/21/25 9:41 AM, Maulik Shah (mkshah) wrote:
> 
> 
> On 11/17/2025 6:04 PM, Konrad Dybcio wrote:
>> On 11/17/25 9:26 AM, Maulik Shah (mkshah) wrote:
>>>
>>>
>>> On 11/12/2025 4:56 PM, Konrad Dybcio wrote:
>>>> On 10/21/25 11:08 PM, Kamal Wadhwa wrote:
>>>>> From: Maulik Shah <maulik.shah@oss.qualcomm.com>
>>>>>
>>>>> All rpmh_*() APIs so far have supported placing votes for various
>>>>> resource settings but the H/W also have option to read resource
>>>>> settings.
>>>>>
>>>>> This change adds a new rpmh_read() API to allow clients
>>>>> to read back resource setting from H/W. This will be useful for
>>>>> clients like regulators, which currently don't have a way to know
>>>>> the settings applied during bootloader stage.
>>>>>
>>>>> Link: https://lore.kernel.org/r/20250623-add-rpmh-read-support-v1-1-ae583d260195@oss.qualcomm.com
>>>>> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
>>>>> Signed-off-by: Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>
>>>>> ---
>>>>
>>>> [...]
>>>>
>>>>> +int rpmh_read(const struct device *dev, struct tcs_cmd *cmd)
>>>>> +{
>>>>> +	DECLARE_COMPLETION_ONSTACK(compl);
>>>>> +	DEFINE_RPMH_MSG_ONSTACK(dev, RPMH_ACTIVE_ONLY_STATE, &compl, rpm_msg);
>>>>> +	int ret;
>>>>> +
>>>>> +	ret = __fill_rpmh_msg(&rpm_msg, RPMH_ACTIVE_ONLY_STATE, cmd, 1, true);
>>>>> +	if (ret)
>>>>> +		return ret;
>>>>> +
>>>>> +	ret = __rpmh_write(dev, RPMH_ACTIVE_ONLY_STATE, &rpm_msg);
>>>>
>>>> Is there a reason for making this ACTIVE_ONLY?
>>>
>>> Yes, using ACTIVE_ONLY makes the read request place immediately to read back the current resource setting.
>>> Sleep/Wake are H/W based trigger and are not applicable for this API.
>>
>> Huh? So if I send a read request with e.g. SLEEP_STATE, it would only
>> get fulfilled upon an active->sleep transition?
>>
> 
> Read requests will get fulfilled immediately with the return of the current resource setting,
> there is no separate active/sleep/wake vote values that can be read, put it other way the
> rpmh_read() API do not take any "enum rpmh_state state" argument like various rpmh_write_*() APIs.

I see, thanks

Konrad
Re: [PATCH v2 2/4] soc: qcom: rpmh: Add support to read back resource settings
Posted by Bjorn Andersson 3 months, 2 weeks ago
On Wed, Oct 22, 2025 at 02:38:54AM +0530, Kamal Wadhwa wrote:
> From: Maulik Shah <maulik.shah@oss.qualcomm.com>
> 
> All rpmh_*() APIs so far have supported placing votes for various
> resource settings but the H/W also have option to read resource
> settings.
> 
> This change adds a new rpmh_read() API to allow clients
> to read back resource setting from H/W. This will be useful for
> clients like regulators, which currently don't have a way to know
> the settings applied during bootloader stage.
> 

Allow me to express my disappointment over the fact that you sat on this
for 7 years!

> Link: https://lore.kernel.org/r/20250623-add-rpmh-read-support-v1-1-ae583d260195@oss.qualcomm.com

Why is there a Link here?

> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> Signed-off-by: Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>
> ---
>  drivers/soc/qcom/rpmh-rsc.c | 13 +++++++++++--
>  drivers/soc/qcom/rpmh.c     | 47 +++++++++++++++++++++++++++++++++++++++++----
>  include/soc/qcom/rpmh.h     |  5 +++++
>  include/soc/qcom/tcs.h      |  2 ++
>  4 files changed, 61 insertions(+), 6 deletions(-)
> 
[..]
> diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
[..]
> +/**
> + * rpmh_read: Read a resource value
> + *
> + * @dev: The device making the request
> + * @cmd: The payload having address of resource to read
> + *
> + * Reads the value for the resource address given in tcs_cmd->addr
> + * and returns the tcs_cmd->data filled with same.
> + *
> + * May sleep. Do not call from atomic contexts.

* Context: May sleep...

Regards,
Bjorn

> + *
> + * Return: 0 on success, negative errno on failure
> + */
> +int rpmh_read(const struct device *dev, struct tcs_cmd *cmd)
Re: [PATCH v2 2/4] soc: qcom: rpmh: Add support to read back resource settings
Posted by Maulik Shah (mkshah) 3 months, 2 weeks ago

On 10/23/2025 2:51 AM, Bjorn Andersson wrote:
> On Wed, Oct 22, 2025 at 02:38:54AM +0530, Kamal Wadhwa wrote:
>> From: Maulik Shah <maulik.shah@oss.qualcomm.com>
>>
>> All rpmh_*() APIs so far have supported placing votes for various
>> resource settings but the H/W also have option to read resource
>> settings.
>>
>> This change adds a new rpmh_read() API to allow clients
>> to read back resource setting from H/W. This will be useful for
>> clients like regulators, which currently don't have a way to know
>> the settings applied during bootloader stage.
>>
> 
> Allow me to express my disappointment over the fact that you sat on this
> for 7 years!

This was a dead API (even in downstream) with no user since SDM845/ 7 years.
Read support was eventually removed from downstream driver too for the same reason.
There were early discussions to remove read support from RSC H/W, due to lack of users.
Its not realized yet and all SoCs still supports read.

Now we have a regulator client requirement to read resource votes and reason to bring back this API.

> 
>> Link: https://lore.kernel.org/r/20250623-add-rpmh-read-support-v1-1-ae583d260195@oss.qualcomm.com
> 
> Why is there a Link here?

I will address this in next revision.

> 
>> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
>> Signed-off-by: Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>
>> ---
>>  drivers/soc/qcom/rpmh-rsc.c | 13 +++++++++++--
>>  drivers/soc/qcom/rpmh.c     | 47 +++++++++++++++++++++++++++++++++++++++++----
>>  include/soc/qcom/rpmh.h     |  5 +++++
>>  include/soc/qcom/tcs.h      |  2 ++
>>  4 files changed, 61 insertions(+), 6 deletions(-)
>>
> [..]
>> diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
> [..]
>> +/**
>> + * rpmh_read: Read a resource value
>> + *
>> + * @dev: The device making the request
>> + * @cmd: The payload having address of resource to read
>> + *
>> + * Reads the value for the resource address given in tcs_cmd->addr
>> + * and returns the tcs_cmd->data filled with same.
>> + *
>> + * May sleep. Do not call from atomic contexts.
> 
> * Context: May sleep...

I will address this in next revision.

Thanks,
Maulik> 
> Regards,
> Bjorn
> 
>> + *
>> + * Return: 0 on success, negative errno on failure
>> + */
>> +int rpmh_read(const struct device *dev, struct tcs_cmd *cmd)
Re: [PATCH v2 2/4] soc: qcom: rpmh: Add support to read back resource settings
Posted by Konrad Dybcio 3 months, 2 weeks ago
On 10/23/25 6:46 AM, Maulik Shah (mkshah) wrote:
> 
> 
> On 10/23/2025 2:51 AM, Bjorn Andersson wrote:
>> On Wed, Oct 22, 2025 at 02:38:54AM +0530, Kamal Wadhwa wrote:
>>> From: Maulik Shah <maulik.shah@oss.qualcomm.com>
>>>
>>> All rpmh_*() APIs so far have supported placing votes for various
>>> resource settings but the H/W also have option to read resource
>>> settings.
>>>
>>> This change adds a new rpmh_read() API to allow clients
>>> to read back resource setting from H/W. This will be useful for
>>> clients like regulators, which currently don't have a way to know
>>> the settings applied during bootloader stage.
>>>
>>
>> Allow me to express my disappointment over the fact that you sat on this
>> for 7 years!
> 
> This was a dead API (even in downstream) with no user since SDM845/ 7 years.
> Read support was eventually removed from downstream driver too for the same reason.
> There were early discussions to remove read support from RSC H/W, due to lack of users.
> Its not realized yet and all SoCs still supports read.

Can we read BCM states from HLOS this way too?

Konrad
Re: [PATCH v2 2/4] soc: qcom: rpmh: Add support to read back resource settings
Posted by Maulik Shah (mkshah) 3 months, 2 weeks ago

On 10/23/2025 1:47 PM, Konrad Dybcio wrote:
> On 10/23/25 6:46 AM, Maulik Shah (mkshah) wrote:
>>
>>
>> On 10/23/2025 2:51 AM, Bjorn Andersson wrote:
>>> On Wed, Oct 22, 2025 at 02:38:54AM +0530, Kamal Wadhwa wrote:
>>>> From: Maulik Shah <maulik.shah@oss.qualcomm.com>
>>>>
>>>> All rpmh_*() APIs so far have supported placing votes for various
>>>> resource settings but the H/W also have option to read resource
>>>> settings.
>>>>
>>>> This change adds a new rpmh_read() API to allow clients
>>>> to read back resource setting from H/W. This will be useful for
>>>> clients like regulators, which currently don't have a way to know
>>>> the settings applied during bootloader stage.
>>>>
>>>
>>> Allow me to express my disappointment over the fact that you sat on this
>>> for 7 years!
>>
>> This was a dead API (even in downstream) with no user since SDM845/ 7 years.
>> Read support was eventually removed from downstream driver too for the same reason.
>> There were early discussions to remove read support from RSC H/W, due to lack of users.
>> Its not realized yet and all SoCs still supports read.
> 
> Can we read BCM states from HLOS this way too?

Yes, Any of ARC/BCM/VRM can be read to get HLOS/DRV2 votes.

Thanks,
Maulik

> 
> Konrad
Re: [PATCH v2 2/4] soc: qcom: rpmh: Add support to read back resource settings
Posted by Konrad Dybcio 3 months, 2 weeks ago
On 10/23/25 10:57 AM, Maulik Shah (mkshah) wrote:
> 
> 
> On 10/23/2025 1:47 PM, Konrad Dybcio wrote:
>> On 10/23/25 6:46 AM, Maulik Shah (mkshah) wrote:
>>>
>>>
>>> On 10/23/2025 2:51 AM, Bjorn Andersson wrote:
>>>> On Wed, Oct 22, 2025 at 02:38:54AM +0530, Kamal Wadhwa wrote:
>>>>> From: Maulik Shah <maulik.shah@oss.qualcomm.com>
>>>>>
>>>>> All rpmh_*() APIs so far have supported placing votes for various
>>>>> resource settings but the H/W also have option to read resource
>>>>> settings.
>>>>>
>>>>> This change adds a new rpmh_read() API to allow clients
>>>>> to read back resource setting from H/W. This will be useful for
>>>>> clients like regulators, which currently don't have a way to know
>>>>> the settings applied during bootloader stage.
>>>>>
>>>>
>>>> Allow me to express my disappointment over the fact that you sat on this
>>>> for 7 years!
>>>
>>> This was a dead API (even in downstream) with no user since SDM845/ 7 years.
>>> Read support was eventually removed from downstream driver too for the same reason.
>>> There were early discussions to remove read support from RSC H/W, due to lack of users.
>>> Its not realized yet and all SoCs still supports read.
>>
>> Can we read BCM states from HLOS this way too?
> 
> Yes, Any of ARC/BCM/VRM can be read to get HLOS/DRV2 votes.

Wow this is amazing..

Do you have code for this already, or should I hack on it?

Konrad
Re: [PATCH v2 2/4] soc: qcom: rpmh: Add support to read back resource settings
Posted by Maulik Shah (mkshah) 3 months, 2 weeks ago

On 10/23/2025 2:39 PM, Konrad Dybcio wrote:
> On 10/23/25 10:57 AM, Maulik Shah (mkshah) wrote:
>>
>>
>> On 10/23/2025 1:47 PM, Konrad Dybcio wrote:
>>> On 10/23/25 6:46 AM, Maulik Shah (mkshah) wrote:
>>>>
>>>>
>>>> On 10/23/2025 2:51 AM, Bjorn Andersson wrote:
>>>>> On Wed, Oct 22, 2025 at 02:38:54AM +0530, Kamal Wadhwa wrote:
>>>>>> From: Maulik Shah <maulik.shah@oss.qualcomm.com>
>>>>>>
>>>>>> All rpmh_*() APIs so far have supported placing votes for various
>>>>>> resource settings but the H/W also have option to read resource
>>>>>> settings.
>>>>>>
>>>>>> This change adds a new rpmh_read() API to allow clients
>>>>>> to read back resource setting from H/W. This will be useful for
>>>>>> clients like regulators, which currently don't have a way to know
>>>>>> the settings applied during bootloader stage.
>>>>>>
>>>>>
>>>>> Allow me to express my disappointment over the fact that you sat on this
>>>>> for 7 years!
>>>>
>>>> This was a dead API (even in downstream) with no user since SDM845/ 7 years.
>>>> Read support was eventually removed from downstream driver too for the same reason.
>>>> There were early discussions to remove read support from RSC H/W, due to lack of users.
>>>> Its not realized yet and all SoCs still supports read.
>>>
>>> Can we read BCM states from HLOS this way too?
>>
>> Yes, Any of ARC/BCM/VRM can be read to get HLOS/DRV2 votes.
> 
> Wow this is amazing..
> 
> Do you have code for this already, or should I hack on it?

No, it won't be of much help, as i said above it gets HLOS/DRV2 votes only for a given resource.
More specifically, the read does not give the aggregated vote result across all the DRVs.

Thanks,
Maulik

> 
> Konrad
Re: [PATCH v2 2/4] soc: qcom: rpmh: Add support to read back resource settings
Posted by Konrad Dybcio 3 months, 2 weeks ago
On 10/23/25 11:46 AM, Maulik Shah (mkshah) wrote:
> 
> 
> On 10/23/2025 2:39 PM, Konrad Dybcio wrote:
>> On 10/23/25 10:57 AM, Maulik Shah (mkshah) wrote:
>>>
>>>
>>> On 10/23/2025 1:47 PM, Konrad Dybcio wrote:
>>>> On 10/23/25 6:46 AM, Maulik Shah (mkshah) wrote:
>>>>>
>>>>>
>>>>> On 10/23/2025 2:51 AM, Bjorn Andersson wrote:
>>>>>> On Wed, Oct 22, 2025 at 02:38:54AM +0530, Kamal Wadhwa wrote:
>>>>>>> From: Maulik Shah <maulik.shah@oss.qualcomm.com>
>>>>>>>
>>>>>>> All rpmh_*() APIs so far have supported placing votes for various
>>>>>>> resource settings but the H/W also have option to read resource
>>>>>>> settings.
>>>>>>>
>>>>>>> This change adds a new rpmh_read() API to allow clients
>>>>>>> to read back resource setting from H/W. This will be useful for
>>>>>>> clients like regulators, which currently don't have a way to know
>>>>>>> the settings applied during bootloader stage.
>>>>>>>
>>>>>>
>>>>>> Allow me to express my disappointment over the fact that you sat on this
>>>>>> for 7 years!
>>>>>
>>>>> This was a dead API (even in downstream) with no user since SDM845/ 7 years.
>>>>> Read support was eventually removed from downstream driver too for the same reason.
>>>>> There were early discussions to remove read support from RSC H/W, due to lack of users.
>>>>> Its not realized yet and all SoCs still supports read.
>>>>
>>>> Can we read BCM states from HLOS this way too?
>>>
>>> Yes, Any of ARC/BCM/VRM can be read to get HLOS/DRV2 votes.
>>
>> Wow this is amazing..
>>
>> Do you have code for this already, or should I hack on it?
> 
> No, it won't be of much help, as i said above it gets HLOS/DRV2 votes only for a given resource.
> More specifically, the read does not give the aggregated vote result across all the DRVs.

Hm, perhaps it could still be of *some* use

But maybe reading back rpmhpd and rpmhcc states would be of more
use!

Konrad
Re: [PATCH v2 2/4] soc: qcom: rpmh: Add support to read back resource settings
Posted by Neil Armstrong 3 months, 2 weeks ago
On 10/27/25 14:29, Konrad Dybcio wrote:
> On 10/23/25 11:46 AM, Maulik Shah (mkshah) wrote:
>>
>>
>> On 10/23/2025 2:39 PM, Konrad Dybcio wrote:
>>> On 10/23/25 10:57 AM, Maulik Shah (mkshah) wrote:
>>>>
>>>>
>>>> On 10/23/2025 1:47 PM, Konrad Dybcio wrote:
>>>>> On 10/23/25 6:46 AM, Maulik Shah (mkshah) wrote:
>>>>>>
>>>>>>
>>>>>> On 10/23/2025 2:51 AM, Bjorn Andersson wrote:
>>>>>>> On Wed, Oct 22, 2025 at 02:38:54AM +0530, Kamal Wadhwa wrote:
>>>>>>>> From: Maulik Shah <maulik.shah@oss.qualcomm.com>
>>>>>>>>
>>>>>>>> All rpmh_*() APIs so far have supported placing votes for various
>>>>>>>> resource settings but the H/W also have option to read resource
>>>>>>>> settings.
>>>>>>>>
>>>>>>>> This change adds a new rpmh_read() API to allow clients
>>>>>>>> to read back resource setting from H/W. This will be useful for
>>>>>>>> clients like regulators, which currently don't have a way to know
>>>>>>>> the settings applied during bootloader stage.
>>>>>>>>
>>>>>>>
>>>>>>> Allow me to express my disappointment over the fact that you sat on this
>>>>>>> for 7 years!
>>>>>>
>>>>>> This was a dead API (even in downstream) with no user since SDM845/ 7 years.
>>>>>> Read support was eventually removed from downstream driver too for the same reason.
>>>>>> There were early discussions to remove read support from RSC H/W, due to lack of users.
>>>>>> Its not realized yet and all SoCs still supports read.
>>>>>
>>>>> Can we read BCM states from HLOS this way too?
>>>>
>>>> Yes, Any of ARC/BCM/VRM can be read to get HLOS/DRV2 votes.
>>>
>>> Wow this is amazing..
>>>
>>> Do you have code for this already, or should I hack on it?
>>
>> No, it won't be of much help, as i said above it gets HLOS/DRV2 votes only for a given resource.
>> More specifically, the read does not give the aggregated vote result across all the DRVs.
> 
> Hm, perhaps it could still be of *some* use
> 
> But maybe reading back rpmhpd and rpmhcc states would be of more
> use!

The interconnect core definitely supports reading back the state at boot.

Neil

> 
> Konrad
Re: [PATCH v2 2/4] soc: qcom: rpmh: Add support to read back resource settings
Posted by Konrad Dybcio 3 months, 2 weeks ago
On 10/27/25 3:38 PM, Neil Armstrong wrote:
> On 10/27/25 14:29, Konrad Dybcio wrote:
>> On 10/23/25 11:46 AM, Maulik Shah (mkshah) wrote:
>>>
>>>
>>> On 10/23/2025 2:39 PM, Konrad Dybcio wrote:
>>>> On 10/23/25 10:57 AM, Maulik Shah (mkshah) wrote:
>>>>>
>>>>>
>>>>> On 10/23/2025 1:47 PM, Konrad Dybcio wrote:
>>>>>> On 10/23/25 6:46 AM, Maulik Shah (mkshah) wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 10/23/2025 2:51 AM, Bjorn Andersson wrote:
>>>>>>>> On Wed, Oct 22, 2025 at 02:38:54AM +0530, Kamal Wadhwa wrote:
>>>>>>>>> From: Maulik Shah <maulik.shah@oss.qualcomm.com>
>>>>>>>>>
>>>>>>>>> All rpmh_*() APIs so far have supported placing votes for various
>>>>>>>>> resource settings but the H/W also have option to read resource
>>>>>>>>> settings.
>>>>>>>>>
>>>>>>>>> This change adds a new rpmh_read() API to allow clients
>>>>>>>>> to read back resource setting from H/W. This will be useful for
>>>>>>>>> clients like regulators, which currently don't have a way to know
>>>>>>>>> the settings applied during bootloader stage.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Allow me to express my disappointment over the fact that you sat on this
>>>>>>>> for 7 years!
>>>>>>>
>>>>>>> This was a dead API (even in downstream) with no user since SDM845/ 7 years.
>>>>>>> Read support was eventually removed from downstream driver too for the same reason.
>>>>>>> There were early discussions to remove read support from RSC H/W, due to lack of users.
>>>>>>> Its not realized yet and all SoCs still supports read.
>>>>>>
>>>>>> Can we read BCM states from HLOS this way too?
>>>>>
>>>>> Yes, Any of ARC/BCM/VRM can be read to get HLOS/DRV2 votes.
>>>>
>>>> Wow this is amazing..
>>>>
>>>> Do you have code for this already, or should I hack on it?
>>>
>>> No, it won't be of much help, as i said above it gets HLOS/DRV2 votes only for a given resource.
>>> More specifically, the read does not give the aggregated vote result across all the DRVs.
>>
>> Hm, perhaps it could still be of *some* use
>>
>> But maybe reading back rpmhpd and rpmhcc states would be of more
>> use!
> 
> The interconnect core definitely supports reading back the state at boot.

Maulik probably isn't impressed with us only being able to provide
information about HLOS votes, as e.g. ADSP could be voting on the same
bus in parallel.

I suppose the very same applies to what I suggested with clk and rpmhpd
although probably it's less of a problem there

Konrad
Re: [PATCH v2 2/4] soc: qcom: rpmh: Add support to read back resource settings
Posted by Bjorn Andersson 2 months, 4 weeks ago
On Mon, Oct 27, 2025 at 04:47:26PM +0100, Konrad Dybcio wrote:
> On 10/27/25 3:38 PM, Neil Armstrong wrote:
> > On 10/27/25 14:29, Konrad Dybcio wrote:
> >> On 10/23/25 11:46 AM, Maulik Shah (mkshah) wrote:
> >>>
> >>>
> >>> On 10/23/2025 2:39 PM, Konrad Dybcio wrote:
> >>>> On 10/23/25 10:57 AM, Maulik Shah (mkshah) wrote:
> >>>>>
> >>>>>
> >>>>> On 10/23/2025 1:47 PM, Konrad Dybcio wrote:
> >>>>>> On 10/23/25 6:46 AM, Maulik Shah (mkshah) wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>> On 10/23/2025 2:51 AM, Bjorn Andersson wrote:
> >>>>>>>> On Wed, Oct 22, 2025 at 02:38:54AM +0530, Kamal Wadhwa wrote:
> >>>>>>>>> From: Maulik Shah <maulik.shah@oss.qualcomm.com>
> >>>>>>>>>
> >>>>>>>>> All rpmh_*() APIs so far have supported placing votes for various
> >>>>>>>>> resource settings but the H/W also have option to read resource
> >>>>>>>>> settings.
> >>>>>>>>>
> >>>>>>>>> This change adds a new rpmh_read() API to allow clients
> >>>>>>>>> to read back resource setting from H/W. This will be useful for
> >>>>>>>>> clients like regulators, which currently don't have a way to know
> >>>>>>>>> the settings applied during bootloader stage.
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> Allow me to express my disappointment over the fact that you sat on this
> >>>>>>>> for 7 years!
> >>>>>>>
> >>>>>>> This was a dead API (even in downstream) with no user since SDM845/ 7 years.
> >>>>>>> Read support was eventually removed from downstream driver too for the same reason.
> >>>>>>> There were early discussions to remove read support from RSC H/W, due to lack of users.
> >>>>>>> Its not realized yet and all SoCs still supports read.
> >>>>>>
> >>>>>> Can we read BCM states from HLOS this way too?
> >>>>>
> >>>>> Yes, Any of ARC/BCM/VRM can be read to get HLOS/DRV2 votes.
> >>>>
> >>>> Wow this is amazing..
> >>>>
> >>>> Do you have code for this already, or should I hack on it?
> >>>
> >>> No, it won't be of much help, as i said above it gets HLOS/DRV2 votes only for a given resource.
> >>> More specifically, the read does not give the aggregated vote result across all the DRVs.
> >>
> >> Hm, perhaps it could still be of *some* use
> >>
> >> But maybe reading back rpmhpd and rpmhcc states would be of more
> >> use!
> > 
> > The interconnect core definitely supports reading back the state at boot.
> 
> Maulik probably isn't impressed with us only being able to provide
> information about HLOS votes, as e.g. ADSP could be voting on the same
> bus in parallel.
> 
> I suppose the very same applies to what I suggested with clk and rpmhpd
> although probably it's less of a problem there
> 

Reading back the state serves the purpose of dealing with smooth
transition from bootloader, which we very much would like to have.

Being able to read the aggregated state is useful for debugging, but
it's a shared state that can change at any point in time, so we should
never act upon such information.

Regards,
Bjorn

> Konrad
Re: [PATCH v2 2/4] soc: qcom: rpmh: Add support to read back resource settings
Posted by Dmitry Baryshkov 3 months, 2 weeks ago
On Wed, Oct 22, 2025 at 02:38:54AM +0530, Kamal Wadhwa wrote:
> From: Maulik Shah <maulik.shah@oss.qualcomm.com>
> 
> All rpmh_*() APIs so far have supported placing votes for various
> resource settings but the H/W also have option to read resource
> settings.

Is it supported since SDM845?

> 
> This change adds a new rpmh_read() API to allow clients

See Documentation/process/submitting-patches.rst, "This patch ..."

> to read back resource setting from H/W. This will be useful for
> clients like regulators, which currently don't have a way to know
> the settings applied during bootloader stage.
> 
> Link: https://lore.kernel.org/r/20250623-add-rpmh-read-support-v1-1-ae583d260195@oss.qualcomm.com

This is useless, please drop.

> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> Signed-off-by: Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>
> ---
>  drivers/soc/qcom/rpmh-rsc.c | 13 +++++++++++--
>  drivers/soc/qcom/rpmh.c     | 47 +++++++++++++++++++++++++++++++++++++++++----
>  include/soc/qcom/rpmh.h     |  5 +++++
>  include/soc/qcom/tcs.h      |  2 ++
>  4 files changed, 61 insertions(+), 6 deletions(-)
> 

-- 
With best wishes
Dmitry
Re: [PATCH v2 2/4] soc: qcom: rpmh: Add support to read back resource settings
Posted by Maulik Shah (mkshah) 3 months, 2 weeks ago

On 10/22/2025 3:58 AM, Dmitry Baryshkov wrote:
> On Wed, Oct 22, 2025 at 02:38:54AM +0530, Kamal Wadhwa wrote:
>> From: Maulik Shah <maulik.shah@oss.qualcomm.com>
>>
>> All rpmh_*() APIs so far have supported placing votes for various
>> resource settings but the H/W also have option to read resource
>> settings.
> 
> Is it supported since SDM845?

Yes, H/W supports reads since SDM845.> 
>>
>> This change adds a new rpmh_read() API to allow clients
> 
> See Documentation/process/submitting-patches.rst, "This patch ..."

I will address in next revision.

> 
>> to read back resource setting from H/W. This will be useful for
>> clients like regulators, which currently don't have a way to know
>> the settings applied during bootloader stage.
>>
>> Link: https://lore.kernel.org/r/20250623-add-rpmh-read-support-v1-1-ae583d260195@oss.qualcomm.com
> 
> This is useless, please drop.

I will address in next revision.

Thanks,
Maulik

> 
>> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
>> Signed-off-by: Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>
>> ---
>>  drivers/soc/qcom/rpmh-rsc.c | 13 +++++++++++--
>>  drivers/soc/qcom/rpmh.c     | 47 +++++++++++++++++++++++++++++++++++++++++----
>>  include/soc/qcom/rpmh.h     |  5 +++++
>>  include/soc/qcom/tcs.h      |  2 ++
>>  4 files changed, 61 insertions(+), 6 deletions(-)
>>
>