diff mbox series

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

Message ID 20220228134505.203270-5-tudor.ambarus@microchip.com (mailing list archive)
State New, archived
Headers show
Series mtd: spi-nor: Handle ID collisions | expand

Commit Message

Tudor Ambarus Feb. 28, 2022, 1:45 p.m. UTC 
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(-)

Comments

Heiko Thiery March 1, 2022, 7:55 a.m. UTC | #1 
Hi Tudor,

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
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.

For the flash that does support SFDP parsing the
spi_nor_manufacturer_init_params() is not called. Is that expected to
be?

--
Heiko
Tudor Ambarus March 1, 2022, 8:52 a.m. UTC | #2 
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?

> 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:

Cheers,
ta
Heiko Thiery March 1, 2022, 9:31 a.m. UTC | #3 
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.
diff mbox series

Patch

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 |