|
| 1 | +From git@z Thu Jan 1 00:00:00 1970 |
| 2 | +Subject: [PATCH] dmaengine: mmp_pdma: fix DMA mask handling |
| 3 | +From: Guodong Xu < [email protected]> |
| 4 | +Date: Thu, 18 Sep 2025 22:27:27 +0800 |
| 5 | +Message-Id: <20250918-mmp-pdma-simplify-dma-addressing-v1-1-5c2be2b85696@riscstar.com> |
| 6 | +MIME-Version: 1.0 |
| 7 | +Content-Type: text/plain; charset="utf-8" |
| 8 | +Content-Transfer-Encoding: 7bit |
| 9 | + |
| 10 | +The driver's existing logic for setting the DMA mask for "marvell,pdma-1.0" |
| 11 | +was flawed. It incorrectly relied on pdev->dev->coherent_dma_mask instead |
| 12 | +of declaring the hardware's fixed addressing capability. A cleaner and |
| 13 | +more correct approach is to define the mask directly based on the hardware |
| 14 | +limitations. |
| 15 | + |
| 16 | +The MMP/PXA PDMA controller is a 32-bit DMA engine. This is supported by |
| 17 | +datasheets and various dtsi files for PXA25x, PXA27x, PXA3xx, and MMP2, |
| 18 | +all of which are 32-bit systems. |
| 19 | + |
| 20 | +This patch simplifies the driver's logic by replacing the 'u64 dma_mask' |
| 21 | +field with a simpler 'u32 dma_width' to store the addressing capability |
| 22 | +in bits. The complex if/else block in probe() is then replaced with a |
| 23 | +single, clear call to dma_set_mask_and_coherent(). This sets a fixed |
| 24 | +32-bit DMA mask for "marvell,pdma-1.0" and a 64-bit mask for |
| 25 | +"spacemit,k1-pdma," matching each device's hardware capabilities. |
| 26 | + |
| 27 | +Finally, this change also works around a specific build error encountered |
| 28 | +with clang-20 on x86_64 allyesconfig. The shift-count-overflow error is |
| 29 | +caused by a known clang compiler issue where the DMA_BIT_MASK(n) macro's |
| 30 | +ternary operator is not correctly evaluated in static initializers. By |
| 31 | +moving the macro's evaluation into the probe() function, the driver avoids |
| 32 | +this compiler bug. |
| 33 | + |
| 34 | +Fixes: 5cfe585d8624 ("dmaengine: mmp_pdma: Add SpacemiT K1 PDMA support with 64-bit addressing") |
| 35 | +Reported-by: Naresh Kamboju < [email protected]> |
| 36 | +Closes: https://lore.kernel.org/lkml/CA+G9fYsPcMfW-e_0_TRqu4cnwqOqYF3aJOeKUYk6Z4qRStdFvg@mail.gmail.com |
| 37 | +Suggested-by: Arnd Bergmann < [email protected]> |
| 38 | +Signed-off-by: Guodong Xu < [email protected]> |
| 39 | +Tested-by: Naresh Kamboju < [email protected]> |
| 40 | +Tested-by: Nathan Chancellor < [email protected]> # build |
| 41 | +Reviewed-by: Arnd Bergmann < [email protected]> |
| 42 | +Link: https://patch.msgid.link/20250918-mmp-pdma-simplify-dma-addressing-v1-1-5c2be2b85696@riscstar.com |
| 43 | +--- |
| 44 | +Hi Vinod, Arnd, and all, |
| 45 | + |
| 46 | +This patch fixes a build failure in the mmp_pdma driver when using |
| 47 | +clang-20, as reported by Naresh Kamboju [1]. The error, |
| 48 | +a -Wshift-count-overflow warning, is caused by a known issue in the clang |
| 49 | +compiler. When the DMA_BIT_MASK(64) macro is used in a static initializer, |
| 50 | +it triggers a long-standing bug in how clang evaluates compile-time |
| 51 | +constants [2, 3]. Thanks to Nathan Chancellor for providing these links. |
| 52 | + |
| 53 | +While investigating this, Arnd Bergmann pointed out that the driver's |
| 54 | +logic for setting the DMA mask was unnecessarily complex. This logic, |
| 55 | +which was inherited from the original Marvel PDMA driver when adding |
| 56 | +support for SpacemiT K1 DMA, could be simplified. A better solution is to |
| 57 | +set each device's DMA mask directly based on its hardware capability. |
| 58 | + |
| 59 | +This patch implements Arnd's suggestion. It refactors the driver to store |
| 60 | +the controller's DMA width (either 32 or 64 bits) and generates the mask |
| 61 | +at runtime within the probe() function. |
| 62 | + |
| 63 | +This approach also avoids the clang false error reporting. |
| 64 | + |
| 65 | +This patch is based on dmaengine.git next [4]. |
| 66 | + |
| 67 | +Thanks, |
| 68 | +Guodong Xu |
| 69 | + |
| 70 | +[1] https://lore.kernel.org/lkml/CA+G9fYsPcMfW-e_0_TRqu4cnwqOqYF3aJOeKUYk6Z4qRStdFvg@mail.gmail.com/ |
| 71 | +[2] https://github.com/ClangBuiltLinux/linux/issues/92 |
| 72 | +[3] https://github.com/llvm/llvm-project/issues/38137 |
| 73 | +[4] https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/dmaengine.git/log/?h=next |
| 74 | +--- |
| 75 | + drivers/dma/mmp_pdma.c | 20 ++++++++------------ |
| 76 | + 1 file changed, 8 insertions(+), 12 deletions(-) |
| 77 | + |
| 78 | +diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c |
| 79 | +index d07229a748868b8115892c63c54c16130d88e326..86661eb3cde1ff6d6d8f02b6f0d4142878b5a890 100644 |
| 80 | +--- a/drivers/dma/mmp_pdma.c |
| 81 | ++++ b/drivers/dma/mmp_pdma.c |
| 82 | +@@ -152,8 +152,8 @@ struct mmp_pdma_phy { |
| 83 | + * |
| 84 | + * Controller Configuration: |
| 85 | + * @run_bits: Control bits in DCSR register for channel start/stop |
| 86 | +- * @dma_mask: DMA addressing capability of controller. 0 to use OF/platform |
| 87 | +- * settings, or explicit mask like DMA_BIT_MASK(32/64) |
| 88 | ++ * @dma_width: DMA addressing width in bits (32 or 64). Determines the |
| 89 | ++ * DMA mask capability of the controller hardware. |
| 90 | + */ |
| 91 | + struct mmp_pdma_ops { |
| 92 | + /* Hardware Register Operations */ |
| 93 | +@@ -173,7 +173,7 @@ struct mmp_pdma_ops { |
| 94 | + |
| 95 | + /* Controller Configuration */ |
| 96 | + u32 run_bits; |
| 97 | +- u64 dma_mask; |
| 98 | ++ u32 dma_width; |
| 99 | + }; |
| 100 | + |
| 101 | + struct mmp_pdma_device { |
| 102 | +@@ -1172,7 +1172,7 @@ static const struct mmp_pdma_ops marvell_pdma_v1_ops = { |
| 103 | + .get_desc_src_addr = get_desc_src_addr_32, |
| 104 | + .get_desc_dst_addr = get_desc_dst_addr_32, |
| 105 | + .run_bits = (DCSR_RUN), |
| 106 | +- .dma_mask = 0, /* let OF/platform set DMA mask */ |
| 107 | ++ .dma_width = 32, |
| 108 | + }; |
| 109 | + |
| 110 | + static const struct mmp_pdma_ops spacemit_k1_pdma_ops = { |
| 111 | +@@ -1185,7 +1185,7 @@ static const struct mmp_pdma_ops spacemit_k1_pdma_ops = { |
| 112 | + .get_desc_src_addr = get_desc_src_addr_64, |
| 113 | + .get_desc_dst_addr = get_desc_dst_addr_64, |
| 114 | + .run_bits = (DCSR_RUN | DCSR_LPAEEN), |
| 115 | +- .dma_mask = DMA_BIT_MASK(64), /* force 64-bit DMA addr capability */ |
| 116 | ++ .dma_width = 64, |
| 117 | + }; |
| 118 | + |
| 119 | + static const struct of_device_id mmp_pdma_dt_ids[] = { |
| 120 | +@@ -1314,13 +1314,9 @@ static int mmp_pdma_probe(struct platform_device *op) |
| 121 | + pdev->device.directions = BIT(DMA_MEM_TO_DEV) | BIT(DMA_DEV_TO_MEM); |
| 122 | + pdev->device.residue_granularity = DMA_RESIDUE_GRANULARITY_DESCRIPTOR; |
| 123 | + |
| 124 | +- /* Set DMA mask based on ops->dma_mask, or OF/platform */ |
| 125 | +- if (pdev->ops->dma_mask) |
| 126 | +- dma_set_mask(pdev->dev, pdev->ops->dma_mask); |
| 127 | +- else if (pdev->dev->coherent_dma_mask) |
| 128 | +- dma_set_mask(pdev->dev, pdev->dev->coherent_dma_mask); |
| 129 | +- else |
| 130 | +- dma_set_mask(pdev->dev, DMA_BIT_MASK(64)); |
| 131 | ++ /* Set DMA mask based on controller hardware capabilities */ |
| 132 | ++ dma_set_mask_and_coherent(pdev->dev, |
| 133 | ++ DMA_BIT_MASK(pdev->ops->dma_width)); |
| 134 | + |
| 135 | + ret = dma_async_device_register(&pdev->device); |
| 136 | + if (ret) { |
| 137 | + |
| 138 | +--- |
| 139 | +base-commit: cc0bacac6de7763a038550cf43cb94634d8be9cd |
| 140 | +change-id: 20250904-mmp-pdma-simplify-dma-addressing-f6aef03e07c3 |
| 141 | + |
| 142 | +Best regards, |
| 143 | +-- |
| 144 | + |
| 145 | + |
0 commit comments