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 Tested-by: Elliot Berman quic_eberman@quicinc.com # sm8650-qrd Signed-off-by: Maulik Shah quic_mkshah@quicinc.com --- Changes in v4: - Simplify cmd_db_match_resource_addr() - Remove unrelated changes to newly added logic - Update function description comments - Replace Signed-off-by: with Tested-by: from Elliot - Link to v3: https://lore.kernel.org/r/20240212-rpmh-rsc-fixes-v3-1-1be0d705dbb5@quicinc.... 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.... Changes in v2: - Use GENMASK() and FIELD_GET() - Link to v1: https://lore.kernel.org/r/20240117-rpmh-rsc-fixes-v1-1-71ee4f8f72a4@quicinc.... --- drivers/soc/qcom/cmd-db.c | 32 +++++++++++++++++++++++++++++++- drivers/soc/qcom/rpmh-rsc.c | 3 ++- include/soc/qcom/cmd-db.h | 10 +++++++++- 3 files changed, 42 insertions(+), 3 deletions(-)
diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c index a5fd68411bed..86fb2cd4f484 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> @@ -17,6 +21,8 @@ #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 @@ -220,6 +226,30 @@ const void *cmd_db_read_aux_data(const char *id, size_t *len) } EXPORT_SYMBOL_GPL(cmd_db_read_aux_data);
+/** + * cmd_db_match_resource_addr() - Compare if both Resource addresses are same + * + * @addr1: Resource address to compare + * @addr2: Resource address to compare + * + * Return: true if two addresses refer to the same resource, 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 (addr1 == addr2) + return true; + else if (SLAVE_ID(addr1) == CMD_DB_HW_VRM && VRM_ADDR(addr1) == VRM_ADDR(addr2)) + return true; + + return false; +} +EXPORT_SYMBOL_GPL(cmd_db_match_resource_addr); + /** * cmd_db_read_slave_id - Get the slave ID for a given resource address * 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; }
--- base-commit: 615d300648869c774bd1fe54b4627bb0c20faed4 change-id: 20240210-rpmh-rsc-fixes-372a79ab364b
Best regards,
On Thu, Feb 15, 2024 at 10:55:44AM +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 Tested-by: Elliot Berman quic_eberman@quicinc.com # sm8650-qrd Signed-off-by: Maulik Shah quic_mkshah@quicinc.com
Changes in v4:
- Simplify cmd_db_match_resource_addr()
- Remove unrelated changes to newly added logic
- Update function description comments
You changed things, but retained Konrad's Reviewed-by...
- Replace Signed-off-by: with Tested-by: from Elliot
Did Elliot test v4, or did you just translate his incorrect(?) s-o-b from v1 into a tested-by?
We're about to write into the git history that Elliot says this version is tested to the best of his abilities...
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....
Changes in v2:
- Use GENMASK() and FIELD_GET()
- Link to v1: https://lore.kernel.org/r/20240117-rpmh-rsc-fixes-v1-1-71ee4f8f72a4@quicinc....
drivers/soc/qcom/cmd-db.c | 32 +++++++++++++++++++++++++++++++- drivers/soc/qcom/rpmh-rsc.c | 3 ++- include/soc/qcom/cmd-db.h | 10 +++++++++- 3 files changed, 42 insertions(+), 3 deletions(-)
diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c index a5fd68411bed..86fb2cd4f484 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> @@ -17,6 +21,8 @@ #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
@@ -220,6 +226,30 @@ const void *cmd_db_read_aux_data(const char *id, size_t *len) } EXPORT_SYMBOL_GPL(cmd_db_read_aux_data); +/**
- cmd_db_match_resource_addr() - Compare if both Resource addresses are same
- @addr1: Resource address to compare
- @addr2: Resource address to compare
- Return: true if two addresses refer to the same resource, 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 (addr1 == addr2)
return true;- else if (SLAVE_ID(addr1) == CMD_DB_HW_VRM && VRM_ADDR(addr1) == VRM_ADDR(addr2))
Is it a fact or a coincidence that GENMASK(19, 16) of the address happens to have the same value as the associated rsc_hdr's slv_id?
I recognize that we probably don't want to start naively traversing the headers here, but as previously requested I would still like to have an interface where I can pass in an address and have its entry_header->id resolved, for tracing/debug purposes. Perhaps there's some common ground?
Regards, Bjorn
return true;- return false;
+} +EXPORT_SYMBOL_GPL(cmd_db_match_resource_addr);
/**
- cmd_db_read_slave_id - Get the slave ID for a given resource address
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; }
base-commit: 615d300648869c774bd1fe54b4627bb0c20faed4 change-id: 20240210-rpmh-rsc-fixes-372a79ab364b
Best regards,
Maulik Shah quic_mkshah@quicinc.com
On Sun, Mar 17, 2024 at 10:18:25PM -0500, Bjorn Andersson wrote:
On Thu, Feb 15, 2024 at 10:55:44AM +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 Tested-by: Elliot Berman quic_eberman@quicinc.com # sm8650-qrd Signed-off-by: Maulik Shah quic_mkshah@quicinc.com
Changes in v4:
- Simplify cmd_db_match_resource_addr()
- Remove unrelated changes to newly added logic
- Update function description comments
You changed things, but retained Konrad's Reviewed-by...
- Replace Signed-off-by: with Tested-by: from Elliot
Did Elliot test v4, or did you just translate his incorrect(?) s-o-b from v1 into a tested-by?
We're about to write into the git history that Elliot says this version is tested to the best of his abilities...
No, I asked for it:
https://lore.kernel.org/all/20240214105605460-0800.eberman@hu-eberman-lv.qua...
On Thu, 15 Feb 2024 10:55:44 +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.
[...]
Applied, thanks!
[1/1] soc: qcom: rpmh-rsc: Enhance check for VRM in-flight request commit: f592cc5794747b81e53b53dd6e80219ee25f0611
Best regards,
linux-stable-mirror@lists.linaro.org
-
Bjorn Andersson -
Elliot Berman -
Maulik Shah