On 19/02/21 10:56AM, tkuw584...@gmail.com wrote: > From: Takahiro Kuwano <takahiro.kuw...@infineon.com> > > This patch adds Flash specific fixups and hooks for Cypress > S25HL-T/S25HS-T family, based on the features introduced in [0][1][2].
Instead of linking the patches like this, it would be a better idea to simply include them in your series. This will make your series independent from mine and will make it easier for the maintainer to apply it. > > The nor->ready() and spansion_sr_ready() introduced in #5 and #6 in this > series are used for multi-die package parts. Nitpick: Once this patch makes it into the Git history there is no patch #5 or #6. Probably a better idea to just say "introduced earlier" or something similar. > > The nor->quad_enable() sets the volatile QE bit on each die. > > The mtd._erase() is hooked if the device is single-die package and not > configured to uniform sectors, assuming it is in factory default > configuration that has 32 x 4KB sectors overlaid on bottom address. > Other configurations, top and split, are not supported at this point. > Will submit additional patches to support it as needed. Ah, this patch does check for uniform sector map. Nice. I assume you have tested with both configurations. > > The post_bfpt/sfdp() fixes the params wrongly advertised in SFDP. > > [0] > https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-7-p.ya...@ti.com/ > [1] > https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-8-p.ya...@ti.com/ > [2] > https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-9-p.ya...@ti.com/ > > Signed-off-by: Takahiro Kuwano <takahiro.kuw...@infineon.com> > --- > Changes in v5: > - Add s25hx_t_erase_non_uniform() > - Change mtd.writesize and mtd.flags in s25hx_t_setup() > - Fix page size and erase size issues in s25hx_t_post_bfpt_fixup() > > drivers/mtd/spi/spi-nor-core.c | 155 +++++++++++++++++++++++++++++++++ > include/linux/mtd/spi-nor.h | 3 + > 2 files changed, 158 insertions(+) > > diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c > index 8d63681cb3..315e26ab27 100644 > --- a/drivers/mtd/spi/spi-nor-core.c > +++ b/drivers/mtd/spi/spi-nor-core.c > @@ -2677,8 +2677,163 @@ static int spi_nor_init(struct spi_nor *nor) > return 0; > } > > +#ifdef CONFIG_SPI_FLASH_SPANSION > +static int s25hx_t_mdp_ready(struct spi_nor *nor) > +{ > + u32 addr; > + int ret; > + > + for (addr = 0; addr < nor->mtd.size; addr += SZ_128M) { > + ret = spansion_sr_ready(nor, addr, 0); > + if (ret != 1) Nitpick: Use if (!ret) since spansion_sr_ready() returns a boolean value. > + return ret; > + } > + > + return 1; > +} > + > +static int s25hx_t_quad_enable(struct spi_nor *nor) > +{ > + u32 addr; > + int ret; > + > + for (addr = 0; addr < nor->mtd.size; addr += SZ_128M) { > + ret = spansion_quad_enable_volatile(nor, addr, 0); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > +static int s25hx_t_erase_non_uniform(struct mtd_info *mtd, > + struct erase_info *instr) > +{ > + /* Support factory default configuration (32 x 4KB sectors at bottom) */ > + return spansion_erase_non_uniform(mtd, instr, 0, SZ_128K); > +} > + > +static int s25hx_t_setup(struct spi_nor *nor, const struct flash_info *info, > + const struct spi_nor_flash_parameter *params, > + const struct spi_nor_hwcaps *hwcaps) > +{ > +#ifdef CONFIG_SPI_FLASH_BAR > + return -ENOTSUPP; /* Bank Address Register is not supported */ > +#endif > + /* > + * The Cypress Semper family has transparent ECC. To preserve > + * ECC enabled, multi-pass programming within the same 16-byte > + * ECC data unit needs to be avoided. > + */ > + nor->mtd.writesize = 16; Sorry, I forgot what your end goal was with setting the writesize. Which layer is doing multi-pass writes? You would need to check if those layers actually respect this value. In Linux some consumers of the MTD layer simply assumed the writesize for a NOR flash is 1 (like UBI for example). I had to patch them before everything was back to normal. > + > + if (nor->mtd.size > SZ_128M) { > + /* > + * For the multi-die package parts, the ready() hook is needed > + * to check all dies' status via read any register. > + */ > + nor->ready = s25hx_t_mdp_ready; > + } else { > + int ret; > + u8 cfr3v; > + > + /* > + * Read CFR3V to check if uniform sector is selected. If not, > + * assign an erase hook that supports non-uniform erase, > + * assuming the part has factory default configuration, > + * 32 x 4KB sectors at bottom. > + */ > + ret = spansion_read_any_reg(nor, SPINOR_REG_ADDR_CFR3V, 0, > + &cfr3v); > + if (ret) > + return ret; > + > + if (!(cfr3v & CFR3V_UNHYSA)) > + nor->mtd._erase = s25hx_t_erase_non_uniform; Multi-die parts can also have uniform sector sizes. Why not do this check for them as well? > + } > + > + return spi_nor_default_setup(nor, info, params, hwcaps); > +} > + > +static void s25hx_t_default_init(struct spi_nor *nor) > +{ > + nor->setup = s25hx_t_setup; > +} Ok. > + > +static int s25hx_t_post_bfpt_fixup(struct spi_nor *nor, > + const struct sfdp_parameter_header *header, > + const struct sfdp_bfpt *bfpt, > + struct spi_nor_flash_parameter *params) > +{ > + int ret; > + u32 addr; > + u8 cfr3v; > + > + /* erase size in case it is set to 4K from BFPT */ > + nor->erase_opcode = SPINOR_OP_SE_4B; > + nor->mtd.erasesize = nor->info->sector_size; > + > + /* Enter 4-byte addressing mode for Read Any Register */ > + ret = set_4byte(nor, nor->info, 1); > + if (ret) > + return ret; You enter 4byte addressing but don't exit it before returning. Is this intentional? > + nor->addr_width = 4; > + > + /* > + * The page_size is set to 512B from BFPT, but it actually depends on > + * the configuration register. Look up the CFR3V and determine the > + * page_size. For multi-die package parts, use 512B only when the all > + * dies are configured to 512B buffer. > + */ > + for (addr = 0; addr < params->size; addr += SZ_128M) { > + ret = spansion_read_any_reg(nor, addr + SPINOR_REG_ADDR_CFR3V, > + 0, &cfr3v); > + if (ret) > + return ret; > + > + if (!(cfr3v & CFR3V_PGMBUF)) { > + params->page_size = 256; > + return 0; > + } > + } > + params->page_size = 512; Ok. > + > + return 0; > +} > + > +static void s25hx_t_post_sfdp_fixup(struct spi_nor *nor, > + struct spi_nor_flash_parameter *params) > +{ > + /* READ_FAST_4B (0Ch) requires mode cycles*/ > + params->reads[SNOR_CMD_READ_FAST].num_mode_clocks = 8; > + /* PP_1_1_4 is not supported */ > + params->hwcaps.mask &= ~SNOR_HWCAPS_PP_1_1_4; > + /* Use volatile register to enable quad */ > + params->quad_enable = s25hx_t_quad_enable; > +} > + > +static struct spi_nor_fixups s25hx_t_fixups = { > + .default_init = s25hx_t_default_init, > + .post_bfpt = s25hx_t_post_bfpt_fixup, > + .post_sfdp = s25hx_t_post_sfdp_fixup, > +}; > +#endif > + > static void spi_nor_set_fixups(struct spi_nor *nor) > { > +#ifdef CONFIG_SPI_FLASH_SPANSION > + if (JEDEC_MFR(nor->info) == SNOR_MFR_CYPRESS) { > + switch (nor->info->id[1]) { > + case 0x2a: /* S25HL (QSPI, 3.3V) */ > + case 0x2b: /* S25HS (QSPI, 1.8V) */ > + nor->fixups = &s25hx_t_fixups; > + break; Ok. > + > + default: > + break; > + } > + } > +#endif > } > > int spi_nor_scan(struct spi_nor *nor) > diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h > index 25234177de..cfb2104fee 100644 > --- a/include/linux/mtd/spi-nor.h > +++ b/include/linux/mtd/spi-nor.h > @@ -125,6 +125,9 @@ > #define SPINOR_OP_WRAR 0x71 /* Write any register */ > #define SPINOR_REG_ADDR_STR1V 0x00800000 > #define SPINOR_REG_ADDR_CFR1V 0x00800002 > +#define SPINOR_REG_ADDR_CFR3V 0x00800004 > +#define CFR3V_UNHYSA BIT(3) /* Uniform sectors or not */ > +#define CFR3V_PGMBUF BIT(4) /* Program buffer size */ Ok. > > /* Used for Micron flashes only. */ > #define SPINOR_OP_RD_EVCR 0x65 /* Read EVCR register */ > -- > 2.25.1 > -- Regards, Pratyush Yadav Texas Instruments Inc.
