[U-Boot] [PATCH v7 01/17] sf: Add extended read commands support
Current sf uses FAST_READ command, this patch adds support to use the different/extended read command.
This implementation will determine the fastest command by taking the supported commands from the flash and the controller, controller is always been a priority.
Signed-off-by: Jagannadha Sutradharudu Teki jaganna@xilinx.com --- drivers/mtd/spi/sf_internal.h | 2 + drivers/mtd/spi/sf_ops.c | 2 +- drivers/mtd/spi/sf_probe.c | 190 +++++++++++++++++++++++------------------- include/spi.h | 8 ++ include/spi_flash.h | 10 +++ 5 files changed, 126 insertions(+), 86 deletions(-)
diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h index d291746..938a78e 100644 --- a/drivers/mtd/spi/sf_internal.h +++ b/drivers/mtd/spi/sf_internal.h @@ -36,6 +36,8 @@ /* 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_ID 0x9f
/* Bank addr access commands */ diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c index e316a69..49ceef0 100644 --- a/drivers/mtd/spi/sf_ops.c +++ b/drivers/mtd/spi/sf_ops.c @@ -285,7 +285,7 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset, return 0; }
- cmd[0] = CMD_READ_ARRAY_FAST; + cmd[0] = flash->read_cmd; cmd[4] = 0x00;
while (len) { diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c index b863a98..c0baac6 100644 --- a/drivers/mtd/spi/sf_probe.c +++ b/drivers/mtd/spi/sf_probe.c @@ -27,6 +27,7 @@ DECLARE_GLOBAL_DATA_PTR; * @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 */ struct spi_flash_params { @@ -35,110 +36,111 @@ struct spi_flash_params { u16 ext_jedec; u32 sector_size; u32 nr_sectors; + u8 e_rd_cmd; u16 flags; };
static const struct spi_flash_params spi_flash_params_table[] = { #ifdef CONFIG_SPI_FLASH_ATMEL /* ATMEL */ - {"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}, + {"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}, #endif #ifdef CONFIG_SPI_FLASH_EON /* EON */ - {"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}, + {"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}, #endif #ifdef CONFIG_SPI_FLASH_GIGADEVICE /* GIGADEVICE */ - {"GD25Q64B", 0xc84017, 0x0, 64 * 1024, 128, SECT_4K}, - {"GD25LQ32", 0xc86016, 0x0, 64 * 1024, 64, SECT_4K}, + {"GD25Q64B", 0xc84017, 0x0, 64 * 1024, 128, 0, SECT_4K}, + {"GD25LQ32", 0xc86016, 0x0, 64 * 1024, 64, 0, SECT_4K}, #endif #ifdef CONFIG_SPI_FLASH_MACRONIX /* MACRONIX */ - {"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, 0}, - {"MX25L25635F", 0xc22019, 0x0, 64 * 1024, 512, 0}, - {"MX25L51235F", 0xc2201a, 0x0, 64 * 1024, 1024, 0}, - {"MX25L12855E", 0xc22618, 0x0, 64 * 1024, 256, 0}, + {"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, 0, 0}, + {"MX25L25635F", 0xc22019, 0x0, 64 * 1024, 512, 0, 0}, + {"MX25L51235F", 0xc2201a, 0x0, 64 * 1024, 1024, 0, 0}, + {"MX25L12855E", 0xc22618, 0x0, 64 * 1024, 256, 0, 0}, #endif #ifdef CONFIG_SPI_FLASH_SPANSION /* SPANSION */ - {"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, 0}, - {"S25FL128P_64K", 0x012018, 0x0301, 64 * 1024, 256, 0}, - {"S25FL032P", 0x010215, 0x4d00, 64 * 1024, 64, 0}, - {"S25FL064P", 0x010216, 0x4d00, 64 * 1024, 128, 0}, - {"S25FL128S_64K", 0x012018, 0x4d01, 64 * 1024, 256, 0}, - {"S25FL256S_256K", 0x010219, 0x4d00, 64 * 1024, 512, 0}, - {"S25FL256S_64K", 0x010219, 0x4d01, 64 * 1024, 512, 0}, - {"S25FL512S_256K", 0x010220, 0x4d00, 64 * 1024, 1024, 0}, - {"S25FL512S_64K", 0x010220, 0x4d01, 64 * 1024, 1024, 0}, + {"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, 0, 0}, + {"S25FL128P_64K", 0x012018, 0x0301, 64 * 1024, 256, 0, 0}, + {"S25FL032P", 0x010215, 0x4d00, 64 * 1024, 64, 0, 0}, + {"S25FL064P", 0x010216, 0x4d00, 64 * 1024, 128, 0, 0}, + {"S25FL128S_64K", 0x012018, 0x4d01, 64 * 1024, 256, 0, 0}, + {"S25FL256S_256K", 0x010219, 0x4d00, 64 * 1024, 512, RD_EXTN, 0}, + {"S25FL256S_64K", 0x010219, 0x4d01, 64 * 1024, 512, RD_EXTN, 0}, + {"S25FL512S_256K", 0x010220, 0x4d00, 64 * 1024, 1024, 0, 0}, + {"S25FL512S_64K", 0x010220, 0x4d01, 64 * 1024, 1024, 0, 0}, #endif #ifdef CONFIG_SPI_FLASH_STMICRO /* STMICRO */ - {"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, SECT_4K}, - {"N25Q32A", 0x20bb16, 0x0, 64 * 1024, 64, SECT_4K}, - {"N25Q64", 0x20ba17, 0x0, 64 * 1024, 128, SECT_4K}, - {"N25Q64A", 0x20bb17, 0x0, 64 * 1024, 128, SECT_4K}, - {"N25Q128", 0x20ba18, 0x0, 64 * 1024, 256, SECT_4K}, - {"N25Q128A", 0x20bb18, 0x0, 64 * 1024, 256, SECT_4K}, - {"N25Q256", 0x20ba19, 0x0, 64 * 1024, 512, SECT_4K}, - {"N25Q256A", 0x20bb19, 0x0, 64 * 1024, 512, SECT_4K}, - {"N25Q512", 0x20ba20, 0x0, 64 * 1024, 1024, E_FSR | SECT_4K}, - {"N25Q512A", 0x20bb20, 0x0, 64 * 1024, 1024, E_FSR | SECT_4K}, - {"N25Q1024", 0x20ba21, 0x0, 64 * 1024, 2048, E_FSR | SECT_4K}, - {"N25Q1024A", 0x20bb21, 0x0, 64 * 1024, 2048, E_FSR | SECT_4K}, + {"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, 0, SECT_4K}, + {"N25Q32A", 0x20bb16, 0x0, 64 * 1024, 64, 0, SECT_4K}, + {"N25Q64", 0x20ba17, 0x0, 64 * 1024, 128, 0, SECT_4K}, + {"N25Q64A", 0x20bb17, 0x0, 64 * 1024, 128, 0, SECT_4K}, + {"N25Q128", 0x20ba18, 0x0, 64 * 1024, 256, 0, SECT_4K}, + {"N25Q128A", 0x20bb18, 0x0, 64 * 1024, 256, 0, SECT_4K}, + {"N25Q256", 0x20ba19, 0x0, 64 * 1024, 512, 0, SECT_4K}, + {"N25Q256A", 0x20bb19, 0x0, 64 * 1024, 512, 0, SECT_4K}, + {"N25Q512", 0x20ba20, 0x0, 64 * 1024, 1024, 0, E_FSR | SECT_4K}, + {"N25Q512A", 0x20bb20, 0x0, 64 * 1024, 1024, 0, E_FSR | SECT_4K}, + {"N25Q1024", 0x20ba21, 0x0, 64 * 1024, 2048, 0, E_FSR | SECT_4K}, + {"N25Q1024A", 0x20bb21, 0x0, 64 * 1024, 2048, 0, E_FSR | SECT_4K}, #endif #ifdef CONFIG_SPI_FLASH_SST /* SST */ - {"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}, + {"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}, #endif #ifdef CONFIG_SPI_FLASH_WINBOND /* WINBOND */ - {"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, SECT_4K}, - {"W25Q16CL", 0xef4015, 0x0, 64 * 1024, 32, SECT_4K}, - {"W25Q32BV", 0xef4016, 0x0, 64 * 1024, 64, SECT_4K}, - {"W25Q64CV", 0xef4017, 0x0, 64 * 1024, 128, SECT_4K}, - {"W25Q128BV", 0xef4018, 0x0, 64 * 1024, 256, SECT_4K}, - {"W25Q256", 0xef4019, 0x0, 64 * 1024, 512, SECT_4K}, - {"W25Q80BW", 0xef5014, 0x0, 64 * 1024, 16, SECT_4K}, - {"W25Q16DW", 0xef6015, 0x0, 64 * 1024, 32, SECT_4K}, - {"W25Q32DW", 0xef6016, 0x0, 64 * 1024, 64, SECT_4K}, - {"W25Q64DW", 0xef6017, 0x0, 64 * 1024, 128, SECT_4K}, - {"W25Q128FW", 0xef6018, 0x0, 64 * 1024, 256, SECT_4K}, + {"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, 0, SECT_4K}, + {"W25Q16CL", 0xef4015, 0x0, 64 * 1024, 32, 0, SECT_4K}, + {"W25Q32BV", 0xef4016, 0x0, 64 * 1024, 64, 0, SECT_4K}, + {"W25Q64CV", 0xef4017, 0x0, 64 * 1024, 128, 0, SECT_4K}, + {"W25Q128BV", 0xef4018, 0x0, 64 * 1024, 256, 0, SECT_4K}, + {"W25Q256", 0xef4019, 0x0, 64 * 1024, 512, 0, SECT_4K}, + {"W25Q80BW", 0xef5014, 0x0, 64 * 1024, 16, 0, SECT_4K}, + {"W25Q16DW", 0xef6015, 0x0, 64 * 1024, 32, 0, SECT_4K}, + {"W25Q32DW", 0xef6016, 0x0, 64 * 1024, 64, 0, SECT_4K}, + {"W25Q64DW", 0xef6017, 0x0, 64 * 1024, 128, 0, SECT_4K}, + {"W25Q128FW", 0xef6018, 0x0, 64 * 1024, 256, 0, SECT_4K}, #endif /* * Note: @@ -155,12 +157,20 @@ static const struct spi_flash_params spi_flash_params_table[] = { */ };
+/* Read commands array */ +static u8 spi_read_cmds_array[] = { + CMD_READ_ARRAY_SLOW, + CMD_READ_DUAL_OUTPUT_FAST, + CMD_READ_DUAL_IO_FAST, +}; + static struct spi_flash *spi_flash_validate_params(struct spi_slave *spi, u8 *idcode) { const struct spi_flash_params *params; struct spi_flash *flash; int i; + u8 cmd; u16 jedec = idcode[1] << 8 | idcode[2]; u16 ext_jedec = idcode[3] << 8 | idcode[4];
@@ -222,6 +232,16 @@ 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 for default supported read cmd */ + flash->read_cmd = CMD_READ_ARRAY_FAST; + } + /* Poll cmd seclection */ flash->poll_cmd = CMD_READ_STATUS; #ifdef CONFIG_SPI_FLASH_STMICRO diff --git a/include/spi.h b/include/spi.h index aba7922..31195a3 100644 --- a/include/spi.h +++ b/include/spi.h @@ -31,6 +31,12 @@ #define SPI_XFER_MMAP_END 0x10 /* Memory Mapped End */ #define SPI_XFER_ONCE (SPI_XFER_BEGIN | SPI_XFER_END)
+/* 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_EXTN SPI_OPM_RX_AS | SPI_OPM_RX_DOUT | SPI_OPM_RX_DIO + /* Header byte that marks the start of the message */ #define SPI_PREAMBLE_END_BYTE 0xec
@@ -43,6 +49,7 @@ * * @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. * @wordlen: Size of SPI word in number of bits * @max_write_size: If non-zero, the maximum number of bytes which can * be written at once, excluding command bytes. @@ -51,6 +58,7 @@ struct spi_slave { unsigned int bus; unsigned int cs; + u8 op_mode_rx; unsigned int wordlen; unsigned int max_write_size; void *memory_map; diff --git a/include/spi_flash.h b/include/spi_flash.h index afc3a58..692e143 100644 --- a/include/spi_flash.h +++ b/include/spi_flash.h @@ -19,6 +19,14 @@ #include <linux/types.h> #include <linux/compiler.h>
+/* Enum list - Extended read commands */ +enum spi_read_cmds { + ARRAY_SLOW = 1 << 0, + DUAL_OUTPUT_FAST = 1 << 1, + DUAL_IO_FAST = 1 << 2, +}; +#define RD_EXTN ARRAY_SLOW | DUAL_OUTPUT_FAST | DUAL_IO_FAST + /** * struct spi_flash - SPI flash structure * @@ -33,6 +41,7 @@ * @bank_curr: Current flash bank * @poll_cmd: Poll cmd - for flash erase/program * @erase_cmd: Erase cmd 4K, 32K, 64K + * @read_cmd: Read cmd - Array Fast and Extn read * @memory_map: Address of read-only SPI flash access * @read: Flash read ops: Read len bytes at offset into buf * Supported cmds: Fast Array Read @@ -57,6 +66,7 @@ struct spi_flash { #endif u8 poll_cmd; u8 erase_cmd; + u8 read_cmd;
void *memory_map; int (*read)(struct spi_flash *flash, u32 offset, size_t len, void *buf);
On Sun, Jan 12, 2014 at 22:29 +0530, Jagannadha Sutradharudu Teki wrote:
Current sf uses FAST_READ command, this patch adds support to use the different/extended read command.
This implementation will determine the fastest command by taking the supported commands from the flash and the controller, controller is always been a priority.
I don't quite get this. You have a combination of several components which all have to participate appropriately when you want the total of it to work correctly.
How can one specific component "be a priority" there? Isn't the lowest common denominator required instead? The strength of a chain is determined by its weakest link after all.
The other issue I have here (as well as in Linux kernel changes and discussions) is that people seem to forget that the board is involved as well.
Your commit message suggests that "parallel" communication is used as soon as - the SPI controller has such a feature - a flash chip was detected which according to a fixed database supports this feature
This decision is automatic. Neither is there a "how is the board wired?" condition involved, nor can users or even developers adjust this behaviour (say, by optionally setting a maximum width).
I'd suggest to provide a "maximum number of lines to use" option, for the same reasons that we may want to limit transfer rates instead of always using the maximum of what individual chips could do (the reasons: the number of lines connected as well as their electrical and mechanical properties do have an impact).
Alternatively one may implement auto-detection, starting at the most capable feature and falling back until a working condition was found. But this is fragile if the communication used at detection time does not involve all potentially used lines (and in all directions, mind you). This won't solve potential bugs in chips either which the user may want to override. Or EMI related restrictions which a developer may want to apply despite the chips' and board's being more capable. Or pin mux settings which dedicate unused "upper SPI data lines" to other features (as we have seen with MMC cards recently).
virtually yours Gerhard Sittig
Hi Gerhard Sittig,
Thanks for your comments, please see below
On Mon, Jan 13, 2014 at 3:04 PM, Gerhard Sittig gsi@denx.de wrote:
On Sun, Jan 12, 2014 at 22:29 +0530, Jagannadha Sutradharudu Teki wrote:
Current sf uses FAST_READ command, this patch adds support to use the different/extended read command.
This implementation will determine the fastest command by taking the supported commands from the flash and the controller, controller is always been a priority.
I don't quite get this. You have a combination of several components which all have to participate appropriately when you want the total of it to work correctly.
How can one specific component "be a priority" there? Isn't the lowest common denominator required instead? The strength of a chain is determined by its weakest link after all.
The other issue I have here (as well as in Linux kernel changes and discussions) is that people seem to forget that the board is involved as well.
Your commit message suggests that "parallel" communication is used as soon as
- the SPI controller has such a feature
- a flash chip was detected which according to a fixed database supports this feature
This decision is automatic. Neither is there a "how is the board wired?" condition involved, nor can users or even developers adjust this behaviour (say, by optionally setting a maximum width).
I'd suggest to provide a "maximum number of lines to use" option, for the same reasons that we may want to limit transfer rates instead of always using the maximum of what individual chips could do (the reasons: the number of lines connected as well as their electrical and mechanical properties do have an impact).
Alternatively one may implement auto-detection, starting at the most capable feature and falling back until a working condition was found. But this is fragile if the communication used at detection time does not involve all potentially used lines (and in all directions, mind you). This won't solve potential bugs in chips either which the user may want to override. Or EMI related restrictions which a developer may want to apply despite the chips' and board's being more capable. Or pin mux settings which dedicate unused "upper SPI data lines" to other features (as we have seen with MMC cards recently).
Yes the main reason for going maximum or based on controller setting transfer is to improve sf performance. I missed your point as didn't see much use cases from board to connected flash perceptive.
I thought we have two options to meet this requirement. 1. dynamic - auto-detection: Get the max_nof_lines based on the board to connected flash. So-that we can configure the transfer mode based on the lines. no idea yet, how to get the max_nof_lines dynamically - any inputs. 2. static: from user level we may give the max_nof_lines ex: sf probe probe [[[bus:]cs]:max_nof_line] [hz] [mode] If user can't input max_nof_line so-that driver will go with single and the rest case transfer modes assigned based on the given input.
Please comment?
Hi,
On Mon, Jan 13, 2014 at 4:26 PM, Jagan Teki jagannadh.teki@gmail.com wrote:
Hi Gerhard Sittig,
Thanks for your comments, please see below
On Mon, Jan 13, 2014 at 3:04 PM, Gerhard Sittig gsi@denx.de wrote:
On Sun, Jan 12, 2014 at 22:29 +0530, Jagannadha Sutradharudu Teki wrote:
Current sf uses FAST_READ command, this patch adds support to use the different/extended read command.
This implementation will determine the fastest command by taking the supported commands from the flash and the controller, controller is always been a priority.
I don't quite get this. You have a combination of several components which all have to participate appropriately when you want the total of it to work correctly.
How can one specific component "be a priority" there? Isn't the lowest common denominator required instead? The strength of a chain is determined by its weakest link after all.
The other issue I have here (as well as in Linux kernel changes and discussions) is that people seem to forget that the board is involved as well.
Your commit message suggests that "parallel" communication is used as soon as
- the SPI controller has such a feature
- a flash chip was detected which according to a fixed database supports this feature
This decision is automatic. Neither is there a "how is the board wired?" condition involved, nor can users or even developers adjust this behaviour (say, by optionally setting a maximum width).
I'd suggest to provide a "maximum number of lines to use" option, for the same reasons that we may want to limit transfer rates instead of always using the maximum of what individual chips could do (the reasons: the number of lines connected as well as their electrical and mechanical properties do have an impact).
Alternatively one may implement auto-detection, starting at the most capable feature and falling back until a working condition was found. But this is fragile if the communication used at detection time does not involve all potentially used lines (and in all directions, mind you). This won't solve potential bugs in chips either which the user may want to override. Or EMI related restrictions which a developer may want to apply despite the chips' and board's being more capable. Or pin mux settings which dedicate unused "upper SPI data lines" to other features (as we have seen with MMC cards recently).
Yes the main reason for going maximum or based on controller setting transfer is to improve sf performance. I missed your point as didn't see much use cases from board to connected flash perceptive.
I thought we have two options to meet this requirement.
- dynamic - auto-detection: Get the max_nof_lines based on the board to connected flash. So-that we can configure the transfer mode based on the lines. no idea yet, how to get the max_nof_lines dynamically - any inputs.
- static: from user level we may give the max_nof_lines ex: sf probe probe [[[bus:]cs]:max_nof_line] [hz] [mode] If user can't input max_nof_line so-that driver will go with single and the rest case transfer modes assigned based on the given input.
As this change involves mostly on the driver parts I a thinking to go-ahead and try to push this series to master. yes I would mark this as my TODO list and will trigger one more thread.
Hope this idea sounds valid, let me know for any issues.
On Mon, Jan 13, 2014 at 18:15 +0530, Jagan Teki wrote:
On Mon, Jan 13, 2014 at 4:26 PM, Jagan Teki jagannadh.teki@gmail.com wrote:
Yes the main reason for going maximum or based on controller setting transfer is to improve sf performance. I missed your point as didn't see much use cases from board to connected flash perceptive.
I thought we have two options to meet this requirement.
- dynamic - auto-detection: Get the max_nof_lines based on the board to connected flash. So-that we can configure the transfer mode based on the lines. no idea yet, how to get the max_nof_lines dynamically - any inputs.
- static: from user level we may give the max_nof_lines ex: sf probe probe [[[bus:]cs]:max_nof_line] [hz] [mode] If user can't input max_nof_line so-that driver will go with single and the rest case transfer modes assigned based on the given input.
As outlined in my previous reply, I'm afraid that the auto detection will always end up being inferior (unreliable, potentially dangerous, still not under the user's control).
A simple approach might be to have the user specify the number of data lines to use (as in your "static" example above). The question remains whether this specified number shall be taken as the exact number or as the maximum number of data lines.
As this change involves mostly on the driver parts I a thinking to go-ahead and try to push this series to master. yes I would mark this as my TODO list and will trigger one more thread.
Hope this idea sounds valid, let me know for any issues.
Agreed, as there are few or no complaints and concerns (yet?), let's address this limiting the number of data lines at a later point in time. Just keep awareness, that was my most important issue here. :)
Given that Quad-SPI is rather new, I'd expect new designs to be "consistent" (i.e. always quad capable flash chips attached to quad capable controllers and connected via the maximum number of lines known by that time), while only after some more time the "interesting" combinations crop up (like starting with non-quad flash chip and only the necessary connections and a quad capable controller, then dropping in more recent chips without re-rolling the PCB layout; or upgrading/replacing SoCs and flash chips with "compatible" devices and not being aware of the software's "detecting" the maximum while ignoring the wires on board).
virtually yours Gerhard Sittig
participants (3)
-
Gerhard Sittig -
Jagan Teki -
Jagannadha Sutradharudu Teki