2023-01-07 22:09:51

by Winkler, Tomas

[permalink] [raw]
Subject: [PATCH v2] mtd: spi-nor: macronix: add support for mx77l51250f

Add support for mx77l51250f spi-nor chips.

SPI NOR sysfs:

$ cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
mx77l51250f
$ cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
c2751a
$ cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
macronix
$ xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
53464450060103ff00060110300000ffc2000104100100ff030001020001
00ff84000102c00000ffffffffffffffffffe520f3ffffffff1f44eb086b
083b04bbeeffffffffff00ffffff00ff0c200f5210d800ff8341bd0082a7
04db4403373830b030b0f7a9d55c009e29fff050f985ffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffff7f0ff0ff215cdcffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffff3c9b96f0c5a4c2ffffffffffffff
ffff003600279df9c064fecfffffffffffff

$ md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
36fb8e3e6f82c45bfabea45ec73c08a8
/sys/bus/spi/devices/spi0.0/spi-nor/sfdp

Note: The test_qspi.sh wasn not run as this device in intel setup is used
for BIOS and platform firmware storage.

Signed-off-by: Tomas Winkler <[email protected]>
Signed-off-by: Alexander Usyskin <[email protected]>
---
V2:
1. This chip supports SFDP, parse it instead of the manual configuration.
2. Add required output of sysfs attributes

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

diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index 6853ec9ae65d..6c3b4192a8ae 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -100,6 +100,8 @@ static const struct flash_info macronix_nor_parts[] = {
{ "mx66u2g45g", INFO(0xc2253c, 0, 64 * 1024, 4096)
NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
+ { "mx77l51250f", INFO(0xc2751a, 0, 64 * 1024, 4096)
+ PARSE_SFDP },
};

static void macronix_nor_default_init(struct spi_nor *nor)
--
2.39.0


2023-01-09 05:51:01

by Dhruva Gole

[permalink] [raw]
Subject: Re: [PATCH v2] mtd: spi-nor: macronix: add support for mx77l51250f

Hey Tomas,

On 08/01/23 03:13, Tomas Winkler wrote:
> Add support for mx77l51250f spi-nor chips.
>
> SPI NOR sysfs:
>
> $ cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
> mx77l51250f
> $ cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
> c2751a
> $ cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
> macronix
> $ xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> 53464450060103ff00060110300000ffc2000104100100ff030001020001
> 00ff84000102c00000ffffffffffffffffffe520f3ffffffff1f44eb086b
> 083b04bbeeffffffffff00ffffff00ff0c200f5210d800ff8341bd0082a7
> 04db4403373830b030b0f7a9d55c009e29fff050f985ffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffff7f0ff0ff215cdcffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffff3c9b96f0c5a4c2ffffffffffffff
> ffff003600279df9c064fecfffffffffffff
>
> $ md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> 36fb8e3e6f82c45bfabea45ec73c08a8
> /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
>
> Note: The test_qspi.sh wasn not run as this device in intel setup is used
> for BIOS and platform firmware storage.
Thanks for doing the testing but one small suggestion, move this below
the "---" lines before sending the patch email.
>
> Signed-off-by: Tomas Winkler <[email protected]>
> Signed-off-by: Alexander Usyskin <[email protected]>
> ---
For the code and idea itself, pending the changes I have advised below.

Reviewed-by: Dhruva Gole <[email protected]>

> V2:
> 1. This chip supports SFDP, parse it instead of the manual configuration.

This sounds good! Do the other flashes in macronix_nor_parts support
this? Maybe they can be updated in some later patches as well if they do
support SFDP Parsing.

> 2. Add required output of sysfs attributes
Yes, but I am not sure in this is the way to do it in the commit body
itself.
When Tudor asked you for those test logs, I think he meant for you to
paste it in the cover letter, or below here not in the patch email. Not
in the commit body.
Refer
https://lore.kernel.org/all/[email protected]/
where Takahiro has provided the tested details for ID, SFDP, Test logs.
>
> drivers/mtd/spi-nor/macronix.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> index 6853ec9ae65d..6c3b4192a8ae 100644
> --- a/drivers/mtd/spi-nor/macronix.c
> +++ b/drivers/mtd/spi-nor/macronix.c
> @@ -100,6 +100,8 @@ static const struct flash_info macronix_nor_parts[] = {
> { "mx66u2g45g", INFO(0xc2253c, 0, 64 * 1024, 4096)
> NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
> + { "mx77l51250f", INFO(0xc2751a, 0, 64 * 1024, 4096)
> + PARSE_SFDP },
> };
>
> static void macronix_nor_default_init(struct spi_nor *nor)

--
Thanks and Regards,
Dhruva Gole

2023-01-09 09:56:03

by Michael Walle

[permalink] [raw]
Subject: Re: [PATCH v2] mtd: spi-nor: macronix: add support for mx77l51250f

Am 2023-01-07 22:43, schrieb Tomas Winkler:
> $ cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
> mx77l51250f
> $ cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
> c2751a
> $ cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
> macronix
> $ xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> 53464450060103ff00060110300000ffc2000104100100ff030001020001
> 00ff84000102c00000ffffffffffffffffffe520f3ffffffff1f44eb086b
> 083b04bbeeffffffffff00ffffff00ff0c200f5210d800ff8341bd0082a7
> 04db4403373830b030b0f7a9d55c009e29fff050f985ffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffff7f0ff0ff215cdcffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffff3c9b96f0c5a4c2ffffffffffffff
> ffff003600279df9c064fecfffffffffffff
>
> $ md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> 36fb8e3e6f82c45bfabea45ec73c08a8
> /sys/bus/spi/devices/spi0.0/spi-nor/sfdp

Thanks for the dumps, as Dhruva mentioned this should go
in the comment section of this patch. But also see below.


> --- a/drivers/mtd/spi-nor/macronix.c
> +++ b/drivers/mtd/spi-nor/macronix.c
> @@ -100,6 +100,8 @@ static const struct flash_info macronix_nor_parts[]
> = {
> { "mx66u2g45g", INFO(0xc2253c, 0, 64 * 1024, 4096)
> NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
> + { "mx77l51250f", INFO(0xc2751a, 0, 64 * 1024, 4096)
> + PARSE_SFDP },

With the newest generic spi nor driver [1] this patch shouldn't
be needed at all anymore. Could you verify, that your flash will work
without it?

-michael

[1]
https://elixir.bootlin.com/linux/v6.2-rc3/source/drivers/mtd/spi-nor/core.c#L1637

2023-01-09 14:32:19

by Usyskin, Alexander

[permalink] [raw]
Subject: RE: [PATCH v2] mtd: spi-nor: macronix: add support for mx77l51250f


> > --- a/drivers/mtd/spi-nor/macronix.c
> > +++ b/drivers/mtd/spi-nor/macronix.c
> > @@ -100,6 +100,8 @@ static const struct flash_info macronix_nor_parts[]
> > = {
> > { "mx66u2g45g", INFO(0xc2253c, 0, 64 * 1024, 4096)
> > NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ |
> SPI_NOR_QUAD_READ)
> > FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
> > + { "mx77l51250f", INFO(0xc2751a, 0, 64 * 1024, 4096)
> > + PARSE_SFDP },
>
> With the newest generic spi nor driver [1] this patch shouldn't
> be needed at all anymore. Could you verify, that your flash will work
> without it?
>
> -michael
>
> [1]
> https://elixir.bootlin.com/linux/v6.2-rc3/source/drivers/mtd/spi-
> nor/core.c#L1637

Tested now, the v6.2-rc3 recognizes the chip.
Does this mean that all SFDP-supporting chips will be recognized automatically?

Tomas, we can abandon upstreaming effort, I'll save the patch if it will needed by older kernel versions.

--
Thanks,
Sasha


2023-01-09 14:32:54

by Michael Walle

[permalink] [raw]
Subject: Re: [PATCH v2] mtd: spi-nor: macronix: add support for mx77l51250f

Am 2023-01-09 15:09, schrieb Usyskin, Alexander:
>> > --- a/drivers/mtd/spi-nor/macronix.c
>> > +++ b/drivers/mtd/spi-nor/macronix.c
>> > @@ -100,6 +100,8 @@ static const struct flash_info macronix_nor_parts[]
>> > = {
>> > { "mx66u2g45g", INFO(0xc2253c, 0, 64 * 1024, 4096)
>> > NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ |
>> SPI_NOR_QUAD_READ)
>> > FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
>> > + { "mx77l51250f", INFO(0xc2751a, 0, 64 * 1024, 4096)
>> > + PARSE_SFDP },
>>
>> With the newest generic spi nor driver [1] this patch shouldn't
>> be needed at all anymore. Could you verify, that your flash will work
>> without it?
>>
>> -michael
>>
>> [1]
>> https://elixir.bootlin.com/linux/v6.2-rc3/source/drivers/mtd/spi-
>> nor/core.c#L1637
>
> Tested now, the v6.2-rc3 recognizes the chip.
> Does this mean that all SFDP-supporting chips will be recognized
> automatically?

As long as you just want to use the standard features described by SFDP,
yes. There might be lacking support for some SFDP features, though.
These
should then be added to the SFDP parser.

But, there are also features, which aren't supported by SFDP, i.e.
locking
or OTP. If you want to use these, you still need a flash table entry.

-michael

> Tomas, we can abandon upstreaming effort, I'll save the patch if it
> will needed by older kernel versions.