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

[Esben Haabendal <esben@geanix.com>]

"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.

I like your idea.

Did you discuss this with Tudor?

On 9/23/24 7:04 PM, Tudor Ambarus wrote:
>>> * Always read Macronix chips SFDP, as Macronix replaced all old chips
>>> in the Manufacture table.
>>
>> I'll NACK it unless you prove that the old flavors of flashes are not
>> used anymore in the kernel.
>
> Even if you can prove that the older flashes are not used in the kernel
> anymore, we can't just switch to parsing SFDP, because we have seen in
> the past flashes with wrong SFDP data that made the flashes misbehave.
> The recommended way is to update just the flashes that you can test.

Does it make sense if I work on a patch implementing the proposal you
put forward, or is it possible to discuss it further before doing that
work?

If it will certainly be NACK'ed, I might as well try to find a different
approach.

/Esben