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