| Message ID | 20221023065838.53616-1-mika.westerberg@linux.intel.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Ambarus Tudor |
| Headers | show |
| Series | [v2] mtd: spi-nor: gigadevice: Add support for gd25lr256e | expand |
On 10/23/22 09:58, Mika Westerberg wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Add support for this 32MB serial flash. > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > --- > The previous version of the patch can be found here: > > https://lore.kernel.org/linux-mtd/20220922135646.49906-1-mika.westerberg@linux.intel.com/ > > This version uses SFDP instead as the chip supports it just fine. > > drivers/mtd/spi-nor/gigadevice.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/mtd/spi-nor/gigadevice.c b/drivers/mtd/spi-nor/gigadevice.c > index 119b38e6fc2a..29ae4048bb0f 100644 > --- a/drivers/mtd/spi-nor/gigadevice.c > +++ b/drivers/mtd/spi-nor/gigadevice.c > @@ -57,6 +57,9 @@ static const struct flash_info gigadevice_nor_parts[] = { > FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_TB_SR_BIT6) > FIXUP_FLAGS(SPI_NOR_4B_OPCODES) > .fixups = &gd25q256_fixups }, > + { "gd25lr256e", INFO(0xc86719, 0, 64 * 1024, 512) > + PARSE_SFDP > + FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) }, > }; > > const struct spi_nor_manufacturer spi_nor_gigadevice = { Hi, Mika! When submitting flash updates or new flash additions, we require contributors to do a little test using mtd-utils and to dump the SPI NOR sysfs entries. Would you please do that? Here's the simple test: Run the test_qspi.sh script: #!/bin/sh dd if=/dev/urandom of=./qspi_test bs=1M count=6 mtd_debug write /dev/mtd5 0 6291456 qspi_test mtd_debug erase /dev/mtd5 0 6291456 mtd_debug read /dev/mtd5 0 6291456 qspi_read hexdump qspi_read mtd_debug write /dev/mtd5 0 6291456 qspi_test mtd_debug read /dev/mtd5 0 6291456 qspi_read sha1sum qspi_test qspi_read The two SHA-1 sums must be the same to pass this test. Here's an example on how to dumps the sysfs entries: zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/partname s25hl02gt zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id 342a1c0f0090 zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer spansion zynq> xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp 53464450080104ff00080114000100ff84000102500100ff81000118e001 00ff8700011c580100ff88000106c80100ffffffffffffffffffffffffff ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff ffffffffffffffffffffffffffffffffe720faffffffff7f48eb086b00ff 88bbfeffffffffff00ffffff48eb0c2000ff00ff12d823faff8b82e7ffec ec031c608a857a75f766805c8cd6ddfff938c0a1000000000000bc000000 0000f7f5ffff7b920ffe21ffffdc0000800000000000c0ffc3fbc8ffe3fb 00650090066500b10065009600650095716503d0716503d000000000b02e 000088a489aa716503967165039600000000000000000000000000000000 000000000000000000000000000000000000000000000000716505d57165 05d50000a015000080080000000800008010000000100000801800000018 fc65ff0804008000fc65ff0402008000fc65ff0804008008fd65ff040200 8008fe0202fff1ff0100f8ff0100f8fffb0ffe0902fff8fffb0ff8ff0100 f1ff0100fe0104fff1ff0100f8ff0100f8fff70ff8ff0100f1ff0100ff0a 00fff8ffff0f zynq> md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp 86aef254bcfdf763bdb92e4c31667242 /sys/bus/spi/devices/spi0.0/spi-nor/sfdp Thanks!
Hi, On Mon, Nov 21, 2022 at 03:03:51PM +0000, Tudor.Ambarus@microchip.com wrote: > On 10/23/22 09:58, Mika Westerberg wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > Add support for this 32MB serial flash. > > > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > --- > > The previous version of the patch can be found here: > > > > https://lore.kernel.org/linux-mtd/20220922135646.49906-1-mika.westerberg@linux.intel.com/ > > > > This version uses SFDP instead as the chip supports it just fine. > > > > drivers/mtd/spi-nor/gigadevice.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/mtd/spi-nor/gigadevice.c b/drivers/mtd/spi-nor/gigadevice.c > > index 119b38e6fc2a..29ae4048bb0f 100644 > > --- a/drivers/mtd/spi-nor/gigadevice.c > > +++ b/drivers/mtd/spi-nor/gigadevice.c > > @@ -57,6 +57,9 @@ static const struct flash_info gigadevice_nor_parts[] = { > > FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_TB_SR_BIT6) > > FIXUP_FLAGS(SPI_NOR_4B_OPCODES) > > .fixups = &gd25q256_fixups }, > > + { "gd25lr256e", INFO(0xc86719, 0, 64 * 1024, 512) > > + PARSE_SFDP > > + FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) }, > > }; > > > > const struct spi_nor_manufacturer spi_nor_gigadevice = { > > Hi, Mika! > > When submitting flash updates or new flash additions, we require contributors > to do a little test using mtd-utils and to dump the SPI NOR sysfs entries. > Would you please do that? > > Here's the simple test: > > Run the test_qspi.sh script: > #!/bin/sh > > dd if=/dev/urandom of=./qspi_test bs=1M count=6 > mtd_debug write /dev/mtd5 0 6291456 qspi_test > mtd_debug erase /dev/mtd5 0 6291456 > mtd_debug read /dev/mtd5 0 6291456 qspi_read > hexdump qspi_read > mtd_debug write /dev/mtd5 0 6291456 qspi_test > mtd_debug read /dev/mtd5 0 6291456 qspi_read > sha1sum qspi_test qspi_read > > The two SHA-1 sums must be the same to pass this test. That SPI flash holds the system BIOS and I'm using it remotely now so there is no way to revive if something goes wrong so I wonder if it is OK to skip the above step? > Here's an example on how to dumps the sysfs entries: > zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/partname > s25hl02gt > zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id > 342a1c0f0090 > zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer > spansion > zynq> xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp > 53464450080104ff00080114000100ff84000102500100ff81000118e001 > 00ff8700011c580100ff88000106c80100ffffffffffffffffffffffffff > ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > ffffffffffffffffffffffffffffffffe720faffffffff7f48eb086b00ff > 88bbfeffffffffff00ffffff48eb0c2000ff00ff12d823faff8b82e7ffec > ec031c608a857a75f766805c8cd6ddfff938c0a1000000000000bc000000 > 0000f7f5ffff7b920ffe21ffffdc0000800000000000c0ffc3fbc8ffe3fb > 00650090066500b10065009600650095716503d0716503d000000000b02e > 000088a489aa716503967165039600000000000000000000000000000000 > 000000000000000000000000000000000000000000000000716505d57165 > 05d50000a015000080080000000800008010000000100000801800000018 > fc65ff0804008000fc65ff0402008000fc65ff0804008008fd65ff040200 > 8008fe0202fff1ff0100f8ff0100f8fffb0ffe0902fff8fffb0ff8ff0100 > f1ff0100fe0104fff1ff0100f8ff0100f8fff70ff8ff0100f1ff0100ff0a > 00fff8ffff0f These I was able to read: # cat /sys/bus/spi/devices/spi0.0/spi-nor/partname gd25lr256e # cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id c86719 # cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer gigadevice # xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp 53464450060103ff00060110300000ffc8000103900000ff84000102c000 00ff03000102e00000ffffffffffffffffffe520eaffffffff0f44eb086b 003b00bbfeffffffffff00ffffff44eb0c200f5210d800ff4362c9fe82e9 9c58ec6006337a757a7504bdd55c2906740008500001ffffffffffffffff ffffffffffffffffffffffffffffffffffffffffffffffff002050169df9 8156d9c8ffffffffffffffffffffffffffffffffffffffffffffffffffff fffffffffffffffffffffffff38ff0ff215cdcffffffffffffffffffffff ffffffffffffffffffffffffffff389b96f0e1a6b2ff
On 11/22/22 13:22, Mika Westerberg wrote: cut >> Here's the simple test: >> >> Run the test_qspi.sh script: >> #!/bin/sh >> >> dd if=/dev/urandom of=./qspi_test bs=1M count=6 >> mtd_debug write /dev/mtd5 0 6291456 qspi_test >> mtd_debug erase /dev/mtd5 0 6291456 >> mtd_debug read /dev/mtd5 0 6291456 qspi_read >> hexdump qspi_read >> mtd_debug write /dev/mtd5 0 6291456 qspi_test >> mtd_debug read /dev/mtd5 0 6291456 qspi_read >> sha1sum qspi_test qspi_read >> >> The two SHA-1 sums must be the same to pass this test. > > That SPI flash holds the system BIOS and I'm using it remotely now so > there is no way to revive if something goes wrong so I wonder if it is > OK to skip the above step? sure, it's fine, yes. Did you test the locking part of your patch? I'm asking because if you don't need the locking or you're not able to test it, then you can use the spi-nor-generic driver introduced by Michael at [1] and you won't need an entry for the flash at all. > cut > These I was able to read: > > # cat /sys/bus/spi/devices/spi0.0/spi-nor/partname > gd25lr256e > # cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id > c86719 > # cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer > gigadevice > # xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp > 53464450060103ff00060110300000ffc8000103900000ff84000102c000 > 00ff03000102e00000ffffffffffffffffffe520eaffffffff0f44eb086b > 003b00bbfeffffffffff00ffffff44eb0c200f5210d800ff4362c9fe82e9 > 9c58ec6006337a757a7504bdd55c2906740008500001ffffffffffffffff > ffffffffffffffffffffffffffffffffffffffffffffffff002050169df9 > 8156d9c8ffffffffffffffffffffffffffffffffffffffffffffffffffff > fffffffffffffffffffffffff38ff0ff215cdcffffffffffffffffffffff > ffffffffffffffffffffffffffff389b96f0e1a6b2ff Okay, thanks. [1] https://lore.kernel.org/linux-mtd/166903807811.85501.6803386075881922742.b4-ty@microchip.com/T/#t
Hi, On Tue, Nov 22, 2022 at 11:59:47AM +0000, Tudor.Ambarus@microchip.com wrote: > sure, it's fine, yes. Did you test the locking part of your patch? > I'm asking because if you don't need the locking or you're not able to test > it, then you can use the spi-nor-generic driver introduced by Michael at > [1] and you won't need an entry for the flash at all. Nice! I think this one should be enough for us. It's coming for v6.2 right?
On 11/22/22 14:25, Mika Westerberg wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Hi, > > On Tue, Nov 22, 2022 at 11:59:47AM +0000, Tudor.Ambarus@microchip.com wrote: >> sure, it's fine, yes. Did you test the locking part of your patch? >> I'm asking because if you don't need the locking or you're not able to test >> it, then you can use the spi-nor-generic driver introduced by Michael at >> [1] and you won't need an entry for the flash at all. > > Nice! I think this one should be enough for us. It's coming for v6.2 > right? yes, I queued the support.
diff --git a/drivers/mtd/spi-nor/gigadevice.c b/drivers/mtd/spi-nor/gigadevice.c index 119b38e6fc2a..29ae4048bb0f 100644 --- a/drivers/mtd/spi-nor/gigadevice.c +++ b/drivers/mtd/spi-nor/gigadevice.c @@ -57,6 +57,9 @@ static const struct flash_info gigadevice_nor_parts[] = { FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_TB_SR_BIT6) FIXUP_FLAGS(SPI_NOR_4B_OPCODES) .fixups = &gd25q256_fixups }, + { "gd25lr256e", INFO(0xc86719, 0, 64 * 1024, 512) + PARSE_SFDP + FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) }, }; const struct spi_nor_manufacturer spi_nor_gigadevice = {
Add support for this 32MB serial flash. Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- The previous version of the patch can be found here: https://lore.kernel.org/linux-mtd/20220922135646.49906-1-mika.westerberg@linux.intel.com/ This version uses SFDP instead as the chip supports it just fine. drivers/mtd/spi-nor/gigadevice.c | 3 +++ 1 file changed, 3 insertions(+)