Messages in this thread | | | Subject | Re: [PATCH v3] ASoC: qcom: Fix for DMA interrupt clear reg overwriting | From | Srinivas Kandagatla <> | Date | Fri, 18 Jun 2021 12:41:09 +0100 |
| |
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 >
| |