Skip to content
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

machine/ncr5385.cpp: raise IRQ for INT_INVALID_CMD #12524

Merged
merged 3 commits into from
Jul 7, 2024

Conversation

Elektraglide
Copy link
Contributor

  • Tek4404 issues cmd=0xff which expects an IRQ even though its an INT_INVALID_CMD
  • minor readability; replaced magic hex value with symbolic names

@@ -816,7 +817,9 @@ void ncr5385_device::set_dreq(bool dreq)

void ncr5385_device::update_int()
{
bool const int_state = m_int_status & 0x5f;
bool const int_state = m_int_status & (INT_FUNC_COMPLETE | INT_BUS_SERVICE |
Copy link
Contributor

Choose a reason for hiding this comment

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

Excellent clarification!

@cuavas
Copy link
Member

cuavas commented Jun 28, 2024

Also, please give the pull request a meaningful title. What does it change? What does it fix?

@Elektraglide Elektraglide changed the title Ncr5385 fix Ncr5385 fix for missing raising IRQ for INT_INVALID_CMD Jun 30, 2024
@Elektraglide
Copy link
Contributor Author

Also, please give the pull request a meaningful title. What does it change? What does it fix?

Title amended. In large code base like this, I've generally used PR titles to give 'area of interest' for reviewers and used following lines to explain the details. But if having a detailed title is the MAME thang, happy to go with it. :-)

@rb6502 rb6502 changed the title Ncr5385 fix for missing raising IRQ for INT_INVALID_CMD machine/ncr5385.cpp: raise IRQ for INT_INVALID_CMD Jul 7, 2024
@rb6502 rb6502 merged commit 2506679 into mamedev:master Jul 7, 2024
5 checks passed
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.

4 participants