[U-Boot] Reg. CFI flash_init and hardware write protected devices
We have a board that feature NOR flash and hardware write protection is handled by controlling the write enable pin. When write protection is enabled, the nWE pin is forced high by external logic. The memory controller and/or CFI logic is unaware of this, and since CFI uses write enable as part of the CFI command set, a CFI probing will fail when write protection is enabled.
The rationale for controlling nWE and not WP (write protection) is that WP only protects the first sector of the device. In our case this is less than the size of the bootloader (U-boot), since that occupies two sectors. Due to this the built-in NOR write protection is rather useless.
Our current solution based on controlling nWE is to hardcode flash geometry in board code when flash protection is enabled. In order to use CFI as intended when write protection is disabled, we call the generic flash_init function as defined in drivers/mtd/cfi_flash.c. When protection is enabled we hardcode the geometry info in board code. In order separate our flash_init and the generic flash_init, and be able to call both, we've introduced a new ifdef to cfi_flash.c called CONFIG_CFI_FLASH_OVERRIDE_INIT. Like this:
----
diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c index 6039e1f..772096e 100644 --- a/drivers/mtd/cfi_flash.c +++ b/drivers/mtd/cfi_flash.c @@ -176,6 +176,10 @@ u64 flash_read64(void *addr)__attribute__((weak, alias("__flash_read64"))); #define flash_read64 __flash_read64 #endif
+#ifdef CONFIG_CFI_FLASH_OVERRIDE_INIT +#define flash_init __flash_init +#endif + /*----------------------------------------------------------------------- */ #if defined(CONFIG_ENV_IS_IN_FLASH) || defined(CONFIG_ENV_ADDR_REDUND) || (CONFIG_SYS_MONITOR_BASE >=
----
Now, in board code our redefined flash_init will be called. But if write protection is off, we call the original function, eg. __flash_init.
Using the preprocessor is often considered bad design. However, the alternatives here such as adding a weak attribute to flash_init will not make us able to call the generic/original function. Therefore we consider adding an ifdef as better design than making the function weak, and it'll reduce the amount of redundant code in board code.
What do you think about this?
Best regards, Frank E. Svendsbøe
On Tuesday, May 31, 2011 04:35:17 Frank Svendsbøe wrote:
Now, in board code our redefined flash_init will be called. But if write protection is off, we call the original function, eg. __flash_init.
if your code is simply setting the pin high at init time and then never bringing it back down, then just do that in your board_early_init_f func. -mike
Hi Mike,
On Tue, May 31, 2011 at 2:49 PM, Mike Frysinger vapier@gentoo.org wrote:
On Tuesday, May 31, 2011 04:35:17 Frank Svendsbøe wrote:
Now, in board code our redefined flash_init will be called. But if write protection is off, we call the original function, eg. __flash_init.
if your code is simply setting the pin high at init time and then never bringing it back down, then just do that in your board_early_init_f func. -mike
No this is not possible, and I'm sorry for not explaining it better.
The pin is not controlled by me. It is implicitly set/cleared via the memory controller, but it doesn't matter because we have external logic forcing it high.
Having the possibility to control this pin in software wouldn't make it "hardware write protection" would it?
The protection state is kept by an external AVR and in order to enable write access both a jumper and a write protection flag must be set.
Best regards, Frank
On Tuesday, May 31, 2011 09:25:54 Frank Svendsbøe wrote:
Having the possibility to control this pin in software wouldn't make it "hardware write protection" would it?
yes, it would. "software write protection" is what u-boot offers today -- the software checks the addresses before allowing writes. hardware write protection is where u-boot puts out the write command anyways the request is rejected by the hardware. just because you have a knob to toggle the hardware write protection doesnt mean it is no longer protected in hardware. -mike
Hi Frank,
On Tuesday 31 May 2011 10:35:17 Frank Svendsbøe wrote:
We have a board that feature NOR flash and hardware write protection is handled by controlling the write enable pin. When write protection is enabled, the nWE pin is forced high by external logic. The memory controller and/or CFI logic is unaware of this, and since CFI uses write enable as part of the CFI command set, a CFI probing will fail when write protection is enabled.
The rationale for controlling nWE and not WP (write protection) is that WP only protects the first sector of the device. In our case this is less than the size of the bootloader (U-boot), since that occupies two sectors. Due to this the built-in NOR write protection is rather useless.
Understood. But why don't you disable write-protection when you first call flash_init()? And enable the write-protection after the chip is correctly detected?
Our current solution based on controlling nWE is to hardcode flash geometry in board code when flash protection is enabled. In order to use CFI as intended when write protection is disabled, we call the generic flash_init function as defined in drivers/mtd/cfi_flash.c.
How is write-protection enabled/disabled on your board?
When protection is enabled we hardcode the geometry info in board code. In order separate our flash_init and the generic flash_init, and be able to call both, we've introduced a new ifdef to cfi_flash.c called CONFIG_CFI_FLASH_OVERRIDE_INIT. Like this:
diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c index 6039e1f..772096e 100644 --- a/drivers/mtd/cfi_flash.c +++ b/drivers/mtd/cfi_flash.c @@ -176,6 +176,10 @@ u64 flash_read64(void *addr)__attribute__((weak, alias("__flash_read64"))); #define flash_read64 __flash_read64 #endif
+#ifdef CONFIG_CFI_FLASH_OVERRIDE_INIT +#define flash_init __flash_init +#endif
/*----------------------------------------------------------------------- */ #if defined(CONFIG_ENV_IS_IN_FLASH) || defined(CONFIG_ENV_ADDR_REDUND) || (CONFIG_SYS_MONITOR_BASE >=
Now, in board code our redefined flash_init will be called. But if write protection is off, we call the original function, eg. __flash_init.
Using the preprocessor is often considered bad design. However, the alternatives here such as adding a weak attribute to flash_init will not make us able to call the generic/original function. Therefore we consider adding an ifdef as better design than making the function weak, and it'll reduce the amount of redundant code in board code.
Why don't you think that you can't access the original function if it's defined as a weak default? This should work just fine, see for example ft_board_setup() in arch/powerpc/cpu/ppc4xx/fdt.c:
void __ft_board_setup(void *blob, bd_t *bd) { ... } void ft_board_setup(void *blob, bd_t *bd) __attribute__((weak, alias("__ft_board_setup")));
And then this weak default is overridden and still referenced in board/amcc/canyonlands/canyonlands.c:
void ft_board_setup(void *blob, bd_t *bd) { __ft_board_setup(blob, bd); ...
So no need for this ifdef in the common CFI driver. Or am I missing something here?
Best regards, Stefan
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office@denx.de
Hi Stefan,
On Tue, May 31, 2011 at 3:10 PM, Stefan Roese sr@denx.de wrote:
Hi Frank,
On Tuesday 31 May 2011 10:35:17 Frank Svendsbøe wrote:
We have a board that feature NOR flash and hardware write protection is handled by controlling the write enable pin. When write protection is enabled, the nWE pin is forced high by external logic. The memory controller and/or CFI logic is unaware of this, and since CFI uses write enable as part of the CFI command set, a CFI probing will fail when write protection is enabled.
The rationale for controlling nWE and not WP (write protection) is that WP only protects the first sector of the device. In our case this is less than the size of the bootloader (U-boot), since that occupies two sectors. Due to this the built-in NOR write protection is rather useless.
Understood. But why don't you disable write-protection when you first call flash_init()? And enable the write-protection after the chip is correctly detected?
Simply because disabling write-protection is not impossible after installation. Our device will be located 3000m below sea level.
As I explained Mike Frysinger, the write-protection settings is not controlled by the PPC device running U-Boot. We can enable write-protection in the lab (by setting a jumper), but not write software.
The whole purpose of this is to keep it "impossible" to destroy a factory default version. For "mutable" software, we utilize another flash.
Our current solution based on controlling nWE is to hardcode flash geometry in board code when flash protection is enabled. In order to use CFI as intended when write protection is disabled, we call the generic flash_init function as defined in drivers/mtd/cfi_flash.c.
How is write-protection enabled/disabled on your board?
Two ways/levels: 1) A hardware jumper on the factory default flash. 2) On the non-factory default flash, write protection is enabled/disabled by an FPGA and implicitly and AVR. To make it short, we cannot change protection scheme from U-Boot (but we can via an SPI driver I wrote for Linux).
When protection is enabled we hardcode the geometry info in board code. In order separate our flash_init and the generic flash_init, and be able to call both, we've introduced a new ifdef to cfi_flash.c called CONFIG_CFI_FLASH_OVERRIDE_INIT. Like this:
diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c index 6039e1f..772096e 100644 --- a/drivers/mtd/cfi_flash.c +++ b/drivers/mtd/cfi_flash.c @@ -176,6 +176,10 @@ u64 flash_read64(void *addr)__attribute__((weak, alias("__flash_read64"))); #define flash_read64 __flash_read64 #endif
+#ifdef CONFIG_CFI_FLASH_OVERRIDE_INIT +#define flash_init __flash_init +#endif
/*----------------------------------------------------------------------- */ #if defined(CONFIG_ENV_IS_IN_FLASH) || defined(CONFIG_ENV_ADDR_REDUND) || (CONFIG_SYS_MONITOR_BASE >=
Now, in board code our redefined flash_init will be called. But if write protection is off, we call the original function, eg. __flash_init.
Using the preprocessor is often considered bad design. However, the alternatives here such as adding a weak attribute to flash_init will not make us able to call the generic/original function. Therefore we consider adding an ifdef as better design than making the function weak, and it'll reduce the amount of redundant code in board code.
Why don't you think that you can't access the original function if it's defined as a weak default? This should work just fine, see for example ft_board_setup() in arch/powerpc/cpu/ppc4xx/fdt.c:
void __ft_board_setup(void *blob, bd_t *bd) { ... } void ft_board_setup(void *blob, bd_t *bd) __attribute__((weak, alias("__ft_board_setup")));
And then this weak default is overridden and still referenced in board/amcc/canyonlands/canyonlands.c:
void ft_board_setup(void *blob, bd_t *bd) { __ft_board_setup(blob, bd); ...
So no need for this ifdef in the common CFI driver. Or am I missing something here?
Oh. I didn't knew I could access the function that was overridden by the weak attribute. I guess that's the alias is for right? If both can be called, I'm happy to remove the ifdef.
I'll test that tomorrow and provide a patch if it works.
Best regards, Frank
Hi Frank,
On Tuesday 31 May 2011 15:55:56 Frank Svendsbøe wrote:
Understood. But why don't you disable write-protection when you first call flash_init()? And enable the write-protection after the chip is correctly detected?
Simply because disabling write-protection is not impossible after installation. Our device will be located 3000m below sea level.
I see.
As I explained Mike Frysinger, the write-protection settings is not controlled by the PPC device running U-Boot. We can enable write-protection in the lab (by setting a jumper), but not write software.
The whole purpose of this is to keep it "impossible" to destroy a factory default version. For "mutable" software, we utilize another flash.
Our current solution based on controlling nWE is to hardcode flash geometry in board code when flash protection is enabled. In order to use CFI as intended when write protection is disabled, we call the generic flash_init function as defined in drivers/mtd/cfi_flash.c.
How is write-protection enabled/disabled on your board?
Two ways/levels: 1) A hardware jumper on the factory default flash. 2) On the non-factory default flash, write protection is enabled/disabled by an FPGA and implicitly and AVR. To make it short, we cannot change protection scheme from U-Boot (but we can via an SPI driver I wrote for Linux).
Theoretically also possible with U-Boot. But I understand that you don't want to do this.
<snip>
Why don't you think that you can't access the original function if it's defined as a weak default? This should work just fine, see for example ft_board_setup() in arch/powerpc/cpu/ppc4xx/fdt.c:
void __ft_board_setup(void *blob, bd_t *bd) { ... } void ft_board_setup(void *blob, bd_t *bd) __attribute__((weak, alias("__ft_board_setup")));
And then this weak default is overridden and still referenced in board/amcc/canyonlands/canyonlands.c:
void ft_board_setup(void *blob, bd_t *bd) { __ft_board_setup(blob, bd); ...
So no need for this ifdef in the common CFI driver. Or am I missing something here?
Oh. I didn't knew I could access the function that was overridden by the weak attribute. I guess that's the alias is for right?
Yep.
If both can be called, I'm happy to remove the ifdef.
I'll test that tomorrow and provide a patch if it works.
Good luck...
Best regards, Stefan
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office@denx.de
Hi Stefan,
On Tue, May 31, 2011 at 4:37 PM, Stefan Roese sr@denx.de wrote:
Hi Frank,
On Tuesday 31 May 2011 15:55:56 Frank Svendsbøe wrote:
Understood. But why don't you disable write-protection when you first call flash_init()? And enable the write-protection after the chip is correctly detected?
Simply because disabling write-protection is not impossible after installation. Our device will be located 3000m below sea level.
I see.
Hmm.. then you read my mind :) I meant to say "is not possible" and not "is not impossible" :)
As I explained Mike Frysinger, the write-protection settings is not controlled by the PPC device running U-Boot. We can enable write-protection in the lab (by setting a jumper), but not write software.
.. and here I meant to say "explained to", and "but not via software".
Two ways/levels: 1) A hardware jumper on the factory default flash. 2) On the non-factory default flash, write protection is enabled/disabled by an FPGA and implicitly and AVR. To make it short, we cannot change protection scheme from U-Boot (but we can via an SPI driver I wrote for Linux).
.. and yet another self-correction: s/and implicitly and/and implicitly an/ .. guess I had too much coffee yesterday :)
Theoretically also possible with U-Boot. But I understand that you don't want to do this.
True. We could write a driver for U-Boot that could access the AVR and command the FPGA to disable write protection. But in the case for the "default factory flash" where the flash write protection is enabled by a demounted jumper, and cannot be modified after installation, CFI probing wouldn't work anyway. So I think the use for such a driver would be limited.
Note that for normal operation, our system is running from a non-hardware protected flash. But even in this mode, we must command the AVR to enable write access before we're permitted to write to it. This is one of the features supported by the SPI driver running in GNU/Linux.
In addition to this, we have the usual CFI/MTD protection scheme where the filesystems are mounted as read-only, etc.
<snip>
Why don't you think that you can't access the original function if it's defined as a weak default? This should work just fine, see for example ft_board_setup() in arch/powerpc/cpu/ppc4xx/fdt.c:
void __ft_board_setup(void *blob, bd_t *bd) { ... } void ft_board_setup(void *blob, bd_t *bd) __attribute__((weak, alias("__ft_board_setup")));
And then this weak default is overridden and still referenced in board/amcc/canyonlands/canyonlands.c:
void ft_board_setup(void *blob, bd_t *bd) { __ft_board_setup(blob, bd); ...
So no need for this ifdef in the common CFI driver. Or am I missing something here?
Oh. I didn't knew I could access the function that was overridden by the weak attribute. I guess that's the alias is for right?
Yep.
If both can be called, I'm happy to remove the ifdef.
I'll test that tomorrow and provide a patch if it works.
Good luck...
Thanks, this worked for me. Is it ok for you too?
diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c index 6039e1f..99360af 100644 --- a/drivers/mtd/cfi_flash.c +++ b/drivers/mtd/cfi_flash.c @@ -176,6 +176,7 @@ u64 flash_read64(void *addr)__attribute__((weak, alias("__flash_read64"))); #define flash_read64 __flash_read64 #endif
+ /*----------------------------------------------------------------------- */ #if defined(CONFIG_ENV_IS_IN_FLASH) || defined(CONFIG_ENV_ADDR_REDUND) || (CONFIG_SYS_MONITOR_BASE >= CONFIG_SYS_FLASH_BASE) @@ -2151,7 +2152,8 @@ void flash_protect_default(void) #endif }
-unsigned long flash_init (void) +unsigned long flash_init(void) __attribute__((weak, alias("__flash_init"))); +unsigned long __flash_init (void) { unsigned long size = 0; int i;
Hi Frank,
On Wednesday 01 June 2011 16:33:14 Frank Svendsbøe wrote:
Simply because disabling write-protection is not impossible after installation. Our device will be located 3000m below sea level.
I see.
Hmm.. then you read my mind :) I meant to say "is not possible" and not "is not impossible" :)
Yep. I read so fast, that I didn't catch the misspelled words. ;)
Thanks, this worked for me. Is it ok for you too?
diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c index 6039e1f..99360af 100644 --- a/drivers/mtd/cfi_flash.c +++ b/drivers/mtd/cfi_flash.c @@ -176,6 +176,7 @@ u64 flash_read64(void *addr)__attribute__((weak, alias("__flash_read64"))); #define flash_read64 __flash_read64 #endif
Don't add this empty line.
/*----------------------------------------------------------------------- */ #if defined(CONFIG_ENV_IS_IN_FLASH) || defined(CONFIG_ENV_ADDR_REDUND) || (CONFIG_SYS_MONITOR_BASE >= CONFIG_SYS_FLASH_BASE) @@ -2151,7 +2152,8 @@ void flash_protect_default(void) #endif }
-unsigned long flash_init (void) +unsigned long flash_init(void) __attribute__((weak, alias("__flash_init"))); +unsigned long __flash_init (void) { unsigned long size = 0; int i;
Looks good. I have no objections. Please go ahead and send this as a "proper" patch.
Best regards, Stefan
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office@denx.de
On Wed, Jun 1, 2011 at 5:34 PM, Stefan Roese sr@denx.de wrote:
Hi Frank,
On Wednesday 01 June 2011 16:33:14 Frank Svendsbøe wrote:
Simply because disabling write-protection is not impossible after installation. Our device will be located 3000m below sea level.
I see.
Hmm.. then you read my mind :) I meant to say "is not possible" and not "is not impossible" :)
Yep. I read so fast, that I didn't catch the misspelled words. ;)
Thanks, this worked for me. Is it ok for you too?
diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c index 6039e1f..99360af 100644 --- a/drivers/mtd/cfi_flash.c +++ b/drivers/mtd/cfi_flash.c @@ -176,6 +176,7 @@ u64 flash_read64(void *addr)__attribute__((weak, alias("__flash_read64"))); #define flash_read64 __flash_read64 #endif
Don't add this empty line.
Thanks for spotting this. I'll remove this of course.
/*----------------------------------------------------------------------- */ #if defined(CONFIG_ENV_IS_IN_FLASH) || defined(CONFIG_ENV_ADDR_REDUND) || (CONFIG_SYS_MONITOR_BASE >= CONFIG_SYS_FLASH_BASE) @@ -2151,7 +2152,8 @@ void flash_protect_default(void) #endif }
-unsigned long flash_init (void) +unsigned long flash_init(void) __attribute__((weak, alias("__flash_init"))); +unsigned long __flash_init (void) { unsigned long size = 0; int i;
Looks good. I have no objections. Please go ahead and send this as a "proper" patch.
Will do (as soon as I get back to work).
Best regards, Frank.
On Wed, Jun 1, 2011 at 6:59 PM, Frank Svendsbøe frank.svendsboe@gmail.com wrote:
On Wed, Jun 1, 2011 at 5:34 PM, Stefan Roese sr@denx.de wrote:
Hi Frank,
On Wednesday 01 June 2011 16:33:14 Frank Svendsbøe wrote:
Simply because disabling write-protection is not impossible after installation. Our device will be located 3000m below sea level.
I see.
Hmm.. then you read my mind :) I meant to say "is not possible" and not "is not impossible" :)
Yep. I read so fast, that I didn't catch the misspelled words. ;)
Thanks, this worked for me. Is it ok for you too?
diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c index 6039e1f..99360af 100644 --- a/drivers/mtd/cfi_flash.c +++ b/drivers/mtd/cfi_flash.c @@ -176,6 +176,7 @@ u64 flash_read64(void *addr)__attribute__((weak, alias("__flash_read64"))); #define flash_read64 __flash_read64 #endif
Don't add this empty line.
Thanks for spotting this. I'll remove this of course.
/*----------------------------------------------------------------------- */ #if defined(CONFIG_ENV_IS_IN_FLASH) || defined(CONFIG_ENV_ADDR_REDUND) || (CONFIG_SYS_MONITOR_BASE >= CONFIG_SYS_FLASH_BASE) @@ -2151,7 +2152,8 @@ void flash_protect_default(void) #endif }
-unsigned long flash_init (void) +unsigned long flash_init(void) __attribute__((weak, alias("__flash_init"))); +unsigned long __flash_init (void) { unsigned long size = 0; int i;
Looks good. I have no objections. Please go ahead and send this as a "proper" patch.
Will do (as soon as I get back to work).
Best regards, Frank.
Hi Stefan, A few weeks ago, I applied this change and everything worked fine in U-Boot. However, I didn't succeed doing the same hack in Linux. I've done some hardcoding experiments in drivers/mtd/chips/cfi_probe.c, but so far no success. After reading some CFI manuals, it seems CFI will/should use bus write operations even during CFI read operations. Can you confirm this? Then my next question is how come U-Boot read operations works by just hardcoding the flash geometry. It seems U-Boot utilize just plain localbus accesses to read flash data, ie. not the "CFI protocol". What did I miss here.. ?
Now I'm considering two options: 1) Either write a custom flash map driver, and access the flash the same way U-Boot does, or 2) try to convince the FPGA designers that our way of forcing write protection is not according the the CFI specifications, and should be done otherwise. What do you think is the best solution?
Is there anybody else out there supporting hardware write protection on CFI devices?
Best regards, Frank
Dear Frank,
In message BANLkTinFMA7FN-BD7z2-ZYAT+-ubWn7S9Q@mail.gmail.com you wrote:
A few weeks ago, I applied this change and everything worked fine in U-Boot. However, I didn't succeed doing the same hack in Linux. I've done some hardcoding experiments in drivers/mtd/chips/cfi_probe.c, but so far no success. After reading some CFI manuals, it seems CFI will/should use bus write operations even during CFI read operations.
What exactly do you mean by "CFI read operations"?
Can you confirm this? Then my next question is how come U-Boot read operations works by just hardcoding the flash geometry. It seems U-Boot utilize just plain localbus accesses to read flash data, ie. not the "CFI protocol". What did I miss here.. ?
It seems you misunderstand the different modes. In command mode, you will use the specific command sequences defined in the CFI protocol to talk to the controller poart of the flash chip. In this mode, you can read for example query information, flash geometry data, status information, etc. Yes, any such command sequence includes write operations to the flash device, even when you actually want to read some specific data. In data mode, flash is working just as normal real-only memory. No specific protocol is used, you just use plain read accesses to read the user data programmed into the flash chip.
Any kind of probing, auto-identification etc. of the flash chip will need to use CFI command sequences, and thus write access to the device. As long as you just want to read the data or execute code from the flash no write accesses are needed.
Now I'm considering two options: 1) Either write a custom flash map driver, and access the flash the same way U-Boot does, or 2) try to convince the FPGA designers that our way of forcing write protection is not according the the CFI specifications, and should be done otherwise. What do you think is the best solution?
Linux can also read from ROM memory. You just have to make sure that you do not try to run the normal CFI flash drivers on your flash devices - these will not work, because they will try to probe the flash chips.
Is there anybody else out there supporting hardware write protection on CFI devices?
CFI flash chips with the level of write protection you implemented have to be dealt with in the same way as other ROM memory. Normal CFI driver are not appropriate here.
Best regards,
Wolfgang Denk
On Thu, Jun 23, 2011 at 5:21 PM, Wolfgang Denk wd@denx.de wrote:
Dear Frank,
In message BANLkTinFMA7FN-BD7z2-ZYAT+-ubWn7S9Q@mail.gmail.com you wrote:
A few weeks ago, I applied this change and everything worked fine in U-Boot. However, I didn't succeed doing the same hack in Linux. I've done some hardcoding experiments in drivers/mtd/chips/cfi_probe.c, but so far no success. After reading some CFI manuals, it seems CFI will/should use bus write operations even during CFI read operations.
What exactly do you mean by "CFI read operations"?
Good question. When reading Intels CFI manual I was referring to something called "read array" which used bus write commands. I guess normal byte/word read access don't utilize this then.
Can you confirm this? Then my next question is how come U-Boot read operations works by just hardcoding the flash geometry. It seems U-Boot utilize just plain localbus accesses to read flash data, ie. not the "CFI protocol". What did I miss here.. ?
It seems you misunderstand the different modes. In command mode, you will use the specific command sequences defined in the CFI protocol to talk to the controller poart of the flash chip. In this mode, you can read for example query information, flash geometry data, status information, etc. Yes, any such command sequence includes write operations to the flash device, even when you actually want to read some specific data. In data mode, flash is working just as normal real-only memory. No specific protocol is used, you just use plain read accesses to read the user data programmed into the flash chip.
Right, and thanks for the explanation. What I've done to U-Boot is bypass the probing which uses the command mode and thus avoid getting problems with bus write accesses when flash protection is on.
Any kind of probing, auto-identification etc. of the flash chip will need to use CFI command sequences, and thus write access to the device. As long as you just want to read the data or execute code from the flash no write accesses are needed.
Now I'm considering two options: 1) Either write a custom flash map driver, and access the flash the same way U-Boot does, or 2) try to convince the FPGA designers that our way of forcing write protection is not according the the CFI specifications, and should be done otherwise. What do you think is the best solution?
Linux can also read from ROM memory. You just have to make sure that you do not try to run the normal CFI flash drivers on your flash devices - these will not work, because they will try to probe the flash chips.
Yes, well I tried to fool Linux's CFI driver too, but no luck so far. It was after this I started reading about CFI and came to the (wrong) conclusion that CFI uses bus write operations during read accesses too.
Is there anybody else out there supporting hardware write protection on CFI devices?
CFI flash chips with the level of write protection you implemented have to be dealt with in the same way as other ROM memory. Normal CFI driver are not appropriate here.
Ok. So you recommend I stop "hacking" the CFI probing and instead write a custom flash map driver?
Best regards, Frank
Dear Frank,
In message BANLkTi=p9gdOGOETWojRqkmrxXM7Do_tBg@mail.gmail.com you wrote:
CFI flash chips with the level of write protection you implemented have to be dealt with in the same way as other ROM memory. =A0Normal CFI driver are not appropriate here.
Ok. So you recommend I stop "hacking" the CFI probing and instead write a custom flash map driver?
No. Never write new drivers when you can use existing ones :-)
Describe the flash memory as "mtd-ram" (instead of "cfi-flash") in the device tree and select the physmap driver in your kernel config.
Best regards,
Wolfgang Denk
On Thu, Jun 23, 2011 at 7:55 PM, Wolfgang Denk wd@denx.de wrote:
Dear Frank,
In message BANLkTi=p9gdOGOETWojRqkmrxXM7Do_tBg@mail.gmail.com you wrote:
CFI flash chips with the level of write protection you implemented have to be dealt with in the same way as other ROM memory. =A0Normal CFI driver are not appropriate here.
Ok. So you recommend I stop "hacking" the CFI probing and instead write a custom flash map driver?
No. Never write new drivers when you can use existing ones :-)
Describe the flash memory as "mtd-ram" (instead of "cfi-flash") in the device tree and select the physmap driver in your kernel config.
Oh. I will try that. Thanks a lot for this tip :)
.. and sorry for asking about Linux related topics on the U-Boot ML.
Best regards, Frank
Ok. So you recommend I stop "hacking" the CFI probing and instead write a custom flash map driver?
No. Never write new drivers when you can use existing ones :-)
Describe the flash memory as "mtd-ram" (instead of "cfi-flash") in the device tree and select the physmap driver in your kernel config.
Hi Wolfgang, I did as you recommended and replaced cfi-flash with mtd-ram in the device tree. I also defined CONFIG_MTD_PHYSMAP_OF, CONFIG_MTD_MTDRAM, CONFIG_MTDRAM_TOTAL_SIZE according to our specifications. The default erase-size was 128k, which is what we have too, so I didn't touch that. Now when I boot the kernel recognizes all the partitions I've defined in the dts. But, when mounting a jffs2-filesystem, it ends with a jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found...
Do you have any other tips?
When working with this, I realised that if I could get it to work we'd still might have a problem. You see, we need write access for one of the flashes when upgrading software. We can't treat this as a simple ROM. So do we need CFI working in order to set the device into write-mode, erase flash sectors, etc.? Or do mtd-ram handle flash write operations too?
In theory, I guess I could unmount the root fs, unload the mtd module, insert the cfi-flash module, mount the filesystem, then write, etc.. But isn't that harder than write a custom map driver?
Best regards, Frank
Dear =?ISO-8859-1?Q?Frank_Svendsb=F8e?=,
In message BANLkTimt0LYXr-B5PAr1ONEaEoWKCyEHbw@mail.gmail.com you wrote:
Hi Wolfgang, I did as you recommended and replaced cfi-flash with mtd-ram in the device tree. I also defined CONFIG_MTD_PHYSMAP_OF, CONFIG_MTD_MTDRAM, CONFIG_MTDRAM_TOTAL_SIZE according to our specifications. The default erase-size was 128k, which is what we have too, so I didn't touch that. Now when I boot the kernel recognizes all the partitions I've defined in the dts. But, when mounting a jffs2-filesystem, it ends with a jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found...
Do you have any other tips?
Difficult to speculate - I don't know your hardware (eventually you have two 16 bit flash chips in parallel to build a 32 bit bus, and have to double the chip's erase block size?), and I don;t know how you created the JFFS2 file system.
Are you sure you want to use JFFS2? UBIFS is considered to be a better choice these days...
When working with this, I realised that if I could get it to work we'd still might have a problem. You see, we need write access for one of the flashes when upgrading software. We can't treat this as a simple ROM. So do we need CFI working in order to set the device into write-mode, erase flash sectors, etc.? Or do mtd-ram handle flash write operations too?
mtd-ram provides a pure memory interface, i. e. you cannot use this to erase or program any blocks in a CFI flash device. To do so, you need the CFI driver.
In theory, I guess I could unmount the root fs, unload the mtd module, insert the cfi-flash module, mount the filesystem, then write, etc..
Yes, or you could start with the CFI driver in the first place.
But isn't that harder than write a custom map driver?
I consider your chances to get such a customdriver into mainline to be epsilon. And you don't really want to use any out of tree drivers.
Best regards,
Wolfgang Denk
On Fri, Jun 24, 2011 at 4:26 PM, Wolfgang Denk wd@denx.de wrote:
Dear =?ISO-8859-1?Q?Frank_Svendsb=F8e?=,
In message BANLkTimt0LYXr-B5PAr1ONEaEoWKCyEHbw@mail.gmail.com you wrote:
Hi Wolfgang, I did as you recommended and replaced cfi-flash with mtd-ram in the device tree. I also defined CONFIG_MTD_PHYSMAP_OF, CONFIG_MTD_MTDRAM, CONFIG_MTDRAM_TOTAL_SIZE according to our specifications. The default erase-size was 128k, which is what we have too, so I didn't touch that. Now when I boot the kernel recognizes all the partitions I've defined in the dts. But, when mounting a jffs2-filesystem, it ends with a jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found...
Do you have any other tips?
Difficult to speculate - I don't know your hardware (eventually you have two 16 bit flash chips in parallel to build a 32 bit bus, and have to double the chip's erase block size?), and I don;t know how you created the JFFS2 file system.
Yes I know, and I don't expect you to find the answer based on so little data. FYI: We have two flash chips yes, but not to build a 32-bit bus, we have two for redundancy issues (one is storing a factory default version). 16 bits databus and each sector is 64k words, ie. 128k bytes. I created the jffs2 images using ELDKs mkfs.jffs2. Thanks for providing me this tool :)
Are you sure you want to use JFFS2? UBIFS is considered to be a better choice these days...
No, not sure. I've got two reasons for staying with JFFS2. 1) We normally don't write to flash except when upgrading, so flash performance/duration isn't very critical 2) I discovered UBIFS after I had JFFS2 working and haven't seen the real need for it yet.
When working with this, I realised that if I could get it to work we'd still might have a problem. You see, we need write access for one of the flashes when upgrading software. We can't treat this as a simple ROM. So do we need CFI working in order to set the device into write-mode, erase flash sectors, etc.? Or do mtd-ram handle flash write operations too?
mtd-ram provides a pure memory interface, i. e. you cannot use this to erase or program any blocks in a CFI flash device. To do so, you need the CFI driver.
Right. mtd-ram is thus a dead-end solution for our purpose. We need to be able to write in some specific cases.
In theory, I guess I could unmount the root fs, unload the mtd module, insert the cfi-flash module, mount the filesystem, then write, etc..
Yes, or you could start with the CFI driver in the first place.
That's what I did before you started interrupting me about mtd-ram... No, just kidding :-)
Did you mean I should adapt the general CFI driver to support our configuration, or... Based on what you say below I'm not sure what you actually mean.
But isn't that harder than write a custom map driver?
I consider your chances to get such a customdriver into mainline to be epsilon. And you don't really want to use any out of tree drivers.
Me too. Small chances. So we have two options I guess: 1) Ditch hardware write protection and use CFI as it is supposed to be used, or 2) write a custom out-of-tree driver. What would you go for?
I'll consider if and how to get things into mainline once we have things working as we want. If my port isn't accepted I will at least learn why. Maybe the next will :)
Best regards, Frank
Dear Frank,
In message BANLkTindk4f2An2VqhSftPcGuvsEqLY7wQ@mail.gmail.com you wrote:
Difficult to speculate - I don't know your hardware (eventually you have two 16 bit flash chips in parallel to build a 32 bit bus, and have to double the chip's erase block size?), and I don;t know how you created the JFFS2 file system.
...
factory default version). 16 bits databus and each sector is 64k words, ie. 128k bytes. I created the jffs2 images using ELDKs mkfs.jffs2. Thanks for providing me this tool :)
You need to provide the correct parameters to mkfs.jffs2...
No, not sure. I've got two reasons for staying with JFFS2. 1) We normally don't write to flash except when upgrading, so flash performance/duration isn't very critical 2) I discovered UBIFS after I
Hm... JFFS2 is probaly not a good choice anyway. On NOR, where you don't have to deal with bad blocks, you might use ext2 or cramfs or ... - all of them are more efficient than JFFS2 then.
had JFFS2 working and haven't seen the real need for it yet.
Mount time?
Did you mean I should adapt the general CFI driver to support our configuration, or... Based on what you say below I'm not sure what you actually mean.
You have two different use cases that require (if we don't want to use a proprietary driver) two different, mutually exclusive drivers. I would load the driver I need as module, and unload it if I need the other one. This is pretty simple in principle, but a bit ore of a challange in your case as you use this as a root file system (including to load the modules from).
Me too. Small chances. So we have two options I guess: 1) Ditch hardware write protection and use CFI as it is supposed to be used, or 2) write a custom out-of-tree driver. What would you go for?
If this is an option: 1)
Best regards,
Wolfgang Denk
On Fri, Jun 24, 2011 at 10:26 PM, Wolfgang Denk wd@denx.de wrote:
Dear Frank,
In message BANLkTindk4f2An2VqhSftPcGuvsEqLY7wQ@mail.gmail.com you wrote:
Difficult to speculate - I don't know your hardware (eventually you have two 16 bit flash chips in parallel to build a 32 bit bus, and have to double the chip's erase block size?), and I don;t know how you created the JFFS2 file system.
...
factory default version). 16 bits databus and each sector is 64k words, ie. 128k bytes. I created the jffs2 images using ELDKs mkfs.jffs2. Thanks for providing me this tool :)
You need to provide the correct parameters to mkfs.jffs2...
Of course, but I don't see how changing from CFI to raw memory accesses on target change the way the jffs2 image should be created. What didn't I understand?
I'm currently using: mkfs.jss2 -U -b -d /${input} -e 0x20000 -o ${output}
No, not sure. I've got two reasons for staying with JFFS2. 1) We normally don't write to flash except when upgrading, so flash performance/duration isn't very critical 2) I discovered UBIFS after I
Hm... JFFS2 is probaly not a good choice anyway. On NOR, where you don't have to deal with bad blocks, you might use ext2 or cramfs or ... - all of them are more efficient than JFFS2 then.
Oh. Thanks for letting me know. I knew cramfs would be more efficient, but surprised that ext2 would be too. Thought ext2 was "heavier".
had JFFS2 working and haven't seen the real need for it yet.
Mount time?
Maybe 1-2 seconds. Haven't measured it yet. Maybe boot-time could've been improved yes.
Did you mean I should adapt the general CFI driver to support our configuration, or... Based on what you say below I'm not sure what you actually mean.
You have two different use cases that require (if we don't want to use a proprietary driver) two different, mutually exclusive drivers. I would load the driver I need as module, and unload it if I need the other one. This is pretty simple in principle, but a bit ore of a challange in your case as you use this as a root file system (including to load the modules from).
When thinking a bit more about this, I realize we don't need to support writing to the current filesystem in use. I've partitioned our device into three region, where each "region" consists of four partitions (dtb, kernel, rootfs and "home"). When upgrading we write a complete "region image" into another flash region, which is not part of the region we're running. So maybe we could have both mtd-ram and cfi-flash running at the same time, since we don't actually need CFI for the flash we're currently booting from.
I currently use mtd-utils' flash_erase to erase, and 'dd if=/dev/mtdX' to write. The mtds is created based on info in the dts.. Regarding DTS: Is it possible to have different partitions within the same physical flash using different drivers? Like this:
flash@0,0 { compatible = "amd,s29gl512n", "cfi-flash", reg = <0x0 0x0 0x4000000>; partition@... }
... into something like this:
flash@0,0 { compatible = "amd,s29gl512n", "cfi-flash", reg = <0x0, 0x0 0x2000000>; partition@.. }
flash@0,2000000 { compatible = "amd,s29gl512n", "mtd-ram", reg = <0x2000000, 0x0, 0x2000000>; partition@ }
In other words, partition a flash to use different drivers when accessing different partitions? I think that could solve the problems.
Me too. Small chances. So we have two options I guess: 1) Ditch hardware write protection and use CFI as it is supposed to be used, or 2) write a custom out-of-tree driver. What would you go for?
If this is an option: 1)
Good to know your opinion on this.
Best regards,
Wolfgang Denk
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de Do not simplify the design of a program if a way can be found to make it complex and wonderful.
Hehe. Who wrote that?
Best regards, Frank
participants (4)
-
Frank Svendsbøe -
Mike Frysinger -
Stefan Roese -
Wolfgang Denk