Re: [PATCH] mtd: spi-nor: Fix quad-enable for flashes with QER bit in SR1

["Michael Walle" <mwalle@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1584 bytes --]

On Fri Sep 11, 2026 at 11:19 AM CEST, Miquel Raynal wrote:
>
>>> -	ret = spi_nor_read_sr1_and_sr2(nor, sr);
>>> +	if (qe_mask[0])
>>> +		ret = spi_nor_read_sr1(nor, sr);
>>> +	else
>>> +		ret = spi_nor_read_sr1_and_sr2(nor, sr);
>>
>> Shouldn't that be the other way around?
>>
>> +	if (qe_mask[1])
>> +		ret = spi_nor_read_sr1_and_sr2(nor, sr);
>> +	else
>> +		ret = spi_nor_read_sr1(nor, sr);
>>
>> I know qe_mask won't be spread across two SRs. Just to be
>> correct and make it easier to grok.
>
> Why is the second version easier to parse? First version is maybe more
> future proof since we are treating a specific case. Because either it is
> a specific chip (Mxic) or we end up in the more common case. There is
> only one QER entry for qe_mask[0], many for qe_mask[1], and chances are
> that in the future we will mostly see new chips only using a qe_mask[1]
> mask. So having this in the else doesn't sound so strange to me? I also
> don't think we will ever see masks spanning the two registers.

Because for the first one you'll have to know that qe_mask wont span
two different registers, where with my proposal, you don't have to
have that knowledge, no?

If qe_mask[1] is used regardless of qe_mask[0], you know you have to
write sr2, otherwise you'd be skipping something.

With the former, qe_mask[1] might or might not be 0 and you could
end up in spi_nor_read_sr1().

What do you mean with future prove? In the end it's the very same
logic iff there will only be one bit set in qe_mask[] and
sizeof(qe_mask) == 2.

-michael

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]