mamedev / mame

MAME
https://www.mamedev.org/
Other
8.29k stars 2.02k forks source link

MC68340 wrong data bus width ? #10353

Open Paul-Arnold opened 2 years ago

Paul-Arnold commented 2 years ago

Hi,

I've just been looking at the MC68340 device (src\devices\machine\68340.cpp) and the device is declared as having a 32 bit address and data bus. However, the device actually only has a 16 bit external data bus.

Is there a reason for it being declared as 32/32 or is that just a mistake ? I realise changing it to a 16 bit data bus is going to break things but very few drivers use the cpu so it shouldn't be too difficult to fix. I don't want to change it if it's declared that way for a reason.

cuavas commented 2 years ago

Well, the only potential issue I can see is that it derives from the Frescale CPU32 device, which is 32/32. Are there potential issues with simply changing the numbers in the derived class, or will the fscpu32_device class deal with it gracefully?

Paul-Arnold commented 2 years ago

Having tried changing from 32/32 to 16/32 it definitely fails :(

The 68340 installs a number of 32 bit read/write handlers as it treats some internal registers as 32 bits when they're actually two 16 bit registers. Even after fixing those and changing the driver which uses the m68340 it now fails on "Fatal error: Requesting cache() with data width 32 while the config says 16"

I probably don't know enough about how this all works :(

ghost commented 2 years ago

That would be my mistake then, I was told at the time it was 32/32, like a 68020/68040.

Feel free to change it, MAME is probably a bit more flexible with those handlers these days, but they might need some adjustment.

Paul-Arnold commented 2 years ago

OK, so it looks like all devices using the cpu32 core are 16 bit data externally. I'll do some more digging before I change anything.

Ideally I'd like to fix the problems with the moves instruction raised here https://github.com/mamedev/mame/issues/9336 if anyone has any strong feelings on how best to fix that.