Hi Marek,

Thanks for testing next and reporting this back.

On 17/06/2021 20:35, Marek Szyprowski wrote:

Hi,

On 09.06.2021 09:23, 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

This patch landed recently in linux-next as commit da0363f7bfd3 ("ASoC: qcom: Fix for DMA interrupt clear reg overwriting"). It breaks ALSA playback on DragonBoard 410c (arch/arm64/boot/dts/qcom/apq8016-sbc.dts). After applying this patch, running 'speaker-test -l1' never finishes. There is no error nor kernel warning message. Before that commit, the playback worked fine on that board.

TBH, I should have looked at the hw register description that is being updated in the patch. This is a software write only and hardware readable register for which update_bits really does not make sense at all. While digging out I found an issue with regmap_cache reads which should have reported an error while attempting to even do this. I sent out a patch to fix this.

Now for this patch itself, We should send a patch to revert it.

thanks, srini

...

---

Changes since v2: -- Removed redundant variables. Changes since v1: -- Subject lines changed. sound/soc/qcom/lpass-platform.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c index 0df9481ea4c6..f9df76d37858 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,10 +650,11 @@ 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, mask; struct regmap *map; unsigned int dai_id = cpu_dai->driver->id;

• mask = LPAIF_IRQ_ALL(chan); switch (dai_id) { case LPASS_DP_RX: map = drvdata->hdmiif_map;

@@ -676,8 +677,7 @@ 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);
  

• rv = regmap_update_bits(map, reg, mask, (LPAIF_IRQ_PER(chan) | val));
  if (rv) {
  	dev_err(soc_runtime->dev,
  		"error writing to irqclear reg: %d\n", rv);
  

@@ -688,7 +688,7 @@ static irqreturn_t lpass_dma_interrupt_handler( }

if (interrupts & LPAIF_IRQ_XRUN(chan)) {

• rv = regmap_write(map, reg, LPAIF_IRQ_XRUN(chan) | val);
  

• rv = regmap_update_bits(map, reg, mask, (LPAIF_IRQ_XRUN(chan) | val));
  if (rv) {
  	dev_err(soc_runtime->dev,
  		"error writing to irqclear reg: %d\n", rv);
  

@@ -700,7 +700,7 @@ static irqreturn_t lpass_dma_interrupt_handler( }

if (interrupts & LPAIF_IRQ_ERR(chan)) {

• rv = regmap_write(map, reg, LPAIF_IRQ_ERR(chan) | val);
  

• rv = regmap_update_bits(map, reg, mask, (LPAIF_IRQ_ERR(chan) | val));
  if (rv) {
  	dev_err(soc_runtime->dev,
  		"error writing to irqclear reg: %d\n", rv);
  

Best regards

Hi,

On 21.06.2021 14:52, Srinivas Kandagatla wrote:

On 17/06/2021 20:35, Marek Szyprowski wrote:

...

On 09.06.2021 09:23, 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

Can you please try this patch and let us know if this fixes the issue

------------------------->cut<------------------------------- Author: Srinivas Kandagatla srinivas.kandagatla@linaro.org Date:   Mon Jun 21 12:38:43 2021 +0100

ASoC: qcom: lpass-cpu: mark IRQ_CLEAR register as volatile and readable

Currently IRQ_CLEAR register is marked as write-only, however using     regmap_update_bits on this register will have some side effects.     so mark IRQ_CLEAR register appropriately as readable and volatile.

Signed-off-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org

This fixes the issue observed on DragonBoard410c. Feel free to add:

Reported-by: Marek Szyprowski m.szyprowski@samsung.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com

diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c index 0b9cbf2ce505..8998697cd1e1 100644 --- a/sound/soc/qcom/lpass-cpu.c +++ b/sound/soc/qcom/lpass-cpu.c @@ -525,6 +525,8 @@ static bool lpass_cpu_regmap_readable(struct device *dev, unsigned int reg)                         return true;

for (i = 0; i < v->irq_ports; ++i) { +               if (reg == LPAIF_IRQCLEAR_REG(v, i)) +                       return true;                 if (reg == LPAIF_IRQEN_REG(v, i))                         return true;                 if (reg == LPAIF_IRQSTAT_REG(v, i)) @@ -566,9 +568,12 @@ static bool lpass_cpu_regmap_volatile(struct device *dev, unsigned int reg)         struct lpass_variant *v = drvdata->variant;         int i;

-       for (i = 0; i < v->irq_ports; ++i) +       for (i = 0; i < v->irq_ports; ++i) { +               if (reg == LPAIF_IRQCLEAR_REG(v, i)) +                       return true;                 if (reg == LPAIF_IRQSTAT_REG(v, i))                         return true; +       }

for (i = 0; i < v->rdma_channels; ++i)                 if (reg == LPAIF_RDMACURR_REG(v, i))

------------------------->cut<-------------------------------

--srini

...

This patch landed recently in linux-next as commit da0363f7bfd3 ("ASoC: qcom: Fix for DMA interrupt clear reg overwriting"). It breaks ALSA playback on DragonBoard 410c (arch/arm64/boot/dts/qcom/apq8016-sbc.dts). After applying this patch, running 'speaker-test -l1' never finishes. There is no error nor kernel warning message. Before that commit, the playback worked fine on that board.

...

---

Changes since v2:     -- Removed redundant variables. Changes since v1:     -- Subject lines changed.    sound/soc/qcom/lpass-platform.c | 12 ++++++------    1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c index 0df9481ea4c6..f9df76d37858 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,10 +650,11 @@ 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, mask;        struct regmap *map;        unsigned int dai_id = cpu_dai->driver->id;    +    mask = LPAIF_IRQ_ALL(chan);        switch (dai_id) {        case LPASS_DP_RX:            map = drvdata->hdmiif_map; @@ -676,8 +677,7 @@ 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); +        rv = regmap_update_bits(map, reg, mask, (LPAIF_IRQ_PER(chan) | val));            if (rv) {                dev_err(soc_runtime->dev,                    "error writing to irqclear reg: %d\n", rv); @@ -688,7 +688,7 @@ static irqreturn_t lpass_dma_interrupt_handler(        }           if (interrupts & LPAIF_IRQ_XRUN(chan)) { -        rv = regmap_write(map, reg, LPAIF_IRQ_XRUN(chan) | val); +        rv = regmap_update_bits(map, reg, mask, (LPAIF_IRQ_XRUN(chan) | val));            if (rv) {                dev_err(soc_runtime->dev,                    "error writing to irqclear reg: %d\n", rv); @@ -700,7 +700,7 @@ static irqreturn_t lpass_dma_interrupt_handler(        }           if (interrupts & LPAIF_IRQ_ERR(chan)) { -        rv = regmap_write(map, reg, LPAIF_IRQ_ERR(chan) | val); +        rv = regmap_update_bits(map, reg, mask, (LPAIF_IRQ_ERR(chan) | val));            if (rv) {                dev_err(soc_runtime->dev,                    "error writing to irqclear reg: %d\n", rv);

Best regards