* [PATCH v4 01/17] dmaengine: sh: rz-dmac: Move interrupt request after everything is set up [not found] <20260411114303.2814115-1-claudiu.beznea.uj@bp.renesas.com> @ 2026-04-11 11:42 ` Claudiu 2026-04-11 12:17 ` Biju Das 2026-04-20 12:33 ` sashiko.dev review (Re: [PATCH v4 01/17] dmaengine: sh: rz-dmac: Move interrupt request after everything is set up) Claudiu Beznea 2026-04-11 11:42 ` [PATCH v4 02/17] dmaengine: sh: rz-dmac: Fix incorrect NULL check on list_first_entry() Claudiu 1 sibling, 2 replies; 5+ messages in thread From: Claudiu @ 2026-04-11 11:42 UTC (permalink / raw) To: vkoul, Frank.Li, lgirdwood, broonie, perex, tiwai, biju.das.jz, prabhakar.mahadev-lad.rj, p.zabel, geert+renesas, fabrizio.castro.jz, long.luu.ur Cc: claudiu.beznea, dmaengine, linux-kernel, linux-sound, linux-renesas-soc, Claudiu Beznea, stable From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Once the interrupt is requested, the interrupt handler may run immediately. Since the IRQ handler can access channel->ch_base, which is initialized only after requesting the IRQ, this may lead to invalid memory access. Likewise, the IRQ thread may access uninitialized data (the ld_free, ld_queue, and ld_active lists), which may also lead to issues. Request the interrupts only after everything is set up. To keep the error path simpler, use dmam_alloc_coherent() instead of dma_alloc_coherent(). Fixes: 5000d37042a6 ("dmaengine: sh: Add DMAC driver for RZ/G2L SoC") Cc: stable@vger.kernel.org Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> --- Changes in v4: - none, this patch is new drivers/dma/sh/rz-dmac.c | 88 +++++++++++++++------------------------- 1 file changed, 33 insertions(+), 55 deletions(-) diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c index 625ff29024de..9f206a33dcc6 100644 --- a/drivers/dma/sh/rz-dmac.c +++ b/drivers/dma/sh/rz-dmac.c @@ -981,25 +981,6 @@ static int rz_dmac_chan_probe(struct rz_dmac *dmac, channel->index = index; channel->mid_rid = -EINVAL; - /* Request the channel interrupt. */ - scnprintf(pdev_irqname, sizeof(pdev_irqname), "ch%u", index); - irq = platform_get_irq_byname(pdev, pdev_irqname); - if (irq < 0) - return irq; - - irqname = devm_kasprintf(dmac->dev, GFP_KERNEL, "%s:%u", - dev_name(dmac->dev), index); - if (!irqname) - return -ENOMEM; - - ret = devm_request_threaded_irq(dmac->dev, irq, rz_dmac_irq_handler, - rz_dmac_irq_handler_thread, 0, - irqname, channel); - if (ret) { - dev_err(dmac->dev, "failed to request IRQ %u (%d)\n", irq, ret); - return ret; - } - /* Set io base address for each channel */ if (index < 8) { channel->ch_base = dmac->base + CHANNEL_0_7_OFFSET + @@ -1012,9 +993,9 @@ static int rz_dmac_chan_probe(struct rz_dmac *dmac, } /* Allocate descriptors */ - lmdesc = dma_alloc_coherent(&pdev->dev, - sizeof(struct rz_lmdesc) * DMAC_NR_LMDESC, - &channel->lmdesc.base_dma, GFP_KERNEL); + lmdesc = dmam_alloc_coherent(&pdev->dev, + sizeof(struct rz_lmdesc) * DMAC_NR_LMDESC, + &channel->lmdesc.base_dma, GFP_KERNEL); if (!lmdesc) { dev_err(&pdev->dev, "Can't allocate memory (lmdesc)\n"); return -ENOMEM; @@ -1030,7 +1011,24 @@ static int rz_dmac_chan_probe(struct rz_dmac *dmac, INIT_LIST_HEAD(&channel->ld_free); INIT_LIST_HEAD(&channel->ld_active); - return 0; + /* Request the channel interrupt. */ + scnprintf(pdev_irqname, sizeof(pdev_irqname), "ch%u", index); + irq = platform_get_irq_byname(pdev, pdev_irqname); + if (irq < 0) + return irq; + + irqname = devm_kasprintf(dmac->dev, GFP_KERNEL, "%s:%u", + dev_name(dmac->dev), index); + if (!irqname) + return -ENOMEM; + + ret = devm_request_threaded_irq(dmac->dev, irq, rz_dmac_irq_handler, + rz_dmac_irq_handler_thread, 0, + irqname, channel); + if (ret) + dev_err(dmac->dev, "failed to request IRQ %u (%d)\n", irq, ret); + + return ret; } static void rz_dmac_put_device(void *_dev) @@ -1099,7 +1097,6 @@ static int rz_dmac_probe(struct platform_device *pdev) const char *irqname = "error"; struct dma_device *engine; struct rz_dmac *dmac; - int channel_num; int ret; int irq; u8 i; @@ -1132,18 +1129,6 @@ static int rz_dmac_probe(struct platform_device *pdev) return PTR_ERR(dmac->ext_base); } - /* Register interrupt handler for error */ - irq = platform_get_irq_byname_optional(pdev, irqname); - if (irq > 0) { - ret = devm_request_irq(&pdev->dev, irq, rz_dmac_irq_handler, 0, - irqname, NULL); - if (ret) { - dev_err(&pdev->dev, "failed to request IRQ %u (%d)\n", - irq, ret); - return ret; - } - } - /* Initialize the channels. */ INIT_LIST_HEAD(&dmac->engine.channels); @@ -1169,6 +1154,18 @@ static int rz_dmac_probe(struct platform_device *pdev) goto err; } + /* Register interrupt handler for error */ + irq = platform_get_irq_byname_optional(pdev, irqname); + if (irq > 0) { + ret = devm_request_irq(&pdev->dev, irq, rz_dmac_irq_handler, 0, + irqname, NULL); + if (ret) { + dev_err(&pdev->dev, "failed to request IRQ %u (%d)\n", + irq, ret); + goto err; + } + } + /* Register the DMAC as a DMA provider for DT. */ ret = of_dma_controller_register(pdev->dev.of_node, rz_dmac_of_xlate, NULL); @@ -1210,16 +1207,6 @@ static int rz_dmac_probe(struct platform_device *pdev) dma_register_err: of_dma_controller_free(pdev->dev.of_node); err: - channel_num = i ? i - 1 : 0; - for (i = 0; i < channel_num; i++) { - struct rz_dmac_chan *channel = &dmac->channels[i]; - - dma_free_coherent(&pdev->dev, - sizeof(struct rz_lmdesc) * DMAC_NR_LMDESC, - channel->lmdesc.base, - channel->lmdesc.base_dma); - } - reset_control_assert(dmac->rstc); err_pm_runtime_put: pm_runtime_put(&pdev->dev); @@ -1232,18 +1219,9 @@ static int rz_dmac_probe(struct platform_device *pdev) static void rz_dmac_remove(struct platform_device *pdev) { struct rz_dmac *dmac = platform_get_drvdata(pdev); - unsigned int i; dma_async_device_unregister(&dmac->engine); of_dma_controller_free(pdev->dev.of_node); - for (i = 0; i < dmac->n_channels; i++) { - struct rz_dmac_chan *channel = &dmac->channels[i]; - - dma_free_coherent(&pdev->dev, - sizeof(struct rz_lmdesc) * DMAC_NR_LMDESC, - channel->lmdesc.base, - channel->lmdesc.base_dma); - } reset_control_assert(dmac->rstc); pm_runtime_put(&pdev->dev); pm_runtime_disable(&pdev->dev); -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [PATCH v4 01/17] dmaengine: sh: rz-dmac: Move interrupt request after everything is set up 2026-04-11 11:42 ` [PATCH v4 01/17] dmaengine: sh: rz-dmac: Move interrupt request after everything is set up Claudiu @ 2026-04-11 12:17 ` Biju Das 2026-04-11 12:34 ` Claudiu Beznea 2026-04-20 12:33 ` sashiko.dev review (Re: [PATCH v4 01/17] dmaengine: sh: rz-dmac: Move interrupt request after everything is set up) Claudiu Beznea 1 sibling, 1 reply; 5+ messages in thread From: Biju Das @ 2026-04-11 12:17 UTC (permalink / raw) To: Claudiu.Beznea, vkoul@kernel.org, Frank.Li@kernel.org, lgirdwood@gmail.com, broonie@kernel.org, perex@perex.cz, tiwai@suse.com, Prabhakar Mahadev Lad, p.zabel@pengutronix.de, geert+renesas@glider.be, Fabrizio Castro, Long Luu Cc: Claudiu.Beznea, dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org, linux-sound@vger.kernel.org, linux-renesas-soc@vger.kernel.org, Claudiu Beznea, stable@vger.kernel.org > -----Original Message----- > From: Claudiu <claudiu.beznea@tuxon.dev> > Sent: 11 April 2026 12:43 > Subject: [PATCH v4 01/17] dmaengine: sh: rz-dmac: Move interrupt request after everything is set up > > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Once the interrupt is requested, the interrupt handler may run immediately. > Since the IRQ handler can access channel->ch_base, which is initialized only after requesting the IRQ, > this may lead to invalid memory access. > Likewise, the IRQ thread may access uninitialized data (the ld_free, ld_queue, and ld_active lists), > which may also lead to issues. > > Request the interrupts only after everything is set up. To keep the error path simpler, use > dmam_alloc_coherent() instead of dma_alloc_coherent(). > > Fixes: 5000d37042a6 ("dmaengine: sh: Add DMAC driver for RZ/G2L SoC") > Cc: stable@vger.kernel.org > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > --- > > Changes in v4: > - none, this patch is new > > drivers/dma/sh/rz-dmac.c | 88 +++++++++++++++------------------------- > 1 file changed, 33 insertions(+), 55 deletions(-) > > diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c index 625ff29024de..9f206a33dcc6 > 100644 > --- a/drivers/dma/sh/rz-dmac.c > +++ b/drivers/dma/sh/rz-dmac.c > @@ -981,25 +981,6 @@ static int rz_dmac_chan_probe(struct rz_dmac *dmac, > channel->index = index; > channel->mid_rid = -EINVAL; > > - /* Request the channel interrupt. */ > - scnprintf(pdev_irqname, sizeof(pdev_irqname), "ch%u", index); > - irq = platform_get_irq_byname(pdev, pdev_irqname); > - if (irq < 0) > - return irq; > - > - irqname = devm_kasprintf(dmac->dev, GFP_KERNEL, "%s:%u", > - dev_name(dmac->dev), index); > - if (!irqname) > - return -ENOMEM; > - > - ret = devm_request_threaded_irq(dmac->dev, irq, rz_dmac_irq_handler, > - rz_dmac_irq_handler_thread, 0, > - irqname, channel); > - if (ret) { > - dev_err(dmac->dev, "failed to request IRQ %u (%d)\n", irq, ret); > - return ret; > - } > - > /* Set io base address for each channel */ > if (index < 8) { > channel->ch_base = dmac->base + CHANNEL_0_7_OFFSET + @@ -1012,9 +993,9 @@ static int > rz_dmac_chan_probe(struct rz_dmac *dmac, > } > > /* Allocate descriptors */ > - lmdesc = dma_alloc_coherent(&pdev->dev, > - sizeof(struct rz_lmdesc) * DMAC_NR_LMDESC, > - &channel->lmdesc.base_dma, GFP_KERNEL); > + lmdesc = dmam_alloc_coherent(&pdev->dev, > + sizeof(struct rz_lmdesc) * DMAC_NR_LMDESC, > + &channel->lmdesc.base_dma, GFP_KERNEL); > if (!lmdesc) { > dev_err(&pdev->dev, "Can't allocate memory (lmdesc)\n"); > return -ENOMEM; > @@ -1030,7 +1011,24 @@ static int rz_dmac_chan_probe(struct rz_dmac *dmac, > INIT_LIST_HEAD(&channel->ld_free); > INIT_LIST_HEAD(&channel->ld_active); > > - return 0; > + /* Request the channel interrupt. */ > + scnprintf(pdev_irqname, sizeof(pdev_irqname), "ch%u", index); > + irq = platform_get_irq_byname(pdev, pdev_irqname); > + if (irq < 0) > + return irq; > + > + irqname = devm_kasprintf(dmac->dev, GFP_KERNEL, "%s:%u", > + dev_name(dmac->dev), index); > + if (!irqname) > + return -ENOMEM; > + > + ret = devm_request_threaded_irq(dmac->dev, irq, rz_dmac_irq_handler, > + rz_dmac_irq_handler_thread, 0, > + irqname, channel); > + if (ret) > + dev_err(dmac->dev, "failed to request IRQ %u (%d)\n", irq, ret); As per [1], it is redundant. [1] https://elixir.bootlin.com/linux/v7.0-rc7/source/kernel/irq/devres.c#L108 > + > + return ret; > } > > static void rz_dmac_put_device(void *_dev) @@ -1099,7 +1097,6 @@ static int rz_dmac_probe(struct > platform_device *pdev) > const char *irqname = "error"; > struct dma_device *engine; > struct rz_dmac *dmac; > - int channel_num; > int ret; > int irq; > u8 i; > @@ -1132,18 +1129,6 @@ static int rz_dmac_probe(struct platform_device *pdev) > return PTR_ERR(dmac->ext_base); > } > > - /* Register interrupt handler for error */ > - irq = platform_get_irq_byname_optional(pdev, irqname); > - if (irq > 0) { > - ret = devm_request_irq(&pdev->dev, irq, rz_dmac_irq_handler, 0, > - irqname, NULL); > - if (ret) { > - dev_err(&pdev->dev, "failed to request IRQ %u (%d)\n", > - irq, ret); > - return ret; > - } > - } > - > /* Initialize the channels. */ > INIT_LIST_HEAD(&dmac->engine.channels); > > @@ -1169,6 +1154,18 @@ static int rz_dmac_probe(struct platform_device *pdev) > goto err; > } > > + /* Register interrupt handler for error */ > + irq = platform_get_irq_byname_optional(pdev, irqname); > + if (irq > 0) { > + ret = devm_request_irq(&pdev->dev, irq, rz_dmac_irq_handler, 0, > + irqname, NULL); > + if (ret) { > + dev_err(&pdev->dev, "failed to request IRQ %u (%d)\n", > + irq, ret); Same case here Cheers, Biju > + goto err; > + } > + } > + > /* Register the DMAC as a DMA provider for DT. */ > ret = of_dma_controller_register(pdev->dev.of_node, rz_dmac_of_xlate, > NULL); > @@ -1210,16 +1207,6 @@ static int rz_dmac_probe(struct platform_device *pdev) > dma_register_err: > of_dma_controller_free(pdev->dev.of_node); > err: > - channel_num = i ? i - 1 : 0; > - for (i = 0; i < channel_num; i++) { > - struct rz_dmac_chan *channel = &dmac->channels[i]; > - > - dma_free_coherent(&pdev->dev, > - sizeof(struct rz_lmdesc) * DMAC_NR_LMDESC, > - channel->lmdesc.base, > - channel->lmdesc.base_dma); > - } > - > reset_control_assert(dmac->rstc); > err_pm_runtime_put: > pm_runtime_put(&pdev->dev); > @@ -1232,18 +1219,9 @@ static int rz_dmac_probe(struct platform_device *pdev) static void > rz_dmac_remove(struct platform_device *pdev) { > struct rz_dmac *dmac = platform_get_drvdata(pdev); > - unsigned int i; > > dma_async_device_unregister(&dmac->engine); > of_dma_controller_free(pdev->dev.of_node); > - for (i = 0; i < dmac->n_channels; i++) { > - struct rz_dmac_chan *channel = &dmac->channels[i]; > - > - dma_free_coherent(&pdev->dev, > - sizeof(struct rz_lmdesc) * DMAC_NR_LMDESC, > - channel->lmdesc.base, > - channel->lmdesc.base_dma); > - } > reset_control_assert(dmac->rstc); > pm_runtime_put(&pdev->dev); > pm_runtime_disable(&pdev->dev); > -- > 2.43.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4 01/17] dmaengine: sh: rz-dmac: Move interrupt request after everything is set up 2026-04-11 12:17 ` Biju Das @ 2026-04-11 12:34 ` Claudiu Beznea 0 siblings, 0 replies; 5+ messages in thread From: Claudiu Beznea @ 2026-04-11 12:34 UTC (permalink / raw) To: Biju Das, vkoul@kernel.org, Frank.Li@kernel.org, lgirdwood@gmail.com, broonie@kernel.org, perex@perex.cz, tiwai@suse.com, Prabhakar Mahadev Lad, p.zabel@pengutronix.de, geert+renesas@glider.be, Fabrizio Castro, Long Luu Cc: dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org, linux-sound@vger.kernel.org, linux-renesas-soc@vger.kernel.org, Claudiu Beznea, stable@vger.kernel.org On 4/11/26 15:17, Biju Das wrote: > >> -----Original Message----- >> From: Claudiu<claudiu.beznea@tuxon.dev> >> Sent: 11 April 2026 12:43 >> Subject: [PATCH v4 01/17] dmaengine: sh: rz-dmac: Move interrupt request after everything is set up >> >> From: Claudiu Beznea<claudiu.beznea.uj@bp.renesas.com> >> >> Once the interrupt is requested, the interrupt handler may run immediately. >> Since the IRQ handler can access channel->ch_base, which is initialized only after requesting the IRQ, >> this may lead to invalid memory access. >> Likewise, the IRQ thread may access uninitialized data (the ld_free, ld_queue, and ld_active lists), >> which may also lead to issues. >> >> Request the interrupts only after everything is set up. To keep the error path simpler, use >> dmam_alloc_coherent() instead of dma_alloc_coherent(). >> >> Fixes: 5000d37042a6 ("dmaengine: sh: Add DMAC driver for RZ/G2L SoC") >> Cc:stable@vger.kernel.org >> Signed-off-by: Claudiu Beznea<claudiu.beznea.uj@bp.renesas.com> >> --- >> >> Changes in v4: >> - none, this patch is new >> >> drivers/dma/sh/rz-dmac.c | 88 +++++++++++++++------------------------- >> 1 file changed, 33 insertions(+), 55 deletions(-) >> >> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c index 625ff29024de..9f206a33dcc6 >> 100644 >> --- a/drivers/dma/sh/rz-dmac.c >> +++ b/drivers/dma/sh/rz-dmac.c >> @@ -981,25 +981,6 @@ static int rz_dmac_chan_probe(struct rz_dmac *dmac, >> channel->index = index; >> channel->mid_rid = -EINVAL; >> >> - /* Request the channel interrupt. */ >> - scnprintf(pdev_irqname, sizeof(pdev_irqname), "ch%u", index); >> - irq = platform_get_irq_byname(pdev, pdev_irqname); >> - if (irq < 0) >> - return irq; >> - >> - irqname = devm_kasprintf(dmac->dev, GFP_KERNEL, "%s:%u", >> - dev_name(dmac->dev), index); >> - if (!irqname) >> - return -ENOMEM; >> - >> - ret = devm_request_threaded_irq(dmac->dev, irq, rz_dmac_irq_handler, >> - rz_dmac_irq_handler_thread, 0, >> - irqname, channel); >> - if (ret) { >> - dev_err(dmac->dev, "failed to request IRQ %u (%d)\n", irq, ret); >> - return ret; >> - } >> - >> /* Set io base address for each channel */ >> if (index < 8) { >> channel->ch_base = dmac->base + CHANNEL_0_7_OFFSET + @@ -1012,9 +993,9 @@ static int >> rz_dmac_chan_probe(struct rz_dmac *dmac, >> } >> >> /* Allocate descriptors */ >> - lmdesc = dma_alloc_coherent(&pdev->dev, >> - sizeof(struct rz_lmdesc) * DMAC_NR_LMDESC, >> - &channel->lmdesc.base_dma, GFP_KERNEL); >> + lmdesc = dmam_alloc_coherent(&pdev->dev, >> + sizeof(struct rz_lmdesc) * DMAC_NR_LMDESC, >> + &channel->lmdesc.base_dma, GFP_KERNEL); >> if (!lmdesc) { >> dev_err(&pdev->dev, "Can't allocate memory (lmdesc)\n"); >> return -ENOMEM; >> @@ -1030,7 +1011,24 @@ static int rz_dmac_chan_probe(struct rz_dmac *dmac, >> INIT_LIST_HEAD(&channel->ld_free); >> INIT_LIST_HEAD(&channel->ld_active); >> >> - return 0; >> + /* Request the channel interrupt. */ >> + scnprintf(pdev_irqname, sizeof(pdev_irqname), "ch%u", index); >> + irq = platform_get_irq_byname(pdev, pdev_irqname); >> + if (irq < 0) >> + return irq; >> + >> + irqname = devm_kasprintf(dmac->dev, GFP_KERNEL, "%s:%u", >> + dev_name(dmac->dev), index); >> + if (!irqname) >> + return -ENOMEM; >> + >> + ret = devm_request_threaded_irq(dmac->dev, irq, rz_dmac_irq_handler, >> + rz_dmac_irq_handler_thread, 0, >> + irqname, channel); >> + if (ret) >> + dev_err(dmac->dev, "failed to request IRQ %u (%d)\n", irq, ret); > As per [1], it is redundant. > > [1] > https://elixir.bootlin.com/linux/v7.0-rc7/source/kernel/irq/devres.c#L108 This is a fix patch, it just moves code around, intended to be backported to older kernels (e.g. v6.1, v6.12). However devm_request_result() is introduced in: commit 55b48e23f5c4 Author: Pan Chuang <panchuang@vivo.com> Date: Tue Aug 5 17:29:22 2025 +0800 genirq/devres: Add error handling in devm_request_*_irq() devm_request_threaded_irq() and devm_request_any_context_irq() currently don't print any error message when interrupt registration fails. This forces each driver to implement redundant error logging - over 2,000 lines of error messages exist across drivers. Additionally, when upper-layer functions propagate these errors without logging, critical debugging information is lost. Add devm_request_result() helper to unify error reporting via dev_err_probe(), Use it in devm_request_threaded_irq() and devm_request_any_context_irq() printing device name, IRQ number, handler functions, and error code on failure automatically. Co-developed-by: Yangtao Li <frank.li@vivo.com> Signed-off-by: Yangtao Li <frank.li@vivo.com> Signed-off-by: Pan Chuang <panchuang@vivo.com> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Link: https://lore.kernel.org/all/20250805092922.135500-2-panchuang@vivo.com And it is not present in v6.1, v6.12 kernels. To have a clean backport (at least to the above mentioned kernel versions), would be better to have the alignment to devm_request_result() done in a later cleanup patch. Thank you, Claudiu ^ permalink raw reply [flat|nested] 5+ messages in thread
* sashiko.dev review (Re: [PATCH v4 01/17] dmaengine: sh: rz-dmac: Move interrupt request after everything is set up) 2026-04-11 11:42 ` [PATCH v4 01/17] dmaengine: sh: rz-dmac: Move interrupt request after everything is set up Claudiu 2026-04-11 12:17 ` Biju Das @ 2026-04-20 12:33 ` Claudiu Beznea 1 sibling, 0 replies; 5+ messages in thread From: Claudiu Beznea @ 2026-04-20 12:33 UTC (permalink / raw) To: vkoul, Frank.Li, lgirdwood, broonie, perex, tiwai, biju.das.jz, prabhakar.mahadev-lad.rj, p.zabel, geert+renesas, fabrizio.castro.jz, long.luu.ur Cc: dmaengine, linux-kernel, linux-sound, linux-renesas-soc, Claudiu Beznea, stable > commit b3dfdd5fbd2463db407b1396ee1ae79801cb96bf > Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > dmaengine: sh: rz-dmac: Move interrupt request after everything is set up > This patch moves the channel interrupt request to the end of the channel > probe function to prevent the IRQ handler from accessing uninitialized > data. It also replaces dma_alloc_coherent() with dmam_alloc_coherent() > to simplify the error path. > > diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c > > index 625ff29024de0..9f206a33dcc6b 100644 > > --- a/drivers/dma/sh/rz-dmac.c > > +++ b/drivers/dma/sh/rz-dmac.c > [ ... ] > > @@ -1210,16 +1207,6 @@ static int rz_dmac_probe(struct platform_device *pdev) > > dma_register_err: > > of_dma_controller_free(pdev->dev.of_node); > > err: > > - channel_num = i ? i - 1 : 0; > > - for (i = 0; i < channel_num; i++) { > > - struct rz_dmac_chan *channel = &dmac->channels[i]; > > - > > - dma_free_coherent(&pdev->dev, > > - sizeof(struct rz_lmdesc) * DMAC_NR_LMDESC, > > - channel->lmdesc.base, > > - channel->lmdesc.base_dma); > > - } > > - > > reset_control_assert(dmac->rstc); > > err_pm_runtime_put: > > pm_runtime_put(&pdev->dev); > > @@ -1232,18 +1219,9 @@ static int rz_dmac_probe(struct platform_device *pdev) > > static void rz_dmac_remove(struct platform_device *pdev) > > { > > struct rz_dmac *dmac = platform_get_drvdata(pdev); > > - unsigned int i; > > > > dma_async_device_unregister(&dmac->engine); > > of_dma_controller_free(pdev->dev.of_node); > > - for (i = 0; i < dmac->n_channels; i++) { > > - struct rz_dmac_chan *channel = &dmac->channels[i]; > > - > > - dma_free_coherent(&pdev->dev, > > - sizeof(struct rz_lmdesc) * DMAC_NR_LMDESC, > > - channel->lmdesc.base, > > - channel->lmdesc.base_dma); > > - } > > reset_control_assert(dmac->rstc); > > pm_runtime_put(&pdev->dev); > > pm_runtime_disable(&pdev->dev); > > } > While this patch fixes the initialization races, does it leave a similar > vulnerability exposed during teardown? > Since devm_request_threaded_irq() is used to allocate the interrupts, they > will remain active until after rz_dmac_remove() and the rz_dmac_probe() error > paths complete. > If an interrupt fires during or just after rz_dmac_remove(), could the handler > attempt to access hardware registers while the device is in reset or powered > down by pm_runtime_put()? > Would it be safer to explicitly free or disable the IRQs before asserting the > hardware reset and disabling runtime PM, or perhaps manage the reset and PM > states via devm actions to guarantee correct teardown ordering? On either failure or remove path the device is with runtime PM put (clocks being disabled) and in reset state. The controller cannot generate interrupts from this state. However, to keep this series simple, I'll add this in a cleanup patch after the current series will be merged. ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v4 02/17] dmaengine: sh: rz-dmac: Fix incorrect NULL check on list_first_entry() [not found] <20260411114303.2814115-1-claudiu.beznea.uj@bp.renesas.com> 2026-04-11 11:42 ` [PATCH v4 01/17] dmaengine: sh: rz-dmac: Move interrupt request after everything is set up Claudiu @ 2026-04-11 11:42 ` Claudiu 1 sibling, 0 replies; 5+ messages in thread From: Claudiu @ 2026-04-11 11:42 UTC (permalink / raw) To: vkoul, Frank.Li, lgirdwood, broonie, perex, tiwai, biju.das.jz, prabhakar.mahadev-lad.rj, p.zabel, geert+renesas, fabrizio.castro.jz, long.luu.ur Cc: claudiu.beznea, dmaengine, linux-kernel, linux-sound, linux-renesas-soc, Claudiu Beznea, stable From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> The list passed as argument to list_first_entry() is expected to be not empty. Use list_first_entry_or_null() to avoid dereferencing invalid memory. Fixes: 21323b118c16 ("dmaengine: sh: rz-dmac: Add device_tx_status() callback") Cc: stable@vger.kernel.org Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> --- Changes in v4: - none, this patch is new drivers/dma/sh/rz-dmac.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c index 9f206a33dcc6..6d80cb668957 100644 --- a/drivers/dma/sh/rz-dmac.c +++ b/drivers/dma/sh/rz-dmac.c @@ -723,8 +723,8 @@ static u32 rz_dmac_chan_get_residue(struct rz_dmac_chan *channel, u32 crla, crtb, i; /* Get current processing virtual descriptor */ - current_desc = list_first_entry(&channel->ld_active, - struct rz_dmac_desc, node); + current_desc = list_first_entry_or_null(&channel->ld_active, + struct rz_dmac_desc, node); if (!current_desc) return 0; -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-04-20 12:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260411114303.2814115-1-claudiu.beznea.uj@bp.renesas.com>
2026-04-11 11:42 ` [PATCH v4 01/17] dmaengine: sh: rz-dmac: Move interrupt request after everything is set up Claudiu
2026-04-11 12:17 ` Biju Das
2026-04-11 12:34 ` Claudiu Beznea
2026-04-20 12:33 ` sashiko.dev review (Re: [PATCH v4 01/17] dmaengine: sh: rz-dmac: Move interrupt request after everything is set up) Claudiu Beznea
2026-04-11 11:42 ` [PATCH v4 02/17] dmaengine: sh: rz-dmac: Fix incorrect NULL check on list_first_entry() Claudiu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox