Re: [PATCH v3 00/15] mtd: spi-nor: macronix: workaround for device id re-use

[Tudor Ambarus <tudor.ambarus@linaro.org>]

On 9/26/24 11:47 AM, Tudor Ambarus wrote:
> Hi, Esben,
> 
> Thank you for the perseverance :D
> 
> On 9/26/24 8:56 AM, Esben Haabendal wrote:
>> "Michael Walle" <mwalle@kernel.org> writes:
>>
>>> Hi,
>>>
>>> On Thu Jul 11, 2024 at 3:00 PM CEST, Esben Haabendal wrote:
>>>> As a consequence, the SPI_NOR_SKIP_SFDP flag is no more, and all
>>>> drivers that have been doing optional SFDP is now marked explicitly to
>>>> do that using the SPI_NOR_TRY_SFDP.
>>>
>>> First, I haven't looked at your patchset at the moment. But I'd like
>>> to take the opportunity to discuss the following (and excuse me that
>>> I didn't had this idea before all your work on this).
>>>
>>> First, I'd like to see it the other way around, that is, doing SFDP
>>> by default and let the driver opt-out instead of opt-in. This will
>>> also align with the current "SFDP only driver", i.e. if a flash is
>>> not known we try SFDP anyway. Going forward, I'd say this is also
>>> the sane behavior and we don't have to add any flags if someone want
>>> to add support for an (older) flash with the same ID but without
>>> SFDP. With the current approach, we'd have to add the TRY_SFDP flag.
>>>
>>> Now we might play it safe and add that SPI_NOR_SKIP_SFDP to any
>>> flash which doesn't do the SFDP parsing (because it has size != 0
>>> and not any of the magic flags set) - or we might just go ahead and
>>> do the probing first for *any* flashes. Yes we might issue an
>>> unsupported opcode, but we already do that with the generic SFDP
>>> flash driver. So no big deal maybe (?). Vendors where we know for a
>>> fact that they don't have any SFDP (Everspin I'm looking at you),
>>> would of course set the SKIP_SFDP flag.
> 
> Issuing RDSFDP for flashes that don't support it shouldn't be too bad
> indeed, it's not recommended, but it shall be fine. What I'm worried
> about is flashes with wrong SFDP data (see all the SFDP fixup hooks that
> we have). So my suggestion is to play it safe unless one of you guys
> step up and commit that will fix or help fix each bug that results from
> this.
> 
> I'd like you to also consider the SFDP versions and how many parameters
> they are exposing. I can't tell exactly, but if I remember correctly,
> rev A had 9 dwords for BFPT, revB 16, and revC and other more. If we
> consider static init folowed by SFDP with rollback option, point 3/ in
> your cover letter, are we sure that all the parameters that are
> initialized before parsing SFDP are overwritten at SFDP parsing time? If
> yes, we shall be fine.
> 
> Esben, to speed up the things on both ends, I recommend you for v4 to
> draft just a core patch if you care, then you can handle the vendor
> drivers. Or send us some pseudocode and we can talk on that.
> 

To summarize, I like the idea too, as far as we can keep backward
compatibility.