From 208b88e72aa8cf33279dc11f8bf45eefe1707ea4 Mon Sep 17 00:00:00 2001 From: Laurentiu Mihalcea Date: Fri, 22 Nov 2024 12:48:34 +0200 Subject: [PATCH] dma: dma_nxp_edma: perform IRQ enable/disable on channel request/release Commit 48b98a9284c3 ("drivers: dma: dma_nxp_edma: disable IRQs when not needed") moved the IRQ enable operation to edma_start() and added an IRQ disable operation in edma_stop(). This is wrong because it breaks the DMA API contract w.r.t dma_start() being `isr-ok` on imx8qm/imx8qxp. As such, move the IRQ enable and disable operations in dma_request_channel() and dma_release_channel(). Note1: managing the interrupts like this is only really needed when dealing with interrupt controllers that have a power domain associated with it (which is the case for irqstr on imx8qm/imx8qxp). Note2: Zephyr has no reference count for shared interrupts so disabling a shared interrupt without checking if someone else is using it is dangerous. Based on the aforementioned notes, the irq_disable() operation is only performed if irqstr is used as an interrupt controller (which is only the case for imx8qm/imx8qxp). Otherwise, the operation isn't needed. Fixes #80573. Signed-off-by: Laurentiu Mihalcea --- drivers/dma/dma_nxp_edma.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/drivers/dma/dma_nxp_edma.c b/drivers/dma/dma_nxp_edma.c index 449720edabc808a..673e7ef6a7049e8 100644 --- a/drivers/dma/dma_nxp_edma.c +++ b/drivers/dma/dma_nxp_edma.c @@ -462,11 +462,8 @@ static int edma_stop(const struct device *dev, uint32_t chan_id) /* disable HW requests */ EDMA_ChannelRegUpdate(data->hal_cfg, chan_id, EDMA_TCD_CH_CSR, 0, EDMA_TCD_CH_CSR_ERQ_MASK); - out_release_channel: - irq_disable(chan->irq); - /* clear the channel MUX so that it can used by a different peripheral. * * note: because the channel is released during dma_stop() that means @@ -515,8 +512,6 @@ static int edma_start(const struct device *dev, uint32_t chan_id) LOG_DBG("starting channel %u", chan_id); - irq_enable(chan->irq); - /* enable HW requests */ EDMA_ChannelRegUpdate(data->hal_cfg, chan_id, EDMA_TCD_CH_CSR, EDMA_TCD_CH_CSR_ERQ_MASK, 0); @@ -583,19 +578,36 @@ static int edma_get_attribute(const struct device *dev, uint32_t type, uint32_t static bool edma_channel_filter(const struct device *dev, int chan_id, void *param) { - int *requested_channel; + struct edma_channel *chan; if (!param) { return false; } - requested_channel = param; + if (*(int *)param != chan_id) { + return false; + } - if (*requested_channel == chan_id && lookup_channel(dev, chan_id)) { - return true; + chan = lookup_channel(dev, chan_id); + if (!chan) { + return false; } - return false; + irq_enable(chan->irq); + + return true; +} + +static void edma_channel_release(const struct device *dev, uint32_t chan_id) +{ + struct edma_channel *chan = lookup_channel(dev, chan_id); + if (!chan) { + return; + } + +#ifdef CONFIG_NXP_IRQSTEER + irq_disable(chan->irq); +#endif /* CONFIG_NXP_IRQSTEER */ } static const struct dma_driver_api edma_api = { @@ -608,6 +620,7 @@ static const struct dma_driver_api edma_api = { .get_status = edma_get_status, .get_attribute = edma_get_attribute, .chan_filter = edma_channel_filter, + .chan_release = edma_channel_release, }; static edma_config_t *edma_hal_cfg_get(const struct edma_config *cfg)