Re: [PATCH v2 2/3] mtd: spi-nor: add SFDP fixups for Quad Page Program

[Tudor.Ambarus]

On 8/10/22 11:06, Tudor.Ambarus@xxxxxxxxxxxxx wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 8/9/22 23:14, Sudip Mukherjee wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> SFDP table of some flash chips do not advertise support of Quad Input
>> Page Program even though it has support. Use fixup flags and add hardware
>> cap for these chips.
>>
>> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@xxxxxxxxxx>
>> ---
>>  drivers/mtd/spi-nor/core.c | 9 +++++++++
>>  drivers/mtd/spi-nor/core.h | 2 ++
>>  2 files changed, 11 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>> index f2c64006f8d7..7542404332a5 100644
>> --- a/drivers/mtd/spi-nor/core.c
>> +++ b/drivers/mtd/spi-nor/core.c
>> @@ -1962,6 +1962,12 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)
>>         if (nor->flags & SNOR_F_BROKEN_RESET)
>>                 *hwcaps &= ~(SNOR_HWCAPS_X_X_X | SNOR_HWCAPS_X_X_X_DTR);
>>
>> +       if (nor->flags & SNOR_F_HAS_QUAD_PP) {
>> +               *hwcaps |= SNOR_HWCAPS_PP_1_1_4;
>> +               spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_1_1_4],
>> +                                       SPINOR_OP_PP_1_1_4, SNOR_PROTO_1_1_4);
>> +       }
> 
> setting SPINOR_OP_PP_1_1_4 should be done in spi_nor_late_init_params().
> spi_nor_late_init_params() is used to adjust the ops supported by the flash

^ s/spi_nor_late_init_params/spi_nor_spimem_adjust_hwcaps

> with the ones supported by the controller.
> 
>> +
>>         for (cap = 0; cap < sizeof(*hwcaps) * BITS_PER_BYTE; cap++) {
>>                 int rdidx, ppidx;
>>
>> @@ -2446,6 +2452,9 @@ static void spi_nor_init_fixup_flags(struct spi_nor *nor)
>>
>>         if (fixup_flags & SPI_NOR_IO_MODE_EN_VOLATILE)
>>                 nor->flags |= SNOR_F_IO_MODE_EN_VOLATILE;
>> +
>> +       if (fixup_flags & SPI_NOR_QUAD_PP)
>> +               nor->flags |= SNOR_F_HAS_QUAD_PP;
>>  }
>>
>>  /**
>> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
>> index 85b0cf254e97..7dbdf16a67b4 100644
>> --- a/drivers/mtd/spi-nor/core.h
>> +++ b/drivers/mtd/spi-nor/core.h
>> @@ -130,6 +130,7 @@ enum spi_nor_option_flags {
>>         SNOR_F_IO_MODE_EN_VOLATILE = BIT(11),
>>         SNOR_F_SOFT_RESET       = BIT(12),
>>         SNOR_F_SWP_IS_VOLATILE  = BIT(13),
>> +       SNOR_F_HAS_QUAD_PP      = BIT(14),
> 
> you won't need this
>>  };
>>
>>  struct spi_nor_read_command {
>> @@ -520,6 +521,7 @@ struct flash_info {
>>         u8 fixup_flags;
>>  #define SPI_NOR_4B_OPCODES             BIT(0)
>>  #define SPI_NOR_IO_MODE_EN_VOLATILE    BIT(1)
>> +#define SPI_NOR_QUAD_PP                        BIT(2)
> 
> No, as I previously said, SPI_NOR_QUAD_PP should be declared as a
> info->flags, not as info->fixup_flags.
> 
>>
>>         u8 mfr_flags;
>>
>> --
>> 2.30.2
>>
> 
> 
> --
> Cheers,
> ta
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/


-- 
Cheers,
ta