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

[Marek Vasut <marex@denx.de>]

On Monday, April 14, 2014 at 04:44:01 PM, Huang Shijie wrote:
> On Mon, Apr 14, 2014 at 01:53:07PM +0200, Marek Vasut wrote:
> > > @@ -614,8 +616,23 @@ static const struct spi_device_id
> > > *spi_nor_read_id(struct spi_nor *nor) for (tmp = 0; tmp <
> > > ARRAY_SIZE(spi_nor_ids) - 1; tmp++) {
> > > 
> > >  		info = (void *)spi_nor_ids[tmp].driver_data;
> > >  		if (info->jedec_id == jedec) {
> > > 
> > > -			if (info->ext_id == 0 || info->ext_id == ext_jedec)
> > > +			if (info->ext_id == 0)
> > > 
> > >  				return &spi_nor_ids[tmp];
> > > 
> > > +
> > > +			/* the legacy two bytes ext_id */
> > > +			if ((info->ext_id >> 16) == 0) {
> > > +				if (info->ext_id == ext_jedec)
> > > +					matched = tmp;
> > > +			} else {
> > > +				/* check the sixth byte now */
> > > +				ext_jedec = ext_jedec << 8 | id[5];
> > > +				if (info->ext_id == ext_jedec)
> > > +					return &spi_nor_ids[tmp];
> > > +			}
> > > +		} else {
> > > +			/* shortcut */
> > > +			if (matched != -1)
> > > +				return &spi_nor_ids[matched];
> > 
> > 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
Chip 2 has IDs: 0xf00b42   0x42f0 and readID[6] returns 0x420bf0f04242
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.

Does it make sense now please ?

Best regards,
Marek Vasut