Re: [PATCH v2 17/35] mtd: spi-nor: Introduce spi_nor_nonsfdp_flags_init()

Tudor.Ambarus
2021-10-22T12:43:09Z

On 10/22/21 3:10 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 22/10/21 01:21PM, Michael Walle wrote:
>> Am 2021-08-17 12:24, schrieb Pratyush Yadav:
>>> On 27/07/21 07:52AM, Tudor Ambarus wrote:
>>>> Used to initialize the NOR flags for settings that are not defined
>>>> in the JESD216 SFDP standard, thus can not be retrieved when parsing
>>>> SFDP. No functional change.
>>>
>>> I am worried if the order in which these flags are set can cause some
>>> subtle bugs.
>>>
>>> I can see one instance of it with SNOR_F_HAS_LOCK.
>>> spi_nor_late_init_params() checks for SNOR_F_HAS_LOCK and if there are
>>> no locking ops specified, it sets the default locking ops. This works
>>> fine before this patch because the flag is set before the function is
>>> called. But now, the flag will be set _after_ the function is called,
>>> and so you will never be able to set the default flags.
>>
>> Maybe we should just forbid to look at the SNOR_F_ flags in these
>> functions. Instead the information could also be deduced by looking at
>> the SPI_NOR_ flags.

not true.

>
> TBH I never quite understood why we have two sets of flags in the first
> place, when the SNOR_F* flags pretty much mirror what SPI_NOR* flags
> mean. Dunno...
>
struct spi_nor {
cut
const struct flash_info *info;
cut
}

const, which is the way it should be. Every flag that is discovered at SFDP
time will fill the SNOR_F correspondent, and only SNOR_F will be used across
the core driver.