[PATCH v5 09/14] mtd: spi-nor: core: Init all flash parameters based on SFDP where possible

[Tudor.Ambarus at microchip.com]

On 12/6/21 5:04 PM, Pratyush Yadav wrote:
>>>> @@ -2773,24 +2807,28 @@ static void spi_nor_late_init_params(struct spi_nor *nor)
>>>>   * parameters that are not declared in the JESD216 SFDP standard, or where SFDP
>>>>   * tables are not defined at all.
>>>>   *           spi_nor_late_init_params()
>>>> + *
>>>> + * Return: 0 on success, -errno otherwise.
>>>>   */
>>>>  static int spi_nor_init_params(struct spi_nor *nor)
>>>>  {
>>>> +     int ret;
>>>> +
>>>>       nor->params = devm_kzalloc(nor->dev, sizeof(*nor->params), GFP_KERNEL);
>>>>       if (!nor->params)
>>>>               return -ENOMEM;
>>>>
>>>> -     spi_nor_info_init_params(nor);
>>>> -
>>>> -     spi_nor_manufacturer_init_params(nor);
>>>> +     spi_nor_init_default_params(nor);
>>>>
>>>> -     if ((nor->info->parse_sfdp ||
>>>> -          (nor->info->no_sfdp_flags & (SPI_NOR_DUAL_READ |
>>>> -                                       SPI_NOR_QUAD_READ |
>>>> -                                       SPI_NOR_OCTAL_READ |
>>>> -                                       SPI_NOR_OCTAL_DTR_READ))) &&
>>>> -         !(nor->info->no_sfdp_flags & SPI_NOR_SKIP_SFDP))
>>>> -             spi_nor_sfdp_init_params(nor);
>>>> +     if (nor->info->parse_sfdp) {
>>>> +             ret = spi_nor_parse_sfdp(nor);
>>>> +             if (ret) {
>>>> +                     dev_err(nor->dev, "BFPT parsing failed. Please consider using SPI_NOR_SKIP_SFDP when declaring the flash\n");
>>>> +                     return ret;
>>>> +             }
>>>> +     } else {
>>>> +             spi_nor_init_params_deprecated(nor);
>>> Hmm, a flash without SFDP would always go via the "deprecated" path even
>>> if it is a new entry. It makes no difference in practice, but is just
>>> slightly confusing logically. Can we make this a bit clearer? How about
>>> this:
>>>
>>>   if (nor->info->parse_sfdp) {
>>>         ...
>>>   } else if (nor->info->no_sfdp_flags & SPI_NOR_SKIP_SFDP) {
>>>         spi_nor_no_sfdp_init_params();
>>>         spi_nor_manufacturer_init_params();

We'll get rid of the default_init() hook anyway, so I will
remove the call to spi_nor_manufacturer_init_params().

Cheers,
ta

>>>   } else {
>>>         spi_nor_init_params_deprecated();
>>>   }
>>>
>>>> +     }
>> Ok, will do. Can I add your R-b tag on a local v6 with this change that you
>> proposed? It will spare me of sending v6.
> Sure, go ahead.
>