[U-Boot] [PATCH v2 4/8] spi: sun4i: Access registers and bits via enum offsets
André Przywara
andre.przywara at arm.com
Thu Feb 14 23:56:59 UTC 2019
On 14/02/2019 08:36, Jagan Teki wrote:
> Allwinner support two different SPI controllers one for A10 and
> another for A31 with minimal changes in register offsets and
> respective register bits, but the logic for accessing the SPI
> master via SPI slave remains nearly similar.
>
> Add enum offsets for register set and register bits, so-that
> it can access both classes of SPI controllers.
>
> Assign same control register for global, transfer and fifo control
> registers to make the same code compatible with A31 SPI controller.
>
> Signed-off-by: Jagan Teki <jagan at amarulasolutions.com>
> ---
> Note: SPI_REG still seems to have checkpatch warning.
It's a CHECK, and it's fine, as long as you pass only "priv" in, at
least. I think we can leave it this way.
>
> drivers/spi/sun4i_spi.c | 153 +++++++++++++++++++++++++++++-----------
> 1 file changed, 111 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/spi/sun4i_spi.c b/drivers/spi/sun4i_spi.c
> index 0b1663038c..afc351c292 100644
> --- a/drivers/spi/sun4i_spi.c
> +++ b/drivers/spi/sun4i_spi.c
> @@ -83,7 +83,6 @@
> #define SUN4I_XMIT_CNT(cnt) ((cnt) & SUN4I_MAX_XFER_SIZE)
>
> #define SUN4I_FIFO_STA_REG 0x28
> -#define SUN4I_FIFO_STA_RF_CNT_MASK 0x7f
> #define SUN4I_FIFO_STA_RF_CNT_BITS 0
> #define SUN4I_FIFO_STA_TF_CNT_MASK 0x7f
> #define SUN4I_FIFO_STA_TF_CNT_BITS 16
> @@ -93,28 +92,55 @@
> #define SUN4I_SPI_DEFAULT_RATE 1000000
> #define SUN4I_SPI_TIMEOUT_US 1000000
>
> -/* sun4i spi register set */
> -struct sun4i_spi_regs {
> - u32 rxdata;
> - u32 txdata;
> - u32 ctl;
> - u32 intctl;
> - u32 st;
> - u32 dmactl;
> - u32 wait;
> - u32 cctl;
> - u32 bc;
> - u32 tc;
> - u32 fifo_sta;
> +#define SPI_REG(priv, reg) ((priv)->base_addr + \
> + (priv)->variant->regs[reg])
> +#define SPI_BIT(priv, bit) ((priv)->variant->bits[bit])
> +#define SPI_CS(cs, priv) (((cs) << SPI_BIT(priv, SPI_TCR_CS_SEL)) & \
> + SPI_BIT(priv, SPI_TCR_CS_MASK))
Any chance you can swap the order of the parameters for SPI_CS? It's
quite error prone to have it different from the other two macros ....
> +
> +/* sun spi register set */
> +enum sun4i_spi_regs {
> + SPI_GCR,
> + SPI_TCR,
> + SPI_FCR,
> + SPI_FSR,
> + SPI_CCR,
> + SPI_BC,
> + SPI_TC,
> + SPI_BCTL,
> + SPI_TXD,
> + SPI_RXD,
> +};
> +
> +/* sun spi register bits */
> +enum sun4i_spi_bits {
> + SPI_GCR_TP,
> + SPI_TCR_CPHA,
> + SPI_TCR_CPOL,
> + SPI_TCR_CS_ACTIVE_LOW,
> + SPI_TCR_CS_SEL,
> + SPI_TCR_CS_MASK,
> + SPI_TCR_XCH,
> + SPI_TCR_CS_MANUAL,
> + SPI_TCR_CS_LEVEL,
> + SPI_FCR_TF_RST,
> + SPI_FCR_RF_RST,
> + SPI_FSR_RF_CNT_MASK,
> +};
> +
> +struct sun4i_spi_variant {
> + const unsigned long *regs, *bits;
> };
>
> struct sun4i_spi_platdata {
> + struct sun4i_spi_variant *variant;
> u32 base_addr;
> u32 max_hz;
> };
>
> struct sun4i_spi_priv {
> - struct sun4i_spi_regs *regs;
> + struct sun4i_spi_variant *variant;
> + u32 base_addr;
> u32 freq;
> u32 mode;
>
> @@ -129,7 +155,7 @@ static inline void sun4i_spi_drain_fifo(struct sun4i_spi_priv *priv, int len)
> u8 byte;
>
> while (len--) {
> - byte = readb(&priv->regs->rxdata);
> + byte = readb(SPI_REG(priv, SPI_RXD));
> if (priv->rx_buf)
> *priv->rx_buf++ = byte;
> }
> @@ -141,7 +167,7 @@ static inline void sun4i_spi_fill_fifo(struct sun4i_spi_priv *priv, int len)
>
> while (len--) {
> byte = priv->tx_buf ? *priv->tx_buf++ : 0;
> - writeb(byte, &priv->regs->txdata);
> + writeb(byte, SPI_REG(priv, SPI_TXD));
> }
> }
>
> @@ -150,17 +176,17 @@ static void sun4i_spi_set_cs(struct udevice *bus, u8 cs, bool enable)
> struct sun4i_spi_priv *priv = dev_get_priv(bus);
> u32 reg;
>
> - reg = readl(&priv->regs->ctl);
> + reg = readl(SPI_REG(priv, SPI_TCR));
>
> - reg &= ~SUN4I_CTL_CS_MASK;
> - reg |= SUN4I_CTL_CS(cs);
> + reg &= ~SPI_BIT(priv, SPI_TCR_CS_MASK);
> + reg |= SPI_CS(cs, priv);
>
> if (enable)
> - reg &= ~SUN4I_CTL_CS_LEVEL;
> + reg &= ~SPI_BIT(priv, SPI_TCR_CS_LEVEL);
> else
> - reg |= SUN4I_CTL_CS_LEVEL;
> + reg |= SPI_BIT(priv, SPI_TCR_CS_LEVEL);
>
> - writel(reg, &priv->regs->ctl);
> + writel(reg, SPI_REG(priv, SPI_TCR));
> }
>
> static int sun4i_spi_parse_pins(struct udevice *dev)
> @@ -255,6 +281,7 @@ static int sun4i_spi_ofdata_to_platdata(struct udevice *bus)
> int node = dev_of_offset(bus);
>
> plat->base_addr = devfdt_get_addr(bus);
> + plat->variant = (struct sun4i_spi_variant *)dev_get_driver_data(bus);
> plat->max_hz = fdtdec_get_int(gd->fdt_blob, node,
> "spi-max-frequency",
> SUN4I_SPI_DEFAULT_RATE);
> @@ -273,7 +300,8 @@ static int sun4i_spi_probe(struct udevice *bus)
> sun4i_spi_enable_clock();
> sun4i_spi_parse_pins(bus);
>
> - priv->regs = (struct sun4i_spi_regs *)(uintptr_t)plat->base_addr;
> + priv->variant = plat->variant;
> + priv->base_addr = plat->base_addr;
> priv->freq = plat->max_hz;
>
> return 0;
> @@ -283,9 +311,11 @@ static int sun4i_spi_claim_bus(struct udevice *dev)
> {
> struct sun4i_spi_priv *priv = dev_get_priv(dev->parent);
>
> - setbits_le32(&priv->regs->ctl, SUN4I_CTL_ENABLE |
> - SUN4I_CTL_MASTER | SUN4I_CTL_TP |
> - SUN4I_CTL_CS_MANUAL | SUN4I_CTL_CS_ACTIVE_LOW);
> + setbits_le32(SPI_REG(priv, SPI_GCR), SUN4I_CTL_ENABLE |
> + SUN4I_CTL_MASTER | SPI_BIT(priv, SPI_GCR_TP));
> +
> + setbits_le32(SPI_REG(priv, SPI_TCR), SPI_BIT(priv, SPI_TCR_CS_MANUAL) |
> + SPI_BIT(priv, SPI_TCR_CS_ACTIVE_LOW));
>
> return 0;
> }
> @@ -294,7 +324,7 @@ static int sun4i_spi_release_bus(struct udevice *dev)
> {
> struct sun4i_spi_priv *priv = dev_get_priv(dev->parent);
>
> - clrbits_le32(&priv->regs->ctl, SUN4I_CTL_ENABLE);
> + clrbits_le32(SPI_REG(priv, SPI_GCR), SUN4I_CTL_ENABLE);
>
> return 0;
> }
> @@ -323,25 +353,29 @@ static int sun4i_spi_xfer(struct udevice *dev, unsigned int bitlen,
> sun4i_spi_set_cs(bus, slave_plat->cs, true);
>
> /* Reset FIFOs */
> - setbits_le32(&priv->regs->ctl, SUN4I_CTL_RF_RST | SUN4I_CTL_TF_RST);
> + setbits_le32(SPI_REG(priv, SPI_FCR), SPI_BIT(priv, SPI_FCR_RF_RST) |
> + SPI_BIT(priv, SPI_FCR_TF_RST));
>
> while (len) {
> /* Setup the transfer now... */
> nbytes = min(len, (u32)(SUN4I_FIFO_DEPTH - 1));
>
> /* Setup the counters */
> - writel(SUN4I_BURST_CNT(nbytes), &priv->regs->bc);
> - writel(SUN4I_XMIT_CNT(nbytes), &priv->regs->tc);
> + writel(SUN4I_BURST_CNT(nbytes), SPI_REG(priv, SPI_BC));
> + writel(SUN4I_XMIT_CNT(nbytes), SPI_REG(priv, SPI_TC));
>
> /* Fill the TX FIFO */
> sun4i_spi_fill_fifo(priv, nbytes);
>
> /* Start the transfer */
> - setbits_le32(&priv->regs->ctl, SUN4I_CTL_XCH);
> + setbits_le32(SPI_REG(priv, SPI_TCR),
> + SPI_BIT(priv, SPI_TCR_XCH));
>
> /* Wait till RX FIFO to be empty */
> - ret = readl_poll_timeout(&priv->regs->fifo_sta, rx_fifocnt,
> - (((rx_fifocnt & SUN4I_FIFO_STA_RF_CNT_MASK) >>
> + ret = readl_poll_timeout(SPI_REG(priv, SPI_FSR),
> + rx_fifocnt,
> + (((rx_fifocnt &
> + SPI_BIT(priv, SPI_FSR_RF_CNT_MASK)) >>
> SUN4I_FIFO_STA_RF_CNT_BITS) >= nbytes),
> SUN4I_SPI_TIMEOUT_US);
> if (ret < 0) {
> @@ -390,7 +424,7 @@ static int sun4i_spi_set_speed(struct udevice *dev, uint speed)
> */
>
> div = SUN4I_SPI_MAX_RATE / (2 * speed);
> - reg = readl(&priv->regs->cctl);
> + reg = readl(SPI_REG(priv, SPI_CCR));
>
> if (div <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
> if (div > 0)
> @@ -405,7 +439,7 @@ static int sun4i_spi_set_speed(struct udevice *dev, uint speed)
> }
>
> priv->freq = speed;
> - writel(reg, &priv->regs->cctl);
> + writel(reg, SPI_REG(priv, SPI_CCR));
>
> return 0;
> }
> @@ -415,17 +449,17 @@ static int sun4i_spi_set_mode(struct udevice *dev, uint mode)
> struct sun4i_spi_priv *priv = dev_get_priv(dev);
> u32 reg;
>
> - reg = readl(&priv->regs->ctl);
> - reg &= ~(SUN4I_CTL_CPOL | SUN4I_CTL_CPHA);
> + reg = readl(SPI_REG(priv, SPI_TCR));
> + reg &= ~(SPI_BIT(priv, SPI_TCR_CPOL) | SPI_BIT(priv, SPI_TCR_CPHA));
>
> if (mode & SPI_CPOL)
> - reg |= SUN4I_CTL_CPOL;
> + reg |= SPI_BIT(priv, SPI_TCR_CPOL);
>
> if (mode & SPI_CPHA)
> - reg |= SUN4I_CTL_CPHA;
> + reg |= SPI_BIT(priv, SPI_TCR_CPHA);
>
> priv->mode = mode;
> - writel(reg, &priv->regs->ctl);
> + writel(reg, SPI_REG(priv, SPI_TCR));
>
> return 0;
> }
> @@ -438,8 +472,43 @@ static const struct dm_spi_ops sun4i_spi_ops = {
> .set_mode = sun4i_spi_set_mode,
> };
>
> +static const unsigned long sun4i_spi_regs[] = {
Please, don't use long here, that's totally pointless. You can actually
use uint16_t, since it's just register offsets.
> + [SPI_GCR] = SUN4I_CTL_REG,
> + [SPI_TCR] = SUN4I_CTL_REG,
> + [SPI_FCR] = SUN4I_CTL_REG,
> + [SPI_FSR] = SUN4I_FIFO_STA_REG,
> + [SPI_CCR] = SUN4I_CLK_CTL_REG,
> + [SPI_BC] = SUN4I_BURST_CNT_REG,
> + [SPI_TC] = SUN4I_XMIT_CNT_REG,
> + [SPI_TXD] = SUN4I_TXDATA_REG,
> + [SPI_RXD] = SUN4I_RXDATA_REG,
> +};
> +
> +static const unsigned long sun4i_spi_bits[] = {
Same here, make it uint32_t, since it describes register masks in 32 bit
registers.
> + [SPI_GCR_TP] = BIT(18),
> + [SPI_TCR_CPHA] = BIT(2),
> + [SPI_TCR_CPOL] = BIT(3),
> + [SPI_TCR_CS_ACTIVE_LOW] = BIT(4),
> + [SPI_TCR_XCH] = BIT(10),
> + [SPI_TCR_CS_SEL] = 12,
> + [SPI_TCR_CS_MASK] = 0x3000,
> + [SPI_TCR_CS_MANUAL] = BIT(16),
> + [SPI_TCR_CS_LEVEL] = BIT(17),
> + [SPI_FCR_TF_RST] = BIT(8),
> + [SPI_FCR_RF_RST] = BIT(9),
> + [SPI_FSR_RF_CNT_MASK] = GENMASK(6, 0),
> +};
> +
> +static const struct sun4i_spi_variant sun4i_a10_spi_variant = {
> + .regs = sun4i_spi_regs,
> + .bits = sun4i_spi_bits,
> +};
> +
> static const struct udevice_id sun4i_spi_ids[] = {
> - { .compatible = "allwinner,sun4i-a10-spi" },
> + {
> + .compatible = "allwinner,sun4i-a10-spi",
> + .data = (ulong)&sun4i_a10_spi_variant,
> + },
> { }
> };
>
>
I checked the rest as good as my brain allows me at 11pm, but it's still
quite a change with a lot of bits here and there :-(
Stefan, can you please test that this still works for you on the A20? If
I find some time I can try to hook up some SPI chip to my BananaPi, but
I guess it's easier for you to test.
Cheers,
Andre.
More information about the U-Boot
mailing list