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