Re: [RFC PATCH 13/34] mtd: spi-nor: Add the concept of SPI NOR manufacturer driver

[Boris Brezillon <boris.brezillon@xxxxxxxxxxx>]

Hi Vignesh,

On Mon, 10 Dec 2018 13:50:03 +0530
Vignesh R <vigneshr@xxxxxx> wrote:

> >  static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
> >  {
> >  	int			tmp;
> > @@ -1734,13 +1751,21 @@ static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
> >  		return ERR_PTR(tmp);
> >  	}
> >  
> > -	for (tmp = 0; tmp < ARRAY_SIZE(spi_nor_ids) - 1; tmp++) {
> > -		info = &spi_nor_ids[tmp];
> > -		if (info->id_len) {
> > -			if (!memcmp(info->id, id, info->id_len))
> > -				return &spi_nor_ids[tmp];
> > +	for (tmp = 0; tmp < ARRAY_SIZE(manufacturers); tmp++) {
> > +		info = spi_nor_search_part_by_id(manufacturers[tmp]->parts,
> > +						 manufacturers[tmp]->nparts,
> > +						 id);  
> 
> This could probably be optimized a bit by comparing manufacturer ID and
> then looking through parts list of that particular manufacturer only.
> (with expection of winbond manufacturer ID, where will have to go
> through spansion parts list as well)

That's what I did at first, but it doesn't work. Some manufacturer IDs
are prefixed by one or several continuation codes (0x7F). That was done
to extend the 256 IDs space (see spec JEP106AX). We could have the
manufacturer ID encoded with an array of byte instead of a single byte,
but there's more: some flashes that are supposed to used continuation
codes, just output the last (non-continuation) byte (see the ISSI IDs
here [1] if you need a proof ;-)). So we're screwed.

Regards,

Boris

[1]https://elixir.bootlin.com/linux/v4.20-rc6/source/drivers/mtd/spi-nor/spi-nor.c#L1351

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