[U-Boot] [PATCH 2/3] sf: Shrink spi_slave {}
Combined spi flash stuff as minimum as possible.
Signed-off-by: Jagannadha Sutradharudu Teki jaganna@xilinx.com Cc: Marek Vasut marex@denx.de --- 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(-)
diff --git a/drivers/mtd/spi/sf.c b/drivers/mtd/spi/sf.c index 664e860..fb91ac6 100644 --- a/drivers/mtd/spi/sf.c +++ b/drivers/mtd/spi/sf.c @@ -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; diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c index 9650486..0d87e1b 100644 --- a/drivers/mtd/spi/sf_ops.c +++ b/drivers/mtd/spi/sf_ops.c @@ -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) { @@ -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 diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c index e84ab13..003f635 100644 --- a/drivers/mtd/spi/sf_probe.c +++ b/drivers/mtd/spi/sf_probe.c @@ -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 + 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"); diff --git a/include/spi.h b/include/spi.h index ffd6647..07ba4d0 100644 --- a/include/spi.h +++ b/include/spi.h @@ -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. * @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. * @flags: Indication of SPI flags. */ struct spi_slave { unsigned int bus; unsigned int cs; - u8 op_mode_rx; - u8 op_mode_tx; unsigned int wordlen; unsigned int max_write_size; + u16 rx_mode; + u16 mode_bits; void *memory_map; - u8 option; - u8 flags; };
/** diff --git a/include/spi_flash.h b/include/spi_flash.h index f79f0ea..b4b2f9b 100644 --- a/include/spi_flash.h +++ b/include/spi_flash.h @@ -38,9 +38,9 @@ enum spi_read_cmds {
/* Dual SPI flash memories */ enum spi_dual_flash { - SF_SINGLE_FLASH = 0, - SF_DUAL_STACKED_FLASH = 1 << 0, - SF_DUAL_PARALLEL_FLASH = 1 << 1, + SF_SINGLE = 0, + SF_STACKED = 1 << 0, + SF_PARALLEL = 1 << 1, };
/**
On Friday, January 17, 2014 at 03:41:46 PM, Jagannadha Sutradharudu Teki wrote:
Combined spi flash stuff as minimum as possible.
Squashing the names of the flags only makes it unreadable :-(
Signed-off-by: Jagannadha Sutradharudu Teki jaganna@xilinx.com Cc: Marek Vasut marex@denx.de
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(-)
diff --git a/drivers/mtd/spi/sf.c b/drivers/mtd/spi/sf.c index 664e860..fb91ac6 100644 --- a/drivers/mtd/spi/sf.c +++ b/drivers/mtd/spi/sf.c @@ -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; diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c index 9650486..0d87e1b 100644 --- a/drivers/mtd/spi/sf_ops.c +++ b/drivers/mtd/spi/sf_ops.c @@ -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;
flash->spi->flags &= ~SPI_XFER_U_PAGE;
flash->spi->mode_bits &= ~SPI_U_PAGE;
- 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 diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c index e84ab13..003f635 100644 --- a/drivers/mtd/spi/sf_probe.c +++ b/drivers/mtd/spi/sf_probe.c @@ -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.
- 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))) {diff --git a/include/spi.h b/include/spi.h index ffd6647..07ba4d0 100644 --- a/include/spi.h +++ b/include/spi.h @@ -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.
- @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.
[...]
On Fri, Jan 17, 2014 at 10:01 PM, Marek Vasut marex@denx.de wrote:
On Friday, January 17, 2014 at 03:41:46 PM, Jagannadha Sutradharudu Teki wrote:
Combined spi flash stuff as minimum as possible.
Squashing the names of the flags only makes it unreadable :-(
Signed-off-by: Jagannadha Sutradharudu Teki jaganna@xilinx.com Cc: Marek Vasut marex@denx.de
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(-)
diff --git a/drivers/mtd/spi/sf.c b/drivers/mtd/spi/sf.c index 664e860..fb91ac6 100644 --- a/drivers/mtd/spi/sf.c +++ b/drivers/mtd/spi/sf.c @@ -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; diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c index 9650486..0d87e1b 100644 --- a/drivers/mtd/spi/sf_ops.c +++ b/drivers/mtd/spi/sf_ops.c @@ -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) { @@ -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 diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c index e84ab13..003f635 100644 --- a/drivers/mtd/spi/sf_probe.c +++ b/drivers/mtd/spi/sf_probe.c @@ -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.
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");diff --git a/include/spi.h b/include/spi.h index ffd6647..07ba4d0 100644 --- a/include/spi.h +++ b/include/spi.h @@ -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.
- @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.
rx_mode need to separate here as it involve some calculation to find fastest command.
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.
On Fri, Jan 17, 2014 at 22:42 +0530, Jagan Teki wrote:
On Fri, Jan 17, 2014 at 10:01 PM, Marek Vasut marex@denx.de wrote:
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.
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.
virtually yours Gerhard Sittig
On Friday, January 17, 2014 at 07:29:54 PM, Gerhard Sittig wrote:
On Fri, Jan 17, 2014 at 22:42 +0530, Jagan Teki wrote:
On Fri, Jan 17, 2014 at 10:01 PM, Marek Vasut marex@denx.de wrote:
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.
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).
Heh, really ? :) Otherwise I agree with you.
btw. I honestly don't quite understand this inclination to building separate SPI NOR controller instead of building full-fat SPI bus controller :(
I was about to make a similar comment, that trimming the identifiers so rigorously leads to code that only "initiated" people can read.
OK, I have to admit I am rather blunt and my rambling may sound nasty. Please don't take it personally ;-)
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.
Thanks for expainding so and please keep me in the loop on the qspi :)
Best regards, Marek Vasut
On Friday, January 17, 2014 at 06:12:49 PM, Jagan Teki wrote:
On Fri, Jan 17, 2014 at 10:01 PM, Marek Vasut marex@denx.de wrote:
On Friday, January 17, 2014 at 03:41:46 PM, Jagannadha Sutradharudu Teki
wrote:
Combined spi flash stuff as minimum as possible.
Squashing the names of the flags only makes it unreadable :-(
Signed-off-by: Jagannadha Sutradharudu Teki jaganna@xilinx.com Cc: Marek Vasut marex@denx.de
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(-)
diff --git a/drivers/mtd/spi/sf.c b/drivers/mtd/spi/sf.c index 664e860..fb91ac6 100644 --- a/drivers/mtd/spi/sf.c +++ b/drivers/mtd/spi/sf.c @@ -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;diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c index 9650486..0d87e1b 100644 --- a/drivers/mtd/spi/sf_ops.c +++ b/drivers/mtd/spi/sf_ops.c @@ -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) {@@ -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
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c index e84ab13..003f635 100644 --- a/drivers/mtd/spi/sf_probe.c +++ b/drivers/mtd/spi/sf_probe.c @@ -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.
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");diff --git a/include/spi.h b/include/spi.h index ffd6647..07ba4d0 100644 --- a/include/spi.h +++ b/include/spi.h @@ -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.
- @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.
rx_mode need to separate here as it involve some calculation to find fastest command.
... and this is really starting to become horrible nonsense. Fastest command for SPI NOR, right ? But this is struct spi_slave {} describing generic SPI slave here, not an SPI NOR!
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.
It's pretty clear in it's own right, do not polute generic code with SPI NOR specific stuff .
participants (4)
-
Gerhard Sittig -
Jagan Teki -
Jagannadha Sutradharudu Teki -
Marek Vasut