* [U-Boot] [PATCH 1/6] sf: ops: Squash the malloc+memset combo
[not found] <1390075593-11226-1-git-send-email-jaganna@xilinx.com>
@ 2014-01-18 20:06 ` Jagannadha Sutradharudu Teki
2014-01-18 20:34 ` Marek Vasut
2014-01-18 20:06 ` [U-Boot] [PATCH 2/6] sf: Optimize flash features code Jagannadha Sutradharudu Teki
` (4 subsequent siblings)
5 siblings, 1 reply; 27+ messages in thread
From: Jagannadha Sutradharudu Teki @ 2014-01-18 20:06 UTC (permalink / raw)
To: u-boot
Squash the malloc()+memset() combo in favor of calloc().
Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
Cc: Marek Vasut <marex@denx.de>
---
drivers/mtd/spi/sf_ops.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
index 1f1bb36..1fac63a 100644
--- a/drivers/mtd/spi/sf_ops.c
+++ b/drivers/mtd/spi/sf_ops.c
@@ -381,8 +381,11 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
}
cmdsz = SPI_FLASH_CMD_LEN + flash->dummy_byte;
- cmd = malloc(cmdsz);
- memset(cmd, 0, cmdsz);
+ cmd = calloc(1, cmdsz);
+ if (!cmd) {
+ printf("SF: Failed to allocate cmd\n");
+ return ret;
+ }
cmd[0] = flash->read_cmd;
while (len) {
--
1.8.3
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/6] sf: Optimize flash features code
[not found] <1390075593-11226-1-git-send-email-jaganna@xilinx.com>
2014-01-18 20:06 ` [U-Boot] [PATCH 1/6] sf: ops: Squash the malloc+memset combo Jagannadha Sutradharudu Teki
@ 2014-01-18 20:06 ` Jagannadha Sutradharudu Teki
2014-01-18 20:39 ` Marek Vasut
2014-01-18 20:06 ` [U-Boot] [PATCH 3/6] sf: Renames on dual_flash stuff Jagannadha Sutradharudu Teki
` (3 subsequent siblings)
5 siblings, 1 reply; 27+ messages in thread
From: Jagannadha Sutradharudu Teki @ 2014-01-18 20:06 UTC (permalink / raw)
To: u-boot
- Shrink spi_slave {}
- Shrink spi_flash_params {}
- Documentation for sf features
Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
Cc: Marek Vasut <marex@denx.de>
---
doc/SPI/README.sf-features | 122 ++++++++++++++++++++++++++++++
drivers/mtd/spi/sf.c | 4 +-
drivers/mtd/spi/sf_internal.h | 1 -
drivers/mtd/spi/sf_ops.c | 8 +-
drivers/mtd/spi/sf_params.c | 172 +++++++++++++++++++++---------------------
drivers/mtd/spi/sf_probe.c | 71 ++++++++---------
include/spi.h | 42 ++++-------
include/spi_flash.h | 24 +++---
8 files changed, 270 insertions(+), 174 deletions(-)
create mode 100644 doc/SPI/README.sf-features
diff --git a/doc/SPI/README.sf-features b/doc/SPI/README.sf-features
new file mode 100644
index 0000000..d35f56d
--- /dev/null
+++ b/doc/SPI/README.sf-features
@@ -0,0 +1,122 @@
+SPI FLASH feature enhancements:
+==============================
+
+This document describes how to extend the current data structures in spi subsystem
+for making use of new flash features/operations w.r.t to controller driver support.
+
+1. spi_slave:
+
+struct spi_slave {
+ ..........
+ u32 mode_bits;
+ ........
+};
+
+ at mode_bits can be used to expose the SPI RX/TX operation modes, bus options and
+few flags which are used to extended the flash specific features/operations
+- include/spi.h
+
+mode_bits:
+- SPI_TX_QPP: 4-Wire tx transfer operation quad page program
+- SPI_RX_SLOW: 1-wire rx transfer operation array slow read
+- SPI_RX_DUAL: 2-wire rx transfer operation dual fast read
+- SPI_RX_DUAL_IO: 2-wire rx transfer operation dual io fast read
+- SPI_RX_QUAD: 4-wire rx transfer operation quad fast read
+- SPI_RX_QUAD_IO: 4-wire rx transfer operation quad io fast read
+- SPI_SHARED: dual flash devices are connected in shared bus connection
+- SPI_SEPARATED: dual flash devices are connected in separate bus connection
+- SPI_U_PAGE: select the upper flash in dual flash shared bus connection [1]
+
+2. spi_flash_params:
+
+struct spi_flash_params {
+ ................
+ u16 flags;
+ ..............
+};
+
+ at flags can be use to verify the flash supported features/operations with respect
+to controller driven through @mode_bits and also some internal flash specific
+operations - include/spi_flash.h
+
+flags:
+- SST_WP: SST flash write protection
+- SECT_4K: 4K erase sector
+- SECT_32K: 32 erase sector
+- E_FSR: Flag status register for erase/write for micron < 256MB flash
+- WR_QPP: Quad page program
+- RD_SLOW: Array slow read
+- RD_DUAL: Dual fast read
+- RD_DUAL_IO: Dual IO read
+- RD_QUAD: Quad fast read
+- RD_QUAD_IO: Quad IO read
+- RD_2WIRE: All 2-wire read commands
+- RD_FULL: All read commands
+- ALL_CMDS: All read and write commands [2]
+
+3. spi_flash:
+
+struct spi_flash {
+ ...............
+ u8 dual_flash;
+ u8 shift;
+ u8 poll_cmd;
+ u8 erase_cmd;
+ u8 read_cmd;
+ u8 write_cmd;
+ u8 dummy_byte;
+ ................
+};
+
+Few varibles from spi_flash {} can be used to perform the internal operations
+based on the selected flash features/operations from spi_slave {} and
+spi_flash_params {} - include/spi_flash.h
+
+ at dual_flash: flash can be operated in dual flash [3]
+ at shift: variable shift operator useful for dual parallel
+ at poll_cmd: find the read_status or flag_status for polling erase/write operations
+ at erase_cmd: discovered erase command
+ at read_cmd: discovered read command
+ at write_cmd: discovered write command
+@dummy_byte: read dummy_byte based on read dummy_cycles.
+
+dummy byte is determined based on the dummy cycles of a particular command.
+Fast commands - dummy_byte = dummy_cycles/8
+I/O commands- dummy_byte = (dummy_cycles * no.of lines)/8
+For I/O commands except cmd[0] everything goes on no.of lines based on
+particular command but in case of fast commands except data all go on
+single line irrespective of command.
+
+4. Usage:
+
+In drivers/spi/*.c assign spi_slave {} with supported features through mode_bits.
+Ex: drivers/spi/ti_qspi.c
+
+struct ti_qspi_slave {
+ ................
+ struct spi_slave slave;
+ ..............
+}
+
+struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
+ unsigned int max_hz, unsigned int mode)
+{
+ .........
+
+ qslave = spi_alloc_slave(struct ti_qspi_slave, bus, cs);
+ if (!qslave) {
+ printf("SPI_error: Fail to allocate ti_qspi_slave\n");
+ return NULL;
+ }
+
+ qslave->slave.mode_bits = SPI_TX_QPP | SPI_RX_QUAD;
+ ........
+}
+
+[1] http://www.xilinx.com/support/documentation/user_guides/ug585-Zynq-7000-TRM.pdf
+[2] http://www.spansion.com/Support/Datasheets/S25FL128S_256S_00.pdf
+[3] doc/SPI/README.dual-flash
+
+--
+Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
+Sat Jan 18 14:44:28 IST 2014
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_internal.h b/drivers/mtd/spi/sf_internal.h
index 6bcd522..47d5ac2 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -67,7 +67,6 @@
/* SST specific */
#ifdef CONFIG_SPI_FLASH_SST
-# define SST_WP 0x01 /* Supports AAI word program */
# define CMD_SST_BP 0x02 /* Byte Program */
# define CMD_SST_AAI_WP 0xAD /* Auto Address Incr Word Program */
diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
index 1fac63a..2f3b03a 100644
--- a/drivers/mtd/spi/sf_ops.c
+++ b/drivers/mtd/spi/sf_ops.c
@@ -138,9 +138,9 @@ static void spi_flash_dual_flash(struct spi_flash *flash, u32 *addr)
case SF_DUAL_STACKED_FLASH:
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:
@@ -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) {
diff --git a/drivers/mtd/spi/sf_params.c b/drivers/mtd/spi/sf_params.c
index eb372b7..9b394f2 100644
--- a/drivers/mtd/spi/sf_params.c
+++ b/drivers/mtd/spi/sf_params.c
@@ -14,106 +14,106 @@
/* SPI/QSPI flash device params structure */
const struct spi_flash_params spi_flash_params_table[] = {
#ifdef CONFIG_SPI_FLASH_ATMEL /* ATMEL */
- {"AT45DB011D", 0x1f2200, 0x0, 64 * 1024, 4, 0, SECT_4K},
- {"AT45DB021D", 0x1f2300, 0x0, 64 * 1024, 8, 0, SECT_4K},
- {"AT45DB041D", 0x1f2400, 0x0, 64 * 1024, 8, 0, SECT_4K},
- {"AT45DB081D", 0x1f2500, 0x0, 64 * 1024, 16, 0, SECT_4K},
- {"AT45DB161D", 0x1f2600, 0x0, 64 * 1024, 32, 0, SECT_4K},
- {"AT45DB321D", 0x1f2700, 0x0, 64 * 1024, 64, 0, SECT_4K},
- {"AT45DB641D", 0x1f2800, 0x0, 64 * 1024, 128, 0, SECT_4K},
- {"AT25DF321", 0x1f4701, 0x0, 64 * 1024, 64, 0, SECT_4K},
+ {"AT45DB011D", 0x1f2200, 0x0, 64 * 1024, 4, SECT_4K},
+ {"AT45DB021D", 0x1f2300, 0x0, 64 * 1024, 8, SECT_4K},
+ {"AT45DB041D", 0x1f2400, 0x0, 64 * 1024, 8, SECT_4K},
+ {"AT45DB081D", 0x1f2500, 0x0, 64 * 1024, 16, SECT_4K},
+ {"AT45DB161D", 0x1f2600, 0x0, 64 * 1024, 32, SECT_4K},
+ {"AT45DB321D", 0x1f2700, 0x0, 64 * 1024, 64, SECT_4K},
+ {"AT45DB641D", 0x1f2800, 0x0, 64 * 1024, 128, SECT_4K},
+ {"AT25DF321", 0x1f4701, 0x0, 64 * 1024, 64, SECT_4K},
#endif
#ifdef CONFIG_SPI_FLASH_EON /* EON */
- {"EN25Q32B", 0x1c3016, 0x0, 64 * 1024, 64, 0, 0},
- {"EN25Q64", 0x1c3017, 0x0, 64 * 1024, 128, 0, SECT_4K},
- {"EN25Q128B", 0x1c3018, 0x0, 64 * 1024, 256, 0, 0},
- {"EN25S64", 0x1c3817, 0x0, 64 * 1024, 128, 0, 0},
+ {"EN25Q32B", 0x1c3016, 0x0, 64 * 1024, 64, 0},
+ {"EN25Q64", 0x1c3017, 0x0, 64 * 1024, 128, SECT_4K},
+ {"EN25Q128B", 0x1c3018, 0x0, 64 * 1024, 256, 0},
+ {"EN25S64", 0x1c3817, 0x0, 64 * 1024, 128, 0},
#endif
#ifdef CONFIG_SPI_FLASH_GIGADEVICE /* GIGADEVICE */
- {"GD25Q64B", 0xc84017, 0x0, 64 * 1024, 128, 0, SECT_4K},
- {"GD25LQ32", 0xc86016, 0x0, 64 * 1024, 64, 0, SECT_4K},
+ {"GD25Q64B", 0xc84017, 0x0, 64 * 1024, 128, SECT_4K},
+ {"GD25LQ32", 0xc86016, 0x0, 64 * 1024, 64, SECT_4K},
#endif
#ifdef CONFIG_SPI_FLASH_MACRONIX /* MACRONIX */
- {"MX25L2006E", 0xc22012, 0x0, 64 * 1024, 4, 0, 0},
- {"MX25L4005", 0xc22013, 0x0, 64 * 1024, 8, 0, 0},
- {"MX25L8005", 0xc22014, 0x0, 64 * 1024, 16, 0, 0},
- {"MX25L1605D", 0xc22015, 0x0, 64 * 1024, 32, 0, 0},
- {"MX25L3205D", 0xc22016, 0x0, 64 * 1024, 64, 0, 0},
- {"MX25L6405D", 0xc22017, 0x0, 64 * 1024, 128, 0, 0},
- {"MX25L12805", 0xc22018, 0x0, 64 * 1024, 256, RD_FULL, WR_QPP},
- {"MX25L25635F", 0xc22019, 0x0, 64 * 1024, 512, RD_FULL, WR_QPP},
- {"MX25L51235F", 0xc2201a, 0x0, 64 * 1024, 1024, RD_FULL, WR_QPP},
- {"MX25L12855E", 0xc22618, 0x0, 64 * 1024, 256, RD_FULL, WR_QPP},
+ {"MX25L2006E", 0xc22012, 0x0, 64 * 1024, 4, 0},
+ {"MX25L4005", 0xc22013, 0x0, 64 * 1024, 8, 0},
+ {"MX25L8005", 0xc22014, 0x0, 64 * 1024, 16, 0},
+ {"MX25L1605D", 0xc22015, 0x0, 64 * 1024, 32, 0},
+ {"MX25L3205D", 0xc22016, 0x0, 64 * 1024, 64, 0},
+ {"MX25L6405D", 0xc22017, 0x0, 64 * 1024, 128, 0},
+ {"MX25L12805", 0xc22018, 0x0, 64 * 1024, 256, ALL_CMDS},
+ {"MX25L25635F", 0xc22019, 0x0, 64 * 1024, 512, ALL_CMDS},
+ {"MX25L51235F", 0xc2201a, 0x0, 64 * 1024, 1024, ALL_CMDS},
+ {"MX25L12855E", 0xc22618, 0x0, 64 * 1024, 256, ALL_CMDS},
#endif
#ifdef CONFIG_SPI_FLASH_SPANSION /* SPANSION */
- {"S25FL008A", 0x010213, 0x0, 64 * 1024, 16, 0, 0},
- {"S25FL016A", 0x010214, 0x0, 64 * 1024, 32, 0, 0},
- {"S25FL032A", 0x010215, 0x0, 64 * 1024, 64, 0, 0},
- {"S25FL064A", 0x010216, 0x0, 64 * 1024, 128, 0, 0},
- {"S25FL128P_256K", 0x012018, 0x0300, 256 * 1024, 64, RD_FULL, WR_QPP},
- {"S25FL128P_64K", 0x012018, 0x0301, 64 * 1024, 256, RD_FULL, WR_QPP},
- {"S25FL032P", 0x010215, 0x4d00, 64 * 1024, 64, RD_FULL, WR_QPP},
- {"S25FL064P", 0x010216, 0x4d00, 64 * 1024, 128, RD_FULL, WR_QPP},
- {"S25FL128S_256K", 0x012018, 0x4d00, 256 * 1024, 64, RD_FULL, WR_QPP},
- {"S25FL128S_64K", 0x012018, 0x4d01, 64 * 1024, 256, RD_FULL, WR_QPP},
- {"S25FL256S_256K", 0x010219, 0x4d00, 256 * 1024, 128, RD_FULL, WR_QPP},
- {"S25FL256S_64K", 0x010219, 0x4d01, 64 * 1024, 512, RD_FULL, WR_QPP},
- {"S25FL512S_256K", 0x010220, 0x4d00, 256 * 1024, 256, RD_FULL, WR_QPP},
- {"S25FL512S_64K", 0x010220, 0x4d01, 64 * 1024, 1024, RD_FULL, WR_QPP},
+ {"S25FL008A", 0x010213, 0x0, 64 * 1024, 16, 0},
+ {"S25FL016A", 0x010214, 0x0, 64 * 1024, 32, 0},
+ {"S25FL032A", 0x010215, 0x0, 64 * 1024, 64, 0},
+ {"S25FL064A", 0x010216, 0x0, 64 * 1024, 128, 0},
+ {"S25FL128P_256K", 0x012018, 0x0300, 256 * 1024, 64, ALL_CMDS},
+ {"S25FL128P_64K", 0x012018, 0x0301, 64 * 1024, 256, ALL_CMDS},
+ {"S25FL032P", 0x010215, 0x4d00, 64 * 1024, 64, ALL_CMDS},
+ {"S25FL064P", 0x010216, 0x4d00, 64 * 1024, 128, ALL_CMDS},
+ {"S25FL128S_256K", 0x012018, 0x4d00, 256 * 1024, 64, ALL_CMDS},
+ {"S25FL128S_64K", 0x012018, 0x4d01, 64 * 1024, 256, ALL_CMDS},
+ {"S25FL256S_256K", 0x010219, 0x4d00, 256 * 1024, 128, ALL_CMDS},
+ {"S25FL256S_64K", 0x010219, 0x4d01, 64 * 1024, 512, ALL_CMDS},
+ {"S25FL512S_256K", 0x010220, 0x4d00, 256 * 1024, 256, ALL_CMDS},
+ {"S25FL512S_64K", 0x010220, 0x4d01, 64 * 1024, 1024, ALL_CMDS},
#endif
#ifdef CONFIG_SPI_FLASH_STMICRO /* STMICRO */
- {"M25P10", 0x202011, 0x0, 32 * 1024, 4, 0, 0},
- {"M25P20", 0x202012, 0x0, 64 * 1024, 4, 0, 0},
- {"M25P40", 0x202013, 0x0, 64 * 1024, 8, 0, 0},
- {"M25P80", 0x202014, 0x0, 64 * 1024, 16, 0, 0},
- {"M25P16", 0x202015, 0x0, 64 * 1024, 32, 0, 0},
- {"M25P32", 0x202016, 0x0, 64 * 1024, 64, 0, 0},
- {"M25P64", 0x202017, 0x0, 64 * 1024, 128, 0, 0},
- {"M25P128", 0x202018, 0x0, 256 * 1024, 64, 0, 0},
- {"N25Q32", 0x20ba16, 0x0, 64 * 1024, 64, RD_FULL, WR_QPP | SECT_4K},
- {"N25Q32A", 0x20bb16, 0x0, 64 * 1024, 64, RD_FULL, WR_QPP | SECT_4K},
- {"N25Q64", 0x20ba17, 0x0, 64 * 1024, 128, RD_FULL, WR_QPP | SECT_4K},
- {"N25Q64A", 0x20bb17, 0x0, 64 * 1024, 128, RD_FULL, WR_QPP | SECT_4K},
- {"N25Q128", 0x20ba18, 0x0, 64 * 1024, 256, RD_FULL, WR_QPP},
- {"N25Q128A", 0x20bb18, 0x0, 64 * 1024, 256, RD_FULL, WR_QPP},
- {"N25Q256", 0x20ba19, 0x0, 64 * 1024, 512, RD_FULL, WR_QPP | SECT_4K},
- {"N25Q256A", 0x20bb19, 0x0, 64 * 1024, 512, RD_FULL, WR_QPP | SECT_4K},
- {"N25Q512", 0x20ba20, 0x0, 64 * 1024, 1024, RD_FULL, WR_QPP | E_FSR | SECT_4K},
- {"N25Q512A", 0x20bb20, 0x0, 64 * 1024, 1024, RD_FULL, WR_QPP | E_FSR | SECT_4K},
- {"N25Q1024", 0x20ba21, 0x0, 64 * 1024, 2048, RD_FULL, WR_QPP | E_FSR | SECT_4K},
- {"N25Q1024A", 0x20bb21, 0x0, 64 * 1024, 2048, RD_FULL, WR_QPP | E_FSR | SECT_4K},
+ {"M25P10", 0x202011, 0x0, 32 * 1024, 4, 0},
+ {"M25P20", 0x202012, 0x0, 64 * 1024, 4, 0},
+ {"M25P40", 0x202013, 0x0, 64 * 1024, 8, 0},
+ {"M25P80", 0x202014, 0x0, 64 * 1024, 16, 0},
+ {"M25P16", 0x202015, 0x0, 64 * 1024, 32, 0},
+ {"M25P32", 0x202016, 0x0, 64 * 1024, 64, 0},
+ {"M25P64", 0x202017, 0x0, 64 * 1024, 128, 0},
+ {"M25P128", 0x202018, 0x0, 256 * 1024, 64, 0},
+ {"N25Q32", 0x20ba16, 0x0, 64 * 1024, 64, ALL_CMDS | SECT_4K},
+ {"N25Q32A", 0x20bb16, 0x0, 64 * 1024, 64, ALL_CMDS | SECT_4K},
+ {"N25Q64", 0x20ba17, 0x0, 64 * 1024, 128, ALL_CMDS | SECT_4K},
+ {"N25Q64A", 0x20bb17, 0x0, 64 * 1024, 128, ALL_CMDS | SECT_4K},
+ {"N25Q128", 0x20ba18, 0x0, 64 * 1024, 256, ALL_CMDS},
+ {"N25Q128A", 0x20bb18, 0x0, 64 * 1024, 256, ALL_CMDS},
+ {"N25Q256", 0x20ba19, 0x0, 64 * 1024, 512, ALL_CMDS | SECT_4K},
+ {"N25Q256A", 0x20bb19, 0x0, 64 * 1024, 512, ALL_CMDS | SECT_4K},
+ {"N25Q512", 0x20ba20, 0x0, 64 * 1024, 1024, ALL_CMDS | E_FSR | SECT_4K},
+ {"N25Q512A", 0x20bb20, 0x0, 64 * 1024, 1024, ALL_CMDS | E_FSR | SECT_4K},
+ {"N25Q1024", 0x20ba21, 0x0, 64 * 1024, 2048, ALL_CMDS | E_FSR | SECT_4K},
+ {"N25Q1024A", 0x20bb21, 0x0, 64 * 1024, 2048, ALL_CMDS | E_FSR | SECT_4K},
#endif
#ifdef CONFIG_SPI_FLASH_SST /* SST */
- {"SST25VF040B", 0xbf258d, 0x0, 64 * 1024, 8, 0, SECT_4K | SST_WP},
- {"SST25VF080B", 0xbf258e, 0x0, 64 * 1024, 16, 0, SECT_4K | SST_WP},
- {"SST25VF016B", 0xbf2541, 0x0, 64 * 1024, 32, 0, SECT_4K | SST_WP},
- {"SST25VF032B", 0xbf254a, 0x0, 64 * 1024, 64, 0, SECT_4K | SST_WP},
- {"SST25VF064C", 0xbf254b, 0x0, 64 * 1024, 128, 0, SECT_4K},
- {"SST25WF512", 0xbf2501, 0x0, 64 * 1024, 1, 0, SECT_4K | SST_WP},
- {"SST25WF010", 0xbf2502, 0x0, 64 * 1024, 2, 0, SECT_4K | SST_WP},
- {"SST25WF020", 0xbf2503, 0x0, 64 * 1024, 4, 0, SECT_4K | SST_WP},
- {"SST25WF040", 0xbf2504, 0x0, 64 * 1024, 8, 0, SECT_4K | SST_WP},
- {"SST25WF080", 0xbf2505, 0x0, 64 * 1024, 16, 0, SECT_4K | SST_WP},
+ {"SST25VF040B", 0xbf258d, 0x0, 64 * 1024, 8, SECT_4K | SST_WP},
+ {"SST25VF080B", 0xbf258e, 0x0, 64 * 1024, 16, SECT_4K | SST_WP},
+ {"SST25VF016B", 0xbf2541, 0x0, 64 * 1024, 32, SECT_4K | SST_WP},
+ {"SST25VF032B", 0xbf254a, 0x0, 64 * 1024, 64, SECT_4K | SST_WP},
+ {"SST25VF064C", 0xbf254b, 0x0, 64 * 1024, 128, SECT_4K},
+ {"SST25WF512", 0xbf2501, 0x0, 64 * 1024, 1, SECT_4K | SST_WP},
+ {"SST25WF010", 0xbf2502, 0x0, 64 * 1024, 2, SECT_4K | SST_WP},
+ {"SST25WF020", 0xbf2503, 0x0, 64 * 1024, 4, SECT_4K | SST_WP},
+ {"SST25WF040", 0xbf2504, 0x0, 64 * 1024, 8, SECT_4K | SST_WP},
+ {"SST25WF080", 0xbf2505, 0x0, 64 * 1024, 16, SECT_4K | SST_WP},
#endif
#ifdef CONFIG_SPI_FLASH_WINBOND /* WINBOND */
- {"W25P80", 0xef2014, 0x0, 64 * 1024, 16, 0, 0},
- {"W25P16", 0xef2015, 0x0, 64 * 1024, 32, 0, 0},
- {"W25P32", 0xef2016, 0x0, 64 * 1024, 64, 0, 0},
- {"W25X40", 0xef3013, 0x0, 64 * 1024, 8, 0, SECT_4K},
- {"W25X16", 0xef3015, 0x0, 64 * 1024, 32, 0, SECT_4K},
- {"W25X32", 0xef3016, 0x0, 64 * 1024, 64, 0, SECT_4K},
- {"W25X64", 0xef3017, 0x0, 64 * 1024, 128, 0, SECT_4K},
- {"W25Q80BL", 0xef4014, 0x0, 64 * 1024, 16, RD_FULL, WR_QPP | SECT_4K},
- {"W25Q16CL", 0xef4015, 0x0, 64 * 1024, 32, RD_FULL, WR_QPP | SECT_4K},
- {"W25Q32BV", 0xef4016, 0x0, 64 * 1024, 64, RD_FULL, WR_QPP | SECT_4K},
- {"W25Q64CV", 0xef4017, 0x0, 64 * 1024, 128, RD_FULL, WR_QPP | SECT_4K},
- {"W25Q128BV", 0xef4018, 0x0, 64 * 1024, 256, RD_FULL, WR_QPP | SECT_4K},
- {"W25Q256", 0xef4019, 0x0, 64 * 1024, 512, RD_FULL, WR_QPP | SECT_4K},
- {"W25Q80BW", 0xef5014, 0x0, 64 * 1024, 16, RD_FULL, WR_QPP | SECT_4K},
- {"W25Q16DW", 0xef6015, 0x0, 64 * 1024, 32, RD_FULL, WR_QPP | SECT_4K},
- {"W25Q32DW", 0xef6016, 0x0, 64 * 1024, 64, RD_FULL, WR_QPP | SECT_4K},
- {"W25Q64DW", 0xef6017, 0x0, 64 * 1024, 128, RD_FULL, WR_QPP | SECT_4K},
- {"W25Q128FW", 0xef6018, 0x0, 64 * 1024, 256, RD_FULL, WR_QPP | SECT_4K},
+ {"W25P80", 0xef2014, 0x0, 64 * 1024, 16, 0},
+ {"W25P16", 0xef2015, 0x0, 64 * 1024, 32, 0},
+ {"W25P32", 0xef2016, 0x0, 64 * 1024, 64, 0},
+ {"W25X40", 0xef3013, 0x0, 64 * 1024, 8, SECT_4K},
+ {"W25X16", 0xef3015, 0x0, 64 * 1024, 32, SECT_4K},
+ {"W25X32", 0xef3016, 0x0, 64 * 1024, 64, SECT_4K},
+ {"W25X64", 0xef3017, 0x0, 64 * 1024, 128, SECT_4K},
+ {"W25Q80BL", 0xef4014, 0x0, 64 * 1024, 16, ALL_CMDS | SECT_4K},
+ {"W25Q16CL", 0xef4015, 0x0, 64 * 1024, 32, ALL_CMDS | SECT_4K},
+ {"W25Q32BV", 0xef4016, 0x0, 64 * 1024, 64, ALL_CMDS | SECT_4K},
+ {"W25Q64CV", 0xef4017, 0x0, 64 * 1024, 128, ALL_CMDS | SECT_4K},
+ {"W25Q128BV", 0xef4018, 0x0, 64 * 1024, 256, ALL_CMDS | SECT_4K},
+ {"W25Q256", 0xef4019, 0x0, 64 * 1024, 512, ALL_CMDS | SECT_4K},
+ {"W25Q80BW", 0xef5014, 0x0, 64 * 1024, 16, ALL_CMDS | SECT_4K},
+ {"W25Q16DW", 0xef6015, 0x0, 64 * 1024, 32, ALL_CMDS | SECT_4K},
+ {"W25Q32DW", 0xef6016, 0x0, 64 * 1024, 64, ALL_CMDS | SECT_4K},
+ {"W25Q64DW", 0xef6017, 0x0, 64 * 1024, 128, ALL_CMDS | SECT_4K},
+ {"W25Q128FW", 0xef6018, 0x0, 64 * 1024, 256, ALL_CMDS | SECT_4K},
#endif
/*
* Note:
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index e84ab13..d1ebf72 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -19,15 +19,6 @@
DECLARE_GLOBAL_DATA_PTR;
-/* Read commands array */
-static u8 spi_read_cmds_array[] = {
- CMD_READ_ARRAY_SLOW,
- CMD_READ_DUAL_OUTPUT_FAST,
- CMD_READ_DUAL_IO_FAST,
- CMD_READ_QUAD_OUTPUT_FAST,
- CMD_READ_QUAD_IO_FAST,
-};
-
#ifdef CONFIG_SPI_FLASH_MACRONIX
static int spi_flash_set_qeb_mxic(struct spi_flash *flash)
{
@@ -100,7 +91,6 @@ static struct spi_flash *spi_flash_validate_params(struct spi_slave *spi,
{
const struct spi_flash_params *params;
struct spi_flash *flash;
- u8 cmd;
u16 jedec = idcode[1] << 8 | idcode[2];
u16 ext_jedec = idcode[3] << 8 | idcode[4];
@@ -136,13 +126,13 @@ static struct spi_flash *spi_flash_validate_params(struct spi_slave *spi,
flash->dual_flash = flash->spi->option;
/* Assign spi_flash ops */
+ flash->read = spi_flash_cmd_read_ops;
+ flash->erase = spi_flash_cmd_erase_ops;
flash->write = spi_flash_cmd_write_ops;
#ifdef CONFIG_SPI_FLASH_SST
if (params->flags & SST_WP)
flash->write = sst_write_wp;
#endif
- flash->erase = spi_flash_cmd_erase_ops;
- flash->read = spi_flash_cmd_read_ops;
/* Compute the flash size */
flash->shift = (flash->dual_flash & SF_DUAL_PARALLEL_FLASH) ? 1 : 0;
@@ -166,18 +156,38 @@ static struct spi_flash *spi_flash_validate_params(struct spi_slave *spi,
flash->erase_size = flash->sector_size;
}
- /* Look for the fastest read cmd */
- cmd = fls(params->e_rd_cmd & flash->spi->op_mode_rx);
- if (cmd) {
- cmd = spi_read_cmds_array[cmd - 1];
- flash->read_cmd = cmd;
- } else {
- /* Go for default supported read cmd */
- flash->read_cmd = CMD_READ_ARRAY_FAST;
+ /* Compute read command and dummy_byte */
+ flash->read_cmd = CMD_READ_ARRAY_FAST;
+ flash->dummy_byte = 1;
+ switch (SPI_RX_MODES & flash->spi->mode_bits) {
+ case SPI_RX_SLOW:
+ if (params->flags & RD_SLOW) {
+ flash->read_cmd = CMD_READ_ARRAY_FAST;
+ flash->dummy_byte = 0;
+ }
+ break;
+ case SPI_RX_DUAL:
+ if (params->flags & RD_DUAL)
+ flash->read_cmd = CMD_READ_DUAL_OUTPUT_FAST;
+ break;
+ case SPI_RX_DUAL_IO:
+ if (params->flags & RD_DUAL_IO)
+ flash->read_cmd = CMD_READ_DUAL_OUTPUT_IO;
+ break;
+ case SPI_RX_QUAD:
+ if (params->flags & RD_QUAD)
+ flash->read_cmd = CMD_READ_QUAD_OUTPUT_FAST;
+ break;
+ case SPI_RX_QUAD_IO:
+ if (params->flags & RD_QUAD_IO) {
+ flash->read_cmd = CMD_READ_QUAD_OUTPUT_FAST_IO;
+ flash->dummy_byte = 2;
+ }
+ break;
}
/* 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 (flash->spi->mode_bits & SPI_TX_QPP && params->flags & WR_QPP)
flash->write_cmd = CMD_QUAD_PAGE_PROGRAM;
else
/* Go for default supported write cmd */
@@ -193,25 +203,6 @@ static struct spi_flash *spi_flash_validate_params(struct spi_slave *spi,
}
}
- /* Read dummy_byte: dummy byte is determined based on the
- * dummy cycles of a particular command.
- * Fast commands - dummy_byte = dummy_cycles/8
- * I/O commands- dummy_byte = (dummy_cycles * no.of lines)/8
- * For I/O commands except cmd[0] everything goes on no.of lines
- * based on particular command but incase of fast commands except
- * data all go on single line irrespective of command.
- */
- switch (flash->read_cmd) {
- case CMD_READ_QUAD_IO_FAST:
- flash->dummy_byte = 2;
- break;
- case CMD_READ_ARRAY_SLOW:
- flash->dummy_byte = 0;
- break;
- default:
- flash->dummy_byte = 1;
- }
-
/* Poll cmd selection */
flash->poll_cmd = CMD_READ_STATUS;
#ifdef CONFIG_SPI_FLASH_STMICRO
diff --git a/include/spi.h b/include/spi.h
index ffd6647..45a3094 100644
--- a/include/spi.h
+++ b/include/spi.h
@@ -30,29 +30,23 @@
#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
-#define SPI_DEFAULT_WORDLEN 8
+/* SPI mode bits */
+#define SPI_TX_QPP 1 << 0
+#define SPI_RX_SLOW 1 << 1
+#define SPI_RX_DUAL 1 << 2
+#define SPI_RX_DUAL_IO 1 << 3
+#define SPI_RX_QUAD 1 << 4
+#define SPI_RX_QUAD_IO 1 << 5
+#define SPI_RX_MODES (SPI_RX_SLOW | SPI_RX_DUAL | SPI_RX_DUAL_IO | \
+ SPI_RX_QUAD | SPI_RX_QUAD_IO)
+#define SPI_SHARED 1 << 6
+#define SPI_SEPARATED 1 << 7
+#define SPI_U_PAGE 1 << 8
/**
* struct spi_slave - Representation of a SPI slave
@@ -61,25 +55,19 @@
*
* @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@once, excluding command bytes.
+ * @mode_bits: SPI RX/TX operation modes, bus options and few flags.
* @memory_map: Address of read-only SPI flash access.
- * @option: Varies SPI bus options - separate, shared bus.
- * @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 mode_bits;
void *memory_map;
- u8 option;
- u8 flags;
};
/**
diff --git a/include/spi_flash.h b/include/spi_flash.h
index f79f0ea..16ca294 100644
--- a/include/spi_flash.h
+++ b/include/spi_flash.h
@@ -20,21 +20,19 @@
#include <linux/compiler.h>
/* sf param flags */
+#define SST_WP 1 << 0
#define SECT_4K 1 << 1
#define SECT_32K 1 << 2
#define E_FSR 1 << 3
#define WR_QPP 1 << 4
-
-/* Enum list - Full read commands */
-enum spi_read_cmds {
- ARRAY_SLOW = 1 << 0,
- DUAL_OUTPUT_FAST = 1 << 1,
- DUAL_IO_FAST = 1 << 2,
- QUAD_OUTPUT_FAST = 1 << 3,
- QUAD_IO_FAST = 1 << 4,
-};
-#define RD_EXTN ARRAY_SLOW | DUAL_OUTPUT_FAST | DUAL_IO_FAST
-#define RD_FULL RD_EXTN | QUAD_OUTPUT_FAST | QUAD_IO_FAST
+#define RD_SLOW 1 << 5
+#define RD_DUAL 1 << 6
+#define RD_DUAL_IO 1 << 7
+#define RD_QUAD 1 << 8
+#define RD_QUAD_IO 1 << 9
+#define RD_2WIRE (RD_SLOW | RD_DUAL | RD_DUAL_IO)
+#define RD_FULL (RD_2WIRE | RD_QUAD | RD_QUAD_IO)
+#define ALL_CMDS (WR_QPP | RD_FULL)
/* Dual SPI flash memories */
enum spi_dual_flash {
@@ -51,8 +49,7 @@ enum spi_dual_flash {
* @ext_jedec: Device ext_jedec ID
* @sector_size: Sector size of this device
* @nr_sectors: No.of sectors on this device
- * @e_rd_cmd: Enum list for read commands
- * @flags: Importent param, for flash specific behaviour
+ * @flags: Importent params, for flash specific behaviour
*/
struct spi_flash_params {
const char *name;
@@ -60,7 +57,6 @@ struct spi_flash_params {
u16 ext_jedec;
u32 sector_size;
u32 nr_sectors;
- u8 e_rd_cmd;
u16 flags;
};
--
1.8.3
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 3/6] sf: Renames on dual_flash stuff
[not found] <1390075593-11226-1-git-send-email-jaganna@xilinx.com>
2014-01-18 20:06 ` [U-Boot] [PATCH 1/6] sf: ops: Squash the malloc+memset combo Jagannadha Sutradharudu Teki
2014-01-18 20:06 ` [U-Boot] [PATCH 2/6] sf: Optimize flash features code Jagannadha Sutradharudu Teki
@ 2014-01-18 20:06 ` Jagannadha Sutradharudu Teki
2014-01-18 20:37 ` Marek Vasut
2014-01-18 20:06 ` [U-Boot] [PATCH 4/6] sf: Update read/write command macros Jagannadha Sutradharudu Teki
` (2 subsequent siblings)
5 siblings, 1 reply; 27+ messages in thread
From: Jagannadha Sutradharudu Teki @ 2014-01-18 20:06 UTC (permalink / raw)
To: u-boot
- Used small names for dual_flash macros
- Updated doc/SPI/README.dual-flash
Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
Cc: Marek Vasut <marex@denx.de>
---
doc/SPI/README.dual-flash | 9 +++++----
doc/SPI/README.sf-features | 3 +++
drivers/mtd/spi/sf_ops.c | 10 +++++-----
drivers/mtd/spi/sf_probe.c | 18 +++++++++++-------
include/spi_flash.h | 14 +++++++-------
5 files changed, 31 insertions(+), 23 deletions(-)
diff --git a/doc/SPI/README.dual-flash b/doc/SPI/README.dual-flash
index 6c88d65..6c4236d 100644
--- a/doc/SPI/README.dual-flash
+++ b/doc/SPI/README.dual-flash
@@ -9,11 +9,12 @@ to a given controller with single chip select line, but there are some
hw logics(ex: xilinx zynq qspi) that describes two/dual memories are
connected with a single chip select line from a controller.
-"dual_flash" from include/spi.h describes these types of connection mode
+"dual_flash" from include/spi_flash.h describes these types of connection mode
+in spi flash side and "mode_bits" options for controller driver.
Possible connections:
--------------------
-SF_SINGLE_FLASH:
+SF_SINGLE:
- single spi flash memory connected with single chip select line.
+------------+ CS +---------------+
@@ -24,7 +25,7 @@ SF_SINGLE_FLASH:
| |----------------------->| |
+------------+ +---------------+
-SF_DUAL_STACKED_FLASH:
+SF_STACKED:
- dual spi/qspi flash memories are connected with a single chipselect
line and these two memories are operating stacked fasion with shared buses.
- xilinx zynq qspi controller has implemented this feature [1]
@@ -54,7 +55,7 @@ SF_DUAL_STACKED_FLASH:
by default, if U_PAGE is unset lower memory should accessible,
once user wants to access upper memory need to set U_PAGE.
-SPI_FLASH_CONN_DUALPARALLEL:
+SF_PARALLEL:
- dual spi/qspi flash memories are connected with a single chipselect
line and these two memories are operating parallel with separate buses.
- xilinx zynq qspi controller has implemented this feature [1]
diff --git a/doc/SPI/README.sf-features b/doc/SPI/README.sf-features
index d35f56d..851dfa8 100644
--- a/doc/SPI/README.sf-features
+++ b/doc/SPI/README.sf-features
@@ -73,6 +73,9 @@ based on the selected flash features/operations from spi_slave {} and
spi_flash_params {} - include/spi_flash.h
@dual_flash: flash can be operated in dual flash [3]
+- SF_SINGLE: default connection single flash
+- SF_STACKED: dual flash with dual stacked connection
+- SF_PARALLEL: dual flash with dual parallel connection
@shift: variable shift operator useful for dual parallel
@poll_cmd: find the read_status or flag_status for polling erase/write operations
@erase_cmd: discovered erase command
diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
index 2f3b03a..6cbbfe3 100644
--- a/drivers/mtd/spi/sf_ops.c
+++ b/drivers/mtd/spi/sf_ops.c
@@ -135,7 +135,7 @@ 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->mode_bits |= SPI_U_PAGE;
@@ -143,7 +143,7 @@ static void spi_flash_dual_flash(struct spi_flash *flash, u32 *addr)
flash->spi->mode_bits &= ~SPI_U_PAGE;
}
break;
- case SF_DUAL_PARALLEL_FLASH:
+ case SF_PARALLEL:
*addr >>= flash->shift;
break;
default:
@@ -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
@@ -392,7 +392,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 d1ebf72..79fbad7 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -123,7 +123,6 @@ 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;
/* Assign spi_flash ops */
flash->read = spi_flash_cmd_read_ops;
@@ -133,17 +132,22 @@ static struct spi_flash *spi_flash_validate_params(struct spi_slave *spi,
if (params->flags & SST_WP)
flash->write = sst_write_wp;
#endif
-
+ /* Get the dual flash connection modes */
+#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
/* 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
-
/* Compute erase sector and command */
if (params->flags & SECT_4K) {
flash->erase_cmd = CMD_ERASE_4K;
@@ -320,9 +324,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_flash.h b/include/spi_flash.h
index 16ca294..11488e1 100644
--- a/include/spi_flash.h
+++ b/include/spi_flash.h
@@ -19,6 +19,13 @@
#include <linux/types.h>
#include <linux/compiler.h>
+/* Dual SPI flash memories */
+enum spi_dual_flash {
+ SF_SINGLE = 0,
+ SF_STACKED = 1 << 0,
+ SF_PARALLEL = 1 << 1,
+};
+
/* sf param flags */
#define SST_WP 1 << 0
#define SECT_4K 1 << 1
@@ -34,13 +41,6 @@
#define RD_FULL (RD_2WIRE | RD_QUAD | RD_QUAD_IO)
#define ALL_CMDS (WR_QPP | RD_FULL)
-/* Dual SPI flash memories */
-enum spi_dual_flash {
- SF_SINGLE_FLASH = 0,
- SF_DUAL_STACKED_FLASH = 1 << 0,
- SF_DUAL_PARALLEL_FLASH = 1 << 1,
-};
-
/**
* struct spi_flash_params - SPI/QSPI flash device params structure
*
--
1.8.3
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 4/6] sf: Update read/write command macros
[not found] <1390075593-11226-1-git-send-email-jaganna@xilinx.com>
` (2 preceding siblings ...)
2014-01-18 20:06 ` [U-Boot] [PATCH 3/6] sf: Renames on dual_flash stuff Jagannadha Sutradharudu Teki
@ 2014-01-18 20:06 ` Jagannadha Sutradharudu Teki
2014-01-18 20:36 ` Marek Vasut
2014-01-18 20:06 ` [U-Boot] [PATCH 5/6] sf: Minor macro cleanups Jagannadha Sutradharudu Teki
2014-01-18 20:06 ` [U-Boot] [PATCH 6/6] sf: Update bank configuration Jagannadha Sutradharudu Teki
5 siblings, 1 reply; 27+ messages in thread
From: Jagannadha Sutradharudu Teki @ 2014-01-18 20:06 UTC (permalink / raw)
To: u-boot
- Used readable names for read/write command macros
- Added comments for the same
Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Simon Glass <sjg@chromium.org>
---
drivers/mtd/spi/ramtron.c | 10 ++++------
drivers/mtd/spi/sandbox.c | 12 ++++++------
drivers/mtd/spi/sf_internal.h | 16 ++++++++--------
drivers/mtd/spi/sf_probe.c | 20 ++++++++++----------
4 files changed, 28 insertions(+), 30 deletions(-)
diff --git a/drivers/mtd/spi/ramtron.c b/drivers/mtd/spi/ramtron.c
index d50da37..bdf69f7 100644
--- a/drivers/mtd/spi/ramtron.c
+++ b/drivers/mtd/spi/ramtron.c
@@ -167,7 +167,7 @@ static int ramtron_common(struct spi_flash *flash,
return ret;
}
- if (command == CMD_PAGE_PROGRAM) {
+ if (command == CMD_WR_PAGE) {
/* send WREN */
ret = spi_flash_cmd_write_enable(flash);
if (ret < 0) {
@@ -177,7 +177,7 @@ static int ramtron_common(struct spi_flash *flash,
}
/* do the transaction */
- if (command == CMD_PAGE_PROGRAM)
+ if (command == CMD_WR_PAGE)
ret = spi_flash_cmd_write(flash->spi, cmd, cmd_len, buf, len);
else
ret = spi_flash_cmd_read(flash->spi, cmd, cmd_len, buf, len);
@@ -193,15 +193,13 @@ releasebus:
static int ramtron_read(struct spi_flash *flash,
u32 offset, size_t len, void *buf)
{
- return ramtron_common(flash, offset, len, buf,
- CMD_READ_ARRAY_SLOW);
+ return ramtron_common(flash, offset, len, buf, CMD_RD_SLOW);
}
static int ramtron_write(struct spi_flash *flash,
u32 offset, size_t len, const void *buf)
{
- return ramtron_common(flash, offset, len, (void *)buf,
- CMD_PAGE_PROGRAM);
+ return ramtron_common(flash, offset, len, (void *)buf, CMD_WR_PAGE);
}
static int ramtron_erase(struct spi_flash *flash, u32 offset, size_t len)
diff --git a/drivers/mtd/spi/sandbox.c b/drivers/mtd/spi/sandbox.c
index a62ef4c..bebfb32 100644
--- a/drivers/mtd/spi/sandbox.c
+++ b/drivers/mtd/spi/sandbox.c
@@ -219,10 +219,10 @@ static int sandbox_sf_process_cmd(struct sandbox_spi_flash *sbsf, const u8 *rx,
sbsf->state = SF_ID;
sbsf->cmd = SF_ID;
break;
- case CMD_READ_ARRAY_FAST:
+ case CMD_RD_FAST:
sbsf->pad_addr_bytes = 1;
- case CMD_READ_ARRAY_SLOW:
- case CMD_PAGE_PROGRAM:
+ case CMD_RD_SLOW:
+ case CMD_WR_PAGE:
state_addr:
sbsf->state = SF_ADDR;
break;
@@ -339,11 +339,11 @@ static int sandbox_sf_xfer(void *priv, const u8 *rx, u8 *tx,
return 1;
}
switch (sbsf->cmd) {
- case CMD_READ_ARRAY_FAST:
- case CMD_READ_ARRAY_SLOW:
+ case CMD_RD_FAST:
+ case CMD_RD_SLOW:
sbsf->state = SF_READ;
break;
- case CMD_PAGE_PROGRAM:
+ case CMD_WR_PAGE:
sbsf->state = SF_WRITE;
break;
default:
diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 47d5ac2..6b6fa22 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -28,22 +28,22 @@
/* Write commands */
#define CMD_WRITE_STATUS 0x01
-#define CMD_PAGE_PROGRAM 0x02
+#define CMD_WR_PAGE 0x02 /* Page program */
#define CMD_WRITE_DISABLE 0x04
#define CMD_READ_STATUS 0x05
-#define CMD_QUAD_PAGE_PROGRAM 0x32
+#define CMD_WR_QUAD 0x32 /* Quad page program */
#define CMD_READ_STATUS1 0x35
#define CMD_WRITE_ENABLE 0x06
#define CMD_READ_CONFIG 0x35
#define CMD_FLAG_STATUS 0x70
/* Read commands */
-#define CMD_READ_ARRAY_SLOW 0x03
-#define CMD_READ_ARRAY_FAST 0x0b
-#define CMD_READ_DUAL_OUTPUT_FAST 0x3b
-#define CMD_READ_DUAL_IO_FAST 0xbb
-#define CMD_READ_QUAD_OUTPUT_FAST 0x6b
-#define CMD_READ_QUAD_IO_FAST 0xeb
+#define CMD_RD_SLOW 0x03 /* Array slow */
+#define CMD_RD_FAST 0x0b /* Array fast */
+#define CMD_RD_DUAL 0x3b /* Dual output fast */
+#define CMD_RD_DUAL_IO 0xbb /* Dual IO fast */
+#define CMD_RD_QUAD 0x6b /* Quad output fast */
+#define CMD_RD_QUAD_IO 0xeb /* Quad IO fast */
#define CMD_READ_ID 0x9f
/* Bank addr access commands */
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index 79fbad7..7ba0605 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -161,30 +161,30 @@ static struct spi_flash *spi_flash_validate_params(struct spi_slave *spi,
}
/* Compute read command and dummy_byte */
- flash->read_cmd = CMD_READ_ARRAY_FAST;
+ flash->read_cmd = CMD_RD_FAST;
flash->dummy_byte = 1;
switch (SPI_RX_MODES & flash->spi->mode_bits) {
case SPI_RX_SLOW:
if (params->flags & RD_SLOW) {
- flash->read_cmd = CMD_READ_ARRAY_FAST;
+ flash->read_cmd = CMD_RD_SLOW;
flash->dummy_byte = 0;
}
break;
case SPI_RX_DUAL:
if (params->flags & RD_DUAL)
- flash->read_cmd = CMD_READ_DUAL_OUTPUT_FAST;
+ flash->read_cmd = CMD_RD_DUAL;
break;
case SPI_RX_DUAL_IO:
if (params->flags & RD_DUAL_IO)
- flash->read_cmd = CMD_READ_DUAL_OUTPUT_IO;
+ flash->read_cmd = CMD_RD_DUAL_IO;
break;
case SPI_RX_QUAD:
if (params->flags & RD_QUAD)
- flash->read_cmd = CMD_READ_QUAD_OUTPUT_FAST;
+ flash->read_cmd = CMD_RD_QUAD;
break;
case SPI_RX_QUAD_IO:
if (params->flags & RD_QUAD_IO) {
- flash->read_cmd = CMD_READ_QUAD_OUTPUT_FAST_IO;
+ flash->read_cmd = CMD_RD_QUAD_IO;
flash->dummy_byte = 2;
}
break;
@@ -195,12 +195,12 @@ static struct spi_flash *spi_flash_validate_params(struct spi_slave *spi,
flash->write_cmd = CMD_QUAD_PAGE_PROGRAM;
else
/* Go for default supported write cmd */
- flash->write_cmd = CMD_PAGE_PROGRAM;
+ flash->write_cmd = CMD_WR_PAGE;
/* 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 ((flash->read_cmd == CMD_RD_QUAD) ||
+ (flash->read_cmd == CMD_RD_QUAD_IO) ||
+ (flash->write_cmd == CMD_WR_QUAD)) {
if (spi_flash_set_qeb(flash, idcode[0])) {
debug("SF: Fail to set QEB for %02x\n", idcode[0]);
return NULL;
--
1.8.3
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 5/6] sf: Minor macro cleanups
[not found] <1390075593-11226-1-git-send-email-jaganna@xilinx.com>
` (3 preceding siblings ...)
2014-01-18 20:06 ` [U-Boot] [PATCH 4/6] sf: Update read/write command macros Jagannadha Sutradharudu Teki
@ 2014-01-18 20:06 ` Jagannadha Sutradharudu Teki
2014-01-18 20:06 ` [U-Boot] [PATCH 6/6] sf: Update bank configuration Jagannadha Sutradharudu Teki
5 siblings, 0 replies; 27+ messages in thread
From: Jagannadha Sutradharudu Teki @ 2014-01-18 20:06 UTC (permalink / raw)
To: u-boot
Renamed SPI_FLASH with SF in few places in sf code.
Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
---
drivers/mtd/spi/sf_internal.h | 20 ++++++++++----------
drivers/mtd/spi/sf_ops.c | 23 +++++++++++------------
drivers/mtd/spi/sf_probe.c | 19 +++++++++----------
3 files changed, 30 insertions(+), 32 deletions(-)
diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 6b6fa22..fba7218 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -10,15 +10,15 @@
#ifndef _SF_INTERNAL_H_
#define _SF_INTERNAL_H_
-#define SPI_FLASH_3B_ADDR_LEN 3
-#define SPI_FLASH_CMD_LEN (1 + SPI_FLASH_3B_ADDR_LEN)
-#define SPI_FLASH_16MB_BOUN 0x1000000
+#define SF_3BYTE_ADDR 3
+#define SF_CMD_LEN (1 + SF_3BYTE_ADDR)
+#define SF_16MB_BOUN 0x1000000
/* CFI Manufacture ID's */
-#define SPI_FLASH_CFI_MFR_SPANSION 0x01
-#define SPI_FLASH_CFI_MFR_STMICRO 0x20
-#define SPI_FLASH_CFI_MFR_MACRONIX 0xc2
-#define SPI_FLASH_CFI_MFR_WINBOND 0xef
+#define SF_CFI_MFR_SPANSION 0x01
+#define SF_CFI_MFR_STMICRO 0x20
+#define SF_CFI_MFR_MACRONIX 0xc2
+#define SF_CFI_MFR_WINBOND 0xef
/* Erase commands */
#define CMD_ERASE_4K 0x20
@@ -61,9 +61,9 @@
#define STATUS_PEC (1 << 7)
/* Flash timeout values */
-#define SPI_FLASH_PROG_TIMEOUT (2 * CONFIG_SYS_HZ)
-#define SPI_FLASH_PAGE_ERASE_TIMEOUT (5 * CONFIG_SYS_HZ)
-#define SPI_FLASH_SECTOR_ERASE_TIMEOUT (10 * CONFIG_SYS_HZ)
+#define SF_PROG_TIMEOUT (2 * CONFIG_SYS_HZ)
+#define SF_PAGE_ERASE_TIMEOUT (5 * CONFIG_SYS_HZ)
+#define SF_SECTOR_ERASE_TIMEOUT (10 * CONFIG_SYS_HZ)
/* SST specific */
#ifdef CONFIG_SPI_FLASH_SST
diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
index 6cbbfe3..02b12c1 100644
--- a/drivers/mtd/spi/sf_ops.c
+++ b/drivers/mtd/spi/sf_ops.c
@@ -119,7 +119,7 @@ static int spi_flash_bank(struct spi_flash *flash, u32 offset)
u8 bank_sel;
int ret;
- bank_sel = offset / (SPI_FLASH_16MB_BOUN << flash->shift);
+ bank_sel = offset / (SF_16MB_BOUN << flash->shift);
ret = spi_flash_cmd_bankaddr_write(flash, bank_sel);
if (ret) {
@@ -207,11 +207,11 @@ int spi_flash_write_common(struct spi_flash *flash, const u8 *cmd,
size_t cmd_len, const void *buf, size_t buf_len)
{
struct spi_slave *spi = flash->spi;
- unsigned long timeout = SPI_FLASH_PROG_TIMEOUT;
+ unsigned long timeout = SF_PROG_TIMEOUT;
int ret;
if (buf == NULL)
- timeout = SPI_FLASH_PAGE_ERASE_TIMEOUT;
+ timeout = SF_PAGE_ERASE_TIMEOUT;
ret = spi_claim_bus(flash->spi);
if (ret) {
@@ -233,9 +233,8 @@ int spi_flash_write_common(struct spi_flash *flash, const u8 *cmd,
ret = spi_flash_cmd_wait_ready(flash, timeout);
if (ret < 0) {
- debug("SF: write %s timed out\n",
- timeout == SPI_FLASH_PROG_TIMEOUT ?
- "program" : "page erase");
+ debug("SF: write %s timed out\n", buf == NULL ?
+ "page erase" : "program");
return ret;
}
@@ -247,7 +246,7 @@ int spi_flash_write_common(struct spi_flash *flash, const u8 *cmd,
int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len)
{
u32 erase_size, erase_addr;
- u8 cmd[SPI_FLASH_CMD_LEN];
+ u8 cmd[SF_CMD_LEN];
int ret = -1;
erase_size = flash->erase_size;
@@ -293,7 +292,7 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 offset,
unsigned long byte_addr, page_size;
u32 write_addr;
size_t chunk_len, actual;
- u8 cmd[SPI_FLASH_CMD_LEN];
+ u8 cmd[SF_CMD_LEN];
int ret = -1;
page_size = flash->page_size;
@@ -380,7 +379,7 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
return 0;
}
- cmdsz = SPI_FLASH_CMD_LEN + flash->dummy_byte;
+ cmdsz = SF_CMD_LEN + flash->dummy_byte;
cmd = calloc(1, cmdsz);
if (!cmd) {
printf("SF: Failed to allocate cmd\n");
@@ -400,7 +399,7 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
if (bank_sel < 0)
return ret;
#endif
- remain_len = ((SPI_FLASH_16MB_BOUN << flash->shift) *
+ remain_len = ((SF_16MB_BOUN << flash->shift) *
(bank_sel + 1)) - offset;
if (len < remain_len)
read_len = len;
@@ -445,7 +444,7 @@ static int sst_byte_write(struct spi_flash *flash, u32 offset, const void *buf)
if (ret)
return ret;
- return spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT);
+ return spi_flash_cmd_wait_ready(flash, SF_PROG_TIMEOUT);
}
int sst_write_wp(struct spi_flash *flash, u32 offset, size_t len,
@@ -492,7 +491,7 @@ int sst_write_wp(struct spi_flash *flash, u32 offset, size_t len,
break;
}
- ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT);
+ ret = spi_flash_cmd_wait_ready(flash, SF_PROG_TIMEOUT);
if (ret)
break;
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index 7ba0605..8f92333 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -67,16 +67,16 @@ static int spi_flash_set_qeb(struct spi_flash *flash, u8 idcode0)
{
switch (idcode0) {
#ifdef CONFIG_SPI_FLASH_MACRONIX
- case SPI_FLASH_CFI_MFR_MACRONIX:
+ case SF_CFI_MFR_MACRONIX:
return spi_flash_set_qeb_mxic(flash);
#endif
#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND)
- case SPI_FLASH_CFI_MFR_SPANSION:
- case SPI_FLASH_CFI_MFR_WINBOND:
+ case SF_CFI_MFR_SPANSION:
+ case SF_CFI_MFR_WINBOND:
return spi_flash_set_qeb_winspan(flash);
#endif
#ifdef CONFIG_SPI_FLASH_STMICRO
- case SPI_FLASH_CFI_MFR_STMICRO:
+ case SF_CFI_MFR_STMICRO:
debug("SF: QEB is volatile for %02x flash\n", idcode0);
return 0;
#endif
@@ -190,11 +190,10 @@ static struct spi_flash *spi_flash_validate_params(struct spi_slave *spi,
break;
}
- /* Not require to look for fastest only two write cmds yet */
+ /* Compute write command */
if (flash->spi->mode_bits & SPI_TX_QPP && params->flags & WR_QPP)
- flash->write_cmd = CMD_QUAD_PAGE_PROGRAM;
+ flash->write_cmd = CMD_WR_QUAD;
else
- /* Go for default supported write cmd */
flash->write_cmd = CMD_WR_PAGE;
/* Set the quad enable bit - only for quad commands */
@@ -217,7 +216,7 @@ static struct spi_flash *spi_flash_validate_params(struct spi_slave *spi,
/* Configure the BAR - discover bank cmds and read current bank */
#ifdef CONFIG_SPI_FLASH_BAR
u8 curr_bank = 0;
- if (flash->size > SPI_FLASH_16MB_BOUN) {
+ if (flash->size > SF_16MB_BOUN) {
flash->bank_read_cmd = (idcode[0] == 0x01) ?
CMD_BANKADDR_BRRD : CMD_EXTNADDR_RDEAR;
flash->bank_write_cmd = (idcode[0] == 0x01) ?
@@ -325,9 +324,9 @@ static struct spi_flash *spi_flash_probe_slave(struct spi_slave *spi)
#endif
#ifndef CONFIG_SPI_FLASH_BAR
if (((flash->dual_flash == SF_SINGLE) &&
- (flash->size > SPI_FLASH_16MB_BOUN)) ||
+ (flash->size > SF_16MB_BOUN)) ||
((flash->dual_flash > SF_SINGLE) &&
- (flash->size > SPI_FLASH_16MB_BOUN << 1))) {
+ (flash->size > SF_16MB_BOUN << 1))) {
puts("SF: Warning - Only lower 16MiB accessible,");
puts(" Full access #define CONFIG_SPI_FLASH_BAR\n");
}
--
1.8.3
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 6/6] sf: Update bank configuration
[not found] <1390075593-11226-1-git-send-email-jaganna@xilinx.com>
` (4 preceding siblings ...)
2014-01-18 20:06 ` [U-Boot] [PATCH 5/6] sf: Minor macro cleanups Jagannadha Sutradharudu Teki
@ 2014-01-18 20:06 ` Jagannadha Sutradharudu Teki
5 siblings, 0 replies; 27+ messages in thread
From: Jagannadha Sutradharudu Teki @ 2014-01-18 20:06 UTC (permalink / raw)
To: u-boot
Updated bank configuration code to more readable.
Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
---
drivers/mtd/spi/sf_probe.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index 8f92333..06f99da 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -215,21 +215,23 @@ static struct spi_flash *spi_flash_validate_params(struct spi_slave *spi,
/* Configure the BAR - discover bank cmds and read current bank */
#ifdef CONFIG_SPI_FLASH_BAR
- u8 curr_bank = 0;
+ flash->bank_curr = 0;
if (flash->size > SF_16MB_BOUN) {
- flash->bank_read_cmd = (idcode[0] == 0x01) ?
- CMD_BANKADDR_BRRD : CMD_EXTNADDR_RDEAR;
- flash->bank_write_cmd = (idcode[0] == 0x01) ?
- CMD_BANKADDR_BRWR : CMD_EXTNADDR_WREAR;
+ switch (idcode[0]) {
+ case SF_CFI_MFR_SPANSION:
+ flash->bank_read_cmd = CMD_BANKADDR_BRRD;
+ flash->bank_write_cmd = CMD_BANKADDR_BRWR;
+ break;
+ default:
+ flash->bank_read_cmd = CMD_EXTNADDR_RDEAR;
+ flash->bank_write_cmd = CMD_EXTNADDR_WREAR;
+ }
if (spi_flash_read_common(flash, &flash->bank_read_cmd, 1,
- &curr_bank, 1)) {
- debug("SF: fail to read bank addr register\n");
+ &flash->bank_curr, 1)) {
+ debug("SF: Fail to read bank addr register\n");
return NULL;
}
- flash->bank_curr = curr_bank;
- } else {
- flash->bank_curr = curr_bank;
}
#endif
--
1.8.3
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 1/6] sf: ops: Squash the malloc+memset combo
2014-01-18 20:06 ` [U-Boot] [PATCH 1/6] sf: ops: Squash the malloc+memset combo Jagannadha Sutradharudu Teki
@ 2014-01-18 20:34 ` Marek Vasut
0 siblings, 0 replies; 27+ messages in thread
From: Marek Vasut @ 2014-01-18 20:34 UTC (permalink / raw)
To: u-boot
On Saturday, January 18, 2014 at 09:06:28 PM, Jagannadha Sutradharudu Teki
wrote:
> Squash the malloc()+memset() combo in favor of calloc().
>
> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
> Cc: Marek Vasut <marex@denx.de>
> ---
> drivers/mtd/spi/sf_ops.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
> index 1f1bb36..1fac63a 100644
> --- a/drivers/mtd/spi/sf_ops.c
> +++ b/drivers/mtd/spi/sf_ops.c
> @@ -381,8 +381,11 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash,
> u32 offset, }
>
> cmdsz = SPI_FLASH_CMD_LEN + flash->dummy_byte;
> - cmd = malloc(cmdsz);
> - memset(cmd, 0, cmdsz);
> + cmd = calloc(1, cmdsz);
> + if (!cmd) {
> + printf("SF: Failed to allocate cmd\n");
Usually if your memory allocation fails on 4 bytes of data, the printf() here
will not happen either :) Just use debug().
btw. if you use printf() without args, rather use puts()
> + return ret;
> + }
>
> cmd[0] = flash->read_cmd;
> while (len) {
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 4/6] sf: Update read/write command macros
2014-01-18 20:06 ` [U-Boot] [PATCH 4/6] sf: Update read/write command macros Jagannadha Sutradharudu Teki
@ 2014-01-18 20:36 ` Marek Vasut
2014-01-18 20:45 ` Jagan Teki
0 siblings, 1 reply; 27+ messages in thread
From: Marek Vasut @ 2014-01-18 20:36 UTC (permalink / raw)
To: u-boot
On Saturday, January 18, 2014 at 09:06:31 PM, Jagannadha Sutradharudu Teki
wrote:
> - Used readable names for read/write command macros
> - Added comments for the same
>
> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Simon Glass <sjg@chromium.org>
Does this patch have any impact other than making the code harder to understand
? :-(
What's the rationale for making the code more cryptic ?
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 3/6] sf: Renames on dual_flash stuff
2014-01-18 20:06 ` [U-Boot] [PATCH 3/6] sf: Renames on dual_flash stuff Jagannadha Sutradharudu Teki
@ 2014-01-18 20:37 ` Marek Vasut
2014-01-18 20:49 ` Jagan Teki
0 siblings, 1 reply; 27+ messages in thread
From: Marek Vasut @ 2014-01-18 20:37 UTC (permalink / raw)
To: u-boot
On Saturday, January 18, 2014 at 09:06:30 PM, Jagannadha Sutradharudu Teki
wrote:
> - Used small names for dual_flash macros
> - Updated doc/SPI/README.dual-flash
>
> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
> Cc: Marek Vasut <marex@denx.de>
I agree with the documentation, but disagree with the rename. Please split the
patch into multiple logical blocks so these can be reviewed separatelly. One
would be the documentation, the next the rename and I also see some new #ifdef
in the code, which would be a third block.
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/6] sf: Optimize flash features code
2014-01-18 20:06 ` [U-Boot] [PATCH 2/6] sf: Optimize flash features code Jagannadha Sutradharudu Teki
@ 2014-01-18 20:39 ` Marek Vasut
2014-01-18 20:51 ` Jagan Teki
0 siblings, 1 reply; 27+ messages in thread
From: Marek Vasut @ 2014-01-18 20:39 UTC (permalink / raw)
To: u-boot
On Saturday, January 18, 2014 at 09:06:29 PM, Jagannadha Sutradharudu Teki
wrote:
> - Shrink spi_slave {}
> - Shrink spi_flash_params {}
> - Documentation for sf features
>
> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
> Cc: Marek Vasut <marex@denx.de>
> ---
> doc/SPI/README.sf-features | 122 ++++++++++++++++++++++++++++++
> drivers/mtd/spi/sf.c | 4 +-
> drivers/mtd/spi/sf_internal.h | 1 -
> drivers/mtd/spi/sf_ops.c | 8 +-
> drivers/mtd/spi/sf_params.c | 172
> +++++++++++++++++++++--------------------- drivers/mtd/spi/sf_probe.c |
> 71 ++++++++---------
> include/spi.h | 42 ++++-------
> include/spi_flash.h | 24 +++---
> 8 files changed, 270 insertions(+), 174 deletions(-)
> create mode 100644 doc/SPI/README.sf-features
>
> diff --git a/doc/SPI/README.sf-features b/doc/SPI/README.sf-features
> new file mode 100644
> index 0000000..d35f56d
> --- /dev/null
> +++ b/doc/SPI/README.sf-features
> @@ -0,0 +1,122 @@
> +SPI FLASH feature enhancements:
> +==============================
> +
> +This document describes how to extend the current data structures in spi
> subsystem +for making use of new flash features/operations w.r.t to
> controller driver support. +
> +1. spi_slave:
> +
> +struct spi_slave {
> + ..........
> + u32 mode_bits;
> + ........
> +};
> +
> + at mode_bits can be used to expose the SPI RX/TX operation modes, bus
> options and +few flags which are used to extended the flash specific
> features/operations +- include/spi.h
> +
> +mode_bits:
> +- SPI_TX_QPP: 4-Wire tx transfer operation quad page program
> +- SPI_RX_SLOW: 1-wire rx transfer operation array slow read
> +- SPI_RX_DUAL: 2-wire rx transfer operation dual fast read
> +- SPI_RX_DUAL_IO: 2-wire rx transfer operation dual io fast read
> +- SPI_RX_QUAD: 4-wire rx transfer operation quad fast read
> +- SPI_RX_QUAD_IO: 4-wire rx transfer operation quad io fast read
> +- SPI_SHARED: dual flash devices are connected in shared bus connection
> +- SPI_SEPARATED: dual flash devices are connected in separate bus
> connection +- SPI_U_PAGE: select the upper flash in dual flash shared bus
> connection [1] +
A generic SPI controller _does_ _not_ _care_ about any SPI flash crud. The SPI
bus controller (which is what this is for) and SPI-NOR controller are two
different things and must have two different slave structures.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 4/6] sf: Update read/write command macros
2014-01-18 20:36 ` Marek Vasut
@ 2014-01-18 20:45 ` Jagan Teki
2014-01-20 11:13 ` Detlev Zundel
0 siblings, 1 reply; 27+ messages in thread
From: Jagan Teki @ 2014-01-18 20:45 UTC (permalink / raw)
To: u-boot
On Sun, Jan 19, 2014 at 2:06 AM, Marek Vasut <marex@denx.de> wrote:
> On Saturday, January 18, 2014 at 09:06:31 PM, Jagannadha Sutradharudu Teki
> wrote:
>> - Used readable names for read/write command macros
>> - Added comments for the same
>>
>> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
>> Cc: Marek Vasut <marex@denx.de>
>> Cc: Simon Glass <sjg@chromium.org>
>
> Does this patch have any impact other than making the code harder to understand
> ? :-(
>
> What's the rationale for making the code more cryptic ?
No issues I guess with the readability as each macro we can easily understand.
like CMD_RD_QUAD --> command_read_quad
CMD_WR_PAGE --> command_write_page_program
And this will minimize the macro length - good for in coding and more over
description is added in drivers/mtd/spi/sf_internal.h anyway.
--
Thanks,
Jagan.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 3/6] sf: Renames on dual_flash stuff
2014-01-18 20:37 ` Marek Vasut
@ 2014-01-18 20:49 ` Jagan Teki
2014-01-20 13:16 ` Marek Vasut
0 siblings, 1 reply; 27+ messages in thread
From: Jagan Teki @ 2014-01-18 20:49 UTC (permalink / raw)
To: u-boot
On Sun, Jan 19, 2014 at 2:07 AM, Marek Vasut <marex@denx.de> wrote:
> On Saturday, January 18, 2014 at 09:06:30 PM, Jagannadha Sutradharudu Teki
> wrote:
>> - Used small names for dual_flash macros
>> - Updated doc/SPI/README.dual-flash
>>
>> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
>> Cc: Marek Vasut <marex@denx.de>
>
> I agree with the documentation, but disagree with the rename. Please split the
> patch into multiple logical blocks so these can be reviewed separatelly. One
> would be the documentation, the next the rename and I also see some new #ifdef
> in the code, which would be a third block.
I don't think we need to separate here! as documentation got changed because of
rename of macros' and also new #ifdef is already added one CONFIG_SF_DUAL_FLASH
which is also dual_flash specific.
--
Thanks,
Jagan.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/6] sf: Optimize flash features code
2014-01-18 20:39 ` Marek Vasut
@ 2014-01-18 20:51 ` Jagan Teki
2014-01-20 13:19 ` Marek Vasut
0 siblings, 1 reply; 27+ messages in thread
From: Jagan Teki @ 2014-01-18 20:51 UTC (permalink / raw)
To: u-boot
On Sun, Jan 19, 2014 at 2:09 AM, Marek Vasut <marex@denx.de> wrote:
> On Saturday, January 18, 2014 at 09:06:29 PM, Jagannadha Sutradharudu Teki
> wrote:
>> - Shrink spi_slave {}
>> - Shrink spi_flash_params {}
>> - Documentation for sf features
>>
>> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
>> Cc: Marek Vasut <marex@denx.de>
>> ---
>> doc/SPI/README.sf-features | 122 ++++++++++++++++++++++++++++++
>> drivers/mtd/spi/sf.c | 4 +-
>> drivers/mtd/spi/sf_internal.h | 1 -
>> drivers/mtd/spi/sf_ops.c | 8 +-
>> drivers/mtd/spi/sf_params.c | 172
>> +++++++++++++++++++++--------------------- drivers/mtd/spi/sf_probe.c |
>> 71 ++++++++---------
>> include/spi.h | 42 ++++-------
>> include/spi_flash.h | 24 +++---
>> 8 files changed, 270 insertions(+), 174 deletions(-)
>> create mode 100644 doc/SPI/README.sf-features
>>
>> diff --git a/doc/SPI/README.sf-features b/doc/SPI/README.sf-features
>> new file mode 100644
>> index 0000000..d35f56d
>> --- /dev/null
>> +++ b/doc/SPI/README.sf-features
>> @@ -0,0 +1,122 @@
>> +SPI FLASH feature enhancements:
>> +==============================
>> +
>> +This document describes how to extend the current data structures in spi
>> subsystem +for making use of new flash features/operations w.r.t to
>> controller driver support. +
>> +1. spi_slave:
>> +
>> +struct spi_slave {
>> + ..........
>> + u32 mode_bits;
>> + ........
>> +};
>> +
>> + at mode_bits can be used to expose the SPI RX/TX operation modes, bus
>> options and +few flags which are used to extended the flash specific
>> features/operations +- include/spi.h
>> +
>> +mode_bits:
>> +- SPI_TX_QPP: 4-Wire tx transfer operation quad page program
>> +- SPI_RX_SLOW: 1-wire rx transfer operation array slow read
>> +- SPI_RX_DUAL: 2-wire rx transfer operation dual fast read
>> +- SPI_RX_DUAL_IO: 2-wire rx transfer operation dual io fast read
>> +- SPI_RX_QUAD: 4-wire rx transfer operation quad fast read
>> +- SPI_RX_QUAD_IO: 4-wire rx transfer operation quad io fast read
>> +- SPI_SHARED: dual flash devices are connected in shared bus connection
>> +- SPI_SEPARATED: dual flash devices are connected in separate bus
>> connection +- SPI_U_PAGE: select the upper flash in dual flash shared bus
>> connection [1] +
>
> A generic SPI controller _does_ _not_ _care_ about any SPI flash crud. The SPI
> bus controller (which is what this is for) and SPI-NOR controller are two
> different things and must have two different slave structures.
You mean mode_bits need to move in one more structure.
Just leave about new SPI-NOR as of now for this release we discuss more soon.
--
Thanks,
Jagan.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 4/6] sf: Update read/write command macros
2014-01-18 20:45 ` Jagan Teki
@ 2014-01-20 11:13 ` Detlev Zundel
2014-01-20 11:46 ` Jagan Teki
0 siblings, 1 reply; 27+ messages in thread
From: Detlev Zundel @ 2014-01-20 11:13 UTC (permalink / raw)
To: u-boot
Hi Jagan,
> On Sun, Jan 19, 2014 at 2:06 AM, Marek Vasut <marex@denx.de> wrote:
>> On Saturday, January 18, 2014 at 09:06:31 PM, Jagannadha
>> Sutradharudu Teki
>> wrote:
>>> - Used readable names for read/write command macros
>>> - Added comments for the same
>>>
>>> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
>>> Cc: Marek Vasut <marex@denx.de>
>>> Cc: Simon Glass <sjg@chromium.org>
>>
>> Does this patch have any impact other than making the code harder to
>> understand
>> ? :-(
>>
>> What's the rationale for making the code more cryptic ?
>
> No issues I guess with the readability as each macro we can easily
> understand.
> like CMD_RD_QUAD --> command_read_quad
> CMD_WR_PAGE --> command_write_page_program
>
> And this will minimize the macro length - good for in coding and more over
> description is added in drivers/mtd/spi/sf_internal.h anyway.
Again I agree with Marek that readability of code is more important than
saving a few characters while coding. This is especially true as
editors can support you in coding (Emacs has lots of packages to help
here for example).
Cheers
Detlev
--
(7) It is always something
(7a) (corollary). Good, Fast, Cheap: Pick any two (you can't have all three).
-- The Twelve Networking Truths (RFC 1925)
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 4/6] sf: Update read/write command macros
2014-01-20 11:13 ` Detlev Zundel
@ 2014-01-20 11:46 ` Jagan Teki
2014-01-20 13:06 ` Marek Vasut
2014-01-20 14:33 ` Detlev Zundel
0 siblings, 2 replies; 27+ messages in thread
From: Jagan Teki @ 2014-01-20 11:46 UTC (permalink / raw)
To: u-boot
On Mon, Jan 20, 2014 at 4:43 PM, Detlev Zundel <dzu@denx.de> wrote:
> Hi Jagan,
>
>> On Sun, Jan 19, 2014 at 2:06 AM, Marek Vasut <marex@denx.de> wrote:
>>> On Saturday, January 18, 2014 at 09:06:31 PM, Jagannadha
>>> Sutradharudu Teki
>>> wrote:
>>>> - Used readable names for read/write command macros
>>>> - Added comments for the same
>>>>
>>>> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
>>>> Cc: Marek Vasut <marex@denx.de>
>>>> Cc: Simon Glass <sjg@chromium.org>
>>>
>>> Does this patch have any impact other than making the code harder to
>>> understand
>>> ? :-(
>>>
>>> What's the rationale for making the code more cryptic ?
>>
>> No issues I guess with the readability as each macro we can easily
>> understand.
>> like CMD_RD_QUAD --> command_read_quad
>> CMD_WR_PAGE --> command_write_page_program
>>
>> And this will minimize the macro length - good for in coding and more over
>> description is added in drivers/mtd/spi/sf_internal.h anyway.
>
> Again I agree with Marek that readability of code is more important than
> saving a few characters while coding. This is especially true as
> editors can support you in coding (Emacs has lots of packages to help
> here for example).
I don't think nothing much gone the readability with these updated:
CMD_READ_ARRAY_FAST has updated CMD_RD_FAST and it seems like
easy to understand. and anyway I have added comments for full name as well.
Few of the flashes can be call this as array fast read and fewer call
this as fast read
and few more call this as high frequency read. CMD_RD_FAST will suits
all these names.
Comments please!
--
Thanks,
Jagan.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 4/6] sf: Update read/write command macros
2014-01-20 11:46 ` Jagan Teki
@ 2014-01-20 13:06 ` Marek Vasut
2014-01-20 13:10 ` Jagan Teki
2014-01-20 14:33 ` Detlev Zundel
1 sibling, 1 reply; 27+ messages in thread
From: Marek Vasut @ 2014-01-20 13:06 UTC (permalink / raw)
To: u-boot
On Monday, January 20, 2014 at 12:46:07 PM, Jagan Teki wrote:
> On Mon, Jan 20, 2014 at 4:43 PM, Detlev Zundel <dzu@denx.de> wrote:
> > Hi Jagan,
> >
> >> On Sun, Jan 19, 2014 at 2:06 AM, Marek Vasut <marex@denx.de> wrote:
> >>> On Saturday, January 18, 2014 at 09:06:31 PM, Jagannadha
> >>> Sutradharudu Teki
> >>>
> >>> wrote:
> >>>> - Used readable names for read/write command macros
> >>>> - Added comments for the same
> >>>>
> >>>> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
> >>>> Cc: Marek Vasut <marex@denx.de>
> >>>> Cc: Simon Glass <sjg@chromium.org>
> >>>
> >>> Does this patch have any impact other than making the code harder to
> >>> understand
> >>> ? :-(
> >>>
> >>> What's the rationale for making the code more cryptic ?
> >>
> >> No issues I guess with the readability as each macro we can easily
> >> understand.
> >> like CMD_RD_QUAD --> command_read_quad
> >>
> >> CMD_WR_PAGE --> command_write_page_program
> >>
> >> And this will minimize the macro length - good for in coding and more
> >> over description is added in drivers/mtd/spi/sf_internal.h anyway.
> >
> > Again I agree with Marek that readability of code is more important than
> > saving a few characters while coding. This is especially true as
> > editors can support you in coding (Emacs has lots of packages to help
> > here for example).
>
> I don't think nothing much gone the readability with these updated:
> CMD_READ_ARRAY_FAST has updated CMD_RD_FAST and it seems like
> easy to understand. and anyway I have added comments for full name as well.
CMD_READ_ARRAY_FAST contains all the necessary bits for me to understand what
the macro means. CMD_RD_FAST does not. I fail to see the rationale behind
changing the names.
> Few of the flashes can be call this as array fast read and fewer call
> this as fast read
> and few more call this as high frequency read. CMD_RD_FAST will suits
> all these names.
>
> Comments please!
If you want to align the names with anything, align then with linux's m25p80.c
driver . But I see this change as moot and confusing, sorry.
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 4/6] sf: Update read/write command macros
2014-01-20 13:06 ` Marek Vasut
@ 2014-01-20 13:10 ` Jagan Teki
2014-01-20 13:13 ` Jagan Teki
0 siblings, 1 reply; 27+ messages in thread
From: Jagan Teki @ 2014-01-20 13:10 UTC (permalink / raw)
To: u-boot
On Mon, Jan 20, 2014 at 6:36 PM, Marek Vasut <marex@denx.de> wrote:
> On Monday, January 20, 2014 at 12:46:07 PM, Jagan Teki wrote:
>> On Mon, Jan 20, 2014 at 4:43 PM, Detlev Zundel <dzu@denx.de> wrote:
>> > Hi Jagan,
>> >
>> >> On Sun, Jan 19, 2014 at 2:06 AM, Marek Vasut <marex@denx.de> wrote:
>> >>> On Saturday, January 18, 2014 at 09:06:31 PM, Jagannadha
>> >>> Sutradharudu Teki
>> >>>
>> >>> wrote:
>> >>>> - Used readable names for read/write command macros
>> >>>> - Added comments for the same
>> >>>>
>> >>>> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
>> >>>> Cc: Marek Vasut <marex@denx.de>
>> >>>> Cc: Simon Glass <sjg@chromium.org>
>> >>>
>> >>> Does this patch have any impact other than making the code harder to
>> >>> understand
>> >>> ? :-(
>> >>>
>> >>> What's the rationale for making the code more cryptic ?
>> >>
>> >> No issues I guess with the readability as each macro we can easily
>> >> understand.
>> >> like CMD_RD_QUAD --> command_read_quad
>> >>
>> >> CMD_WR_PAGE --> command_write_page_program
>> >>
>> >> And this will minimize the macro length - good for in coding and more
>> >> over description is added in drivers/mtd/spi/sf_internal.h anyway.
>> >
>> > Again I agree with Marek that readability of code is more important than
>> > saving a few characters while coding. This is especially true as
>> > editors can support you in coding (Emacs has lots of packages to help
>> > here for example).
>>
>> I don't think nothing much gone the readability with these updated:
>> CMD_READ_ARRAY_FAST has updated CMD_RD_FAST and it seems like
>> easy to understand. and anyway I have added comments for full name as well.
>
> CMD_READ_ARRAY_FAST contains all the necessary bits for me to understand what
> the macro means. CMD_RD_FAST does not. I fail to see the rationale behind
> changing the names.
>
>> Few of the flashes can be call this as array fast read and fewer call
>> this as fast read
>> and few more call this as high frequency read. CMD_RD_FAST will suits
>> all these names.
>>
>> Comments please!
>
> If you want to align the names with anything, align then with linux's m25p80.c
> driver . But I see this change as moot and confusing, sorry.
No issues, we can skip these as of now for this release.!
--
Thanks,
Jagan.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 4/6] sf: Update read/write command macros
2014-01-20 13:10 ` Jagan Teki
@ 2014-01-20 13:13 ` Jagan Teki
0 siblings, 0 replies; 27+ messages in thread
From: Jagan Teki @ 2014-01-20 13:13 UTC (permalink / raw)
To: u-boot
On Mon, Jan 20, 2014 at 6:40 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
> On Mon, Jan 20, 2014 at 6:36 PM, Marek Vasut <marex@denx.de> wrote:
>> On Monday, January 20, 2014 at 12:46:07 PM, Jagan Teki wrote:
>>> On Mon, Jan 20, 2014 at 4:43 PM, Detlev Zundel <dzu@denx.de> wrote:
>>> > Hi Jagan,
>>> >
>>> >> On Sun, Jan 19, 2014 at 2:06 AM, Marek Vasut <marex@denx.de> wrote:
>>> >>> On Saturday, January 18, 2014 at 09:06:31 PM, Jagannadha
>>> >>> Sutradharudu Teki
>>> >>>
>>> >>> wrote:
>>> >>>> - Used readable names for read/write command macros
>>> >>>> - Added comments for the same
>>> >>>>
>>> >>>> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
>>> >>>> Cc: Marek Vasut <marex@denx.de>
>>> >>>> Cc: Simon Glass <sjg@chromium.org>
>>> >>>
>>> >>> Does this patch have any impact other than making the code harder to
>>> >>> understand
>>> >>> ? :-(
>>> >>>
>>> >>> What's the rationale for making the code more cryptic ?
>>> >>
>>> >> No issues I guess with the readability as each macro we can easily
>>> >> understand.
>>> >> like CMD_RD_QUAD --> command_read_quad
>>> >>
>>> >> CMD_WR_PAGE --> command_write_page_program
>>> >>
>>> >> And this will minimize the macro length - good for in coding and more
>>> >> over description is added in drivers/mtd/spi/sf_internal.h anyway.
>>> >
>>> > Again I agree with Marek that readability of code is more important than
>>> > saving a few characters while coding. This is especially true as
>>> > editors can support you in coding (Emacs has lots of packages to help
>>> > here for example).
>>>
>>> I don't think nothing much gone the readability with these updated:
>>> CMD_READ_ARRAY_FAST has updated CMD_RD_FAST and it seems like
>>> easy to understand. and anyway I have added comments for full name as well.
>>
>> CMD_READ_ARRAY_FAST contains all the necessary bits for me to understand what
>> the macro means. CMD_RD_FAST does not. I fail to see the rationale behind
>> changing the names.
>>
>>> Few of the flashes can be call this as array fast read and fewer call
>>> this as fast read
>>> and few more call this as high frequency read. CMD_RD_FAST will suits
>>> all these names.
>>>
>>> Comments please!
>>
>> If you want to align the names with anything, align then with linux's m25p80.c
>> driver . But I see this change as moot and confusing, sorry.
>
> No issues, we can skip these as of now for this release.!
Just fyi: if we need a reference of m25p80.c the name as OPCODE_NORM_READ
which is similar to what i did :)
--
Thanks,
Jagan.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 3/6] sf: Renames on dual_flash stuff
2014-01-18 20:49 ` Jagan Teki
@ 2014-01-20 13:16 ` Marek Vasut
2014-01-20 13:35 ` Jagan Teki
0 siblings, 1 reply; 27+ messages in thread
From: Marek Vasut @ 2014-01-20 13:16 UTC (permalink / raw)
To: u-boot
On Saturday, January 18, 2014 at 09:49:11 PM, Jagan Teki wrote:
> On Sun, Jan 19, 2014 at 2:07 AM, Marek Vasut <marex@denx.de> wrote:
> > On Saturday, January 18, 2014 at 09:06:30 PM, Jagannadha Sutradharudu
> > Teki
> >
> > wrote:
> >> - Used small names for dual_flash macros
> >> - Updated doc/SPI/README.dual-flash
> >>
> >> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
> >> Cc: Marek Vasut <marex@denx.de>
> >
> > I agree with the documentation, but disagree with the rename. Please
> > split the patch into multiple logical blocks so these can be reviewed
> > separatelly. One would be the documentation, the next the rename and I
> > also see some new #ifdef in the code, which would be a third block.
>
> I don't think we need to separate here! as documentation got changed
> because of rename of macros' and also new #ifdef is already added one
> CONFIG_SF_DUAL_FLASH which is also dual_flash specific.
I still see documentation fixes, renames and even newly added code. This really
makes no sense to me to meld all these into a single patch.
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/6] sf: Optimize flash features code
2014-01-18 20:51 ` Jagan Teki
@ 2014-01-20 13:19 ` Marek Vasut
2014-01-20 13:32 ` Jagan Teki
0 siblings, 1 reply; 27+ messages in thread
From: Marek Vasut @ 2014-01-20 13:19 UTC (permalink / raw)
To: u-boot
On Saturday, January 18, 2014 at 09:51:56 PM, Jagan Teki wrote:
> On Sun, Jan 19, 2014 at 2:09 AM, Marek Vasut <marex@denx.de> wrote:
> > On Saturday, January 18, 2014 at 09:06:29 PM, Jagannadha Sutradharudu
> > Teki
> >
> > wrote:
> >> - Shrink spi_slave {}
> >> - Shrink spi_flash_params {}
> >> - Documentation for sf features
> >>
> >> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
> >> Cc: Marek Vasut <marex@denx.de>
> >> ---
> >>
> >> doc/SPI/README.sf-features | 122 ++++++++++++++++++++++++++++++
> >> drivers/mtd/spi/sf.c | 4 +-
> >> drivers/mtd/spi/sf_internal.h | 1 -
> >> drivers/mtd/spi/sf_ops.c | 8 +-
> >> drivers/mtd/spi/sf_params.c | 172
> >>
> >> +++++++++++++++++++++--------------------- drivers/mtd/spi/sf_probe.c
> >> |
> >>
> >> 71 ++++++++---------
> >> include/spi.h | 42 ++++-------
> >> include/spi_flash.h | 24 +++---
> >> 8 files changed, 270 insertions(+), 174 deletions(-)
> >> create mode 100644 doc/SPI/README.sf-features
> >>
> >> diff --git a/doc/SPI/README.sf-features b/doc/SPI/README.sf-features
> >> new file mode 100644
> >> index 0000000..d35f56d
> >> --- /dev/null
> >> +++ b/doc/SPI/README.sf-features
> >> @@ -0,0 +1,122 @@
> >> +SPI FLASH feature enhancements:
> >> +==============================
> >> +
> >> +This document describes how to extend the current data structures in
> >> spi subsystem +for making use of new flash features/operations w.r.t to
> >> controller driver support. +
> >> +1. spi_slave:
> >> +
> >> +struct spi_slave {
> >> + ..........
> >> + u32 mode_bits;
> >> + ........
> >> +};
> >> +
> >> + at mode_bits can be used to expose the SPI RX/TX operation modes, bus
> >> options and +few flags which are used to extended the flash specific
> >> features/operations +- include/spi.h
> >> +
> >> +mode_bits:
> >> +- SPI_TX_QPP: 4-Wire tx transfer operation quad page program
> >> +- SPI_RX_SLOW: 1-wire rx transfer operation array slow read
> >> +- SPI_RX_DUAL: 2-wire rx transfer operation dual fast read
> >> +- SPI_RX_DUAL_IO: 2-wire rx transfer operation dual io fast read
> >> +- SPI_RX_QUAD: 4-wire rx transfer operation quad fast read
> >> +- SPI_RX_QUAD_IO: 4-wire rx transfer operation quad io fast read
> >> +- SPI_SHARED: dual flash devices are connected in shared bus connection
> >> +- SPI_SEPARATED: dual flash devices are connected in separate bus
> >> connection +- SPI_U_PAGE: select the upper flash in dual flash shared
> >> bus connection [1] +
> >
> > A generic SPI controller _does_ _not_ _care_ about any SPI flash crud.
> > The SPI bus controller (which is what this is for) and SPI-NOR
> > controller are two different things and must have two different slave
> > structures.
>
> You mean mode_bits need to move in one more structure.
> Just leave about new SPI-NOR as of now for this release we discuss more
> soon.
The mode_bits have no place in this structure. The slave can indicate whether it
can be connected over 1,2,4... lines , but must not indicate that it supports
some SPI-flash specific properties.
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/6] sf: Optimize flash features code
2014-01-20 13:19 ` Marek Vasut
@ 2014-01-20 13:32 ` Jagan Teki
2014-01-20 23:01 ` Marek Vasut
0 siblings, 1 reply; 27+ messages in thread
From: Jagan Teki @ 2014-01-20 13:32 UTC (permalink / raw)
To: u-boot
On Mon, Jan 20, 2014 at 6:49 PM, Marek Vasut <marex@denx.de> wrote:
> On Saturday, January 18, 2014 at 09:51:56 PM, Jagan Teki wrote:
>> On Sun, Jan 19, 2014 at 2:09 AM, Marek Vasut <marex@denx.de> wrote:
>> > On Saturday, January 18, 2014 at 09:06:29 PM, Jagannadha Sutradharudu
>> > Teki
>> >
>> > wrote:
>> >> - Shrink spi_slave {}
>> >> - Shrink spi_flash_params {}
>> >> - Documentation for sf features
>> >>
>> >> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
>> >> Cc: Marek Vasut <marex@denx.de>
>> >> ---
>> >>
>> >> doc/SPI/README.sf-features | 122 ++++++++++++++++++++++++++++++
>> >> drivers/mtd/spi/sf.c | 4 +-
>> >> drivers/mtd/spi/sf_internal.h | 1 -
>> >> drivers/mtd/spi/sf_ops.c | 8 +-
>> >> drivers/mtd/spi/sf_params.c | 172
>> >>
>> >> +++++++++++++++++++++--------------------- drivers/mtd/spi/sf_probe.c
>> >> |
>> >>
>> >> 71 ++++++++---------
>> >> include/spi.h | 42 ++++-------
>> >> include/spi_flash.h | 24 +++---
>> >> 8 files changed, 270 insertions(+), 174 deletions(-)
>> >> create mode 100644 doc/SPI/README.sf-features
>> >>
>> >> diff --git a/doc/SPI/README.sf-features b/doc/SPI/README.sf-features
>> >> new file mode 100644
>> >> index 0000000..d35f56d
>> >> --- /dev/null
>> >> +++ b/doc/SPI/README.sf-features
>> >> @@ -0,0 +1,122 @@
>> >> +SPI FLASH feature enhancements:
>> >> +==============================
>> >> +
>> >> +This document describes how to extend the current data structures in
>> >> spi subsystem +for making use of new flash features/operations w.r.t to
>> >> controller driver support. +
>> >> +1. spi_slave:
>> >> +
>> >> +struct spi_slave {
>> >> + ..........
>> >> + u32 mode_bits;
>> >> + ........
>> >> +};
>> >> +
>> >> + at mode_bits can be used to expose the SPI RX/TX operation modes, bus
>> >> options and +few flags which are used to extended the flash specific
>> >> features/operations +- include/spi.h
>> >> +
>> >> +mode_bits:
>> >> +- SPI_TX_QPP: 4-Wire tx transfer operation quad page program
>> >> +- SPI_RX_SLOW: 1-wire rx transfer operation array slow read
>> >> +- SPI_RX_DUAL: 2-wire rx transfer operation dual fast read
>> >> +- SPI_RX_DUAL_IO: 2-wire rx transfer operation dual io fast read
>> >> +- SPI_RX_QUAD: 4-wire rx transfer operation quad fast read
>> >> +- SPI_RX_QUAD_IO: 4-wire rx transfer operation quad io fast read
>> >> +- SPI_SHARED: dual flash devices are connected in shared bus connection
>> >> +- SPI_SEPARATED: dual flash devices are connected in separate bus
>> >> connection +- SPI_U_PAGE: select the upper flash in dual flash shared
>> >> bus connection [1] +
>> >
>> > A generic SPI controller _does_ _not_ _care_ about any SPI flash crud.
>> > The SPI bus controller (which is what this is for) and SPI-NOR
>> > controller are two different things and must have two different slave
>> > structures.
>>
>> You mean mode_bits need to move in one more structure.
>> Just leave about new SPI-NOR as of now for this release we discuss more
>> soon.
>
> The mode_bits have no place in this structure. The slave can indicate whether it
> can be connected over 1,2,4... lines , but must not indicate that it supports
> some SPI-flash specific properties.
What do you mean by this - can you elaborate.
As of now drivers in drivers/spi need to inform the flash through spi_slave {}
no other alternative ie way remaining flash properties memory_map etc.. handle.
Even Linux follow the same w/o new SPI-NOR framework.
If your question, like need a separate structure for flash specific properties
please wait will wound-up all these in new framework.
I'm planning to push in today release.
--
Thanks,
Jagan.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 3/6] sf: Renames on dual_flash stuff
2014-01-20 13:16 ` Marek Vasut
@ 2014-01-20 13:35 ` Jagan Teki
0 siblings, 0 replies; 27+ messages in thread
From: Jagan Teki @ 2014-01-20 13:35 UTC (permalink / raw)
To: u-boot
On Mon, Jan 20, 2014 at 6:46 PM, Marek Vasut <marex@denx.de> wrote:
> On Saturday, January 18, 2014 at 09:49:11 PM, Jagan Teki wrote:
>> On Sun, Jan 19, 2014 at 2:07 AM, Marek Vasut <marex@denx.de> wrote:
>> > On Saturday, January 18, 2014 at 09:06:30 PM, Jagannadha Sutradharudu
>> > Teki
>> >
>> > wrote:
>> >> - Used small names for dual_flash macros
>> >> - Updated doc/SPI/README.dual-flash
>> >>
>> >> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
>> >> Cc: Marek Vasut <marex@denx.de>
>> >
>> > I agree with the documentation, but disagree with the rename. Please
>> > split the patch into multiple logical blocks so these can be reviewed
>> > separatelly. One would be the documentation, the next the rename and I
>> > also see some new #ifdef in the code, which would be a third block.
>>
>> I don't think we need to separate here! as documentation got changed
>> because of rename of macros' and also new #ifdef is already added one
>> CONFIG_SF_DUAL_FLASH which is also dual_flash specific.
>
> I still see documentation fixes, renames and even newly added code. This really
> makes no sense to me to meld all these into a single patch.
Will be clear in next series for sure!
--
Thanks,
Jagan.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 4/6] sf: Update read/write command macros
2014-01-20 11:46 ` Jagan Teki
2014-01-20 13:06 ` Marek Vasut
@ 2014-01-20 14:33 ` Detlev Zundel
1 sibling, 0 replies; 27+ messages in thread
From: Detlev Zundel @ 2014-01-20 14:33 UTC (permalink / raw)
To: u-boot
Hi Jagan,
[...]
> I don't think nothing much gone the readability with these updated:
> CMD_READ_ARRAY_FAST has updated CMD_RD_FAST and it seems like easy to
> understand. and anyway I have added comments for full name as well.
Comments in a far away place cannot compensate for self-explaining
constructs at the location where they are used. Stating that a
constants needs comments to explain it is actually a good sign that its
name is not chosen carefully enough.
Really, naming constants and variables for readable and maintainable
code is a much harder problem than it looks (cf. my signature) and
unfortunately not easily measurable. But I assure you that good names
can make a world of a difference. That's why Marek and me are so
passionate about this seemingly "trivial" change.
> Few of the flashes can be call this as array fast read and fewer call
> this as fast read and few more call this as high frequency
> read. CMD_RD_FAST will suits all these names.
>
> Comments please!
When we change code, we don't do this for the sake of changing it, but
in order to improve one aspect of it, i.e. the performance, the
maintainability or the features. When everything "stays the same", we
are even _opposed_ to a change because there is nothing to outweigh the
effort to adjusting to the new things.
To summarize - we need proof that a change _improves_ something.
Showing that we "do not loose something" is clearly not enough.
Now in this specific case, we have multiple people voicing the concern
that the renaming looses vital information, thus effectively making
reading and maintining the code harder. On the other hand even you
agree that "something" although "not much" will be gone after the
rename. So taking this into account we have only "saving of a few
keystorkes" on the positive side and substantial degradation on
readability and maintainability on the negative side.
Best wishes
Detlev
--
There are two hard things in computer science: cache invalidation,
naming things, and off-by-one errors.
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/6] sf: Optimize flash features code
2014-01-20 13:32 ` Jagan Teki
@ 2014-01-20 23:01 ` Marek Vasut
2014-01-21 7:39 ` Jagan Teki
0 siblings, 1 reply; 27+ messages in thread
From: Marek Vasut @ 2014-01-20 23:01 UTC (permalink / raw)
To: u-boot
On Monday, January 20, 2014 at 02:32:27 PM, Jagan Teki wrote:
> On Mon, Jan 20, 2014 at 6:49 PM, Marek Vasut <marex@denx.de> wrote:
> > On Saturday, January 18, 2014 at 09:51:56 PM, Jagan Teki wrote:
> >> On Sun, Jan 19, 2014 at 2:09 AM, Marek Vasut <marex@denx.de> wrote:
> >> > On Saturday, January 18, 2014 at 09:06:29 PM, Jagannadha Sutradharudu
> >> > Teki
> >> >
> >> > wrote:
> >> >> - Shrink spi_slave {}
> >> >> - Shrink spi_flash_params {}
> >> >> - Documentation for sf features
> >> >>
> >> >> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
> >> >> Cc: Marek Vasut <marex@denx.de>
> >> >> ---
> >> >>
> >> >> doc/SPI/README.sf-features | 122 ++++++++++++++++++++++++++++++
> >> >> drivers/mtd/spi/sf.c | 4 +-
> >> >> drivers/mtd/spi/sf_internal.h | 1 -
> >> >> drivers/mtd/spi/sf_ops.c | 8 +-
> >> >> drivers/mtd/spi/sf_params.c | 172
> >> >>
> >> >> +++++++++++++++++++++--------------------- drivers/mtd/spi/sf_probe.c
> >> >>
> >> >> 71 ++++++++---------
> >> >> include/spi.h | 42 ++++-------
> >> >> include/spi_flash.h | 24 +++---
> >> >> 8 files changed, 270 insertions(+), 174 deletions(-)
> >> >> create mode 100644 doc/SPI/README.sf-features
> >> >>
> >> >> diff --git a/doc/SPI/README.sf-features b/doc/SPI/README.sf-features
> >> >> new file mode 100644
> >> >> index 0000000..d35f56d
> >> >> --- /dev/null
> >> >> +++ b/doc/SPI/README.sf-features
> >> >> @@ -0,0 +1,122 @@
> >> >> +SPI FLASH feature enhancements:
> >> >> +==============================
> >> >> +
> >> >> +This document describes how to extend the current data structures in
> >> >> spi subsystem +for making use of new flash features/operations w.r.t
> >> >> to controller driver support. +
> >> >> +1. spi_slave:
> >> >> +
> >> >> +struct spi_slave {
> >> >> + ..........
> >> >> + u32 mode_bits;
> >> >> + ........
> >> >> +};
> >> >> +
> >> >> + at mode_bits can be used to expose the SPI RX/TX operation modes, bus
> >> >> options and +few flags which are used to extended the flash specific
> >> >> features/operations +- include/spi.h
> >> >> +
> >> >> +mode_bits:
> >> >> +- SPI_TX_QPP: 4-Wire tx transfer operation quad page program
> >> >> +- SPI_RX_SLOW: 1-wire rx transfer operation array slow read
> >> >> +- SPI_RX_DUAL: 2-wire rx transfer operation dual fast read
> >> >> +- SPI_RX_DUAL_IO: 2-wire rx transfer operation dual io fast read
> >> >> +- SPI_RX_QUAD: 4-wire rx transfer operation quad fast read
> >> >> +- SPI_RX_QUAD_IO: 4-wire rx transfer operation quad io fast read
> >> >> +- SPI_SHARED: dual flash devices are connected in shared bus
> >> >> connection +- SPI_SEPARATED: dual flash devices are connected in
> >> >> separate bus connection +- SPI_U_PAGE: select the upper flash in
> >> >> dual flash shared bus connection [1] +
> >> >
> >> > A generic SPI controller _does_ _not_ _care_ about any SPI flash crud.
> >> > The SPI bus controller (which is what this is for) and SPI-NOR
> >> > controller are two different things and must have two different slave
> >> > structures.
> >>
> >> You mean mode_bits need to move in one more structure.
> >> Just leave about new SPI-NOR as of now for this release we discuss more
> >> soon.
> >
> > The mode_bits have no place in this structure. The slave can indicate
> > whether it can be connected over 1,2,4... lines , but must not indicate
> > that it supports some SPI-flash specific properties.
>
> What do you mean by this - can you elaborate.
I mean there do exist SPI controllers which can output data via more than one
(MOSI) line . They do support 1, 2 or even 4 lines. This comes useful when
driving SPI flashes indeed, _BUT_ (!) this is not tied to driving SPI flashes.
On the contrary, you can connect any device which can be driven over such
"parallelised" SPI and it will work. And only such property shall also land in
the struct spi_slave {} .
Properties like "let's assume the SPI slave is a SPI NOR and it supports SPI NOR
command FOO" do not go into struct spi_slave.
> As of now drivers in drivers/spi need to inform the flash through spi_slave
> {} no other alternative ie way remaining flash properties memory_map etc..
> handle. Even Linux follow the same w/o new SPI-NOR framework.
Linux is now going for a separate SPI-NOR controller framework and this is what
we will do as well. The SF layer will be only a unifying layer providing access
to either SPI-NOR API (for the SPI-NOR controllers) or a translation layer to
communicate SPI-NOR commands over generic SPI API (for the old-school regular
way of doing SPI NOR connection to SPI bus).
> If your question, like need a separate structure for flash specific
> properties please wait will wound-up all these in new framework.
>
> I'm planning to push in today release.
I doubt anything is going in today's release ;-)
Actually, you need to really start discerning which _must_ go into a release and
which simply does not go into a release and waits for -next. This is important
role of a maintainer and plays important role in keeping the codebase in good
shape.
Code which is untested, code which changes core stuff and isn't an obvious
bugfix etc. simply can wait. Code which is obvious bugfix goes in even in the -
rc period. Of course, there are all kinds of exceptions, that depends on the
rationale and negotiation.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/6] sf: Optimize flash features code
2014-01-20 23:01 ` Marek Vasut
@ 2014-01-21 7:39 ` Jagan Teki
2014-01-21 7:45 ` Jagan Teki
0 siblings, 1 reply; 27+ messages in thread
From: Jagan Teki @ 2014-01-21 7:39 UTC (permalink / raw)
To: u-boot
Hi Marek,
On Tue, Jan 21, 2014 at 4:31 AM, Marek Vasut <marex@denx.de> wrote:
> On Monday, January 20, 2014 at 02:32:27 PM, Jagan Teki wrote:
>> On Mon, Jan 20, 2014 at 6:49 PM, Marek Vasut <marex@denx.de> wrote:
>> > On Saturday, January 18, 2014 at 09:51:56 PM, Jagan Teki wrote:
>> >> On Sun, Jan 19, 2014 at 2:09 AM, Marek Vasut <marex@denx.de> wrote:
>> >> > On Saturday, January 18, 2014 at 09:06:29 PM, Jagannadha Sutradharudu
>> >> > Teki
>> >> >
>> >> > wrote:
>> >> >> - Shrink spi_slave {}
>> >> >> - Shrink spi_flash_params {}
>> >> >> - Documentation for sf features
>> >> >>
>> >> >> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
>> >> >> Cc: Marek Vasut <marex@denx.de>
>> >> >> ---
>> >> >>
>> >> >> doc/SPI/README.sf-features | 122 ++++++++++++++++++++++++++++++
>> >> >> drivers/mtd/spi/sf.c | 4 +-
>> >> >> drivers/mtd/spi/sf_internal.h | 1 -
>> >> >> drivers/mtd/spi/sf_ops.c | 8 +-
>> >> >> drivers/mtd/spi/sf_params.c | 172
>> >> >>
>> >> >> +++++++++++++++++++++--------------------- drivers/mtd/spi/sf_probe.c
>> >> >>
>> >> >> 71 ++++++++---------
>> >> >> include/spi.h | 42 ++++-------
>> >> >> include/spi_flash.h | 24 +++---
>> >> >> 8 files changed, 270 insertions(+), 174 deletions(-)
>> >> >> create mode 100644 doc/SPI/README.sf-features
>> >> >>
>> >> >> diff --git a/doc/SPI/README.sf-features b/doc/SPI/README.sf-features
>> >> >> new file mode 100644
>> >> >> index 0000000..d35f56d
>> >> >> --- /dev/null
>> >> >> +++ b/doc/SPI/README.sf-features
>> >> >> @@ -0,0 +1,122 @@
>> >> >> +SPI FLASH feature enhancements:
>> >> >> +==============================
>> >> >> +
>> >> >> +This document describes how to extend the current data structures in
>> >> >> spi subsystem +for making use of new flash features/operations w.r.t
>> >> >> to controller driver support. +
>> >> >> +1. spi_slave:
>> >> >> +
>> >> >> +struct spi_slave {
>> >> >> + ..........
>> >> >> + u32 mode_bits;
>> >> >> + ........
>> >> >> +};
>> >> >> +
>> >> >> + at mode_bits can be used to expose the SPI RX/TX operation modes, bus
>> >> >> options and +few flags which are used to extended the flash specific
>> >> >> features/operations +- include/spi.h
>> >> >> +
>> >> >> +mode_bits:
>> >> >> +- SPI_TX_QPP: 4-Wire tx transfer operation quad page program
>> >> >> +- SPI_RX_SLOW: 1-wire rx transfer operation array slow read
>> >> >> +- SPI_RX_DUAL: 2-wire rx transfer operation dual fast read
>> >> >> +- SPI_RX_DUAL_IO: 2-wire rx transfer operation dual io fast read
>> >> >> +- SPI_RX_QUAD: 4-wire rx transfer operation quad fast read
>> >> >> +- SPI_RX_QUAD_IO: 4-wire rx transfer operation quad io fast read
>> >> >> +- SPI_SHARED: dual flash devices are connected in shared bus
>> >> >> connection +- SPI_SEPARATED: dual flash devices are connected in
>> >> >> separate bus connection +- SPI_U_PAGE: select the upper flash in
>> >> >> dual flash shared bus connection [1] +
>> >> >
>> >> > A generic SPI controller _does_ _not_ _care_ about any SPI flash crud.
>> >> > The SPI bus controller (which is what this is for) and SPI-NOR
>> >> > controller are two different things and must have two different slave
>> >> > structures.
>> >>
>> >> You mean mode_bits need to move in one more structure.
>> >> Just leave about new SPI-NOR as of now for this release we discuss more
>> >> soon.
>> >
>> > The mode_bits have no place in this structure. The slave can indicate
>> > whether it can be connected over 1,2,4... lines , but must not indicate
>> > that it supports some SPI-flash specific properties.
>>
>> What do you mean by this - can you elaborate.
>
> I mean there do exist SPI controllers which can output data via more than one
> (MOSI) line . They do support 1, 2 or even 4 lines. This comes useful when
> driving SPI flashes indeed, _BUT_ (!) this is not tied to driving SPI flashes.
> On the contrary, you can connect any device which can be driven over such
> "parallelised" SPI and it will work. And only such property shall also land in
> the struct spi_slave {} .
>
> Properties like "let's assume the SPI slave is a SPI NOR and it supports SPI NOR
> command FOO" do not go into struct spi_slave.
>
>> As of now drivers in drivers/spi need to inform the flash through spi_slave
>> {} no other alternative ie way remaining flash properties memory_map etc..
>> handle. Even Linux follow the same w/o new SPI-NOR framework.
>
> Linux is now going for a separate SPI-NOR controller framework and this is what
> we will do as well. The SF layer will be only a unifying layer providing access
> to either SPI-NOR API (for the SPI-NOR controllers) or a translation layer to
> communicate SPI-NOR commands over generic SPI API (for the old-school regular
> way of doing SPI NOR connection to SPI bus).
Please correct me logic here!
Just take an example of ti qspi controller.
cmd_sf.c
------------
spi_flash.h
--------------
sf (_probe, _ops), sf.c
-------------------------------------------
drivers/spi/ti_qspi.c (inform flash info mode_bits through spi_slave {}
------------------------------------------------------------
Suppose If we have one more qspi controller(zynq) which is connected to SPI-NOR
cmd_sf.c
-----------
spi_flash.h
---------------
SF-NOR
------------------------------------------------------------------------------------
drivers/spi/ti_qspi.c
drivers/mtd/sf_nor/zynq_qspi.c
(inform flash properties
(inform flash properties
through "mode_bits" of
through "mode_bits" of
spi_slave {})
>
>> If your question, like need a separate structure for flash specific
>> properties please wait will wound-up all these in new framework.
>>
>> I'm planning to push in today release.
>
> I doubt anything is going in today's release ;-)
>
> Actually, you need to really start discerning which _must_ go into a release and
> which simply does not go into a release and waits for -next. This is important
> role of a maintainer and plays important role in keeping the codebase in good
> shape.
>
> Code which is untested, code which changes core stuff and isn't an obvious
> bugfix etc. simply can wait. Code which is obvious bugfix goes in even in the -
> rc period. Of course, there are all kinds of exceptions, that depends on the
> rationale and negotiation.
--
Jagan.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/6] sf: Optimize flash features code
2014-01-21 7:39 ` Jagan Teki
@ 2014-01-21 7:45 ` Jagan Teki
2014-01-21 17:48 ` Marek Vasut
0 siblings, 1 reply; 27+ messages in thread
From: Jagan Teki @ 2014-01-21 7:45 UTC (permalink / raw)
To: u-boot
Hi Marek,
Please ignore the last sent!
On Tue, Jan 21, 2014 at 1:09 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
> Hi Marek,
>
> On Tue, Jan 21, 2014 at 4:31 AM, Marek Vasut <marex@denx.de> wrote:
>> On Monday, January 20, 2014 at 02:32:27 PM, Jagan Teki wrote:
>>> On Mon, Jan 20, 2014 at 6:49 PM, Marek Vasut <marex@denx.de> wrote:
>>> > On Saturday, January 18, 2014 at 09:51:56 PM, Jagan Teki wrote:
>>> >> On Sun, Jan 19, 2014 at 2:09 AM, Marek Vasut <marex@denx.de> wrote:
>>> >> > On Saturday, January 18, 2014 at 09:06:29 PM, Jagannadha Sutradharudu
>>> >> > Teki
>>> >> >
>>> >> > wrote:
>>> >> >> - Shrink spi_slave {}
>>> >> >> - Shrink spi_flash_params {}
>>> >> >> - Documentation for sf features
>>> >> >>
>>> >> >> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
>>> >> >> Cc: Marek Vasut <marex@denx.de>
>>> >> >> ---
>>> >> >>
>>> >> >> doc/SPI/README.sf-features | 122 ++++++++++++++++++++++++++++++
>>> >> >> drivers/mtd/spi/sf.c | 4 +-
>>> >> >> drivers/mtd/spi/sf_internal.h | 1 -
>>> >> >> drivers/mtd/spi/sf_ops.c | 8 +-
>>> >> >> drivers/mtd/spi/sf_params.c | 172
>>> >> >>
>>> >> >> +++++++++++++++++++++--------------------- drivers/mtd/spi/sf_probe.c
>>> >> >>
>>> >> >> 71 ++++++++---------
>>> >> >> include/spi.h | 42 ++++-------
>>> >> >> include/spi_flash.h | 24 +++---
>>> >> >> 8 files changed, 270 insertions(+), 174 deletions(-)
>>> >> >> create mode 100644 doc/SPI/README.sf-features
>>> >> >>
>>> >> >> diff --git a/doc/SPI/README.sf-features b/doc/SPI/README.sf-features
>>> >> >> new file mode 100644
>>> >> >> index 0000000..d35f56d
>>> >> >> --- /dev/null
>>> >> >> +++ b/doc/SPI/README.sf-features
>>> >> >> @@ -0,0 +1,122 @@
>>> >> >> +SPI FLASH feature enhancements:
>>> >> >> +==============================
>>> >> >> +
>>> >> >> +This document describes how to extend the current data structures in
>>> >> >> spi subsystem +for making use of new flash features/operations w.r.t
>>> >> >> to controller driver support. +
>>> >> >> +1. spi_slave:
>>> >> >> +
>>> >> >> +struct spi_slave {
>>> >> >> + ..........
>>> >> >> + u32 mode_bits;
>>> >> >> + ........
>>> >> >> +};
>>> >> >> +
>>> >> >> + at mode_bits can be used to expose the SPI RX/TX operation modes, bus
>>> >> >> options and +few flags which are used to extended the flash specific
>>> >> >> features/operations +- include/spi.h
>>> >> >> +
>>> >> >> +mode_bits:
>>> >> >> +- SPI_TX_QPP: 4-Wire tx transfer operation quad page program
>>> >> >> +- SPI_RX_SLOW: 1-wire rx transfer operation array slow read
>>> >> >> +- SPI_RX_DUAL: 2-wire rx transfer operation dual fast read
>>> >> >> +- SPI_RX_DUAL_IO: 2-wire rx transfer operation dual io fast read
>>> >> >> +- SPI_RX_QUAD: 4-wire rx transfer operation quad fast read
>>> >> >> +- SPI_RX_QUAD_IO: 4-wire rx transfer operation quad io fast read
>>> >> >> +- SPI_SHARED: dual flash devices are connected in shared bus
>>> >> >> connection +- SPI_SEPARATED: dual flash devices are connected in
>>> >> >> separate bus connection +- SPI_U_PAGE: select the upper flash in
>>> >> >> dual flash shared bus connection [1] +
>>> >> >
>>> >> > A generic SPI controller _does_ _not_ _care_ about any SPI flash crud.
>>> >> > The SPI bus controller (which is what this is for) and SPI-NOR
>>> >> > controller are two different things and must have two different slave
>>> >> > structures.
>>> >>
>>> >> You mean mode_bits need to move in one more structure.
>>> >> Just leave about new SPI-NOR as of now for this release we discuss more
>>> >> soon.
>>> >
>>> > The mode_bits have no place in this structure. The slave can indicate
>>> > whether it can be connected over 1,2,4... lines , but must not indicate
>>> > that it supports some SPI-flash specific properties.
>>>
>>> What do you mean by this - can you elaborate.
>>
>> I mean there do exist SPI controllers which can output data via more than one
>> (MOSI) line . They do support 1, 2 or even 4 lines. This comes useful when
>> driving SPI flashes indeed, _BUT_ (!) this is not tied to driving SPI flashes.
>> On the contrary, you can connect any device which can be driven over such
>> "parallelised" SPI and it will work. And only such property shall also land in
>> the struct spi_slave {} .
>>
>> Properties like "let's assume the SPI slave is a SPI NOR and it supports SPI NOR
>> command FOO" do not go into struct spi_slave.
>>
>>> As of now drivers in drivers/spi need to inform the flash through spi_slave
>>> {} no other alternative ie way remaining flash properties memory_map etc..
>>> handle. Even Linux follow the same w/o new SPI-NOR framework.
>>
>> Linux is now going for a separate SPI-NOR controller framework and this is what
>> we will do as well. The SF layer will be only a unifying layer providing access
>> to either SPI-NOR API (for the SPI-NOR controllers) or a translation layer to
>> communicate SPI-NOR commands over generic SPI API (for the old-school regular
>> way of doing SPI NOR connection to SPI bus).
Just take an example of ti qspi controller which is connected to serial nor
Current design:
cmd_sf.c
------------
spi_flash.h
--------------
sf (_probe, _ops), sf.c
-------------------------------------------
drivers/spi/ti_qspi.c (inform flash properties through "mode_bits" spi_slave {}
-------------------------------------------------------------------------------
Proposed one (may be):
Suppose If we have one more qspi controller(zynq) which is connected
to serial nor
cmd_sf.c
-----------
spi_flash.h
---------------
SF-NOR
------------------------------------------------------------------------------------------------
drivers/spi/ti_qspi.c
drivers/mtd/sf_nor/zynq_qspi.c
(inform flash properties
(inform flash properties
through "mode_bits" of
through "mode_bits" of
spi_slave {})
sf_nor_device {}
Please comment on above proposed design will ask you more
once this set..thanks!
--
Jagan.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/6] sf: Optimize flash features code
2014-01-21 7:45 ` Jagan Teki
@ 2014-01-21 17:48 ` Marek Vasut
0 siblings, 0 replies; 27+ messages in thread
From: Marek Vasut @ 2014-01-21 17:48 UTC (permalink / raw)
To: u-boot
On Tuesday, January 21, 2014 at 08:45:52 AM, Jagan Teki wrote:
[...]
> Current design:
>
> cmd_sf.c
This is just the one of the users of the SPI flash API, but not the only one.
> ------------
> spi_flash.h
> --------------
> sf (_probe, _ops), sf.c
> -------------------------------------------
> drivers/spi/ti_qspi.c (inform flash properties through "mode_bits"
> spi_slave {}
> --------------------------------------------------------------------------
> -----
>
> Proposed one (may be):
>
> Suppose If we have one more qspi controller(zynq) which is connected
> to serial nor
>
> cmd_sf.c
> -----------
> spi_flash.h
> ---------------
> SF-NOR
> ---------------------------------------------------------------------------
> --------------------- drivers/spi/ti_qspi.c
> drivers/mtd/sf_nor/zynq_qspi.c
> (inform flash properties
> (inform flash properties
> through "mode_bits" of
> through "mode_bits" of
> spi_slave {})
> sf_nor_device {}
Did the formating here go awry ?
But anyway, your sf_probe.c/sf_ops.c/sf.c will only need to somehow discern
whether to call the regular SPI API or SPI-NOR API functions.
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2014-01-21 17:48 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1390075593-11226-1-git-send-email-jaganna@xilinx.com>
2014-01-18 20:06 ` [U-Boot] [PATCH 1/6] sf: ops: Squash the malloc+memset combo Jagannadha Sutradharudu Teki
2014-01-18 20:34 ` Marek Vasut
2014-01-18 20:06 ` [U-Boot] [PATCH 2/6] sf: Optimize flash features code Jagannadha Sutradharudu Teki
2014-01-18 20:39 ` Marek Vasut
2014-01-18 20:51 ` Jagan Teki
2014-01-20 13:19 ` Marek Vasut
2014-01-20 13:32 ` Jagan Teki
2014-01-20 23:01 ` Marek Vasut
2014-01-21 7:39 ` Jagan Teki
2014-01-21 7:45 ` Jagan Teki
2014-01-21 17:48 ` Marek Vasut
2014-01-18 20:06 ` [U-Boot] [PATCH 3/6] sf: Renames on dual_flash stuff Jagannadha Sutradharudu Teki
2014-01-18 20:37 ` Marek Vasut
2014-01-18 20:49 ` Jagan Teki
2014-01-20 13:16 ` Marek Vasut
2014-01-20 13:35 ` Jagan Teki
2014-01-18 20:06 ` [U-Boot] [PATCH 4/6] sf: Update read/write command macros Jagannadha Sutradharudu Teki
2014-01-18 20:36 ` Marek Vasut
2014-01-18 20:45 ` Jagan Teki
2014-01-20 11:13 ` Detlev Zundel
2014-01-20 11:46 ` Jagan Teki
2014-01-20 13:06 ` Marek Vasut
2014-01-20 13:10 ` Jagan Teki
2014-01-20 13:13 ` Jagan Teki
2014-01-20 14:33 ` Detlev Zundel
2014-01-18 20:06 ` [U-Boot] [PATCH 5/6] sf: Minor macro cleanups Jagannadha Sutradharudu Teki
2014-01-18 20:06 ` [U-Boot] [PATCH 6/6] sf: Update bank configuration Jagannadha Sutradharudu Teki
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.