* [PATCH v3 1/2] mtd: spi-nor: Make sure SFDP-based 4B_OPCODE support detection works correctly
@ 2018-10-31 14:45 Boris Brezillon
2018-10-31 14:45 ` [PATCH v3 2/2] mtd: spi-nor: Use 4B opcodes when the NOR advertises both 3B and 4B Boris Brezillon
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Boris Brezillon @ 2018-10-31 14:45 UTC (permalink / raw)
To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
Richard Weinberger, linux-mtd
Cc: Alexandre Belloni, Tudor Ambarus, Cyrille Pitchen
Some flash_info entries have the SPI_NOR_4B_OPCODES flag set 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>
Tested-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
Changes in v3:
- Clear SNOR_F_4B_OPCODES flag when SFDP fails
- Add Alexandre R-b
Changes in v2:
- Fix the commit message
- Fix the ->addr_width check
- Add a comma at the end of the SNOR_F_4B_OPCODES definition
---
drivers/mtd/spi-nor/spi-nor.c | 12 +++++++++---
include/linux/mtd/spi-nor.h | 1 +
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 3e54e31889c7..798915b5c2b0 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;
@@ -3252,6 +3253,7 @@ static int spi_nor_init_params(struct spi_nor *nor,
if (spi_nor_parse_sfdp(nor, &sfdp_params)) {
nor->addr_width = 0;
+ nor->flags &= ~SNOR_F_4B_OPCODES;
/* restore previous erase map */
memcpy(&nor->erase_map, &prev_map,
sizeof(nor->erase_map));
@@ -3554,7 +3556,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
@@ -3588,7 +3590,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);
}
@@ -3746,11 +3748,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 (nor->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..5bc75110d5ea 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.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 2/2] mtd: spi-nor: Use 4B opcodes when the NOR advertises both 3B and 4B
2018-10-31 14:45 [PATCH v3 1/2] mtd: spi-nor: Make sure SFDP-based 4B_OPCODE support detection works correctly Boris Brezillon
@ 2018-10-31 14:45 ` Boris Brezillon
2018-11-09 10:49 ` Tudor.Ambarus
2018-11-08 16:55 ` [PATCH v3 1/2] mtd: spi-nor: Make sure SFDP-based 4B_OPCODE support detection works correctly Tudor.Ambarus
2018-11-09 9:57 ` Tudor.Ambarus
2 siblings, 1 reply; 14+ messages in thread
From: Boris Brezillon @ 2018-10-31 14:45 UTC (permalink / raw)
To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
Richard Weinberger, linux-mtd
Cc: Alexandre Belloni, Tudor Ambarus, Cyrille Pitchen
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>
Tested-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Reviewed-by: Cyrille Pitchen <cyrille.pitchen@wedev4u.fr>
---
Changes in v3:
- Add Alexandre R-b+T-b
- Add Cyrille R-b
Changes in v2
- None
---
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 798915b5c2b0..b20bc4b36f0f 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.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/2] mtd: spi-nor: Make sure SFDP-based 4B_OPCODE support detection works correctly
2018-10-31 14:45 [PATCH v3 1/2] mtd: spi-nor: Make sure SFDP-based 4B_OPCODE support detection works correctly Boris Brezillon
2018-10-31 14:45 ` [PATCH v3 2/2] mtd: spi-nor: Use 4B opcodes when the NOR advertises both 3B and 4B Boris Brezillon
@ 2018-11-08 16:55 ` Tudor.Ambarus
2018-11-08 17:08 ` Boris Brezillon
2018-11-09 9:57 ` Tudor.Ambarus
2 siblings, 1 reply; 14+ messages in thread
From: Tudor.Ambarus @ 2018-11-08 16:55 UTC (permalink / raw)
To: boris.brezillon, dwmw2, computersforpeace, marek.vasut, richard,
linux-mtd
Cc: alexandre.belloni, cyrille.pitchen
On 10/31/2018 04:45 PM, Boris Brezillon wrote:
> Some flash_info entries have the SPI_NOR_4B_OPCODES flag set 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>
> Tested-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
> Changes in v3:
> - Clear SNOR_F_4B_OPCODES flag when SFDP fails
> - Add Alexandre R-b
>
> Changes in v2:
> - Fix the commit message
> - Fix the ->addr_width check
> - Add a comma at the end of the SNOR_F_4B_OPCODES definition
> ---
> drivers/mtd/spi-nor/spi-nor.c | 12 +++++++++---
> include/linux/mtd/spi-nor.h | 1 +
> 2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 3e54e31889c7..798915b5c2b0 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;
>
> @@ -3252,6 +3253,7 @@ static int spi_nor_init_params(struct spi_nor *nor,
>
> if (spi_nor_parse_sfdp(nor, &sfdp_params)) {
> nor->addr_width = 0;
> + nor->flags &= ~SNOR_F_4B_OPCODES;
> /* restore previous erase map */
> memcpy(&nor->erase_map, &prev_map,
> sizeof(nor->erase_map));
> @@ -3554,7 +3556,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
> @@ -3588,7 +3590,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);
> }
> @@ -3746,11 +3748,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 (nor->addr_width == 4 &&
Shouldn't SNOR_F_4B_OPCODES already imply nor->addr_width == 4?
When SNOR_F_4B_OPCODES comes from bfpt, addr_width is set to 4. For the id-based
caps detection, when mtd->size > 0x1000000, we set nor->addr_width = 4 too.
The only uncovered case would be when
} else if (info->addr_width) {
nor->addr_width = info->addr_width;
but this can be solved by reordering the else if cases.
if (nor->addr_width) {
/* already configured from SFDP */
} else if (mtd->size > 0x1000000) {
...
} else if (info->addr_width) {
nor->addr_width = info->addr_width;
} else {
nor->addr_width = 3;
}
What do you think?
> + 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..5bc75110d5ea 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),
> };
>
> /**
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/2] mtd: spi-nor: Make sure SFDP-based 4B_OPCODE support detection works correctly
2018-11-08 16:55 ` [PATCH v3 1/2] mtd: spi-nor: Make sure SFDP-based 4B_OPCODE support detection works correctly Tudor.Ambarus
@ 2018-11-08 17:08 ` Boris Brezillon
2018-11-09 9:57 ` Tudor.Ambarus
0 siblings, 1 reply; 14+ messages in thread
From: Boris Brezillon @ 2018-11-08 17:08 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: dwmw2, computersforpeace, marek.vasut, richard, linux-mtd,
alexandre.belloni, cyrille.pitchen
On Thu, 8 Nov 2018 16:55:29 +0000
<Tudor.Ambarus@microchip.com> wrote:
> On 10/31/2018 04:45 PM, Boris Brezillon wrote:
> > Some flash_info entries have the SPI_NOR_4B_OPCODES flag set 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>
> > Tested-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> > ---
> > Changes in v3:
> > - Clear SNOR_F_4B_OPCODES flag when SFDP fails
> > - Add Alexandre R-b
> >
> > Changes in v2:
> > - Fix the commit message
> > - Fix the ->addr_width check
> > - Add a comma at the end of the SNOR_F_4B_OPCODES definition
> > ---
> > drivers/mtd/spi-nor/spi-nor.c | 12 +++++++++---
> > include/linux/mtd/spi-nor.h | 1 +
> > 2 files changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> > index 3e54e31889c7..798915b5c2b0 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;
> >
> > @@ -3252,6 +3253,7 @@ static int spi_nor_init_params(struct spi_nor *nor,
> >
> > if (spi_nor_parse_sfdp(nor, &sfdp_params)) {
> > nor->addr_width = 0;
> > + nor->flags &= ~SNOR_F_4B_OPCODES;
> > /* restore previous erase map */
> > memcpy(&nor->erase_map, &prev_map,
> > sizeof(nor->erase_map));
> > @@ -3554,7 +3556,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
> > @@ -3588,7 +3590,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);
> > }
> > @@ -3746,11 +3748,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 (nor->addr_width == 4 &&
>
> Shouldn't SNOR_F_4B_OPCODES already imply nor->addr_width == 4?
Can't we have NORs supporting 4B opcodes but are less than 16MB? In
this case SNOR_F_4B_OPCODES would be set and ->addr_width would be 3.
> When SNOR_F_4B_OPCODES comes from bfpt, addr_width is set to 4. For the id-based
> caps detection, when mtd->size > 0x1000000, we set nor->addr_width = 4 too.
>
> The only uncovered case would be when
> } else if (info->addr_width) {
> nor->addr_width = info->addr_width;
>
> but this can be solved by reordering the else if cases.
>
> if (nor->addr_width) {
> /* already configured from SFDP */
> } else if (mtd->size > 0x1000000) {
> ...
> } else if (info->addr_width) {
> nor->addr_width = info->addr_width;
> } else {
> nor->addr_width = 3;
> }
>
> What do you think?
I'd rather not change that in this patch, but feel free to propose
a patch on top of mine to simplify the logic if you think it's
needed.
>
> > + 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..5bc75110d5ea 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),
> > };
> >
> > /**
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/2] mtd: spi-nor: Make sure SFDP-based 4B_OPCODE support detection works correctly
2018-11-08 17:08 ` Boris Brezillon
@ 2018-11-09 9:57 ` Tudor.Ambarus
2018-11-09 10:03 ` Boris Brezillon
2018-11-09 10:15 ` Boris Brezillon
0 siblings, 2 replies; 14+ messages in thread
From: Tudor.Ambarus @ 2018-11-09 9:57 UTC (permalink / raw)
To: boris.brezillon
Cc: dwmw2, computersforpeace, marek.vasut, richard, linux-mtd,
alexandre.belloni, cyrille.pitchen
On 11/08/2018 07:08 PM, Boris Brezillon wrote:
> On Thu, 8 Nov 2018 16:55:29 +0000
> <Tudor.Ambarus@microchip.com> wrote:
>
>> On 10/31/2018 04:45 PM, Boris Brezillon wrote:
>>> Some flash_info entries have the SPI_NOR_4B_OPCODES flag set 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>
>>> Tested-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>>> ---
>>> Changes in v3:
>>> - Clear SNOR_F_4B_OPCODES flag when SFDP fails
>>> - Add Alexandre R-b
>>>
>>> Changes in v2:
>>> - Fix the commit message
>>> - Fix the ->addr_width check
>>> - Add a comma at the end of the SNOR_F_4B_OPCODES definition
>>> ---
>>> drivers/mtd/spi-nor/spi-nor.c | 12 +++++++++---
>>> include/linux/mtd/spi-nor.h | 1 +
>>> 2 files changed, 10 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>>> index 3e54e31889c7..798915b5c2b0 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;
>>>
>>> @@ -3252,6 +3253,7 @@ static int spi_nor_init_params(struct spi_nor *nor,
>>>
>>> if (spi_nor_parse_sfdp(nor, &sfdp_params)) {
>>> nor->addr_width = 0;
>>> + nor->flags &= ~SNOR_F_4B_OPCODES;
>>> /* restore previous erase map */
>>> memcpy(&nor->erase_map, &prev_map,
>>> sizeof(nor->erase_map));
>>> @@ -3554,7 +3556,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
>>> @@ -3588,7 +3590,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);
>>> }
>>> @@ -3746,11 +3748,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 (nor->addr_width == 4 &&
>>
>> Shouldn't SNOR_F_4B_OPCODES already imply nor->addr_width == 4?
>
> Can't we have NORs supporting 4B opcodes but are less than 16MB? In
> this case SNOR_F_4B_OPCODES would be set and ->addr_width would be 3.
I guess not, it wouldn't make sense, but who knows ... :)
The 4-byte opcodes indicate that 4-bytes of address follow the instruction. And
4-byte addresses make sense for spi memories that exceed the 16MB density.
>
>> When SNOR_F_4B_OPCODES comes from bfpt, addr_width is set to 4. For the id-based
>> caps detection, when mtd->size > 0x1000000, we set nor->addr_width = 4 too.
>>
>> The only uncovered case would be when
>> } else if (info->addr_width) {
>> nor->addr_width = info->addr_width;
>>
>> but this can be solved by reordering the else if cases.
>>
>> if (nor->addr_width) {
>> /* already configured from SFDP */
>> } else if (mtd->size > 0x1000000) {
>> ...
>> } else if (info->addr_width) {
>> nor->addr_width = info->addr_width;
>> } else {
>> nor->addr_width = 3;
>> }
>>
>> What do you think?
>
> I'd rather not change that in this patch, but feel free to propose
> a patch on top of mine to simplify the logic if you think it's
> needed.
>
yeah, it can be made in a separate patch.
Cheers,
ta
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/2] mtd: spi-nor: Make sure SFDP-based 4B_OPCODE support detection works correctly
2018-10-31 14:45 [PATCH v3 1/2] mtd: spi-nor: Make sure SFDP-based 4B_OPCODE support detection works correctly Boris Brezillon
2018-10-31 14:45 ` [PATCH v3 2/2] mtd: spi-nor: Use 4B opcodes when the NOR advertises both 3B and 4B Boris Brezillon
2018-11-08 16:55 ` [PATCH v3 1/2] mtd: spi-nor: Make sure SFDP-based 4B_OPCODE support detection works correctly Tudor.Ambarus
@ 2018-11-09 9:57 ` Tudor.Ambarus
2 siblings, 0 replies; 14+ messages in thread
From: Tudor.Ambarus @ 2018-11-09 9:57 UTC (permalink / raw)
To: boris.brezillon, dwmw2, computersforpeace, marek.vasut, richard,
linux-mtd
Cc: alexandre.belloni, cyrille.pitchen
On 10/31/2018 04:45 PM, Boris Brezillon wrote:
> Some flash_info entries have the SPI_NOR_4B_OPCODES flag set 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>
> Tested-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
> Changes in v3:
> - Clear SNOR_F_4B_OPCODES flag when SFDP fails
> - Add Alexandre R-b
>
> Changes in v2:
> - Fix the commit message
> - Fix the ->addr_width check
> - Add a comma at the end of the SNOR_F_4B_OPCODES definition
> ---
> drivers/mtd/spi-nor/spi-nor.c | 12 +++++++++---
> include/linux/mtd/spi-nor.h | 1 +
> 2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 3e54e31889c7..798915b5c2b0 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;
>
> @@ -3252,6 +3253,7 @@ static int spi_nor_init_params(struct spi_nor *nor,
>
> if (spi_nor_parse_sfdp(nor, &sfdp_params)) {
> nor->addr_width = 0;
> + nor->flags &= ~SNOR_F_4B_OPCODES;
> /* restore previous erase map */
> memcpy(&nor->erase_map, &prev_map,
> sizeof(nor->erase_map));
> @@ -3554,7 +3556,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
> @@ -3588,7 +3590,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);
> }
> @@ -3746,11 +3748,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 (nor->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..5bc75110d5ea 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),
> };
>
> /**
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/2] mtd: spi-nor: Make sure SFDP-based 4B_OPCODE support detection works correctly
2018-11-09 9:57 ` Tudor.Ambarus
@ 2018-11-09 10:03 ` Boris Brezillon
2018-11-09 10:15 ` Boris Brezillon
1 sibling, 0 replies; 14+ messages in thread
From: Boris Brezillon @ 2018-11-09 10:03 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: dwmw2, computersforpeace, marek.vasut, richard, linux-mtd,
alexandre.belloni, cyrille.pitchen
On Fri, 9 Nov 2018 09:57:13 +0000
<Tudor.Ambarus@microchip.com> wrote:
> On 11/08/2018 07:08 PM, Boris Brezillon wrote:
> > On Thu, 8 Nov 2018 16:55:29 +0000
> > <Tudor.Ambarus@microchip.com> wrote:
> >
> >> On 10/31/2018 04:45 PM, Boris Brezillon wrote:
> >>> Some flash_info entries have the SPI_NOR_4B_OPCODES flag set 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>
> >>> Tested-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> >>> ---
> >>> Changes in v3:
> >>> - Clear SNOR_F_4B_OPCODES flag when SFDP fails
> >>> - Add Alexandre R-b
> >>>
> >>> Changes in v2:
> >>> - Fix the commit message
> >>> - Fix the ->addr_width check
> >>> - Add a comma at the end of the SNOR_F_4B_OPCODES definition
> >>> ---
> >>> drivers/mtd/spi-nor/spi-nor.c | 12 +++++++++---
> >>> include/linux/mtd/spi-nor.h | 1 +
> >>> 2 files changed, 10 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> >>> index 3e54e31889c7..798915b5c2b0 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;
> >>>
> >>> @@ -3252,6 +3253,7 @@ static int spi_nor_init_params(struct spi_nor *nor,
> >>>
> >>> if (spi_nor_parse_sfdp(nor, &sfdp_params)) {
> >>> nor->addr_width = 0;
> >>> + nor->flags &= ~SNOR_F_4B_OPCODES;
> >>> /* restore previous erase map */
> >>> memcpy(&nor->erase_map, &prev_map,
> >>> sizeof(nor->erase_map));
> >>> @@ -3554,7 +3556,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
> >>> @@ -3588,7 +3590,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);
> >>> }
> >>> @@ -3746,11 +3748,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 (nor->addr_width == 4 &&
> >>
> >> Shouldn't SNOR_F_4B_OPCODES already imply nor->addr_width == 4?
> >
> > Can't we have NORs supporting 4B opcodes but are less than 16MB? In
> > this case SNOR_F_4B_OPCODES would be set and ->addr_width would be 3.
>
> I guess not, it wouldn't make sense, but who knows ... :)
Don't under-estimate flash manufacturers :-).
>
> The 4-byte opcodes indicate that 4-bytes of address follow the instruction. And
> 4-byte addresses make sense for spi memories that exceed the 16MB density.
I'm pretty sure they use the same controller for <=16MB and >16MB
chips, so I wouldn't be surprised if some of their modern <=16MB chips
support 4B opcodes even if it's not needed.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/2] mtd: spi-nor: Make sure SFDP-based 4B_OPCODE support detection works correctly
2018-11-09 9:57 ` Tudor.Ambarus
2018-11-09 10:03 ` Boris Brezillon
@ 2018-11-09 10:15 ` Boris Brezillon
1 sibling, 0 replies; 14+ messages in thread
From: Boris Brezillon @ 2018-11-09 10:15 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: dwmw2, computersforpeace, marek.vasut, richard, linux-mtd,
alexandre.belloni, cyrille.pitchen
On Fri, 9 Nov 2018 09:57:13 +0000
<Tudor.Ambarus@microchip.com> wrote:
> >> When SNOR_F_4B_OPCODES comes from bfpt, addr_width is set to 4. For the id-based
> >> caps detection, when mtd->size > 0x1000000, we set nor->addr_width = 4 too.
> >>
> >> The only uncovered case would be when
> >> } else if (info->addr_width) {
> >> nor->addr_width = info->addr_width;
> >>
> >> but this can be solved by reordering the else if cases.
> >>
> >> if (nor->addr_width) {
> >> /* already configured from SFDP */
> >> } else if (mtd->size > 0x1000000) {
> >> ...
> >> } else if (info->addr_width) {
> >> nor->addr_width = info->addr_width;
> >> } else {
> >> nor->addr_width = 3;
> >> }
> >>
> >> What do you think?
> >
> > I'd rather not change that in this patch, but feel free to propose
> > a patch on top of mine to simplify the logic if you think it's
> > needed.
> >
>
> yeah, it can be made in a separate patch.
If you're okay with the current version, can you add a R-b?
Thanks,
Boris
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/2] mtd: spi-nor: Use 4B opcodes when the NOR advertises both 3B and 4B
2018-10-31 14:45 ` [PATCH v3 2/2] mtd: spi-nor: Use 4B opcodes when the NOR advertises both 3B and 4B Boris Brezillon
@ 2018-11-09 10:49 ` Tudor.Ambarus
2018-11-16 11:57 ` Tudor.Ambarus
0 siblings, 1 reply; 14+ messages in thread
From: Tudor.Ambarus @ 2018-11-09 10:49 UTC (permalink / raw)
To: boris.brezillon, dwmw2, computersforpeace, marek.vasut, richard,
linux-mtd
Cc: alexandre.belloni, cyrille.pitchen
On 10/31/2018 04:45 PM, Boris Brezillon wrote:
> 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>
> Tested-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Reviewed-by: Cyrille Pitchen <cyrille.pitchen@wedev4u.fr>
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
> Changes in v3:
> - Add Alexandre R-b+T-b
> - Add Cyrille R-b
>
> Changes in v2
> - None
> ---
> 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 798915b5c2b0..b20bc4b36f0f 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;
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/2] mtd: spi-nor: Use 4B opcodes when the NOR advertises both 3B and 4B
2018-11-09 10:49 ` Tudor.Ambarus
@ 2018-11-16 11:57 ` Tudor.Ambarus
2018-11-16 13:42 ` Alexandre Belloni
2018-11-21 12:57 ` Boris Brezillon
0 siblings, 2 replies; 14+ messages in thread
From: Tudor.Ambarus @ 2018-11-16 11:57 UTC (permalink / raw)
To: boris.brezillon, dwmw2, computersforpeace, marek.vasut, richard,
linux-mtd
Cc: alexandre.belloni, cyrille.pitchen
On 11/09/2018 12:49 PM, Tudor.Ambarus@microchip.com wrote:
>
>
> On 10/31/2018 04:45 PM, Boris Brezillon wrote:
>> 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>
>> Tested-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>> Reviewed-by: Cyrille Pitchen <cyrille.pitchen@wedev4u.fr>
>
> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>
I propose to stall this patch for a week or so, until we will have a clearer
view on how are defined the flashes that don't have 4B opcodes, but can enter
the 4-Byte mode on command.
>> ---
>> Changes in v3:
>> - Add Alexandre R-b+T-b
>> - Add Cyrille R-b
>>
>> Changes in v2
>> - None
>> ---
>> 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 798915b5c2b0..b20bc4b36f0f 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;
Maybe we will also want to check that BFPT DWORD16[31:24] has value xx1x_xxxxb -
it indicates that 4B opcodes are supported.
Cheers,
ta
>> nor->addr_width = 4;
>> break;
>>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/2] mtd: spi-nor: Use 4B opcodes when the NOR advertises both 3B and 4B
2018-11-16 11:57 ` Tudor.Ambarus
@ 2018-11-16 13:42 ` Alexandre Belloni
2018-11-20 13:18 ` Tudor.Ambarus
2018-11-21 12:57 ` Boris Brezillon
1 sibling, 1 reply; 14+ messages in thread
From: Alexandre Belloni @ 2018-11-16 13:42 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: boris.brezillon, dwmw2, computersforpeace, marek.vasut, richard,
linux-mtd, cyrille.pitchen
On 16/11/2018 11:57:10+0000, Tudor.Ambarus@microchip.com wrote:
>
>
> On 11/09/2018 12:49 PM, Tudor.Ambarus@microchip.com wrote:
> >
> >
> > On 10/31/2018 04:45 PM, Boris Brezillon wrote:
> >> 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>
> >> Tested-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> >> Reviewed-by: Cyrille Pitchen <cyrille.pitchen@wedev4u.fr>
> >
> > Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> >
>
> I propose to stall this patch for a week or so, until we will have a clearer
> view on how are defined the flashes that don't have 4B opcodes, but can enter
> the 4-Byte mode on command.
>
Note that this patch is badly needed for some of our boards. without it,
they can't reboot properly. I would very much like to see it enter
upstream and be backported sooner than later.
> >> ---
> >> Changes in v3:
> >> - Add Alexandre R-b+T-b
> >> - Add Cyrille R-b
> >>
> >> Changes in v2
> >> - None
> >> ---
> >> 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 798915b5c2b0..b20bc4b36f0f 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;
>
>
> Maybe we will also want to check that BFPT DWORD16[31:24] has value xx1x_xxxxb -
> it indicates that 4B opcodes are supported.
>
> Cheers,
> ta
>
> >> nor->addr_width = 4;
> >> break;
> >>
> > ______________________________________________________
> > Linux MTD discussion mailing list
> > http://lists.infradead.org/mailman/listinfo/linux-mtd/
> >
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/2] mtd: spi-nor: Use 4B opcodes when the NOR advertises both 3B and 4B
2018-11-16 13:42 ` Alexandre Belloni
@ 2018-11-20 13:18 ` Tudor.Ambarus
2018-11-20 15:01 ` Alexandre Belloni
0 siblings, 1 reply; 14+ messages in thread
From: Tudor.Ambarus @ 2018-11-20 13:18 UTC (permalink / raw)
To: alexandre.belloni
Cc: marek.vasut, richard, boris.brezillon, linux-mtd,
cyrille.pitchen, computersforpeace, dwmw2
Hi, Alexandre,
On 11/16/2018 03:42 PM, Alexandre Belloni wrote:
> On 16/11/2018 11:57:10+0000, Tudor.Ambarus@microchip.com wrote:
>>
>>
>> On 11/09/2018 12:49 PM, Tudor.Ambarus@microchip.com wrote:
>>>
>>>
>>> On 10/31/2018 04:45 PM, Boris Brezillon wrote:
>>>> 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>
>>>> Tested-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>>>> Reviewed-by: Cyrille Pitchen <cyrille.pitchen@wedev4u.fr>
>>>
>>> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>>>
>>
>> I propose to stall this patch for a week or so, until we will have a clearer
>> view on how are defined the flashes that don't have 4B opcodes, but can enter
>> the 4-Byte mode on command.
>>
>
> Note that this patch is badly needed for some of our boards. without it,
> they can't reboot properly. I would very much like to see it enter
> upstream and be backported sooner than later.
What flash do the boards use? Does your flash support SFDP 4-Byte Instruction
table? If yes, the following patch should solve your problem indirectly:
https://lore.kernel.org/patchwork/patch/1015036/
You should also apply the following if you want to test it:
https://lore.kernel.org/patchwork/patch/1013294/
Cheers,
ta
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/2] mtd: spi-nor: Use 4B opcodes when the NOR advertises both 3B and 4B
2018-11-20 13:18 ` Tudor.Ambarus
@ 2018-11-20 15:01 ` Alexandre Belloni
0 siblings, 0 replies; 14+ messages in thread
From: Alexandre Belloni @ 2018-11-20 15:01 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: marek.vasut, richard, boris.brezillon, linux-mtd,
cyrille.pitchen, computersforpeace, dwmw2
On 20/11/2018 13:18:49+0000, Tudor.Ambarus@microchip.com wrote:
> Hi, Alexandre,
>
> On 11/16/2018 03:42 PM, Alexandre Belloni wrote:
> > On 16/11/2018 11:57:10+0000, Tudor.Ambarus@microchip.com wrote:
> >>
> >>
> >> On 11/09/2018 12:49 PM, Tudor.Ambarus@microchip.com wrote:
> >>>
> >>>
> >>> On 10/31/2018 04:45 PM, Boris Brezillon wrote:
> >>>> 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>
> >>>> Tested-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> >>>> Reviewed-by: Cyrille Pitchen <cyrille.pitchen@wedev4u.fr>
> >>>
> >>> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> >>>
> >>
> >> I propose to stall this patch for a week or so, until we will have a clearer
> >> view on how are defined the flashes that don't have 4B opcodes, but can enter
> >> the 4-Byte mode on command.
> >>
> >
> > Note that this patch is badly needed for some of our boards. without it,
> > they can't reboot properly. I would very much like to see it enter
> > upstream and be backported sooner than later.
>
> What flash do the boards use? Does your flash support SFDP 4-Byte Instruction
> table? If yes, the following patch should solve your problem indirectly:
>
It is a MX25L25635FMI-10G
> https://lore.kernel.org/patchwork/patch/1015036/
>
> You should also apply the following if you want to test it:
>
> https://lore.kernel.org/patchwork/patch/1013294/
>
Applying those two patches only doesn't fix the reboot issue.
spi_nor_parse_4bait() is never called.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/2] mtd: spi-nor: Use 4B opcodes when the NOR advertises both 3B and 4B
2018-11-16 11:57 ` Tudor.Ambarus
2018-11-16 13:42 ` Alexandre Belloni
@ 2018-11-21 12:57 ` Boris Brezillon
1 sibling, 0 replies; 14+ messages in thread
From: Boris Brezillon @ 2018-11-21 12:57 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: dwmw2, computersforpeace, marek.vasut, richard, linux-mtd,
alexandre.belloni, cyrille.pitchen
On Fri, 16 Nov 2018 11:57:10 +0000
<Tudor.Ambarus@microchip.com> wrote:
> On 11/09/2018 12:49 PM, Tudor.Ambarus@microchip.com wrote:
> >
> >
> > On 10/31/2018 04:45 PM, Boris Brezillon wrote:
> >> 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>
> >> Tested-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> >> Reviewed-by: Cyrille Pitchen <cyrille.pitchen@wedev4u.fr>
> >
> > Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> >
>
> I propose to stall this patch for a week or so, until we will have a clearer
> view on how are defined the flashes that don't have 4B opcodes, but can enter
> the 4-Byte mode on command.
>
> >> ---
> >> Changes in v3:
> >> - Add Alexandre R-b+T-b
> >> - Add Cyrille R-b
> >>
> >> Changes in v2
> >> - None
> >> ---
> >> 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 798915b5c2b0..b20bc4b36f0f 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;
>
>
> Maybe we will also want to check that BFPT DWORD16[31:24] has value xx1x_xxxxb -
> it indicates that 4B opcodes are supported.
Alexandre tested that, and it seems DWORD16 is not properly populated.
We also considered setting the SPI_NOR_4B_OPCODES flag to the flash_id
entry, but Macronix use the same device ID for both MX25L25635E and
MX25L25635F, and only the F revision supports 4B opcodes.
Not the first time this sort of things happen, so I guess we have one
more reason to add the ->fixup() we talked about multiple times ;-).
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-11-21 12:57 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-31 14:45 [PATCH v3 1/2] mtd: spi-nor: Make sure SFDP-based 4B_OPCODE support detection works correctly Boris Brezillon
2018-10-31 14:45 ` [PATCH v3 2/2] mtd: spi-nor: Use 4B opcodes when the NOR advertises both 3B and 4B Boris Brezillon
2018-11-09 10:49 ` Tudor.Ambarus
2018-11-16 11:57 ` Tudor.Ambarus
2018-11-16 13:42 ` Alexandre Belloni
2018-11-20 13:18 ` Tudor.Ambarus
2018-11-20 15:01 ` Alexandre Belloni
2018-11-21 12:57 ` Boris Brezillon
2018-11-08 16:55 ` [PATCH v3 1/2] mtd: spi-nor: Make sure SFDP-based 4B_OPCODE support detection works correctly Tudor.Ambarus
2018-11-08 17:08 ` Boris Brezillon
2018-11-09 9:57 ` Tudor.Ambarus
2018-11-09 10:03 ` Boris Brezillon
2018-11-09 10:15 ` Boris Brezillon
2018-11-09 9:57 ` Tudor.Ambarus
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.