Re: [PATCH] mtd: spi-nor: Fixup page size and map selection for S25FS-S

[Yicong Yang <yangyicong@xxxxxxxxxxxxx>]

Hi John and Tudor,


On 2020/4/27 15:27, John Garry wrote:
> + Yicong Yang
>
> Hi Tudor,
>
>>
>> On Monday, March 2, 2020 8:25:48 PM EEST John Garry wrote:
>>> So do you know how we can tell if the part is s25fl129p1 or S25FS128S?
>>> Is it based on family id? For the part of my board, it has the same id
>>> according to "s25fl129p1" entry in the spi-nor driver, yet the SFDP
>>> signature is not present (signature reads as 0x4d182001 vs expected
>>
>>   0x4d182001 looks like the flash id, but in reversed order.
>>
>>> 0x50444653). I printed the family id, and it is 81h, which seems to
>>> align with S25FS (which should support SFDP). Confused.
>>>
>>
>> We can differentiate between flashes by the family id:  80h for FL-S and 81h
>> for FS-S. If I understood correctly your flash id is 0x01, 0x20, 0x18, 0x4d,
>> 0x01, 0x81. According to the spansion datasheets, this should identify with a
>> s25fs128s1 entry. Please check the patch from below, let me know if it's ok.
>>
>>> What's more, the spi-nor probe is failing for this part since I enabled
>>> quad spi. So I am interested to know if there is some differences
>>> between these part families for that.
>>
>> In which conditions is it failing? Please open a separate thread.
>
> So my colleague Yicon debugged this, and it seems to be an issue with our controller. The background is that we can blacklist certain commands in firmware, and some relevant commands were blacklisted such that quad enable failed.
>
> But we have it working now, I think. Yicon can confirm (or start a thread please for outstanding issues).

Yes, now the flash is fully enabled. It's the firmware matters, not about the kernel drivers nor the flash.

>
>> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
>> index 88183eba8ac1..ea72f0e5be73 100644
>> --- a/drivers/mtd/spi-nor/spansion.c
>> +++ b/drivers/mtd/spi-nor/spansion.c
>> @@ -22,6 +22,9 @@ static const struct flash_info spansion_parts[] = {
>>          { "s25fl128s1", INFO6(0x012018, 0x4d0180, 64 * 1024, 256,
>>                                SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
>>                                USE_CLSR) },
>> +       { "s25fs128s1", INFO6(0x012018, 0x4d0181,  64 * 1024, 256,
>> +                             SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
>> +                             USE_CLSR) },
>
> I wasn't sure if you wanted to add a separate entry if it has same properties as other part, due to extra maintenance. It is nice to know the exact part, though.

The flash need some post bfpt fixup. As the page size may configured as 512byte, not the 256byte
parsed from bfpt. So a separate entry is needed. Both Alexander and Sergei provide the solution,
and I add it in my patch.

Regards,
Yicong

>
>>          { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128,
>>                               SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
>>                               USE_CLSR) },
>>
>>
>> .
>>
>
> .
>


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/