| Message ID | 20230606145402.474866-1-abe.kohandel@intel.com |
|---|---|
| Headers | show |
| Series | spi: dw: Add compatible for Intel Mount Evans SoC | expand |
On Tue, Jun 06, 2023 at 07:40:40PM +0300, Serge Semin wrote: > Mark, next time please wait at least for a few days before applying. > Give me a chance to review.) It's a trivial quirk for a platform, it seemed more hassle to wait TBH. > * In this case I have had a question regarding the in-code comment > which may have been needed to be fixed a bit. That can always be fixed incrementally if there's issues.
On Tue, Jun 06, 2023 at 05:47:40PM +0100, Mark Brown wrote: > On Tue, Jun 06, 2023 at 07:40:40PM +0300, Serge Semin wrote: > > > Mark, next time please wait at least for a few days before applying. > > Give me a chance to review.) > > It's a trivial quirk for a platform, it seemed more hassle to wait TBH. > > > * In this case I have had a question regarding the in-code comment > > which may have been needed to be fixed a bit. > > That can always be fixed incrementally if there's issues. Ok. I've sent my comment in that regards. Let's see what the author responds. -Serge(y)
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@intel.com> > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > --- > > 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
Hi Serge, > 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 Patch submited in: https://lore.kernel.org/linux-spi/20230606231844.726272-1-abe.kohandel@intel.com/ Thanks, Abe
The Intel Mount Evans SoC's Integrated Management Complex has a DW apb_ssi_v4.02a controller. This series adds support for this controller. No SoC level chip select override is provided and as such no DMA configuration is done for the controller. Thanks, Abe Abe Kohandel (2): spi: dw: Add compatible for Intel Mount Evans SoC dt-bindings: spi: snps,dw-apb-ssi: Add compatible for Intel Mount Evans SoC .../bindings/spi/snps,dw-apb-ssi.yaml | 2 ++ drivers/spi/spi-dw-mmio.c | 29 +++++++++++++++++++ 2 files changed, 31 insertions(+)