Re: [U-Boot] [PATCH v2 8/8] spi: sun4i: Driver cleanup

2019-02-15 Thread Jagan Teki 
On Fri, Feb 15, 2019 at 5:30 AM André Przywara  wrote:
>
> On 14/02/2019 08:36, Jagan Teki wrote:
> > - drop unused macros.
> > - use base instead of base_addr, for better code readability
>
> Actually this part is now pretty pointless, since we use it only a few
> times, and base_addr is actually more descriptive than just "base".

base can be easily understood as base address, I would usually prefer
as simple and meaningful as possible.

>
> > - move .probe and .ofdata_to_platdata functions in required
> >   places to add platdata support in future.
>
> I don't get the reason for that move?

As I explained above, if you add platdata in future we need separate
ifdefs, move above U_BOOT_DRIVER macro would keep the common code in
one place with one macro. and in fact it just follow similar to other
spi dm drivers. where probe and of_pladata core calls are below.

Example: drivers/spi/davinci_spi.c

>
> > - use sentinel sun4i_spi_ids.
> >
> > Signed-off-by: Jagan Teki 
> > ---
> >  drivers/spi/sun4i_spi.c | 190 +---
> >  image.map   |   4 +
>
> Please keep this file for you ;-)

Sorry, its 3:00 AM work ;)
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v2 8/8] spi: sun4i: Driver cleanup

2019-02-14 Thread André Przywara 
On 14/02/2019 08:36, Jagan Teki wrote:
> - drop unused macros.
> - use base instead of base_addr, for better code readability

Actually this part is now pretty pointless, since we use it only a few
times, and base_addr is actually more descriptive than just "base".

> - move .probe and .ofdata_to_platdata functions in required
>   places to add platdata support in future.

I don't get the reason for that move?

> - use sentinel sun4i_spi_ids.
> 
> Signed-off-by: Jagan Teki 
> ---
>  drivers/spi/sun4i_spi.c | 190 +---
>  image.map   |   4 +

Please keep this file for you ;-)

Cheers,
Andre

>  2 files changed, 84 insertions(+), 110 deletions(-)
>  create mode 100644 image.map
> 
> diff --git a/drivers/spi/sun4i_spi.c b/drivers/spi/sun4i_spi.c
> index 96d0db5929..36afcf2c73 100644
> --- a/drivers/spi/sun4i_spi.c
> +++ b/drivers/spi/sun4i_spi.c
> @@ -33,57 +33,16 @@
>  
>  #include 
>  
> -#define SUN4I_RXDATA_REG 0x00
> -
> -#define SUN4I_TXDATA_REG 0x04
> -
> -#define SUN4I_CTL_REG0x08
> -#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_XCHBIT(10)
> -#define SUN4I_CTL_CS_MASK0x3000
> -#define SUN4I_CTL_CS(cs) (((cs) << 12) & SUN4I_CTL_CS_MASK)
> -#define SUN4I_CTL_DHBBIT(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_REG0x0c
> -#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_REG0x10
> -
> -#define SUN4I_DMA_CTL_REG0x14
> -
> -#define SUN4I_WAIT_REG   0x18
> -
> -#define SUN4I_CLK_CTL_REG0x1c
> -#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_DRSBIT(12)
> -
> -#define SUN4I_MAX_XFER_SIZE  0xff
> -
> -#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_REG0x08
> +#define SUN4I_CLK_CTL_REG0x1c
> +#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_REG0x04
> @@ -97,12 +56,25 @@
>  #define SUN6I_TXDATA_REG 0x200
>  #define SUN6I_RXDATA_REG 0x300
>  
> -#define SUN4I_SPI_MAX_RATE   2400
> -#define SUN4I_SPI_MIN_RATE   3000
> -#define SUN4I_SPI_DEFAULT_RATE   100
> -#define SUN4I_SPI_TIMEOUT_US 100
> +/* 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_DRSBIT(12)
> +#define SUN4I_MAX_XFER_SIZE  0xff
> +#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   2400
> +#define SUN4I_SPI_MIN_RATE   3000
> +#define SUN4I_SPI_DEFAULT_RATE   100
> +#define SUN4I_SPI_TIMEOUT_US 100
>  
> -#define SPI_REG(priv, reg)   ((priv)->base_addr + \
> +#define SPI_REG(priv, reg)   ((priv)->base + \
>   (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)) & \
> @@ -146,7 +118,7 @@ struct

[U-Boot] [PATCH v2 8/8] spi: sun4i: Driver cleanup

2019-02-14 Thread Jagan Teki 
- 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 
---
 drivers/spi/sun4i_spi.c | 190 +---
 image.map   |   4 +
 2 files changed, 84 insertions(+), 110 deletions(-)
 create mode 100644 image.map

diff --git a/drivers/spi/sun4i_spi.c b/drivers/spi/sun4i_spi.c
index 96d0db5929..36afcf2c73 100644
--- a/drivers/spi/sun4i_spi.c
+++ b/drivers/spi/sun4i_spi.c
@@ -33,57 +33,16 @@
 
 #include 
 
-#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_LOWBIT(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_MANUALBIT(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_MASK0xff
-#define SUN4I_CLK_CTL_CDR2(div)((div) & 
SUN4I_CLK_CTL_CDR2_MASK)
-#define SUN4I_CLK_CTL_CDR1_MASK0xf
-#define SUN4I_CLK_CTL_CDR1(div)(((div) & 
SUN4I_CLK_CTL_CDR1_MASK) << 8)
-#define SUN4I_CLK_CTL_DRS  BIT(12)
-
-#define SUN4I_MAX_XFER_SIZE0xff
-
-#define SUN4I_BURST_CNT_REG0x20
-#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_REG0x20
+#define SUN4I_XMIT_CNT_REG 0x24
+#define SUN4I_FIFO_STA_REG 0x28
 
 /* sun6i spi registers */
 #define SUN6I_GBL_CTL_REG  0x04
@@ -97,12 +56,25 @@
 #define SUN6I_TXDATA_REG   0x200
 #define SUN6I_RXDATA_REG   0x300
 
-#define SUN4I_SPI_MAX_RATE 2400
-#define SUN4I_SPI_MIN_RATE 3000
-#define SUN4I_SPI_DEFAULT_RATE 100
-#define SUN4I_SPI_TIMEOUT_US   100
+/* sun spi bits */
+#define SUN4I_CTL_ENABLE   BIT(0)
+#define SUN4I_CTL_MASTER   BIT(1)
+#define SUN4I_CLK_CTL_CDR2_MASK0xff
+#define SUN4I_CLK_CTL_CDR2(div)((div) & 
SUN4I_CLK_CTL_CDR2_MASK)
+#define SUN4I_CLK_CTL_CDR1_MASK0xf
+#define SUN4I_CLK_CTL_CDR1(div)(((div) & 
SUN4I_CLK_CTL_CDR1_MASK) << 8)
+#define SUN4I_CLK_CTL_DRS  BIT(12)
+#define SUN4I_MAX_XFER_SIZE0xff
+#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 2400
+#define SUN4I_SPI_MIN_RATE 3000
+#define SUN4I_SPI_DEFAULT_RATE 100
+#define SUN4I_SPI_TIMEOUT_US   100
 
-#define SPI_REG(priv, reg) ((priv)->base_addr + \
+#define SPI_REG(priv, reg) ((priv)->base + \
(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)) & \
@@ -146,7 +118,7 @@ struct sun4i_spi_variant {
 
 struct sun4i_spi_platdata {
struct sun4i_spi_variant *variant;
-   u32 base_addr;
+   u32 base;
u32 max_hz;
 };
 
@@ -154,7 +126,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;
 
@@ -162,8 +134,6 @@ struct sun4i_spi_priv {
u8