[PATCH RESEND v5 13/13] spi: airoha: buffer must be 0xff-ed before writing

Mikhail Kshevetskiy posted 13 patches 1 day, 22 hours ago
There is a newer version of this series
[PATCH RESEND v5 13/13] spi: airoha: buffer must be 0xff-ed before writing
Posted by Mikhail Kshevetskiy 1 day, 22 hours ago
During writing, the entire flash page (including OOB) will be updated
with the values from the temporary buffer, so we need to fill the
untouched areas of the buffer with 0xff value to prevent accidental
data overwriting.

Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
---
 drivers/spi/spi-airoha-snfi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spi-airoha-snfi.c b/drivers/spi/spi-airoha-snfi.c
index 437ab6745b1a..57b1950e853f 100644
--- a/drivers/spi/spi-airoha-snfi.c
+++ b/drivers/spi/spi-airoha-snfi.c
@@ -776,6 +776,7 @@ static ssize_t airoha_snand_dirmap_write(struct spi_mem_dirmap_desc *desc,
 		return -EOPNOTSUPP;
 	}
 
+	memset(txrx_buf, 0xff, bytes);
 	memcpy(txrx_buf + offs, buf, len);
 
 	err = airoha_snand_set_mode(as_ctrl, SPI_MODE_DMA);
-- 
2.51.0
Re: [PATCH RESEND v5 13/13] spi: airoha: buffer must be 0xff-ed before writing
Posted by AngeloGioacchino Del Regno 13 hours ago
Il 30/09/25 04:26, Mikhail Kshevetskiy ha scritto:
> During writing, the entire flash page (including OOB) will be updated
> with the values from the temporary buffer, so we need to fill the
> untouched areas of the buffer with 0xff value to prevent accidental
> data overwriting.
> 
> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
> ---
>   drivers/spi/spi-airoha-snfi.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/spi/spi-airoha-snfi.c b/drivers/spi/spi-airoha-snfi.c
> index 437ab6745b1a..57b1950e853f 100644
> --- a/drivers/spi/spi-airoha-snfi.c
> +++ b/drivers/spi/spi-airoha-snfi.c
> @@ -776,6 +776,7 @@ static ssize_t airoha_snand_dirmap_write(struct spi_mem_dirmap_desc *desc,
>   		return -EOPNOTSUPP;
>   	}
>   
> +	memset(txrx_buf, 0xff, bytes);

As you are refactoring this a bit, reading the function isn't straightforward
without applying the series locally.

It looks like you're filling the entire txrx_buf with 0xff.

While that will work for sure, for the sake of performance you should change this
to memset(0xff) only the portions of buffer that the next memcpy call will not
overwrite, avoiding to effectively write twice to that buffer.

Is there any reason why you didn't do just that?

...also because, your commit message really looks like saying what I'm proposing
to do here.

Cheers,
Angelo

>   	memcpy(txrx_buf + offs, buf, len);
>   
>   	err = airoha_snand_set_mode(as_ctrl, SPI_MODE_DMA);
Re: [PATCH RESEND v5 13/13] spi: airoha: buffer must be 0xff-ed before writing
Posted by Mikhail Kshevetskiy 2 hours ago
On 01.10.2025 14:30, AngeloGioacchino Del Regno wrote:
> Il 30/09/25 04:26, Mikhail Kshevetskiy ha scritto:
>> During writing, the entire flash page (including OOB) will be updated
>> with the values from the temporary buffer, so we need to fill the
>> untouched areas of the buffer with 0xff value to prevent accidental
>> data overwriting.
>>
>> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
>> ---
>>   drivers/spi/spi-airoha-snfi.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/spi/spi-airoha-snfi.c
>> b/drivers/spi/spi-airoha-snfi.c
>> index 437ab6745b1a..57b1950e853f 100644
>> --- a/drivers/spi/spi-airoha-snfi.c
>> +++ b/drivers/spi/spi-airoha-snfi.c
>> @@ -776,6 +776,7 @@ static ssize_t airoha_snand_dirmap_write(struct
>> spi_mem_dirmap_desc *desc,
>>           return -EOPNOTSUPP;
>>       }
>>   +    memset(txrx_buf, 0xff, bytes);
>
> As you are refactoring this a bit, reading the function isn't
> straightforward
> without applying the series locally.
>
> It looks like you're filling the entire txrx_buf with 0xff.
>
> While that will work for sure, for the sake of performance you should
> change this
> to memset(0xff) only the portions of buffer that the next memcpy call
> will not
> overwrite, avoiding to effectively write twice to that buffer.
>
> Is there any reason why you didn't do just that?
>
> ...also because, your commit message really looks like saying what I'm
> proposing
> to do here.
>
In the worst case 2 memset() operations must be done:
1) memset() of buffer head with non-rounded size
2) memset() of buffer tail with unaligned beginning address and
non-rounded size

In summary it could be slower than a single memset() of buffer with
aligned boundaries.
But anyway the difference will be small.

Anyway, I fix this in next version.

Regards,
Mikhail

> Cheers,
> Angelo
>
>>       memcpy(txrx_buf + offs, buf, len);
>>         err = airoha_snand_set_mode(as_ctrl, SPI_MODE_DMA);
>
>