[PATCH v10 3/3] firmware: qcom_scm: Use TASK_IDLE state in wait_for_wq_completion()

Shivendra Pratap posted 3 patches 2 months, 1 week ago
There is a newer version of this series
[PATCH v10 3/3] firmware: qcom_scm: Use TASK_IDLE state in wait_for_wq_completion()
Posted by Shivendra Pratap 2 months, 1 week ago
From: Unnathi Chalicheemala <unnathi.chalicheemala@oss.qualcomm.com>

When the kernel issues an SMC (Secure Monitor Call) and the firmware
requests the kernel to wait, the waiting thread enters an
uninterruptible (D) state. In case of an extended wait request by the
firmware, any device suspend request, cannot proceed because of the
thread stuck in D state. This blocks the device suspend.

Replace wait_for_completion() with wait_for_completion_state(...,
TASK_IDLE), so that the waiting thread, show up in TASK_IDLE state,
instead of TASK_UNINTERRUPTIBLE (D state). This allows the thread to
block until completion, without blocking the device suspend.

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Unnathi Chalicheemala <unnathi.chalicheemala@oss.qualcomm.com>
Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
---
 drivers/firmware/qcom/qcom_scm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index ef3d81a5340512a79c252430db5f09cd8c253173..45951c04f377b725cdde4696d834435abd92f015 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -2316,7 +2316,7 @@ int qcom_scm_wait_for_wq_completion(u32 wq_ctx)
 	if (IS_ERR(wq))
 		return PTR_ERR(wq);
 
-	wait_for_completion(wq);
+	wait_for_completion_state(wq, TASK_IDLE);
 
 	return 0;
 }

-- 
2.34.1
Re: [PATCH v10 3/3] firmware: qcom_scm: Use TASK_IDLE state in wait_for_wq_completion()
Posted by Mukesh Ojha 2 months, 1 week ago
On Sun, Nov 30, 2025 at 08:11:04PM +0530, Shivendra Pratap wrote:
> From: Unnathi Chalicheemala <unnathi.chalicheemala@oss.qualcomm.com>
> 
> When the kernel issues an SMC (Secure Monitor Call) and the firmware
> requests the kernel to wait, the waiting thread enters an
> uninterruptible (D) state. In case of an extended wait request by the
> firmware, any device suspend request, cannot proceed because of the
> thread stuck in D state. This blocks the device suspend.

Not only that, it is stuck in D state after holding a mutex, which is
more severe because none of the SMC calls can go through. However, this
has been the case since the day the firmware started supporting waitq
with a single context, which is somewhat acceptable as the firmware does
not allow otherwise. Since, you are adding the multiple waitq context
supported by firmware with your other patches,

Do you plan to support multiple waitq contexts i.e parallel SCM calls,
in SCM driver as well ?

> 
> Replace wait_for_completion() with wait_for_completion_state(...,
> TASK_IDLE), so that the waiting thread, show up in TASK_IDLE state,
> instead of TASK_UNINTERRUPTIBLE (D state). This allows the thread to
> block until completion, without blocking the device suspend.
> 
> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> Signed-off-by: Unnathi Chalicheemala <unnathi.chalicheemala@oss.qualcomm.com>
> Signed-off-by: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>
> ---
>  drivers/firmware/qcom/qcom_scm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index ef3d81a5340512a79c252430db5f09cd8c253173..45951c04f377b725cdde4696d834435abd92f015 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -2316,7 +2316,7 @@ int qcom_scm_wait_for_wq_completion(u32 wq_ctx)
>  	if (IS_ERR(wq))
>  		return PTR_ERR(wq);
>  
> -	wait_for_completion(wq);
> +	wait_for_completion_state(wq, TASK_IDLE);
>  
>  	return 0;
>  }
> 
> -- 
> 2.34.1
> 

-- 
-Mukesh Ojha
Re: [PATCH v10 3/3] firmware: qcom_scm: Use TASK_IDLE state in wait_for_wq_completion()
Posted by Shivendra Pratap 2 months, 1 week ago

On 12/3/2025 2:01 PM, Mukesh Ojha wrote:
> On Sun, Nov 30, 2025 at 08:11:04PM +0530, Shivendra Pratap wrote:
>> From: Unnathi Chalicheemala <unnathi.chalicheemala@oss.qualcomm.com>
>>
>> When the kernel issues an SMC (Secure Monitor Call) and the firmware
>> requests the kernel to wait, the waiting thread enters an
>> uninterruptible (D) state. In case of an extended wait request by the
>> firmware, any device suspend request, cannot proceed because of the
>> thread stuck in D state. This blocks the device suspend.
> 
> Not only that, it is stuck in D state after holding a mutex, which is
> more severe because none of the SMC calls can go through. However, this
> has been the case since the day the firmware started supporting waitq
> with a single context, which is somewhat acceptable as the firmware does
> not allow otherwise. Since, you are adding the multiple waitq context
> supported by firmware with your other patches,
> 
> Do you plan to support multiple waitq contexts i.e parallel SCM calls,
> in SCM driver as well ?

Yes. Parallel SCM can be enabled once this waitq enablement support is
in place.

thanks,
Shivendra
Re: [PATCH v10 3/3] firmware: qcom_scm: Use TASK_IDLE state in wait_for_wq_completion()
Posted by Mukesh Ojha 2 months, 1 week ago
On Wed, Dec 03, 2025 at 04:27:25PM +0530, Shivendra Pratap wrote:
> 
> 
> On 12/3/2025 2:01 PM, Mukesh Ojha wrote:
> > On Sun, Nov 30, 2025 at 08:11:04PM +0530, Shivendra Pratap wrote:
> >> From: Unnathi Chalicheemala <unnathi.chalicheemala@oss.qualcomm.com>
> >>
> >> When the kernel issues an SMC (Secure Monitor Call) and the firmware
> >> requests the kernel to wait, the waiting thread enters an
> >> uninterruptible (D) state. In case of an extended wait request by the
> >> firmware, any device suspend request, cannot proceed because of the
> >> thread stuck in D state. This blocks the device suspend.
> > 
> > Not only that, it is stuck in D state after holding a mutex, which is
> > more severe because none of the SMC calls can go through. However, this
> > has been the case since the day the firmware started supporting waitq
> > with a single context, which is somewhat acceptable as the firmware does
> > not allow otherwise. Since, you are adding the multiple waitq context
> > supported by firmware with your other patches,
> > 
> > Do you plan to support multiple waitq contexts i.e parallel SCM calls,
> > in SCM driver as well ?
> 
> Yes. Parallel SCM can be enabled once this waitq enablement support is
> in place.

Thanks.

Reviewed-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>

> 
> thanks,
> Shivendra

-- 
-Mukesh Ojha