[PATCH] mtd: spi-nor: Add support for BoHong bh25q128as

[PATCH] mtd: spi-nor: Add support for BoHong bh25q128as

[David Bauer]

Add MTD support for the BoHong bh25q128as SPI NOR chip.
The chip has 16MB of total capacity, divided into a total of 256
sectors, each 64KB sized. The chip also supports 4KB sectors.
Additionally, it supports dual and quad read modes.

Functionality was verified on an Tenbay WR1800K / MTK MT7621 board.

Signed-off-by: David Bauer <mail@david-bauer.net>
---
 drivers/mtd/spi-nor/Makefile |  1 +
 drivers/mtd/spi-nor/bohong.c | 21 +++++++++++++++++++++
 drivers/mtd/spi-nor/core.c   |  1 +
 drivers/mtd/spi-nor/core.h   |  1 +
 4 files changed, 24 insertions(+)
 create mode 100644 drivers/mtd/spi-nor/bohong.c

diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
index 653923896205..de0b2d3bcb1c 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -2,6 +2,7 @@
 
 spi-nor-objs			:= core.o sfdp.o
 spi-nor-objs			+= atmel.o
+spi-nor-objs			+= bohong.o
 spi-nor-objs			+= catalyst.o
 spi-nor-objs			+= eon.o
 spi-nor-objs			+= esmt.o
diff --git a/drivers/mtd/spi-nor/bohong.c b/drivers/mtd/spi-nor/bohong.c
new file mode 100644
index 000000000000..20aeceb1b2d1
--- /dev/null
+++ b/drivers/mtd/spi-nor/bohong.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2005, Intec Automation Inc.
+ * Copyright (C) 2014, Freescale Semiconductor, Inc.
+ */
+
+#include <linux/mtd/spi-nor.h>
+
+#include "core.h"
+
+static const struct flash_info bohong_parts[] = {
+	/* BoHong Microelectronics */
+	{ "bh25q128as", INFO(0x684018, 0, 64 * 1024, 256,
+			    SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
+};
+
+const struct spi_nor_manufacturer spi_nor_bohong = {
+	.name = "bohong",
+	.parts = bohong_parts,
+	.nparts = ARRAY_SIZE(bohong_parts),
+};
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 0522304f52fa..03a05bce6231 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2199,6 +2199,7 @@ int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor)
 
 static const struct spi_nor_manufacturer *manufacturers[] = {
 	&spi_nor_atmel,
+	&spi_nor_bohong,
 	&spi_nor_catalyst,
 	&spi_nor_eon,
 	&spi_nor_esmt,
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 4a3f7f150b5d..b71323317235 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -409,6 +409,7 @@ struct spi_nor_manufacturer {
 
 /* Manufacturer drivers. */
 extern const struct spi_nor_manufacturer spi_nor_atmel;
+extern const struct spi_nor_manufacturer spi_nor_bohong;
 extern const struct spi_nor_manufacturer spi_nor_catalyst;
 extern const struct spi_nor_manufacturer spi_nor_eon;
 extern const struct spi_nor_manufacturer spi_nor_esmt;
-- 
2.31.1


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

Re: [PATCH] mtd: spi-nor: Add support for BoHong bh25q128as

[Michael Walle]

Hi David,

Am 2021-05-09 16:47, schrieb David Bauer:
> Add MTD support for the BoHong bh25q128as SPI NOR chip.
> The chip has 16MB of total capacity, divided into a total of 256
> sectors, each 64KB sized. The chip also supports 4KB sectors.
> Additionally, it supports dual and quad read modes.
> 
> Functionality was verified on an Tenbay WR1800K / MTK MT7621 board.
> 
> Signed-off-by: David Bauer <mail@david-bauer.net>
> ---
>  drivers/mtd/spi-nor/Makefile |  1 +
>  drivers/mtd/spi-nor/bohong.c | 21 +++++++++++++++++++++
>  drivers/mtd/spi-nor/core.c   |  1 +
>  drivers/mtd/spi-nor/core.h   |  1 +
>  4 files changed, 24 insertions(+)
>  create mode 100644 drivers/mtd/spi-nor/bohong.c
> 
> diff --git a/drivers/mtd/spi-nor/Makefile 
> b/drivers/mtd/spi-nor/Makefile
> index 653923896205..de0b2d3bcb1c 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -2,6 +2,7 @@
> 
>  spi-nor-objs			:= core.o sfdp.o
>  spi-nor-objs			+= atmel.o
> +spi-nor-objs			+= bohong.o
>  spi-nor-objs			+= catalyst.o
>  spi-nor-objs			+= eon.o
>  spi-nor-objs			+= esmt.o
> diff --git a/drivers/mtd/spi-nor/bohong.c 
> b/drivers/mtd/spi-nor/bohong.c
> new file mode 100644
> index 000000000000..20aeceb1b2d1
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/bohong.c
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2005, Intec Automation Inc.
> + * Copyright (C) 2014, Freescale Semiconductor, Inc.

I guess this could be dropped. There is no much left from these in this 
file.

> + */
> +
> +#include <linux/mtd/spi-nor.h>
> +
> +#include "core.h"
> +
> +static const struct flash_info bohong_parts[] = {
> +	/* BoHong Microelectronics */
> +	{ "bh25q128as", INFO(0x684018, 0, 64 * 1024, 256,

I couldn't find "BoHong" in JEP106BC. 0x68 (without continuation codes)
is "Convex Computer". So this is wrong. OTOH I'm not sure, how many
SPI flashes "convex computer" have, if any ;) This company was brought
by HP in the end.

In any case, this patch depends on how we handle continuation codes or
if we can handle them at all. Or if this flash just lie about its
manufacturer id and don't and CC.

See also:
https://lkml.org/lkml/2021/2/7/223
https://www.spinics.net/lists/kernel/msg3808260.html

> +			    SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> +};
> +
> +const struct spi_nor_manufacturer spi_nor_bohong = {
> +	.name = "bohong",
> +	.parts = bohong_parts,
> +	.nparts = ARRAY_SIZE(bohong_parts),
> +};
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 0522304f52fa..03a05bce6231 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2199,6 +2199,7 @@ int spi_nor_sr2_bit7_quad_enable(struct spi_nor 
> *nor)
> 
>  static const struct spi_nor_manufacturer *manufacturers[] = {
>  	&spi_nor_atmel,
> +	&spi_nor_bohong,
>  	&spi_nor_catalyst,
>  	&spi_nor_eon,
>  	&spi_nor_esmt,
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 4a3f7f150b5d..b71323317235 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -409,6 +409,7 @@ struct spi_nor_manufacturer {
> 
>  /* Manufacturer drivers. */
>  extern const struct spi_nor_manufacturer spi_nor_atmel;
> +extern const struct spi_nor_manufacturer spi_nor_bohong;
>  extern const struct spi_nor_manufacturer spi_nor_catalyst;
>  extern const struct spi_nor_manufacturer spi_nor_eon;
>  extern const struct spi_nor_manufacturer spi_nor_esmt;

-- 
-michael

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

Re: [PATCH] mtd: spi-nor: Add support for BoHong bh25q128as

[David Bauer]

Hi Michael,

thanks for your review.

On 5/10/21 10:00 AM, Michael Walle wrote

[...]

>> +static const struct flash_info bohong_parts[] = {
>> +    /* BoHong Microelectronics */
>> +    { "bh25q128as", INFO(0x684018, 0, 64 * 1024, 256,
> 
> I couldn't find "BoHong" in JEP106BC. 0x68 (without continuation codes)
> is "Convex Computer". So this is wrong. OTOH I'm not sure, how many
> SPI flashes "convex computer" have, if any ;) This company was brought
> by HP in the end.
> 
> In any case, this patch depends on how we handle continuation codes or
> if we can handle them at all. Or if this flash just lie about its
> manufacturer id and don't and CC.

First of all, BoHong and Boya microelectronics seems to be the same company, as their
datasheets seem to copy each other. There's not much information about either of both,
so I'd say that's a fair assumption.

Regarding the continuation codes, Boya is listed in bank nine, however in this case I
should currently read an all 0x7f ID shouldn't I? The datasheet also only specifies 3
bytes as a return value for register 0x9fh :(

Best
David

> 
> See also:
> https://lkml.org/lkml/2021/2/7/223
> https://www.spinics.net/lists/kernel/msg3808260.html
> 
>> +                SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>> +};
>> +
>> +const struct spi_nor_manufacturer spi_nor_bohong = {
>> +    .name = "bohong",
>> +    .parts = bohong_parts,
>> +    .nparts = ARRAY_SIZE(bohong_parts),
>> +};
>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>> index 0522304f52fa..03a05bce6231 100644
>> --- a/drivers/mtd/spi-nor/core.c
>> +++ b/drivers/mtd/spi-nor/core.c
>> @@ -2199,6 +2199,7 @@ int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor)
>>
>>  static const struct spi_nor_manufacturer *manufacturers[] = {
>>      &spi_nor_atmel,
>> +    &spi_nor_bohong,
>>      &spi_nor_catalyst,
>>      &spi_nor_eon,
>>      &spi_nor_esmt,
>> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
>> index 4a3f7f150b5d..b71323317235 100644
>> --- a/drivers/mtd/spi-nor/core.h
>> +++ b/drivers/mtd/spi-nor/core.h
>> @@ -409,6 +409,7 @@ struct spi_nor_manufacturer {
>>
>>  /* Manufacturer drivers. */
>>  extern const struct spi_nor_manufacturer spi_nor_atmel;
>> +extern const struct spi_nor_manufacturer spi_nor_bohong;
>>  extern const struct spi_nor_manufacturer spi_nor_catalyst;
>>  extern const struct spi_nor_manufacturer spi_nor_eon;
>>  extern const struct spi_nor_manufacturer spi_nor_esmt;
> 

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

Re: [PATCH] mtd: spi-nor: Add support for BoHong bh25q128as

[Michael Walle]

Hi David,

Am 2021-05-10 11:28, schrieb David Bauer:
> On 5/10/21 10:00 AM, Michael Walle wrote
> 
> [...]
> 
>>> +static const struct flash_info bohong_parts[] = {
>>> +    /* BoHong Microelectronics */
>>> +    { "bh25q128as", INFO(0x684018, 0, 64 * 1024, 256,
>> 
>> I couldn't find "BoHong" in JEP106BC. 0x68 (without continuation 
>> codes)
>> is "Convex Computer". So this is wrong. OTOH I'm not sure, how many
>> SPI flashes "convex computer" have, if any ;) This company was brought
>> by HP in the end.
>> 
>> In any case, this patch depends on how we handle continuation codes or
>> if we can handle them at all. Or if this flash just lie about its
>> manufacturer id and don't and CC.
> 
> First of all, BoHong and Boya microelectronics seems to be the same
> company, as their datasheets seem to copy each other. There's not much
> information about either of both, so I'd say that's a fair assumption.
> 
> Regarding the continuation codes, Boya is listed in bank nine, however
> in this case I should currently read an all 0x7f ID shouldn't I?

I'd guess so, yes.

> The datasheet also only specifies 3 bytes as a return value for
> register 0x9fh :(

Yeah. So, this flash falls into the same category "simply hijacks
a manuf id" as all the other flashes.

We still need to come up with a solution for this problem.

-michael

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

Re: [PATCH] mtd: spi-nor: Add support for BoHong bh25q128as

[David Bauer]

Hi Michael,

On 5/10/21 11:35 AM, Michael Walle wrote:
> Hi David,
> 
> Am 2021-05-10 11:28, schrieb David Bauer:
>> On 5/10/21 10:00 AM, Michael Walle wrote
>>
>> [...]
>>
>>>> +static const struct flash_info bohong_parts[] = {
>>>> +    /* BoHong Microelectronics */
>>>> +    { "bh25q128as", INFO(0x684018, 0, 64 * 1024, 256,
>>>
>>> I couldn't find "BoHong" in JEP106BC. 0x68 (without continuation codes)
>>> is "Convex Computer". So this is wrong. OTOH I'm not sure, how many
>>> SPI flashes "convex computer" have, if any ;) This company was brought
>>> by HP in the end.
>>>
>>> In any case, this patch depends on how we handle continuation codes or
>>> if we can handle them at all. Or if this flash just lie about its
>>> manufacturer id and don't and CC.
>>
>> First of all, BoHong and Boya microelectronics seems to be the same
>> company, as their datasheets seem to copy each other. There's not much
>> information about either of both, so I'd say that's a fair assumption.
>>
>> Regarding the continuation codes, Boya is listed in bank nine, however
>> in this case I should currently read an all 0x7f ID shouldn't I?
> 
> I'd guess so, yes.
> 
>> The datasheet also only specifies 3 bytes as a return value for
>> register 0x9fh :(
> 
> Yeah. So, this flash falls into the same category "simply hijacks
> a manuf id" as all the other flashes.

From a quick check, this is also be the case for GigaDevices and XMC.

My spontaneous idea would be to extend support for JEDEC IDs to read
the up to 9 banks of the vendor ID and fix up the existing offenders.

To not break existing boards, we could either skip the continuation
bytes of the kernel ID definitions for all flash chips or flag the
already existing ones and only perform this on such flagged chips.

Personally, I'd say that only performing this on existing chips would
be better, as new vendors with this violation scheme might probably
appear and cause conflicts.

As we still lack auto detection for new chips with that, configuring
the flash chip used with the chip name via DT would allow to set the
exact chip used and also validate if the manufacturer / product after
the continuation bits matches the one read from the chip.

What do you think?

Best
David

> 
> We still need to come up with a solution for this problem.
> 
> -michael

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

Re: [PATCH] mtd: spi-nor: Add support for BoHong bh25q128as

[Michael Walle]

Am 2021-05-10 12:27, schrieb David Bauer:
> On 5/10/21 11:35 AM, Michael Walle wrote:
>> Am 2021-05-10 11:28, schrieb David Bauer:
>>> On 5/10/21 10:00 AM, Michael Walle wrote
>>> 
>>> [...]
>>> 
>>>>> +static const struct flash_info bohong_parts[] = {
>>>>> +    /* BoHong Microelectronics */
>>>>> +    { "bh25q128as", INFO(0x684018, 0, 64 * 1024, 256,
>>>> 
>>>> I couldn't find "BoHong" in JEP106BC. 0x68 (without continuation 
>>>> codes)
>>>> is "Convex Computer". So this is wrong. OTOH I'm not sure, how many
>>>> SPI flashes "convex computer" have, if any ;) This company was 
>>>> brought
>>>> by HP in the end.
>>>> 
>>>> In any case, this patch depends on how we handle continuation codes 
>>>> or
>>>> if we can handle them at all. Or if this flash just lie about its
>>>> manufacturer id and don't and CC.
>>> 
>>> First of all, BoHong and Boya microelectronics seems to be the same
>>> company, as their datasheets seem to copy each other. There's not 
>>> much
>>> information about either of both, so I'd say that's a fair 
>>> assumption.
>>> 
>>> Regarding the continuation codes, Boya is listed in bank nine, 
>>> however
>>> in this case I should currently read an all 0x7f ID shouldn't I?
>> 
>> I'd guess so, yes.
>> 
>>> The datasheet also only specifies 3 bytes as a return value for
>>> register 0x9fh :(
>> 
>> Yeah. So, this flash falls into the same category "simply hijacks
>> a manuf id" as all the other flashes.
> 
> From a quick check, this is also be the case for GigaDevices and XMC.
> 
> My spontaneous idea would be to extend support for JEDEC IDs to read
> the up to 9 banks of the vendor ID and fix up the existing offenders.

you mean gigadevices and xmc? I'd presume they are also lacking the
continuation bytes.

> To not break existing boards, we could either skip the continuation
> bytes of the kernel ID definitions for all flash chips or flag the
> already existing ones and only perform this on such flagged chips.
> 
> Personally, I'd say that only performing this on existing chips would
> be better, as new vendors with this violation scheme might probably
> appear and cause conflicts.
> 
> As we still lack auto detection for new chips with that, configuring
> the flash chip used with the chip name via DT would allow to set the
> exact chip used and also validate if the manufacturer / product after
> the continuation bits matches the one read from the chip.
> 
> What do you think?

If you'd ask me, unless there is a real world conflict, I'd just go
ahead and add them as is. If there is a conflict we'd need to find
a per device resolution for it.

There is another problem: shared device ids per vendor. Some (most?)
flash vendor share device ids on "similar" flashes, which we still
need tell apart in the kernel. So technically, this is the same problem
as with non-existing continuation bytes. Two different flashes sharing
the same id. For now, we look for differences in the SFDP.

Right now, the flash is probed first by its id and the the SFDP is
read and parsed. There are ideas to first read the SFDP. Having this
might come in handy here, too. Eg. we could fingerprint the flash
by its SFDP.

I know Vingesh is working on the continuation bytes stuff. So I might
be late to the party ;)

Regarding the device tree compatibles, the maintainers doesn't like
that very much. Maybe as a last resort method.

-michael

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

Re: [PATCH] mtd: spi-nor: Add support for BoHong bh25q128as

[David Bauer]

Hi Michael,

On 5/10/21 12:56 PM, Michael Walle wrote:
> Am 2021-05-10 12:27, schrieb David Bauer:
>> On 5/10/21 11:35 AM, Michael Walle wrote:
>>> Am 2021-05-10 11:28, schrieb David Bauer:
>>>> On 5/10/21 10:00 AM, Michael Walle wrote
>>>>
>>>> [...]
>>>>
>>>>>> +static const struct flash_info bohong_parts[] = {
>>>>>> +    /* BoHong Microelectronics */
>>>>>> +    { "bh25q128as", INFO(0x684018, 0, 64 * 1024, 256,
>>>>>
>>>>> I couldn't find "BoHong" in JEP106BC. 0x68 (without continuation codes)
>>>>> is "Convex Computer". So this is wrong. OTOH I'm not sure, how many
>>>>> SPI flashes "convex computer" have, if any ;) This company was brought
>>>>> by HP in the end.
>>>>>
>>>>> In any case, this patch depends on how we handle continuation codes or
>>>>> if we can handle them at all. Or if this flash just lie about its
>>>>> manufacturer id and don't and CC.
>>>>
>>>> First of all, BoHong and Boya microelectronics seems to be the same
>>>> company, as their datasheets seem to copy each other. There's not much
>>>> information about either of both, so I'd say that's a fair assumption.
>>>>
>>>> Regarding the continuation codes, Boya is listed in bank nine, however
>>>> in this case I should currently read an all 0x7f ID shouldn't I?
>>>
>>> I'd guess so, yes.
>>>
>>>> The datasheet also only specifies 3 bytes as a return value for
>>>> register 0x9fh :(
>>>
>>> Yeah. So, this flash falls into the same category "simply hijacks
>>> a manuf id" as all the other flashes.
>>
>> From a quick check, this is also be the case for GigaDevices and XMC.
>>
>> My spontaneous idea would be to extend support for JEDEC IDs to read
>> the up to 9 banks of the vendor ID and fix up the existing offenders.
> 
> you mean gigadevices and xmc? I'd presume they are also lacking the
> continuation bytes.

Correct, same story with them.

> 
>> To not break existing boards, we could either skip the continuation
>> bytes of the kernel ID definitions for all flash chips or flag the
>> already existing ones and only perform this on such flagged chips.
>>
>> Personally, I'd say that only performing this on existing chips would
>> be better, as new vendors with this violation scheme might probably
>> appear and cause conflicts.
>>
>> As we still lack auto detection for new chips with that, configuring
>> the flash chip used with the chip name via DT would allow to set the
>> exact chip used and also validate if the manufacturer / product after
>> the continuation bits matches the one read from the chip.
>>
>> What do you think?
> 
> If you'd ask me, unless there is a real world conflict, I'd just go
> ahead and add them as is. If there is a conflict we'd need to find
> a per device resolution for it.

Okay, I'll resend a v2 with the removed copyright then.

> 
> There is another problem: shared device ids per vendor. Some (most?)
> flash vendor share device ids on "similar" flashes, which we still
> need tell apart in the kernel. So technically, this is the same problem
> as with non-existing continuation bytes. Two different flashes sharing
> the same id. For now, we look for differences in the SFDP.
> 
> Right now, the flash is probed first by its id and the the SFDP is
> read and parsed. There are ideas to first read the SFDP. Having this
> might come in handy here, too. Eg. we could fingerprint the flash
> by its SFDP.

I was also thinking about that and we're also already bitten by identical
JEDEC IDs for different models. It's really a pity that there is no real
unique model identifier for us to use, which is not hacked to support
legacy implementations like with this chip :(

Best
David

> 
> I know Vingesh is working on the continuation bytes stuff. So I might
> be late to the party ;)
> 
> Regarding the device tree compatibles, the maintainers doesn't like
> that very much. Maybe as a last resort method.
> 
> -michael

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

Re: [PATCH] mtd: spi-nor: Add support for BoHong bh25q128as

[Michael Walle]

Hi David,

Am 2021-05-10 13:04, schrieb David Bauer:
> On 5/10/21 12:56 PM, Michael Walle wrote:
>> Am 2021-05-10 12:27, schrieb David Bauer:
>>> On 5/10/21 11:35 AM, Michael Walle wrote:
>>>> Am 2021-05-10 11:28, schrieb David Bauer:
>>>>> On 5/10/21 10:00 AM, Michael Walle wrote
>>>>> 
>>>>> [...]
>>>>> 
>>>>>>> +static const struct flash_info bohong_parts[] = {
>>>>>>> +    /* BoHong Microelectronics */
>>>>>>> +    { "bh25q128as", INFO(0x684018, 0, 64 * 1024, 256,
>>>>>> 
>>>>>> I couldn't find "BoHong" in JEP106BC. 0x68 (without continuation 
>>>>>> codes)
>>>>>> is "Convex Computer". So this is wrong. OTOH I'm not sure, how 
>>>>>> many
>>>>>> SPI flashes "convex computer" have, if any ;) This company was 
>>>>>> brought
>>>>>> by HP in the end.
>>>>>> 
>>>>>> In any case, this patch depends on how we handle continuation 
>>>>>> codes or
>>>>>> if we can handle them at all. Or if this flash just lie about its
>>>>>> manufacturer id and don't and CC.
>>>>> 
>>>>> First of all, BoHong and Boya microelectronics seems to be the same
>>>>> company, as their datasheets seem to copy each other. There's not 
>>>>> much
>>>>> information about either of both, so I'd say that's a fair 
>>>>> assumption.
>>>>> 
>>>>> Regarding the continuation codes, Boya is listed in bank nine, 
>>>>> however
>>>>> in this case I should currently read an all 0x7f ID shouldn't I?
>>>> 
>>>> I'd guess so, yes.
>>>> 
>>>>> The datasheet also only specifies 3 bytes as a return value for
>>>>> register 0x9fh :(
>>>> 
>>>> Yeah. So, this flash falls into the same category "simply hijacks
>>>> a manuf id" as all the other flashes.
>>> 
>>> From a quick check, this is also be the case for GigaDevices and XMC.
>>> 
>>> My spontaneous idea would be to extend support for JEDEC IDs to read
>>> the up to 9 banks of the vendor ID and fix up the existing offenders.
>> 
>> you mean gigadevices and xmc? I'd presume they are also lacking the
>> continuation bytes.
> 
> Correct, same story with them.
> 
>> 
>>> To not break existing boards, we could either skip the continuation
>>> bytes of the kernel ID definitions for all flash chips or flag the
>>> already existing ones and only perform this on such flagged chips.
>>> 
>>> Personally, I'd say that only performing this on existing chips would
>>> be better, as new vendors with this violation scheme might probably
>>> appear and cause conflicts.
>>> 
>>> As we still lack auto detection for new chips with that, configuring
>>> the flash chip used with the chip name via DT would allow to set the
>>> exact chip used and also validate if the manufacturer / product after
>>> the continuation bits matches the one read from the chip.
>>> 
>>> What do you think?
>> 
>> If you'd ask me, unless there is a real world conflict, I'd just go
>> ahead and add them as is. If there is a conflict we'd need to find
>> a per device resolution for it.
> 
> Okay, I'll resend a v2 with the removed copyright then.

Could you also apply my SFDP patch [1] and send the dump (if there
is any)? 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.

>> There is another problem: shared device ids per vendor. Some (most?)
>> flash vendor share device ids on "similar" flashes, which we still
>> need tell apart in the kernel. So technically, this is the same 
>> problem
>> as with non-existing continuation bytes. Two different flashes sharing
>> the same id. For now, we look for differences in the SFDP.
>> 
>> Right now, the flash is probed first by its id and the the SFDP is
>> read and parsed. There are ideas to first read the SFDP. Having this
>> might come in handy here, too. Eg. we could fingerprint the flash
>> by its SFDP.
> 
> I was also thinking about that and we're also already bitten by 
> identical
> JEDEC IDs for different models. It's really a pity that there is no 
> real
> unique model identifier for us to use, which is not hacked to support
> legacy implementations like with this chip :(

Unfortunately, they aren't legacy and new chips still have this
behavior..

-michael

[1] 
https://lore.kernel.org/linux-mtd/20210503155651.30889-1-michael@walle.cc/

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

Re: [PATCH] mtd: spi-nor: Add support for BoHong bh25q128as

[David Bauer]

Hi Michael,

Sorry for the late reply, was not feeling well past week.

On 5/10/21 1:22 PM, Michael Walle wrote:
> Hi David,
> 
> Am 2021-05-10 13:04, schrieb David Bauer:
>> On 5/10/21 12:56 PM, Michael Walle wrote:
>>> Am 2021-05-10 12:27, schrieb David Bauer:
>>>> On 5/10/21 11:35 AM, Michael Walle wrote:
>>>>> Am 2021-05-10 11:28, schrieb David Bauer:
>>>>>> On 5/10/21 10:00 AM, Michael Walle wrote
>>>>>>
>>>>>> [...]
>>>>>>
>>>>>>>> +static const struct flash_info bohong_parts[] = {
>>>>>>>> +    /* BoHong Microelectronics */
>>>>>>>> +    { "bh25q128as", INFO(0x684018, 0, 64 * 1024, 256,
>>>>>>>
>>>>>>> I couldn't find "BoHong" in JEP106BC. 0x68 (without continuation codes)
>>>>>>> is "Convex Computer". So this is wrong. OTOH I'm not sure, how many
>>>>>>> SPI flashes "convex computer" have, if any ;) This company was brought
>>>>>>> by HP in the end.
>>>>>>>
>>>>>>> In any case, this patch depends on how we handle continuation codes or
>>>>>>> if we can handle them at all. Or if this flash just lie about its
>>>>>>> manufacturer id and don't and CC.
>>>>>>
>>>>>> First of all, BoHong and Boya microelectronics seems to be the same
>>>>>> company, as their datasheets seem to copy each other. There's not much
>>>>>> information about either of both, so I'd say that's a fair assumption.
>>>>>>
>>>>>> Regarding the continuation codes, Boya is listed in bank nine, however
>>>>>> in this case I should currently read an all 0x7f ID shouldn't I?
>>>>>
>>>>> I'd guess so, yes.
>>>>>
>>>>>> The datasheet also only specifies 3 bytes as a return value for
>>>>>> register 0x9fh :(
>>>>>
>>>>> Yeah. So, this flash falls into the same category "simply hijacks
>>>>> a manuf id" as all the other flashes.
>>>>
>>>> From a quick check, this is also be the case for GigaDevices and XMC.
>>>>
>>>> My spontaneous idea would be to extend support for JEDEC IDs to read
>>>> the up to 9 banks of the vendor ID and fix up the existing offenders.
>>>
>>> you mean gigadevices and xmc? I'd presume they are also lacking the
>>> continuation bytes.
>>
>> Correct, same story with them.
>>
>>>
>>>> To not break existing boards, we could either skip the continuation
>>>> bytes of the kernel ID definitions for all flash chips or flag the
>>>> already existing ones and only perform this on such flagged chips.
>>>>
>>>> Personally, I'd say that only performing this on existing chips would
>>>> be better, as new vendors with this violation scheme might probably
>>>> appear and cause conflicts.
>>>>
>>>> As we still lack auto detection for new chips with that, configuring
>>>> the flash chip used with the chip name via DT would allow to set the
>>>> exact chip used and also validate if the manufacturer / product after
>>>> the continuation bits matches the one read from the chip.
>>>>
>>>> What do you think?
>>>
>>> If you'd ask me, unless there is a real world conflict, I'd just go
>>> ahead and add them as is. If there is a conflict we'd need to find
>>> a per device resolution for it.
>>
>> Okay, I'll resend a v2 with the removed copyright then.
> 
> Could you also apply my SFDP patch [1] and send the dump (if there
> is any)? 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.

Bad news, I'm not able to get a SFDP with your patches, as the SFDP extraction
fails at the version check.

Is there anything else I can try?

Best
David

> 
>>> There is another problem: shared device ids per vendor. Some (most?)
>>> flash vendor share device ids on "similar" flashes, which we still
>>> need tell apart in the kernel. So technically, this is the same problem
>>> as with non-existing continuation bytes. Two different flashes sharing
>>> the same id. For now, we look for differences in the SFDP.
>>>
>>> Right now, the flash is probed first by its id and the the SFDP is
>>> read and parsed. There are ideas to first read the SFDP. Having this
>>> might come in handy here, too. Eg. we could fingerprint the flash
>>> by its SFDP.
>>
>> I was also thinking about that and we're also already bitten by identical
>> JEDEC IDs for different models. It's really a pity that there is no real
>> unique model identifier for us to use, which is not hacked to support
>> legacy implementations like with this chip :(
> 
> Unfortunately, they aren't legacy and new chips still have this
> behavior..
> 
> -michael
> 
> [1] https://lore.kernel.org/linux-mtd/20210503155651.30889-1-michael@walle.cc/

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

Re: [PATCH] mtd: spi-nor: Add support for BoHong bh25q128as

[Tudor.Ambarus]

On 5/18/21 10:39 PM, David Bauer wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Michael,
> 
> Sorry for the late reply, was not feeling well past week.
> 
> On 5/10/21 1:22 PM, Michael Walle wrote:
>> Hi David,
>>
>> Am 2021-05-10 13:04, schrieb David Bauer:
>>> On 5/10/21 12:56 PM, Michael Walle wrote:
>>>> Am 2021-05-10 12:27, schrieb David Bauer:
>>>>> On 5/10/21 11:35 AM, Michael Walle wrote:
>>>>>> Am 2021-05-10 11:28, schrieb David Bauer:
>>>>>>> On 5/10/21 10:00 AM, Michael Walle wrote
>>>>>>>
>>>>>>> [...]
>>>>>>>
>>>>>>>>> +static const struct flash_info bohong_parts[] = {
>>>>>>>>> +    /* BoHong Microelectronics */
>>>>>>>>> +    { "bh25q128as", INFO(0x684018, 0, 64 * 1024, 256,
>>>>>>>>
>>>>>>>> I couldn't find "BoHong" in JEP106BC. 0x68 (without continuation codes)
>>>>>>>> is "Convex Computer". So this is wrong. OTOH I'm not sure, how many
>>>>>>>> SPI flashes "convex computer" have, if any ;) This company was brought
>>>>>>>> by HP in the end.
>>>>>>>>
>>>>>>>> In any case, this patch depends on how we handle continuation codes or
>>>>>>>> if we can handle them at all. Or if this flash just lie about its
>>>>>>>> manufacturer id and don't and CC.
>>>>>>>
>>>>>>> First of all, BoHong and Boya microelectronics seems to be the same
>>>>>>> company, as their datasheets seem to copy each other. There's not much
>>>>>>> information about either of both, so I'd say that's a fair assumption.
>>>>>>>
>>>>>>> Regarding the continuation codes, Boya is listed in bank nine, however
>>>>>>> in this case I should currently read an all 0x7f ID shouldn't I?
>>>>>>
>>>>>> I'd guess so, yes.
>>>>>>
>>>>>>> The datasheet also only specifies 3 bytes as a return value for
>>>>>>> register 0x9fh :(
>>>>>>
>>>>>> Yeah. So, this flash falls into the same category "simply hijacks
>>>>>> a manuf id" as all the other flashes.
>>>>>
>>>>> From a quick check, this is also be the case for GigaDevices and XMC.
>>>>>
>>>>> My spontaneous idea would be to extend support for JEDEC IDs to read
>>>>> the up to 9 banks of the vendor ID and fix up the existing offenders.
>>>>
>>>> you mean gigadevices and xmc? I'd presume they are also lacking the
>>>> continuation bytes.
>>>
>>> Correct, same story with them.
>>>
>>>>
>>>>> To not break existing boards, we could either skip the continuation
>>>>> bytes of the kernel ID definitions for all flash chips or flag the
>>>>> already existing ones and only perform this on such flagged chips.
>>>>>
>>>>> Personally, I'd say that only performing this on existing chips would
>>>>> be better, as new vendors with this violation scheme might probably
>>>>> appear and cause conflicts.
>>>>>
>>>>> As we still lack auto detection for new chips with that, configuring
>>>>> the flash chip used with the chip name via DT would allow to set the
>>>>> exact chip used and also validate if the manufacturer / product after
>>>>> the continuation bits matches the one read from the chip.
>>>>>
>>>>> What do you think?
>>>>
>>>> If you'd ask me, unless there is a real world conflict, I'd just go
>>>> ahead and add them as is. If there is a conflict we'd need to find
>>>> a per device resolution for it.
>>>
>>> Okay, I'll resend a v2 with the removed copyright then.
>>
>> Could you also apply my SFDP patch [1] and send the dump (if there
>> is any)? 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.
> 
> Bad news, I'm not able to get a SFDP with your patches, as the SFDP extraction
> fails at the version check.
> 
> Is there anything else I can try?
> 

So no SFDP data?
Have you tried to read more of ID bytes, maybe there's an extended ID? Please
dump 15 bytes of ID.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: Add support for BoHong bh25q128as

[Tudor.Ambarus]

On 6/28/21 8:48 AM, Tudor.Ambarus@microchip.com wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 5/18/21 10:39 PM, David Bauer wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> Hi Michael,
>>
>> Sorry for the late reply, was not feeling well past week.
>>
>> On 5/10/21 1:22 PM, Michael Walle wrote:
>>> Hi David,
>>>
>>> Am 2021-05-10 13:04, schrieb David Bauer:
>>>> On 5/10/21 12:56 PM, Michael Walle wrote:
>>>>> Am 2021-05-10 12:27, schrieb David Bauer:
>>>>>> On 5/10/21 11:35 AM, Michael Walle wrote:
>>>>>>> Am 2021-05-10 11:28, schrieb David Bauer:
>>>>>>>> On 5/10/21 10:00 AM, Michael Walle wrote
>>>>>>>>
>>>>>>>> [...]
>>>>>>>>
>>>>>>>>>> +static const struct flash_info bohong_parts[] = {
>>>>>>>>>> +    /* BoHong Microelectronics */
>>>>>>>>>> +    { "bh25q128as", INFO(0x684018, 0, 64 * 1024, 256,
>>>>>>>>>
>>>>>>>>> I couldn't find "BoHong" in JEP106BC. 0x68 (without continuation codes)
>>>>>>>>> is "Convex Computer". So this is wrong. OTOH I'm not sure, how many
>>>>>>>>> SPI flashes "convex computer" have, if any ;) This company was brought
>>>>>>>>> by HP in the end.
>>>>>>>>>
>>>>>>>>> In any case, this patch depends on how we handle continuation codes or
>>>>>>>>> if we can handle them at all. Or if this flash just lie about its
>>>>>>>>> manufacturer id and don't and CC.
>>>>>>>>
>>>>>>>> First of all, BoHong and Boya microelectronics seems to be the same
>>>>>>>> company, as their datasheets seem to copy each other. There's not much
>>>>>>>> information about either of both, so I'd say that's a fair assumption.
>>>>>>>>
>>>>>>>> Regarding the continuation codes, Boya is listed in bank nine, however
>>>>>>>> in this case I should currently read an all 0x7f ID shouldn't I?
>>>>>>>
>>>>>>> I'd guess so, yes.
>>>>>>>
>>>>>>>> The datasheet also only specifies 3 bytes as a return value for
>>>>>>>> register 0x9fh :(
>>>>>>>
>>>>>>> Yeah. So, this flash falls into the same category "simply hijacks
>>>>>>> a manuf id" as all the other flashes.
>>>>>>
>>>>>> From a quick check, this is also be the case for GigaDevices and XMC.
>>>>>>
>>>>>> My spontaneous idea would be to extend support for JEDEC IDs to read
>>>>>> the up to 9 banks of the vendor ID and fix up the existing offenders.
>>>>>
>>>>> you mean gigadevices and xmc? I'd presume they are also lacking the
>>>>> continuation bytes.
>>>>
>>>> Correct, same story with them.
>>>>
>>>>>
>>>>>> To not break existing boards, we could either skip the continuation
>>>>>> bytes of the kernel ID definitions for all flash chips or flag the
>>>>>> already existing ones and only perform this on such flagged chips.
>>>>>>
>>>>>> Personally, I'd say that only performing this on existing chips would
>>>>>> be better, as new vendors with this violation scheme might probably
>>>>>> appear and cause conflicts.
>>>>>>
>>>>>> As we still lack auto detection for new chips with that, configuring
>>>>>> the flash chip used with the chip name via DT would allow to set the
>>>>>> exact chip used and also validate if the manufacturer / product after
>>>>>> the continuation bits matches the one read from the chip.
>>>>>>
>>>>>> What do you think?
>>>>>
>>>>> If you'd ask me, unless there is a real world conflict, I'd just go
>>>>> ahead and add them as is. If there is a conflict we'd need to find
>>>>> a per device resolution for it.
>>>>
>>>> Okay, I'll resend a v2 with the removed copyright then.
>>>
>>> Could you also apply my SFDP patch [1] and send the dump (if there
>>> is any)? 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.
>>
>> Bad news, I'm not able to get a SFDP with your patches, as the SFDP extraction
>> fails at the version check.
>>
>> Is there anything else I can try?
>>
> 
> So no SFDP data?
> Have you tried to read more of ID bytes, maybe there's an extended ID? Please
> dump 15 bytes of ID.

what's the difference between by25q128as and bh25q128as? I see they share the
same flash ID.

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

Re: [PATCH] mtd: spi-nor: Add support for BoHong bh25q128as

[George Brooke]

Hi Tudor,

Tudor.Ambarus@microchip.com writes:

> On 6/28/21 8:48 AM, Tudor.Ambarus@microchip.com wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 5/18/21 10:39 PM, David Bauer wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> Hi Michael,
>>>
>>> Sorry for the late reply, was not feeling well past week.
>>>
>>> On 5/10/21 1:22 PM, Michael Walle wrote:
>>>> Hi David,
>>>>
>>>> Am 2021-05-10 13:04, schrieb David Bauer:
>>>>> On 5/10/21 12:56 PM, Michael Walle wrote:
>>>>>> Am 2021-05-10 12:27, schrieb David Bauer:
>>>>>>> On 5/10/21 11:35 AM, Michael Walle wrote:
>>>>>>>> Am 2021-05-10 11:28, schrieb David Bauer:
>>>>>>>>> On 5/10/21 10:00 AM, Michael Walle wrote
>>>>>>>>>
>>>>>>>>> [...]
>>>>>>>>>
>>>>>>>>>>> +static const struct flash_info bohong_parts[] = {
>>>>>>>>>>> +    /* BoHong Microelectronics */
>>>>>>>>>>> +    { "bh25q128as", INFO(0x684018, 0, 64 * 1024, 256,
>>>>>>>>>>
>>>>>>>>>> I couldn't find "BoHong" in JEP106BC. 0x68 (without continuation codes)
>>>>>>>>>> is "Convex Computer". So this is wrong. OTOH I'm not sure, how many
>>>>>>>>>> SPI flashes "convex computer" have, if any ;) This company was brought
>>>>>>>>>> by HP in the end.
>>>>>>>>>>
>>>>>>>>>> In any case, this patch depends on how we handle continuation codes or
>>>>>>>>>> if we can handle them at all. Or if this flash just lie about its
>>>>>>>>>> manufacturer id and don't and CC.
>>>>>>>>>
>>>>>>>>> First of all, BoHong and Boya microelectronics seems to be the same
>>>>>>>>> company, as their datasheets seem to copy each other. There's not much
>>>>>>>>> information about either of both, so I'd say that's a fair assumption.
>>>>>>>>>
>>>>>>>>> Regarding the continuation codes, Boya is listed in bank nine, however
>>>>>>>>> in this case I should currently read an all 0x7f ID shouldn't I?
>>>>>>>>
>>>>>>>> I'd guess so, yes.
>>>>>>>>
>>>>>>>>> The datasheet also only specifies 3 bytes as a return value for
>>>>>>>>> register 0x9fh :(
>>>>>>>>
>>>>>>>> Yeah. So, this flash falls into the same category "simply hijacks
>>>>>>>> a manuf id" as all the other flashes.
>>>>>>>
>>>>>>> From a quick check, this is also be the case for GigaDevices and XMC.
>>>>>>>
>>>>>>> My spontaneous idea would be to extend support for JEDEC IDs to read
>>>>>>> the up to 9 banks of the vendor ID and fix up the existing offenders.
>>>>>>
>>>>>> you mean gigadevices and xmc? I'd presume they are also lacking the
>>>>>> continuation bytes.
>>>>>
>>>>> Correct, same story with them.
>>>>>
>>>>>>
>>>>>>> To not break existing boards, we could either skip the continuation
>>>>>>> bytes of the kernel ID definitions for all flash chips or flag the
>>>>>>> already existing ones and only perform this on such flagged chips.
>>>>>>>
>>>>>>> Personally, I'd say that only performing this on existing chips would
>>>>>>> be better, as new vendors with this violation scheme might probably
>>>>>>> appear and cause conflicts.
>>>>>>>
>>>>>>> As we still lack auto detection for new chips with that, configuring
>>>>>>> the flash chip used with the chip name via DT would allow to set the
>>>>>>> exact chip used and also validate if the manufacturer / product after
>>>>>>> the continuation bits matches the one read from the chip.
>>>>>>>
>>>>>>> What do you think?
>>>>>>
>>>>>> If you'd ask me, unless there is a real world conflict, I'd just go
>>>>>> ahead and add them as is. If there is a conflict we'd need to find
>>>>>> a per device resolution for it.
>>>>>
>>>>> Okay, I'll resend a v2 with the removed copyright then.
>>>>
>>>> Could you also apply my SFDP patch [1] and send the dump (if there
>>>> is any)? 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.
>>>
>>> Bad news, I'm not able to get a SFDP with your patches, as the SFDP extraction
>>> fails at the version check.
>>>
>>> Is there anything else I can try?
>>>
>>
>> So no SFDP data?
>> Have you tried to read more of ID bytes, maybe there's an extended ID? Please
>> dump 15 bytes of ID.
>
> what's the difference between by25q128as and bh25q128as? I see they share the
> same flash ID.
>

I've got the by25q128as, so I compiled the SFDP and sysfs patch kernel
to read it out.

figgyc@figgyc-pi:~ $ ls /sys/class/spi_master/spi0/spi0.0/spi-nor/
jedec_id  manufacturer  partname
$ cat /sys/class/spi_master/spi0/spi0.0/spi-nor/jedec_id
684018
$ cat /sys/class/spi_master/spi0/spi0.0/spi-nor/manufacturer
boya
$ cat /sys/class/spi_master/spi0/spi0.0/spi-nor/partname
by25q128as
(this is using my patch for the chip support)

There was no sfdp file for me either, failed the version check like
David's chip (I added a dev_dbg to check).

One thing I noticed reading the datasheet[1] again was this line:
"Security Register 0 can be used to store the Flash Discoverable
Parameters, The feature is upon special order, please contact Boya
Microelectronics for details."
The same line is also present in the BoHong datasheet but it says
HuaHong instead of Boya. That makes me wonder if the meaning of
"Discoverable Parameters (SFDP) register" in the datasheet does not
actually mean that it has SFDP data programmed in by default, which
would be quite strange, but if true then that would be quite annoying
because then I don't think there are any differences between Boya and
BoHong. Very strange design decision in my opinion but it is what it
is.

The only other explanation I could think of is that erasing the chip
might erase security register 0? Unfortunately I only have one chip so
I can't test that. Even if that were the case it would still be
unhelpful.

I dumped extra ID in a previous email thread, IIRC it just loops, no
extra 7f bytes like there should be.

In conclusion it seems to me as though the two chips behave
identically, there's probably no way to know for certain though without
asking the manufacturer.

[1] http://www.bmsemi.com/upload/file/20180425/15246261557309416.pdf

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

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

Re: [PATCH] mtd: spi-nor: Add support for BoHong bh25q128as

[Michael Walle]

Am 3. Juli 2021 17:58:57 MESZ schrieb George Brooke <figgyc@figgyc.uk>:
>Hi Tudor,
>
>Tudor.Ambarus@microchip.com writes:
>
>> On 6/28/21 8:48 AM, Tudor.Ambarus@microchip.com wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>know the content is safe
>>>
>>> On 5/18/21 10:39 PM, David Bauer wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>know the content is safe
>>>>
>>>> Hi Michael,
>>>>
>>>> Sorry for the late reply, was not feeling well past week.
>>>>
>>>> On 5/10/21 1:22 PM, Michael Walle wrote:
>>>>> Hi David,
>>>>>
>>>>> Am 2021-05-10 13:04, schrieb David Bauer:
>>>>>> On 5/10/21 12:56 PM, Michael Walle wrote:
>>>>>>> Am 2021-05-10 12:27, schrieb David Bauer:
>>>>>>>> On 5/10/21 11:35 AM, Michael Walle wrote:
>>>>>>>>> Am 2021-05-10 11:28, schrieb David Bauer:
>>>>>>>>>> On 5/10/21 10:00 AM, Michael Walle wrote
>>>>>>>>>>
>>>>>>>>>> [...]
>>>>>>>>>>
>>>>>>>>>>>> +static const struct flash_info bohong_parts[] = {
>>>>>>>>>>>> +    /* BoHong Microelectronics */
>>>>>>>>>>>> +    { "bh25q128as", INFO(0x684018, 0, 64 * 1024, 256,
>>>>>>>>>>>
>>>>>>>>>>> I couldn't find "BoHong" in JEP106BC. 0x68 (without
>continuation codes)
>>>>>>>>>>> is "Convex Computer". So this is wrong. OTOH I'm not sure,
>how many
>>>>>>>>>>> SPI flashes "convex computer" have, if any ;) This company
>was brought
>>>>>>>>>>> by HP in the end.
>>>>>>>>>>>
>>>>>>>>>>> In any case, this patch depends on how we handle
>continuation codes or
>>>>>>>>>>> if we can handle them at all. Or if this flash just lie
>about its
>>>>>>>>>>> manufacturer id and don't and CC.
>>>>>>>>>>
>>>>>>>>>> First of all, BoHong and Boya microelectronics seems to be
>the same
>>>>>>>>>> company, as their datasheets seem to copy each other. There's
>not much
>>>>>>>>>> information about either of both, so I'd say that's a fair
>assumption.
>>>>>>>>>>
>>>>>>>>>> Regarding the continuation codes, Boya is listed in bank
>nine, however
>>>>>>>>>> in this case I should currently read an all 0x7f ID shouldn't
>I?
>>>>>>>>>
>>>>>>>>> I'd guess so, yes.
>>>>>>>>>
>>>>>>>>>> The datasheet also only specifies 3 bytes as a return value
>for
>>>>>>>>>> register 0x9fh :(
>>>>>>>>>
>>>>>>>>> Yeah. So, this flash falls into the same category "simply
>hijacks
>>>>>>>>> a manuf id" as all the other flashes.
>>>>>>>>
>>>>>>>> From a quick check, this is also be the case for GigaDevices
>and XMC.
>>>>>>>>
>>>>>>>> My spontaneous idea would be to extend support for JEDEC IDs to
>read
>>>>>>>> the up to 9 banks of the vendor ID and fix up the existing
>offenders.
>>>>>>>
>>>>>>> you mean gigadevices and xmc? I'd presume they are also lacking
>the
>>>>>>> continuation bytes.
>>>>>>
>>>>>> Correct, same story with them.
>>>>>>
>>>>>>>
>>>>>>>> To not break existing boards, we could either skip the
>continuation
>>>>>>>> bytes of the kernel ID definitions for all flash chips or flag
>the
>>>>>>>> already existing ones and only perform this on such flagged
>chips.
>>>>>>>>
>>>>>>>> Personally, I'd say that only performing this on existing chips
>would
>>>>>>>> be better, as new vendors with this violation scheme might
>probably
>>>>>>>> appear and cause conflicts.
>>>>>>>>
>>>>>>>> As we still lack auto detection for new chips with that,
>configuring
>>>>>>>> the flash chip used with the chip name via DT would allow to
>set the
>>>>>>>> exact chip used and also validate if the manufacturer / product
>after
>>>>>>>> the continuation bits matches the one read from the chip.
>>>>>>>>
>>>>>>>> What do you think?
>>>>>>>
>>>>>>> If you'd ask me, unless there is a real world conflict, I'd just
>go
>>>>>>> ahead and add them as is. If there is a conflict we'd need to
>find
>>>>>>> a per device resolution for it.
>>>>>>
>>>>>> Okay, I'll resend a v2 with the removed copyright then.
>>>>>
>>>>> Could you also apply my SFDP patch [1] and send the dump (if there
>>>>> is any)? 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.
>>>>
>>>> Bad news, I'm not able to get a SFDP with your patches, as the SFDP
>extraction
>>>> fails at the version check.
>>>>
>>>> Is there anything else I can try?
>>>>
>>>
>>> So no SFDP data?
>>> Have you tried to read more of ID bytes, maybe there's an extended
>ID? Please
>>> dump 15 bytes of ID.
>>
>> what's the difference between by25q128as and bh25q128as? I see they
>share the
>> same flash ID.
>>
>
>I've got the by25q128as, so I compiled the SFDP and sysfs patch kernel
>to read it out.
>
>figgyc@figgyc-pi:~ $ ls /sys/class/spi_master/spi0/spi0.0/spi-nor/
>jedec_id  manufacturer  partname
>$ cat /sys/class/spi_master/spi0/spi0.0/spi-nor/jedec_id
>684018
>$ cat /sys/class/spi_master/spi0/spi0.0/spi-nor/manufacturer
>boya
>$ cat /sys/class/spi_master/spi0/spi0.0/spi-nor/partname
>by25q128as
>(this is using my patch for the chip support)
>
>There was no sfdp file for me either, failed the version check like
>David's chip (I added a dev_dbg to check).

Then it seems it doesn't have SFDP. 


>One thing I noticed reading the datasheet[1] again was this line:
>"Security Register 0 can be used to store the Flash Discoverable
>Parameters, The feature is upon special order, please contact Boya
>Microelectronics for details."
>The same line is also present in the BoHong datasheet but it says
>HuaHong instead of Boya. That makes me wonder if the meaning of
>"Discoverable Parameters (SFDP) register" in the datasheet does not
>actually mean that it has SFDP data programmed in by default, which
>would be quite strange, but if true then that would be quite annoying
>because then I don't think there are any differences between Boya and
>BoHong. Very strange design decision in my opinion but it is what it
>is.

I'd say it is exactly this. There is no SFDP. only on "special request", I guess that means "we were too lazy and if there is a client big enough we'll do it".

>The only other explanation I could think of is that erasing the chip
>might erase security register 0? Unfortunately I only have one chip so
>I can't test that. Even if that were the case it would still be
>unhelpful.

There should be a special command to erase security registers, aka OTP. Winbond does the same, just return the first security register content if you send a RDSFDP. Just that it's programmed by default and the data sheet doesn't mention security register 0 (and treat it as reserved). 

-michael

>I dumped extra ID in a previous email thread, IIRC it just loops, no
>extra 7f bytes like there should be.
>
>In conclusion it seems to me as though the two chips behave
>identically, there's probably no way to know for certain though without
>asking the manufacturer.
>
>[1] http://www.bmsemi.com/upload/file/20180425/15246261557309416.pdf
>
>> ______________________________________________________
>> Linux MTD discussion mailing list
>> http://lists.infradead.org/mailman/listinfo/linux-mtd/


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

[PATCH] mtd: spi-nor: Add support for BoHong bh25q128as

[Christian Marangi]

From: David Bauer <mail@david-bauer.net>

Add MTD support for the BoHong bh25q128as SPI NOR chip.
The chip has 16MB of total capacity, divided into a total of 256
sectors, each 64KB sized. The chip also supports 4KB sectors.
Additionally, it supports dual and quad read modes.

Datasheet is public and can be found here [1].

Functionality was verified on an Tenbay WR1800K / MTK MT7621 board.

[1] https://www.e-interlink.com.tw/userUpload/files/BH25Q128AS_v1_0.pdf

Signed-off-by: David Bauer <mail@david-bauer.net>
[ reworked to new flash_info format ]
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/mtd/spi-nor/Makefile |  1 +
 drivers/mtd/spi-nor/bohong.c | 24 ++++++++++++++++++++++++
 drivers/mtd/spi-nor/core.c   |  1 +
 drivers/mtd/spi-nor/core.h   |  1 +
 4 files changed, 27 insertions(+)
 create mode 100644 drivers/mtd/spi-nor/bohong.c

diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
index 5e68468b72fc..c8849cf5124f 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -2,6 +2,7 @@
 
 spi-nor-objs			:= core.o sfdp.o swp.o otp.o sysfs.o
 spi-nor-objs			+= atmel.o
+spi-nor-objs			+= bohong.o
 spi-nor-objs			+= eon.o
 spi-nor-objs			+= esmt.o
 spi-nor-objs			+= everspin.o
diff --git a/drivers/mtd/spi-nor/bohong.c b/drivers/mtd/spi-nor/bohong.c
new file mode 100644
index 000000000000..26988c139262
--- /dev/null
+++ b/drivers/mtd/spi-nor/bohong.c
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2005, Intec Automation Inc.
+ * Copyright (C) 2014, Freescale Semiconductor, Inc.
+ */
+
+#include <linux/mtd/spi-nor.h>
+
+#include "core.h"
+
+static const struct flash_info bohong_parts[] = {
+	{
+		.id = SNOR_ID(0x68, 0x40, 0x18),
+		.name = "bh25q128as",
+		.size = SZ_16M,
+		.no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ,
+	},
+};
+
+const struct spi_nor_manufacturer spi_nor_bohong = {
+	.name = "bohong",
+	.parts = bohong_parts,
+	.nparts = ARRAY_SIZE(bohong_parts),
+};
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 4129764fad8c..29c28ee683a1 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2037,6 +2037,7 @@ int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor)
 
 static const struct spi_nor_manufacturer *manufacturers[] = {
 	&spi_nor_atmel,
+	&spi_nor_bohong,
 	&spi_nor_eon,
 	&spi_nor_esmt,
 	&spi_nor_everspin,
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index d36c0e072954..c293568ae827 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -601,6 +601,7 @@ struct sfdp {
 
 /* Manufacturer drivers. */
 extern const struct spi_nor_manufacturer spi_nor_atmel;
+extern const struct spi_nor_manufacturer spi_nor_bohong;
 extern const struct spi_nor_manufacturer spi_nor_eon;
 extern const struct spi_nor_manufacturer spi_nor_esmt;
 extern const struct spi_nor_manufacturer spi_nor_everspin;
-- 
2.43.0


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

[PATCH] mtd: spi-nor: Add support for BoHong bh25q128as

[Christian Marangi]

From: David Bauer <mail@david-bauer.net>

Add MTD support for the BoHong bh25q128as SPI NOR chip.
The chip has 16MB of total capacity, divided into a total of 256
sectors, each 64KB sized. The chip also supports 4KB sectors.
Additionally, it supports dual and quad read modes.

Datasheet is public and can be found here [1].

Functionality was verified on an Tenbay WR1800K / MTK MT7621 board.

[1] https://www.e-interlink.com.tw/userUpload/files/BH25Q128AS_v1_0.pdf

Signed-off-by: David Bauer <mail@david-bauer.net>
[ reworked to new flash_info format ]
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/mtd/spi-nor/Makefile |  1 +
 drivers/mtd/spi-nor/bohong.c | 24 ++++++++++++++++++++++++
 drivers/mtd/spi-nor/core.c   |  1 +
 drivers/mtd/spi-nor/core.h   |  1 +
 4 files changed, 27 insertions(+)
 create mode 100644 drivers/mtd/spi-nor/bohong.c

diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
index 5e68468b72fc..c8849cf5124f 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -2,6 +2,7 @@
 
 spi-nor-objs			:= core.o sfdp.o swp.o otp.o sysfs.o
 spi-nor-objs			+= atmel.o
+spi-nor-objs			+= bohong.o
 spi-nor-objs			+= eon.o
 spi-nor-objs			+= esmt.o
 spi-nor-objs			+= everspin.o
diff --git a/drivers/mtd/spi-nor/bohong.c b/drivers/mtd/spi-nor/bohong.c
new file mode 100644
index 000000000000..26988c139262
--- /dev/null
+++ b/drivers/mtd/spi-nor/bohong.c
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2005, Intec Automation Inc.
+ * Copyright (C) 2014, Freescale Semiconductor, Inc.
+ */
+
+#include <linux/mtd/spi-nor.h>
+
+#include "core.h"
+
+static const struct flash_info bohong_parts[] = {
+	{
+		.id = SNOR_ID(0x68, 0x40, 0x18),
+		.name = "bh25q128as",
+		.size = SZ_16M,
+		.no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ,
+	},
+};
+
+const struct spi_nor_manufacturer spi_nor_bohong = {
+	.name = "bohong",
+	.parts = bohong_parts,
+	.nparts = ARRAY_SIZE(bohong_parts),
+};
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 4129764fad8c..29c28ee683a1 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2037,6 +2037,7 @@ int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor)
 
 static const struct spi_nor_manufacturer *manufacturers[] = {
 	&spi_nor_atmel,
+	&spi_nor_bohong,
 	&spi_nor_eon,
 	&spi_nor_esmt,
 	&spi_nor_everspin,
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index d36c0e072954..c293568ae827 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -601,6 +601,7 @@ struct sfdp {
 
 /* Manufacturer drivers. */
 extern const struct spi_nor_manufacturer spi_nor_atmel;
+extern const struct spi_nor_manufacturer spi_nor_bohong;
 extern const struct spi_nor_manufacturer spi_nor_eon;
 extern const struct spi_nor_manufacturer spi_nor_esmt;
 extern const struct spi_nor_manufacturer spi_nor_everspin;
-- 
2.43.0

Re: [PATCH] mtd: spi-nor: Add support for BoHong bh25q128as

[Michael Walle]

[-- Attachment #1: Type: text/plain, Size: 3750 bytes --]

Hi,

On Sat Feb 17, 2024 at 1:20 PM CET, Christian Marangi wrote:
> From: David Bauer <mail@david-bauer.net>
>
> Add MTD support for the BoHong bh25q128as SPI NOR chip.
> The chip has 16MB of total capacity, divided into a total of 256
> sectors, each 64KB sized. The chip also supports 4KB sectors.
> Additionally, it supports dual and quad read modes.
>
> Datasheet is public and can be found here [1].

Last time it wasn't clear if this flash will support SFDP or not.
Could you please try to dump the SFDP again, see [1].


> Functionality was verified on an Tenbay WR1800K / MTK MT7621 board.

Also per [1], you'd need to provide your test results.

> [1] https://www.e-interlink.com.tw/userUpload/files/BH25Q128AS_v1_0.pdf

Link: right above your SoB please.

> Signed-off-by: David Bauer <mail@david-bauer.net>
> [ reworked to new flash_info format ]
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  drivers/mtd/spi-nor/Makefile |  1 +
>  drivers/mtd/spi-nor/bohong.c | 24 ++++++++++++++++++++++++
>  drivers/mtd/spi-nor/core.c   |  1 +
>  drivers/mtd/spi-nor/core.h   |  1 +
>  4 files changed, 27 insertions(+)
>  create mode 100644 drivers/mtd/spi-nor/bohong.c
>
> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> index 5e68468b72fc..c8849cf5124f 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -2,6 +2,7 @@
>  
>  spi-nor-objs			:= core.o sfdp.o swp.o otp.o sysfs.o
>  spi-nor-objs			+= atmel.o
> +spi-nor-objs			+= bohong.o
>  spi-nor-objs			+= eon.o
>  spi-nor-objs			+= esmt.o
>  spi-nor-objs			+= everspin.o
> diff --git a/drivers/mtd/spi-nor/bohong.c b/drivers/mtd/spi-nor/bohong.c
> new file mode 100644
> index 000000000000..26988c139262
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/bohong.c
> @@ -0,0 +1,24 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2005, Intec Automation Inc.
> + * Copyright (C) 2014, Freescale Semiconductor, Inc.

Please remove, there is nothing from the old code left here.

> + */
> +
> +#include <linux/mtd/spi-nor.h>
> +
> +#include "core.h"
> +
> +static const struct flash_info bohong_parts[] = {
> +	{
> +		.id = SNOR_ID(0x68, 0x40, 0x18),
> +		.name = "bh25q128as",
No names anymore, please.

> +		.size = SZ_16M,
> +		.no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ,
> +	},
> +};
> +
> +const struct spi_nor_manufacturer spi_nor_bohong = {
> +	.name = "bohong",

This should be dropped, too. Otherwise looks good, if SFDP is not
supported.

-michael

[1] https://docs.kernel.org/driver-api/mtd/spi-nor.html

> +	.parts = bohong_parts,
> +	.nparts = ARRAY_SIZE(bohong_parts),
> +};
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 4129764fad8c..29c28ee683a1 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2037,6 +2037,7 @@ int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor)
>  
>  static const struct spi_nor_manufacturer *manufacturers[] = {
>  	&spi_nor_atmel,
> +	&spi_nor_bohong,
>  	&spi_nor_eon,
>  	&spi_nor_esmt,
>  	&spi_nor_everspin,
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index d36c0e072954..c293568ae827 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -601,6 +601,7 @@ struct sfdp {
>  
>  /* Manufacturer drivers. */
>  extern const struct spi_nor_manufacturer spi_nor_atmel;
> +extern const struct spi_nor_manufacturer spi_nor_bohong;
>  extern const struct spi_nor_manufacturer spi_nor_eon;
>  extern const struct spi_nor_manufacturer spi_nor_esmt;
>  extern const struct spi_nor_manufacturer spi_nor_everspin;


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

Re: [PATCH] mtd: spi-nor: Add support for BoHong bh25q128as

[Michael Walle]

[-- Attachment #1.1: Type: text/plain, Size: 3750 bytes --]

Hi,

On Sat Feb 17, 2024 at 1:20 PM CET, Christian Marangi wrote:
> From: David Bauer <mail@david-bauer.net>
>
> Add MTD support for the BoHong bh25q128as SPI NOR chip.
> The chip has 16MB of total capacity, divided into a total of 256
> sectors, each 64KB sized. The chip also supports 4KB sectors.
> Additionally, it supports dual and quad read modes.
>
> Datasheet is public and can be found here [1].

Last time it wasn't clear if this flash will support SFDP or not.
Could you please try to dump the SFDP again, see [1].


> Functionality was verified on an Tenbay WR1800K / MTK MT7621 board.

Also per [1], you'd need to provide your test results.

> [1] https://www.e-interlink.com.tw/userUpload/files/BH25Q128AS_v1_0.pdf

Link: right above your SoB please.

> Signed-off-by: David Bauer <mail@david-bauer.net>
> [ reworked to new flash_info format ]
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  drivers/mtd/spi-nor/Makefile |  1 +
>  drivers/mtd/spi-nor/bohong.c | 24 ++++++++++++++++++++++++
>  drivers/mtd/spi-nor/core.c   |  1 +
>  drivers/mtd/spi-nor/core.h   |  1 +
>  4 files changed, 27 insertions(+)
>  create mode 100644 drivers/mtd/spi-nor/bohong.c
>
> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> index 5e68468b72fc..c8849cf5124f 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -2,6 +2,7 @@
>  
>  spi-nor-objs			:= core.o sfdp.o swp.o otp.o sysfs.o
>  spi-nor-objs			+= atmel.o
> +spi-nor-objs			+= bohong.o
>  spi-nor-objs			+= eon.o
>  spi-nor-objs			+= esmt.o
>  spi-nor-objs			+= everspin.o
> diff --git a/drivers/mtd/spi-nor/bohong.c b/drivers/mtd/spi-nor/bohong.c
> new file mode 100644
> index 000000000000..26988c139262
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/bohong.c
> @@ -0,0 +1,24 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2005, Intec Automation Inc.
> + * Copyright (C) 2014, Freescale Semiconductor, Inc.

Please remove, there is nothing from the old code left here.

> + */
> +
> +#include <linux/mtd/spi-nor.h>
> +
> +#include "core.h"
> +
> +static const struct flash_info bohong_parts[] = {
> +	{
> +		.id = SNOR_ID(0x68, 0x40, 0x18),
> +		.name = "bh25q128as",
No names anymore, please.

> +		.size = SZ_16M,
> +		.no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ,
> +	},
> +};
> +
> +const struct spi_nor_manufacturer spi_nor_bohong = {
> +	.name = "bohong",

This should be dropped, too. Otherwise looks good, if SFDP is not
supported.

-michael

[1] https://docs.kernel.org/driver-api/mtd/spi-nor.html

> +	.parts = bohong_parts,
> +	.nparts = ARRAY_SIZE(bohong_parts),
> +};
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 4129764fad8c..29c28ee683a1 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2037,6 +2037,7 @@ int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor)
>  
>  static const struct spi_nor_manufacturer *manufacturers[] = {
>  	&spi_nor_atmel,
> +	&spi_nor_bohong,
>  	&spi_nor_eon,
>  	&spi_nor_esmt,
>  	&spi_nor_everspin,
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index d36c0e072954..c293568ae827 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -601,6 +601,7 @@ struct sfdp {
>  
>  /* Manufacturer drivers. */
>  extern const struct spi_nor_manufacturer spi_nor_atmel;
> +extern const struct spi_nor_manufacturer spi_nor_bohong;
>  extern const struct spi_nor_manufacturer spi_nor_eon;
>  extern const struct spi_nor_manufacturer spi_nor_esmt;
>  extern const struct spi_nor_manufacturer spi_nor_everspin;


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

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

Re: [PATCH] mtd: spi-nor: Add support for BoHong bh25q128as

[Christian Marangi]

On Mon, Feb 19, 2024 at 09:35:27AM +0100, Michael Walle wrote:
> Hi,
> 
> On Sat Feb 17, 2024 at 1:20 PM CET, Christian Marangi wrote:
> > From: David Bauer <mail@david-bauer.net>
> >
> > Add MTD support for the BoHong bh25q128as SPI NOR chip.
> > The chip has 16MB of total capacity, divided into a total of 256
> > sectors, each 64KB sized. The chip also supports 4KB sectors.
> > Additionally, it supports dual and quad read modes.
> >
> > Datasheet is public and can be found here [1].
> 
> Last time it wasn't clear if this flash will support SFDP or not.
> Could you please try to dump the SFDP again, see [1].
>

Ok will include in v2.

> 
> > Functionality was verified on an Tenbay WR1800K / MTK MT7621 board.
> 
> Also per [1], you'd need to provide your test results.
> 
> > [1] https://www.e-interlink.com.tw/userUpload/files/BH25Q128AS_v1_0.pdf
> 
> Link: right above your SoB please.
> 
> > Signed-off-by: David Bauer <mail@david-bauer.net>
> > [ reworked to new flash_info format ]
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > ---
> >  drivers/mtd/spi-nor/Makefile |  1 +
> >  drivers/mtd/spi-nor/bohong.c | 24 ++++++++++++++++++++++++
> >  drivers/mtd/spi-nor/core.c   |  1 +
> >  drivers/mtd/spi-nor/core.h   |  1 +
> >  4 files changed, 27 insertions(+)
> >  create mode 100644 drivers/mtd/spi-nor/bohong.c
> >
> > diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> > index 5e68468b72fc..c8849cf5124f 100644
> > --- a/drivers/mtd/spi-nor/Makefile
> > +++ b/drivers/mtd/spi-nor/Makefile
> > @@ -2,6 +2,7 @@
> >  
> >  spi-nor-objs			:= core.o sfdp.o swp.o otp.o sysfs.o
> >  spi-nor-objs			+= atmel.o
> > +spi-nor-objs			+= bohong.o
> >  spi-nor-objs			+= eon.o
> >  spi-nor-objs			+= esmt.o
> >  spi-nor-objs			+= everspin.o
> > diff --git a/drivers/mtd/spi-nor/bohong.c b/drivers/mtd/spi-nor/bohong.c
> > new file mode 100644
> > index 000000000000..26988c139262
> > --- /dev/null
> > +++ b/drivers/mtd/spi-nor/bohong.c
> > @@ -0,0 +1,24 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2005, Intec Automation Inc.
> > + * Copyright (C) 2014, Freescale Semiconductor, Inc.
> 
> Please remove, there is nothing from the old code left here.
> 
> > + */
> > +
> > +#include <linux/mtd/spi-nor.h>
> > +
> > +#include "core.h"
> > +
> > +static const struct flash_info bohong_parts[] = {
> > +	{
> > +		.id = SNOR_ID(0x68, 0x40, 0x18),
> > +		.name = "bh25q128as",
> No names anymore, please.
> 

Mhhh why this change? Doesn't this makes the thing problematic to
identify?

> > +		.size = SZ_16M,
> > +		.no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ,
> > +	},
> > +};
> > +
> > +const struct spi_nor_manufacturer spi_nor_bohong = {
> > +	.name = "bohong",
> 
> This should be dropped, too. Otherwise looks good, if SFDP is not
> supported.
>

Ok, thanks a lot for the review!

> 
> [1] https://docs.kernel.org/driver-api/mtd/spi-nor.html
> 
> > +	.parts = bohong_parts,
> > +	.nparts = ARRAY_SIZE(bohong_parts),
> > +};
> > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> > index 4129764fad8c..29c28ee683a1 100644
> > --- a/drivers/mtd/spi-nor/core.c
> > +++ b/drivers/mtd/spi-nor/core.c
> > @@ -2037,6 +2037,7 @@ int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor)
> >  
> >  static const struct spi_nor_manufacturer *manufacturers[] = {
> >  	&spi_nor_atmel,
> > +	&spi_nor_bohong,
> >  	&spi_nor_eon,
> >  	&spi_nor_esmt,
> >  	&spi_nor_everspin,
> > diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> > index d36c0e072954..c293568ae827 100644
> > --- a/drivers/mtd/spi-nor/core.h
> > +++ b/drivers/mtd/spi-nor/core.h
> > @@ -601,6 +601,7 @@ struct sfdp {
> >  
> >  /* Manufacturer drivers. */
> >  extern const struct spi_nor_manufacturer spi_nor_atmel;
> > +extern const struct spi_nor_manufacturer spi_nor_bohong;
> >  extern const struct spi_nor_manufacturer spi_nor_eon;
> >  extern const struct spi_nor_manufacturer spi_nor_esmt;
> >  extern const struct spi_nor_manufacturer spi_nor_everspin;
> 



-- 
	Ansuel

Re: [PATCH] mtd: spi-nor: Add support for BoHong bh25q128as

[Christian Marangi]

On Mon, Feb 19, 2024 at 09:35:27AM +0100, Michael Walle wrote:
> Hi,
> 
> On Sat Feb 17, 2024 at 1:20 PM CET, Christian Marangi wrote:
> > From: David Bauer <mail@david-bauer.net>
> >
> > Add MTD support for the BoHong bh25q128as SPI NOR chip.
> > The chip has 16MB of total capacity, divided into a total of 256
> > sectors, each 64KB sized. The chip also supports 4KB sectors.
> > Additionally, it supports dual and quad read modes.
> >
> > Datasheet is public and can be found here [1].
> 
> Last time it wasn't clear if this flash will support SFDP or not.
> Could you please try to dump the SFDP again, see [1].
>

Ok will include in v2.

> 
> > Functionality was verified on an Tenbay WR1800K / MTK MT7621 board.
> 
> Also per [1], you'd need to provide your test results.
> 
> > [1] https://www.e-interlink.com.tw/userUpload/files/BH25Q128AS_v1_0.pdf
> 
> Link: right above your SoB please.
> 
> > Signed-off-by: David Bauer <mail@david-bauer.net>
> > [ reworked to new flash_info format ]
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > ---
> >  drivers/mtd/spi-nor/Makefile |  1 +
> >  drivers/mtd/spi-nor/bohong.c | 24 ++++++++++++++++++++++++
> >  drivers/mtd/spi-nor/core.c   |  1 +
> >  drivers/mtd/spi-nor/core.h   |  1 +
> >  4 files changed, 27 insertions(+)
> >  create mode 100644 drivers/mtd/spi-nor/bohong.c
> >
> > diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> > index 5e68468b72fc..c8849cf5124f 100644
> > --- a/drivers/mtd/spi-nor/Makefile
> > +++ b/drivers/mtd/spi-nor/Makefile
> > @@ -2,6 +2,7 @@
> >  
> >  spi-nor-objs			:= core.o sfdp.o swp.o otp.o sysfs.o
> >  spi-nor-objs			+= atmel.o
> > +spi-nor-objs			+= bohong.o
> >  spi-nor-objs			+= eon.o
> >  spi-nor-objs			+= esmt.o
> >  spi-nor-objs			+= everspin.o
> > diff --git a/drivers/mtd/spi-nor/bohong.c b/drivers/mtd/spi-nor/bohong.c
> > new file mode 100644
> > index 000000000000..26988c139262
> > --- /dev/null
> > +++ b/drivers/mtd/spi-nor/bohong.c
> > @@ -0,0 +1,24 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2005, Intec Automation Inc.
> > + * Copyright (C) 2014, Freescale Semiconductor, Inc.
> 
> Please remove, there is nothing from the old code left here.
> 
> > + */
> > +
> > +#include <linux/mtd/spi-nor.h>
> > +
> > +#include "core.h"
> > +
> > +static const struct flash_info bohong_parts[] = {
> > +	{
> > +		.id = SNOR_ID(0x68, 0x40, 0x18),
> > +		.name = "bh25q128as",
> No names anymore, please.
> 

Mhhh why this change? Doesn't this makes the thing problematic to
identify?

> > +		.size = SZ_16M,
> > +		.no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ,
> > +	},
> > +};
> > +
> > +const struct spi_nor_manufacturer spi_nor_bohong = {
> > +	.name = "bohong",
> 
> This should be dropped, too. Otherwise looks good, if SFDP is not
> supported.
>

Ok, thanks a lot for the review!

> 
> [1] https://docs.kernel.org/driver-api/mtd/spi-nor.html
> 
> > +	.parts = bohong_parts,
> > +	.nparts = ARRAY_SIZE(bohong_parts),
> > +};
> > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> > index 4129764fad8c..29c28ee683a1 100644
> > --- a/drivers/mtd/spi-nor/core.c
> > +++ b/drivers/mtd/spi-nor/core.c
> > @@ -2037,6 +2037,7 @@ int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor)
> >  
> >  static const struct spi_nor_manufacturer *manufacturers[] = {
> >  	&spi_nor_atmel,
> > +	&spi_nor_bohong,
> >  	&spi_nor_eon,
> >  	&spi_nor_esmt,
> >  	&spi_nor_everspin,
> > diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> > index d36c0e072954..c293568ae827 100644
> > --- a/drivers/mtd/spi-nor/core.h
> > +++ b/drivers/mtd/spi-nor/core.h
> > @@ -601,6 +601,7 @@ struct sfdp {
> >  
> >  /* Manufacturer drivers. */
> >  extern const struct spi_nor_manufacturer spi_nor_atmel;
> > +extern const struct spi_nor_manufacturer spi_nor_bohong;
> >  extern const struct spi_nor_manufacturer spi_nor_eon;
> >  extern const struct spi_nor_manufacturer spi_nor_esmt;
> >  extern const struct spi_nor_manufacturer spi_nor_everspin;
> 



-- 
	Ansuel

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