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

[Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>]

On Wed, Apr 15, 2020 at 10:58:36PM +0530, Vignesh Raghavendra wrote:
> 
> 
> On 09/03/20 2:10 pm, Sascha Hauer wrote:
> > The Cypress cy15b104q and cy15v104q are 4Mbit serial SPI F-RAM devices.
> > Add support for them to the spi-nor driver.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
> > ---
> > 
> > Changes since v1:
> > - Instead of specifying 1024 sectors with a sector size of 512 specify
> >   512 * 1024 sectos with a sector size of 1. The device has no idea of
> >   sectors and is not erasable, so a sector size of 1 seems to better
> >   reflect reality.
> > 
> >  drivers/mtd/spi-nor/spi-nor.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> > index 4fc632ec18fe..a5c1d684364c 100644
> > --- a/drivers/mtd/spi-nor/spi-nor.c
> > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > @@ -2335,6 +2335,9 @@ static const struct flash_info spi_nor_ids[] = {
> >  
> >  	{ "at45db081d", INFO(0x1f2500, 0, 64 * 1024, 16, SECT_4K) },
> >  
> > +	/* Cypress */
> > +	{ "cy15x104q",  INFO6(0x042cc2, 0x7f7f7f, 512 * 1024, 1, SPI_NOR_NO_ERASE) },
> 
> This seems wrong
> 
> As the datasheet for the device says:
> 
> "Device ID
> The CY15B104Q device can be interrogated for its manufacturer, product
> identification, and die revision. The RDID opcode 9Fh allows
> the user to read the manufacturer ID and product ID, both of which are
> read-only bytes. The JEDEC-assigned manufacturer ID places
> the Cypress (Ramtron) identifier in bank 7; therefore, there are six
> bytes of the continuation code 7Fh followed by the single byte C2h.
> There are two bytes of product ID, which includes a family code, a
> density code, a sub code, and the product revision code."
> 
> I am not sure how are you reading 0x4 as the first byte. It should have
> been 6 bytes of 0x7F followed by 0xc2 right?
> 
> Also 0x7F is continuation code and not actual device ID (See JEDEC
> standard JEP106). 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

I digged a bit deeper.

According to the datasheet the data sent as response to the read id
command is:

7f 7f 7f 7f 7f 7f c2 2c 04

What I read from the device instead is:

04 2c c2 7f 7f 7f 7f 7f 7f

You see the order of the values is reversed. And in fact this matches
the datasheet. Read on in the paragraph you cited from:

| Note: The least significant data byte (Byte 0) shifts out first and
| the most significant data byte (Byte 8) shifts out last.

This sounds very buggy to me. The number of continuation codes (0x7f)
says which bank in jep106 I have to look at. I increased
SPI_NOR_MAX_ID_LEN to see what the device sends after the ID above and
the device happily sends more 0x7f continuation codes. How should I
count the number of continuation codes when the device sends any desired
number of them at the end of the message?

I don't know what's going on here, to me it looks like they accidently
mixed the order of the bytes and instead of fixing it a note was added
in the datasheet. I hope it's different and you tell me what's wrong in
my understanding ;)

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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