Re: [PATCH] mtd: spi-nor: gigadevice: add lock flags for GD25Q128/256 and GD25LQ128D

[Jakub "Kuba" Czapiga]

On Tue, Mar 18, 2025 at 8:09 AM Tudor Ambarus <tudor.ambarus@xxxxxxxxxx> wrote:
>
>
>
> On 17.03.2025 20:20, Jakub Czapiga wrote:
> > Set appropriate FLASH lock feature flags.
> > Set top-bottom protection configuration bit flags.
> >
> > Modified chips:
> > - GD25Q128 (+lock, +tb)
> > - GD25Q256 (+lock)
> > - GD25Q256D, GD25Q256E (+tb)
> > - GD25LQ128D (+lock, +tb)
> >
> > Signed-off-by: Jakub Czapiga <czapiga@xxxxxxxxxx>
> > ---
> >  drivers/mtd/spi-nor/gigadevice.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/mtd/spi-nor/gigadevice.c b/drivers/mtd/spi-nor/gigadevice.c
> > index ef1edd0add70..8eec6557b036 100644
> > --- a/drivers/mtd/spi-nor/gigadevice.c
> > +++ b/drivers/mtd/spi-nor/gigadevice.c
> > @@ -16,6 +16,7 @@ gd25q256_post_bfpt(struct spi_nor *nor,
> >       /*
> >        * GD25Q256C supports the first version of JESD216 which does not define
> >        * the Quad Enable methods. Overwrite the default Quad Enable method.
> > +      * Otherwise set TB to SR(6).
> >        *
> >        * GD25Q256 GENERATION | SFDP MAJOR VERSION | SFDP MINOR VERSION
> >        *      GD25Q256C      | SFDP_JESD216_MAJOR | SFDP_JESD216_MINOR
> > @@ -25,6 +26,8 @@ gd25q256_post_bfpt(struct spi_nor *nor,
> >       if (bfpt_header->major == SFDP_JESD216_MAJOR &&
> >           bfpt_header->minor == SFDP_JESD216_MINOR)
> >               nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
> > +     else
> > +             nor->flags |= SNOR_F_HAS_SR_TB | SNOR_F_HAS_SR_TB_BIT6;
>
> why do you tie locking by SFDP absence?

GD25Q256C has a different Status Registers layout compared to GD25Q256D/E:
GD25Q256C has QE (Quad Enable) bit on SR1(6)/S6. GD25Q256D/E has it on
SR2(1)/S9.
GD25Q256C has TB (Top-Bottom) on SR2(3)/S11. GD25Q256D/E has it on SR1(6)/S6.
The default TB value is zero, so "top", which is also the default
route in the swp.c if TB is not present. Moreover current API and
implementation do not allow to configure TB in place other than
SR1(5)/S5 or SR1(6)/S6. I figured that it's better to leave TB unset
for the older chip variant and allow the driver to only access BP/Lock
bits in this case.