Re: [PATCH v11 2/3] mtd: spi-nor: spansion: Add local function to discover page size

[Takahiro Kuwano <tkuw584924@gmail.com>]

On 4/19/2022 5:06 PM, Tudor.Ambarus@microchip.com wrote:
> On 4/18/22 08:41, tkuw584924@gmail.com wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>>
>> The page size check in s28hs512t fixup can be used for s25hs/hl-t as well.
>> Move that to a newly created local function.
>>
>> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>> ---
>> Changes in v11:
>>   - Rebase on top of Tudor's series
>>     https://patchwork.ozlabs.org/project/linux-mtd/list/?series=294490
>>   - Add addr_width param
>>
>>  drivers/mtd/spi-nor/spansion.c | 54 ++++++++++++++++++++--------------
>>  1 file changed, 32 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
>> index 6bcd25180af4..493240ebfd70 100644
>> --- a/drivers/mtd/spi-nor/spansion.c
>> +++ b/drivers/mtd/spi-nor/spansion.c
>> @@ -177,6 +177,37 @@ static int cypress_nor_quad_enable_volatile(struct spi_nor *nor)
>>         return 0;
>>  }
>>
>> +/**
>> + * cypress_nor_set_page_size() - Set page size which corresponds to the flash
>> + *                               configuration.
>> + * @nor:       pointer to a 'struct spi_nor'
>> + * @addr_width:        address width used in Read Any Register op
>> + *
>> + * 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_set_page_size(struct spi_nor *nor, u8 addr_width)
>> +{
>> +       struct spi_mem_op op =
>> +               CYPRESS_NOR_RD_ANY_REG_OP(addr_width, SPINOR_REG_CYPRESS_CFR3V,
>> +                                         nor->bouncebuf);
>> +       int ret;
>> +
>> +       ret = spi_nor_read_reg(nor, &op, nor->reg_proto);
>> +       if (ret)
>> +               return ret;
>> +
>> +       if (nor->bouncebuf[0] & SPINOR_REG_CYPRESS_CFR3V_PGSZ)
>> +               nor->params->page_size = 512;
>> +       else
>> +               nor->params->page_size = 256;
>> +
>> +       return 0;
>> +}
>> +
>>  /**
>>   * cypress_nor_octal_dtr_enable() - Enable octal DTR on Cypress flashes.
>>   * @nor:               pointer to a 'struct spi_nor'
>> @@ -231,28 +262,7 @@ static int s28hs512t_post_bfpt_fixup(struct spi_nor *nor,
>>                                      const struct sfdp_parameter_header *bfpt_header,
>>                                      const struct sfdp_bfpt *bfpt)
>>  {
>> -       /*
>> -        * The BFPT table advertises a 512B page size but the page size is
>> -        * actually configurable (with the default being 256B). Read from
>> -        * CFR3V[4] and set the correct size.
>> -        */
>> -       struct spi_mem_op op =
>> -               CYPRESS_NOR_RD_ANY_REG_OP(3, SPINOR_REG_CYPRESS_CFR3V,
>> -                                         nor->bouncebuf);
>> -       int ret;
>> -
>> -       spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
>> -
>> -       ret = spi_mem_exec_op(nor->spimem, &op);
>> -       if (ret)
>> -               return ret;
>> -
>> -       if (nor->bouncebuf[0] & SPINOR_REG_CYPRESS_CFR3V_PGSZ)
>> -               nor->params->page_size = 512;
>> -       else
>> -               nor->params->page_size = 256;
>> -
>> -       return 0;
>> +       return cypress_nor_set_page_size(nor, 3);
> 
> not related to this patch, but can we use nor->add_width instead of 3?
> 
I confirmed with a s28hs512t device on hand that nor->addr_width is set
to 3 during BFPT parse. We can use nor->addr_width and eliminate addr_width
param from cypress_nor_set_page_size(). I will do it in next rev.

> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
>>  }
>>
>>  static const struct spi_nor_fixups s28hs512t_fixups = {
>> --
>> 2.25.1
>>
> 

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/