[PATCH] mtd: spi-nor: read 6 bytes for the ID

[Huang Shijie]

On Tue, Apr 15, 2014 at 03:35:05PM +0200, Marek Vasut wrote:
> On Tuesday, April 15, 2014 at 07:22:39 AM, Huang Shijie wrote:
> > On Mon, Apr 14, 2014 at 08:23:47PM +0200, Marek Vasut wrote:
> 
> [...]
> 
> > > > > I wonder if the ID-bytes wraparound cannot cause us trouble here. For
> > > > > example if we try to detect a SPI NOR which has 5-byte ID code, but
> > > > > in the table, we'd also have a SPI NOR with has a 6-byte code where
> > > > > the last byte of ext-jedec matches the first byte of JEDEC ID , this
> > > > > would actually match on the later.
> > > > 
> > > > could you give me detail example?
> > > > 
> > > > I feel sorry that i do not quit understand your meaning.
> > > 
> > > Imagine two chips with two IDs:
> > > Chip 1 has IDs: 0xf00b42 0x4242f0 and readID[6] returns 0x420bf0f04242
> > 
> > It will not return 0x420bf0f04242.
> > 
> > The readID[6] should be: f0, 0b, 42, 42, 42, f0.
> > 
> > > Chip 2 has IDs: 0xf00b42   0x42f0 and readID[6] returns 0x420bf0f04242
> > 
> > the readID[6] should be: f0, 0b, 42, 42, f0, XX.
> > 
> >  "XX" stands for the sixth byte.
> > 
> > The current patch can distinguish these two chips.
> > 
> > > This is because in the second chips' case the ID wraps around at 5 bytes.
> > > But chip #1 matches the ID, so if chip #1 is earlier in the list of SPI
> > > NOR flashes, we will get an incorrect detection of that chip.
> > 
> > I guess your meaning is that the chip 2 has IDs: 0xf00b42   0x4242
> > and the sixth byte is 0xf0 which wraps the first byte.
> 
> Huang, what I meant is that if you read 6 bytes of ID from a chip which wraps 
> the READID command output on 5 bytes AND the first and last byte match in the 
> table for some 6-byte chip, then this 6-byte chip will be used as a 
> configuration for the different 5-byte chip.
what are you saying is that we meet the same 6 bytes for two different NOR flashes.


> 
> This code should be future-proof, but if we keep adding such special cases, we 
> will end up with false matches sooner or later anyway I'm afraid.
> 
> What do you say we add the READID length field into the table ?

could you tell us how do you implement the "add the READID length field .."?

IMHO, even we add the readid length field, we also can not avoid the case you
mentioned.

hi Brian & Clark, what's your opinion about this?

thanks
Huang Shijie