diff mbox series

[v2] ASoC: qcom: lpass-platform: Reset irq clear reg post handling interrupts

Message ID 20210908142535.31106-1-srivasam@codeaurora.org
State New
Headers show
Series [v2] ASoC: qcom: lpass-platform: Reset irq clear reg post handling interrupts | expand

Commit Message

Srinivasa Rao Mandadapu Sept. 8, 2021, 2:25 p.m. UTC
Update interrupt clear register with reset value after addressing
all interrupts. This is to fix playback or capture hanging issue in
simultaneous playback and capture usecase.

Fixes: 4f629e4b8705f ("ASoC: qcom: Add ability to handle interrupts per dma channel")

Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
---
Changes since v1:
    -- Update comments Header information with fixes tag

 sound/soc/qcom/lpass-platform.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Srinivas Kandagatla Sept. 8, 2021, 4:07 p.m. UTC | #1
On 08/09/2021 15:25, Srinivasa Rao Mandadapu wrote:
> Update interrupt clear register with reset value after addressing
> all interrupts. This is to fix playback or capture hanging issue in
> simultaneous playback and capture usecase.

Could explain bit more about the issue.
Specifically which interrupt and which ports is this issue seen.

> 
> Fixes: 4f629e4b8705f ("ASoC: qcom: Add ability to handle interrupts per dma channel")
> 
> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
> ---
> Changes since v1:
>      -- Update comments Header information with fixes tag
> 
>   sound/soc/qcom/lpass-platform.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
> index f9df76d37858..1a0a4b0b1a03 100644
> --- a/sound/soc/qcom/lpass-platform.c
> +++ b/sound/soc/qcom/lpass-platform.c
> @@ -749,6 +749,12 @@ static irqreturn_t lpass_platform_lpaif_irq(int irq, void *data)
>   		}
>   	}
>   
> +	rv = regmap_write(drvdata->lpaif_map, LPAIF_IRQCLEAR_REG(v, LPAIF_IRQ_PORT_HOST), 0x0);

Writing 1 to a bit of this register will clear the corresponding latched 
interrupt. So I don't really understand how writing 0 is really helping 
here?

Do you have this patch in your tree?

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/sound/soc/qcom/lpass-cpu.c?h=v5.14&id=6a7f5bd6185e1c86256d5e52c3bb7a4d390d6e19


--srini

> +	if (rv) {
> +		pr_err("error writing to irqstat reg: %d\n", rv);
> +		return IRQ_NONE;
> +	}
> +
>   	return IRQ_HANDLED;
>   }
>   
>
Srinivasa Rao Mandadapu Sept. 9, 2021, 12:11 p.m. UTC | #2
Thanks for Your time Srini!!

On 9/8/2021 9:37 PM, Srinivas Kandagatla wrote:
>
>
> On 08/09/2021 15:25, Srinivasa Rao Mandadapu wrote:
>> Update interrupt clear register with reset value after addressing
>> all interrupts. This is to fix playback or capture hanging issue in
>> simultaneous playback and capture usecase.
>
> Could explain bit more about the issue.
> Specifically which interrupt and which ports is this issue seen.

The test case is repeated test of simultaneous playback and capture on 
audio jack. i.e. 10 sec playback and 10 secs Capture.

i.e. sequence is Capture --> Playback, Capture -> Playback

In interrupt handler reading IRQ status reg and processing all the bits 
in loop. By the time loop ends in handler,

another capture interrupt is raised and it's never getting cleared and 
whole test case is getting hanged.

>
>>
>> Fixes: 4f629e4b8705f ("ASoC: qcom: Add ability to handle interrupts 
>> per dma channel")
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
>> ---
>> Changes since v1:
>>      -- Update comments Header information with fixes tag
>>
>>   sound/soc/qcom/lpass-platform.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/sound/soc/qcom/lpass-platform.c 
>> b/sound/soc/qcom/lpass-platform.c
>> index f9df76d37858..1a0a4b0b1a03 100644
>> --- a/sound/soc/qcom/lpass-platform.c
>> +++ b/sound/soc/qcom/lpass-platform.c
>> @@ -749,6 +749,12 @@ static irqreturn_t lpass_platform_lpaif_irq(int 
>> irq, void *data)
>>           }
>>       }
>>   +    rv = regmap_write(drvdata->lpaif_map, LPAIF_IRQCLEAR_REG(v, 
>> LPAIF_IRQ_PORT_HOST), 0x0);
>
> Writing 1 to a bit of this register will clear the corresponding 
> latched interrupt. So I don't really understand how writing 0 is 
> really helping here?

Yes. Your point is valid. But use-case passed with below patch and this 
change. will revert this change and check again with only below patch .

https://patchwork.kernel.org/project/alsa-devel/patch/20210524142114.18676-1-srivasam@codeaurora.org/ 
<https://www.google.com/url?q=https://patchwork.kernel.org/project/alsa-devel/patch/20210524142114.18676-1-srivasam@codeaurora.org/&sa=D&usg=AOvVaw3VEq8XuEI8cJkNpAxTm3d2>

>
> Do you have this patch in your tree?
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/sound/soc/qcom/lpass-cpu.c?h=v5.14&id=6a7f5bd6185e1c86256d5e52c3bb7a4d390d6e19 
>
yes. This patch is available.

> --srini
>
>> +    if (rv) {
>> +        pr_err("error writing to irqstat reg: %d\n", rv);
>> +        return IRQ_NONE;
>> +    }
>> +
>>       return IRQ_HANDLED;
>>   }
>>
diff mbox series

Patch

diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
index f9df76d37858..1a0a4b0b1a03 100644
--- a/sound/soc/qcom/lpass-platform.c
+++ b/sound/soc/qcom/lpass-platform.c
@@ -749,6 +749,12 @@  static irqreturn_t lpass_platform_lpaif_irq(int irq, void *data)
 		}
 	}
 
+	rv = regmap_write(drvdata->lpaif_map, LPAIF_IRQCLEAR_REG(v, LPAIF_IRQ_PORT_HOST), 0x0);
+	if (rv) {
+		pr_err("error writing to irqstat reg: %d\n", rv);
+		return IRQ_NONE;
+	}
+
 	return IRQ_HANDLED;
 }