Re: [PATCH v2 2/2] mtd: spi-nor: issi: Add support for is25wx01g

["Nuno Sá" <nuno.sa@analog.com>]

On Mon, Sep 14, 2026 at 04:04:40PM +0200, Michael Walle wrote:
> Hi,
> 
> On Mon Sep 14, 2026 at 3:42 PM CEST, Nuno Sá wrote:
> > (*): I should note that the command actually failed with -EIO but it
> > actually unlocked the chip! And the reason is because the flash as the same
> > FSR register than the micron-st flash. So WEL is set to 1 but can only
> > be cleared when clearing the FSR register.
> 
> Why doesn't this affect only the locking operation? WEL polling is
> used also during write and erase.

Not sure if I fully understand. But AFAICT, the reason why erase and
write is silent is because the default spi_nor_sr_ready() only looks at
SR_WIP [1]:

OTOH, on the unlock path we do spi_nor_write_sr1_and_sr2_and_check() and
give no special handling to WEL so I imagine that we try to set it as 0
but read it as 1 (given that it clears only with FSR) and hence I got
the -EIO in [2].

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/tree/drivers/mtd/spi-nor/core.c?h=spi-nor/next#n558
[2]: https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/tree/drivers/mtd/spi-nor/core.c?h=spi-nor/next#n970

> 
> > AFAICT, we should do something similar as micron so the writing to an
> > actual protected region fails rather than being silently discarded with
> > that status bit set. The question would be how to do it? The code is
> > pretty much identical to [1]. The masks, the opcoded... So should we
> > somehow handle this in the core (by having some common helper) that
> > could be set in .late_init() under a common MFR_FSR flag? Or just keep
> > both implementations separate for now?
> 
> I'd like to keep that out of the core.c, but also like to avoid any
> code duplication esp. because there is already handling for the
> intel spi controller in there. So maybe move it it into a new
> common.c.

Also don't like the dup tbh. Could that be a follow up or should it be
v3. From the top of my head I could think on a mfr_common.c kind of thing.
Don't thing this FSR register is standard?

Thx!
- Nuno Sá

> 
> Apart from that, this patch set looks good to go.
> 
> -michael
> 
> > [1]: https://elixir.bootlin.com/linux/v7.2.5/source/drivers/mtd/spi-nor/micron-st.c#L585



> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/