* [PATCH v2] ASoC: qcom: Fix for DMA interrupt clear reg overwriting
@ 2021-06-05 11:38 Srinivasa Rao Mandadapu
2021-06-07 14:50 ` Srinivas Kandagatla
0 siblings, 1 reply; 3+ messages in thread
From: Srinivasa Rao Mandadapu @ 2021-06-05 11:38 UTC (permalink / raw)
To: agross, bjorn.andersson, lgirdwood, broonie, robh+dt, plai,
bgoswami, perex, tiwai, srinivas.kandagatla, rohitkr,
linux-arm-msm, alsa-devel, devicetree, linux-kernel, swboyd,
judyhsiao
Cc: Srinivasa Rao Mandadapu
The DMA interrupt clear register overwritten during
simultaneous playback and capture in lpass platform
interrupt handler. It's causing playback or capture stuck
in similtaneous plaback on speaker and capture on dmic test.
Update appropriate reg fields of corresponding channel instead
of entire register write.
Fixes: commit c5c8635a04711 ("ASoC: qcom: Add LPASS platform driver")
Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
---
sound/soc/qcom/lpass-platform.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
index 0df9481ea4c6..f220a2739ac3 100644
--- a/sound/soc/qcom/lpass-platform.c
+++ b/sound/soc/qcom/lpass-platform.c
@@ -526,7 +526,7 @@ static int lpass_platform_pcmops_trigger(struct snd_soc_component *component,
return -EINVAL;
}
- ret = regmap_write(map, reg_irqclr, val_irqclr);
+ ret = regmap_update_bits(map, reg_irqclr, val_irqclr, val_irqclr);
if (ret) {
dev_err(soc_runtime->dev, "error writing to irqclear reg: %d\n", ret);
return ret;
@@ -650,7 +650,7 @@ static irqreturn_t lpass_dma_interrupt_handler(
struct lpass_variant *v = drvdata->variant;
irqreturn_t ret = IRQ_NONE;
int rv;
- unsigned int reg = 0, val = 0;
+ unsigned int reg, val, val_clr, val_mask;
struct regmap *map;
unsigned int dai_id = cpu_dai->driver->id;
@@ -676,8 +676,9 @@ static irqreturn_t lpass_dma_interrupt_handler(
return -EINVAL;
}
if (interrupts & LPAIF_IRQ_PER(chan)) {
-
- rv = regmap_write(map, reg, LPAIF_IRQ_PER(chan) | val);
+ val_clr = LPAIF_IRQ_PER(chan) | val;
+ val_mask = LPAIF_IRQ_ALL(chan);
+ rv = regmap_update_bits(map, reg, val_mask, val_clr);
if (rv) {
dev_err(soc_runtime->dev,
"error writing to irqclear reg: %d\n", rv);
@@ -688,7 +689,9 @@ static irqreturn_t lpass_dma_interrupt_handler(
}
if (interrupts & LPAIF_IRQ_XRUN(chan)) {
- rv = regmap_write(map, reg, LPAIF_IRQ_XRUN(chan) | val);
+ val_clr = (LPAIF_IRQ_XRUN(chan) | val);
+ val_mask = LPAIF_IRQ_ALL(chan);
+ rv = regmap_update_bits(map, reg, val_mask, val_clr);
if (rv) {
dev_err(soc_runtime->dev,
"error writing to irqclear reg: %d\n", rv);
@@ -700,7 +703,9 @@ static irqreturn_t lpass_dma_interrupt_handler(
}
if (interrupts & LPAIF_IRQ_ERR(chan)) {
- rv = regmap_write(map, reg, LPAIF_IRQ_ERR(chan) | val);
+ val_clr = (LPAIF_IRQ_ERR(chan) | val);
+ val_mask = LPAIF_IRQ_ALL(chan);
+ rv = regmap_update_bits(map, reg, val_mask, val_clr);
if (rv) {
dev_err(soc_runtime->dev,
"error writing to irqclear reg: %d\n", rv);
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v2] ASoC: qcom: Fix for DMA interrupt clear reg overwriting
2021-06-05 11:38 [PATCH v2] ASoC: qcom: Fix for DMA interrupt clear reg overwriting Srinivasa Rao Mandadapu
@ 2021-06-07 14:50 ` Srinivas Kandagatla
2021-06-07 15:04 ` Srinivasa Rao Mandadapu
0 siblings, 1 reply; 3+ messages in thread
From: Srinivas Kandagatla @ 2021-06-07 14:50 UTC (permalink / raw)
To: Srinivasa Rao Mandadapu, agross, bjorn.andersson, lgirdwood,
broonie, robh+dt, plai, bgoswami, perex, tiwai, rohitkr,
linux-arm-msm, alsa-devel, devicetree, linux-kernel, swboyd,
judyhsiao
On 05/06/2021 12:38, Srinivasa Rao Mandadapu wrote:
> The DMA interrupt clear register overwritten during
> simultaneous playback and capture in lpass platform
> interrupt handler. It's causing playback or capture stuck
> in similtaneous plaback on speaker and capture on dmic test.
> Update appropriate reg fields of corresponding channel instead
> of entire register write.
>
> Fixes: commit c5c8635a04711 ("ASoC: qcom: Add LPASS platform driver")
>
> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
> ---
> sound/soc/qcom/lpass-platform.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
> index 0df9481ea4c6..f220a2739ac3 100644
> --- a/sound/soc/qcom/lpass-platform.c
> +++ b/sound/soc/qcom/lpass-platform.c
> @@ -526,7 +526,7 @@ static int lpass_platform_pcmops_trigger(struct snd_soc_component *component,
> return -EINVAL;
> }
>
> - ret = regmap_write(map, reg_irqclr, val_irqclr);
> + ret = regmap_update_bits(map, reg_irqclr, val_irqclr, val_irqclr);
> if (ret) {
> dev_err(soc_runtime->dev, "error writing to irqclear reg: %d\n", ret);
> return ret;
> @@ -650,7 +650,7 @@ static irqreturn_t lpass_dma_interrupt_handler(
> struct lpass_variant *v = drvdata->variant;
> irqreturn_t ret = IRQ_NONE;
> int rv;
> - unsigned int reg = 0, val = 0;
> + unsigned int reg, val, val_clr, val_mask;
minor nit here, variable name val_clr is pretty confusing to readers, It
might be okay for irq clr register but we are using the same name of
writing to other registers. So can I suggest you to reuse val variable.
other thing is val_mask, please rename this to mask and just set it in
the start of function so you can avoid 3 extra lines below.
Other than that patch looks good to me!
--srini
> struct regmap *map;
> unsigned int dai_id = cpu_dai->driver->id;
>
> @@ -676,8 +676,9 @@ static irqreturn_t lpass_dma_interrupt_handler(
> return -EINVAL;
> }
> if (interrupts & LPAIF_IRQ_PER(chan)) {
> -
> - rv = regmap_write(map, reg, LPAIF_IRQ_PER(chan) | val);
> + val_clr = LPAIF_IRQ_PER(chan) | val;
> + val_mask = LPAIF_IRQ_ALL(chan);
> + rv = regmap_update_bits(map, reg, val_mask, val_clr);
> if (rv) {
> dev_err(soc_runtime->dev,
> "error writing to irqclear reg: %d\n", rv);
> @@ -688,7 +689,9 @@ static irqreturn_t lpass_dma_interrupt_handler(
> }
>
> if (interrupts & LPAIF_IRQ_XRUN(chan)) {
> - rv = regmap_write(map, reg, LPAIF_IRQ_XRUN(chan) | val);
> + val_clr = (LPAIF_IRQ_XRUN(chan) | val);
> + val_mask = LPAIF_IRQ_ALL(chan);
> + rv = regmap_update_bits(map, reg, val_mask, val_clr);
> if (rv) {
> dev_err(soc_runtime->dev,
> "error writing to irqclear reg: %d\n", rv);
> @@ -700,7 +703,9 @@ static irqreturn_t lpass_dma_interrupt_handler(
> }
>
> if (interrupts & LPAIF_IRQ_ERR(chan)) {
> - rv = regmap_write(map, reg, LPAIF_IRQ_ERR(chan) | val);
> + val_clr = (LPAIF_IRQ_ERR(chan) | val);
> + val_mask = LPAIF_IRQ_ALL(chan);
> + rv = regmap_update_bits(map, reg, val_mask, val_clr);
> if (rv) {
> dev_err(soc_runtime->dev,
> "error writing to irqclear reg: %d\n", rv);
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v2] ASoC: qcom: Fix for DMA interrupt clear reg overwriting
2021-06-07 14:50 ` Srinivas Kandagatla
@ 2021-06-07 15:04 ` Srinivasa Rao Mandadapu
0 siblings, 0 replies; 3+ messages in thread
From: Srinivasa Rao Mandadapu @ 2021-06-07 15:04 UTC (permalink / raw)
To: Srinivas Kandagatla, agross, bjorn.andersson, lgirdwood, broonie,
robh+dt, plai, bgoswami, perex, tiwai, rohitkr, linux-arm-msm,
alsa-devel, devicetree, linux-kernel, swboyd, judyhsiao
Hi Srini,
Thanks for your review comments!!!
On 6/7/2021 8:20 PM, Srinivas Kandagatla wrote:
>
>
> On 05/06/2021 12:38, Srinivasa Rao Mandadapu wrote:
>> The DMA interrupt clear register overwritten during
>> simultaneous playback and capture in lpass platform
>> interrupt handler. It's causing playback or capture stuck
>> in similtaneous plaback on speaker and capture on dmic test.
>> Update appropriate reg fields of corresponding channel instead
>> of entire register write.
>>
>> Fixes: commit c5c8635a04711 ("ASoC: qcom: Add LPASS platform driver")
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
>> ---
>> sound/soc/qcom/lpass-platform.c | 17 +++++++++++------
>> 1 file changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/sound/soc/qcom/lpass-platform.c
>> b/sound/soc/qcom/lpass-platform.c
>> index 0df9481ea4c6..f220a2739ac3 100644
>> --- a/sound/soc/qcom/lpass-platform.c
>> +++ b/sound/soc/qcom/lpass-platform.c
>> @@ -526,7 +526,7 @@ static int lpass_platform_pcmops_trigger(struct
>> snd_soc_component *component,
>> return -EINVAL;
>> }
>> - ret = regmap_write(map, reg_irqclr, val_irqclr);
>> + ret = regmap_update_bits(map, reg_irqclr, val_irqclr,
>> val_irqclr);
>> if (ret) {
>> dev_err(soc_runtime->dev, "error writing to irqclear
>> reg: %d\n", ret);
>> return ret;
>> @@ -650,7 +650,7 @@ static irqreturn_t lpass_dma_interrupt_handler(
>> struct lpass_variant *v = drvdata->variant;
>> irqreturn_t ret = IRQ_NONE;
>> int rv;
>> - unsigned int reg = 0, val = 0;
>> + unsigned int reg, val, val_clr, val_mask;
>
> minor nit here, variable name val_clr is pretty confusing to readers,
> It might be okay for irq clr register but we are using the same name
> of writing to other registers. So can I suggest you to reuse val
> variable.
>
> other thing is val_mask, please rename this to mask and just set it in
> the start of function so you can avoid 3 extra lines below.
Ok will do accordingly and repost patch.
>
> Other than that patch looks good to me!
>
> --srini
>> struct regmap *map;
>> unsigned int dai_id = cpu_dai->driver->id;
>> @@ -676,8 +676,9 @@ static irqreturn_t lpass_dma_interrupt_handler(
>> return -EINVAL;
>> }
>> if (interrupts & LPAIF_IRQ_PER(chan)) {
>> -
>> - rv = regmap_write(map, reg, LPAIF_IRQ_PER(chan) | val);
>> + val_clr = LPAIF_IRQ_PER(chan) | val;
>> + val_mask = LPAIF_IRQ_ALL(chan);
>> + rv = regmap_update_bits(map, reg, val_mask, val_clr);
>> if (rv) {
>> dev_err(soc_runtime->dev,
>> "error writing to irqclear reg: %d\n", rv);
>> @@ -688,7 +689,9 @@ static irqreturn_t lpass_dma_interrupt_handler(
>> }
>> if (interrupts & LPAIF_IRQ_XRUN(chan)) {
>> - rv = regmap_write(map, reg, LPAIF_IRQ_XRUN(chan) | val);
>> + val_clr = (LPAIF_IRQ_XRUN(chan) | val);
>> + val_mask = LPAIF_IRQ_ALL(chan);
>> + rv = regmap_update_bits(map, reg, val_mask, val_clr);
>> if (rv) {
>> dev_err(soc_runtime->dev,
>> "error writing to irqclear reg: %d\n", rv);
>> @@ -700,7 +703,9 @@ static irqreturn_t lpass_dma_interrupt_handler(
>> }
>> if (interrupts & LPAIF_IRQ_ERR(chan)) {
>> - rv = regmap_write(map, reg, LPAIF_IRQ_ERR(chan) | val);
>> + val_clr = (LPAIF_IRQ_ERR(chan) | val);
>> + val_mask = LPAIF_IRQ_ALL(chan);
>> + rv = regmap_update_bits(map, reg, val_mask, val_clr);
>> if (rv) {
>> dev_err(soc_runtime->dev,
>> "error writing to irqclear reg: %d\n", rv);
>>
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-06-07 15:05 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-05 11:38 [PATCH v2] ASoC: qcom: Fix for DMA interrupt clear reg overwriting Srinivasa Rao Mandadapu
2021-06-07 14:50 ` Srinivas Kandagatla
2021-06-07 15:04 ` Srinivasa Rao Mandadapu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).