Improvements to Technics KN5000 #12684
Conversation
felipesanches
force-pushed
the
kn5000_squashed
branch
from
ddc1861
to
2222c0a
Compare
|
After @ajrhacker made this comment (#11835 (comment)) I investigated the problem a bit more and I was able to get rid of the segfault on the extension interface for the Technics KN5000 (which is used to install the HDAE5000 board). I've just force-pushed the amended fix. |
| m_subcpu->portc_read().set([this] { return ioport("CN12")->read(); }); | ||
| m_subcpu->portc_write().set([this] (u8 data) { m_checking_device_led_cn12 = (BIT(data, 1) == 0); }); |
There was a problem hiding this comment.
Please avoid unnecessary on-the-fly tag lookups. You can look at the other options devcb provides (set_ioport is particularly relevant here).
| m_maincpu->portz_read().set([this] { | ||
| return ioport("COM_SELECT")->read() | (m_sstat << 2); | ||
| }); |
There was a problem hiding this comment.
Please avoid on-the-fly tag lookups.
You can get m_sstat read through the I/O port with PORT_CUSTOM_MEMBER or PORT_READ_LINE_MEMBER.
| // TODO: m_maincpu->portd_write().set([this] (u8 data) { m_fdc->reset_w(BIT(data, 0)); }); | ||
| m_maincpu->portd_write().set([this] (u8 data) { m_fdc->reset_w(BIT(data, 0)); }); |
There was a problem hiding this comment.
You can use transforms like .set(m_fdc, FUNC(whatever_device::reset_w)).bit(0) etc.
| // TODO: m_maincpu->porte_read().set([] { return 1; }); //checked at EF05A6 (v10 ROM) | ||
| m_maincpu->porte_read().set([] { return 1; }); //checked at EF05A6 (v10 ROM) |
| if(m_extension) | ||
| { |
There was a problem hiding this comment.
The existing code in this file (as well as the majority of MAME) uses a space between flow control keywords and the opening parenthesis of the controlling expression, like if (whatever).
| { | ||
| buf = string_format("%s", m_sfr_names[imm]); | ||
| } |
There was a problem hiding this comment.
You don’t need string_format for unformatted strings, << is a thing.
| protected: | ||
| // device-level overrides | ||
| virtual void device_config_complete() override; |
There was a problem hiding this comment.
Minor thing, but please don’t copy/paste this comment – it doesn’t even make sense. Use “device_t implementation” or similar.
| // Port 0: 8 bit I/O. Shared with d0-d7 | ||
| devcb_read8 m_port0_read; | ||
| devcb_write8 m_port0_write; | ||
|
|
||
| // Port 1: 8 bit I/O. Shared with d8-d15 | ||
| devcb_read8 m_port1_read; | ||
| devcb_write8 m_port1_write; | ||
|
|
||
| // Port 2: 8 bit I/O. Shared with d16-d23 | ||
| devcb_read8 m_port2_read; | ||
| devcb_write8 m_port2_write; |
There was a problem hiding this comment.
Can you use a devcb_read8::array and devcb_write8::array for these to make it more concise?
| uint8_t tmp94c241_device::p0_r() | ||
| { | ||
| return m_port0_read(0); | ||
| } | ||
|
|
||
| void tmp94c241_device::p0_w(uint8_t data) | ||
| { | ||
| m_port_latch[0] = data; | ||
| m_port0_write(0, data, 0xff); | ||
| } | ||
|
|
||
| void tmp94c241_device::p0cr_w(uint8_t data) | ||
| { | ||
| m_port_control[0] = data; | ||
| } | ||
|
|
||
| void tmp94c241_device::p0fc_w(uint8_t data) | ||
| { | ||
| m_port_function[0] = data; | ||
| } | ||
|
|
||
| uint8_t tmp94c241_device::p1_r() | ||
| { | ||
| return m_port1_read(0); | ||
| } | ||
|
|
||
| void tmp94c241_device::p1_w(uint8_t data) | ||
| { | ||
| m_port_latch[1] = data; | ||
| m_port1_write(0, data, 0xff); | ||
| } | ||
|
|
||
| void tmp94c241_device::p1cr_w(uint8_t data) | ||
| { | ||
| m_port_control[1] = data; | ||
| } |
There was a problem hiding this comment.
Can you use function templates to avoid copy/pasta?
|
I cribbed a small part of this in 23e9abc as a release is coming very soon. |
This is ready for review and replaces PR #11835 but does not include the kn1500 driver, which I'll leave for a future PR.