sf: Tuning spi-flash layer

Rename spi_flash_validate_params to spi_flash_scan as this code not only deals with params setup but also configure all spi_flash attributes.

And also moved all flash related code into spi_flash_scan for future functionality addition.

Reviewed-by Simon Glass <[email protected]>

Signed-off-by Jagan Teki <[email protected]>

---
 drivers/mtd/spi/sf_probe.c | 145 +++++++++++++++++++++++----------------------
 1 file changed, 75 insertions(+), 70 deletions(-)

drivers/mtd/spi/sf_probe.c +20 -1

index a619182..0e20088 100644
@@ -130,13 +130,42 @@ bank_end:
 }
 #endif
 
-static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
+#if CONFIG_IS_ENABLED(OF_CONTROL)
+int spi_flash_decode_fdt(const void *blob, struct spi_flash *flash)
+{
+       fdt_addr_t addr;
+       fdt_size_t size;
+       int node;
+
+       /* If there is no node, do nothing */
+       node = fdtdec_next_compatible(blob, 0, COMPAT_GENERIC_SPI_FLASH);
+       if (node < 0)
+               return 0;
+
+       addr = fdtdec_get_addr_size(blob, node, "memory-map", &size);
+       if (addr == FDT_ADDR_T_NONE) {
+               debug("%s: Cannot decode address\n", __func__);
+               return 0;
+       }
+
+       if (flash->size != size) {
+               debug("%s: Memory map must cover entire device\n", __func__);

drivers/mtd/spi/sf_probe.c +46 -63

 {
        const struct spi_flash_params *params;
        u8 cmd;
        u16 jedec = idcode[1] << 8 | idcode[2];
        u16 ext_jedec = idcode[3] << 8 | idcode[4];
+       int ret;
 
        /* Validate params from spi_flash_params table */
        params = spi_flash_params_table;

@@ -158,6 +187,13 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
                return -EPROTONOSUPPORT;
        }
 
+       /* Flash powers up read-only, so clear BP# bits */
+#if defined(CONFIG_SPI_FLASH_ATMEL) || \
+       defined(CONFIG_SPI_FLASH_MACRONIX) || \
+       defined(CONFIG_SPI_FLASH_SST)
+               spi_flash_cmd_write_status(flash, 0);
+#endif
+
        /* Assign spi data */
        flash->spi = spi;
        flash->name = params->name;

@@ -253,6 +289,17 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
                /* Go for default supported write cmd */
                flash->write_cmd = CMD_PAGE_PROGRAM;
 
+       /* Set the quad enable bit - only for quad commands */
+       if ((flash->read_cmd == CMD_READ_QUAD_OUTPUT_FAST) ||
+           (flash->read_cmd == CMD_READ_QUAD_IO_FAST) ||
+           (flash->write_cmd == CMD_QUAD_PAGE_PROGRAM)) {
+               ret = spi_flash_set_qeb(flash, idcode[0]);
+               if (ret) {
+                       debug("SF: Fail to set QEB for %02x\n", idcode[0]);
+                       return -EINVAL;
+               }
+       }
+
        /* Read dummy_byte: dummy byte is determined based on the
         * dummy cycles of a particular command.
         * Fast commands - dummy_byte = dummy_cycles/8

@@ -279,48 +326,41 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
 
        /* Configure the BAR - discover bank cmds and read current bank */
 #ifdef CONFIG_SPI_FLASH_BAR
-       int ret = spi_flash_read_bank(flash, idcode[0]);
+       ret = spi_flash_read_bank(flash, idcode[0]);
        if (ret < 0)
                return ret;
 #endif
 
-       /* Flash powers up read-only, so clear BP# bits */
-#if defined(CONFIG_SPI_FLASH_ATMEL) || \
-       defined(CONFIG_SPI_FLASH_MACRONIX) || \
-       defined(CONFIG_SPI_FLASH_SST)
-               spi_flash_cmd_write_status(flash, 0);
-#endif
-
-       return 0;
-}
-
 #if CONFIG_IS_ENABLED(OF_CONTROL)
-int spi_flash_decode_fdt(const void *blob, struct spi_flash *flash)
-{
-       fdt_addr_t addr;
-       fdt_size_t size;
-       int node;
-
-       /* If there is no node, do nothing */
-       node = fdtdec_next_compatible(blob, 0, COMPAT_GENERIC_SPI_FLASH);
-       if (node < 0)
-               return 0;
-
-       addr = fdtdec_get_addr_size(blob, node, "memory-map", &size);
-       if (addr == FDT_ADDR_T_NONE) {
-               debug("%s: Cannot decode address\n", __func__);
-               return 0;
+       ret = spi_flash_decode_fdt(gd->fdt_blob, flash);
+       if (ret) {
+               debug("SF: FDT decode error\n");
+               return -EINVAL;
        }
+#endif
 
-       if (flash->size != size) {
-               debug("%s: Memory map must cover entire device\n", __func__);
-               return -1;
+#ifndef CONFIG_SPL_BUILD
+       printf("SF: Detected %s with page size ", flash->name);
+       print_size(flash->page_size, ", erase size ");
+       print_size(flash->erase_size, ", total ");
+       print_size(flash->size, "");
+       if (flash->memory_map)
+               printf(", mapped at %p", flash->memory_map);
+       puts("\n");
+#endif
+
+#ifndef CONFIG_SPI_FLASH_BAR
+       if (((flash->dual_flash == SF_SINGLE_FLASH) &&
+            (flash->size > SPI_FLASH_16MB_BOUN)) ||
+            ((flash->dual_flash > SF_SINGLE_FLASH) &&
+            (flash->size > SPI_FLASH_16MB_BOUN << 1))) {
+               puts("SF: Warning - Only lower 16MiB accessible,");
+               puts(" Full access #define CONFIG_SPI_FLASH_BAR\n");
        }
-       flash->memory_map = map_sysmem(addr, size);
+#endif
 
-       return 0;
+       return ret;
 }
-#endif /* CONFIG_IS_ENABLED(OF_CONTROL) */
 
 /**
  * spi_flash_probe_slave() - Probe for a SPI flash device on a bus

@@ -359,47 +399,12 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash)
        print_buffer(0, idcode, 1, sizeof(idcode), 0);
 #endif
 
-       if (spi_flash_validate_params(spi, idcode, flash)) {
+       ret = spi_flash_scan(spi, idcode, flash);
+       if (ret) {
                ret = -EINVAL;
                goto err_read_id;
        }
 
-       /* Set the quad enable bit - only for quad commands */
-       if ((flash->read_cmd == CMD_READ_QUAD_OUTPUT_FAST) ||
-           (flash->read_cmd == CMD_READ_QUAD_IO_FAST) ||
-           (flash->write_cmd == CMD_QUAD_PAGE_PROGRAM)) {
-               if (spi_flash_set_qeb(flash, idcode[0])) {
-                       debug("SF: Fail to set QEB for %02x\n", idcode[0]);
-                       ret = -EINVAL;
-                       goto err_read_id;
-               }
-       }
-
-#if CONFIG_IS_ENABLED(OF_CONTROL)
-       if (spi_flash_decode_fdt(gd->fdt_blob, flash)) {
-               debug("SF: FDT decode error\n");
-               ret = -EINVAL;
-               goto err_read_id;
-       }
-#endif
-#ifndef CONFIG_SPL_BUILD
-       printf("SF: Detected %s with page size ", flash->name);
-       print_size(flash->page_size, ", erase size ");
-       print_size(flash->erase_size, ", total ");
-       print_size(flash->size, "");
-       if (flash->memory_map)
-               printf(", mapped at %p", flash->memory_map);
-       puts("\n");
-#endif
-#ifndef CONFIG_SPI_FLASH_BAR
-       if (((flash->dual_flash == SF_SINGLE_FLASH) &&
-            (flash->size > SPI_FLASH_16MB_BOUN)) ||

Quoted 46 lines · Jagan Teki

Rename spi_flash_validate_params to spi_flash_scan as this code not only deals with params setup but also configure all spi_flash attributes.

And also moved all flash related code into spi_flash_scan for future functionality addition.

Reviewed-by Simon Glass <[email protected]>

Signed-off-by Jagan Teki <[email protected]>

---
 drivers/mtd/spi/sf_probe.c | 145 +++++++++++++++++++++++----------------------
 1 file changed, 75 insertions(+), 70 deletions(-)

drivers/mtd/spi/sf_probe.c +24 -1

index a619182..0e20088 100644
@@ -130,13 +130,42 @@ bank_end:
 }
 #endif
 
-static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
+#if CONFIG_IS_ENABLED(OF_CONTROL)
+int spi_flash_decode_fdt(const void *blob, struct spi_flash *flash)
+{
+       fdt_addr_t addr;
+       fdt_size_t size;
+       int node;
+
+       /* If there is no node, do nothing */
+       node = fdtdec_next_compatible(blob, 0, COMPAT_GENERIC_SPI_FLASH);
+       if (node < 0)
+               return 0;
+
+       addr = fdtdec_get_addr_size(blob, node, "memory-map", &size);
+       if (addr == FDT_ADDR_T_NONE) {
+               debug("%s: Cannot decode address\n", __func__);
+               return 0;
+       }
+
+       if (flash->size != size) {
+               debug("%s: Memory map must cover entire device\n", __func__);
+               return -1;
+       }
+       flash->memory_map = map_sysmem(addr, size);
+

drivers/mtd/spi/sf_probe.c bank_end: · line 162

Quoted 6 lines

drivers/mtd/spi/sf_probe.c +5 -0

+       return 0;
+}
+#endif /* CONFIG_IS_ENABLED(OF_CONTROL) */
+
+static int spi_flash_scan(struct spi_slave *spi, u8 *idcode,
                                     struct spi_flash *flash)

nits: please align above line to the left (

Quoted 157 lines

drivers/mtd/spi/sf_probe.c +46 -63

 {
        const struct spi_flash_params *params;
        u8 cmd;
        u16 jedec = idcode[1] << 8 | idcode[2];
        u16 ext_jedec = idcode[3] << 8 | idcode[4];
+       int ret;
 
        /* Validate params from spi_flash_params table */
        params = spi_flash_params_table;

@@ -158,6 +187,13 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
                return -EPROTONOSUPPORT;
        }
 
+       /* Flash powers up read-only, so clear BP# bits */
+#if defined(CONFIG_SPI_FLASH_ATMEL) || \
+       defined(CONFIG_SPI_FLASH_MACRONIX) || \
+       defined(CONFIG_SPI_FLASH_SST)
+               spi_flash_cmd_write_status(flash, 0);
+#endif
+
        /* Assign spi data */
        flash->spi = spi;
        flash->name = params->name;

@@ -253,6 +289,17 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
                /* Go for default supported write cmd */
                flash->write_cmd = CMD_PAGE_PROGRAM;
 
+       /* Set the quad enable bit - only for quad commands */
+       if ((flash->read_cmd == CMD_READ_QUAD_OUTPUT_FAST) ||
+           (flash->read_cmd == CMD_READ_QUAD_IO_FAST) ||
+           (flash->write_cmd == CMD_QUAD_PAGE_PROGRAM)) {
+               ret = spi_flash_set_qeb(flash, idcode[0]);
+               if (ret) {
+                       debug("SF: Fail to set QEB for %02x\n", idcode[0]);
+                       return -EINVAL;
+               }
+       }
+
        /* Read dummy_byte: dummy byte is determined based on the
         * dummy cycles of a particular command.
         * Fast commands - dummy_byte = dummy_cycles/8

@@ -279,48 +326,41 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
 
        /* Configure the BAR - discover bank cmds and read current bank */
 #ifdef CONFIG_SPI_FLASH_BAR
-       int ret = spi_flash_read_bank(flash, idcode[0]);
+       ret = spi_flash_read_bank(flash, idcode[0]);
        if (ret < 0)
                return ret;
 #endif
 
-       /* Flash powers up read-only, so clear BP# bits */
-#if defined(CONFIG_SPI_FLASH_ATMEL) || \
-       defined(CONFIG_SPI_FLASH_MACRONIX) || \
-       defined(CONFIG_SPI_FLASH_SST)
-               spi_flash_cmd_write_status(flash, 0);
-#endif
-
-       return 0;
-}
-
 #if CONFIG_IS_ENABLED(OF_CONTROL)
-int spi_flash_decode_fdt(const void *blob, struct spi_flash *flash)
-{
-       fdt_addr_t addr;
-       fdt_size_t size;
-       int node;
-
-       /* If there is no node, do nothing */
-       node = fdtdec_next_compatible(blob, 0, COMPAT_GENERIC_SPI_FLASH);
-       if (node < 0)
-               return 0;
-
-       addr = fdtdec_get_addr_size(blob, node, "memory-map", &size);
-       if (addr == FDT_ADDR_T_NONE) {
-               debug("%s: Cannot decode address\n", __func__);
-               return 0;
+       ret = spi_flash_decode_fdt(gd->fdt_blob, flash);
+       if (ret) {
+               debug("SF: FDT decode error\n");
+               return -EINVAL;
        }
+#endif
 
-       if (flash->size != size) {
-               debug("%s: Memory map must cover entire device\n", __func__);
-               return -1;
+#ifndef CONFIG_SPL_BUILD
+       printf("SF: Detected %s with page size ", flash->name);
+       print_size(flash->page_size, ", erase size ");
+       print_size(flash->erase_size, ", total ");
+       print_size(flash->size, "");
+       if (flash->memory_map)
+               printf(", mapped at %p", flash->memory_map);
+       puts("\n");
+#endif
+
+#ifndef CONFIG_SPI_FLASH_BAR
+       if (((flash->dual_flash == SF_SINGLE_FLASH) &&
+            (flash->size > SPI_FLASH_16MB_BOUN)) ||
+            ((flash->dual_flash > SF_SINGLE_FLASH) &&
+            (flash->size > SPI_FLASH_16MB_BOUN << 1))) {
+               puts("SF: Warning - Only lower 16MiB accessible,");
+               puts(" Full access #define CONFIG_SPI_FLASH_BAR\n");
        }
-       flash->memory_map = map_sysmem(addr, size);
+#endif
 
-       return 0;
+       return ret;
 }
-#endif /* CONFIG_IS_ENABLED(OF_CONTROL) */
 
 /**
  * spi_flash_probe_slave() - Probe for a SPI flash device on a bus

@@ -359,47 +399,12 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash)
        print_buffer(0, idcode, 1, sizeof(idcode), 0);
 #endif
 
-       if (spi_flash_validate_params(spi, idcode, flash)) {
+       ret = spi_flash_scan(spi, idcode, flash);
+       if (ret) {
                ret = -EINVAL;
                goto err_read_id;
        }
 
-       /* Set the quad enable bit - only for quad commands */
-       if ((flash->read_cmd == CMD_READ_QUAD_OUTPUT_FAST) ||
-           (flash->read_cmd == CMD_READ_QUAD_IO_FAST) ||
-           (flash->write_cmd == CMD_QUAD_PAGE_PROGRAM)) {
-               if (spi_flash_set_qeb(flash, idcode[0])) {
-                       debug("SF: Fail to set QEB for %02x\n", idcode[0]);
-                       ret = -EINVAL;
-                       goto err_read_id;
-               }
-       }
-
-#if CONFIG_IS_ENABLED(OF_CONTROL)
-       if (spi_flash_decode_fdt(gd->fdt_blob, flash)) {
-               debug("SF: FDT decode error\n");
-               ret = -EINVAL;
-               goto err_read_id;
-       }
-#endif
-#ifndef CONFIG_SPL_BUILD
-       printf("SF: Detected %s with page size ", flash->name);
-       print_size(flash->page_size, ", erase size ");
-       print_size(flash->erase_size, ", total ");
-       print_size(flash->size, "");
-       if (flash->memory_map)
-               printf(", mapped at %p", flash->memory_map);
-       puts("\n");
-#endif
-#ifndef CONFIG_SPI_FLASH_BAR
-       if (((flash->dual_flash == SF_SINGLE_FLASH) &&
-            (flash->size > SPI_FLASH_16MB_BOUN)) ||

drivers/mtd/spi/sf_probe.c int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash)

Quoted 10 lines

drivers/mtd/spi/sf_probe.c +0 -7

-            ((flash->dual_flash > SF_SINGLE_FLASH) &&
-            (flash->size > SPI_FLASH_16MB_BOUN << 1))) {
-               puts("SF: Warning - Only lower 16MiB accessible,");
-               puts(" Full access #define CONFIG_SPI_FLASH_BAR\n");
-       }
-#endif
 #ifdef CONFIG_SPI_FLASH_MTD
        ret = spi_flash_mtd_register(flash);
 #endif
--

Other than that,

Reviewed-by Bin Meng <[email protected]>

Tested-by Bin Meng <[email protected]>

Quoted 47 lines · Bin Meng

Quoted 46 lines · Jagan Teki

Rename spi_flash_validate_params to spi_flash_scan as this code not only deals with params setup but also configure all spi_flash attributes.

And also moved all flash related code into spi_flash_scan for future functionality addition.

Reviewed-by Simon Glass <[email protected]>

Signed-off-by Jagan Teki <[email protected]>

---
 drivers/mtd/spi/sf_probe.c | 145 +++++++++++++++++++++++----------------------
 1 file changed, 75 insertions(+), 70 deletions(-)

drivers/mtd/spi/sf_probe.c +24 -1

index a619182..0e20088 100644
@@ -130,13 +130,42 @@ bank_end:
 }
 #endif
 
-static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
+#if CONFIG_IS_ENABLED(OF_CONTROL)
+int spi_flash_decode_fdt(const void *blob, struct spi_flash *flash)
+{
+       fdt_addr_t addr;
+       fdt_size_t size;
+       int node;
+
+       /* If there is no node, do nothing */
+       node = fdtdec_next_compatible(blob, 0, COMPAT_GENERIC_SPI_FLASH);
+       if (node < 0)
+               return 0;
+
+       addr = fdtdec_get_addr_size(blob, node, "memory-map", &size);
+       if (addr == FDT_ADDR_T_NONE) {
+               debug("%s: Cannot decode address\n", __func__);
+               return 0;
+       }
+
+       if (flash->size != size) {
+               debug("%s: Memory map must cover entire device\n", __func__);
+               return -1;
+       }
+       flash->memory_map = map_sysmem(addr, size);
+

Quoted 8 lines

drivers/mtd/spi/sf_probe.c bank_end: · line 162

Quoted 6 lines

drivers/mtd/spi/sf_probe.c +5 -0

+       return 0;
+}
+#endif /* CONFIG_IS_ENABLED(OF_CONTROL) */
+
+static int spi_flash_scan(struct spi_slave *spi, u8 *idcode,
                                     struct spi_flash *flash)

nits: please align above line to the left (

What is the reason, will it be an alignment issue.

Hi Jagan,

Quoted 57 lines · Jagan Teki

Quoted 55 lines · Bin Meng

Quoted 42 lines · Jagan Teki

Rename spi_flash_validate_params to spi_flash_scan as this code not only deals with params setup but also configure all spi_flash attributes.

And also moved all flash related code into spi_flash_scan for future functionality addition.

Reviewed-by Simon Glass <[email protected]>

Signed-off-by Jagan Teki <[email protected]>

---
  drivers/mtd/spi/sf_probe.c | 145 +++++++++++++++++++++++----------------------
  1 file changed, 75 insertions(+), 70 deletions(-)

drivers/mtd/spi/sf_probe.c +20 -1

index a619182..0e20088 100644
@@ -130,13 +130,42 @@ bank_end:
  }
  #endif
 
-static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
+#if CONFIG_IS_ENABLED(OF_CONTROL)
+int spi_flash_decode_fdt(const void *blob, struct spi_flash *flash)
+{
+       fdt_addr_t addr;
+       fdt_size_t size;
+       int node;
+
+       /* If there is no node, do nothing */
+       node = fdtdec_next_compatible(blob, 0, COMPAT_GENERIC_SPI_FLASH);
+       if (node < 0)
+               return 0;
+
+       addr = fdtdec_get_addr_size(blob, node, "memory-map", &size);
+       if (addr == FDT_ADDR_T_NONE) {
+               debug("%s: Cannot decode address\n", __func__);
+               return 0;
+       }
+
+       if (flash->size != size) {
+               debug("%s: Memory map must cover entire device\n", __func__);

drivers/mtd/spi/sf_probe.c bank_end: · line 162

Quoted 10 lines

drivers/mtd/spi/sf_probe.c +9 -0

+               return -1;
+       }
+       flash->memory_map = map_sysmem(addr, size);
+
+       return 0;
+}
+#endif /* CONFIG_IS_ENABLED(OF_CONTROL) */
+
+static int spi_flash_scan(struct spi_slave *spi, u8 *idcode,
                                      struct spi_flash *flash)

nits: please align above line to the left (

Quoted 1 line

What is the reason, will it be an alignment issue.

checkpatch.pl will report such warnings.

[snip]

Quoted 70 lines · Bin Meng

Hi Jagan,

Quoted 66 lines · Jagan Teki

Quoted 62 lines · Bin Meng

Quoted 47 lines · Jagan Teki

Rename spi_flash_validate_params to spi_flash_scan as this code not only deals with params setup but also configure all spi_flash attributes.

And also moved all flash related code into spi_flash_scan for future functionality addition.

Reviewed-by Simon Glass <[email protected]>

Signed-off-by Jagan Teki <[email protected]>

---
  drivers/mtd/spi/sf_probe.c | 145
+++++++++++++++++++++++----------------------
  1 file changed, 75 insertions(+), 70 deletions(-)

drivers/mtd/spi/sf_probe.c +20 -1

index a619182..0e20088 100644
@@ -130,13 +130,42 @@ bank_end:
  }
  #endif
 
-static int spi_flash_validate_params(struct spi_slave *spi, u8
 *idcode,
+#if CONFIG_IS_ENABLED(OF_CONTROL)
+int spi_flash_decode_fdt(const void *blob, struct spi_flash *flash)
+{
+       fdt_addr_t addr;
+       fdt_size_t size;
+       int node;
+
+       /* If there is no node, do nothing */
+       node = fdtdec_next_compatible(blob, 0,
 COMPAT_GENERIC_SPI_FLASH);
+       if (node < 0)
+               return 0;
+
+       addr = fdtdec_get_addr_size(blob, node, "memory-map", &size);
+       if (addr == FDT_ADDR_T_NONE) {
+               debug("%s: Cannot decode address\n", __func__);
+               return 0;
+       }
+
+       if (flash->size != size) {
+               debug("%s: Memory map must cover entire device\n",
 __func__);

drivers/mtd/spi/sf_probe.c bank_end: · line 165

Quoted 10 lines

drivers/mtd/spi/sf_probe.c +9 -0

+               return -1;
+       }
+       flash->memory_map = map_sysmem(addr, size);
+
+       return 0;
+}
+#endif /* CONFIG_IS_ENABLED(OF_CONTROL) */
+
+static int spi_flash_scan(struct spi_slave *spi, u8 *idcode,
                                      struct spi_flash *flash)

nits: please align above line to the left (

Quoted 7 lines

Quoted 1 line

What is the reason, will it be an alignment issue.

checkpatch.pl will report such warnings.

[snip]

$> ./scripts/checkpatch.pl 0001-sf-spi_flash_validate_params-spi_flash_scan.patch
total: 0 errors, 0 warnings, 0 checks, 195 lines checked

NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE USLEEP_RANGE

0001-sf-spi_flash_validate_params-spi_flash_scan.patch has no obvious style problems and is ready for submission.

Intension is that sf_ops should deals all spi_flash related stuff and sf_probe (which should renamed future) should be an interface layer for spi_flash versus spi drivers.

sf_ops => spi_flash interface
sf_probe => interface layer vs spi_flash(sf_probe) to spi drivers

Reviewed-by Simon Glass <[email protected]>

Signed-off-by Jagan Teki <[email protected]>

---
 drivers/mtd/spi/sf_internal.h |  14 ++
 drivers/mtd/spi/sf_ops.c      | 343 ++++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/spi/sf_probe.c    | 342 -----------------------------------------
 3 files changed, 357 insertions(+), 342 deletions(-)

read_id code is related to spi_flash stuff
hence moved to sf_ops.

Reviewed-by Simon Glass <[email protected]>

Signed-off-by Jagan Teki <[email protected]>

---
 drivers/mtd/spi/sf_internal.h |  3 +--
 drivers/mtd/spi/sf_ops.c      | 21 ++++++++++++++++++---
 drivers/mtd/spi/sf_probe.c    | 15 +--------------
 3 files changed, 20 insertions(+), 19 deletions(-)

- Move bar read code below the bar write hance both at once place, hence it easy for #ifdef macro only once and readable.
- Move read_cmd_array at top

Reviewed-by Simon Glass <[email protected]>

Signed-off-by Jagan Teki <[email protected]>

---
 drivers/mtd/spi/sf_ops.c | 80 +++++++++++++++++++++++-------------------------
 1 file changed, 39 insertions(+), 41 deletions(-)

Use static for file-scope functions and removed
them from header files.

Signed-off-by Jagan Teki <[email protected]>

---
 drivers/mtd/spi/sf_internal.h | 18 ------------------
 drivers/mtd/spi/sf_ops.c      | 11 ++++++-----
 2 files changed, 6 insertions(+), 23 deletions(-)

Most of the register access function are static,
so used simple name to represent each.

Reviewed-by Simon Glass <[email protected]>

Signed-off-by Jagan Teki <[email protected]>

---
 drivers/mtd/spi/sf_ops.c | 52 ++++++++++++++++++++++++------------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

Using macro's for flash power up read-only access code leads wrong behaviour hence use idcode0 for runtime detection, hence the flash which require this functionality gets detected at runtime.

Reviewed-by Simon Glass <[email protected]>

Signed-off-by Jagan Teki <[email protected]>

---
 drivers/mtd/spi/sf_internal.h | 1 +
 drivers/mtd/spi/sf_ops.c      | 7 +++----
 2 files changed, 4 insertions(+), 4 deletions(-)

Removed unneeded header includes in sf_ops and sf_probe

Reviewed-by Simon Glass <[email protected]>

Signed-off-by Jagan Teki <[email protected]>

---
 drivers/mtd/spi/sf_ops.c   | 2 --
 drivers/mtd/spi/sf_probe.c | 3 ---
 2 files changed, 5 deletions(-)

SST parts added on sf_params.c supports both SST_WR which consits of both BP and WP and there is a spi controller ich which supports only BP so the relevent _write hook set based on "slave->op_mode_tx" hence there is no respective change required from flash side hance removed these.

Cc Bin Meng <[email protected]>

Signed-off-by Jagan Teki <[email protected]>

---
 drivers/mtd/spi/sf_internal.h | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

For assigning read_bar commands in spansion case, break is missing this patch add that break.

Signed-off-by Jagan Teki <[email protected]>

---
 drivers/mtd/spi/sf_ops.c | 1 +
 1 file changed, 1 insertion(+)

Since spi_slave is a spi pointer in spi_flash{} then assign spi_slave{} pointer to flash->spi and remove spi_slave pointer argument to
- spi_flash_probe_slave
- spi_flash_scan

Cc Simon Glass <[email protected]>

Signed-off-by Jagan Teki <[email protected]>

---
 drivers/mtd/spi/sf_internal.h |  3 +--
 drivers/mtd/spi/sf_ops.c      |  4 ++--
 drivers/mtd/spi/sf_probe.c    | 12 +++++++-----
 3 files changed, 10 insertions(+), 9 deletions(-)

Since spi_slave is a spi pointer in spi_flash{} then assign spi_slave{} pointer to flash->spi and remove spi_slave pointer argument to
- spi_flash_probe_slave
- spi_flash_scan

Cc Simon Glass <[email protected]>

Signed-off-by Jagan Teki <[email protected]>

---
 drivers/mtd/spi/sf_internal.h |  3 +--
 drivers/mtd/spi/sf_ops.c      |  4 ++--
 drivers/mtd/spi/sf_probe.c    | 12 +++++++-----
 3 files changed, 10 insertions(+), 9 deletions(-)

Used static for file-scope functions in sf_probe.c

Cc Simon Glass <[email protected]>

Signed-off-by Jagan Teki <[email protected]>

---
 drivers/mtd/spi/sf_probe.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Used static for file-scope functions in sf_probe.c

Cc Simon Glass <[email protected]>

Signed-off-by Jagan Teki <[email protected]>

---
 drivers/mtd/spi/sf_probe.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Since all spi-flash core operations are moved into sf_ops.c then it's better to renamed as spi-flash.c

Signed-off-by Jagan Teki <[email protected]>

---
 drivers/mtd/spi/Makefile                  | 2 +-
 drivers/mtd/spi/{sf_ops.c => spi-flash.c} | 7 ++++---
 2 files changed, 5 insertions(+), 4 deletions(-)
 rename drivers/mtd/spi/{sf_ops.c => spi-flash.c} (99%)

Quoted 9 lines · Jagan Teki

Since all spi-flash core operations are moved into sf_ops.c then it's better to renamed as spi-flash.c

Signed-off-by Jagan Teki <[email protected]>

---
 drivers/mtd/spi/Makefile                  | 2 +-
 drivers/mtd/spi/{sf_ops.c => spi-flash.c} | 7 ++++---
 2 files changed, 5 insertions(+), 4 deletions(-)
 rename drivers/mtd/spi/{sf_ops.c => spi-flash.c} (99%)

Reviewed-by Simon Glass <[email protected]>

(but I suggest spi_flash.c is better as it fits with the other files)

Quoted 9 lines · Jagan Teki

Since all spi-flash core operations are moved into sf_ops.c then it's better to renamed as spi-flash.c

Signed-off-by Jagan Teki <[email protected]>

---
 drivers/mtd/spi/Makefile                  | 2 +-
 drivers/mtd/spi/{sf_ops.c => spi-flash.c} | 7 ++++---
 2 files changed, 5 insertions(+), 4 deletions(-)
 rename drivers/mtd/spi/{sf_ops.c => spi-flash.c} (99%)

Reviewed-by Simon Glass <[email protected]>

(but I suggest spi_flash.c is better as it fits with the other files)

Quoted 11 lines · Simon Glass

Quoted 9 lines · Jagan Teki

Since all spi-flash core operations are moved into sf_ops.c then it's better to renamed as spi-flash.c

Signed-off-by Jagan Teki <[email protected]>

---
 drivers/mtd/spi/Makefile                  | 2 +-
 drivers/mtd/spi/{sf_ops.c => spi-flash.c} | 7 ++++---
 2 files changed, 5 insertions(+), 4 deletions(-)
 rename drivers/mtd/spi/{sf_ops.c => spi-flash.c} (99%)

Quoted 3 lines

Reviewed-by Simon Glass <[email protected]>

(but I suggest spi_flash.c is better as it fits with the other files)

Agreed. spi_flash.c makes more sense. So far it looks that only driver model uclass driver is using - in the file name, others are using _.

Other than this,

Reviewed-by Bin Meng <[email protected]>

Quoted 14 lines · Bin Meng

Quoted 13 lines · Simon Glass

Quoted 9 lines · Jagan Teki

Since all spi-flash core operations are moved into sf_ops.c then it's better to renamed as spi-flash.c

Signed-off-by Jagan Teki <[email protected]>

---
 drivers/mtd/spi/Makefile                  | 2 +-
 drivers/mtd/spi/{sf_ops.c => spi-flash.c} | 7 ++++---
 2 files changed, 5 insertions(+), 4 deletions(-)
 rename drivers/mtd/spi/{sf_ops.c => spi-flash.c} (99%)

Reviewed-by Simon Glass <[email protected]>

Quoted 4 lines

Quoted 1 line

(but I suggest spi_flash.c is better as it fits with the other files)

Agreed. spi_flash.c makes more sense. So far it looks that only driver model uclass driver is using - in the file name, others are using _.

Clear, but this file will handle common spi-flash core functionalities it shouldn't be dm even now or later and more over underlying sf_probe which is calling this through spi_flash_scan has a driver model on it.

Quoted 5 lines

Other than this,

Reviewed-by Bin Meng <[email protected]>

thanks!
--

Hi Jagan,

Quoted 20 lines · Jagan Teki

Quoted 18 lines · Bin Meng

Quoted 11 lines · Simon Glass

Quoted 9 lines · Jagan Teki

Since all spi-flash core operations are moved into sf_ops.c then it's better to renamed as spi-flash.c

Signed-off-by Jagan Teki <[email protected]>

---
  drivers/mtd/spi/Makefile                  | 2 +-
  drivers/mtd/spi/{sf_ops.c => spi-flash.c} | 7 ++++---
  2 files changed, 5 insertions(+), 4 deletions(-)

rename drivers/mtd/spi/{sf_ops.c => spi-flash.c} (99%)

Quoted 3 lines

Reviewed-by Simon Glass <[email protected]>

(but I suggest spi_flash.c is better as it fits with the other files)

Agreed. spi_flash.c makes more sense. So far it looks that only driver model uclass driver is using - in the file name, others are using _.

Quoted 3 lines

Clear, but this file will handle common spi-flash core functionalities it shouldn't be dm even now or later and more over underlying sf_probe which is calling this through spi_flash_scan has a driver model on it.

Sorry, I don't quite understand what you mean. But my comment is to rename sf_ops.c to sf_flash.c, not sf-flash.c.

Quoted 31 lines · Bin Meng

Hi Jagan,

Quoted 27 lines · Jagan Teki

Quoted 23 lines · Bin Meng

Quoted 14 lines · Simon Glass

Quoted 10 lines · Jagan Teki

Since all spi-flash core operations are moved into sf_ops.c then it's better to renamed as spi-flash.c

Signed-off-by Jagan Teki <[email protected]>

---
  drivers/mtd/spi/Makefile                  | 2 +-
  drivers/mtd/spi/{sf_ops.c => spi-flash.c} | 7 ++++---
  2 files changed, 5 insertions(+), 4 deletions(-)

rename drivers/mtd/spi/{sf_ops.c => spi-flash.c} (99%)

Quoted 3 lines

Reviewed-by Simon Glass <[email protected]>

(but I suggest spi_flash.c is better as it fits with the other files)

Agreed. spi_flash.c makes more sense. So far it looks that only driver model uclass driver is using - in the file name, others are using _.

Quoted 7 lines

Quoted 3 lines

Clear, but this file will handle common spi-flash core functionalities it shouldn't be dm even now or later and more over underlying sf_probe which is calling this through spi_flash_scan has a driver model on it.

Sorry, I don't quite understand what you mean. But my comment is to rename sf_ops.c to sf_flash.c, not sf-flash.c.

spi-flash.c (the function spi_flash_scan from sf_probe, so this never be a dm driver and it handles all core functionalities
======================================================================
sf_probe.c (this has dm support)
=================================

Since you're saying dm has - and ie the reason I'm saying spi-flash.c should technically a dm supported core.

Let me know if you're not clear though.

Hi Jagan,

Quoted 50 lines · Jagan Teki

Quoted 31 lines · Bin Meng

Hi Jagan,

Quoted 27 lines · Jagan Teki

Quoted 23 lines · Bin Meng

Quoted 14 lines · Simon Glass

Quoted 10 lines · Jagan Teki

Since all spi-flash core operations are moved into sf_ops.c then it's better to renamed as spi-flash.c

Signed-off-by Jagan Teki <[email protected]>

---
   drivers/mtd/spi/Makefile                  | 2 +-
   drivers/mtd/spi/{sf_ops.c => spi-flash.c} | 7 ++++---
   2 files changed, 5 insertions(+), 4 deletions(-)

rename drivers/mtd/spi/{sf_ops.c => spi-flash.c} (99%)

Quoted 3 lines

Reviewed-by Simon Glass <[email protected]>

(but I suggest spi_flash.c is better as it fits with the other files)

Agreed. spi_flash.c makes more sense. So far it looks that only driver model uclass driver is using - in the file name, others are using _.

Quoted 7 lines

Quoted 3 lines

Clear, but this file will handle common spi-flash core functionalities it shouldn't be dm even now or later and more over underlying sf_probe which is calling this through spi_flash_scan has a driver model on it.

Sorry, I don't quite understand what you mean. But my comment is to rename sf_ops.c to sf_flash.c, not sf-flash.c.

spi-flash.c (the function spi_flash_scan from sf_probe, so this never be a dm driver and it handles all core functionalities
======================================================================
sf_probe.c (this has dm support)
=================================

Quoted 3 lines

Since you're saying dm has - and ie the reason I'm saying spi-flash.c should technically a dm supported core.

I was saying it looks to me that only dm uclass driver is allowed to have -, like sf-uclass.c or pci-uclass.c. Other files we should use _.

Quoted 71 lines · Bin Meng

Hi Jagan,

Quoted 66 lines · Jagan Teki

Quoted 39 lines · Bin Meng

Hi Jagan,

Quoted 34 lines · Jagan Teki

Quoted 28 lines · Bin Meng

Quoted 17 lines · Simon Glass

Quoted 11 lines · Jagan Teki

Since all spi-flash core operations are moved into sf_ops.c then it's better to renamed as spi-flash.c

Signed-off-by Jagan Teki <[email protected]>

---
   drivers/mtd/spi/Makefile                  | 2 +-
   drivers/mtd/spi/{sf_ops.c => spi-flash.c} | 7 ++++---
   2 files changed, 5 insertions(+), 4 deletions(-)

rename drivers/mtd/spi/{sf_ops.c => spi-flash.c} (99%)

Quoted 3 lines

Reviewed-by Simon Glass <[email protected]>

(but I suggest spi_flash.c is better as it fits with the other files)

Agreed. spi_flash.c makes more sense. So far it looks that only driver model uclass driver is using - in the file name, others are using _.

Quoted 8 lines

Quoted 3 lines

Clear, but this file will handle common spi-flash core functionalities it shouldn't be dm even now or later and more over underlying sf_probe which is calling this through spi_flash_scan has a driver model on it.

Sorry, I don't quite understand what you mean. But my comment is to rename sf_ops.c to sf_flash.c, not sf-flash.c.

spi-flash.c (the function spi_flash_scan from sf_probe, so this never be a
dm driver and it handles all core functionalities
======================================================================
sf_probe.c (this has dm support)
=================================

Since you're saying dm has - and ie the reason I'm saying spi-flash.c should
technically a dm supported core.

Quoted 2 lines

I was saying it looks to me that only dm uclass driver is allowed to have -, like sf-uclass.c or pci-uclass.c. Other files we should use _.

sf_probe.c is a dm driver - agree?
If ie the case probably this is the first file has a code moved from dm driver into different file which is spi-flash in this case.

This series bypasses MTD changes from previous series[1][2] and entire series tunned spi-flash layer for better code handling.

Changes in v8:
- Rebase to master
- PATCH v8 12/14, 13/14, 14/14 added newly.

Changes in v7:
- Rebase to master
- Few patch bisectable separations

Changes in v6, v5, v4, v3, v2:
- One patch bisectable separation
- Rebase to master

Testing:

$ git clone git://git.denx.de/u-boot-spi.git
$ cd u-boot-spi
$ git checkout -b next origin/next

[1] http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/242397
[2] http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/233262

thanks!
Jagan.

Jagan Teki (14):
sf: spi_flash_validate_params => spi_flash_scan
sf: Move spi_flash_scan code to sf_ops
sf: Move read_id code to sf_ops
sf: probe: Code cleanup
sf: Use static for file-scope functions
sf: Fix Makefile
sf: Use simple name for register access functions
sf: Flash power up read-only based on idcode0
sf: Remove unneeded header includes
sf: Remove unneeded SST_BP and SST_WP
sf: ops: Fix missing break on spansion read_bar
sf: sf_probe: Remove spi_slave pointer argument
sf: Use static for file-scope functions
sf: Rename sf_ops.c to spi-flash.c

 drivers/mtd/spi/Makefile                  |   6 +-
 drivers/mtd/spi/sf_internal.h             |  38 ++-
 drivers/mtd/spi/sf_probe.c                | 373 +--------------------------
 drivers/mtd/spi/{sf_ops.c => spi-flash.c} | 406 ++++++++++++++++++++++++++++--
 4 files changed, 408 insertions(+), 415 deletions(-)
 rename drivers/mtd/spi/{sf_ops.c => spi-flash.c} (60%)

--
1.9.1

Hi Bin,

Quoted 15 lines · Jagan Teki

This series bypasses MTD changes from previous series[1][2] and entire series tunned spi-flash layer for better code handling.

Changes in v8:
- Rebase to master
- PATCH v8 12/14, 13/14, 14/14 added newly.

Changes in v7:
- Rebase to master
- Few patch bisectable separations

Changes in v6, v5, v4, v3, v2:
- One patch bisectable separation
- Rebase to master

Quoted 4 lines

Testing:

$ git clone git://git.denx.de/u-boot-spi.git
$ cd u-boot-spi
$ git checkout -b next origin/next

Please test this.

Quoted 23 lines

[1] http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/242397
[2] http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/233262

thanks!
Jagan.

Jagan Teki (14):
sf: spi_flash_validate_params => spi_flash_scan
sf: Move spi_flash_scan code to sf_ops
sf: Move read_id code to sf_ops
sf: probe: Code cleanup
sf: Use static for file-scope functions
sf: Fix Makefile
sf: Use simple name for register access functions
sf: Flash power up read-only based on idcode0
sf: Remove unneeded header includes
sf: Remove unneeded SST_BP and SST_WP
sf: ops: Fix missing break on spansion read_bar
sf: sf_probe: Remove spi_slave pointer argument
sf: Use static for file-scope functions
sf: Rename sf_ops.c to spi-flash.c

Quoted 7 lines

 drivers/mtd/spi/Makefile                  |   6 +-
 drivers/mtd/spi/sf_internal.h             |  38 ++-
 drivers/mtd/spi/sf_probe.c                | 373 +--------------------------
 drivers/mtd/spi/{sf_ops.c => spi-flash.c} | 406 ++++++++++++++++++++++++++++--
 4 files changed, 408 insertions(+), 415 deletions(-)
 rename drivers/mtd/spi/{sf_ops.c => spi-flash.c} (60%)

This series bypasses MTD changes from previous series[1][2] and entire series tunned spi-flash layer for better code handling.

Changes in v8:
- Rebase to master
- PATCH v8 12/14, 13/14, 14/14 added newly.

Changes in v7:
- Rebase to master
- Few patch bisectable separations

Changes in v6, v5, v4, v3, v2:
- One patch bisectable separation
- Rebase to master

Testing:

$ git clone git://git.denx.de/u-boot-spi.git
$ cd u-boot-spi
$ git checkout -b next origin/next

[1] http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/242397
[2] http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/233262

Hi Tom/Wolfgang,

"Need a separate repo for spi-nor and related subsystem."

Why?
- Currently I'm using u-boot-spi for both spi and spi-flash - some flash stuff been added to spi area which is hard for adding/maintaining new features.
- Both are two separate(not different) Subsystems one is generic SPI drivers/spi and another one is flash slave device drivers/mtd/spi
- Development is going on for SPI-NOR Subsystem similar like mtd nand, cfi-flash etc.

Quoted 15 lines

This series bypasses MTD changes from previous series[1][2] and entire series tunned spi-flash layer for better code handling.

Changes in v8:
- Rebase to master
- PATCH v8 12/14, 13/14, 14/14 added newly.

Changes in v7:
- Rebase to master
- Few patch bisectable separations

Changes in v6, v5, v4, v3, v2:
- One patch bisectable separation
- Rebase to master

Quoted 7 lines

Testing:

$ git clone git://git.denx.de/u-boot-spi.git
$ cd u-boot-spi
$ git checkout -b next origin/next

[1] http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/242397
[2] http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/233262

This series bypasses MTD changes from previous series[1][2] and entire series tunned spi-flash layer for better code handling.

Changes in v8:
- Rebase to master
- PATCH v8 12/14, 13/14, 14/14 added newly.

Changes in v7:
- Rebase to master
- Few patch bisectable separations

Changes in v6, v5, v4, v3, v2:
- One patch bisectable separation
- Rebase to master

Testing:

$ git clone git://git.denx.de/u-boot-spi.git
$ cd u-boot-spi
$ git checkout -b next origin/next

[1] http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/242397
[2] http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/233262

This series bypasses MTD changes from previous series[1][2] and entire series tunned spi-flash layer for better code handling.

Changes in v8:
- Rebase to master
- PATCH v8 12/14, 13/14, 14/14 added newly.

Changes in v7:
- Rebase to master
- Few patch bisectable separations

Changes in v6, v5, v4, v3, v2:
- One patch bisectable separation
- Rebase to master

Testing:

$ git clone git://git.denx.de/u-boot-spi.git
$ cd u-boot-spi
$ git checkout -b next origin/next

[1] http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/242397
[2] http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/233262

thanks!
Jagan.

Jagan Teki (14):
sf: spi_flash_validate_params => spi_flash_scan
sf: Move spi_flash_scan code to sf_ops
sf: Move read_id code to sf_ops
sf: probe: Code cleanup
sf: Use static for file-scope functions
sf: Fix Makefile
sf: Use simple name for register access functions
sf: Flash power up read-only based on idcode0
sf: Remove unneeded header includes
sf: Remove unneeded SST_BP and SST_WP
sf: ops: Fix missing break on spansion read_bar
sf: sf_probe: Remove spi_slave pointer argument
sf: Use static for file-scope functions
sf: Rename sf_ops.c to spi-flash.c

 drivers/mtd/spi/Makefile                  |   6 +-
 drivers/mtd/spi/sf_internal.h             |  38 ++-
 drivers/mtd/spi/sf_probe.c                | 373 +--------------------------
 drivers/mtd/spi/{sf_ops.c => spi-flash.c} | 406 ++++++++++++++++++++++++++++--
 4 files changed, 408 insertions(+), 415 deletions(-)
 rename drivers/mtd/spi/{sf_ops.c => spi-flash.c} (60%)

--
1.9.1