Re: [U-Boot] [PATCH 04/10] spi: sun4i: Access registers and bits via enum offsets

2019-02-12 Thread André Przywara
On 09/02/2019 13:14, 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 
> ---
>  drivers/spi/sun4i_spi.c | 162 +---
>  1 file changed, 120 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/spi/sun4i_spi.c b/drivers/spi/sun4i_spi.c
> index 5446cebe7c..c06028890b 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,49 @@
>  #define SUN4I_SPI_DEFAULT_RATE   100
>  #define SUN4I_SPI_TIMEOUT_US 100
>  
> -/* 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;
> +/* 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;
>  
> @@ -126,10 +146,11 @@ 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(>regs->rxdata);
> + byte = readb(priv->base_addr + variant->regs[SPI_RXD]);

What about making this register address calculation a macro? That would
also allow you to save the variant variable everywhere.

#define SPI_REG(priv, reg) ((priv)->base_addr + \
(priv)->variant->regs[reg])

Cheers,
Andre.


>   if (priv->rx_buf)
>   *priv->rx_buf++ = byte;
>   }
> @@ -137,30 +158,33 @@ static inline void sun4i_spi_drain_fifo(struct 
> sun4i_spi_priv *priv, int len)
>  
>  static inline void sun4i_spi_fill_fifo(struct sun4i_spi_priv *priv, int len)
>  {
> + struct sun4i_spi_variant *variant = priv->variant;
>   u8 byte;
>  
>   while (len--) {
>   byte = priv->tx_buf ? *priv->tx_buf++ : 0;
> - writeb(byte, >regs->txdata);
> + writeb(byte, priv->base_addr + variant->regs[SPI_TXD]);
>   }
>  }
>  
>  static void sun4i_spi_set_cs(struct udevice *bus, u8 cs, bool enable)
>  {
>   struct sun4i_spi_priv *priv = dev_get_priv(bus);
> + struct sun4i_spi_variant *variant = priv->variant;
>   u32 reg;
>  
> - reg = readl(>regs->ctl);
> + reg = readl(priv->base_addr + variant->regs[SPI_TCR]);
>  
> - reg &= ~SUN4I_CTL_CS_MASK;
> - reg |= SUN4I_CTL_CS(cs);
> + reg &= ~variant->bits[SPI_TCR_CS_MASK];
> + reg |= ((cs << variant->bits[SPI_TCR_CS_SEL]) &
> + variant->bits[SPI_TCR_CS_MASK]);
>  
>   if (enable)
> - reg &= ~SUN4I_CTL_CS_LEVEL;
> + reg &= ~variant->bits[SPI_TCR_CS_LEVEL];
>   else
> - reg |= SUN4I_CTL_CS_LEVEL;
> + reg |= variant->bits[SPI_TCR_CS_LEVEL];
>  
> - writel(reg, >regs->ctl);
> + writel(reg, priv->base_addr + variant->regs[SPI_TCR]);
>  }
>  
>  static int sun4i_spi_parse_pins(struct udevice *dev)
> @@ -255,6 +279,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,
>

[U-Boot] [PATCH 04/10] spi: sun4i: Access registers and bits via enum offsets

2019-02-09 Thread Jagan Teki
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 
---
 drivers/spi/sun4i_spi.c | 162 +---
 1 file changed, 120 insertions(+), 42 deletions(-)

diff --git a/drivers/spi/sun4i_spi.c b/drivers/spi/sun4i_spi.c
index 5446cebe7c..c06028890b 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,49 @@
 #define SUN4I_SPI_DEFAULT_RATE 100
 #define SUN4I_SPI_TIMEOUT_US   100
 
-/* 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;
+/* 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;
 
@@ -126,10 +146,11 @@ 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(>regs->rxdata);
+   byte = readb(priv->base_addr + variant->regs[SPI_RXD]);
if (priv->rx_buf)
*priv->rx_buf++ = byte;
}
@@ -137,30 +158,33 @@ static inline void sun4i_spi_drain_fifo(struct 
sun4i_spi_priv *priv, int len)
 
 static inline void sun4i_spi_fill_fifo(struct sun4i_spi_priv *priv, int len)
 {
+   struct sun4i_spi_variant *variant = priv->variant;
u8 byte;
 
while (len--) {
byte = priv->tx_buf ? *priv->tx_buf++ : 0;
-   writeb(byte, >regs->txdata);
+   writeb(byte, priv->base_addr + variant->regs[SPI_TXD]);
}
 }
 
 static void sun4i_spi_set_cs(struct udevice *bus, u8 cs, bool enable)
 {
struct sun4i_spi_priv *priv = dev_get_priv(bus);
+   struct sun4i_spi_variant *variant = priv->variant;
u32 reg;
 
-   reg = readl(>regs->ctl);
+   reg = readl(priv->base_addr + variant->regs[SPI_TCR]);
 
-   reg &= ~SUN4I_CTL_CS_MASK;
-   reg |= SUN4I_CTL_CS(cs);
+   reg &= ~variant->bits[SPI_TCR_CS_MASK];
+   reg |= ((cs << variant->bits[SPI_TCR_CS_SEL]) &
+   variant->bits[SPI_TCR_CS_MASK]);
 
if (enable)
-   reg &= ~SUN4I_CTL_CS_LEVEL;
+   reg &= ~variant->bits[SPI_TCR_CS_LEVEL];
else
-   reg |= SUN4I_CTL_CS_LEVEL;
+   reg |= variant->bits[SPI_TCR_CS_LEVEL];
 
-   writel(reg, >regs->ctl);
+   writel(reg, priv->base_addr + variant->regs[SPI_TCR]);
 }
 
 static int sun4i_spi_parse_pins(struct udevice *dev)
@@ -255,6 +279,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 +298,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;