Re: [PATCH v2 06/35] mtd: spi-nor: manuf-id-collisions: Add support for xt25f128b

From: <Tudor.Ambarus@microchip.com>
To: <macromorgan@hotmail.com>
Cc: <michael@walle.cc>, <vigneshr@ti.com>, <p.yadav@ti.com>,
	<figgyc@figgyc.uk>, <mail@david-bauer.net>,
	<linux@rasmusvillemoes.dk>, <esben@geanix.com>,
	<knaerzche@gmail.com>, <code@reto-schneider.ch>,
	<zhengxunli@mxic.com.tw>, <jaimeliao@mxic.com.tw>,
	<heiko.thiery@gmail.com>, <sr@denx.de>,
	<miquel.raynal@bootlin.com>, <richard@nod.at>,
	<linux-mtd@lists.infradead.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<Nicolas.Ferre@microchip.com>
Subject: Re: [PATCH v2 06/35] mtd: spi-nor: manuf-id-collisions: Add support for xt25f128b
Date: Wed, 28 Jul 2021 04:10:59 +0000	[thread overview]
Message-ID: <6cfcc22e-5f78-59a6-9781-613be62f3178@microchip.com> (raw)
In-Reply-To: <SN6PR06MB534253FF244275974E04E2EBA5E99@SN6PR06MB5342.namprd06.prod.outlook.com>

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
>>

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