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

[Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>]

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);
>
>