* [PATCH 0/2] dmaengine: dw-axi-dmac: PM cleanup and reset control support @ 2025-10-12 9:59 Artem Shimko 2025-10-12 9:59 ` [PATCH 1/2] dmaengine: dw-axi-dmac: simplify PM functions and use modern macros Artem Shimko 2025-10-12 10:00 ` [PATCH 2/2] dmaengine: dw-axi-dmac: add reset control support Artem Shimko 0 siblings, 2 replies; 7+ messages in thread From: Artem Shimko @ 2025-10-12 9:59 UTC (permalink / raw) To: linux-kernel, Vinod Koul; +Cc: a.shimko.dev, dmaengine, Eugeniy Paltsev Hello maintainers and reviewers, This patch series improves the dw-axi-dmac driver in two areas: Patch 1 simplifies the power management code by using modern kernel macros and removing redundant wrapper functions, making the code more maintainable and aligned with current kernel practices. Patch 2 adds proper reset control support to ensure reliable initialization and power management, handling resets during probe, remove, and suspend/resume operations. Best regards, Artem Shimko Artem Shimko (2): dmaengine: dw-axi-dmac: simplify PM functions and use modern macros dmaengine: dw-axi-dmac: add reset control support .../dma/dw-axi-dmac/dw-axi-dmac-platform.c | 72 +++++++++---------- drivers/dma/dw-axi-dmac/dw-axi-dmac.h | 2 + 2 files changed, 37 insertions(+), 37 deletions(-) -- 2.43.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] dmaengine: dw-axi-dmac: simplify PM functions and use modern macros 2025-10-12 9:59 [PATCH 0/2] dmaengine: dw-axi-dmac: PM cleanup and reset control support Artem Shimko @ 2025-10-12 9:59 ` Artem Shimko 2025-10-12 10:00 ` [PATCH 2/2] dmaengine: dw-axi-dmac: add reset control support Artem Shimko 1 sibling, 0 replies; 7+ messages in thread From: Artem Shimko @ 2025-10-12 9:59 UTC (permalink / raw) To: Eugeniy Paltsev, Vinod Koul; +Cc: a.shimko.dev, dmaengine, linux-kernel Simplify the power management code by removing redundant wrapper functions and using modern kernel PM macros. This reduces code duplication and improves maintainability. The changes convert the suspend/resume functions to take device pointer directly instead of the chip structure, allowing removal of the runtime PM wrapper functions. The manual PM ops definition is replaced with DEFINE_RUNTIME_DEV_PM_OPS() macro and pm_ptr(). Probe and remove functions are updated to call PM functions with device pointer. Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com> --- .../dma/dw-axi-dmac/dw-axi-dmac-platform.c | 31 ++++++------------- 1 file changed, 9 insertions(+), 22 deletions(-) diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c index b23536645ff7..8b7cf3baf5d3 100644 --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c @@ -1314,8 +1314,10 @@ static int dma_chan_resume(struct dma_chan *dchan) return 0; } -static int axi_dma_suspend(struct axi_dma_chip *chip) +static int axi_dma_suspend(struct device *dev) { + struct axi_dma_chip *chip = dev_get_drvdata(dev); + axi_dma_irq_disable(chip); axi_dma_disable(chip); @@ -1325,9 +1327,10 @@ static int axi_dma_suspend(struct axi_dma_chip *chip) return 0; } -static int axi_dma_resume(struct axi_dma_chip *chip) +static int axi_dma_resume(struct device *dev) { int ret; + struct axi_dma_chip *chip = dev_get_drvdata(dev); ret = clk_prepare_enable(chip->cfgr_clk); if (ret < 0) @@ -1343,20 +1346,6 @@ static int axi_dma_resume(struct axi_dma_chip *chip) return 0; } -static int __maybe_unused axi_dma_runtime_suspend(struct device *dev) -{ - struct axi_dma_chip *chip = dev_get_drvdata(dev); - - return axi_dma_suspend(chip); -} - -static int __maybe_unused axi_dma_runtime_resume(struct device *dev) -{ - struct axi_dma_chip *chip = dev_get_drvdata(dev); - - return axi_dma_resume(chip); -} - static struct dma_chan *dw_axi_dma_of_xlate(struct of_phandle_args *dma_spec, struct of_dma *ofdma) { @@ -1590,7 +1579,7 @@ static int dw_probe(struct platform_device *pdev) * driver to work also without Runtime PM. */ pm_runtime_get_noresume(chip->dev); - ret = axi_dma_resume(chip); + ret = axi_dma_resume(chip->dev); if (ret < 0) goto err_pm_disable; @@ -1638,7 +1627,7 @@ static void dw_remove(struct platform_device *pdev) axi_dma_disable(chip); pm_runtime_disable(chip->dev); - axi_dma_suspend(chip); + axi_dma_suspend(chip->dev); for (i = 0; i < DMAC_MAX_CHANNELS; i++) if (chip->irq[i] > 0) @@ -1653,9 +1642,7 @@ static void dw_remove(struct platform_device *pdev) } } -static const struct dev_pm_ops dw_axi_dma_pm_ops = { - SET_RUNTIME_PM_OPS(axi_dma_runtime_suspend, axi_dma_runtime_resume, NULL) -}; +static DEFINE_RUNTIME_DEV_PM_OPS(dw_axi_dma_pm_ops, axi_dma_suspend, axi_dma_resume, NULL); static const struct of_device_id dw_dma_of_id_table[] = { { @@ -1680,7 +1667,7 @@ static struct platform_driver dw_driver = { .driver = { .name = KBUILD_MODNAME, .of_match_table = dw_dma_of_id_table, - .pm = &dw_axi_dma_pm_ops, + .pm = pm_ptr(&dw_axi_dma_pm_ops), }, }; module_platform_driver(dw_driver); -- 2.43.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] dmaengine: dw-axi-dmac: add reset control support 2025-10-12 9:59 [PATCH 0/2] dmaengine: dw-axi-dmac: PM cleanup and reset control support Artem Shimko 2025-10-12 9:59 ` [PATCH 1/2] dmaengine: dw-axi-dmac: simplify PM functions and use modern macros Artem Shimko @ 2025-10-12 10:00 ` Artem Shimko 2025-10-13 8:41 ` Philipp Zabel 1 sibling, 1 reply; 7+ messages in thread From: Artem Shimko @ 2025-10-12 10:00 UTC (permalink / raw) To: Eugeniy Paltsev, Vinod Koul, Philipp Zabel Cc: a.shimko.dev, dmaengine, linux-kernel Add proper reset control handling to the AXI DMA driver to ensure reliable initialization and power management. The driver now manages resets during probe, remove, and system suspend/resume operations. The implementation stores reset control in the chip structure and adds reset assert/deassert calls at the appropriate points: resets are deasserted during probe after clock acquisition, asserted during remove and error cleanup, and properly managed during suspend/resume cycles. Additionally, proper error handling is implemented for reset control operations to ensure robust behavior. This ensures the controller is properly reset during power transitions and prevents potential issues with incomplete initialization. Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com> --- .../dma/dw-axi-dmac/dw-axi-dmac-platform.c | 41 ++++++++++++------- drivers/dma/dw-axi-dmac/dw-axi-dmac.h | 2 + 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c index 8b7cf3baf5d3..3f4dd2178498 100644 --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c @@ -1321,6 +1321,9 @@ static int axi_dma_suspend(struct device *dev) axi_dma_irq_disable(chip); axi_dma_disable(chip); + if (chip->has_resets) + reset_control_assert(chip->resets); + clk_disable_unprepare(chip->core_clk); clk_disable_unprepare(chip->cfgr_clk); @@ -1340,6 +1343,9 @@ static int axi_dma_resume(struct device *dev) if (ret < 0) return ret; + if (chip->has_resets) + reset_control_deassert(chip->resets); + axi_dma_enable(chip); axi_dma_irq_enable(chip); @@ -1455,7 +1461,6 @@ static int dw_probe(struct platform_device *pdev) struct axi_dma_chip *chip; struct dw_axi_dma *dw; struct dw_axi_dma_hcfg *hdata; - struct reset_control *resets; unsigned int flags; u32 i; int ret; @@ -1487,16 +1492,6 @@ static int dw_probe(struct platform_device *pdev) return PTR_ERR(chip->apb_regs); } - if (flags & AXI_DMA_FLAG_HAS_RESETS) { - resets = devm_reset_control_array_get_exclusive(&pdev->dev); - if (IS_ERR(resets)) - return PTR_ERR(resets); - - ret = reset_control_deassert(resets); - if (ret) - return ret; - } - chip->dw->hdata->use_cfg2 = !!(flags & AXI_DMA_FLAG_USE_CFG2); chip->core_clk = devm_clk_get(chip->dev, "core-clk"); @@ -1507,18 +1502,31 @@ static int dw_probe(struct platform_device *pdev) if (IS_ERR(chip->cfgr_clk)) return PTR_ERR(chip->cfgr_clk); + chip->has_resets = !!(flags & AXI_DMA_FLAG_HAS_RESETS); + if (chip->has_resets) { + chip->resets = devm_reset_control_array_get_exclusive(&pdev->dev); + if (IS_ERR(chip->resets)) + return PTR_ERR(chip->resets); + + ret = reset_control_deassert(chip->resets); + if (ret) + return dev_err_probe(&pdev->dev, ret, "Failed to deassert resets\n"); + } + ret = parse_device_properties(chip); if (ret) - return ret; + goto err_exit; dw->chan = devm_kcalloc(chip->dev, hdata->nr_channels, sizeof(*dw->chan), GFP_KERNEL); - if (!dw->chan) - return -ENOMEM; + if (!dw->chan) { + ret = -ENOMEM; + goto err_exit; + } ret = axi_req_irqs(pdev, chip); if (ret) - return ret; + goto err_exit; INIT_LIST_HEAD(&dw->dma.channels); for (i = 0; i < hdata->nr_channels; i++) { @@ -1605,6 +1613,9 @@ static int dw_probe(struct platform_device *pdev) err_pm_disable: pm_runtime_disable(chip->dev); +err_exit: + if (chip->has_resets) + reset_control_assert(chip->resets); return ret; } diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h index b842e6a8d90d..56dc3d75fe92 100644 --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h @@ -71,6 +71,8 @@ struct axi_dma_chip { struct clk *core_clk; struct clk *cfgr_clk; struct dw_axi_dma *dw; + struct reset_control *resets; + bool has_resets; }; /* LLI == Linked List Item */ -- 2.43.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] dmaengine: dw-axi-dmac: add reset control support 2025-10-12 10:00 ` [PATCH 2/2] dmaengine: dw-axi-dmac: add reset control support Artem Shimko @ 2025-10-13 8:41 ` Philipp Zabel 2025-10-13 14:56 ` Artem Shimko 0 siblings, 1 reply; 7+ messages in thread From: Philipp Zabel @ 2025-10-13 8:41 UTC (permalink / raw) To: Artem Shimko, Eugeniy Paltsev, Vinod Koul; +Cc: dmaengine, linux-kernel On So, 2025-10-12 at 13:00 +0300, Artem Shimko wrote: > Add proper reset control handling to the AXI DMA driver to ensure > reliable initialization and power management. The driver now manages > resets during probe, remove, and system suspend/resume operations. > > The implementation stores reset control in the chip structure and adds > reset assert/deassert calls at the appropriate points: resets are > deasserted during probe after clock acquisition, asserted during remove > and error cleanup, and properly managed during suspend/resume cycles. > Additionally, proper error handling is implemented for reset control > operations to ensure robust behavior. > > This ensures the controller is properly reset during power transitions > and prevents potential issues with incomplete initialization. > > Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com> > --- > .../dma/dw-axi-dmac/dw-axi-dmac-platform.c | 41 ++++++++++++------- > drivers/dma/dw-axi-dmac/dw-axi-dmac.h | 2 + > 2 files changed, 28 insertions(+), 15 deletions(-) > > diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > index 8b7cf3baf5d3..3f4dd2178498 100644 > --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > @@ -1321,6 +1321,9 @@ static int axi_dma_suspend(struct device *dev) > axi_dma_irq_disable(chip); > axi_dma_disable(chip); > > + if (chip->has_resets) > + reset_control_assert(chip->resets); reset_control_assert/deassert() handle NULL pointers, so you could drop the chip->has_resets flag and just reset_control_assert(chip->resets); unconditionally. > + > clk_disable_unprepare(chip->core_clk); > clk_disable_unprepare(chip->cfgr_clk); > > @@ -1340,6 +1343,9 @@ static int axi_dma_resume(struct device *dev) > if (ret < 0) > return ret; > > + if (chip->has_resets) > + reset_control_deassert(chip->resets); > + Same as above. > axi_dma_enable(chip); > axi_dma_irq_enable(chip); > > @@ -1455,7 +1461,6 @@ static int dw_probe(struct platform_device *pdev) > struct axi_dma_chip *chip; > struct dw_axi_dma *dw; > struct dw_axi_dma_hcfg *hdata; > - struct reset_control *resets; > unsigned int flags; > u32 i; > int ret; > @@ -1487,16 +1492,6 @@ static int dw_probe(struct platform_device *pdev) > return PTR_ERR(chip->apb_regs); > } > > - if (flags & AXI_DMA_FLAG_HAS_RESETS) { > - resets = devm_reset_control_array_get_exclusive(&pdev->dev); > - if (IS_ERR(resets)) > - return PTR_ERR(resets); > - > - ret = reset_control_deassert(resets); > - if (ret) > - return ret; > - } > - > chip->dw->hdata->use_cfg2 = !!(flags & AXI_DMA_FLAG_USE_CFG2); > > chip->core_clk = devm_clk_get(chip->dev, "core-clk"); > @@ -1507,18 +1502,31 @@ static int dw_probe(struct platform_device *pdev) > if (IS_ERR(chip->cfgr_clk)) > return PTR_ERR(chip->cfgr_clk); > > + chip->has_resets = !!(flags & AXI_DMA_FLAG_HAS_RESETS); > + if (chip->has_resets) { > + chip->resets = devm_reset_control_array_get_exclusive(&pdev->dev); > + if (IS_ERR(chip->resets)) > + return PTR_ERR(chip->resets); > + > + ret = reset_control_deassert(chip->resets); > + if (ret) > + return dev_err_probe(&pdev->dev, ret, "Failed to deassert resets\n"); > + } > + Why is this moved down here? > ret = parse_device_properties(chip); > if (ret) > - return ret; > + goto err_exit; > > dw->chan = devm_kcalloc(chip->dev, hdata->nr_channels, > sizeof(*dw->chan), GFP_KERNEL); > - if (!dw->chan) > - return -ENOMEM; > + if (!dw->chan) { > + ret = -ENOMEM; > + goto err_exit; > + } > > ret = axi_req_irqs(pdev, chip); > if (ret) > - return ret; > + goto err_exit; > > INIT_LIST_HEAD(&dw->dma.channels); > for (i = 0; i < hdata->nr_channels; i++) { > @@ -1605,6 +1613,9 @@ static int dw_probe(struct platform_device *pdev) > > err_pm_disable: > pm_runtime_disable(chip->dev); > +err_exit: > + if (chip->has_resets) > + reset_control_assert(chip->resets); If it is ok to keep the module in reset, shouldn't the reset control be asserted on device remove() as well? regards Philipp ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] dmaengine: dw-axi-dmac: add reset control support 2025-10-13 8:41 ` Philipp Zabel @ 2025-10-13 14:56 ` Artem Shimko 2025-10-13 15:41 ` Philipp Zabel 0 siblings, 1 reply; 7+ messages in thread From: Artem Shimko @ 2025-10-13 14:56 UTC (permalink / raw) To: Philipp Zabel; +Cc: Eugeniy Paltsev, Vinod Koul, dmaengine, linux-kernel Hi Philipp, On Mon, Oct 13, 2025 at 11:41 AM Philipp Zabel <p.zabel@pengutronix.de> wrote: > reset_control_assert/deassert() handle NULL pointers, so you could drop > the chip->has_resets flag and just > > reset_control_assert(chip->resets); > > unconditionally. Thanks, I'll fix that > Why is this moved down here? Reset operations typically require clock signals to be available. By moving reset after clock acquisition (devm_clk_get), we ensure that the clock is ready to operate when reset is performed. > If it is ok to keep the module in reset, shouldn't the reset control be > asserted on device remove() as well? dw_remove() has axi_dma_suspend() function, which is where the reset assertion occurs via reset_control_assert(). Best regards, Artem Shimko ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] dmaengine: dw-axi-dmac: add reset control support 2025-10-13 14:56 ` Artem Shimko @ 2025-10-13 15:41 ` Philipp Zabel 2025-10-13 20:08 ` Artem Shimko 0 siblings, 1 reply; 7+ messages in thread From: Philipp Zabel @ 2025-10-13 15:41 UTC (permalink / raw) To: Artem Shimko; +Cc: Eugeniy Paltsev, Vinod Koul, dmaengine, linux-kernel On Mo, 2025-10-13 at 17:56 +0300, Artem Shimko wrote: [...] > dw_remove() has axi_dma_suspend() function, which is where the reset assertion > occurs via reset_control_assert(). It looks to me like dw_remove() is now missing a deassert before accessing registers, in case the device is removed while runtime suspended. regards Philipp ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] dmaengine: dw-axi-dmac: add reset control support 2025-10-13 15:41 ` Philipp Zabel @ 2025-10-13 20:08 ` Artem Shimko 0 siblings, 0 replies; 7+ messages in thread From: Artem Shimko @ 2025-10-13 20:08 UTC (permalink / raw) To: Philipp Zabel; +Cc: Eugeniy Paltsev, Vinod Koul, dmaengine, linux-kernel Hi Philipp, On Mon, Oct 13, 2025 at 6:41 PM Philipp Zabel <p.zabel@pengutronix.de> wrote: > > On Mo, 2025-10-13 at 17:56 +0300, Artem Shimko wrote: > > [...] > > dw_remove() has axi_dma_suspend() function, which is where the reset assertion > > occurs via reset_control_assert(). > > It looks to me like dw_remove() is now missing a deassert before > accessing registers, in case the device is removed while runtime > suspended. > > regards > Philipp oh, I see, it looks like we have to insert reset_control_deassert(chip->resets); just after clk_prepare_enable(chip->cfgr_clk);... You're absolutely right! Just realized I may have been too quick with v2. Will wait for more feedback and send v3. Best regards, Artem Shimko ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-10-13 20:08 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-10-12 9:59 [PATCH 0/2] dmaengine: dw-axi-dmac: PM cleanup and reset control support Artem Shimko 2025-10-12 9:59 ` [PATCH 1/2] dmaengine: dw-axi-dmac: simplify PM functions and use modern macros Artem Shimko 2025-10-12 10:00 ` [PATCH 2/2] dmaengine: dw-axi-dmac: add reset control support Artem Shimko 2025-10-13 8:41 ` Philipp Zabel 2025-10-13 14:56 ` Artem Shimko 2025-10-13 15:41 ` Philipp Zabel 2025-10-13 20:08 ` Artem Shimko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox