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

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@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) = {