mspm0: stub flashing, support for newer parts #2189

Please mark the constants used here as unsigned with U

Why are you removing this #ifdef? It's correct that this cast to void is only required when DEBUG_TARGET is defined to do nothing.

There is no harm to cast the length to void regardless if DEBUG_TARGET is defined or not. So without it we have less noise. But I can put the #ifdef back, no worries.

We would prefer to keep the #ifdef please. This way if someone does something.. silly.. when DEBUG_TARGET should be defined to do something, the compiler will complain - that's why the preprocessor logic exists.

This, considerably more complicated encoding scheme, might be more space efficient, however does that actually result in space savings in the routines below having to be quite a bit more complicated to access this data? Especially as it prevents directly indexing and forces scanning through..

As you might have seen in the later updates, I have removed the dumping of configuration registers altogether. The reason being, TI added a number of layouts of said registers depending on the chip and the effort to maintain that is high with questionable value of usefulness. It is possible to read/write flash using common bmd(a) functionality anyways. Maybe that can be expanded so it also supports "directly typed in" numeric values in addition to reading from a file if that is deemed desirable.

What's the g_ suffix supposed to tell here? Note, we don't prefix globals with anything particular.. that's not part of the code style for BMD.

Please move this list of part numbers up to above the first function defined in this file, along with all the other global data. This makes all the lists of data defined for the targets supported easier to discover and find.

Please suffix the constant with U to prevent signed-unsigned conversions taking place here.

This outer loop needs its {}'s - the if statement inside should be considered to make this multi-line.

Rather than using goto and a label, we would much prefer i get defined above the for loop as a properly named loop variable such as index or part_idx or so.. and then used in an if statement for an early return w/ break; used to exit the loop early. It'll likely generate about the same assembly, but results in much easier to read and understand code.

Please use the project's

/*
 * Multi-line
 * comment
 * syntax
 */

and please note the careful layout indicated w/ the opening /* being on its own line, along with the */ and single spaces in front of the *'s that prefix the lines.

I honestly can't understand how this happened. Fixed.

Things like the RP2040 stub need a stack, as in

- however they don't go to all this work to define one this way, instead picking a location to call theirs in SRAM and running with it in-stub. Is there a particular reason to avoid this approach here?

Similar question: does the stub even actually use its stack allocation, or does the compiler keep everything in registers?

So I did try to set up the stack in the stub and it didn't work. In case function was marked naked, local function stack frame would not be created/initialized and then the values of the registers would be lost/corrupted. If not marked naked, then stack would be used before the code that sets the SP is called. This led me to the right conclusion that the stack pointer has to be properly initialized before entering the function. What I didn't figure out was that there can be a small landing function (without stack frame!) which initializes SP and calls the meat as in RP case and that led me to the code that initializes the stack before calling into the target routine itself.

Anyway, I split the stub code into two functions, with a preallocated stack area and now it works fine. Cleaned up the other global changes and while now more arcane stuff happens in the stub, it all actually made the target code simpler.