Re: [PATCH v2 03/35] mtd: spi-nor: macronix: Handle ID collision b/w MX25L3233F and MX25L3205D

[Michael Walle <michael@walle.cc>]

Am 2021-07-27 06:51, schrieb Tudor Ambarus:
> Macronix has a bad habbit of reusing flash IDs. While MX25L3233F 
> supports
> RDSFDP opcode, MX25L3205D does not support it and does not recommend
> issuing opcodes that are not supported ("It is not recommended to adopt
> any other code not in the command definition table, which will 
> potentially
> enter the hidden mode.").
> 
> We tested the RDSFDP on the MX25L3205D and the conclusion is that the
> flash didn't reply anything. Given that it is unlikely that RDSFDP will
> cause any problems for the old MX25L3205D, differentiate between the 
> two
> flashes by parsing SFDP.
> 
> Tested MX25L3233F. Generated a 256 Kbyte random data and did an erase,
> write, read back and compare test. The flash uses for reads
> SPINOR_OP_READ_1_4_4 0xeb, for erases SPINOR_OP_BE_4K 0x20, and for 
> writes
> SPINOR_OP_PP 0x02.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> Acked-by: Pratyush Yadav <p.yadav@ti.com>
> ---
> root@sama5d2-xplained:~# find / -iname spi-nor
> /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor
> /sys/devices/platform/ahb/ahb:apb/f8000000.spi/spi_master/spi0/spi0.0/spi-nor
> /sys/bus/spi/drivers/spi-nor
> root@sama5d2-xplained:~# ls -al
> /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor
> total 0
> drwxr-xr-x 2 root root    0 Mar  9 14:51 .
> drwxr-xr-x 6 root root    0 Mar  9 14:50 ..
> -r--r--r-- 1 root root 4096 Mar  9 14:51 jedec_id
> -r--r--r-- 1 root root 4096 Mar  9 14:51 manufacturer
> -r--r--r-- 1 root root 4096 Mar  9 14:51 partname
> -r--r--r-- 1 root root    0 Mar  9 14:51 sfdp
> root@sama5d2-xplained:~# cat
> /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/jedec_id
> c22016
> root@sama5d2-xplained:~# cat
> /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/manufacturer
> macronix
> root@sama5d2-xplained:~# cat
> /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/partname
> mx25l3233f
> root@sama5d2-xplained:~# cat
> /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/sfdp
> > mx25l3233f-sfdp
> root@sama5d2-xplained:~# hexdump mx25l3233f-sfdp

use xxd if possible and the sha1sum/md5sum is missing.

> 0000000 4653 5044 0100 ff01 0000 0901 0030 ff00
> 0000010 00c2 0401 0060 ff00 ffff ffff ffff ffff
> 0000020 ffff ffff ffff ffff ffff ffff ffff ffff
> 0000030 20e5 fff1 ffff 01ff eb44 6b08 3b08 bb04
> 0000040 ffee ffff ffff ff00 ffff ff00 200c 520f
> 0000050 d810 ff00 ffff ffff ffff ffff ffff ffff
> 0000060 3600 2650 f99c 6477 cffe ffff ffff ffff
> 0000070
> 
>  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 27498ed0cc0d..68f6ac060bc6 100644
> --- a/drivers/mtd/spi-nor/macronix.c
> +++ b/drivers/mtd/spi-nor/macronix.c
> @@ -8,6 +8,24 @@
> 
>  #include "core.h"
> 
> +static int mx25l3233f_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: MX25L3233F collides
> +	 * with MX25L3205D. MX25L3233F defines SFDP tables, while the older
> +	 * variant does not.
> +	 */
> +	nor->name = "mx25l3233f";
> +
> +	return 0;
> +}
> +
> +static struct spi_nor_fixups mx25l3233f_fixups = {
> +	.post_bfpt = mx25l3233f_post_bfpt_fixups,
> +};
> +
>  static int
>  mx25l25635_post_bfpt_fixups(struct spi_nor *nor,
>  			    const struct sfdp_parameter_header *bfpt_header,
> @@ -39,7 +57,10 @@ static const struct flash_info macronix_parts[] = {
>  	{ "mx25l4005a",  INFO(0xc22013, 0, 64 * 1024,   8, SECT_4K) },
>  	{ "mx25l8005",   INFO(0xc22014, 0, 64 * 1024,  16, 0) },
>  	{ "mx25l1606e",  INFO(0xc22015, 0, 64 * 1024,  32, SECT_4K) },
> -	{ "mx25l3205d",  INFO(0xc22016, 0, 64 * 1024,  64, SECT_4K) },
> +	{ "mx25l3205d",  INFO(0xc22016, 0, 64 * 1024,  64, SPI_NOR_PARSE_SFDP 
> |
> +			      SECT_4K)
> +		/* ID collision with mx25l3233f. */
> +		.fixups = &mx25l3233f_fixups },

Shouldn't we use mx25l3205d_fixups as name here? What if there are more
flashes with the same id. Using the name of the colliding flash here
doesn't really scale.

-michael

>  	{ "mx25l3255e",  INFO(0xc29e16, 0, 64 * 1024,  64, SECT_4K) },
>  	{ "mx25l6405d",  INFO(0xc22017, 0, 64 * 1024, 128, SECT_4K) },
>  	{ "mx25u2033e",  INFO(0xc22532, 0, 64 * 1024,   4, SECT_4K) },

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