| Message ID | 20240724154739.582367-5-wsadowski@marvell.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Marvell HW overlay support for Cadence xSPI | expand |
Hi Witold, On Wed, Jul 24, 2024 at 5:48 PM Witold Sadowski <wsadowski@marvell.com> wrote: > In Marvell xSPI implementation any access to SDMA register will result > in 8 byte SPI data transfer. Reading less data(eg. 1B) will result in > losing remaining bytes. To avoid that read/write 8 bytes into temporary > buffer, and read/write whole temporary buffer into SDMA. > > Signed-off-by: Witold Sadowski <wsadowski@marvell.com> Thanks for your patch, which is now commit 75128e2a14a9f443 ("spi: cadence: Add Marvell SDMA operations") in linux-next/master next-20240730 spi/for-next > --- a/drivers/spi/spi-cadence-xspi.c > +++ b/drivers/spi/spi-cadence-xspi.c > @@ -310,6 +310,7 @@ struct cdns_xspi_dev { > u8 hw_num_banks; > > const struct cdns_xspi_driver_data *driver_data; > + void (*sdma_handler)(struct cdns_xspi_dev *cdns_xspi); > }; > > static void cdns_xspi_reset_dll(struct cdns_xspi_dev *cdns_xspi) > @@ -515,6 +516,78 @@ static void cdns_xspi_sdma_handle(struct cdns_xspi_dev *cdns_xspi) > } > } > > +static void m_ioreadq(void __iomem *addr, void *buf, int len) > +{ > + if (IS_ALIGNED((long)buf, 8) && len >= 8) { > + u64 full_ops = len / 8; > + u64 *buffer = buf; > + > + len -= full_ops * 8; > + buf += full_ops * 8; > + > + do { > + u64 b = readq(addr); noreply@ellerman.id.au reports build failures on 32-bit (e.g. [1]): drivers/spi/spi-cadence-xspi.c:612:33: error: implicit declaration of function 'readq'; did you mean 'readb'? [-Werror=implicit-function-declaration] drivers/spi/spi-cadence-xspi.c:638:25: error: implicit declaration of function 'writeq'; did you mean 'writel'? [-Werror=implicit-function-declaration] readq() and writeq() are not available on 32-bit platforms, so this driver has to depend on 64BIT (for compile-testing). > + *buffer++ = b; > + } while (--full_ops); > + } [1] http://kisskb.ellerman.id.au/kisskb/buildresult/15210014/ Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Greet > drivers/spi/spi-cadence-xspi.c:612:33: error: implicit declaration of > function 'readq'; did you mean 'readb'? > [-Werror=implicit-function-declaration] > drivers/spi/spi-cadence-xspi.c:638:25: error: implicit declaration of > function 'writeq'; did you mean 'writel'? > [-Werror=implicit-function-declaration] > > > readq() and writeq() are not available on 32-bit platforms, so this > driver has to > depend on 64BIT (for compile-testing). > > > + *buffer++ = b; > > + } while (--full_ops); > > + } How can I limit that driver for 64bit test only? Regards Witek
Hi Witold, On Tue, Jul 30, 2024 at 12:06 PM Witold Sadowski <wsadowski@marvell.com> wrote: > > drivers/spi/spi-cadence-xspi.c:612:33: error: implicit declaration of > > function 'readq'; did you mean 'readb'? > > [-Werror=implicit-function-declaration] > > drivers/spi/spi-cadence-xspi.c:638:25: error: implicit declaration of > > function 'writeq'; did you mean 'writel'? > > [-Werror=implicit-function-declaration] > > > > > > readq() and writeq() are not available on 32-bit platforms, so this > > driver has to > > depend on 64BIT (for compile-testing). > > > > > + *buffer++ = b; > > > + } while (--full_ops); > > > + } > > How can I limit that driver for 64bit test only? drivers/spi/Kconfig, config SPI_CADENCE_XSPI: -depends on OF && HAS_IOMEM +depends on OF && HAS_IOMEM && 64BIT Gr{oetje,eeting}s, Geert
Hi Geert > drivers/spi/Kconfig, config SPI_CADENCE_XSPI: > -depends on OF && HAS_IOMEM > +depends on OF && HAS_IOMEM && 64BIT Can that be send as separate patch, or whole patch series should be updated? Regards Witek
On Tue, Jul 30, 2024 at 10:17:43AM +0000, Witold Sadowski wrote: > > drivers/spi/Kconfig, config SPI_CADENCE_XSPI: > > -depends on OF && HAS_IOMEM > > +depends on OF && HAS_IOMEM && 64BIT > Can that be send as separate patch, or whole patch series should be updated? Please respin the series.
diff --git a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-xspi.c index d0222284c507..c177bf4ba536 100644 --- a/drivers/spi/spi-cadence-xspi.c +++ b/drivers/spi/spi-cadence-xspi.c @@ -310,6 +310,7 @@ struct cdns_xspi_dev { u8 hw_num_banks; const struct cdns_xspi_driver_data *driver_data; + void (*sdma_handler)(struct cdns_xspi_dev *cdns_xspi); }; static void cdns_xspi_reset_dll(struct cdns_xspi_dev *cdns_xspi) @@ -515,6 +516,78 @@ static void cdns_xspi_sdma_handle(struct cdns_xspi_dev *cdns_xspi) } } +static void m_ioreadq(void __iomem *addr, void *buf, int len) +{ + if (IS_ALIGNED((long)buf, 8) && len >= 8) { + u64 full_ops = len / 8; + u64 *buffer = buf; + + len -= full_ops * 8; + buf += full_ops * 8; + + do { + u64 b = readq(addr); + *buffer++ = b; + } while (--full_ops); + } + + + while (len) { + u64 tmp_buf; + + tmp_buf = readq(addr); + memcpy(buf, &tmp_buf, min(len, 8)); + len = len > 8 ? len - 8 : 0; + buf += 8; + } +} + +static void m_iowriteq(void __iomem *addr, const void *buf, int len) +{ + if (IS_ALIGNED((long)buf, 8) && len >= 8) { + u64 full_ops = len / 8; + const u64 *buffer = buf; + + len -= full_ops * 8; + buf += full_ops * 8; + + do { + writeq(*buffer++, addr); + } while (--full_ops); + } + + while (len) { + u64 tmp_buf; + + memcpy(&tmp_buf, buf, min(len, 8)); + writeq(tmp_buf, addr); + len = len > 8 ? len - 8 : 0; + buf += 8; + } +} + +static void marvell_xspi_sdma_handle(struct cdns_xspi_dev *cdns_xspi) +{ + u32 sdma_size, sdma_trd_info; + u8 sdma_dir; + + sdma_size = readl(cdns_xspi->iobase + CDNS_XSPI_SDMA_SIZE_REG); + sdma_trd_info = readl(cdns_xspi->iobase + CDNS_XSPI_SDMA_TRD_INFO_REG); + sdma_dir = FIELD_GET(CDNS_XSPI_SDMA_DIR, sdma_trd_info); + + switch (sdma_dir) { + case CDNS_XSPI_SDMA_DIR_READ: + m_ioreadq(cdns_xspi->sdmabase, + cdns_xspi->in_buffer, sdma_size); + break; + + case CDNS_XSPI_SDMA_DIR_WRITE: + m_iowriteq(cdns_xspi->sdmabase, + cdns_xspi->out_buffer, sdma_size); + break; + } +} + static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi, const struct spi_mem_op *op, bool data_phase) @@ -566,7 +639,7 @@ static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi, cdns_xspi_set_interrupts(cdns_xspi, false); return -EIO; } - cdns_xspi_sdma_handle(cdns_xspi); + cdns_xspi->sdma_handler(cdns_xspi); } wait_for_completion(&cdns_xspi->cmd_complete); @@ -736,10 +809,13 @@ static int cdns_xspi_probe(struct platform_device *pdev) if (!cdns_xspi->driver_data) return -ENODEV; - if (cdns_xspi->driver_data->mrvl_hw_overlay) + if (cdns_xspi->driver_data->mrvl_hw_overlay) { host->mem_ops = &marvell_xspi_mem_ops; - else + cdns_xspi->sdma_handler = &marvell_xspi_sdma_handle; + } else { host->mem_ops = &cadence_xspi_mem_ops; + cdns_xspi->sdma_handler = &cdns_xspi_sdma_handle; + } host->dev.of_node = pdev->dev.of_node; host->bus_num = -1;
In Marvell xSPI implementation any access to SDMA register will result in 8 byte SPI data transfer. Reading less data(eg. 1B) will result in losing remaining bytes. To avoid that read/write 8 bytes into temporary buffer, and read/write whole temporary buffer into SDMA. Signed-off-by: Witold Sadowski <wsadowski@marvell.com> --- drivers/spi/spi-cadence-xspi.c | 82 ++++++++++++++++++++++++++++++++-- 1 file changed, 79 insertions(+), 3 deletions(-)