On Tue, Jun 06, 2023 at 12:07:56PM -0700, Abe Kohandel wrote: > 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. Ok. Thanks. Could you submit a fixup patch then? Mark has already merged the series in as is: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git/commit/?id=0760d5d0e9f0c0e2200a0323a61d1995bb745dee -Serge(y) > > Thanks, > Abe
