Re: [PATCH] spi: dw: Remove misleading comment for Mount Evans SoC

[Serge Semin <fancer.lancer@xxxxxxxxx>]

On Tue, Jun 06, 2023 at 04:18:44PM -0700, Abe Kohandel wrote:
> Remove a misleading comment about the DMA operations of the Intel Mount
> Evans SoC's SPI Controller as requested by Serge.
> 

> Signed-off-by: Abe Kohandel <abe.kohandel@xxxxxxxxx>
> Link: https://lore.kernel.org/linux-spi/20230606191333.247ucbf7h3tlooxf@mobilestation/
> Fixes: 0760d5d0e9f0 ("spi: dw: Add compatible for Intel Mount Evans SoC")

Note Fixes tag normally goes first. In this case it seems redundant
though.

> ---
>  drivers/spi/spi-dw-mmio.c | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
> index c1d16157de61..a699ce496cc5 100644
> --- a/drivers/spi/spi-dw-mmio.c
> +++ b/drivers/spi/spi-dw-mmio.c
> @@ -237,14 +237,7 @@ static int dw_spi_intel_init(struct platform_device *pdev,
>  }
>  
>  /*

> - * The Intel Mount Evans SoC's Integrated Management Complex uses the
> - * SPI controller for access to a NOR SPI FLASH. However, the SoC doesn't
> - * provide a mechanism to override the native chip select signal.

I had nothing against this part of the comment but only about the
second chunk of the text.

> - *
> - * This driver doesn't use DMA for memory operations when a chip select
> - * override is not provided due to the native chip select timing behavior.
> - * As a result no DMA configuration is done for the controller and this
> - * configuration is not tested.

> + * DMA-based mem ops are not configured for this device and are not tested.

* Note mem-ops is just a feature of the DW APB/AHB SSI controllers
* which provides a way to perform write-then-read and write-only
* transfers (see Transmit only and EEPROM read transfer modes in the
* hw manual). It works irrespective of whether your controller has a
* DMA-engine connected or doesn't have. Modern DW SSI controllers
* support Enhanced SPI modes with the extended SPI-bus width
* capability. But it's a whole another story and such modes aren't
* currently supported by the driver.

Just a question for the sake of the discussion history. Does your
platform have a DMA-engine synthesized to work with this DW SSI
controller? That is does your controller has the DMA Controller
Interface (handshake signals) connected to any DMA-engine on your
platform? I am asking because if there is no such DMA-engine then
the last part of your statement is just redundant since you can't test
something which isn't supported by design.

Anyway the change in this patch looks good. Thanks for submitting it.

Reviewed-by: Serge Semin <fancer.lancer@xxxxxxxxx>

-Serge(y)

>   */
>  static int dw_spi_mountevans_imc_init(struct platform_device *pdev,
>  				      struct dw_spi_mmio *dwsmmio)
> -- 
> 2.40.1
>