diff mbox series

[3/6] firmware: qcom_scm: Workaround lack of "is available" call on SC7180

Message ID 20210223214539.1336155-4-swboyd@chromium.org
State Accepted
Commit 257f2935cbbf14b16912c635fcd8ff43345c953b
Headers show
Series [1/6] firmware: qcom_scm: Make __qcom_scm_is_call_available() return bool | expand

Commit Message

Stephen Boyd Feb. 23, 2021, 9:45 p.m. UTC
Some SC7180 firmwares don't implement the QCOM_SCM_INFO_IS_CALL_AVAIL
API, so we can't probe the calling convention. We detect the legacy
calling convention on these firmwares, because the availability call
always fails and legacy is the fallback. This leads to problems where
the rmtfs driver fails to probe, because it tries to assign memory with
a bad calling convention, which then leads to modem failing to load and
all networking, even wifi, to fail. Ouch!

Let's force the calling convention to be what it always is on this SoC,
i.e. arm64. Of course, the calling convention is not the same thing as
implementing the QCOM_SCM_INFO_IS_CALL_AVAIL API. The absence of the "is
this call available" API from the firmware means that any call to
__qcom_scm_is_call_available() fails. This is OK for now though because
none of the calls that are checked for existence are implemented on
firmware running on sc7180. If such a call needs to be checked for
existence in the future, we presume that firmware will implement this
API and then things will "just work".

Cc: Elliot Berman <eberman@codeaurora.org>
Cc: Brian Masney <masneyb@onstation.org>
Cc: Stephan Gerhold <stephan@gerhold.net>
Cc: Jeffrey Hugo <jhugo@codeaurora.org>
Cc: Douglas Anderson <dianders@chromium.org>
Fixes: 9a434cee773a ("firmware: qcom_scm: Dynamically support SMCCC and legacy conventions")
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/firmware/qcom_scm.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

Comments

Jeffrey Hugo Feb. 23, 2021, 11:38 p.m. UTC | #1
On 2/23/2021 2:45 PM, Stephen Boyd wrote:
> Some SC7180 firmwares don't implement the QCOM_SCM_INFO_IS_CALL_AVAIL
> API, so we can't probe the calling convention. We detect the legacy
> calling convention on these firmwares, because the availability call
> always fails and legacy is the fallback. This leads to problems where
> the rmtfs driver fails to probe, because it tries to assign memory with
> a bad calling convention, which then leads to modem failing to load and
> all networking, even wifi, to fail. Ouch!
> 
> Let's force the calling convention to be what it always is on this SoC,
> i.e. arm64. Of course, the calling convention is not the same thing as
> implementing the QCOM_SCM_INFO_IS_CALL_AVAIL API. The absence of the "is
> this call available" API from the firmware means that any call to
> __qcom_scm_is_call_available() fails. This is OK for now though because
> none of the calls that are checked for existence are implemented on
> firmware running on sc7180. If such a call needs to be checked for
> existence in the future, we presume that firmware will implement this
> API and then things will "just work".
> 
> Cc: Elliot Berman <eberman@codeaurora.org>
> Cc: Brian Masney <masneyb@onstation.org>
> Cc: Stephan Gerhold <stephan@gerhold.net>
> Cc: Jeffrey Hugo <jhugo@codeaurora.org>
> Cc: Douglas Anderson <dianders@chromium.org>
> Fixes: 9a434cee773a ("firmware: qcom_scm: Dynamically support SMCCC and legacy conventions")
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>   drivers/firmware/qcom_scm.c | 18 ++++++++++++++++--
>   1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> index 21e07a464bd9..9ac84b5d6ce0 100644
> --- a/drivers/firmware/qcom_scm.c
> +++ b/drivers/firmware/qcom_scm.c
> @@ -131,6 +131,7 @@ static enum qcom_scm_convention __get_convention(void)
>   	struct qcom_scm_res res;
>   	enum qcom_scm_convention probed_convention;
>   	int ret;
> +	bool forced = false;
>   
>   	if (likely(qcom_scm_convention != SMC_CONVENTION_UNKNOWN))
>   		return qcom_scm_convention;
> @@ -144,6 +145,18 @@ static enum qcom_scm_convention __get_convention(void)
>   	if (!ret && res.result[0] == 1)
>   		goto found;
>   
> +	/*
> +	 * Some SC7180 firmwares didn't implement the
> +	 * QCOM_SCM_INFO_IS_CALL_AVAIL call, so we fallback to forcing ARM_64
> +	 * calling conventions on these firmwares. Luckily we don't make any
> +	 * early calls into the firmware on these SoCs so the device pointer
> +	 * will be valid here to check if the compatible matches.
> +	 */
> +	if (of_device_is_compatible(__scm ? __scm->dev->of_node : NULL, "qcom,scm-sc7180")) {
> +		forced = true;
> +		goto found;
> +	}

All SC7180 targets run DT?  None have ACPI?
Stephen Boyd Feb. 23, 2021, 11:46 p.m. UTC | #2
Quoting Jeffrey Hugo (2021-02-23 15:38:38)
> On 2/23/2021 2:45 PM, Stephen Boyd wrote:

> > diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c

> > index 21e07a464bd9..9ac84b5d6ce0 100644

> > --- a/drivers/firmware/qcom_scm.c

> > +++ b/drivers/firmware/qcom_scm.c

> > @@ -144,6 +145,18 @@ static enum qcom_scm_convention __get_convention(void)

> >       if (!ret && res.result[0] == 1)

> >               goto found;

> >   

> > +     /*

> > +      * Some SC7180 firmwares didn't implement the

> > +      * QCOM_SCM_INFO_IS_CALL_AVAIL call, so we fallback to forcing ARM_64

> > +      * calling conventions on these firmwares. Luckily we don't make any

> > +      * early calls into the firmware on these SoCs so the device pointer

> > +      * will be valid here to check if the compatible matches.

> > +      */

> > +     if (of_device_is_compatible(__scm ? __scm->dev->of_node : NULL, "qcom,scm-sc7180")) {

> > +             forced = true;

> > +             goto found;

> > +     }

> 

> All SC7180 targets run DT?  None have ACPI?

> 


Yes, as far as I know all sc7180 boards are using DT. If they aren't,
then presumably they implemented this QCOM_SCM_INFO_IS_CALL_AVAIL call
so this check is still fine.
diff mbox series

Patch

diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 21e07a464bd9..9ac84b5d6ce0 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -131,6 +131,7 @@  static enum qcom_scm_convention __get_convention(void)
 	struct qcom_scm_res res;
 	enum qcom_scm_convention probed_convention;
 	int ret;
+	bool forced = false;
 
 	if (likely(qcom_scm_convention != SMC_CONVENTION_UNKNOWN))
 		return qcom_scm_convention;
@@ -144,6 +145,18 @@  static enum qcom_scm_convention __get_convention(void)
 	if (!ret && res.result[0] == 1)
 		goto found;
 
+	/*
+	 * Some SC7180 firmwares didn't implement the
+	 * QCOM_SCM_INFO_IS_CALL_AVAIL call, so we fallback to forcing ARM_64
+	 * calling conventions on these firmwares. Luckily we don't make any
+	 * early calls into the firmware on these SoCs so the device pointer
+	 * will be valid here to check if the compatible matches.
+	 */
+	if (of_device_is_compatible(__scm ? __scm->dev->of_node : NULL, "qcom,scm-sc7180")) {
+		forced = true;
+		goto found;
+	}
+
 	probed_convention = SMC_CONVENTION_ARM_32;
 	ret = __scm_smc_call(NULL, &desc, probed_convention, &res, true);
 	if (!ret && res.result[0] == 1)
@@ -154,8 +167,9 @@  static enum qcom_scm_convention __get_convention(void)
 	spin_lock_irqsave(&scm_query_lock, flags);
 	if (probed_convention != qcom_scm_convention) {
 		qcom_scm_convention = probed_convention;
-		pr_info("qcom_scm: convention: %s\n",
-			qcom_scm_convention_names[qcom_scm_convention]);
+		pr_info("qcom_scm: convention: %s%s\n",
+			qcom_scm_convention_names[qcom_scm_convention],
+			forced ? " (forced)" : "");
 	}
 	spin_unlock_irqrestore(&scm_query_lock, flags);