diff mbox series

[v1,6/7] nor: add post bfpt fix handler for is25wp256 device

Message ID 1579810566-11675-7-git-send-email-sagar.kadam@sifive.com
State New
Headers show
Series Fix currently available support for flash on HiFive Unleashed | expand

Commit Message

Sagar Shrikant Kadam Jan. 23, 2020, 8:16 p.m. UTC
Update vendor id for ISSI flash, enable SFDP as ISSI flash
supports it and add support for spi_nor_fixups similar to
that done in linux. Flash vendor specific fixups can be
registered in spi_nor_ids, and will be called after BFPT
parsing to fix any wrong parameter read from SFDP.

Signed-off-by: Sagar Shrikant Kadam <sagar.kadam at sifive.com>
---
 board/sifive/fu540/Kconfig     |  1 +
 drivers/mtd/spi/sf_internal.h  | 16 +++++++++++
 drivers/mtd/spi/spi-nor-core.c | 63 ++++++++++++++++++++++++++++++++++++++++--
 drivers/mtd/spi/spi-nor-ids.c  |  7 ++++-
 include/linux/mtd/spi-nor.h    |  1 +
 5 files changed, 85 insertions(+), 3 deletions(-)

Comments

Vignesh Raghavendra Jan. 24, 2020, 4:53 a.m. UTC | #1
Hi,

On 24/01/20 1:46 am, Sagar Shrikant Kadam wrote:
> Update vendor id for ISSI flash, enable SFDP as ISSI flash
> supports it and add support for spi_nor_fixups similar to
> that done in linux. Flash vendor specific fixups can be
> registered in spi_nor_ids, and will be called after BFPT
> parsing to fix any wrong parameter read from SFDP.
> 
> Signed-off-by: Sagar Shrikant Kadam <sagar.kadam at sifive.com>
> ---
>  board/sifive/fu540/Kconfig     |  1 +
>  drivers/mtd/spi/sf_internal.h  | 16 +++++++++++
>  drivers/mtd/spi/spi-nor-core.c | 63 ++++++++++++++++++++++++++++++++++++++++--
>  drivers/mtd/spi/spi-nor-ids.c  |  7 ++++-
>  include/linux/mtd/spi-nor.h    |  1 +
>  5 files changed, 85 insertions(+), 3 deletions(-)
> 
> diff --git a/board/sifive/fu540/Kconfig b/board/sifive/fu540/Kconfig
> index 75661f3..d9e4956 100644
> --- a/board/sifive/fu540/Kconfig
> +++ b/board/sifive/fu540/Kconfig
> @@ -42,6 +42,7 @@ config BOARD_SPECIFIC_OPTIONS # dummy
>  	imply SPI
>  	imply SPI_SIFIVE
>  	imply SPI_FLASH
> +	imply SPI_FLASH_SFDP_SUPPORT
>  	imply SPI_FLASH_ISSI
>  	imply MMC
>  	imply MMC_SPI

This does not belong to this patch. Its better that this change goes via
SiFive SoC tree.

> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
> index 5c64303..856866f 100644
> --- a/drivers/mtd/spi/sf_internal.h
> +++ b/drivers/mtd/spi/sf_internal.h
> @@ -66,8 +66,24 @@ struct flash_info {
>  #define SPI_NOR_SKIP_SFDP	BIT(13)	/* Skip parsing of SFDP tables */
>  #define USE_CLSR		BIT(14)	/* use CLSR command */
>  #define SPI_NOR_HAS_SST26LOCK	BIT(15)	/* Flash supports lock/unlock via BPR */
> +
> +#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT

Change above ifdef to:

	#if CONFIG_IS_ENABLED(SPI_FLASH_SFDP_SUPPORT)

throughout the patch to take care of both SPL and U-Boot builds

> +	/* Part specific fixup hooks */
> +	const struct spi_nor_fixups	*fixups;
> +#endif
>  };
>  
> +#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
> +/*
> + * Declare manufacturer specific fixup handlers that
> + * can be registered as fixup's in flash info table
> + * so as to update any wrong/broken SFDP parameter.
> + */
> +#ifdef CONFIG_SPI_FLASH_ISSI
> +extern struct spi_nor_fixups is25wp256_fixups;
> +#endif
> +#endif
> +
>  extern const struct flash_info spi_nor_ids[];
>  
>  #define JEDEC_MFR(info)	((info)->id[0])
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index 6e7fc23..c55116f 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -296,7 +296,6 @@ static void spi_nor_set_4byte_opcodes(struct spi_nor *nor,
>  		nor->mtd.erasesize = info->sector_size;
>  		break;
>  
> -	default:

Is this an intentional change?

>  		break;
>  	}
>  
> @@ -1800,6 +1799,57 @@ static const struct sfdp_bfpt_erase sfdp_bfpt_erases[] = {
>  
>  static int spi_nor_hwcaps_read2cmd(u32 hwcaps);
>  
> +#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
> +/**
> + * struct spi_nor_fixups - SPI NOR fixup hooks
> + * @post_bfpt: called after the BFPT table has been parsed
> + *
> + * Those hooks can be used to tweak the SPI NOR configuration when the SFDP
> + * table is broken or not available.
> + */
> +struct spi_nor_fixups {
> +	int (*post_bfpt)(struct spi_nor *nor,
> +			 const struct sfdp_parameter_header *bfpt_header,
> +			 const struct sfdp_bfpt *bfpt,
> +			 struct spi_nor_flash_parameter *params);
> +};
> +
> +static int spi_nor_post_bfpt_fixups(struct spi_nor *nor,
> +				    const struct sfdp_parameter_header
> +						*bfpt_header,
> +				    const struct sfdp_bfpt *bfpt,
> +				    struct spi_nor_flash_parameter *params)
> +{
> +	if (nor->info->fixups && nor->info->fixups->post_bfpt)
> +		return nor->info->fixups->post_bfpt(nor, bfpt_header, bfpt,
> +				params);
> +
> +	return 0;
> +}
> +
> +static int is25wp256_post_bfpt_fixups(struct spi_nor *nor,
> +				      const struct sfdp_parameter_header
> +						*bfpt_header,
> +				      const struct sfdp_bfpt *bfpt,
> +				      struct spi_nor_flash_parameter *params)
> +
> +{
> +	/* IS25WP256 supports 4B opcodes, but the BFPT advertises a
> +	 * BFPT_DWORD1_ADDRESS_BYTES_3_ONLY address width.
> +	 * Overwrite the address width advertised by the BFPT.
> +	 */
> +	if ((bfpt->dwords[BFPT_DWORD(1)] & BFPT_DWORD1_ADDRESS_BYTES_MASK) ==
> +			BFPT_DWORD1_ADDRESS_BYTES_3_ONLY)
> +		nor->addr_width = 4;
> +
> +	return 0;
> +}
> +
> +struct spi_nor_fixups is25wp256_fixups = {
> +	.post_bfpt = is25wp256_post_bfpt_fixups,
> +};
> +#endif
> +
>  /**
>   * spi_nor_parse_bfpt() - read and parse the Basic Flash Parameter Table.
>   * @nor:		pointer to a 'struct spi_nor'
> @@ -1971,7 +2021,13 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>  		return -EINVAL;
>  	}
>  
> -	return 0;
> +#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
> +	err = spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt,
> +				       params);

Isn't this function (spi_nor_parse_bfpt()) already under
CONFIG_IS_ENABLED(CONFIG_SPI_FLASH_SFDP_SUPPORT)?

> +#else
> +	err = 0;
> +#endif
> +	return err;
>  }
>  
>  /**
> @@ -2209,6 +2265,9 @@ static int spi_nor_init_params(struct spi_nor *nor,
>  	    !(info->flags & SPI_NOR_SKIP_SFDP)) {
>  		struct spi_nor_flash_parameter sfdp_params;
>  
> +		/* Update flash structure information to nor structure */
> +		nor->info = info;
> +

Instead, could you update spi_nor_scan() to initialize nor->info before
calling spi_nor_init_params()?

>  		memcpy(&sfdp_params, params, sizeof(sfdp_params));
>  		if (spi_nor_parse_sfdp(nor, &sfdp_params)) {
>  			nor->addr_width = 0;
> diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
> index 973b6f8..5a29c8b 100644
> --- a/drivers/mtd/spi/spi-nor-ids.c
> +++ b/drivers/mtd/spi/spi-nor-ids.c
> @@ -135,7 +135,12 @@ const struct flash_info spi_nor_ids[] = {
>  	{ INFO("is25wp128",  0x9d7018, 0, 64 * 1024, 256,
>  			SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>  	{ INFO("is25wp256",  0x9d7019, 0, 64 * 1024, 512,
> -			SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> +			SECT_4K | SPI_NOR_4B_OPCODES | SPI_NOR_DUAL_READ
> +			| SPI_NOR_QUAD_READ)
> +#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
> +			.fixups = &is25wp256_fixups
> +#endif
> +	},
>  #endif
>  #ifdef CONFIG_SPI_FLASH_MACRONIX	/* MACRONIX */
>  	/* Macronix */
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 1d91177..44b7479 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -23,6 +23,7 @@
>  #define SNOR_MFR_ST		CFI_MFR_ST /* ST Micro <--> Micron */
>  #define SNOR_MFR_MICRON		CFI_MFR_MICRON /* ST Micro <--> Micron */
>  #define SNOR_MFR_MACRONIX	CFI_MFR_MACRONIX
> +#define SNOR_MFR_ISSI		CFI_MFR_PMC
>  #define SNOR_MFR_SPANSION	CFI_MFR_AMD
>  #define SNOR_MFR_SST		CFI_MFR_SST
>  #define SNOR_MFR_WINBOND	0xef /* Also used by some Spansion */
>
Sagar Shrikant Kadam Jan. 24, 2020, 6:28 a.m. UTC | #2
Hello Vignesh,

> -----Original Message-----
> From: Vignesh Raghavendra <vigneshr at ti.com>
> Sent: Friday, January 24, 2020 10:24 AM
> To: Sagar Kadam <sagar.kadam at sifive.com>; u-boot at lists.denx.de
> Cc: Paul Walmsley ( Sifive) <paul.walmsley at sifive.com>;
> anup.patel at wdc.com; ick at andestech.com; atish.patra at wdc.com;
> jagan at amarulasolutions.com
> Subject: Re: [U-Boot Patch v1 6/7] nor: add post bfpt fix handler for
> is25wp256 device
> 
> Hi,
> 
> On 24/01/20 1:46 am, Sagar Shrikant Kadam wrote:
> > Update vendor id for ISSI flash, enable SFDP as ISSI flash supports it
> > and add support for spi_nor_fixups similar to that done in linux.
> > Flash vendor specific fixups can be registered in spi_nor_ids, and
> > will be called after BFPT parsing to fix any wrong parameter read from
> > SFDP.
> >
> > Signed-off-by: Sagar Shrikant Kadam <sagar.kadam at sifive.com>
> > ---
> >  board/sifive/fu540/Kconfig     |  1 +
> >  drivers/mtd/spi/sf_internal.h  | 16 +++++++++++
> > drivers/mtd/spi/spi-nor-core.c | 63
> > ++++++++++++++++++++++++++++++++++++++++--
> >  drivers/mtd/spi/spi-nor-ids.c  |  7 ++++-
> >  include/linux/mtd/spi-nor.h    |  1 +
> >  5 files changed, 85 insertions(+), 3 deletions(-)
> >
> > diff --git a/board/sifive/fu540/Kconfig b/board/sifive/fu540/Kconfig
> > index 75661f3..d9e4956 100644
> > --- a/board/sifive/fu540/Kconfig
> > +++ b/board/sifive/fu540/Kconfig
> > @@ -42,6 +42,7 @@ config BOARD_SPECIFIC_OPTIONS # dummy
> >  	imply SPI
> >  	imply SPI_SIFIVE
> >  	imply SPI_FLASH
> > +	imply SPI_FLASH_SFDP_SUPPORT
> >  	imply SPI_FLASH_ISSI
> >  	imply MMC
> >  	imply MMC_SPI
> 
> This does not belong to this patch. Its better that this change goes via SiFive
> SoC tree.

Thanks for your inputs. I will move it.
Just that I understand this correctly shall I add this config change as a separate patch apart from the series
or as a different patch containing only this change within this series.

> 
> > diff --git a/drivers/mtd/spi/sf_internal.h
> > b/drivers/mtd/spi/sf_internal.h index 5c64303..856866f 100644
> > --- a/drivers/mtd/spi/sf_internal.h
> > +++ b/drivers/mtd/spi/sf_internal.h
> > @@ -66,8 +66,24 @@ struct flash_info {
> >  #define SPI_NOR_SKIP_SFDP	BIT(13)	/* Skip parsing of SFDP tables */
> >  #define USE_CLSR		BIT(14)	/* use CLSR command */
> >  #define SPI_NOR_HAS_SST26LOCK	BIT(15)	/* Flash supports lock/unlock
> via BPR */
> > +
> > +#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
> 
> Change above ifdef to:
> 
> 	#if CONFIG_IS_ENABLED(SPI_FLASH_SFDP_SUPPORT)
> 
> throughout the patch to take care of both SPL and U-Boot builds
> 
Sure, will modify this and send it in V2.

> > +	/* Part specific fixup hooks */
> > +	const struct spi_nor_fixups	*fixups;
> > +#endif
> >  };
> >
> > +#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
> > +/*
> > + * Declare manufacturer specific fixup handlers that
> > + * can be registered as fixup's in flash info table
> > + * so as to update any wrong/broken SFDP parameter.
> > + */
> > +#ifdef CONFIG_SPI_FLASH_ISSI
> > +extern struct spi_nor_fixups is25wp256_fixups; #endif #endif
> > +
> >  extern const struct flash_info spi_nor_ids[];
> >
> >  #define JEDEC_MFR(info)	((info)->id[0])
> > diff --git a/drivers/mtd/spi/spi-nor-core.c
> > b/drivers/mtd/spi/spi-nor-core.c index 6e7fc23..c55116f 100644
> > --- a/drivers/mtd/spi/spi-nor-core.c
> > +++ b/drivers/mtd/spi/spi-nor-core.c
> > @@ -296,7 +296,6 @@ static void spi_nor_set_4byte_opcodes(struct
> spi_nor *nor,
> >  		nor->mtd.erasesize = info->sector_size;
> >  		break;
> >
> > -	default:
> 
> Is this an intentional change?
> 
No this was not intentional, got reverted in 7/7 
I will correct it.

> >  		break;
> >  	}
> >
> > @@ -1800,6 +1799,57 @@ static const struct sfdp_bfpt_erase
> > sfdp_bfpt_erases[] = {
> >
> >  static int spi_nor_hwcaps_read2cmd(u32 hwcaps);
> >
> > +#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
> > +/**
> > + * struct spi_nor_fixups - SPI NOR fixup hooks
> > + * @post_bfpt: called after the BFPT table has been parsed
> > + *
> > + * Those hooks can be used to tweak the SPI NOR configuration when
> > +the SFDP
> > + * table is broken or not available.
> > + */
> > +struct spi_nor_fixups {
> > +	int (*post_bfpt)(struct spi_nor *nor,
> > +			 const struct sfdp_parameter_header *bfpt_header,
> > +			 const struct sfdp_bfpt *bfpt,
> > +			 struct spi_nor_flash_parameter *params); };
> > +
> > +static int spi_nor_post_bfpt_fixups(struct spi_nor *nor,
> > +				    const struct sfdp_parameter_header
> > +						*bfpt_header,
> > +				    const struct sfdp_bfpt *bfpt,
> > +				    struct spi_nor_flash_parameter *params) {
> > +	if (nor->info->fixups && nor->info->fixups->post_bfpt)
> > +		return nor->info->fixups->post_bfpt(nor, bfpt_header, bfpt,
> > +				params);
> > +
> > +	return 0;
> > +}
> > +
> > +static int is25wp256_post_bfpt_fixups(struct spi_nor *nor,
> > +				      const struct sfdp_parameter_header
> > +						*bfpt_header,
> > +				      const struct sfdp_bfpt *bfpt,
> > +				      struct spi_nor_flash_parameter *params)
> > +
> > +{
> > +	/* IS25WP256 supports 4B opcodes, but the BFPT advertises a
> > +	 * BFPT_DWORD1_ADDRESS_BYTES_3_ONLY address width.
> > +	 * Overwrite the address width advertised by the BFPT.
> > +	 */
> > +	if ((bfpt->dwords[BFPT_DWORD(1)] &
> BFPT_DWORD1_ADDRESS_BYTES_MASK) ==
> > +			BFPT_DWORD1_ADDRESS_BYTES_3_ONLY)
> > +		nor->addr_width = 4;
> > +
> > +	return 0;
> > +}
> > +
> > +struct spi_nor_fixups is25wp256_fixups = {
> > +	.post_bfpt = is25wp256_post_bfpt_fixups, }; #endif
> > +
> >  /**
> >   * spi_nor_parse_bfpt() - read and parse the Basic Flash Parameter Table.
> >   * @nor:		pointer to a 'struct spi_nor'
> > @@ -1971,7 +2021,13 @@ static int spi_nor_parse_bfpt(struct spi_nor
> *nor,
> >  		return -EINVAL;
> >  	}
> >
> > -	return 0;
> > +#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
> > +	err = spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt,
> > +				       params);
> 
> Isn't this function (spi_nor_parse_bfpt()) already under
> CONFIG_IS_ENABLED(CONFIG_SPI_FLASH_SFDP_SUPPORT)?
>

Oh yes, thanks for catching this I will remove this guard here.
 
> > +#else
> > +	err = 0;
> > +#endif
> > +	return err;
> >  }
> >
> >  /**
> > @@ -2209,6 +2265,9 @@ static int spi_nor_init_params(struct spi_nor
> *nor,
> >  	    !(info->flags & SPI_NOR_SKIP_SFDP)) {
> >  		struct spi_nor_flash_parameter sfdp_params;
> >
> > +		/* Update flash structure information to nor structure */
> > +		nor->info = info;
> > +
> 
> Instead, could you update spi_nor_scan() to initialize nor->info before calling
> spi_nor_init_params()?
> 
Yes, I will move it to spi_nor_scan()

Thanks,
-Sagar

> >  		memcpy(&sfdp_params, params, sizeof(sfdp_params));
> >  		if (spi_nor_parse_sfdp(nor, &sfdp_params)) {
> >  			nor->addr_width = 0;
> > diff --git a/drivers/mtd/spi/spi-nor-ids.c
> > b/drivers/mtd/spi/spi-nor-ids.c index 973b6f8..5a29c8b 100644
> > --- a/drivers/mtd/spi/spi-nor-ids.c
> > +++ b/drivers/mtd/spi/spi-nor-ids.c
> > @@ -135,7 +135,12 @@ const struct flash_info spi_nor_ids[] = {
> >  	{ INFO("is25wp128",  0x9d7018, 0, 64 * 1024, 256,
> >  			SECT_4K | SPI_NOR_DUAL_READ |
> SPI_NOR_QUAD_READ) },
> >  	{ INFO("is25wp256",  0x9d7019, 0, 64 * 1024, 512,
> > -			SECT_4K | SPI_NOR_DUAL_READ |
> SPI_NOR_QUAD_READ) },
> > +			SECT_4K | SPI_NOR_4B_OPCODES |
> SPI_NOR_DUAL_READ
> > +			| SPI_NOR_QUAD_READ)
> > +#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
> > +			.fixups = &is25wp256_fixups
> > +#endif
> > +	},
> >  #endif
> >  #ifdef CONFIG_SPI_FLASH_MACRONIX	/* MACRONIX */
> >  	/* Macronix */
> > diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> > index 1d91177..44b7479 100644
> > --- a/include/linux/mtd/spi-nor.h
> > +++ b/include/linux/mtd/spi-nor.h
> > @@ -23,6 +23,7 @@
> >  #define SNOR_MFR_ST		CFI_MFR_ST /* ST Micro <--> Micron
> */
> >  #define SNOR_MFR_MICRON		CFI_MFR_MICRON /* ST
> Micro <--> Micron */
> >  #define SNOR_MFR_MACRONIX	CFI_MFR_MACRONIX
> > +#define SNOR_MFR_ISSI		CFI_MFR_PMC
> >  #define SNOR_MFR_SPANSION	CFI_MFR_AMD
> >  #define SNOR_MFR_SST		CFI_MFR_SST
> >  #define SNOR_MFR_WINBOND	0xef /* Also used by some Spansion
> */
> >
> 
> --
> Regards
> Vignesh
Vignesh Raghavendra Jan. 27, 2020, 5:17 a.m. UTC | #3
On 24/01/20 11:58 am, Sagar Kadam wrote:
> Hello Vignesh,
> 
>> -----Original Message-----
>> From: Vignesh Raghavendra <vigneshr at ti.com>
>> Sent: Friday, January 24, 2020 10:24 AM
>> To: Sagar Kadam <sagar.kadam at sifive.com>; u-boot at lists.denx.de
>> Cc: Paul Walmsley ( Sifive) <paul.walmsley at sifive.com>;
>> anup.patel at wdc.com; ick at andestech.com; atish.patra at wdc.com;
>> jagan at amarulasolutions.com
>> Subject: Re: [U-Boot Patch v1 6/7] nor: add post bfpt fix handler for
>> is25wp256 device
>>
>> Hi,
>>
>> On 24/01/20 1:46 am, Sagar Shrikant Kadam wrote:
>>> Update vendor id for ISSI flash, enable SFDP as ISSI flash supports it
>>> and add support for spi_nor_fixups similar to that done in linux.
>>> Flash vendor specific fixups can be registered in spi_nor_ids, and
>>> will be called after BFPT parsing to fix any wrong parameter read from
>>> SFDP.
>>>
>>> Signed-off-by: Sagar Shrikant Kadam <sagar.kadam at sifive.com>
>>> ---
>>>  board/sifive/fu540/Kconfig     |  1 +
>>>  drivers/mtd/spi/sf_internal.h  | 16 +++++++++++
>>> drivers/mtd/spi/spi-nor-core.c | 63
>>> ++++++++++++++++++++++++++++++++++++++++--
>>>  drivers/mtd/spi/spi-nor-ids.c  |  7 ++++-
>>>  include/linux/mtd/spi-nor.h    |  1 +
>>>  5 files changed, 85 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/board/sifive/fu540/Kconfig b/board/sifive/fu540/Kconfig
>>> index 75661f3..d9e4956 100644
>>> --- a/board/sifive/fu540/Kconfig
>>> +++ b/board/sifive/fu540/Kconfig
>>> @@ -42,6 +42,7 @@ config BOARD_SPECIFIC_OPTIONS # dummy
>>>  	imply SPI
>>>  	imply SPI_SIFIVE
>>>  	imply SPI_FLASH
>>> +	imply SPI_FLASH_SFDP_SUPPORT
>>>  	imply SPI_FLASH_ISSI
>>>  	imply MMC
>>>  	imply MMC_SPI
>>
>> This does not belong to this patch. Its better that this change goes via SiFive
>> SoC tree.
> 
> Thanks for your inputs. I will move it.
> Just that I understand this correctly shall I add this config change as a separate patch apart from the series
> or as a different patch containing only this change within this series.
> 

Unless there is a real dependency, it would be great to separate out
SPI-NOR related changes to different series, so that it could go via
Jagan's SPI tree.

Regards
Vignesh

>>
>>> diff --git a/drivers/mtd/spi/sf_internal.h
>>> b/drivers/mtd/spi/sf_internal.h index 5c64303..856866f 100644
>>> --- a/drivers/mtd/spi/sf_internal.h
>>> +++ b/drivers/mtd/spi/sf_internal.h
>>> @@ -66,8 +66,24 @@ struct flash_info {
>>>  #define SPI_NOR_SKIP_SFDP	BIT(13)	/* Skip parsing of SFDP tables */
>>>  #define USE_CLSR		BIT(14)	/* use CLSR command */
>>>  #define SPI_NOR_HAS_SST26LOCK	BIT(15)	/* Flash supports lock/unlock
>> via BPR */
>>> +
>>> +#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
>>
>> Change above ifdef to:
>>
>> 	#if CONFIG_IS_ENABLED(SPI_FLASH_SFDP_SUPPORT)
>>
>> throughout the patch to take care of both SPL and U-Boot builds
>>
> Sure, will modify this and send it in V2.
> 
>>> +	/* Part specific fixup hooks */
>>> +	const struct spi_nor_fixups	*fixups;
>>> +#endif
>>>  };
>>>
>>> +#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
>>> +/*
>>> + * Declare manufacturer specific fixup handlers that
>>> + * can be registered as fixup's in flash info table
>>> + * so as to update any wrong/broken SFDP parameter.
>>> + */
>>> +#ifdef CONFIG_SPI_FLASH_ISSI
>>> +extern struct spi_nor_fixups is25wp256_fixups; #endif #endif
>>> +
>>>  extern const struct flash_info spi_nor_ids[];
>>>
>>>  #define JEDEC_MFR(info)	((info)->id[0])
>>> diff --git a/drivers/mtd/spi/spi-nor-core.c
>>> b/drivers/mtd/spi/spi-nor-core.c index 6e7fc23..c55116f 100644
>>> --- a/drivers/mtd/spi/spi-nor-core.c
>>> +++ b/drivers/mtd/spi/spi-nor-core.c
>>> @@ -296,7 +296,6 @@ static void spi_nor_set_4byte_opcodes(struct
>> spi_nor *nor,
>>>  		nor->mtd.erasesize = info->sector_size;
>>>  		break;
>>>
>>> -	default:
>>
>> Is this an intentional change?
>>
> No this was not intentional, got reverted in 7/7 
> I will correct it.
> 
>>>  		break;
>>>  	}
>>>
>>> @@ -1800,6 +1799,57 @@ static const struct sfdp_bfpt_erase
>>> sfdp_bfpt_erases[] = {
>>>
>>>  static int spi_nor_hwcaps_read2cmd(u32 hwcaps);
>>>
>>> +#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
>>> +/**
>>> + * struct spi_nor_fixups - SPI NOR fixup hooks
>>> + * @post_bfpt: called after the BFPT table has been parsed
>>> + *
>>> + * Those hooks can be used to tweak the SPI NOR configuration when
>>> +the SFDP
>>> + * table is broken or not available.
>>> + */
>>> +struct spi_nor_fixups {
>>> +	int (*post_bfpt)(struct spi_nor *nor,
>>> +			 const struct sfdp_parameter_header *bfpt_header,
>>> +			 const struct sfdp_bfpt *bfpt,
>>> +			 struct spi_nor_flash_parameter *params); };
>>> +
>>> +static int spi_nor_post_bfpt_fixups(struct spi_nor *nor,
>>> +				    const struct sfdp_parameter_header
>>> +						*bfpt_header,
>>> +				    const struct sfdp_bfpt *bfpt,
>>> +				    struct spi_nor_flash_parameter *params) {
>>> +	if (nor->info->fixups && nor->info->fixups->post_bfpt)
>>> +		return nor->info->fixups->post_bfpt(nor, bfpt_header, bfpt,
>>> +				params);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int is25wp256_post_bfpt_fixups(struct spi_nor *nor,
>>> +				      const struct sfdp_parameter_header
>>> +						*bfpt_header,
>>> +				      const struct sfdp_bfpt *bfpt,
>>> +				      struct spi_nor_flash_parameter *params)
>>> +
>>> +{
>>> +	/* IS25WP256 supports 4B opcodes, but the BFPT advertises a
>>> +	 * BFPT_DWORD1_ADDRESS_BYTES_3_ONLY address width.
>>> +	 * Overwrite the address width advertised by the BFPT.
>>> +	 */
>>> +	if ((bfpt->dwords[BFPT_DWORD(1)] &
>> BFPT_DWORD1_ADDRESS_BYTES_MASK) ==
>>> +			BFPT_DWORD1_ADDRESS_BYTES_3_ONLY)
>>> +		nor->addr_width = 4;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +struct spi_nor_fixups is25wp256_fixups = {
>>> +	.post_bfpt = is25wp256_post_bfpt_fixups, }; #endif
>>> +
>>>  /**
>>>   * spi_nor_parse_bfpt() - read and parse the Basic Flash Parameter Table.
>>>   * @nor:		pointer to a 'struct spi_nor'
>>> @@ -1971,7 +2021,13 @@ static int spi_nor_parse_bfpt(struct spi_nor
>> *nor,
>>>  		return -EINVAL;
>>>  	}
>>>
>>> -	return 0;
>>> +#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
>>> +	err = spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt,
>>> +				       params);
>>
>> Isn't this function (spi_nor_parse_bfpt()) already under
>> CONFIG_IS_ENABLED(CONFIG_SPI_FLASH_SFDP_SUPPORT)?
>>
> 
> Oh yes, thanks for catching this I will remove this guard here.
>  
>>> +#else
>>> +	err = 0;
>>> +#endif
>>> +	return err;
>>>  }
>>>
>>>  /**
>>> @@ -2209,6 +2265,9 @@ static int spi_nor_init_params(struct spi_nor
>> *nor,
>>>  	    !(info->flags & SPI_NOR_SKIP_SFDP)) {
>>>  		struct spi_nor_flash_parameter sfdp_params;
>>>
>>> +		/* Update flash structure information to nor structure */
>>> +		nor->info = info;
>>> +
>>
>> Instead, could you update spi_nor_scan() to initialize nor->info before calling
>> spi_nor_init_params()?
>>
> Yes, I will move it to spi_nor_scan()
> 
> Thanks,
> -Sagar
> 
>>>  		memcpy(&sfdp_params, params, sizeof(sfdp_params));
>>>  		if (spi_nor_parse_sfdp(nor, &sfdp_params)) {
>>>  			nor->addr_width = 0;
>>> diff --git a/drivers/mtd/spi/spi-nor-ids.c
>>> b/drivers/mtd/spi/spi-nor-ids.c index 973b6f8..5a29c8b 100644
>>> --- a/drivers/mtd/spi/spi-nor-ids.c
>>> +++ b/drivers/mtd/spi/spi-nor-ids.c
>>> @@ -135,7 +135,12 @@ const struct flash_info spi_nor_ids[] = {
>>>  	{ INFO("is25wp128",  0x9d7018, 0, 64 * 1024, 256,
>>>  			SECT_4K | SPI_NOR_DUAL_READ |
>> SPI_NOR_QUAD_READ) },
>>>  	{ INFO("is25wp256",  0x9d7019, 0, 64 * 1024, 512,
>>> -			SECT_4K | SPI_NOR_DUAL_READ |
>> SPI_NOR_QUAD_READ) },
>>> +			SECT_4K | SPI_NOR_4B_OPCODES |
>> SPI_NOR_DUAL_READ
>>> +			| SPI_NOR_QUAD_READ)
>>> +#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
>>> +			.fixups = &is25wp256_fixups
>>> +#endif
>>> +	},
>>>  #endif
>>>  #ifdef CONFIG_SPI_FLASH_MACRONIX	/* MACRONIX */
>>>  	/* Macronix */
>>> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
>>> index 1d91177..44b7479 100644
>>> --- a/include/linux/mtd/spi-nor.h
>>> +++ b/include/linux/mtd/spi-nor.h
>>> @@ -23,6 +23,7 @@
>>>  #define SNOR_MFR_ST		CFI_MFR_ST /* ST Micro <--> Micron
>> */
>>>  #define SNOR_MFR_MICRON		CFI_MFR_MICRON /* ST
>> Micro <--> Micron */
>>>  #define SNOR_MFR_MACRONIX	CFI_MFR_MACRONIX
>>> +#define SNOR_MFR_ISSI		CFI_MFR_PMC
>>>  #define SNOR_MFR_SPANSION	CFI_MFR_AMD
>>>  #define SNOR_MFR_SST		CFI_MFR_SST
>>>  #define SNOR_MFR_WINBOND	0xef /* Also used by some Spansion
>> */
>>>
>>
>> --
>> Regards
>> Vignesh
Sagar Shrikant Kadam Jan. 28, 2020, 4:14 a.m. UTC | #4
Hi Vignesh,

> -----Original Message-----
> From: Vignesh Raghavendra <vigneshr at ti.com>
> Sent: Monday, January 27, 2020 10:48 AM
> To: Sagar Kadam <sagar.kadam at sifive.com>; u-boot at lists.denx.de
> Cc: Paul Walmsley ( Sifive) <paul.walmsley at sifive.com>;
> anup.patel at wdc.com; atish.patra at wdc.com; jagan at amarulasolutions.com;
> Rick Chen <rick at andestech.com>
> Subject: Re: [U-Boot Patch v1 6/7] nor: add post bfpt fix handler for
> is25wp256 device
> 
> 
> 
> On 24/01/20 11:58 am, Sagar Kadam wrote:
> > Hello Vignesh,
> >
> >> -----Original Message-----
> >> From: Vignesh Raghavendra <vigneshr at ti.com>
> >> Sent: Friday, January 24, 2020 10:24 AM
> >> To: Sagar Kadam <sagar.kadam at sifive.com>; u-boot at lists.denx.de
> >> Cc: Paul Walmsley ( Sifive) <paul.walmsley at sifive.com>;
> >> anup.patel at wdc.com; ick at andestech.com; atish.patra at wdc.com;
> >> jagan at amarulasolutions.com
> >> Subject: Re: [U-Boot Patch v1 6/7] nor: add post bfpt fix handler for
> >> is25wp256 device
> >>
> >> Hi,
> >>
> >> On 24/01/20 1:46 am, Sagar Shrikant Kadam wrote:
> >>> Update vendor id for ISSI flash, enable SFDP as ISSI flash supports it
> >>> and add support for spi_nor_fixups similar to that done in linux.
> >>> Flash vendor specific fixups can be registered in spi_nor_ids, and
> >>> will be called after BFPT parsing to fix any wrong parameter read from
> >>> SFDP.
> >>>
> >>> Signed-off-by: Sagar Shrikant Kadam <sagar.kadam at sifive.com>
> >>> ---
> >>>  board/sifive/fu540/Kconfig     |  1 +
> >>>  drivers/mtd/spi/sf_internal.h  | 16 +++++++++++
> >>> drivers/mtd/spi/spi-nor-core.c | 63
> >>> ++++++++++++++++++++++++++++++++++++++++--
> >>>  drivers/mtd/spi/spi-nor-ids.c  |  7 ++++-
> >>>  include/linux/mtd/spi-nor.h    |  1 +
> >>>  5 files changed, 85 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/board/sifive/fu540/Kconfig b/board/sifive/fu540/Kconfig
> >>> index 75661f3..d9e4956 100644
> >>> --- a/board/sifive/fu540/Kconfig
> >>> +++ b/board/sifive/fu540/Kconfig
> >>> @@ -42,6 +42,7 @@ config BOARD_SPECIFIC_OPTIONS # dummy
> >>>  	imply SPI
> >>>  	imply SPI_SIFIVE
> >>>  	imply SPI_FLASH
> >>> +	imply SPI_FLASH_SFDP_SUPPORT
> >>>  	imply SPI_FLASH_ISSI
> >>>  	imply MMC
> >>>  	imply MMC_SPI
> >>
> >> This does not belong to this patch. Its better that this change goes via
> SiFive
> >> SoC tree.
> >
> > Thanks for your inputs. I will move it.
> > Just that I understand this correctly shall I add this config change as a
> separate patch apart from the series
> > or as a different patch containing only this change within this series.
> >
> 
> Unless there is a real dependency, it would be great to separate out
> SPI-NOR related changes to different series, so that it could go via
> Jagan's SPI tree.
> 
> Regards
> Vignesh
>
Yes, I will exclude this patch from this series.

-BR,
Sagar
> >>
> >>> diff --git a/drivers/mtd/spi/sf_internal.h
> >>> b/drivers/mtd/spi/sf_internal.h index 5c64303..856866f 100644
> >>> --- a/drivers/mtd/spi/sf_internal.h
> >>> +++ b/drivers/mtd/spi/sf_internal.h
> >>> @@ -66,8 +66,24 @@ struct flash_info {
> >>>  #define SPI_NOR_SKIP_SFDP	BIT(13)	/* Skip parsing of SFDP tables
> */
> >>>  #define USE_CLSR		BIT(14)	/* use CLSR command */
> >>>  #define SPI_NOR_HAS_SST26LOCK	BIT(15)	/* Flash supports lock/unlock
> >> via BPR */
> >>> +
> >>> +#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
> >>
> >> Change above ifdef to:
> >>
> >> 	#if CONFIG_IS_ENABLED(SPI_FLASH_SFDP_SUPPORT)
> >>
> >> throughout the patch to take care of both SPL and U-Boot builds
> >>
> > Sure, will modify this and send it in V2.
> >
> >>> +	/* Part specific fixup hooks */
> >>> +	const struct spi_nor_fixups	*fixups;
> >>> +#endif
> >>>  };
> >>>
> >>> +#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
> >>> +/*
> >>> + * Declare manufacturer specific fixup handlers that
> >>> + * can be registered as fixup's in flash info table
> >>> + * so as to update any wrong/broken SFDP parameter.
> >>> + */
> >>> +#ifdef CONFIG_SPI_FLASH_ISSI
> >>> +extern struct spi_nor_fixups is25wp256_fixups; #endif #endif
> >>> +
> >>>  extern const struct flash_info spi_nor_ids[];
> >>>
> >>>  #define JEDEC_MFR(info)	((info)->id[0])
> >>> diff --git a/drivers/mtd/spi/spi-nor-core.c
> >>> b/drivers/mtd/spi/spi-nor-core.c index 6e7fc23..c55116f 100644
> >>> --- a/drivers/mtd/spi/spi-nor-core.c
> >>> +++ b/drivers/mtd/spi/spi-nor-core.c
> >>> @@ -296,7 +296,6 @@ static void spi_nor_set_4byte_opcodes(struct
> >> spi_nor *nor,
> >>>  		nor->mtd.erasesize = info->sector_size;
> >>>  		break;
> >>>
> >>> -	default:
> >>
> >> Is this an intentional change?
> >>
> > No this was not intentional, got reverted in 7/7
> > I will correct it.
> >
> >>>  		break;
> >>>  	}
> >>>
> >>> @@ -1800,6 +1799,57 @@ static const struct sfdp_bfpt_erase
> >>> sfdp_bfpt_erases[] = {
> >>>
> >>>  static int spi_nor_hwcaps_read2cmd(u32 hwcaps);
> >>>
> >>> +#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
> >>> +/**
> >>> + * struct spi_nor_fixups - SPI NOR fixup hooks
> >>> + * @post_bfpt: called after the BFPT table has been parsed
> >>> + *
> >>> + * Those hooks can be used to tweak the SPI NOR configuration when
> >>> +the SFDP
> >>> + * table is broken or not available.
> >>> + */
> >>> +struct spi_nor_fixups {
> >>> +	int (*post_bfpt)(struct spi_nor *nor,
> >>> +			 const struct sfdp_parameter_header *bfpt_header,
> >>> +			 const struct sfdp_bfpt *bfpt,
> >>> +			 struct spi_nor_flash_parameter *params); };
> >>> +
> >>> +static int spi_nor_post_bfpt_fixups(struct spi_nor *nor,
> >>> +				    const struct sfdp_parameter_header
> >>> +						*bfpt_header,
> >>> +				    const struct sfdp_bfpt *bfpt,
> >>> +				    struct spi_nor_flash_parameter *params) {
> >>> +	if (nor->info->fixups && nor->info->fixups->post_bfpt)
> >>> +		return nor->info->fixups->post_bfpt(nor, bfpt_header, bfpt,
> >>> +				params);
> >>> +
> >>> +	return 0;
> >>> +}
> >>> +
> >>> +static int is25wp256_post_bfpt_fixups(struct spi_nor *nor,
> >>> +				      const struct sfdp_parameter_header
> >>> +						*bfpt_header,
> >>> +				      const struct sfdp_bfpt *bfpt,
> >>> +				      struct spi_nor_flash_parameter *params)
> >>> +
> >>> +{
> >>> +	/* IS25WP256 supports 4B opcodes, but the BFPT advertises a
> >>> +	 * BFPT_DWORD1_ADDRESS_BYTES_3_ONLY address width.
> >>> +	 * Overwrite the address width advertised by the BFPT.
> >>> +	 */
> >>> +	if ((bfpt->dwords[BFPT_DWORD(1)] &
> >> BFPT_DWORD1_ADDRESS_BYTES_MASK) ==
> >>> +			BFPT_DWORD1_ADDRESS_BYTES_3_ONLY)
> >>> +		nor->addr_width = 4;
> >>> +
> >>> +	return 0;
> >>> +}
> >>> +
> >>> +struct spi_nor_fixups is25wp256_fixups = {
> >>> +	.post_bfpt = is25wp256_post_bfpt_fixups, }; #endif
> >>> +
> >>>  /**
> >>>   * spi_nor_parse_bfpt() - read and parse the Basic Flash Parameter
> Table.
> >>>   * @nor:		pointer to a 'struct spi_nor'
> >>> @@ -1971,7 +2021,13 @@ static int spi_nor_parse_bfpt(struct spi_nor
> >> *nor,
> >>>  		return -EINVAL;
> >>>  	}
> >>>
> >>> -	return 0;
> >>> +#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
> >>> +	err = spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt,
> >>> +				       params);
> >>
> >> Isn't this function (spi_nor_parse_bfpt()) already under
> >> CONFIG_IS_ENABLED(CONFIG_SPI_FLASH_SFDP_SUPPORT)?
> >>
> >
> > Oh yes, thanks for catching this I will remove this guard here.
> >
> >>> +#else
> >>> +	err = 0;
> >>> +#endif
> >>> +	return err;
> >>>  }
> >>>
> >>>  /**
> >>> @@ -2209,6 +2265,9 @@ static int spi_nor_init_params(struct spi_nor
> >> *nor,
> >>>  	    !(info->flags & SPI_NOR_SKIP_SFDP)) {
> >>>  		struct spi_nor_flash_parameter sfdp_params;
> >>>
> >>> +		/* Update flash structure information to nor structure */
> >>> +		nor->info = info;
> >>> +
> >>
> >> Instead, could you update spi_nor_scan() to initialize nor->info before
> calling
> >> spi_nor_init_params()?
> >>
> > Yes, I will move it to spi_nor_scan()
> >
> > Thanks,
> > -Sagar
> >
> >>>  		memcpy(&sfdp_params, params, sizeof(sfdp_params));
> >>>  		if (spi_nor_parse_sfdp(nor, &sfdp_params)) {
> >>>  			nor->addr_width = 0;
> >>> diff --git a/drivers/mtd/spi/spi-nor-ids.c
> >>> b/drivers/mtd/spi/spi-nor-ids.c index 973b6f8..5a29c8b 100644
> >>> --- a/drivers/mtd/spi/spi-nor-ids.c
> >>> +++ b/drivers/mtd/spi/spi-nor-ids.c
> >>> @@ -135,7 +135,12 @@ const struct flash_info spi_nor_ids[] = {
> >>>  	{ INFO("is25wp128",  0x9d7018, 0, 64 * 1024, 256,
> >>>  			SECT_4K | SPI_NOR_DUAL_READ |
> >> SPI_NOR_QUAD_READ) },
> >>>  	{ INFO("is25wp256",  0x9d7019, 0, 64 * 1024, 512,
> >>> -			SECT_4K | SPI_NOR_DUAL_READ |
> >> SPI_NOR_QUAD_READ) },
> >>> +			SECT_4K | SPI_NOR_4B_OPCODES |
> >> SPI_NOR_DUAL_READ
> >>> +			| SPI_NOR_QUAD_READ)
> >>> +#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
> >>> +			.fixups = &is25wp256_fixups
> >>> +#endif
> >>> +	},
> >>>  #endif
> >>>  #ifdef CONFIG_SPI_FLASH_MACRONIX	/* MACRONIX */
> >>>  	/* Macronix */
> >>> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> >>> index 1d91177..44b7479 100644
> >>> --- a/include/linux/mtd/spi-nor.h
> >>> +++ b/include/linux/mtd/spi-nor.h
> >>> @@ -23,6 +23,7 @@
> >>>  #define SNOR_MFR_ST		CFI_MFR_ST /* ST Micro <--> Micron
> >> */
> >>>  #define SNOR_MFR_MICRON		CFI_MFR_MICRON /* ST
> >> Micro <--> Micron */
> >>>  #define SNOR_MFR_MACRONIX	CFI_MFR_MACRONIX
> >>> +#define SNOR_MFR_ISSI		CFI_MFR_PMC
> >>>  #define SNOR_MFR_SPANSION	CFI_MFR_AMD
> >>>  #define SNOR_MFR_SST		CFI_MFR_SST
> >>>  #define SNOR_MFR_WINBOND	0xef /* Also used by some Spansion
> >> */
> >>>
> >>
> >> --
> >> Regards
> >> Vignesh
> 
> --
> Regards
> Vignesh
diff mbox series

Patch

diff --git a/board/sifive/fu540/Kconfig b/board/sifive/fu540/Kconfig
index 75661f3..d9e4956 100644
--- a/board/sifive/fu540/Kconfig
+++ b/board/sifive/fu540/Kconfig
@@ -42,6 +42,7 @@  config BOARD_SPECIFIC_OPTIONS # dummy
 	imply SPI
 	imply SPI_SIFIVE
 	imply SPI_FLASH
+	imply SPI_FLASH_SFDP_SUPPORT
 	imply SPI_FLASH_ISSI
 	imply MMC
 	imply MMC_SPI
diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 5c64303..856866f 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -66,8 +66,24 @@  struct flash_info {
 #define SPI_NOR_SKIP_SFDP	BIT(13)	/* Skip parsing of SFDP tables */
 #define USE_CLSR		BIT(14)	/* use CLSR command */
 #define SPI_NOR_HAS_SST26LOCK	BIT(15)	/* Flash supports lock/unlock via BPR */
+
+#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
+	/* Part specific fixup hooks */
+	const struct spi_nor_fixups	*fixups;
+#endif
 };
 
+#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
+/*
+ * Declare manufacturer specific fixup handlers that
+ * can be registered as fixup's in flash info table
+ * so as to update any wrong/broken SFDP parameter.
+ */
+#ifdef CONFIG_SPI_FLASH_ISSI
+extern struct spi_nor_fixups is25wp256_fixups;
+#endif
+#endif
+
 extern const struct flash_info spi_nor_ids[];
 
 #define JEDEC_MFR(info)	((info)->id[0])
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index 6e7fc23..c55116f 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -296,7 +296,6 @@  static void spi_nor_set_4byte_opcodes(struct spi_nor *nor,
 		nor->mtd.erasesize = info->sector_size;
 		break;
 
-	default:
 		break;
 	}
 
@@ -1800,6 +1799,57 @@  static const struct sfdp_bfpt_erase sfdp_bfpt_erases[] = {
 
 static int spi_nor_hwcaps_read2cmd(u32 hwcaps);
 
+#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
+/**
+ * struct spi_nor_fixups - SPI NOR fixup hooks
+ * @post_bfpt: called after the BFPT table has been parsed
+ *
+ * Those hooks can be used to tweak the SPI NOR configuration when the SFDP
+ * table is broken or not available.
+ */
+struct spi_nor_fixups {
+	int (*post_bfpt)(struct spi_nor *nor,
+			 const struct sfdp_parameter_header *bfpt_header,
+			 const struct sfdp_bfpt *bfpt,
+			 struct spi_nor_flash_parameter *params);
+};
+
+static int spi_nor_post_bfpt_fixups(struct spi_nor *nor,
+				    const struct sfdp_parameter_header
+						*bfpt_header,
+				    const struct sfdp_bfpt *bfpt,
+				    struct spi_nor_flash_parameter *params)
+{
+	if (nor->info->fixups && nor->info->fixups->post_bfpt)
+		return nor->info->fixups->post_bfpt(nor, bfpt_header, bfpt,
+				params);
+
+	return 0;
+}
+
+static int is25wp256_post_bfpt_fixups(struct spi_nor *nor,
+				      const struct sfdp_parameter_header
+						*bfpt_header,
+				      const struct sfdp_bfpt *bfpt,
+				      struct spi_nor_flash_parameter *params)
+
+{
+	/* IS25WP256 supports 4B opcodes, but the BFPT advertises a
+	 * BFPT_DWORD1_ADDRESS_BYTES_3_ONLY address width.
+	 * Overwrite the address width advertised by the BFPT.
+	 */
+	if ((bfpt->dwords[BFPT_DWORD(1)] & BFPT_DWORD1_ADDRESS_BYTES_MASK) ==
+			BFPT_DWORD1_ADDRESS_BYTES_3_ONLY)
+		nor->addr_width = 4;
+
+	return 0;
+}
+
+struct spi_nor_fixups is25wp256_fixups = {
+	.post_bfpt = is25wp256_post_bfpt_fixups,
+};
+#endif
+
 /**
  * spi_nor_parse_bfpt() - read and parse the Basic Flash Parameter Table.
  * @nor:		pointer to a 'struct spi_nor'
@@ -1971,7 +2021,13 @@  static int spi_nor_parse_bfpt(struct spi_nor *nor,
 		return -EINVAL;
 	}
 
-	return 0;
+#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
+	err = spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt,
+				       params);
+#else
+	err = 0;
+#endif
+	return err;
 }
 
 /**
@@ -2209,6 +2265,9 @@  static int spi_nor_init_params(struct spi_nor *nor,
 	    !(info->flags & SPI_NOR_SKIP_SFDP)) {
 		struct spi_nor_flash_parameter sfdp_params;
 
+		/* Update flash structure information to nor structure */
+		nor->info = info;
+
 		memcpy(&sfdp_params, params, sizeof(sfdp_params));
 		if (spi_nor_parse_sfdp(nor, &sfdp_params)) {
 			nor->addr_width = 0;
diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
index 973b6f8..5a29c8b 100644
--- a/drivers/mtd/spi/spi-nor-ids.c
+++ b/drivers/mtd/spi/spi-nor-ids.c
@@ -135,7 +135,12 @@  const struct flash_info spi_nor_ids[] = {
 	{ INFO("is25wp128",  0x9d7018, 0, 64 * 1024, 256,
 			SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 	{ INFO("is25wp256",  0x9d7019, 0, 64 * 1024, 512,
-			SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
+			SECT_4K | SPI_NOR_4B_OPCODES | SPI_NOR_DUAL_READ
+			| SPI_NOR_QUAD_READ)
+#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
+			.fixups = &is25wp256_fixups
+#endif
+	},
 #endif
 #ifdef CONFIG_SPI_FLASH_MACRONIX	/* MACRONIX */
 	/* Macronix */
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 1d91177..44b7479 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -23,6 +23,7 @@ 
 #define SNOR_MFR_ST		CFI_MFR_ST /* ST Micro <--> Micron */
 #define SNOR_MFR_MICRON		CFI_MFR_MICRON /* ST Micro <--> Micron */
 #define SNOR_MFR_MACRONIX	CFI_MFR_MACRONIX
+#define SNOR_MFR_ISSI		CFI_MFR_PMC
 #define SNOR_MFR_SPANSION	CFI_MFR_AMD
 #define SNOR_MFR_SST		CFI_MFR_SST
 #define SNOR_MFR_WINBOND	0xef /* Also used by some Spansion */