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] [day] [month] [year] [list]

Message-ID: <87cy98g6ts.fsf@bootlin.com>
Date: Wed, 06 Aug 2025 11:03:43 +0200
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: Mikhail Kshevetskiy <mikhail.kshevetskiy@...sys.eu>
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

Hello,

>>> @@ -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:

Yes, and that is misleading. It feels like the code is unclear this
way. Could we find a better organization?

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

Thanks,
Miquèl