Re: [PATCH] mtd: spi-nor: boya: add support for boya by25q128as

Re: [PATCH] mtd: spi-nor: boya: add support for boya by25q128as

[Tudor.Ambarus]

Hi, George,

On 2/7/21 2:30 PM, George Brooke wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Adds support for the Boya Microelectronics BY25Q128AS 128 Mbit flash.
> I tested this on the Creality WB-01 embedded device which uses this,
> although that was with OpenWrt which is still using 5.4 so I had to
> do a bit of porting work. Don't see how that would make much of a
> difference though.
> 
> Signed-off-by: George Brooke <figgyc@figgyc.uk>
> ---
>  drivers/mtd/spi-nor/Makefile |  1 +
>  drivers/mtd/spi-nor/boya.c   | 23 +++++++++++++++++++++++
>  drivers/mtd/spi-nor/core.c   |  1 +
>  drivers/mtd/spi-nor/core.h   |  1 +
>  4 files changed, 26 insertions(+)
>  create mode 100644 drivers/mtd/spi-nor/boya.c
> 
> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> index 653923896205..7d1551fbfbaa 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                   += boya.o
>  spi-nor-objs                   += catalyst.o
>  spi-nor-objs                   += eon.o
>  spi-nor-objs                   += esmt.o
> diff --git a/drivers/mtd/spi-nor/boya.c b/drivers/mtd/spi-nor/boya.c
> new file mode 100644
> index 000000000000..014b0087048a
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/boya.c
> @@ -0,0 +1,23 @@
> +// 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 boya_parts[] = {
> +       /* Boya */
> +       { "by25q128as", INFO(0x684018, 0, 64 * 1024, 256,

The manufacturer’s identification code is defined by one or more
eight (8) bit fields each consisting of seven (7) data bits plus one (1)
odd parity bit. It is a single field limiting the possible number of
vendors to 126. To expand the maximum number of identification codes a
continuation scheme has been defined.

According to JEP106BA, the manufacturer ID for Boya should be preceded by
eight continuation codes. So I would expect the manufacturer ID for this
flash to be: 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x68.

Without the continuation codes, we will have collisions between
manufacturer IDs, Convex Computer being an example.

I see that the datasheet [1] for this flash doesn't specify anything
about the continuation codes, so I suspect that Boya just got it wrong.
We'll have to check other datasheets from Boya and see if they got
their manufacturer ID wrong for all their flashes. We'll have to add
some fixup mechanism for the manufacturers ID handling, in order to
avoid collisions with other manufacturers IDs.

Cheers,
ta

[1] https://datasheet.lcsc.com/szlcsc/1904091402_BOYAMICRO-BY25Q128ASSIG_C383794.pdf

> +                       SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
> +                       SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
> +       },
> +};
> +
> +const struct spi_nor_manufacturer spi_nor_boya = {
> +       .name = "boya",
> +       .parts = boya_parts,
> +       .nparts = ARRAY_SIZE(boya_parts),
> +};
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 20df44b753da..4d0d003e9c3f 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2160,6 +2160,7 @@ int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor)
> 
>  static const struct spi_nor_manufacturer *manufacturers[] = {
>         &spi_nor_atmel,
> +       &spi_nor_boya,
>         &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 d631ee299de3..d5ed5217228b 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_boya;
>  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.30.0
> 

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

Re: [PATCH] mtd: spi-nor: boya: add support for boya by25q128as

[Tudor.Ambarus]

On 2/13/21 7:10 PM, George Brooke wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hello Tudor,

Hi, George,

Sorry for the long delay :(. I added in To: Chris and Andreas, they
encounter a similar problem with other vendors.
 
> 
> Tudor.Ambarus@microchip.com writes:
> 
>> Hi, George,
>>
>> On 2/7/21 2:30 PM, George Brooke wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless
>>> you know the content is safe
>>>
>>> Adds support for the Boya Microelectronics BY25Q128AS 128 Mbit
>>> flash.
>>> I tested this on the Creality WB-01 embedded device which uses
>>> this,
>>> although that was with OpenWrt which is still using 5.4 so I
>>> had to
>>> do a bit of porting work. Don't see how that would make much of
>>> a
>>> difference though.
>>>
>>> Signed-off-by: George Brooke <figgyc@figgyc.uk>
>>> ---
>>>  drivers/mtd/spi-nor/Makefile |  1 +
>>>  drivers/mtd/spi-nor/boya.c   | 23 +++++++++++++++++++++++
>>>  drivers/mtd/spi-nor/core.c   |  1 +
>>>  drivers/mtd/spi-nor/core.h   |  1 +
>>>  4 files changed, 26 insertions(+)
>>>  create mode 100644 drivers/mtd/spi-nor/boya.c
>>>
>>> diff --git a/drivers/mtd/spi-nor/Makefile
>>> b/drivers/mtd/spi-nor/Makefile
>>> index 653923896205..7d1551fbfbaa 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                   += boya.o
>>>  spi-nor-objs                   += catalyst.o
>>>  spi-nor-objs                   += eon.o
>>>  spi-nor-objs                   += esmt.o
>>> diff --git a/drivers/mtd/spi-nor/boya.c
>>> b/drivers/mtd/spi-nor/boya.c
>>> new file mode 100644
>>> index 000000000000..014b0087048a
>>> --- /dev/null
>>> +++ b/drivers/mtd/spi-nor/boya.c
>>> @@ -0,0 +1,23 @@
>>> +// 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 boya_parts[] = {
>>> +       /* Boya */
>>> +       { "by25q128as", INFO(0x684018, 0, 64 * 1024, 256,
>>
>> The manufacturer’s identification code is defined by one or more
>> eight (8) bit fields each consisting of seven (7) data bits plus
>> one (1)
>> odd parity bit. It is a single field limiting the possible
>> number of
>> vendors to 126. To expand the maximum number of identification
>> codes a
>> continuation scheme has been defined.
>>
>> According to JEP106BA, the manufacturer ID for Boya should be
>> preceded by
>> eight continuation codes. So I would expect the manufacturer ID
>> for this
>> flash to be: 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f,
>> 0x68.
>>
>> Without the continuation codes, we will have collisions between
>> manufacturer IDs, Convex Computer being an example.
>>
>> I see that the datasheet [1] for this flash doesn't specify
>> anything
>> about the continuation codes, so I suspect that Boya just got it
>> wrong.
> 
> It appears you are right. I thought it would be the best idea to
> actually
> interact with the flash chip and read its responses, so I found a
> tool
> called spincl [2] to send some commands to it with a Raspberry Pi:
> 
> $ spincl -ib -m0 -c0 -s0 -p0 6 0x90
> 0x00 0x00 0x00 0x00 0x68 0x17

I wasn't aware of the 0x90 command, it's not in the
include/linux/mtd/spi-nor.h either. Do you know what 0x17 means?

> $ spincl -ib -m0 -c0 -s0 -p0 4 0x9F
> 0x00 0x68 0x40 0x18
> and indeed there doesn't seem to be any continuation codes.
> 
>> We'll have to check other datasheets from Boya and see if they
>> got
>> their manufacturer ID wrong for all their flashes. We'll have to
>> add
> 
> The BY25Q64AS [3] appears to be described similarly.
> 
>> some fixup mechanism for the manufacturers ID handling, in order
>> to
>> avoid collisions with other manufacturers IDs.
> 
> I looked into this a bit more, and what I'm realising is that I'm
> not sure if
> there even is any mechanism to deliver the continuation codes
> within the
> base SPI-NOR standard? Take esmt.c: the f25l32qa has a device id
> 0x8c4116.
> JEP106BA attribytes 8c to Monolithic Memories in the first bank,
> while
> Elite Flash Storage (presumably an alias of ESMT) should be
> identifying
> as 0x7f, 0x7f, 0x7f, 0x8c. Its datasheet [4] appears to be equally
> sparse on
> detail. To my untrained eye, this seems to be the exact same
> situation we
> find ourselves in here. (You probably know a lot more about this
> then I do -
> if I'm wrong do point it out!)

I don't, but on a quick search I see that Elite Flash Storage may be
an alias for ESMT, so probably you're right. I wasn't aware of the
esmt problem, thanks for pointing it out.

> 
> That said I can't seem to find any formalised definition of what
> the 0x90
> "manufacturer and device ID" command is actually supposed to do in
> the case of
> a manufacturer ID that isn't in the first bank. Likewise with 0x9f
> "JEDEC ID".

I don't know if there's a standardized definition. Maybe we can shuffle
through datasheets and check what manufacturers are saying about this.

> Do you know of any flashes made by companies not in the first bank
> that
> identify themselves correctly?

No, I've worked only with flashes that have their ID in the first bank.

> 
> As far as I can tell the only proper method available is in SFDP,
> where the
> Parameter ID of a vendor specific table would be the bank number
> in the MSB
> and the actual manufacturer code in the LSB. However, this is a
> very
> over-engineered solution, wouldn't work on devices with no vendor
> specific
> tables, and in this case, I couldn't even get the SFDP table to
> read out of
> the by25q128as at all:
> $ spincl -ib -m0 -c0 -s0 -p0 32 0x5A
> 0x00 0x00 0x00 0x00 0x00 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> 0xff 0xff ...
> (This may very well be a peculiarity of the way I'm accessing it,
> but in any
> case I don't think this fix would be a very good idea.)

yeah, maybe the tool is broken, because I see that the SFDP table
is defined in the datasheet. The manufacturer specific table
identification looks broken too. MSB should have been 0x08 and LSB
0x68, whereas they have for MSB 0xff and LSB 0x0b? We can hack
the sfdp read to dump the tables. Or use Michael's sfdp dump sysfs
patches:

https://patchwork.ozlabs.org/project/linux-mtd/list/?series=235475
> 
> I'm not sure what would actually make a good fixup in this
> scenario. We'd need
> something that could differentiate a Boya chip from any other
> hypothetical
> 0x68 manufacturer flash.
> Maybe even the missing SFDP output would make a good detection
> routine, but
> that would need testing with a better SPI inspection method and
> across multiple
> flash chips because I doubt that's intentional - surely if SFDP
> didn't work it
> would just not be described in the datasheet. Probably better to
> think of
> something other than that though.
> 

The first collision between a flash from the first bank and any other
flashes from the rest of the banks can be avoided if we move all the
manufacturers with continuation codes at the end of the manufacturers
array, so that at the flash detection code they are treated as last.

Collision between flashes with continuation codes is more difficult
to solve and would require differentiation at run-time. Will have to
check jesd216 to see if there's other unique method to identify a flash.
Can you check it?

Cheers,
ta

> Thanks,
> figgyc
> 
> [2] https://github.com/CrosseyeJack/spincl
> [3]
> https://datasheet.lcsc.com/szlcsc/1904091402_BOYAMICRO-BY25Q64ASSIG_C383793.pdf
> [4]
> https://www.esmt.com.tw/upload/pdf/ESMT/datasheets/F25L32QA.pdf
> 
>>
>> Cheers,
>> ta
>>
>> [1]
>> https://datasheet.lcsc.com/szlcsc/1904091402_BOYAMICRO-BY25Q128ASSIG_C383794.pdf
>>
>>> +                       SECT_4K | SPI_NOR_DUAL_READ |
>>> SPI_NOR_QUAD_READ |
>>> +                       SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
>>> +       },
>>> +};
>>> +
>>> +const struct spi_nor_manufacturer spi_nor_boya = {
>>> +       .name = "boya",
>>> +       .parts = boya_parts,
>>> +       .nparts = ARRAY_SIZE(boya_parts),
>>> +};
>>> diff --git a/drivers/mtd/spi-nor/core.c
>>> b/drivers/mtd/spi-nor/core.c
>>> index 20df44b753da..4d0d003e9c3f 100644
>>> --- a/drivers/mtd/spi-nor/core.c
>>> +++ b/drivers/mtd/spi-nor/core.c
>>> @@ -2160,6 +2160,7 @@ int spi_nor_sr2_bit7_quad_enable(struct
>>> spi_nor *nor)
>>>
>>>  static const struct spi_nor_manufacturer *manufacturers[] = {
>>>         &spi_nor_atmel,
>>> +       &spi_nor_boya,
>>>         &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 d631ee299de3..d5ed5217228b 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_boya;
>>>  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.30.0
>>>

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

Re: [PATCH] mtd: spi-nor: boya: add support for boya by25q128as

[Chris Morgan]

On Tue, Apr 06, 2021 at 05:35:14AM +0000, Tudor.Ambarus@microchip.com wrote:
> On 2/13/21 7:10 PM, George Brooke wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > Hello Tudor,
> 
> Hi, George,
> 
> Sorry for the long delay :(. I added in To: Chris and Andreas, they
> encounter a similar problem with other vendors.
>  
> > 
> > Tudor.Ambarus@microchip.com writes:
> > 
> >> Hi, George,
> >>
> >> On 2/7/21 2:30 PM, George Brooke wrote:
> >>> EXTERNAL EMAIL: Do not click links or open attachments unless
> >>> you know the content is safe
> >>>
> >>> Adds support for the Boya Microelectronics BY25Q128AS 128 Mbit
> >>> flash.
> >>> I tested this on the Creality WB-01 embedded device which uses
> >>> this,
> >>> although that was with OpenWrt which is still using 5.4 so I
> >>> had to
> >>> do a bit of porting work. Don't see how that would make much of
> >>> a
> >>> difference though.
> >>>
> >>> Signed-off-by: George Brooke <figgyc@figgyc.uk>
> >>> ---
> >>>  drivers/mtd/spi-nor/Makefile |  1 +
> >>>  drivers/mtd/spi-nor/boya.c   | 23 +++++++++++++++++++++++
> >>>  drivers/mtd/spi-nor/core.c   |  1 +
> >>>  drivers/mtd/spi-nor/core.h   |  1 +
> >>>  4 files changed, 26 insertions(+)
> >>>  create mode 100644 drivers/mtd/spi-nor/boya.c
> >>>
> >>> diff --git a/drivers/mtd/spi-nor/Makefile
> >>> b/drivers/mtd/spi-nor/Makefile
> >>> index 653923896205..7d1551fbfbaa 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                   += boya.o
> >>>  spi-nor-objs                   += catalyst.o
> >>>  spi-nor-objs                   += eon.o
> >>>  spi-nor-objs                   += esmt.o
> >>> diff --git a/drivers/mtd/spi-nor/boya.c
> >>> b/drivers/mtd/spi-nor/boya.c
> >>> new file mode 100644
> >>> index 000000000000..014b0087048a
> >>> --- /dev/null
> >>> +++ b/drivers/mtd/spi-nor/boya.c
> >>> @@ -0,0 +1,23 @@
> >>> +// 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 boya_parts[] = {
> >>> +       /* Boya */
> >>> +       { "by25q128as", INFO(0x684018, 0, 64 * 1024, 256,
> >>
> >> The manufacturer’s identification code is defined by one or more
> >> eight (8) bit fields each consisting of seven (7) data bits plus
> >> one (1)
> >> odd parity bit. It is a single field limiting the possible
> >> number of
> >> vendors to 126. To expand the maximum number of identification
> >> codes a
> >> continuation scheme has been defined.
> >>
> >> According to JEP106BA, the manufacturer ID for Boya should be
> >> preceded by
> >> eight continuation codes. So I would expect the manufacturer ID
> >> for this
> >> flash to be: 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f,
> >> 0x68.
> >>
> >> Without the continuation codes, we will have collisions between
> >> manufacturer IDs, Convex Computer being an example.
> >>
> >> I see that the datasheet [1] for this flash doesn't specify
> >> anything
> >> about the continuation codes, so I suspect that Boya just got it
> >> wrong.
> > 
> > It appears you are right. I thought it would be the best idea to
> > actually
> > interact with the flash chip and read its responses, so I found a
> > tool
> > called spincl [2] to send some commands to it with a Raspberry Pi:
> > 
> > $ spincl -ib -m0 -c0 -s0 -p0 6 0x90
> > 0x00 0x00 0x00 0x00 0x68 0x17
> 
> I wasn't aware of the 0x90 command, it's not in the
> include/linux/mtd/spi-nor.h either. Do you know what 0x17 means?
> 
> > $ spincl -ib -m0 -c0 -s0 -p0 4 0x9F
> > 0x00 0x68 0x40 0x18
> > and indeed there doesn't seem to be any continuation codes.
> > 
> >> We'll have to check other datasheets from Boya and see if they
> >> got
> >> their manufacturer ID wrong for all their flashes. We'll have to
> >> add
> > 
> > The BY25Q64AS [3] appears to be described similarly.
> > 
> >> some fixup mechanism for the manufacturers ID handling, in order
> >> to
> >> avoid collisions with other manufacturers IDs.
> > 
> > I looked into this a bit more, and what I'm realising is that I'm
> > not sure if
> > there even is any mechanism to deliver the continuation codes
> > within the
> > base SPI-NOR standard? Take esmt.c: the f25l32qa has a device id
> > 0x8c4116.
> > JEP106BA attribytes 8c to Monolithic Memories in the first bank,
> > while
> > Elite Flash Storage (presumably an alias of ESMT) should be
> > identifying
> > as 0x7f, 0x7f, 0x7f, 0x8c. Its datasheet [4] appears to be equally
> > sparse on
> > detail. To my untrained eye, this seems to be the exact same
> > situation we
> > find ourselves in here. (You probably know a lot more about this
> > then I do -
> > if I'm wrong do point it out!)
> 
> I don't, but on a quick search I see that Elite Flash Storage may be
> an alias for ESMT, so probably you're right. I wasn't aware of the
> esmt problem, thanks for pointing it out.
> 
> > 
> > That said I can't seem to find any formalised definition of what
> > the 0x90
> > "manufacturer and device ID" command is actually supposed to do in
> > the case of
> > a manufacturer ID that isn't in the first bank. Likewise with 0x9f
> > "JEDEC ID".
> 
> I don't know if there's a standardized definition. Maybe we can shuffle
> through datasheets and check what manufacturers are saying about this.
> 
> > Do you know of any flashes made by companies not in the first bank
> > that
> > identify themselves correctly?
> 
> No, I've worked only with flashes that have their ID in the first bank.
> 
> > 
> > As far as I can tell the only proper method available is in SFDP,
> > where the
> > Parameter ID of a vendor specific table would be the bank number
> > in the MSB
> > and the actual manufacturer code in the LSB. However, this is a
> > very
> > over-engineered solution, wouldn't work on devices with no vendor
> > specific
> > tables, and in this case, I couldn't even get the SFDP table to
> > read out of
> > the by25q128as at all:
> > $ spincl -ib -m0 -c0 -s0 -p0 32 0x5A
> > 0x00 0x00 0x00 0x00 0x00 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> > 0xff 0xff ...
> > (This may very well be a peculiarity of the way I'm accessing it,
> > but in any
> > case I don't think this fix would be a very good idea.)
> 
> yeah, maybe the tool is broken, because I see that the SFDP table
> is defined in the datasheet. The manufacturer specific table
> identification looks broken too. MSB should have been 0x08 and LSB
> 0x68, whereas they have for MSB 0xff and LSB 0x0b? We can hack
> the sfdp read to dump the tables. Or use Michael's sfdp dump sysfs
> patches:
> 
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.ozlabs.org%2Fproject%2Flinux-mtd%2Flist%2F%3Fseries%3D235475&amp;data=04%7C01%7C%7C90d0d8880aa4411cbc9308d8f8bdc407%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637532841248967062%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=%2Fus52bh6zuBLyFyTTISdegJcmSeFH%2Bpu7Yv0PNIawwQ%3D&amp;reserved=0

To the extent it helps, I've used those sysfs patches to dump the registers
for my XT25F128B (XTX Technology) which also suffers from this problem.

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 don't see anything in here regarding continuation codes or what could be used
to denote which bank the manufacturer ID is in.

Thank you.

> > 
> > I'm not sure what would actually make a good fixup in this
> > scenario. We'd need
> > something that could differentiate a Boya chip from any other
> > hypothetical
> > 0x68 manufacturer flash.
> > Maybe even the missing SFDP output would make a good detection
> > routine, but
> > that would need testing with a better SPI inspection method and
> > across multiple
> > flash chips because I doubt that's intentional - surely if SFDP
> > didn't work it
> > would just not be described in the datasheet. Probably better to
> > think of
> > something other than that though.
> > 
> 
> The first collision between a flash from the first bank and any other
> flashes from the rest of the banks can be avoided if we move all the
> manufacturers with continuation codes at the end of the manufacturers
> array, so that at the flash detection code they are treated as last.
> 
> Collision between flashes with continuation codes is more difficult
> to solve and would require differentiation at run-time. Will have to
> check jesd216 to see if there's other unique method to identify a flash.
> Can you check it?
> 
> Cheers,
> ta
> 
> > Thanks,
> > figgyc
> > 
> > [2] https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FCrosseyeJack%2Fspincl&amp;data=04%7C01%7C%7C90d0d8880aa4411cbc9308d8f8bdc407%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637532841248967062%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=FFUr%2B8MHMySCf0YSkQbfzzjwdNtGEyy13kgz4xFdNjY%3D&amp;reserved=0
> > [3]
> > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdatasheet.lcsc.com%2Fszlcsc%2F1904091402_BOYAMICRO-BY25Q64ASSIG_C383793.pdf&amp;data=04%7C01%7C%7C90d0d8880aa4411cbc9308d8f8bdc407%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637532841248967062%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=XFKo75Nv63%2BxqrLlZ96%2BonbNUImuIPXXR5cMHMrZSUI%3D&amp;reserved=0
> > [4]
> > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.esmt.com.tw%2Fupload%2Fpdf%2FESMT%2Fdatasheets%2FF25L32QA.pdf&amp;data=04%7C01%7C%7C90d0d8880aa4411cbc9308d8f8bdc407%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637532841248972056%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=QevfIg1qV5%2B%2FxQCpxgN6TS%2B4mB6ES2ZnD3utETz40qw%3D&amp;reserved=0
> > 
> >>
> >> Cheers,
> >> ta
> >>
> >> [1]
> >> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdatasheet.lcsc.com%2Fszlcsc%2F1904091402_BOYAMICRO-BY25Q128ASSIG_C383794.pdf&amp;data=04%7C01%7C%7C90d0d8880aa4411cbc9308d8f8bdc407%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637532841248972056%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=d8jeDrgf%2FGgOUvecqkbs9r91rJl2K7X4IG6onmylUGY%3D&amp;reserved=0
> >>
> >>> +                       SECT_4K | SPI_NOR_DUAL_READ |
> >>> SPI_NOR_QUAD_READ |
> >>> +                       SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
> >>> +       },
> >>> +};
> >>> +
> >>> +const struct spi_nor_manufacturer spi_nor_boya = {
> >>> +       .name = "boya",
> >>> +       .parts = boya_parts,
> >>> +       .nparts = ARRAY_SIZE(boya_parts),
> >>> +};
> >>> diff --git a/drivers/mtd/spi-nor/core.c
> >>> b/drivers/mtd/spi-nor/core.c
> >>> index 20df44b753da..4d0d003e9c3f 100644
> >>> --- a/drivers/mtd/spi-nor/core.c
> >>> +++ b/drivers/mtd/spi-nor/core.c
> >>> @@ -2160,6 +2160,7 @@ int spi_nor_sr2_bit7_quad_enable(struct
> >>> spi_nor *nor)
> >>>
> >>>  static const struct spi_nor_manufacturer *manufacturers[] = {
> >>>         &spi_nor_atmel,
> >>> +       &spi_nor_boya,
> >>>         &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 d631ee299de3..d5ed5217228b 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_boya;
> >>>  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.30.0
> >>>
> 

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

Re: [PATCH] mtd: spi-nor: boya: add support for boya by25q128as

[George Brooke]

Hi again Tudor,
I've been busy with other things but I sat down today to have a 
hard
look into this.

Tudor.Ambarus@microchip.com writes:

> On 2/13/21 7:10 PM, George Brooke wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless 
>> you know the content is safe
>> 
>> Hello Tudor,
>
> Hi, George,
>
> Sorry for the long delay :(. I added in To: Chris and Andreas, 
> they
> encounter a similar problem with other vendors.
>  
>> 
>> Tudor.Ambarus@microchip.com writes:
>> 
>>> Hi, George,
>>>
>>> On 2/7/21 2:30 PM, George Brooke wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless
>>>> you know the content is safe
>>>>
>>>> Adds support for the Boya Microelectronics BY25Q128AS 128 
>>>> Mbit
>>>> flash.
>>>> I tested this on the Creality WB-01 embedded device which 
>>>> uses
>>>> this,
>>>> although that was with OpenWrt which is still using 5.4 so I
>>>> had to
>>>> do a bit of porting work. Don't see how that would make much 
>>>> of
>>>> a
>>>> difference though.
>>>>
>>>> Signed-off-by: George Brooke <figgyc@figgyc.uk>
>>>> ---
>>>>  drivers/mtd/spi-nor/Makefile |  1 +
>>>>  drivers/mtd/spi-nor/boya.c   | 23 +++++++++++++++++++++++
>>>>  drivers/mtd/spi-nor/core.c   |  1 +
>>>>  drivers/mtd/spi-nor/core.h   |  1 +
>>>>  4 files changed, 26 insertions(+)
>>>>  create mode 100644 drivers/mtd/spi-nor/boya.c
>>>>
>>>> diff --git a/drivers/mtd/spi-nor/Makefile
>>>> b/drivers/mtd/spi-nor/Makefile
>>>> index 653923896205..7d1551fbfbaa 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                   += boya.o
>>>>  spi-nor-objs                   += catalyst.o
>>>>  spi-nor-objs                   += eon.o
>>>>  spi-nor-objs                   += esmt.o
>>>> diff --git a/drivers/mtd/spi-nor/boya.c
>>>> b/drivers/mtd/spi-nor/boya.c
>>>> new file mode 100644
>>>> index 000000000000..014b0087048a
>>>> --- /dev/null
>>>> +++ b/drivers/mtd/spi-nor/boya.c
>>>> @@ -0,0 +1,23 @@
>>>> +// 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 boya_parts[] = {
>>>> +       /* Boya */
>>>> +       { "by25q128as", INFO(0x684018, 0, 64 * 1024, 256,
>>>
>>> The manufacturer’s identification code is defined by one or 
>>> more
>>> eight (8) bit fields each consisting of seven (7) data bits 
>>> plus
>>> one (1)
>>> odd parity bit. It is a single field limiting the possible
>>> number of
>>> vendors to 126. To expand the maximum number of identification
>>> codes a
>>> continuation scheme has been defined.
>>>
>>> According to JEP106BA, the manufacturer ID for Boya should be
>>> preceded by
>>> eight continuation codes. So I would expect the manufacturer 
>>> ID
>>> for this
>>> flash to be: 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f,
>>> 0x68.
>>>
>>> Without the continuation codes, we will have collisions 
>>> between
>>> manufacturer IDs, Convex Computer being an example.
>>>
>>> I see that the datasheet [1] for this flash doesn't specify
>>> anything
>>> about the continuation codes, so I suspect that Boya just got 
>>> it
>>> wrong.
>> 
>> It appears you are right. I thought it would be the best idea 
>> to
>> actually
>> interact with the flash chip and read its responses, so I found 
>> a
>> tool
>> called spincl [2] to send some commands to it with a Raspberry 
>> Pi:
>> 
>> $ spincl -ib -m0 -c0 -s0 -p0 6 0x90
>> 0x00 0x00 0x00 0x00 0x68 0x17
>
> I wasn't aware of the 0x90 command, it's not in the
> include/linux/mtd/spi-nor.h either. Do you know what 0x17 means?
>
0x90 (and 0x92 for dual spi, 0x94 for quad spi) seems to be in 
Winbond
chips [5], Boya chips [1], the ESMT chip [4] and an XMC chip [6], 
from
the datasheets I've looked at so far.
They all seem to implement it in the same way: a "24 bit address" 
which
is just a flag for device ID first or manufacturer ID first, then 
output
one and the other repeatedly.

It isn't in the Micron MT25QU128ABA [7] or the Atmel AT25FS010 [8] 
at
the very least, so it's not universal.

Interestingly the description of 0x90 in the XMC datasheet is
suspiciously similar to the one in the Winbond datasheet.

0x17 is just listed in the BY25Q128AS datasheet in the "ID 
Definition
Table"; I'm fairly sure it's just arbitrarily decided by the 
vendor for
each chip type.

>> $ spincl -ib -m0 -c0 -s0 -p0 4 0x9F
>> 0x00 0x68 0x40 0x18
>> and indeed there doesn't seem to be any continuation codes.
>> 
>>> We'll have to check other datasheets from Boya and see if they
>>> got
>>> their manufacturer ID wrong for all their flashes. We'll have 
>>> to
>>> add
>> 
>> The BY25Q64AS [3] appears to be described similarly.
>> 
>>> some fixup mechanism for the manufacturers ID handling, in 
>>> order
>>> to
>>> avoid collisions with other manufacturers IDs.
>> 
>> I looked into this a bit more, and what I'm realising is that 
>> I'm
>> not sure if
>> there even is any mechanism to deliver the continuation codes
>> within the
>> base SPI-NOR standard? Take esmt.c: the f25l32qa has a device 
>> id
>> 0x8c4116.
>> JEP106BA attribytes 8c to Monolithic Memories in the first 
>> bank,
>> while
>> Elite Flash Storage (presumably an alias of ESMT) should be
>> identifying
>> as 0x7f, 0x7f, 0x7f, 0x8c. Its datasheet [4] appears to be 
>> equally
>> sparse on
>> detail. To my untrained eye, this seems to be the exact same
>> situation we
>> find ourselves in here. (You probably know a lot more about 
>> this
>> then I do -
>> if I'm wrong do point it out!)
>
> I don't, but on a quick search I see that Elite Flash Storage 
> may be
> an alias for ESMT, so probably you're right. I wasn't aware of 
> the
> esmt problem, thanks for pointing it out.
>
>> 
>> That said I can't seem to find any formalised definition of 
>> what
>> the 0x90
>> "manufacturer and device ID" command is actually supposed to do 
>> in
>> the case of
>> a manufacturer ID that isn't in the first bank. Likewise with 
>> 0x9f
>> "JEDEC ID".
>
> I don't know if there's a standardized definition. Maybe we can 
> shuffle
> through datasheets and check what manufacturers are saying about 
> this.
>
I decided to look at all the manufacturer IDs currently in spi-nor 
to
see if there is any consensus on the handling of device ID 
commands.

In terms of manufacturer ID banks:
- Catalyst and Everspin are non-JEDEC
- Atmel, Fujitsu, Macronix, SST, Micron/ST, Spansion/Cypress, 
  Winbond
 use manufacturer codes in the first bank, so no special handling 
 is
 expected
- EON, ESMT, GigaDevice, ISSI and XMC are all in banks other than 
  the
 first bank, so basically the same situation as I mentioned with 
 ESMT
 in the previous email.

There's two especially interesting cases:

- Xilinx S3AN flashes use 0x1f, despite 0x1f bank 1 is 
  Atmel/Adesto.
In fact, 3S700AN has device ID 0x1f2500 which is collides with 
Atmel
at45db081d, for example. I did some digging and its user guide 
[10] says
system flash is "architecturally similar to the Adesto DataFlash 
SPI
Flash memories" of which it shares ID numbers, so this is not a
collision but just the IP is licensed and they didn't change the 
ID.
In fact the AT45DB081D also has [11] the strange 264-byte pages 
option
in the Xilinx driver, so they might actually be functionally 
identical.

- XMC flashes start 0x20. XMC has 0x20 in bank 10, but 0x20 bank 1 
  is
actually owned by Micron/ST who also make spi-nor flash chips. 
There
don't seem to be any direct ID collisions yet though thankfully.

As a result I was hoping XMC chips may have some sort of 
continuation
code handling but they don't seem to.
For instance XM25QH64A [6]: the datasheet does specify fairly 
clearly
that the manufacturer ID is only one byte in both 0x90 and 0x9f.
I think this is just an unfortunate coincidence and not a 
relabeling
since Micron chips don't have 0x90.

It seems like every 0x9f implementation I have looked at only
outputs the last byte of the manufacturer ID code, no 
continuations.
The AT45DB081D datasheet does actually mention the existence of 
JEP106
continuation codes but 0x1f is in the first bank so they do not 
actually
exist on that device.

>> Do you know of any flashes made by companies not in the first 
>> bank
>> that
>> identify themselves correctly?
>
> No, I've worked only with flashes that have their ID in the 
> first bank.
>
>> 
>> As far as I can tell the only proper method available is in 
>> SFDP,
>> where the
>> Parameter ID of a vendor specific table would be the bank 
>> number
>> in the MSB
>> and the actual manufacturer code in the LSB. However, this is a
>> very
>> over-engineered solution, wouldn't work on devices with no 
>> vendor
>> specific
>> tables, and in this case, I couldn't even get the SFDP table to
>> read out of
>> the by25q128as at all:
>> $ spincl -ib -m0 -c0 -s0 -p0 32 0x5A
>> 0x00 0x00 0x00 0x00 0x00 0xff 0xff 0xff 0xff 0xff 0xff 0xff 
>> 0xff
>> 0xff 0xff ...
>> (This may very well be a peculiarity of the way I'm accessing 
>> it,
>> but in any
>> case I don't think this fix would be a very good idea.)
>
> yeah, maybe the tool is broken, because I see that the SFDP 
> table
> is defined in the datasheet. The manufacturer specific table
> identification looks broken too. MSB should have been 0x08 and 
> LSB
> 0x68, whereas they have for MSB 0xff and LSB 0x0b? We can hack
>
I think you're confusing my chip with the XTX chip Chris is 
looking at.
It [9] has a 0xff 0x0b table. I can't see the SFDP table in my
datasheet?
> the sfdp read to dump the tables. Or use Michael's sfdp dump 
> sysfs
> patches:
>
> https://patchwork.ozlabs.org/project/linux-mtd/list/?series=235475
>
Good idea, I can try and do some device tree so I can connect to 
my mtd
chip on a raspberry pi on a recent kernel to see if I can run 
Michael's
patches. That'd take me a while though so I'll get back to you on 
that.
Not that I'm particularly sure it would help our situation very 
much.
I also see Chris has dumped his while I was writing this email and 
it
does seem to match with the XTX datasheet.

>> 
>> I'm not sure what would actually make a good fixup in this
>> scenario. We'd need
>> something that could differentiate a Boya chip from any other
>> hypothetical
>> 0x68 manufacturer flash.
>> Maybe even the missing SFDP output would make a good detection
>> routine, but
>> that would need testing with a better SPI inspection method and
>> across multiple
>> flash chips because I doubt that's intentional - surely if SFDP
>> didn't work it
>> would just not be described in the datasheet. Probably better 
>> to
>> think of
>> something other than that though.
>> 
>
> The first collision between a flash from the first bank and any 
> other
> flashes from the rest of the banks can be avoided if we move all 
> the
> manufacturers with continuation codes at the end of the 
> manufacturers
> array, so that at the flash detection code they are treated as 
> last.
>
> Collision between flashes with continuation codes is more 
> difficult
> to solve and would require differentiation at run-time. Will 
> have to
> check jesd216 to see if there's other unique method to identify 
> a flash.
> Can you check it?
>
I've read through the entirety of JESD216D.01 and JESD251A and I'm
pretty sure SFDP vendor-specific tables are the only place in the 
entire
standard where manufacturer ID codes are mentioned at all.

It seems kind of crazy to me that our main mechanism of flash ID 
isn't
standardised anywhere, despite the fact that almost every chip 
supports
it in a similar way. I really feel like I must be missing 
something but
at this point I wouldn't know where else to look.

I am led to the conclusion that so far, the common solution to 
JEP106
continuation codes and banks is to pretend they don't exist and 
hope
nobody has any collisions. That basically just leaves us to
distinguishing between manufacturers using unique commands.

0x90 might be a good command for that? It's semi vendor specific 
and has
some sort of unique system, but it still doesn't properly 
implement
JEDEC manufacturer IDs so it definitely doesn't rule out the 
possibility
of collision, and it won't work universally.
The SFDP vendor-specific tables solution also technically exists 
but the
XTX chip shows that's probably not the best idea. Overall this 
doesn't
seem very ideal and I'm not confident on what the best solution 
is.

Thanks,
figgyc

[5] 
https://cdn.datasheetspdf.com/pdf-down/W/2/5/W25Q128BV_Winbond.pdf
[6] http://www.xmcwh.com/Uploads/2018-02-05/5a77d2cf60e04.pdf
[7] 
https://media-www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-a/mt25q_qlhs_u_128_aba_0.pdf
[8] 
https://media.digikey.com/pdf/Data%20Sheets/Atmel%20PDFs/AT25FS010.pdf
[9] 
https://datasheet.lcsc.com/szlcsc/2005251034_XTX-XT25F128BSSIGT_C558844.pdf
[10] 
https://www.xilinx.com/support/documentation/user_guides/ug333.pdf
[11] https://www.adestotech.com/wp-content/uploads/doc3596.pdf

>
> Cheers,
> ta
>
>> Thanks,
>> figgyc
>> 
>> [2] https://github.com/CrosseyeJack/spincl
>> [3]
>> https://datasheet.lcsc.com/szlcsc/1904091402_BOYAMICRO-BY25Q64ASSIG_C383793.pdf
>> [4]
>> https://www.esmt.com.tw/upload/pdf/ESMT/datasheets/F25L32QA.pdf
>> 
>>>
>>> Cheers,
>>> ta
>>>
>>> [1]
>>> https://datasheet.lcsc.com/szlcsc/1904091402_BOYAMICRO-BY25Q128ASSIG_C383794.pdf
>>>
>>>> +                       SECT_4K | SPI_NOR_DUAL_READ |
>>>> SPI_NOR_QUAD_READ |
>>>> +                       SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
>>>> +       },
>>>> +};
>>>> +
>>>> +const struct spi_nor_manufacturer spi_nor_boya = {
>>>> +       .name = "boya",
>>>> +       .parts = boya_parts,
>>>> +       .nparts = ARRAY_SIZE(boya_parts),
>>>> +};
>>>> diff --git a/drivers/mtd/spi-nor/core.c
>>>> b/drivers/mtd/spi-nor/core.c
>>>> index 20df44b753da..4d0d003e9c3f 100644
>>>> --- a/drivers/mtd/spi-nor/core.c
>>>> +++ b/drivers/mtd/spi-nor/core.c
>>>> @@ -2160,6 +2160,7 @@ int spi_nor_sr2_bit7_quad_enable(struct
>>>> spi_nor *nor)
>>>>
>>>>  static const struct spi_nor_manufacturer *manufacturers[] = 
>>>>  {
>>>>         &spi_nor_atmel,
>>>> +       &spi_nor_boya,
>>>>         &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 d631ee299de3..d5ed5217228b 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_boya;
>>>>  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.30.0
>>>>
>
> ______________________________________________________
> 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: boya: add support for boya by25q128as

[Tudor.Ambarus]

On 4/6/21 7:11 PM, George Brooke wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi again Tudor,

Hey,

> I've been busy with other things but I sat down today to have a
> hard
> look into this.

Cool, thanks for the documentation work!
> 
> Tudor.Ambarus@microchip.com writes:
> 
>> On 2/13/21 7:10 PM, George Brooke wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless
>>> you know the content is safe
>>>
>>> Hello Tudor,
>>
>> Hi, George,
>>
>> Sorry for the long delay :(. I added in To: Chris and Andreas,
>> they
>> encounter a similar problem with other vendors.
>>
>>>
>>> Tudor.Ambarus@microchip.com writes:
>>>
>>>> Hi, George,
>>>>
>>>> On 2/7/21 2:30 PM, George Brooke wrote:
>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless
>>>>> you know the content is safe
>>>>>
>>>>> Adds support for the Boya Microelectronics BY25Q128AS 128
>>>>> Mbit
>>>>> flash.
>>>>> I tested this on the Creality WB-01 embedded device which
>>>>> uses
>>>>> this,
>>>>> although that was with OpenWrt which is still using 5.4 so I
>>>>> had to
>>>>> do a bit of porting work. Don't see how that would make much
>>>>> of
>>>>> a
>>>>> difference though.
>>>>>
>>>>> Signed-off-by: George Brooke <figgyc@figgyc.uk>
>>>>> ---
>>>>>  drivers/mtd/spi-nor/Makefile |  1 +
>>>>>  drivers/mtd/spi-nor/boya.c   | 23 +++++++++++++++++++++++
>>>>>  drivers/mtd/spi-nor/core.c   |  1 +
>>>>>  drivers/mtd/spi-nor/core.h   |  1 +
>>>>>  4 files changed, 26 insertions(+)
>>>>>  create mode 100644 drivers/mtd/spi-nor/boya.c
>>>>>
>>>>> diff --git a/drivers/mtd/spi-nor/Makefile
>>>>> b/drivers/mtd/spi-nor/Makefile
>>>>> index 653923896205..7d1551fbfbaa 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                   += boya.o
>>>>>  spi-nor-objs                   += catalyst.o
>>>>>  spi-nor-objs                   += eon.o
>>>>>  spi-nor-objs                   += esmt.o
>>>>> diff --git a/drivers/mtd/spi-nor/boya.c
>>>>> b/drivers/mtd/spi-nor/boya.c
>>>>> new file mode 100644
>>>>> index 000000000000..014b0087048a
>>>>> --- /dev/null
>>>>> +++ b/drivers/mtd/spi-nor/boya.c
>>>>> @@ -0,0 +1,23 @@
>>>>> +// 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 boya_parts[] = {
>>>>> +       /* Boya */
>>>>> +       { "by25q128as", INFO(0x684018, 0, 64 * 1024, 256,
>>>>
>>>> The manufacturer’s identification code is defined by one or
>>>> more
>>>> eight (8) bit fields each consisting of seven (7) data bits
>>>> plus
>>>> one (1)
>>>> odd parity bit. It is a single field limiting the possible
>>>> number of
>>>> vendors to 126. To expand the maximum number of identification
>>>> codes a
>>>> continuation scheme has been defined.
>>>>
>>>> According to JEP106BA, the manufacturer ID for Boya should be
>>>> preceded by
>>>> eight continuation codes. So I would expect the manufacturer
>>>> ID
>>>> for this
>>>> flash to be: 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f,
>>>> 0x68.
>>>>
>>>> Without the continuation codes, we will have collisions
>>>> between
>>>> manufacturer IDs, Convex Computer being an example.
>>>>
>>>> I see that the datasheet [1] for this flash doesn't specify
>>>> anything
>>>> about the continuation codes, so I suspect that Boya just got
>>>> it
>>>> wrong.
>>>
>>> It appears you are right. I thought it would be the best idea
>>> to
>>> actually
>>> interact with the flash chip and read its responses, so I found
>>> a
>>> tool
>>> called spincl [2] to send some commands to it with a Raspberry
>>> Pi:
>>>
>>> $ spincl -ib -m0 -c0 -s0 -p0 6 0x90
>>> 0x00 0x00 0x00 0x00 0x68 0x17
>>
>> I wasn't aware of the 0x90 command, it's not in the
>> include/linux/mtd/spi-nor.h either. Do you know what 0x17 means?
>>
> 0x90 (and 0x92 for dual spi, 0x94 for quad spi) seems to be in
> Winbond
> chips [5], Boya chips [1], the ESMT chip [4] and an XMC chip [6],
> from
> the datasheets I've looked at so far.
> They all seem to implement it in the same way: a "24 bit address"
> which
> is just a flag for device ID first or manufacturer ID first, then
> output
> one and the other repeatedly.
> 
> It isn't in the Micron MT25QU128ABA [7] or the Atmel AT25FS010 [8]
> at
> the very least, so it's not universal.
> 
> Interestingly the description of 0x90 in the XMC datasheet is
> suspiciously similar to the one in the Winbond datasheet.
> 
> 0x17 is just listed in the BY25Q128AS datasheet in the "ID
> Definition
> Table"; I'm fairly sure it's just arbitrarily decided by the
> vendor for
> each chip type.
> 
>>> $ spincl -ib -m0 -c0 -s0 -p0 4 0x9F
>>> 0x00 0x68 0x40 0x18
>>> and indeed there doesn't seem to be any continuation codes.
>>>
>>>> We'll have to check other datasheets from Boya and see if they
>>>> got
>>>> their manufacturer ID wrong for all their flashes. We'll have
>>>> to
>>>> add
>>>
>>> The BY25Q64AS [3] appears to be described similarly.
>>>
>>>> some fixup mechanism for the manufacturers ID handling, in
>>>> order
>>>> to
>>>> avoid collisions with other manufacturers IDs.
>>>
>>> I looked into this a bit more, and what I'm realising is that
>>> I'm
>>> not sure if
>>> there even is any mechanism to deliver the continuation codes
>>> within the
>>> base SPI-NOR standard? Take esmt.c: the f25l32qa has a device
>>> id
>>> 0x8c4116.
>>> JEP106BA attribytes 8c to Monolithic Memories in the first
>>> bank,
>>> while
>>> Elite Flash Storage (presumably an alias of ESMT) should be
>>> identifying
>>> as 0x7f, 0x7f, 0x7f, 0x8c. Its datasheet [4] appears to be
>>> equally
>>> sparse on
>>> detail. To my untrained eye, this seems to be the exact same
>>> situation we
>>> find ourselves in here. (You probably know a lot more about
>>> this
>>> then I do -
>>> if I'm wrong do point it out!)
>>
>> I don't, but on a quick search I see that Elite Flash Storage
>> may be
>> an alias for ESMT, so probably you're right. I wasn't aware of
>> the
>> esmt problem, thanks for pointing it out.
>>
>>>
>>> That said I can't seem to find any formalised definition of
>>> what
>>> the 0x90
>>> "manufacturer and device ID" command is actually supposed to do
>>> in
>>> the case of
>>> a manufacturer ID that isn't in the first bank. Likewise with
>>> 0x9f
>>> "JEDEC ID".
>>
>> I don't know if there's a standardized definition. Maybe we can
>> shuffle
>> through datasheets and check what manufacturers are saying about
>> this.
>>
> I decided to look at all the manufacturer IDs currently in spi-nor
> to
> see if there is any consensus on the handling of device ID
> commands.
> 
> In terms of manufacturer ID banks:
> - Catalyst and Everspin are non-JEDEC
> - Atmel, Fujitsu, Macronix, SST, Micron/ST, Spansion/Cypress,
>  Winbond
> use manufacturer codes in the first bank, so no special handling
> is
> expected
> - EON, ESMT, GigaDevice, ISSI and XMC are all in banks other than
>  the

I have an ISSI flash and a GD somewhere around here, I'll give them
a try and check if they define the continuation codes.

> first bank, so basically the same situation as I mentioned with
> ESMT
> in the previous email.
> 
> There's two especially interesting cases:
> 
> - Xilinx S3AN flashes use 0x1f, despite 0x1f bank 1 is
>  Atmel/Adesto.
> In fact, 3S700AN has device ID 0x1f2500 which is collides with
> Atmel
> at45db081d, for example. I did some digging and its user guide
> [10] says
> system flash is "architecturally similar to the Adesto DataFlash
> SPI
> Flash memories" of which it shares ID numbers, so this is not a
> collision but just the IP is licensed and they didn't change the
> ID.

oh, the horror. Similar is not identical, I wonder if there are any
differences between the two or not.

> In fact the AT45DB081D also has [11] the strange 264-byte pages
> option
> in the Xilinx driver, so they might actually be functionally
> identical.

We'll have to investigate this thoroughly. Anyway, your findings are a
very good starting point.

> 
> - XMC flashes start 0x20. XMC has 0x20 in bank 10, but 0x20 bank 1
>  is
> actually owned by Micron/ST who also make spi-nor flash chips.
> There
> don't seem to be any direct ID collisions yet though thankfully.
> 
> As a result I was hoping XMC chips may have some sort of
> continuation
> code handling but they don't seem to.
> For instance XM25QH64A [6]: the datasheet does specify fairly
> clearly
> that the manufacturer ID is only one byte in both 0x90 and 0x9f.
> I think this is just an unfortunate coincidence and not a
> relabeling
> since Micron chips don't have 0x90.
> 
> It seems like every 0x9f implementation I have looked at only
> outputs the last byte of the manufacturer ID code, no
> continuations.
> The AT45DB081D datasheet does actually mention the existence of
> JEP106
> continuation codes but 0x1f is in the first bank so they do not
> actually
> exist on that device.

OK. All your findings reveal a bigger problem that I've anticipated.

> 
>>> Do you know of any flashes made by companies not in the first
>>> bank
>>> that
>>> identify themselves correctly?
>>
>> No, I've worked only with flashes that have their ID in the
>> first bank.
>>
>>>
>>> As far as I can tell the only proper method available is in
>>> SFDP,
>>> where the
>>> Parameter ID of a vendor specific table would be the bank
>>> number
>>> in the MSB
>>> and the actual manufacturer code in the LSB. However, this is a
>>> very
>>> over-engineered solution, wouldn't work on devices with no
>>> vendor
>>> specific
>>> tables, and in this case, I couldn't even get the SFDP table to
>>> read out of
>>> the by25q128as at all:
>>> $ spincl -ib -m0 -c0 -s0 -p0 32 0x5A
>>> 0x00 0x00 0x00 0x00 0x00 0xff 0xff 0xff 0xff 0xff 0xff 0xff
>>> 0xff
>>> 0xff 0xff ...
>>> (This may very well be a peculiarity of the way I'm accessing
>>> it,
>>> but in any
>>> case I don't think this fix would be a very good idea.)
>>
>> yeah, maybe the tool is broken, because I see that the SFDP
>> table
>> is defined in the datasheet. The manufacturer specific table
>> identification looks broken too. MSB should have been 0x08 and
>> LSB
>> 0x68, whereas they have for MSB 0xff and LSB 0x0b? We can hack
>>
> I think you're confusing my chip with the XTX chip Chris is
> looking at.
> It [9] has a 0xff 0x0b table. I can't see the SFDP table in my
> datasheet?

oh, yes, you're right. I was talking about [9].

>> the sfdp read to dump the tables. Or use Michael's sfdp dump
>> sysfs
>> patches:
>>
>> https://patchwork.ozlabs.org/project/linux-mtd/list/?series=235475
>>
> Good idea, I can try and do some device tree so I can connect to
> my mtd
> chip on a raspberry pi on a recent kernel to see if I can run
> Michael's
> patches. That'd take me a while though so I'll get back to you on
> that.
> Not that I'm particularly sure it would help our situation very
> much.
> I also see Chris has dumped his while I was writing this email and
> it
> does seem to match with the XTX datasheet.
> 
>>>
>>> I'm not sure what would actually make a good fixup in this
>>> scenario. We'd need
>>> something that could differentiate a Boya chip from any other
>>> hypothetical
>>> 0x68 manufacturer flash.
>>> Maybe even the missing SFDP output would make a good detection
>>> routine, but
>>> that would need testing with a better SPI inspection method and
>>> across multiple
>>> flash chips because I doubt that's intentional - surely if SFDP
>>> didn't work it
>>> would just not be described in the datasheet. Probably better
>>> to
>>> think of
>>> something other than that though.
>>>
>>
>> The first collision between a flash from the first bank and any
>> other
>> flashes from the rest of the banks can be avoided if we move all
>> the
>> manufacturers with continuation codes at the end of the
>> manufacturers
>> array, so that at the flash detection code they are treated as
>> last.
>>
>> Collision between flashes with continuation codes is more
>> difficult
>> to solve and would require differentiation at run-time. Will
>> have to
>> check jesd216 to see if there's other unique method to identify
>> a flash.
>> Can you check it?
>>
> I've read through the entirety of JESD216D.01 and JESD251A and I'm
> pretty sure SFDP vendor-specific tables are the only place in the
> entire
> standard where manufacturer ID codes are mentioned at all.
> 
> It seems kind of crazy to me that our main mechanism of flash ID
> isn't
> standardised anywhere, despite the fact that almost every chip
> supports
> it in a similar way. I really feel like I must be missing
> something but
> at this point I wouldn't know where else to look.

I'll duplicate the effort and check the standards too. If we'll
not be able to identify a flash identification mechanism, then
we can try to contact the jedec committee, describe the problem,
and ask for feedback.

> 
> I am led to the conclusion that so far, the common solution to
> JEP106
> continuation codes and banks is to pretend they don't exist and
> hope
> nobody has any collisions. That basically just leaves us to
> distinguishing between manufacturers using unique commands.
> 
> 0x90 might be a good command for that? It's semi vendor specific
> and has> some sort of unique system, but it still doesn't properly
> implement
> JEDEC manufacturer IDs so it definitely doesn't rule out the
> possibility
> of collision, and it won't work universally.
> The SFDP vendor-specific tables solution also technically exists
> but the
> XTX chip shows that's probably not the best idea. Overall this
> doesn't
> seem very ideal and I'm not confident on what the best solution
> is.

I'll have to do some documentation work, and then maybe we'll try to
contact JEDEC JC-42.4. But if things are as bad as we find right now,
I would say that we can deal with this in an incremental way, as follows:
1/ move all the manufacturers with continuation codes at the end of the
manufacturers array -> we'll avoid the collisions between the flashes
without continuation codes and those with continuation codes.
2/ for collisions between flashes with continuation codes we'll try to
differentiate between them at run-time (sfdp, vendor specific commands, etc)
3/ if we can't find a run-time differentiator, we'll be forced to introduce
compatibles with the flash names, which I don't like, but ...

I'll think more about this, maybe I'll come with something better. I'll try
the issi and gd and let you know about the outcome. Also about if I find
something else in the standards. Michael, Pratyush and Vignesh my intervene
too. I'll raise this to their attention.

Cheers,
ta

> 
> Thanks,
> figgyc
> 
> [5]
> https://cdn.datasheetspdf.com/pdf-down/W/2/5/W25Q128BV_Winbond.pdf
> [6] http://www.xmcwh.com/Uploads/2018-02-05/5a77d2cf60e04.pdf
> [7]
> https://media-www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-a/mt25q_qlhs_u_128_aba_0.pdf
> [8]
> https://media.digikey.com/pdf/Data%20Sheets/Atmel%20PDFs/AT25FS010.pdf
> [9]
> https://datasheet.lcsc.com/szlcsc/2005251034_XTX-XT25F128BSSIGT_C558844.pdf
> [10]
> https://www.xilinx.com/support/documentation/user_guides/ug333.pdf
> [11] https://www.adestotech.com/wp-content/uploads/doc3596.pdf
> 
>>
>> Cheers,
>> ta
>>
>>> Thanks,
>>> figgyc
>>>
>>> [2] https://github.com/CrosseyeJack/spincl
>>> [3]
>>> https://datasheet.lcsc.com/szlcsc/1904091402_BOYAMICRO-BY25Q64ASSIG_C383793.pdf
>>> [4]
>>> https://www.esmt.com.tw/upload/pdf/ESMT/datasheets/F25L32QA.pdf
>>>
>>>>
>>>> Cheers,
>>>> ta
>>>>
>>>> [1]
>>>> https://datasheet.lcsc.com/szlcsc/1904091402_BOYAMICRO-BY25Q128ASSIG_C383794.pdf
>>>>
>>>>> +                       SECT_4K | SPI_NOR_DUAL_READ |
>>>>> SPI_NOR_QUAD_READ |
>>>>> +                       SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
>>>>> +       },
>>>>> +};
>>>>> +
>>>>> +const struct spi_nor_manufacturer spi_nor_boya = {
>>>>> +       .name = "boya",
>>>>> +       .parts = boya_parts,
>>>>> +       .nparts = ARRAY_SIZE(boya_parts),
>>>>> +};
>>>>> diff --git a/drivers/mtd/spi-nor/core.c
>>>>> b/drivers/mtd/spi-nor/core.c
>>>>> index 20df44b753da..4d0d003e9c3f 100644
>>>>> --- a/drivers/mtd/spi-nor/core.c
>>>>> +++ b/drivers/mtd/spi-nor/core.c
>>>>> @@ -2160,6 +2160,7 @@ int spi_nor_sr2_bit7_quad_enable(struct
>>>>> spi_nor *nor)
>>>>>
>>>>>  static const struct spi_nor_manufacturer *manufacturers[] =
>>>>>  {
>>>>>         &spi_nor_atmel,
>>>>> +       &spi_nor_boya,
>>>>>         &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 d631ee299de3..d5ed5217228b 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_boya;
>>>>>  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.30.0
>>>>>
>>
>> ______________________________________________________
>> 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: boya: add support for boya by25q128as

[Tudor.Ambarus]

On 4/7/21 9:20 AM, Tudor Ambarus - M18064 wrote:
> On 4/6/21 7:11 PM, George Brooke wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> Hi again Tudor,
> 
> Hey,
> 
>> I've been busy with other things but I sat down today to have a
>> hard
>> look into this.
> 
> Cool, thanks for the documentation work!
>>
>> Tudor.Ambarus@microchip.com writes:
>>
>>> On 2/13/21 7:10 PM, George Brooke wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless
>>>> you know the content is safe
>>>>
>>>> Hello Tudor,
>>>
>>> Hi, George,
>>>
>>> Sorry for the long delay :(. I added in To: Chris and Andreas,
>>> they
>>> encounter a similar problem with other vendors.
>>>
>>>>
>>>> Tudor.Ambarus@microchip.com writes:
>>>>
>>>>> Hi, George,
>>>>>
>>>>> On 2/7/21 2:30 PM, George Brooke wrote:
>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless
>>>>>> you know the content is safe
>>>>>>
>>>>>> Adds support for the Boya Microelectronics BY25Q128AS 128
>>>>>> Mbit
>>>>>> flash.
>>>>>> I tested this on the Creality WB-01 embedded device which
>>>>>> uses
>>>>>> this,
>>>>>> although that was with OpenWrt which is still using 5.4 so I
>>>>>> had to
>>>>>> do a bit of porting work. Don't see how that would make much
>>>>>> of
>>>>>> a
>>>>>> difference though.
>>>>>>
>>>>>> Signed-off-by: George Brooke <figgyc@figgyc.uk>
>>>>>> ---
>>>>>>  drivers/mtd/spi-nor/Makefile |  1 +
>>>>>>  drivers/mtd/spi-nor/boya.c   | 23 +++++++++++++++++++++++
>>>>>>  drivers/mtd/spi-nor/core.c   |  1 +
>>>>>>  drivers/mtd/spi-nor/core.h   |  1 +
>>>>>>  4 files changed, 26 insertions(+)
>>>>>>  create mode 100644 drivers/mtd/spi-nor/boya.c
>>>>>>
>>>>>> diff --git a/drivers/mtd/spi-nor/Makefile
>>>>>> b/drivers/mtd/spi-nor/Makefile
>>>>>> index 653923896205..7d1551fbfbaa 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                   += boya.o
>>>>>>  spi-nor-objs                   += catalyst.o
>>>>>>  spi-nor-objs                   += eon.o
>>>>>>  spi-nor-objs                   += esmt.o
>>>>>> diff --git a/drivers/mtd/spi-nor/boya.c
>>>>>> b/drivers/mtd/spi-nor/boya.c
>>>>>> new file mode 100644
>>>>>> index 000000000000..014b0087048a
>>>>>> --- /dev/null
>>>>>> +++ b/drivers/mtd/spi-nor/boya.c
>>>>>> @@ -0,0 +1,23 @@
>>>>>> +// 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 boya_parts[] = {
>>>>>> +       /* Boya */
>>>>>> +       { "by25q128as", INFO(0x684018, 0, 64 * 1024, 256,
>>>>>
>>>>> The manufacturer’s identification code is defined by one or
>>>>> more
>>>>> eight (8) bit fields each consisting of seven (7) data bits
>>>>> plus
>>>>> one (1)
>>>>> odd parity bit. It is a single field limiting the possible
>>>>> number of
>>>>> vendors to 126. To expand the maximum number of identification
>>>>> codes a
>>>>> continuation scheme has been defined.
>>>>>
>>>>> According to JEP106BA, the manufacturer ID for Boya should be
>>>>> preceded by
>>>>> eight continuation codes. So I would expect the manufacturer
>>>>> ID
>>>>> for this
>>>>> flash to be: 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f,
>>>>> 0x68.
>>>>>
>>>>> Without the continuation codes, we will have collisions
>>>>> between
>>>>> manufacturer IDs, Convex Computer being an example.
>>>>>
>>>>> I see that the datasheet [1] for this flash doesn't specify
>>>>> anything
>>>>> about the continuation codes, so I suspect that Boya just got
>>>>> it
>>>>> wrong.
>>>>
>>>> It appears you are right. I thought it would be the best idea
>>>> to
>>>> actually
>>>> interact with the flash chip and read its responses, so I found
>>>> a
>>>> tool
>>>> called spincl [2] to send some commands to it with a Raspberry
>>>> Pi:
>>>>
>>>> $ spincl -ib -m0 -c0 -s0 -p0 6 0x90
>>>> 0x00 0x00 0x00 0x00 0x68 0x17
>>>
>>> I wasn't aware of the 0x90 command, it's not in the
>>> include/linux/mtd/spi-nor.h either. Do you know what 0x17 means?
>>>
>> 0x90 (and 0x92 for dual spi, 0x94 for quad spi) seems to be in
>> Winbond
>> chips [5], Boya chips [1], the ESMT chip [4] and an XMC chip [6],
>> from
>> the datasheets I've looked at so far.
>> They all seem to implement it in the same way: a "24 bit address"
>> which
>> is just a flag for device ID first or manufacturer ID first, then
>> output
>> one and the other repeatedly.
>>
>> It isn't in the Micron MT25QU128ABA [7] or the Atmel AT25FS010 [8]
>> at
>> the very least, so it's not universal.
>>
>> Interestingly the description of 0x90 in the XMC datasheet is
>> suspiciously similar to the one in the Winbond datasheet.
>>
>> 0x17 is just listed in the BY25Q128AS datasheet in the "ID
>> Definition
>> Table"; I'm fairly sure it's just arbitrarily decided by the
>> vendor for
>> each chip type.
>>
>>>> $ spincl -ib -m0 -c0 -s0 -p0 4 0x9F
>>>> 0x00 0x68 0x40 0x18
>>>> and indeed there doesn't seem to be any continuation codes.
>>>>
>>>>> We'll have to check other datasheets from Boya and see if they
>>>>> got
>>>>> their manufacturer ID wrong for all their flashes. We'll have
>>>>> to
>>>>> add
>>>>
>>>> The BY25Q64AS [3] appears to be described similarly.
>>>>
>>>>> some fixup mechanism for the manufacturers ID handling, in
>>>>> order
>>>>> to
>>>>> avoid collisions with other manufacturers IDs.
>>>>
>>>> I looked into this a bit more, and what I'm realising is that
>>>> I'm
>>>> not sure if
>>>> there even is any mechanism to deliver the continuation codes
>>>> within the
>>>> base SPI-NOR standard? Take esmt.c: the f25l32qa has a device
>>>> id
>>>> 0x8c4116.
>>>> JEP106BA attribytes 8c to Monolithic Memories in the first
>>>> bank,
>>>> while
>>>> Elite Flash Storage (presumably an alias of ESMT) should be
>>>> identifying
>>>> as 0x7f, 0x7f, 0x7f, 0x8c. Its datasheet [4] appears to be
>>>> equally
>>>> sparse on
>>>> detail. To my untrained eye, this seems to be the exact same
>>>> situation we
>>>> find ourselves in here. (You probably know a lot more about
>>>> this
>>>> then I do -
>>>> if I'm wrong do point it out!)
>>>
>>> I don't, but on a quick search I see that Elite Flash Storage
>>> may be
>>> an alias for ESMT, so probably you're right. I wasn't aware of
>>> the
>>> esmt problem, thanks for pointing it out.
>>>
>>>>
>>>> That said I can't seem to find any formalised definition of
>>>> what
>>>> the 0x90
>>>> "manufacturer and device ID" command is actually supposed to do
>>>> in
>>>> the case of
>>>> a manufacturer ID that isn't in the first bank. Likewise with
>>>> 0x9f
>>>> "JEDEC ID".
>>>
>>> I don't know if there's a standardized definition. Maybe we can
>>> shuffle
>>> through datasheets and check what manufacturers are saying about
>>> this.
>>>
>> I decided to look at all the manufacturer IDs currently in spi-nor
>> to
>> see if there is any consensus on the handling of device ID
>> commands.
>>
>> In terms of manufacturer ID banks:
>> - Catalyst and Everspin are non-JEDEC
>> - Atmel, Fujitsu, Macronix, SST, Micron/ST, Spansion/Cypress,
>>  Winbond
>> use manufacturer codes in the first bank, so no special handling
>> is
>> expected
>> - EON, ESMT, GigaDevice, ISSI and XMC are all in banks other than
>>  the
> 
> I have an ISSI flash and a GD somewhere around here, I'll give them
> a try and check if they define the continuation codes.
> 
>> first bank, so basically the same situation as I mentioned with
>> ESMT
>> in the previous email.
>>
>> There's two especially interesting cases:
>>
>> - Xilinx S3AN flashes use 0x1f, despite 0x1f bank 1 is
>>  Atmel/Adesto.
>> In fact, 3S700AN has device ID 0x1f2500 which is collides with
>> Atmel
>> at45db081d, for example. I did some digging and its user guide
>> [10] says
>> system flash is "architecturally similar to the Adesto DataFlash
>> SPI
>> Flash memories" of which it shares ID numbers, so this is not a
>> collision but just the IP is licensed and they didn't change the
>> ID.
> 
> oh, the horror. Similar is not identical, I wonder if there are any
> differences between the two or not.
> 
>> In fact the AT45DB081D also has [11] the strange 264-byte pages
>> option
>> in the Xilinx driver, so they might actually be functionally
>> identical.
> 
> We'll have to investigate this thoroughly. Anyway, your findings are a
> very good starting point.
> 
>>
>> - XMC flashes start 0x20. XMC has 0x20 in bank 10, but 0x20 bank 1
>>  is
>> actually owned by Micron/ST who also make spi-nor flash chips.
>> There
>> don't seem to be any direct ID collisions yet though thankfully.
>>
>> As a result I was hoping XMC chips may have some sort of
>> continuation
>> code handling but they don't seem to.
>> For instance XM25QH64A [6]: the datasheet does specify fairly
>> clearly
>> that the manufacturer ID is only one byte in both 0x90 and 0x9f.
>> I think this is just an unfortunate coincidence and not a
>> relabeling
>> since Micron chips don't have 0x90.
>>
>> It seems like every 0x9f implementation I have looked at only
>> outputs the last byte of the manufacturer ID code, no
>> continuations.
>> The AT45DB081D datasheet does actually mention the existence of
>> JEP106
>> continuation codes but 0x1f is in the first bank so they do not
>> actually
>> exist on that device.
> 
> OK. All your findings reveal a bigger problem that I've anticipated.
> 
>>
>>>> Do you know of any flashes made by companies not in the first
>>>> bank
>>>> that
>>>> identify themselves correctly?
>>>
>>> No, I've worked only with flashes that have their ID in the
>>> first bank.
>>>
>>>>
>>>> As far as I can tell the only proper method available is in
>>>> SFDP,
>>>> where the
>>>> Parameter ID of a vendor specific table would be the bank
>>>> number
>>>> in the MSB
>>>> and the actual manufacturer code in the LSB. However, this is a
>>>> very
>>>> over-engineered solution, wouldn't work on devices with no
>>>> vendor
>>>> specific
>>>> tables, and in this case, I couldn't even get the SFDP table to
>>>> read out of
>>>> the by25q128as at all:
>>>> $ spincl -ib -m0 -c0 -s0 -p0 32 0x5A
>>>> 0x00 0x00 0x00 0x00 0x00 0xff 0xff 0xff 0xff 0xff 0xff 0xff
>>>> 0xff
>>>> 0xff 0xff ...
>>>> (This may very well be a peculiarity of the way I'm accessing
>>>> it,
>>>> but in any
>>>> case I don't think this fix would be a very good idea.)
>>>
>>> yeah, maybe the tool is broken, because I see that the SFDP
>>> table
>>> is defined in the datasheet. The manufacturer specific table
>>> identification looks broken too. MSB should have been 0x08 and
>>> LSB
>>> 0x68, whereas they have for MSB 0xff and LSB 0x0b? We can hack
>>>
>> I think you're confusing my chip with the XTX chip Chris is
>> looking at.
>> It [9] has a 0xff 0x0b table. I can't see the SFDP table in my
>> datasheet?
> 
> oh, yes, you're right. I was talking about [9].
> 
>>> the sfdp read to dump the tables. Or use Michael's sfdp dump
>>> sysfs
>>> patches:
>>>
>>> https://patchwork.ozlabs.org/project/linux-mtd/list/?series=235475
>>>
>> Good idea, I can try and do some device tree so I can connect to
>> my mtd
>> chip on a raspberry pi on a recent kernel to see if I can run
>> Michael's
>> patches. That'd take me a while though so I'll get back to you on
>> that.
>> Not that I'm particularly sure it would help our situation very
>> much.
>> I also see Chris has dumped his while I was writing this email and
>> it
>> does seem to match with the XTX datasheet.
>>
>>>>
>>>> I'm not sure what would actually make a good fixup in this
>>>> scenario. We'd need
>>>> something that could differentiate a Boya chip from any other
>>>> hypothetical
>>>> 0x68 manufacturer flash.
>>>> Maybe even the missing SFDP output would make a good detection
>>>> routine, but
>>>> that would need testing with a better SPI inspection method and
>>>> across multiple
>>>> flash chips because I doubt that's intentional - surely if SFDP
>>>> didn't work it
>>>> would just not be described in the datasheet. Probably better
>>>> to
>>>> think of
>>>> something other than that though.
>>>>
>>>
>>> The first collision between a flash from the first bank and any
>>> other
>>> flashes from the rest of the banks can be avoided if we move all
>>> the
>>> manufacturers with continuation codes at the end of the
>>> manufacturers
>>> array, so that at the flash detection code they are treated as
>>> last.
>>>
>>> Collision between flashes with continuation codes is more
>>> difficult
>>> to solve and would require differentiation at run-time. Will
>>> have to
>>> check jesd216 to see if there's other unique method to identify
>>> a flash.
>>> Can you check it?
>>>
>> I've read through the entirety of JESD216D.01 and JESD251A and I'm
>> pretty sure SFDP vendor-specific tables are the only place in the
>> entire
>> standard where manufacturer ID codes are mentioned at all.
>>
>> It seems kind of crazy to me that our main mechanism of flash ID
>> isn't
>> standardised anywhere, despite the fact that almost every chip
>> supports
>> it in a similar way. I really feel like I must be missing
>> something but
>> at this point I wouldn't know where else to look.
> 
> I'll duplicate the effort and check the standards too. If we'll
> not be able to identify a flash identification mechanism, then
> we can try to contact the jedec committee, describe the problem,
> and ask for feedback.
> 
>>
>> I am led to the conclusion that so far, the common solution to
>> JEP106
>> continuation codes and banks is to pretend they don't exist and
>> hope
>> nobody has any collisions. That basically just leaves us to
>> distinguishing between manufacturers using unique commands.
>>
>> 0x90 might be a good command for that? It's semi vendor specific
>> and has> some sort of unique system, but it still doesn't properly
>> implement
>> JEDEC manufacturer IDs so it definitely doesn't rule out the
>> possibility
>> of collision, and it won't work universally.
>> The SFDP vendor-specific tables solution also technically exists
>> but the
>> XTX chip shows that's probably not the best idea. Overall this
>> doesn't
>> seem very ideal and I'm not confident on what the best solution
>> is.
> 
> I'll have to do some documentation work, and then maybe we'll try to
> contact JEDEC JC-42.4. But if things are as bad as we find right now,
> I would say that we can deal with this in an incremental way, as follows:
> 1/ move all the manufacturers with continuation codes at the end of the
> manufacturers array -> we'll avoid the collisions between the flashes
> without continuation codes and those with continuation codes.

oh, 1/ is wrong it will not help at all because the flash without the
continuation codes will be always hit.

we're left with 2 and 3.

> 2/ for collisions between flashes with continuation codes we'll try to
> differentiate between them at run-time (sfdp, vendor specific commands, etc)
> 3/ if we can't find a run-time differentiator, we'll be forced to introduce
> compatibles with the flash names, which I don't like, but ...
> 
> I'll think more about this, maybe I'll come with something better. I'll try
> the issi and gd and let you know about the outcome. Also about if I find
> something else in the standards. Michael, Pratyush and Vignesh my intervene
> too. I'll raise this to their attention.
> 
> Cheers,
> ta
> 
>>
>> Thanks,
>> figgyc
>>
>> [5]
>> https://cdn.datasheetspdf.com/pdf-down/W/2/5/W25Q128BV_Winbond.pdf
>> [6] http://www.xmcwh.com/Uploads/2018-02-05/5a77d2cf60e04.pdf
>> [7]
>> https://media-www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-a/mt25q_qlhs_u_128_aba_0.pdf
>> [8]
>> https://media.digikey.com/pdf/Data%20Sheets/Atmel%20PDFs/AT25FS010.pdf
>> [9]
>> https://datasheet.lcsc.com/szlcsc/2005251034_XTX-XT25F128BSSIGT_C558844.pdf
>> [10]
>> https://www.xilinx.com/support/documentation/user_guides/ug333.pdf
>> [11] https://www.adestotech.com/wp-content/uploads/doc3596.pdf
>>
>>>
>>> Cheers,
>>> ta
>>>
>>>> Thanks,
>>>> figgyc
>>>>
>>>> [2] https://github.com/CrosseyeJack/spincl
>>>> [3]
>>>> https://datasheet.lcsc.com/szlcsc/1904091402_BOYAMICRO-BY25Q64ASSIG_C383793.pdf
>>>> [4]
>>>> https://www.esmt.com.tw/upload/pdf/ESMT/datasheets/F25L32QA.pdf
>>>>
>>>>>
>>>>> Cheers,
>>>>> ta
>>>>>
>>>>> [1]
>>>>> https://datasheet.lcsc.com/szlcsc/1904091402_BOYAMICRO-BY25Q128ASSIG_C383794.pdf
>>>>>
>>>>>> +                       SECT_4K | SPI_NOR_DUAL_READ |
>>>>>> SPI_NOR_QUAD_READ |
>>>>>> +                       SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
>>>>>> +       },
>>>>>> +};
>>>>>> +
>>>>>> +const struct spi_nor_manufacturer spi_nor_boya = {
>>>>>> +       .name = "boya",
>>>>>> +       .parts = boya_parts,
>>>>>> +       .nparts = ARRAY_SIZE(boya_parts),
>>>>>> +};
>>>>>> diff --git a/drivers/mtd/spi-nor/core.c
>>>>>> b/drivers/mtd/spi-nor/core.c
>>>>>> index 20df44b753da..4d0d003e9c3f 100644
>>>>>> --- a/drivers/mtd/spi-nor/core.c
>>>>>> +++ b/drivers/mtd/spi-nor/core.c
>>>>>> @@ -2160,6 +2160,7 @@ int spi_nor_sr2_bit7_quad_enable(struct
>>>>>> spi_nor *nor)
>>>>>>
>>>>>>  static const struct spi_nor_manufacturer *manufacturers[] =
>>>>>>  {
>>>>>>         &spi_nor_atmel,
>>>>>> +       &spi_nor_boya,
>>>>>>         &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 d631ee299de3..d5ed5217228b 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_boya;
>>>>>>  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.30.0
>>>>>>
>>>
>>> ______________________________________________________
>>> 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/