| Message ID | 20190417054511.29291-1-andrew@aj.id.au |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Revert "mtd: spi-nor: Add a post BFPT fixup for MX25L25635E" | expand |
Ugh, that subject should have had "linux dev-5.0" in it, but y'know. I'll survive. On Wed, 17 Apr 2019, at 15:16, Andrew Jeffery wrote: > This reverts commit 2bffa65da43e399079dad5947c6aa9ab3cfa4ad4. > > 5.0.x fails to boot on Witherspoon machines after an AC-cycle, with the > following error: > > > [ 4.337766] ubi0: default fastmap pool size: 25 > > [ 4.342321] ubi0: default fastmap WL pool size: 12 > > [ 4.347232] ubi0: attaching mtd3 > > [ 4.361487] ubi0: scanning is finished > > [ 4.365525] ubi0 error: ubi_read_volume_table: the layout volume was not found > > [ 4.372989] ubi0 error: ubi_attach_mtd_dev: failed to attach mtd3, error -22 > > [ 4.380193] UBI error: cannot attach mtd3 > > Which leads to: > > > [ 4.553478] VFS: Cannot open root device "ubiblock0_1" or unknown-block(0,0): error -6 > > [ 4.561550] Please append a correct "root=" boot option; here are the available partitions: > > [ 4.570029] 1f00 32768 mtdblock0 > > [ 4.570038] (driver?) > > [ 4.576671] 1f01 384 mtdblock1 > > [ 4.576680] (driver?) > > [ 4.583234] 1f02 128 mtdblock2 > > [ 4.583240] (driver?) > > [ 4.589883] 1f03 32256 mtdblock3 > > [ 4.589892] (driver?) > > [ 4.596520] 1f04 32768 mtdblock4 > > [ 4.596529] (driver?) > > [ 4.603080] 1f05 384 mtdblock5 > > [ 4.603085] (driver?) > > [ 4.609711] 1f06 128 mtdblock6 > > [ 4.609719] (driver?) > > [ 4.616338] 1f07 32256 mtdblock7 > > [ 4.616347] (driver?) > > [ 4.622901] 1f08 131072 mtdblock8 > > [ 4.622907] (driver?) > > [ 4.629539] Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0) > > A hint is provided earlier in the boot process: > > > [ 0.920597] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency > > [ 0.926753] aspeed-smc 1e620000.spi: mx25l25635e (32768 Kbytes) > > [ 0.932704] aspeed-smc 1e620000.spi: CE0 window [ 0x20000000 - 0x22000000 ] 32MB > > [ 0.940230] aspeed-smc 1e620000.spi: CE1 window [ 0x22000000 - 0x2a000000 ] 128MB > > [ 0.947812] aspeed-smc 1e620000.spi: read control register: 203c0641 > > [ 1.435022] 3 fixed-partitions partitions found on MTD device bmc > > [ 1.441249] Creating 3 MTD partitions on "bmc": > > [ 1.445911] 0x000000000000-0x000000060000 : "u-boot" > > [ 1.455264] 0x000000060000-0x000000080000 : "u-boot-env" > > [ 1.465042] 0x000000080000-0x000002000000 : "obmc-ubi" > > [ 1.474914] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency > > [ 1.481102] aspeed-smc 1e620000.spi: mx25l25635e (32768 Kbytes) > > [ 1.487175] aspeed-smc 1e620000.spi: CE1 window [ 0x22000000 - 0x24000000 ] 32MB > > [ 1.494592] aspeed-smc 1e620000.spi: CE2 window [ 0x24000000 - 0x2c000000 ] 128MB > > [ 1.502173] aspeed-smc 1e620000.spi: read control register: 203c0041 > > [ 1.705799] aspeed-smc 1e620000.spi: No good frequency, using dumb slow > > [ 1.717393] 3 fixed-partitions partitions found on MTD device alt-bmc > > [ 1.723851] Creating 3 MTD partitions on "alt-bmc": > > [ 1.728890] 0x000000000000-0x000000060000 : "alt-u-boot" > > [ 1.738950] 0x000000060000-0x000000080000 : "alt-u-boot-env" > > [ 1.748975] 0x000000080000-0x000002000000 : "alt-obmc-ubi" > > Specifically, the control register state is exposed as: > > > [ 0.947812] aspeed-smc 1e620000.spi: read control register: 203c0641 > > The 0x3c opcode translates to DREAD4B for the mx25l25635f part, which is > what is detected. The post BFPT fixup implemented in commit 2bffa65da43e > ("mtd: spi-nor: Add a post BFPT fixup for MX25L25635E") detects the 'e' > vs 'f' part based on the reported support for > BFPT_DWORD5_FAST_READ_4_4_4, which implies 4B opcodes are supported > (e.g. DREAD4B), however the aspeed-smc driver fails to put the > controller into the right addressing mode to support sending DREAD4B, and > so we reach a bad state. > > In the interim, revert the upstream patch adding 'e' vs 'f' detection to > avoid sending 4B opcodes. > > The revert has been tested on a freshly flashed and power-cycled > Witherspoon, both with and without the patch applied, confirming the > broken behaviour in the former case and a successful boot in the latter. > > Cc: Andrew Geissler <geissonator@gmail.com> > Reported-by: George Keishing <gkeishin@in.ibm.com> > Investigated-by: Cédric Le Goater <clg@kaod.org> > Investigated-by: Eddie James <eajames@linux.ibm.com> > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > --- > drivers/mtd/spi-nor/spi-nor.c | 29 +---------------------------- > 1 file changed, 1 insertion(+), 28 deletions(-) > > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c > index 5a5a47651bf9..9f85ab5fc569 100644 > --- a/drivers/mtd/spi-nor/spi-nor.c > +++ b/drivers/mtd/spi-nor/spi-nor.c > @@ -1681,31 +1681,6 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor) > .addr_width = 3, \ > .flags = SPI_NOR_NO_FR | SPI_S3AN, > > -static int > -mx25l25635_post_bfpt_fixups(struct spi_nor *nor, > - const struct sfdp_parameter_header *bfpt_header, > - const struct sfdp_bfpt *bfpt, > - struct spi_nor_flash_parameter *params) > -{ > - /* > - * MX25L25635F supports 4B opcodes but MX25L25635E does not. > - * Unfortunately, Macronix has re-used the same JEDEC ID for both > - * variants which prevents us from defining a new entry in the parts > - * table. > - * We need a way to differentiate MX25L25635E and MX25L25635F, and it > - * seems that the F version advertises support for Fast Read 4-4-4 in > - * its BFPT table. > - */ > - if (bfpt->dwords[BFPT_DWORD(5)] & BFPT_DWORD5_FAST_READ_4_4_4) > - nor->flags |= SNOR_F_4B_OPCODES; > - > - return 0; > -} > - > -static struct spi_nor_fixups mx25l25635_fixups = { > - .post_bfpt = mx25l25635_post_bfpt_fixups, > -}; > - > /* NOTE: double check command sets and memory organization when you add > * more nor chips. This current list focusses on newer chips, which > * have been converging on command sets which including JEDEC ID. > @@ -1843,9 +1818,7 @@ static const struct flash_info spi_nor_ids[] = { > { "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 0) }, > { "mx25u12835f", INFO(0xc22538, 0, 64 * 1024, 256, > SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, > - { "mx25l25635e", INFO(0xc22019, 0, 64 * 1024, 512, > - SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) > - .fixups = &mx25l25635_fixups }, > + { "mx25l25635e", INFO(0xc22019, 0, 64 * 1024, 512, SPI_NOR_DUAL_READ > | SPI_NOR_QUAD_READ) }, > { "mx25u25635f", INFO(0xc22539, 0, 64 * 1024, 512, SECT_4K | > SPI_NOR_4B_OPCODES) }, > { "mx25l25655e", INFO(0xc22619, 0, 64 * 1024, 512, 0) }, > { "mx66l51235f", INFO(0xc2201a, 0, 64 * 1024, 1024, SPI_NOR_DUAL_READ > | SPI_NOR_QUAD_READ) }, > -- > 2.19.1 > >
On 4/17/19 7:45 AM, Andrew Jeffery wrote: > This reverts commit 2bffa65da43e399079dad5947c6aa9ab3cfa4ad4. > > 5.0.x fails to boot on Witherspoon machines after an AC-cycle, with the > following error: > >> [ 4.337766] ubi0: default fastmap pool size: 25 >> [ 4.342321] ubi0: default fastmap WL pool size: 12 >> [ 4.347232] ubi0: attaching mtd3 >> [ 4.361487] ubi0: scanning is finished >> [ 4.365525] ubi0 error: ubi_read_volume_table: the layout volume was not found >> [ 4.372989] ubi0 error: ubi_attach_mtd_dev: failed to attach mtd3, error -22 >> [ 4.380193] UBI error: cannot attach mtd3 > > Which leads to: > >> [ 4.553478] VFS: Cannot open root device "ubiblock0_1" or unknown-block(0,0): error -6 >> [ 4.561550] Please append a correct "root=" boot option; here are the available partitions: >> [ 4.570029] 1f00 32768 mtdblock0 >> [ 4.570038] (driver?) >> [ 4.576671] 1f01 384 mtdblock1 >> [ 4.576680] (driver?) >> [ 4.583234] 1f02 128 mtdblock2 >> [ 4.583240] (driver?) >> [ 4.589883] 1f03 32256 mtdblock3 >> [ 4.589892] (driver?) >> [ 4.596520] 1f04 32768 mtdblock4 >> [ 4.596529] (driver?) >> [ 4.603080] 1f05 384 mtdblock5 >> [ 4.603085] (driver?) >> [ 4.609711] 1f06 128 mtdblock6 >> [ 4.609719] (driver?) >> [ 4.616338] 1f07 32256 mtdblock7 >> [ 4.616347] (driver?) >> [ 4.622901] 1f08 131072 mtdblock8 >> [ 4.622907] (driver?) >> [ 4.629539] Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0) > > A hint is provided earlier in the boot process: > >> [ 0.920597] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency >> [ 0.926753] aspeed-smc 1e620000.spi: mx25l25635e (32768 Kbytes) >> [ 0.932704] aspeed-smc 1e620000.spi: CE0 window [ 0x20000000 - 0x22000000 ] 32MB >> [ 0.940230] aspeed-smc 1e620000.spi: CE1 window [ 0x22000000 - 0x2a000000 ] 128MB >> [ 0.947812] aspeed-smc 1e620000.spi: read control register: 203c0641 >> [ 1.435022] 3 fixed-partitions partitions found on MTD device bmc >> [ 1.441249] Creating 3 MTD partitions on "bmc": >> [ 1.445911] 0x000000000000-0x000000060000 : "u-boot" >> [ 1.455264] 0x000000060000-0x000000080000 : "u-boot-env" >> [ 1.465042] 0x000000080000-0x000002000000 : "obmc-ubi" >> [ 1.474914] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency >> [ 1.481102] aspeed-smc 1e620000.spi: mx25l25635e (32768 Kbytes) >> [ 1.487175] aspeed-smc 1e620000.spi: CE1 window [ 0x22000000 - 0x24000000 ] 32MB >> [ 1.494592] aspeed-smc 1e620000.spi: CE2 window [ 0x24000000 - 0x2c000000 ] 128MB >> [ 1.502173] aspeed-smc 1e620000.spi: read control register: 203c0041 >> [ 1.705799] aspeed-smc 1e620000.spi: No good frequency, using dumb slow >> [ 1.717393] 3 fixed-partitions partitions found on MTD device alt-bmc >> [ 1.723851] Creating 3 MTD partitions on "alt-bmc": >> [ 1.728890] 0x000000000000-0x000000060000 : "alt-u-boot" >> [ 1.738950] 0x000000060000-0x000000080000 : "alt-u-boot-env" >> [ 1.748975] 0x000000080000-0x000002000000 : "alt-obmc-ubi" > > Specifically, the control register state is exposed as: > >> [ 0.947812] aspeed-smc 1e620000.spi: read control register: 203c0641 > > The 0x3c opcode translates to DREAD4B for the mx25l25635f part, which is > what is detected. The post BFPT fixup implemented in commit 2bffa65da43e > ("mtd: spi-nor: Add a post BFPT fixup for MX25L25635E") detects the 'e' > vs 'f' part based on the reported support for > BFPT_DWORD5_FAST_READ_4_4_4, which implies 4B opcodes are supported > (e.g. DREAD4B), however the aspeed-smc driver fails to put the > controller into the right addressing mode to support sending DREAD4B, and > so we reach a bad state. It's more complex than that but I haven't quite cornered the problem. When doing the link training, the first read for the golden image is done in slow mode and it misses the first byte for some reason. So the gold image is incorrect (cough) but other images are correct (cough cough) Why is this first byte being dropped is the question. The flash module might have entered 4B mode ? is it expecting a dummy ? For the moment, this is fine for a workaround but we will need to fully support 4B opcodes (which we do without training). Another solution is to use the kernel command line switch "optimize_read=false" and all works fine, perfectly fine. Which is kind of frustrating. Thanks for the patch. C. > > In the interim, revert the upstream patch adding 'e' vs 'f' detection to > avoid sending 4B opcodes. > > The revert has been tested on a freshly flashed and power-cycled > Witherspoon, both with and without the patch applied, confirming the > broken behaviour in the former case and a successful boot in the latter. > > Cc: Andrew Geissler <geissonator@gmail.com> > Reported-by: George Keishing <gkeishin@in.ibm.com> > Investigated-by: Cédric Le Goater <clg@kaod.org> > Investigated-by: Eddie James <eajames@linux.ibm.com> > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > --- > drivers/mtd/spi-nor/spi-nor.c | 29 +---------------------------- > 1 file changed, 1 insertion(+), 28 deletions(-) > > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c > index 5a5a47651bf9..9f85ab5fc569 100644 > --- a/drivers/mtd/spi-nor/spi-nor.c > +++ b/drivers/mtd/spi-nor/spi-nor.c > @@ -1681,31 +1681,6 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor) > .addr_width = 3, \ > .flags = SPI_NOR_NO_FR | SPI_S3AN, > > -static int > -mx25l25635_post_bfpt_fixups(struct spi_nor *nor, > - const struct sfdp_parameter_header *bfpt_header, > - const struct sfdp_bfpt *bfpt, > - struct spi_nor_flash_parameter *params) > -{ > - /* > - * MX25L25635F supports 4B opcodes but MX25L25635E does not. > - * Unfortunately, Macronix has re-used the same JEDEC ID for both > - * variants which prevents us from defining a new entry in the parts > - * table. > - * We need a way to differentiate MX25L25635E and MX25L25635F, and it > - * seems that the F version advertises support for Fast Read 4-4-4 in > - * its BFPT table. > - */ > - if (bfpt->dwords[BFPT_DWORD(5)] & BFPT_DWORD5_FAST_READ_4_4_4) > - nor->flags |= SNOR_F_4B_OPCODES; > - > - return 0; > -} > - > -static struct spi_nor_fixups mx25l25635_fixups = { > - .post_bfpt = mx25l25635_post_bfpt_fixups, > -}; > - > /* NOTE: double check command sets and memory organization when you add > * more nor chips. This current list focusses on newer chips, which > * have been converging on command sets which including JEDEC ID. > @@ -1843,9 +1818,7 @@ static const struct flash_info spi_nor_ids[] = { > { "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 0) }, > { "mx25u12835f", INFO(0xc22538, 0, 64 * 1024, 256, > SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, > - { "mx25l25635e", INFO(0xc22019, 0, 64 * 1024, 512, > - SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) > - .fixups = &mx25l25635_fixups }, > + { "mx25l25635e", INFO(0xc22019, 0, 64 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, > { "mx25u25635f", INFO(0xc22539, 0, 64 * 1024, 512, SECT_4K | SPI_NOR_4B_OPCODES) }, > { "mx25l25655e", INFO(0xc22619, 0, 64 * 1024, 512, 0) }, > { "mx66l51235f", INFO(0xc2201a, 0, 64 * 1024, 1024, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, >
On Wed, 17 Apr 2019, at 15:41, Cédric Le Goater wrote: > On 4/17/19 7:45 AM, Andrew Jeffery wrote: > > This reverts commit 2bffa65da43e399079dad5947c6aa9ab3cfa4ad4. > > > > 5.0.x fails to boot on Witherspoon machines after an AC-cycle, with the > > following error: > > > >> [ 4.337766] ubi0: default fastmap pool size: 25 > >> [ 4.342321] ubi0: default fastmap WL pool size: 12 > >> [ 4.347232] ubi0: attaching mtd3 > >> [ 4.361487] ubi0: scanning is finished > >> [ 4.365525] ubi0 error: ubi_read_volume_table: the layout volume was not found > >> [ 4.372989] ubi0 error: ubi_attach_mtd_dev: failed to attach mtd3, error -22 > >> [ 4.380193] UBI error: cannot attach mtd3 > > > > Which leads to: > > > >> [ 4.553478] VFS: Cannot open root device "ubiblock0_1" or unknown-block(0,0): error -6 > >> [ 4.561550] Please append a correct "root=" boot option; here are the available partitions: > >> [ 4.570029] 1f00 32768 mtdblock0 > >> [ 4.570038] (driver?) > >> [ 4.576671] 1f01 384 mtdblock1 > >> [ 4.576680] (driver?) > >> [ 4.583234] 1f02 128 mtdblock2 > >> [ 4.583240] (driver?) > >> [ 4.589883] 1f03 32256 mtdblock3 > >> [ 4.589892] (driver?) > >> [ 4.596520] 1f04 32768 mtdblock4 > >> [ 4.596529] (driver?) > >> [ 4.603080] 1f05 384 mtdblock5 > >> [ 4.603085] (driver?) > >> [ 4.609711] 1f06 128 mtdblock6 > >> [ 4.609719] (driver?) > >> [ 4.616338] 1f07 32256 mtdblock7 > >> [ 4.616347] (driver?) > >> [ 4.622901] 1f08 131072 mtdblock8 > >> [ 4.622907] (driver?) > >> [ 4.629539] Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0) > > > > A hint is provided earlier in the boot process: > > > >> [ 0.920597] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency > >> [ 0.926753] aspeed-smc 1e620000.spi: mx25l25635e (32768 Kbytes) > >> [ 0.932704] aspeed-smc 1e620000.spi: CE0 window [ 0x20000000 - 0x22000000 ] 32MB > >> [ 0.940230] aspeed-smc 1e620000.spi: CE1 window [ 0x22000000 - 0x2a000000 ] 128MB > >> [ 0.947812] aspeed-smc 1e620000.spi: read control register: 203c0641 > >> [ 1.435022] 3 fixed-partitions partitions found on MTD device bmc > >> [ 1.441249] Creating 3 MTD partitions on "bmc": > >> [ 1.445911] 0x000000000000-0x000000060000 : "u-boot" > >> [ 1.455264] 0x000000060000-0x000000080000 : "u-boot-env" > >> [ 1.465042] 0x000000080000-0x000002000000 : "obmc-ubi" > >> [ 1.474914] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency > >> [ 1.481102] aspeed-smc 1e620000.spi: mx25l25635e (32768 Kbytes) > >> [ 1.487175] aspeed-smc 1e620000.spi: CE1 window [ 0x22000000 - 0x24000000 ] 32MB > >> [ 1.494592] aspeed-smc 1e620000.spi: CE2 window [ 0x24000000 - 0x2c000000 ] 128MB > >> [ 1.502173] aspeed-smc 1e620000.spi: read control register: 203c0041 > >> [ 1.705799] aspeed-smc 1e620000.spi: No good frequency, using dumb slow > >> [ 1.717393] 3 fixed-partitions partitions found on MTD device alt-bmc > >> [ 1.723851] Creating 3 MTD partitions on "alt-bmc": > >> [ 1.728890] 0x000000000000-0x000000060000 : "alt-u-boot" > >> [ 1.738950] 0x000000060000-0x000000080000 : "alt-u-boot-env" > >> [ 1.748975] 0x000000080000-0x000002000000 : "alt-obmc-ubi" > > > > Specifically, the control register state is exposed as: > > > >> [ 0.947812] aspeed-smc 1e620000.spi: read control register: 203c0641 > > > > The 0x3c opcode translates to DREAD4B for the mx25l25635f part, which is > > what is detected. The post BFPT fixup implemented in commit 2bffa65da43e > > ("mtd: spi-nor: Add a post BFPT fixup for MX25L25635E") detects the 'e' > > vs 'f' part based on the reported support for > > BFPT_DWORD5_FAST_READ_4_4_4, which implies 4B opcodes are supported > > (e.g. DREAD4B), however the aspeed-smc driver fails to put the > > controller into the right addressing mode to support sending DREAD4B, and > > so we reach a bad state. > > It's more complex than that but I haven't quite cornered the problem. > > When doing the link training, the first read for the golden image is > done in slow mode and it misses the first byte for some reason. So the > gold image is incorrect (cough) but other images are correct (cough cough) > > Why is this first byte being dropped is the question. The flash module > might have entered 4B mode ? is it expecting a dummy ? > > For the moment, this is fine for a workaround but we will need to fully > support 4B opcodes (which we do without training). Another solution is > to use the kernel command line switch "optimize_read=false" and all > works fine, perfectly fine. Which is kind of frustrating. Hmm, okay. Sounds like I didn't track it down as well as I thought then. I figured we'd need to set CONTROL_IO_DUAL_ADDR_DATA to send a DREAD4B, but we never put the controller into that mode: there's only one call to aspeed_smc_set_io_mode(), in aspeed_smc_read_user(), which is only called if `io_mode == CONTROL_IO_DUAL_DATA` holds, and we set the IO mode to CONTROL_IO_DUAL_DATA. Suggestions for rewording the commit message? Should I just replace some of it with your words above? Andrew > > Thanks for the patch. > > C. > > > > > > In the interim, revert the upstream patch adding 'e' vs 'f' detection to > > avoid sending 4B opcodes. > > > > The revert has been tested on a freshly flashed and power-cycled > > Witherspoon, both with and without the patch applied, confirming the > > broken behaviour in the former case and a successful boot in the latter. > > > > Cc: Andrew Geissler <geissonator@gmail.com> > > Reported-by: George Keishing <gkeishin@in.ibm.com> > > Investigated-by: Cédric Le Goater <clg@kaod.org> > > Investigated-by: Eddie James <eajames@linux.ibm.com> > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > > --- > > drivers/mtd/spi-nor/spi-nor.c | 29 +---------------------------- > > 1 file changed, 1 insertion(+), 28 deletions(-) > > > > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c > > index 5a5a47651bf9..9f85ab5fc569 100644 > > --- a/drivers/mtd/spi-nor/spi-nor.c > > +++ b/drivers/mtd/spi-nor/spi-nor.c > > @@ -1681,31 +1681,6 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor) > > .addr_width = 3, \ > > .flags = SPI_NOR_NO_FR | SPI_S3AN, > > > > -static int > > -mx25l25635_post_bfpt_fixups(struct spi_nor *nor, > > - const struct sfdp_parameter_header *bfpt_header, > > - const struct sfdp_bfpt *bfpt, > > - struct spi_nor_flash_parameter *params) > > -{ > > - /* > > - * MX25L25635F supports 4B opcodes but MX25L25635E does not. > > - * Unfortunately, Macronix has re-used the same JEDEC ID for both > > - * variants which prevents us from defining a new entry in the parts > > - * table. > > - * We need a way to differentiate MX25L25635E and MX25L25635F, and it > > - * seems that the F version advertises support for Fast Read 4-4-4 in > > - * its BFPT table. > > - */ > > - if (bfpt->dwords[BFPT_DWORD(5)] & BFPT_DWORD5_FAST_READ_4_4_4) > > - nor->flags |= SNOR_F_4B_OPCODES; > > - > > - return 0; > > -} > > - > > -static struct spi_nor_fixups mx25l25635_fixups = { > > - .post_bfpt = mx25l25635_post_bfpt_fixups, > > -}; > > - > > /* NOTE: double check command sets and memory organization when you add > > * more nor chips. This current list focusses on newer chips, which > > * have been converging on command sets which including JEDEC ID. > > @@ -1843,9 +1818,7 @@ static const struct flash_info spi_nor_ids[] = { > > { "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 0) }, > > { "mx25u12835f", INFO(0xc22538, 0, 64 * 1024, 256, > > SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, > > - { "mx25l25635e", INFO(0xc22019, 0, 64 * 1024, 512, > > - SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) > > - .fixups = &mx25l25635_fixups }, > > + { "mx25l25635e", INFO(0xc22019, 0, 64 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, > > { "mx25u25635f", INFO(0xc22539, 0, 64 * 1024, 512, SECT_4K | SPI_NOR_4B_OPCODES) }, > > { "mx25l25655e", INFO(0xc22619, 0, 64 * 1024, 512, 0) }, > > { "mx66l51235f", INFO(0xc2201a, 0, 64 * 1024, 1024, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, > > > >
On Wed, 17 Apr 2019, at 15:53, Andrew Jeffery wrote: > > > On Wed, 17 Apr 2019, at 15:41, Cédric Le Goater wrote: > > On 4/17/19 7:45 AM, Andrew Jeffery wrote: > > > This reverts commit 2bffa65da43e399079dad5947c6aa9ab3cfa4ad4. > > > > > > 5.0.x fails to boot on Witherspoon machines after an AC-cycle, with the > > > following error: > > > > > >> [ 4.337766] ubi0: default fastmap pool size: 25 > > >> [ 4.342321] ubi0: default fastmap WL pool size: 12 > > >> [ 4.347232] ubi0: attaching mtd3 > > >> [ 4.361487] ubi0: scanning is finished > > >> [ 4.365525] ubi0 error: ubi_read_volume_table: the layout volume was not found > > >> [ 4.372989] ubi0 error: ubi_attach_mtd_dev: failed to attach mtd3, error -22 > > >> [ 4.380193] UBI error: cannot attach mtd3 > > > > > > Which leads to: > > > > > >> [ 4.553478] VFS: Cannot open root device "ubiblock0_1" or unknown-block(0,0): error -6 > > >> [ 4.561550] Please append a correct "root=" boot option; here are the available partitions: > > >> [ 4.570029] 1f00 32768 mtdblock0 > > >> [ 4.570038] (driver?) > > >> [ 4.576671] 1f01 384 mtdblock1 > > >> [ 4.576680] (driver?) > > >> [ 4.583234] 1f02 128 mtdblock2 > > >> [ 4.583240] (driver?) > > >> [ 4.589883] 1f03 32256 mtdblock3 > > >> [ 4.589892] (driver?) > > >> [ 4.596520] 1f04 32768 mtdblock4 > > >> [ 4.596529] (driver?) > > >> [ 4.603080] 1f05 384 mtdblock5 > > >> [ 4.603085] (driver?) > > >> [ 4.609711] 1f06 128 mtdblock6 > > >> [ 4.609719] (driver?) > > >> [ 4.616338] 1f07 32256 mtdblock7 > > >> [ 4.616347] (driver?) > > >> [ 4.622901] 1f08 131072 mtdblock8 > > >> [ 4.622907] (driver?) > > >> [ 4.629539] Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0) > > > > > > A hint is provided earlier in the boot process: > > > > > >> [ 0.920597] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency > > >> [ 0.926753] aspeed-smc 1e620000.spi: mx25l25635e (32768 Kbytes) > > >> [ 0.932704] aspeed-smc 1e620000.spi: CE0 window [ 0x20000000 - 0x22000000 ] 32MB > > >> [ 0.940230] aspeed-smc 1e620000.spi: CE1 window [ 0x22000000 - 0x2a000000 ] 128MB > > >> [ 0.947812] aspeed-smc 1e620000.spi: read control register: 203c0641 > > >> [ 1.435022] 3 fixed-partitions partitions found on MTD device bmc > > >> [ 1.441249] Creating 3 MTD partitions on "bmc": > > >> [ 1.445911] 0x000000000000-0x000000060000 : "u-boot" > > >> [ 1.455264] 0x000000060000-0x000000080000 : "u-boot-env" > > >> [ 1.465042] 0x000000080000-0x000002000000 : "obmc-ubi" > > >> [ 1.474914] aspeed-smc 1e620000.spi: Using 50 MHz SPI frequency > > >> [ 1.481102] aspeed-smc 1e620000.spi: mx25l25635e (32768 Kbytes) > > >> [ 1.487175] aspeed-smc 1e620000.spi: CE1 window [ 0x22000000 - 0x24000000 ] 32MB > > >> [ 1.494592] aspeed-smc 1e620000.spi: CE2 window [ 0x24000000 - 0x2c000000 ] 128MB > > >> [ 1.502173] aspeed-smc 1e620000.spi: read control register: 203c0041 > > >> [ 1.705799] aspeed-smc 1e620000.spi: No good frequency, using dumb slow > > >> [ 1.717393] 3 fixed-partitions partitions found on MTD device alt-bmc > > >> [ 1.723851] Creating 3 MTD partitions on "alt-bmc": > > >> [ 1.728890] 0x000000000000-0x000000060000 : "alt-u-boot" > > >> [ 1.738950] 0x000000060000-0x000000080000 : "alt-u-boot-env" > > >> [ 1.748975] 0x000000080000-0x000002000000 : "alt-obmc-ubi" > > > > > > Specifically, the control register state is exposed as: > > > > > >> [ 0.947812] aspeed-smc 1e620000.spi: read control register: 203c0641 > > > > > > The 0x3c opcode translates to DREAD4B for the mx25l25635f part, which is > > > what is detected. The post BFPT fixup implemented in commit 2bffa65da43e > > > ("mtd: spi-nor: Add a post BFPT fixup for MX25L25635E") detects the 'e' > > > vs 'f' part based on the reported support for > > > BFPT_DWORD5_FAST_READ_4_4_4, which implies 4B opcodes are supported > > > (e.g. DREAD4B), however the aspeed-smc driver fails to put the > > > controller into the right addressing mode to support sending DREAD4B, and > > > so we reach a bad state. > > > > It's more complex than that but I haven't quite cornered the problem. > > > > When doing the link training, the first read for the golden image is > > done in slow mode and it misses the first byte for some reason. So the > > gold image is incorrect (cough) but other images are correct (cough cough) > > > > Why is this first byte being dropped is the question. The flash module > > might have entered 4B mode ? is it expecting a dummy ? > > > > For the moment, this is fine for a workaround but we will need to fully > > support 4B opcodes (which we do without training). Another solution is > > to use the kernel command line switch "optimize_read=false" and all > > works fine, perfectly fine. Which is kind of frustrating. > > Hmm, okay. Sounds like I didn't track it down as well as I thought then. > > I figured we'd need to set CONTROL_IO_DUAL_ADDR_DATA to send a > DREAD4B, but we never put the controller into that mode: there's only > one call to aspeed_smc_set_io_mode(), in aspeed_smc_read_user(), > which is only called if `io_mode == CONTROL_IO_DUAL_DATA` holds, > and we set the IO mode to CONTROL_IO_DUAL_DATA. > > Suggestions for rewording the commit message? Should I just replace > some of it with your words above? Inside word is that Cédric has found the root cause, I'll be abandoning this work-around in anticipation of a proper fix. The issue actually appears to be with the link training assuming EN4B mode which is not the configured when 4B opcodes are enabled. Andrew > Andrew > > > > > Thanks for the patch. > > > > C. > > > > > > > > > > In the interim, revert the upstream patch adding 'e' vs 'f' detection to > > > avoid sending 4B opcodes. > > > > > > The revert has been tested on a freshly flashed and power-cycled > > > Witherspoon, both with and without the patch applied, confirming the > > > broken behaviour in the former case and a successful boot in the latter. > > > > > > Cc: Andrew Geissler <geissonator@gmail.com> > > > Reported-by: George Keishing <gkeishin@in.ibm.com> > > > Investigated-by: Cédric Le Goater <clg@kaod.org> > > > Investigated-by: Eddie James <eajames@linux.ibm.com> > > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > > > --- > > > drivers/mtd/spi-nor/spi-nor.c | 29 +---------------------------- > > > 1 file changed, 1 insertion(+), 28 deletions(-) > > > > > > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c > > > index 5a5a47651bf9..9f85ab5fc569 100644 > > > --- a/drivers/mtd/spi-nor/spi-nor.c > > > +++ b/drivers/mtd/spi-nor/spi-nor.c > > > @@ -1681,31 +1681,6 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor) > > > .addr_width = 3, \ > > > .flags = SPI_NOR_NO_FR | SPI_S3AN, > > > > > > -static int > > > -mx25l25635_post_bfpt_fixups(struct spi_nor *nor, > > > - const struct sfdp_parameter_header *bfpt_header, > > > - const struct sfdp_bfpt *bfpt, > > > - struct spi_nor_flash_parameter *params) > > > -{ > > > - /* > > > - * MX25L25635F supports 4B opcodes but MX25L25635E does not. > > > - * Unfortunately, Macronix has re-used the same JEDEC ID for both > > > - * variants which prevents us from defining a new entry in the parts > > > - * table. > > > - * We need a way to differentiate MX25L25635E and MX25L25635F, and it > > > - * seems that the F version advertises support for Fast Read 4-4-4 in > > > - * its BFPT table. > > > - */ > > > - if (bfpt->dwords[BFPT_DWORD(5)] & BFPT_DWORD5_FAST_READ_4_4_4) > > > - nor->flags |= SNOR_F_4B_OPCODES; > > > - > > > - return 0; > > > -} > > > - > > > -static struct spi_nor_fixups mx25l25635_fixups = { > > > - .post_bfpt = mx25l25635_post_bfpt_fixups, > > > -}; > > > - > > > /* NOTE: double check command sets and memory organization when you add > > > * more nor chips. This current list focusses on newer chips, which > > > * have been converging on command sets which including JEDEC ID. > > > @@ -1843,9 +1818,7 @@ static const struct flash_info spi_nor_ids[] = { > > > { "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 0) }, > > > { "mx25u12835f", INFO(0xc22538, 0, 64 * 1024, 256, > > > SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, > > > - { "mx25l25635e", INFO(0xc22019, 0, 64 * 1024, 512, > > > - SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) > > > - .fixups = &mx25l25635_fixups }, > > > + { "mx25l25635e", INFO(0xc22019, 0, 64 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, > > > { "mx25u25635f", INFO(0xc22539, 0, 64 * 1024, 512, SECT_4K | SPI_NOR_4B_OPCODES) }, > > > { "mx25l25655e", INFO(0xc22619, 0, 64 * 1024, 512, 0) }, > > > { "mx66l51235f", INFO(0xc2201a, 0, 64 * 1024, 1024, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, > > > > > > > >
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index 5a5a47651bf9..9f85ab5fc569 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -1681,31 +1681,6 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor) .addr_width = 3, \ .flags = SPI_NOR_NO_FR | SPI_S3AN, -static int -mx25l25635_post_bfpt_fixups(struct spi_nor *nor, - const struct sfdp_parameter_header *bfpt_header, - const struct sfdp_bfpt *bfpt, - struct spi_nor_flash_parameter *params) -{ - /* - * MX25L25635F supports 4B opcodes but MX25L25635E does not. - * Unfortunately, Macronix has re-used the same JEDEC ID for both - * variants which prevents us from defining a new entry in the parts - * table. - * We need a way to differentiate MX25L25635E and MX25L25635F, and it - * seems that the F version advertises support for Fast Read 4-4-4 in - * its BFPT table. - */ - if (bfpt->dwords[BFPT_DWORD(5)] & BFPT_DWORD5_FAST_READ_4_4_4) - nor->flags |= SNOR_F_4B_OPCODES; - - return 0; -} - -static struct spi_nor_fixups mx25l25635_fixups = { - .post_bfpt = mx25l25635_post_bfpt_fixups, -}; - /* NOTE: double check command sets and memory organization when you add * more nor chips. This current list focusses on newer chips, which * have been converging on command sets which including JEDEC ID. @@ -1843,9 +1818,7 @@ static const struct flash_info spi_nor_ids[] = { { "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 0) }, { "mx25u12835f", INFO(0xc22538, 0, 64 * 1024, 256, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, - { "mx25l25635e", INFO(0xc22019, 0, 64 * 1024, 512, - SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) - .fixups = &mx25l25635_fixups }, + { "mx25l25635e", INFO(0xc22019, 0, 64 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, { "mx25u25635f", INFO(0xc22539, 0, 64 * 1024, 512, SECT_4K | SPI_NOR_4B_OPCODES) }, { "mx25l25655e", INFO(0xc22619, 0, 64 * 1024, 512, 0) }, { "mx66l51235f", INFO(0xc2201a, 0, 64 * 1024, 1024, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },