[PATCH 1/3] firmware: qcom-scm: Initialize waitq before setting global __scm

Unnathi Chalicheemala posted 3 patches 1 year, 11 months ago
There is a newer version of this series
[PATCH 1/3] firmware: qcom-scm: Initialize waitq before setting global __scm
Posted by Unnathi Chalicheemala 1 year, 11 months ago
Suppose there's another device probing at the same time as SCM driver.
It calls into SCM and triggers WAITQ sleep but waitq completion
structure may not be initialized yet.
Fix this by moving initialization of waitq before setting global __scm.

Fixes: 6bf325992236 ("firmware: qcom: scm: Add wait-queue handling logic")
Signed-off-by: Unnathi Chalicheemala <quic_uchalich@quicinc.com>
---
 drivers/firmware/qcom/qcom_scm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 520de9b5633a..c1be8270ead1 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -1854,11 +1854,11 @@ static int qcom_scm_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	init_completion(&scm->waitq_comp);
+
 	__scm = scm;
 	__scm->dev = &pdev->dev;
 
-	init_completion(&__scm->waitq_comp);
-
 	irq = platform_get_irq_optional(pdev, 0);
 	if (irq < 0) {
 		if (irq != -ENXIO)

-- 
2.25.1
Re: [PATCH 1/3] firmware: qcom-scm: Initialize waitq before setting global __scm
Posted by Bartosz Golaszewski 1 year, 11 months ago
On Wed, Feb 28, 2024 at 7:50 PM Unnathi Chalicheemala
<quic_uchalich@quicinc.com> wrote:
>
> Suppose there's another device probing at the same time as SCM driver.
> It calls into SCM and triggers WAITQ sleep but waitq completion
> structure may not be initialized yet.
> Fix this by moving initialization of waitq before setting global __scm.
>
> Fixes: 6bf325992236 ("firmware: qcom: scm: Add wait-queue handling logic")
> Signed-off-by: Unnathi Chalicheemala <quic_uchalich@quicinc.com>
> ---
>  drivers/firmware/qcom/qcom_scm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 520de9b5633a..c1be8270ead1 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -1854,11 +1854,11 @@ static int qcom_scm_probe(struct platform_device *pdev)
>         if (ret)
>                 return ret;
>
> +       init_completion(&scm->waitq_comp);
> +
>         __scm = scm;
>         __scm->dev = &pdev->dev;

If another driver can actually probe at the same time then this needs
to be moved to the end of this function, the `__scm->dev` assignment
must be done before the `__scm = scm` one and it must use
WRITE_ONCE().

Bart

>
> -       init_completion(&__scm->waitq_comp);
> -
>         irq = platform_get_irq_optional(pdev, 0);
>         if (irq < 0) {
>                 if (irq != -ENXIO)
>
> --
> 2.25.1
>
>