[PATCH v6 19/21] mtd: spinand: propagate spinand_wait() errors from spinand_write_page()

[Mikhail Kshevetskiy]

On 19.08.2025 13:42, Frieder Schrempf wrote:
> Am 19.08.25 um 12:34 schrieb Mikhail Kshevetskiy:
>> On 19.08.2025 10:26, Frieder Schrempf wrote:
>>> Am 18.08.25 um 12:06 schrieb Mikhail Kshevetskiy:
>>>> From: Gabor Juhos <j4g8y7 at gmail.com>
>>>>
>>>> Since commit 3d1f08b032dc ("mtd: spinand: Use the external ECC engine
>>>> logic") the spinand_write_page() function ignores the errors returned
>>>> by spinand_wait(). Change the code to propagate those up to the stack
>>>> as it was done before the offending change.
>>>>
>>>> This is a port of linux commit
>>>> 091d9e35b85b ("mtd: spinand: propagate spinand_wait() errors from spinand_write_page()")
>>>>
>>>> Signed-off-by: Gabor Juhos <j4g8y7 at gmail.com>
>>>> Signed-off-by: Miquel Raynal <miquel.raynal at bootlin.com>
>>>> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy at iopsys.eu> # U-Boot port
>>> This seems to be a fix for a change that was introduced in patch 10 of
>>> this series. We should avoid introducing regressions and fixing them in
>>> a later patch to preserve bisectability.
>> Hm, this a bit contradict with removing fixes for continuous mode. In
>> linux, continuous mode makes some flashes (mostly macronix) almost
>> unusable on some platforms.
> Sorry, I don't understand how this is related to the continuous mode
> fixes. I'm just asking to squash patch 19 of this series into patch 10
> so the changes of patch 19 are included in patch 10 in order to make
> this a single consistent change.
you write: We should avoid introducing regressions and fixing them in a
later patch to preserve bisectability.
Continuous mode introduce regressions (see:
https://lkml.org/lkml/2025/8/14/349). My patches fixes this regressions,
but it still not accepted to upstream linux.
>
>> There are 2 possible bug scenarios
>> 1) "continuous mode" flash and spi controller without dirmap support,
>> but with restriction on transfer length in adjust_op_size()
>> 2) "continuous mode" flash and spi controller with dirmap support for a
>> single flash page
>>
>> In the first case all reading above restriction in  adjust_op_size()
>> will lead to EIO error
>> In the second case all reading above flash page size will lead to EIO
>> error or spinand driver fooling (because spi driver returns wrong number
>> of bytes was read).
>>
>> The second case looks as less critical, because u-boot up to this patch
>> series have no spinand dirmap support, so no one need to support dirmap
>> in the corresponding spi controllers.
>>
>>> Can you please fold this change into patch 10?
>>>
>>>> ---
>>>>  drivers/mtd/nand/spi/core.c | 5 ++++-
>>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
>>>> index daf6efb87d8..3e21a06dd0f 100644
>>>> --- a/drivers/mtd/nand/spi/core.c
>>>> +++ b/drivers/mtd/nand/spi/core.c
>>>> @@ -691,7 +691,10 @@ int spinand_write_page(struct spinand_device *spinand,
>>>>  			   SPINAND_WRITE_INITIAL_DELAY_US,
>>>>  			   SPINAND_WRITE_POLL_DELAY_US,
>>>>  			   &status);
>>>> -	if (!ret && (status & STATUS_PROG_FAILED))
>>>> +	if (ret)
>>>> +		return ret;
>>>> +
>>>> +	if (status & STATUS_PROG_FAILED)
>>>>  		return -EIO;
>>>>  
>>>>  	return spinand_ondie_ecc_finish_io_req(nand, (struct nand_page_io_req *)req);