[U-Boot] [PATCH 3/6] sf: Renames on dual_flash stuff

- 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 *

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

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.

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

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!
participants (3)
-
Jagan Teki
-
Jagannadha Sutradharudu Teki
-
Marek Vasut