Re: [PATCH v5 14/28] mtd: spi-nor: swp: Create helpers for building the SR register

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Tudor Ambarus <tudor.ambarus@linaro.org>
Cc: Pratyush Yadav <pratyush@kernel.org>,
	 Michael Walle <mwalle@kernel.org>,
	 Takahiro Kuwano <takahiro.kuwano@infineon.com>,
	Richard Weinberger <richard@nod.at>,
	 Vignesh Raghavendra <vigneshr@ti.com>,
	 Jonathan Corbet <corbet@lwn.net>,
	 Shuah Khan <skhan@linuxfoundation.org>,
	 Sean Anderson <sean.anderson@linux.dev>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	 Steam Lin <STLin2@winbond.com>,
	 linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
	 linux-doc@vger.kernel.org
Subject: Re: [PATCH v5 14/28] mtd: spi-nor: swp: Create helpers for building the SR register
Date: Fri, 22 May 2026 18:35:52 +0200	[thread overview]
Message-ID: <87tsrzo107.fsf@bootlin.com> (raw)
In-Reply-To: <0640ef90-52f4-443b-90c9-ac4acb48d59f@linaro.org> (Tudor Ambarus's message of "Fri, 22 May 2026 12:56:55 +0300")

On 22/05/2026 at 12:56:55 +03, Tudor Ambarus <tudor.ambarus@linaro.org> wrote:

> On 5/7/26 7:46 PM, Miquel Raynal wrote:
>> The status register contains 3 or 4 BP (Block Protect) bits, 0 or 1
>> TB (Top/Bottom) bit, soon 0 or 1 CMP (Complement) bit. The last BP bit
>> and the TB bit locations change between vendors. The whole logic of
>> buildling the content of the status register based on some input
>> conditions is used two times and soon will be used 4 times.
>> 
>> Create dedicated helpers for these steps.
>> 
>> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
>> ---
>>  drivers/mtd/spi-nor/swp.c | 83 +++++++++++++++++++++++++++++------------------
>>  1 file changed, 51 insertions(+), 32 deletions(-)
>> 
>> diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
>> index 540cd221c455..8aa0fe297188 100644
>> --- a/drivers/mtd/spi-nor/swp.c
>> +++ b/drivers/mtd/spi-nor/swp.c
>> @@ -125,6 +125,43 @@ static bool spi_nor_is_unlocked_sr(struct spi_nor *nor, loff_t ofs, u64 len,
>>  	return spi_nor_check_lock_status_sr(nor, ofs, len, sr, false);
>>  }
>>  
>> +static int spi_nor_sr_set_bp_mask(struct spi_nor *nor, u8 *sr, u8 pow)
>> +{
>> +	u8 mask = spi_nor_get_sr_bp_mask(nor);
>> +	u8 val = pow << SR_BP_SHIFT;
>> +
>> +	if (nor->flags & SNOR_F_HAS_SR_BP3_BIT6 && val & SR_BP3)
>> +		val = (val & ~SR_BP3) | SR_BP3_BIT6;
>> +
>> +	if (val & ~mask)
>> +		return -EINVAL;
>> +
>> +	sr[0] = val;
>
> As sashiko already noticed, I think too this should have been sr[0] |= val

Yes indeed, that is a very good catch. Doesn't bug with my chips which
carry the QE bit in the second register, but it's unintended. Fixed on
my side, I will continue with Sashiko's feedback and send a v6.

Thanks,
Miquèl

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