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

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

On 11/21/2013 08:18 AM, Huang Shijie wrote:
> 于 2013年11月20日 18:16, Angus Clark 写道:
>> 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) },
> yes.
>> 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.
>>
> Does the s25fl129p1 support the DDR quad read?
> I do not have the datasheet.

No, the s25fl129p1 only supports SDR "Quad Output Read" and "Quad I/O Read".  I
thought the M25P80_QUAD_READ flag was for the SDR operations though, hence why I
said the m25p80 driver was not sensitive to the differences between s25fl129p1
and s25fl128s1.  Is that not the case?

> it's a little strange if we add the flag to the s25fl129p1.
> The one who wants to enable the s25fl128s on its board, will add the the
> name s25fl129p1 in the DTS.

That should work, but the m25p_ids[] table is also used for probing unknown
devices, so we need to be careful when adding a new entry.

>> 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').
>>
> yes. I also think we should probe the 6th READID byte.

I think that is by far the best approach...

Cheers,

Angus

>> 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
> yes. the s25fl128s0 is beter.
> 
> thanks
> Huang Shijie
> 
> 
> 
> 

-- 
-------------------------------------
Angus Clark
ST Microelectronics (R&D) Ltd.
1000 Aztec West, Bristol, BS32 4SQ
email: angus.clark@st.com
tel: +44 (0) 1454 462389
st-tina: 065 2389
-------------------------------------