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 <ja...@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. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot