ASoC: qcom: Fix for DMA interrupt clear reg overwriting
Subsystems (4)
PATCH v2 1/1 ASoC: qcom: Fix for DMA interrupt clear reg overwriting
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.
--- sound/soc/qcom/lpass-platform.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)
sound/soc/qcom/lpass-platform.c +11 -6
@@ -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);
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
Quoted 28 lines · 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 <[email protected]>--- sound/soc/qcom/lpass-platform.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)sound/soc/qcom/lpass-platform.c +1
-1@@ -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;
sound/soc/qcom/lpass-platform.c static irqreturn_t lpass_dma_interrupt_handler( · line 653
Quoted 5 lines
sound/soc/qcom/lpass-platform.c +1
-1@@ -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
Quoted 38 lines
sound/soc/qcom/lpass-platform.c +9
-4struct 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);
Hi Srini,
Thanks for your review comments!!!
Quoted 40 lines · Srinivas Kandagatla
Quoted 29 lines · 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 <[email protected]>---
sound/soc/qcom/lpass-platform.c| 17 +++++++++++------1 file changed, 11 insertions(+), 6 deletions(-)sound/soc/qcom/lpass-platform.c +1
-0@@ -526,7 +526,7 @@ static int lpass_platform_pcmops_trigger(structsnd_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;sound/soc/qcom/lpass-platform.c static irqreturn_t lpass_dma_interrupt_handler( · line 653
Quoted 5 lines
sound/soc/qcom/lpass-platform.c +1
-1@@ -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_clris 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.
Quoted 1 line
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.
Quoted 39 lines
Other than that patch looks good to me!
--srini
Quoted 35 lines
sound/soc/qcom/lpass-platform.c +0
-0struct regmap *map; unsigned int dai_id = cpu_dai->driver->id;@@ -676,8 +676,9 @@ static
irqreturn_tlpass_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);sound/soc/qcom/lpass-platform.c +6
-2@@ -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);
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.