sf: Shrink spi_slave {}

Combined spi flash stuff as minimum as possible.

Signed-off-by Jagannadha Sutradharudu Teki <[email protected]>

Cc Marek Vasut <[email protected]>

---
 drivers/mtd/spi/sf.c       |  4 ++--
 drivers/mtd/spi/sf_ops.c   | 18 +++++++++---------
 drivers/mtd/spi/sf_probe.c | 19 ++++++++++++-------
 include/spi.h              | 45
+++++++++++++++++++-------------------------- include/spi_flash.h        |
 6 +++---
 5 files changed, 45 insertions(+), 47 deletions(-)

drivers/mtd/spi/sf.c +2 -2

index 664e860..fb91ac6 100644
@@ -19,8 +19,8 @@ static int spi_flash_read_write(struct spi_slave *spi,
 	int ret;
 
 #ifdef CONFIG_SF_DUAL_FLASH
-	if (spi->flags & SPI_XFER_U_PAGE)
-		flags |= SPI_XFER_U_PAGE;
+	if (spi->mode_bits & SPI_U_PAGE)
+		flags |= SPI_U_PAGE;
 #endif
 	if (data_len == 0)
 		flags |= SPI_XFER_END;

drivers/mtd/spi/sf_ops.c +0 -0

index 9650486..0d87e1b 100644
@@ -135,15 +135,15 @@ static int spi_flash_bank(struct spi_flash *flash,

u32 offset) static void spi_flash_dual_flash(struct spi_flash *flash, u32 *addr) {

drivers/mtd/spi/sf_ops.c +9 -9

 	switch (flash->dual_flash) {
-	case SF_DUAL_STACKED_FLASH:
+	case SF_STACKED:
 		if (*addr >= (flash->size >> 1)) {
 			*addr -= flash->size >> 1;
-			flash->spi->flags |= SPI_XFER_U_PAGE;
+			flash->spi->mode_bits |= SPI_U_PAGE;
 		} else {
-			flash->spi->flags &= ~SPI_XFER_U_PAGE;
+			flash->spi->mode_bits &= ~SPI_U_PAGE;
 		}
 		break;
-	case SF_DUAL_PARALLEL_FLASH:
+	case SF_PARALLEL:
 		*addr >>= flash->shift;
 		break;
 	default:

@@ -170,8 +170,8 @@ int spi_flash_cmd_wait_ready(struct spi_flash *flash,
 unsigned long timeout) }
 
 #ifdef CONFIG_SF_DUAL_FLASH
-	if (spi->flags & SPI_XFER_U_PAGE)
-		flags |= SPI_XFER_U_PAGE;
+	if (spi->mode_bits & SPI_U_PAGE)
+		flags |= SPI_U_PAGE;
 #endif
 	ret = spi_xfer(spi, 8, &cmd, NULL, flags);
 	if (ret) {

@@ -261,7 +261,7 @@ int spi_flash_cmd_erase_ops(struct spi_flash *flash,
 u32 offset, size_t len) erase_addr = offset;
 
 #ifdef CONFIG_SF_DUAL_FLASH
-		if (flash->dual_flash > SF_SINGLE_FLASH)
+		if (flash->dual_flash > SF_SINGLE)
 			spi_flash_dual_flash(flash, &erase_addr);
 #endif
 #ifdef CONFIG_SPI_FLASH_BAR

@@ -303,7 +303,7 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash,
 u32 offset, write_addr = offset;
 
 #ifdef CONFIG_SF_DUAL_FLASH
-		if (flash->dual_flash > SF_SINGLE_FLASH)
+		if (flash->dual_flash > SF_SINGLE)
 			spi_flash_dual_flash(flash, &write_addr);
 #endif
 #ifdef CONFIG_SPI_FLASH_BAR

@@ -388,7 +388,7 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32
 offset, read_addr = offset;
 
 #ifdef CONFIG_SF_DUAL_FLASH
-		if (flash->dual_flash > SF_SINGLE_FLASH)
+		if (flash->dual_flash > SF_SINGLE)
 			spi_flash_dual_flash(flash, &read_addr);
 #endif
 #ifdef CONFIG_SPI_FLASH_BAR

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

index e84ab13..003f635 100644

drivers/mtd/spi/sf_probe.c +11 -6

+	if (flash->spi->mode_bits & SPI_SHARED)
+		flash->dual_flash = SF_STACKED;
+	else if (flash->spi->mode_bits & SPI_SEPARATED)
+		flash->dual_flash = SF_PARALLEL;
+#endif
 	/* Assign spi_flash ops */
 	flash->write = spi_flash_cmd_write_ops;
 #ifdef CONFIG_SPI_FLASH_SST

@@ -145,12 +150,12 @@ static struct spi_flash
 *spi_flash_validate_params(struct spi_slave *spi, flash->read =
 spi_flash_cmd_read_ops;
 
 	/* Compute the flash size */
-	flash->shift = (flash->dual_flash & SF_DUAL_PARALLEL_FLASH) ? 1 : 0;
+	flash->shift = (flash->dual_flash & SF_PARALLEL) ? 1 : 0;
 	flash->page_size = ((ext_jedec == 0x4d00) ? 512 : 256) << flash->shift;
 	flash->sector_size = params->sector_size << flash->shift;
 	flash->size = flash->sector_size * params->nr_sectors << flash->shift;
 #ifdef CONFIG_SF_DUAL_FLASH
-	if (flash->dual_flash & SF_DUAL_STACKED_FLASH)
+	if (flash->dual_flash & SF_STACKED)
 		flash->size <<= 1;
 #endif

@@ -167,7 +172,7 @@ static struct spi_flash
 *spi_flash_validate_params(struct spi_slave *spi, }
 
 	/* Look for the fastest read cmd */
-	cmd = fls(params->e_rd_cmd & flash->spi->op_mode_rx);
+	cmd = fls(params->e_rd_cmd & flash->spi->rx_mode);
 	if (cmd) {
 		cmd = spi_read_cmds_array[cmd - 1];
 		flash->read_cmd = cmd;

@@ -177,7 +182,7 @@ static struct spi_flash
 *spi_flash_validate_params(struct spi_slave *spi, }
 
 	/* Not require to look for fastest only two write cmds yet */
-	if (params->flags & WR_QPP && flash->spi->op_mode_tx & SPI_OPM_TX_QPP)
+	if (params->flags & WR_QPP && flash->spi->mode_bits & SPI_TX_QPP)
 		flash->write_cmd = CMD_QUAD_PAGE_PROGRAM;
 	else
 		/* Go for default supported write cmd */

@@ -329,9 +334,9 @@ static struct spi_flash *spi_flash_probe_slave(struct
 spi_slave *spi) puts("\n");
 #endif
 #ifndef CONFIG_SPI_FLASH_BAR
-	if (((flash->dual_flash == SF_SINGLE_FLASH) &&
+	if (((flash->dual_flash == SF_SINGLE) &&
 	     (flash->size > SPI_FLASH_16MB_BOUN)) ||
-	     ((flash->dual_flash > SF_SINGLE_FLASH) &&
+	     ((flash->dual_flash > SF_SINGLE) &&
 	     (flash->size > SPI_FLASH_16MB_BOUN << 1))) {
 		puts("SF: Warning - Only lower 16MiB accessible,");
 		puts(" Full access #define CONFIG_SPI_FLASH_BAR\n");

include/spi.h +15 -19

index ffd6647..07ba4d0 100644
@@ -30,29 +30,25 @@
 #define SPI_XFER_MMAP		0x08	/* Memory Mapped start */
 #define SPI_XFER_MMAP_END	0x10	/* Memory Mapped End */
 #define SPI_XFER_ONCE		(SPI_XFER_BEGIN | SPI_XFER_END)
-#define SPI_XFER_U_PAGE		(1 << 5)
-
-/* SPI TX operation modes */
-#define SPI_OPM_TX_QPP		1 << 0
-
-/* SPI RX operation modes */
-#define SPI_OPM_RX_AS		1 << 0
-#define SPI_OPM_RX_DOUT		1 << 1
-#define SPI_OPM_RX_DIO		1 << 2
-#define SPI_OPM_RX_QOF		1 << 3
-#define SPI_OPM_RX_QIOF		1 << 4
-#define SPI_OPM_RX_EXTN		SPI_OPM_RX_AS | SPI_OPM_RX_DOUT | \
-				SPI_OPM_RX_DIO | SPI_OPM_RX_QOF | \
-				SPI_OPM_RX_QIOF
-
-/* SPI bus connection options */
-#define SPI_CONN_DUAL_SHARED	1 << 0
-#define SPI_CONN_DUAL_SEPARATED	1 << 1
 
 /* Header byte that marks the start of the message */
 #define SPI_PREAMBLE_END_BYTE	0xec
+#define SPI_DEFAULT_WORDLEN	8
+
+/* SPI RX operation modes */
+#define SPI_RX_AS	1 << 0
+#define SPI_RX_DOUT	1 << 1
+#define SPI_RX_DIO	1 << 2
+#define SPI_RX_QOF	1 << 3
+#define SPI_RX_QIOF	1 << 4
+#define SPI_RX_EXTN	SPI_RX_AS | SPI_RX_DOUT | SPI_RX_DIO | \
+			SPI_RX_QOF | SPI_RX_QIOF
 
-#define SPI_DEFAULT_WORDLEN 8
+/* SPI mode bits */
+#define SPI_TX_QPP	1 << 0
+#define SPI_SHARED	1 << 1
+#define SPI_SEPARATED	1 << 2
+#define SPI_U_PAGE	1 << 3
 
 /**
  * struct spi_slave - Representation of a SPI slave

@@ -61,25 +57,22 @@
  *
  * @bus:		ID of the bus that the slave is attached to.

Quoted 1 line · Jagannadha Sutradharudu Teki

Combined spi flash stuff as minimum as possible.

Squashing the names of the flags only makes it unreadable :-(

Quoted 93 lines

Signed-off-by Jagannadha Sutradharudu Teki <[email protected]>

Cc Marek Vasut <[email protected]>

---
 drivers/mtd/spi/sf.c       |  4 ++--
 drivers/mtd/spi/sf_ops.c   | 18 +++++++++---------
 drivers/mtd/spi/sf_probe.c | 19 ++++++++++++-------
 include/spi.h              | 45
+++++++++++++++++++-------------------------- include/spi_flash.h        |
 6 +++---
 5 files changed, 45 insertions(+), 47 deletions(-)

drivers/mtd/spi/sf.c +2 -2

index 664e860..fb91ac6 100644
@@ -19,8 +19,8 @@ static int spi_flash_read_write(struct spi_slave *spi,
      int ret;
 
 #ifdef CONFIG_SF_DUAL_FLASH
-     if (spi->flags & SPI_XFER_U_PAGE)
-             flags |= SPI_XFER_U_PAGE;
+     if (spi->mode_bits & SPI_U_PAGE)
+             flags |= SPI_U_PAGE;
 #endif
      if (data_len == 0)
              flags |= SPI_XFER_END;

drivers/mtd/spi/sf_ops.c +0 -0

index 9650486..0d87e1b 100644
@@ -135,15 +135,15 @@ static int spi_flash_bank(struct spi_flash *flash,

u32 offset) static void spi_flash_dual_flash(struct spi_flash *flash, u32

*addr) {
      switch (flash->dual_flash) {

drivers/mtd/spi/sf_ops.c +9 -9

-     case SF_DUAL_STACKED_FLASH:
+     case SF_STACKED:
              if (*addr >= (flash->size >> 1)) {
                      *addr -= flash->size >> 1;
-                     flash->spi->flags |= SPI_XFER_U_PAGE;
+                     flash->spi->mode_bits |= SPI_U_PAGE;
              } else {
-                     flash->spi->flags &= ~SPI_XFER_U_PAGE;
+                     flash->spi->mode_bits &= ~SPI_U_PAGE;
              }
              break;
-     case SF_DUAL_PARALLEL_FLASH:
+     case SF_PARALLEL:
              *addr >>= flash->shift;
              break;
      default:

@@ -170,8 +170,8 @@ int spi_flash_cmd_wait_ready(struct spi_flash *flash,
 unsigned long timeout) }
 
 #ifdef CONFIG_SF_DUAL_FLASH
-     if (spi->flags & SPI_XFER_U_PAGE)
-             flags |= SPI_XFER_U_PAGE;
+     if (spi->mode_bits & SPI_U_PAGE)
+             flags |= SPI_U_PAGE;
 #endif
      ret = spi_xfer(spi, 8, &cmd, NULL, flags);
      if (ret) {

@@ -261,7 +261,7 @@ int spi_flash_cmd_erase_ops(struct spi_flash *flash,
 u32 offset, size_t len) erase_addr = offset;
 
 #ifdef CONFIG_SF_DUAL_FLASH
-             if (flash->dual_flash > SF_SINGLE_FLASH)
+             if (flash->dual_flash > SF_SINGLE)
                      spi_flash_dual_flash(flash, &erase_addr);
 #endif
 #ifdef CONFIG_SPI_FLASH_BAR

@@ -303,7 +303,7 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash,
 u32 offset, write_addr = offset;
 
 #ifdef CONFIG_SF_DUAL_FLASH
-             if (flash->dual_flash > SF_SINGLE_FLASH)
+             if (flash->dual_flash > SF_SINGLE)
                      spi_flash_dual_flash(flash, &write_addr);
 #endif
 #ifdef CONFIG_SPI_FLASH_BAR

@@ -388,7 +388,7 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32
 offset, read_addr = offset;
 
 #ifdef CONFIG_SF_DUAL_FLASH
-             if (flash->dual_flash > SF_SINGLE_FLASH)
+             if (flash->dual_flash > SF_SINGLE)
                      spi_flash_dual_flash(flash, &read_addr);
 #endif
 #ifdef CONFIG_SPI_FLASH_BAR

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

index e84ab13..003f635 100644

drivers/mtd/spi/sf_probe.c static struct spi_flash · line 137

Quoted 6 lines

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

@@ -133,8 +133,13 @@ static struct spi_flash
 *spi_flash_validate_params(struct spi_slave *spi, flash->spi = spi;
      flash->name = params->name;
      flash->memory_map = spi->memory_map;
-     flash->dual_flash = flash->spi->option;
 
+#ifdef CONFIG_SF_DUAL_FLASH

Looks new and unrelated.

Quoted 106 lines

drivers/mtd/spi/sf_probe.c +11 -6

+     if (flash->spi->mode_bits & SPI_SHARED)
+             flash->dual_flash = SF_STACKED;
+     else if (flash->spi->mode_bits & SPI_SEPARATED)
+             flash->dual_flash = SF_PARALLEL;
+#endif
      /* Assign spi_flash ops */
      flash->write = spi_flash_cmd_write_ops;
 #ifdef CONFIG_SPI_FLASH_SST

@@ -145,12 +150,12 @@ static struct spi_flash
 *spi_flash_validate_params(struct spi_slave *spi, flash->read =
 spi_flash_cmd_read_ops;
 
      /* Compute the flash size */
-     flash->shift = (flash->dual_flash & SF_DUAL_PARALLEL_FLASH) ? 1 : 0;
+     flash->shift = (flash->dual_flash & SF_PARALLEL) ? 1 : 0;
      flash->page_size = ((ext_jedec == 0x4d00) ? 512 : 256) << flash->shift;
      flash->sector_size = params->sector_size << flash->shift;
      flash->size = flash->sector_size * params->nr_sectors << flash->shift;
 #ifdef CONFIG_SF_DUAL_FLASH
-     if (flash->dual_flash & SF_DUAL_STACKED_FLASH)
+     if (flash->dual_flash & SF_STACKED)
              flash->size <<= 1;
 #endif

@@ -167,7 +172,7 @@ static struct spi_flash
 *spi_flash_validate_params(struct spi_slave *spi, }
 
      /* Look for the fastest read cmd */
-     cmd = fls(params->e_rd_cmd & flash->spi->op_mode_rx);
+     cmd = fls(params->e_rd_cmd & flash->spi->rx_mode);
      if (cmd) {
              cmd = spi_read_cmds_array[cmd - 1];
              flash->read_cmd = cmd;

@@ -177,7 +182,7 @@ static struct spi_flash
 *spi_flash_validate_params(struct spi_slave *spi, }
 
      /* Not require to look for fastest only two write cmds yet */
-     if (params->flags & WR_QPP && flash->spi->op_mode_tx & SPI_OPM_TX_QPP)
+     if (params->flags & WR_QPP && flash->spi->mode_bits & SPI_TX_QPP)
              flash->write_cmd = CMD_QUAD_PAGE_PROGRAM;
      else
              /* Go for default supported write cmd */

@@ -329,9 +334,9 @@ static struct spi_flash *spi_flash_probe_slave(struct
 spi_slave *spi) puts("\n");
 #endif
 #ifndef CONFIG_SPI_FLASH_BAR
-     if (((flash->dual_flash == SF_SINGLE_FLASH) &&
+     if (((flash->dual_flash == SF_SINGLE) &&
           (flash->size > SPI_FLASH_16MB_BOUN)) ||
-          ((flash->dual_flash > SF_SINGLE_FLASH) &&
+          ((flash->dual_flash > SF_SINGLE) &&
           (flash->size > SPI_FLASH_16MB_BOUN << 1))) {
              puts("SF: Warning - Only lower 16MiB accessible,");
              puts(" Full access #define CONFIG_SPI_FLASH_BAR\n");

include/spi.h +15 -19

index ffd6647..07ba4d0 100644
@@ -30,29 +30,25 @@
 #define SPI_XFER_MMAP                0x08    /* Memory Mapped start */
 #define SPI_XFER_MMAP_END    0x10    /* Memory Mapped End */
 #define SPI_XFER_ONCE                (SPI_XFER_BEGIN | SPI_XFER_END)
-#define SPI_XFER_U_PAGE              (1 << 5)
-
-/* SPI TX operation modes */
-#define SPI_OPM_TX_QPP               1 << 0
-
-/* SPI RX operation modes */
-#define SPI_OPM_RX_AS                1 << 0
-#define SPI_OPM_RX_DOUT              1 << 1
-#define SPI_OPM_RX_DIO               1 << 2
-#define SPI_OPM_RX_QOF               1 << 3
-#define SPI_OPM_RX_QIOF              1 << 4
-#define SPI_OPM_RX_EXTN              SPI_OPM_RX_AS | SPI_OPM_RX_DOUT | \
-                             SPI_OPM_RX_DIO | SPI_OPM_RX_QOF | \
-                             SPI_OPM_RX_QIOF
-
-/* SPI bus connection options */
-#define SPI_CONN_DUAL_SHARED 1 << 0
-#define SPI_CONN_DUAL_SEPARATED      1 << 1
 
 /* Header byte that marks the start of the message */
 #define SPI_PREAMBLE_END_BYTE        0xec
+#define SPI_DEFAULT_WORDLEN  8
+
+/* SPI RX operation modes */
+#define SPI_RX_AS    1 << 0
+#define SPI_RX_DOUT  1 << 1
+#define SPI_RX_DIO   1 << 2
+#define SPI_RX_QOF   1 << 3
+#define SPI_RX_QIOF  1 << 4
+#define SPI_RX_EXTN  SPI_RX_AS | SPI_RX_DOUT | SPI_RX_DIO | \
+                     SPI_RX_QOF | SPI_RX_QIOF
 
-#define SPI_DEFAULT_WORDLEN 8
+/* SPI mode bits */
+#define SPI_TX_QPP   1 << 0
+#define SPI_SHARED   1 << 1
+#define SPI_SEPARATED        1 << 2
+#define SPI_U_PAGE   1 << 3
 
 /**
  * struct spi_slave - Representation of a SPI slave

@@ -61,25 +57,22 @@
  *
  * @bus:             ID of the bus that the slave is attached to.

include/spi.h line 65

Quoted 10 lines

include/spi.h +2 -3

  * @cs:                      ID of the chip select connected to the slave.
- * @op_mode_rx:              SPI RX operation mode.
- * @op_mode_tx:              SPI TX operation mode.
  * @wordlen:         Size of SPI word in number of bits
  * @max_write_size:  If non-zero, the maximum number of bytes which can
  *                   be written at once, excluding command bytes.
  * @memory_map:              Address of read-only SPI flash access.
- * @option:          Varies SPI bus options - separate, shared bus.
+ * @rx_mode:         SPI RX operation mode.
+ * @mode_bits:               SPI TX operation modes, bus options and few

flags.

This naming change doesn't make sense. rx_mode vs. mode_bits don't have any relationship even if they are related apparently.

Anyway, I feel we're sinking deeper and deeper into this sh*t, we should instead take a step back and re-think the whole approach until we break it even more.

Quoted 4 lines · Marek Vasut

Anyway, I feel we're sinking deeper and deeper into this sh*t, we should instead take a step back and re-think the whole approach until we break it even more.

Yes - will shrink once we plan for new approach. But I'm unclear with new SPI-NOR.

Quoted 5 lines · Jagan Teki

Quoted 3 lines · Marek Vasut

Anyway, I feel we're sinking deeper and deeper into this sh*t, we should instead take a step back and re-think the whole approach until we break it even more.

Quoted 2 lines

Yes - will shrink once we plan for new approach. But I'm unclear with new SPI-NOR.

Regarding this specific patch: I assume what Marek suggested was to restrict the "SPI slave" information to what's specific to an SPI slave. It's just not true that every SPI slave is a flash chip (an assumption which QSPI developers appear to fall for rather easily).

I was about to make a similar comment, that trimming the identifiers so rigorously leads to code that only "initiated" people can read.

Even those who want to learn have no chance,
there would not be a legend of any kind (except for the commit message, which soon is buried and not obvious to look up). And even with the legend, it's tedious to train the casual co-developer to those specific abbreviations, which may not even be in wide spread use outside of the U-Boot code base.

So I agree with Marek that we should take a deep breath, and be aware of the consequences before taking a specific direction (and having a clear direction would also be beneficial).

A more involved answer I will send to the quad SPI thread.

Quoted 1 line · Marek Vasut

On Friday, January 17, 2014 at 03:41:46 PM, Jagannadha Sutradharudu Teki

Quoted 302 lines

Quoted 1 line

Combined spi flash stuff as minimum as possible.

Squashing the names of the flags only makes it unreadable :-(

Quoted 130 lines

Signed-off-by Jagannadha Sutradharudu Teki <[email protected]>

Cc Marek Vasut <[email protected]>

---

 drivers/mtd/spi/sf.c       |  4 ++--
 drivers/mtd/spi/sf_ops.c   | 18 +++++++++---------
 drivers/mtd/spi/sf_probe.c | 19 ++++++++++++-------
 include/spi.h              | 45

+++++++++++++++++++-------------------------- include/spi_flash.h |

6 +++---

 5 files changed, 45 insertions(+), 47 deletions(-)

drivers/mtd/spi/sf.c +2 -2

index 664e860..fb91ac6 100644
@@ -19,8 +19,8 @@ static int spi_flash_read_write(struct spi_slave *spi,
 
      int ret;
 
 #ifdef CONFIG_SF_DUAL_FLASH
 
-     if (spi->flags & SPI_XFER_U_PAGE)
-             flags |= SPI_XFER_U_PAGE;
+     if (spi->mode_bits & SPI_U_PAGE)
+             flags |= SPI_U_PAGE;
 

#endif

      if (data_len == 0)
      
              flags |= SPI_XFER_END;

drivers/mtd/spi/sf_ops.c +6 -6

index 9650486..0d87e1b 100644
@@ -135,15 +135,15 @@ static int spi_flash_bank(struct spi_flash *flash,
 u32 offset) static void spi_flash_dual_flash(struct spi_flash *flash,
 u32 *addr) {
 
      switch (flash->dual_flash) {
 
-     case SF_DUAL_STACKED_FLASH:
 
+     case SF_STACKED:
              if (*addr >= (flash->size >> 1)) {
 
                      *addr -= flash->size >> 1;
 
-                     flash->spi->flags |= SPI_XFER_U_PAGE;
+                     flash->spi->mode_bits |= SPI_U_PAGE;
 
              } else {
 
-                     flash->spi->flags &= ~SPI_XFER_U_PAGE;
+                     flash->spi->mode_bits &= ~SPI_U_PAGE;
 
              }
              break;
 
-     case SF_DUAL_PARALLEL_FLASH:
 
+     case SF_PARALLEL:
              *addr >>= flash->shift;
              break;
 
      default:

@@ -170,8 +170,8 @@ int spi_flash_cmd_wait_ready(struct spi_flash
 *flash, unsigned long timeout) }
 
 #ifdef CONFIG_SF_DUAL_FLASH
 
-     if (spi->flags & SPI_XFER_U_PAGE)
-             flags |= SPI_XFER_U_PAGE;
+     if (spi->mode_bits & SPI_U_PAGE)
+             flags |= SPI_U_PAGE;
 
 #endif

 
      ret = spi_xfer(spi, 8, &cmd, NULL, flags);
      if (ret) {

drivers/mtd/spi/sf_ops.c +1 -1

@@ -261,7 +261,7 @@ int spi_flash_cmd_erase_ops(struct spi_flash *flash,
 u32 offset, size_t len) erase_addr = offset;
 
 #ifdef CONFIG_SF_DUAL_FLASH
 
-             if (flash->dual_flash > SF_SINGLE_FLASH)
+             if (flash->dual_flash > SF_SINGLE)
 
                      spi_flash_dual_flash(flash, &erase_addr);

#endif
#ifdef CONFIG_SPI_FLASH_BAR

drivers/mtd/spi/sf_ops.c +1 -1

@@ -303,7 +303,7 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash,
 u32 offset, write_addr = offset;
 
 #ifdef CONFIG_SF_DUAL_FLASH
 
-             if (flash->dual_flash > SF_SINGLE_FLASH)
+             if (flash->dual_flash > SF_SINGLE)
 
                      spi_flash_dual_flash(flash, &write_addr);

#endif
#ifdef CONFIG_SPI_FLASH_BAR

drivers/mtd/spi/sf_ops.c +1 -1

@@ -388,7 +388,7 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash,
 u32 offset, read_addr = offset;
 
 #ifdef CONFIG_SF_DUAL_FLASH
 
-             if (flash->dual_flash > SF_SINGLE_FLASH)
+             if (flash->dual_flash > SF_SINGLE)
 
                      spi_flash_dual_flash(flash, &read_addr);

#endif
#ifdef CONFIG_SPI_FLASH_BAR

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

index e84ab13..003f635 100644

drivers/mtd/spi/sf_probe.c static struct spi_flash · line 139

Quoted 8 lines

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

@@ -133,8 +133,13 @@ static struct spi_flash
 *spi_flash_validate_params(struct spi_slave *spi, flash->spi = spi;
 
      flash->name = params->name;
      flash->memory_map = spi->memory_map;
 
-     flash->dual_flash = flash->spi->option;
 
+#ifdef CONFIG_SF_DUAL_FLASH

Looks new and unrelated.

Quoted 153 lines

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

+     if (flash->spi->mode_bits & SPI_SHARED)
+             flash->dual_flash = SF_STACKED;
+     else if (flash->spi->mode_bits & SPI_SEPARATED)
+             flash->dual_flash = SF_PARALLEL;
+#endif
 

      /* Assign spi_flash ops */
      flash->write = spi_flash_cmd_write_ops;

#ifdef CONFIG_SPI_FLASH_SST

drivers/mtd/spi/sf_probe.c +2 -3

@@ -145,12 +150,12 @@ static struct spi_flash
 *spi_flash_validate_params(struct spi_slave *spi, flash->read =
 spi_flash_cmd_read_ops;
 
      /* Compute the flash size */
 
-     flash->shift = (flash->dual_flash & SF_DUAL_PARALLEL_FLASH) ? 1 :
 0; +     flash->shift = (flash->dual_flash & SF_PARALLEL) ? 1 : 0;
 
      flash->page_size = ((ext_jedec == 0x4d00) ? 512 : 256) <<
      flash->shift; flash->sector_size = params->sector_size <<
      flash->shift; flash->size = flash->sector_size *
      params->nr_sectors << flash->shift;
 
 #ifdef CONFIG_SF_DUAL_FLASH
 
-     if (flash->dual_flash & SF_DUAL_STACKED_FLASH)
+     if (flash->dual_flash & SF_STACKED)
 
              flash->size <<= 1;
 
 #endif

@@ -167,7 +172,7 @@ static struct spi_flash
 *spi_flash_validate_params(struct spi_slave *spi, }
 
      /* Look for the fastest read cmd */
 
-     cmd = fls(params->e_rd_cmd & flash->spi->op_mode_rx);
+     cmd = fls(params->e_rd_cmd & flash->spi->rx_mode);
 
      if (cmd) {

      
              cmd = spi_read_cmds_array[cmd - 1];
              flash->read_cmd = cmd;

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

@@ -177,7 +182,7 @@ static struct spi_flash
 *spi_flash_validate_params(struct spi_slave *spi, }
 
      /* Not require to look for fastest only two write cmds yet */
 
-     if (params->flags & WR_QPP && flash->spi->op_mode_tx &

SPI_OPM_TX_QPP) + if (params->flags & WR_QPP &&
flash->spi->mode_bits & SPI_TX_QPP)

              flash->write_cmd = CMD_QUAD_PAGE_PROGRAM;
      
      else
      
              /* Go for default supported write cmd */

drivers/mtd/spi/sf_probe.c +2 -2

@@ -329,9 +334,9 @@ static struct spi_flash
 *spi_flash_probe_slave(struct spi_slave *spi) puts("\n");
 
 #endif
 #ifndef CONFIG_SPI_FLASH_BAR
 
-     if (((flash->dual_flash == SF_SINGLE_FLASH) &&
+     if (((flash->dual_flash == SF_SINGLE) &&
 
           (flash->size > SPI_FLASH_16MB_BOUN)) ||
 
-          ((flash->dual_flash > SF_SINGLE_FLASH) &&
+          ((flash->dual_flash > SF_SINGLE) &&
 
           (flash->size > SPI_FLASH_16MB_BOUN << 1))) {
 
              puts("SF: Warning - Only lower 16MiB accessible,");
              puts(" Full access #define CONFIG_SPI_FLASH_BAR\n");

include/spi.h +0 -12

index ffd6647..07ba4d0 100644
@@ -30,29 +30,25 @@
 
 #define SPI_XFER_MMAP                0x08    /* Memory Mapped start */
 #define SPI_XFER_MMAP_END    0x10    /* Memory Mapped End */
 #define SPI_XFER_ONCE                (SPI_XFER_BEGIN | SPI_XFER_END)
 
-#define SPI_XFER_U_PAGE              (1 << 5)
-
-/* SPI TX operation modes */
-#define SPI_OPM_TX_QPP               1 << 0
-
-/* SPI RX operation modes */
-#define SPI_OPM_RX_AS                1 << 0
-#define SPI_OPM_RX_DOUT              1 << 1
-#define SPI_OPM_RX_DIO               1 << 2
-#define SPI_OPM_RX_QOF               1 << 3
-#define SPI_OPM_RX_QIOF              1 << 4
-#define SPI_OPM_RX_EXTN              SPI_OPM_RX_AS | SPI_OPM_RX_DOUT |

\ - SPI_OPM_RX_DIO | SPI_OPM_RX_QOF | \ -
SPI_OPM_RX_QIOF

include/spi.h +17 -8

-
-/* SPI bus connection options */
-#define SPI_CONN_DUAL_SHARED 1 << 0
-#define SPI_CONN_DUAL_SEPARATED      1 << 1
 
 /* Header byte that marks the start of the message */
 #define SPI_PREAMBLE_END_BYTE        0xec
 
+#define SPI_DEFAULT_WORDLEN  8
+
+/* SPI RX operation modes */
+#define SPI_RX_AS    1 << 0
+#define SPI_RX_DOUT  1 << 1
+#define SPI_RX_DIO   1 << 2
+#define SPI_RX_QOF   1 << 3
+#define SPI_RX_QIOF  1 << 4
+#define SPI_RX_EXTN  SPI_RX_AS | SPI_RX_DOUT | SPI_RX_DIO | \
+                     SPI_RX_QOF | SPI_RX_QIOF
 
-#define SPI_DEFAULT_WORDLEN 8
+/* SPI mode bits */
+#define SPI_TX_QPP   1 << 0
+#define SPI_SHARED   1 << 1
+#define SPI_SEPARATED        1 << 2
+#define SPI_U_PAGE   1 << 3
 
 /**
 
  * struct spi_slave - Representation of a SPI slave

@@ -61,25 +57,22 @@
 
  *
  * @bus:             ID of the bus that the slave is attached to.
  * @cs:                      ID of the chip select connected to the
  slave.
 
- * @op_mode_rx:              SPI RX operation mode.
- * @op_mode_tx:              SPI TX operation mode.
 
  * @wordlen:         Size of SPI word in number of bits
  * @max_write_size:  If non-zero, the maximum number of bytes which can
  *                   be written at once, excluding command bytes.
  * @memory_map:              Address of read-only SPI flash access.
 
- * @option:          Varies SPI bus options - separate, shared bus.
+ * @rx_mode:         SPI RX operation mode.
+ * @mode_bits:               SPI TX operation modes, bus options and

Quoted 1 line

few

flags.

Quoted 2 lines

This naming change doesn't make sense. rx_mode vs. mode_bits don't have any relationship even if they are related apparently.

rx_mode need to separate here as it involve some calculation to find fastest command.

Quoted 3 lines

Anyway, I feel we're sinking deeper and deeper into this sh*t, we should instead take a step back and re-think the whole approach until we break it even more.

Yes - will shrink once we plan for new approach.
But I'm unclear with new SPI-NOR.

- SPI_FLASH -> SF
- ARRAY_SLOW -> AS
- ARRAY_FAST -> AF
- DUAL_OUTPUT_FAST -> DOF
- DUAL_IO_FAST - DIOF
- QUAD_OUTPUT_FAST - QOF
- QUAD_IO_FAST - QIOF

Quoted 7 lines · Jagannadha Sutradharudu Teki

- SPI_FLASH -> SF
- ARRAY_SLOW -> AS
- ARRAY_FAST -> AF
- DUAL_OUTPUT_FAST -> DOF
- DUAL_IO_FAST - DIOF
- QUAD_OUTPUT_FAST - QOF
- QUAD_IO_FAST - QIOF

Now this really makes the code impossible to understand :-(

Hi Jagan and Marek,

Quoted 10 lines

Quoted 7 lines · Jagannadha Sutradharudu Teki

- SPI_FLASH -> SF
- ARRAY_SLOW -> AS
- ARRAY_FAST -> AF
- DUAL_OUTPUT_FAST -> DOF
- DUAL_IO_FAST - DIOF
- QUAD_OUTPUT_FAST - QOF
- QUAD_IO_FAST - QIOF

Quoted 1 line

Now this really makes the code impossible to understand :-(

I totally agree with Marek that this is the wrong way to go. Doing a change like this is a change to the worse as it removes understandable constants and replaces them with adhoc abbreviations.

Please don't do that.