firmware: qcom_scm: Fix legacy convention SCM accessors

PATCH Applied Jonathan McDowell 3 participants 3 messages Jul 4, 2020 – Jul 13, 2020 Arm Qualcomm mailing list

Share & tools

Jonathan McDowell

author

2020-07-04 17:23 UTC #0 lore raw

PATCH 1/1 firmware: qcom_scm: Fix legacy convention SCM accessors

The move to a combined driver for the QCOM SCM hardware changed the io_writel and io_readl helpers to use non-atomic calls, despite the commit message saying that atomic was a better option. This breaks these helpers on hardware that uses the old legacy convention (access fails with a -95 return code). Switch back to using the atomic calls.

Observed as a failure routing GPIO interrupts to the Apps processor on an IPQ8064; fix is confirmed as correctly allowing the interrupts to be routed and observed.

Fixes 57d3b816718c ("firmware: qcom_scm: Remove thin wrappers")

Cc [email protected]

Signed-off-by Jonathan McDowell <[email protected]>

---
 drivers/firmware/qcom_scm.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

drivers/firmware/qcom_scm.c +3 -4

index 0e7233a20f34..d4fda210adfe 100644
@@ -391,7 +391,7 @@ static int __qcom_scm_set_dload_mode(struct device *dev, bool enable)
 
 	desc.args[1] = enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0;
 
-	return qcom_scm_call(__scm->dev, &desc, NULL);
+	return qcom_scm_call_atomic(__scm->dev, &desc, NULL);
 }
 
 static void qcom_scm_set_download_mode(bool enable)

@@ -650,7 +650,7 @@ int qcom_scm_io_readl(phys_addr_t addr, unsigned int *val)
 	int ret;
 
 
-	ret = qcom_scm_call(__scm->dev, &desc, &res);
+	ret = qcom_scm_call_atomic(__scm->dev, &desc, &res);
 	if (ret >= 0)
 		*val = res.result[0];

@@ -669,8 +669,7 @@ int qcom_scm_io_writel(phys_addr_t addr, unsigned int val)
 		.owner = ARM_SMCCC_OWNER_SIP,
 	};
 
-
-	return qcom_scm_call(__scm->dev, &desc, NULL);
+	return qcom_scm_call_atomic(__scm->dev, &desc, NULL);
 }
 EXPORT_SYMBOL(qcom_scm_io_writel);

Signature · 1 line

2.20.1

Elliot Berman

Reviewed-by on v1 1/1

2020-07-13 16:35 UTC #1 lore raw

For the Qualcomm TrustZone firmwares which I am familiar with:

Reviewed-by Elliot Berman <[email protected]>

Quoted 50 lines · Jonathan McDowell

The move to a combined driver for the QCOM SCM hardware changed the io_writel and io_readl helpers to use non-atomic calls, despite the commit message saying that atomic was a better option. This breaks these helpers on hardware that uses the old legacy convention (access fails with a -95 return code). Switch back to using the atomic calls.
Observed as a failure routing GPIO interrupts to the Apps processor on an IPQ8064; fix is confirmed as correctly allowing the interrupts to be routed and observed.
Fixes 57d3b816718c ("firmware: qcom_scm: Remove thin wrappers")
Cc [email protected]
Signed-off-by Jonathan McDowell <[email protected]>
---
 drivers/firmware/qcom_scm.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/firmware/qcom_scm.c +3 -4
index 0e7233a20f34..d4fda210adfe 100644
@@ -391,7 +391,7 @@ static int __qcom_scm_set_dload_mode(struct device *dev, bool enable)
 
 	desc.args[1] = enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0;
 
-	return qcom_scm_call(__scm->dev, &desc, NULL);
+	return qcom_scm_call_atomic(__scm->dev, &desc, NULL);
 }
 
 static void qcom_scm_set_download_mode(bool enable)
@@ -650,7 +650,7 @@ int qcom_scm_io_readl(phys_addr_t addr, unsigned int *val)
 	int ret;
 
 
-	ret = qcom_scm_call(__scm->dev, &desc, &res);
+	ret = qcom_scm_call_atomic(__scm->dev, &desc, &res);
 	if (ret >= 0)
 		*val = res.result[0];
 
@@ -669,8 +669,7 @@ int qcom_scm_io_writel(phys_addr_t addr, unsigned int val)
 		.owner = ARM_SMCCC_OWNER_SIP,
 	};
 
-
-	return qcom_scm_call(__scm->dev, &desc, NULL);
+	return qcom_scm_call_atomic(__scm->dev, &desc, NULL);
 }
 EXPORT_SYMBOL(qcom_scm_io_writel);
 

Signature · 2 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Bjorn Andersson

Applied

2020-07-13 18:44 UTC #2 lore raw

Quoted 4 lines · Elliot Berman

For the Qualcomm TrustZone firmwares which I am familiar with:
Reviewed-by Elliot Berman <[email protected]>

Thanks for the patch Jonathan and thanks for the review Elliot!

Applied

Regards,
Bjorn

Quoted 55 lines · Jonathan McDowell

Quoted 50 lines
The move to a combined driver for the QCOM SCM hardware changed the io_writel and io_readl helpers to use non-atomic calls, despite the commit message saying that atomic was a better option. This breaks these helpers on hardware that uses the old legacy convention (access fails with a -95 return code). Switch back to using the atomic calls.
Observed as a failure routing GPIO interrupts to the Apps processor on an IPQ8064; fix is confirmed as correctly allowing the interrupts to be routed and observed.
Fixes 57d3b816718c ("firmware: qcom_scm: Remove thin wrappers")
Cc [email protected]
Signed-off-by Jonathan McDowell <[email protected]>
---
 drivers/firmware/qcom_scm.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/firmware/qcom_scm.c +3 -4
index 0e7233a20f34..d4fda210adfe 100644
@@ -391,7 +391,7 @@ static int __qcom_scm_set_dload_mode(struct device *dev, bool enable)
 
 	desc.args[1] = enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0;
 
-	return qcom_scm_call(__scm->dev, &desc, NULL);
+	return qcom_scm_call_atomic(__scm->dev, &desc, NULL);
 }
 
 static void qcom_scm_set_download_mode(bool enable)
@@ -650,7 +650,7 @@ int qcom_scm_io_readl(phys_addr_t addr, unsigned int *val)
 	int ret;
 
 
-	ret = qcom_scm_call(__scm->dev, &desc, &res);
+	ret = qcom_scm_call_atomic(__scm->dev, &desc, &res);
 	if (ret >= 0)
 		*val = res.result[0];
 
@@ -669,8 +669,7 @@ int qcom_scm_io_writel(phys_addr_t addr, unsigned int val)
 		.owner = ARM_SMCCC_OWNER_SIP,
 	};
 
-
-	return qcom_scm_call(__scm->dev, &desc, NULL);
+	return qcom_scm_call_atomic(__scm->dev, &desc, NULL);
 }
 EXPORT_SYMBOL(qcom_scm_io_writel);
 
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project