Document address wrapping behaviour of MBC2 RAM, and note questionable source of $52-$54 ROM size tags

[TheThief]

Also note that 2KB RAM size tag is erroneously used by a lot of "PD" ROMs (no cart was ever made with this size).

[aaaaaa123456789]

ROM and RAM are acronyms and they should be spelled that way, not "rom" or "ram".

[TheThief]

ROM and RAM are acronyms and they should be spelled that way, not "rom" or "ram".

Too true! I'm normally a stickler for that...

[avivace]

Given #108 , this looks good to me. @ISSOtm can you mark the conversations you opened as resolved if you're okay with the clarifications?

[TheThief]

When you merge it back, might want to flatten it... I added a couple more commits fixing formatting etc.

[avivace]

When you merge it back, might want to flatten it... I added a couple more commits fixing formatting etc.

Don't worry, we're gonna rebase/squash in any case

[ISSOtm]

Gonna re-review from scratch

[ISSOtm]

As per #66, are there sources for this claim?

[TheThief]

Yes - the mooneye docs for one:

https://gekkio.fi/files/gb-docs/gbctr.pdf
https://github.com/Gekkio/mooneye-gb/blob/master/tests/emulator-only/mbc2/ram.s

I also found a pin diagram of the MBC2 chip - it only has address pins for the bottom 9 bits and the top 2 - the top two are used to select between ROM 0/1 areas and the cart ram, the bottom 9 index the ram. Addresses with bits 10+ set are simply invisible to the chip as it doesn't have pins for them: https://fms.komkon.org/GameBoy/Tech/Cart2.gif

[ISSOtm]

Alright, though I would also copy the note that the upper 4 bits of reads are undefined and should not be relied upon. That's a fairly important thing to know (if for some reason one were to use MBC2).

[ISSOtm]

Ping @TheThief

[TheThief]

I'm waiting for the final version of the other PR to be merged before I make changes to this one relating to 2k ram etc.

[ISSOtm]

#108 has just been merged.

[TheThief]

@ISSOtm would you re-review?

[ISSOtm]

Just the last unresolved request to go, otherwise LGTM.

[TheThief]

Just the last unresolved request to go, otherwise LGTM.

This request?

Alright, though I would also copy the note that the upper 4 bits of reads are undefined and should not be relied upon. That's a fairly important thing to know (if for some reason one were to use MBC2).

I actually added this text already: "The upper 4 bits of each byte are undefined and should not be
relied upon.":

As the data consists of 4bit values, only the lower 4 bits of the "bytes" in this memory area are used. The upper 4 bits of each byte are undefined and should not be relied upon.

[ISSOtm]

The unresolved item talks about copying that notice to the wrapping behavior of the MBC2's SRAM.

[TheThief]

The unresolved item talks about copying that notice to the wrapping behavior of the MBC2's SRAM.

Ah I get you now. I thought I'd only added the first notice after you made the comment, but yes it makes sense to copy it into the echo section.

[avivace]

Looks good to me, thanks @TheThief for working through this! 🥳