Forums

I have written some routines to be able to erase flash sectors and program data to the flash chip:

* Flash routines: https://gitlab.com/doragasu/frugal-map-save
* Example code using them: https://gitlab.com/doragasu/frugal-map-save-example

Writing these routines is usually tricky because they must be run from RAM (as the flash memory cannot be read while the chip is performing erase/program operations). This is not the first time I write these kind of routines, for example here I did the same for the SEGA Genesis/Megadrive. But there you have a full fledged GCC compiler, making it easy to assign code to a section located on RAM by the linker. Unfortunately here we have SDCC, that as far as I know has no easy and clean way of achieving the same. And things get even worse because my Z80 abilities are clumsy at best.

So, I took inspiration from raphnet flashwrSMS: coded some assembly routines, assembled them, copied the code as hexadecimal data to ROM and then on runtime I copy the routines to RAM and run them.

This is cumbersome but mostly works: I can erase sectors from the console and program bytes to flash. The problem I have is with the function writing arrays of bytes, from flash.c file:

// [...]

typedef void (*program_fn)(uint8_t bank, uint16_t offset, uint8_t value);

// [...]

// program(uint8_t bank, uint16_t offset, uint8_t value_to_write);
// Parameters:
// - a: bank
// - de: offset
// - stack: value_to_write
static const uint8_t PROGRAM[] = {
         // _program::
0xF3,       // di
0x21, 0xFF, 0xFF, // ld hl, #0xffff    ; c <- original_bank
0x4E, // ld c, (hl)
0x77, // ld (hl), a    ; slot2_bank <- bank
0xFD, 0x21, 0x02, 0x00, // ld iy, #2    ; a <- value_to_write
0xFD, 0x39, // add iy, sp
0xFD, 0x7E, 0x00, // ld a, 0(iy)
0x21, 0x55, 0x55, // ld hl, #0x5555    ; Program unlock
0x36, 0xAA, // ld (hl), #0xaa
0x21, 0xAA, 0x2A, // ld hl, #0x2aaa
0x36, 0x55, // ld (hl), #0x55
0x21, 0x55, 0x55, // ld hl, #0x5555
0x36, 0xA0, // ld (hl), #0xa0
0x6B, // ld l, e       ; (bank:offset) <- value_to_write
0x62, // ld h, d
0x77, // ld (hl), a    ; a <- value_to_write
0x47, // ld b, a
         // data_poll:
0x7E,       // ld a, (hl)
0xB8, // cp b
0x20, 0xFC, // jr NZ, data_poll
0x21, 0xFF, 0xFF, // ld hl, #0xffff    ; Restore original_bank
0x71, // ld (hl), c
0xFB, // ei
0xC9 // ret
};

#define PROGRAM_LEN sizeof(PROGRAM)

// [...]

void flash_program(uint32_t addr, const uint8_t *data, int16_t len)
{
uint8_t code[PROGRAM_LEN];

for (int16_t i = 0; i < len; i++, addr++) {
   const uint8_t bank = addr >> 14;
   const uint16_t offset = 0x8000 | (addr & 0x3FFF);
   // Putting this memcpy() inside the loop is weird and slow, but
   // if I keep it outside, the function does not work. Please drop
   // me a line if you know why!
   memcpy(code, PROGRAM, PROGRAM_LEN);
   ((program_fn)code)(bank, offset, data[i]);
}
}

As the comment in the flash_program() function says, the problem is the memcpy() that copies the code dealing with the flash chip from flash to RAM (on the stack). The memcpy() should be outside of the loop, as I only need to copy the code once. But if I move it outside of the loop, only the first byte is written to flash (I think it's indeed writing all the bytes, but to other addresses, maybe the registers and/or the stack get messed by the assembly routine).

So, what did I do wrong here? Suggestions are welcome!

For running code from RAM with SDCC, check out sverx's smstestsuite which does it pretty neatly.

Had a look and it's a nice way, but has the drawback that if I understand the code correctly, it always copies 256 bytes of code to RAM, no matter how long the function is.

It also uses a global 256-byte buffer. I don't like allocating so much memory on a 8 KiB system for the save routines, that's the reason for me copying the code to the stack instead of a global/static variable. But that makes me think: how long is the stack on devkitSMS? Could I be overflowing it in my code?

The stack have no real RAM limit (DFFF - C000)
it will go from DFxx (see CRT0 declaration) and go up until you stop adding things into.

-- dumb things removed ^^--

NB. you can basicly make a generic 256 bytes buffer in RAM, and allocate it for saving when needed...and use is for other things rest of the time.

ichigobankai wrote

The stack have no real RAM limit (DFFF - C000)
it will go from DFxx (see CRT0 declaration) and go up until you stop adding things into.

your save/erase sector function can be in BANK0 (always accessible), so no copy in ram needed. Just a classic function.

NB. you can basicly make a generic 256 bytes buffer in RAM, and allocate it for saving when needed...and use is for other things rest of the time.

I think the problem is the flash chip is not accessible when writing to it, so no code from the ROM can be executed in any of the banks.

you're right ^^

I've digged/retrieved my old code about this, and yes it was copied/called from RAM.

Just be carefull using this with save as you'll need to copy all datas from ROM to RAM before erasing (unless you want to throw 4k of rom for each save).

You can also add a function to verify the chip id/manufacturer (to be sure about command sequence and can be used as a little protection)

last point, could be interesting to add a checksum routine to ensure the quality of the copy.

@doragasu any progress on the mystery of the memcpy loop?

I keep thinking about it, and keep having ideas but then realise I am wrong :/

Have you looked at the assembler that SDCC is generating? I wonder if it could be mishandling some registers / stack manipulation in an attempt to optimise. It's been known to get it wrong before...

I was also wondering if it's all about timing - does the memcpy simply add some delay in which keeps the flashing procedure reliable... I can't imagine why that would be though, as far as I can tell you are doing the polling check correctly, according to the write mechanism described in the datasheet.

Thanks for looking into it.

I didn't dig more into this issue. I already looked to the sdcc generated assembly, but as I have almost zero Z80 experience, for me it's pretty time consuming analysing the output

I've seen the asm output in both cases and I don't see an obvious problem, I'm trying to debug it :-)

willbritton wrote

@doragasu any progress on the mystery of the memcpy loop?

I keep thinking about it, and keep having ideas but then realise I am wrong :/

Have you looked at the assembler that SDCC is generating? I wonder if it could be mishandling some registers / stack manipulation in an attempt to optimise. It's been known to get it wrong before...

I was also wondering if it's all about timing - does the memcpy simply add some delay in which keeps the flashing procedure reliable... I can't imagine why that would be though, as far as I can tell you are doing the polling check correctly, according to the write mechanism described in the datasheet.

This reminds me of the time I used a a delay so an Arduino could read and write from flash memory. Without the delay the Arduino didn't write to flash memory properly. If I remembered correctly an Arduino cannot write a byte faster than 2 or 3 milliseconds .

I am already taking into account the time it takes to write the data. After each byte program, the chip has a polling mechanism in which you can know when it finished programming by polling bit 7, that will have the opposite value of the programmed byte (I am comparing against the entire byte instead of only bit 7 to save the bit mask operation, but as bit 7 is included in the byte comparison, it works the same).

Two more reasons that make me believe the delay is not the problem: first, I use the same mechanism on my cart programmer, and it works perfect; and second, I think the write is in fact performed, but to the wrong place, because after some of the tests I did, just after running the test once, the cart did not boot anymore until reprogrammed externally (so my theory is that the code got overwritten).

So, what I suspect is that something is causing the local function variable with the bank and/or the offset to be overwritten. Putting the memcpy() makes this variable overwrite to go away.

Just to satisfy my curiosity, maybe you could post or share a snippet from your .lst file for the flash_program function?

This is mine with the memcopy moved out of the loop

flash.zip (5 KB)

kusfo wrote

This is mine with the memcopy moved out of the loop

Thanks @kusfo!
Would be interesting to see how doragasu's compares to it, since SDCC can emit some quite different stuff depending on version, settings, etc.

Are you sure that's the version with memcpy outside the loop? Looks at first glance that it's inside.

I'm using the same (yours) docker image, so it should be the same.
And yes, this is the version with memcopy outside the loop.

I'm attaching both versions to compare.

flash-lsts.zip (9.98 KB)

kusfo wrote

I'm using the same (yours) docker image

Oh, cool!

kusfo wrote

And yes, this is the version with memcopy outside the loop.

I'm attaching both versions to compare.

Thanks - you're right, I was looking at the "flash_program_byte" function instead of the "flash_program" function 🤦

Exactly, I am using retcon85/toolchain-sms to build the program (nice image BTW!).

I think I found the problem, but as my Z80 kung fu is newbie level, I would like to confirm my finding, as I could be perfectly wrong. The problem is in how SDCC is passing parameters to the function doing the save.

If you see the attached screenshot (with annotations added to the code), to save offset and pass data[i] using the stack, the generated code does:

push bc
push af
inc sp

This is OK, because for the push af, only a register needs to be pushed. The problem comes after the call:

call ___sdcc_call_hl
pop bc

It's popping bc to restore offset, but sp does not point to offset, it points to data[i]. So I think the code should instead be:

call ___sdcc_call_hl
inc sp
pop bc

Does this make sense? And if affirmative, is there a way to instruct SDCC to generate correct code?

20230601_115056.png (136.84 KB)

Kind of depends how ___sdcc_call_hl is implemented (I was wondering this last time I looked at the problem) because remember that any subroutine should assume that the last item in the stack is a return address and any ret will pop it, so it could be that inc sp simply sets up a return address. Just a vague theory though.

Interesting theory, but I have just confirmed the missing `inc` is the problem. Modified the generated .asm file to add the `inc sp`, generated the .rel, the .sms ROM, tested and... it works.

Now I wonder if there's a way to force the compiler to behave. Otherwise I will have to remove the flash.c and substitute it by the modified .asm, and I would like to avoid that.

doragasu wrote

I think I found the problem, but as my Z80 kung fu is newbie level, I would like to confirm my finding, as I could be perfectly wrong. The problem is in how SDCC is passing parameters to the function doing the save.

If you see the attached screenshot (with annotations added to the code), to save offset and pass data[i] using the stack, the generated code does:

push bc
push af
inc sp

This is OK, because for the push af, only a register needs to be pushed. The problem comes after the call:

call ___sdcc_call_hl
pop bc

It's popping bc to restore offset, but sp does not point to offset, it points to data[i]. So I think the code should instead be:

call ___sdcc_call_hl
inc sp
pop bc

Does this make sense? And if affirmative, is there a way to instruct SDCC to generate correct code?

Where is that screenshot from? Does sdcc have an option to generate listings like that? With all these comments as well?

Comments to the right of the listing are mine, I was adding them while analysing the code and trying to make sense of it. Other than the comments, the file is generated by sdcc as is. Invoking sdcc on a C file generates the .asm, the .lst (shown), the .sym symbols and the .rel object file.

Is the problem here that A is being pushed to the stack but not popped?

Here's what the manual says:

Quote

If __z88dk_callee is not used, after the call, the stack parameters are cleaned
up by the caller, with the following exceptions: functions that do not have variable arguments and return void or a
type of at most 16 bits, or have both a first parameter of type float and a return value of type float.

Okay it reads a bit confusingly, but it could be that SDCC expects your function to clean the parameters from the stack itself - i.e. you need to do the inc sp inside the function maybe?

EDIT: nah, that's rubbish isn't it, because then the ret wouldn't work. Argh!

I feel like you could write a little wrapper function to allow a conventional call from C, which then manually correct the stack in order to call your assembler routine. It would add another level to the call stack, but I think it could tidy the calls up for you.

Exactly, if I increased `sp` inside my routine, the stack would not point properly to the return address (guaranteed crash). It has to be (yet another) a SDCC bug.

I have done this extremely ugly and hacky workaround (see attached screenshot). It's terrible but it's working, so it will stay like that until I find a better solution.

20230601_161610.png (73.71 KB)

In case anybody is wondering, this causes the following assembly to be emitted by SDCC (see attachment).

20230601_161756.png (42.81 KB)

Hum, after a bit more thought, maybe it was not an SDCC bug after all! It's just this "callee cleans the stack" paradigm is weird (I have always worked with "caller cleans the stack" paradigm). So, I found a way to make it work without using weird hacks. First let's see the flash_program() function, nice and clean:

void flash_program(uint32_t addr, const uint8_t *data, int16_t len)
{
uint8_t code[PROGRAM_LEN];

const uint8_t bank = addr >> 14;
uint16_t offset = 0x8000 | (addr & 0x3FFF);
memcpy(code, PROGRAM, PROGRAM_LEN);

while (len--) {
   ((program_fn)code)(bank, offset, *data);
   offset++;
   data++;
}
}

And now the "PROGRAM" block with the code copied to RAM and then called:

// program(uint8_t bank, uint16_t offset, uint8_t value_to_write);
// Parameters:
// - a: bank
// - de: offset
// - stack: value_to_write
static const uint8_t PROGRAM[] = {
         // _program::
0xF3, // di
0x21, 0xFF, 0xFF, // ld hl, #0xffff    ; c <- original_bank
0x4E, // ld c, (hl)
0x77, // ld (hl), a    ; slot2_bank <- bank
0xFD, 0x21, 0x02, 0x00, // ld iy, #2    ; a <- value_to_write
0xFD, 0x39, // add iy, sp
0xFD, 0x7E, 0x00, // ld a, 0(iy)
0x21, 0x55, 0x55, // ld hl, #0x5555    ; Program unlock
0x36, 0xAA, // ld (hl), #0xaa
0x21, 0xAA, 0x2A, // ld hl, #0x2aaa
0x36, 0x55, // ld (hl), #0x55
0x21, 0x55, 0x55, // ld hl, #0x5555
0x36, 0xA0, // ld (hl), #0xa0
0x6B, // ld l, e       ; (bank:offset) <- value_to_write
0x62, // ld h, d
0x77, // ld (hl), a    ; a <- value_to_write & 0x80
0x47, // ld b, a
         // data_poll:
0x7E, // ld a, (hl)
0xB8, // cp b
0x20, 0xFC, // jr NZ, data_poll
0x21, 0xFF, 0xFF, // ld hl, #0xffff    ; Restore original_bank
0x71, // ld (hl), c
0xFB, // ei
0xE1, // pop hl       ; hl <- return address
0x33, // inc sp       ; Clean stack
0xE9 // jp (hl)    ; Return to caller
};

The trick I did here is replacing the ret at the end in previos version, with:

pop hl
inc sp
jp (hl)

Because we want to clean the stack, we cannot use ret to return to the caller anymore (since the stack will not point to the return address after cleaning). So we pop the return address, clean the stack and jump to the return address. So much trouble just the avoid the caller doing an inc sp on its side [shrug].

This is working and no hacks are involved. Yay!

It's weird though isn't it. I don't honestly think the intention is for you to have to write your routines without a ret instruction so it could either be a bug or else some subtlety of proper usage that isn't obvious.

I wonder if "cleaning" the stack in the manual just refers to wiping the data, not necessarily adjusting the stack pointer. That it says the callee is responsible for cleaning the stack for everything except functions returning a void and without variable arguments still represents a huge proportion of functions and under normal circumstances the calls "just work".

Yesterday I was trying to find the code in SDCC which handles function pointer calls to see if I could get any insight about how it's supposed to work, but unfortunately couldn't seem to locate it.

@sverx might be able to provide some advice, but I haven't seen him online for a while, perhaps he's on holiday or taking a break from the forums!

Although weird, i think that's the way it's supposed to be when using callee convention. I am quite convinced, because the SDCC generated assembly for my flash.c file does exactly that, have a look to the end of the flash_program() function code, output by SDCC as is:

;flash/flash.c:144: ((program_fn)code)(bank, offset, *data);
ld l, -4 (ix)
ld h, -3 (ix)
ld a, (hl)
push af
inc sp
ld e, -6 (ix)
ld d, -5 (ix)
ld a, -9 (ix)
ld hl, #1
add hl, sp
call ___sdcc_call_hl
;flash/flash.c:145: offset++;
inc -6 (ix)
jr NZ, 00119$
inc -5 (ix)
00119$:
;flash/flash.c:146: data++;
inc -4 (ix)
jr NZ, 00101$
inc -3 (ix)
jr 00101$
00104$:
;flash/flash.c:148: }
ld sp, ix
pop ix
pop hl
pop af
pop af
jp (hl)
.area _CODE
.area _INITIALIZER
.area _CABS (ABS)

It pops ix (the function pushed it before), then pops hl to get return address, then pops af twice to "clean" the input parameters from the stack and finally jumps to the return address. No ret there. If you are determined to return using a ret, you still can, just replace the jp (hl) above with a push hl followed by the ret, but that would be slower. BTW on a side note, that way of "jumping" using ret is what Retpolines Spectre mitigations do: they push the jump address to the stack and issue a ret (to avoid branch prediction kicking in).

I think it's a weird artefact of C calling conventions and the way certain CPU instruction sets seem to be designed to work with "stacky" calling conventions. (See also the parameterised "ret imm16" in x86.) This is somewhat why the z88dk and other "Z80 friendly" calling conventions were created.

I'm no expert, but couldn't the function in RAM be marked as another calling convention to make it a bit simpler?

I think we are OK to not worry about Spectre mitigations on the Z80...

There are at least three options I could use:
* _sdcccall(0) to use the SDCC version 0 calling convention. This is the "usual" calling convention I have always seen when working with GCC and CLANG: all parameters are passed on the stack, and caller cleans the stack. But switching to this calling convention might be slower to pass parameters (because passing data using registers tends to be faster).
* __smallc: Similar to the above, passes parameters using the stack, with a few differences (e.g. 1 byte params are passed using 2 bytes).
* __naked to avoid compiler to completely emit function prologue and epilogue. Leaves to you the responsibility of passing the parameters and cleaning the stack however you want. Might be useful for things like pure inline assembly functions, I think, or to better optimize code in some situations. But as a general case, it means more work for the coder.

But I'm pretty noob when talking about SDCC and Z80, so I could be wrong and there could be much better ways of doing things.

Maxim wrote

I think we are OK to not worry about Spectre mitigations on the Z80...

We could always write a Z80 core with built-in speculative branch prediction ^_^

The only thing is because you are using a function pointer I don't know if you could apply any of those attributes to it, since SDCC would simply see it as a block of constant data rather than a function.

Again, maybe a wrapper function would work from which you could manually jump into your ROM function and / or handle the stack, but I feel like then you don't really need the compiler options anyway.

willbritton wrote

@sverx might be able to provide some advice, but I haven't seen him online for a while, perhaps he's on holiday or taking a break from the forums!

I was on holiday!
'callee' calling convention(s) means that the called function removes the parameters from the stack, instead of being the caller doing that work. Makes sense when calling asm functions that pop parameters and don't need to preserve them.
Example: here the size parameter is on the stack. Note how you should pop the return address, then the parameter, then restore the return address onto the stack so that the RET at the end works properly.

That makes sense, if functions are small and don't need to keep the parameters in RAM, popping the parameters is faster than indexing using ix/iy.

Author	Message
doragasu Joined: 03 Mar 2022 Posts: 56	Saving data to on-cart flash memory Posted: Mon May 15, 2023 2:34 pm
doragasu Joined: 03 Mar 2022 Posts: 56	I have written some routines to be able to erase flash sectors and program data to the flash chip: * Flash routines: https://gitlab.com/doragasu/frugal-map-save * Example code using them: https://gitlab.com/doragasu/frugal-map-save-example Writing these routines is usually tricky because they must be run from RAM (as the flash memory cannot be read while the chip is performing erase/program operations). This is not the first time I write these kind of routines, for example here I did the same for the SEGA Genesis/Megadrive. But there you have a full fledged GCC compiler, making it easy to assign code to a section located on RAM by the linker. Unfortunately here we have SDCC, that as far as I know has no easy and clean way of achieving the same. And things get even worse because my Z80 abilities are clumsy at best. So, I took inspiration from raphnet flashwrSMS: coded some assembly routines, assembled them, copied the code as hexadecimal data to ROM and then on runtime I copy the routines to RAM and run them. This is cumbersome but mostly works: I can erase sectors from the console and program bytes to flash. The problem I have is with the function writing arrays of bytes, from flash.c file: // [...] typedef void (program_fn)(uint8_t bank, uint16_t offset, uint8_t value); // [...] // program(uint8_t bank, uint16_t offset, uint8_t value_to_write); // Parameters: // - a: bank // - de: offset // - stack: value_to_write static const uint8_t PROGRAM[] = { // _program:: 0xF3, // di 0x21, 0xFF, 0xFF, // ld hl, #0xffff ; c <- original_bank 0x4E, // ld c, (hl) 0x77, // ld (hl), a ; slot2_bank <- bank 0xFD, 0x21, 0x02, 0x00, // ld iy, #2 ; a <- value_to_write 0xFD, 0x39, // add iy, sp 0xFD, 0x7E, 0x00, // ld a, 0(iy) 0x21, 0x55, 0x55, // ld hl, #0x5555 ; Program unlock 0x36, 0xAA, // ld (hl), #0xaa 0x21, 0xAA, 0x2A, // ld hl, #0x2aaa 0x36, 0x55, // ld (hl), #0x55 0x21, 0x55, 0x55, // ld hl, #0x5555 0x36, 0xA0, // ld (hl), #0xa0 0x6B, // ld l, e ; (bank:offset) <- value_to_write 0x62, // ld h, d 0x77, // ld (hl), a ; a <- value_to_write 0x47, // ld b, a // data_poll: 0x7E, // ld a, (hl) 0xB8, // cp b 0x20, 0xFC, // jr NZ, data_poll 0x21, 0xFF, 0xFF, // ld hl, #0xffff ; Restore original_bank 0x71, // ld (hl), c 0xFB, // ei 0xC9 // ret }; #define PROGRAM_LEN sizeof(PROGRAM) // [...] void flash_program(uint32_t addr, const uint8_t data, int16_t len) { uint8_t code[PROGRAM_LEN]; for (int16_t i = 0; i < len; i++, addr++) { const uint8_t bank = addr >> 14; const uint16_t offset = 0x8000 \| (addr & 0x3FFF); // Putting this memcpy() inside the loop is weird and slow, but // if I keep it outside, the function does not work. Please drop // me a line if you know why! memcpy(code, PROGRAM, PROGRAM_LEN); ((program_fn)code)(bank, offset, data[i]); } } As the comment in the flash_program() function says, the problem is the memcpy() that copies the code dealing with the flash chip from flash to RAM (on the stack). The memcpy() should be outside of the loop, as I only need to copy the code once. But if I move it outside of the loop, only the first byte is written to flash (I think it's indeed writing all the bytes, but to other addresses, maybe the registers and/or the stack get messed by the assembly routine). So, what did I do wrong here? Suggestions are welcome!

willbritton Joined: 06 Mar 2022 Posts: 697 Location: London, UK	Posted: Mon May 15, 2023 7:41 pm
	For running code from RAM with SDCC, check out sverx's smstestsuite which does it pretty neatly.

doragasu Joined: 03 Mar 2022 Posts: 56	Posted: Tue May 16, 2023 6:55 am
doragasu Joined: 03 Mar 2022 Posts: 56	Had a look and it's a nice way, but has the drawback that if I understand the code correctly, it always copies 256 bytes of code to RAM, no matter how long the function is. It also uses a global 256-byte buffer. I don't like allocating so much memory on a 8 KiB system for the save routines, that's the reason for me copying the code to the stack instead of a global/static variable. But that makes me think: how long is the stack on devkitSMS? Could I be overflowing it in my code?

ichigobankai Joined: 04 Jul 2010 Posts: 546 Location: Angers, France	Posted: Tue May 16, 2023 8:36 am Last edited by ichigobankai on Tue May 16, 2023 11:47 am; edited 1 time in total
	The stack have no real RAM limit (DFFF - C000) it will go from DFxx (see CRT0 declaration) and go up until you stop adding things into. -- dumb things removed ^^-- NB. you can basicly make a generic 256 bytes buffer in RAM, and allocate it for saving when needed...and use is for other things rest of the time.

kusfo Joined: 29 Mar 2012 Posts: 900 Location: Spain	Posted: Tue May 16, 2023 9:57 am
kusfo Joined: 29 Mar 2012 Posts: 900 Location: Spain	ichigobankai wrote The stack have no real RAM limit (DFFF - C000) it will go from DFxx (see CRT0 declaration) and go up until you stop adding things into. your save/erase sector function can be in BANK0 (always accessible), so no copy in ram needed. Just a classic function. NB. you can basicly make a generic 256 bytes buffer in RAM, and allocate it for saving when needed...and use is for other things rest of the time. I think the problem is the flash chip is not accessible when writing to it, so no code from the ROM can be executed in any of the banks.

ichigobankai Joined: 04 Jul 2010 Posts: 546 Location: Angers, France	Posted: Tue May 16, 2023 11:44 am
	you're right ^^ I've digged/retrieved my old code about this, and yes it was copied/called from RAM. Just be carefull using this with save as you'll need to copy all datas from ROM to RAM before erasing (unless you want to throw 4k of rom for each save). You can also add a function to verify the chip id/manufacturer (to be sure about command sequence and can be used as a little protection) last point, could be interesting to add a checksum routine to ensure the quality of the copy.

willbritton Joined: 06 Mar 2022 Posts: 697 Location: London, UK	Posted: Sun May 21, 2023 11:06 am
	@doragasu any progress on the mystery of the memcpy loop? I keep thinking about it, and keep having ideas but then realise I am wrong :/ Have you looked at the assembler that SDCC is generating? I wonder if it could be mishandling some registers / stack manipulation in an attempt to optimise. It's been known to get it wrong before... I was also wondering if it's all about timing - does the memcpy simply add some delay in which keeps the flashing procedure reliable... I can't imagine why that would be though, as far as I can tell you are doing the polling check correctly, according to the write mechanism described in the datasheet.

doragasu Joined: 03 Mar 2022 Posts: 56	Posted: Wed May 24, 2023 7:20 am
doragasu Joined: 03 Mar 2022 Posts: 56	Thanks for looking into it. I didn't dig more into this issue. I already looked to the sdcc generated assembly, but as I have almost zero Z80 experience, for me it's pretty time consuming analysing the output

kusfo Joined: 29 Mar 2012 Posts: 900 Location: Spain	Posted: Wed May 24, 2023 7:47 am
kusfo Joined: 29 Mar 2012 Posts: 900 Location: Spain	I've seen the asm output in both cases and I don't see an obvious problem, I'm trying to debug it :-)

Jolene Joined: 31 Dec 2022 Posts: 30	Posted: Wed May 24, 2023 12:31 pm
Jolene Joined: 31 Dec 2022 Posts: 30	willbritton wrote @doragasu any progress on the mystery of the memcpy loop? I keep thinking about it, and keep having ideas but then realise I am wrong :/ Have you looked at the assembler that SDCC is generating? I wonder if it could be mishandling some registers / stack manipulation in an attempt to optimise. It's been known to get it wrong before... I was also wondering if it's all about timing - does the memcpy simply add some delay in which keeps the flashing procedure reliable... I can't imagine why that would be though, as far as I can tell you are doing the polling check correctly, according to the write mechanism described in the datasheet. This reminds me of the time I used a a delay so an Arduino could read and write from flash memory. Without the delay the Arduino didn't write to flash memory properly. If I remembered correctly an Arduino cannot write a byte faster than 2 or 3 milliseconds .

doragasu Joined: 03 Mar 2022 Posts: 56	Posted: Wed May 24, 2023 1:20 pm
doragasu Joined: 03 Mar 2022 Posts: 56	I am already taking into account the time it takes to write the data. After each byte program, the chip has a polling mechanism in which you can know when it finished programming by polling bit 7, that will have the opposite value of the programmed byte (I am comparing against the entire byte instead of only bit 7 to save the bit mask operation, but as bit 7 is included in the byte comparison, it works the same). Two more reasons that make me believe the delay is not the problem: first, I use the same mechanism on my cart programmer, and it works perfect; and second, I think the write is in fact performed, but to the wrong place, because after some of the tests I did, just after running the test once, the cart did not boot anymore until reprogrammed externally (so my theory is that the code got overwritten). So, what I suspect is that something is causing the local function variable with the bank and/or the offset to be overwritten. Putting the memcpy() makes this variable overwrite to go away.

willbritton Joined: 06 Mar 2022 Posts: 697 Location: London, UK	Posted: Wed May 24, 2023 8:13 pm
	Just to satisfy my curiosity, maybe you could post or share a snippet from your .lst file for the flash_program function?

kusfo Joined: 29 Mar 2012 Posts: 900 Location: Spain	Posted: Wed May 24, 2023 9:09 pm
kusfo Joined: 29 Mar 2012 Posts: 900 Location: Spain	This is mine with the memcopy moved out of the loop flash.zip (5 KB)

willbritton Joined: 06 Mar 2022 Posts: 697 Location: London, UK	Posted: Wed May 24, 2023 10:55 pm
	kusfo wrote This is mine with the memcopy moved out of the loop Thanks @kusfo! Would be interesting to see how doragasu's compares to it, since SDCC can emit some quite different stuff depending on version, settings, etc. Are you sure that's the version with memcpy outside the loop? Looks at first glance that it's inside.

kusfo Joined: 29 Mar 2012 Posts: 900 Location: Spain	Posted: Thu May 25, 2023 9:04 am
kusfo Joined: 29 Mar 2012 Posts: 900 Location: Spain	I'm using the same (yours) docker image, so it should be the same. And yes, this is the version with memcopy outside the loop. I'm attaching both versions to compare. flash-lsts.zip (9.98 KB)

willbritton Joined: 06 Mar 2022 Posts: 697 Location: London, UK	Posted: Thu May 25, 2023 9:45 am
	kusfo wrote I'm using the same (yours) docker image Oh, cool! kusfo wrote And yes, this is the version with memcopy outside the loop. I'm attaching both versions to compare. Thanks - you're right, I was looking at the "flash_program_byte" function instead of the "flash_program" function 🤦

doragasu Joined: 03 Mar 2022 Posts: 56	Posted: Fri May 26, 2023 7:11 am
doragasu Joined: 03 Mar 2022 Posts: 56	Exactly, I am using retcon85/toolchain-sms to build the program (nice image BTW!).

doragasu Joined: 03 Mar 2022 Posts: 56	Posted: Thu Jun 01, 2023 10:08 am
doragasu Joined: 03 Mar 2022 Posts: 56	I think I found the problem, but as my Z80 kung fu is newbie level, I would like to confirm my finding, as I could be perfectly wrong. The problem is in how SDCC is passing parameters to the function doing the save. If you see the attached screenshot (with annotations added to the code), to save offset and pass data[i] using the stack, the generated code does: push bc push af inc sp This is OK, because for the push af, only a register needs to be pushed. The problem comes after the call: call ___sdcc_call_hl pop bc It's popping bc to restore offset, but sp does not point to offset, it points to data[i]. So I think the code should instead be: call ___sdcc_call_hl inc sp pop bc Does this make sense? And if affirmative, is there a way to instruct SDCC to generate correct code? 20230601_115056.png (136.84 KB)

willbritton Joined: 06 Mar 2022 Posts: 697 Location: London, UK	Posted: Thu Jun 01, 2023 10:32 am
	Kind of depends how ___sdcc_call_hl is implemented (I was wondering this last time I looked at the problem) because remember that any subroutine should assume that the last item in the stack is a return address and any ret will pop it, so it could be that inc sp simply sets up a return address. Just a vague theory though.

doragasu Joined: 03 Mar 2022 Posts: 56	Posted: Thu Jun 01, 2023 11:32 am
doragasu Joined: 03 Mar 2022 Posts: 56	Interesting theory, but I have just confirmed the missing `inc` is the problem. Modified the generated .asm file to add the `inc sp`, generated the .rel, the .sms ROM, tested and... it works. Now I wonder if there's a way to force the compiler to behave. Otherwise I will have to remove the flash.c and substitute it by the modified .asm, and I would like to avoid that.

Calindro Joined: 14 Apr 2013 Posts: 637	Posted: Thu Jun 01, 2023 12:45 pm
Calindro Joined: 14 Apr 2013 Posts: 637	doragasu wrote I think I found the problem, but as my Z80 kung fu is newbie level, I would like to confirm my finding, as I could be perfectly wrong. The problem is in how SDCC is passing parameters to the function doing the save. If you see the attached screenshot (with annotations added to the code), to save offset and pass data[i] using the stack, the generated code does: push bc push af inc sp This is OK, because for the push af, only a register needs to be pushed. The problem comes after the call: call ___sdcc_call_hl pop bc It's popping bc to restore offset, but sp does not point to offset, it points to data[i]. So I think the code should instead be: call ___sdcc_call_hl inc sp pop bc Does this make sense? And if affirmative, is there a way to instruct SDCC to generate correct code? Where is that screenshot from? Does sdcc have an option to generate listings like that? With all these comments as well?

doragasu Joined: 03 Mar 2022 Posts: 56	Posted: Thu Jun 01, 2023 1:17 pm
doragasu Joined: 03 Mar 2022 Posts: 56	Comments to the right of the listing are mine, I was adding them while analysing the code and trying to make sense of it. Other than the comments, the file is generated by sdcc as is. Invoking sdcc on a C file generates the .asm, the .lst (shown), the .sym symbols and the .rel object file.

willbritton Joined: 06 Mar 2022 Posts: 697 Location: London, UK	Posted: Thu Jun 01, 2023 1:56 pm
	Is the problem here that A is being pushed to the stack but not popped? Here's what the manual says: Quote If __z88dk_callee is not used, after the call, the stack parameters are cleaned up by the caller, with the following exceptions: functions that do not have variable arguments and return void or a type of at most 16 bits, or have both a first parameter of type float and a return value of type float. Okay it reads a bit confusingly, but it could be that SDCC expects your function to clean the parameters from the stack itself - i.e. you need to do the inc sp inside the function maybe? EDIT: nah, that's rubbish isn't it, because then the ret wouldn't work. Argh! I feel like you could write a little wrapper function to allow a conventional call from C, which then manually correct the stack in order to call your assembler routine. It would add another level to the call stack, but I think it could tidy the calls up for you.

doragasu Joined: 03 Mar 2022 Posts: 56	Posted: Thu Jun 01, 2023 2:05 pm
doragasu Joined: 03 Mar 2022 Posts: 56	Exactly, if I increased `sp` inside my routine, the stack would not point properly to the return address (guaranteed crash). It has to be (yet another) a SDCC bug. I have done this extremely ugly and hacky workaround (see attached screenshot). It's terrible but it's working, so it will stay like that until I find a better solution. 20230601_161610.png (73.71 KB)

doragasu Joined: 03 Mar 2022 Posts: 56	Posted: Thu Jun 01, 2023 2:18 pm
doragasu Joined: 03 Mar 2022 Posts: 56	In case anybody is wondering, this causes the following assembly to be emitted by SDCC (see attachment). 20230601_161756.png (42.81 KB)

doragasu Joined: 03 Mar 2022 Posts: 56	Posted: Fri Jun 02, 2023 8:40 am
doragasu Joined: 03 Mar 2022 Posts: 56	Hum, after a bit more thought, maybe it was not an SDCC bug after all! It's just this "callee cleans the stack" paradigm is weird (I have always worked with "caller cleans the stack" paradigm). So, I found a way to make it work without using weird hacks. First let's see the flash_program() function, nice and clean: void flash_program(uint32_t addr, const uint8_t data, int16_t len) { uint8_t code[PROGRAM_LEN]; const uint8_t bank = addr >> 14; uint16_t offset = 0x8000 \| (addr & 0x3FFF); memcpy(code, PROGRAM, PROGRAM_LEN); while (len--) { ((program_fn)code)(bank, offset, data); offset++; data++; } } And now the "PROGRAM" block with the code copied to RAM and then called: // program(uint8_t bank, uint16_t offset, uint8_t value_to_write); // Parameters: // - a: bank // - de: offset // - stack: value_to_write static const uint8_t PROGRAM[] = { // _program:: 0xF3, // di 0x21, 0xFF, 0xFF, // ld hl, #0xffff ; c <- original_bank 0x4E, // ld c, (hl) 0x77, // ld (hl), a ; slot2_bank <- bank 0xFD, 0x21, 0x02, 0x00, // ld iy, #2 ; a <- value_to_write 0xFD, 0x39, // add iy, sp 0xFD, 0x7E, 0x00, // ld a, 0(iy) 0x21, 0x55, 0x55, // ld hl, #0x5555 ; Program unlock 0x36, 0xAA, // ld (hl), #0xaa 0x21, 0xAA, 0x2A, // ld hl, #0x2aaa 0x36, 0x55, // ld (hl), #0x55 0x21, 0x55, 0x55, // ld hl, #0x5555 0x36, 0xA0, // ld (hl), #0xa0 0x6B, // ld l, e ; (bank:offset) <- value_to_write 0x62, // ld h, d 0x77, // ld (hl), a ; a <- value_to_write & 0x80 0x47, // ld b, a // data_poll: 0x7E, // ld a, (hl) 0xB8, // cp b 0x20, 0xFC, // jr NZ, data_poll 0x21, 0xFF, 0xFF, // ld hl, #0xffff ; Restore original_bank 0x71, // ld (hl), c 0xFB, // ei 0xE1, // pop hl ; hl <- return address 0x33, // inc sp ; Clean stack 0xE9 // jp (hl) ; Return to caller }; The trick I did here is replacing the ret at the end in previos version, with: pop hl inc sp jp (hl) Because we want to clean the stack, we cannot use ret to return to the caller anymore (since the stack will not point to the return address after cleaning). So we pop the return address, clean the stack and jump to the return address. So much trouble just the avoid the caller doing an inc sp on its side [shrug]. This is working and no hacks are involved. Yay!

willbritton Joined: 06 Mar 2022 Posts: 697 Location: London, UK	Posted: Fri Jun 02, 2023 10:18 am
	It's weird though isn't it. I don't honestly think the intention is for you to have to write your routines without a ret instruction so it could either be a bug or else some subtlety of proper usage that isn't obvious. I wonder if "cleaning" the stack in the manual just refers to wiping the data, not necessarily adjusting the stack pointer. That it says the callee is responsible for cleaning the stack for everything except functions returning a void and without variable arguments still represents a huge proportion of functions and under normal circumstances the calls "just work". Yesterday I was trying to find the code in SDCC which handles function pointer calls to see if I could get any insight about how it's supposed to work, but unfortunately couldn't seem to locate it. @sverx might be able to provide some advice, but I haven't seen him online for a while, perhaps he's on holiday or taking a break from the forums!

doragasu Joined: 03 Mar 2022 Posts: 56	Posted: Fri Jun 02, 2023 11:12 am
doragasu Joined: 03 Mar 2022 Posts: 56	Although weird, i think that's the way it's supposed to be when using callee convention. I am quite convinced, because the SDCC generated assembly for my flash.c file does exactly that, have a look to the end of the flash_program() function code, output by SDCC as is: ;flash/flash.c:144: ((program_fn)code)(bank, offset, data); ld l, -4 (ix) ld h, -3 (ix) ld a, (hl) push af inc sp ld e, -6 (ix) ld d, -5 (ix) ld a, -9 (ix) ld hl, #1 add hl, sp call ___sdcc_call_hl ;flash/flash.c:145: offset++; inc -6 (ix) jr NZ, 00119$ inc -5 (ix) 00119$: ;flash/flash.c:146: data++; inc -4 (ix) jr NZ, 00101$ inc -3 (ix) jr 00101$ 00104$: ;flash/flash.c:148: } ld sp, ix pop ix pop hl pop af pop af jp (hl) .area _CODE .area _INITIALIZER .area _CABS (ABS) It pops ix (the function pushed it before), then pops hl to get return address, then pops af twice to "clean" the input parameters from the stack and finally jumps to the return address. No ret there. If you are determined to return using a ret, you still can, just replace the jp (hl)* above with a push hl followed by the ret, but that would be slower. BTW on a side note, that way of "jumping" using ret is what Retpolines Spectre mitigations do: they push the jump address to the stack and issue a ret (to avoid branch prediction kicking in).

Maxim Site Admin Joined: 19 Oct 1999 Posts: 14959 Location: London	Posted: Fri Jun 02, 2023 1:39 pm
	I think it's a weird artefact of C calling conventions and the way certain CPU instruction sets seem to be designed to work with "stacky" calling conventions. (See also the parameterised "ret imm16" in x86.) This is somewhat why the z88dk and other "Z80 friendly" calling conventions were created. I'm no expert, but couldn't the function in RAM be marked as another calling convention to make it a bit simpler? I think we are OK to not worry about Spectre mitigations on the Z80...

doragasu Joined: 03 Mar 2022 Posts: 56	Posted: Sun Jun 04, 2023 8:38 am
doragasu Joined: 03 Mar 2022 Posts: 56	There are at least three options I could use: * _sdcccall(0) to use the SDCC version 0 calling convention. This is the "usual" calling convention I have always seen when working with GCC and CLANG: all parameters are passed on the stack, and caller cleans the stack. But switching to this calling convention might be slower to pass parameters (because passing data using registers tends to be faster). * __smallc: Similar to the above, passes parameters using the stack, with a few differences (e.g. 1 byte params are passed using 2 bytes). * __naked to avoid compiler to completely emit function prologue and epilogue. Leaves to you the responsibility of passing the parameters and cleaning the stack however you want. Might be useful for things like pure inline assembly functions, I think, or to better optimize code in some situations. But as a general case, it means more work for the coder. But I'm pretty noob when talking about SDCC and Z80, so I could be wrong and there could be much better ways of doing things. Maxim wrote I think we are OK to not worry about Spectre mitigations on the Z80... We could always write a Z80 core with built-in speculative branch prediction ^_^

willbritton Joined: 06 Mar 2022 Posts: 697 Location: London, UK	Posted: Sun Jun 04, 2023 9:29 am
	The only thing is because you are using a function pointer I don't know if you could apply any of those attributes to it, since SDCC would simply see it as a block of constant data rather than a function. Again, maybe a wrapper function would work from which you could manually jump into your ROM function and / or handle the stack, but I feel like then you don't really need the compiler options anyway.

sverx Joined: 05 Sep 2013 Posts: 3941 Location: Stockholm, Sweden	Posted: Mon Jun 05, 2023 8:18 am
	willbritton wrote @sverx might be able to provide some advice, but I haven't seen him online for a while, perhaps he's on holiday or taking a break from the forums! I was on holiday! 'callee' calling convention(s) means that the called function removes the parameters from the stack, instead of being the caller doing that work. Makes sense when calling asm functions that pop parameters and don't need to preserve them. Example: here the size parameter is on the stack. Note how you should pop the return address, then the parameter, then restore the return address onto the stack so that the RET at the end works properly.

doragasu Joined: 03 Mar 2022 Posts: 56	Posted: Mon Jun 05, 2023 1:26 pm
doragasu Joined: 03 Mar 2022 Posts: 56	That makes sense, if functions are small and don't need to keep the parameters in RAM, popping the parameters is faster than indexing using ix/iy.

Forums

View topic - Saving data to on-cart flash memory