#3652 [r2k, r2ka, r3ka] Several bugs in crt0.s

[D-mo]

crt0.s for r2k, r2ka and r3ka creates internal interrupt table in Flash, which has several bugs:

1. Location of the secondary watchdog is incorrectly set to 0x100 intead of 0x110 so it overrides the periodic interrupt's ISR.
2. Several ISRs use reti instruction, which is wrong. It should be ipres and ret instead.
3. Having interrupt table in Flash where it cannot be modified is pointless IMHO. Especially since there is no support in sdcc for creating ISRs. So, one essentially has to re-create it from scratch in RAM.

[D-mo]

So, I've fixed the above things, but I can't for the life of me get interrupts to work. As soon as I enable one of the interrupts, the board hangs.

I've reduced the code to the point, where I have my test bits inside crt0.s (attached). If I don't enable any interrupts, I get blinking lights. As soon as I un-comment either call _enable_periodic or call _enable_timer_b -- no blinking lights.

I do have proper ISR stubs in the vector table for both. Not sure what's going on.

Could someone please help?

[Philipp Klaus Krause]

Have you checked that all your code ends up at the correct addresses (you can see that in the .rst file)?
Also: can you post the .lst and .rst files here?
Have you checked if your program still works with minimal interrupt handlers (i.e. just returning from the interrupt, but not doing anything else)?

[Philipp Klaus Krause]

I've created the current Rabbit crt0 in 2020 - before they were so wrong, they worked with the simulator only. The current ones "worked" (I was able to run benchmarks, and see the results on the serial console) for me on a few RCM boards, but I did not do any further testing of any advanced stuff, such as interrupts.
Back then my goal was to support the Rabbits in the code size / peed regression testing (those graphs at https://sourceforge.net/p/sdcc/code/HEAD/tree/trunk/sdcc-extra/historygraphs/), but OpenRabbit never became reliable enough for that - it tended to hang about once in five times I flashed the Rabbit.
Thus, the only testing the Rabbits get is via the nightly regression testing using the uC simulator. That works very well for ensuring that SDCC generates correct code from C source, but doesn't cover µC-specific stuff, such as interrupts, well.
Also, the Rabbit ports don't have a lot of users - I guess the problem is that very few people are starting new projects with Rabbits, and the current Rabbit users don't want to migrate their existing code from Dynamic C to SDCC, especially, since they often use libraries written for Dynamic C.
So thanks for doing some testing on real hardware, and helping improve the Rabbit crt0(.)s.

[Philipp Klaus Krause]

The interrupt table is in flash with no support in SDCC for creating handlers for historial reasons: This is a reasonable choice for Z80 (interrupt handling varies widely on Z80-based systems, so having an example crt0 that the user then can adapt to their needs and reassemble themselves is all we can do).
The Rabbit ports were derived from the Z80 ports, so they inherited this. I guess, it would make more sense to have a table in flash, but allow specifying the handlers from C via the __interrupt(N) syntax, like e.g. the stm8 port does.

[Philipp Klaus Krause]

1. is fixed in [r14359] .

[D-mo]

Thanks Philipp, I got it sorted out. There were two things going on:

1. If you enable periodic interrupt and don't ack it in the ISR (by reading GCSR), the board will hang.
2. There is a small paragraph in setion 6.1 of the manual, which says:

"The internal interrupt vector table occupies 512 bytes, and the external interrupt vector
table is 256 bytes in size. Since the RST and SYSCALL vectors use all eight bits of the
IIR for addressing, the lowermost bit of IIR should always be set to zero so to keep some
vectors from inadvertently overlapping."

I've highlighted the important bit. I am not doing anything with syscalls or RSTs, but for some reason if IIR is set to an odd number, the interrupts don't work -- the board hangs.

In the current crt0 the interrupt table is at address 0x0100, which means IIR is set to 0x01 (odd number) and the board will hang. In my testing I've moved the interrupt table into RAM at address 0xdd00, which also caused the board to hang (since 0xdd is also an odd number). Once I've moved it to 0xdc00 everything started working fine.

I've ended up re-writing crt0 quite a bit to suit my needs. Feel free to grab any bits from it (attached) if you want.

[Philipp Klaus Krause]

I've now found that sentence in the Rabbit 4000 user manual, and later revisions of the Rabbit 3000 user manual, but not in the Rabbit 2000 user manuals, and earlier revisions of the Rabbit 3000 user manual. AFAIK, the Rabbit 3000 and 4000 have a 512B internal interrupt table, while Rabbit 2000 has a 256B one.
On which hardware did you encounter the problem?
I still wonder why the odd value in iir triggers an issue for you - after all, from the tables, it looks like an odd value in iir would just make rst 0x38 overlap with the interrupt handler for the pwm interrupt, which should be fine as long as those are not used.

[D-mo]

CPU   ID: 0x101 (Rabbit 3000 rev. IL2T/IZ2T/UL2T/UZ2T)
Board ID: 0xf01 (RCM3010 29MHz, 128K SRAM, 256K Flash)
Flash ID: 0xbfd6 (SST SST39[LV]F020)

[D-mo]

I still wonder why the odd value in iir triggers an issue for you - after all, from the tables, it looks like an odd value in iir would just make rst 0x38 overlap with the interrupt handler for the pwm interrupt, which should be fine as long as those are not used.

Yeah, I am not sure either. That sentence honestly is a bit confusing.

I suspect that later Rabbit boards simply ignore the lowest bit of IIR. In table B.1.2 at the end of the manual they list a bunch of ISRs that do that. Eg, for Timer B they say:

{IIR[7:1], 0, 0x00B0}

Although periodic interrupt is not listed, it could just be an omission...

[D-mo]

OK I've just confirmed that bit 0 of IIR gets simply ignored.

I've loaded 0xdd into IIR and placed my periodic ISR at address 0xdc00 (instead of 0xdd00) and everything was working fine.

[Philipp Klaus Krause]

Thanks. I now also found the info in the Rabbit 3000 user manuals. This is clearly a breaking change from the Rabbit 2000.
I'll update the crt0 in SDCC accordingly.

[Philipp Klaus Krause]

Done in [r14361].

[D-mo]

The current ones "worked" (I was able to run benchmarks, and see the results on the serial console) for me on a few RCM boards ...

Hehe. Well, it probably worked for two reasons:
1. all interrupts are disabled by default;
2. the board runs at IP (interrupt priority) 3 by default, so even if you enable any of them, they won't fire until you drop to lower priority.
:)

... OpenRabbit never became reliable enough for that - it tended to hang about once in five times I flashed the Rabbit.

Interesting. I wrote my own utility (rabbit-util) before I came across OpenRabbit and ended up using it as a reference in a few cases. Mine also hangs once in a while but not that often.

Also, the Rabbit ports don't have a lot of users - I guess the problem is that very few people are starting new projects with Rabbits, and the current Rabbit users don't want to migrate their existing code from Dynamic C to SDCC, especially, since they often use libraries written for Dynamic C.

I am one of those people. :) I've got close to 500 Rabbits (if not more) that have been running for over 10 year. Didn't think I'd ever need to touch them. But, now I need to update them all due to circumstances outside of my control.

I've switched to Linux many years ago and didn't really feel like going back to Windows and Dynamic C. So, thanks to you and all the other folks that have ported sdcc to work with them.

[D-mo]

Here is the final version. There are two differences from the original (other than fixing the interrupt table):

1. CPU and perif clock are run at full speed.
2. Zero wait states for the RAM and Flash.

[Philipp Klaus Krause]

I can't put these into SDCC's crt0. The crt0 that comes with SDCC should contain safe defaults. Users can write a custom crt0 based on the SDCC one, or set clocks and wait states in __sdcc_external_startup.
Can you have a look at the crt0 in current SDCC svn, to see if it is okayish?
I haven't made the reti vs. ipres change yet, as I don't feel comfortable enough with Rabbit interrupt handling yet.

[D-mo]

Sorry, I've missed this.

I can't put these into SDCC's crt0. The crt0 that comes with SDCC should contain safe defaults.

No problem, I understand.

Can you have a look at the crt0 in current SDCC svn, to see if it is okayish?
I haven't made the reti vs. ipres change yet, as I don't feel comfortable enough with Rabbit interrupt handling yet.

reti is a chained version of pop ip + ret. So, if you are going to use reti, you need to add push ip as a first instruction in the isr. In reality, I haven't seen it used much even in the Digi's own BIOS code:

user@linux:~/DCRabbit_9$ grep -Rw reti | wc -l
2
user@linux:~/DCRabbit_9$ grep -Rw ipres | wc -l
270

Most of the time you just see ipres + ret, which saves 9 clocks compared to push ip + reti.

[D-mo]

A more fundamental question, I guess, is the usefulness of this table, since:

1. All interrupts are off by default anyway.
2. The table can't be modified (to install user ISRs).
3. The stubs that the table provides are insufficient. Internal interrupts require some action to clear them (see Table 6-3 in the Rabbit 3000 UM) and the current table only clears the periodic interrupt. The rest will make the board hang if enabled.

There is also the External interrupt table (using the EIR register), which is not provided at all.

[D-mo]

If you are cool with my approach of stealing 1K from the stack and using the range of 0xdc00 - 0xdfff for the internal and external tables, I can submit a patched version of crt0.s with just those changes (and some clean-ups).

I wouldn't fill it with any stub ISRs though, for the above reasons (points 1 and 3).

[Philipp Klaus Krause]

I'd rather keep the table in the Flash. The default crt0 can still serve as a starting point for users to make a custom crt0.

[D-mo]

It's up to you of course.

But, in case you change your mind, here is the version I am currently using. I've got periodic interrupt working, as well as interrupts for serial ports A and B.

[Philipp Klaus Krause]

I've made the reti vs ipres / ret change in [r14379]. That should make the crt0 more useful as a starting point for user's custom ones. With this last change, AFAIK all the bugs are fixed (though one might consider further crt0 improvements that would be feature requests).