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

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

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