dmaengine: dw-axi-dmac: PM cleanup and reset control support
Subsystems (2)
v5 cover dmaengine: dw-axi-dmac: PM cleanup and reset control support
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.
For debugging, I used dev_info from the suspend/resume functions. Before pushing, I removed dev_info from the driver.
Suspend:
echo 0 > /sys/module/printk/parameters/console_suspend echo mem > /sys/power/state
...
[ 195.339311] dw_axi_dmac_platform 100NDA00.NDA-axi-dma: drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c axi_dma_suspend [ 195.350274] dw_axi_dmac_platform 100NDA00.NDA-axi-dma: drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c axi_dma_suspend [ 195.361223] dw_axi_dmac_platform 100NDA00.NDA-axi-dma: drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c axi_dma_suspend
...
Resume:
...
[ 200.669945] dw_axi_dmac_platform 100NDA00.NDA-axi-dma: drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c axi_dma_resume [ 200.680975] dw_axi_dmac_platform 100NDA00.NDA-axi-dma: drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c axi_dma_resume [ 200.692108] dw_axi_dmac_platform 100NDA00.NDA-axi-dma: drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c axi_dma_resume
...
Patch 3 resolves the following smatch warnings:drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:1237 dma_chan_pause() warn: inconsistent indenting drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:1284 axi_chan_resume() warn: inconsistent indenting
To check the fix of the warnings:
make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- -j$(nproc) C=2 \
CHECK="../smatch/smatch -p=kernel" \
drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.o--
Best regards,
Artem Shimko
ChangeLog:
v1:
* https://lore.kernel.org/all/[email protected]/T/#t
v2:
* https://lore.kernel.org/all/[email protected]/T/#u
v3:
* https://lore.kernel.org/all/[email protected]/T/#t
v4:
* https://lore.kernel.org/all/[email protected]/T/#t
v5:
* Fix smatch warnings about inconsistent indentation in dma_chan_pause() and axi_chan_resume() functions.
Artem Shimko (3):
dmaengine: dw-axi-dmac: simplify PM functions and use modern macros
dmaengine: dw-axi-dmac: add reset control support
dmaengine: dw-axi-dmac: fix inconsistent indentation in pause/resume
functions
.../dma/dw-axi-dmac/dw-axi-dmac-platform.c | 90 +++++++++++-------- drivers/dma/dw-axi-dmac/dw-axi-dmac.h | 1 + 2 files changed, 52 insertions(+), 39 deletions(-)
Signature · 1 line
2.43.0
PATCH v5 1/3 dmaengine: dw-axi-dmac: simplify PM functions and use modern macros
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() is used for the platform driver. Probe and remove functions are updated to call PM functions with device pointer.
--- .../dma/dw-axi-dmac/dw-axi-dmac-platform.c | 31 ++++++------------- 1 file changed, 9 insertions(+), 22 deletions(-)
drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c +9 -22
@@ -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);
Signature · 1 line
2.43.0
PATCH v5 2/3 dmaengine: dw-axi-dmac: add reset control support
In reply to Artem ShimkoAdd 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.
--- .../dma/dw-axi-dmac/dw-axi-dmac-platform.c | 59 +++++++++++++------ drivers/dma/dw-axi-dmac/dw-axi-dmac.h | 1 + 2 files changed, 43 insertions(+), 17 deletions(-)
drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c +42 -17
@@ -1316,11 +1316,16 @@ static int dma_chan_resume(struct dma_chan *dchan)static int axi_dma_suspend (struct device *dev) { +int ret;struct axi_dma_chip *chip =dev_get_drvdata (dev);axi_dma_irq_disable (chip);axi_dma_disable (chip); + ret =reset_control_assert (chip->resets); +if (ret) +dev_warn (dev,"Failed to assert resets, but continuing suspend\n" ); +clk_disable_unprepare (chip->core_clk);clk_disable_unprepare (chip->cfgr_clk);
@@ -1338,12 +1343,23 @@ static int axi_dma_resume(struct device *dev) ret =clk_prepare_enable (chip->core_clk);if (ret <0 ) -return ret; + goto cfgr_clk_disable; + + ret =reset_control_deassert (chip->resets); +if (ret) + goto core_clk_disable;axi_dma_enable (chip);axi_dma_irq_enable (chip);return 0 ; + +core_clk_disable: +clk_disable_unprepare (chip->core_clk); +cfgr_clk_disable: +clk_disable_unprepare (chip->cfgr_clk); + +return ret; } static struct dma_chan *dw_axi_dma_of_xlate(struct of_phandle_args *dma_spec,
@@ -1455,7 +1471,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 +1502,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 +1512,28 @@ static int dw_probe(struct platform_device *pdev)if (IS_ERR (chip->cfgr_clk))return PTR_ERR (chip->cfgr_clk); + chip->resets =devm_reset_control_array_get_optional_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,20 +1620,30 @@ static int dw_probe(struct platform_device *pdev) err_pm_disable:pm_runtime_disable (chip->dev); +err_exit: +reset_control_assert (chip->resets);return ret; }static void dw_remove (struct platform_device *pdev) { +int ret;struct axi_dma_chip *chip =platform_get_drvdata (pdev);struct dw_axi_dma *dw = chip->dw;struct axi_dma_chan *chan, *_chan;u32 i; -/* Enable clk before accessing to registers */ +/* +* The peripheral must be clocked and out of reset +* before its registers can be accessed. +*/ clk_prepare_enable (chip->cfgr_clk);clk_prepare_enable (chip->core_clk); + ret =reset_control_deassert (chip->resets); +if (ret) +dev_err (&pdev->dev,"Failed to deassert resets\n" ); +axi_dma_irq_disable (chip);for (i =0 ; i < dw->hdata->nr_channels; i++) {axi_chan_disable (&chip->dw->chan[i]);
drivers/dma/dw-axi-dmac/dw-axi-dmac.h +1 -0
@@ -71,6 +71,7 @@ struct axi_dma_chip {struct clk *core_clk;struct clk *cfgr_clk;struct dw_axi_dma *dw; +struct reset_control *resets; };/* LLI == Linked List Item */
Signature · 1 line
2.43.0
PATCH v5 3/3 dmaengine: dw-axi-dmac: fix inconsistent indentation in pause/resume functions
In reply to Artem ShimkoFix smatch warnings about inconsistent indentation in dma_chan_pause() and axi_chan_resume() functions.
The issue was with misaligned closing braces and incorrect indentation of axi_dma_iowrite64() calls following if-else blocks.
The changes address code style violations by correcting misaligned indentation in conditional blocks. Specifically, the closing braces in if-else statements were improperly aligned, and subsequent calls to axi_dma_iowrite64() had incorrect indentation levels.
These fixes ensure consistent code formatting according to kernel coding standards while preserving the original functionality and improving overall code readability.
--- drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c +2 -2
@@ -1234,7 +1234,7 @@ static int dma_chan_pause(struct dma_chan *dchan) val |=BIT (chan->id) <<DMAC_CHAN_SUSP2_SHIFT |BIT (chan->id) <<DMAC_CHAN_SUSP2_WE_SHIFT ; } -axi_dma_iowrite64 (chan->chip,DMAC_CHSUSPREG , val); +axi_dma_iowrite64 (chan->chip,DMAC_CHSUSPREG , val); }else {if (chan->chip->dw->hdata->reg_map_8_channels) { val =axi_dma_ioread32 (chan->chip,DMAC_CHEN );
@@ -1281,7 +1281,7 @@ static inline void axi_chan_resume(struct axi_dma_chan *chan) val &= ~(BIT (chan->id) <<DMAC_CHAN_SUSP2_SHIFT ); val |= (BIT (chan->id) <<DMAC_CHAN_SUSP2_WE_SHIFT ); } -axi_dma_iowrite64 (chan->chip,DMAC_CHSUSPREG , val); +axi_dma_iowrite64 (chan->chip,DMAC_CHSUSPREG , val); }else {if (chan->chip->dw->hdata->reg_map_8_channels) { val =axi_dma_ioread32 (chan->chip,DMAC_CHEN );
Signature · 1 line
2.43.0