linux-kernel - Re: [PATCH 2/4] drivers: mtd: spi-nand: try a regular dirmap if creating a dirmap for continuous reading fails

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <74646064-0c8c-46b9-8ec3-653c4f80b70f@iopsys.eu>
Date: Tue, 5 Aug 2025 18:35:32 +0300
From: Mikhail Kshevetskiy <mikhail.kshevetskiy@...sys.eu>
To: Miquel Raynal <miquel.raynal@...tlin.com>
Cc: Richard Weinberger <richard@....at>, Vignesh Raghavendra
 <vigneshr@...com>, Lorenzo Bianconi <lorenzo@...nel.org>,
 Ray Liu <ray.liu@...oha.com>, Mark Brown <broonie@...nel.org>,
 Tudor Ambarus <tudor.ambarus@...aro.org>,
 Martin Kurbanov <mmkurbanov@...utedevices.com>,
 Takahiro Kuwano <Takahiro.Kuwano@...ineon.com>,
 Cheng Ming Lin <chengminglin@...c.com.tw>, linux-mtd@...ts.infradead.org,
 linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-spi@...r.kernel.org
Subject: Re: [PATCH 2/4] drivers: mtd: spi-nand: try a regular dirmap if
 creating a dirmap for continuous reading fails


On 05.08.2025 18:30, Miquel Raynal wrote:
> On 04/08/2025 at 22:21:30 +03, Mikhail Kshevetskiy <mikhail.kshevetskiy@...sys.eu> wrote:
>
>> Continuous reading may result in multiple flash pages reading in one
>> operation. Typically only one flash page has read/written (a little bit
>> more than 2-4 Kb), but continuous reading requires the spi-controller
>> to read up to 512 Kb in one operation without togling CS in beetween.
>>
>> Roughly speaking spi-controllers can be divided on 2 categories:
>>  * spi-controllers without dirmap acceleration support
>>  * spi-controllers with dirmap acceleration support
>>
>> Usually, first of them have no issues with large reading support.
>> Second group often supports acceleration of single page only reading.
>> Thus enabling of continuous reading can break flash reading.
> I would be more conservative, I believe it is very implementation
> dependent; many controller drivers do not even advertise a max size.
>
> I agree though that controllers with dirmap support may express
> limitations such as the mapped size which may lead to splitting
> operations into smaller chunks, possibly leading to CS changes which
> would break the continuous read mode on the chip's side.
>
>> This patch tries to create dirmap for continuous reading first and
>> fallback to regular reading if spi-controller refuses to create it.
> no '-'                              ^
>
>> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@...sys.eu>
>> ---
>>  drivers/mtd/nand/spi/core.c | 29 +++++++++++++++++++++++++----
>>  1 file changed, 25 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
>> index b42c42ec58a4..ff6a1e2fcfdc 100644
>> --- a/drivers/mtd/nand/spi/core.c
>> +++ b/drivers/mtd/nand/spi/core.c
>> @@ -1114,11 +1114,32 @@ static int spinand_create_dirmap(struct spinand_device *spinand,
>>  
>>  	spinand->dirmaps[plane].wdesc = desc;
>>  
>> -	if (spinand->cont_read_possible)
>> +	desc = NULL;
>> +	if (spinand->cont_read_possible) {
>> +		/*
>> +		 * spi-controllers may return an error if info.length is
>> +		 * too large
>> +		 */
>>  		info.length = nanddev_eraseblock_size(nand);
>> -	info.op_tmpl = *spinand->op_templates.read_cache;
>> -	desc = devm_spi_mem_dirmap_create(&spinand->spimem->spi->dev,
>> -					  spinand->spimem, &info);
>> +		info.op_tmpl = *spinand->op_templates.read_cache;
>> +		desc = devm_spi_mem_dirmap_create(&spinand->spimem->spi->dev,
>> +						  spinand->spimem, &info);
>> +	}
>> +
>> +	if (IS_ERR_OR_NULL(desc)) {
> Here if the problem is continuous reading, I expect an error and not a
> NULL pointer.
NULL is possible if flash does not supports continuous reading
>
>> +		/*
>> +		 * continuous reading is not supported by flash or
> Not by the flash, here if we get an error, it is the spi controller
> (still without '-' ;) ) that fails (please fix the comment).

we can go here in 2 cases:
1) spinand->cont_read_possible is false (flash does not supports
continuous reading)

2) spi controller returns an error (spi controller does not like
continuous reading)

>
>> +		 * its spi-controller, try regular reading
>> +		 */
>> +		spinand->cont_read_possible = false;
>> +
>> +		info.length = nanddev_page_size(nand) +
>> +			      nanddev_per_page_oobsize(nand);
>> +		info.op_tmpl = *spinand->op_templates.read_cache;
>> +		desc = devm_spi_mem_dirmap_create(&spinand->spimem->spi->dev,
>> +						  spinand->spimem, &info);
>> +	}
>> +
>>  	if (IS_ERR(desc))
>>  		return PTR_ERR(desc);