Re: [PATCH v4 4/6] mtd: spi-nor: macronix: Handle ID collision b/w MX25L12805D and MX25L12835F

[Heiko Thiery <heiko.thiery@gmail.com>]

Hi Tudor,

Am Di., 1. März 2022 um 09:52 Uhr schrieb <Tudor.Ambarus@microchip.com>:
>
> On 3/1/22 09:55, Heiko Thiery wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > Hi Tudor,
>
> Hi!
>
> >
> > Am Mo., 28. Feb. 2022 um 14:45 Uhr schrieb Tudor Ambarus
> > <tudor.ambarus@microchip.com>:
> >>
> >> Macronix has a bad habbit of reusing flash IDs. While MX25L12835F supports
> >> RDSFDP opcode, MX25L12805D does not. Given that it is unlikely that RDSFDP
> >> will cause any problems for the old MX25L12805D, differentiate between the
> >> two flashes by parsing SFDP.
> >>
> >> cc: Heiko Thiery <heiko.thiery@gmail.com>
> >> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> >> Acked-by: Pratyush Yadav <p.yadav@ti.com>
> >> ---
> >> # cat /sys/devices/platform/soc@0/30800000.bus/30bb0000.spi/spi_master/spi0/spi0
> >> .0/spi-nor/sfdp | xxd -p
> >> 53464450000101ff00000109300000ffc2000104600000ffffffffffffff
> >> ffffffffffffffffffffffffffffffffffffe520f1ffffffff0744eb086b
> >> 083b04bbfeffffffffff00ffffff44eb0c200f5210d800ffffffffffffff
> >> ffffffffffff003600279df9c06485cbffffffffffff
> >>
> >>  drivers/mtd/spi-nor/macronix.c | 23 ++++++++++++++++++++++-
> >>  1 file changed, 22 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> >> index 2754bbef3d2e..45c2f2c50e56 100644
> >> --- a/drivers/mtd/spi-nor/macronix.c
> >> +++ b/drivers/mtd/spi-nor/macronix.c
> >> @@ -26,6 +26,24 @@ static const struct spi_nor_fixups mx25l3205d_fixups = {
> >>         .post_bfpt = mx25l3205d_post_bfpt_fixups,
> >>  };
> >>
> >> +static int mx25l12805d_post_bfpt_fixups(struct spi_nor *nor,
> >> +                               const struct sfdp_parameter_header *bfpt_header,
> >> +                               const struct sfdp_bfpt *bfpt)
> >> +{
> >> +       /*
> >> +        * Macronix has a bad habit of reusing flash IDs: MX25L12835F collides
> >> +        * with MX25L12805D. MX25L12835F defines SFDP tables, while the older
> >> +        * variant does not.
> >> +        */
> >> +       nor->name = "mx25l12835f";
> >> +
> >> +       return 0;
> >> +}
> >> +
> >> +static const struct spi_nor_fixups mx25l12805d_fixups = {
> >> +       .post_bfpt = mx25l12805d_post_bfpt_fixups,
> >> +};
> >> +
> >>  static int
> >>  mx25l25635_post_bfpt_fixups(struct spi_nor *nor,
> >>                             const struct sfdp_parameter_header *bfpt_header,
> >> @@ -82,8 +100,11 @@ static const struct flash_info macronix_nor_parts[] = {
> >>         { "mx25u6435f",  INFO(0xc22537, 0, 64 * 1024, 128)
> >>                 NO_SFDP_FLAGS(SECT_4K) },
> >>         { "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256)
> >> +               /* ID collision with mx25l12835f. */
> >> +               PARSE_SFDP
> >>                 FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_4BIT_BP)
> >> -               NO_SFDP_FLAGS(SECT_4K) },
> >> +               NO_SFDP_FLAGS(SECT_4K)
> >> +               .fixups = &mx25l12805d_fixups },
> >>         { "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256) },
> >>         { "mx25r1635f",  INFO(0xc22815, 0, 64 * 1024,  32)
> >>                 NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ |
> >> --
> >> 2.25.1
> >>
> >
> > I tried this patch and saw that the flash is no longer detected. I did
>
> Thanks! No longer detected? ReadID fails? What do you get on the console?

[    1.442225] spi-nor spi0.0: CR: read back test failed
[    1.445979] spi-nor spi0.0: quad mode not supported
[    1.445989] spi-nor: probe of spi0.0 failed with error -5

> > some debugging and see now that the correct function to set the quad
> > mode (spi_nor_sr1_bit6_quad_enable) is not called. Instead the
> > spi_nor_sr2_bit1_quad_enable() is invoked. Further debbuging showed me
> > that the macronix specific fixup is not called.
> >
>
> ok, this bit is clear
>
> > For the flash that does support SFDP parsing the
> > spi_nor_manufacturer_init_params() is not called. Is that expected to
> > be?
>
> Yes, I would like to get rid of the default_init() hooks for new code.
> I'm addressing it right now. Would be good if you can help testing it.
> Will add you in to:

Yes, of course. Meanwhile I figured out that the SFDP of this flash
does not contain the required table (DWORD[15]) to figure out what
function should be used for quad_enable.

-- 
Heiko

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