| Message ID | 20200205165736.4964-1-alexander.sverdlin@nokia.com |
|---|---|
| State | Superseded |
| Delegated to: | Ambarus Tudor |
| Headers | show |
| Series | [v2] mtd: spi-nor: Fixup page size for S25FS-S | expand |
On 05/02/2020 16:57, Alexander A Sverdlin wrote: > From: Alexander Sverdlin <alexander.sverdlin@nokia.com> > > Spansion S25FS-S family has an issue in Basic Flash Parameter Table: > DWORD-11 bits 7-4 specify write page size 512 bytes. In reality this > is configurable in the non-volatile CR3NV register and even factory > default configuration is "wrap at 256 bytes". So blind relying on BFPT > breaks write operation on these Flashes. > > All this story is vendor-specific, so add the corresponding fixup hook > which first restores the safe page size of 256 bytes from > struct flash_info but checks is more performant 512 bytes configuration > is active and adjusts the page_size accordingly. > > Cc: stable@vger.kernel.org > Fixes: f384b352c ("mtd: spi-nor: parse Serial Flash Discoverable Parameters (SFDP) tables") > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nokia.com> > --- > Changelog: > v2: Thankfully rebased on the hint from John Garry > > drivers/mtd/spi-nor/spi-nor.c | 39 +++++++++++++++++++++++++++++++++++++-- > include/linux/mtd/spi-nor.h | 5 +++++ > 2 files changed, 42 insertions(+), 2 deletions(-) > > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c > index 928a660..c0a5041 100644 > --- a/drivers/mtd/spi-nor/spi-nor.c > +++ b/drivers/mtd/spi-nor/spi-nor.c > @@ -2302,6 +2302,39 @@ static struct spi_nor_fixups gd25q256_fixups = { > .default_init = gd25q256_default_init, > }; > Hi Alexander, Thanks for the quick turnaround. But, sorry to say, this does not look right: > +/* Spansion S25FS-S SFDP workarounds */ > +static int s25fs_s_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) > +{ > + const struct flash_info *info = nor->info; > + u8 read_opcode, buf; > + int ret; > + > + /* Default is safe */ > + params->page_size = info->page_size; > + > + /* > + * But is the chip configured for more performant 512 bytes write page > + * size? > + */ > + read_opcode = nor->read_opcode; > + > + nor->read_opcode = SPINOR_OP_RDAR; > + ret = nor->read(nor, SPINOR_REG_CR3V, 1, &buf); The read method is now gone from struct spi_nor, moved into spi_nor.controller_ops. And we also support spi_mem ops now. In fact, I find that the SFDP signature is not correct for me for this part, so I need to check that first... Thanks, John > + if (!ret && (buf & CR3V_02H_V)) > + params->page_size = 512; > + > + nor->read_opcode = read_opcode; > + > + return ret; > +} > + > +static const struct spi_nor_fixups s25fs_s_fixups = { > + .post_bfpt = s25fs_s_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. > @@ -2536,7 +2569,8 @@ static const struct flash_info spi_nor_ids[] = { > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, > { "s25fl128s1", INFO6(0x012018, 0x4d0180, 64 * 1024, 256, > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, > - { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, USE_CLSR) }, > + { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, USE_CLSR) > + .fixups = &s25fs_s_fixups, }, > { "s25fl256s1", INFO(0x010219, 0x4d01, 64 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, > { "s25fl512s", INFO6(0x010220, 0x4d0080, 256 * 1024, 256, > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | > @@ -2546,7 +2580,8 @@ static const struct flash_info spi_nor_ids[] = { > { "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024, 64, 0) }, > { "s25sl12801", INFO(0x012018, 0x0301, 64 * 1024, 256, 0) }, > { "s25fl129p0", INFO(0x012018, 0x4d00, 256 * 1024, 64, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, > - { "s25fl129p1", INFO(0x012018, 0x4d01, 64 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, > + { "s25fl129p1", INFO(0x012018, 0x4d01, 64 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) > + .fixups = &s25fs_s_fixups, }, > { "s25sl004a", INFO(0x010212, 0, 64 * 1024, 8, 0) }, > { "s25sl008a", INFO(0x010213, 0, 64 * 1024, 16, 0) }, > { "s25sl016a", INFO(0x010214, 0, 64 * 1024, 32, 0) }, > diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h > index 5abd91c..7ce3e79 100644 > --- a/include/linux/mtd/spi-nor.h > +++ b/include/linux/mtd/spi-nor.h > @@ -116,6 +116,7 @@ > /* Used for Spansion flashes only. */ > #define SPINOR_OP_BRWR 0x17 /* Bank register write */ > #define SPINOR_OP_CLSR 0x30 /* Clear status register 1 */ > +#define SPINOR_OP_RDAR 0x65 /* Read Any Register */ > > /* Used for Micron flashes only. */ > #define SPINOR_OP_RD_EVCR 0x65 /* Read EVCR register */ > @@ -150,6 +151,10 @@ > #define SR2_QUAD_EN_BIT1 BIT(1) > #define SR2_QUAD_EN_BIT7 BIT(7) > > +/* Used for Spansion flashes RDAR command only. */ > +#define SPINOR_REG_CR3V 0x800004 > +#define CR3V_02H_V BIT(4) /* Page Buffer Wrap */ > + > /* Supported SPI protocols */ > #define SNOR_PROTO_INST_MASK GENMASK(23, 16) > #define SNOR_PROTO_INST_SHIFT 16 >
Hi Alexander, On 06/02/20 5:08 pm, John Garry wrote: > On 05/02/2020 16:57, Alexander A Sverdlin wrote: >> From: Alexander Sverdlin <alexander.sverdlin@nokia.com> >> [...] >> +static int s25fs_s_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) >> +{ >> + const struct flash_info *info = nor->info; >> + u8 read_opcode, buf; >> + int ret; >> + >> + /* Default is safe */ >> + params->page_size = info->page_size; >> + >> + /* >> + * But is the chip configured for more performant 512 bytes write >> page >> + * size? >> + */ >> + read_opcode = nor->read_opcode; >> + >> + nor->read_opcode = SPINOR_OP_RDAR; >> + ret = nor->read(nor, SPINOR_REG_CR3V, 1, &buf); > > The read method is now gone from struct spi_nor, moved into > spi_nor.controller_ops. And we also support spi_mem ops now. > Yes, please rebase patch on top of latest spi-nor/next or linux-next tree at: git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git spi-nor/next Regards Vignesh > >> + if (!ret && (buf & CR3V_02H_V)) >> + params->page_size = 512; >> + >> + nor->read_opcode = read_opcode; >> + >> + return ret; >> +} >> + >> +static const struct spi_nor_fixups s25fs_s_fixups = { >> + .post_bfpt = s25fs_s_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. >> @@ -2536,7 +2569,8 @@ static const struct flash_info spi_nor_ids[] = { >> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, >> { "s25fl128s1", INFO6(0x012018, 0x4d0180, 64 * 1024, 256, >> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, >> - { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, USE_CLSR) }, >> + { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, USE_CLSR) >> + .fixups = &s25fs_s_fixups, }, >> { "s25fl256s1", INFO(0x010219, 0x4d01, 64 * 1024, 512, >> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, >> { "s25fl512s", INFO6(0x010220, 0x4d0080, 256 * 1024, 256, >> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | >> @@ -2546,7 +2580,8 @@ static const struct flash_info spi_nor_ids[] = { >> { "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024, 64, 0) }, >> { "s25sl12801", INFO(0x012018, 0x0301, 64 * 1024, 256, 0) }, >> { "s25fl129p0", INFO(0x012018, 0x4d00, 256 * 1024, 64, >> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, >> - { "s25fl129p1", INFO(0x012018, 0x4d01, 64 * 1024, 256, >> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, >> + { "s25fl129p1", INFO(0x012018, 0x4d01, 64 * 1024, 256, >> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) >> + .fixups = &s25fs_s_fixups, }, >> { "s25sl004a", INFO(0x010212, 0, 64 * 1024, 8, 0) }, >> { "s25sl008a", INFO(0x010213, 0, 64 * 1024, 16, 0) }, >> { "s25sl016a", INFO(0x010214, 0, 64 * 1024, 32, 0) }, >> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h >> index 5abd91c..7ce3e79 100644 >> --- a/include/linux/mtd/spi-nor.h >> +++ b/include/linux/mtd/spi-nor.h >> @@ -116,6 +116,7 @@ >> /* Used for Spansion flashes only. */ >> #define SPINOR_OP_BRWR 0x17 /* Bank register write */ >> #define SPINOR_OP_CLSR 0x30 /* Clear status register 1 */ >> +#define SPINOR_OP_RDAR 0x65 /* Read Any Register */ >> /* Used for Micron flashes only. */ >> #define SPINOR_OP_RD_EVCR 0x65 /* Read EVCR register */ >> @@ -150,6 +151,10 @@ >> #define SR2_QUAD_EN_BIT1 BIT(1) >> #define SR2_QUAD_EN_BIT7 BIT(7) >> +/* Used for Spansion flashes RDAR command only. */ >> +#define SPINOR_REG_CR3V 0x800004 >> +#define CR3V_02H_V BIT(4) /* Page Buffer Wrap */ >> + >> /* Supported SPI protocols */ >> #define SNOR_PROTO_INST_MASK GENMASK(23, 16) >> #define SNOR_PROTO_INST_SHIFT 16 >> > > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/
On 07/02/2020 05:03, Vignesh Raghavendra wrote: > Hi Alexander, > > On 06/02/20 5:08 pm, John Garry wrote: >> On 05/02/2020 16:57, Alexander A Sverdlin wrote: >>> From: Alexander Sverdlin <alexander.sverdlin@nokia.com> >>> > [...] >>> +static int s25fs_s_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) >>> +{ >>> + const struct flash_info *info = nor->info; >>> + u8 read_opcode, buf; >>> + int ret; >>> + >>> + /* Default is safe */ >>> + params->page_size = info->page_size; >>> + >>> + /* >>> + * But is the chip configured for more performant 512 bytes write >>> page >>> + * size? >>> + */ >>> + read_opcode = nor->read_opcode; >>> + >>> + nor->read_opcode = SPINOR_OP_RDAR; >>> + ret = nor->read(nor, SPINOR_REG_CR3V, 1, &buf); >> >> The read method is now gone from struct spi_nor, moved into >> spi_nor.controller_ops. And we also support spi_mem ops now. >> > > Yes, please rebase patch on top of latest spi-nor/next or linux-next tree at: > > git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git spi-nor/next > > Regards > Vignesh > I don't mean to hijack this thread, but I have tried to enable QUAD mode for part s25fl129p1, and it fails in spi_nor_write_16bit_cr_and_check(): [ 47.263365] spi-nor spi-PRP0001:00: CR: read back test failed [ 47.306567] spi-nor spi-PRP0001:00: quad mode not supported [ 47.322413] spi-nor: probe of spi-PRP0001:00 failed with error -5 Hacking the flags to set SNOR_F_NO_READ_CR, and at least I can successfully probe the driver. Does anyone know if this part does not support reading the config register. The limited datasheet here doesn't mention it, AFAICT: https://www.cypress.com/file/196851/download Thanks, John > >> >>> + if (!ret && (buf & CR3V_02H_V)) >>> + params->page_size = 512; >>> + >>> + nor->read_opcode = read_opcode; >>> + >>> + return ret; >>> +} >>> + >>> +static const struct spi_nor_fixups s25fs_s_fixups = { >>> + .post_bfpt = s25fs_s_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. >>> @@ -2536,7 +2569,8 @@ static const struct flash_info spi_nor_ids[] = { >>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, >>> { "s25fl128s1", INFO6(0x012018, 0x4d0180, 64 * 1024, 256, >>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, >>> - { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, USE_CLSR) }, >>> + { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, USE_CLSR) >>> + .fixups = &s25fs_s_fixups, }, >>> { "s25fl256s1", INFO(0x010219, 0x4d01, 64 * 1024, 512, >>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, >>> { "s25fl512s", INFO6(0x010220, 0x4d0080, 256 * 1024, 256, >>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | >>> @@ -2546,7 +2580,8 @@ static const struct flash_info spi_nor_ids[] = { >>> { "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024, 64, 0) }, >>> { "s25sl12801", INFO(0x012018, 0x0301, 64 * 1024, 256, 0) }, >>> { "s25fl129p0", INFO(0x012018, 0x4d00, 256 * 1024, 64, >>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, >>> - { "s25fl129p1", INFO(0x012018, 0x4d01, 64 * 1024, 256, >>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, >>> + { "s25fl129p1", INFO(0x012018, 0x4d01, 64 * 1024, 256, >>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) >>> + .fixups = &s25fs_s_fixups, }, >>> { "s25sl004a", INFO(0x010212, 0, 64 * 1024, 8, 0) }, >>> { "s25sl008a", INFO(0x010213, 0, 64 * 1024, 16, 0) }, >>> { "s25sl016a", INFO(0x010214, 0, 64 * 1024, 32, 0) }, >>> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h >>> index 5abd91c..7ce3e79 100644 >>> --- a/include/linux/mtd/spi-nor.h >>> +++ b/include/linux/mtd/spi-nor.h >>> @@ -116,6 +116,7 @@ >>> /* Used for Spansion flashes only. */ >>> #define SPINOR_OP_BRWR 0x17 /* Bank register write */ >>> #define SPINOR_OP_CLSR 0x30 /* Clear status register 1 */ >>> +#define SPINOR_OP_RDAR 0x65 /* Read Any Register */ >>> /* Used for Micron flashes only. */ >>> #define SPINOR_OP_RD_EVCR 0x65 /* Read EVCR register */ >>> @@ -150,6 +151,10 @@ >>> #define SR2_QUAD_EN_BIT1 BIT(1) >>> #define SR2_QUAD_EN_BIT7 BIT(7) >>> +/* Used for Spansion flashes RDAR command only. */ >>> +#define SPINOR_REG_CR3V 0x800004 >>> +#define CR3V_02H_V BIT(4) /* Page Buffer Wrap */ >>> + >>> /* Supported SPI protocols */ >>> #define SNOR_PROTO_INST_MASK GENMASK(23, 16) >>> #define SNOR_PROTO_INST_SHIFT 16 >>> >> >> >> ______________________________________________________ >> Linux MTD discussion mailing list >> http://lists.infradead.org/mailman/listinfo/linux-mtd/ >
On 12/02/20 11:30 pm, John Garry wrote: > On 07/02/2020 05:03, Vignesh Raghavendra wrote: >> Hi Alexander, >> >> On 06/02/20 5:08 pm, John Garry wrote: >>> On 05/02/2020 16:57, Alexander A Sverdlin wrote: >>>> From: Alexander Sverdlin <alexander.sverdlin@nokia.com> >>>> >> [...] [...] > > I don't mean to hijack this thread, but I have tried to enable QUAD mode > for part s25fl129p1, and it fails in spi_nor_write_16bit_cr_and_check(): > > [ 47.263365] spi-nor spi-PRP0001:00: CR: read back test failed > [ 47.306567] spi-nor spi-PRP0001:00: quad mode not supported > [ 47.322413] spi-nor: probe of spi-PRP0001:00 failed with error -5 > > Hacking the flags to set SNOR_F_NO_READ_CR, and at least I can > successfully probe the driver. > > Does anyone know if this part does not support reading the config > register. The limited datasheet here doesn't mention it, AFAICT: > > https://www.cypress.com/file/196851/download Above datasheet is for s25fl128p. Per, s25fl129p datasheet[1], part does support 0x35 (SPINOR_OP_RDCR) command and support 16bit write status register command (0x1) Could you debug further and see what exactly fails to match when read back fails? [1]https://www.cypress.com/file/197121/download Regards Vignesh > > Thanks, > John > >> >>> >>>> + if (!ret && (buf & CR3V_02H_V)) >>>> + params->page_size = 512; >>>> + >>>> + nor->read_opcode = read_opcode; >>>> + >>>> + return ret; >>>> +} >>>> + >>>> +static const struct spi_nor_fixups s25fs_s_fixups = { >>>> + .post_bfpt = s25fs_s_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. >>>> @@ -2536,7 +2569,8 @@ static const struct flash_info spi_nor_ids[] = { >>>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, >>>> { "s25fl128s1", INFO6(0x012018, 0x4d0180, 64 * 1024, 256, >>>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, >>>> - { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, >>>> USE_CLSR) }, >>>> + { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, USE_CLSR) >>>> + .fixups = &s25fs_s_fixups, }, >>>> { "s25fl256s1", INFO(0x010219, 0x4d01, 64 * 1024, 512, >>>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, >>>> { "s25fl512s", INFO6(0x010220, 0x4d0080, 256 * 1024, 256, >>>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | >>>> @@ -2546,7 +2580,8 @@ static const struct flash_info spi_nor_ids[] = { >>>> { "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024, 64, 0) }, >>>> { "s25sl12801", INFO(0x012018, 0x0301, 64 * 1024, 256, 0) }, >>>> { "s25fl129p0", INFO(0x012018, 0x4d00, 256 * 1024, 64, >>>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, >>>> - { "s25fl129p1", INFO(0x012018, 0x4d01, 64 * 1024, 256, >>>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, >>>> + { "s25fl129p1", INFO(0x012018, 0x4d01, 64 * 1024, 256, >>>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) >>>> + .fixups = &s25fs_s_fixups, }, >>>> { "s25sl004a", INFO(0x010212, 0, 64 * 1024, 8, 0) }, >>>> { "s25sl008a", INFO(0x010213, 0, 64 * 1024, 16, 0) }, >>>> { "s25sl016a", INFO(0x010214, 0, 64 * 1024, 32, 0) }, >>>> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h >>>> index 5abd91c..7ce3e79 100644 >>>> --- a/include/linux/mtd/spi-nor.h >>>> +++ b/include/linux/mtd/spi-nor.h >>>> @@ -116,6 +116,7 @@ >>>> /* Used for Spansion flashes only. */ >>>> #define SPINOR_OP_BRWR 0x17 /* Bank register write */ >>>> #define SPINOR_OP_CLSR 0x30 /* Clear status register 1 */ >>>> +#define SPINOR_OP_RDAR 0x65 /* Read Any Register */ >>>> /* Used for Micron flashes only. */ >>>> #define SPINOR_OP_RD_EVCR 0x65 /* Read EVCR register */ >>>> @@ -150,6 +151,10 @@ >>>> #define SR2_QUAD_EN_BIT1 BIT(1) >>>> #define SR2_QUAD_EN_BIT7 BIT(7) >>>> +/* Used for Spansion flashes RDAR command only. */ >>>> +#define SPINOR_REG_CR3V 0x800004 >>>> +#define CR3V_02H_V BIT(4) /* Page Buffer Wrap */ >>>> + >>>> /* Supported SPI protocols */ >>>> #define SNOR_PROTO_INST_MASK GENMASK(23, 16) >>>> #define SNOR_PROTO_INST_SHIFT 16 >>>> >>> >>> >>> ______________________________________________________ >>> Linux MTD discussion mailing list >>> http://lists.infradead.org/mailman/listinfo/linux-mtd/ >> >
On 18/02/2020 04:53, Vignesh Raghavendra wrote: >> I don't mean to hijack this thread, but I have tried to enable QUAD mode >> for part s25fl129p1, and it fails in spi_nor_write_16bit_cr_and_check(): >> >> [ 47.263365] spi-nor spi-PRP0001:00: CR: read back test failed >> [ 47.306567] spi-nor spi-PRP0001:00: quad mode not supported >> [ 47.322413] spi-nor: probe of spi-PRP0001:00 failed with error -5 >> >> Hacking the flags to set SNOR_F_NO_READ_CR, and at least I can >> successfully probe the driver. >> >> Does anyone know if this part does not support reading the config >> register. The limited datasheet here doesn't mention it, AFAICT: >> Hi Vignesh, >> https://www.cypress.com/file/196851/download > Above datasheet is for s25fl128p. > Right, I figured this out soon enough. I shouldn't just click on the first page which google produces... > Per, s25fl129p datasheet[1], part does support 0x35 (SPINOR_OP_RDCR) > command and support 16bit write status register command (0x1) > > Could you debug further and see what exactly fails to match when > read back fails? I was trying to figure out the issue. So in spi_nor_write_16bit_cr_and_check(), the sr check passes (it holds 0, so that may be somewhat inconclusive) but the value for comparison return 0 in the CR also versus expected 3. Maybe it is a host driver issue, but I am doubtful. I can continue to investigate. Any ideas would be appreciated. > > [1]https://www.cypress.com/file/197121/download > > Thanks, John
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index 928a660..c0a5041 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -2302,6 +2302,39 @@ static struct spi_nor_fixups gd25q256_fixups = { .default_init = gd25q256_default_init, }; +/* Spansion S25FS-S SFDP workarounds */ +static int s25fs_s_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) +{ + const struct flash_info *info = nor->info; + u8 read_opcode, buf; + int ret; + + /* Default is safe */ + params->page_size = info->page_size; + + /* + * But is the chip configured for more performant 512 bytes write page + * size? + */ + read_opcode = nor->read_opcode; + + nor->read_opcode = SPINOR_OP_RDAR; + ret = nor->read(nor, SPINOR_REG_CR3V, 1, &buf); + if (!ret && (buf & CR3V_02H_V)) + params->page_size = 512; + + nor->read_opcode = read_opcode; + + return ret; +} + +static const struct spi_nor_fixups s25fs_s_fixups = { + .post_bfpt = s25fs_s_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. @@ -2536,7 +2569,8 @@ static const struct flash_info spi_nor_ids[] = { SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, { "s25fl128s1", INFO6(0x012018, 0x4d0180, 64 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, - { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, USE_CLSR) }, + { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, USE_CLSR) + .fixups = &s25fs_s_fixups, }, { "s25fl256s1", INFO(0x010219, 0x4d01, 64 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, { "s25fl512s", INFO6(0x010220, 0x4d0080, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | @@ -2546,7 +2580,8 @@ static const struct flash_info spi_nor_ids[] = { { "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024, 64, 0) }, { "s25sl12801", INFO(0x012018, 0x0301, 64 * 1024, 256, 0) }, { "s25fl129p0", INFO(0x012018, 0x4d00, 256 * 1024, 64, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, - { "s25fl129p1", INFO(0x012018, 0x4d01, 64 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, + { "s25fl129p1", INFO(0x012018, 0x4d01, 64 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) + .fixups = &s25fs_s_fixups, }, { "s25sl004a", INFO(0x010212, 0, 64 * 1024, 8, 0) }, { "s25sl008a", INFO(0x010213, 0, 64 * 1024, 16, 0) }, { "s25sl016a", INFO(0x010214, 0, 64 * 1024, 32, 0) }, diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index 5abd91c..7ce3e79 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -116,6 +116,7 @@ /* Used for Spansion flashes only. */ #define SPINOR_OP_BRWR 0x17 /* Bank register write */ #define SPINOR_OP_CLSR 0x30 /* Clear status register 1 */ +#define SPINOR_OP_RDAR 0x65 /* Read Any Register */ /* Used for Micron flashes only. */ #define SPINOR_OP_RD_EVCR 0x65 /* Read EVCR register */ @@ -150,6 +151,10 @@ #define SR2_QUAD_EN_BIT1 BIT(1) #define SR2_QUAD_EN_BIT7 BIT(7) +/* Used for Spansion flashes RDAR command only. */ +#define SPINOR_REG_CR3V 0x800004 +#define CR3V_02H_V BIT(4) /* Page Buffer Wrap */ + /* Supported SPI protocols */ #define SNOR_PROTO_INST_MASK GENMASK(23, 16) #define SNOR_PROTO_INST_SHIFT 16