Re: [PATCH v2] mtd: spinand: Do not update the QE bit on devices without one

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Uninitialized return value in `spinand_configure_chip()` can cause device probe failure.
--

commit 25afeaf20864d4f79fe9a314c125190119930965
Author: Sagnik Sasmal <sagnik@sagnik.me>

mtd: spinand: Do not update the QE bit on devices without one

This patch moves the call to spinand_init_quad_enable() inside the
SPINAND_HAS_QE_BIT condition to avoid clearing configuration register bit 0
on devices without a QE bit.

> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> index 43bbe2629b85b..e59581d3596b7 100644
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
> @@ -1813,11 +1813,11 @@ static int spinand_configure_chip(struct spinand_device *spinand)
>  		    spinand->ssdr_op_templates.write_cache->data.buswidth == 4 ||
>  		    spinand->ssdr_op_templates.update_cache->data.buswidth == 4)
>  			quad_enable = true;
> -	}
>  
> -	ret = spinand_init_quad_enable(spinand, quad_enable);
> -	if (ret)
> -		return ret;
> +		ret = spinand_init_quad_enable(spinand, quad_enable);
> +		if (ret)
> +			return ret;
> +	}
>  
>  	if (spinand->configure_chip) {
>  		ret = spinand->configure_chip(spinand, SSDR);

[Severity: High]
Does this code leave 'ret' uninitialized if the device lacks both the
SPINAND_HAS_QE_BIT flag and a configure_chip callback?

Looking at the rest of spinand_configure_chip() in drivers/mtd/nand/spi/core.c:

static int spinand_configure_chip(struct spinand_device *spinand)
{
	bool odtr = false, quad_enable = false;
	int ret;

	...

try_ssdr:
	if (spinand->flags & SPINAND_HAS_QE_BIT) {
		...
		ret = spinand_init_quad_enable(spinand, quad_enable);
		if (ret)
			return ret;
	}

	if (spinand->configure_chip) {
		ret = spinand->configure_chip(spinand, SSDR);
		if (ret)
			return ret;
	}

	return ret;
}

If neither condition is met, it appears the function will return an
uninitialized stack variable. If this stack memory holds a non-zero value,
the caller spinand_init_flash() may treat it as a failure and abort device
initialization.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/010701a08c36e962-eb595dbe-b80b-44eb-858d-c1033f8a86a1-000000@eu-central-1.amazonses.com?part=1