Messages in this thread | | | Date | Wed, 26 Jul 2023 09:56:13 +0300 | Subject | Re: [RESEND PATCH v3 11/11] mtd: spi-nor: spansion: switch cypress_nor_get_page_size() to use vreg_offset | From | Tudor Ambarus <> |
| |
On 26.07.2023 07:15, Takahiro Kuwano wrote: > Hi Tudor, > > On 7/24/2023 5:12 PM, Tudor Ambarus wrote: >> All users of cypress_nor_get_page_size() retrieve n_dice and vreg_offset >> from SFDP. Switch cypress_nor_get_page_size() to always use vreg_offset >> so that we use the same code base for both single and multi chip package >> flashes. cypress_nor_get_page_size() is now called in the post_sfdp() >> hook instead of post_bfpt(), as vreg_offset and n_dice are parsed after >> BFPT and we now use them to get the page size in the post_sfdp hook. >> Consequently the null checks on n_dice and vreg_offset are moved to >> the post_sfdp() hook. >> >> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> >> --- >> drivers/mtd/spi-nor/spansion.c | 105 +++++++++++++-------------------- >> 1 file changed, 42 insertions(+), 63 deletions(-) >> >> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c >> index 4027f0038ce5..0b01af33aa57 100644 >> --- a/drivers/mtd/spi-nor/spansion.c >> +++ b/drivers/mtd/spi-nor/spansion.c >> @@ -471,28 +471,17 @@ static int cypress_nor_set_addr_mode_nbytes(struct spi_nor *nor) >> return 0; >> } >> >> -static int cypress_nor_get_page_size_single_chip(struct spi_nor *nor) >> -{ >> - struct spi_mem_op op = >> - CYPRESS_NOR_RD_ANY_REG_OP(nor->params->addr_mode_nbytes, >> - SPINOR_REG_CYPRESS_CFR3V, 0, >> - nor->bouncebuf); >> - int ret; >> - >> - ret = spi_nor_read_any_reg(nor, &op, nor->reg_proto); >> - if (ret) >> - return ret; >> - >> - if (nor->bouncebuf[0] & SPINOR_REG_CYPRESS_CFR3_PGSZ) >> - nor->params->page_size = 512; >> - else >> - nor->params->page_size = 256; >> - >> - return 0; >> -} >> - >> - >> -static int cypress_nor_get_page_size_mcp(struct spi_nor *nor) >> +/** >> + * cypress_nor_get_page_size() - Get flash page size configuration. >> + * @nor: pointer to a 'struct spi_nor' >> + * >> + * The BFPT table advertises a 512B or 256B page size depending on part but the >> + * page size is actually configurable (with the default being 256B). Read from >> + * CFR3V[4] and set the correct size. >> + * >> + * Return: 0 on success, -errno otherwise. >> + */ >> +static int cypress_nor_get_page_size(struct spi_nor *nor) >> { >> struct spi_mem_op op = >> CYPRESS_NOR_RD_ANY_REG_OP(nor->params->addr_mode_nbytes, >> @@ -522,23 +511,6 @@ static int cypress_nor_get_page_size_mcp(struct spi_nor *nor) >> return 0; >> } >> >> -/** >> - * cypress_nor_get_page_size() - Get flash page size configuration. >> - * @nor: pointer to a 'struct spi_nor' >> - * >> - * The BFPT table advertises a 512B or 256B page size depending on part but the >> - * page size is actually configurable (with the default being 256B). Read from >> - * CFR3V[4] and set the correct size. >> - * >> - * Return: 0 on success, -errno otherwise. >> - */ >> -static int cypress_nor_get_page_size(struct spi_nor *nor) >> -{ >> - if (nor->params->n_dice) >> - return cypress_nor_get_page_size_mcp(nor); >> - return cypress_nor_get_page_size_single_chip(nor); >> -} >> - >> static void cypress_nor_ecc_init(struct spi_nor *nor) >> { >> /* >> @@ -575,20 +547,26 @@ s25fs256t_post_bfpt_fixup(struct spi_nor *nor, >> if (nor->bouncebuf[0]) >> return -ENODEV; >> >> - return cypress_nor_get_page_size(nor); >> + return 0; >> } >> >> static int s25fs256t_post_sfdp_fixup(struct spi_nor *nor) >> { >> struct spi_nor_flash_parameter *params = nor->params; >> >> + if (!params->n_dice || !params->vreg_offset) { >> + dev_err(nor->dev, "%s failed. The volatile register offset could not be retrieved from SFDP.\n", >> + __func__); >> + return -EOPNOTSUPP; >> + } >> + > S25FS256T failed here. I realized that S25FS256T doesn't have SCCR map... > With fix like below, it works. Make sense?
Yes it does, will resubmit. > > params->vreg_offset = devm_kmalloc(nor->dev, sizeof(u32), GFP_KERNEL); > if (!params->vreg_offset) > return -ENOMEM; > > params->vreg_offset[0] = SPINOR_REG_CYPRESS_VREG; > params->n_dice = 1; > > >> /* PP_1_1_4_4B is supported but missing in 4BAIT. */ >> params->hwcaps.mask |= 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_4B, >> SNOR_PROTO_1_1_4); >> >> - return 0; >> + return cypress_nor_get_page_size(nor); >> } >> >> static int s25fs256t_late_init(struct spi_nor *nor) >> @@ -623,10 +601,20 @@ s25hx_t_post_bfpt_fixup(struct spi_nor *nor, >> >> static int s25hx_t_post_sfdp_fixup(struct spi_nor *nor) >> { >> - struct spi_nor_erase_type *erase_type = >> - nor->params->erase_map.erase_type; >> + struct spi_nor_flash_parameter *params = nor->params; >> + struct spi_nor_erase_type *erase_type = params->erase_map.erase_type; >> unsigned int i; >> >> + if (!params->n_dice || !params->vreg_offset) { >> + dev_err(nor->dev, "%s failed. The volatile register offset could not be retrieved from SFDP.\n", >> + __func__); >> + return -EOPNOTSUPP; >> + } >> + >> + /* The 2 Gb parts duplicate info and advertise 4 dice instead of 2. */ >> + if (params->size == SZ_256M) >> + params->n_dice = 2; >> + >> /* >> * In some parts, 3byte erase opcodes are advertised by 4BAIT. >> * Convert them to 4byte erase opcodes. >> @@ -644,10 +632,6 @@ static int s25hx_t_post_sfdp_fixup(struct spi_nor *nor) >> } >> } >> >> - /* The 2 Gb parts duplicate info and advertise 4 dice instead of 2. */ >> - if (nor->params->size == SZ_256M) >> - nor->params->n_dice = 2; >> - >> return cypress_nor_get_page_size(nor); >> } >> >> @@ -655,12 +639,6 @@ static int s25hx_t_late_init(struct spi_nor *nor) >> { >> struct spi_nor_flash_parameter *params = nor->params; >> >> - if (!params->n_dice || !params->vreg_offset) { >> - dev_err(nor->dev, "%s failed. The volatile register offset could not be retrieved from SFDP.\n", >> - __func__); >> - return -EOPNOTSUPP; >> - } >> - >> /* Fast Read 4B requires mode cycles */ >> params->reads[SNOR_CMD_READ_FAST].num_mode_clocks = 8; >> params->ready = cypress_nor_sr_ready_and_clear; >> @@ -694,6 +672,17 @@ static int cypress_nor_set_octal_dtr(struct spi_nor *nor, bool enable) >> static int s28hx_t_post_sfdp_fixup(struct spi_nor *nor) >> { >> struct spi_nor_flash_parameter *params = nor->params; >> + >> + if (!params->n_dice || !params->vreg_offset) { >> + dev_err(nor->dev, "%s failed. The volatile register offset could not be retrieved from SFDP.\n", >> + __func__); >> + return -EOPNOTSUPP; >> + } >> + >> + /* The 2 Gb parts duplicate info and advertise 4 dice instead of 2. */ >> + if (params->size == SZ_256M) >> + params->n_dice = 2; >> + >> /* >> * On older versions of the flash the xSPI Profile 1.0 table has the >> * 8D-8D-8D Fast Read opcode as 0x00. But it actually should be 0xEE. >> @@ -719,10 +708,6 @@ static int s28hx_t_post_sfdp_fixup(struct spi_nor *nor) >> */ >> params->rdsr_addr_nbytes = 4; >> >> - /* The 2 Gb parts duplicate info and advertise 4 dice instead of 2. */ >> - if (params->size == SZ_256M) >> - params->n_dice = 2; >> - >> return cypress_nor_get_page_size(nor); >> } >> >> @@ -737,12 +722,6 @@ static int s28hx_t_late_init(struct spi_nor *nor) >> { >> struct spi_nor_flash_parameter *params = nor->params; >> >> - if (!params->n_dice || !params->vreg_offset) { >> - dev_err(nor->dev, "%s failed. The volatile register offset could not be retrieved from SFDP.\n", >> - __func__); >> - return -EOPNOTSUPP; >> - } >> - >> params->set_octal_dtr = cypress_nor_set_octal_dtr; >> params->ready = cypress_nor_sr_ready_and_clear; >> cypress_nor_ecc_init(nor); > > All other S25Hx and S28Hx devices work fine. >
Good!
| |