| Message ID | 20181031144504.19405-2-boris.brezillon@bootlin.com |
|---|---|
| State | Superseded |
| Delegated to: | Boris Brezillon |
| Headers | show |
| Series | [v3,1/2] mtd: spi-nor: Make sure SFDP-based 4B_OPCODE support detection works correctly | expand |
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; >
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/ >
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/ > >
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
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.
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 ;-).
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;