Re: [PATCH] mtd: m25p80: add support for Spansion s25fl128s chip

[Angus Clark <angus.clark@st.com>]

Hi Huang Shijie,

On 11/20/2013 08:52 AM, Huang Shijie wrote:
> This chip supports the quad read.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
>  drivers/mtd/devices/m25p80.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 7dc2c14..720899b 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -941,6 +941,7 @@ static const struct spi_device_id m25p_ids[] = {
>  	 */
>  	{ "s25sl032p",  INFO(0x010215, 0x4d00,  64 * 1024,  64, 0) },
>  	{ "s25sl064p",  INFO(0x010216, 0x4d00,  64 * 1024, 128, 0) },
> +	{ "s25fl128s",	INFO(0x012018, 0x4d01,  64 * 1024, 256, M25P80_QUAD_READ) },
>  	{ "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, 0) },
>  	{ "s25fl256s1", INFO(0x010219, 0x4d01,  64 * 1024, 512, M25P80_QUAD_READ) },
>  	{ "s25fl512s",  INFO(0x010220, 0x4d00, 256 * 1024, 256, 0) },

I would suggest using the name "s25fl128s1" to indicate the 64KiB sector variant
[1].  However, I would also point out that there is already an entry in the
table that matches the jedec_id/ext_id:

	{ "s25fl129p1", INFO(0x012018, 0x4d01,  64 * 1024, 256, 0) },

As far as I can tell, the m25p80.c driver is not sensitive to the differences
between the 'P' and the 'S' generations; both support M25P80_QUAD_READ, so the
flag could be added to the s25fl129p1 entry if required.

If it was deemed necessary to differentiate between the 'P' and 'S', then the
jedec_probe() code would need to be updated to consider the 6th READID byte
(0x80 for 'S').

Cheers,

Angus

[1] The name should really be "s25fl128s0" where the appended '0' represents the
"model number" 0 for uniform 64KiB sectors.  However, all the other Spansion
entries get this the wrong way round so perhaps it's best to stick with the
existing scheme.