[U-Boot] [PATCH 08/10] spi: sun4i: Driver cleanup

André Przywara andre.przywara at arm.com
Wed Feb 13 01:20:59 UTC 2019


On 09/02/2019 13:14, Jagan Teki wrote:
> - drop unused macros.
> - use base instead of base_addr, for better code readability
> - move .probe and .ofdata_to_platdata functions in required
>   places to add platdata support in future.
> - use sentinel sun4i_spi_ids.
> 
> Signed-off-by: Jagan Teki <jagan at amarulasolutions.com>
> ---
>  drivers/spi/sun4i_spi.c | 224 +++++++++++++++++-----------------------
>  1 file changed, 97 insertions(+), 127 deletions(-)
> 
> diff --git a/drivers/spi/sun4i_spi.c b/drivers/spi/sun4i_spi.c
> index 36f2215f7d..a32f511245 100644
> --- a/drivers/spi/sun4i_spi.c
> +++ b/drivers/spi/sun4i_spi.c
> @@ -33,57 +33,16 @@
>  
>  #include <linux/iopoll.h>
>  
> -#define SUN4I_RXDATA_REG	0x00
> -
> -#define SUN4I_TXDATA_REG	0x04
> -
> -#define SUN4I_CTL_REG		0x08
> -#define SUN4I_CTL_ENABLE		BIT(0)
> -#define SUN4I_CTL_MASTER		BIT(1)
> -#define SUN4I_CTL_CPHA			BIT(2)
> -#define SUN4I_CTL_CPOL			BIT(3)
> -#define SUN4I_CTL_CS_ACTIVE_LOW		BIT(4)
> -#define SUN4I_CTL_LMTF			BIT(6)
> -#define SUN4I_CTL_TF_RST		BIT(8)
> -#define SUN4I_CTL_RF_RST		BIT(9)
> -#define SUN4I_CTL_XCH			BIT(10)
> -#define SUN4I_CTL_CS_MASK		0x3000
> -#define SUN4I_CTL_CS(cs)		(((cs) << 12) & SUN4I_CTL_CS_MASK)
> -#define SUN4I_CTL_DHB			BIT(15)
> -#define SUN4I_CTL_CS_MANUAL		BIT(16)
> -#define SUN4I_CTL_CS_LEVEL		BIT(17)
> -#define SUN4I_CTL_TP			BIT(18)
> -
> -#define SUN4I_INT_CTL_REG	0x0c
> -#define SUN4I_INT_CTL_RF_F34		BIT(4)
> -#define SUN4I_INT_CTL_TF_E34		BIT(12)
> -#define SUN4I_INT_CTL_TC		BIT(16)
> -
> -#define SUN4I_INT_STA_REG	0x10
> -
> -#define SUN4I_DMA_CTL_REG	0x14
> -
> -#define SUN4I_WAIT_REG		0x18
> -
> -#define SUN4I_CLK_CTL_REG	0x1c
> -#define SUN4I_CLK_CTL_CDR2_MASK		0xff
> -#define SUN4I_CLK_CTL_CDR2(div)		((div) & SUN4I_CLK_CTL_CDR2_MASK)
> -#define SUN4I_CLK_CTL_CDR1_MASK		0xf
> -#define SUN4I_CLK_CTL_CDR1(div)		(((div) & SUN4I_CLK_CTL_CDR1_MASK) << 8)
> -#define SUN4I_CLK_CTL_DRS		BIT(12)
> -
> -#define SUN4I_MAX_XFER_SIZE		0xffffff
> -
> -#define SUN4I_BURST_CNT_REG	0x20
> -#define SUN4I_BURST_CNT(cnt)		((cnt) & SUN4I_MAX_XFER_SIZE)
> -
> -#define SUN4I_XMIT_CNT_REG	0x24
> -#define SUN4I_XMIT_CNT(cnt)		((cnt) & SUN4I_MAX_XFER_SIZE)
> +DECLARE_GLOBAL_DATA_PTR;
>  
> -#define SUN4I_FIFO_STA_REG	0x28
> -#define SUN4I_FIFO_STA_RF_CNT_BITS	0
> -#define SUN4I_FIFO_STA_TF_CNT_MASK	0x7f
> -#define SUN4I_FIFO_STA_TF_CNT_BITS	16
> +/* sun4i spi registers */
> +#define SUN4I_RXDATA_REG		0x00
> +#define SUN4I_TXDATA_REG		0x04
> +#define SUN4I_CTL_REG			0x08
> +#define SUN4I_CLK_CTL_REG		0x1c
> +#define SUN4I_BURST_CNT_REG		0x20
> +#define SUN4I_XMIT_CNT_REG		0x24
> +#define SUN4I_FIFO_STA_REG		0x28
>  
>  /* sun6i spi registers */
>  #define SUN6I_GBL_CTL_REG		0x04
> @@ -97,10 +56,23 @@
>  #define SUN6I_TXDATA_REG		0x200
>  #define SUN6I_RXDATA_REG		0x300
>  
> -#define SUN4I_SPI_MAX_RATE	24000000
> -#define SUN4I_SPI_MIN_RATE	3000
> -#define SUN4I_SPI_DEFAULT_RATE	1000000
> -#define SUN4I_SPI_TIMEOUT_US	1000000
> +/* sun spi bits */
> +#define SUN4I_CTL_ENABLE		BIT(0)
> +#define SUN4I_CTL_MASTER		BIT(1)
> +#define SUN4I_CLK_CTL_CDR2_MASK		0xff
> +#define SUN4I_CLK_CTL_CDR2(div)		((div) & SUN4I_CLK_CTL_CDR2_MASK)
> +#define SUN4I_CLK_CTL_CDR1_MASK		0xf
> +#define SUN4I_CLK_CTL_CDR1(div)		(((div) & SUN4I_CLK_CTL_CDR1_MASK) << 8)
> +#define SUN4I_CLK_CTL_DRS		BIT(12)
> +#define SUN4I_MAX_XFER_SIZE		0xffffff
> +#define SUN4I_BURST_CNT(cnt)		((cnt) & SUN4I_MAX_XFER_SIZE)
> +#define SUN4I_XMIT_CNT(cnt)		((cnt) & SUN4I_MAX_XFER_SIZE)
> +#define SUN4I_FIFO_STA_RF_CNT_BITS	0
> +
> +#define SUN4I_SPI_MAX_RATE		24000000
> +#define SUN4I_SPI_MIN_RATE		3000
> +#define SUN4I_SPI_DEFAULT_RATE		1000000
> +#define SUN4I_SPI_TIMEOUT_US		1000000
>  
>  /* sun spi register set */
>  enum sun4i_spi_regs {
> @@ -140,7 +112,7 @@ struct sun4i_spi_variant {
>  
>  struct sun4i_spi_platdata {
>  	struct sun4i_spi_variant *variant;
> -	u32 base_addr;
> +	u32 base;
>  	u32 max_hz;
>  };
>  
> @@ -148,7 +120,7 @@ struct sun4i_spi_priv {
>  	struct sun4i_spi_variant *variant;
>  	struct clk clk_ahb, clk_mod;
>  	struct reset_ctl reset;
> -	u32 base_addr;
> +	u32 base;
>  	u32 freq;
>  	u32 mode;
>  
> @@ -156,15 +128,13 @@ struct sun4i_spi_priv {
>  	u8 *rx_buf;
>  };
>  
> -DECLARE_GLOBAL_DATA_PTR;
> -
>  static inline void sun4i_spi_drain_fifo(struct sun4i_spi_priv *priv, int len)
>  {
>  	struct sun4i_spi_variant *variant = priv->variant;
>  	u8 byte;
>  
>  	while (len--) {
> -		byte = readb(priv->base_addr + variant->regs[SPI_RXD]);
> +		byte = readb(priv->base + variant->regs[SPI_RXD]);

Those parts of this patch would go away when hiding the address
calculation in a macro. Or you do the rename already in patch 04/10,
where you would just need to rename it in three existing places.

Cheers,
Andre.


>  		if (priv->rx_buf)
>  			*priv->rx_buf++ = byte;
>  	}
> @@ -177,7 +147,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->base_addr + variant->regs[SPI_TXD]);
> +		writeb(byte, priv->base + variant->regs[SPI_TXD]);
>  	}
>  }
>  
> @@ -187,7 +157,7 @@ static void sun4i_spi_set_cs(struct udevice *bus, u8 cs, bool enable)
>  	struct sun4i_spi_variant *variant = priv->variant;
>  	u32 reg;
>  
> -	reg = readl(priv->base_addr + variant->regs[SPI_TCR]);
> +	reg = readl(priv->base + variant->regs[SPI_TCR]);
>  
>  	reg &= ~variant->bits[SPI_TCR_CS_MASK];
>  	reg |= ((cs << variant->bits[SPI_TCR_CS_SEL]) &
> @@ -198,7 +168,7 @@ static void sun4i_spi_set_cs(struct udevice *bus, u8 cs, bool enable)
>  	else
>  		reg |= variant->bits[SPI_TCR_CS_LEVEL];
>  
> -	writel(reg, priv->base_addr + variant->regs[SPI_TCR]);
> +	writel(reg, priv->base + variant->regs[SPI_TCR]);
>  }
>  
>  static int sun4i_spi_parse_pins(struct udevice *dev)
> @@ -323,56 +293,6 @@ err_ahb:
>  	return ret;
>  }
>  
> -static int sun4i_spi_ofdata_to_platdata(struct udevice *bus)
> -{
> -	struct sun4i_spi_platdata *plat = dev_get_platdata(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);
> -
> -	if (plat->max_hz > SUN4I_SPI_MAX_RATE)
> -		plat->max_hz = SUN4I_SPI_MAX_RATE;
> -
> -	return 0;
> -}
> -
> -static int sun4i_spi_probe(struct udevice *bus)
> -{
> -	struct sun4i_spi_platdata *plat = dev_get_platdata(bus);
> -	struct sun4i_spi_priv *priv = dev_get_priv(bus);
> -	int ret;
> -
> -	ret = clk_get_by_name(bus, "ahb", &priv->clk_ahb);
> -	if (ret) {
> -		dev_err(dev, "failed to get ahb clock\n");
> -		return ret;
> -	}
> -
> -	ret = clk_get_by_name(bus, "mod", &priv->clk_mod);
> -	if (ret) {
> -		dev_err(dev, "failed to get mod clock\n");
> -		return ret;
> -	}
> -
> -	ret = reset_get_by_index(bus, 0, &priv->reset);
> -	if (ret && ret != -ENOENT) {
> -		dev_err(dev, "failed to get reset\n");
> -		return ret;
> -	}
> -
> -	sun4i_spi_parse_pins(bus);
> -
> -	priv->variant = plat->variant;
> -	priv->base_addr = plat->base_addr;
> -	priv->freq = plat->max_hz;
> -
> -	return 0;
> -}
> -
>  static int sun4i_spi_claim_bus(struct udevice *dev)
>  {
>  	struct sun4i_spi_priv *priv = dev_get_priv(dev->parent);
> @@ -383,15 +303,15 @@ static int sun4i_spi_claim_bus(struct udevice *dev)
>  	if (ret)
>  		return ret;
>  
> -	setbits_le32(priv->base_addr + variant->regs[SPI_GCR],
> +	setbits_le32(priv->base + variant->regs[SPI_GCR],
>  		     SUN4I_CTL_ENABLE | SUN4I_CTL_MASTER |
>  		     variant->bits[SPI_GCR_TP]);
>  
>  	if (IS_ENABLED(CONFIG_SUNXI_GEN_SUN6I))
> -		setbits_le32(priv->base_addr + variant->regs[SPI_GCR],
> +		setbits_le32(priv->base + variant->regs[SPI_GCR],
>  			     variant->bits[SPI_GCR_SRST]);
>  
> -	setbits_le32(priv->base_addr + variant->regs[SPI_TCR],
> +	setbits_le32(priv->base + variant->regs[SPI_TCR],
>  		     variant->bits[SPI_TCR_CS_MANUAL] |
>  		     variant->bits[SPI_TCR_CS_ACTIVE_LOW]);
>  
> @@ -403,7 +323,7 @@ static int sun4i_spi_release_bus(struct udevice *dev)
>  	struct sun4i_spi_priv *priv = dev_get_priv(dev->parent);
>  	struct sun4i_spi_variant *variant = priv->variant;
>  
> -	clrbits_le32(priv->base_addr + variant->regs[SPI_GCR],
> +	clrbits_le32(priv->base + variant->regs[SPI_GCR],
>  		     SUN4I_CTL_ENABLE);
>  
>  	sun4i_spi_set_clock(dev->parent, false);
> @@ -436,7 +356,7 @@ 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->base_addr + variant->regs[SPI_FCR],
> +	setbits_le32(priv->base + variant->regs[SPI_FCR],
>  		     variant->bits[SPI_FCR_RF_RST] |
>  		     variant->bits[SPI_FCR_TF_RST]);
>  
> @@ -446,22 +366,22 @@ static int sun4i_spi_xfer(struct udevice *dev, unsigned int bitlen,
>  
>  		/* Setup the counters */
>  		writel(SUN4I_BURST_CNT(nbytes),
> -		       priv->base_addr + variant->regs[SPI_BC]);
> +		       priv->base + variant->regs[SPI_BC]);
>  		writel(SUN4I_XMIT_CNT(nbytes),
> -		       priv->base_addr + variant->regs[SPI_TC]);
> +		       priv->base + variant->regs[SPI_TC]);
>  		if (IS_ENABLED(CONFIG_SUNXI_GEN_SUN6I))
>  			writel(SUN4I_BURST_CNT(nbytes),
> -			       priv->base_addr + variant->regs[SPI_BCTL]);
> +			       priv->base + variant->regs[SPI_BCTL]);
>  
>  		/* Fill the TX FIFO */
>  		sun4i_spi_fill_fifo(priv, nbytes);
>  
>  		/* Start the transfer */
> -		setbits_le32(priv->base_addr + variant->regs[SPI_TCR],
> +		setbits_le32(priv->base + variant->regs[SPI_TCR],
>  			     variant->bits[SPI_TCR_XCH]);
>  
>  		/* Wait till RX FIFO to be empty */
> -		ret = readl_poll_timeout(priv->base_addr + variant->regs[SPI_FSR],
> +		ret = readl_poll_timeout(priv->base + variant->regs[SPI_FSR],
>  					 rx_fifocnt,
>  					 !(((rx_fifocnt &
>  					 variant->bits[SPI_FSR_RF_CNT_MASK]) >>
> @@ -514,7 +434,7 @@ static int sun4i_spi_set_speed(struct udevice *dev, uint speed)
>  	 */
>  
>  	div = SUN4I_SPI_MAX_RATE / (2 * speed);
> -	reg = readl(priv->base_addr + variant->regs[SPI_CCR]);
> +	reg = readl(priv->base + variant->regs[SPI_CCR]);
>  
>  	if (div <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
>  		if (div > 0)
> @@ -529,7 +449,7 @@ static int sun4i_spi_set_speed(struct udevice *dev, uint speed)
>  	}
>  
>  	priv->freq = speed;
> -	writel(reg, priv->base_addr + variant->regs[SPI_CCR]);
> +	writel(reg, priv->base + variant->regs[SPI_CCR]);
>  
>  	return 0;
>  }
> @@ -540,7 +460,7 @@ static int sun4i_spi_set_mode(struct udevice *dev, uint mode)
>  	struct sun4i_spi_variant *variant = priv->variant;
>  	u32 reg;
>  
> -	reg = readl(priv->base_addr + variant->regs[SPI_TCR]);
> +	reg = readl(priv->base + variant->regs[SPI_TCR]);
>  	reg &= ~(variant->bits[SPI_TCR_CPOL] | variant->bits[SPI_TCR_CPHA]);
>  
>  	if (mode & SPI_CPOL)
> @@ -550,7 +470,7 @@ static int sun4i_spi_set_mode(struct udevice *dev, uint mode)
>  		reg |= variant->bits[SPI_TCR_CPHA];
>  
>  	priv->mode = mode;
> -	writel(reg, priv->base_addr + variant->regs[SPI_TCR]);
> +	writel(reg, priv->base + variant->regs[SPI_TCR]);
>  
>  	return 0;
>  }
> @@ -563,6 +483,56 @@ static const struct dm_spi_ops sun4i_spi_ops = {
>  	.set_mode		= sun4i_spi_set_mode,
>  };
>  
> +static int sun4i_spi_probe(struct udevice *bus)
> +{
> +	struct sun4i_spi_platdata *plat = dev_get_platdata(bus);
> +	struct sun4i_spi_priv *priv = dev_get_priv(bus);
> +	int ret;
> +
> +	ret = clk_get_by_name(bus, "ahb", &priv->clk_ahb);
> +	if (ret) {
> +		dev_err(dev, "failed to get ahb clock\n");
> +		return ret;
> +	}
> +
> +	ret = clk_get_by_name(bus, "mod", &priv->clk_mod);
> +	if (ret) {
> +		dev_err(dev, "failed to get mod clock\n");
> +		return ret;
> +	}
> +
> +	ret = reset_get_by_index(bus, 0, &priv->reset);
> +	if (ret && ret != -ENOENT) {
> +		dev_err(dev, "failed to get reset\n");
> +		return ret;
> +	}
> +
> +	sun4i_spi_parse_pins(bus);
> +
> +	priv->variant = plat->variant;
> +	priv->base = plat->base;
> +	priv->freq = plat->max_hz;
> +
> +	return 0;
> +}
> +
> +static int sun4i_spi_ofdata_to_platdata(struct udevice *bus)
> +{
> +	struct sun4i_spi_platdata *plat = dev_get_platdata(bus);
> +	int node = dev_of_offset(bus);
> +
> +	plat->base = 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);
> +
> +	if (plat->max_hz > SUN4I_SPI_MAX_RATE)
> +		plat->max_hz = SUN4I_SPI_MAX_RATE;
> +
> +	return 0;
> +}
> +
>  static const unsigned long sun4i_spi_regs[] = {
>  	[SPI_GCR]		= SUN4I_CTL_REG,
>  	[SPI_TCR]		= SUN4I_CTL_REG,
> @@ -640,7 +610,7 @@ static const struct udevice_id sun4i_spi_ids[] = {
>  	  .compatible = "allwinner,sun8i-h3-spi",
>  	  .data = (ulong)&sun8i_h3_spi_variant,
>  	},
> -	{ }
> +	{ /* sentinel */ }
>  };
>  
>  U_BOOT_DRIVER(sun4i_spi) = {
> 



More information about the U-Boot mailing list