| Message ID | 20200704172334.GA759@earth.li (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | b88c28280c3f7097546db93824686db1e7dceee1 |
| Headers | show |
| Series | firmware: qcom_scm: Fix legacy convention SCM accessors | expand |
For the Qualcomm TrustZone firmwares which I am familiar with: Reviewed-by: Elliot Berman <eberman@codeaurora.org> On 7/4/2020 10:23 AM, Jonathan McDowell wrote: > 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: stable@vger.kernel.org > Signed-off-by: Jonathan McDowell <noodles@earth.li> > --- > drivers/firmware/qcom_scm.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c > index 0e7233a20f34..d4fda210adfe 100644 > --- a/drivers/firmware/qcom_scm.c > +++ b/drivers/firmware/qcom_scm.c > @@ -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); > >
On Mon 13 Jul 09:35 PDT 2020, Elliot Berman wrote: > For the Qualcomm TrustZone firmwares which I am familiar with: > > Reviewed-by: Elliot Berman <eberman@codeaurora.org> > Thanks for the patch Jonathan and thanks for the review Elliot! Applied Regards, Bjorn > On 7/4/2020 10:23 AM, Jonathan McDowell wrote: > > 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: stable@vger.kernel.org > > Signed-off-by: Jonathan McDowell <noodles@earth.li> > > --- > > drivers/firmware/qcom_scm.c | 7 +++---- > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c > > index 0e7233a20f34..d4fda210adfe 100644 > > --- a/drivers/firmware/qcom_scm.c > > +++ b/drivers/firmware/qcom_scm.c > > @@ -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); > > > > > > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c index 0e7233a20f34..d4fda210adfe 100644 --- a/drivers/firmware/qcom_scm.c +++ b/drivers/firmware/qcom_scm.c @@ -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);
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: stable@vger.kernel.org Signed-off-by: Jonathan McDowell <noodles@earth.li> --- drivers/firmware/qcom_scm.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)