Re: [PATCH v3 2/5] mtd: spi-nor: core: Hook manufacture by checking first byte ID

[Michael Walle <mwalle@kernel.org>]

Am 2023-08-04 11:54, schrieb Jaime Liao:
> From: JaimeLiao <jaimeliao@mxic.com.tw>
> 
> Patch ID for flash support is a thing that we keep striving to do.
> Follow the optimization of software architecture, parse SFDP is
> the mainstream in SPI-NOR subsystem.
> Label "spi-nor-generic" to the flash which didn't include in ID table
> but support SFDP, is a good way for flash support.
> Building upon this fundation, achieve hooking the manufacturer using 
> the
> 1st ID byte.

This won't work beacuse the manufacturer id is not always
one byte long, think of continuation codes. In fact, as the
flash_info table is of now, we cannot even rely on the
continuation codes, but we have to always check for the
complete id_len, i.e. there is at least one hack where
the id is reversed and the manufacturer is the last byte,
iirc. some oddball cypress mram chip.

If you want to get the correct manufacturer for spi-nor-generic,
you should extract it from the SFDP tables. It seems that the
BFPT don't include a manufacturer id, but if there are proprietary
tables, you *might* use that id. I say might, because it only works
with one byte manufacturer ids, no continuation codes... *sigh*

-michael


> Consequently, extend support even for parts not descibed in SFDP.
> 
> Signed-off-by: JaimeLiao <jaimeliao@mxic.com.tw>
> ---
>  drivers/mtd/spi-nor/core.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 5f29fac8669a..eb0a09c06bf4 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2036,10 +2036,13 @@ static const struct flash_info 
> *spi_nor_match_id(struct spi_nor *nor,
>  	for (i = 0; i < ARRAY_SIZE(manufacturers); i++) {
>  		for (j = 0; j < manufacturers[i]->nparts; j++) {
>  			part = &manufacturers[i]->parts[j];
> -			if (part->id_len &&
> -			    !memcmp(part->id, id, part->id_len)) {
> -				nor->manufacturer = manufacturers[i];
> -				return part;
> +			if (part->id_len) {
> +				/* Hook manufacturers when first byte (maf ID) is same as other 
> IDs on ID table */
> +				if (!nor->manufacturer && !memcmp(part->id, id, 1))
> +					nor->manufacturer = manufacturers[i];
> +
> +				if (!memcmp(part->id, id, part->id_len))
> +					return part;
>  			}
>  		}
>  	}

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