Hi Serge, On 23/06/06 08:28PM, Serge Semin wrote: > Hi Abe > > On Tue, Jun 06, 2023 at 07:54:01AM -0700, Abe Kohandel wrote: > > 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. > > > > 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. > > > > The controller also has an errata where a full TX FIFO can result in > > data corruption. The suggested workaround is to never completely fill > > the FIFO. The TX FIFO has a size of 32 so the fifo_len is set to 31. > > > > Signed-off-by: Abe Kohandel <abe.kohandel@xxxxxxxxx> > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > > --- > > drivers/spi/spi-dw-mmio.c | 29 +++++++++++++++++++++++++++++ > > 1 file changed, 29 insertions(+) > > > > diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c > > index 5f2aee69c1c1..c1d16157de61 100644 > > --- a/drivers/spi/spi-dw-mmio.c > > +++ b/drivers/spi/spi-dw-mmio.c > > @@ -236,6 +236,31 @@ static int dw_spi_intel_init(struct platform_device *pdev, > > return 0; > > } > > > > +/* > > + * 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. > > + * > > > + * 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. > > Based on what is written you didn't test the DMA-based memory > operations on your hardware. Well, this driver doesn't use DMA for > memory operations on the platforms with the native CS just because > nobody has implemented that feature so far. AFAICS if DMA-based memory > operations were supported by the driver I don't think that the native > CS auto de-assertion would have been an issue except when there is no > hw-accelerated LLPs list handling in the DMA controller (in the later > case we could have fallen back to the IRQ-less implementation though). > Moreover having the DMA-based memory ops implemented would have been > even better than what the driver provides at the moment since it would > have eliminated the mem-op transfers in the atomic context. So the > comment seems misleading. Another problem is that it refers to a > feature which may be added in future. So the comment will be wrong > then. So I would suggest to either drop the comment or change to > something that just states that the DMA-based mem ops weren't tested > for this hardware. > > Am I wrong in some aspects of understanding your comment? Did you mean > something else than what I inferred from it? > > -Serge(y) > You have interpreted my comments correctly. I can see how the comment is misleading and can become obsolete in the future. I will shorten the comment to just indicated that no DMA-based mem ops are tested for this hardware. Thanks, Abe
