RE: [PATCH v10 2/2] mtd: spi-nor: Set default Quad Enable method for ISSI flashes

[Sagar Kadam <sagar.kadam@xxxxxxxxxx>]

Hello Tudor,

> -----Original Message-----
> From: Tudor.Ambarus@xxxxxxxxxxxxx <Tudor.Ambarus@xxxxxxxxxxxxx>
> Sent: Tuesday, October 22, 2019 10:52 PM
> To: Sagar Kadam <sagar.kadam@xxxxxxxxxx>; vigneshr@xxxxxx
> Cc: linux-mtd@xxxxxxxxxxxxxxxxxxx; Tudor.Ambarus@xxxxxxxxxxxxx
> Subject: [PATCH v10 2/2] mtd: spi-nor: Set default Quad Enable method for
> ISSI flashes
> 
> From: Sagar Shrikant Kadam <sagar.kadam@xxxxxxxxxx>
> 
> Set the default Quad Enable method for ISSI flashes. Used for ISSI flashes
> (IS25WP256D-JMLE) that do not support SFDP tables and can not determine
> the Quad Enable method by parsing BFPT.
> 
> Based on code originally written by Wesley Terpstra <wesley@xxxxxxxxxx>
> and/or Palmer Dabbelt <palmer@xxxxxxxxxx> https://github.com/riscv/riscv-
> linux/commit/c94e267766d62bc9a669611c3d0c8ed5ea26569b
> 
> Signed-off-by: Sagar Shrikant Kadam <sagar.kadam@xxxxxxxxxx>
> [tudor.ambarus@xxxxxxxxxxxxx:
> - rebase, split and adapt for v5.4-rc4,

Thanks for considering this patch and  rebasing it on v5.4-rc4.

> - use PMC CFI ID for ISSI. According to JEP106BA, "Programmable Micro
> Corp"
>   changed its name to Integrated Silicon Solution (ISSI)]
> Signed-off-by: Tudor Ambarus <tudor.ambarus@xxxxxxxxxxxxx>
> ---
> Sagar, this is needed just for the ISSI flashes that can't retrieve the Quad
> Enable method from BFPT. It deserves a separate patch. Let me know if you
> are ok with how I reorganized the patches.
>
Yes, your changes for using the PMC CFI ID looks good to me, as it also indicates
that ISSI was  earlier Programmable Micro Corp. (Thanks for the pointer to 
JEP106BA where details on name changes are provided).

Due to some reason, I am unable to see the other patch in my inbox viz
[v10,1/2] mtd: spi-nor: Add support for is25wp256.
(maybe something might be wrong in my end, so including reply to it in this mail
I hope that's okay). Your changes to include the fixup hook also looks good to me.

Thanks & BR,
Sagar Kadam

>  drivers/mtd/spi-nor/spi-nor.c | 9 +++++++++
>  include/linux/mtd/spi-nor.h   | 1 +
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 3fc1eb20dca5..122e5bedf4f2 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -4385,6 +4385,11 @@ static int spi_nor_setup(struct spi_nor *nor,
>  	return nor->params.setup(nor, hwcaps);  }
> 
> +static void issi_set_default_init(struct spi_nor *nor) {
> +	nor->params.quad_enable = macronix_quad_enable; }
> +
>  static void macronix_set_default_init(struct spi_nor *nor)  {
>  	nor->params.quad_enable = macronix_quad_enable; @@ -4412,6
> +4417,10 @@ static void spi_nor_manufacturer_init_params(struct spi_nor
> *nor)  {
>  	/* Init flash parameters based on MFR */
>  	switch (JEDEC_MFR(nor->info)) {
> +	case SNOR_MFR_ISSI:
> +		issi_set_default_init(nor);
> +		break;
> +
>  	case SNOR_MFR_MACRONIX:
>  		macronix_set_default_init(nor);
>  		break;
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index
> fc0b4b19c900..b4636ff707ad 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -22,6 +22,7 @@
>  #define SNOR_MFR_INTEL		CFI_MFR_INTEL
>  #define SNOR_MFR_ST		CFI_MFR_ST	/* ST Micro */
>  #define SNOR_MFR_MICRON		CFI_MFR_MICRON	/* Micron */
> +#define SNOR_MFR_ISSI		CFI_MFR_PMC
>  #define SNOR_MFR_MACRONIX	CFI_MFR_MACRONIX
>  #define SNOR_MFR_SPANSION	CFI_MFR_AMD
>  #define SNOR_MFR_SST		CFI_MFR_SST
> --
> 2.9.5


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