probe-rs / flash-algorithm-template

A template for writing CMSIS-Pack flash algorithms in Rust
Apache License 2.0
12 stars 8 forks source link

`flash-start-address` put into sector start address #16

Closed pkoevesdi closed 1 week ago

pkoevesdi commented 2 weeks ago

If I start a new project with

cargo generate gh:probe-rs/flash-algorithm-template  --name=algotest \
-d target-arch=thumbv7em-none-eabihf \
-d ram-start-address=0x20000000 \
-d ram-size=0x20000 \
-d flash-size=0x8000000 \
-d flash-page-size=0x100 \
-d flash-sector-size=0x10000 \
-d empty-byte-value=0xff

I see whatever I enter into flash-start-address ends up as the first (and only) sector start address in main.rs in line 18: sectors: [{address: ### }], which is wrong, because this is asserted to be zero here, as I learned here: https://github.com/probe-rs/flash-algorithm-template/issues/12#issuecomment-2457104668.

In my opinion, the flash-start-address is supposed to only end up in template.yaml in line 20 Nvm/range/start and in main.rs in line 12: flash_address.

Yatekii commented 1 week ago

I don't believe this is incorrect.

I believe the assert is incorrect. It is ARM specific. Flash sectors can be mapped to any address space.

pkoevesdi commented 1 week ago

@9names, could you look at it again? It was your comment where I learned it from. #12 My opinion: this is not the address where the flash gets mapped to, it's the start address of it's own memory area, and I can't really imagine, that this doesn't start at 0x0 inside any chip. And even if, it wouldn't necessarily start at the same address as where if gets mapped to inside the MCU. But little do I know....

9names commented 1 week ago

@Yatekii if you look at the example here, you can see a flash algo targetting flash at 0x40000. You can see that the first set of sectors start at 0x0. The assert would accept this algorithm.

If we consider this example to be correct, then the template is wrong and target-gen's assert is correct.

Yatekii commented 1 week ago

But why would the sector point to 0 if the flash is not there? I feel like that might just be an assumption rather than anything else.

9names commented 1 week ago

But why would the sector point to 0 if the flash is not there?

It's not really saying address zero though. It's algo base address (0x40000) + offset (0x0). Yes, I agree that is strange when the algos themselves all take absolute physical addresses.

I feel like that might just be an assumption rather than anything else

Yes, I am making an assumption that the reference example implementation was written this way specifically to convey this information. The choice of 0x40000 for the algo flash address is odd enough for me to consider it contrived.

We could look at what mcu vendors have done when they encountered this issue if we want to confirm what the de-facto standard is here.

Yatekii commented 1 week ago

Ah, sorry, I meant that the flash algo example makes the assumption that flash is always at 0.

Let's investigate :)

pkoevesdi commented 1 week ago

if you look at the example here, you can see a flash algo targetting flash at 0x40000.

@9names, did You mix up a line here? I see the device start address 0x00000000. So, this example doesn't proof anything in our discussion here, since we can't see in the example, whether the sector start adress is absolute or relative. Still, I'm totally sure, as You are, it is relative.

I think, some of the misunderstanding also has something to do with this: The Open-CMSIS spec has nothing to do with memory-mapping or any MCU-internal adress space. It's purely specifying functions to write, erase, verify data from external flash. Whether this happens by some low-level SPI commands, or some MCU-internal diver with memory mapping, doesn't matter, this interface spec is supposed to be agnostic of that. That's why You'll find nothing there about MCU-internally mapped memory addresses and I think in the example, even the Device Start Address is inside the external flash's address space. (As a matter of fact, this being-agnostic is what probe-rs breaks with it's own verify function, ignoring the Open-CMSIS-specified, user-written one, see here: https://github.com/probe-rs/probe-rs/issues/2617)

pkoevesdi commented 1 week ago

One thing I found that supports our thesis: a flash algo by @huming2207, also setting the sector address to 0x0, while the flash address ist 0x08000000, see https://github.com/huming2207/stm32wle5-flash-algo/blob/master/src/main.rs

9names commented 1 week ago

did You mix up a line here? I see the device start address 0x00000000.

No, I was viewing that example on mobile and the comments are displayed on the next line instead of inline. Oops.