| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <9cc81d00-7cce-93ef-7d43-adef519b1c95@linaro.org>
Date: Wed, 26 Jul 2023 09:56:13 +0300
From: Tudor Ambarus <tudor.ambarus@...aro.org>
To: Takahiro Kuwano <tkuw584924@...il.com>,
takahiro.kuwano@...ineon.com, michael@...le.cc
Cc: pratyush@...nel.org, linux-mtd@...ts.infradead.org,
linux-kernel@...r.kernel.org, bacem.daassi@...ineon.com,
miquel.raynal@...tlin.com, richard@....at
Subject: Re: [RESEND PATCH v3 11/11] mtd: spi-nor: spansion: switch
cypress_nor_get_page_size() to use vreg_offset
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@...aro.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!
Powered by blists - more mailing lists