| Message ID | 1415257333-6488-1-git-send-email-zajec5@gmail.com |
|---|---|
| State | Rejected |
| Headers | show |
On Thu, Nov 06, 2014 at 08:02:13AM +0100, Rafał Miłecki wrote: > With two entries sharing first ID bytes but one of them having more of > them, we should prefer the more specific one. Consider following: > 1) n25q128a11 with ID 0x20bb18 > 2) n25q128a13 with ID 0x20bb181234 > we should prefer entry with the longer ID. I perfer to make a limit before we add new item to the table: Put the longest one in the first place. By this way, we can avoid the too much "for"s in the following code which makes the code a little complicated. So It should looks like this in the table. 1) n25q128a13 with ID 0x20bb181234 2) n25q128a11 with ID 0x20bb18 Hi Brian, what's your opinion? thanks Huang Shijie > > Signed-off-by: Rafał Miłecki <zajec5@gmail.com> > Cc: Bean Huo <beanhuo@micron.com> > Cc: Huang Shijie <shijie.huang@intel.com> > --- > V2: Rebase on top of Huang patches > --- > drivers/mtd/spi-nor/spi-nor.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c > index 209e701..80e333f 100644 > --- a/drivers/mtd/spi-nor/spi-nor.c > +++ b/drivers/mtd/spi-nor/spi-nor.c > @@ -667,6 +667,7 @@ static const struct spi_device_id *spi_nor_read_id(struct spi_nor *nor) > int tmp; > u8 id[SPI_NOR_MAX_ID_LEN]; > struct flash_info *info; > + u8 len; > > tmp = nor->read_reg(nor, SPINOR_OP_RDID, id, SPI_NOR_MAX_ID_LEN); > if (tmp < 0) { > @@ -674,10 +675,15 @@ static const struct spi_device_id *spi_nor_read_id(struct spi_nor *nor) > return ERR_PTR(tmp); > } > > - for (tmp = 0; tmp < ARRAY_SIZE(spi_nor_ids) - 1; tmp++) { > - info = (void *)spi_nor_ids[tmp].driver_data; > - if (info->id_len) { > - if (!memcmp(info->id, id, info->id_len)) > + /* > + * Start looking for entries with the longest ID, then go to the less > + * specific ones. > + */ > + for (len = SPI_NOR_MAX_ID_LEN; len > 0; len--) { > + for (tmp = 0; tmp < ARRAY_SIZE(spi_nor_ids) - 1; tmp++) { > + info = (void *)spi_nor_ids[tmp].driver_data; > + if (info->id_len == len && > + !memcmp(info->id, id, info->id_len)) > return &spi_nor_ids[tmp]; > } > } > -- > 1.8.4.5 >
>> 2) n25q128a13 with ID 0x20bb181234 >> we should prefer entry with the longer ID. >I perfer to make a limit before we add new item to the table: > Put the longest one in the first place. >By this way, we can avoid the too much "for"s in the following code which makes the code a little complicated. >So It should looks like this in the table. > 1) n25q128a13 with ID 0x20bb181234 > 2) n25q128a11 with ID 0x20bb18 >Hi Brian, what's your opinion? >thanks >Huang Shijie In general,spi nor include three parts Id infor, First,it is Manufacturer ID, size is 1 byte. Second,it is Device ID,and size is 2 bytes,this sector will include memory type and capacity information. Third,it is unique ID,and size is 17 bytes(now only used 2 bytes in linux),first byte will indicate length of data to follow,second byte is real Extended id,and very important.Next 14 bytes is optional,it can be customized factory data. Now in our linux,former two parts all be used for Deivce Id ,ant its size is 3 bytes.third part(extended id)be used 2 bytes,and called extended id. In our Micron spi nor serials,there are some spi nor that the same Deivce id with different extended ID,so in order to distinguish this,you only add extended id infor in struct spi_device_id spi_nor_ids,and don't let relevant spi nor Extended ID field is NULL.just as Spansion: { "s25sl032p", INFO(0x010215, 0x4d00, 64 * 1024, 64, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, { "s25sl064p", INFO(0x010216, 0x4d00, 64 * 1024, 128, 0) }, { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, 0) }, BeanHuo
On Fri, Nov 07, 2014 at 01:49:36AM +0000, Bean Huo 霍斌斌 (beanhuo) wrote: > >> 2) n25q128a13 with ID 0x20bb181234 > >> we should prefer entry with the longer ID. > >I perfer to make a limit before we add new item to the table: > > Put the longest one in the first place. > > >By this way, we can avoid the too much "for"s in the following code which makes the code a little complicated. > > >So It should looks like this in the table. > > 1) n25q128a13 with ID 0x20bb181234 > > 2) n25q128a11 with ID 0x20bb18 > > >Hi Brian, what's your opinion? > > >thanks > >Huang Shijie > > In general,spi nor include three parts Id infor, > > First,it is Manufacturer ID, size is 1 byte. > > Second,it is Device ID,and size is 2 bytes,this sector will include memory > type and capacity information. > > Third,it is unique ID,and size is 17 bytes(now only used 2 bytes in linux),first byte > will indicate length of data to follow,second byte is real Extended id,and > very important.Next 14 bytes is optional,it can be customized factory data. > > Now in our linux,former two parts all be used for Deivce Id ,ant its size > is 3 bytes.third part(extended id)be used 2 bytes,and called extended id. > > In our Micron spi nor serials,there are some spi nor that the same Deivce id > with different extended ID,so in order to distinguish this,you only add extended id > infor in struct spi_device_id spi_nor_ids,and don't let relevant spi nor Extended ID field > is NULL.just as Spansion: > > { "s25sl032p", INFO(0x010215, 0x4d00, 64 * 1024, 64, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, > { "s25sl064p", INFO(0x010216, 0x4d00, 64 * 1024, 128, 0) }, > { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, 0) }, Sometimes, five bytes ID is not enough, please read the patch: http://lists.infradead.org/pipermail/linux-mtd/2014-August/055018.html In current table, Micron's chips do not have five bytes ID yet. But Spansion has already had many five bytes ID chips now. thanks Huang Shijie
>Sometimes, five bytes ID is not enough, please read the patch: > http://lists.infradead.org/pipermail/linux-mtd/2014-August/055018.html >In current table, Micron's chips do not have five bytes ID yet. >But Spansion has already had many five bytes ID chips now. Yes,we now intend to add it,and submit a patch about it . BeanHuo
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index 209e701..80e333f 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -667,6 +667,7 @@ static const struct spi_device_id *spi_nor_read_id(struct spi_nor *nor) int tmp; u8 id[SPI_NOR_MAX_ID_LEN]; struct flash_info *info; + u8 len; tmp = nor->read_reg(nor, SPINOR_OP_RDID, id, SPI_NOR_MAX_ID_LEN); if (tmp < 0) { @@ -674,10 +675,15 @@ static const struct spi_device_id *spi_nor_read_id(struct spi_nor *nor) return ERR_PTR(tmp); } - for (tmp = 0; tmp < ARRAY_SIZE(spi_nor_ids) - 1; tmp++) { - info = (void *)spi_nor_ids[tmp].driver_data; - if (info->id_len) { - if (!memcmp(info->id, id, info->id_len)) + /* + * Start looking for entries with the longest ID, then go to the less + * specific ones. + */ + for (len = SPI_NOR_MAX_ID_LEN; len > 0; len--) { + for (tmp = 0; tmp < ARRAY_SIZE(spi_nor_ids) - 1; tmp++) { + info = (void *)spi_nor_ids[tmp].driver_data; + if (info->id_len == len && + !memcmp(info->id, id, info->id_len)) return &spi_nor_ids[tmp]; } }
With two entries sharing first ID bytes but one of them having more of them, we should prefer the more specific one. Consider following: 1) n25q128a11 with ID 0x20bb18 2) n25q128a13 with ID 0x20bb181234 we should prefer entry with the longer ID. Signed-off-by: Rafał Miłecki <zajec5@gmail.com> Cc: Bean Huo <beanhuo@micron.com> Cc: Huang Shijie <shijie.huang@intel.com> --- V2: Rebase on top of Huang patches --- drivers/mtd/spi-nor/spi-nor.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)