arm: socfpga: Add Cadence QSPI support
Subsystems (2)
v3 cover arm: socfpga: Add Cadence QSPI support
Hi!
So this is my 3rd posting regarding the Candence SPI driver on SoCFPGA.
The current status is, that SPI NOR flash works now without problems. And dcache is still enabled. The previous disabling was not needed. And only caused problems while booting into Linux. No cache flush or invalidate is needed. As no DMA engine is involved in this transfer.
The "solution" I used to get SPI NOR flash support working now is implemented in patch 0004 (still marked as RFC). Here a software reset is issued on the Micron N25Q256A SPI NOR flash. This seems to solve all problems and reading / writing to the SPI NOR flash seems to work now just fine. Even rebooting via pushbutton-reset or reset command works.
I think this version is now in much better state. Thats why I removed the WIP from the subject lines.
Again, this is tested on the EBV SoCrates eval board.
Thanks,
Stefan
PATCH v3 1/4 spi: Add Cadence QSPI driver used by SoCFPGA
This driver is copied directly from the Altera Rockerboard.org U-Boot repository. I used this git tag: ACDS14.0.1_REL_GSRD_RC2. With minimal changes to enable compilation in mainline U-Boot.
---
v3:
- Added SPDX License Indentifier
- Removed CQSPI_WRITEL / CQSPI_READL
drivers/spi/Makefile | 1 + drivers/spi/cadence_qspi.c | 339 ++++++++++++++++ drivers/spi/cadence_qspi.h | 54 +++ drivers/spi/cadence_qspi_apb.c | 900 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 1294 insertions(+) create mode 100644 drivers/spi/cadence_qspi.c create mode 100644 drivers/spi/cadence_qspi.h create mode 100644 drivers/spi/cadence_qspi_apb.c
drivers/spi/Makefile +1 -0
@@ -16,6 +16,7 @@ obj-$(CONFIG_ATMEL_DATAFLASH_SPI) += atmel_dataflash_spi.o obj-$(CONFIG_ATMEL_SPI) += atmel_spi.o obj-$(CONFIG_BFIN_SPI) += bfin_spi.o obj-$(CONFIG_BFIN_SPI6XX) += bfin_spi6xx.o +obj-$(CONFIG_CADENCE_QSPI) += cadence_qspi.o cadence_qspi_apb.o obj-$(CONFIG_CF_SPI) += cf_spi.o obj-$(CONFIG_CF_QSPI) += cf_qspi.o obj-$(CONFIG_DAVINCI_SPI) += davinci_spi.o
drivers/spi/cadence_qspi.c new file +339 -0
339 lines not on this page · raw message
drivers/spi/cadence_qspi.h new file +54 -0
54 lines not on this page · raw message
drivers/spi/cadence_qspi_apb.c new file +900 -0
900 lines not on this page · raw message
Signature · 1 line
2.1.1
PATCH v3 2/4 arm: socfpga: Add Cadence QSPI support to config header
In reply to Stefan Roese--- include/configs/socfpga_cyclone5_common.h | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
include/configs/socfpga_cyclone5_common.h +31 -0
@@ -142,6 +142,37 @@#endif /* +* QSPI support +*/ +#ifdef CONFIG_CMD_SF +#define CONFIG_CADENCE_QSPI +#define CONFIG_CQSPI_BASE (SOCFPGA_QSPI_ADDRESS) +#define CONFIG_CQSPI_AHB_BASE (SOCFPGA_QSPIDATA_ADDRESS) +#define CONFIG_SPI_FLASH /* SPI flash subsystem */ +#define CONFIG_SPI_FLASH_STMICRO /* Micron/Numonyx flash */ +#define CONFIG_SPI_FLASH_SPANSION /* Spansion flash */ +#define CONFIG_SPI_FLASH_MTD +/* Flash device info */ +#define CONFIG_SF_DEFAULT_SPEED (50000000) +#define CONFIG_SF_DEFAULT_MODE SPI_MODE_3 +#define CONFIG_SPI_FLASH_QUAD (1) +/* QSPI reference clock */ +#ifndef __ASSEMBLY__ +unsigned int cm_get_qspi_controller_clk_hz (void ); +#define CONFIG_CQSPI_REF_CLK cm_get_qspi_controller_clk_hz() +#endif +/* QSPI page size and block size */ +#define CONFIG_CQSPI_PAGE_SIZE (256) +#define CONFIG_CQSPI_BLOCK_SIZE (16) +/* QSPI Delay timing */ +#define CONFIG_CQSPI_TSHSL_NS (200) +#define CONFIG_CQSPI_TSD2D_NS (255) +#define CONFIG_CQSPI_TCHSH_NS (20) +#define CONFIG_CQSPI_TSLCH_NS (20) +#define CONFIG_CQSPI_DECODER (0) +#endif /* CONFIG_CMD_SF */ + +/* * Serial Driver */ #define CONFIG_SYS_NS16550
Signature · 1 line
2.1.1
PATCH v3 3/4 arm: socfpga: Don't define CONFIG_SPI_FLASH_QUAD
In reply to Stefan RoeseThis define is currently not supported in mainline U-Boot. So don't define it. Otherwise the drivers doesn't work correctly.
--- include/configs/socfpga_cyclone5_common.h | 2 ++ 1 file changed, 2 insertions(+)
include/configs/socfpga_cyclone5_common.h +2 -0
@@ -155,7 +155,9 @@/* Flash device info */ #define CONFIG_SF_DEFAULT_SPEED (50000000)#define CONFIG_SF_DEFAULT_MODE SPI_MODE_3 +#if 0 /* not supported in mainline right now */ #define CONFIG_SPI_FLASH_QUAD (1) +#endif /* QSPI reference clock */ #ifndef __ASSEMBLY__unsigned int cm_get_qspi_controller_clk_hz (void );
Signature · 1 line
2.1.1
RFC PATCH v3 4/4 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
In reply to Stefan RoeseThis is needed for the SoCFPGA booting from SPI NOR flash e.g. (N25Q256A). With these changes, the SoCrates can boot and re-boot (reset) from SPI NOR flash without any problems.
--- drivers/mtd/spi/sf_probe.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
drivers/mtd/spi/sf_probe.c +31 -0
@@ -355,6 +355,37 @@ static struct spi_flash *spi_flash_probe_slave(struct spi_slave *spi) } } +#ifdef CONFIG_SPI_N25Q256A_RESET +#define CMD_RESET_ENABLE 0x66 +#define CMD_RESET_MEMORY 0x99 +/* +* This is needed for the SoCFPGA booting from SPI NOR flash +* e.g. (N25Q256A). Additionally its necessary to change +* this line in the Linux SPI NOR flash driver: +* +* { "n25q256a", INFO(0x20ba19, 0, 64 * 1024, 512, +* SECT_4K | SHUTDOWN_3BYTE) }, +* +* Add SHUTDOWN_3BYTE here. +* +* With these changes, the SoCrates can boot and re-boot +* (reset) from SPI NOR flash without any problems. +*/ + ret =spi_flash_cmd (spi,CMD_RESET_ENABLE ,NULL ,0 ); +if (ret) { +printf ("SF: Failed issue reset command\n" ); + goto err_read_id; + } + + ret =spi_flash_cmd (spi,CMD_RESET_MEMORY ,NULL ,0 ); +if (ret) { +printf ("SF: Failed issue reset command\n" ); + goto err_read_id; + } + +printf ("SF: Device software reset\n" ); +#endif +#ifdef CONFIG_OF_CONTROL if (spi_flash_decode_fdt (gd->fdt_blob, flash)) {debug ("SF: FDT decode error\n" );
Signature · 1 line
2.1.1
RFC PATCH v3 4/4 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 3 lines · Stefan Roese
This is needed for the SoCFPGA booting from SPI NOR flash e.g. (N25Q256A). With these changes, the SoCrates can boot and re-boot (reset) from SPI NOR flash without any problems.
Seems like your SPI NOR reset logic is buggy. Does any of [1] apply to your board please?
[1] http://www.rocketboards.org/foswiki/Documentation/SocBoardQspiBoot
Best regards,
Marek Vasut
RFC PATCH v3 4/4 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 9 lines · Marek Vasut
Quoted 3 lines · Stefan Roese
This is needed for the SoCFPGA booting from SPI NOR flash e.g. (N25Q256A). With these changes, the SoCrates can boot and re-boot (reset) from SPI NOR flash without any problems.
Seems like your SPI NOR reset logic is buggy. Does any of [1] apply to your board please?
[1] http://www.rocketboards.org/foswiki/Documentation/SocBoardQspiBoot
Yes. This seems to be that case. But I can't change it right now. So this "solution" with the soft-reset is better than nothing.
Thanks,
Stefan
RFC PATCH v3 4/4 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 13 lines · Stefan Roese
Quoted 12 lines · Marek Vasut
Quoted 4 lines · Stefan Roese
This is needed for the SoCFPGA booting from SPI NOR flash e.g. (N25Q256A). With these changes, the SoCrates can boot and re-boot (reset) from SPI NOR flash without any problems.
Seems like your SPI NOR reset logic is buggy. Does any of [1] apply to your
board please?
Quoted 5 lines
Quoted 1 line
[1] http://www.rocketboards.org/foswiki/Documentation/SocBoardQspiBoot
Yes. This seems to be that case. But I can't change it right now. So this "solution" with the soft-reset is better than nothing.
If this is some think that must require, any possibility to this resetting prior to u-boot?
like preloader or in first stage boot loader or something.
I feel this is mostly a kind of hardware treat, and looks like it's touching generic sf framework
which is not so good.
thanks!
Signature · 1 line
Jagan.
RFC PATCH v3 4/4 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 12 lines · Jagan Teki
Quoted 12 lines
Quoted 10 lines
Quoted 3 lines
This is needed for the SoCFPGA booting from SPI NOR flash e.g. (N25Q256A). With these changes, the SoCrates can boot and re-boot (reset) from SPI NOR flash without any problems.
Seems like your SPI NOR reset logic is buggy. Does any of [1] apply to your
board please?[1] http://www.rocketboards.org/foswiki/Documentation/SocBoardQspiBoot
Quoted 6 lines
Quoted 2 lines
Yes. This seems to be that case. But I can't change it right now. So this "solution" with the soft-reset is better than nothing.
If this is some think that must require, any possibility to this resetting prior to u-boot?
like preloader or in first stage boot loader or something.
Perhaps I was not clear with the intention of this patch. Its more to show how the problem with SPI flash on this platform / board (SoCrates in this case) can be solved. So that others have a reference. Thats why I marked it as RFC. Its not really meant for inclusion into mainline.
The real solution is a board rework. If not possible, the preloader should be changed. As I don't have access to the preloader code right now, this "solution" (I know, its more a hack) didn't seem too bad.
Quoted 3 lines
I feel this is mostly a kind of hardware treat, and looks like it's touching generic sf framework
which is not so good.
Full ack on this. So please don't pull in into mainline.
Thanks,
Stefan
RFC PATCH v3 4/4 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 20 lines · Stefan Roese
Quoted 12 lines · Jagan Teki
Quoted 12 lines
Quoted 10 lines
Quoted 3 lines
This is needed for the SoCFPGA booting from SPI NOR flash e.g. (N25Q256A). With these changes, the SoCrates can boot and re-boot (reset) from SPI NOR flash without any problems.
Seems like your SPI NOR reset logic is buggy. Does any of [1] apply to your
board please?[1] http://www.rocketboards.org/foswiki/Documentation/SocBoardQspiBoot
Quoted 6 lines
Quoted 2 lines
Yes. This seems to be that case. But I can't change it right now. So this "solution" with the soft-reset is better than nothing.
If this is some think that must require, any possibility to this resetting prior to u-boot?
like preloader or in first stage boot loader or something.
Quoted 8 lines
Perhaps I was not clear with the intention of this patch. Its more to show how the problem with SPI flash on this platform / board (SoCrates in this case) can be solved. So that others have a reference. Thats why I marked it as RFC. Its not really meant for inclusion into mainline.
The real solution is a board rework. If not possible, the preloader should be changed. As I don't have access to the preloader code right now, this "solution" (I know, its more a hack) didn't seem too bad.
For the record, I do not think preloader is good place for such workaround. Preloader works with SDRAM, and should load real u-boot as fast as possible. If it does not need to touch SPI (it does not, right?) it should not need to work around bugs there.
Actually, if SPI driver is not compiled into the u-boot, kernel will have to do the workaround.
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
RFC PATCH v3 4/4 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 26 lines · Pavel Machek
Quoted 25 lines · Stefan Roese
Quoted 12 lines · Jagan Teki
Quoted 12 lines
Quoted 10 lines
Quoted 3 lines
This is needed for the SoCFPGA booting from SPI NOR flash e.g. (N25Q256A). With these changes, the SoCrates can boot and re-boot (reset) from SPI NOR flash without any problems.
Seems like your SPI NOR reset logic is buggy. Does any of [1] apply to your
board please?[1] http://www.rocketboards.org/foswiki/Documentation/SocBoardQspiBoot
Quoted 6 lines
Quoted 2 lines
Yes. This seems to be that case. But I can't change it right now. So this "solution" with the soft-reset is better than nothing.
If this is some think that must require, any possibility to this resetting prior to u-boot?
like preloader or in first stage boot loader or something.Perhaps I was not clear with the intention of this patch. Its more to show how the problem with SPI flash on this platform / board (SoCrates in this case) can be solved. So that others have a reference. Thats why I marked it as RFC. Its not really meant for inclusion into mainline.
Quoted 8 lines
Quoted 3 lines
The real solution is a board rework. If not possible, the preloader should be changed. As I don't have access to the preloader code right now, this "solution" (I know, its more a hack) didn't seem too bad.
For the record, I do not think preloader is good place for such workaround. Preloader works with SDRAM, and should load real u-boot as fast as possible. If it does not need to touch SPI (it does not, right?) it should not need to work around bugs there.
In this case, where the board boots from SPI NOR flash, the Preloader (SPL U-Boot version) does use SPI. To load the main U-Boot image from the SPI NOR flash.
My current best guess is that this define in the Preloader (older SPL U-Boot version) causes these problems:
#define CONFIG_SPI_FLASH_QUAD (1)
Once I have full access to the Preloader source (in a few days hopefully) I can verify this.
Thanks,
Stefan
RFC PATCH v3 4/4 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
In reply to Jagan TekiQuoted 11 lines · Jagan Teki
Quoted 10 lines · Stefan Roese
Quoted 9 lines · Marek Vasut
Quoted 3 lines · Stefan Roese
This is needed for the SoCFPGA booting from SPI NOR flash e.g. (N25Q256A). With these changes, the SoCrates can boot and re-boot (reset) from SPI NOR flash without any problems.
Seems like your SPI NOR reset logic is buggy. Does any of [1] apply to your
board please?
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
[1] http://www.rocketboards.org/foswiki/Documentation/SocBoardQspiBoot
Yes. This seems to be that case. But I can't change it right now. So this "solution" with the soft-reset is better than nothing.
If this is some think that must require, any possibility to this resetting prior to u-boot?
like preloader or in first stage boot loader or something.
You do understand, that this is a hardware bug on one particular board, right ? This can _not_ be reliably solved in software, not ever. I keep seeing people implementing one such workaround after the other in linux-mtd list, but sooner or later, they discover that their workaround is not reliable. Without proper reset logic in place, a system simply cannot reliably reboot, since it has no way to put all the hardware into defined state.
Also, the N25Qxxx chips are particularly crappy in this aspect, since they tend to happily get stuck in some weird undefined state during reboot. Proper reset logic solves this issue, see the link above please.
Best regards,
Marek Vasut
RFC PATCH v3 4/4 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 21 lines · Marek Vasut
Quoted 11 lines · Jagan Teki
Quoted 10 lines · Stefan Roese
Quoted 9 lines · Marek Vasut
Quoted 3 lines · Stefan Roese
This is needed for the SoCFPGA booting from SPI NOR flash e.g. (N25Q256A). With these changes, the SoCrates can boot and re-boot (reset) from SPI NOR flash without any problems.
Seems like your SPI NOR reset logic is buggy. Does any of [1] apply to your
board please?Quoted 8 lines
Quoted 4 lines
Quoted 1 line
[1] http://www.rocketboards.org/foswiki/Documentation/SocBoardQspiBoot
Yes. This seems to be that case. But I can't change it right now. So this "solution" with the soft-reset is better than nothing.
If this is some think that must require, any possibility to this resetting prior to u-boot?
like preloader or in first stage boot loader or something.
Quoted 6 lines
You do understand, that this is a hardware bug on one particular board, right ? This can _not_ be reliably solved in software, not ever. I keep seeing people implementing one such workaround after the other in linux-mtd list, but sooner or later, they discover that their workaround is not reliable. Without proper reset logic in place, a system simply cannot reliably reboot, since it has no way to put all the hardware into defined state.
Well, if you have >16M flash and if you need bootrom to work with it.
AFAICT, as long as you avoid using SPI from bootrom (socrates will happily run from SD card, for example), reliable operation should be possible. And you can still use SPI from Linux and u-boot...
Pavel
RFC PATCH v3 4/4 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 31 lines · Pavel Machek
Quoted 22 lines · Marek Vasut
Quoted 14 lines · Jagan Teki
Quoted 13 lines · Stefan Roese
Quoted 11 lines · Marek Vasut
Quoted 3 lines · Stefan Roese
This is needed for the SoCFPGA booting from SPI NOR flash e.g. (N25Q256A). With these changes, the SoCrates can boot and re-boot (reset) from SPI NOR flash without any problems.
Seems like your SPI NOR reset logic is buggy. Does any of [1] apply to your
board please?[1]
http://www.rocketboards.org/foswiki/Documentation/SocBoardQspiBootQuoted 6 lines
Quoted 2 lines
Yes. This seems to be that case. But I can't change it right now. So this "solution" with the soft-reset is better than nothing.
If this is some think that must require, any possibility to this resetting prior to u-boot?
like preloader or in first stage boot loader or something.Quoted 7 lines
You do understand, that this is a hardware bug on one particular board, right ? This can _not_ be reliably solved in software, not ever. I keep seeing people implementing one such workaround after the other in linux-mtd list, but sooner or later, they discover that their workaround is not reliable. Without proper reset logic in place, a system simply cannot reliably reboot, since it has no way to put all the hardware into defined state.
Quoted 5 lines
Well, if you have >16M flash and if you need bootrom to work with it.
AFAICT, as long as you avoid using SPI from bootrom (socrates will happily run from SD card, for example), reliable operation should be possible. And you can still use SPI from Linux and u-boot...
SD has the same problem if you don't have proper reset logic for it ;-)
Best regards,
Marek Vasut
RFC PATCH v3 4/4 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
In reply to Jagan TekiQuoted 17 lines · Jagan Teki
Quoted 16 lines · Stefan Roese
Quoted 13 lines · Marek Vasut
Quoted 4 lines · Stefan Roese
This is needed for the SoCFPGA booting from SPI NOR flash e.g. (N25Q256A). With these changes, the SoCrates can boot and re-boot (reset) from SPI NOR flash without any problems.
Seems like your SPI NOR reset logic is buggy. Does any of [1] apply to your
board please?[1] http://www.rocketboards.org/foswiki/Documentation/SocBoardQspiBoot
Quoted 6 lines
Quoted 2 lines
Yes. This seems to be that case. But I can't change it right now. So this "solution" with the soft-reset is better than nothing.
If this is some think that must require, any possibility to this resetting prior to u-boot?
like preloader or in first stage boot loader or something.
u-boot-spl 2013.01/altera set it up like this for us; and mainline u-boot-spl does not work on socfpga... that's why we need to do it here, and that's why you don't see it on your board.
Best regards,
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
RFC PATCH v3 4/4 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 17 lines · Pavel Machek
Quoted 16 lines · Jagan Teki
Quoted 14 lines · Stefan Roese
Quoted 10 lines · Marek Vasut
Quoted 3 lines · Stefan Roese
This is needed for the SoCFPGA booting from SPI NOR flash e.g. (N25Q256A). With these changes, the SoCrates can boot and re-boot (reset) from SPI NOR flash without any problems.
Seems like your SPI NOR reset logic is buggy. Does any of [1] apply to your
board please?[1] http://www.rocketboards.org/foswiki/Documentation/SocBoardQspiBoot
Yes. This seems to be that case. But I can't change it right now. So this "solution" with the soft-reset is better than nothing.
Quoted 7 lines
Quoted 3 lines
If this is some think that must require, any possibility to this resetting prior to u-boot?
like preloader or in first stage boot loader or something.u-boot-spl 2013.01/altera set it up like this for us; and mainline u-boot-spl does not work on socfpga... that's why we need to do it here, and that's why you don't see it on your board.
As discussed in person, please rebase and repost.
You're right that it's a good idea to add an option to restart the SPI NOR in software if needed be.
Best regards,
Marek Vasut
PATCH 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
This is needed for the SoCFPGA booting from SPI NOR flash e.g. (N25Q256A) as long as u-boot-spl 2013 is used (newer one is not available).
---
Ported to today's u-boot version.
drivers/mtd/spi/sf_probe.c +30 -0
@@ -347,6 +348,36 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash) } } +#ifdef CONFIG_SPI_N25Q256A_RESET +#define CMD_RESET_ENABLE 0x66 +#define CMD_RESET_MEMORY 0x99 +/* +* This is needed for the SoCFPGA booting from SPI NOR flash +* e.g. (N25Q256A), as U-Boot SPL 2013-socfpga (only version +* working on that board) sets 4-byt addressing mode. +* +* Additionally it may be good idea to change +* this line in the Linux SPI NOR flash driver: +* +* { "n25q256a", INFO(0x20ba19, 0, 64 * 1024, 512, +* SECT_4K | SHUTDOWN_3BYTE) }, +* +* Add SHUTDOWN_3BYTE here. +*/ + ret =spi_flash_cmd (spi,CMD_RESET_ENABLE ,NULL ,0 ); +if (ret) { +printf ("SF: Failed issue reset command\n" ); + goto err_read_id; + } + + ret =spi_flash_cmd (spi,CMD_RESET_MEMORY ,NULL ,0 ); +if (ret) { +printf ("SF: Failed issue reset command\n" ); + goto err_read_id; + } + +printf ("SF: Device software reset\n" ); +#endif #ifdef CONFIG_OF_CONTROL if (spi_flash_decode_fdt (gd->fdt_blob, flash)) {debug ("SF: FDT decode error\n" );
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
PATCH 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 16 lines · Pavel Machek
This is needed for the SoCFPGA booting from SPI NOR flash e.g. (N25Q256A) as long as u-boot-spl 2013 is used (newer one is not available).
Signed-off-by Stefan Roese <[email protected]>Signed-off-by Pavel Machek <[email protected]>---
Ported to today's u-boot version.
drivers/mtd/spi/sf_probe.c +0
-0
drivers/mtd/spi/sf_probe.c int spi_flash_probe_slave(struct spi_slave *spi, · line 351
Quoted 4 lines
drivers/mtd/spi/sf_probe.c +1
-0@@ -347,6 +348,36 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash) } } +#ifdef CONFIG_SPI_N25Q256A_RESET
Should be CONFIG_SPI_MICRON_RESET, since other parts which can also be used would have similar issue.
This should also be documented in README I believe. It'd be nice if you added diffstat into your patches as it makes things easier during review.
drivers/mtd/spi/sf_probe.c int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash) · line 360
Quoted 9 lines
drivers/mtd/spi/sf_probe.c +9
-0+#define CMD_RESET_ENABLE 0x66 +#define CMD_RESET_MEMORY 0x99 +/* +* This is needed for the SoCFPGA booting from SPI NOR flash +* e.g. (N25Q256A), as U-Boot SPL 2013-socfpga (only version +* working on that board) sets 4-byt addressing mode. +* +* Additionally it may be good idea to change +* this line in the Linux SPI NOR flash driver:
Please submit a patch for Linux then. But this will be extremely crappy and unreliable solution, so you should at least make the kernel spit some warning upon shutdown so people are aware they are doing something terribly wrong.
drivers/mtd/spi/sf_probe.c int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash) · line 369
Quoted 8 lines
drivers/mtd/spi/sf_probe.c +8
-0+* { "n25q256a", INFO(0x20ba19, 0, 64 * 1024, 512, +* SECT_4K | SHUTDOWN_3BYTE) }, +* +* Add SHUTDOWN_3BYTE here. +*/ + ret =spi_flash_cmd (spi,CMD_RESET_ENABLE ,NULL ,0 ); +if (ret) { +printf ("SF: Failed issue reset command\n" );
I thought this was just a reset-enable command. If this command fails, user won't be able to tell which of these two failed, so it's a bad idea to use the same error message for both.
Quoted 4 lines
drivers/mtd/spi/sf_probe.c +4
-0+ goto err_read_id; + } + + ret =spi_flash_cmd (spi,CMD_RESET_MEMORY ,NULL ,0 );
drivers/mtd/spi/sf_probe.c int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash) · line 383
Quoted 10 lines
drivers/mtd/spi/sf_probe.c +7
-0+if (ret) { +printf ("SF: Failed issue reset command\n" ); + goto err_read_id; + } + +printf ("SF: Device software reset\n" ); +#endif #ifdef CONFIG_OF_CONTROL if (spi_flash_decode_fdt (gd->fdt_blob, flash)) {debug ("SF: FDT decode error\n" );
Best regards,
Marek Vasut
PATCH 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 9 lines · Marek Vasut
Quoted 8 lines · Pavel Machek
drivers/mtd/spi/sf_probe.c +0
-0@@ -347,6 +348,36 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash) } }
Quoted 4 lines
drivers/mtd/spi/sf_probe.c int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash) · line 351
Quoted 1 line
drivers/mtd/spi/sf_probe.c +1
-0+#ifdef CONFIG_SPI_N25Q256A_RESET Should be
CONFIG_SPI_MICRON_RESET, since other parts which can also be used would have similar issue.
I'm pretty sure some Micron parts use different interface.
Quoted 3 lines
It'd be nice if you
added diffstat into your patches as it makes things easier during review.
Yes, it also makes patch harder to create (as it is tricky to hand-edit the patches), and having diffstat for a patch that fits on a screen is just stupid.
Quoted 4 lines
Quoted 4 lines
drivers/mtd/spi/sf_probe.c +4
-0+* { "n25q256a", INFO(0x20ba19, 0, 64 * 1024, 512, +* SECT_4K | SHUTDOWN_3BYTE) }, +* +* Add SHUTDOWN_3BYTE here.
Quoted 8 lines
drivers/mtd/spi/sf_probe.c int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash) · line 369
Quoted 4 lines
drivers/mtd/spi/sf_probe.c +4
-0+*/ + ret =spi_flash_cmd (spi,CMD_RESET_ENABLE ,NULL ,0 ); +if (ret) { +printf ("SF: Failed issue reset command\n" );I thought this was just a reset-enable command. If this command fails, user won't be able to tell which of these two failed, so it's a bad idea to use the same error message for both.
Ok.
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
PATCH 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 9 lines · Pavel Machek
Quoted 8 lines · Marek Vasut
Quoted 7 lines · Pavel Machek
drivers/mtd/spi/sf_probe.c +0
-0@@ -347,6 +348,36 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash) }
Quoted 8 lines
Quoted 6 lines
drivers/mtd/spi/sf_probe.c int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash) · line 351
Quoted 3 lines
drivers/mtd/spi/sf_probe.c +1
-0} +#ifdef CONFIG_SPI_N25Q256A_RESET Should be
CONFIG_SPI_MICRON_RESET, since other parts which can also be used would have similar issue.I'm pretty sure some Micron parts use different interface.
Which ones ?
Quoted 6 lines
Quoted 3 lines
It'd be nice if you
added diffstat into your patches as it makes things easier during review.Yes, it also makes patch harder to create (as it is tricky to hand-edit the patches)
git format-patch automatically inserts the diffstat for you.
Quoted 2 lines
, and having diffstat for a patch that fits on a
screen is just stupid.
It's a 2-line diffstat for this patch. If you get a 1-screen big diffstat, then your patch is most likely wrong.
Quoted 13 lines
Quoted 4 lines
Quoted 4 lines
drivers/mtd/spi/sf_probe.c +4
-0+* { "n25q256a", INFO(0x20ba19, 0, 64 * 1024, 512, +* SECT_4K | SHUTDOWN_3BYTE) }, +* +* Add SHUTDOWN_3BYTE here. Quoted 8 lines
drivers/mtd/spi/sf_probe.c int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash) · line 369
Quoted 4 lines
drivers/mtd/spi/sf_probe.c +4
-0+*/ + ret =spi_flash_cmd (spi,CMD_RESET_ENABLE ,NULL ,0 ); +if (ret) { +printf ("SF: Failed issue reset command\n" );I thought this was just a reset-enable command. If this command fails, user won't be able to tell which of these two failed, so it's a bad idea to use the same error message for both.
Quoted 2 lines
Ok.
Pavel
Best regards,
Marek Vasut
PATCH 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 12 lines · Marek Vasut
Quoted 11 lines · Pavel Machek
Quoted 10 lines · Marek Vasut
Quoted 9 lines · Pavel Machek
drivers/mtd/spi/sf_probe.c +0
-0@@ -347,6 +348,36 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash) } }
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
drivers/mtd/spi/sf_probe.c int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash) · line 351
Quoted 1 line
drivers/mtd/spi/sf_probe.c +1
-0+#ifdef CONFIG_SPI_N25Q256A_RESET Should be
CONFIG_SPI_MICRON_RESET, since other parts which can also be used would have similar issue.I'm pretty sure some Micron parts use different interface.
Which ones ?
N25Q128?
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
It'd be nice if you
added diffstat into your patches as it makes things easier during review.Yes, it also makes patch harder to create (as it is tricky to hand-edit the patches)
git format-patch automatically inserts the diffstat for you.
As I have already told you, I'm not using git to submit my patches.
Quoted 5 lines
Quoted 2 lines
, and having diffstat for a patch that fits on a
screen is just stupid.It's a 2-line diffstat for this patch. If you get a 1-screen big diffstat, then your patch is most likely wrong.
As I said, diffstat for patch that fits on screen is useless and stupid.
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
PATCH v2 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Add reset for N25Q256A SPI NOR, as U-Boot SPL 2013-socfpga (only version working on that board) sets 4-byte addressing mode.
---
Diff to v2: Add README.
README +4 -0
@@ -2775,6 +2775,10 @@ CBFS (Coreboot Filesystem) support Timeout for waiting until spi transfer completed. default: (CONFIG_SYS_HZ/100) /* 10 ms */ + CONFIG_SPI_N25Q256A_RESET + Reset SPI NOR flash. Needed when preloader sets it to 4-byte + mode, for example. + - FPGA Support: CONFIG_FPGA Enables FPGA subsystem.
drivers/mtd/spi/sf_probe.c +22 -0
@@ -347,6 +348,28 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash) } } +#ifdef CONFIG_SPI_N25Q256A_RESET +#define CMD_RESET_ENABLE 0x66 +#define CMD_RESET_MEMORY 0x99 +/* +* This is needed for the SoCFPGA booting from SPI NOR flash +* e.g. (N25Q256A), as U-Boot SPL 2013-socfpga (only version +* working on that board) sets 4-byte addressing mode. +*/ + ret =spi_flash_cmd (spi,CMD_RESET_ENABLE ,NULL ,0 ); +if (ret) { +printf ("SF: Failed issue enable reset command\n" ); + goto err_read_id; + } + + ret =spi_flash_cmd (spi,CMD_RESET_MEMORY ,NULL ,0 ); +if (ret) { +printf ("SF: Failed issue reset command\n" ); + goto err_read_id; + } + +printf ("SF: Device software reset\n" ); +#endif #ifdef CONFIG_OF_CONTROL if (spi_flash_decode_fdt (gd->fdt_blob, flash)) {debug ("SF: FDT decode error\n" );
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
PATCH v2 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 4 lines · Pavel Machek
Add reset for N25Q256A SPI NOR, as U-Boot SPL 2013-socfpga (only version working on that board) sets 4-byte addressing mode.
Signed-off-by Pavel Machek <[email protected]>
I'm still convinced this should be MICRON specific, not N25Q256 specific .
Best regards,
Marek Vasut
PATCH v3 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
In reply to Pavel MachekOn Sun 2015-05-10 11:15:41, Pavel Machek wrote:
Add reset for N25Q256A SPI NOR, as U-Boot SPL 2013-socfpga (only version working on that board) sets 4-byte addressing mode.
---
Diff to v3: Marek insists this is Micron specific, so lets rename the config option.
README +4 -0
@@ -2775,6 +2775,10 @@ CBFS (Coreboot Filesystem) support Timeout for waiting until spi transfer completed. default: (CONFIG_SYS_HZ/100) /* 10 ms */ + CONFIG_SPI_MICRON_RESET + Reset SPI NOR flash. Needed when preloader sets it to 4-byte + mode, for example. + - FPGA Support: CONFIG_FPGA Enables FPGA subsystem.
drivers/mtd/spi/sf_probe.c +22 -0
@@ -347,6 +348,28 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash) } } +#ifdef CONFIG_SPI_MICRON_RESET +#define CMD_RESET_ENABLE 0x66 +#define CMD_RESET_MEMORY 0x99 +/* +* This is needed for the SoCFPGA booting from SPI NOR flash +* e.g. (MICRON), as U-Boot SPL 2013-socfpga (only version +* working on that board) sets 4-byte addressing mode. +*/ + ret =spi_flash_cmd (spi,CMD_RESET_ENABLE ,NULL ,0 ); +if (ret) { +printf ("SF: Failed issue enable reset command\n" ); + goto err_read_id; + } + + ret =spi_flash_cmd (spi,CMD_RESET_MEMORY ,NULL ,0 ); +if (ret) { +printf ("SF: Failed issue reset command\n" ); + goto err_read_id; + } + +printf ("SF: Device software reset\n" ); +#endif #ifdef CONFIG_OF_CONTROL if (spi_flash_decode_fdt (gd->fdt_blob, flash)) {debug ("SF: FDT decode error\n" );
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
PATCH v3 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 49 lines · Pavel Machek
On Sun 2015-05-10 11:15:41, Pavel Machek wrote:
Add reset for N25Q256A SPI NOR, as U-Boot SPL 2013-socfpga (only version working on that board) sets 4-byte addressing mode.Signed-off-by Pavel Machek <[email protected]>---
Diff to v3: Marek insists this is Micron specific, so lets rename the config option.README +4
-0@@ -2775,6 +2775,10 @@ CBFS (Coreboot Filesystem) support Timeout for waiting until spi transfer completed. default: (CONFIG_SYS_HZ/100) /* 10 ms */ + CONFIG_SPI_MICRON_RESET + Reset SPI NOR flash. Needed when preloader sets it to 4-byte + mode, for example. + - FPGA Support: CONFIG_FPGA Enables FPGA subsystem.drivers/mtd/spi/sf_probe.c +15
-0@@ -347,6 +348,28 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash) } } +#ifdef CONFIG_SPI_MICRON_RESET +#define CMD_RESET_ENABLE 0x66 +#define CMD_RESET_MEMORY 0x99 +/* +* This is needed for the SoCFPGA booting from SPI NOR flash +* e.g. (MICRON), as U-Boot SPL 2013-socfpga (only version +* working on that board) sets 4-byte addressing mode. +*/ + ret =spi_flash_cmd (spi,CMD_RESET_ENABLE ,NULL ,0 ); +if (ret) { +printf ("SF: Failed issue enable reset command\n" ); + goto err_read_id; + } + + ret =spi_flash_cmd (spi,CMD_RESET_MEMORY ,NULL ,0 );
drivers/mtd/spi/sf_probe.c int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash) · line 375
Quoted 10 lines
drivers/mtd/spi/sf_probe.c +7
-0+if (ret) { +printf ("SF: Failed issue reset command\n" ); + goto err_read_id; + } + +printf ("SF: Device software reset\n" ); +#endif #ifdef CONFIG_OF_CONTROL if (spi_flash_decode_fdt (gd->fdt_blob, flash)) {debug ("SF: FDT decode error\n" );
As per my early comments on this thread, I'm not happy with this approach of being added flash specific in generic code by simply added like this.
I encounter similar issue before and I simply reverted [1]
http://git.denx.de/?p=u-boot.git;a=commit;h=122d805fd4bd478bb83536348291d34ae648364b
Please think in a different perceptive like adding any flags to specific vendor with specific part, but that also require proper tested.
thanks!
Signature · 2 lines
Jagan Teki, Openedev.
PATCH v3 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 6 lines · Jagan Teki
Quoted 5 lines · Pavel Machek
On Sun 2015-05-10 11:15:41, Pavel Machek wrote:
Add reset for N25Q256A SPI NOR, as U-Boot SPL 2013-socfpga (only version working on that board) sets 4-byte addressing mode.Signed-off-by Pavel Machek <[email protected]>
So, I took one more look into the datasheet [1]. With this chip, which is 128Mbit (16MByte), you don't even use 4-byte addressing so you don't care about it at all.
But you actually do care about it if your hardware is broken and you use the N25Q256A [2] part. If the problem really is just between the U-Boot SPL and U-Boot (which I doubt btw), you can probably augment U-Boot such that it reads FSR (Flag Status Register, see page 27, table 17, bit 0). This bit tells you whether the part is in 3-byte or 4-byte mode and you can read this register in either mode.
Same applies to N25Q512A [3], FSR bit 0 indicates the mode.
[1] https://www.micron.com/~/media/documents/products/data-sheet/nor-
flash/serial-nor/n25q/n25q_128mb_1_8v_65nm.pdf
[2] https://www.micron.com/~/media/documents/products/data-sheet/nor-
flash/serial-nor/n25q/n25q_256mb_3v_65nm.pdf
[3] https://www.micron.com/~/media/documents/products/data-sheet/nor-
flash/serial-nor/n25q/n25q_512mb_1ce_3v_65nm.pdf
Best regards,
Marek Vasut
PATCH v3 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 8 lines · Marek Vasut
Quoted 6 lines · Jagan Teki
Quoted 5 lines · Pavel Machek
On Sun 2015-05-10 11:15:41, Pavel Machek wrote:
Add reset for N25Q256A SPI NOR, as U-Boot SPL 2013-socfpga (only version working on that board) sets 4-byte addressing mode.Signed-off-by Pavel Machek <[email protected]>
Quoted 7 lines
So, I took one more look into the datasheet [1]. With this chip, which is 128Mbit (16MByte), you don't even use 4-byte addressing so you don't care about it at all.
But you actually do care about it if your hardware is broken and you use the N25Q256A [2] part. If the problem really is just between the U-Boot
No, I actually care whenever I use the U-boot SPL 2013, which is the only option today. And I explained it to you already. (Additionally, hardware might be broken. That has nothing to do with _this_ problem).
Quoted 4 lines
SPL and U-Boot (which I doubt btw), you can probably augment U-Boot such that it reads FSR (Flag Status Register, see page 27, table 17, bit 0). This bit tells you whether the part is in 3-byte or 4-byte mode and you can read this register in either mode.
How does reading Micron-specific register help with code being too Micron-specific is unclear to me.
Pavel
Quoted 12 lines
Same applies to N25Q512A [3], FSR bit 0 indicates the mode.
[1] https://www.micron.com/~/media/documents/products/data-sheet/nor-
flash/serial-nor/n25q/n25q_128mb_1_8v_65nm.pdf
[2] https://www.micron.com/~/media/documents/products/data-sheet/nor-
flash/serial-nor/n25q/n25q_256mb_3v_65nm.pdf
[3] https://www.micron.com/~/media/documents/products/data-sheet/nor-
flash/serial-nor/n25q/n25q_512mb_1ce_3v_65nm.pdfBest regards,
Marek Vasut
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
PATCH v3 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 13 lines · Pavel Machek
Quoted 12 lines · Marek Vasut
Quoted 6 lines · Jagan Teki
Quoted 5 lines · Pavel Machek
On Sun 2015-05-10 11:15:41, Pavel Machek wrote:
Add reset for N25Q256A SPI NOR, as U-Boot SPL 2013-socfpga (only version working on that board) sets 4-byte addressing mode.Signed-off-by Pavel Machek <[email protected]>So, I took one more look into the datasheet [1]. With this chip, which is 128Mbit (16MByte), you don't even use 4-byte addressing so you don't care about it at all.
Quoted 7 lines
Quoted 3 lines
But you actually do care about it if your hardware is broken and you use the N25Q256A [2] part. If the problem really is just between the U-Boot
No, I actually care whenever I use the U-boot SPL 2013, which is the only option today. And I explained it to you already. (Additionally, hardware might be broken. That has nothing to do with _this_ problem).
OK
Quoted 7 lines
Quoted 4 lines
SPL and U-Boot (which I doubt btw), you can probably augment U-Boot such that it reads FSR (Flag Status Register, see page 27, table 17, bit 0). This bit tells you whether the part is in 3-byte or 4-byte mode and you can read this register in either mode.
How does reading Micron-specific register help with code being too Micron-specific is unclear to me.
Just add a micron-specific hook and check the register on Micron parts only. This would eliminate the need for new CONFIG_foo_bar option, which is always a plus.
Best regards,
Marek Vasut
PATCH v3 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
In reply to Jagan TekiQuoted 64 lines · Jagan Teki
Quoted 49 lines · Pavel Machek
On Sun 2015-05-10 11:15:41, Pavel Machek wrote:
Add reset for N25Q256A SPI NOR, as U-Boot SPL 2013-socfpga (only version working on that board) sets 4-byte addressing mode.Signed-off-by Pavel Machek <[email protected]>---
Diff to v3: Marek insists this is Micron specific, so lets rename the config option.README +4
-0@@ -2775,6 +2775,10 @@ CBFS (Coreboot Filesystem) support Timeout for waiting until spi transfer completed. default: (CONFIG_SYS_HZ/100) /* 10 ms */ + CONFIG_SPI_MICRON_RESET + Reset SPI NOR flash. Needed when preloader sets it to 4-byte + mode, for example. + - FPGA Support: CONFIG_FPGA Enables FPGA subsystem.drivers/mtd/spi/sf_probe.c +15
-0@@ -347,6 +348,28 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash) } } +#ifdef CONFIG_SPI_MICRON_RESET +#define CMD_RESET_ENABLE 0x66 +#define CMD_RESET_MEMORY 0x99 +/* +* This is needed for the SoCFPGA booting from SPI NOR flash +* e.g. (MICRON), as U-Boot SPL 2013-socfpga (only version +* working on that board) sets 4-byte addressing mode. +*/ + ret =spi_flash_cmd (spi,CMD_RESET_ENABLE ,NULL ,0 ); +if (ret) { +printf ("SF: Failed issue enable reset command\n" ); + goto err_read_id; + } + + ret =spi_flash_cmd (spi,CMD_RESET_MEMORY ,NULL ,0 );drivers/mtd/spi/sf_probe.c int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash) · line 375
Quoted 10 lines
drivers/mtd/spi/sf_probe.c +7
-0+if (ret) { +printf ("SF: Failed issue reset command\n" ); + goto err_read_id; + } + +printf ("SF: Device software reset\n" ); +#endif #ifdef CONFIG_OF_CONTROL if (spi_flash_decode_fdt (gd->fdt_blob, flash)) {debug ("SF: FDT decode error\n" );As per my early comments on this thread, I'm not happy with this approach of being added flash specific in generic code by simply added like this.
Quoted 5 lines
I encounter similar issue before and I simply reverted [1]
http://git.denx.de/?p=u-boot.git;a=commit;h=122d805fd4bd478bb83536348291d34ae648364bPlease think in a different perceptive like adding any flags to specific vendor with specific part, but that also require proper tested.
Ok, could you explain how you'd like to have it solved? Should we just call spi_flash_cmd() from socfpga-specific code?
But I'm pretty sure similar issue will be encountered on different boards, so it would be good to have it in shared place. Do you want me to create sf_probe_micron and move it there?
Best regards,
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
PATCH v3 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
On Sunday, May 10, 2015 at 07:53:46 PM, Pavel Machek wrote:
[...]
Quoted 10 lines
Quoted 9 lines
As per my early comments on this thread, I'm not happy with this approach of being added flash specific in generic code by simply added like this.
I encounter similar issue before and I simply reverted [1]
http://git.denx.de/?p=u-boot.git;a=commit;h=122d805fd4bd478bb83536348291d34ae648364bPlease think in a different perceptive like adding any flags to specific vendor with specific part, but that also require proper tested.
Quoted 6 lines
Ok, could you explain how you'd like to have it solved? Should we just call
spi_flash_cmd()from socfpga-specific code?But I'm pretty sure similar issue will be encountered on different boards, so it would be good to have it in shared place. Do you want me to create
sf_probe_micronand move it there?
I'd just add a flag into the SPI NOR table and in the code, I'd check if the flag is set and if so, trigger the Micron-specific code. That ought to be simple and it doesn't introduce any new config options.
Best regards,
Marek Vasut
PATCH v3 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 17 lines · Marek Vasut
On Sunday, May 10, 2015 at 07:53:46 PM, Pavel Machek wrote:
[...]
Quoted 13 lines
Quoted 9 lines
As per my early comments on this thread, I'm not happy with this approach of being added flash specific in generic code by simply added like this.
I encounter similar issue before and I simply reverted [1]
http://git.denx.de/?p=u-boot.git;a=commit;h=122d805fd4bd478bb83536348291d34ae648364bPlease think in a different perceptive like adding any flags to specific vendor with specific part, but that also require proper tested.
Ok, could you explain how you'd like to have it solved? Should we just call
spi_flash_cmd()from socfpga-specific code?
Quoted 7 lines
Quoted 3 lines
But I'm pretty sure similar issue will be encountered on different boards, so it would be good to have it in shared place. Do you want me to create
sf_probe_micronand move it there?I'd just add a flag into the SPI NOR table and in the code, I'd check if the flag is set and if so, trigger the Micron-specific code. That ought to be simple and it doesn't introduce any new config options.
Ok, solutions - what about Linux same issue or this requirement should be only for u-boot or bootloaders?
thanks!
Signature · 2 lines
Jagan Teki, Openedev.
PATCH v3 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 22 lines · Jagan Teki
Quoted 21 lines · Marek Vasut
On Sunday, May 10, 2015 at 07:53:46 PM, Pavel Machek wrote:
[...]
Quoted 16 lines
Quoted 9 lines
As per my early comments on this thread, I'm not happy with this approach of being added flash specific in generic code by simply added like this.
I encounter similar issue before and I simply reverted [1]
http://git.denx.de/?p=u-boot.git;a=commit;h=122d805fd4bd478bb83536348291d34ae648364bPlease think in a different perceptive like adding any flags to specific vendor with specific part, but that also require proper tested.
Ok, could you explain how you'd like to have it solved? Should we just call
spi_flash_cmd()from socfpga-specific code?But I'm pretty sure similar issue will be encountered on different boards, so it would be good to have it in shared place. Do you want me to create
sf_probe_micronand move it there?
Quoted 6 lines
Quoted 3 lines
I'd just add a flag into the SPI NOR table and in the code, I'd check if the flag is set and if so, trigger the Micron-specific code. That ought to be simple and it doesn't introduce any new config options.
Ok, solutions - what about Linux same issue or this requirement should be only for u-boot or bootloaders?
Linux probably has same issue.
Do you like Marek's "add a flag to table" proposal?
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
PATCH v3 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 23 lines · Pavel Machek
Quoted 22 lines · Jagan Teki
Quoted 21 lines · Marek Vasut
On Sunday, May 10, 2015 at 07:53:46 PM, Pavel Machek wrote:
[...]
Quoted 16 lines
Quoted 9 lines
As per my early comments on this thread, I'm not happy with this approach of being added flash specific in generic code by simply added like this.
I encounter similar issue before and I simply reverted [1]
http://git.denx.de/?p=u-boot.git;a=commit;h=122d805fd4bd478bb83536348291d34ae648364bPlease think in a different perceptive like adding any flags to specific vendor with specific part, but that also require proper tested.
Ok, could you explain how you'd like to have it solved? Should we just call
spi_flash_cmd()from socfpga-specific code?But I'm pretty sure similar issue will be encountered on different boards, so it would be good to have it in shared place. Do you want me to create
sf_probe_micronand move it there?
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
I'd just add a flag into the SPI NOR table and in the code, I'd check if the flag is set and if so, trigger the Micron-specific code. That ought to be simple and it doesn't introduce any new config options.
Ok, solutions - what about Linux same issue or this requirement should be only for u-boot or bootloaders?
Linux probably has same issue.
Please check the same.
Quoted 2 lines
Do you like Marek's "add a flag to table" proposal?
Please send the patch to both the ML's.
thanks!
Signature · 2 lines
Jagan Teki, Openedev.
PATCH v3 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 31 lines · Jagan Teki
Quoted 30 lines · Pavel Machek
Quoted 28 lines · Jagan Teki
Quoted 17 lines · Marek Vasut
On Sunday, May 10, 2015 at 07:53:46 PM, Pavel Machek wrote:
[...]
Quoted 13 lines
Quoted 9 lines
As per my early comments on this thread, I'm not happy with this approach of being added flash specific in generic code by simply added like this.
I encounter similar issue before and I simply reverted [1]
http://git.denx.de/?p=u-boot.git;a=commit;h=122d805fd4bd478bb83536348291d34ae648364bPlease think in a different perceptive like adding any flags to specific vendor with specific part, but that also require proper tested.
Ok, could you explain how you'd like to have it solved? Should we just call
spi_flash_cmd()from socfpga-specific code?Quoted 7 lines
Quoted 3 lines
But I'm pretty sure similar issue will be encountered on different boards, so it would be good to have it in shared place. Do you want me to create
sf_probe_micronand move it there?I'd just add a flag into the SPI NOR table and in the code, I'd check if the flag is set and if so, trigger the Micron-specific code. That ought to be simple and it doesn't introduce any new config options.
Ok, solutions - what about Linux same issue or this requirement should be only for u-boot or bootloaders?
Quoted 8 lines
Quoted 1 line
Linux probably has same issue.
Please check the same.
Quoted 2 lines
Do you like Marek's "add a flag to table" proposal?
Please send the patch to both the ML's.
Which mailing lists do you mean?
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
PATCH v3 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 34 lines · Pavel Machek
Quoted 33 lines · Jagan Teki
Quoted 31 lines · Pavel Machek
Quoted 22 lines · Jagan Teki
Quoted 21 lines · Marek Vasut
On Sunday, May 10, 2015 at 07:53:46 PM, Pavel Machek wrote:
[...]
Quoted 16 lines
Quoted 9 lines
As per my early comments on this thread, I'm not happy with this approach of being added flash specific in generic code by simply added like this.
I encounter similar issue before and I simply reverted [1]
http://git.denx.de/?p=u-boot.git;a=commit;h=122d805fd4bd478bb83536348291d34ae648364bPlease think in a different perceptive like adding any flags to specific vendor with specific part, but that also require proper tested.
Ok, could you explain how you'd like to have it solved? Should we just call
spi_flash_cmd()from socfpga-specific code?But I'm pretty sure similar issue will be encountered on different boards, so it would be good to have it in shared place. Do you want me to create
sf_probe_micronand move it there?Quoted 6 lines
Quoted 3 lines
I'd just add a flag into the SPI NOR table and in the code, I'd check if the flag is set and if so, trigger the Micron-specific code. That ought to be simple and it doesn't introduce any new config options.
Ok, solutions - what about Linux same issue or this requirement should be only for u-boot or bootloaders?
Linux probably has same issue.
Quoted 8 lines
Quoted 6 lines
Please check the same.
Quoted 2 lines
Do you like Marek's "add a flag to table" proposal?
Please send the patch to both the ML's.
Which mailing lists do you mean?
I meant to say if something is not working with Linux as well, send the same patch to both U-Boot and Linux.
thanks!
Signature · 2 lines
Jagan Teki, Openedev.
PATCH v3 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 38 lines · Jagan Teki
Quoted 37 lines · Pavel Machek
Quoted 36 lines · Jagan Teki
Quoted 23 lines · Pavel Machek
Quoted 22 lines · Jagan Teki
Quoted 21 lines · Marek Vasut
On Sunday, May 10, 2015 at 07:53:46 PM, Pavel Machek wrote:
[...]
Quoted 16 lines
Quoted 9 lines
As per my early comments on this thread, I'm not happy with this approach of being added flash specific in generic code by simply added like this.
I encounter similar issue before and I simply reverted [1]
http://git.denx.de/?p=u-boot.git;a=commit;h=122d805fd4bd478bb83536348291d34ae648364bPlease think in a different perceptive like adding any flags to specific vendor with specific part, but that also require proper tested.
Ok, could you explain how you'd like to have it solved? Should we just call
spi_flash_cmd()from socfpga-specific code?But I'm pretty sure similar issue will be encountered on different boards, so it would be good to have it in shared place. Do you want me to create
sf_probe_micronand move it there?Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
I'd just add a flag into the SPI NOR table and in the code, I'd check if the flag is set and if so, trigger the Micron-specific code. That ought to be simple and it doesn't introduce any new config options.
Ok, solutions - what about Linux same issue or this requirement should be only for u-boot or bootloaders?
Linux probably has same issue.
Please check the same.
Quoted 1 line
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
Do you like Marek's "add a flag to table" proposal?
Please send the patch to both the ML's.
Which mailing lists do you mean?
I meant to say if something is not working with Linux as well, send the same patch to both U-Boot and Linux.
Linux already handles it, see drivers/mtd/spi-nor/spi-nor.c, functions spi_nor_scan and set_4byte.
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
PATCH v3 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 43 lines · Pavel Machek
Quoted 42 lines · Jagan Teki
Quoted 41 lines · Pavel Machek
Quoted 39 lines · Jagan Teki
Quoted 23 lines · Pavel Machek
Quoted 22 lines · Jagan Teki
Quoted 21 lines · Marek Vasut
On Sunday, May 10, 2015 at 07:53:46 PM, Pavel Machek wrote:
[...]
Quoted 16 lines
Quoted 9 lines
As per my early comments on this thread, I'm not happy with this approach of being added flash specific in generic code by simply added like this.
I encounter similar issue before and I simply reverted [1]
http://git.denx.de/?p=u-boot.git;a=commit;h=122d805fd4bd478bb83536348291d34ae648364bPlease think in a different perceptive like adding any flags to specific vendor with specific part, but that also require proper tested.
Ok, could you explain how you'd like to have it solved? Should we just call
spi_flash_cmd()from socfpga-specific code?But I'm pretty sure similar issue will be encountered on different boards, so it would be good to have it in shared place. Do you want me to create
sf_probe_micronand move it there?Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
I'd just add a flag into the SPI NOR table and in the code, I'd check if the flag is set and if so, trigger the Micron-specific code. That ought to be simple and it doesn't introduce any new config options.
Ok, solutions - what about Linux same issue or this requirement should be only for u-boot or bootloaders?
Linux probably has same issue.
Please check the same.
Quoted 2 lines
Do you like Marek's "add a flag to table" proposal?
Please send the patch to both the ML's.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Which mailing lists do you mean?
I meant to say if something is not working with Linux as well, send the same patch to both U-Boot and Linux.
Linux already handles it, see
drivers/mtd/spi-nor/spi-nor.c, functionsspi_nor_scanandset_4byte.
Seems like nothing much doing in micron side
switch (JEDEC_MFR(info)) {
case CFI_MFR_ST: /* Micron, actually */
/* Some Micron need WREN command; all will accept it */
need_wren = true;thanks!
Signature · 2 lines
Jagan Teki, Openedev.
PATCH v3 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 49 lines · Jagan Teki
Quoted 48 lines · Pavel Machek
Quoted 46 lines · Jagan Teki
Quoted 34 lines · Pavel Machek
Quoted 33 lines · Jagan Teki
Quoted 31 lines · Pavel Machek
Quoted 22 lines · Jagan Teki
Quoted 21 lines · Marek Vasut
On Sunday, May 10, 2015 at 07:53:46 PM, Pavel Machek wrote:
[...]
Quoted 16 lines
Quoted 9 lines
As per my early comments on this thread, I'm not happy with this approach of being added flash specific in generic code by simply added like this.
I encounter similar issue before and I simply reverted [1]
http://git.denx.de/?p=u-boot.git;a=commit;h=122d805fd4bd478bb83536348291d34ae648364bPlease think in a different perceptive like adding any flags to specific vendor with specific part, but that also require proper tested.
Ok, could you explain how you'd like to have it solved? Should we just call
spi_flash_cmd()from socfpga-specific code?But I'm pretty sure similar issue will be encountered on different boards, so it would be good to have it in shared place. Do you want me to create
sf_probe_micronand move it there?Quoted 6 lines
Quoted 3 lines
I'd just add a flag into the SPI NOR table and in the code, I'd check if the flag is set and if so, trigger the Micron-specific code. That ought to be simple and it doesn't introduce any new config options.
Ok, solutions - what about Linux same issue or this requirement should be only for u-boot or bootloaders?
Linux probably has same issue.
Quoted 8 lines
Quoted 6 lines
Please check the same.
Quoted 2 lines
Do you like Marek's "add a flag to table" proposal?
Please send the patch to both the ML's.
Which mailing lists do you mean?
I meant to say if something is not working with Linux as well, send the same patch to both U-Boot and Linux.
Quoted 8 lines
Quoted 2 lines
Linux already handles it, see
drivers/mtd/spi-nor/spi-nor.c, functionsspi_nor_scanandset_4byte.Seems like nothing much doing in micron side
switch (JEDEC_MFR(info)) { case CFI_MFR_ST: /* Micron, actually */ /* Some Micron need WREN command; all will accept it */ need_wren = true;
Umm. Take a closer look.
There's no break, so it continues below.
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
PATCH v3 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 59 lines · Pavel Machek
Quoted 49 lines · Jagan Teki
Quoted 48 lines · Pavel Machek
Quoted 46 lines · Jagan Teki
Quoted 34 lines · Pavel Machek
Quoted 33 lines · Jagan Teki
Quoted 31 lines · Pavel Machek
Quoted 22 lines · Jagan Teki
Quoted 21 lines · Marek Vasut
On Sunday, May 10, 2015 at 07:53:46 PM, Pavel Machek wrote:
[...]
Quoted 16 lines
Quoted 9 lines
As per my early comments on this thread, I'm not happy with this approach of being added flash specific in generic code by simply added like this.
I encounter similar issue before and I simply reverted [1]
http://git.denx.de/?p=u-boot.git;a=commit;h=122d805fd4bd478bb83536348291d34ae648364bPlease think in a different perceptive like adding any flags to specific vendor with specific part, but that also require proper tested.
Ok, could you explain how you'd like to have it solved? Should we just call
spi_flash_cmd()from socfpga-specific code?But I'm pretty sure similar issue will be encountered on different boards, so it would be good to have it in shared place. Do you want me to create
sf_probe_micronand move it there?Quoted 6 lines
Quoted 3 lines
I'd just add a flag into the SPI NOR table and in the code, I'd check if the flag is set and if so, trigger the Micron-specific code. That ought to be simple and it doesn't introduce any new config options.
Ok, solutions - what about Linux same issue or this requirement should be only for u-boot or bootloaders?
Linux probably has same issue.
Quoted 8 lines
Quoted 6 lines
Please check the same.
Quoted 2 lines
Do you like Marek's "add a flag to table" proposal?
Please send the patch to both the ML's.
Which mailing lists do you mean?
I meant to say if something is not working with Linux as well, send the same patch to both U-Boot and Linux.
Quoted 8 lines
Quoted 2 lines
Linux already handles it, see
drivers/mtd/spi-nor/spi-nor.c, functionsspi_nor_scanandset_4byte.Seems like nothing much doing in micron side
switch (JEDEC_MFR(info)) { case CFI_MFR_ST: /* Micron, actually */ /* Some Micron need WREN command; all will accept it */ need_wren = true;
Quoted 3 lines
Umm. Take a closer look.
There's no break, so it continues below.
Ohh.. So Linux works for you.. then is it?
thanks!
Signature · 2 lines
Jagan Teki, Openedev.
PATCH v3 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
Seems like nothing much doing in micron side
switch (JEDEC_MFR(info)) { case CFI_MFR_ST: /* Micron, actually */ /* Some Micron need WREN command; all will accept it */ need_wren = true;
Quoted 5 lines
Quoted 3 lines
Umm. Take a closer look.
There's no break, so it continues below.
Ohh.. So Linux works for you.. then is it?
Well, yes, but this code patch is actually not tested in my case, because u-boot already resets it for Linux.
Anyway, is this what you'd like to see?
Pavel
drivers/mtd/spi/sf_internal.h +1 -0
@@ -43,6 +43,7 @@ enum {SST_BP =1 <<3 ,SST_WP =1 <<4 ,WR_QPP =1 <<5 , + MICRON_RESET = 1 << 6, };#define SST_WR (SST_BP | SST_WP)
drivers/mtd/spi/sf_params.c +1 -1
@@ -85,7 +85,7 @@ const struct spi_flash_params spi_flash_params_table[] = { {"N25Q128A", 0x20bb18, 0x0, 64 * 1024, 256, RD_FULL, WR_QPP}, {"N25Q256", 0x20ba19, 0x0, 64 * 1024, 512, RD_FULL, WR_QPP | SECT_4K}, {"N25Q256A", 0x20bb19, 0x0, 64 * 1024, 512, RD_FULL, WR_QPP | SECT_4K}, - {"N25Q512", 0x20ba20, 0x0, 64 * 1024, 1024, RD_FULL, WR_QPP | E_FSR | SECT_4K}, + {"N25Q512", 0x20ba20, 0x0, 64 * 1024, 1024, RD_FULL, WR_QPP | E_FSR | SECT_4K | MICRON_RESET}, {"N25Q512A", 0x20bb20, 0x0, 64 * 1024, 1024, RD_FULL, WR_QPP | E_FSR | SECT_4K}, {"N25Q1024", 0x20ba21, 0x0, 64 * 1024, 2048, RD_FULL, WR_QPP | E_FSR | SECT_4K}, {"N25Q1024A", 0x20bb21, 0x0, 64 * 1024, 2048, RD_FULL, WR_QPP | E_FSR | SECT_4K},
drivers/mtd/spi/sf_probe.c +26 -1
@@ -8,6 +8,7 @@* SPDX-License-Identifier: GPL-2.0+ */ +#define DEBUG #include <common.h> #include <dm.h> #include <errno.h>
@@ -233,6 +234,30 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode, flash->poll_cmd =CMD_FLAG_STATUS ;#endif +if (params->flags &MICRON_RESET ) { +#define CMD_RESET_ENABLE 0x66 +#define CMD_RESET_MEMORY 0x99 +int ret; +/* +* This is needed for the SoCFPGA booting from SPI NOR flash +* e.g. (N25Q256A), as U-Boot SPL 2013-socfpga (only version +* working on that board) sets 4-byte addressing mode. +*/ + ret =spi_flash_cmd (spi,CMD_RESET_ENABLE ,NULL ,0 ); +if (ret) { +printf ("SF: Failed issue enable reset command\n" ); +return ret; + } + + ret =spi_flash_cmd (spi,CMD_RESET_MEMORY ,NULL ,0 ); +if (ret) { +printf ("SF: Failed issue reset command\n" ); +return ret; + } + +printf ("SF: Device software reset\n" ); + } +/* Configure the BAR - discover bank cmds and read current bank */ #ifdef CONFIG_SPI_FLASH_BAR u8 curr_bank =0 ;
@@ -369,7 +394,7 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash) ((flash->dual_flash > SF_SINGLE_FLASH) && (flash->size > SPI_FLASH_16MB_BOUN << 1))) {puts ("SF: Warning - Only lower 16MiB accessible," ); -puts (" Full access #define CONFIG_SPI_FLASH_BAR\n" ); +puts (" For full access, #define CONFIG_SPI_FLASH_BAR\n" ); }#endif
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
PATCH v3 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 10 lines · Pavel Machek
Quoted 10 lines
Quoted 10 lines
Quoted 7 lines
Seems like nothing much doing in micron side
switch (JEDEC_MFR(info)) { case CFI_MFR_ST: /* Micron, actually */ /* Some Micron need WREN command; all will accept it */ need_wren = true;Umm. Take a closer look.
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
There's no break, so it continues below.
Ohh.. So Linux works for you.. then is it?
Well, yes, but this code patch is actually not tested in my case, because u-boot already resets it for Linux.
Anyway, is this what you'd like to see?
Was my idea of checking in which mode the SPI NOR is first discarded ?
[...]
drivers/mtd/spi/sf_probe.c static int spi_flash_validate_params(struct spi_slave · line 240
Quoted 8 lines
drivers/mtd/spi/sf_probe.c +4
-0@@ -233,6 +234,30 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode, flash->poll_cmd =CMD_FLAG_STATUS ;#endif +if (params->flags &MICRON_RESET ) { +#define CMD_RESET_ENABLE 0x66 +#define CMD_RESET_MEMORY 0x99 +int ret;
The indent here seems funny.
I think it'd be better to pull this reset procedure into a separate function.
Quoted 9 lines
drivers/mtd/spi/sf_probe.c +9
-0+/* +* This is needed for the SoCFPGA booting from SPI NOR flash +* e.g. (N25Q256A), as U-Boot SPL 2013-socfpga (only version +* working on that board) sets 4-byte addressing mode. +*/ + ret =spi_flash_cmd (spi,CMD_RESET_ENABLE ,NULL ,0 ); +if (ret) { +printf ("SF: Failed issue enable reset command\n" ); +return ret;
drivers/mtd/spi/sf_probe.c static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode, · line 259
Quoted 10 lines
drivers/mtd/spi/sf_probe.c +10
-0+ } + + ret =spi_flash_cmd (spi,CMD_RESET_MEMORY ,NULL ,0 ); +if (ret) { +printf ("SF: Failed issue reset command\n" ); +return ret; + } + +printf ("SF: Device software reset\n" ); + }
Otherwise I think it's OK.
PATCH v3 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
Quoted 13 lines · Marek Vasut
Quoted 12 lines · Pavel Machek
Quoted 12 lines
Quoted 11 lines
Quoted 7 lines
Seems like nothing much doing in micron side
switch (JEDEC_MFR(info)) { case CFI_MFR_ST: /* Micron, actually */ /* Some Micron need WREN command; all will accept it */ need_wren = true;Umm. Take a closer look.
There's no break, so it continues below.
Quoted 8 lines
Quoted 6 lines
Quoted 1 line
Ohh.. So Linux works for you.. then is it?
Well, yes, but this code patch is actually not tested in my case, because u-boot already resets it for Linux.
Anyway, is this what you'd like to see?
Was my idea of checking in which mode the SPI NOR is first discarded ?
Yes, better just reset it.
Quoted 13 lines
[...]
drivers/mtd/spi/sf_probe.c static int spi_flash_validate_params(struct spi_slave · line 240
Quoted 8 lines
drivers/mtd/spi/sf_probe.c +4
-0@@ -233,6 +234,30 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode, flash->poll_cmd =CMD_FLAG_STATUS ;#endif +if (params->flags &MICRON_RESET ) { +#define CMD_RESET_ENABLE 0x66 +#define CMD_RESET_MEMORY 0x99 +int ret;The indent here seems funny.
Quoted 2 lines
I think it'd be better to pull this reset procedure into a separate function.
Can do, but I need to know if this is what Jagan wanted, first.
Pavel
Quoted 21 lines
Quoted 9 lines
drivers/mtd/spi/sf_probe.c +9
-0+/* +* This is needed for the SoCFPGA booting from SPI NOR flash +* e.g. (N25Q256A), as U-Boot SPL 2013-socfpga (only version +* working on that board) sets 4-byte addressing mode. +*/ + ret =spi_flash_cmd (spi,CMD_RESET_ENABLE ,NULL ,0 ); +if (ret) { +printf ("SF: Failed issue enable reset command\n" ); +return ret;drivers/mtd/spi/sf_probe.c static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode, · line 259
Quoted 10 lines
drivers/mtd/spi/sf_probe.c +10
-0+ } + + ret =spi_flash_cmd (spi,CMD_RESET_MEMORY ,NULL ,0 ); +if (ret) { +printf ("SF: Failed issue reset command\n" ); +return ret; + } + +printf ("SF: Device software reset\n" ); + }Otherwise I think it's OK.
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
PATCH 1/1 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
In reply to Pavel MachekQuoted 18 lines · Pavel Machek
Quoted 17 lines · Marek Vasut
Quoted 16 lines · Pavel Machek
Quoted 14 lines · Marek Vasut
drivers/mtd/spi/sf_probe.c int spi_flash_probe_slave(struct spi_slave · line 352
Quoted 10 lines · Pavel Machek
(quoted diff) +0
-0
b/drivers/mtd/spi/sf_probe.cindex 201471c..f7cfbd9 100644drivers/mtd/spi/sf_probe.c +1
-0@@ -347,6 +348,36 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash) } } +#ifdef CONFIG_SPI_N25Q256A_RESET Should be
CONFIG_SPI_MICRON_RESET, since other parts which can also be used would have similar issue.
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
I'm pretty sure some Micron parts use different interface.
Which ones ?
N25Q128?
According to [1] page 28, the interface is the same. Did I miss something?
[1] https://www.micron.com/~/media/documents/products/data-sheet/nor-
flash/serial-nor/n25q/n25q_128_3_volt_with_boot_sector.pdf
Quoted 10 lines
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
It'd be nice if you
added diffstat into your patches as it makes things easier during review.Yes, it also makes patch harder to create (as it is tricky to hand-edit the patches)
git format-patch automatically inserts the diffstat for you.
As I have already told you, I'm not using git to submit my patches.
Please fix your tools then.
[...]
Best regards,
Marek Vasut
RFC PATCH v3 4/4 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
In reply to Marek VasutQuoted 7 lines · Marek Vasut
Quoted 3 lines · Stefan Roese
This is needed for the SoCFPGA booting from SPI NOR flash e.g. (N25Q256A). With these changes, the SoCrates can boot and re-boot (reset) from SPI NOR flash without any problems.
Seems like your SPI NOR reset logic is buggy. Does any of [1] apply to your board please?
Actually, this has nothing to do with SPI NOR reset logic.
Yes, we need to reset NOR flash during u-boot startup, because hardware was already initialized to 4-byte addressing by u-boot-spl. No matter what reset logic does, if it passes u-boot-spl (and SPI NOR is actually used), it will be in 4-byte mode.
And no, mainline u-boot-spl is not usable on socfpga, and probably will not be usable for a whill, so u-boot-spl from altera is only option here.
So.. what to do?
Do you want this patch? Do you want patch that know how to switch back to 3-byte addressing on this particular flash?
Best regards,
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
RFC PATCH v3 4/4 mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset
In reply to Stefan RoeseQuoted 44 lines · Stefan Roese
This is needed for the SoCFPGA booting from SPI NOR flash e.g. (N25Q256A). With these changes, the SoCrates can boot and re-boot (reset) from SPI NOR flash without any problems.
Signed-off-by Stefan Roese <[email protected]>Cc Chin Liang See <[email protected]>Cc Dinh Nguyen <[email protected]>Cc Vince Bridgers <[email protected]>Cc Marek Vasut <[email protected]>Cc Pavel Machek <[email protected]>Cc Michael Trimarchi <[email protected]>Cc Jagannadha Sutradharudu Teki <[email protected]>--- drivers/mtd/spi/sf_probe.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)drivers/mtd/spi/sf_probe.c +20
-0@@ -355,6 +355,37 @@ static struct spi_flash *spi_flash_probe_slave(struct spi_slave *spi) } } +#ifdef CONFIG_SPI_N25Q256A_RESET +#define CMD_RESET_ENABLE 0x66 +#define CMD_RESET_MEMORY 0x99 +/* +* This is needed for the SoCFPGA booting from SPI NOR flash +* e.g. (N25Q256A). Additionally its necessary to change +* this line in the Linux SPI NOR flash driver: +* +* { "n25q256a", INFO(0x20ba19, 0, 64 * 1024, 512, +* SECT_4K | SHUTDOWN_3BYTE) }, +* +* Add SHUTDOWN_3BYTE here. +* +* With these changes, the SoCrates can boot and re-boot +* (reset) from SPI NOR flash without any problems. +*/ + ret =spi_flash_cmd (spi,CMD_RESET_ENABLE ,NULL ,0 ); +if (ret) { +printf ("SF: Failed issue reset command\n" ); + goto err_read_id;
drivers/mtd/spi/sf_probe.c static struct spi_flash *spi_flash_probe_slave(struct spi_slave *spi) · line 387
Quoted 10 lines
drivers/mtd/spi/sf_probe.c +10
-0+ } + + ret =spi_flash_cmd (spi,CMD_RESET_MEMORY ,NULL ,0 ); +if (ret) { +printf ("SF: Failed issue reset command\n" ); + goto err_read_id; + } + +printf ("SF: Device software reset\n" ); +#endif
I have tested N25Q256A on zynq, couldn't encounter this either. This seems to be SPI flash logic more hardwired to N25Q256A.
Any comments?
Quoted 7 lines
drivers/mtd/spi/sf_probe.c +1
-1+#ifdef CONFIG_OF_CONTROL if (spi_flash_decode_fdt (gd->fdt_blob, flash)) {debug ("SF: FDT decode error\n" ); --2.1.1
thanks!
Signature · 1 line
Jagan.
v3 cover arm: socfpga: Add Cadence QSPI support
In reply to Stefan RoeseQuoted 15 lines · Stefan Roese
Hi!
So this is my 3rd posting regarding the Candence SPI driver on SoCFPGA.
The current status is, that SPI NOR flash works now without problems. And dcache is still enabled. The previous disabling was not needed. And only caused problems while booting into Linux. No cache flush or invalidate is needed. As no DMA engine is involved in this transfer.
The "solution" I used to get SPI NOR flash support working now is implemented in patch 0004 (still marked as RFC). Here a software reset is issued on the Micron N25Q256A SPI NOR flash. This seems to solve all problems and reading / writing to the SPI NOR flash seems to work now just fine. Even rebooting via pushbutton-reset or reset command works.
Quoted 4 lines
I think this version is now in much better state. Thats why I removed the WIP from the subject lines.
Again, this is tested on the EBV SoCrates eval board.
I picked this up into u-boot-socfpga:topic/drivers/qspi-wip-20141003 so it doesn't get lost. Also, they are updated to use the latest socfpga stuff, esp. since the socfpga_cyclone5_common.h was renamed to socfpga_common.h .
Best regards,
Marek Vasut