On Wed, Sep 3, 2025, at 12:08, Naresh Kamboju wrote:
Build error: drivers/dma/mmp_pdma.c:1188:14: error: shift count >= width of type [-Werror,-Wshift-count-overflow] 1188 | .dma_mask = DMA_BIT_MASK(64), /* force 64-bit DMA addr capability */ | ^~~~~~~~~~~~~~~~ include/linux/dma-mapping.h:73:54: note: expanded from macro 'DMA_BIT_MASK' 73 | #define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1)) | ^ ~~~
I see two separate issues:
1. The current DMA_BIT_MASK() definition seems unfortunate, as the '(n) == 64' check is meant to avoid this problem, but I think this only works inside of a function, not in a static structure definition. This could perhaps be avoided by replacing the ?: operator with __builtin_choose_expr(), but that likely causes other build failures.
2. The dma_mask logic in this driver looks very strange and makes no sense to me.
Guodong Xu just added the line above, to set the dma mask for the spacemit variant, with the new logic being:
+ /* Set DMA mask based on ops->dma_mask, or OF/platform */ + if (pdev->ops->dma_mask) + dma_set_mask(pdev->dev, pdev->ops->dma_mask); + else if (pdev->dev->coherent_dma_mask) dma_set_mask(pdev->dev, pdev->dev->coherent_dma_mask); else dma_set_mask(pdev->dev, DMA_BIT_MASK(64));
This has multiple problems:
- the coherent_dma_mask is still left at the default 32-bit mask for spacemit, which I think is a mistake, even if the effect is the same - The existing dma_set_mask(pdev->dev, pdev->dev->coherent_dma_mask); is completely bogus, as the driver should just set a fixed 32-bit mask based on the capabilities of the device. No other driver bsides mmp_pdma.c and pxa_dma.c does this. - The pxa/mmp variant clearly supports 32-bit addressing, no more, no less, so just setting the 32-bit mask should be enough.
Guodong, how about a patch to drop all the custom dma_mask handling and instead just use dma_set_mask_and_coherent(DMA_BIT_MASK(64)) or dma_set_mask_and_coherent(DMA_BIT_MASK(32)) here? Instead of passing the mask in the mmp_pdma_ops, you can replace it e.g. with a 'bool addr64' flag, or an 'int dma_width' number that gets passed into the DMA_MASK_MASK().
Arnd