| Message ID | 20220920184808.44876-1-sudip.mukherjee@sifive.com |
|---|---|
| State | Accepted |
| Delegated to: | Ambarus Tudor |
| Headers | show |
| Series | [v3,1/2] mtd: spi-nor: issi: is25wp256: Init flash based on SFDP | expand |
On Tue, Sep 20, 2022 at 7:48 PM Sudip Mukherjee <sudip.mukherjee@sifive.com> wrote: > > The datasheet of is25wp256 says it supports SFDP. Get rid of the static > initialization of the flash parameters and init them when parsing SFDP. > > Testing showed the flash using SPINOR_OP_READ_1_1_4_4B 0x6c, > SPINOR_OP_PP_4B 0x12 and SPINOR_OP_BE_4K_4B 0x21 before enabling SFDP. > After this patch, it parses the SFDP information and still uses the > same opcodes. > > Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com> > --- > > No change from v2, just resending again with the next patch. > Results from the tests given by Tudor in the following mail. Test result after this patch: # dd if=/dev/urandom of=./qspi_test bs=1M count=6 6+0 records in 6+0 records out # mtd_debug write /dev/mtd4 0 6291456 qspi_test Copied 6291456 bytes from qspi_test to address 0x00000000 in flash # mtd_debug erase /dev/mtd4 0 6291456 Erased 6291456 bytes from address 0x00000000 in flash # mtd_debug read /dev/mtd4 0 6291456 qspi_read Copied 6291456 bytes from address 0x00000000 in flash to qspi_read # hexdump qspi_read 0000000 ffff ffff ffff ffff ffff ffff ffff ffff * 0600000 # mtd_debug write /dev/mtd4 0 6291456 qspi_test Copied 6291456 bytes from qspi_test to address 0x00000000 in flash # mtd_debug read /dev/mtd4 0 6291456 qspi_read Copied 6291456 bytes from address 0x00000000 in flash to qspi_read # sha1sum qspi_test qspi_read 57f8d4fee65622104e24276e865f662844f12242 qspi_test 57f8d4fee65622104e24276e865f662844f12242 qspi_read # cat /sys/bus/spi/devices/spi0.0/spi-nor/partname is25wp256 # cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id 9d7019 # cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer issi # xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp 53464450060101ff00060110300000ff9d05010380000002ffffffffffff ffffffffffffffffffffffffffffffffffffe520f9ffffffff0f44eb086b 083b80bbfeffffffffff00ffffff44eb0c200f5210d800ff234ac90082d8 11cecccd68467a757a75f7aed55c4a422cfff030faa9ffffffffffffffff ffffffffffffffff501950169ff9c0648fefffff # md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp ba14818b9ec42713f24d94d66bb90ba0 /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
Am 2022-09-20 20:48, schrieb Sudip Mukherjee: > The datasheet of is25wp256 says it supports SFDP. Get rid of the static > initialization of the flash parameters and init them when parsing SFDP. > > Testing showed the flash using SPINOR_OP_READ_1_1_4_4B 0x6c, > SPINOR_OP_PP_4B 0x12 and SPINOR_OP_BE_4K_4B 0x21 before enabling SFDP. > After this patch, it parses the SFDP information and still uses the > same opcodes. > > Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com> > --- > > No change from v2, just resending again with the next patch. > Results from the tests given by Tudor in the following mail. > > > drivers/mtd/spi-nor/issi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c > index 89a66a19d754f..8b48459b5054c 100644 > --- a/drivers/mtd/spi-nor/issi.c > +++ b/drivers/mtd/spi-nor/issi.c > @@ -71,7 +71,7 @@ static const struct flash_info issi_nor_parts[] = { > { "is25wp128", INFO(0x9d7018, 0, 64 * 1024, 256) > NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, > { "is25wp256", INFO(0x9d7019, 0, 64 * 1024, 512) I'm curious, does it also work with: { "is25wp256", INFO(0x9d7019, 0, 0, 0) PARSE_SFDP -michael > - NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) > + PARSE_SFDP > FIXUP_FLAGS(SPI_NOR_4B_OPCODES) > .fixups = &is25lp256_fixups },
HI Michael, On Thu, Sep 22, 2022 at 3:19 PM Michael Walle <michael@walle.cc> wrote: > > Am 2022-09-20 20:48, schrieb Sudip Mukherjee: > > The datasheet of is25wp256 says it supports SFDP. Get rid of the static > > initialization of the flash parameters and init them when parsing SFDP. > > > > Testing showed the flash using SPINOR_OP_READ_1_1_4_4B 0x6c, > > SPINOR_OP_PP_4B 0x12 and SPINOR_OP_BE_4K_4B 0x21 before enabling SFDP. > > After this patch, it parses the SFDP information and still uses the > > same opcodes. > > > > Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com> > > --- > > > > No change from v2, just resending again with the next patch. > > Results from the tests given by Tudor in the following mail. > > > > > > drivers/mtd/spi-nor/issi.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c > > index 89a66a19d754f..8b48459b5054c 100644 > > --- a/drivers/mtd/spi-nor/issi.c > > +++ b/drivers/mtd/spi-nor/issi.c > > @@ -71,7 +71,7 @@ static const struct flash_info issi_nor_parts[] = { > > { "is25wp128", INFO(0x9d7018, 0, 64 * 1024, 256) > > NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, > > { "is25wp256", INFO(0x9d7019, 0, 64 * 1024, 512) > > I'm curious, does it also work with: > { "is25wp256", INFO(0x9d7019, 0, 0, 0) > PARSE_SFDP Applied this on top of my patch series: diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c index 014cd9038bed..a0ddad2afffc 100644 --- a/drivers/mtd/spi-nor/issi.c +++ b/drivers/mtd/spi-nor/issi.c @@ -70,7 +70,7 @@ static const struct flash_info issi_nor_parts[] = { NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, { "is25wp128", INFO(0x9d7018, 0, 64 * 1024, 256) NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, - { "is25wp256", INFO(0x9d7019, 0, 64 * 1024, 512) + { "is25wp256", INFO(0x9d7019, 0, 0, 0) PARSE_SFDP FIXUP_FLAGS(SPI_NOR_4B_OPCODES) FLAGS(SPI_NOR_QUAD_PP) And, I can see it working. dmesg showed: [ 35.299888] spi-nor spi0.0: is25wp256 (32768 Kbytes) [ 35.403858] spi-nor spi0.0: mtd .name = spi0.0, .size = 0x2000000 (32MiB), .erasesize = 0x00001000 (4KiB) .numeraseregions = 0 The system booted and I can see all the partitions.
On Tue, 20 Sep 2022 19:48:07 +0100, Sudip Mukherjee wrote: > The datasheet of is25wp256 says it supports SFDP. Get rid of the static > initialization of the flash parameters and init them when parsing SFDP. > > Testing showed the flash using SPINOR_OP_READ_1_1_4_4B 0x6c, > SPINOR_OP_PP_4B 0x12 and SPINOR_OP_BE_4K_4B 0x21 before enabling SFDP. > After this patch, it parses the SFDP information and still uses the > same opcodes. > > [...] Applied to spi-nor/next, thanks! [1/2] mtd: spi-nor: issi: is25wp256: Init flash based on SFDP https://git.kernel.org/mtd/c/a30f53d8bc0fi [tudor.ambarus@microchip.com: set sector_size and n_sectors to zero] [2/2] mtd: spi-nor: add SFDP fixups for Quad Page Program https://git.kernel.org/mtd/c/1799cd8540b6 [tudor.ambarus@microchip.com: move pp setting in spi_nor_init_default_params] Best regards,
Hi, On Tue, Sep 20, 2022 at 07:48:07PM +0100, Sudip Mukherjee wrote: > The datasheet of is25wp256 says it supports SFDP. Get rid of the static > initialization of the flash parameters and init them when parsing SFDP. > > Testing showed the flash using SPINOR_OP_READ_1_1_4_4B 0x6c, > SPINOR_OP_PP_4B 0x12 and SPINOR_OP_BE_4K_4B 0x21 before enabling SFDP. > After this patch, it parses the SFDP information and still uses the > same opcodes. > > Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com> With this patch in place, qemu emulations with is25wp256 fail to instantiate the flash. Specifically, this affects the qemu sifive_u emulation. The resulting error message is: spi-nor spi0.0: BFPT parsing failed. Please consider using SPI_NOR_SKIP_SFDP when declaring the flash spi-nor: probe of spi0.0 failed with error -22 qemu command line: qemu-system-riscv64 -M sifive_u -m 512M \ -no-reboot -kernel arch/riscv/boot/Image -snapshot \ -drive file=flash.8Vtb7,format=raw,if=mtd -bios default \ -append "root=/dev/mtdblock0 mtdparts=spi0.0:- console=ttySIF0,115200 earlycon" \ -nographic -monitor none Reverting this patch fixes the problem. Guenter --- # bad: [1e4fa020d574768445fca2d9bbfe473ec8bbd224] Merge tag 'mtd/for-6.2' of git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux # good: [a594533df0f6ca391da003f43d53b336a2d23ffa] Merge tag 'drm-next-2022-12-13' of git://anongit.freedesktop.org/drm/drm git bisect start '1e4fa020d574' 'a594533df0f6' # good: [2399401feee27c639addc5b7e6ba519d3ca341bf] mtd: maps: pxa2xx-flash: fix memory leak in probe git bisect good 2399401feee27c639addc5b7e6ba519d3ca341bf # good: [bcc0c61e6134066f4629845691a514ea33465653] mtd: spi-nor: micron-st: Enable locking for mt25qu256a git bisect good bcc0c61e6134066f4629845691a514ea33465653 # good: [a50ae8c98e5766a4fcb78e76f13cc658b784eac1] mtd: nand: drop EXPORT_SYMBOL_GPL for nanddev_erase() git bisect good a50ae8c98e5766a4fcb78e76f13cc658b784eac1 # good: [6408cc05a50aaf88074a5a31d065e5af87a456f5] mtd: rawnand: Drop obsolete dependencies on COMPILE_TEST git bisect good 6408cc05a50aaf88074a5a31d065e5af87a456f5 # bad: [1799cd8540b67b88514c82f5fae1c75b986bcbd8] mtd: spi-nor: add SFDP fixups for Quad Page Program git bisect bad 1799cd8540b67b88514c82f5fae1c75b986bcbd8 # bad: [a30f53d8bc0f9b55b4e8eea0e17b68cfd1f07f34] mtd: spi-nor: issi: is25wp256: Init flash based on SFDP git bisect bad a30f53d8bc0f9b55b4e8eea0e17b68cfd1f07f34 # good: [ef434f08b0562069cf431873a052692357d325a1] mtd: spi-nor: winbond: add support for W25Q512NW-IQ git bisect good ef434f08b0562069cf431873a052692357d325a1 # first bad commit: [a30f53d8bc0f9b55b4e8eea0e17b68cfd1f07f34] mtd: spi-nor: issi: is25wp256: Init flash based on SFDP
On 21.12.2022 02:30, Guenter Roeck wrote: > Hi, Hi, Guenter, Thanks for the report. > > On Tue, Sep 20, 2022 at 07:48:07PM +0100, Sudip Mukherjee wrote: >> The datasheet of is25wp256 says it supports SFDP. Get rid of the static >> initialization of the flash parameters and init them when parsing SFDP. >> >> Testing showed the flash using SPINOR_OP_READ_1_1_4_4B 0x6c, >> SPINOR_OP_PP_4B 0x12 and SPINOR_OP_BE_4K_4B 0x21 before enabling SFDP. >> After this patch, it parses the SFDP information and still uses the >> same opcodes. >> >> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com> > > With this patch in place, qemu emulations with is25wp256 fail to > instantiate the flash. Specifically, this affects the qemu sifive_u > emulation. The resulting error message is: > > spi-nor spi0.0: BFPT parsing failed. Please consider using SPI_NOR_SKIP_SFDP when declaring the flash > spi-nor: probe of spi0.0 failed with error -22 > > qemu command line: > > qemu-system-riscv64 -M sifive_u -m 512M \ > -no-reboot -kernel arch/riscv/boot/Image -snapshot \ > -drive file=flash.8Vtb7,format=raw,if=mtd -bios default \ > -append "root=/dev/mtdblock0 mtdparts=spi0.0:- console=ttySIF0,115200 earlycon" \ > -nographic -monitor none > > Reverting this patch fixes the problem. I need some more debug info in order to have both flavors of the flash work correctly. Would you please revert the patch locally and do the tests from below? Thanks! ta --- # dd if=/dev/urandom of=./qspi_test bs=1M count=6 6+0 records in 6+0 records out # mtd_debug write /dev/mtd4 0 6291456 qspi_test Copied 6291456 bytes from qspi_test to address 0x00000000 in flash # mtd_debug erase /dev/mtd4 0 6291456 Erased 6291456 bytes from address 0x00000000 in flash # mtd_debug read /dev/mtd4 0 6291456 qspi_read Copied 6291456 bytes from address 0x00000000 in flash to qspi_read # hexdump qspi_read 0000000 ffff ffff ffff ffff ffff ffff ffff ffff * 0600000 # mtd_debug write /dev/mtd4 0 6291456 qspi_test Copied 6291456 bytes from qspi_test to address 0x00000000 in flash # mtd_debug read /dev/mtd4 0 6291456 qspi_read Copied 6291456 bytes from address 0x00000000 in flash to qspi_read # sha1sum qspi_test qspi_read 57f8d4fee65622104e24276e865f662844f12242 qspi_test 57f8d4fee65622104e24276e865f662844f12242 qspi_read # cat /sys/bus/spi/devices/spi0.0/spi-nor/partname is25wp256 # cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id 9d7019 # cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer issi # xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp 53464450060101ff00060110300000ff9d05010380000002ffffffffffff ffffffffffffffffffffffffffffffffffffe520f9ffffffff0f44eb086b 083b80bbfeffffffffff00ffffff44eb0c200f5210d800ff234ac90082d8 11cecccd68467a757a75f7aed55c4a422cfff030faa9ffffffffffffffff ffffffffffffffff501950169ff9c0648fefffff # md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp ba14818b9ec42713f24d94d66bb90ba0 /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
Hi, Am 2022-12-21 01:30, schrieb Guenter Roeck: > With this patch in place, qemu emulations with is25wp256 fail to > instantiate the flash. Specifically, this affects the qemu sifive_u > emulation. The resulting error message is: > > spi-nor spi0.0: BFPT parsing failed. Please consider using > SPI_NOR_SKIP_SFDP when declaring the flash > spi-nor: probe of spi0.0 failed with error -22 qemu is to blame here. It doesn't fully emulate the flash. Esp. not the SFDP tables. See also [1]. Adding the .sfdp_read hook with the corresponding binary [2] should fix your issue. [1] https://lore.kernel.org/qemu-devel/7827c791b7f91f4eb216b33473dd6103@walle.cc/ [2] https://lore.kernel.org/linux-mtd/CAHyZL-ch=mr0x_19e6P9G+Z4sW2iJXW9nTnXUNAhn+pTptN2ww@mail.gmail.com/ -michael
On 21.12.2022 09:03, Tudor Ambarus wrote: > Would you please revert the patch locally and do the > tests from below? Thanks! Disregard this, I thought you have a real flash. Michael is right about qemu.
Hi Michael, On Wed, Dec 21, 2022 at 09:22:04AM +0100, Michael Walle wrote: > Hi, > > Am 2022-12-21 01:30, schrieb Guenter Roeck: > > With this patch in place, qemu emulations with is25wp256 fail to > > instantiate the flash. Specifically, this affects the qemu sifive_u > > emulation. The resulting error message is: > > > > spi-nor spi0.0: BFPT parsing failed. Please consider using > > SPI_NOR_SKIP_SFDP when declaring the flash > > spi-nor: probe of spi0.0 failed with error -22 > > qemu is to blame here. It doesn't fully emulate the flash. Esp. not > the SFDP tables. See also [1]. Adding the .sfdp_read hook with the > corresponding binary [2] should fix your issue. > > [1] > https://lore.kernel.org/qemu-devel/7827c791b7f91f4eb216b33473dd6103@walle.cc/ > [2] https://lore.kernel.org/linux-mtd/CAHyZL-ch=mr0x_19e6P9G+Z4sW2iJXW9nTnXUNAhn+pTptN2ww@mail.gmail.com/ > Great, that helped. I'll prepare and send a qemu patch. Thanks, Guenter
diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c index 89a66a19d754f..8b48459b5054c 100644 --- a/drivers/mtd/spi-nor/issi.c +++ b/drivers/mtd/spi-nor/issi.c @@ -71,7 +71,7 @@ static const struct flash_info issi_nor_parts[] = { { "is25wp128", INFO(0x9d7018, 0, 64 * 1024, 256) NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, { "is25wp256", INFO(0x9d7019, 0, 64 * 1024, 512) - NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) + PARSE_SFDP FIXUP_FLAGS(SPI_NOR_4B_OPCODES) .fixups = &is25lp256_fixups },
The datasheet of is25wp256 says it supports SFDP. Get rid of the static initialization of the flash parameters and init them when parsing SFDP. Testing showed the flash using SPINOR_OP_READ_1_1_4_4B 0x6c, SPINOR_OP_PP_4B 0x12 and SPINOR_OP_BE_4K_4B 0x21 before enabling SFDP. After this patch, it parses the SFDP information and still uses the same opcodes. Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com> --- No change from v2, just resending again with the next patch. Results from the tests given by Tudor in the following mail. drivers/mtd/spi-nor/issi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)