Re: next-20250903 x86_64 clang-20 allyesconfig mmp_pdma.c:1188:14: error: shift count >= width of type [-Werror,-Wshift-count-overflow]

["Arnd Bergmann" <arnd@arndb.de>]

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