Message ID | 20220809201428.118523-3-sudip.mukherjee@sifive.com |
---|---|
State | Superseded |
Delegated to: | Ambarus Tudor |
Headers | show |
Series | Add support for Quad Input Page Program to is25wp256 | expand |
On 8/9/22 23:14, Sudip Mukherjee wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > SFDP table of some flash chips do not advertise support of Quad Input > Page Program even though it has support. Use fixup flags and add hardware > cap for these chips. > > Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com> > --- > drivers/mtd/spi-nor/core.c | 9 +++++++++ > drivers/mtd/spi-nor/core.h | 2 ++ > 2 files changed, 11 insertions(+) > > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c > index f2c64006f8d7..7542404332a5 100644 > --- a/drivers/mtd/spi-nor/core.c > +++ b/drivers/mtd/spi-nor/core.c > @@ -1962,6 +1962,12 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps) > if (nor->flags & SNOR_F_BROKEN_RESET) > *hwcaps &= ~(SNOR_HWCAPS_X_X_X | SNOR_HWCAPS_X_X_X_DTR); > > + if (nor->flags & SNOR_F_HAS_QUAD_PP) { > + *hwcaps |= SNOR_HWCAPS_PP_1_1_4; > + spi_nor_set_pp_settings(¶ms->page_programs[SNOR_CMD_PP_1_1_4], > + SPINOR_OP_PP_1_1_4, SNOR_PROTO_1_1_4); > + } setting SPINOR_OP_PP_1_1_4 should be done in spi_nor_late_init_params(). spi_nor_late_init_params() is used to adjust the ops supported by the flash with the ones supported by the controller. > + > for (cap = 0; cap < sizeof(*hwcaps) * BITS_PER_BYTE; cap++) { > int rdidx, ppidx; > > @@ -2446,6 +2452,9 @@ static void spi_nor_init_fixup_flags(struct spi_nor *nor) > > if (fixup_flags & SPI_NOR_IO_MODE_EN_VOLATILE) > nor->flags |= SNOR_F_IO_MODE_EN_VOLATILE; > + > + if (fixup_flags & SPI_NOR_QUAD_PP) > + nor->flags |= SNOR_F_HAS_QUAD_PP; > } > > /** > diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h > index 85b0cf254e97..7dbdf16a67b4 100644 > --- a/drivers/mtd/spi-nor/core.h > +++ b/drivers/mtd/spi-nor/core.h > @@ -130,6 +130,7 @@ enum spi_nor_option_flags { > SNOR_F_IO_MODE_EN_VOLATILE = BIT(11), > SNOR_F_SOFT_RESET = BIT(12), > SNOR_F_SWP_IS_VOLATILE = BIT(13), > + SNOR_F_HAS_QUAD_PP = BIT(14), you won't need this > }; > > struct spi_nor_read_command { > @@ -520,6 +521,7 @@ struct flash_info { > u8 fixup_flags; > #define SPI_NOR_4B_OPCODES BIT(0) > #define SPI_NOR_IO_MODE_EN_VOLATILE BIT(1) > +#define SPI_NOR_QUAD_PP BIT(2) No, as I previously said, SPI_NOR_QUAD_PP should be declared as a info->flags, not as info->fixup_flags. > > u8 mfr_flags; > > -- > 2.30.2 >
On 8/10/22 11:06, Tudor.Ambarus@microchip.com wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 8/9/22 23:14, Sudip Mukherjee wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> SFDP table of some flash chips do not advertise support of Quad Input >> Page Program even though it has support. Use fixup flags and add hardware >> cap for these chips. >> >> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com> >> --- >> drivers/mtd/spi-nor/core.c | 9 +++++++++ >> drivers/mtd/spi-nor/core.h | 2 ++ >> 2 files changed, 11 insertions(+) >> >> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c >> index f2c64006f8d7..7542404332a5 100644 >> --- a/drivers/mtd/spi-nor/core.c >> +++ b/drivers/mtd/spi-nor/core.c >> @@ -1962,6 +1962,12 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps) >> if (nor->flags & SNOR_F_BROKEN_RESET) >> *hwcaps &= ~(SNOR_HWCAPS_X_X_X | SNOR_HWCAPS_X_X_X_DTR); >> >> + if (nor->flags & SNOR_F_HAS_QUAD_PP) { >> + *hwcaps |= SNOR_HWCAPS_PP_1_1_4; >> + spi_nor_set_pp_settings(¶ms->page_programs[SNOR_CMD_PP_1_1_4], >> + SPINOR_OP_PP_1_1_4, SNOR_PROTO_1_1_4); >> + } > > setting SPINOR_OP_PP_1_1_4 should be done in spi_nor_late_init_params(). > spi_nor_late_init_params() is used to adjust the ops supported by the flash ^ s/spi_nor_late_init_params/spi_nor_spimem_adjust_hwcaps > with the ones supported by the controller. > >> + >> for (cap = 0; cap < sizeof(*hwcaps) * BITS_PER_BYTE; cap++) { >> int rdidx, ppidx; >> >> @@ -2446,6 +2452,9 @@ static void spi_nor_init_fixup_flags(struct spi_nor *nor) >> >> if (fixup_flags & SPI_NOR_IO_MODE_EN_VOLATILE) >> nor->flags |= SNOR_F_IO_MODE_EN_VOLATILE; >> + >> + if (fixup_flags & SPI_NOR_QUAD_PP) >> + nor->flags |= SNOR_F_HAS_QUAD_PP; >> } >> >> /** >> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h >> index 85b0cf254e97..7dbdf16a67b4 100644 >> --- a/drivers/mtd/spi-nor/core.h >> +++ b/drivers/mtd/spi-nor/core.h >> @@ -130,6 +130,7 @@ enum spi_nor_option_flags { >> SNOR_F_IO_MODE_EN_VOLATILE = BIT(11), >> SNOR_F_SOFT_RESET = BIT(12), >> SNOR_F_SWP_IS_VOLATILE = BIT(13), >> + SNOR_F_HAS_QUAD_PP = BIT(14), > > you won't need this >> }; >> >> struct spi_nor_read_command { >> @@ -520,6 +521,7 @@ struct flash_info { >> u8 fixup_flags; >> #define SPI_NOR_4B_OPCODES BIT(0) >> #define SPI_NOR_IO_MODE_EN_VOLATILE BIT(1) >> +#define SPI_NOR_QUAD_PP BIT(2) > > No, as I previously said, SPI_NOR_QUAD_PP should be declared as a > info->flags, not as info->fixup_flags. > >> >> u8 mfr_flags; >> >> -- >> 2.30.2 >> > > > -- > Cheers, > ta > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/
On Wed, Aug 10, 2022 at 9:06 AM <Tudor.Ambarus@microchip.com> wrote: > > On 8/9/22 23:14, Sudip Mukherjee wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > SFDP table of some flash chips do not advertise support of Quad Input > > Page Program even though it has support. Use fixup flags and add hardware > > cap for these chips. > > <snip> > > @@ -520,6 +521,7 @@ struct flash_info { > > u8 fixup_flags; > > #define SPI_NOR_4B_OPCODES BIT(0) > > #define SPI_NOR_IO_MODE_EN_VOLATILE BIT(1) > > +#define SPI_NOR_QUAD_PP BIT(2) > > No, as I previously said, SPI_NOR_QUAD_PP should be declared as a > info->flags, not as info->fixup_flags. Sorry, I was confused as info->fixup_flags says "it indicates support that can be discovered via SFDP ideally, but can not be discovered for this particular flash because the SFDP table that indicates this support is not defined by the flash." -- Regards Sudip
On Wed, Aug 10, 2022 at 9:25 AM <Tudor.Ambarus@microchip.com> wrote: > > On 8/10/22 11:06, Tudor.Ambarus@microchip.com wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > On 8/9/22 23:14, Sudip Mukherjee wrote: > >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > >> > >> SFDP table of some flash chips do not advertise support of Quad Input > >> Page Program even though it has support. Use fixup flags and add hardware > >> cap for these chips. > >> > >> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com> > >> --- > >> drivers/mtd/spi-nor/core.c | 9 +++++++++ > >> drivers/mtd/spi-nor/core.h | 2 ++ > >> 2 files changed, 11 insertions(+) > >> > >> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c > >> index f2c64006f8d7..7542404332a5 100644 > >> --- a/drivers/mtd/spi-nor/core.c > >> +++ b/drivers/mtd/spi-nor/core.c > >> @@ -1962,6 +1962,12 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps) > >> if (nor->flags & SNOR_F_BROKEN_RESET) > >> *hwcaps &= ~(SNOR_HWCAPS_X_X_X | SNOR_HWCAPS_X_X_X_DTR); > >> > >> + if (nor->flags & SNOR_F_HAS_QUAD_PP) { > >> + *hwcaps |= SNOR_HWCAPS_PP_1_1_4; > >> + spi_nor_set_pp_settings(¶ms->page_programs[SNOR_CMD_PP_1_1_4], > >> + SPINOR_OP_PP_1_1_4, SNOR_PROTO_1_1_4); > >> + } > > > > setting SPINOR_OP_PP_1_1_4 should be done in spi_nor_late_init_params(). > > spi_nor_late_init_params() is used to adjust the ops supported by the flash > > ^ s/spi_nor_late_init_params/spi_nor_spimem_adjust_hwcaps So, do you mean something like this: diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c index f2c64006f8d7..2f41937b826d 100644 --- a/drivers/mtd/spi-nor/core.c +++ b/drivers/mtd/spi-nor/core.c @@ -1962,6 +1962,12 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps) if (nor->flags & SNOR_F_BROKEN_RESET) *hwcaps &= ~(SNOR_HWCAPS_X_X_X | SNOR_HWCAPS_X_X_X_DTR); + if (nor->info->flags & SPI_NOR_QUAD_PP) { + *hwcaps |= SNOR_HWCAPS_PP_1_1_4; + spi_nor_set_pp_settings(¶ms->page_programs[SNOR_CMD_PP_1_1_4], + SPINOR_OP_PP_1_1_4, SNOR_PROTO_1_1_4); + } + for (cap = 0; cap < sizeof(*hwcaps) * BITS_PER_BYTE; cap++) { int rdidx, ppidx; diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h index 85b0cf254e97..10aa1c72000f 100644 --- a/drivers/mtd/spi-nor/core.h +++ b/drivers/mtd/spi-nor/core.h @@ -507,6 +507,7 @@ struct flash_info { #define SPI_NOR_NO_ERASE BIT(6) #define NO_CHIP_ERASE BIT(7) #define SPI_NOR_NO_FR BIT(8) +#define SPI_NOR_QUAD_PP BIT(9) u8 no_sfdp_flags; #define SPI_NOR_SKIP_SFDP BIT(0) diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c index 89a66a19d754..014cd9038bed 100644 --- a/drivers/mtd/spi-nor/issi.c +++ b/drivers/mtd/spi-nor/issi.c @@ -71,8 +71,9 @@ 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) + FLAGS(SPI_NOR_QUAD_PP) .fixups = &is25lp256_fixups }, /* PMC */ -- Regards Sudip
On 8/10/22 18:14, Sudip Mukherjee wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On Wed, Aug 10, 2022 at 9:06 AM <Tudor.Ambarus@microchip.com> wrote: >> >> On 8/9/22 23:14, Sudip Mukherjee wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>> >>> SFDP table of some flash chips do not advertise support of Quad Input >>> Page Program even though it has support. Use fixup flags and add hardware >>> cap for these chips. >>> > > <snip> > >>> @@ -520,6 +521,7 @@ struct flash_info { >>> u8 fixup_flags; >>> #define SPI_NOR_4B_OPCODES BIT(0) >>> #define SPI_NOR_IO_MODE_EN_VOLATILE BIT(1) >>> +#define SPI_NOR_QUAD_PP BIT(2) >> >> No, as I previously said, SPI_NOR_QUAD_PP should be declared as a >> info->flags, not as info->fixup_flags. > > Sorry, I was confused as info->fixup_flags says "it indicates support > that can be discovered via SFDP ideally, but can not be discovered > for this particular flash because the SFDP table that indicates this > support is not defined by the flash." > Right, I've just sent a patch addressing this, hopefully is clearer now. Check it here: https://lore.kernel.org/linux-mtd/20220810155924.1366072-1-tudor.ambarus@microchip.com/T/#u > -- > Regards > Sudip
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c index f2c64006f8d7..7542404332a5 100644 --- a/drivers/mtd/spi-nor/core.c +++ b/drivers/mtd/spi-nor/core.c @@ -1962,6 +1962,12 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps) if (nor->flags & SNOR_F_BROKEN_RESET) *hwcaps &= ~(SNOR_HWCAPS_X_X_X | SNOR_HWCAPS_X_X_X_DTR); + if (nor->flags & SNOR_F_HAS_QUAD_PP) { + *hwcaps |= SNOR_HWCAPS_PP_1_1_4; + spi_nor_set_pp_settings(¶ms->page_programs[SNOR_CMD_PP_1_1_4], + SPINOR_OP_PP_1_1_4, SNOR_PROTO_1_1_4); + } + for (cap = 0; cap < sizeof(*hwcaps) * BITS_PER_BYTE; cap++) { int rdidx, ppidx; @@ -2446,6 +2452,9 @@ static void spi_nor_init_fixup_flags(struct spi_nor *nor) if (fixup_flags & SPI_NOR_IO_MODE_EN_VOLATILE) nor->flags |= SNOR_F_IO_MODE_EN_VOLATILE; + + if (fixup_flags & SPI_NOR_QUAD_PP) + nor->flags |= SNOR_F_HAS_QUAD_PP; } /** diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h index 85b0cf254e97..7dbdf16a67b4 100644 --- a/drivers/mtd/spi-nor/core.h +++ b/drivers/mtd/spi-nor/core.h @@ -130,6 +130,7 @@ enum spi_nor_option_flags { SNOR_F_IO_MODE_EN_VOLATILE = BIT(11), SNOR_F_SOFT_RESET = BIT(12), SNOR_F_SWP_IS_VOLATILE = BIT(13), + SNOR_F_HAS_QUAD_PP = BIT(14), }; struct spi_nor_read_command { @@ -520,6 +521,7 @@ struct flash_info { u8 fixup_flags; #define SPI_NOR_4B_OPCODES BIT(0) #define SPI_NOR_IO_MODE_EN_VOLATILE BIT(1) +#define SPI_NOR_QUAD_PP BIT(2) u8 mfr_flags;
SFDP table of some flash chips do not advertise support of Quad Input Page Program even though it has support. Use fixup flags and add hardware cap for these chips. Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com> --- drivers/mtd/spi-nor/core.c | 9 +++++++++ drivers/mtd/spi-nor/core.h | 2 ++ 2 files changed, 11 insertions(+)