Looks right to me. I was about to throw a tiny tantrum about resetstub being in segment CODE while reset_fastrom was in segment CODE7, but I understand why you did that (and I was going to write a rebuttal against it, but after some deep thought, I figured that actually makes the most overall sense). I'll assume that the template sets CODE7 to bank $80 or at least between bank $80-FF.
Just please remember that the lorom256k.cfg template I saw (when doing stuff for Espozo) used MAPPER_LOROM (mode $20, which is 100% okay -- banks $80-FF get mirrored through bank $00-7D, with $7E/7F lost because of WRAM), and ROMSPEED_200NS (not ROMSPEED_120NS, which I think you'd need to fix).
1. Use of rep #$38, rather than cld/rep #$30. This is another example of ridiculous optimisation for little gain -- you save 1 byte and 2 cycles at the expensive of confusing a newbie. 98% of code out there is going to use rep/sep with $10/$20/$30 and nothing else. We've already seen evidence of this being confusing for Espozo, so clearing of d of P through rep rather than just doing cld is silly, IMO.
2. I thought you and thefox said that it was possible to get the address of a MEMORY or SEGMENTS label within code itself? If that's the case, then the STACK_BASE/STACK_SIZE/LAST_STACK_ADDR and ZEROPAGE_BASE stuff shouldn't exist -- you should just be able to use whatever magic there is to do the equivalent of (pseudo code coming up) ldx #(.SEGMENT(STACK)+.sizeof(.SEGMENT(STACK))-1)/txs, and lda #.SEGMENT(DIRECTPAGE)/tcd.
3. The register equates should end up in a separate file if at all possible, and as mentioned before (again your call), use the official SNES label designation. (No I don't like the names of some of them, but consistency is good)
So now onto the formatting, and some of the above integrated:
Code: Select all
; Make sure these conform to the linker script (e.g. lorom256.cfg).
ZEROPAGE_BASE = $0000
STACK_BASE = $0100
STACK_SIZE = $0100
LAST_STACK_ADDR = STACK_BASE + STACK_SIZE - 1
PPU_BASE = $2100
CPUIO_BASE = $4200
sei ; turn off IRQs
xce ; turn off 6502 emulation mode
cld ; disable decimal mode
rep #$30 ; A=16, X/Y=16
txs ; set the stack pointer
; Initialize the CPU I/O registers to predictable values
tcd ; temporarily move direct page to SNES MMIO region for fast MMIO access
; 8< 8< 8< a bunch of stuff omitted >8 >8 >8
tcd ; return direct page to real zero page
jml main ; this is why I say the program bank is main >> 16
Key things changed here:
1. Spacing of equate/= assignments. These should line up, making it visually easy for the person to know what's what. You (to date) are the only 65xx programmer I've met who doesn't "align" his formatting. Hate to single you out, but I'm being honest. Your style is your style and if I was working with you on a project and you did most of the work when I started, I'd conform to your style no matter what (that's the approach I take when collaborating), but if it was from the start I would be screaming loudly over "just two spaces then a semicolon, who cares". It really doesn't flow well.
2. Comments should be aligned as best as possible. The way I do this is the following: in the "general area" of the code, I find the longest line/instruction, then add 3 spaces, and that's where the ;
for comments should "align" to throughout the surrounding code. If there are large amounts of "alignment" changes, then I tend to add an extra newline between "parts" of code, e.g. between two separate .procs, two separate .segments, etc.. You can see an example of this in resetstub vs. reset_fastrom, and the extra newline before .segment CODE7. In fact, adding some spacing (newlines) before a new .segment is very useful (to me) -- otherwise it's very easy to "forget" what segment you're working in.
3. Changed rep #$38 into rep #$30/cld, and moved the cld up to inside of resetstub. This makes it clearer what's going on within reset_fastrom. Also added comment explaining what cld does.
4. I documented what exactly rep #$30 did to A and X/Y sizes. You can ABSOLUTELY remove this comment if needed; I've gotten in the habit of doing this in the past few weeks due to Espozo, so it's a habit at this point. If I was writing my own code I wouldn't comment it.
5. Moved comment about "temporarily move direct page" to the actual tcd statement, since for your ldx #LAST_STACK_ADDR/txs example above, you had the equivalent of that type of command on the txs line, not the ldx line. I did the same for the lda #ZEROPAGE_BASE/tcd line. It's important to be consistent about your comments and where you put them.
I hope my comments (pun intended) are taken as constructive.