[PATCH v5 09/10] mtd: spi-nor-core: Add fixups for Cypress s25hl-t/s25hs-t
Pratyush Yadav
p.yadav at ti.com
Wed Feb 24 13:40:45 CET 2021
On 19/02/21 10:56AM, tkuw584924 at gmail.com wrote:
> From: Takahiro Kuwano <Takahiro.Kuwano at 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.yadav@ti.com/
> [1] https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-8-p.yadav@ti.com/
> [2] https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-9-p.yadav@ti.com/
>
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano at 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.
More information about the U-Boot
mailing list