| Message ID | 20210727045222.905056-7-tudor.ambarus@microchip.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mtd: spi-nor: Handle ID collisions and clean params init | expand |
On Tue, Jul 27, 2021 at 07:51:53AM +0300, Tudor Ambarus wrote: > Flash does not support continuation codes and may collide with a flash > of other manufacturer, Intersil being an example . > > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> > --- > 0000000 4653 5044 0100 ff01 0000 0901 0030 ff00 > 0000010 000b 0301 0060 ff00 ffff ffff ffff ffff > 0000020 ffff ffff ffff ffff ffff ffff ffff ffff > 0000030 20e5 fff1 ffff 07ff eb44 6b08 3b08 bb42 > 0000040 ffee ffff ffff ff00 ffff ff00 200c 520f > 0000050 d810 ff00 ffff ffff ffff ffff ffff ffff > 0000060 3600 2700 f99f 6477 e8d9 ffff > > drivers/mtd/spi-nor/manuf-id-collisions.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/mtd/spi-nor/manuf-id-collisions.c b/drivers/mtd/spi-nor/manuf-id-collisions.c > index bf7dba34f018..db31470ebf6a 100644 > --- a/drivers/mtd/spi-nor/manuf-id-collisions.c > +++ b/drivers/mtd/spi-nor/manuf-id-collisions.c > @@ -13,6 +13,10 @@ static const struct flash_info id_collision_parts[] = { > { "by25q128as", INFO(0x684018, 0, 64 * 1024, 256, SPI_NOR_SKIP_SFDP | > SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | > SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) }, > + > + /* XTX (XTX Technology Limited) */ > + { "xt25f128b", INFO(0x0b4018, 0, 64 * 1024, 256, SPI_NOR_PARSE_SFDP | > + SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) }, My apologies for being ignorant of this, but I'm not 100% sure of these two values (SPI_NOR_HAS_LOCK and SPI_NOR_HAS_TB), even though I included them in my original commit. Looking at the datasheet for this I can see that there are 5 block protect bits (BP0 - BP4) corresponding to status registers SR2 through SR6. Status register bits SR7 and SR8 correspond to "status register protect 0 and status register protect 1" bits as well. The Rockchip engineer I was testing the SFC with did not have these flags as well on their driver they were using for this chip too. I have tested with and without, and they seem to work regardless. Is there a way to know for sure if these should or should not be here? Here is a link to the datasheet I was working off of: https://datasheet.lcsc.com/szlcsc/2005251034_XTX-XT25F128BSSIGT_C558844.pdf When this is confirmed I'll be glad to provide my "Tested-by" line. Thank you. > }; > > const struct spi_nor_manufacturer spi_nor_manuf_id_collisions = { > -- > 2.25.1 >
On 7/27/21 6:52 PM, Chris Morgan wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On Tue, Jul 27, 2021 at 07:51:53AM +0300, Tudor Ambarus wrote: >> Flash does not support continuation codes and may collide with a flash >> of other manufacturer, Intersil being an example . >> >> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> >> --- >> 0000000 4653 5044 0100 ff01 0000 0901 0030 ff00 >> 0000010 000b 0301 0060 ff00 ffff ffff ffff ffff >> 0000020 ffff ffff ffff ffff ffff ffff ffff ffff >> 0000030 20e5 fff1 ffff 07ff eb44 6b08 3b08 bb42 >> 0000040 ffee ffff ffff ff00 ffff ff00 200c 520f >> 0000050 d810 ff00 ffff ffff ffff ffff ffff ffff >> 0000060 3600 2700 f99f 6477 e8d9 ffff >> >> drivers/mtd/spi-nor/manuf-id-collisions.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/mtd/spi-nor/manuf-id-collisions.c b/drivers/mtd/spi-nor/manuf-id-collisions.c >> index bf7dba34f018..db31470ebf6a 100644 >> --- a/drivers/mtd/spi-nor/manuf-id-collisions.c >> +++ b/drivers/mtd/spi-nor/manuf-id-collisions.c >> @@ -13,6 +13,10 @@ static const struct flash_info id_collision_parts[] = { >> { "by25q128as", INFO(0x684018, 0, 64 * 1024, 256, SPI_NOR_SKIP_SFDP | >> SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | >> SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) }, >> + >> + /* XTX (XTX Technology Limited) */ >> + { "xt25f128b", INFO(0x0b4018, 0, 64 * 1024, 256, SPI_NOR_PARSE_SFDP | >> + SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) }, > > My apologies for being ignorant of this, but I'm not 100% sure of these > two values (SPI_NOR_HAS_LOCK and SPI_NOR_HAS_TB), even though I Oh yes, I forgot you have already mentioned this in the previous email thread. > included them in my original commit. Looking at the datasheet for this > I can see that there are 5 block protect bits (BP0 - BP4) corresponding > to status registers SR2 through SR6. Status register bits SR7 and SR8 > correspond to "status register protect 0 and status register protect 1" > bits as well. The Rockchip engineer I was testing the SFC with did > not have these flags as well on their driver they were using for this > chip too. I have tested with and without, and they seem to work > regardless. Is there a way to know for sure if these should or should > not be here? We should do some locking tests to verify if these flags are ok for this flash. > > Here is a link to the datasheet I was working off of: > https://datasheet.lcsc.com/szlcsc/2005251034_XTX-XT25F128BSSIGT_C558844.pdf > I'll take a look and try to provide some guidelines on how the locking part can be tested. > When this is confirmed I'll be glad to provide my "Tested-by" line. Cool, thanks! ta > > Thank you. > >> }; >> >> const struct spi_nor_manufacturer spi_nor_manuf_id_collisions = { >> -- >> 2.25.1 >>
On 27/07/21 07:51AM, Tudor Ambarus wrote: > Flash does not support continuation codes and may collide with a flash > of other manufacturer, Intersil being an example . Nitpick: ^ drop the extra space > > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> > --- > 0000000 4653 5044 0100 ff01 0000 0901 0030 ff00 > 0000010 000b 0301 0060 ff00 ffff ffff ffff ffff > 0000020 ffff ffff ffff ffff ffff ffff ffff ffff > 0000030 20e5 fff1 ffff 07ff eb44 6b08 3b08 bb42 > 0000040 ffee ffff ffff ff00 ffff ff00 200c 520f > 0000050 d810 ff00 ffff ffff ffff ffff ffff ffff > 0000060 3600 2700 f99f 6477 e8d9 ffff I think you should list all the other sysfs parameters as well. See Michael's comments on patch 35. > > drivers/mtd/spi-nor/manuf-id-collisions.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/mtd/spi-nor/manuf-id-collisions.c b/drivers/mtd/spi-nor/manuf-id-collisions.c > index bf7dba34f018..db31470ebf6a 100644 > --- a/drivers/mtd/spi-nor/manuf-id-collisions.c > +++ b/drivers/mtd/spi-nor/manuf-id-collisions.c > @@ -13,6 +13,10 @@ static const struct flash_info id_collision_parts[] = { > { "by25q128as", INFO(0x684018, 0, 64 * 1024, 256, SPI_NOR_SKIP_SFDP | > SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | > SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) }, > + > + /* XTX (XTX Technology Limited) */ > + { "xt25f128b", INFO(0x0b4018, 0, 64 * 1024, 256, SPI_NOR_PARSE_SFDP | > + SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) }, I have not went and checked, but I assume these two flags can't be discovered via SFDP? > }; > > const struct spi_nor_manufacturer spi_nor_manuf_id_collisions = { > -- > 2.25.1 >
On 8/16/21 9:43 PM, Pratyush Yadav wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 27/07/21 07:51AM, Tudor Ambarus wrote: >> Flash does not support continuation codes and may collide with a flash >> of other manufacturer, Intersil being an example . > Nitpick: ^ drop the extra space thanks > >> >> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> >> --- >> 0000000 4653 5044 0100 ff01 0000 0901 0030 ff00 >> 0000010 000b 0301 0060 ff00 ffff ffff ffff ffff >> 0000020 ffff ffff ffff ffff ffff ffff ffff ffff >> 0000030 20e5 fff1 ffff 07ff eb44 6b08 3b08 bb42 >> 0000040 ffee ffff ffff ff00 ffff ff00 200c 520f >> 0000050 d810 ff00 ffff ffff ffff ffff ffff ffff >> 0000060 3600 2700 f99f 6477 e8d9 ffff > > I think you should list all the other sysfs parameters as well. See > Michael's comments on patch 35. ok > >> >> drivers/mtd/spi-nor/manuf-id-collisions.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/mtd/spi-nor/manuf-id-collisions.c b/drivers/mtd/spi-nor/manuf-id-collisions.c >> index bf7dba34f018..db31470ebf6a 100644 >> --- a/drivers/mtd/spi-nor/manuf-id-collisions.c >> +++ b/drivers/mtd/spi-nor/manuf-id-collisions.c >> @@ -13,6 +13,10 @@ static const struct flash_info id_collision_parts[] = { >> { "by25q128as", INFO(0x684018, 0, 64 * 1024, 256, SPI_NOR_SKIP_SFDP | >> SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | >> SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) }, >> + >> + /* XTX (XTX Technology Limited) */ >> + { "xt25f128b", INFO(0x0b4018, 0, 64 * 1024, 256, SPI_NOR_PARSE_SFDP | >> + SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) }, > > I have not went and checked, but I assume these two flags can't be > discovered via SFDP? Locking is not covered by SFDP yet. The non SFDP discoverable flags will be parsed in a spi_nor_nonsfdp_flags_init() method. I saw you've spotted a bug there, will handle it. Cheers, ta > >> }; >> >> const struct spi_nor_manufacturer spi_nor_manuf_id_collisions = { >> -- >> 2.25.1 >> > > -- > Regards, > Pratyush Yadav > Texas Instruments Inc. >
diff --git a/drivers/mtd/spi-nor/manuf-id-collisions.c b/drivers/mtd/spi-nor/manuf-id-collisions.c index bf7dba34f018..db31470ebf6a 100644 --- a/drivers/mtd/spi-nor/manuf-id-collisions.c +++ b/drivers/mtd/spi-nor/manuf-id-collisions.c @@ -13,6 +13,10 @@ static const struct flash_info id_collision_parts[] = { { "by25q128as", INFO(0x684018, 0, 64 * 1024, 256, SPI_NOR_SKIP_SFDP | SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) }, + + /* XTX (XTX Technology Limited) */ + { "xt25f128b", INFO(0x0b4018, 0, 64 * 1024, 256, SPI_NOR_PARSE_SFDP | + SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) }, }; const struct spi_nor_manufacturer spi_nor_manuf_id_collisions = {
Flash does not support continuation codes and may collide with a flash of other manufacturer, Intersil being an example . Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> --- 0000000 4653 5044 0100 ff01 0000 0901 0030 ff00 0000010 000b 0301 0060 ff00 ffff ffff ffff ffff 0000020 ffff ffff ffff ffff ffff ffff ffff ffff 0000030 20e5 fff1 ffff 07ff eb44 6b08 3b08 bb42 0000040 ffee ffff ffff ff00 ffff ff00 200c 520f 0000050 d810 ff00 ffff ffff ffff ffff ffff ffff 0000060 3600 2700 f99f 6477 e8d9 ffff drivers/mtd/spi-nor/manuf-id-collisions.c | 4 ++++ 1 file changed, 4 insertions(+)