Thread (22 messages) 22 messages, 5 authors, 2024-02-07

Re: [PATCH 3/4] spi: s3c64xx: add s3c64xx_iowrite{8,16}_32_rep accessors

From: Sam Protsenko <semen.protsenko@linaro.org>
Date: 2024-02-06 18:44:25
Also in: linux-devicetree, linux-samsung-soc, linux-spi, lkml

On Tue, Feb 6, 2024 at 2:52 AM Tudor Ambarus [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Allow SoCs that require 32 bits register accesses to write data in
chunks of 8 or 16 bits. One SoC that requires 32 bit register accesses
is the google gs101. The operation is rare, thus open code it in the
driver rather than making it generic (through asm-generic/io.h)

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 drivers/spi/spi-s3c64xx.c | 70 +++++++++++++++++++++++++++++++--------
 1 file changed, 56 insertions(+), 14 deletions(-)
diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index c15ca6a910dc..cb45ad615f3d 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -142,6 +142,7 @@ struct s3c64xx_spi_dma_data {
  *     prescaler unit.
  * @clk_ioclk: True if clock is present on this device
  * @has_loopback: True if loopback mode can be supported
+ * @use_32bit_io: True if the SoC allows just 32-bit register accesses.
A matter of taste, but: just -> only.
quoted hunk ↗ jump to hunk
  *
  * The Samsung s3c64xx SPI controller are used on various Samsung SoC's but
  * differ in some aspects such as the size of the fifo and spi bus clock
@@ -158,6 +159,7 @@ struct s3c64xx_spi_port_config {
        bool    clk_from_cmu;
        bool    clk_ioclk;
        bool    has_loopback;
+       bool    use_32bit_io;
 };

 /**
@@ -412,6 +414,59 @@ static bool s3c64xx_spi_can_dma(struct spi_controller *host,
        return false;
 }

+static void s3c64xx_iowrite8_32_rep(volatile void __iomem *addr,
+                                   const void *buffer, unsigned int count)
+{
+       if (count) {
+               const u8 *buf = buffer;
+
+               do {
+                       __raw_writel(*buf++, addr);
+               } while (--count);
+       }
How about:

    while (count--)
        __raw_writel(*buf++, addr);

This way "if" condition is not needed. The same goes for the function below.
+}
+
+static void s3c64xx_iowrite16_32_rep(volatile void __iomem *addr,
+                                    const void *buffer, unsigned int count)
+{
+       if (count) {
+               const u16 *buf = buffer;
+
+               do {
+                       __raw_writel(*buf++, addr);
+               } while (--count);
+       }
+}
+
+static void s3c64xx_iowrite_rep(const struct s3c64xx_spi_driver_data *sdd,
+                               struct spi_transfer *xfer)
+{
+       void __iomem *regs = sdd->regs;
Suggest declaring aliases here, like this:

    void __iomem *addr = sdd->regs + S3C64XX_SPI_TX_DATA;
    const void *buf = xfer->tx_buf;
    unsigned int len = xfer->len;

Using those in the code below makes it more compact and easier to read.
quoted hunk ↗ jump to hunk
+
+       switch (sdd->cur_bpw) {
+       case 32:
+               iowrite32_rep(regs + S3C64XX_SPI_TX_DATA,
+                             xfer->tx_buf, xfer->len / 4);
+               break;
+       case 16:
+               if (sdd->port_conf->use_32bit_io)
+                       s3c64xx_iowrite16_32_rep(regs + S3C64XX_SPI_TX_DATA,
+                                                xfer->tx_buf, xfer->len / 2);
+               else
+                       iowrite16_rep(regs + S3C64XX_SPI_TX_DATA,
+                                     xfer->tx_buf, xfer->len / 2);
+               break;
+       default:
+               if (sdd->port_conf->use_32bit_io)
+                       s3c64xx_iowrite8_32_rep(regs + S3C64XX_SPI_TX_DATA,
+                                               xfer->tx_buf, xfer->len);
+               else
+                       iowrite8_rep(regs + S3C64XX_SPI_TX_DATA,
+                                    xfer->tx_buf, xfer->len);
+               break;
+       }
+}
+
 static int s3c64xx_enable_datapath(struct s3c64xx_spi_driver_data *sdd,
                                    struct spi_transfer *xfer, int dma_mode)
 {
@@ -445,20 +500,7 @@ static int s3c64xx_enable_datapath(struct s3c64xx_spi_driver_data *sdd,
                        modecfg |= S3C64XX_SPI_MODE_TXDMA_ON;
                        ret = s3c64xx_prepare_dma(&sdd->tx_dma, &xfer->tx_sg);
                } else {
-                       switch (sdd->cur_bpw) {
-                       case 32:
-                               iowrite32_rep(regs + S3C64XX_SPI_TX_DATA,
-                                       xfer->tx_buf, xfer->len / 4);
-                               break;
-                       case 16:
-                               iowrite16_rep(regs + S3C64XX_SPI_TX_DATA,
-                                       xfer->tx_buf, xfer->len / 2);
-                               break;
-                       default:
-                               iowrite8_rep(regs + S3C64XX_SPI_TX_DATA,
-                                       xfer->tx_buf, xfer->len);
-                               break;
-                       }
+                       s3c64xx_iowrite_rep(sdd, xfer);
This extraction (with no functional change yet) could've been a
preceding separate patch, preparing things for this rework.
                }
        }

--
2.43.0.594.gd9cf4e227d-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help