Re: [PATCH v3] mtd: spi-nor: Add support for Cypress cy15x104q

[Vignesh Raghavendra <vigneshr@ti.com>]

Hi,

On 11/6/20 12:24 AM, Tim Harvey wrote:
> On Thu, May 28, 2020 at 1:13 AM <Tudor.Ambarus@microchip.com> wrote:
>>
>> On Friday, April 24, 2020 9:56:26 AM EEST Sascha Hauer wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
>>> content is safe
>>>
>>> The Cypress cy15b104q and cy15v104q are 4Mbit serial SPI F-RAM devices.
>>> Add support for them to the spi-nor driver.
>>>
>>> The actual Device ID of this chip is 7f 7f 7f 7f 7f 7f c2 2c 04. That is
>>> six times the continuation code 7f followed by c2 for Ramtron.
>>> Unfortunately the chip sends the Device ID in reversed order, so the
>>> continuation code is not at the beginning, but instead at the end. Even
>>> more unfortunate is that when reading further the chip sends more 7f
>>> codes which means we are not even able to count the continuation codes.
>>> We can only hope that this reversed Device ID will never match any other
>>> devices ID.
>>>
>>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>>
>> Collisions are improbable as of now, the solution from above is good
>> enough. In case of future collisions one can introduce an INFO9 macro,
>> with the downsize that struct flash_info would grow and we have lots of
>> flashes. A more elegant solution would be to introduce dedicated
>> flash ID tables for each bank in JESP106BA.
>>
>> Amended commit description with the above text and applied. Thanks.
>>
> 
> Greetings,
> 
> I've got a board with a  CY15B104Q-LHXI on it and I found that it's id
> reads as  7f 7f 7f 7f 7f 7f c2 26 08 so it fails the match added above
> in a2644d5f36: mtd: spi-nor: Add support for Cypress cy15x104q
> 
> I also found a reference in
> https://community.cypress.com/docs/DOC-14647 that states "that newer
> 1.8 V devices have the 9 device ID bytes coming out in the other order
> which requires a minor change in the ID entry" which would explain the
> situation you encountered.
> 

So, Cypress seems to have fixed ID table to follow JEP106BA standard.

> Adding '{ "cy15x104q",  INFO(0x7f7f7f, 0, 512 * 1024, 1,
> SPI_NOR_NO_ERASE) },' works for the part I have.
> 

NACK, 0x7F is continuation code and not actual flash ID code. See JEDEC
standard JEP106BA for more details. In short, driver should just keep
looking at next byte in the sequence for manf ID when it reads a value
of 0x7F.

You will have to extend spi_nor_read_id() function to
take care of continuation code and read more bytes in order to get the
actual Manuf/device ID. And then walk through
spi_nor_manufacturer->parts table looking for the corresponding entry.

Would need to store bank number (implied by number of continuation
codes) as part of flash_info struct for cross reference during lookup


> Would it be wrong to add additional parts with the same name and
> different id's? It seems to me duplicate names would break
> spi_nor_scan when passed a name but work fine for scanning by id. 


Ideally yes, but in case of cy15x104q there are two revisions of flash,
one with correct ID code and other with ID code reversed and
unfortunately, would need to make an exception here.

> I also think that '{ "cy15x104q",  INFO6(0x042cc2, 0x7f7f7f, 512 * 1024,
> 1, SPI_NOR_NO_ERASE) },' is way too specific for 'cy15x104q' because
> it is won't match any of the other identified parts:
> 
> CY15B104Q-PZXI     7F7F7F7F7F7FC22C03 51-85075 8-pin PDIP Industrial
> CY15B104QN-50SXA   7F7F7F7F7F7FC22C40 001-85261 8-pin SOIC (EIAJ) Automotive
> CY15V104QN-50SXI   7F7F7F7F7F7FC22C04 002-18131 8-pin GQFN Industrial
> CY15B104QN-20LPXC  7F7F7F7F7F7FC22CA1 002-18131 8-pin GQFN Commercial
> CY15B104QN-20LPXI  7F7F7F7F7F7FC22C01 002-18131 8-pin GQFN Industrial
> CY15V104QN-20LPXC  7F7F7F7F7F7FC22CA5 002-18131 8-pin GQFN Commercial
> CY15V104QN-20LPXI  7F7F7F7F7F7FC22C05 002-18131 8-pin GQFN Industrial
> CY15B104QN-50LPXI  7F7F7F7F7F7FC22C00 002-18131 8-pin GQFN Industrial
> 

Indeed.. A patch fixing the same would be very much appreciated. Thanks!

Regards
Vignesh

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