Skip to content

New machines marked as NOT_WORKING: MIT Cadr #13340

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 38 commits into
base: master
Choose a base branch
from

Conversation

wilbertpol
Copy link
Contributor

New machines marked as NOT_WORKING

MIT Cadr [Bitsavers, Wilbert Pol]

cadr.xml: Added 4 not working items.

New NOT_WORKING software list additions

System 78.48 [ams]
System 100.0 [ams]
System 300.0 [ams]
System 301.0 [ams]

Comment on lines +23 to +31
static constexpr u64 NOP_MASK = u64(0x7fffffffeffff);

static const char *const bool_op[0x10];
static const char *const arith_op[0x10];
static const char *const arith_op_c[0x10];
static const char *const mult_div_op[0x20];
static const char *const output_bus_control[0x04];
static const char *const q_control[0x04];
static const char *const rp[0x04];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these really need to be in the header? It’s just exposed implementation details that slow down compiling downstream stuff and increase the chance of causing a recompile if the implementation changes. Throw them in an anonymous namespace in the .cpp file.

Comment on lines 32780 to 32785
tx0_8kw // 1962 MIT TX-0 (8kw RAM)

@source:mit/cadr.cpp
cadr

@source:mits/altair.cpp
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep this file sorted – “c” comes before “t”. Also I think you’ll get merge conflicts from the comments being stripped.

}


void cadr_disassembler::m_source(std::ostream &stream, u64 op)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These helper functions that don’t depend on the disassembler object state at all can be turned into free functions in an anonymous namespace to keep implementation details out of the header.

bool m_popj;
bool m_interrupt_pending;

void program_map(address_map &map);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don’t forget ATTR_COLD on this.

Comment on lines +617 to +618
default:
fatalerror("%x(%o): output %02x not implemented", m_prev_pc, m_prev_pc, (m_ir >> 19) & 0x1f); break;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There isn’t any point putting a break after a noreturn function (there are a few cases in this file). If you want to make it more explicit that it’s leaving via an exception, you can change fatalerror to throw emu_fatalerror.

Comment on lines +39 to +79
static constexpr u16 IRQ_VECTOR_KEYBOARD = 0xb0;
static constexpr u16 IRQ_VECTOR_MOUSE = 0xb4;
static constexpr u16 IRQ_VECTOR_CHAOSNET = 0xb8;
static constexpr u16 IRQ_VECTOR_CLOCK = 0xbc;
static constexpr u16 IRQ_VECTOR_CHAOS_TRANSMIT = 0x100;
static constexpr u16 IRQ_VECTOR_CHAOS_RECEIVE = 0x104;

static constexpr int CSR_REMOTE_MOUSE_ENABLE_BIT = 0;
static constexpr int CSR_MOUSE_IRQ_ENABLE_BIT = 1;
static constexpr int CSR_KEYBOARD_IRQ_ENABLE_BIT = 2;
static constexpr int CSR_CLOCK_IRQ_ENABLE_BIT = 3;
static constexpr int CSR_MOUSE_READY_BIT = 4;
static constexpr u16 CSR_MOUSE_READY = 1 << CSR_MOUSE_READY_BIT;
static constexpr int CSR_KEYBOARD_READY_BIT = 5;
static constexpr u16 CSR_KEYBOARD_READY = 1 << CSR_KEYBOARD_READY_BIT;
static constexpr int CSR_CLOCK_READY_BIT = 6;
static constexpr u16 CSR_CLOCK_READY = 1 << CSR_CLOCK_READY_BIT;

static constexpr int CHAOSNET_TIMER_IRQ_ENABLE_BIT = 0;
static constexpr u16 CHAOSNET_TIMER_IRQ_ENABLE = 1 << CHAOSNET_TIMER_IRQ_ENABLE_BIT;
static constexpr int CHAOSNET_LOOPBACK_BIT = 1;
static constexpr u16 CHAOSNET_LOOKBACK = 1 << CHAOSNET_LOOPBACK_BIT;
static constexpr int CHAOSNET_ANY_DESTINATION_BIT = 2;
static constexpr u16 CHAOSNET_ANY_DESTINATION = 1 << CHAOSNET_ANY_DESTINATION_BIT;
static constexpr int CHAOSNET_RESET_RECEIVE_BIT = 3;
static constexpr int CHAOSNET_RECEIVE_IRQ_ENABLE_BIT = 4;
static constexpr u16 CHAOSNET_RECEIVE_IRQ_ENABLE = 1 << CHAOSNET_RECEIVE_IRQ_ENABLE_BIT;
static constexpr int CHAOSNET_TRANSMIT_IRQ_ENABLE_BIT = 5;
static constexpr u16 CHAOSNET_TRANSMIT_IRQ_ENABLE = 1 << CHAOSNET_TRANSMIT_IRQ_ENABLE_BIT;
static constexpr int CHAOSNET_TRANSMIT_ABORTED_BIT = 6;
static constexpr u16 CHAOSNET_TRANSMIT_ABORTED = 1 << CHAOSNET_TRANSMIT_ABORTED_BIT;
static constexpr int CHAOSNET_TRANSMIT_DONE_BIT = 7;
static constexpr u16 CHAOSNET_TRANSMIT_DONE = 1 << CHAOSNET_TRANSMIT_DONE_BIT;
static constexpr int CHAOSNET_RESET_TRANSMIT_BIT = 8;
static constexpr u16 CHAOSNET_LOST_COUNT = 0x1e00;
static constexpr int CHAOSNET_RESET_BIT = 13;
static constexpr u16 CHAOSNET_RESET = 1 << CHAOSNET_RESET_BIT;
static constexpr int CHAOSNET_RECEIVE_DONE_BIT = 15;
static constexpr u16 CHAOSNET_RECEIVE_DONE = 1 << CHAOSNET_RECEIVE_DONE_BIT;

static constexpr u16 CHAOS_BUFFER_SIZE = 512 / 2;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move constants that don’t need to be here into an anonymous namespace.


PORT_START("KEY.2")
PORT_BIT(0x01, IP_ACTIVE_HIGH, IPT_KEYBOARD) PORT_CODE(KEYCODE_LCONTROL) PORT_CHAR(UCHAR_SHIFT_2) // f2 02 20
PORT_BIT(0x02, IP_ACTIVE_HIGH, IPT_KEYBOARD) PORT_NAME(": ±") // f2 02 22
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doe this actually produce colon and ± characters, and if so, is there a good reason not to use PORT_CHAR for this?

Comment on lines +27 to +30
// device-level overrides
virtual void device_start() override ATTR_COLD;
virtual void device_reset() override ATTR_COLD;
virtual void device_add_mconfig(machine_config &config) override;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as other device headers.

Comment on lines +327 to +361
u32 cadr_disk_device::read(offs_t offset)
{
switch (offset)
{
case 0x04:
return status_r();
case 0x05:
return command_list_r();
case 0x06:
return disk_address_r();
case 0x07:
return error_correction_r();
}
return 0xffffffff;
}


void cadr_disk_device::write(offs_t offset, u32 data)
{
switch (offset)
{
case 0x04:
command_w(data);
break;
case 0x05:
command_list_w(data);
break;
case 0x06:
disk_address_w(data);
break;
case 0x07:
start_w(data);
break;
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you be better off with a device address map function rather than these dispatch functions?

Comment on lines +112 to +123
u32 cadr_disk_device::status_r()
{
LOG("Disk Controller status read\n");
return m_status;
}


u32 cadr_disk_device::command_list_r()
{
LOG("Disk Controller command list pointer read\n");
return m_clp;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should check for disabled side effects before logging in read handlers to avoid spam when debugger windows are open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants