Re: [PATCH v2 05/35] mtd: spi-nor: Introduce Manufacturer ID collisions driver

[Michael Walle <michael@walle.cc>]

Am 2021-07-27 06:51, schrieb Tudor Ambarus:
> Some manufacturers completely ignore the manufacturer's identification 
> code
> standard (JEP106) and do not define the manufacturer ID continuation
> scheme. This will result in manufacturer ID collisions.
> 
> An an example, JEP106BA requires Boya that it's manufacturer ID to be
> preceded by 8 continuation codes. Boya's identification code must be:
> 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x68. But Boya ignores 
> the
> continuation scheme and its ID collides with the manufacturer defined 
> in
> bank one: Convex Computer.
> 
> Introduce the manuf-id-collisions driver in order to address ID 
> collisions
> between manufacturers. flash_info entries will be added in a first 
> come,
> first served manner. Differentiation between flashes will be done at
> runtime if possible. Where runtime differentiation is not possible, new
> compatibles will be introduced, but this will be done as a last resort.
> Every new flash addition that define the SFDP tables, should dump its 
> SFDP
> tables in the patch's comment section below the --- line, so that we 
> can
> reference it in case of collisions.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
>  drivers/mtd/spi-nor/Makefile              |  1 +
>  drivers/mtd/spi-nor/core.c                |  1 +
>  drivers/mtd/spi-nor/core.h                |  1 +
>  drivers/mtd/spi-nor/manuf-id-collisions.c | 22 ++++++++++++++++++++++
>  4 files changed, 25 insertions(+)
>  create mode 100644 drivers/mtd/spi-nor/manuf-id-collisions.c
> 
> diff --git a/drivers/mtd/spi-nor/Makefile 
> b/drivers/mtd/spi-nor/Makefile
> index 6b904e439372..48763d10daad 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -1,6 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
> 
>  spi-nor-objs			:= core.o sfdp.o swp.o otp.o sysfs.o
> +spi-nor-objs			+= manuf-id-collisions.o
>  spi-nor-objs			+= atmel.o
>  spi-nor-objs			+= catalyst.o
>  spi-nor-objs			+= eon.o
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 1a278d33b02d..d30c8f350dc1 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -1829,6 +1829,7 @@ int spi_nor_sr2_bit7_quad_enable(struct spi_nor 
> *nor)
>  }
> 
>  static const struct spi_nor_manufacturer *manufacturers[] = {
> +	&spi_nor_manuf_id_collisions,

Intentionally at the beginning of all the flashes? So these
might take precedence over "normal" ones? Shouldn't it be
the other way around?

>  	&spi_nor_atmel,
>  	&spi_nor_catalyst,
>  	&spi_nor_eon,
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 55fceb0ec894..e9896cd60695 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -476,6 +476,7 @@ struct sfdp {
>  };
> 
>  /* Manufacturer drivers. */
> +extern const struct spi_nor_manufacturer spi_nor_manuf_id_collisions;
>  extern const struct spi_nor_manufacturer spi_nor_atmel;
>  extern const struct spi_nor_manufacturer spi_nor_catalyst;
>  extern const struct spi_nor_manufacturer spi_nor_eon;
> diff --git a/drivers/mtd/spi-nor/manuf-id-collisions.c
> b/drivers/mtd/spi-nor/manuf-id-collisions.c
> new file mode 100644
> index 000000000000..bf7dba34f018
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/manuf-id-collisions.c
> @@ -0,0 +1,22 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Used to handle collisions between manufacturers, where 
> manufacturers are
> + * ignorant enough to not implement the ID continuation scheme 
> described in the
> + * JEP106 JEDEC standard.
> + */
> +
> +#include <linux/mtd/spi-nor.h>
> +#include "core.h"
> +
> +static const struct flash_info id_collision_parts[] = {
> +	/* Boya */
> +	{ "by25q128as", INFO(0x684018, 0, 64 * 1024, 256, SPI_NOR_SKIP_SFDP |
> +			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
> +			     SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) },
> +};
> +
> +const struct spi_nor_manufacturer spi_nor_manuf_id_collisions = {
> +	.name = "manufacturer ID collisions",

mhh, so we loose the manufacturer name. Doh. Can we do better?

-michael

> +	.parts = id_collision_parts,
> +	.nparts = ARRAY_SIZE(id_collision_parts),
> +};

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel