On Fri, Jan 17, 2014 at 10:01 PM, Marek Vasut <[email protected]> wrote: > On Friday, January 17, 2014 at 03:41:46 PM, Jagannadha Sutradharudu Teki > wrote: >> Combined spi flash stuff as minimum as possible. > > Squashing the names of the flags only makes it unreadable :-( > >> Signed-off-by: Jagannadha Sutradharudu Teki <[email protected]> >> Cc: Marek Vasut <[email protected]> >> --- >> drivers/mtd/spi/sf.c | 4 ++-- >> drivers/mtd/spi/sf_ops.c | 18 +++++++++--------- >> drivers/mtd/spi/sf_probe.c | 19 ++++++++++++------- >> include/spi.h | 45 >> +++++++++++++++++++-------------------------- include/spi_flash.h | >> 6 +++--- >> 5 files changed, 45 insertions(+), 47 deletions(-) >> >> diff --git a/drivers/mtd/spi/sf.c b/drivers/mtd/spi/sf.c >> index 664e860..fb91ac6 100644 >> --- a/drivers/mtd/spi/sf.c >> +++ b/drivers/mtd/spi/sf.c >> @@ -19,8 +19,8 @@ static int spi_flash_read_write(struct spi_slave *spi, >> int ret; >> >> #ifdef CONFIG_SF_DUAL_FLASH >> - if (spi->flags & SPI_XFER_U_PAGE) >> - flags |= SPI_XFER_U_PAGE; >> + if (spi->mode_bits & SPI_U_PAGE) >> + flags |= SPI_U_PAGE; >> #endif >> if (data_len == 0) >> flags |= SPI_XFER_END; >> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c >> index 9650486..0d87e1b 100644 >> --- a/drivers/mtd/spi/sf_ops.c >> +++ b/drivers/mtd/spi/sf_ops.c >> @@ -135,15 +135,15 @@ static int spi_flash_bank(struct spi_flash *flash, >> u32 offset) static void spi_flash_dual_flash(struct spi_flash *flash, u32 >> *addr) { >> switch (flash->dual_flash) { >> - case SF_DUAL_STACKED_FLASH: >> + case SF_STACKED: >> if (*addr >= (flash->size >> 1)) { >> *addr -= flash->size >> 1; >> - flash->spi->flags |= SPI_XFER_U_PAGE; >> + flash->spi->mode_bits |= SPI_U_PAGE; >> } else { >> - flash->spi->flags &= ~SPI_XFER_U_PAGE; >> + flash->spi->mode_bits &= ~SPI_U_PAGE; >> } >> break; >> - case SF_DUAL_PARALLEL_FLASH: >> + case SF_PARALLEL: >> *addr >>= flash->shift; >> break; >> default: >> @@ -170,8 +170,8 @@ int spi_flash_cmd_wait_ready(struct spi_flash *flash, >> unsigned long timeout) } >> >> #ifdef CONFIG_SF_DUAL_FLASH >> - if (spi->flags & SPI_XFER_U_PAGE) >> - flags |= SPI_XFER_U_PAGE; >> + if (spi->mode_bits & SPI_U_PAGE) >> + flags |= SPI_U_PAGE; >> #endif >> ret = spi_xfer(spi, 8, &cmd, NULL, flags); >> if (ret) { >> @@ -261,7 +261,7 @@ int spi_flash_cmd_erase_ops(struct spi_flash *flash, >> u32 offset, size_t len) erase_addr = offset; >> >> #ifdef CONFIG_SF_DUAL_FLASH >> - if (flash->dual_flash > SF_SINGLE_FLASH) >> + if (flash->dual_flash > SF_SINGLE) >> spi_flash_dual_flash(flash, &erase_addr); >> #endif >> #ifdef CONFIG_SPI_FLASH_BAR >> @@ -303,7 +303,7 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash, >> u32 offset, write_addr = offset; >> >> #ifdef CONFIG_SF_DUAL_FLASH >> - if (flash->dual_flash > SF_SINGLE_FLASH) >> + if (flash->dual_flash > SF_SINGLE) >> spi_flash_dual_flash(flash, &write_addr); >> #endif >> #ifdef CONFIG_SPI_FLASH_BAR >> @@ -388,7 +388,7 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 >> offset, read_addr = offset; >> >> #ifdef CONFIG_SF_DUAL_FLASH >> - if (flash->dual_flash > SF_SINGLE_FLASH) >> + if (flash->dual_flash > SF_SINGLE) >> spi_flash_dual_flash(flash, &read_addr); >> #endif >> #ifdef CONFIG_SPI_FLASH_BAR >> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c >> index e84ab13..003f635 100644 >> --- a/drivers/mtd/spi/sf_probe.c >> +++ b/drivers/mtd/spi/sf_probe.c >> @@ -133,8 +133,13 @@ static struct spi_flash >> *spi_flash_validate_params(struct spi_slave *spi, flash->spi = spi; >> flash->name = params->name; >> flash->memory_map = spi->memory_map; >> - flash->dual_flash = flash->spi->option; >> >> +#ifdef CONFIG_SF_DUAL_FLASH > > Looks new and unrelated. > >> + if (flash->spi->mode_bits & SPI_SHARED) >> + flash->dual_flash = SF_STACKED; >> + else if (flash->spi->mode_bits & SPI_SEPARATED) >> + flash->dual_flash = SF_PARALLEL; >> +#endif >> /* Assign spi_flash ops */ >> flash->write = spi_flash_cmd_write_ops; >> #ifdef CONFIG_SPI_FLASH_SST >> @@ -145,12 +150,12 @@ static struct spi_flash >> *spi_flash_validate_params(struct spi_slave *spi, flash->read = >> spi_flash_cmd_read_ops; >> >> /* Compute the flash size */ >> - flash->shift = (flash->dual_flash & SF_DUAL_PARALLEL_FLASH) ? 1 : 0; >> + flash->shift = (flash->dual_flash & SF_PARALLEL) ? 1 : 0; >> flash->page_size = ((ext_jedec == 0x4d00) ? 512 : 256) << flash->shift; >> flash->sector_size = params->sector_size << flash->shift; >> flash->size = flash->sector_size * params->nr_sectors << flash->shift; >> #ifdef CONFIG_SF_DUAL_FLASH >> - if (flash->dual_flash & SF_DUAL_STACKED_FLASH) >> + if (flash->dual_flash & SF_STACKED) >> flash->size <<= 1; >> #endif >> >> @@ -167,7 +172,7 @@ static struct spi_flash >> *spi_flash_validate_params(struct spi_slave *spi, } >> >> /* Look for the fastest read cmd */ >> - cmd = fls(params->e_rd_cmd & flash->spi->op_mode_rx); >> + cmd = fls(params->e_rd_cmd & flash->spi->rx_mode); >> if (cmd) { >> cmd = spi_read_cmds_array[cmd - 1]; >> flash->read_cmd = cmd; >> @@ -177,7 +182,7 @@ static struct spi_flash >> *spi_flash_validate_params(struct spi_slave *spi, } >> >> /* Not require to look for fastest only two write cmds yet */ >> - if (params->flags & WR_QPP && flash->spi->op_mode_tx & SPI_OPM_TX_QPP) >> + if (params->flags & WR_QPP && flash->spi->mode_bits & SPI_TX_QPP) >> flash->write_cmd = CMD_QUAD_PAGE_PROGRAM; >> else >> /* Go for default supported write cmd */ >> @@ -329,9 +334,9 @@ static struct spi_flash *spi_flash_probe_slave(struct >> spi_slave *spi) puts("\n"); >> #endif >> #ifndef CONFIG_SPI_FLASH_BAR >> - if (((flash->dual_flash == SF_SINGLE_FLASH) && >> + if (((flash->dual_flash == SF_SINGLE) && >> (flash->size > SPI_FLASH_16MB_BOUN)) || >> - ((flash->dual_flash > SF_SINGLE_FLASH) && >> + ((flash->dual_flash > SF_SINGLE) && >> (flash->size > SPI_FLASH_16MB_BOUN << 1))) { >> puts("SF: Warning - Only lower 16MiB accessible,"); >> puts(" Full access #define CONFIG_SPI_FLASH_BAR\n"); >> diff --git a/include/spi.h b/include/spi.h >> index ffd6647..07ba4d0 100644 >> --- a/include/spi.h >> +++ b/include/spi.h >> @@ -30,29 +30,25 @@ >> #define SPI_XFER_MMAP 0x08 /* Memory Mapped start */ >> #define SPI_XFER_MMAP_END 0x10 /* Memory Mapped End */ >> #define SPI_XFER_ONCE (SPI_XFER_BEGIN | SPI_XFER_END) >> -#define SPI_XFER_U_PAGE (1 << 5) >> - >> -/* SPI TX operation modes */ >> -#define SPI_OPM_TX_QPP 1 << 0 >> - >> -/* SPI RX operation modes */ >> -#define SPI_OPM_RX_AS 1 << 0 >> -#define SPI_OPM_RX_DOUT 1 << 1 >> -#define SPI_OPM_RX_DIO 1 << 2 >> -#define SPI_OPM_RX_QOF 1 << 3 >> -#define SPI_OPM_RX_QIOF 1 << 4 >> -#define SPI_OPM_RX_EXTN SPI_OPM_RX_AS | SPI_OPM_RX_DOUT | \ >> - SPI_OPM_RX_DIO | SPI_OPM_RX_QOF | \ >> - SPI_OPM_RX_QIOF >> - >> -/* SPI bus connection options */ >> -#define SPI_CONN_DUAL_SHARED 1 << 0 >> -#define SPI_CONN_DUAL_SEPARATED 1 << 1 >> >> /* Header byte that marks the start of the message */ >> #define SPI_PREAMBLE_END_BYTE 0xec >> +#define SPI_DEFAULT_WORDLEN 8 >> + >> +/* SPI RX operation modes */ >> +#define SPI_RX_AS 1 << 0 >> +#define SPI_RX_DOUT 1 << 1 >> +#define SPI_RX_DIO 1 << 2 >> +#define SPI_RX_QOF 1 << 3 >> +#define SPI_RX_QIOF 1 << 4 >> +#define SPI_RX_EXTN SPI_RX_AS | SPI_RX_DOUT | SPI_RX_DIO | \ >> + SPI_RX_QOF | SPI_RX_QIOF >> >> -#define SPI_DEFAULT_WORDLEN 8 >> +/* SPI mode bits */ >> +#define SPI_TX_QPP 1 << 0 >> +#define SPI_SHARED 1 << 1 >> +#define SPI_SEPARATED 1 << 2 >> +#define SPI_U_PAGE 1 << 3 >> >> /** >> * struct spi_slave - Representation of a SPI slave >> @@ -61,25 +57,22 @@ >> * >> * @bus: ID of the bus that the slave is attached to. >> * @cs: ID of the chip select connected to the slave. >> - * @op_mode_rx: SPI RX operation mode. >> - * @op_mode_tx: SPI TX operation mode. >> * @wordlen: Size of SPI word in number of bits >> * @max_write_size: If non-zero, the maximum number of bytes which can >> * be written at once, excluding command bytes. >> * @memory_map: Address of read-only SPI flash access. >> - * @option: Varies SPI bus options - separate, shared bus. >> + * @rx_mode: SPI RX operation mode. >> + * @mode_bits: SPI TX operation modes, bus options and few > flags. > > This naming change doesn't make sense. rx_mode vs. mode_bits don't have any > relationship even if they are related apparently.
rx_mode need to separate here as it involve some calculation to find fastest command. > > Anyway, I feel we're sinking deeper and deeper into this sh*t, we should > instead > take a step back and re-think the whole approach until we break it even more. Yes - will shrink once we plan for new approach. But I'm unclear with new SPI-NOR. -- Thanks, Jagan. _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
