[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