Re: [U-Boot] [PATCH v2 03/16] drivers: spi: ti_qspi: prepare driver for DM conversion

2015-11-18 Thread Mugunthan V N
Jagan

On Tuesday 17 November 2015 11:51 AM, Jagan Teki wrote:
>> +   void *ctrl_mod_mmap;
> Looks like this patch manages to prepare for non-dm addition by using
> dm functions is it? and other than that some new things got added like
> RD_DUAL or ctrl_mod_mmap, please add them separately add do the dm
> conversion only on existing code.
> 

ctrl_mod_map is used in non DM mode as well. Earlier there was a define,
I changed it to a variable while preparing driver for DM conversion.

Regards
Mugunthan V N
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 03/16] drivers: spi: ti_qspi: prepare driver for DM conversion

2015-11-16 Thread Jagan Teki
On 4 November 2015 at 13:46, Mugunthan V N  wrote:
> Prepare driver for DM conversion.
>
> Signed-off-by: Mugunthan V N 
> ---
>  drivers/spi/ti_qspi.c | 287 
> --
>  1 file changed, 161 insertions(+), 126 deletions(-)
>
> diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c
> index 44c5762..003df80 100644
> --- a/drivers/spi/ti_qspi.c
> +++ b/drivers/spi/ti_qspi.c
> @@ -28,6 +28,7 @@
>  #define QSPI_3_PIN  BIT(18)
>  #define QSPI_RD_SNGLBIT(16)
>  #define QSPI_WR_SNGL(2 << 16)
> +#define QSPI_RD_DUAL(7 << 16)
>  #define QSPI_INVAL  (4 << 16)
>  #define QSPI_RD_QUAD(7 << 16)
>  /* device control */
> @@ -89,46 +90,16 @@ struct ti_qspi_regs {
>  struct ti_qspi_priv {
> struct spi_slave slave;
> struct ti_qspi_regs *base;
> +   void *ctrl_mod_mmap;

Looks like this patch manages to prepare for non-dm addition by using
dm functions is it? and other than that some new things got added like
RD_DUAL or ctrl_mod_mmap, please add them separately add do the dm
conversion only on existing code.

> unsigned int mode;
> u32 cmd;
> u32 dc;
> +   u32 num_cs;
> +   u32 rx_bus_width;
>  };
>
> -static inline struct ti_qspi_priv *to_ti_qspi_priv(struct spi_slave *slave)
> -{
> -   return container_of(slave, struct ti_qspi_priv, slave);
> -}
> -
> -static void ti_spi_setup_spi_register(struct ti_qspi_priv *priv)
> -{
> -   struct spi_slave *slave = >slave;
> -   u32 memval = 0;
> -
> -#if defined(CONFIG_DRA7XX) || defined(CONFIG_AM57XX)
> -   slave->memory_map = (void *)MMAP_START_ADDR_DRA;
> -#else
> -   slave->memory_map = (void *)MMAP_START_ADDR_AM43x;
> -#endif
> -
> -#ifdef CONFIG_QSPI_QUAD_SUPPORT
> -   memval |= (QSPI_CMD_READ_QUAD | QSPI_SETUP0_NUM_A_BYTES |
> -   QSPI_SETUP0_NUM_D_BYTES_8_BITS |
> -   QSPI_SETUP0_READ_QUAD | QSPI_CMD_WRITE |
> -   QSPI_NUM_DUMMY_BITS);
> -   slave->op_mode_rx = SPI_OPM_RX_QOF;
> -#else
> -   memval |= QSPI_CMD_READ | QSPI_SETUP0_NUM_A_BYTES |
> -   QSPI_SETUP0_NUM_D_BYTES_NO_BITS |
> -   QSPI_SETUP0_READ_NORMAL | QSPI_CMD_WRITE |
> -   QSPI_NUM_DUMMY_BITS;
> -#endif
> -
> -   writel(memval, >base->setup0);
> -}
> -
> -static void ti_spi_set_speed(struct spi_slave *slave, uint hz)
> +static void ti_spi_set_speed(struct ti_qspi_priv *priv, uint hz)
>  {
> -   struct ti_qspi_priv *priv = to_ti_qspi_priv(slave);
> uint clk_div;
>
> debug("ti_spi_set_speed: hz: %d, clock divider %d\n", hz, clk_div);
> @@ -152,130 +123,80 @@ static void ti_spi_set_speed(struct spi_slave *slave, 
> uint hz)
> writel(QSPI_CLK_EN | clk_div, >base->clk_ctrl);
>  }
>
> -int spi_cs_is_valid(unsigned int bus, unsigned int cs)
> -{
> -   return 1;
> -}
> -
> -void spi_cs_activate(struct spi_slave *slave)
> +static void ti_qspi_cs_deactivate(struct ti_qspi_priv *priv)
>  {
> -   /* CS handled in xfer */
> -   return;
> -}
> -
> -void spi_cs_deactivate(struct spi_slave *slave)
> -{
> -   struct ti_qspi_priv *priv = to_ti_qspi_priv(slave);
> -
> -   debug("spi_cs_deactivate: 0x%08x\n", (u32)slave);
> -
> writel(priv->cmd | QSPI_INVAL, >base->cmd);
>  }
>
> -void spi_init(void)
> +static int __ti_qspi_set_mode(struct ti_qspi_priv *priv, unsigned int mode)
>  {
> -   /* nothing to do */
> -}
> -
> -struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
> - unsigned int max_hz, unsigned int mode)
> -{
> -   struct ti_qspi_priv *priv;
> -
> -#ifdef CONFIG_AM43XX
> -   gpio_request(CONFIG_QSPI_SEL_GPIO, "qspi_gpio");
> -   gpio_direction_output(CONFIG_QSPI_SEL_GPIO, 1);
> -#endif
> -
> -   priv = spi_alloc_slave(struct ti_qspi_priv, bus, cs);
> -   if (!priv) {
> -   printf("SPI_error: Fail to allocate ti_qspi_priv\n");
> -   return NULL;
> -   }
> -
> -   priv->base = (struct ti_qspi_regs *)QSPI_BASE;
> -   priv->mode = mode;
> -
> -   ti_spi_set_speed(>slave, max_hz);
> -
> -#ifdef CONFIG_TI_SPI_MMAP
> -   ti_spi_setup_spi_register(priv);
> -#endif
> -
> -   return >slave;
> -}
> -
> -void spi_free_slave(struct spi_slave *slave)
> -{
> -   struct ti_qspi_priv *priv = to_ti_qspi_priv(slave);
> -   free(priv);
> +   priv->dc = 0;
> +   if (mode & SPI_CPHA)
> +   priv->dc |= QSPI_CKPHA(0);
> +   if (mode & SPI_CPOL)
> +   priv->dc |= QSPI_CKPOL(0);
> +   if (mode & SPI_CS_HIGH)
> +   priv->dc |= QSPI_CSPOL(0);
> +
> +   if (mode & SPI_RX_QUAD)
> +   priv->rx_bus_width = QSPI_RD_QUAD;
> +   else if (mode & SPI_RX_QUAD)
> +   priv->rx_bus_width 

Re: [U-Boot] [PATCH v2 03/16] drivers: spi: ti_qspi: prepare driver for DM conversion

2015-11-16 Thread Mugunthan V N
On Friday 06 November 2015 05:37 PM, Simon Glass wrote:
> Hi Mugunthan,
> 
> On 4 November 2015 at 01:16, Mugunthan V N  wrote:
>> Prepare driver for DM conversion.
>>
>> Signed-off-by: Mugunthan V N 
>> ---
>>  drivers/spi/ti_qspi.c | 287 
>> --
>>  1 file changed, 161 insertions(+), 126 deletions(-)
> 
> This looks OK except for the lack of a change log and one thing below.
>>
>> diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c
>> index 44c5762..003df80 100644
>> --- a/drivers/spi/ti_qspi.c
>> +++ b/drivers/spi/ti_qspi.c
>> @@ -28,6 +28,7 @@
>>  #define QSPI_3_PIN  BIT(18)
>>  #define QSPI_RD_SNGLBIT(16)
>>  #define QSPI_WR_SNGL(2 << 16)
>> +#define QSPI_RD_DUAL(7 << 16)
>>  #define QSPI_INVAL  (4 << 16)
>>  #define QSPI_RD_QUAD(7 << 16)
>>  /* device control */
>> @@ -89,46 +90,16 @@ struct ti_qspi_regs {
>>  struct ti_qspi_priv {
>> struct spi_slave slave;
> 
> This should not be present when driver model is used. Can you put an
> #ifdef around it and store anything else you need directly in the
> structure, rather than using struct spi_slave?

Yeah, will do it in next version.

> 
>> struct ti_qspi_regs *base;
>> +   void *ctrl_mod_mmap;
>> unsigned int mode;
>> u32 cmd;
>> u32 dc;
>> +   u32 num_cs;
>> +   u32 rx_bus_width;
>>  };
>>

Regards
Mugunthan V N
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 03/16] drivers: spi: ti_qspi: prepare driver for DM conversion

2015-11-16 Thread Mugunthan V N
On Tuesday 17 November 2015 11:51 AM, Jagan Teki wrote:
> On 4 November 2015 at 13:46, Mugunthan V N  wrote:
>> Prepare driver for DM conversion.
>>
>> Signed-off-by: Mugunthan V N 
>> ---
>>  drivers/spi/ti_qspi.c | 287 
>> --
>>  1 file changed, 161 insertions(+), 126 deletions(-)
>>
>> diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c
>> index 44c5762..003df80 100644
>> --- a/drivers/spi/ti_qspi.c
>> +++ b/drivers/spi/ti_qspi.c
>> @@ -28,6 +28,7 @@
>>  #define QSPI_3_PIN  BIT(18)
>>  #define QSPI_RD_SNGLBIT(16)
>>  #define QSPI_WR_SNGL(2 << 16)
>> +#define QSPI_RD_DUAL(7 << 16)
>>  #define QSPI_INVAL  (4 << 16)
>>  #define QSPI_RD_QUAD(7 << 16)
>>  /* device control */
>> @@ -89,46 +90,16 @@ struct ti_qspi_regs {
>>  struct ti_qspi_priv {
>> struct spi_slave slave;
>> struct ti_qspi_regs *base;
>> +   void *ctrl_mod_mmap;
> 
> Looks like this patch manages to prepare for non-dm addition by using
> dm functions is it? and other than that some new things got added like
> RD_DUAL or ctrl_mod_mmap, please add them separately add do the dm
> conversion only on existing code.
> 

Will move all DM variables under #ifdef.

Regards
Mugunthan V N

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 03/16] drivers: spi: ti_qspi: prepare driver for DM conversion

2015-11-06 Thread Simon Glass
Hi Mugunthan,

On 4 November 2015 at 01:16, Mugunthan V N  wrote:
> Prepare driver for DM conversion.
>
> Signed-off-by: Mugunthan V N 
> ---
>  drivers/spi/ti_qspi.c | 287 
> --
>  1 file changed, 161 insertions(+), 126 deletions(-)

This looks OK except for the lack of a change log and one thing below.
>
> diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c
> index 44c5762..003df80 100644
> --- a/drivers/spi/ti_qspi.c
> +++ b/drivers/spi/ti_qspi.c
> @@ -28,6 +28,7 @@
>  #define QSPI_3_PIN  BIT(18)
>  #define QSPI_RD_SNGLBIT(16)
>  #define QSPI_WR_SNGL(2 << 16)
> +#define QSPI_RD_DUAL(7 << 16)
>  #define QSPI_INVAL  (4 << 16)
>  #define QSPI_RD_QUAD(7 << 16)
>  /* device control */
> @@ -89,46 +90,16 @@ struct ti_qspi_regs {
>  struct ti_qspi_priv {
> struct spi_slave slave;

This should not be present when driver model is used. Can you put an
#ifdef around it and store anything else you need directly in the
structure, rather than using struct spi_slave?

> struct ti_qspi_regs *base;
> +   void *ctrl_mod_mmap;
> unsigned int mode;
> u32 cmd;
> u32 dc;
> +   u32 num_cs;
> +   u32 rx_bus_width;
>  };
>
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH v2 03/16] drivers: spi: ti_qspi: prepare driver for DM conversion

2015-11-04 Thread Mugunthan V N
Prepare driver for DM conversion.

Signed-off-by: Mugunthan V N 
---
 drivers/spi/ti_qspi.c | 287 --
 1 file changed, 161 insertions(+), 126 deletions(-)

diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c
index 44c5762..003df80 100644
--- a/drivers/spi/ti_qspi.c
+++ b/drivers/spi/ti_qspi.c
@@ -28,6 +28,7 @@
 #define QSPI_3_PIN  BIT(18)
 #define QSPI_RD_SNGLBIT(16)
 #define QSPI_WR_SNGL(2 << 16)
+#define QSPI_RD_DUAL(7 << 16)
 #define QSPI_INVAL  (4 << 16)
 #define QSPI_RD_QUAD(7 << 16)
 /* device control */
@@ -89,46 +90,16 @@ struct ti_qspi_regs {
 struct ti_qspi_priv {
struct spi_slave slave;
struct ti_qspi_regs *base;
+   void *ctrl_mod_mmap;
unsigned int mode;
u32 cmd;
u32 dc;
+   u32 num_cs;
+   u32 rx_bus_width;
 };
 
-static inline struct ti_qspi_priv *to_ti_qspi_priv(struct spi_slave *slave)
-{
-   return container_of(slave, struct ti_qspi_priv, slave);
-}
-
-static void ti_spi_setup_spi_register(struct ti_qspi_priv *priv)
-{
-   struct spi_slave *slave = >slave;
-   u32 memval = 0;
-
-#if defined(CONFIG_DRA7XX) || defined(CONFIG_AM57XX)
-   slave->memory_map = (void *)MMAP_START_ADDR_DRA;
-#else
-   slave->memory_map = (void *)MMAP_START_ADDR_AM43x;
-#endif
-
-#ifdef CONFIG_QSPI_QUAD_SUPPORT
-   memval |= (QSPI_CMD_READ_QUAD | QSPI_SETUP0_NUM_A_BYTES |
-   QSPI_SETUP0_NUM_D_BYTES_8_BITS |
-   QSPI_SETUP0_READ_QUAD | QSPI_CMD_WRITE |
-   QSPI_NUM_DUMMY_BITS);
-   slave->op_mode_rx = SPI_OPM_RX_QOF;
-#else
-   memval |= QSPI_CMD_READ | QSPI_SETUP0_NUM_A_BYTES |
-   QSPI_SETUP0_NUM_D_BYTES_NO_BITS |
-   QSPI_SETUP0_READ_NORMAL | QSPI_CMD_WRITE |
-   QSPI_NUM_DUMMY_BITS;
-#endif
-
-   writel(memval, >base->setup0);
-}
-
-static void ti_spi_set_speed(struct spi_slave *slave, uint hz)
+static void ti_spi_set_speed(struct ti_qspi_priv *priv, uint hz)
 {
-   struct ti_qspi_priv *priv = to_ti_qspi_priv(slave);
uint clk_div;
 
debug("ti_spi_set_speed: hz: %d, clock divider %d\n", hz, clk_div);
@@ -152,130 +123,80 @@ static void ti_spi_set_speed(struct spi_slave *slave, 
uint hz)
writel(QSPI_CLK_EN | clk_div, >base->clk_ctrl);
 }
 
-int spi_cs_is_valid(unsigned int bus, unsigned int cs)
-{
-   return 1;
-}
-
-void spi_cs_activate(struct spi_slave *slave)
+static void ti_qspi_cs_deactivate(struct ti_qspi_priv *priv)
 {
-   /* CS handled in xfer */
-   return;
-}
-
-void spi_cs_deactivate(struct spi_slave *slave)
-{
-   struct ti_qspi_priv *priv = to_ti_qspi_priv(slave);
-
-   debug("spi_cs_deactivate: 0x%08x\n", (u32)slave);
-
writel(priv->cmd | QSPI_INVAL, >base->cmd);
 }
 
-void spi_init(void)
+static int __ti_qspi_set_mode(struct ti_qspi_priv *priv, unsigned int mode)
 {
-   /* nothing to do */
-}
-
-struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
- unsigned int max_hz, unsigned int mode)
-{
-   struct ti_qspi_priv *priv;
-
-#ifdef CONFIG_AM43XX
-   gpio_request(CONFIG_QSPI_SEL_GPIO, "qspi_gpio");
-   gpio_direction_output(CONFIG_QSPI_SEL_GPIO, 1);
-#endif
-
-   priv = spi_alloc_slave(struct ti_qspi_priv, bus, cs);
-   if (!priv) {
-   printf("SPI_error: Fail to allocate ti_qspi_priv\n");
-   return NULL;
-   }
-
-   priv->base = (struct ti_qspi_regs *)QSPI_BASE;
-   priv->mode = mode;
-
-   ti_spi_set_speed(>slave, max_hz);
-
-#ifdef CONFIG_TI_SPI_MMAP
-   ti_spi_setup_spi_register(priv);
-#endif
-
-   return >slave;
-}
-
-void spi_free_slave(struct spi_slave *slave)
-{
-   struct ti_qspi_priv *priv = to_ti_qspi_priv(slave);
-   free(priv);
+   priv->dc = 0;
+   if (mode & SPI_CPHA)
+   priv->dc |= QSPI_CKPHA(0);
+   if (mode & SPI_CPOL)
+   priv->dc |= QSPI_CKPOL(0);
+   if (mode & SPI_CS_HIGH)
+   priv->dc |= QSPI_CSPOL(0);
+
+   if (mode & SPI_RX_QUAD)
+   priv->rx_bus_width = QSPI_RD_QUAD;
+   else if (mode & SPI_RX_QUAD)
+   priv->rx_bus_width = QSPI_RD_DUAL;
+   else
+   priv->rx_bus_width = QSPI_RD_SNGL;
+   return 0;
 }
 
-int spi_claim_bus(struct spi_slave *slave)
+static int __ti_qspi_claim_bus(struct ti_qspi_priv *priv, int cs)
 {
-   struct ti_qspi_priv *priv = to_ti_qspi_priv(slave);
-
-   debug("spi_claim_bus: bus:%i cs:%i\n", slave->bus, slave->cs);
-
-   priv->dc = 0;
-   if (priv->mode & SPI_CPHA)
-   priv->dc |= QSPI_CKPHA(slave->cs);
-   if (priv->mode & SPI_CPOL)
-   priv->dc |= QSPI_CKPOL(slave->cs);
-   if (priv->mode & SPI_CS_HIGH)
-