[PATCH 1/2] mtd: spi-nor: Make SFDP-based 4B_OPCODE support detection works correctly

[PATCH 1/2] mtd: spi-nor: Make SFDP-based 4B_OPCODE support detection works correctly

[Boris Brezillon]

Some flash_info entries have the SPI_NOR_4B_OPCODES to let the core
know that the flash supports 4B opcode. While this solution works fine
for id-based caps detection, it doesn't work that well when relying on
SFDP-based caps detection. Let's add an SNOR_F_4B_OPCODES flag so that
spi_nor_parse_bfpt() can add it when the BFPT_DWORD1_ADDRESS_BYTES
field is set to BFPT_DWORD1_ADDRESS_BYTES_4_ONLY.

Reported-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 11 ++++++++---
 include/linux/mtd/spi-nor.h   |  1 +
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 9407ca5f9443..85e57e9ea1b5 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2643,6 +2643,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
 		break;
 
 	case BFPT_DWORD1_ADDRESS_BYTES_4_ONLY:
+		nor->flags |= SNOR_F_4B_OPCODES;
 		nor->addr_width = 4;
 		break;
 
@@ -3552,7 +3553,7 @@ static int spi_nor_init(struct spi_nor *nor)
 
 	if ((nor->addr_width == 4) &&
 	    (JEDEC_MFR(nor->info) != SNOR_MFR_SPANSION) &&
-	    !(nor->info->flags & SPI_NOR_4B_OPCODES)) {
+	    !(nor->flags & SNOR_F_4B_OPCODES)) {
 		/*
 		 * If the RESET# pin isn't hooked up properly, or the system
 		 * otherwise doesn't perform a reset command in the boot
@@ -3586,7 +3587,7 @@ void spi_nor_restore(struct spi_nor *nor)
 	/* restore the addressing mode */
 	if ((nor->addr_width == 4) &&
 	    (JEDEC_MFR(nor->info) != SNOR_MFR_SPANSION) &&
-	    !(nor->info->flags & SPI_NOR_4B_OPCODES) &&
+	    !(nor->flags & SNOR_F_4B_OPCODES) &&
 	    (nor->flags & SNOR_F_BROKEN_RESET))
 		set_4byte(nor, nor->info, 0);
 }
@@ -3744,11 +3745,15 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 		nor->addr_width = 4;
 		if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
 		    info->flags & SPI_NOR_4B_OPCODES)
-			spi_nor_set_4byte_opcodes(nor, info);
+			nor->flags |= SNOR_F_4B_OPCODES;
 	} else {
 		nor->addr_width = 3;
 	}
 
+	if (info->addr_width == 4 &&
+	    nor->flags & SNOR_F_4B_OPCODES)
+		spi_nor_set_4byte_opcodes(nor, info);
+
 	if (nor->addr_width > SPI_NOR_MAX_ADDR_WIDTH) {
 		dev_err(dev, "address width is too large: %u\n",
 			nor->addr_width);
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 7f0c7303575e..4ffb165f4f85 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -236,6 +236,7 @@ enum spi_nor_option_flags {
 	SNOR_F_READY_XSR_RDY	= BIT(4),
 	SNOR_F_USE_CLSR		= BIT(5),
 	SNOR_F_BROKEN_RESET	= BIT(6),
+	SNOR_F_4B_OPCODES	= BIT(7)
 };
 
 /**
-- 
2.14.1

[PATCH 2/2] mtd: spi-nor: Use 4B opcodes when the NOR advertises both 3B and 4B

[Boris Brezillon]

When the NOR supports 4 bytes opcodes we should use those instead of
switching the flash in 4-bytes mode. This way, we don't have to restore
the addressing mode when resetting the board.

Reported-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 85e57e9ea1b5..260ed1757a08 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2643,6 +2643,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
 		break;
 
 	case BFPT_DWORD1_ADDRESS_BYTES_4_ONLY:
+	case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4:
 		nor->flags |= SNOR_F_4B_OPCODES;
 		nor->addr_width = 4;
 		break;
-- 
2.14.1

Re: [PATCH 1/2] mtd: spi-nor: Make SFDP-based 4B_OPCODE support detection works correctly

[Boris Brezillon]

In the subject: s/Make/Make sure/

On Wed, 17 Oct 2018 16:44:23 +0200
Boris Brezillon <boris.brezillon@bootlin.com> wrote:

> Some flash_info entries have the SPI_NOR_4B_OPCODES to let the core

						     ^ flag set

> know that the flash supports 4B opcode. While this solution works fine
> for id-based caps detection, it doesn't work that well when relying on
> SFDP-based caps detection. Let's add an SNOR_F_4B_OPCODES flag so that
> spi_nor_parse_bfpt() can add it when the BFPT_DWORD1_ADDRESS_BYTES
> field is set to BFPT_DWORD1_ADDRESS_BYTES_4_ONLY.
> 
> Reported-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 11 ++++++++---
>  include/linux/mtd/spi-nor.h   |  1 +
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 9407ca5f9443..85e57e9ea1b5 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2643,6 +2643,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>  		break;
>  
>  	case BFPT_DWORD1_ADDRESS_BYTES_4_ONLY:
> +		nor->flags |= SNOR_F_4B_OPCODES;
>  		nor->addr_width = 4;
>  		break;
>  
> @@ -3552,7 +3553,7 @@ static int spi_nor_init(struct spi_nor *nor)
>  
>  	if ((nor->addr_width == 4) &&
>  	    (JEDEC_MFR(nor->info) != SNOR_MFR_SPANSION) &&
> -	    !(nor->info->flags & SPI_NOR_4B_OPCODES)) {
> +	    !(nor->flags & SNOR_F_4B_OPCODES)) {
>  		/*
>  		 * If the RESET# pin isn't hooked up properly, or the system
>  		 * otherwise doesn't perform a reset command in the boot
> @@ -3586,7 +3587,7 @@ void spi_nor_restore(struct spi_nor *nor)
>  	/* restore the addressing mode */
>  	if ((nor->addr_width == 4) &&
>  	    (JEDEC_MFR(nor->info) != SNOR_MFR_SPANSION) &&
> -	    !(nor->info->flags & SPI_NOR_4B_OPCODES) &&
> +	    !(nor->flags & SNOR_F_4B_OPCODES) &&
>  	    (nor->flags & SNOR_F_BROKEN_RESET))
>  		set_4byte(nor, nor->info, 0);
>  }
> @@ -3744,11 +3745,15 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>  		nor->addr_width = 4;
>  		if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
>  		    info->flags & SPI_NOR_4B_OPCODES)
> -			spi_nor_set_4byte_opcodes(nor, info);
> +			nor->flags |= SNOR_F_4B_OPCODES;
>  	} else {
>  		nor->addr_width = 3;
>  	}
>  
> +	if (info->addr_width == 4 &&

We should check nor->addr_width not info->addr_width.

> +	    nor->flags & SNOR_F_4B_OPCODES)
> +		spi_nor_set_4byte_opcodes(nor, info);
> +
>  	if (nor->addr_width > SPI_NOR_MAX_ADDR_WIDTH) {
>  		dev_err(dev, "address width is too large: %u\n",
>  			nor->addr_width);
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 7f0c7303575e..4ffb165f4f85 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -236,6 +236,7 @@ enum spi_nor_option_flags {
>  	SNOR_F_READY_XSR_RDY	= BIT(4),
>  	SNOR_F_USE_CLSR		= BIT(5),
>  	SNOR_F_BROKEN_RESET	= BIT(6),
> +	SNOR_F_4B_OPCODES	= BIT(7)

Missing comma at the end of the line.

I'll send a v2 soon.

Re: [PATCH 1/2] mtd: spi-nor: Make SFDP-based 4B_OPCODE support detection works correctly

[Cyrille Pitchen - M19942]

Hi Boris,

looks good, just a small remark below:

Le 17/10/2018 à 16:44, Boris Brezillon a écrit :
> Some flash_info entries have the SPI_NOR_4B_OPCODES to let the core
> know that the flash supports 4B opcode. While this solution works fine
> for id-based caps detection, it doesn't work that well when relying on
> SFDP-based caps detection. Let's add an SNOR_F_4B_OPCODES flag so that
> spi_nor_parse_bfpt() can add it when the BFPT_DWORD1_ADDRESS_BYTES
> field is set to BFPT_DWORD1_ADDRESS_BYTES_4_ONLY.
> 
> Reported-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 11 ++++++++---
>  include/linux/mtd/spi-nor.h   |  1 +
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 9407ca5f9443..85e57e9ea1b5 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2643,6 +2643,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>  		break;
>  
>  	case BFPT_DWORD1_ADDRESS_BYTES_4_ONLY:
> +		nor->flags |= SNOR_F_4B_OPCODES;

if spi_nor_parse_sdfp() fails, there is a kind of roll-back operation done
in spi_nor_init_params() to set the struct spi_nor *nor back to its previous
state.

		if (spi_nor_parse_sfdp(nor, &sfdp_params)) {
			nor->addr_width = 0;
			nor->mtd.erasesize = 0;
		} else {
			[...]

maybe "nor->flags &= ~SNOR_F_4B_OPCODES;" should be added there.
If this roll-back block grows too much, maybe we could introduce a
void spi_nor_roll_back_sfdp(struct spi_nor *nor) function.
Also it would make the roll back operation more explicit.

Best regards,

Cyrille

>  		nor->addr_width = 4;
>  		break;
>  
> @@ -3552,7 +3553,7 @@ static int spi_nor_init(struct spi_nor *nor)
>  
>  	if ((nor->addr_width == 4) &&
>  	    (JEDEC_MFR(nor->info) != SNOR_MFR_SPANSION) &&
> -	    !(nor->info->flags & SPI_NOR_4B_OPCODES)) {
> +	    !(nor->flags & SNOR_F_4B_OPCODES)) {
>  		/*
>  		 * If the RESET# pin isn't hooked up properly, or the system
>  		 * otherwise doesn't perform a reset command in the boot
> @@ -3586,7 +3587,7 @@ void spi_nor_restore(struct spi_nor *nor)
>  	/* restore the addressing mode */
>  	if ((nor->addr_width == 4) &&
>  	    (JEDEC_MFR(nor->info) != SNOR_MFR_SPANSION) &&
> -	    !(nor->info->flags & SPI_NOR_4B_OPCODES) &&
> +	    !(nor->flags & SNOR_F_4B_OPCODES) &&
>  	    (nor->flags & SNOR_F_BROKEN_RESET))
>  		set_4byte(nor, nor->info, 0);
>  }
> @@ -3744,11 +3745,15 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>  		nor->addr_width = 4;
>  		if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
>  		    info->flags & SPI_NOR_4B_OPCODES)
> -			spi_nor_set_4byte_opcodes(nor, info);
> +			nor->flags |= SNOR_F_4B_OPCODES;
>  	} else {
>  		nor->addr_width = 3;
>  	}
>  
> +	if (info->addr_width == 4 &&
> +	    nor->flags & SNOR_F_4B_OPCODES)
> +		spi_nor_set_4byte_opcodes(nor, info);
> +
>  	if (nor->addr_width > SPI_NOR_MAX_ADDR_WIDTH) {
>  		dev_err(dev, "address width is too large: %u\n",
>  			nor->addr_width);
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 7f0c7303575e..4ffb165f4f85 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -236,6 +236,7 @@ enum spi_nor_option_flags {
>  	SNOR_F_READY_XSR_RDY	= BIT(4),
>  	SNOR_F_USE_CLSR		= BIT(5),
>  	SNOR_F_BROKEN_RESET	= BIT(6),
> +	SNOR_F_4B_OPCODES	= BIT(7)
>  };
>  
>  /**
> 

Re: [PATCH 1/2] mtd: spi-nor: Make SFDP-based 4B_OPCODE support detection works correctly

[Boris Brezillon]

On Fri, 19 Oct 2018 09:50:31 +0200
Cyrille Pitchen - M19942 <cyrille.pitchen@microchip.com> wrote:

> Hi Boris,
> 
> looks good, just a small remark below:
> 
> Le 17/10/2018 à 16:44, Boris Brezillon a écrit :
> > Some flash_info entries have the SPI_NOR_4B_OPCODES to let the core
> > know that the flash supports 4B opcode. While this solution works fine
> > for id-based caps detection, it doesn't work that well when relying on
> > SFDP-based caps detection. Let's add an SNOR_F_4B_OPCODES flag so that
> > spi_nor_parse_bfpt() can add it when the BFPT_DWORD1_ADDRESS_BYTES
> > field is set to BFPT_DWORD1_ADDRESS_BYTES_4_ONLY.
> > 
> > Reported-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> > ---
> >  drivers/mtd/spi-nor/spi-nor.c | 11 ++++++++---
> >  include/linux/mtd/spi-nor.h   |  1 +
> >  2 files changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> > index 9407ca5f9443..85e57e9ea1b5 100644
> > --- a/drivers/mtd/spi-nor/spi-nor.c
> > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > @@ -2643,6 +2643,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
> >  		break;
> >  
> >  	case BFPT_DWORD1_ADDRESS_BYTES_4_ONLY:
> > +		nor->flags |= SNOR_F_4B_OPCODES;  
> 
> if spi_nor_parse_sdfp() fails, there is a kind of roll-back operation done
> in spi_nor_init_params() to set the struct spi_nor *nor back to its previous
> state.
> 
> 		if (spi_nor_parse_sfdp(nor, &sfdp_params)) {
> 			nor->addr_width = 0;
> 			nor->mtd.erasesize = 0;
> 		} else {
> 			[...]
> 
> maybe "nor->flags &= ~SNOR_F_4B_OPCODES;" should be added there.

Actually, it should be

	if (!(info->flags & SPI_NOR_4B_OPCODES))
		nor->flags &= ~SNOR_F_4B_OPCODES;

but yes, this is missing. I'll fix that in v3.

> If this roll-back block grows too much, maybe we could introduce a
> void spi_nor_roll_back_sfdp(struct spi_nor *nor) function.
> Also it would make the roll back operation more explicit.

I'm wondering why we revert everything when a single bit is bit
reported as inconsistent in the SFDP table? I mean, it's not unusual
for NOR vendors to make mistake, and we should probably allow
vendor/chip specific code to fix the SFDP table at runtime instead of
discarding all the useful information we might have extracted.

Note that we recently introduced such a ->fixup() hook for the ONFI
param page in the raw NAND framework [1]. 

[1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/mtd/nand/raw?h=v4.19-rc8&id=00ce4e039ad5bded462931606c3063ff691964b7

Re: [PATCH 1/2] mtd: spi-nor: Make SFDP-based 4B_OPCODE support detection works correctly

[Cyrille Pitchen]

Le 19/10/2018 à 10:05, Boris Brezillon a écrit :
> On Fri, 19 Oct 2018 09:50:31 +0200
> Cyrille Pitchen - M19942 <cyrille.pitchen@microchip.com> wrote:
> 
>> Hi Boris,
>>
>> looks good, just a small remark below:
>>
>> Le 17/10/2018 à 16:44, Boris Brezillon a écrit :
>>> Some flash_info entries have the SPI_NOR_4B_OPCODES to let the core
>>> know that the flash supports 4B opcode. While this solution works fine
>>> for id-based caps detection, it doesn't work that well when relying on
>>> SFDP-based caps detection. Let's add an SNOR_F_4B_OPCODES flag so that
>>> spi_nor_parse_bfpt() can add it when the BFPT_DWORD1_ADDRESS_BYTES
>>> field is set to BFPT_DWORD1_ADDRESS_BYTES_4_ONLY.
>>>
>>> Reported-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>>> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
>>> ---
>>>  drivers/mtd/spi-nor/spi-nor.c | 11 ++++++++---
>>>  include/linux/mtd/spi-nor.h   |  1 +
>>>  2 files changed, 9 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>>> index 9407ca5f9443..85e57e9ea1b5 100644
>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>> @@ -2643,6 +2643,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>>>  		break;
>>>  
>>>  	case BFPT_DWORD1_ADDRESS_BYTES_4_ONLY:
>>> +		nor->flags |= SNOR_F_4B_OPCODES;  
>>
>> if spi_nor_parse_sdfp() fails, there is a kind of roll-back operation done
>> in spi_nor_init_params() to set the struct spi_nor *nor back to its previous
>> state.
>>
>> 		if (spi_nor_parse_sfdp(nor, &sfdp_params)) {
>> 			nor->addr_width = 0;
>> 			nor->mtd.erasesize = 0;
>> 		} else {
>> 			[...]
>>
>> maybe "nor->flags &= ~SNOR_F_4B_OPCODES;" should be added there.
> 
> Actually, it should be
> 
> 	if (!(info->flags & SPI_NOR_4B_OPCODES))
> 		nor->flags &= ~SNOR_F_4B_OPCODES;
> 
> but yes, this is missing. I'll fix that in v3.
> 
>> If this roll-back block grows too much, maybe we could introduce a
>> void spi_nor_roll_back_sfdp(struct spi_nor *nor) function.
>> Also it would make the roll back operation more explicit.
> 
> I'm wondering why we revert everything when a single bit is bit
> reported as inconsistent in the SFDP table? I mean, it's not unusual
> for NOR vendors to make mistake, and we should probably allow
> vendor/chip specific code to fix the SFDP table at runtime instead of
> discarding all the useful information we might have extracted.
>

I tend to agree with you: it would have been better to keep as much
settings as possible. The correct settings of course. The question was
how to decide which settings are correct and which are wrong.
Some might be wrong even if they're semantically correct.

However since we could now use a new fixup mechanism, we can try to statically
fix known issues in SFDP table. For unknown issue dynamically found during
the parsing, maybe we should keep the roll-back mechanism. When the 
fix-up is introduced later for the faulty memory part, we can keep the
relevant settings and discard wrong settings.
Without an explicit fixup, I guess it's hard to know which settings could
be kept, so maybe a roll-back operation is more straight forward solution.

We just have to cross the fingers and pray for faulty SFDP tables not to be
so likely. You know how much I believe in SPI NOR memory manufacturer ;)
 
> Note that we recently introduced such a ->fixup() hook for the ONFI
> param page in the raw NAND framework [1]. 
> 
> [1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/mtd/nand/raw?h=v4.19-rc8&id=00ce4e039ad5bded462931606c3063ff691964b7
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 

Re: [PATCH 1/2] mtd: spi-nor: Make SFDP-based 4B_OPCODE support detection works correctly

[Boris Brezillon]

On Fri, 19 Oct 2018 09:50:31 +0200
Cyrille Pitchen - M19942 <cyrille.pitchen@microchip.com> wrote:

> Hi Boris,
> 
> looks good, just a small remark below:
> 
> Le 17/10/2018 à 16:44, Boris Brezillon a écrit :
> > Some flash_info entries have the SPI_NOR_4B_OPCODES to let the core
> > know that the flash supports 4B opcode. While this solution works fine
> > for id-based caps detection, it doesn't work that well when relying on
> > SFDP-based caps detection. Let's add an SNOR_F_4B_OPCODES flag so that
> > spi_nor_parse_bfpt() can add it when the BFPT_DWORD1_ADDRESS_BYTES
> > field is set to BFPT_DWORD1_ADDRESS_BYTES_4_ONLY.
> > 
> > Reported-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> > ---
> >  drivers/mtd/spi-nor/spi-nor.c | 11 ++++++++---
> >  include/linux/mtd/spi-nor.h   |  1 +
> >  2 files changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> > index 9407ca5f9443..85e57e9ea1b5 100644
> > --- a/drivers/mtd/spi-nor/spi-nor.c
> > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > @@ -2643,6 +2643,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
> >  		break;
> >  
> >  	case BFPT_DWORD1_ADDRESS_BYTES_4_ONLY:
> > +		nor->flags |= SNOR_F_4B_OPCODES;  
> 
> if spi_nor_parse_sdfp() fails, there is a kind of roll-back operation done
> in spi_nor_init_params() to set the struct spi_nor *nor back to its previous
> state.
> 
> 		if (spi_nor_parse_sfdp(nor, &sfdp_params)) {
> 			nor->addr_width = 0;

Looks like the ->addr_width=0; as been lost when adding support for
non-uniform erase size.

> 			nor->mtd.erasesize = 0;
> 		} else {
> 			[...]
> 
> maybe "nor->flags &= ~SNOR_F_4B_OPCODES;" should be added there.
> If this roll-back block grows too much, maybe we could introduce a
> void spi_nor_roll_back_sfdp(struct spi_nor *nor) function.
> Also it would make the roll back operation more explicit.
> 
> Best regards,
> 
> Cyrille
> 
> >  		nor->addr_width = 4;
> >  		break;
> >  
> > @@ -3552,7 +3553,7 @@ static int spi_nor_init(struct spi_nor *nor)
> >  
> >  	if ((nor->addr_width == 4) &&
> >  	    (JEDEC_MFR(nor->info) != SNOR_MFR_SPANSION) &&
> > -	    !(nor->info->flags & SPI_NOR_4B_OPCODES)) {
> > +	    !(nor->flags & SNOR_F_4B_OPCODES)) {
> >  		/*
> >  		 * If the RESET# pin isn't hooked up properly, or the system
> >  		 * otherwise doesn't perform a reset command in the boot
> > @@ -3586,7 +3587,7 @@ void spi_nor_restore(struct spi_nor *nor)
> >  	/* restore the addressing mode */
> >  	if ((nor->addr_width == 4) &&
> >  	    (JEDEC_MFR(nor->info) != SNOR_MFR_SPANSION) &&
> > -	    !(nor->info->flags & SPI_NOR_4B_OPCODES) &&
> > +	    !(nor->flags & SNOR_F_4B_OPCODES) &&
> >  	    (nor->flags & SNOR_F_BROKEN_RESET))
> >  		set_4byte(nor, nor->info, 0);
> >  }
> > @@ -3744,11 +3745,15 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> >  		nor->addr_width = 4;
> >  		if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
> >  		    info->flags & SPI_NOR_4B_OPCODES)
> > -			spi_nor_set_4byte_opcodes(nor, info);
> > +			nor->flags |= SNOR_F_4B_OPCODES;
> >  	} else {
> >  		nor->addr_width = 3;
> >  	}
> >  
> > +	if (info->addr_width == 4 &&
> > +	    nor->flags & SNOR_F_4B_OPCODES)
> > +		spi_nor_set_4byte_opcodes(nor, info);
> > +
> >  	if (nor->addr_width > SPI_NOR_MAX_ADDR_WIDTH) {
> >  		dev_err(dev, "address width is too large: %u\n",
> >  			nor->addr_width);
> > diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> > index 7f0c7303575e..4ffb165f4f85 100644
> > --- a/include/linux/mtd/spi-nor.h
> > +++ b/include/linux/mtd/spi-nor.h
> > @@ -236,6 +236,7 @@ enum spi_nor_option_flags {
> >  	SNOR_F_READY_XSR_RDY	= BIT(4),
> >  	SNOR_F_USE_CLSR		= BIT(5),
> >  	SNOR_F_BROKEN_RESET	= BIT(6),
> > +	SNOR_F_4B_OPCODES	= BIT(7)
> >  };
> >  
> >  /**
> >