Re: [PATCH v3 3/6] spi: spi-fsl-dspi: Stub out DMA functions

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

On Wed, Jun 25, 2025, at 12:19, James Clark wrote:
> On 25/06/2025 11:00 am, Arnd Bergmann wrote:
>> On Wed, Jun 25, 2025, at 11:19, James Clark wrote:
>>> On 24/06/2025 6:16 pm, Arnd Bergmann wrote:
>
> Wouldn't that allow someone to break it by disabling (or not enabling) 
> that option? The driver won't fall back to the other modes if DMA isn't 
> configured, it just won't work. In this case it seems like it's better 
> to depend directly on DMA_ENGINE because that fixes the randconfig 
> issues, which is the whole reason for the discussion.

It would be the same as disabling DMA_ENGINE today when running on an
SoC that supports DMA mode in DSPI. Ideally that should fall back
to non-accelerated mode. I see a lot of checks for
trans_mode == DSPI_DMA_MODE, and I it's probably best to change
them to a function call like

static inline bool dsp_dma_mode(struct fsl_dspi *dspi)
{
     if (!IS_ENABLED(CONFIG_DMA_ENGINE)) // or CONFIG_FSL_DSPI_USE_DMA
          return false;

     return dspi->devtype_data->trans_mode == DSPI_DMA_MODE;
}

> Would someone really want an option to disable compilation of two 
> functions if their DSPI device is one that doesn't use DMA? Seems like 
> this option would always be on anyway.

It's probably mainly relevant if they want to completely turn off
CONFIG_DMA_ENGINE, which is substantially bigger. Using a check
for that symbol in the driver is certainly simpler for the user,
as they can't accidentally turn it off the custom symbol.

In theory you may also want to turn off DMA mode for testing,
which is supported by at least the DW_DMA driver.

I see that SPI_TEGRA114 has a dependency on TEGRA20_APB_DMA,
which is yet another variation. This is clearly done for
usability purposes since that SPI driver only ever works with
the specific DMA driver in practice, but it seems worse
conceptually.

     Arnd