[U-Boot] [PATCH 4/4] sf: Add Quad-input Page Program(32h) instruction support
Simon Glass
sjg at chromium.org
Wed Dec 12 07:41:00 CET 2012
Hi,
On Mon, Dec 10, 2012 at 6:42 AM, Jagannadha Sutradharudu Teki
<jagannadh.teki at gmail.com> wrote:
> This patch provides support to program a flash using
> Quad-input Page Program(32h) instruction.
>
> This will effectively increases the data transfer rate
> by up to four times, as compared to the Page Program(PP) instruction.
>
> Respective flash drivers need to use spi_flash_cmd_write_multi_qpp()
> based on their usage.
>
> Signed-off-by: Jagannadha Sutradharudu Teki <jagannadh.teki at gmail.com>
It's great to have this support. A few comments below.
> ---
> README | 1 +
> common/cmd_sf.c | 12 +++-
> drivers/mtd/spi/spi_flash.c | 108 ++++++++++++++++++++++++++++++++++
> drivers/mtd/spi/spi_flash_internal.h | 13 ++++
> include/spi_flash.h | 12 ++++
> 5 files changed, 144 insertions(+), 2 deletions(-)
>
> diff --git a/README b/README
> index 5a86ae9..a01de13 100644
> --- a/README
> +++ b/README
> @@ -869,6 +869,7 @@ The following options need to be configured:
> CONFIG_CMD_SETGETDCR Support for DCR Register access
> (4xx only)
> CONFIG_CMD_SF * Read/write/erase SPI NOR flash
> + CONFIG_CMD_SF_QPP * Program SPI flash using Quad-input Page Program
> CONFIG_CMD_SHA1SUM print sha1 memory digest
> (requires CONFIG_CMD_MEMORY)
> CONFIG_CMD_SOURCE "source" command Support
> diff --git a/common/cmd_sf.c b/common/cmd_sf.c
> index 5ac1d0c..a449d2c 100644
> --- a/common/cmd_sf.c
> +++ b/common/cmd_sf.c
> @@ -228,6 +228,10 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
> ret = spi_flash_update(flash, offset, len, buf);
> else if (strcmp(argv[0], "read") == 0)
> ret = spi_flash_read(flash, offset, len, buf);
> +#ifdef CONFIG_CMD_SF_QPP
> + else if (strcmp(argv[0], "write.qpp") == 0)
> + ret = spi_flash_write_qpp(flash, offset, len, buf);
> +#endif
> else
> ret = spi_flash_write(flash, offset, len, buf);
>
> @@ -300,7 +304,7 @@ static int do_spi_flash(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[
> }
>
> if (strcmp(cmd, "read") == 0 || strcmp(cmd, "write") == 0 ||
> - strcmp(cmd, "update") == 0)
> + strcmp(cmd, "update") == 0 || strcmp(cmd, "write.qpp") == 0)
> ret = do_spi_flash_read_write(argc, argv);
> else if (strcmp(cmd, "erase") == 0)
> ret = do_spi_flash_erase(argc, argv);
> @@ -327,5 +331,9 @@ U_BOOT_CMD(
> "sf erase offset [+]len - erase `len' bytes from `offset'\n"
> " `+len' round up `len' to block size\n"
> "sf update addr offset len - erase and write `len' bytes from memory\n"
> - " at `addr' to flash at `offset'"
> + " at `addr' to flash at `offset'\n"
> +#ifdef CONFIG_CMD_SF_QPP
> + "sf write.qpp addr offset len - write `len' bytes from memory\n"
> + " at `addr' to flash at `offset' using Quad-input Page Program(32h)"
> +#endif
> );
> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
> index a8f0af0..3545f59 100644
> --- a/drivers/mtd/spi/spi_flash.c
> +++ b/drivers/mtd/spi/spi_flash.c
> @@ -122,6 +122,114 @@ int spi_flash_cmd_write_multi(struct spi_flash *flash, u32 offset,
> return ret;
> }
>
> +#ifdef CONFIG_CMD_SF_QPP
> +int spi_flash_set_quad_enable_bit(struct spi_flash *flash)
> +{
> + u8 cmd = CMD_READ_CONFIG;
> + u8 data = 0;
> + int ret;
> +
> + ret = spi_flash_read_common(flash,
> + &cmd, sizeof(cmd), &data, sizeof(data));
> + if (ret < 0) {
> + debug("SF: fail to read config register\n");
> + return ret;
> + }
> +
> + if (data & 0x2) {
Can we have defines/enums for this please?
> + debug("SF: quad enable bit is already set.\n");
> + return ret;
> + } else {
> + debug("SF: need to set quad enable bit\n");
> +
> + ret = spi_flash_cmd_write_config(flash, 0x0200);
and here?
> + if (ret < 0) {
> + debug("SF: fail to write quad enable bit\n");
> + return ret;
> + }
> +
> + ret = spi_flash_read_common(flash,
> + &cmd, sizeof(cmd), &data, sizeof(data));
> + if (ret < 0) {
> + debug("SF: fail to read config register\n");
> + return ret;
> + }
It almost seems like you could have a loop here: read it, return if
ok, write it, then loop again (up to two times). Might reduce the code
length, but perhaps your way is better.
> +
> + if (data & 0x2)
> + debug("SF: successfully set quad enable bit\n");
> + else {
> + printf("SF: fail to set quad enable bit\n");
> + return -1;
> + }
> + }
> +
> + return ret;
> +}
> +
> +int spi_flash_cmd_write_multi_qpp(struct spi_flash *flash, u32 offset,
> + size_t len, const void *buf)
> +{
> + unsigned long page_addr, byte_addr, page_size;
> + size_t chunk_len, actual;
> + int ret;
> + u8 cmd[4];
> +
> + page_size = flash->page_size;
> + page_addr = offset / page_size;
> + byte_addr = offset % page_size;
> +
> + ret = spi_claim_bus(flash->spi);
> + if (ret) {
> + debug("SF: unable to claim SPI bus\n");
> + return ret;
> + }
> +
> + ret = spi_flash_set_quad_enable_bit(flash);
> + if (ret) {
> + debug("SF: set quad enable bit failed\n");
> + return ret;
> + }
> +
> + cmd[0] = CMD_QUAD_PAGE_PROGRAM;
> + for (actual = 0; actual < len; actual += chunk_len) {
> + chunk_len = min(len - actual, page_size - byte_addr);
> +
> + cmd[1] = page_addr >> 8;
> + cmd[2] = page_addr;
> + cmd[3] = byte_addr;
> +
> + debug("PP: 0x%p => cmd = { 0x%02x 0x%02x%02x%02x } chunk_len = %zu\n",
> + buf + actual, cmd[0], cmd[1], cmd[2], cmd[3], chunk_len);
> +
> + ret = spi_flash_cmd_write_enable(flash);
> + if (ret < 0) {
> + debug("SF: enabling write failed\n");
> + break;
> + }
> +
> + ret = spi_flash_cmd_write(flash->spi, cmd, 4,
> + buf + actual, chunk_len);
> + if (ret < 0) {
> + debug("SF: write failed\n");
> + break;
> + }
> +
> + ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT);
> + if (ret)
> + break;
> +
> + page_addr++;
> + byte_addr = 0;
This seems very similar to the existing write function. Can you pull
out the common code?
> + }
> +
> + printf("SF: program %s %zu bytes @ %#x\n",
> + ret ? "failure" : "success", len, offset);
I think this should go in the cmdline code, not here. I may have
mentioned that already :-)
> +
> + spi_release_bus(flash->spi);
> + return ret;
> +}
> +#endif
> +
> int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd,
> size_t cmd_len, void *data, size_t data_len)
> {
> diff --git a/drivers/mtd/spi/spi_flash_internal.h b/drivers/mtd/spi/spi_flash_internal.h
> index 9287778..cb157ea 100644
> --- a/drivers/mtd/spi/spi_flash_internal.h
> +++ b/drivers/mtd/spi/spi_flash_internal.h
> @@ -20,8 +20,10 @@
>
> #define CMD_WRITE_STATUS 0x01
> #define CMD_PAGE_PROGRAM 0x02
> +#define CMD_QUAD_PAGE_PROGRAM 0x32
> #define CMD_WRITE_DISABLE 0x04
> #define CMD_READ_STATUS 0x05
> +#define CMD_READ_CONFIG 0x35
> #define CMD_WRITE_ENABLE 0x06
> #define CMD_ERASE_4K 0x20
> #define CMD_ERASE_32K 0x52
> @@ -54,10 +56,21 @@ int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len,
> /*
> * Write the requested data out breaking it up into multiple write
> * commands as needed per the write size.
> + * Programming a flash using Page Program(PP, 02h)
> */
> int spi_flash_cmd_write_multi(struct spi_flash *flash, u32 offset,
> size_t len, const void *buf);
>
> +#ifdef CONFIG_CMD_SF_QPP
> +/*
> + * Write the requested data out breaking it up into multiple write
> + * commands as needed per the write size.
> + * Programming a flash using Quad-input Page Program(QPP, 32h)
> + */
> +int spi_flash_cmd_write_multi_qpp(struct spi_flash *flash, u32 offset,
> + size_t len, const void *buf);
> +#endif
> +
> /*
> * Enable writing on the SPI flash.
> */
> diff --git a/include/spi_flash.h b/include/spi_flash.h
> index 9da9062..61f0c19 100644
> --- a/include/spi_flash.h
> +++ b/include/spi_flash.h
> @@ -43,6 +43,10 @@ struct spi_flash {
> size_t len, void *buf);
> int (*write)(struct spi_flash *flash, u32 offset,
> size_t len, const void *buf);
> +#ifdef CONFIG_CMD_SF_QPP
> + int (*write_qpp)(struct spi_flash *flash, u32 offset,
> + size_t len, const void *buf);
> +#endif
This is ok, but it almost feels like we should add a flags parameter here?
> int (*erase)(struct spi_flash *flash, u32 offset,
> size_t len);
> };
> @@ -63,6 +67,14 @@ static inline int spi_flash_write(struct spi_flash *flash, u32 offset,
> return flash->write(flash, offset, len, buf);
> }
>
> +#ifdef CONFIG_CMD_SF_QPP
> +static inline int spi_flash_write_qpp(struct spi_flash *flash, u32 offset,
> + size_t len, const void *buf)
> +{
> + return flash->write_qpp(flash, offset, len, buf);
> +}
> +#endif
> +
> static inline int spi_flash_erase(struct spi_flash *flash, u32 offset,
> size_t len)
> {
> --
> 1.7.0.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
Regards,
Simon
More information about the U-Boot
mailing list