| Message ID | 20240220200325.19089-1-ansuelsmth@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Ambarus Tudor |
| Headers | show |
| Series | [v2] mtd: spi-nor: Add support for BoHong bh25q128as | expand |
Hi, On Tue Feb 20, 2024 at 9:03 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. Thanks for your patch! > Datasheet is public and can be found at the referenced link. > > Functionality was verified on an Tenbay WR1800K / MTK MT7621 board. > > Link: 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> > --- > Read/write/erase test are done by flashing and installing an OpenWrt > image on the spi and by using overlayfs on it confirming correct > read/write. > > root@OpenWrt:/tmp/tmp2# cat /sys/bus/spi/devices/spi0.0/spi-nor/partname > bh25q128as > root@OpenWrt:/tmp/tmp2# cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id > 684018 > root@OpenWrt:/tmp/tmp2# cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer > bohong > > root@OpenWrt:/# xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp > 53464450000101ff00000109300000ff68000103600000ffffffffffffff > ffffffffffffffffffffffffffffffffffffe520f1ffffffff0744eb086b > 083b42bbeeffffffffff00ffffff44eb0c200f5210d800ffffffffffffff > ffffffffffff003600279ef97764fcebffff Thanks! That's surprising, because in the past thread we were told, this flash doesn't have SFDP :) Even better then. But.. > > root@OpenWrt:/# sha256sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp > e2e374124e998c9c430a5a4c368ded374186637f48301dcb3943b81af2987995 /sys/bus/spi/devices/spi0.0/spi-np > > root@OpenWrt:/tmp/tmp2# hexdump -Cv /sys/bus/spi/devices/spi0.0/spi-nor/sfdp > 00000000 53 46 44 50 00 01 01 ff 00 00 01 09 30 00 00 ff |SFDP........0...| > 00000010 68 00 01 03 60 00 00 ff ff ff ff ff ff ff ff ff |h...`...........| > 00000020 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................| > 00000030 e5 20 f1 ff ff ff ff 07 44 eb 08 6b 08 3b 42 bb |. ......D..k.;B.| > 00000040 ee ff ff ff ff ff 00 ff ff ff 44 eb 0c 20 0f 52 |..........D.. .R| > 00000050 10 d8 00 ff ff ff ff ff ff ff ff ff ff ff ff ff |................| > 00000060 00 36 00 27 9e f9 77 64 fc eb ff ff |.6.'..wd....| > 0000006c > > root@OpenWrt:/tmp/tmp2# sha256sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp > e2e374124e998c9c430a5a4c368ded374186637f48301dcb3943b81af2987995 /sys/bus/spi/devices/spi0.0/spi-nor/sfdp > > root@OpenWrt:/tmp/tmp2# cat /sys/kernel/debug/spi-nor/spi0.0/capabilities > Supported read modes by the flash > 1S-1S-1S > opcode 0x03 > mode cycles 0 > dummy cycles 0 > 1S-1S-2S > opcode 0x3b > mode cycles 0 > dummy cycles 8 > 1S-2S-2S > opcode 0xbb > mode cycles 2 > dummy cycles 2 > 1S-1S-4S > opcode 0x6b > mode cycles 0 > dummy cycles 8 > 1S-4S-4S > opcode 0xeb > mode cycles 2 > dummy cycles 4 > > Supported page program modes by the flash > 1S-1S-1S > opcode 0x02 > > root@OpenWrt:/tmp/tmp2# cat /sys/kernel/debug/spi-nor/spi0.0/params > name bh25q128as > id 68 40 18 > size 16.0 MiB > write size 1 > page size 256 > address nbytes 3 > flags HAS_16BIT_SR > > opcodes > read 0x03 > dummy cycles 0 > erase 0xd8 > program 0x02 > 8D extension none > > protocols > read 1S-1S-1S > write 1S-1S-1S > register 1S-1S-1S > > erase commands > 20 (4.00 KiB) [1] > 52 (32.0 KiB) [2] > d8 (64.0 KiB) [3] > c7 (16.0 MiB) > > sector map > region (in hex) | erase mask | flags > ------------------+------------+---------- > 00000000-00ffffff | [ 123] | > > Changes v2: > - Drop wrong copyright header > - Drop name > - Add requested info about sfpd and others > > drivers/mtd/spi-nor/Makefile | 1 + > drivers/mtd/spi-nor/bohong.c | 18 ++++++++++++++++++ > drivers/mtd/spi-nor/core.c | 1 + > drivers/mtd/spi-nor/core.h | 1 + > 4 files changed, 21 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..8ba4f3a32040 > --- /dev/null > +++ b/drivers/mtd/spi-nor/bohong.c > @@ -0,0 +1,18 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#include <linux/mtd/spi-nor.h> > + > +#include "core.h" > + > +static const struct flash_info bohong_parts[] = { > + { > + .id = SNOR_ID(0x68, 0x40, 0x18), > + .size = SZ_16M, > + .no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ, .. the SFDP will likely make your patch superfluous, because all of this should be parsed from the SFDP. Did you try your flash without any flash_info entries at all? Regarding your question about the names, the identification should work by the IDs only. Maybe there will be a userspace tool which will have a flashdb instead. That is a mapping from IDs to manufacturer and part name. -michael > + }, > +}; > + > +const struct spi_nor_manufacturer spi_nor_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;
On 2/20/24 20:03, 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 at the referenced link. > > Functionality was verified on an Tenbay WR1800K / MTK MT7621 board. > > Link: 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> > --- > Read/write/erase test are done by flashing and installing an OpenWrt > image on the spi and by using overlayfs on it confirming correct > read/write. > > root@OpenWrt:/tmp/tmp2# cat /sys/bus/spi/devices/spi0.0/spi-nor/partname > bh25q128as > root@OpenWrt:/tmp/tmp2# cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id > 684018 > root@OpenWrt:/tmp/tmp2# cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer > bohong > > root@OpenWrt:/# xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp > 53464450000101ff00000109300000ff68000103600000ffffffffffffff > ffffffffffffffffffffffffffffffffffffe520f1ffffffff0744eb086b > 083b42bbeeffffffffff00ffffff44eb0c200f5210d800ffffffffffffff > ffffffffffff003600279ef97764fcebffff > Hey, Christian, Seems that the flash defines the SFDP tables, thus it's likely you won't need a flash entry at all, and instead rely on the generic SFDP driver to parse the flash settings from SFDP and initialize the flash based on that. Have you tried testing the flash without adding a dedicated entry for it? Just specifying the jedec,spi-nor compatible in dt will do the trick. As you do now, but without this patch :). Cheers, ta
On Thu, Feb 22, 2024 at 09:19:48AM +0000, Tudor Ambarus wrote: > > > On 2/20/24 20:03, 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 at the referenced link. > > > > Functionality was verified on an Tenbay WR1800K / MTK MT7621 board. > > > > Link: 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> > > --- > > Read/write/erase test are done by flashing and installing an OpenWrt > > image on the spi and by using overlayfs on it confirming correct > > read/write. > > > > root@OpenWrt:/tmp/tmp2# cat /sys/bus/spi/devices/spi0.0/spi-nor/partname > > bh25q128as > > root@OpenWrt:/tmp/tmp2# cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id > > 684018 > > root@OpenWrt:/tmp/tmp2# cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer > > bohong > > > > root@OpenWrt:/# xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp > > 53464450000101ff00000109300000ff68000103600000ffffffffffffff > > ffffffffffffffffffffffffffffffffffffe520f1ffffffff0744eb086b > > 083b42bbeeffffffffff00ffffff44eb0c200f5210d800ffffffffffffff > > ffffffffffff003600279ef97764fcebffff > > > > Hey, Christian, > > Seems that the flash defines the SFDP tables, thus it's likely you won't > need a flash entry at all, and instead rely on the generic SFDP driver > to parse the flash settings from SFDP and initialize the flash based on > that. > > Have you tried testing the flash without adding a dedicated entry for > it? Just specifying the jedec,spi-nor compatible in dt will do the > trick. As you do now, but without this patch :). > The user just tested this and It seems there is a problem in JEDEC id? [ 0.726451] spi spi0.0: setup: ignoring unsupported mode bits a00 [ 0.732850] spi-nor spi0.0: unrecognized JEDEC id bytes: 68 40 18 68 40 18 [ 0.739725] spi-nor: probe of spi0.0 failed with error -2
Hi, On Sat Feb 24, 2024 at 12:18 PM CET, Christian Marangi wrote: > The user just tested this and It seems there is a problem in JEDEC id? > > [ 0.726451] spi spi0.0: setup: ignoring unsupported mode bits a00 What SPI controller is used in this case? > [ 0.732850] spi-nor spi0.0: unrecognized JEDEC id bytes: 68 40 18 68 40 18 > [ 0.739725] spi-nor: probe of spi0.0 failed with error -2 And what kernel version is this? This should only happen if the SFDP header is wrong, but according to your dump, it is correct. -michael
On 26.02.2024 10:51, Michael Walle wrote: > Hi, > > On Sat Feb 24, 2024 at 12:18 PM CET, Christian Marangi wrote: >> The user just tested this and It seems there is a problem in JEDEC id? >> >> [ 0.726451] spi spi0.0: setup: ignoring unsupported mode bits a00 > > What SPI controller is used in this case? > >> [ 0.732850] spi-nor spi0.0: unrecognized JEDEC id bytes: 68 40 18 68 40 18 >> [ 0.739725] spi-nor: probe of spi0.0 failed with error -2 > > And what kernel version is this? This should only happen if the SFDP > header is wrong, but according to your dump, it is correct. > I assume the test was done on an older kernel, where the identification of the flash based on SFDP is not yet available.
On Mon, Feb 26, 2024 at 10:53:34AM +0200, Tudor Ambarus wrote: > > > On 26.02.2024 10:51, Michael Walle wrote: > > Hi, > > > > On Sat Feb 24, 2024 at 12:18 PM CET, Christian Marangi wrote: > >> The user just tested this and It seems there is a problem in JEDEC id? > >> > >> [ 0.726451] spi spi0.0: setup: ignoring unsupported mode bits a00 > > > > What SPI controller is used in this case? > > > >> [ 0.732850] spi-nor spi0.0: unrecognized JEDEC id bytes: 68 40 18 68 40 18 > >> [ 0.739725] spi-nor: probe of spi0.0 failed with error -2 > > > > And what kernel version is this? This should only happen if the SFDP > > header is wrong, but according to your dump, it is correct. > > > > I assume the test was done on an older kernel, where the identification > of the flash based on SFDP is not yet available. The test has been done on 5.15 and 6.1. The support for this was introduced later? Can you point me to the commits so I can backport them?
On 2/27/24 09:33, Christian Marangi wrote: > On Mon, Feb 26, 2024 at 10:53:34AM +0200, Tudor Ambarus wrote: >> >> >> On 26.02.2024 10:51, Michael Walle wrote: >>> Hi, >>> >>> On Sat Feb 24, 2024 at 12:18 PM CET, Christian Marangi wrote: >>>> The user just tested this and It seems there is a problem in JEDEC id? >>>> >>>> [ 0.726451] spi spi0.0: setup: ignoring unsupported mode bits a00 >>> >>> What SPI controller is used in this case? >>> >>>> [ 0.732850] spi-nor spi0.0: unrecognized JEDEC id bytes: 68 40 18 68 40 18 >>>> [ 0.739725] spi-nor: probe of spi0.0 failed with error -2 >>> >>> And what kernel version is this? This should only happen if the SFDP >>> header is wrong, but according to your dump, it is correct. >>> >> >> I assume the test was done on an older kernel, where the identification >> of the flash based on SFDP is not yet available. > > The test has been done on 5.15 and 6.1. The support for this was > introduced later? Can you point me to the commits so I can backport > them? > Use git blame. 773bbe104497 ("mtd: spi-nor: add generic flash driver")
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..8ba4f3a32040 --- /dev/null +++ b/drivers/mtd/spi-nor/bohong.c @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <linux/mtd/spi-nor.h> + +#include "core.h" + +static const struct flash_info bohong_parts[] = { + { + .id = SNOR_ID(0x68, 0x40, 0x18), + .size = SZ_16M, + .no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ, + }, +}; + +const struct spi_nor_manufacturer spi_nor_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;