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

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

Hello,

On 10/09/2026 at 11:33:23 GMT, Sagnik Sasmal <sagnik@sagnik.me> wrote:

> Commit be0b86c648bf ("mtd: spinand: Gather all the bus interface
> steps in one single function") moved quad-enable setup into
> spinand_configure_chip(). The new code only determines whether quad
> mode is needed when SPINAND_HAS_QE_BIT is set, but calls
> spinand_init_quad_enable() unconditionally. This clears configuration
> register bit 0 on devices without a QE bit.
>
> That bit is not universally a QE bit. On the Winbond W25N02KV it is
> H-DIS, which disables the active-low HOLD function. Clearing H-DIS
> enables HOLD during single and dual I/O operations. If IO3 is not kept
> high, the flash can pause a command and ignore clock and data.
>
> H-DIS is not restored by the FFh reset command, allowing the incorrect
> state to survive an SoC warm reboot while the flash remains powered.
>
> Before the refactoring, spinand_init_quad_enable() returned without
> touching the configuration register when SPINAND_HAS_QE_BIT was not
> set. Restore that behavior.
>
> The regression was reproduced on a Jio JIDU6401 with an MT7986 SoC and
> a W25N02KV. With Linux 6.18.44, sysupgrade failed and the following warm
> reboot hung in BL2. With this change applied, both sysupgrade and warm
> reboot completed successfully.
>
> Fixes: be0b86c648bf ("mtd: spinand: Gather all the bus interface steps in one single function")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Sagnik Sasmal <sagnik@sagnik.me>

Thanks for the report, and the debug and the proposal. Sorry for the
breakage.

While your solution works, I believe it would be cleaner do it this way:

--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -1797,11 +1797,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);

Do you mind validating this and using something like this in your v2?
This would avoid to check the same flag in two different places. It also
keeps the spinand_init_quad_enable() minimal.

Thanks,
Miquèl