Re: [PATCH v8 08/10] ASoC: qcom: Add QAIF IRQ handling and platform register

From: Ajay Kumar Nandam

Date: Tue Sep 29 2026 - 07:37:06 EST

+ case SNDRV_PCM_TRIGGER_STOP:
+ case SNDRV_PCM_TRIGGER_SUSPEND:
+ case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+ ret = regmap_update_bits(map,
+ qaif_dmactl_reg(v, idx, substream->stream, dai_id),
+ QAIF_DMACTL_ENABLE_BIT, 0);
+ if (ret) {
+ dev_err(soc_runtime->dev, "error disabling DMA: %d\n", ret);
+ return ret;
+ }
+ ret = regmap_update_bits(map,
+ qaif_dmacfg_reg(v, idx, substream->stream, dai_id),
+ QAIF_DMACFG_DYNCLK_BIT, 0);
+ if (ret)
+ dev_err(soc_runtime->dev, "error disabling dma_dynclk: %d\n", ret);

Should this preserve the first error while still attempting the IRQ-disable cleanup? As written, if clearing `QAIF_DMACFG_DYNCLK_BIT` fails but `qaif_platform_irq_op(..., QAIF_IRQ_DISABLE)` succeeds, `ret` is overwritten and the trigger callback returns 0.

+ ret = qaif_platform_irq_op(drvdata, substream->stream, irq_type,
+ idx, QAIF_IRQ_DISABLE);
+ if (ret)
+ dev_err(soc_runtime->dev, "error disabling irq regs: %d\n", ret);
+ break;

The QAIF programming guide's STOP sequences describe one more hardware completion step after clearing DMA ENABLE: wait for the relevant DMA shutdown status to complete, then clear/disable the associated IRQs. This applies to the AIF RDDMA/WRDMA paths and also to the codec RDDMA/WRDMA paths.

This patch already has register definitions for the shutdown-status registers
(`QAIF_RDDMA_SHUTDOWN_STAT_REG()`, `QAIF_WRDMA_SHUTDOWN_STAT_REG()`,
QAIF_CDC_RDDMA_SHUTDOWN_STAT_REG(),&QAIF_CDC_WRDMA_SHUTDOWN_STAT_REG()), but I do not see them used in this stop path or in `sync_stop()`.

Should the driver poll the appropriate `*_SHUTDOWN_STATUS` register, for
example with `regmap_read_poll_timeout()`, before disabling DYNCLK and IRQs? Otherwise a fast STOP->START or suspend/resume path may race with a DMA channel that hardware still reports as shutting down.

Thanks
Ajay Kumar Nandam

+ default:
+ return -EINVAL;
+ }
+ return ret;
+}
+
+static int qaif_platform_pcmops_sync_stop(struct snd_soc_component *component,
+ struct snd_pcm_substream *substream)
+{
+ struct snd_soc_pcm_runtime *soc_runtime = snd_soc_substream_to_rtd(substream);
+ struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(soc_runtime, 0);
+ struct qaif_drv_data *drvdata = snd_soc_component_get_drvdata(component);
+ struct qaif_pcm_data *data = substream->runtime->private_data;
+ unsigned int dai_id = cpu_dai->driver->id;
+ enum qaif_irq_type irq_type = qaif_is_cif_port(dai_id) ? QAIF_CIF_IRQ : QAIF_AIF_IRQ;
+ int idx = data->dma_reg_idx;
+ int ret;
+
+ synchronize_irq(drvdata->audio_qaif_irq);
+ ret = qaif_platform_irq_op(drvdata, substream->stream, irq_type,
+ idx, QAIF_IRQ_CLEAR);
+ if (ret)
+ dev_err(soc_runtime->dev, "error clearing irq regs: %d\n", ret);
+ return ret;
+}
+
+static irqreturn_t qaif_process_dma_irq(struct qaif_drv_data *drvdata,
+ u32 stat_reg_addr,
+ u32 clr_reg_addr,
+ enum qaif_irq_type irq_type,
+ enum qaif_dma_dir dma_type,
+ enum qaif_irq irq,
+ struct snd_pcm_substream **substream)
+{
+ const struct qaif_variant *v = drvdata->variant;
+ unsigned int reg = 0;
+ int dma_idx, stream_dma_idx, rv, num_dma, stream_offset, array_size;
+ u32 mask;
+ struct snd_pcm_substream *ss;
+
+ stream_offset = (dma_type == QAIF_DMA_WRDMA) ?
+ ((irq_type == QAIF_AIF_IRQ) ? v->wrdma_start : v->codec_wrdma_start) : 0;
+ num_dma = (dma_type == QAIF_DMA_WRDMA) ?
+ ((irq_type == QAIF_AIF_IRQ) ? v->num_wrdma : v->num_codec_wrdma) :
+ ((irq_type == QAIF_AIF_IRQ) ? v->num_rddma : v->num_codec_rddma);
+ if (!num_dma)
+ return IRQ_NONE;
+
+ array_size = (irq_type == QAIF_AIF_IRQ) ?
+ QAIF_MAX_AIF_DMA_IDX : QAIF_MAX_CIF_DMA_IDX;
+ if (stream_offset + num_dma > array_size) {
+ dev_err(regmap_get_device(drvdata->audio_qaif_map),
+ "DMA index range %d+%d exceeds substream table %d\n",
+ stream_offset, num_dma, array_size);
+ return IRQ_NONE;
+ }
+ mask = GENMASK(num_dma - 1, 0);
+
+ rv = regmap_read(drvdata->audio_qaif_map, stat_reg_addr, &reg);
+ if (rv) {
+ dev_err_ratelimited(regmap_get_device(drvdata->audio_qaif_map),
+ "error reading stat reg 0x%x: %d\n", stat_reg_addr, rv);
+ return IRQ_NONE;
+ }
+
+ if (!(reg & mask)) {
+ dev_warn_ratelimited(regmap_get_device(drvdata->audio_qaif_map),
+ "spurious IRQ: stat reg 0x%x status 0x%x mask 0x%x\n",
+ stat_reg_addr, reg, mask);
+ return IRQ_NONE;
+ }
+
+ if (regmap_write(drvdata->audio_qaif_map, clr_reg_addr, reg & mask))
+ return IRQ_NONE;
+
+ for (dma_idx = 0; dma_idx < num_dma; dma_idx++) {
+ stream_dma_idx = dma_idx + stream_offset;
+ ss = READ_ONCE(substream[stream_dma_idx]);
+ if (!(reg & BIT(dma_idx)) || !ss)
+ continue;
+ switch (irq) {
+ case QAIF_IRQ_PERIOD:
+ snd_pcm_period_elapsed(ss);
+ break;
+ case QAIF_IRQ_OVERFLOW:
+ case QAIF_IRQ_UNDERFLOW:
+ dev_warn_ratelimited(regmap_get_device(drvdata->audio_qaif_map),
+ "QAIF DMA xRun\n");
+ snd_pcm_stop_xrun(ss);
+ break;
+ case QAIF_IRQ_ERROR:
+ snd_pcm_stop_xrun(ss);
+ dev_err_ratelimited(regmap_get_device(drvdata->audio_qaif_map),
+ "QAIF bus error\n");
+ break;
+ }
+ }
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t qaif_aif_irq_handler(struct qaif_drv_data *drvdata, u32 status)
+{
+ const struct qaif_variant *v = drvdata->variant;
+ struct snd_pcm_substream **ss = drvdata->aif_substream;
+ irqreturn_t ret = IRQ_NONE;
+
+ if (status & QAIF_SUMMARY_BITMASK_AIF_PERIOD_RDDMA)
+ ret |= qaif_process_dma_irq(drvdata,
+ QAIF_EE_RDDMA_PERIOD_IRQ_STAT_REG(v, QAIF_AIF_IRQ),
+ QAIF_EE_RDDMA_PERIOD_IRQ_CLR_REG(v, QAIF_AIF_IRQ),
+ QAIF_AIF_IRQ, QAIF_DMA_RDDMA, QAIF_IRQ_PERIOD, ss);
+ if (status & QAIF_SUMMARY_BITMASK_AIF_PERIOD_WRDMA)
+ ret |= qaif_process_dma_irq(drvdata,
+ QAIF_EE_WRDMA_PERIOD_IRQ_STAT_REG(v, QAIF_AIF_IRQ),
+ QAIF_EE_WRDMA_PERIOD_IRQ_CLR_REG(v, QAIF_AIF_IRQ),
+ QAIF_AIF_IRQ, QAIF_DMA_WRDMA, QAIF_IRQ_PERIOD, ss);
+ if (status & QAIF_SUMMARY_BITMASK_AIF_OVERFLOW_WRDMA)
+ ret |= qaif_process_dma_irq(drvdata,
+ QAIF_EE_WRDMA_OF_IRQ_STAT_REG(v, QAIF_AIF_IRQ),
+ QAIF_EE_WRDMA_OF_IRQ_CLR_REG(v, QAIF_AIF_IRQ),
+ QAIF_AIF_IRQ, QAIF_DMA_WRDMA, QAIF_IRQ_OVERFLOW, ss);
+ if (status & QAIF_SUMMARY_BITMASK_AIF_UNDERFLOW_RDDMA)
+ ret |= qaif_process_dma_irq(drvdata,
+ QAIF_EE_RDDMA_UF_IRQ_STAT_REG(v, QAIF_AIF_IRQ),
+ QAIF_EE_RDDMA_UF_IRQ_CLR_REG(v, QAIF_AIF_IRQ),
+ QAIF_AIF_IRQ, QAIF_DMA_RDDMA, QAIF_IRQ_UNDERFLOW, ss);
+ if (status & QAIF_SUMMARY_BITMASK_AIF_ERR_RSP_RDDMA)
+ ret |= qaif_process_dma_irq(drvdata,
+ QAIF_EE_RDDMA_ERR_RSP_IRQ_STAT_REG(v, QAIF_AIF_IRQ),
+ QAIF_EE_RDDMA_ERR_RSP_IRQ_CLR_REG(v, QAIF_AIF_IRQ),
+ QAIF_AIF_IRQ, QAIF_DMA_RDDMA, QAIF_IRQ_ERROR, ss);
+ if (status & QAIF_SUMMARY_BITMASK_AIF_ERR_RSP_WRDMA)
+ ret |= qaif_process_dma_irq(drvdata,
+ QAIF_EE_WRDMA_ERR_RSP_IRQ_STAT_REG(v, QAIF_AIF_IRQ),
+ QAIF_EE_WRDMA_ERR_RSP_IRQ_CLR_REG(v, QAIF_AIF_IRQ),
+ QAIF_AIF_IRQ, QAIF_DMA_WRDMA, QAIF_IRQ_ERROR, ss);
+ return ret;
+}
+
+static irqreturn_t qaif_cif_irq_handler(struct qaif_drv_data *drvdata, u32 status)
+{
+ const struct qaif_variant *v = drvdata->variant;
+ struct snd_pcm_substream **ss = drvdata->cif_substream;
+ irqreturn_t ret = IRQ_NONE;
+
+ if (status & QAIF_SUMMARY_BITMASK_CIF_PERIOD_RDDMA)
+ ret |= qaif_process_dma_irq(drvdata,
+ QAIF_EE_RDDMA_PERIOD_IRQ_STAT_REG(v, QAIF_CIF_IRQ),
+ QAIF_EE_RDDMA_PERIOD_IRQ_CLR_REG(v, QAIF_CIF_IRQ),
+ QAIF_CIF_IRQ, QAIF_DMA_RDDMA, QAIF_IRQ_PERIOD, ss);
+ if (status & QAIF_SUMMARY_BITMASK_CIF_PERIOD_WRDMA)
+ ret |= qaif_process_dma_irq(drvdata,
+ QAIF_EE_WRDMA_PERIOD_IRQ_STAT_REG(v, QAIF_CIF_IRQ),
+ QAIF_EE_WRDMA_PERIOD_IRQ_CLR_REG(v, QAIF_CIF_IRQ),
+ QAIF_CIF_IRQ, QAIF_DMA_WRDMA, QAIF_IRQ_PERIOD, ss);
+ if (status & QAIF_SUMMARY_BITMASK_CIF_OVERFLOW_WRDMA)
+ ret |= qaif_process_dma_irq(drvdata,
+ QAIF_EE_WRDMA_OF_IRQ_STAT_REG(v, QAIF_CIF_IRQ),
+ QAIF_EE_WRDMA_OF_IRQ_CLR_REG(v, QAIF_CIF_IRQ),
+ QAIF_CIF_IRQ, QAIF_DMA_WRDMA, QAIF_IRQ_OVERFLOW, ss);
+ if (status & QAIF_SUMMARY_BITMASK_CIF_UNDERFLOW_RDDMA)
+ ret |= qaif_process_dma_irq(drvdata,
+ QAIF_EE_RDDMA_UF_IRQ_STAT_REG(v, QAIF_CIF_IRQ),
+ QAIF_EE_RDDMA_UF_IRQ_CLR_REG(v, QAIF_CIF_IRQ),
+ QAIF_CIF_IRQ, QAIF_DMA_RDDMA, QAIF_IRQ_UNDERFLOW, ss);
+ if (status & QAIF_SUMMARY_BITMASK_CIF_ERR_RSP_RDDMA)
+ ret |= qaif_process_dma_irq(drvdata,
+ QAIF_EE_RDDMA_ERR_RSP_IRQ_STAT_REG(v, QAIF_CIF_IRQ),
+ QAIF_EE_RDDMA_ERR_RSP_IRQ_CLR_REG(v, QAIF_CIF_IRQ),
+ QAIF_CIF_IRQ, QAIF_DMA_RDDMA, QAIF_IRQ_ERROR, ss);
+ if (status & QAIF_SUMMARY_BITMASK_CIF_ERR_RSP_WRDMA)
+ ret |= qaif_process_dma_irq(drvdata,
+ QAIF_EE_WRDMA_ERR_RSP_IRQ_STAT_REG(v, QAIF_CIF_IRQ),
+ QAIF_EE_WRDMA_ERR_RSP_IRQ_CLR_REG(v, QAIF_CIF_IRQ),
+ QAIF_CIF_IRQ, QAIF_DMA_WRDMA, QAIF_IRQ_ERROR, ss);
+ return ret;
+}
+
+static const struct qaif_irq_map qaif_irq_clients[] = {
+ { QAIF_CLIENT_ID_AIF_DMA, QAIF_BITMASK_AIF_RDDMA_WRDMA, qaif_aif_irq_handler },
+ { QAIF_CLIENT_ID_CIF_DMA, QAIF_BITMASK_CIF_RDDMA_WRDMA, qaif_cif_irq_handler },
+};
+
+static irqreturn_t asoc_platform_qaif_irq(int irq, void *data)
+{
+ struct qaif_drv_data *drvdata = data;
+ const struct qaif_variant *v = drvdata->variant;
+ u32 summary_irq_status;
+ int rv, client;
+ irqreturn_t ret = IRQ_NONE;
+
+ rv = regmap_read(drvdata->audio_qaif_map, QAIF_SUMMARY_IRQSTAT_REG(v),
+ &summary_irq_status);
+ if (rv) {
+ dev_err(regmap_get_device(drvdata->audio_qaif_map),
+ "error reading from irqstat reg: %d\n", rv);
+ return IRQ_NONE;
+ }
+
+ if (!(summary_irq_status & QAIF_ALL_CLIENTS_MASK))
+ return IRQ_NONE;
+
+ for (client = 0; client < ARRAY_SIZE(qaif_irq_clients); client++) {
+ if (summary_irq_status & qaif_irq_clients[client].mask)
+ ret |= qaif_irq_clients[client].client_irq_handler(drvdata,
+ summary_irq_status);
+ }
+ return ret;
+}
+
+static const struct snd_soc_component_driver qaif_component_driver = {
+ .name = DRV_NAME,
+ .pcm_new = qaif_platform_pcm_new,
+ .open = qaif_platform_pcmops_open,
+ .close = qaif_platform_pcmops_close,
+ .hw_params = qaif_platform_pcmops_hw_params,
+ .hw_free = qaif_platform_pcmops_hw_free,
+ .prepare = qaif_platform_pcmops_prepare,
+ .trigger = qaif_platform_pcmops_trigger,
+ .sync_stop = qaif_platform_pcmops_sync_stop,
+ .pointer = qaif_platform_pcmops_pointer,
+ .mmap = qaif_platform_pcmops_mmap,
+};
+
+int asoc_qcom_qaif_platform_register(struct platform_device *pdev)
+{
+ struct qaif_drv_data *drvdata = platform_get_drvdata(pdev);
+ struct of_phandle_args iommu_spec;
+ u32 sid;
+ int ret;
+
+ if (!drvdata->variant->alloc_stream_dma_idx ||
+ !drvdata->variant->free_stream_dma_idx)
+ return dev_err_probe(&pdev->dev, -EINVAL,
+ "Variant missing mandatory callbacks\n");
+
+ ret = of_parse_phandle_with_args(pdev->dev.of_node,
+ "iommus", "#iommu-cells", 0,
+ &iommu_spec);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "failed to parse iommus property\n");
+ sid = iommu_spec.args[0];
+ of_node_put(iommu_spec.np);
+ drvdata->smmu_csid_bits = sid & QAIF_CSID_MASK;
+
+ /*
+ * The DMA base register only holds the low 32 bits of the buffer
+ * address, so the coherent buffer must come from the low 32-bit
+ * address space.
+ */
+ ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "Failed to set 32-bit DMA mask\n");
+
+ drvdata->audio_qaif_irq = platform_get_irq(pdev, 0);
+ if (drvdata->audio_qaif_irq < 0)
+ return dev_err_probe(&pdev->dev, drvdata->audio_qaif_irq,
+ "Failed to get IRQ\n");
+
+ drvdata->qaif_hw_configured = false;
+ mutex_init(&drvdata->stream_lock);
+
+ ret = devm_request_irq(&pdev->dev, drvdata->audio_qaif_irq,
+ asoc_platform_qaif_irq, 0,
+ "qaif-irq-audio-core", drvdata);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret, "irq request failed\n");
+
+ ret = devm_snd_soc_register_component(&pdev->dev, &qaif_component_driver, NULL, 0);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "Failed to register platform component\n");
+ return 0;
+}
+EXPORT_SYMBOL_GPL(asoc_qcom_qaif_platform_register);
+
+MODULE_DESCRIPTION("Qualcomm Audio Interface (QAIF) PCM platform driver");
+MODULE_AUTHOR("Harendra Gautam <harendra.gautam@xxxxxxxxxxxxxxxx>");
+MODULE_LICENSE("GPL");