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

[Miquel Raynal <miquel.raynal@bootlin.com>]

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