| Message ID | 20190924055617.20397-1-vigneshr@ti.com |
|---|---|
| Headers | show |
| Series | spi-nor: spi-nor-ids: Fix 4 Byte addressing for n25q256 and n25q512* | expand |
Hi Vignesh, On Tue, Sep 24, 2019 at 7:55 AM Vignesh Raghavendra <vigneshr@ti.com> wrote: > > This series removes SPI_NOR_4B_OPCODES flags from legacy variants of > n25q256* and n25q512* and adds entries for newer variants of those > flashes that support 4 Byte opcodes. > > I don't have the flash devices. So its only compile tested. > > Ashish, Simon, > > I would greatly appreciate if you could test these patches and make sure > 4 Byte opcodes are being used. (Probably by enabling/adding prints to > cmd->opcode in spi_mem_exec_op() in drivers/spi/spi-mem.c As written in my last mail, I currently cannot get SFDP to work on my board: I keep getting 0xffdddfdf instead of 0x50444653 (SFDP_SIGNATURE). Any idea for a reason of that? That device I have here seems to be equipped with an MT25QL256ABA, but that should not be an issue, I think. Regards, Simon > > Euginey, > > Could you test this series on top of latest u-boot master and confirm > that your test cases still work? > > Regards > Vignesh > > Vignesh Raghavendra (3): > spi-nor: spi-nor-ids: Disable SPI_NOR_4B_OPCODES for n25q512* and > n25q256* > spi-nor: spi-nor-ids: Rename mt25qu512a entry > spi-nor: spi-nor-ids: Add entries for newer variants of n25q256* and > n25q512* > > drivers/mtd/spi/spi-nor-ids.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > -- > 2.23.0 >
Hi Simon, On 24/09/19 12:32 PM, Simon Goldschmidt wrote: > Hi Vignesh, > > On Tue, Sep 24, 2019 at 7:55 AM Vignesh Raghavendra <vigneshr@ti.com> wrote: >> >> This series removes SPI_NOR_4B_OPCODES flags from legacy variants of >> n25q256* and n25q512* and adds entries for newer variants of those >> flashes that support 4 Byte opcodes. >> >> I don't have the flash devices. So its only compile tested. >> >> Ashish, Simon, >> >> I would greatly appreciate if you could test these patches and make sure >> 4 Byte opcodes are being used. (Probably by enabling/adding prints to >> cmd->opcode in spi_mem_exec_op() in drivers/spi/spi-mem.c > > As written in my last mail, I currently cannot get SFDP to work on my board: > I keep getting 0xffdddfdf instead of 0x50444653 (SFDP_SIGNATURE). > Did it ever work on your board ie did you test it when you were testing my patches porting SPI NOR framework from kernel? What's the controller driver? Cadence QSPI? Regards Vignesh > Any idea for a reason of that? That device I have here seems to be equipped > with an MT25QL256ABA, but that should not be an issue, I think. > > Regards, > Simon > >> >> Euginey, >> >> Could you test this series on top of latest u-boot master and confirm >> that your test cases still work? >> >> Regards >> Vignesh >> >> Vignesh Raghavendra (3): >> spi-nor: spi-nor-ids: Disable SPI_NOR_4B_OPCODES for n25q512* and >> n25q256* >> spi-nor: spi-nor-ids: Rename mt25qu512a entry >> spi-nor: spi-nor-ids: Add entries for newer variants of n25q256* and >> n25q512* >> >> drivers/mtd/spi/spi-nor-ids.c | 13 ++++++++----- >> 1 file changed, 8 insertions(+), 5 deletions(-) >> >> -- >> 2.23.0 >>
HI Vignesh, On Tue, Sep 24, 2019 at 9:59 AM Vignesh Raghavendra <vigneshr@ti.com> wrote: > > Hi Simon, > > On 24/09/19 12:32 PM, Simon Goldschmidt wrote: > > Hi Vignesh, > > > > On Tue, Sep 24, 2019 at 7:55 AM Vignesh Raghavendra <vigneshr@ti.com> wrote: > >> > >> This series removes SPI_NOR_4B_OPCODES flags from legacy variants of > >> n25q256* and n25q512* and adds entries for newer variants of those > >> flashes that support 4 Byte opcodes. > >> > >> I don't have the flash devices. So its only compile tested. > >> > >> Ashish, Simon, > >> > >> I would greatly appreciate if you could test these patches and make sure > >> 4 Byte opcodes are being used. (Probably by enabling/adding prints to > >> cmd->opcode in spi_mem_exec_op() in drivers/spi/spi-mem.c > > > > As written in my last mail, I currently cannot get SFDP to work on my board: > > I keep getting 0xffdddfdf instead of 0x50444653 (SFDP_SIGNATURE). > > > > Did it ever work on your board ie did you test it when you were testing > my patches porting SPI NOR framework from kernel? I tested that series on the EBV Socrates (socfpga_socrates), while this is our custom board. It is similar, but the flash chips are different. So unfortunately, I don't know if it ever worked... I'll try to run this series on socfpga_socrates soon. > > What's the controller driver? Cadence QSPI? Yes. Regards, Simon > > Regards > Vignesh > > > Any idea for a reason of that? That device I have here seems to be equipped > > with an MT25QL256ABA, but that should not be an issue, I think. > > > > Regards, > > Simon > > > >> > >> Euginey, > >> > >> Could you test this series on top of latest u-boot master and confirm > >> that your test cases still work? > >> > >> Regards > >> Vignesh > >> > >> Vignesh Raghavendra (3): > >> spi-nor: spi-nor-ids: Disable SPI_NOR_4B_OPCODES for n25q512* and > >> n25q256* > >> spi-nor: spi-nor-ids: Rename mt25qu512a entry > >> spi-nor: spi-nor-ids: Add entries for newer variants of n25q256* and > >> n25q512* > >> > >> drivers/mtd/spi/spi-nor-ids.c | 13 ++++++++----- > >> 1 file changed, 8 insertions(+), 5 deletions(-) > >> > >> -- > >> 2.23.0 > >> > > -- > Regards > Vignesh
On 24/09/19 2:13 PM, Simon Goldschmidt wrote: > HI Vignesh, > > On Tue, Sep 24, 2019 at 9:59 AM Vignesh Raghavendra <vigneshr@ti.com> wrote: >> >> Hi Simon, >> >> On 24/09/19 12:32 PM, Simon Goldschmidt wrote: >>> Hi Vignesh, >>> >>> On Tue, Sep 24, 2019 at 7:55 AM Vignesh Raghavendra <vigneshr@ti.com> wrote: >>>> >>>> This series removes SPI_NOR_4B_OPCODES flags from legacy variants of >>>> n25q256* and n25q512* and adds entries for newer variants of those >>>> flashes that support 4 Byte opcodes. >>>> >>>> I don't have the flash devices. So its only compile tested. >>>> >>>> Ashish, Simon, >>>> >>>> I would greatly appreciate if you could test these patches and make sure >>>> 4 Byte opcodes are being used. (Probably by enabling/adding prints to >>>> cmd->opcode in spi_mem_exec_op() in drivers/spi/spi-mem.c >>> >>> As written in my last mail, I currently cannot get SFDP to work on my board: >>> I keep getting 0xffdddfdf instead of 0x50444653 (SFDP_SIGNATURE). >>> >> >> Did it ever work on your board ie did you test it when you were testing >> my patches porting SPI NOR framework from kernel? > > I tested that series on the EBV Socrates (socfpga_socrates), while this is our > custom board. It is similar, but the flash chips are different. So > unfortunately, I don't know if it ever worked... > > I'll try to run this series on socfpga_socrates soon. > >> >> What's the controller driver? Cadence QSPI? > > Yes. What does the DT snippet for flash node looks like? I think cadence QSPI is broken for SFDP if it has: spi-rx-bus-width = <4>; Driver does: if (rx_width & SPI_RX_QUAD) /* Instruction and address at DQ0, data at DQ0-3. */ rd_reg |= CQSPI_INST_TYPE_QUAD << CQSPI_REG_RD_INSTR_TYPE_DATA_LSB; So, driver tries to execute SFDP command in 1-1-4 mode which is wrong. For now try setting spi-rx-bus-width to 1 and see if SFDP works (of course this not the fix) I have patches[1] to convert this driver to implement spi-mem framework which should help in fixing this issues. Will post them probably at the start of merge window after some more testing. If you have time, please give it a try [1] https://github.com/r-vignesh/u-boot.git branch: cqspi Regards Vignesh > > Regards, > Simon > >> >> Regards >> Vignesh >> >>> Any idea for a reason of that? That device I have here seems to be equipped >>> with an MT25QL256ABA, but that should not be an issue, I think. >>> >>> Regards, >>> Simon >>> >>>> >>>> Euginey, >>>> >>>> Could you test this series on top of latest u-boot master and confirm >>>> that your test cases still work? >>>> >>>> Regards >>>> Vignesh >>>> >>>> Vignesh Raghavendra (3): >>>> spi-nor: spi-nor-ids: Disable SPI_NOR_4B_OPCODES for n25q512* and >>>> n25q256* >>>> spi-nor: spi-nor-ids: Rename mt25qu512a entry >>>> spi-nor: spi-nor-ids: Add entries for newer variants of n25q256* and >>>> n25q512* >>>> >>>> drivers/mtd/spi/spi-nor-ids.c | 13 ++++++++----- >>>> 1 file changed, 8 insertions(+), 5 deletions(-) >>>> >>>> -- >>>> 2.23.0 >>>> >> >> -- >> Regards >> Vignesh
Hi Vignesh, On Tue, Sep 24, 2019 at 11:17 AM Vignesh Raghavendra <vigneshr@ti.com> wrote: > > > > On 24/09/19 2:13 PM, Simon Goldschmidt wrote: > > HI Vignesh, > > > > On Tue, Sep 24, 2019 at 9:59 AM Vignesh Raghavendra <vigneshr@ti.com> wrote: > >> > >> Hi Simon, > >> > >> On 24/09/19 12:32 PM, Simon Goldschmidt wrote: > >>> Hi Vignesh, > >>> > >>> On Tue, Sep 24, 2019 at 7:55 AM Vignesh Raghavendra <vigneshr@ti.com> wrote: > >>>> > >>>> This series removes SPI_NOR_4B_OPCODES flags from legacy variants of > >>>> n25q256* and n25q512* and adds entries for newer variants of those > >>>> flashes that support 4 Byte opcodes. > >>>> > >>>> I don't have the flash devices. So its only compile tested. > >>>> > >>>> Ashish, Simon, > >>>> > >>>> I would greatly appreciate if you could test these patches and make sure > >>>> 4 Byte opcodes are being used. (Probably by enabling/adding prints to > >>>> cmd->opcode in spi_mem_exec_op() in drivers/spi/spi-mem.c > >>> > >>> As written in my last mail, I currently cannot get SFDP to work on my board: > >>> I keep getting 0xffdddfdf instead of 0x50444653 (SFDP_SIGNATURE). > >>> > >> > >> Did it ever work on your board ie did you test it when you were testing > >> my patches porting SPI NOR framework from kernel? > > > > I tested that series on the EBV Socrates (socfpga_socrates), while this is our > > custom board. It is similar, but the flash chips are different. So > > unfortunately, I don't know if it ever worked... > > > > I'll try to run this series on socfpga_socrates soon. > > > >> > >> What's the controller driver? Cadence QSPI? > > > > Yes. > > What does the DT snippet for flash node looks like? > I think cadence QSPI is broken for SFDP if it has: > > spi-rx-bus-width = <4>; > > Driver does: > > if (rx_width & SPI_RX_QUAD) > /* Instruction and address at DQ0, data at DQ0-3. */ > rd_reg |= CQSPI_INST_TYPE_QUAD << CQSPI_REG_RD_INSTR_TYPE_DATA_LSB; > > So, driver tries to execute SFDP command in 1-1-4 mode which is wrong. > For now try setting spi-rx-bus-width to 1 and see if SFDP works (of course this not the fix) Right. That makes it work, thanks. I'll send the SFDP tables dump in the other thread. Plus I can now test this series :-) > > I have patches[1] to convert this driver to implement spi-mem framework which should help in fixing this issues. > Will post them probably at the start of merge window after some more testing. If you have time, please give it a try > > [1] https://github.com/r-vignesh/u-boot.git branch: cqspi Oh, cool. That was on my list for some time now, but I haven't gotten around to even starting it. Regards, Simon > > Regards > Vignesh > > > > > > Regards, > > Simon > > > >> > >> Regards > >> Vignesh > >> > >>> Any idea for a reason of that? That device I have here seems to be equipped > >>> with an MT25QL256ABA, but that should not be an issue, I think. > >>> > >>> Regards, > >>> Simon > >>> > >>>> > >>>> Euginey, > >>>> > >>>> Could you test this series on top of latest u-boot master and confirm > >>>> that your test cases still work? > >>>> > >>>> Regards > >>>> Vignesh > >>>> > >>>> Vignesh Raghavendra (3): > >>>> spi-nor: spi-nor-ids: Disable SPI_NOR_4B_OPCODES for n25q512* and > >>>> n25q256* > >>>> spi-nor: spi-nor-ids: Rename mt25qu512a entry > >>>> spi-nor: spi-nor-ids: Add entries for newer variants of n25q256* and > >>>> n25q512* > >>>> > >>>> drivers/mtd/spi/spi-nor-ids.c | 13 ++++++++----- > >>>> 1 file changed, 8 insertions(+), 5 deletions(-) > >>>> > >>>> -- > >>>> 2.23.0 > >>>> > >> > >> -- > >> Regards > >> Vignesh > > -- > Regards > Vignesh
Hi, Simon, On 09/24/2019 10:02 AM, Simon Goldschmidt wrote: > External E-Mail > > > Hi Vignesh, > > On Tue, Sep 24, 2019 at 7:55 AM Vignesh Raghavendra <vigneshr@ti.com> wrote: >> >> This series removes SPI_NOR_4B_OPCODES flags from legacy variants of >> n25q256* and n25q512* and adds entries for newer variants of those >> flashes that support 4 Byte opcodes. >> >> I don't have the flash devices. So its only compile tested. >> >> Ashish, Simon, >> >> I would greatly appreciate if you could test these patches and make sure >> 4 Byte opcodes are being used. (Probably by enabling/adding prints to >> cmd->opcode in spi_mem_exec_op() in drivers/spi/spi-mem.c > > As written in my last mail, I currently cannot get SFDP to work on my board: > I keep getting 0xffdddfdf instead of 0x50444653 (SFDP_SIGNATURE). > > Any idea for a reason of that? That device I have here seems to be equipped > with an MT25QL256ABA, but that should not be an issue, I think. Can you please check what's written on the chip itself? Usually manufacturers write the chip name, and then the week and the year in the form week||year. Ex 4214, where week is 42 and the year 2014. MT25QL256ABA datasheet says that: "Beginning week 42 (2014), Micron's MT25Q production parts will include SFDP data that aligns with revision 1.6." So maybe earlier versions don't have sfdp support? Also, can you try this kind of patch? Let's dump the sfdp header. SPI_FLASH_SFDP_SUPPORT has to be selected. diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 1acff745d1a2..7e93be90fc09 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -1815,14 +1815,22 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor, { const struct sfdp_parameter_header *param_header, *bfpt_header; struct sfdp_parameter_header *param_headers = NULL; - struct sfdp_header header; + struct sfdp_header header = {}; + u32 *sfdp_table = (u32 *)&header; size_t psize; int i, err; /* Get the SFDP header. */ err = spi_nor_read_sfdp(nor, 0, sizeof(header), &header); - if (err < 0) + if (err < 0) { + dev_err(nor->dev, "spi_nor_read_sfdp err = %d\n", err); return err; + } + + for (i = 0; i < sizeof(header) / sizeof(u32); i++) { + sfdp_table[i] = le32_to_cpu(sfdp_table[i]); + dev_err(nor->dev, "sfdp_table[%d] = %08x\n", i, sfdp_table[i]); + } > > Regards, > Simon > >> >> Euginey, >> >> Could you test this series on top of latest u-boot master and confirm >> that your test cases still work? >> >> Regards >> Vignesh >> >> Vignesh Raghavendra (3): >> spi-nor: spi-nor-ids: Disable SPI_NOR_4B_OPCODES for n25q512* and >> n25q256* >> spi-nor: spi-nor-ids: Rename mt25qu512a entry >> spi-nor: spi-nor-ids: Add entries for newer variants of n25q256* and >> n25q512* >> >> drivers/mtd/spi/spi-nor-ids.c | 13 ++++++++----- >> 1 file changed, 8 insertions(+), 5 deletions(-) >> >> -- >> 2.23.0 >> > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot >
Hi Tudor, On Tue, Sep 24, 2019 at 11:23 AM <Tudor.Ambarus@microchip.com> wrote: > > Hi, Simon, > > On 09/24/2019 10:02 AM, Simon Goldschmidt wrote: > > External E-Mail > > > > > > Hi Vignesh, > > > > On Tue, Sep 24, 2019 at 7:55 AM Vignesh Raghavendra <vigneshr@ti.com> wrote: > >> > >> This series removes SPI_NOR_4B_OPCODES flags from legacy variants of > >> n25q256* and n25q512* and adds entries for newer variants of those > >> flashes that support 4 Byte opcodes. > >> > >> I don't have the flash devices. So its only compile tested. > >> > >> Ashish, Simon, > >> > >> I would greatly appreciate if you could test these patches and make sure > >> 4 Byte opcodes are being used. (Probably by enabling/adding prints to > >> cmd->opcode in spi_mem_exec_op() in drivers/spi/spi-mem.c > > > > As written in my last mail, I currently cannot get SFDP to work on my board: > > I keep getting 0xffdddfdf instead of 0x50444653 (SFDP_SIGNATURE). > > > > Any idea for a reason of that? That device I have here seems to be equipped > > with an MT25QL256ABA, but that should not be an issue, I think. > > Can you please check what's written on the chip itself? Usually manufacturers > write the chip name, and then the week and the year in the form week||year. Ex > 4214, where week is 42 and the year 2014. > > MT25QL256ABA datasheet says that: "Beginning > week 42 (2014), Micron's MT25Q production parts will include SFDP data that aligns > with revision 1.6." > > So maybe earlier versions don't have sfdp support? Problem solved. It was a driver issue, not a chip issue (see last mails). Regards, Simon > > Also, can you try this kind of patch? Let's dump the sfdp header. > SPI_FLASH_SFDP_SUPPORT has to be selected. > > diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c > index 1acff745d1a2..7e93be90fc09 100644 > --- a/drivers/mtd/spi/spi-nor-core.c > +++ b/drivers/mtd/spi/spi-nor-core.c > @@ -1815,14 +1815,22 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor, > { > const struct sfdp_parameter_header *param_header, *bfpt_header; > struct sfdp_parameter_header *param_headers = NULL; > - struct sfdp_header header; > + struct sfdp_header header = {}; > + u32 *sfdp_table = (u32 *)&header; > size_t psize; > int i, err; > > /* Get the SFDP header. */ > err = spi_nor_read_sfdp(nor, 0, sizeof(header), &header); > - if (err < 0) > + if (err < 0) { > + dev_err(nor->dev, "spi_nor_read_sfdp err = %d\n", err); > return err; > + } > + > + for (i = 0; i < sizeof(header) / sizeof(u32); i++) { > + sfdp_table[i] = le32_to_cpu(sfdp_table[i]); > + dev_err(nor->dev, "sfdp_table[%d] = %08x\n", i, sfdp_table[i]); > + } > > > > > > Regards, > > Simon > > > >> > >> Euginey, > >> > >> Could you test this series on top of latest u-boot master and confirm > >> that your test cases still work? > >> > >> Regards > >> Vignesh > >> > >> Vignesh Raghavendra (3): > >> spi-nor: spi-nor-ids: Disable SPI_NOR_4B_OPCODES for n25q512* and > >> n25q256* > >> spi-nor: spi-nor-ids: Rename mt25qu512a entry > >> spi-nor: spi-nor-ids: Add entries for newer variants of n25q256* and > >> n25q512* > >> > >> drivers/mtd/spi/spi-nor-ids.c | 13 ++++++++----- > >> 1 file changed, 8 insertions(+), 5 deletions(-) > >> > >> -- > >> 2.23.0 > >> > > _______________________________________________ > > U-Boot mailing list > > U-Boot@lists.denx.de > > https://lists.denx.de/listinfo/u-boot > >
Hi Vignesh, I've check this patches on top of 31e086e460f. The read/write/erase seems to work. However, as I can see 'sf protect lock' doesn't work - it finish successfully but the area remains unlocked. As I remember It worked with old u-boot spi-nor code, but I need to check it. --- Eugeniy Paltsev
On 24/09/19 10:53 PM, Eugeniy Paltsev wrote: > Hi Vignesh, > > I've check this patches on top of 31e086e460f. > The read/write/erase seems to work. > > However, as I can see 'sf protect lock' doesn't work - it finish successfully but the area remains unlocked. Did you verify that area is indeed unlocked by writing data and then reading it back? I was able to find a board with mt25qu512a which is same as n25q512a in terms of locking I see it works fine: => sf probe SF: Detected n25q512a with page size 256 Bytes, erase size 4 KiB, total 64 MiB => sf protect lock 0 0x4000000; echo $? 0 => sf write 0x82000000 0x3FF0000 0x100 device 0 offset 0x3ff0000, size 0x100 SF: 256 bytes @ 0x3ff0000 Written: ERROR -5 If you still see failures wrt locking, could you provide debug logs from spi_mem_exec_op() (in drivers/spi/spi-mem.c) just like last time? Regards Vignesh > As I remember It worked with old u-boot spi-nor code, but I need to check it. > > --- > Eugeniy Paltsev > > > ________________________________________ > From: Vignesh Raghavendra <vigneshr@ti.com> > Sent: Tuesday, September 24, 2019 08:56 > To: Jagan Teki; Eugeniy Paltsev; Ashish Kumar; Simon Goldschmidt > Cc: Vignesh Raghavendra; u-boot@lists.denx.de; Tom Rini; Alexey Brodkin > Subject: [PATCH RFT 0/3] spi-nor: spi-nor-ids: Fix 4 Byte addressing for n25q256 and n25q512* > > This series removes SPI_NOR_4B_OPCODES flags from legacy variants of > n25q256* and n25q512* and adds entries for newer variants of those > flashes that support 4 Byte opcodes. > > I don't have the flash devices. So its only compile tested. > > Ashish, Simon, > > I would greatly appreciate if you could test these patches and make sure > 4 Byte opcodes are being used. (Probably by enabling/adding prints to > cmd->opcode in spi_mem_exec_op() in drivers/spi/spi-mem.c > > Euginey, > > Could you test this series on top of latest u-boot master and confirm > that your test cases still work? > > Regards > Vignesh > > Vignesh Raghavendra (3): > spi-nor: spi-nor-ids: Disable SPI_NOR_4B_OPCODES for n25q512* and > n25q256* > spi-nor: spi-nor-ids: Rename mt25qu512a entry > spi-nor: spi-nor-ids: Add entries for newer variants of n25q256* and > n25q512* > > drivers/mtd/spi/spi-nor-ids.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > -- > 2.23.0 >
Hi Vignesh, I've tested your "[U-Boot,RFT,v2,0/3] spi-nor: spi-nor-ids: Fix 4 Byte addressing " series applies on the latest master (879396a2405). 'axs103_defconfig' was used without changes. Probe/read/write/erase work for n25q512ax3. Lock/unlock don't work, so here is debug log: (I've tried to run lock/unlock after unlock/lock and write/erase after lock, so I guess it will cover all combinations useful for debug) --------------------->8-------------------------- AXS# sf probe && echo OK 9f | [6B in] 20 ba 20 10 00 00 [ret 0] 06 | [0B -] [ret 0] b7 | [0B -] [ret 0] 04 | [0B -] [ret 0] SF: Detected n25q512ax3 with page size 256 Bytes, erase size 4 KiB, total 64 MiB OK AXS# sf protect lock 0x0 0x4000000 && echo OK 05 | [1B in] 9c [ret 0] OK AXS# mw 0x81000000 0 5 && sf write 0x81000000 0x180000 0x10 && echo OK device 0 offset 0x180000, size 0x10 06 | [0B -] [ret 0] 02 00 18 00 00 | [16B out] 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ret 0] 05 | [1B in] 9c [ret 0] 70 | [1B in] 81 [ret 0] SF: 16 bytes @ 0x180000 Written: OK OK AXS# sf protect unlock 0x0 0x4000000 && echo OK 05 | [1B in] 9c [ret 0] 06 | [0B -] [ret 0] 01 | [1B out] 00 [ret 0] 05 | [1B in] 00 [ret 0] 70 | [1B in] 81 [ret 0] 05 | [1B in] 00 [ret 0] OK AXS# sf protect lock 0x0 0x4000000 && echo OK 05 | [1B in] 00 [ret 0] 06 | [0B -] [ret 0] 01 | [1B out] 9c [ret 0] 05 | [1B in] 9c [ret 0] 70 | [1B in] 81 [ret 0] 05 | [1B in] 9c [ret 0] OK AXS# sf protect lock 0x0 0x4000000 && echo OK 05 | [1B in] 9c [ret 0] OK AXS# AXS# AXS# sf protect unlock 0x0 0x4000000 && echo OK 05 | [1B in] 9c [ret 0] 06 | [0B -] [ret 0] 01 | [1B out] 00 [ret 0] 05 | [1B in] 00 [ret 0] 70 | [1B in] 81 [ret 0] 05 | [1B in] 00 [ret 0] OK AXS# sf protect unlock 0x0 0x4000000 && echo OK 05 | [1B in] 00 [ret 0] OK AXS# AXS# AXS# sf protect lock 0x0 0x4000000 && echo OK 05 | [1B in] 00 [ret 0] 06 | [0B -] [ret 0] 01 | [1B out] 9c [ret 0] 05 | [1B in] 9c [ret 0] 70 | [1B in] 81 [ret 0] 05 | [1B in] 9c [ret 0] OK AXS# sf erase 0x180000 0x1000 && echo OK 06 | [0B -] [ret 0] 20 00 18 00 00 | [0B -] [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9f [ret 0] 70 | [1B in] 01 [ret 0] 05 | [1B in] 9c [ret 0] 70 | [1B in] 81 [ret 0] 04 | [0B -] [ret 0] SF: 4096 bytes @ 0x180000 Erased: OK OK AXS# sf read 0x81000000 0x180000 0x10 && md.b 0x81000000 0x10 && echo OK device 0 offset 0x180000, size 0x10 0b 00 18 00 00 | [16B in] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ret 0] SF: 16 bytes @ 0x180000 Read: OK 81000000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ OK AXS# mw 0x81000000 0 5 && sf write 0x81000000 0x180000 0x10 && echo OK device 0 offset 0x180000, size 0x10 06 | [0B -] [ret 0] 02 00 18 00 00 | [16B out] 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ret 0] 05 | [1B in] 9c [ret 0] 70 | [1B in] 81 [ret 0] SF: 16 bytes @ 0x180000 Written: OK OK AXS# sf read 0x81000000 0x180000 0x10 && echo OK device 0 offset 0x180000, size 0x10 0b 00 18 00 00 | [16B in] 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ret 0] SF: 16 bytes @ 0x180000 Read: OK OK --------------------->8-------------------------- --- Eugeniy Paltsev
Hi Eugeniy, On 07/10/19 8:16 PM, Eugeniy Paltsev wrote: > Hi Vignesh, > > I've tested your "[U-Boot,RFT,v2,0/3] spi-nor: spi-nor-ids: Fix 4 Byte addressing " series > applies on the latest master (879396a2405). > 'axs103_defconfig' was used without changes. > > Probe/read/write/erase work for n25q512ax3. > Thanks for the testing! > Lock/unlock don't work, so here is debug log: > (I've tried to run lock/unlock after unlock/lock and write/erase after lock, so I guess it will cover all combinations useful for debug) > > --------------------->8-------------------------- > AXS# sf probe && echo OK > 9f | [6B in] 20 ba 20 10 00 00 [ret 0] > 06 | [0B -] [ret 0] > b7 | [0B -] [ret 0] > 04 | [0B -] [ret 0] > SF: Detected n25q512ax3 with page size 256 Bytes, erase size 4 KiB, total 64 MiB > OK > AXS# sf protect lock 0x0 0x4000000 && echo OK > 05 | [1B in] 9c [ret 0] > OK You are trying to lock entire 64MB space. Per Data sheet of flash, this requires setting BP3-BP0 bits in Status register. But only BP2-BP0 are set Neither the old framework nor current framework (or even Linux kernel) supports handling of BP3 bit. So this failure is _not a regression_. In this particular case, U-Boot code sets only BP2-BP0 when asked to lock entire 64MB range. But this effectively locks sectors 960 to 1023. Regards Vignesh > AXS# mw 0x81000000 0 5 && sf write 0x81000000 0x180000 0x10 && echo OK > device 0 offset 0x180000, size 0x10 > 06 | [0B -] [ret 0] > 02 00 18 00 00 | [16B out] 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ret 0] > 05 | [1B in] 9c [ret 0] > 70 | [1B in] 81 [ret 0] > SF: 16 bytes @ 0x180000 Written: OK > OK > AXS# sf protect unlock 0x0 0x4000000 && echo OK > 05 | [1B in] 9c [ret 0] > 06 | [0B -] [ret 0] > 01 | [1B out] 00 [ret 0] > 05 | [1B in] 00 [ret 0] > 70 | [1B in] 81 [ret 0] > 05 | [1B in] 00 [ret 0] > OK > AXS# sf protect lock 0x0 0x4000000 && echo OK > 05 | [1B in] 00 [ret 0] > 06 | [0B -] [ret 0] > 01 | [1B out] 9c [ret 0] > 05 | [1B in] 9c [ret 0] > 70 | [1B in] 81 [ret 0] > 05 | [1B in] 9c [ret 0] > OK > AXS# sf protect lock 0x0 0x4000000 && echo OK > 05 | [1B in] 9c [ret 0] > OK > AXS# > AXS# > AXS# sf protect unlock 0x0 0x4000000 && echo OK > 05 | [1B in] 9c [ret 0] > 06 | [0B -] [ret 0] > 01 | [1B out] 00 [ret 0] > 05 | [1B in] 00 [ret 0] > 70 | [1B in] 81 [ret 0] > 05 | [1B in] 00 [ret 0] > OK > AXS# sf protect unlock 0x0 0x4000000 && echo OK > 05 | [1B in] 00 [ret 0] > OK > AXS# > AXS# > AXS# sf protect lock 0x0 0x4000000 && echo OK > 05 | [1B in] 00 [ret 0] > 06 | [0B -] [ret 0] > 01 | [1B out] 9c [ret 0] > 05 | [1B in] 9c [ret 0] > 70 | [1B in] 81 [ret 0] > 05 | [1B in] 9c [ret 0] > OK > AXS# sf erase 0x180000 0x1000 && echo OK > 06 | [0B -] [ret 0] > 20 00 18 00 00 | [0B -] [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9f [ret 0] > 70 | [1B in] 01 [ret 0] > 05 | [1B in] 9c [ret 0] > 70 | [1B in] 81 [ret 0] > 04 | [0B -] [ret 0] > SF: 4096 bytes @ 0x180000 Erased: OK > OK > AXS# sf read 0x81000000 0x180000 0x10 && md.b 0x81000000 0x10 && echo OK > device 0 offset 0x180000, size 0x10 > 0b 00 18 00 00 | [16B in] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ret 0] > SF: 16 bytes @ 0x180000 Read: OK > 81000000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ > OK > AXS# mw 0x81000000 0 5 && sf write 0x81000000 0x180000 0x10 && echo OK > device 0 offset 0x180000, size 0x10 > 06 | [0B -] [ret 0] > 02 00 18 00 00 | [16B out] 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ret 0] > 05 | [1B in] 9c [ret 0] > 70 | [1B in] 81 [ret 0] > SF: 16 bytes @ 0x180000 Written: OK > OK > AXS# sf read 0x81000000 0x180000 0x10 && echo OK > device 0 offset 0x180000, size 0x10 > 0b 00 18 00 00 | [16B in] 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ret 0] > SF: 16 bytes @ 0x180000 Read: OK > OK > --------------------->8-------------------------- > > --- > Eugeniy Paltsev > > ________________________________________ > From: Vignesh Raghavendra <vigneshr@ti.com> > Sent: Wednesday, September 25, 2019 11:11 > To: Eugeniy Paltsev; Jagan Teki; Ashish Kumar; Simon Goldschmidt > Cc: u-boot@lists.denx.de; Tom Rini; Alexey Brodkin > Subject: Re: [PATCH RFT 0/3] spi-nor: spi-nor-ids: Fix 4 Byte addressing for n25q256 and n25q512* > > > > On 24/09/19 10:53 PM, Eugeniy Paltsev wrote: >> Hi Vignesh, >> >> I've check this patches on top of 31e086e460f. >> The read/write/erase seems to work. >> >> However, as I can see 'sf protect lock' doesn't work - it finish successfully but the area remains unlocked. > > Did you verify that area is indeed unlocked by writing data and then reading it back? > I was able to find a board with mt25qu512a which is same as n25q512a in terms of locking > I see it works fine: > > => sf probe > SF: Detected n25q512a with page size 256 Bytes, erase size 4 KiB, total 64 MiB > => sf protect lock 0 0x4000000; echo $? > 0 > => sf write 0x82000000 0x3FF0000 0x100 > device 0 offset 0x3ff0000, size 0x100 > SF: 256 bytes @ 0x3ff0000 Written: ERROR -5 > > If you still see failures wrt locking, could you provide debug logs from > spi_mem_exec_op() (in drivers/spi/spi-mem.c) just like last time? > > > Regards > Vignesh > >> As I remember It worked with old u-boot spi-nor code, but I need to check it. >> >> --- >> Eugeniy Paltsev >> >> >> ________________________________________ >> From: Vignesh Raghavendra <vigneshr@ti.com> >> Sent: Tuesday, September 24, 2019 08:56 >> To: Jagan Teki; Eugeniy Paltsev; Ashish Kumar; Simon Goldschmidt >> Cc: Vignesh Raghavendra; u-boot@lists.denx.de; Tom Rini; Alexey Brodkin >> Subject: [PATCH RFT 0/3] spi-nor: spi-nor-ids: Fix 4 Byte addressing for n25q256 and n25q512* >> >> This series removes SPI_NOR_4B_OPCODES flags from legacy variants of >> n25q256* and n25q512* and adds entries for newer variants of those >> flashes that support 4 Byte opcodes. >> >> I don't have the flash devices. So its only compile tested. >> >> Ashish, Simon, >> >> I would greatly appreciate if you could test these patches and make sure >> 4 Byte opcodes are being used. (Probably by enabling/adding prints to >> cmd->opcode in spi_mem_exec_op() in drivers/spi/spi-mem.c >> >> Euginey, >> >> Could you test this series on top of latest u-boot master and confirm >> that your test cases still work? >> >> Regards >> Vignesh >> >> Vignesh Raghavendra (3): >> spi-nor: spi-nor-ids: Disable SPI_NOR_4B_OPCODES for n25q512* and >> n25q256* >> spi-nor: spi-nor-ids: Rename mt25qu512a entry >> spi-nor: spi-nor-ids: Add entries for newer variants of n25q256* and >> n25q512* >> >> drivers/mtd/spi/spi-nor-ids.c | 13 ++++++++----- >> 1 file changed, 8 insertions(+), 5 deletions(-) >> >> -- >> 2.23.0 >> > > -- > Regards > Vignesh >
This series removes SPI_NOR_4B_OPCODES flags from legacy variants of n25q256* and n25q512* and adds entries for newer variants of those flashes that support 4 Byte opcodes. I don't have the flash devices. So its only compile tested. Ashish, Simon, I would greatly appreciate if you could test these patches and make sure 4 Byte opcodes are being used. (Probably by enabling/adding prints to cmd->opcode in spi_mem_exec_op() in drivers/spi/spi-mem.c Euginey, Could you test this series on top of latest u-boot master and confirm that your test cases still work? Regards Vignesh Vignesh Raghavendra (3): spi-nor: spi-nor-ids: Disable SPI_NOR_4B_OPCODES for n25q512* and n25q256* spi-nor: spi-nor-ids: Rename mt25qu512a entry spi-nor: spi-nor-ids: Add entries for newer variants of n25q256* and n25q512* drivers/mtd/spi/spi-nor-ids.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)