Re: [U-Boot] [RFC PATCH] dm:spi:fsl_qspi add DM support
Hi, Simon
On 1/23/2015 6:29 AM, Simon Glass wrote:
Hi Peng,
On 16 January 2015 at 22:59, Peng Fan wrote:
Hi Simon ,Jagan
This patch is based on git://git.denx.de/u-boot-spi.git master branch,
since some fsl_qspi's new feature is still in this git repo and have
not been merged to mainline.
I saw Simon sent out a new patch that remove the per_child_auto_alloc_size
from the platforms' driver code and move it to spi uclass driver. In
this patch I do not remove it, since this is a RFC version, and Jagan's
spi git repo still has it. I can remove it in formal version if needed.
Please take your time to review and comment this patch.
Thanks.
This patch adds DM support for fsl_qspi driver, the DM part needs
device tree support.
Partial of the original driver code is reused, such as the AHB part,
the LUT initialization and etc. The driver now supports new DM and original
driver by define "CONFIG_DM_SPI". Until device tree is integrated, the
original part can be discarded.
The driver code file is now as following:
"
Common code that needs both by DM or original driver code.
#if defined(CONFIG_DM_SPI)
DM part
#else
Original driver code
#endif
"
In DM part, some reconstruction is done. A fsl_qspi_runcmd is included to
simplify the code, but not the original qspi_op_s. fsl_qspi_get_seqid
is included to get seqid, but not hardcoded in original qspi_op_s.
Not sure where this driver is going - I'm happy to pick it up if you
work things out with Jagan. For now I'll just make a few comments, in
case they are useful.
Thanks for your comments. Since some features in spi repo of Jagan are
not into mainline, if DM support is not based on with the spi repo, more
work from your side will needed to resolve merging conflict. Also i'd
like to do this with the new qspi driver.
Signed-off-by: Peng Fan
---
drivers/spi/fsl_qspi.c | 420 +++--
drivers/spi/fsl_qspi.h | 1 +
2 files changed, 405 insertions(+), 16 deletions(-)
diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c
index 5e0b069..ee151b3 100644
--- a/drivers/spi/fsl_qspi.c
+++ b/drivers/spi/fsl_qspi.c
@@ -13,6 +13,13 @@
#include
#include "fsl_qspi.h"
+#ifdef CONFIG_DM_SPI
Shouldn't need these #ifdefs around header files.
will remove uneccesary `ifdefs`
+#include
+#include
+#include
+#include
+#endif
+
#define RX_BUFFER_SIZE 0x80
#ifdef CONFIG_MX6SX
#define TX_BUFFER_SIZE 0x200
@@ -71,27 +78,41 @@
#define qspi_write32 out_be32
#endif
-static unsigned long spi_bases[] = {
- QSPI0_BASE_ADDR,
-#ifdef CONFIG_MX6SX
- QSPI1_BASE_ADDR,
-#endif
-};
+#ifdef CONFIG_DM_SPI
+DECLARE_GLOBAL_DATA_PTR;
+#define QUADSPI_AHBMAP_BANK_MAXSIZESZ_64M
-static unsigned long amba_bases[] = {
- QSPI0_AMBA_BASE,
-#ifdef CONFIG_MX6SX
- QSPI1_AMBA_BASE,
-#endif
+struct fsl_qspi_platdata {
+ u32 max_hz;
+ u32 reg_base;
+ u32 amba_base;
+ u32 flash_num;
};
struct fsl_qspi {
+ u32 reg_base;
+ u32 amba_base;
Should these be structure pointers?
Yeah. using structure pointer for reg_base is better.
+ size_t cmd_len;
+ u8 cmd_buf[32];
+ size_t data_len;
+ int qspi_is_init;
+ size_t flash_size;
+ u32 bank_memmap_phy[4];
+ int cs;
Do you need cs in here? The per-child platform data has it if you base
it on u-boot-dm/i2c-working (which is what I suggest).
I need `cs` to configure qspi controller each time `sf probe bus:cs`. If
not use `cs` to configure controller, wrong chips will be accessed. I'll
look into i2c-working branch to see how to use it.
+ u32 sf_addr;
+ int flash_num;
+ u8 cur_seqid;
+ u32 freq;
+};
+#else
Will there still be non-driver-model users of this driver? Can we
convert them over?
yeah. The platforms that using this driver does not support DM/DT in
their default configuration file, so i'd like this driver support DM and
no-DM.
+struct fsl_qspi {
struct spi_slave slave;
unsigned long reg_base;
unsigned long amba_base;
u32 sf_addr;
u8 cur_seqid;
};
+#endif
/* QSPI support swapping the flash read/write data
* in hardware for LS102xA, but not for VF610 */
@@ -104,11 +125,6 @@ static inline u32 qspi_endian_xchg(u32 data)
#endif
}
-static inline struct fsl_qspi *to_qspi_spi(struct spi_slave *slave)
-{
- return container_of(slave, struct fsl_qspi, slave);
-}
-
static void qspi_set_lut(struct fsl_qspi *qspi)
{
struct fsl_qspi_regs *regs = (struct fsl_qspi_regs *)qspi->reg_base;
@@ -367,6 +383,377 @@ static void qspi_init_ahb_read(struct fsl_qspi_regs *regs)
}
#endif
+#ifdef CONFIG_DM_SPI
+/* Get the SEQID for the command
Re: [U-Boot] [RFC PATCH] dm:spi:fsl_qspi add DM support
Hi Peng,
On 16 January 2015 at 22:59, Peng Fan wrote:
> Hi Simon ,Jagan
>
> This patch is based on git://git.denx.de/u-boot-spi.git master branch,
> since some fsl_qspi's new feature is still in this git repo and have
> not been merged to mainline.
> I saw Simon sent out a new patch that remove the per_child_auto_alloc_size
> from the platforms' driver code and move it to spi uclass driver. In
> this patch I do not remove it, since this is a RFC version, and Jagan's
> spi git repo still has it. I can remove it in formal version if needed.
> Please take your time to review and comment this patch.
>
> Thanks.
>
> This patch adds DM support for fsl_qspi driver, the DM part needs
> device tree support.
>
> Partial of the original driver code is reused, such as the AHB part,
> the LUT initialization and etc. The driver now supports new DM and original
> driver by define "CONFIG_DM_SPI". Until device tree is integrated, the
> original part can be discarded.
>
> The driver code file is now as following:
> "
>
> Common code that needs both by DM or original driver code.
>
> #if defined(CONFIG_DM_SPI)
>
> DM part
>
> #else
>
> Original driver code
>
> #endif
> "
>
> In DM part, some reconstruction is done. A fsl_qspi_runcmd is included to
> simplify the code, but not the original qspi_op_s. fsl_qspi_get_seqid
> is included to get seqid, but not hardcoded in original qspi_op_s.
Not sure where this driver is going - I'm happy to pick it up if you
work things out with Jagan. For now I'll just make a few comments, in
case they are useful.
>
> Signed-off-by: Peng Fan
> ---
> drivers/spi/fsl_qspi.c | 420
> +++--
> drivers/spi/fsl_qspi.h | 1 +
> 2 files changed, 405 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c
> index 5e0b069..ee151b3 100644
> --- a/drivers/spi/fsl_qspi.c
> +++ b/drivers/spi/fsl_qspi.c
> @@ -13,6 +13,13 @@
> #include
> #include "fsl_qspi.h"
>
> +#ifdef CONFIG_DM_SPI
Shouldn't need these #ifdefs around header files.
> +#include
> +#include
> +#include
> +#include
> +#endif
> +
> #define RX_BUFFER_SIZE 0x80
> #ifdef CONFIG_MX6SX
> #define TX_BUFFER_SIZE 0x200
> @@ -71,27 +78,41 @@
> #define qspi_write32 out_be32
> #endif
>
> -static unsigned long spi_bases[] = {
> - QSPI0_BASE_ADDR,
> -#ifdef CONFIG_MX6SX
> - QSPI1_BASE_ADDR,
> -#endif
> -};
> +#ifdef CONFIG_DM_SPI
> +DECLARE_GLOBAL_DATA_PTR;
> +#define QUADSPI_AHBMAP_BANK_MAXSIZESZ_64M
>
> -static unsigned long amba_bases[] = {
> - QSPI0_AMBA_BASE,
> -#ifdef CONFIG_MX6SX
> - QSPI1_AMBA_BASE,
> -#endif
> +struct fsl_qspi_platdata {
> + u32 max_hz;
> + u32 reg_base;
> + u32 amba_base;
> + u32 flash_num;
> };
>
> struct fsl_qspi {
> + u32 reg_base;
> + u32 amba_base;
Should these be structure pointers?
> + size_t cmd_len;
> + u8 cmd_buf[32];
> + size_t data_len;
> + int qspi_is_init;
> + size_t flash_size;
> + u32 bank_memmap_phy[4];
> + int cs;
Do you need cs in here? The per-child platform data has it if you base
it on u-boot-dm/i2c-working (which is what I suggest).
> + u32 sf_addr;
> + int flash_num;
> + u8 cur_seqid;
> + u32 freq;
> +};
> +#else
Will there still be non-driver-model users of this driver? Can we
convert them over?
> +struct fsl_qspi {
> struct spi_slave slave;
> unsigned long reg_base;
> unsigned long amba_base;
> u32 sf_addr;
> u8 cur_seqid;
> };
> +#endif
>
> /* QSPI support swapping the flash read/write data
> * in hardware for LS102xA, but not for VF610 */
> @@ -104,11 +125,6 @@ static inline u32 qspi_endian_xchg(u32 data)
> #endif
> }
>
> -static inline struct fsl_qspi *to_qspi_spi(struct spi_slave *slave)
> -{
> - return container_of(slave, struct fsl_qspi, slave);
> -}
> -
> static void qspi_set_lut(struct fsl_qspi *qspi)
> {
> struct fsl_qspi_regs *regs = (struct fsl_qspi_regs *)qspi->reg_base;
> @@ -367,6 +383,377 @@ static void qspi_init_ahb_read(struct fsl_qspi_regs
> *regs)
> }
> #endif
>
> +#ifdef CONFIG_DM_SPI
> +/* Get the SEQID for the command */
> +static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd)
s/q/priv/ or something a bit longer than one character.
> +{
> + switch (cmd) {
> + case QSPI_CMD_FAST_READ:
> + case QSPI_CMD_FAST_READ_4B:
> + return SEQID_FAST_READ;
> + case QSPI_CMD_WREN:
> + return SEQID_WREN;
> + case QSPI_CMD_RDSR:
> + return SEQID_RDSR;
> + case QSPI_CMD_SE:
> + return SEQID_SE;
> + case QSPI_CMD_PP:
> + case QSPI_CMD_PP_4B:
> +
Re: [U-Boot] [RFC PATCH] dm:spi:fsl_qspi add DM support
Hi Jagan,
On 1/19/2015 2:47 PM, Jagan Teki wrote:
Hi Peng,
On 17 January 2015 at 11:29, Peng Fan wrote:
Hi Simon ,Jagan
This patch is based on git://git.denx.de/u-boot-spi.git master branch,
since some fsl_qspi's new feature is still in this git repo and have
not been merged to mainline.
I saw Simon sent out a new patch that remove the per_child_auto_alloc_size
from the platforms' driver code and move it to spi uclass driver. In
this patch I do not remove it, since this is a RFC version, and Jagan's
spi git repo still has it. I can remove it in formal version if needed.
Please take your time to review and comment this patch.
Appreciate for your work on adding dm on fsl-qspi.
But, I'm sending v2 RFC for spi-nor stuff where your driver be part of that
instead of drivers/spi - I'm planning to send it in this MW.
ok.
My plan is we review this dm stuff but in anyway if the new spi-nor is been
merged you'r driver needs to move on spi-nor with relevant changes.
Comments?
ok. I can do some work to make the driver match the new spi-nor stuff.
If you have anytime, you can review the dm stuff.
There are small issues about register configuration in this patch, and I
am fixing it in my side. Anyway, I'll wait your v2 patch, and based on
your spi-nor stuff to add the dm stuff for fsl_qspi driver.
This patch adds DM support for fsl_qspi driver, the DM part needs
device tree support.
Partial of the original driver code is reused, such as the AHB part,
the LUT initialization and etc. The driver now supports new DM and original
driver by define "CONFIG_DM_SPI". Until device tree is integrated, the
original part can be discarded.
The driver code file is now as following:
"
Common code that needs both by DM or original driver code.
#if defined(CONFIG_DM_SPI)
DM part
#else
Original driver code
#endif
"
In DM part, some reconstruction is done. A fsl_qspi_runcmd is included to
simplify the code, but not the original qspi_op_s. fsl_qspi_get_seqid
is included to get seqid, but not hardcoded in original qspi_op_s.
Signed-off-by: Peng Fan
---
drivers/spi/fsl_qspi.c | 420 +++--
drivers/spi/fsl_qspi.h | 1 +
2 files changed, 405 insertions(+), 16 deletions(-)
diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c
index 5e0b069..ee151b3 100644
--- a/drivers/spi/fsl_qspi.c
+++ b/drivers/spi/fsl_qspi.c
@@ -13,6 +13,13 @@
#include
#include "fsl_qspi.h"
+#ifdef CONFIG_DM_SPI
+#include
+#include
+#include
+#include
+#endif
+
#define RX_BUFFER_SIZE 0x80
#ifdef CONFIG_MX6SX
#define TX_BUFFER_SIZE 0x200
@@ -71,27 +78,41 @@
#define qspi_write32 out_be32
#endif
-static unsigned long spi_bases[] = {
- QSPI0_BASE_ADDR,
-#ifdef CONFIG_MX6SX
- QSPI1_BASE_ADDR,
-#endif
-};
+#ifdef CONFIG_DM_SPI
+DECLARE_GLOBAL_DATA_PTR;
+#define QUADSPI_AHBMAP_BANK_MAXSIZESZ_64M
-static unsigned long amba_bases[] = {
- QSPI0_AMBA_BASE,
-#ifdef CONFIG_MX6SX
- QSPI1_AMBA_BASE,
-#endif
+struct fsl_qspi_platdata {
+ u32 max_hz;
+ u32 reg_base;
+ u32 amba_base;
+ u32 flash_num;
};
struct fsl_qspi {
+ u32 reg_base;
+ u32 amba_base;
+ size_t cmd_len;
+ u8 cmd_buf[32];
+ size_t data_len;
+ int qspi_is_init;
+ size_t flash_size;
+ u32 bank_memmap_phy[4];
+ int cs;
+ u32 sf_addr;
+ int flash_num;
+ u8 cur_seqid;
+ u32 freq;
+};
+#else
+struct fsl_qspi {
struct spi_slave slave;
unsigned long reg_base;
unsigned long amba_base;
u32 sf_addr;
u8 cur_seqid;
};
+#endif
/* QSPI support swapping the flash read/write data
* in hardware for LS102xA, but not for VF610 */
@@ -104,11 +125,6 @@ static inline u32 qspi_endian_xchg(u32 data)
#endif
}
-static inline struct fsl_qspi *to_qspi_spi(struct spi_slave *slave)
-{
- return container_of(slave, struct fsl_qspi, slave);
-}
-
static void qspi_set_lut(struct fsl_qspi *qspi)
{
struct fsl_qspi_regs *regs = (struct fsl_qspi_regs *)qspi->reg_base;
@@ -367,6 +383,377 @@ static void qspi_init_ahb_read(struct fsl_qspi_regs *regs)
}
#endif
+#ifdef CONFIG_DM_SPI
+/* Get the SEQID for the command */
+static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd)
+{
+ switch (cmd) {
+ case QSPI_CMD_FAST_READ:
+ case QSPI_CMD_FAST_READ_4B:
+ return SEQID_FAST_READ;
+ case QSPI_CMD_WREN:
+ return SEQID_WREN;
+ case QSPI_CMD_RDSR:
+ return SEQID_RDSR;
+ case QSPI_CMD_SE:
+ return SEQID_SE;
+ case QSPI_CMD_PP:
+ case QSPI_CMD_PP_4B:
+ return SEQID_PP;
+ case QSPI_CMD_RDID:
+
Re: [U-Boot] [RFC PATCH] dm:spi:fsl_qspi add DM support
Hi Peng,
On 17 January 2015 at 11:29, Peng Fan wrote:
> Hi Simon ,Jagan
>
> This patch is based on git://git.denx.de/u-boot-spi.git master branch,
> since some fsl_qspi's new feature is still in this git repo and have
> not been merged to mainline.
> I saw Simon sent out a new patch that remove the per_child_auto_alloc_size
> from the platforms' driver code and move it to spi uclass driver. In
> this patch I do not remove it, since this is a RFC version, and Jagan's
> spi git repo still has it. I can remove it in formal version if needed.
> Please take your time to review and comment this patch.
Appreciate for your work on adding dm on fsl-qspi.
But, I'm sending v2 RFC for spi-nor stuff where your driver be part of that
instead of drivers/spi - I'm planning to send it in this MW.
My plan is we review this dm stuff but in anyway if the new spi-nor is been
merged you'r driver needs to move on spi-nor with relevant changes.
Comments?
> This patch adds DM support for fsl_qspi driver, the DM part needs
> device tree support.
>
> Partial of the original driver code is reused, such as the AHB part,
> the LUT initialization and etc. The driver now supports new DM and original
> driver by define "CONFIG_DM_SPI". Until device tree is integrated, the
> original part can be discarded.
>
> The driver code file is now as following:
> "
>
> Common code that needs both by DM or original driver code.
>
> #if defined(CONFIG_DM_SPI)
>
> DM part
>
> #else
>
> Original driver code
>
> #endif
> "
>
> In DM part, some reconstruction is done. A fsl_qspi_runcmd is included to
> simplify the code, but not the original qspi_op_s. fsl_qspi_get_seqid
> is included to get seqid, but not hardcoded in original qspi_op_s.
>
> Signed-off-by: Peng Fan
> ---
> drivers/spi/fsl_qspi.c | 420
> +++--
> drivers/spi/fsl_qspi.h | 1 +
> 2 files changed, 405 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c
> index 5e0b069..ee151b3 100644
> --- a/drivers/spi/fsl_qspi.c
> +++ b/drivers/spi/fsl_qspi.c
> @@ -13,6 +13,13 @@
> #include
> #include "fsl_qspi.h"
>
> +#ifdef CONFIG_DM_SPI
> +#include
> +#include
> +#include
> +#include
> +#endif
> +
> #define RX_BUFFER_SIZE 0x80
> #ifdef CONFIG_MX6SX
> #define TX_BUFFER_SIZE 0x200
> @@ -71,27 +78,41 @@
> #define qspi_write32 out_be32
> #endif
>
> -static unsigned long spi_bases[] = {
> - QSPI0_BASE_ADDR,
> -#ifdef CONFIG_MX6SX
> - QSPI1_BASE_ADDR,
> -#endif
> -};
> +#ifdef CONFIG_DM_SPI
> +DECLARE_GLOBAL_DATA_PTR;
> +#define QUADSPI_AHBMAP_BANK_MAXSIZESZ_64M
>
> -static unsigned long amba_bases[] = {
> - QSPI0_AMBA_BASE,
> -#ifdef CONFIG_MX6SX
> - QSPI1_AMBA_BASE,
> -#endif
> +struct fsl_qspi_platdata {
> + u32 max_hz;
> + u32 reg_base;
> + u32 amba_base;
> + u32 flash_num;
> };
>
> struct fsl_qspi {
> + u32 reg_base;
> + u32 amba_base;
> + size_t cmd_len;
> + u8 cmd_buf[32];
> + size_t data_len;
> + int qspi_is_init;
> + size_t flash_size;
> + u32 bank_memmap_phy[4];
> + int cs;
> + u32 sf_addr;
> + int flash_num;
> + u8 cur_seqid;
> + u32 freq;
> +};
> +#else
> +struct fsl_qspi {
> struct spi_slave slave;
> unsigned long reg_base;
> unsigned long amba_base;
> u32 sf_addr;
> u8 cur_seqid;
> };
> +#endif
>
> /* QSPI support swapping the flash read/write data
> * in hardware for LS102xA, but not for VF610 */
> @@ -104,11 +125,6 @@ static inline u32 qspi_endian_xchg(u32 data)
> #endif
> }
>
> -static inline struct fsl_qspi *to_qspi_spi(struct spi_slave *slave)
> -{
> - return container_of(slave, struct fsl_qspi, slave);
> -}
> -
> static void qspi_set_lut(struct fsl_qspi *qspi)
> {
> struct fsl_qspi_regs *regs = (struct fsl_qspi_regs *)qspi->reg_base;
> @@ -367,6 +383,377 @@ static void qspi_init_ahb_read(struct fsl_qspi_regs
> *regs)
> }
> #endif
>
> +#ifdef CONFIG_DM_SPI
> +/* Get the SEQID for the command */
> +static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd)
> +{
> + switch (cmd) {
> + case QSPI_CMD_FAST_READ:
> + case QSPI_CMD_FAST_READ_4B:
> + return SEQID_FAST_READ;
> + case QSPI_CMD_WREN:
> + return SEQID_WREN;
> + case QSPI_CMD_RDSR:
> + return SEQID_RDSR;
> + case QSPI_CMD_SE:
> + return SEQID_SE;
> + case QSPI_CMD_PP:
> + case QSPI_CMD_PP_4B:
> + return SEQID_PP;
> + case QSPI_CMD_RDID:
> + return SEQID_RDID;
> + case QSPI_CMD_BE_4K:
> + return SEQID_BE_4K;
> +#ifdef CONFIG_SPI_FLASH_BAR
> +
