diff mbox

[v3,2/9] mtd: spi-nor: add an alternative method to support memory >16MiB

Message ID 87ace1a307f4d9899a84108de999a2eeb151da1a.1477325128.git.cyrille.pitchen@atmel.com
State Superseded
Headers show

Commit Message

Cyrille Pitchen Oct. 24, 2016, 4:34 p.m. UTC
This patch provides an alternative mean to support memory above 16MiB
(128Mib) by replacing 3byte address op codes by their associated 4byte
address versions.

Using the dedicated 4byte address op codes doesn't change the internal
state of the SPI NOR memory as opposed to using other means such as
updating a Base Address Register (BAR) and sending command to enter/leave
the 4byte mode.

Hence when a CPU reset occurs, early bootloaders don't need to be aware
of BAR value or 4byte mode being enabled: they can still access the first
16MiB of the SPI NOR memory using the regular 3byte address op codes.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Tested-by: Vignesh R <vigneshr@ti.com>
---
 drivers/mtd/devices/serial_flash_cmds.h |   7 ---
 drivers/mtd/devices/st_spi_fsm.c        |  28 ++++-----
 drivers/mtd/spi-nor/spi-nor.c           | 104 +++++++++++++++++++++++++-------
 include/linux/mtd/spi-nor.h             |  22 +++++--
 4 files changed, 113 insertions(+), 48 deletions(-)

Comments

Marek Vasut Oct. 24, 2016, 10:10 p.m. UTC | #1
On 10/24/2016 06:34 PM, Cyrille Pitchen wrote:
> This patch provides an alternative mean to support memory above 16MiB
> (128Mib) by replacing 3byte address op codes by their associated 4byte
> address versions.
> 
> Using the dedicated 4byte address op codes doesn't change the internal
> state of the SPI NOR memory as opposed to using other means such as
> updating a Base Address Register (BAR) and sending command to enter/leave
> the 4byte mode.
> 
> Hence when a CPU reset occurs, early bootloaders don't need to be aware
> of BAR value or 4byte mode being enabled: they can still access the first
> 16MiB of the SPI NOR memory using the regular 3byte address op codes.
> 
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> Tested-by: Vignesh R <vigneshr@ti.com>
> ---
>  drivers/mtd/devices/serial_flash_cmds.h |   7 ---
>  drivers/mtd/devices/st_spi_fsm.c        |  28 ++++-----
>  drivers/mtd/spi-nor/spi-nor.c           | 104 +++++++++++++++++++++++++-------
>  include/linux/mtd/spi-nor.h             |  22 +++++--
>  4 files changed, 113 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/mtd/devices/serial_flash_cmds.h b/drivers/mtd/devices/serial_flash_cmds.h
> index f59a125295d0..8b81e15105dd 100644
> --- a/drivers/mtd/devices/serial_flash_cmds.h
> +++ b/drivers/mtd/devices/serial_flash_cmds.h
> @@ -18,19 +18,12 @@
>  #define SPINOR_OP_RDVCR		0x85
>  
>  /* JEDEC Standard - Serial Flash Discoverable Parmeters (SFDP) Commands */
> -#define SPINOR_OP_READ_1_2_2	0xbb	/* DUAL I/O READ */
> -#define SPINOR_OP_READ_1_4_4	0xeb	/* QUAD I/O READ */
> -
>  #define SPINOR_OP_WRITE		0x02	/* PAGE PROGRAM */
>  #define SPINOR_OP_WRITE_1_1_2	0xa2	/* DUAL INPUT PROGRAM */
>  #define SPINOR_OP_WRITE_1_2_2	0xd2	/* DUAL INPUT EXT PROGRAM */
>  #define SPINOR_OP_WRITE_1_1_4	0x32	/* QUAD INPUT PROGRAM */
>  #define SPINOR_OP_WRITE_1_4_4	0x12	/* QUAD INPUT EXT PROGRAM */
>  
> -/* READ commands with 32-bit addressing */
> -#define SPINOR_OP_READ4_1_2_2	0xbc
> -#define SPINOR_OP_READ4_1_4_4	0xec
> -

It'd be nice to have this move/rename bit factored out into a separate
patch.

>  /* Configuration flags */
>  #define FLASH_FLAG_SINGLE	0x000000ff
>  #define FLASH_FLAG_READ_WRITE	0x00000001
> diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
> index 5454b4113589..804313a33f2b 100644

[...]

> +static u8 spi_nor_convert_opcode(u8 opcode,
> +				 const struct spi_nor_address_entry *entries,
> +				 size_t num_entries)
> +{
> +	int min, max;
> +
> +	min = 0;
> +	max = num_entries - 1;
> +	while (min <= max) {

It's really not clear at all what this function does, so please add a
comment.

> +		int mid = (min + max) >> 1;
> +		const struct spi_nor_address_entry *entry = &entries[mid];
> +
> +		if (opcode == entry->src_opcode)
> +			return entry->dst_opcode;
> +
> +		if (opcode < entry->src_opcode)
> +			max = mid - 1;
> +		else
> +			min = mid + 1;
> +	}
> +
> +	/* No conversion found */
> +	return opcode;
> +}
> +
> +static u8 spi_nor_3to4_opcode(u8 opcode)
> +{
> +	/* MUST be sorted by 3byte opcode */

Because ... why ?

> +#define ENTRY_3TO4(_opcode)	{ _opcode, _opcode##_4B }

Will this break/be unflexible for flashes with some different 4B opcodes ?

> +	static const struct spi_nor_address_entry spi_nor_3to4_table[] = {
> +		ENTRY_3TO4(SPINOR_OP_PP),		/* 0x02 */
> +		ENTRY_3TO4(SPINOR_OP_READ),		/* 0x03 */
> +		ENTRY_3TO4(SPINOR_OP_READ_FAST),	/* 0x0b */
> +		ENTRY_3TO4(SPINOR_OP_BE_4K),		/* 0x20 */
> +		ENTRY_3TO4(SPINOR_OP_PP_1_1_4),		/* 0x32 */
> +		ENTRY_3TO4(SPINOR_OP_PP_1_4_4),		/* 0x38 */
> +		ENTRY_3TO4(SPINOR_OP_READ_1_1_2),	/* 0x3b */
> +		ENTRY_3TO4(SPINOR_OP_BE_32K),		/* 0x52 */
> +		ENTRY_3TO4(SPINOR_OP_READ_1_1_4),	/* 0x6b */
> +		ENTRY_3TO4(SPINOR_OP_READ_1_2_2),	/* 0xbb */
> +		ENTRY_3TO4(SPINOR_OP_SE),		/* 0xd8 */
> +		ENTRY_3TO4(SPINOR_OP_READ_1_4_4),	/* 0xeb */
> +	};
> +#undef ENTRY_3TO4
> +
> +	return spi_nor_convert_opcode(opcode, spi_nor_3to4_table,
> +				      ARRAY_SIZE(spi_nor_3to4_table));
> +}
> +

[...]
Cyrille Pitchen Oct. 25, 2016, 9:18 a.m. UTC | #2
Le 25/10/2016 à 00:10, Marek Vasut a écrit :
> On 10/24/2016 06:34 PM, Cyrille Pitchen wrote:
>> This patch provides an alternative mean to support memory above 16MiB
>> (128Mib) by replacing 3byte address op codes by their associated 4byte
>> address versions.
>>
>> Using the dedicated 4byte address op codes doesn't change the internal
>> state of the SPI NOR memory as opposed to using other means such as
>> updating a Base Address Register (BAR) and sending command to enter/leave
>> the 4byte mode.
>>
>> Hence when a CPU reset occurs, early bootloaders don't need to be aware
>> of BAR value or 4byte mode being enabled: they can still access the first
>> 16MiB of the SPI NOR memory using the regular 3byte address op codes.
>>
>> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
>> Tested-by: Vignesh R <vigneshr@ti.com>
>> ---
>>  drivers/mtd/devices/serial_flash_cmds.h |   7 ---
>>  drivers/mtd/devices/st_spi_fsm.c        |  28 ++++-----
>>  drivers/mtd/spi-nor/spi-nor.c           | 104 +++++++++++++++++++++++++-------
>>  include/linux/mtd/spi-nor.h             |  22 +++++--
>>  4 files changed, 113 insertions(+), 48 deletions(-)
>>
>> diff --git a/drivers/mtd/devices/serial_flash_cmds.h b/drivers/mtd/devices/serial_flash_cmds.h
>> index f59a125295d0..8b81e15105dd 100644
>> --- a/drivers/mtd/devices/serial_flash_cmds.h
>> +++ b/drivers/mtd/devices/serial_flash_cmds.h
>> @@ -18,19 +18,12 @@
>>  #define SPINOR_OP_RDVCR		0x85
>>  
>>  /* JEDEC Standard - Serial Flash Discoverable Parmeters (SFDP) Commands */
>> -#define SPINOR_OP_READ_1_2_2	0xbb	/* DUAL I/O READ */
>> -#define SPINOR_OP_READ_1_4_4	0xeb	/* QUAD I/O READ */
>> -
>>  #define SPINOR_OP_WRITE		0x02	/* PAGE PROGRAM */
>>  #define SPINOR_OP_WRITE_1_1_2	0xa2	/* DUAL INPUT PROGRAM */
>>  #define SPINOR_OP_WRITE_1_2_2	0xd2	/* DUAL INPUT EXT PROGRAM */
>>  #define SPINOR_OP_WRITE_1_1_4	0x32	/* QUAD INPUT PROGRAM */
>>  #define SPINOR_OP_WRITE_1_4_4	0x12	/* QUAD INPUT EXT PROGRAM */
>>  
>> -/* READ commands with 32-bit addressing */
>> -#define SPINOR_OP_READ4_1_2_2	0xbc
>> -#define SPINOR_OP_READ4_1_4_4	0xec
>> -
> 
> It'd be nice to have this move/rename bit factored out into a separate
> patch.
> 

OK, I will try to do this. Anyway, this rename seems to break the freshly
merged drivers/spi/spi-bcm-qspi.c, which uses the SPINOR_OP_READ4_* macro so
I have to do something about this driver. IMHO, the broadcom driver should not
select its own op code but use the one provided by read_opcode member of the
struct spi_flash_read_message.

>>  /* Configuration flags */
>>  #define FLASH_FLAG_SINGLE	0x000000ff
>>  #define FLASH_FLAG_READ_WRITE	0x00000001
>> diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
>> index 5454b4113589..804313a33f2b 100644
> 
> [...]
> 
>> +static u8 spi_nor_convert_opcode(u8 opcode,
>> +				 const struct spi_nor_address_entry *entries,
>> +				 size_t num_entries)
>> +{
>> +	int min, max;
>> +
>> +	min = 0;
>> +	max = num_entries - 1;
>> +	while (min <= max) {
> 
> It's really not clear at all what this function does, so please add a
> comment.
> 

This is just a dichotomic search to reduce the number of comparisons: it 
assumes the entries[] array is sorted by ascending src_opcode.

I will add a comment to clarify this point.

>> +		int mid = (min + max) >> 1;
>> +		const struct spi_nor_address_entry *entry = &entries[mid];
>> +
>> +		if (opcode == entry->src_opcode)
>> +			return entry->dst_opcode;
>> +
>> +		if (opcode < entry->src_opcode)
>> +			max = mid - 1;
>> +		else
>> +			min = mid + 1;
>> +	}
>> +
>> +	/* No conversion found */
>> +	return opcode;
>> +}
>> +
>> +static u8 spi_nor_3to4_opcode(u8 opcode)
>> +{
>> +	/* MUST be sorted by 3byte opcode */
> 
> Because ... why ?
>

As explained above, the dichotomic search implemented in
spi_nor_convert_opcode() assumes that the input array is sorted by src_opcode.
 
>> +#define ENTRY_3TO4(_opcode)	{ _opcode, _opcode##_4B }
> 
> Will this break/be unflexible for flashes with some different 4B opcodes ?
>

Of course I cannot provide any guarantee that it will never happen but
currently it seems that all manufacturers use the same op codes. Besides, 
the 4-byte address instruction set is part the of JESD216B specification,
so there is a standard for these op codes and new SPI NOR memories tend
to match this specification by providing the SFDP tables.

Indeed this instruction set is maybe one of the few things where all
manufacturers seem to agree :)
 
>> +	static const struct spi_nor_address_entry spi_nor_3to4_table[] = {
>> +		ENTRY_3TO4(SPINOR_OP_PP),		/* 0x02 */
>> +		ENTRY_3TO4(SPINOR_OP_READ),		/* 0x03 */
>> +		ENTRY_3TO4(SPINOR_OP_READ_FAST),	/* 0x0b */
>> +		ENTRY_3TO4(SPINOR_OP_BE_4K),		/* 0x20 */
>> +		ENTRY_3TO4(SPINOR_OP_PP_1_1_4),		/* 0x32 */
>> +		ENTRY_3TO4(SPINOR_OP_PP_1_4_4),		/* 0x38 */
>> +		ENTRY_3TO4(SPINOR_OP_READ_1_1_2),	/* 0x3b */
>> +		ENTRY_3TO4(SPINOR_OP_BE_32K),		/* 0x52 */
>> +		ENTRY_3TO4(SPINOR_OP_READ_1_1_4),	/* 0x6b */
>> +		ENTRY_3TO4(SPINOR_OP_READ_1_2_2),	/* 0xbb */
>> +		ENTRY_3TO4(SPINOR_OP_SE),		/* 0xd8 */
>> +		ENTRY_3TO4(SPINOR_OP_READ_1_4_4),	/* 0xeb */
>> +	};
>> +#undef ENTRY_3TO4
>> +
>> +	return spi_nor_convert_opcode(opcode, spi_nor_3to4_table,
>> +				      ARRAY_SIZE(spi_nor_3to4_table));
>> +}
>> +
> 
> [...]
>
Marek Vasut Oct. 25, 2016, 2:53 p.m. UTC | #3
On 10/25/2016 11:18 AM, Cyrille Pitchen wrote:
> Le 25/10/2016 à 00:10, Marek Vasut a écrit :
>> On 10/24/2016 06:34 PM, Cyrille Pitchen wrote:
>>> This patch provides an alternative mean to support memory above 16MiB
>>> (128Mib) by replacing 3byte address op codes by their associated 4byte
>>> address versions.
>>>
>>> Using the dedicated 4byte address op codes doesn't change the internal
>>> state of the SPI NOR memory as opposed to using other means such as
>>> updating a Base Address Register (BAR) and sending command to enter/leave
>>> the 4byte mode.
>>>
>>> Hence when a CPU reset occurs, early bootloaders don't need to be aware
>>> of BAR value or 4byte mode being enabled: they can still access the first
>>> 16MiB of the SPI NOR memory using the regular 3byte address op codes.
>>>
>>> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
>>> Tested-by: Vignesh R <vigneshr@ti.com>
>>> ---
>>>  drivers/mtd/devices/serial_flash_cmds.h |   7 ---
>>>  drivers/mtd/devices/st_spi_fsm.c        |  28 ++++-----
>>>  drivers/mtd/spi-nor/spi-nor.c           | 104 +++++++++++++++++++++++++-------
>>>  include/linux/mtd/spi-nor.h             |  22 +++++--
>>>  4 files changed, 113 insertions(+), 48 deletions(-)
>>>
>>> diff --git a/drivers/mtd/devices/serial_flash_cmds.h b/drivers/mtd/devices/serial_flash_cmds.h
>>> index f59a125295d0..8b81e15105dd 100644
>>> --- a/drivers/mtd/devices/serial_flash_cmds.h
>>> +++ b/drivers/mtd/devices/serial_flash_cmds.h
>>> @@ -18,19 +18,12 @@
>>>  #define SPINOR_OP_RDVCR		0x85
>>>  
>>>  /* JEDEC Standard - Serial Flash Discoverable Parmeters (SFDP) Commands */
>>> -#define SPINOR_OP_READ_1_2_2	0xbb	/* DUAL I/O READ */
>>> -#define SPINOR_OP_READ_1_4_4	0xeb	/* QUAD I/O READ */
>>> -
>>>  #define SPINOR_OP_WRITE		0x02	/* PAGE PROGRAM */
>>>  #define SPINOR_OP_WRITE_1_1_2	0xa2	/* DUAL INPUT PROGRAM */
>>>  #define SPINOR_OP_WRITE_1_2_2	0xd2	/* DUAL INPUT EXT PROGRAM */
>>>  #define SPINOR_OP_WRITE_1_1_4	0x32	/* QUAD INPUT PROGRAM */
>>>  #define SPINOR_OP_WRITE_1_4_4	0x12	/* QUAD INPUT EXT PROGRAM */
>>>  
>>> -/* READ commands with 32-bit addressing */
>>> -#define SPINOR_OP_READ4_1_2_2	0xbc
>>> -#define SPINOR_OP_READ4_1_4_4	0xec
>>> -
>>
>> It'd be nice to have this move/rename bit factored out into a separate
>> patch.
>>
> 
> OK, I will try to do this. Anyway, this rename seems to break the freshly
> merged drivers/spi/spi-bcm-qspi.c, which uses the SPINOR_OP_READ4_* macro so
> I have to do something about this driver. IMHO, the broadcom driver should not
> select its own op code but use the one provided by read_opcode member of the
> struct spi_flash_read_message.

Thanks

>>>  /* Configuration flags */
>>>  #define FLASH_FLAG_SINGLE	0x000000ff
>>>  #define FLASH_FLAG_READ_WRITE	0x00000001
>>> diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
>>> index 5454b4113589..804313a33f2b 100644
>>
>> [...]
>>
>>> +static u8 spi_nor_convert_opcode(u8 opcode,
>>> +				 const struct spi_nor_address_entry *entries,
>>> +				 size_t num_entries)
>>> +{
>>> +	int min, max;
>>> +
>>> +	min = 0;
>>> +	max = num_entries - 1;
>>> +	while (min <= max) {
>>
>> It's really not clear at all what this function does, so please add a
>> comment.
>>
> 
> This is just a dichotomic search to reduce the number of comparisons: it 
> assumes the entries[] array is sorted by ascending src_opcode.
> 
> I will add a comment to clarify this point.

Cool, that'd help. Are you invoking this every time an opcode is
submitted to the SPI NOR ?

>>> +		int mid = (min + max) >> 1;
>>> +		const struct spi_nor_address_entry *entry = &entries[mid];
>>> +
>>> +		if (opcode == entry->src_opcode)
>>> +			return entry->dst_opcode;
>>> +
>>> +		if (opcode < entry->src_opcode)
>>> +			max = mid - 1;
>>> +		else
>>> +			min = mid + 1;
>>> +	}
>>> +
>>> +	/* No conversion found */
>>> +	return opcode;
>>> +}
>>> +
>>> +static u8 spi_nor_3to4_opcode(u8 opcode)
>>> +{
>>> +	/* MUST be sorted by 3byte opcode */
>>
>> Because ... why ?
>>
> 
> As explained above, the dichotomic search implemented in
> spi_nor_convert_opcode() assumes that the input array is sorted by src_opcode.

Maybe you should drop that assumption and do normal traversal of the
array ? I feel this is a bit fragile and will break once someone will
cluelessly add a new opcode.

... or add a comment with a big WARNING statement.

>>> +#define ENTRY_3TO4(_opcode)	{ _opcode, _opcode##_4B }
>>
>> Will this break/be unflexible for flashes with some different 4B opcodes ?
>>
> 
> Of course I cannot provide any guarantee that it will never happen but
> currently it seems that all manufacturers use the same op codes. Besides, 
> the 4-byte address instruction set is part the of JESD216B specification,
> so there is a standard for these op codes and new SPI NOR memories tend
> to match this specification by providing the SFDP tables.

Great. Still, will the code need an overhaul if some vendor decides to
deviate ?

> Indeed this instruction set is maybe one of the few things where all
> manufacturers seem to agree :)

That's a good start :)

>>> +	static const struct spi_nor_address_entry spi_nor_3to4_table[] = {
>>> +		ENTRY_3TO4(SPINOR_OP_PP),		/* 0x02 */
>>> +		ENTRY_3TO4(SPINOR_OP_READ),		/* 0x03 */
>>> +		ENTRY_3TO4(SPINOR_OP_READ_FAST),	/* 0x0b */
>>> +		ENTRY_3TO4(SPINOR_OP_BE_4K),		/* 0x20 */
>>> +		ENTRY_3TO4(SPINOR_OP_PP_1_1_4),		/* 0x32 */
>>> +		ENTRY_3TO4(SPINOR_OP_PP_1_4_4),		/* 0x38 */
>>> +		ENTRY_3TO4(SPINOR_OP_READ_1_1_2),	/* 0x3b */
>>> +		ENTRY_3TO4(SPINOR_OP_BE_32K),		/* 0x52 */
>>> +		ENTRY_3TO4(SPINOR_OP_READ_1_1_4),	/* 0x6b */
>>> +		ENTRY_3TO4(SPINOR_OP_READ_1_2_2),	/* 0xbb */
>>> +		ENTRY_3TO4(SPINOR_OP_SE),		/* 0xd8 */
>>> +		ENTRY_3TO4(SPINOR_OP_READ_1_4_4),	/* 0xeb */
>>> +	};
>>> +#undef ENTRY_3TO4
>>> +
>>> +	return spi_nor_convert_opcode(opcode, spi_nor_3to4_table,
>>> +				      ARRAY_SIZE(spi_nor_3to4_table));
>>> +}
>>> +
>>
>> [...]
>>
>
Jagan Teki Oct. 31, 2016, 6:51 p.m. UTC | #4
Hi Cyrille,

On Mon, Oct 24, 2016 at 10:04 PM, Cyrille Pitchen
<cyrille.pitchen@atmel.com> wrote:
> This patch provides an alternative mean to support memory above 16MiB
> (128Mib) by replacing 3byte address op codes by their associated 4byte
> address versions.
>
> Using the dedicated 4byte address op codes doesn't change the internal
> state of the SPI NOR memory as opposed to using other means such as
> updating a Base Address Register (BAR) and sending command to enter/leave
> the 4byte mode.
>
> Hence when a CPU reset occurs, early bootloaders don't need to be aware
> of BAR value or 4byte mode being enabled: they can still access the first
> 16MiB of the SPI NOR memory using the regular 3byte address op codes.
>
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> Tested-by: Vignesh R <vigneshr@ti.com>
> ---
>  drivers/mtd/devices/serial_flash_cmds.h |   7 ---
>  drivers/mtd/devices/st_spi_fsm.c        |  28 ++++-----
>  drivers/mtd/spi-nor/spi-nor.c           | 104 +++++++++++++++++++++++++-------
>  include/linux/mtd/spi-nor.h             |  22 +++++--
>  4 files changed, 113 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/mtd/devices/serial_flash_cmds.h b/drivers/mtd/devices/serial_flash_cmds.h
> index f59a125295d0..8b81e15105dd 100644
> --- a/drivers/mtd/devices/serial_flash_cmds.h
> +++ b/drivers/mtd/devices/serial_flash_cmds.h
> @@ -18,19 +18,12 @@
>  #define SPINOR_OP_RDVCR                0x85
>
>  /* JEDEC Standard - Serial Flash Discoverable Parmeters (SFDP) Commands */
> -#define SPINOR_OP_READ_1_2_2   0xbb    /* DUAL I/O READ */
> -#define SPINOR_OP_READ_1_4_4   0xeb    /* QUAD I/O READ */
> -
>  #define SPINOR_OP_WRITE                0x02    /* PAGE PROGRAM */
>  #define SPINOR_OP_WRITE_1_1_2  0xa2    /* DUAL INPUT PROGRAM */
>  #define SPINOR_OP_WRITE_1_2_2  0xd2    /* DUAL INPUT EXT PROGRAM */
>  #define SPINOR_OP_WRITE_1_1_4  0x32    /* QUAD INPUT PROGRAM */
>  #define SPINOR_OP_WRITE_1_4_4  0x12    /* QUAD INPUT EXT PROGRAM */
>
> -/* READ commands with 32-bit addressing */
> -#define SPINOR_OP_READ4_1_2_2  0xbc
> -#define SPINOR_OP_READ4_1_4_4  0xec
> -
>  /* Configuration flags */
>  #define FLASH_FLAG_SINGLE      0x000000ff
>  #define FLASH_FLAG_READ_WRITE  0x00000001
> diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
> index 5454b4113589..804313a33f2b 100644
> --- a/drivers/mtd/devices/st_spi_fsm.c
> +++ b/drivers/mtd/devices/st_spi_fsm.c
> @@ -507,13 +507,13 @@ static struct seq_rw_config n25q_read3_configs[] = {
>   *     - 'FAST' variants configured for 8 dummy cycles (see note above.)
>   */
>  static struct seq_rw_config n25q_read4_configs[] = {
> -       {FLASH_FLAG_READ_1_4_4, SPINOR_OP_READ4_1_4_4,  0, 4, 4, 0x00, 0, 8},
> -       {FLASH_FLAG_READ_1_1_4, SPINOR_OP_READ4_1_1_4,  0, 1, 4, 0x00, 0, 8},
> -       {FLASH_FLAG_READ_1_2_2, SPINOR_OP_READ4_1_2_2,  0, 2, 2, 0x00, 0, 8},
> -       {FLASH_FLAG_READ_1_1_2, SPINOR_OP_READ4_1_1_2,  0, 1, 2, 0x00, 0, 8},
> -       {FLASH_FLAG_READ_FAST,  SPINOR_OP_READ4_FAST,   0, 1, 1, 0x00, 0, 8},
> -       {FLASH_FLAG_READ_WRITE, SPINOR_OP_READ4,        0, 1, 1, 0x00, 0, 0},
> -       {0x00,                  0,                      0, 0, 0, 0x00, 0, 0},
> +       {FLASH_FLAG_READ_1_4_4, SPINOR_OP_READ_1_4_4_4B, 0, 4, 4, 0x00, 0, 8},
> +       {FLASH_FLAG_READ_1_1_4, SPINOR_OP_READ_1_1_4_4B, 0, 1, 4, 0x00, 0, 8},
> +       {FLASH_FLAG_READ_1_2_2, SPINOR_OP_READ_1_2_2_4B, 0, 2, 2, 0x00, 0, 8},
> +       {FLASH_FLAG_READ_1_1_2, SPINOR_OP_READ_1_1_2_4B, 0, 1, 2, 0x00, 0, 8},
> +       {FLASH_FLAG_READ_FAST,  SPINOR_OP_READ_FAST_4B,  0, 1, 1, 0x00, 0, 8},
> +       {FLASH_FLAG_READ_WRITE, SPINOR_OP_READ_4B,       0, 1, 1, 0x00, 0, 0},
> +       {0x00,                  0,                       0, 0, 0, 0x00, 0, 0},
>  };
>
>  /*
> @@ -553,13 +553,13 @@ static int stfsm_mx25_en_32bit_addr_seq(struct stfsm_seq *seq)
>   * entering a state that is incompatible with the SPIBoot Controller.
>   */
>  static struct seq_rw_config stfsm_s25fl_read4_configs[] = {
> -       {FLASH_FLAG_READ_1_4_4,  SPINOR_OP_READ4_1_4_4,  0, 4, 4, 0x00, 2, 4},
> -       {FLASH_FLAG_READ_1_1_4,  SPINOR_OP_READ4_1_1_4,  0, 1, 4, 0x00, 0, 8},
> -       {FLASH_FLAG_READ_1_2_2,  SPINOR_OP_READ4_1_2_2,  0, 2, 2, 0x00, 4, 0},
> -       {FLASH_FLAG_READ_1_1_2,  SPINOR_OP_READ4_1_1_2,  0, 1, 2, 0x00, 0, 8},
> -       {FLASH_FLAG_READ_FAST,   SPINOR_OP_READ4_FAST,   0, 1, 1, 0x00, 0, 8},
> -       {FLASH_FLAG_READ_WRITE,  SPINOR_OP_READ4,        0, 1, 1, 0x00, 0, 0},
> -       {0x00,                   0,                      0, 0, 0, 0x00, 0, 0},
> +       {FLASH_FLAG_READ_1_4_4,  SPINOR_OP_READ_1_4_4_4B,  0, 4, 4, 0x00, 2, 4},
> +       {FLASH_FLAG_READ_1_1_4,  SPINOR_OP_READ_1_1_4_4B,  0, 1, 4, 0x00, 0, 8},
> +       {FLASH_FLAG_READ_1_2_2,  SPINOR_OP_READ_1_2_2_4B,  0, 2, 2, 0x00, 4, 0},
> +       {FLASH_FLAG_READ_1_1_2,  SPINOR_OP_READ_1_1_2_4B,  0, 1, 2, 0x00, 0, 8},
> +       {FLASH_FLAG_READ_FAST,   SPINOR_OP_READ_FAST_4B,   0, 1, 1, 0x00, 0, 8},
> +       {FLASH_FLAG_READ_WRITE,  SPINOR_OP_READ_4B,        0, 1, 1, 0x00, 0, 0},
> +       {0x00,                   0,                        0, 0, 0, 0x00, 0, 0},
>  };
>
>  static struct seq_rw_config stfsm_s25fl_write4_configs[] = {
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 5c87b2d99507..423448c1c7a8 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -75,6 +75,10 @@ struct flash_info {
>                                          * bit. Must be used with
>                                          * SPI_NOR_HAS_LOCK.
>                                          */
> +#define SPI_NOR_4B_OPCODES     BIT(10) /*
> +                                        * Use dedicated 4byte address op codes
> +                                        * to support memory size above 128Mib.
> +                                        */
>  };
>
>  #define JEDEC_MFR(info)        ((info)->id[0])
> @@ -188,6 +192,81 @@ static inline struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
>         return mtd->priv;
>  }
>
> +
> +struct spi_nor_address_entry {
> +       u8      src_opcode;
> +       u8      dst_opcode;
> +};
> +
> +static u8 spi_nor_convert_opcode(u8 opcode,
> +                                const struct spi_nor_address_entry *entries,
> +                                size_t num_entries)
> +{
> +       int min, max;
> +
> +       min = 0;
> +       max = num_entries - 1;
> +       while (min <= max) {
> +               int mid = (min + max) >> 1;
> +               const struct spi_nor_address_entry *entry = &entries[mid];
> +
> +               if (opcode == entry->src_opcode)
> +                       return entry->dst_opcode;
> +
> +               if (opcode < entry->src_opcode)
> +                       max = mid - 1;
> +               else
> +                       min = mid + 1;
> +       }
> +
> +       /* No conversion found */
> +       return opcode;
> +}
> +
> +static u8 spi_nor_3to4_opcode(u8 opcode)
> +{
> +       /* MUST be sorted by 3byte opcode */
> +#define ENTRY_3TO4(_opcode)    { _opcode, _opcode##_4B }
> +       static const struct spi_nor_address_entry spi_nor_3to4_table[] = {
> +               ENTRY_3TO4(SPINOR_OP_PP),               /* 0x02 */
> +               ENTRY_3TO4(SPINOR_OP_READ),             /* 0x03 */
> +               ENTRY_3TO4(SPINOR_OP_READ_FAST),        /* 0x0b */
> +               ENTRY_3TO4(SPINOR_OP_BE_4K),            /* 0x20 */
> +               ENTRY_3TO4(SPINOR_OP_PP_1_1_4),         /* 0x32 */
> +               ENTRY_3TO4(SPINOR_OP_PP_1_4_4),         /* 0x38 */
> +               ENTRY_3TO4(SPINOR_OP_READ_1_1_2),       /* 0x3b */
> +               ENTRY_3TO4(SPINOR_OP_BE_32K),           /* 0x52 */
> +               ENTRY_3TO4(SPINOR_OP_READ_1_1_4),       /* 0x6b */
> +               ENTRY_3TO4(SPINOR_OP_READ_1_2_2),       /* 0xbb */
> +               ENTRY_3TO4(SPINOR_OP_SE),               /* 0xd8 */
> +               ENTRY_3TO4(SPINOR_OP_READ_1_4_4),       /* 0xeb */
> +       };
> +#undef ENTRY_3TO4
> +
> +       return spi_nor_convert_opcode(opcode, spi_nor_3to4_table,
> +                                     ARRAY_SIZE(spi_nor_3to4_table));
> +}
> +
> +static void spi_nor_set_4byte_opcodes(struct spi_nor *nor,
> +                                     const struct flash_info *info)
> +{
> +       /* Do some manufacturer fixups first */
> +       switch (JEDEC_MFR(info)) {
> +       case SNOR_MFR_SPANSION:
> +               /* No small sector erase for 4-byte command set */
> +               nor->erase_opcode = SPINOR_OP_SE;
> +               nor->mtd.erasesize = info->sector_size;
> +               break;
> +
> +       default:
> +               break;
> +       }
> +
> +       nor->read_opcode        = spi_nor_3to4_opcode(nor->read_opcode);
> +       nor->program_opcode     = spi_nor_3to4_opcode(nor->program_opcode);
> +       nor->erase_opcode       = spi_nor_3to4_opcode(nor->erase_opcode);
> +}
> +
>  /* Enable/disable 4-byte addressing mode. */
>  static inline int set_4byte(struct spi_nor *nor, const struct flash_info *info,
>                             int enable)
> @@ -1476,27 +1555,10 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>         else if (mtd->size > 0x1000000) {
>                 /* enable 4-byte addressing if the device exceeds 16MiB */
>                 nor->addr_width = 4;
> -               if (JEDEC_MFR(info) == SNOR_MFR_SPANSION) {
> -                       /* Dedicated 4-byte command set */
> -                       switch (nor->flash_read) {
> -                       case SPI_NOR_QUAD:
> -                               nor->read_opcode = SPINOR_OP_READ4_1_1_4;
> -                               break;
> -                       case SPI_NOR_DUAL:
> -                               nor->read_opcode = SPINOR_OP_READ4_1_1_2;
> -                               break;
> -                       case SPI_NOR_FAST:
> -                               nor->read_opcode = SPINOR_OP_READ4_FAST;
> -                               break;
> -                       case SPI_NOR_NORMAL:
> -                               nor->read_opcode = SPINOR_OP_READ4;
> -                               break;
> -                       }
> -                       nor->program_opcode = SPINOR_OP_PP_4B;
> -                       /* No small sector erase for 4-byte command set */
> -                       nor->erase_opcode = SPINOR_OP_SE_4B;
> -                       mtd->erasesize = info->sector_size;
> -               } else
> +               if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
> +                   info->flags & SPI_NOR_4B_OPCODES)
> +                       spi_nor_set_4byte_opcodes(nor, info);

I am giving my inputs based on the discussion on this thread[1].

Seems like winbond[2], stmicron, spansion look better way or equally
same for accessing 4-byte addressing commands. and I find few of the
Macronix [3] have support of 4B addressing(see Table 5) but
lack/unable to find the 4B opcodes.

And about BAR support, based on my experience in u-boot and research
on above chips only require when controller isn't supporting 4-byte
addressing even if connected flash chip has > 16MiB, that means there
is no exact or equivalent requirement for flash side either.

So, adding the flags on flash table might be the good option instead
making code to convert 3B opcode to 4B because anyway the chips
require SPI_NOR_4B_OPCODES and also except winbond, stmicron, spansion
not too many flash needed > 16MiB support as of now.

[1] https://lkml.org/lkml/2016/10/24/276
[2] https://www.winbond.com/resource-files/w25q256fv_revg1_120214_qpi_website_rev_g.pdf
[3] http://www.zlgmcu.com/mxic/pdf/NOR_Flash_c/MX25L25635E_DS_EN.pdf

thanks!
Cyrille Pitchen Nov. 2, 2016, 10:49 a.m. UTC | #5
Hi,

Le 31/10/2016 à 19:51, Jagan Teki a écrit :
> Hi Cyrille,
> 
> On Mon, Oct 24, 2016 at 10:04 PM, Cyrille Pitchen
> <cyrille.pitchen@atmel.com> wrote:
>> This patch provides an alternative mean to support memory above 16MiB
>> (128Mib) by replacing 3byte address op codes by their associated 4byte
>> address versions.
>>
>> Using the dedicated 4byte address op codes doesn't change the internal
>> state of the SPI NOR memory as opposed to using other means such as
>> updating a Base Address Register (BAR) and sending command to enter/leave
>> the 4byte mode.
>>
>> Hence when a CPU reset occurs, early bootloaders don't need to be aware
>> of BAR value or 4byte mode being enabled: they can still access the first
>> 16MiB of the SPI NOR memory using the regular 3byte address op codes.
>>
>> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
>> Tested-by: Vignesh R <vigneshr@ti.com>
>> ---
>>  drivers/mtd/devices/serial_flash_cmds.h |   7 ---
>>  drivers/mtd/devices/st_spi_fsm.c        |  28 ++++-----
>>  drivers/mtd/spi-nor/spi-nor.c           | 104 +++++++++++++++++++++++++-------
>>  include/linux/mtd/spi-nor.h             |  22 +++++--
>>  4 files changed, 113 insertions(+), 48 deletions(-)
>>
>> diff --git a/drivers/mtd/devices/serial_flash_cmds.h b/drivers/mtd/devices/serial_flash_cmds.h
>> index f59a125295d0..8b81e15105dd 100644
>> --- a/drivers/mtd/devices/serial_flash_cmds.h
>> +++ b/drivers/mtd/devices/serial_flash_cmds.h
>> @@ -18,19 +18,12 @@
>>  #define SPINOR_OP_RDVCR                0x85
>>
>>  /* JEDEC Standard - Serial Flash Discoverable Parmeters (SFDP) Commands */
>> -#define SPINOR_OP_READ_1_2_2   0xbb    /* DUAL I/O READ */
>> -#define SPINOR_OP_READ_1_4_4   0xeb    /* QUAD I/O READ */
>> -
>>  #define SPINOR_OP_WRITE                0x02    /* PAGE PROGRAM */
>>  #define SPINOR_OP_WRITE_1_1_2  0xa2    /* DUAL INPUT PROGRAM */
>>  #define SPINOR_OP_WRITE_1_2_2  0xd2    /* DUAL INPUT EXT PROGRAM */
>>  #define SPINOR_OP_WRITE_1_1_4  0x32    /* QUAD INPUT PROGRAM */
>>  #define SPINOR_OP_WRITE_1_4_4  0x12    /* QUAD INPUT EXT PROGRAM */
>>
>> -/* READ commands with 32-bit addressing */
>> -#define SPINOR_OP_READ4_1_2_2  0xbc
>> -#define SPINOR_OP_READ4_1_4_4  0xec
>> -
>>  /* Configuration flags */
>>  #define FLASH_FLAG_SINGLE      0x000000ff
>>  #define FLASH_FLAG_READ_WRITE  0x00000001
>> diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
>> index 5454b4113589..804313a33f2b 100644
>> --- a/drivers/mtd/devices/st_spi_fsm.c
>> +++ b/drivers/mtd/devices/st_spi_fsm.c
>> @@ -507,13 +507,13 @@ static struct seq_rw_config n25q_read3_configs[] = {
>>   *     - 'FAST' variants configured for 8 dummy cycles (see note above.)
>>   */
>>  static struct seq_rw_config n25q_read4_configs[] = {
>> -       {FLASH_FLAG_READ_1_4_4, SPINOR_OP_READ4_1_4_4,  0, 4, 4, 0x00, 0, 8},
>> -       {FLASH_FLAG_READ_1_1_4, SPINOR_OP_READ4_1_1_4,  0, 1, 4, 0x00, 0, 8},
>> -       {FLASH_FLAG_READ_1_2_2, SPINOR_OP_READ4_1_2_2,  0, 2, 2, 0x00, 0, 8},
>> -       {FLASH_FLAG_READ_1_1_2, SPINOR_OP_READ4_1_1_2,  0, 1, 2, 0x00, 0, 8},
>> -       {FLASH_FLAG_READ_FAST,  SPINOR_OP_READ4_FAST,   0, 1, 1, 0x00, 0, 8},
>> -       {FLASH_FLAG_READ_WRITE, SPINOR_OP_READ4,        0, 1, 1, 0x00, 0, 0},
>> -       {0x00,                  0,                      0, 0, 0, 0x00, 0, 0},
>> +       {FLASH_FLAG_READ_1_4_4, SPINOR_OP_READ_1_4_4_4B, 0, 4, 4, 0x00, 0, 8},
>> +       {FLASH_FLAG_READ_1_1_4, SPINOR_OP_READ_1_1_4_4B, 0, 1, 4, 0x00, 0, 8},
>> +       {FLASH_FLAG_READ_1_2_2, SPINOR_OP_READ_1_2_2_4B, 0, 2, 2, 0x00, 0, 8},
>> +       {FLASH_FLAG_READ_1_1_2, SPINOR_OP_READ_1_1_2_4B, 0, 1, 2, 0x00, 0, 8},
>> +       {FLASH_FLAG_READ_FAST,  SPINOR_OP_READ_FAST_4B,  0, 1, 1, 0x00, 0, 8},
>> +       {FLASH_FLAG_READ_WRITE, SPINOR_OP_READ_4B,       0, 1, 1, 0x00, 0, 0},
>> +       {0x00,                  0,                       0, 0, 0, 0x00, 0, 0},
>>  };
>>
>>  /*
>> @@ -553,13 +553,13 @@ static int stfsm_mx25_en_32bit_addr_seq(struct stfsm_seq *seq)
>>   * entering a state that is incompatible with the SPIBoot Controller.
>>   */
>>  static struct seq_rw_config stfsm_s25fl_read4_configs[] = {
>> -       {FLASH_FLAG_READ_1_4_4,  SPINOR_OP_READ4_1_4_4,  0, 4, 4, 0x00, 2, 4},
>> -       {FLASH_FLAG_READ_1_1_4,  SPINOR_OP_READ4_1_1_4,  0, 1, 4, 0x00, 0, 8},
>> -       {FLASH_FLAG_READ_1_2_2,  SPINOR_OP_READ4_1_2_2,  0, 2, 2, 0x00, 4, 0},
>> -       {FLASH_FLAG_READ_1_1_2,  SPINOR_OP_READ4_1_1_2,  0, 1, 2, 0x00, 0, 8},
>> -       {FLASH_FLAG_READ_FAST,   SPINOR_OP_READ4_FAST,   0, 1, 1, 0x00, 0, 8},
>> -       {FLASH_FLAG_READ_WRITE,  SPINOR_OP_READ4,        0, 1, 1, 0x00, 0, 0},
>> -       {0x00,                   0,                      0, 0, 0, 0x00, 0, 0},
>> +       {FLASH_FLAG_READ_1_4_4,  SPINOR_OP_READ_1_4_4_4B,  0, 4, 4, 0x00, 2, 4},
>> +       {FLASH_FLAG_READ_1_1_4,  SPINOR_OP_READ_1_1_4_4B,  0, 1, 4, 0x00, 0, 8},
>> +       {FLASH_FLAG_READ_1_2_2,  SPINOR_OP_READ_1_2_2_4B,  0, 2, 2, 0x00, 4, 0},
>> +       {FLASH_FLAG_READ_1_1_2,  SPINOR_OP_READ_1_1_2_4B,  0, 1, 2, 0x00, 0, 8},
>> +       {FLASH_FLAG_READ_FAST,   SPINOR_OP_READ_FAST_4B,   0, 1, 1, 0x00, 0, 8},
>> +       {FLASH_FLAG_READ_WRITE,  SPINOR_OP_READ_4B,        0, 1, 1, 0x00, 0, 0},
>> +       {0x00,                   0,                        0, 0, 0, 0x00, 0, 0},
>>  };
>>
>>  static struct seq_rw_config stfsm_s25fl_write4_configs[] = {
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index 5c87b2d99507..423448c1c7a8 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -75,6 +75,10 @@ struct flash_info {
>>                                          * bit. Must be used with
>>                                          * SPI_NOR_HAS_LOCK.
>>                                          */
>> +#define SPI_NOR_4B_OPCODES     BIT(10) /*
>> +                                        * Use dedicated 4byte address op codes
>> +                                        * to support memory size above 128Mib.
>> +                                        */
>>  };
>>
>>  #define JEDEC_MFR(info)        ((info)->id[0])
>> @@ -188,6 +192,81 @@ static inline struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
>>         return mtd->priv;
>>  }
>>
>> +
>> +struct spi_nor_address_entry {
>> +       u8      src_opcode;
>> +       u8      dst_opcode;
>> +};
>> +
>> +static u8 spi_nor_convert_opcode(u8 opcode,
>> +                                const struct spi_nor_address_entry *entries,
>> +                                size_t num_entries)
>> +{
>> +       int min, max;
>> +
>> +       min = 0;
>> +       max = num_entries - 1;
>> +       while (min <= max) {
>> +               int mid = (min + max) >> 1;
>> +               const struct spi_nor_address_entry *entry = &entries[mid];
>> +
>> +               if (opcode == entry->src_opcode)
>> +                       return entry->dst_opcode;
>> +
>> +               if (opcode < entry->src_opcode)
>> +                       max = mid - 1;
>> +               else
>> +                       min = mid + 1;
>> +       }
>> +
>> +       /* No conversion found */
>> +       return opcode;
>> +}
>> +
>> +static u8 spi_nor_3to4_opcode(u8 opcode)
>> +{
>> +       /* MUST be sorted by 3byte opcode */
>> +#define ENTRY_3TO4(_opcode)    { _opcode, _opcode##_4B }
>> +       static const struct spi_nor_address_entry spi_nor_3to4_table[] = {
>> +               ENTRY_3TO4(SPINOR_OP_PP),               /* 0x02 */
>> +               ENTRY_3TO4(SPINOR_OP_READ),             /* 0x03 */
>> +               ENTRY_3TO4(SPINOR_OP_READ_FAST),        /* 0x0b */
>> +               ENTRY_3TO4(SPINOR_OP_BE_4K),            /* 0x20 */
>> +               ENTRY_3TO4(SPINOR_OP_PP_1_1_4),         /* 0x32 */
>> +               ENTRY_3TO4(SPINOR_OP_PP_1_4_4),         /* 0x38 */
>> +               ENTRY_3TO4(SPINOR_OP_READ_1_1_2),       /* 0x3b */
>> +               ENTRY_3TO4(SPINOR_OP_BE_32K),           /* 0x52 */
>> +               ENTRY_3TO4(SPINOR_OP_READ_1_1_4),       /* 0x6b */
>> +               ENTRY_3TO4(SPINOR_OP_READ_1_2_2),       /* 0xbb */
>> +               ENTRY_3TO4(SPINOR_OP_SE),               /* 0xd8 */
>> +               ENTRY_3TO4(SPINOR_OP_READ_1_4_4),       /* 0xeb */
>> +       };
>> +#undef ENTRY_3TO4
>> +
>> +       return spi_nor_convert_opcode(opcode, spi_nor_3to4_table,
>> +                                     ARRAY_SIZE(spi_nor_3to4_table));
>> +}
>> +
>> +static void spi_nor_set_4byte_opcodes(struct spi_nor *nor,
>> +                                     const struct flash_info *info)
>> +{
>> +       /* Do some manufacturer fixups first */
>> +       switch (JEDEC_MFR(info)) {
>> +       case SNOR_MFR_SPANSION:
>> +               /* No small sector erase for 4-byte command set */
>> +               nor->erase_opcode = SPINOR_OP_SE;
>> +               nor->mtd.erasesize = info->sector_size;
>> +               break;
>> +
>> +       default:
>> +               break;
>> +       }
>> +
>> +       nor->read_opcode        = spi_nor_3to4_opcode(nor->read_opcode);
>> +       nor->program_opcode     = spi_nor_3to4_opcode(nor->program_opcode);
>> +       nor->erase_opcode       = spi_nor_3to4_opcode(nor->erase_opcode);
>> +}
>> +
>>  /* Enable/disable 4-byte addressing mode. */
>>  static inline int set_4byte(struct spi_nor *nor, const struct flash_info *info,
>>                             int enable)
>> @@ -1476,27 +1555,10 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>         else if (mtd->size > 0x1000000) {
>>                 /* enable 4-byte addressing if the device exceeds 16MiB */
>>                 nor->addr_width = 4;
>> -               if (JEDEC_MFR(info) == SNOR_MFR_SPANSION) {
>> -                       /* Dedicated 4-byte command set */
>> -                       switch (nor->flash_read) {
>> -                       case SPI_NOR_QUAD:
>> -                               nor->read_opcode = SPINOR_OP_READ4_1_1_4;
>> -                               break;
>> -                       case SPI_NOR_DUAL:
>> -                               nor->read_opcode = SPINOR_OP_READ4_1_1_2;
>> -                               break;
>> -                       case SPI_NOR_FAST:
>> -                               nor->read_opcode = SPINOR_OP_READ4_FAST;
>> -                               break;
>> -                       case SPI_NOR_NORMAL:
>> -                               nor->read_opcode = SPINOR_OP_READ4;
>> -                               break;
>> -                       }
>> -                       nor->program_opcode = SPINOR_OP_PP_4B;
>> -                       /* No small sector erase for 4-byte command set */
>> -                       nor->erase_opcode = SPINOR_OP_SE_4B;
>> -                       mtd->erasesize = info->sector_size;
>> -               } else
>> +               if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
>> +                   info->flags & SPI_NOR_4B_OPCODES)
>> +                       spi_nor_set_4byte_opcodes(nor, info);
> 
> I am giving my inputs based on the discussion on this thread[1].
> 
> Seems like winbond[2], stmicron, spansion look better way or equally
> same for accessing 4-byte addressing commands. and I find few of the
> Macronix [3] have support of 4B addressing(see Table 5) but
> lack/unable to find the 4B opcodes.
> 

The Macronix mx25l25635e has no 4-byte address op codes. Instead you can
use the EN4B (enter 4-byte mode) B7h op code to make the memory enter its
4-byte mode: hence you still write one of the 3-byte address op code but now
this op code is followed by a 4-byte address.
Currently the spi-nor framework uses this EN4B op code for all but Spansion
memories. However entering this 4-byte mode is statefull.
For the Macronix 35e won't be able to use the 4-byte address instruction set
and we will keep on entering the 4-byte mode.

The idea behind the patch is to use the 4-byte address instruction set when
we are absolutely sure this set is supported by a given memory but keep the
current behaviour for other memories.

> And about BAR support, based on my experience in u-boot and research
> on above chips only require when controller isn't supporting 4-byte
> addressing even if connected flash chip has > 16MiB, that means there
> is no exact or equivalent requirement for flash side either.
> 
> So, adding the flags on flash table might be the good option instead
> making code to convert 3B opcode to 4B because anyway the chips
> require SPI_NOR_4B_OPCODES and also except winbond, stmicron, spansion
> not too many flash needed > 16MiB support as of now.
> 

Sorry but I don't understand what you mean or suggest here!

> [1] https://lkml.org/lkml/2016/10/24/276
> [2] https://www.winbond.com/resource-files/w25q256fv_revg1_120214_qpi_website_rev_g.pdf
> [3] http://www.zlgmcu.com/mxic/pdf/NOR_Flash_c/MX25L25635E_DS_EN.pdf
> 
> thanks!
>
Jagan Teki Nov. 2, 2016, 11:11 a.m. UTC | #6
On Wed, Nov 2, 2016 at 4:19 PM, Cyrille Pitchen
<cyrille.pitchen@atmel.com> wrote:
> Hi,
>
> Le 31/10/2016 à 19:51, Jagan Teki a écrit :
>> Hi Cyrille,
>>
>> On Mon, Oct 24, 2016 at 10:04 PM, Cyrille Pitchen
>> <cyrille.pitchen@atmel.com> wrote:
>>> This patch provides an alternative mean to support memory above 16MiB
>>> (128Mib) by replacing 3byte address op codes by their associated 4byte
>>> address versions.
>>>
>>> Using the dedicated 4byte address op codes doesn't change the internal
>>> state of the SPI NOR memory as opposed to using other means such as
>>> updating a Base Address Register (BAR) and sending command to enter/leave
>>> the 4byte mode.
>>>
>>> Hence when a CPU reset occurs, early bootloaders don't need to be aware
>>> of BAR value or 4byte mode being enabled: they can still access the first
>>> 16MiB of the SPI NOR memory using the regular 3byte address op codes.
>>>
>>> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
>>> Tested-by: Vignesh R <vigneshr@ti.com>
>>> ---
>>>  drivers/mtd/devices/serial_flash_cmds.h |   7 ---
>>>  drivers/mtd/devices/st_spi_fsm.c        |  28 ++++-----
>>>  drivers/mtd/spi-nor/spi-nor.c           | 104 +++++++++++++++++++++++++-------
>>>  include/linux/mtd/spi-nor.h             |  22 +++++--
>>>  4 files changed, 113 insertions(+), 48 deletions(-)
>>>
>>> diff --git a/drivers/mtd/devices/serial_flash_cmds.h b/drivers/mtd/devices/serial_flash_cmds.h
>>> index f59a125295d0..8b81e15105dd 100644
>>> --- a/drivers/mtd/devices/serial_flash_cmds.h
>>> +++ b/drivers/mtd/devices/serial_flash_cmds.h
>>> @@ -18,19 +18,12 @@
>>>  #define SPINOR_OP_RDVCR                0x85
>>>
>>>  /* JEDEC Standard - Serial Flash Discoverable Parmeters (SFDP) Commands */
>>> -#define SPINOR_OP_READ_1_2_2   0xbb    /* DUAL I/O READ */
>>> -#define SPINOR_OP_READ_1_4_4   0xeb    /* QUAD I/O READ */
>>> -
>>>  #define SPINOR_OP_WRITE                0x02    /* PAGE PROGRAM */
>>>  #define SPINOR_OP_WRITE_1_1_2  0xa2    /* DUAL INPUT PROGRAM */
>>>  #define SPINOR_OP_WRITE_1_2_2  0xd2    /* DUAL INPUT EXT PROGRAM */
>>>  #define SPINOR_OP_WRITE_1_1_4  0x32    /* QUAD INPUT PROGRAM */
>>>  #define SPINOR_OP_WRITE_1_4_4  0x12    /* QUAD INPUT EXT PROGRAM */
>>>
>>> -/* READ commands with 32-bit addressing */
>>> -#define SPINOR_OP_READ4_1_2_2  0xbc
>>> -#define SPINOR_OP_READ4_1_4_4  0xec
>>> -
>>>  /* Configuration flags */
>>>  #define FLASH_FLAG_SINGLE      0x000000ff
>>>  #define FLASH_FLAG_READ_WRITE  0x00000001
>>> diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
>>> index 5454b4113589..804313a33f2b 100644
>>> --- a/drivers/mtd/devices/st_spi_fsm.c
>>> +++ b/drivers/mtd/devices/st_spi_fsm.c
>>> @@ -507,13 +507,13 @@ static struct seq_rw_config n25q_read3_configs[] = {
>>>   *     - 'FAST' variants configured for 8 dummy cycles (see note above.)
>>>   */
>>>  static struct seq_rw_config n25q_read4_configs[] = {
>>> -       {FLASH_FLAG_READ_1_4_4, SPINOR_OP_READ4_1_4_4,  0, 4, 4, 0x00, 0, 8},
>>> -       {FLASH_FLAG_READ_1_1_4, SPINOR_OP_READ4_1_1_4,  0, 1, 4, 0x00, 0, 8},
>>> -       {FLASH_FLAG_READ_1_2_2, SPINOR_OP_READ4_1_2_2,  0, 2, 2, 0x00, 0, 8},
>>> -       {FLASH_FLAG_READ_1_1_2, SPINOR_OP_READ4_1_1_2,  0, 1, 2, 0x00, 0, 8},
>>> -       {FLASH_FLAG_READ_FAST,  SPINOR_OP_READ4_FAST,   0, 1, 1, 0x00, 0, 8},
>>> -       {FLASH_FLAG_READ_WRITE, SPINOR_OP_READ4,        0, 1, 1, 0x00, 0, 0},
>>> -       {0x00,                  0,                      0, 0, 0, 0x00, 0, 0},
>>> +       {FLASH_FLAG_READ_1_4_4, SPINOR_OP_READ_1_4_4_4B, 0, 4, 4, 0x00, 0, 8},
>>> +       {FLASH_FLAG_READ_1_1_4, SPINOR_OP_READ_1_1_4_4B, 0, 1, 4, 0x00, 0, 8},
>>> +       {FLASH_FLAG_READ_1_2_2, SPINOR_OP_READ_1_2_2_4B, 0, 2, 2, 0x00, 0, 8},
>>> +       {FLASH_FLAG_READ_1_1_2, SPINOR_OP_READ_1_1_2_4B, 0, 1, 2, 0x00, 0, 8},
>>> +       {FLASH_FLAG_READ_FAST,  SPINOR_OP_READ_FAST_4B,  0, 1, 1, 0x00, 0, 8},
>>> +       {FLASH_FLAG_READ_WRITE, SPINOR_OP_READ_4B,       0, 1, 1, 0x00, 0, 0},
>>> +       {0x00,                  0,                       0, 0, 0, 0x00, 0, 0},
>>>  };
>>>
>>>  /*
>>> @@ -553,13 +553,13 @@ static int stfsm_mx25_en_32bit_addr_seq(struct stfsm_seq *seq)
>>>   * entering a state that is incompatible with the SPIBoot Controller.
>>>   */
>>>  static struct seq_rw_config stfsm_s25fl_read4_configs[] = {
>>> -       {FLASH_FLAG_READ_1_4_4,  SPINOR_OP_READ4_1_4_4,  0, 4, 4, 0x00, 2, 4},
>>> -       {FLASH_FLAG_READ_1_1_4,  SPINOR_OP_READ4_1_1_4,  0, 1, 4, 0x00, 0, 8},
>>> -       {FLASH_FLAG_READ_1_2_2,  SPINOR_OP_READ4_1_2_2,  0, 2, 2, 0x00, 4, 0},
>>> -       {FLASH_FLAG_READ_1_1_2,  SPINOR_OP_READ4_1_1_2,  0, 1, 2, 0x00, 0, 8},
>>> -       {FLASH_FLAG_READ_FAST,   SPINOR_OP_READ4_FAST,   0, 1, 1, 0x00, 0, 8},
>>> -       {FLASH_FLAG_READ_WRITE,  SPINOR_OP_READ4,        0, 1, 1, 0x00, 0, 0},
>>> -       {0x00,                   0,                      0, 0, 0, 0x00, 0, 0},
>>> +       {FLASH_FLAG_READ_1_4_4,  SPINOR_OP_READ_1_4_4_4B,  0, 4, 4, 0x00, 2, 4},
>>> +       {FLASH_FLAG_READ_1_1_4,  SPINOR_OP_READ_1_1_4_4B,  0, 1, 4, 0x00, 0, 8},
>>> +       {FLASH_FLAG_READ_1_2_2,  SPINOR_OP_READ_1_2_2_4B,  0, 2, 2, 0x00, 4, 0},
>>> +       {FLASH_FLAG_READ_1_1_2,  SPINOR_OP_READ_1_1_2_4B,  0, 1, 2, 0x00, 0, 8},
>>> +       {FLASH_FLAG_READ_FAST,   SPINOR_OP_READ_FAST_4B,   0, 1, 1, 0x00, 0, 8},
>>> +       {FLASH_FLAG_READ_WRITE,  SPINOR_OP_READ_4B,        0, 1, 1, 0x00, 0, 0},
>>> +       {0x00,                   0,                        0, 0, 0, 0x00, 0, 0},
>>>  };
>>>
>>>  static struct seq_rw_config stfsm_s25fl_write4_configs[] = {
>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>>> index 5c87b2d99507..423448c1c7a8 100644
>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>> @@ -75,6 +75,10 @@ struct flash_info {
>>>                                          * bit. Must be used with
>>>                                          * SPI_NOR_HAS_LOCK.
>>>                                          */
>>> +#define SPI_NOR_4B_OPCODES     BIT(10) /*
>>> +                                        * Use dedicated 4byte address op codes
>>> +                                        * to support memory size above 128Mib.
>>> +                                        */
>>>  };
>>>
>>>  #define JEDEC_MFR(info)        ((info)->id[0])
>>> @@ -188,6 +192,81 @@ static inline struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
>>>         return mtd->priv;
>>>  }
>>>
>>> +
>>> +struct spi_nor_address_entry {
>>> +       u8      src_opcode;
>>> +       u8      dst_opcode;
>>> +};
>>> +
>>> +static u8 spi_nor_convert_opcode(u8 opcode,
>>> +                                const struct spi_nor_address_entry *entries,
>>> +                                size_t num_entries)
>>> +{
>>> +       int min, max;
>>> +
>>> +       min = 0;
>>> +       max = num_entries - 1;
>>> +       while (min <= max) {
>>> +               int mid = (min + max) >> 1;
>>> +               const struct spi_nor_address_entry *entry = &entries[mid];
>>> +
>>> +               if (opcode == entry->src_opcode)
>>> +                       return entry->dst_opcode;
>>> +
>>> +               if (opcode < entry->src_opcode)
>>> +                       max = mid - 1;
>>> +               else
>>> +                       min = mid + 1;
>>> +       }
>>> +
>>> +       /* No conversion found */
>>> +       return opcode;
>>> +}
>>> +
>>> +static u8 spi_nor_3to4_opcode(u8 opcode)
>>> +{
>>> +       /* MUST be sorted by 3byte opcode */
>>> +#define ENTRY_3TO4(_opcode)    { _opcode, _opcode##_4B }
>>> +       static const struct spi_nor_address_entry spi_nor_3to4_table[] = {
>>> +               ENTRY_3TO4(SPINOR_OP_PP),               /* 0x02 */
>>> +               ENTRY_3TO4(SPINOR_OP_READ),             /* 0x03 */
>>> +               ENTRY_3TO4(SPINOR_OP_READ_FAST),        /* 0x0b */
>>> +               ENTRY_3TO4(SPINOR_OP_BE_4K),            /* 0x20 */
>>> +               ENTRY_3TO4(SPINOR_OP_PP_1_1_4),         /* 0x32 */
>>> +               ENTRY_3TO4(SPINOR_OP_PP_1_4_4),         /* 0x38 */
>>> +               ENTRY_3TO4(SPINOR_OP_READ_1_1_2),       /* 0x3b */
>>> +               ENTRY_3TO4(SPINOR_OP_BE_32K),           /* 0x52 */
>>> +               ENTRY_3TO4(SPINOR_OP_READ_1_1_4),       /* 0x6b */
>>> +               ENTRY_3TO4(SPINOR_OP_READ_1_2_2),       /* 0xbb */
>>> +               ENTRY_3TO4(SPINOR_OP_SE),               /* 0xd8 */
>>> +               ENTRY_3TO4(SPINOR_OP_READ_1_4_4),       /* 0xeb */
>>> +       };
>>> +#undef ENTRY_3TO4
>>> +
>>> +       return spi_nor_convert_opcode(opcode, spi_nor_3to4_table,
>>> +                                     ARRAY_SIZE(spi_nor_3to4_table));
>>> +}
>>> +
>>> +static void spi_nor_set_4byte_opcodes(struct spi_nor *nor,
>>> +                                     const struct flash_info *info)
>>> +{
>>> +       /* Do some manufacturer fixups first */
>>> +       switch (JEDEC_MFR(info)) {
>>> +       case SNOR_MFR_SPANSION:
>>> +               /* No small sector erase for 4-byte command set */
>>> +               nor->erase_opcode = SPINOR_OP_SE;
>>> +               nor->mtd.erasesize = info->sector_size;
>>> +               break;
>>> +
>>> +       default:
>>> +               break;
>>> +       }
>>> +
>>> +       nor->read_opcode        = spi_nor_3to4_opcode(nor->read_opcode);
>>> +       nor->program_opcode     = spi_nor_3to4_opcode(nor->program_opcode);
>>> +       nor->erase_opcode       = spi_nor_3to4_opcode(nor->erase_opcode);
>>> +}
>>> +
>>>  /* Enable/disable 4-byte addressing mode. */
>>>  static inline int set_4byte(struct spi_nor *nor, const struct flash_info *info,
>>>                             int enable)
>>> @@ -1476,27 +1555,10 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>>         else if (mtd->size > 0x1000000) {
>>>                 /* enable 4-byte addressing if the device exceeds 16MiB */
>>>                 nor->addr_width = 4;
>>> -               if (JEDEC_MFR(info) == SNOR_MFR_SPANSION) {
>>> -                       /* Dedicated 4-byte command set */
>>> -                       switch (nor->flash_read) {
>>> -                       case SPI_NOR_QUAD:
>>> -                               nor->read_opcode = SPINOR_OP_READ4_1_1_4;
>>> -                               break;
>>> -                       case SPI_NOR_DUAL:
>>> -                               nor->read_opcode = SPINOR_OP_READ4_1_1_2;
>>> -                               break;
>>> -                       case SPI_NOR_FAST:
>>> -                               nor->read_opcode = SPINOR_OP_READ4_FAST;
>>> -                               break;
>>> -                       case SPI_NOR_NORMAL:
>>> -                               nor->read_opcode = SPINOR_OP_READ4;
>>> -                               break;
>>> -                       }
>>> -                       nor->program_opcode = SPINOR_OP_PP_4B;
>>> -                       /* No small sector erase for 4-byte command set */
>>> -                       nor->erase_opcode = SPINOR_OP_SE_4B;
>>> -                       mtd->erasesize = info->sector_size;
>>> -               } else
>>> +               if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
>>> +                   info->flags & SPI_NOR_4B_OPCODES)
>>> +                       spi_nor_set_4byte_opcodes(nor, info);
>>
>> I am giving my inputs based on the discussion on this thread[1].
>>
>> Seems like winbond[2], stmicron, spansion look better way or equally
>> same for accessing 4-byte addressing commands. and I find few of the
>> Macronix [3] have support of 4B addressing(see Table 5) but
>> lack/unable to find the 4B opcodes.
>>
>
> The Macronix mx25l25635e has no 4-byte address op codes. Instead you can
> use the EN4B (enter 4-byte mode) B7h op code to make the memory enter its
> 4-byte mode: hence you still write one of the 3-byte address op code but now
> this op code is followed by a 4-byte address.
> Currently the spi-nor framework uses this EN4B op code for all but Spansion
> memories. However entering this 4-byte mode is statefull.
> For the Macronix 35e won't be able to use the 4-byte address instruction set
> and we will keep on entering the 4-byte mode.
>
> The idea behind the patch is to use the 4-byte address instruction set when
> we are absolutely sure this set is supported by a given memory but keep the
> current behaviour for other memories.
>
>> And about BAR support, based on my experience in u-boot and research
>> on above chips only require when controller isn't supporting 4-byte
>> addressing even if connected flash chip has > 16MiB, that means there
>> is no exact or equivalent requirement for flash side either.
>>
>> So, adding the flags on flash table might be the good option instead
>> making code to convert 3B opcode to 4B because anyway the chips
>> require SPI_NOR_4B_OPCODES and also except winbond, stmicron, spansion
>> not too many flash needed > 16MiB support as of now.
>>
>
> Sorry but I don't understand what you mean or suggest here!

OK, What I am trying to say here is except the Macronix reaming chips
- Winbond, Stmicron, Spansion has a similar way of opcode handling. So
why can't we add the flags for supporting chips instead of 3B_to_4B
conversion I'm thinking this conversion of opcodes not much helpful
here as we dealing only few vendor chips and also not adequate for all
scenarios.

thanks!
Cyrille Pitchen Nov. 2, 2016, 1:34 p.m. UTC | #7
Le 02/11/2016 à 12:11, Jagan Teki a écrit :
> On Wed, Nov 2, 2016 at 4:19 PM, Cyrille Pitchen
> <cyrille.pitchen@atmel.com> wrote:
>> Hi,
>>
>> Le 31/10/2016 à 19:51, Jagan Teki a écrit :
>>> Hi Cyrille,
>>>
>>> On Mon, Oct 24, 2016 at 10:04 PM, Cyrille Pitchen
>>> <cyrille.pitchen@atmel.com> wrote:
>>>> This patch provides an alternative mean to support memory above 16MiB
>>>> (128Mib) by replacing 3byte address op codes by their associated 4byte
>>>> address versions.
>>>>
>>>> Using the dedicated 4byte address op codes doesn't change the internal
>>>> state of the SPI NOR memory as opposed to using other means such as
>>>> updating a Base Address Register (BAR) and sending command to enter/leave
>>>> the 4byte mode.
>>>>
>>>> Hence when a CPU reset occurs, early bootloaders don't need to be aware
>>>> of BAR value or 4byte mode being enabled: they can still access the first
>>>> 16MiB of the SPI NOR memory using the regular 3byte address op codes.
>>>>
>>>> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
>>>> Tested-by: Vignesh R <vigneshr@ti.com>
>>>> ---
>>>>  drivers/mtd/devices/serial_flash_cmds.h |   7 ---
>>>>  drivers/mtd/devices/st_spi_fsm.c        |  28 ++++-----
>>>>  drivers/mtd/spi-nor/spi-nor.c           | 104 +++++++++++++++++++++++++-------
>>>>  include/linux/mtd/spi-nor.h             |  22 +++++--
>>>>  4 files changed, 113 insertions(+), 48 deletions(-)
>>>>
>>>> diff --git a/drivers/mtd/devices/serial_flash_cmds.h b/drivers/mtd/devices/serial_flash_cmds.h
>>>> index f59a125295d0..8b81e15105dd 100644
>>>> --- a/drivers/mtd/devices/serial_flash_cmds.h
>>>> +++ b/drivers/mtd/devices/serial_flash_cmds.h
>>>> @@ -18,19 +18,12 @@
>>>>  #define SPINOR_OP_RDVCR                0x85
>>>>
>>>>  /* JEDEC Standard - Serial Flash Discoverable Parmeters (SFDP) Commands */
>>>> -#define SPINOR_OP_READ_1_2_2   0xbb    /* DUAL I/O READ */
>>>> -#define SPINOR_OP_READ_1_4_4   0xeb    /* QUAD I/O READ */
>>>> -
>>>>  #define SPINOR_OP_WRITE                0x02    /* PAGE PROGRAM */
>>>>  #define SPINOR_OP_WRITE_1_1_2  0xa2    /* DUAL INPUT PROGRAM */
>>>>  #define SPINOR_OP_WRITE_1_2_2  0xd2    /* DUAL INPUT EXT PROGRAM */
>>>>  #define SPINOR_OP_WRITE_1_1_4  0x32    /* QUAD INPUT PROGRAM */
>>>>  #define SPINOR_OP_WRITE_1_4_4  0x12    /* QUAD INPUT EXT PROGRAM */
>>>>
>>>> -/* READ commands with 32-bit addressing */
>>>> -#define SPINOR_OP_READ4_1_2_2  0xbc
>>>> -#define SPINOR_OP_READ4_1_4_4  0xec
>>>> -
>>>>  /* Configuration flags */
>>>>  #define FLASH_FLAG_SINGLE      0x000000ff
>>>>  #define FLASH_FLAG_READ_WRITE  0x00000001
>>>> diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
>>>> index 5454b4113589..804313a33f2b 100644
>>>> --- a/drivers/mtd/devices/st_spi_fsm.c
>>>> +++ b/drivers/mtd/devices/st_spi_fsm.c
>>>> @@ -507,13 +507,13 @@ static struct seq_rw_config n25q_read3_configs[] = {
>>>>   *     - 'FAST' variants configured for 8 dummy cycles (see note above.)
>>>>   */
>>>>  static struct seq_rw_config n25q_read4_configs[] = {
>>>> -       {FLASH_FLAG_READ_1_4_4, SPINOR_OP_READ4_1_4_4,  0, 4, 4, 0x00, 0, 8},
>>>> -       {FLASH_FLAG_READ_1_1_4, SPINOR_OP_READ4_1_1_4,  0, 1, 4, 0x00, 0, 8},
>>>> -       {FLASH_FLAG_READ_1_2_2, SPINOR_OP_READ4_1_2_2,  0, 2, 2, 0x00, 0, 8},
>>>> -       {FLASH_FLAG_READ_1_1_2, SPINOR_OP_READ4_1_1_2,  0, 1, 2, 0x00, 0, 8},
>>>> -       {FLASH_FLAG_READ_FAST,  SPINOR_OP_READ4_FAST,   0, 1, 1, 0x00, 0, 8},
>>>> -       {FLASH_FLAG_READ_WRITE, SPINOR_OP_READ4,        0, 1, 1, 0x00, 0, 0},
>>>> -       {0x00,                  0,                      0, 0, 0, 0x00, 0, 0},
>>>> +       {FLASH_FLAG_READ_1_4_4, SPINOR_OP_READ_1_4_4_4B, 0, 4, 4, 0x00, 0, 8},
>>>> +       {FLASH_FLAG_READ_1_1_4, SPINOR_OP_READ_1_1_4_4B, 0, 1, 4, 0x00, 0, 8},
>>>> +       {FLASH_FLAG_READ_1_2_2, SPINOR_OP_READ_1_2_2_4B, 0, 2, 2, 0x00, 0, 8},
>>>> +       {FLASH_FLAG_READ_1_1_2, SPINOR_OP_READ_1_1_2_4B, 0, 1, 2, 0x00, 0, 8},
>>>> +       {FLASH_FLAG_READ_FAST,  SPINOR_OP_READ_FAST_4B,  0, 1, 1, 0x00, 0, 8},
>>>> +       {FLASH_FLAG_READ_WRITE, SPINOR_OP_READ_4B,       0, 1, 1, 0x00, 0, 0},
>>>> +       {0x00,                  0,                       0, 0, 0, 0x00, 0, 0},
>>>>  };
>>>>
>>>>  /*
>>>> @@ -553,13 +553,13 @@ static int stfsm_mx25_en_32bit_addr_seq(struct stfsm_seq *seq)
>>>>   * entering a state that is incompatible with the SPIBoot Controller.
>>>>   */
>>>>  static struct seq_rw_config stfsm_s25fl_read4_configs[] = {
>>>> -       {FLASH_FLAG_READ_1_4_4,  SPINOR_OP_READ4_1_4_4,  0, 4, 4, 0x00, 2, 4},
>>>> -       {FLASH_FLAG_READ_1_1_4,  SPINOR_OP_READ4_1_1_4,  0, 1, 4, 0x00, 0, 8},
>>>> -       {FLASH_FLAG_READ_1_2_2,  SPINOR_OP_READ4_1_2_2,  0, 2, 2, 0x00, 4, 0},
>>>> -       {FLASH_FLAG_READ_1_1_2,  SPINOR_OP_READ4_1_1_2,  0, 1, 2, 0x00, 0, 8},
>>>> -       {FLASH_FLAG_READ_FAST,   SPINOR_OP_READ4_FAST,   0, 1, 1, 0x00, 0, 8},
>>>> -       {FLASH_FLAG_READ_WRITE,  SPINOR_OP_READ4,        0, 1, 1, 0x00, 0, 0},
>>>> -       {0x00,                   0,                      0, 0, 0, 0x00, 0, 0},
>>>> +       {FLASH_FLAG_READ_1_4_4,  SPINOR_OP_READ_1_4_4_4B,  0, 4, 4, 0x00, 2, 4},
>>>> +       {FLASH_FLAG_READ_1_1_4,  SPINOR_OP_READ_1_1_4_4B,  0, 1, 4, 0x00, 0, 8},
>>>> +       {FLASH_FLAG_READ_1_2_2,  SPINOR_OP_READ_1_2_2_4B,  0, 2, 2, 0x00, 4, 0},
>>>> +       {FLASH_FLAG_READ_1_1_2,  SPINOR_OP_READ_1_1_2_4B,  0, 1, 2, 0x00, 0, 8},
>>>> +       {FLASH_FLAG_READ_FAST,   SPINOR_OP_READ_FAST_4B,   0, 1, 1, 0x00, 0, 8},
>>>> +       {FLASH_FLAG_READ_WRITE,  SPINOR_OP_READ_4B,        0, 1, 1, 0x00, 0, 0},
>>>> +       {0x00,                   0,                        0, 0, 0, 0x00, 0, 0},
>>>>  };
>>>>
>>>>  static struct seq_rw_config stfsm_s25fl_write4_configs[] = {
>>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>>>> index 5c87b2d99507..423448c1c7a8 100644
>>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>>> @@ -75,6 +75,10 @@ struct flash_info {
>>>>                                          * bit. Must be used with
>>>>                                          * SPI_NOR_HAS_LOCK.
>>>>                                          */
>>>> +#define SPI_NOR_4B_OPCODES     BIT(10) /*
>>>> +                                        * Use dedicated 4byte address op codes
>>>> +                                        * to support memory size above 128Mib.
>>>> +                                        */
>>>>  };
>>>>
>>>>  #define JEDEC_MFR(info)        ((info)->id[0])
>>>> @@ -188,6 +192,81 @@ static inline struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
>>>>         return mtd->priv;
>>>>  }
>>>>
>>>> +
>>>> +struct spi_nor_address_entry {
>>>> +       u8      src_opcode;
>>>> +       u8      dst_opcode;
>>>> +};
>>>> +
>>>> +static u8 spi_nor_convert_opcode(u8 opcode,
>>>> +                                const struct spi_nor_address_entry *entries,
>>>> +                                size_t num_entries)
>>>> +{
>>>> +       int min, max;
>>>> +
>>>> +       min = 0;
>>>> +       max = num_entries - 1;
>>>> +       while (min <= max) {
>>>> +               int mid = (min + max) >> 1;
>>>> +               const struct spi_nor_address_entry *entry = &entries[mid];
>>>> +
>>>> +               if (opcode == entry->src_opcode)
>>>> +                       return entry->dst_opcode;
>>>> +
>>>> +               if (opcode < entry->src_opcode)
>>>> +                       max = mid - 1;
>>>> +               else
>>>> +                       min = mid + 1;
>>>> +       }
>>>> +
>>>> +       /* No conversion found */
>>>> +       return opcode;
>>>> +}
>>>> +
>>>> +static u8 spi_nor_3to4_opcode(u8 opcode)
>>>> +{
>>>> +       /* MUST be sorted by 3byte opcode */
>>>> +#define ENTRY_3TO4(_opcode)    { _opcode, _opcode##_4B }
>>>> +       static const struct spi_nor_address_entry spi_nor_3to4_table[] = {
>>>> +               ENTRY_3TO4(SPINOR_OP_PP),               /* 0x02 */
>>>> +               ENTRY_3TO4(SPINOR_OP_READ),             /* 0x03 */
>>>> +               ENTRY_3TO4(SPINOR_OP_READ_FAST),        /* 0x0b */
>>>> +               ENTRY_3TO4(SPINOR_OP_BE_4K),            /* 0x20 */
>>>> +               ENTRY_3TO4(SPINOR_OP_PP_1_1_4),         /* 0x32 */
>>>> +               ENTRY_3TO4(SPINOR_OP_PP_1_4_4),         /* 0x38 */
>>>> +               ENTRY_3TO4(SPINOR_OP_READ_1_1_2),       /* 0x3b */
>>>> +               ENTRY_3TO4(SPINOR_OP_BE_32K),           /* 0x52 */
>>>> +               ENTRY_3TO4(SPINOR_OP_READ_1_1_4),       /* 0x6b */
>>>> +               ENTRY_3TO4(SPINOR_OP_READ_1_2_2),       /* 0xbb */
>>>> +               ENTRY_3TO4(SPINOR_OP_SE),               /* 0xd8 */
>>>> +               ENTRY_3TO4(SPINOR_OP_READ_1_4_4),       /* 0xeb */
>>>> +       };
>>>> +#undef ENTRY_3TO4
>>>> +
>>>> +       return spi_nor_convert_opcode(opcode, spi_nor_3to4_table,
>>>> +                                     ARRAY_SIZE(spi_nor_3to4_table));
>>>> +}
>>>> +
>>>> +static void spi_nor_set_4byte_opcodes(struct spi_nor *nor,
>>>> +                                     const struct flash_info *info)
>>>> +{
>>>> +       /* Do some manufacturer fixups first */
>>>> +       switch (JEDEC_MFR(info)) {
>>>> +       case SNOR_MFR_SPANSION:
>>>> +               /* No small sector erase for 4-byte command set */
>>>> +               nor->erase_opcode = SPINOR_OP_SE;
>>>> +               nor->mtd.erasesize = info->sector_size;
>>>> +               break;
>>>> +
>>>> +       default:
>>>> +               break;
>>>> +       }
>>>> +
>>>> +       nor->read_opcode        = spi_nor_3to4_opcode(nor->read_opcode);
>>>> +       nor->program_opcode     = spi_nor_3to4_opcode(nor->program_opcode);
>>>> +       nor->erase_opcode       = spi_nor_3to4_opcode(nor->erase_opcode);
>>>> +}
>>>> +
>>>>  /* Enable/disable 4-byte addressing mode. */
>>>>  static inline int set_4byte(struct spi_nor *nor, const struct flash_info *info,
>>>>                             int enable)
>>>> @@ -1476,27 +1555,10 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>>>         else if (mtd->size > 0x1000000) {
>>>>                 /* enable 4-byte addressing if the device exceeds 16MiB */
>>>>                 nor->addr_width = 4;
>>>> -               if (JEDEC_MFR(info) == SNOR_MFR_SPANSION) {
>>>> -                       /* Dedicated 4-byte command set */
>>>> -                       switch (nor->flash_read) {
>>>> -                       case SPI_NOR_QUAD:
>>>> -                               nor->read_opcode = SPINOR_OP_READ4_1_1_4;
>>>> -                               break;
>>>> -                       case SPI_NOR_DUAL:
>>>> -                               nor->read_opcode = SPINOR_OP_READ4_1_1_2;
>>>> -                               break;
>>>> -                       case SPI_NOR_FAST:
>>>> -                               nor->read_opcode = SPINOR_OP_READ4_FAST;
>>>> -                               break;
>>>> -                       case SPI_NOR_NORMAL:
>>>> -                               nor->read_opcode = SPINOR_OP_READ4;
>>>> -                               break;
>>>> -                       }
>>>> -                       nor->program_opcode = SPINOR_OP_PP_4B;
>>>> -                       /* No small sector erase for 4-byte command set */
>>>> -                       nor->erase_opcode = SPINOR_OP_SE_4B;
>>>> -                       mtd->erasesize = info->sector_size;
>>>> -               } else
>>>> +               if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
>>>> +                   info->flags & SPI_NOR_4B_OPCODES)
>>>> +                       spi_nor_set_4byte_opcodes(nor, info);
>>>
>>> I am giving my inputs based on the discussion on this thread[1].
>>>
>>> Seems like winbond[2], stmicron, spansion look better way or equally
>>> same for accessing 4-byte addressing commands. and I find few of the
>>> Macronix [3] have support of 4B addressing(see Table 5) but
>>> lack/unable to find the 4B opcodes.
>>>
>>
>> The Macronix mx25l25635e has no 4-byte address op codes. Instead you can
>> use the EN4B (enter 4-byte mode) B7h op code to make the memory enter its
>> 4-byte mode: hence you still write one of the 3-byte address op code but now
>> this op code is followed by a 4-byte address.
>> Currently the spi-nor framework uses this EN4B op code for all but Spansion
>> memories. However entering this 4-byte mode is statefull.
>> For the Macronix 35e won't be able to use the 4-byte address instruction set
>> and we will keep on entering the 4-byte mode.
>>
>> The idea behind the patch is to use the 4-byte address instruction set when
>> we are absolutely sure this set is supported by a given memory but keep the
>> current behaviour for other memories.
>>
>>> And about BAR support, based on my experience in u-boot and research
>>> on above chips only require when controller isn't supporting 4-byte
>>> addressing even if connected flash chip has > 16MiB, that means there
>>> is no exact or equivalent requirement for flash side either.
>>>
>>> So, adding the flags on flash table might be the good option instead
>>> making code to convert 3B opcode to 4B because anyway the chips
>>> require SPI_NOR_4B_OPCODES and also except winbond, stmicron, spansion
>>> not too many flash needed > 16MiB support as of now.
>>>
>>
>> Sorry but I don't understand what you mean or suggest here!
> 
> OK, What I am trying to say here is except the Macronix reaming chips
> - Winbond, Stmicron, Spansion has a similar way of opcode handling. So

Be careful about Micron memories too! many part numbers share the very same
entry in the spi_nor_ids[] array but some of them don't support the Page
4-byte address Program 1-1-4; the op code should be 12h.

For instance, with n25q512* memories about the 3-byte address:
"The code 38h is only valid for line items that enable the additional RESET#
pin; otherwise, code 12h is valid."

n25q512a8* parts have the additional RESET# pin:
- 4-byte address Page Program 1-1-1: 12h
- 3-byte address Page Program x-4-4: 38h

n25q512a1* parts don't have the additional RESET# pin:
- 4-byte address Page Program 1-1-1: N/A => entering the 4-byte mode is still
                                            needed here.
- 3-byte address Page Program x-4-4: 12h

> why can't we add the flags for supporting chips instead of 3B_to_4B
Which flag do you refer to?
The SPI_NOR_4B_OPCODES flag I've proposed in my patch is to enable the op
code conversion for a given memory when we know all parts handled by a single
spi_nor_ids[] entry support the 4-byte address instruction set.

If you think the conversion is useless, I don't understand what flag you
suggest to use instead.

> conversion I'm thinking this conversion of opcodes not much helpful
> here as we dealing only few vendor chips and also not adequate for all
> scenarios.
> 
Currently, a conversion is already done for Spansion memories. The conversion
is done for (Fast) Read op codes by a switch(nor->flash_read) statement.
Also for Page Program operations, currently only the SPI 1-1-1 is supported
so the spi-nor framework always use the SPINOR_OP_PP_4B op code.

My patch simply helps translating more op codes since further patches in the
series add support to more SPI protocols, hence more op code are needed.

> thanks!
>
diff mbox

Patch

diff --git a/drivers/mtd/devices/serial_flash_cmds.h b/drivers/mtd/devices/serial_flash_cmds.h
index f59a125295d0..8b81e15105dd 100644
--- a/drivers/mtd/devices/serial_flash_cmds.h
+++ b/drivers/mtd/devices/serial_flash_cmds.h
@@ -18,19 +18,12 @@ 
 #define SPINOR_OP_RDVCR		0x85
 
 /* JEDEC Standard - Serial Flash Discoverable Parmeters (SFDP) Commands */
-#define SPINOR_OP_READ_1_2_2	0xbb	/* DUAL I/O READ */
-#define SPINOR_OP_READ_1_4_4	0xeb	/* QUAD I/O READ */
-
 #define SPINOR_OP_WRITE		0x02	/* PAGE PROGRAM */
 #define SPINOR_OP_WRITE_1_1_2	0xa2	/* DUAL INPUT PROGRAM */
 #define SPINOR_OP_WRITE_1_2_2	0xd2	/* DUAL INPUT EXT PROGRAM */
 #define SPINOR_OP_WRITE_1_1_4	0x32	/* QUAD INPUT PROGRAM */
 #define SPINOR_OP_WRITE_1_4_4	0x12	/* QUAD INPUT EXT PROGRAM */
 
-/* READ commands with 32-bit addressing */
-#define SPINOR_OP_READ4_1_2_2	0xbc
-#define SPINOR_OP_READ4_1_4_4	0xec
-
 /* Configuration flags */
 #define FLASH_FLAG_SINGLE	0x000000ff
 #define FLASH_FLAG_READ_WRITE	0x00000001
diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
index 5454b4113589..804313a33f2b 100644
--- a/drivers/mtd/devices/st_spi_fsm.c
+++ b/drivers/mtd/devices/st_spi_fsm.c
@@ -507,13 +507,13 @@  static struct seq_rw_config n25q_read3_configs[] = {
  *	- 'FAST' variants configured for 8 dummy cycles (see note above.)
  */
 static struct seq_rw_config n25q_read4_configs[] = {
-	{FLASH_FLAG_READ_1_4_4, SPINOR_OP_READ4_1_4_4,	0, 4, 4, 0x00, 0, 8},
-	{FLASH_FLAG_READ_1_1_4, SPINOR_OP_READ4_1_1_4,	0, 1, 4, 0x00, 0, 8},
-	{FLASH_FLAG_READ_1_2_2, SPINOR_OP_READ4_1_2_2,	0, 2, 2, 0x00, 0, 8},
-	{FLASH_FLAG_READ_1_1_2, SPINOR_OP_READ4_1_1_2,	0, 1, 2, 0x00, 0, 8},
-	{FLASH_FLAG_READ_FAST,	SPINOR_OP_READ4_FAST,	0, 1, 1, 0x00, 0, 8},
-	{FLASH_FLAG_READ_WRITE, SPINOR_OP_READ4,	0, 1, 1, 0x00, 0, 0},
-	{0x00,			0,			0, 0, 0, 0x00, 0, 0},
+	{FLASH_FLAG_READ_1_4_4, SPINOR_OP_READ_1_4_4_4B, 0, 4, 4, 0x00, 0, 8},
+	{FLASH_FLAG_READ_1_1_4, SPINOR_OP_READ_1_1_4_4B, 0, 1, 4, 0x00, 0, 8},
+	{FLASH_FLAG_READ_1_2_2, SPINOR_OP_READ_1_2_2_4B, 0, 2, 2, 0x00, 0, 8},
+	{FLASH_FLAG_READ_1_1_2, SPINOR_OP_READ_1_1_2_4B, 0, 1, 2, 0x00, 0, 8},
+	{FLASH_FLAG_READ_FAST,	SPINOR_OP_READ_FAST_4B,  0, 1, 1, 0x00, 0, 8},
+	{FLASH_FLAG_READ_WRITE, SPINOR_OP_READ_4B,       0, 1, 1, 0x00, 0, 0},
+	{0x00,			0,                       0, 0, 0, 0x00, 0, 0},
 };
 
 /*
@@ -553,13 +553,13 @@  static int stfsm_mx25_en_32bit_addr_seq(struct stfsm_seq *seq)
  * entering a state that is incompatible with the SPIBoot Controller.
  */
 static struct seq_rw_config stfsm_s25fl_read4_configs[] = {
-	{FLASH_FLAG_READ_1_4_4,  SPINOR_OP_READ4_1_4_4,  0, 4, 4, 0x00, 2, 4},
-	{FLASH_FLAG_READ_1_1_4,  SPINOR_OP_READ4_1_1_4,  0, 1, 4, 0x00, 0, 8},
-	{FLASH_FLAG_READ_1_2_2,  SPINOR_OP_READ4_1_2_2,  0, 2, 2, 0x00, 4, 0},
-	{FLASH_FLAG_READ_1_1_2,  SPINOR_OP_READ4_1_1_2,  0, 1, 2, 0x00, 0, 8},
-	{FLASH_FLAG_READ_FAST,   SPINOR_OP_READ4_FAST,   0, 1, 1, 0x00, 0, 8},
-	{FLASH_FLAG_READ_WRITE,  SPINOR_OP_READ4,        0, 1, 1, 0x00, 0, 0},
-	{0x00,                   0,                      0, 0, 0, 0x00, 0, 0},
+	{FLASH_FLAG_READ_1_4_4,  SPINOR_OP_READ_1_4_4_4B,  0, 4, 4, 0x00, 2, 4},
+	{FLASH_FLAG_READ_1_1_4,  SPINOR_OP_READ_1_1_4_4B,  0, 1, 4, 0x00, 0, 8},
+	{FLASH_FLAG_READ_1_2_2,  SPINOR_OP_READ_1_2_2_4B,  0, 2, 2, 0x00, 4, 0},
+	{FLASH_FLAG_READ_1_1_2,  SPINOR_OP_READ_1_1_2_4B,  0, 1, 2, 0x00, 0, 8},
+	{FLASH_FLAG_READ_FAST,   SPINOR_OP_READ_FAST_4B,   0, 1, 1, 0x00, 0, 8},
+	{FLASH_FLAG_READ_WRITE,  SPINOR_OP_READ_4B,        0, 1, 1, 0x00, 0, 0},
+	{0x00,                   0,                        0, 0, 0, 0x00, 0, 0},
 };
 
 static struct seq_rw_config stfsm_s25fl_write4_configs[] = {
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 5c87b2d99507..423448c1c7a8 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -75,6 +75,10 @@  struct flash_info {
 					 * bit. Must be used with
 					 * SPI_NOR_HAS_LOCK.
 					 */
+#define SPI_NOR_4B_OPCODES	BIT(10)	/*
+					 * Use dedicated 4byte address op codes
+					 * to support memory size above 128Mib.
+					 */
 };
 
 #define JEDEC_MFR(info)	((info)->id[0])
@@ -188,6 +192,81 @@  static inline struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
 	return mtd->priv;
 }
 
+
+struct spi_nor_address_entry {
+	u8	src_opcode;
+	u8	dst_opcode;
+};
+
+static u8 spi_nor_convert_opcode(u8 opcode,
+				 const struct spi_nor_address_entry *entries,
+				 size_t num_entries)
+{
+	int min, max;
+
+	min = 0;
+	max = num_entries - 1;
+	while (min <= max) {
+		int mid = (min + max) >> 1;
+		const struct spi_nor_address_entry *entry = &entries[mid];
+
+		if (opcode == entry->src_opcode)
+			return entry->dst_opcode;
+
+		if (opcode < entry->src_opcode)
+			max = mid - 1;
+		else
+			min = mid + 1;
+	}
+
+	/* No conversion found */
+	return opcode;
+}
+
+static u8 spi_nor_3to4_opcode(u8 opcode)
+{
+	/* MUST be sorted by 3byte opcode */
+#define ENTRY_3TO4(_opcode)	{ _opcode, _opcode##_4B }
+	static const struct spi_nor_address_entry spi_nor_3to4_table[] = {
+		ENTRY_3TO4(SPINOR_OP_PP),		/* 0x02 */
+		ENTRY_3TO4(SPINOR_OP_READ),		/* 0x03 */
+		ENTRY_3TO4(SPINOR_OP_READ_FAST),	/* 0x0b */
+		ENTRY_3TO4(SPINOR_OP_BE_4K),		/* 0x20 */
+		ENTRY_3TO4(SPINOR_OP_PP_1_1_4),		/* 0x32 */
+		ENTRY_3TO4(SPINOR_OP_PP_1_4_4),		/* 0x38 */
+		ENTRY_3TO4(SPINOR_OP_READ_1_1_2),	/* 0x3b */
+		ENTRY_3TO4(SPINOR_OP_BE_32K),		/* 0x52 */
+		ENTRY_3TO4(SPINOR_OP_READ_1_1_4),	/* 0x6b */
+		ENTRY_3TO4(SPINOR_OP_READ_1_2_2),	/* 0xbb */
+		ENTRY_3TO4(SPINOR_OP_SE),		/* 0xd8 */
+		ENTRY_3TO4(SPINOR_OP_READ_1_4_4),	/* 0xeb */
+	};
+#undef ENTRY_3TO4
+
+	return spi_nor_convert_opcode(opcode, spi_nor_3to4_table,
+				      ARRAY_SIZE(spi_nor_3to4_table));
+}
+
+static void spi_nor_set_4byte_opcodes(struct spi_nor *nor,
+				      const struct flash_info *info)
+{
+	/* Do some manufacturer fixups first */
+	switch (JEDEC_MFR(info)) {
+	case SNOR_MFR_SPANSION:
+		/* No small sector erase for 4-byte command set */
+		nor->erase_opcode = SPINOR_OP_SE;
+		nor->mtd.erasesize = info->sector_size;
+		break;
+
+	default:
+		break;
+	}
+
+	nor->read_opcode	= spi_nor_3to4_opcode(nor->read_opcode);
+	nor->program_opcode	= spi_nor_3to4_opcode(nor->program_opcode);
+	nor->erase_opcode	= spi_nor_3to4_opcode(nor->erase_opcode);
+}
+
 /* Enable/disable 4-byte addressing mode. */
 static inline int set_4byte(struct spi_nor *nor, const struct flash_info *info,
 			    int enable)
@@ -1476,27 +1555,10 @@  int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 	else if (mtd->size > 0x1000000) {
 		/* enable 4-byte addressing if the device exceeds 16MiB */
 		nor->addr_width = 4;
-		if (JEDEC_MFR(info) == SNOR_MFR_SPANSION) {
-			/* Dedicated 4-byte command set */
-			switch (nor->flash_read) {
-			case SPI_NOR_QUAD:
-				nor->read_opcode = SPINOR_OP_READ4_1_1_4;
-				break;
-			case SPI_NOR_DUAL:
-				nor->read_opcode = SPINOR_OP_READ4_1_1_2;
-				break;
-			case SPI_NOR_FAST:
-				nor->read_opcode = SPINOR_OP_READ4_FAST;
-				break;
-			case SPI_NOR_NORMAL:
-				nor->read_opcode = SPINOR_OP_READ4;
-				break;
-			}
-			nor->program_opcode = SPINOR_OP_PP_4B;
-			/* No small sector erase for 4-byte command set */
-			nor->erase_opcode = SPINOR_OP_SE_4B;
-			mtd->erasesize = info->sector_size;
-		} else
+		if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
+		    info->flags & SPI_NOR_4B_OPCODES)
+			spi_nor_set_4byte_opcodes(nor, info);
+		else
 			set_4byte(nor, info, 1);
 	} else {
 		nor->addr_width = 3;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index c425c7b4c2a0..8b02fd7864d0 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -43,9 +43,13 @@ 
 #define SPINOR_OP_WRSR		0x01	/* Write status register 1 byte */
 #define SPINOR_OP_READ		0x03	/* Read data bytes (low frequency) */
 #define SPINOR_OP_READ_FAST	0x0b	/* Read data bytes (high frequency) */
-#define SPINOR_OP_READ_1_1_2	0x3b	/* Read data bytes (Dual SPI) */
-#define SPINOR_OP_READ_1_1_4	0x6b	/* Read data bytes (Quad SPI) */
+#define SPINOR_OP_READ_1_1_2	0x3b	/* Read data bytes (Dual Output SPI) */
+#define SPINOR_OP_READ_1_2_2	0xbb	/* Read data bytes (Dual I/O SPI) */
+#define SPINOR_OP_READ_1_1_4	0x6b	/* Read data bytes (Quad Output SPI) */
+#define SPINOR_OP_READ_1_4_4	0xeb	/* Read data bytes (Quad I/O SPI) */
 #define SPINOR_OP_PP		0x02	/* Page program (up to 256 bytes) */
+#define SPINOR_OP_PP_1_1_4	0x32	/* Quad page program */
+#define SPINOR_OP_PP_1_4_4	0x38	/* Quad page program */
 #define SPINOR_OP_BE_4K		0x20	/* Erase 4KiB block */
 #define SPINOR_OP_BE_4K_PMC	0xd7	/* Erase 4KiB block on PMC chips */
 #define SPINOR_OP_BE_32K	0x52	/* Erase 32KiB block */
@@ -56,11 +60,17 @@ 
 #define SPINOR_OP_RDFSR		0x70	/* Read flag status register */
 
 /* 4-byte address opcodes - used on Spansion and some Macronix flashes. */
-#define SPINOR_OP_READ4		0x13	/* Read data bytes (low frequency) */
-#define SPINOR_OP_READ4_FAST	0x0c	/* Read data bytes (high frequency) */
-#define SPINOR_OP_READ4_1_1_2	0x3c	/* Read data bytes (Dual SPI) */
-#define SPINOR_OP_READ4_1_1_4	0x6c	/* Read data bytes (Quad SPI) */
+#define SPINOR_OP_READ_4B	0x13	/* Read data bytes (low frequency) */
+#define SPINOR_OP_READ_FAST_4B	0x0c	/* Read data bytes (high frequency) */
+#define SPINOR_OP_READ_1_1_2_4B	0x3c	/* Read data bytes (Dual Output SPI) */
+#define SPINOR_OP_READ_1_2_2_4B	0xbc	/* Read data bytes (Dual I/O SPI) */
+#define SPINOR_OP_READ_1_1_4_4B	0x6c	/* Read data bytes (Quad Output SPI) */
+#define SPINOR_OP_READ_1_4_4_4B	0xec	/* Read data bytes (Quad I/O SPI) */
 #define SPINOR_OP_PP_4B		0x12	/* Page program (up to 256 bytes) */
+#define SPINOR_OP_PP_1_1_4_4B	0x34	/* Quad page program */
+#define SPINOR_OP_PP_1_4_4_4B	0x3e	/* Quad page program */
+#define SPINOR_OP_BE_4K_4B	0x21	/* Erase 4KiB block */
+#define SPINOR_OP_BE_32K_4B	0x5c	/* Erase 32KiB block */
 #define SPINOR_OP_SE_4B		0xdc	/* Sector erase (usually 64KiB) */
 
 /* Used for SST flashes only. */