diff mbox series

[v3] soc: qcom: rpmh-rsc: Enhance check for VRM in-flight request

Message ID 20240212-rpmh-rsc-fixes-v3-1-1be0d705dbb5@quicinc.com
State New
Headers show
Series [v3] soc: qcom: rpmh-rsc: Enhance check for VRM in-flight request | expand

Commit Message

Maulik Shah (mkshah) Feb. 12, 2024, 4:48 a.m. UTC 
Each RPMh VRM accelerator resource has 3 or 4 contiguous 4-byte aligned
addresses associated with it. These control voltage, enable state, mode,
and in legacy targets, voltage headroom. The current in-flight request
checking logic looks for exact address matches. Requests for different
addresses of the same RPMh resource as thus not detected as in-flight.

Add new cmd-db API cmd_db_match_resource_addr() to enhance the in-flight
request check for VRM requests by ignoring the address offset.

This ensures that only one request is allowed to be in-flight for a given
VRM resource. This is needed to avoid scenarios where request commands are
carried out by RPMh hardware out-of-order leading to LDO regulator
over-current protection triggering.

Fixes: 658628e7ef78 ("drivers: qcom: rpmh-rsc: add RPMH controller for QCOM SoCs")
cc: stable@vger.kernel.org
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
---
Changes in v3:
- Fix s-o-b chain
- Add cmd-db API to compare addresses
- Reuse already defined resource types in cmd-db
- Add Fixes tag and Cc to stable
- Retain Reviewed-by tag of v2
- Link to v2: https://lore.kernel.org/r/20240119-rpmh-rsc-fixes-v2-1-e42c0a9e36f0@quicinc.com
Changes in v2:
- Use GENMASK() and FIELD_GET()
- Link to v1: https://lore.kernel.org/r/20240117-rpmh-rsc-fixes-v1-1-71ee4f8f72a4@quicinc.com
---
 drivers/soc/qcom/cmd-db.c   | 41 +++++++++++++++++++++++++++++++++++------
 drivers/soc/qcom/rpmh-rsc.c |  3 ++-
 include/soc/qcom/cmd-db.h   | 10 +++++++++-
 3 files changed, 46 insertions(+), 8 deletions(-)


---
base-commit: 615d300648869c774bd1fe54b4627bb0c20faed4
change-id: 20240210-rpmh-rsc-fixes-372a79ab364b

Best regards,

Comments

Maulik Shah (mkshah) Feb. 15, 2024, 4:04 a.m. UTC | #1 
Hi,

On 2/15/2024 12:29 AM, Elliot Berman wrote:
> On Wed, Feb 14, 2024 at 12:08:43AM -0600, Bjorn Andersson wrote:
>> On Mon, Feb 12, 2024 at 10:18:08AM +0530, Maulik Shah wrote:
>>> Each RPMh VRM accelerator resource has 3 or 4 contiguous 4-byte aligned
>>> addresses associated with it. These control voltage, enable state, mode,
>>> and in legacy targets, voltage headroom. The current in-flight request
>>> checking logic looks for exact address matches. Requests for different
>>> addresses of the same RPMh resource as thus not detected as in-flight.
>>>
>>> Add new cmd-db API cmd_db_match_resource_addr() to enhance the in-flight
>>> request check for VRM requests by ignoring the address offset.
>>>
>>> This ensures that only one request is allowed to be in-flight for a given
>>> VRM resource. This is needed to avoid scenarios where request commands are
>>> carried out by RPMh hardware out-of-order leading to LDO regulator
>>> over-current protection triggering.
>>>
>>> Fixes: 658628e7ef78 ("drivers: qcom: rpmh-rsc: add RPMH controller for QCOM SoCs")
>>> cc: stable@vger.kernel.org
>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>>> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
>>> Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
>>
>> This says, "Elliot first certified the origin of the patch, then Maulik
>> took and certified the origin of the patch". But according to the From:
>> the author of the patch is you, Maulik.
>>
>> How was Elliot able to certify the patch's origin before you, when
>> you're the author?
>>
>> If the two of you collaborated, also add Co-developed-by: Elliot above
>> his s-o-b.
>>
> 
> Even my Co-developed-by is being generous :-) All I had done was copy
> the commit and resolve couple merge conflicts.
> 
> Maulik, you can swap my S-o-B for a:
> 
> Tested-by: Elliot Berman <quic_eberman@quicinc.com> # sm8650-qrd
> 

Addressed in v4 to change to tested-by.

Thanks,
Maulik
diff mbox series

Patch

diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c
index a5fd68411bed..e87682b9755e 100644
--- a/drivers/soc/qcom/cmd-db.c
+++ b/drivers/soc/qcom/cmd-db.c
@@ -1,6 +1,10 @@ 
 /* SPDX-License-Identifier: GPL-2.0 */
-/* Copyright (c) 2016-2018, 2020, The Linux Foundation. All rights reserved. */
+/*
+ * Copyright (c) 2016-2018, 2020, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
+ */
 
+#include <linux/bitfield.h>
 #include <linux/debugfs.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -15,8 +19,8 @@ 
 
 #define NUM_PRIORITY		2
 #define MAX_SLV_ID		8
-#define SLAVE_ID_MASK		0x7
-#define SLAVE_ID_SHIFT		16
+#define SLAVE_ID(addr)		FIELD_GET(GENMASK(19, 16), addr)
+#define VRM_ADDR(addr)		FIELD_GET(GENMASK(19, 4), addr)
 
 /**
  * struct entry_header: header for each entry in cmddb
@@ -221,9 +225,34 @@  const void *cmd_db_read_aux_data(const char *id, size_t *len)
 EXPORT_SYMBOL_GPL(cmd_db_read_aux_data);
 
 /**
- * cmd_db_read_slave_id - Get the slave ID for a given resource address
+ * cmd_db_match_resource_addr - Compare if both Resource addresses are same
+ *
+ * @addr1: Resource address to compare
+ * @addr2: Resource address to compare
+ *
+ * Return: true on matching addresses, false otherwise
+ */
+bool cmd_db_match_resource_addr(u32 addr1, u32 addr2)
+{
+	/*
+	 * Each RPMh VRM accelerator resource has 3 or 4 contiguous 4-byte
+	 * aligned addresses associated with it. Ignore the offset to check
+	 * for VRM requests.
+	 */
+	if (SLAVE_ID(addr1) == CMD_DB_HW_VRM
+	    && VRM_ADDR(addr1) == VRM_ADDR(addr2))
+		return true;
+	else if (addr1 == addr2)
+		return true;
+	else
+		return false;
+}
+EXPORT_SYMBOL_GPL(cmd_db_match_resource_addr);
+
+/**
+ * cmd_db_read_slave_id - Get the slave ID for a given resource name
  *
- * @id: Resource id to query the DB for version
+ * @id: Resource id to query the DB for slave id
  *
  * Return: cmd_db_hw_type enum on success, CMD_DB_HW_INVALID on error
  */
@@ -238,7 +267,7 @@  enum cmd_db_hw_type cmd_db_read_slave_id(const char *id)
 		return CMD_DB_HW_INVALID;
 
 	addr = le32_to_cpu(ent->addr);
-	return (addr >> SLAVE_ID_SHIFT) & SLAVE_ID_MASK;
+	return SLAVE_ID(addr);
 }
 EXPORT_SYMBOL_GPL(cmd_db_read_slave_id);
 
diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index a021dc71807b..daf64be966fe 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -1,6 +1,7 @@ 
 // SPDX-License-Identifier: GPL-2.0
 /*
  * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2023-2024, Qualcomm Innovation Center, Inc. All rights reserved.
  */
 
 #define pr_fmt(fmt) "%s " fmt, KBUILD_MODNAME
@@ -557,7 +558,7 @@  static int check_for_req_inflight(struct rsc_drv *drv, struct tcs_group *tcs,
 		for_each_set_bit(j, &curr_enabled, MAX_CMDS_PER_TCS) {
 			addr = read_tcs_cmd(drv, drv->regs[RSC_DRV_CMD_ADDR], i, j);
 			for (k = 0; k < msg->num_cmds; k++) {
-				if (addr == msg->cmds[k].addr)
+				if (cmd_db_match_resource_addr(msg->cmds[k].addr, addr))
 					return -EBUSY;
 			}
 		}
diff --git a/include/soc/qcom/cmd-db.h b/include/soc/qcom/cmd-db.h
index c8bb56e6852a..47a6cab75e63 100644
--- a/include/soc/qcom/cmd-db.h
+++ b/include/soc/qcom/cmd-db.h
@@ -1,5 +1,8 @@ 
 /* SPDX-License-Identifier: GPL-2.0 */
-/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. */
+/*
+ * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
+ */
 
 #ifndef __QCOM_COMMAND_DB_H__
 #define __QCOM_COMMAND_DB_H__
@@ -21,6 +24,8 @@  u32 cmd_db_read_addr(const char *resource_id);
 
 const void *cmd_db_read_aux_data(const char *resource_id, size_t *len);
 
+bool cmd_db_match_resource_addr(u32 addr1, u32 addr2);
+
 enum cmd_db_hw_type cmd_db_read_slave_id(const char *resource_id);
 
 int cmd_db_ready(void);
@@ -31,6 +36,9 @@  static inline u32 cmd_db_read_addr(const char *resource_id)
 static inline const void *cmd_db_read_aux_data(const char *resource_id, size_t *len)
 { return ERR_PTR(-ENODEV); }
 
+static inline bool cmd_db_match_resource_addr(u32 addr1, u32 addr2)
+{ return false; }
+
 static inline enum cmd_db_hw_type cmd_db_read_slave_id(const char *resource_id)
 { return -ENODEV; }