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

[Abe Kohandel <abe.kohandel@xxxxxxxxx>]

On 23/06/07 02:27PM, Serge Semin wrote:
> 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.
> 

Thanks, will do this in the future.

> > - * 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.

Thinking about it a bit more there is nothing precluding this controller from
being used for other purposes in the future. It is configured with two chip
selects, only one of which is used today. I removed it to so it wouldn't become
inaccurate if that happens.

> > + * 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.

The platform does have a DMA-engine synthesized but I have been having some
challenges with getting it to work which may require some further quirks added
to the DMA driver. One example being the system uses 40-bit addressing but the
DMA-engine is only synthesized with 32-bit address capability and is meant to
only target a specific region of memory where it "knowns" the upper byte of the
address.

Anyhow, I hope to work through some of those challenges and enable this in the
future.

Thanks,
Abe