Re: [PATCH] spi: spi_amd: Add HIDDMA basic write support

["Rangoju, Raju" <raju.rangoju@amd.com>]

On 5/12/2025 7:47 PM, Geert Uytterhoeven wrote:
> Hi Rangoju,
> 
> On Mon, 12 May 2025 at 09:29, Rangoju, Raju <raju.rangoju@amd.com> wrote:
>> On 5/11/2025 3:51 AM, kernel test robot wrote:
>>> kernel test robot noticed the following build warnings:
>>>
>>> [auto build test WARNING on v6.15-rc5]
>>> [also build test WARNING on linus/master]
>>> [cannot apply to broonie-spi/for-next next-20250509]
>>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>>> And when submitting patch, we suggest to use '--base' as documented in
>>> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>>>
>>> url:    https://github.com/intel-lab-lkp/linux/commits/Raju-Rangoju/spi-spi_amd-Add-HIDDMA-basic-write-support/20250510-021954
>>> base:   v6.15-rc5
>>> patch link:    https://lore.kernel.org/r/20250509181737.997167-1-Raju.Rangoju%40amd.com
>>> patch subject: [PATCH] spi: spi_amd: Add HIDDMA basic write support
>>> config: m68k-randconfig-r111-20250511 (https://download.01.org/0day-ci/archive/20250511/202505110641.zLT16Dv7-lkp@intel.com/config)
>>> compiler: m68k-linux-gcc (GCC) 14.2.0
>>
>> Thanks for reporting this. We do not support m68k.
> 
> All write[bwlq]() functions take a volatile void __iomem pointer
> (https://elixir.bootlin.com/linux/v6.14.6/source/include/asm-generic/io.h#L174)
> while you are passing a void *, so sparse should complain about this
> on all architectures.  

My bad, with the following flags included, sparse now complains this on 
all architectures.

-fmax-errors=unlimited -fmax-warnings=unlimited

And sparse is right, this driver is using MMIO
> accessors on allocated DMA memory, which is just plain wrong:
> 
>      amd_spi->dma_virt_addr = dma_alloc_coherent(dev, AMD_SPI_HID2_DMA_SIZE,
>              &amd_spi->phy_dma_buf, GFP_KERNEL);
> 
>       for (i = 0; left_data >= 8; i++, left_data -= 8)
>              *buf_64++ = readq((u8 __iomem *)amd_spi->dma_virt_addr + (i * 8));
> 
>> Will re-spin v2 with necessary changes in Kconfig.
> 
> Please fix the real issue instead ;-)

We are using read*/write* calls instead of memcpy to copy data to/from 
DMA memory due to performance concerns, as we observed better throughput 
during continuous read/write compared to the memcpy functions.

Additionally, the DMA operations are entirely handled by the hardware, 
and the software's role is limited to copying data to the DMA buffer.

> 
>>> reproduce: (https://download.01.org/0day-ci/archive/20250511/202505110641.zLT16Dv7-lkp@intel.com/reproduce)
>>>
>>> If you fix the issue in a separate patch/commit (i.e. not just a new version of
>>> the same patch/commit), kindly add following tags
>>> | Reported-by: kernel test robot <lkp@intel.com>
>>> | Closes: https://lore.kernel.org/oe-kbuild-all/202505110641.zLT16Dv7-lkp@intel.com/
>>>
>>> sparse warnings: (new ones prefixed by >>)
>>>>> drivers/spi/spi-amd.c:594:57: sparse: sparse: incorrect type in argument 2 (different address spaces) @@     expected void volatile [noderef] __iomem *addr @@     got void * @@
>>>      drivers/spi/spi-amd.c:594:57: sparse:     expected void volatile [noderef] __iomem *addr
>>>      drivers/spi/spi-amd.c:594:57: sparse:     got void *
> 
> Gr{oetje,eeting}s,
> 
>                          Geert
>