Use the bitfield access macros in order to clean and to make the driver
easier to read.
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/spi/spi-s3c64xx.c | 196 +++++++++++++++++++-------------------
1 file changed, 99 insertions(+), 97 deletions(-)
diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index db1e1d6ee732..16eea56892a2 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -4,6 +4,7 @@
// Jaswinder Singh <[email protected]>
#include <linux/bits.h>
+#include <linux/bitfield.h>
#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/dma-mapping.h>
@@ -17,91 +18,91 @@
#include <linux/pm_runtime.h>
#include <linux/spi/spi.h>
-#define MAX_SPI_PORTS 12
-#define S3C64XX_SPI_QUIRK_CS_AUTO (1 << 1)
-#define AUTOSUSPEND_TIMEOUT 2000
+#define MAX_SPI_PORTS 12
+#define S3C64XX_SPI_QUIRK_CS_AUTO BIT(1)
+#define AUTOSUSPEND_TIMEOUT 2000
/* Registers and bit-fields */
-#define S3C64XX_SPI_CH_CFG 0x00
-#define S3C64XX_SPI_CLK_CFG 0x04
-#define S3C64XX_SPI_MODE_CFG 0x08
-#define S3C64XX_SPI_CS_REG 0x0C
-#define S3C64XX_SPI_INT_EN 0x10
-#define S3C64XX_SPI_STATUS 0x14
-#define S3C64XX_SPI_TX_DATA 0x18
-#define S3C64XX_SPI_RX_DATA 0x1C
-#define S3C64XX_SPI_PACKET_CNT 0x20
-#define S3C64XX_SPI_PENDING_CLR 0x24
-#define S3C64XX_SPI_SWAP_CFG 0x28
-#define S3C64XX_SPI_FB_CLK 0x2C
-
-#define S3C64XX_SPI_CH_HS_EN (1<<6) /* High Speed Enable */
-#define S3C64XX_SPI_CH_SW_RST (1<<5)
-#define S3C64XX_SPI_CH_SLAVE (1<<4)
-#define S3C64XX_SPI_CPOL_L (1<<3)
-#define S3C64XX_SPI_CPHA_B (1<<2)
-#define S3C64XX_SPI_CH_RXCH_ON (1<<1)
-#define S3C64XX_SPI_CH_TXCH_ON (1<<0)
-
-#define S3C64XX_SPI_CLKSEL_SRCMSK (3<<9)
-#define S3C64XX_SPI_CLKSEL_SRCSHFT 9
-#define S3C64XX_SPI_ENCLK_ENABLE (1<<8)
-#define S3C64XX_SPI_PSR_MASK 0xff
-
-#define S3C64XX_SPI_MODE_CH_TSZ_BYTE (0<<29)
-#define S3C64XX_SPI_MODE_CH_TSZ_HALFWORD (1<<29)
-#define S3C64XX_SPI_MODE_CH_TSZ_WORD (2<<29)
-#define S3C64XX_SPI_MODE_CH_TSZ_MASK (3<<29)
-#define S3C64XX_SPI_MODE_BUS_TSZ_BYTE (0<<17)
-#define S3C64XX_SPI_MODE_BUS_TSZ_HALFWORD (1<<17)
-#define S3C64XX_SPI_MODE_BUS_TSZ_WORD (2<<17)
-#define S3C64XX_SPI_MODE_BUS_TSZ_MASK (3<<17)
+#define S3C64XX_SPI_CH_CFG 0x00
+#define S3C64XX_SPI_CLK_CFG 0x04
+#define S3C64XX_SPI_MODE_CFG 0x08
+#define S3C64XX_SPI_CS_REG 0x0C
+#define S3C64XX_SPI_INT_EN 0x10
+#define S3C64XX_SPI_STATUS 0x14
+#define S3C64XX_SPI_TX_DATA 0x18
+#define S3C64XX_SPI_RX_DATA 0x1C
+#define S3C64XX_SPI_PACKET_CNT 0x20
+#define S3C64XX_SPI_PENDING_CLR 0x24
+#define S3C64XX_SPI_SWAP_CFG 0x28
+#define S3C64XX_SPI_FB_CLK 0x2C
+
+#define S3C64XX_SPI_CH_HS_EN BIT(6) /* High Speed Enable */
+#define S3C64XX_SPI_CH_SW_RST BIT(5)
+#define S3C64XX_SPI_CH_SLAVE BIT(4)
+#define S3C64XX_SPI_CPOL_L BIT(3)
+#define S3C64XX_SPI_CPHA_B BIT(2)
+#define S3C64XX_SPI_CH_RXCH_ON BIT(1)
+#define S3C64XX_SPI_CH_TXCH_ON BIT(0)
+
+#define S3C64XX_SPI_CLKSEL_SRCMSK GENMASK(10, 9)
+#define S3C64XX_SPI_ENCLK_ENABLE BIT(8)
+#define S3C64XX_SPI_PSR_MASK GENMASK(15, 0)
+
+#define S3C64XX_SPI_MODE_CH_TSZ_MASK GENMASK(30, 29)
+#define S3C64XX_SPI_MODE_CH_TSZ_BYTE 0
+#define S3C64XX_SPI_MODE_CH_TSZ_HALFWORD 1
+#define S3C64XX_SPI_MODE_CH_TSZ_WORD 2
+#define S3C64XX_SPI_MAX_TRAILCNT_MASK GENMASK(28, 19)
+#define S3C64XX_SPI_MODE_BUS_TSZ_MASK GENMASK(18, 17)
+#define S3C64XX_SPI_MODE_BUS_TSZ_BYTE 0
+#define S3C64XX_SPI_MODE_BUS_TSZ_HALFWORD 1
+#define S3C64XX_SPI_MODE_BUS_TSZ_WORD 2
#define S3C64XX_SPI_MODE_RX_RDY_LVL GENMASK(16, 11)
-#define S3C64XX_SPI_MODE_RX_RDY_LVL_SHIFT 11
-#define S3C64XX_SPI_MODE_SELF_LOOPBACK (1<<3)
-#define S3C64XX_SPI_MODE_RXDMA_ON (1<<2)
-#define S3C64XX_SPI_MODE_TXDMA_ON (1<<1)
-#define S3C64XX_SPI_MODE_4BURST (1<<0)
-
-#define S3C64XX_SPI_CS_NSC_CNT_2 (2<<4)
-#define S3C64XX_SPI_CS_AUTO (1<<1)
-#define S3C64XX_SPI_CS_SIG_INACT (1<<0)
-
-#define S3C64XX_SPI_INT_TRAILING_EN (1<<6)
-#define S3C64XX_SPI_INT_RX_OVERRUN_EN (1<<5)
-#define S3C64XX_SPI_INT_RX_UNDERRUN_EN (1<<4)
-#define S3C64XX_SPI_INT_TX_OVERRUN_EN (1<<3)
-#define S3C64XX_SPI_INT_TX_UNDERRUN_EN (1<<2)
-#define S3C64XX_SPI_INT_RX_FIFORDY_EN (1<<1)
-#define S3C64XX_SPI_INT_TX_FIFORDY_EN (1<<0)
-
-#define S3C64XX_SPI_ST_RX_OVERRUN_ERR (1<<5)
-#define S3C64XX_SPI_ST_RX_UNDERRUN_ERR (1<<4)
-#define S3C64XX_SPI_ST_TX_OVERRUN_ERR (1<<3)
-#define S3C64XX_SPI_ST_TX_UNDERRUN_ERR (1<<2)
-#define S3C64XX_SPI_ST_RX_FIFORDY (1<<1)
-#define S3C64XX_SPI_ST_TX_FIFORDY (1<<0)
-
-#define S3C64XX_SPI_PACKET_CNT_EN (1<<16)
+#define S3C64XX_SPI_MODE_SELF_LOOPBACK BIT(3)
+#define S3C64XX_SPI_MODE_RXDMA_ON BIT(2)
+#define S3C64XX_SPI_MODE_TXDMA_ON BIT(1)
+#define S3C64XX_SPI_MODE_4BURST BIT(0)
+
+#define S3C64XX_SPI_CS_NSC_CNT_MASK GENMASK(9, 4)
+#define S3C64XX_SPI_CS_NSC_CNT_2 2
+#define S3C64XX_SPI_CS_AUTO BIT(1)
+#define S3C64XX_SPI_CS_SIG_INACT BIT(0)
+
+#define S3C64XX_SPI_INT_TRAILING_EN BIT(6)
+#define S3C64XX_SPI_INT_RX_OVERRUN_EN BIT(5)
+#define S3C64XX_SPI_INT_RX_UNDERRUN_EN BIT(4)
+#define S3C64XX_SPI_INT_TX_OVERRUN_EN BIT(3)
+#define S3C64XX_SPI_INT_TX_UNDERRUN_EN BIT(2)
+#define S3C64XX_SPI_INT_RX_FIFORDY_EN BIT(1)
+#define S3C64XX_SPI_INT_TX_FIFORDY_EN BIT(0)
+
+#define S3C64XX_SPI_ST_RX_OVERRUN_ERR BIT(5)
+#define S3C64XX_SPI_ST_RX_UNDERRUN_ERR BIT(4)
+#define S3C64XX_SPI_ST_TX_OVERRUN_ERR BIT(3)
+#define S3C64XX_SPI_ST_TX_UNDERRUN_ERR BIT(2)
+#define S3C64XX_SPI_ST_RX_FIFORDY BIT(1)
+#define S3C64XX_SPI_ST_TX_FIFORDY BIT(0)
+
+#define S3C64XX_SPI_PACKET_CNT_EN BIT(16)
#define S3C64XX_SPI_PACKET_CNT_MASK GENMASK(15, 0)
-#define S3C64XX_SPI_PND_TX_UNDERRUN_CLR (1<<4)
-#define S3C64XX_SPI_PND_TX_OVERRUN_CLR (1<<3)
-#define S3C64XX_SPI_PND_RX_UNDERRUN_CLR (1<<2)
-#define S3C64XX_SPI_PND_RX_OVERRUN_CLR (1<<1)
-#define S3C64XX_SPI_PND_TRAILING_CLR (1<<0)
+#define S3C64XX_SPI_PND_TX_UNDERRUN_CLR BIT(4)
+#define S3C64XX_SPI_PND_TX_OVERRUN_CLR BIT(3)
+#define S3C64XX_SPI_PND_RX_UNDERRUN_CLR BIT(2)
+#define S3C64XX_SPI_PND_RX_OVERRUN_CLR BIT(1)
+#define S3C64XX_SPI_PND_TRAILING_CLR BIT(0)
-#define S3C64XX_SPI_SWAP_RX_HALF_WORD (1<<7)
-#define S3C64XX_SPI_SWAP_RX_BYTE (1<<6)
-#define S3C64XX_SPI_SWAP_RX_BIT (1<<5)
-#define S3C64XX_SPI_SWAP_RX_EN (1<<4)
-#define S3C64XX_SPI_SWAP_TX_HALF_WORD (1<<3)
-#define S3C64XX_SPI_SWAP_TX_BYTE (1<<2)
-#define S3C64XX_SPI_SWAP_TX_BIT (1<<1)
-#define S3C64XX_SPI_SWAP_TX_EN (1<<0)
+#define S3C64XX_SPI_SWAP_RX_HALF_WORD BIT(7)
+#define S3C64XX_SPI_SWAP_RX_BYTE BIT(6)
+#define S3C64XX_SPI_SWAP_RX_BIT BIT(5)
+#define S3C64XX_SPI_SWAP_RX_EN BIT(4)
+#define S3C64XX_SPI_SWAP_TX_HALF_WORD BIT(3)
+#define S3C64XX_SPI_SWAP_TX_BYTE BIT(2)
+#define S3C64XX_SPI_SWAP_TX_BIT BIT(1)
+#define S3C64XX_SPI_SWAP_TX_EN BIT(0)
-#define S3C64XX_SPI_FBCLK_MSK (3<<0)
+#define S3C64XX_SPI_FBCLK_MASK GENMASK(1, 0)
#define FIFO_LVL_MASK(i) ((i)->port_conf->fifo_lvl_mask[i->port_id])
#define S3C64XX_SPI_ST_TX_DONE(v, i) (((v) & \
@@ -111,18 +112,13 @@
FIFO_LVL_MASK(i))
#define FIFO_DEPTH(i) ((FIFO_LVL_MASK(i) >> 1) + 1)
-#define S3C64XX_SPI_MAX_TRAILCNT 0x3ff
-#define S3C64XX_SPI_TRAILCNT_OFF 19
-
-#define S3C64XX_SPI_TRAILCNT S3C64XX_SPI_MAX_TRAILCNT
-
#define S3C64XX_SPI_POLLING_SIZE 32
#define msecs_to_loops(t) (loops_per_jiffy / 1000 * HZ * t)
#define is_polling(x) (x->cntrlr_info->polling)
-#define RXBUSY (1<<2)
-#define TXBUSY (1<<3)
+#define RXBUSY BIT(2)
+#define TXBUSY BIT(3)
struct s3c64xx_spi_dma_data {
struct dma_chan *ch;
@@ -341,8 +337,9 @@ static void s3c64xx_spi_set_cs(struct spi_device *spi, bool enable)
} else {
u32 ssel = readl(sdd->regs + S3C64XX_SPI_CS_REG);
- ssel |= (S3C64XX_SPI_CS_AUTO |
- S3C64XX_SPI_CS_NSC_CNT_2);
+ ssel |= S3C64XX_SPI_CS_AUTO |
+ FIELD_PREP(S3C64XX_SPI_CS_NSC_CNT_MASK,
+ S3C64XX_SPI_CS_NSC_CNT_2);
writel(ssel, sdd->regs + S3C64XX_SPI_CS_REG);
}
} else {
@@ -665,16 +662,22 @@ static int s3c64xx_spi_config(struct s3c64xx_spi_driver_data *sdd)
switch (sdd->cur_bpw) {
case 32:
- val |= S3C64XX_SPI_MODE_BUS_TSZ_WORD;
- val |= S3C64XX_SPI_MODE_CH_TSZ_WORD;
+ val |= FIELD_PREP(S3C64XX_SPI_MODE_BUS_TSZ_MASK,
+ S3C64XX_SPI_MODE_BUS_TSZ_WORD) |
+ FIELD_PREP(S3C64XX_SPI_MODE_CH_TSZ_MASK,
+ S3C64XX_SPI_MODE_CH_TSZ_WORD);
break;
case 16:
- val |= S3C64XX_SPI_MODE_BUS_TSZ_HALFWORD;
- val |= S3C64XX_SPI_MODE_CH_TSZ_HALFWORD;
+ val |= FIELD_PREP(S3C64XX_SPI_MODE_BUS_TSZ_MASK,
+ S3C64XX_SPI_MODE_BUS_TSZ_HALFWORD) |
+ FIELD_PREP(S3C64XX_SPI_MODE_CH_TSZ_MASK,
+ S3C64XX_SPI_MODE_CH_TSZ_HALFWORD);
break;
default:
- val |= S3C64XX_SPI_MODE_BUS_TSZ_BYTE;
- val |= S3C64XX_SPI_MODE_CH_TSZ_BYTE;
+ val |= FIELD_PREP(S3C64XX_SPI_MODE_BUS_TSZ_MASK,
+ S3C64XX_SPI_MODE_BUS_TSZ_BYTE) |
+ FIELD_PREP(S3C64XX_SPI_MODE_CH_TSZ_MASK,
+ S3C64XX_SPI_MODE_CH_TSZ_BYTE);
break;
}
@@ -800,7 +803,7 @@ static int s3c64xx_spi_transfer_one(struct spi_controller *host,
val = readl(sdd->regs + S3C64XX_SPI_MODE_CFG);
val &= ~S3C64XX_SPI_MODE_RX_RDY_LVL;
- val |= (rdy_lv << S3C64XX_SPI_MODE_RX_RDY_LVL_SHIFT);
+ val |= FIELD_PREP(S3C64XX_SPI_MODE_RX_RDY_LVL, rdy_lv);
writel(val, sdd->regs + S3C64XX_SPI_MODE_CFG);
/* Enable FIFO_RDY_EN IRQ */
@@ -1073,8 +1076,8 @@ static void s3c64xx_spi_hwinit(struct s3c64xx_spi_driver_data *sdd)
writel(0, regs + S3C64XX_SPI_INT_EN);
if (!sdd->port_conf->clk_from_cmu)
- writel(sci->src_clk_nr << S3C64XX_SPI_CLKSEL_SRCSHFT,
- regs + S3C64XX_SPI_CLK_CFG);
+ writel(FIELD_PREP(S3C64XX_SPI_CLKSEL_SRCMSK, sci->src_clk_nr),
+ regs + S3C64XX_SPI_CLK_CFG);
writel(0, regs + S3C64XX_SPI_MODE_CFG);
writel(0, regs + S3C64XX_SPI_PACKET_CNT);
@@ -1090,8 +1093,7 @@ static void s3c64xx_spi_hwinit(struct s3c64xx_spi_driver_data *sdd)
val = readl(regs + S3C64XX_SPI_MODE_CFG);
val &= ~S3C64XX_SPI_MODE_4BURST;
- val &= ~(S3C64XX_SPI_MAX_TRAILCNT << S3C64XX_SPI_TRAILCNT_OFF);
- val |= (S3C64XX_SPI_TRAILCNT << S3C64XX_SPI_TRAILCNT_OFF);
+ val |= S3C64XX_SPI_MAX_TRAILCNT_MASK;
writel(val, regs + S3C64XX_SPI_MODE_CFG);
s3c64xx_flush_fifo(sdd);
--
2.43.0.429.g432eaa2c6b-goog
Hi Tudor,
On Tue, Jan 23, 2024 at 03:34:06PM +0000, Tudor Ambarus wrote:
> Use the bitfield access macros in order to clean and to make the driver
> easier to read.
most of the changes done here are allignment. I would mention it
in the log.
> Signed-off-by: Tudor Ambarus <[email protected]>
> ---
..
> -#define S3C64XX_SPI_CLKSEL_SRCMSK (3<<9)
> -#define S3C64XX_SPI_CLKSEL_SRCSHFT 9
> -#define S3C64XX_SPI_ENCLK_ENABLE (1<<8)
> -#define S3C64XX_SPI_PSR_MASK 0xff
> -
> -#define S3C64XX_SPI_MODE_CH_TSZ_BYTE (0<<29)
> -#define S3C64XX_SPI_MODE_CH_TSZ_HALFWORD (1<<29)
> -#define S3C64XX_SPI_MODE_CH_TSZ_WORD (2<<29)
> -#define S3C64XX_SPI_MODE_CH_TSZ_MASK (3<<29)
> -#define S3C64XX_SPI_MODE_BUS_TSZ_BYTE (0<<17)
> -#define S3C64XX_SPI_MODE_BUS_TSZ_HALFWORD (1<<17)
> -#define S3C64XX_SPI_MODE_BUS_TSZ_WORD (2<<17)
> -#define S3C64XX_SPI_MODE_BUS_TSZ_MASK (3<<17)
> +#define S3C64XX_SPI_CH_CFG 0x00
> +#define S3C64XX_SPI_CLK_CFG 0x04
> +#define S3C64XX_SPI_MODE_CFG 0x08
> +#define S3C64XX_SPI_CS_REG 0x0C
> +#define S3C64XX_SPI_INT_EN 0x10
> +#define S3C64XX_SPI_STATUS 0x14
> +#define S3C64XX_SPI_TX_DATA 0x18
> +#define S3C64XX_SPI_RX_DATA 0x1C
> +#define S3C64XX_SPI_PACKET_CNT 0x20
> +#define S3C64XX_SPI_PENDING_CLR 0x24
> +#define S3C64XX_SPI_SWAP_CFG 0x28
> +#define S3C64XX_SPI_FB_CLK 0x2C
> +
> +#define S3C64XX_SPI_CH_HS_EN BIT(6) /* High Speed Enable */
> +#define S3C64XX_SPI_CH_SW_RST BIT(5)
> +#define S3C64XX_SPI_CH_SLAVE BIT(4)
> +#define S3C64XX_SPI_CPOL_L BIT(3)
> +#define S3C64XX_SPI_CPHA_B BIT(2)
> +#define S3C64XX_SPI_CH_RXCH_ON BIT(1)
> +#define S3C64XX_SPI_CH_TXCH_ON BIT(0)
> +
> +#define S3C64XX_SPI_CLKSEL_SRCMSK GENMASK(10, 9)
> +#define S3C64XX_SPI_ENCLK_ENABLE BIT(8)
> +#define S3C64XX_SPI_PSR_MASK GENMASK(15, 0)
I find it easier as 0xff to be honest, but I'm not going to be
picky.
> +
> +#define S3C64XX_SPI_MODE_CH_TSZ_MASK GENMASK(30, 29)
> +#define S3C64XX_SPI_MODE_CH_TSZ_BYTE 0
> +#define S3C64XX_SPI_MODE_CH_TSZ_HALFWORD 1
> +#define S3C64XX_SPI_MODE_CH_TSZ_WORD 2
I personally find this pattern harder to read. Perhaps you can
already define these as FIELD_PREP here.
> +#define S3C64XX_SPI_MAX_TRAILCNT_MASK GENMASK(28, 19)
> +#define S3C64XX_SPI_MODE_BUS_TSZ_MASK GENMASK(18, 17)
..
> -#define S3C64XX_SPI_FBCLK_MSK (3<<0)
> +#define S3C64XX_SPI_FBCLK_MASK GENMASK(1, 0)
0x3 to me is more understandable than (3<<0) and GENMASK(1, 0).
Bit operation defines should be used when they really simplify
the reading. But when they make it more difficult, then, I don't
see the point.
Overall looks good, though.
Andi