[Openocd-development] STM32: "flash write_image" has an alignment issue and flash protect/erase is broken

[Johnny Halfmoon]

Hiya,

I've found a few issues in OpenOCD and thought that I'd share before hacking a solution together. The following revision is used:

commit 8f446fcf676e9cd13cf53d9946f0cae5d29a10ec
Date: Thu Nov 19 13:23:49 2009 -0800

When doing "flash write_image erase unlock image.bin 0x8000000 bin" on an STM32, the following error is generated:

auto erase enabled
auto unlock enabled
Info : device id = 0x20036410
Info : flash size = 128kbytes
Warn : Error: start and end sectors must be on a 4 sector boundary
Error: failed setting protection for areas 0 to 21 (-901)

So it seems we need write_image to be a bit smarter and do some boundary aligning before doing the unprotect / erase . The way I was thinking to do this is to add a configuration parameter to OpenOCD which tells it to be smart ( or not ) when encountering alignment issues like this one. For example "smartalign on" and "smartalign off". "smartalign on" will make OpenOCD adjust alignment automatically to be valid. Anyone go a better idea?

Also I've noticed that the following two commands have stopped working, at least on STM32:

- flash protect 0 0 last off
- flash erase_sector 0 0 last

OpenOCD just exits and the -d parameter only yields this:

User : 208 1276 command.c:608 jim_echo(): ---Removing write protection---
Debug: 210 1279 command.c:64 script_debug(): command - protect
Debug: 211 1279 command.c:74 script_debug(): protect - argv[0]=ocd_flash_protect
Debug: 212 1279 command.c:74 script_debug(): protect - argv[1]=0
Debug: 213 1279 command.c:74 script_debug(): protect - argv[2]=0
Debug: 214 1279 command.c:74 script_debug(): protect - argv[3]=last
Debug: 215 1279 command.c:74 script_debug(): protect - argv[4]=off
User : 217 1282 command.c:675 openocd_jim_vfprintf():
User : 219 1282 command.c:675 openocd_jim_vfprintf():
User : 221 1282 command.c:675 openocd_jim_vfprintf():
User : 223 1282 command.c:675 openocd_jim_vfprintf():
User : 224 1283 command.c:675 openocd_jim_vfprintf():
User : 227 1283 command.c:675 openocd_jim_vfprintf():
User : 229 1283 command.c:675 openocd_jim_vfprintf():
User : 231 1283 command.c:675 openocd_jim_vfprintf():
User : 233 1283 command.c:675 openocd_jim_vfprintf():
User : 235 1283 command.c:675 openocd_jim_vfprintf():
User : 237 1283 command.c:675 openocd_jim_vfprintf():
make: *** [flash] Error 1

and

User : 207 1027 command.c:608 jim_echo(): ---Erasing---
Debug: 209 1029 command.c:64 script_debug(): command - erase_sector
Debug: 210 1029 command.c:74 script_debug(): erase_sector - argv[0]=ocd_flash_erase_sector
Debug: 211 1029 command.c:74 script_debug(): erase_sector - argv[1]=0
Debug: 212 1029 command.c:74 script_debug(): erase_sector - argv[2]=0
Debug: 213 1029 command.c:74 script_debug(): erase_sector - argv[3]=last
User : 215 1032 command.c:675 openocd_jim_vfprintf():
User : 216 1032 command.c:675 openocd_jim_vfprintf():
User : 219 1032 command.c:675 openocd_jim_vfprintf():
User : 221 1032 command.c:675 openocd_jim_vfprintf():
User : 223 1032 command.c:675 openocd_jim_vfprintf():
User : 225 1032 command.c:675 openocd_jim_vfprintf():
User : 227 1032 command.c:675 openocd_jim_vfprintf():
User : 229 1032 command.c:675 openocd_jim_vfprintf():
make: *** [flash] Error 1

OpenOCD apparently exits with an error, but why; I don't know. The mass_erase command, on the other hand, does work as it should. I've yet do dig in to this, but for lack of time, haven't done so yet. Perhaps somebody already knows the answer. If not, I'll try and find the cause some time next week.

Cheers,

Johnny

[Øyvind Harboe]

On Thu, Nov 19, 2009 at 11:46 PM, Johnny Halfmoon

Post by Johnny Halfmoon
Hiya,
commit 8f446fcf676e9cd13cf53d9946f0cae5d29a10ec
Date:   Thu Nov 19 13:23:49 2009 -0800
auto erase enabled
auto unlock enabled
Info : device id = 0x20036410
Info : flash size = 128kbytes
Warn : Error: start and end sectors must be on a 4 sector boundary
Error: failed setting protection for areas 0 to 21 (-901)
So it seems we need write_image to be a bit smarter and do some boundary
aligning before doing the unprotect / erase.

Could you include a debug_level 3 trace?

The conclusion seems a bit premature based on the limited information
above.

[Johnny Halfmoon]

Hi,

Post by Øyvind Harboe
Could you include a debug_level 3 trace?
The conclusion seems a bit premature based on the limited information
above.

Maybe the following example (output is pasted below) can illustrate the issue a bit better. First I try to flash a 20KB block, then a 21KB block and finally a 24KB block. Only the blocks of 20 and 24 succeed, because they happen to be nicely 4K alligned. All blocks that are not 4K alligned will fail. OpenOCD tries to erase, (un)protect and flash only those sectors that are modified, but doesn't take into account that some platforms require certain alligments. Debug level 3 yields nothing interresting:

User : 209 1107 command.c:608 jim_echo(): ---Flashing---
Debug: 211 1110 command.c:64 script_debug(): command - write_image
Debug: 212 1110 command.c:74 script_debug(): write_image - argv[0]=ocd_flash_write_image
Debug: 213 1110 command.c:74 script_debug(): write_image - argv[1]=erase
Debug: 214 1110 command.c:74 script_debug(): write_image - argv[2]=unlock
Debug: 215 1110 command.c:74 script_debug(): write_image - argv[3]=out/monster_flash.bin
Debug: 216 1110 command.c:74 script_debug(): write_image - argv[4]=0x8000000
Debug: 217 1110 command.c:74 script_debug(): write_image - argv[5]=bin
User : 218 1110 command.c:405 command_print(): auto erase enabled
User : 219 1110 command.c:405 command_print(): auto unlock enabled
Debug: 220 1123 configuration.c:83 find_file(): found out/monster_flash.bin
Debug: 221 1126 target.c:1516 target_read_u32(): address: 0xe0042000, value: 0x20036410
Info : 222 1126 stm32x.c:677 stm32x_probe(): device id = 0x20036410
Debug: 223 1128 target.c:1544 target_read_u16(): address: 0x1ffff7e0, value: 0x0080
Info : 224 1128 stm32x.c:752 stm32x_probe(): flash size = 128kbytes
Warn : 225 1129 stm32x.c:366 stm32x_protect(): Error: start and end sectors must be on a 4 sector boundary
Error: 226 1129 flash.c:117 flash_driver_protect(): failed setting protection for areas 0 to 20 (-901)
Debug: 227 1129 command.c:472 run_command(): Command failed with error code -901
User : 228 1129 command.c:675 openocd_jim_vfprintf():
User : 231 1129 command.c:675 openocd_jim_vfprintf():
User : 233 1129 command.c:675 openocd_jim_vfprintf():
User : 235 1129 command.c:675 openocd_jim_vfprintf():
User : 237 1129 command.c:675 openocd_jim_vfprintf():
User : 239 1129 command.c:675 openocd_jim_vfprintf():
User : 241 1129 command.c:675 openocd_jim_vfprintf():
make: *** [flash] Error 1

Here's the output of the examples I mentioned:

bla /1/arm/workspace/monster $ dd if=/dev/zero of=out/monster_flash.bin bs=1024 count=20
20+0 records in
20+0 records out
20480 bytes (20 kB) copied, 0.00668803 s, 3.1 MB/s
bla /1/arm/workspace/monster $ rm flash -f ;make flash
---Flashing--- auto erase enabled
auto unlock enabled
Info : device id = 0x20036410
Info : flash size = 128kbytes
wrote 20480 byte from file out/monster_flash.bin in 2.049710s (9.757 kb/s)
---Done---

bla /1/arm/workspace/monster $ dd if=/dev/zero of=out/monster_flash.bin bs=1024 count=21
21+0 records in
21+0 records out
21504 bytes (22 kB) copied, 0.00829303 s, 2.6 MB/s
bla /1/arm/workspace/monster $ rm flash -f ;make flash
---Flashing---
auto erase enabled
auto unlock enabled
Info : device id = 0x20036410
Info : flash size = 128kbytes
Warn : Error: start and end sectors must be on a 4 sector boundary
Error: failed setting protection for areas 0 to 20 (-901)
make: *** [flash] Error 1

bla /1/arm/workspace/monster $ dd if=/dev/zero of=out/monster_flash.bin bs=1024 count=24
24+0 records in
24+0 records out
24576 bytes (25 kB) copied, 0.00896715 s, 2.7 MB/s
bla /1/arm/workspace/monster $ rm flash -f ;make flash
---Flashing---
auto erase enabled
auto unlock enabled
Info : device id = 0x20036410
Info : flash size = 128kbytes
wrote 24576 byte from file out/monster_flash.bin in 2.299153s (10.439 kb/s)
---Done---

[Øyvind Harboe]

Post by Johnny Halfmoon
Maybe the following example (output is pasted below) can illustrate the issue
a bit better. First I try to flash a 20KB block, then a 21KB block and
finally a 24KB block. Only the blocks of 20 and 24 succeed, because they happen
to be nicely 4K alligned. All blocks that are not 4K alligned will fail. OpenOCD
tries to erase, (un)protect and flash only those sectors that are modified,
but doesn't take into account that some platforms require certain alligments.

This error message is specific to stm32.

Did you look at the stm32 source code?

Are you *sure* the error message isn't in fact telling you that this is a quaint
requirement of stm32?

Warn : 225 1129 stm32x.c:366 stm32x_protect(): Error: start and end
sectors must be on a 4 sector boundary

There is a bug in stm32 code in that it gives you a warning and not
an error, but that's minor.


if ((first && (first % stm32x_info->ppage_size)) || ((last + 1) &&
(last + 1) % stm32x_info->ppage_size))
{
LOG_WARNING("Error: start and end sectors must be on a %d sector
boundary", stm32x_info->ppage_size);
return ERROR_FLASH_SECTOR_INVALID;
}

[Johnny Halfmoon]

Post by Øyvind Harboe

Post by Johnny Halfmoon
Maybe the following example (output is pasted below) can illustrate the issue
a bit better. First I try to flash a 20KB block, then a 21KB block and
finally a 24KB block. Only the blocks of 20 and 24 succeed, because they happen
to be nicely 4K alligned. All blocks that are not 4K alligned will fail. OpenOCD
tries to erase, (un)protect and flash only those sectors that are modified,
but doesn't take into account that some platforms require certain alligments.

This error message is specific to stm32.
Did you look at the stm32 source code?

Yes, I know that code. I've got a committed patch of mine in there.

Post by Øyvind Harboe
Are you *sure* the error message isn't in fact telling you that this is a quaint
requirement of stm32?

It most certainly an STM32 specific quirk. This is sort of what I was trying to say in the first place. Let met explain further:

* The "flash write_image" function is trying to be smart by only deleting, (un)protecting and flashing those sectors that have to be modified. This is a good thing.

* The STM32 specific flash code has certain alignment requirements, which currently only generate an error in case of a non-compliant situation. This is not very bad, but it can be done better.

* My suggestion is to add a global parameter which tells OpenOCD to handle alignment issues in a smart way (globally); by adjusting the sector counts automatically in such that these errors do not appear. I suggest that we do not make this the default setting, because the "flash write_image" and "flash write_sectors" use the same code to write to the flash. If the default settings are such that OpenOCD automatically adjusts sector counts, then the user might get unexpected results when using "flash write_sectors". So to be safe, OpenOCD must not adjust sector counts, but only when told to do so.

* For now, the smart_sector_aligment setting will only be regarded by the STM32-specific code. Any other platforms that are eligible for alligment smartness? A quick grep through the code gives:

./src/flash/lpc288x.c:320: return ERROR_FLASH_DST_BREAKS_ALIGNMENT;
./src/flash/lpc2000.c:568: return ERROR_FLASH_DST_BREAKS_ALIGNMENT;
./src/flash/stellaris.c:946: return ERROR_FLASH_DST_BREAKS_ALIGNMENT;
./src/flash/stm32x.c:572: return ERROR_FLASH_DST_BREAKS_ALIGNMENT;
./src/flash/str9xpec.c:589: return ERROR_FLASH_DST_BREAKS_ALIGNMENT;
./src/flash/pic32mx.c:482: return ERROR_FLASH_DST_BREAKS_ALIGNMENT;
./src/flash/at91sam7.c:975: return ERROR_FLASH_DST_BREAKS_ALIGNMENT;
./src/flash/avrf.c:229: return ERROR_FLASH_DST_BREAKS_ALIGNMENT;
./src/flash/str9x.c:481: return ERROR_FLASH_DST_BREAKS_ALIGNMENT;
./src/flash/str7x.c:449: return ERROR_FLASH_DST_BREAKS_ALIGNMENT;

So, I'll try and figure out where smart_aligment might be useful. As these are quite a diverse bunch of controllers, I'm not sure I dare touch all of the code. I'll have to see if it's worth the trouble, if at all do-able.

Post by Øyvind Harboe
There is a bug in stm32 code in that it gives you a warning and not
an error, but that's minor.
if ((first && (first % stm32x_info->ppage_size)) || ((last + 1) &&
(last + 1) % stm32x_info->ppage_size))
{
LOG_WARNING("Error: start and end sectors must be on a %d sector
boundary", stm32x_info->ppage_size);
return ERROR_FLASH_SECTOR_INVALID;
}

Correct. And I will remove this little "beauty mark" along with adding the alignment parameter then :-)

[Øyvind Harboe]

Clearly you know more about this problem than I intend to
learn about it :-)

Would it be possible to consider a change in the flash
driver model that would allow the drivers to just "deal
with it"?

I'm loathe to add obscure and hard to grasp options...

"flash write_image unlock erase xxx" should "just work dammit" with
no funny options.

[Øyvind Harboe]

Perhaps more of flash write_image needs to be pushed
into the flash drivers to give the flash drivers enough context?

Some new fn call in the driver with a default implementation?

[Johnny Halfmoon]

Post by Øyvind Harboe
Perhaps more of flash write_image needs to be pushed
into the flash drivers to give the flash drivers enough context?
Some new fn call in the driver with a default implementation?

That was my first thought. But then I thought that that would affect all drivers, so I liked the thought of a quick and simple solution like a 'just_shutup_and_work' flag.

But I'm afraid that your proposal might be the way to go. I need to take a better look at the rest of the code to get a better picture first. I'll try and have a look this week and see what will be the most future-proof sollution. (Crap. Me and my big mouth :-)

Cheers,

Johnny