[PATCH V1] ufs: core: Fix interrupt handling for MCQ Mode in ufshcd_intr

[PATCH V1] ufs: core: Fix interrupt handling for MCQ Mode in ufshcd_intr

[Nitin Rawat]

Commit 3c7ac40d7322 ("scsi: ufs: core: Delegate the interrupt service
routine to a threaded IRQ handler") introduced a regression where
the UFS interrupt status register (IS) was not cleared in
ufshcd_intr() when operating in MCQ mode. As a result, the IS register
remained uncleared.

This led to a persistent issue during UIC interrupts:
ufshcd_is_auto_hibern8_error() consistently returned true because the
UFSHCD_UIC_HIBERN8_MASK bit was set, while the active command was
neither UIC_CMD_DME_HIBER_ENTER nor UIC_CMD_DME_HIBER_EXIT. This
caused continuous auto hibern8 enter errors and device failed to boot.

To fix this, the patch ensures that the interrupt status register is
properly cleared in the ufshcd_intr() function for both MCQ mode with
ESI enabled.

[    4.553226] ufshcd-qcom 1d84000.ufs: ufshcd_check_errors: Auto
Hibern8 Enter failed - status: 0x00000040, upmcrs: 0x00000001
[    4.553229] ufshcd-qcom 1d84000.ufs: ufshcd_check_errors: saved_err
0x40 saved_uic_err 0x0
[    4.553311] host_regs: 00000000: d5c7033f 20e0071f 00000400 00000000
[    4.553312] host_regs: 00000010: 01000000 00010217 00000c96 00000000
[    4.553314] host_regs: 00000020: 00000440 00170ef5 00000000 00000000
[    4.553316] host_regs: 00000030: 0000010f 00000001 00000000 00000000
[    4.553317] host_regs: 00000040: 00000000 00000000 00000000 00000000
[    4.553319] host_regs: 00000050: fffdf000 0000000f 00000000 00000000
[    4.553320] host_regs: 00000060: 00000001 80000000 00000000 00000000
[    4.553322] host_regs: 00000070: fffde000 0000000f 00000000 00000000
[    4.553323] host_regs: 00000080: 00000001 00000000 00000000 00000000
[    4.553325] host_regs: 00000090: 00000002 d0020000 00000000 01930200

Fixes: 3c7ac40d7322 ("scsi: ufs: core: Delegate the interrupt service routine to a threaded IRQ handler")
Signed-off-by: Palash Kambar <quic_pkambar@quicinc.com>
Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>

---
 drivers/ufs/core/ufshcd.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index fd8015ed36a4..5413464d63c8 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -7145,14 +7145,19 @@ static irqreturn_t ufshcd_threaded_intr(int irq, void *__hba)
 static irqreturn_t ufshcd_intr(int irq, void *__hba)
 {
 	struct ufs_hba *hba = __hba;
+	u32 intr_status, enabled_intr_status;

 	/* Move interrupt handling to thread when MCQ & ESI are not enabled */
 	if (!hba->mcq_enabled || !hba->mcq_esi_enabled)
 		return IRQ_WAKE_THREAD;

+	intr_status = ufshcd_readl(hba, REG_INTERRUPT_STATUS);
+	enabled_intr_status = intr_status & ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
+
+	ufshcd_writel(hba, intr_status, REG_INTERRUPT_STATUS);
+
 	/* Directly handle interrupts since MCQ ESI handlers does the hard job */
-	return ufshcd_sl_intr(hba, ufshcd_readl(hba, REG_INTERRUPT_STATUS) &
-				   ufshcd_readl(hba, REG_INTERRUPT_ENABLE));
+	return ufshcd_sl_intr(hba, enabled_intr_status);
 }

 static int ufshcd_clear_tm_cmd(struct ufs_hba *hba, int tag)
--
2.48.1

Re: [PATCH V1] ufs: core: Fix interrupt handling for MCQ Mode in ufshcd_intr

[Bart Van Assche]

On 7/28/25 3:57 PM, Nitin Rawat wrote:
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index fd8015ed36a4..5413464d63c8 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -7145,14 +7145,19 @@ static irqreturn_t ufshcd_threaded_intr(int irq, void *__hba)
>   static irqreturn_t ufshcd_intr(int irq, void *__hba)
>   {
>   	struct ufs_hba *hba = __hba;
> +	u32 intr_status, enabled_intr_status;
> 
>   	/* Move interrupt handling to thread when MCQ & ESI are not enabled */
>   	if (!hba->mcq_enabled || !hba->mcq_esi_enabled)
>   		return IRQ_WAKE_THREAD;
> 
> +	intr_status = ufshcd_readl(hba, REG_INTERRUPT_STATUS);
> +	enabled_intr_status = intr_status & ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
> +
> +	ufshcd_writel(hba, intr_status, REG_INTERRUPT_STATUS);
> +
>   	/* Directly handle interrupts since MCQ ESI handlers does the hard job */
> -	return ufshcd_sl_intr(hba, ufshcd_readl(hba, REG_INTERRUPT_STATUS) &
> -				   ufshcd_readl(hba, REG_INTERRUPT_ENABLE));
> +	return ufshcd_sl_intr(hba, enabled_intr_status);
>   }

Hi Nitin,

Thank you for having published this patch. It seems like we both have 
been working on a fix independently and without knowing about each 
other's efforts. Can you please take a look at this patch and let me
know which version you prefer?

https://lore.kernel.org/linux-scsi/20250728212731.899429-1-bvanassche@acm.org/

Thanks,

Bart.

Re: [PATCH V1] ufs: core: Fix interrupt handling for MCQ Mode in ufshcd_intr

[Nitin Rawat]

On 7/29/2025 5:11 AM, Bart Van Assche wrote:
> On 7/28/25 3:57 PM, Nitin Rawat wrote:
>> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
>> index fd8015ed36a4..5413464d63c8 100644
>> --- a/drivers/ufs/core/ufshcd.c
>> +++ b/drivers/ufs/core/ufshcd.c
>> @@ -7145,14 +7145,19 @@ static irqreturn_t ufshcd_threaded_intr(int 
>> irq, void *__hba)
>>   static irqreturn_t ufshcd_intr(int irq, void *__hba)
>>   {
>>       struct ufs_hba *hba = __hba;
>> +    u32 intr_status, enabled_intr_status;
>>
>>       /* Move interrupt handling to thread when MCQ & ESI are not 
>> enabled */
>>       if (!hba->mcq_enabled || !hba->mcq_esi_enabled)
>>           return IRQ_WAKE_THREAD;
>>
>> +    intr_status = ufshcd_readl(hba, REG_INTERRUPT_STATUS);
>> +    enabled_intr_status = intr_status & ufshcd_readl(hba, 
>> REG_INTERRUPT_ENABLE);
>> +
>> +    ufshcd_writel(hba, intr_status, REG_INTERRUPT_STATUS);
>> +
>>       /* Directly handle interrupts since MCQ ESI handlers does the 
>> hard job */
>> -    return ufshcd_sl_intr(hba, ufshcd_readl(hba, REG_INTERRUPT_STATUS) &
>> -                   ufshcd_readl(hba, REG_INTERRUPT_ENABLE));
>> +    return ufshcd_sl_intr(hba, enabled_intr_status);
>>   }
> 
> Hi Nitin,
> 
> Thank you for having published this patch. It seems like we both have 
> been working on a fix independently and without knowing about each 
> other's efforts. Can you please take a look at this patch and let me
> know which version you prefer?

Hi Bart,

I reviewed your patch and test it locally—it resolves the issue.

The patch looks good. Since this path handles only UIC, TM, and error 
conditions with no IO for MCQ, we still check for outstanding_reqs and 
UTP_TRANSFER_REQ_COMPL for the error case within ufshcd_threaded_intr in 
the patch. In my opinion, we can skip these additional checks.

Thanks,
Nitin

> 
> https://lore.kernel.org/linux-scsi/20250728212731.899429-1- 
> bvanassche@acm.org/
> 
> Thanks,
> 
> Bart.

Re: [PATCH V1] ufs: core: Fix interrupt handling for MCQ Mode in ufshcd_intr

[Bart Van Assche]

On 7/29/25 7:37 AM, Nitin Rawat wrote:
> I reviewed your patch and test it locally—it resolves the issue.

Thanks!

> The patch looks good. Since this path handles only UIC, TM, and error 
> conditions with no IO for MCQ, we still check for outstanding_reqs and 
> UTP_TRANSFER_REQ_COMPL for the error case within ufshcd_threaded_intr in 
> the patch. In my opinion, we can skip these additional checks.

We can only skip the outstanding_reqs check if MCQ is enabled. André
Draszik is working on a patch that will cause ufshcd_intr() to be called
again for legacy mode so I prefer to keep the outstanding_reqs check.

Bart.

Re: [PATCH V1] ufs: core: Fix interrupt handling for MCQ Mode in ufshcd_intr

[Peter Wang (王信友)]

On Tue, 2025-07-29 at 09:24 -0700, Bart Van Assche wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 7/29/25 7:37 AM, Nitin Rawat wrote:
> > I reviewed your patch and test it locally—it resolves the issue.
> 
> Thanks!
> 
> > The patch looks good. Since this path handles only UIC, TM, and
> > error
> > conditions with no IO for MCQ, we still check for outstanding_reqs
> > and
> > UTP_TRANSFER_REQ_COMPL for the error case within
> > ufshcd_threaded_intr in
> > the patch. In my opinion, we can skip these additional checks.
> 
> We can only skip the outstanding_reqs check if MCQ is enabled. André
> Draszik is working on a patch that will cause ufshcd_intr() to be
> called
> again for legacy mode so I prefer to keep the outstanding_reqs check.
> 
> Bart.

Hi Bart,

The threaded ISR was separated out specifically to address
the issues of the traditional ISR, because a traditional ISR
must be very fast and short, as it blocks other interrupts.
But your patch letting the traditional ISR call the threaded
ISR, doesn’t this bring back the problem where the threaded
ISR might block other interrupts?

So, I prefer this patch clear the interrupt status register
(IS) directly.

Thanks.
Peter

Re: [PATCH V1] ufs: core: Fix interrupt handling for MCQ Mode in ufshcd_intr

[Bart Van Assche]

On 7/29/25 11:41 PM, Peter Wang (王信友) wrote:
> The threaded ISR was separated out specifically to address
> the issues of the traditional ISR, because a traditional ISR
> must be very fast and short, as it blocks other interrupts.
> But your patch letting the traditional ISR call the threaded
> ISR, doesn’t this bring back the problem where the threaded
> ISR might block other interrupts?
> 
> So, I prefer this patch clear the interrupt status register
> (IS) directly.
Hi Peter,

Thanks for having taken a look but I think that you misunderstood my
patch. My patch does not modify the behavior of the UFS driver on
systems using the legacy single doorbell mode. It only modifies the
behavior on systems supporting MCQ. And on systems that support MCQ, it
restores the behavior from before when commit 3c7ac40d7322 ("scsi: ufs:
core: Delegate the interrupt service routine to a threaded IRQ handler")
got merged.

On MCQ systems, if the completion interrupt is processed by the CPU core
that submitted the I/O, then I/O workloads are self-regulating. The more
time that is spent in the completion interrupt handler, the more the
submitter will be slowed down. This is not the case in legacy single
doorbell mode since when using that mode all completion interrupts are
processed by a single CPU core.

Bart.

Re: [PATCH V1] ufs: core: Fix interrupt handling for MCQ Mode in ufshcd_intr

[neil.armstrong]

On 29/07/2025 00:57, Nitin Rawat wrote:
> Commit 3c7ac40d7322 ("scsi: ufs: core: Delegate the interrupt service
> routine to a threaded IRQ handler") introduced a regression where
> the UFS interrupt status register (IS) was not cleared in
> ufshcd_intr() when operating in MCQ mode. As a result, the IS register
> remained uncleared.
> 
> This led to a persistent issue during UIC interrupts:
> ufshcd_is_auto_hibern8_error() consistently returned true because the
> UFSHCD_UIC_HIBERN8_MASK bit was set, while the active command was
> neither UIC_CMD_DME_HIBER_ENTER nor UIC_CMD_DME_HIBER_EXIT. This
> caused continuous auto hibern8 enter errors and device failed to boot.
> 
> To fix this, the patch ensures that the interrupt status register is
> properly cleared in the ufshcd_intr() function for both MCQ mode with
> ESI enabled.
> 
> [    4.553226] ufshcd-qcom 1d84000.ufs: ufshcd_check_errors: Auto
> Hibern8 Enter failed - status: 0x00000040, upmcrs: 0x00000001
> [    4.553229] ufshcd-qcom 1d84000.ufs: ufshcd_check_errors: saved_err
> 0x40 saved_uic_err 0x0
> [    4.553311] host_regs: 00000000: d5c7033f 20e0071f 00000400 00000000
> [    4.553312] host_regs: 00000010: 01000000 00010217 00000c96 00000000
> [    4.553314] host_regs: 00000020: 00000440 00170ef5 00000000 00000000
> [    4.553316] host_regs: 00000030: 0000010f 00000001 00000000 00000000
> [    4.553317] host_regs: 00000040: 00000000 00000000 00000000 00000000
> [    4.553319] host_regs: 00000050: fffdf000 0000000f 00000000 00000000
> [    4.553320] host_regs: 00000060: 00000001 80000000 00000000 00000000
> [    4.553322] host_regs: 00000070: fffde000 0000000f 00000000 00000000
> [    4.553323] host_regs: 00000080: 00000001 00000000 00000000 00000000
> [    4.553325] host_regs: 00000090: 00000002 d0020000 00000000 01930200
> 
> Fixes: 3c7ac40d7322 ("scsi: ufs: core: Delegate the interrupt service routine to a threaded IRQ handler")
> Signed-off-by: Palash Kambar <quic_pkambar@quicinc.com>
> Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
> 
> ---
>   drivers/ufs/core/ufshcd.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index fd8015ed36a4..5413464d63c8 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -7145,14 +7145,19 @@ static irqreturn_t ufshcd_threaded_intr(int irq, void *__hba)
>   static irqreturn_t ufshcd_intr(int irq, void *__hba)
>   {
>   	struct ufs_hba *hba = __hba;
> +	u32 intr_status, enabled_intr_status;
> 
>   	/* Move interrupt handling to thread when MCQ & ESI are not enabled */
>   	if (!hba->mcq_enabled || !hba->mcq_esi_enabled)
>   		return IRQ_WAKE_THREAD;
> 
> +	intr_status = ufshcd_readl(hba, REG_INTERRUPT_STATUS);
> +	enabled_intr_status = intr_status & ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
> +
> +	ufshcd_writel(hba, intr_status, REG_INTERRUPT_STATUS);
> +
>   	/* Directly handle interrupts since MCQ ESI handlers does the hard job */
> -	return ufshcd_sl_intr(hba, ufshcd_readl(hba, REG_INTERRUPT_STATUS) &
> -				   ufshcd_readl(hba, REG_INTERRUPT_ENABLE));
> +	return ufshcd_sl_intr(hba, enabled_intr_status);
>   }
> 
>   static int ufshcd_clear_tm_cmd(struct ufs_hba *hba, int tag)
> --
> 2.48.1
> 
> 

Thanks to [1], I was able to test this change on SM8650-QRD, and it fixed the issue:

Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8650-QRD

Thanks!
Neil

[1] https://lore.kernel.org/all/20250730082229.23475-1-quic_rdwivedi@quicinc.com/

Re: [PATCH V1] ufs: core: Fix interrupt handling for MCQ Mode in ufshcd_intr

[Peter Wang (王信友)]

On Wed, 2025-07-30 at 09:04 -0700, Bart Van Assche wrote:
> 
> Hi Peter,
> 
> Thanks for having taken a look but I think that you misunderstood my
> patch. My patch does not modify the behavior of the UFS driver on
> systems using the legacy single doorbell mode. It only modifies the
> behavior on systems supporting MCQ. And on systems that support MCQ,
> it
> restores the behavior from before when commit 3c7ac40d7322 ("scsi:
> ufs:
> core: Delegate the interrupt service routine to a threaded IRQ
> handler")
> got merged.
> 
> On MCQ systems, if the completion interrupt is processed by the CPU
> core
> that submitted the I/O, then I/O workloads are self-regulating. The
> more
> time that is spent in the completion interrupt handler, the more the
> submitter will be slowed down. This is not the case in legacy single
> doorbell mode since when using that mode all completion interrupts
> are
> processed by a single CPU core.
> 
> Bart.


Hi Bart,

I am fully agree that your patch is 100% correct.

But what I want to express is that this is a patch
that makes me feel confused about the code.
Originally, ufshcd_intr moved some heavy (legacy) work
to ufshcd_threaded_intr because those work could take
a long time to execute and affect other module IRQs.
This means, in terms of execution time, ufshcd_threaded_intr
could be longer than ufshcd_intr. But if ufshcd_intr calls 
ufshcd_threaded_intr, then the execution time of 
ufshcd_intr becomes equal to ufshcd_threaded_intr, 
and that’s the part confuses me.

Of course, if you look deeper into ufshcd_threaded_intr,
you'll see that in MCQ mode, it doesn’t actually run for 
very long time. But compared to this patch, the readability
would be a bit worse. So I prefer this patch.

Thanks
Peter

Re: [PATCH V1] ufs: core: Fix interrupt handling for MCQ Mode in ufshcd_intr

[Bart Van Assche]

On 7/28/25 3:57 PM, Nitin Rawat wrote:
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index fd8015ed36a4..5413464d63c8 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -7145,14 +7145,19 @@ static irqreturn_t ufshcd_threaded_intr(int irq, void *__hba)
>   static irqreturn_t ufshcd_intr(int irq, void *__hba)
>   {
>   	struct ufs_hba *hba = __hba;
> +	u32 intr_status, enabled_intr_status;
> 
>   	/* Move interrupt handling to thread when MCQ & ESI are not enabled */
>   	if (!hba->mcq_enabled || !hba->mcq_esi_enabled)
>   		return IRQ_WAKE_THREAD;
> 
> +	intr_status = ufshcd_readl(hba, REG_INTERRUPT_STATUS);
> +	enabled_intr_status = intr_status & ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
> +
> +	ufshcd_writel(hba, intr_status, REG_INTERRUPT_STATUS);
> +
>   	/* Directly handle interrupts since MCQ ESI handlers does the hard job */
> -	return ufshcd_sl_intr(hba, ufshcd_readl(hba, REG_INTERRUPT_STATUS) &
> -				   ufshcd_readl(hba, REG_INTERRUPT_ENABLE));
> +	return ufshcd_sl_intr(hba, enabled_intr_status);
>   }
> 
>   static int ufshcd_clear_tm_cmd(struct ufs_hba *hba, int tag)

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

Martin, this patch seems to be more popular than my "[PATCH v2] ufs:
core: Fix interrupt handling". Please consider queuing this patch when
this is convenient for you.

Thanks,

Bart.

Re: [PATCH V1] ufs: core: Fix interrupt handling for MCQ Mode in ufshcd_intr

[Peter Wang (王信友)]

On Tue, 2025-07-29 at 04:27 +0530, Nitin Rawat wrote:
> Commit 3c7ac40d7322 ("scsi: ufs: core: Delegate the interrupt service
> routine to a threaded IRQ handler") introduced a regression where
> the UFS interrupt status register (IS) was not cleared in
> ufshcd_intr() when operating in MCQ mode. As a result, the IS
> register
> remained uncleared.
> 
> This led to a persistent issue during UIC interrupts:
> ufshcd_is_auto_hibern8_error() consistently returned true because the
> UFSHCD_UIC_HIBERN8_MASK bit was set, while the active command was
> neither UIC_CMD_DME_HIBER_ENTER nor UIC_CMD_DME_HIBER_EXIT. This
> caused continuous auto hibern8 enter errors and device failed to
> boot.
> 
> To fix this, the patch ensures that the interrupt status register is
> properly cleared in the ufshcd_intr() function for both MCQ mode with
> ESI enabled.
> 
> [    4.553226] ufshcd-qcom 1d84000.ufs: ufshcd_check_errors: Auto
> Hibern8 Enter failed - status: 0x00000040, upmcrs: 0x00000001
> [    4.553229] ufshcd-qcom 1d84000.ufs: ufshcd_check_errors:
> saved_err
> 0x40 saved_uic_err 0x0
> [    4.553311] host_regs: 00000000: d5c7033f 20e0071f 00000400
> 00000000
> [    4.553312] host_regs: 00000010: 01000000 00010217 00000c96
> 00000000
> [    4.553314] host_regs: 00000020: 00000440 00170ef5 00000000
> 00000000
> [    4.553316] host_regs: 00000030: 0000010f 00000001 00000000
> 00000000
> [    4.553317] host_regs: 00000040: 00000000 00000000 00000000
> 00000000
> [    4.553319] host_regs: 00000050: fffdf000 0000000f 00000000
> 00000000
> [    4.553320] host_regs: 00000060: 00000001 80000000 00000000
> 00000000
> [    4.553322] host_regs: 00000070: fffde000 0000000f 00000000
> 00000000
> [    4.553323] host_regs: 00000080: 00000001 00000000 00000000
> 00000000
> [    4.553325] host_regs: 00000090: 00000002 d0020000 00000000
> 01930200
> 
> Fixes: 3c7ac40d7322 ("scsi: ufs: core: Delegate the interrupt service
> routine to a threaded IRQ handler")
> Signed-off-by: Palash Kambar <quic_pkambar@quicinc.com>
> Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
> 

Reviewed-by: Peter Wang <peter.wang@mediatek.com>

Re: [PATCH V1] ufs: core: Fix interrupt handling for MCQ Mode in ufshcd_intr

[Martin K. Petersen]

>> Commit 3c7ac40d7322 ("scsi: ufs: core: Delegate the interrupt service
>> routine to a threaded IRQ handler") introduced a regression where the
>> UFS interrupt status register (IS) was not cleared in ufshcd_intr()
>> when operating in MCQ mode. As a result, the IS register remained
>> uncleared.

Applied to 6.17/scsi-staging, thanks!

-- 
Martin K. Petersen