From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pratyush Yadav Date: Mon, 8 Mar 2021 14:24:34 +0530 Subject: [PATCH v5 09/10] mtd: spi-nor-core: Add fixups for Cypress s25hl-t/s25hs-t In-Reply-To: References: <20210224124043.4cwamgxb5lt3ibgu@ti.com> Message-ID: <20210308085432.qc22vvc7ffphkvb4@ti.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 08/03/21 05:47PM, Takahiro Kuwano wrote: > On 2/24/2021 9:40 PM, Pratyush Yadav wrote: > > On 19/02/21 10:56AM, tkuw584924 at gmail.com wrote: > >> From: Takahiro Kuwano > >> > >> 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. > > > Noted with thanks. > > >> > >> 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. > > > OK. > > >> > >> 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. > > > Yes. I tested both. > > >> > >> The post_bfpt/sfdp() fixes the params wrongly advertised in SFDP. > >> > >> [0] https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-7-p.yadav at ti.com/ > >> [1] https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-8-p.yadav at ti.com/ > >> [2] https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-9-p.yadav at ti.com/ > >> > >> Signed-off-by: Takahiro Kuwano > >> --- > >> 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 [...] > >> +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? > > > Yes. The nor->addr_width must be 4 for read/write/erase to work correctly. > I think device's addressing mode should be in sync with nor->addr_width. Right. But the comment above implies that 4-byte mode is only enabled for the "Read Any Register" command. Either remove the comment completely or make it clear that you are changing to 4-byte for all further operations, not just the Read Any Register command. > > >> + 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; > >> +} > >> + -- Regards, Pratyush Yadav Texas Instruments Inc.