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

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: "Michael Walle" <mwalle@kernel.org>
Cc: "Pratyush Yadav" <pratyush@kernel.org>,
	 "Takahiro Kuwano" <takahiro.kuwano@infineon.com>,
	 "Richard Weinberger" <richard@nod.at>,
	"Vignesh Raghavendra" <vigneshr@ti.com>,
	 "Thomas Petazzoni" <thomas.petazzoni@bootlin.com>,
	 "Jon Hunter" <jonathanh@nvidia.com>,
	"Steam Lin" <STLin2@winbond.com>,
	 <linux-mtd@lists.infradead.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] mtd: spi-nor: Fix quad-enable for flashes with QER bit in SR1
Date: Fri, 11 Sep 2026 11:19:39 +0200	[thread overview]
Message-ID: <87o6e4qh5g.fsf@bootlin.com> (raw)
In-Reply-To: <DLCD7NN2IW82.33ISZCKO1E7UY@kernel.org> (Michael Walle's message of "Fri, 11 Sep 2026 11:04:31 +0200")

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

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/