[PATCH v1] mtd: spi-nor: Add support for XM25QH64C

[PATCH v1] mtd: spi-nor: Add support for XM25QH64C

[Reto Schneider]

From: Reto Schneider <reto.schneider@husqvarnagroup.com>

The data sheets can be found here:
http://xmcwh.com/Uploads/2020-12-17/XM25QH64C_Ver1.1.pdf

This chip has been (briefly) tested on the MediaTek MT7688 based GARDENA
smart gateway.

Signed-off-by: Reto Schneider <reto.schneider@husqvarnagroup.com>

---

 drivers/mtd/spi-nor/xmc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mtd/spi-nor/xmc.c b/drivers/mtd/spi-nor/xmc.c
index 2c7773b68993..b6ac37bd59b8 100644
--- a/drivers/mtd/spi-nor/xmc.c
+++ b/drivers/mtd/spi-nor/xmc.c
@@ -12,6 +12,8 @@ static const struct flash_info xmc_parts[] = {
 	/* XMC (Wuhan Xinxin Semiconductor Manufacturing Corp.) */
 	{ "XM25QH64A", INFO(0x207017, 0, 64 * 1024, 128,
 			    SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
+	{ "XM25QH64C", INFO(0x204017, 0, 64 * 1024, 128,
+			    SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 	{ "XM25QH128A", INFO(0x207018, 0, 64 * 1024, 256,
 			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 };
-- 
2.30.2

Re: [PATCH v1] mtd: spi-nor: Add support for XM25QH64C

[Michael Walle]

Hi Reto,

Am 2021-06-13 14:12, schrieb Reto Schneider:
> From: Reto Schneider <reto.schneider@husqvarnagroup.com>
> 
> The data sheets can be found here:
> http://xmcwh.com/Uploads/2020-12-17/XM25QH64C_Ver1.1.pdf

Could you add that as a "Datasheet:" tag before your Sob tag?

> This chip has been (briefly) tested on the MediaTek MT7688 based 
> GARDENA
> smart gateway.

Could you also apply my SFDP patch [1] and send the dump? Unfortunately,
I can't think of a good way to do that along with the patch and if this
in some way regarded as copyrighted material. So feel free to send it to
me privately. I'm starting to build a database.

> Signed-off-by: Reto Schneider <reto.schneider@husqvarnagroup.com>

With that fixed:
Reviewed-by: Michael Walle <michael@walle.cc>

NB. XMC ignores the continuation codes and this particular device will
collide with M25PE64/M45PE64. Although I couldn't find any datasheet,
so I don't know if these devices actually exist.

-michael

[1] https://patchwork.ozlabs.org/project/linux-mtd/list/?series=241877

> 
> ---
> 
>  drivers/mtd/spi-nor/xmc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/xmc.c b/drivers/mtd/spi-nor/xmc.c
> index 2c7773b68993..b6ac37bd59b8 100644
> --- a/drivers/mtd/spi-nor/xmc.c
> +++ b/drivers/mtd/spi-nor/xmc.c
> @@ -12,6 +12,8 @@ static const struct flash_info xmc_parts[] = {
>  	/* XMC (Wuhan Xinxin Semiconductor Manufacturing Corp.) */
>  	{ "XM25QH64A", INFO(0x207017, 0, 64 * 1024, 128,
>  			    SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> +	{ "XM25QH64C", INFO(0x204017, 0, 64 * 1024, 128,
> +			    SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>  	{ "XM25QH128A", INFO(0x207018, 0, 64 * 1024, 256,
>  			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>  };

Re: [PATCH v1] mtd: spi-nor: Add support for XM25QH64C

[Reto Schneider]

Hi Michael,

Thanks for your feedback.

On 14.06.21 08:56, Michael Walle wrote:
> Could you add that as a "Datasheet:" tag before your Sob tag?

Will do for v2.

>> This chip has been (briefly) tested on the MediaTek MT7688 based GARDENA
>> smart gateway.
> 
> Could you also apply my SFDP patch [1] and send the dump? Unfortunately,
> I can't think of a good way to do that along with the patch and if this
> in some way regarded as copyrighted material. So feel free to send it to
> me privately. I'm starting to build a database.

Will do. If interested, I could also include the predecessor (XM25QH64A).

> NB. XMC ignores the continuation codes and this particular device will
> collide with M25PE64/M45PE64. Although I couldn't find any datasheet,
> so I don't know if these devices actually exist.

M25PE64 yields quite some hits on Google. Is supporting this (all?) XMC 
device in upstream a good idea nevertheless? Does "first come, first 
served" apply here?

Kind regards,
Reto

Re: [PATCH v1] mtd: spi-nor: Add support for XM25QH64C

[Michael Walle]

Hi Reto,

Am 2021-06-17 13:59, schrieb Reto Schneider:
>> NB. XMC ignores the continuation codes and this particular device will
>> collide with M25PE64/M45PE64. Although I couldn't find any datasheet,
>> so I don't know if these devices actually exist.
> 
> M25PE64 yields quite some hits on Google. Is supporting this (all?)
> XMC device in upstream a good idea nevertheless? Does "first come,
> first served" apply here?

That is up to the maintainers, but sooner or later we will have to face
the problem regarding the duplicate IDs.

-michael

Re: [PATCH v1] mtd: spi-nor: Add support for XM25QH64C

[Reto Schneider]

Hi all,

On 17.06.21 16:19, Michael Walle wrote:
> That is up to the maintainers, but sooner or later we will have to face
> the problem regarding the duplicate IDs.

A strongly worded statement by the maintainer(s) ("the Linux crowd") 
against ignoring the continuation codes might help (me) to put pressure 
on non-compliant manufacturers.

Any chance something like this is already out there? Could not find 
anything.

Kind regards,
Reto

Re: [PATCH v1] mtd: spi-nor: Add support for XM25QH64C

[Tudor.Ambarus]

On 6/14/21 9:56 AM, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Reto,
> 
> Am 2021-06-13 14:12, schrieb Reto Schneider:
>> From: Reto Schneider <reto.schneider@husqvarnagroup.com>
>>
>> The data sheets can be found here:
>> http://xmcwh.com/Uploads/2020-12-17/XM25QH64C_Ver1.1.pdf
> 
> Could you add that as a "Datasheet:" tag before your Sob tag?
> 
>> This chip has been (briefly) tested on the MediaTek MT7688 based
>> GARDENA
>> smart gateway.
> 
> Could you also apply my SFDP patch [1] and send the dump? Unfortunately,
> I can't think of a good way to do that along with the patch and if this
> in some way regarded as copyrighted material. So feel free to send it to
> me privately. I'm starting to build a database.
>

Can the SFDP dump fit in the commit message when introducing a new flash ID?
The SFDP standard is public. SFDP reveals just what the flash supports, why would
that be sensitive information? Reto, would you please dump the SFDP tables here?

Re: [PATCH v1] mtd: spi-nor: Add support for XM25QH64C

[Tudor.Ambarus]

On 6/17/21 6:06 PM, Reto Schneider wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi all,
> 
> On 17.06.21 16:19, Michael Walle wrote:
>> That is up to the maintainers, but sooner or later we will have to face
>> the problem regarding the duplicate IDs.
> 
> A strongly worded statement by the maintainer(s) ("the Linux crowd")
> against ignoring the continuation codes might help (me) to put pressure
> on non-compliant manufacturers.
> 

We saw few ID collisions even between flashes of the same manufacturer, macronix.
Reusing flash IDs without providing an extended ID field, or ignoring the
continuation codes is bad design, and I would avoid using such flash. But I won't
reject new flash additions solely because they have a wrong identification mechanism.
We'll cast an indirect public shame on the collisions by writing hackish code when
trying to differentiate between the flashes in software.

Re: [PATCH v1] mtd: spi-nor: Add support for XM25QH64C

[Tudor.Ambarus]

Hi, Reto,

On 6/14/21 9:56 AM, Michael Walle wrote:
> 
> NB. XMC ignores the continuation codes and this particular device will
> collide with M25PE64/M45PE64. Although I couldn't find any datasheet,
> so I don't know if these devices actually exist.

Would you please dump more bytes of ID when reading it? Maybe they put the
manufacturer continuation codes, after the manufacturer ID and the flash ID.
Or maybe they defined an extended ID, let's see what's there.

Also, does this flash has a vendor specific table? Check the JEDEC Standard No. 216D.01,
section "6.3.3 Definition of Parameter ID Field". Parameter IDS with "An MSB of 01h though
7Fh indicates a Vendor Owned table and provides the bank number of a JEDEC JEP106 assigned
Manufacturer ID." We may get the bank number from there.

Cheers,
ta

Re: [PATCH v1] mtd: spi-nor: Add support for XM25QH64C

[Michael Walle]

Am 2021-06-28 06:55, schrieb Tudor.Ambarus@microchip.com:
> On 6/14/21 9:56 AM, Michael Walle wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
>> the content is safe
>> 
>> Hi Reto,
>> 
>> Am 2021-06-13 14:12, schrieb Reto Schneider:
>>> From: Reto Schneider <reto.schneider@husqvarnagroup.com>
>>> 
>>> The data sheets can be found here:
>>> http://xmcwh.com/Uploads/2020-12-17/XM25QH64C_Ver1.1.pdf
>> 
>> Could you add that as a "Datasheet:" tag before your Sob tag?
>> 
>>> This chip has been (briefly) tested on the MediaTek MT7688 based
>>> GARDENA
>>> smart gateway.
>> 
>> Could you also apply my SFDP patch [1] and send the dump? 
>> Unfortunately,
>> I can't think of a good way to do that along with the patch and if 
>> this
>> in some way regarded as copyrighted material. So feel free to send it 
>> to
>> me privately. I'm starting to build a database.
>> 
> 
> Can the SFDP dump fit in the commit message when introducing a new 
> flash ID?

As ASCII hex dump? I'd guess we need some instructions how to do
that. 4k would be 256 lines with 16 byte per line.

But yes, I had the same thought.

> The SFDP standard is public. SFDP reveals just what the flash
> supports, why would that be sensitive information?

copyright material must not be sensitive information, no? I'm no
laywer, thus I'd played it safe.

> Reto, would you please dump the SFDP tables here?

FWIW, he already provided me a binary dump for the flash (actually
for the former version, too) and its already in my DB which you
should have access, too.

If we mandate that (and I am all for it) for new flashes, we will
have to come up with a common format. Something like

  echo "-----BEGIN SFDP (JEDEC ID ...)------"
  cat sfdp | xxd -ps -c 20
  echo "-----END SFDP------"

I really don't know if such meta data belongs into the
commit logs, though.

-michael

Re: [PATCH v1] mtd: spi-nor: Add support for XM25QH64C

[Tudor.Ambarus]

On 6/30/21 5:15 PM, Michael Walle wrote:
>>
>> Can the SFDP dump fit in the commit message when introducing a new
>> flash ID?
> 
> As ASCII hex dump? I'd guess we need some instructions how to do
> that. 4k would be 256 lines with 16 byte per line.
> 
> But yes, I had the same thought.

How about asking for the SFDP data in the cover later each time a
new flash addition is proposed?

Re: [PATCH v1] mtd: spi-nor: Add support for XM25QH64C

[Tudor.Ambarus]

On 6/30/21 9:28 PM, Tudor.Ambarus@microchip.com wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 6/30/21 5:15 PM, Michael Walle wrote:
>>>
>>> Can the SFDP dump fit in the commit message when introducing a new
>>> flash ID?
>>
>> As ASCII hex dump? I'd guess we need some instructions how to do
>> that. 4k would be 256 lines with 16 byte per line.
>>
>> But yes, I had the same thought.
> 
> How about asking for the SFDP data in the cover later each time a

s/later/letter

> new flash addition is proposed?
Vendors shouldn't complain if we publish the SFDP data, since it can be freely read
from their flash products. I'll update the documentation and add this requirement for
new flash additions.

Re: [PATCH v1] mtd: spi-nor: Add support for XM25QH64C

[Michael Walle]

Am 2021-06-30 20:28, schrieb Tudor.Ambarus@microchip.com:
> On 6/30/21 5:15 PM, Michael Walle wrote:
>>> 
>>> Can the SFDP dump fit in the commit message when introducing a new
>>> flash ID?
>> 
>> As ASCII hex dump? I'd guess we need some instructions how to do
>> that. 4k would be 256 lines with 16 byte per line.
>> 
>> But yes, I had the same thought.
> 
> How about asking for the SFDP data in the cover later each time a
> new flash addition is proposed?

Ahh, I like the idea to put it onto the ML only. What do you think
of putting it into the comment section of the patch (ie. the one
after "---". This way we can have a backreference with a Link:
tag of the patch.

-michael