From 489e4c49bc9156889a34bcba3c388e352bd99db9 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sat, 21 Jan 2017 18:42:18 -0500 Subject: [PATCH 1/6] EXI_Channel: Move private interface below public interface --- Source/Core/Core/HW/EXI/EXI_Channel.h | 52 +++++++++++++-------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/Source/Core/Core/HW/EXI/EXI_Channel.h b/Source/Core/Core/HW/EXI/EXI_Channel.h index a0cb404705..4dc53b2430 100644 --- a/Source/Core/Core/HW/EXI/EXI_Channel.h +++ b/Source/Core/Core/HW/EXI/EXI_Channel.h @@ -18,6 +18,32 @@ class Mapping; class CEXIChannel { +public: + CEXIChannel(u32 ChannelId); + ~CEXIChannel(); + + // get device + IEXIDevice* GetDevice(const u8 _CHIP_SELECT); + IEXIDevice* FindDevice(TEXIDevices device_type, int customIndex = -1); + + void RegisterMMIO(MMIO::Mapping* mmio, u32 base); + + void SendTransferComplete(); + + void AddDevice(const TEXIDevices device_type, const int device_num); + void AddDevice(std::unique_ptr device, const int device_num, + bool notify_presence_changed = true); + + // Remove all devices + void RemoveDevices(); + + bool IsCausingInterrupt(); + void DoState(PointerWrap& p); + void PauseAndLock(bool doLock, bool unpauseOnUnlock); + + // This should only be used to transition interrupts from SP1 to Channel 2 + void SetEXIINT(bool exiint) { m_Status.EXIINT = !!exiint; } + private: enum { @@ -86,30 +112,4 @@ private: // Since channels operate a bit differently from each other u32 m_ChannelId; - -public: - // get device - IEXIDevice* GetDevice(const u8 _CHIP_SELECT); - IEXIDevice* FindDevice(TEXIDevices device_type, int customIndex = -1); - - CEXIChannel(u32 ChannelId); - ~CEXIChannel(); - - void RegisterMMIO(MMIO::Mapping* mmio, u32 base); - - void SendTransferComplete(); - - void AddDevice(const TEXIDevices device_type, const int device_num); - void AddDevice(std::unique_ptr device, const int device_num, - bool notify_presence_changed = true); - - // Remove all devices - void RemoveDevices(); - - bool IsCausingInterrupt(); - void DoState(PointerWrap& p); - void PauseAndLock(bool doLock, bool unpauseOnUnlock); - - // This should only be used to transition interrupts from SP1 to Channel 2 - void SetEXIINT(bool exiint) { m_Status.EXIINT = !!exiint; } }; From 688225616e0980eb542f6d14f40a6f5a39b98790 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sat, 21 Jan 2017 18:44:54 -0500 Subject: [PATCH 2/6] EXI_Channel: In-class initialize variables --- Source/Core/Core/HW/EXI/EXI_Channel.cpp | 6 +----- Source/Core/Core/HW/EXI/EXI_Channel.h | 14 +++++++------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/Source/Core/Core/HW/EXI/EXI_Channel.cpp b/Source/Core/Core/HW/EXI/EXI_Channel.cpp index 4cd3b1e3db..ff9e6ed46a 100644 --- a/Source/Core/Core/HW/EXI/EXI_Channel.cpp +++ b/Source/Core/Core/HW/EXI/EXI_Channel.cpp @@ -20,12 +20,8 @@ enum EXI_READWRITE }; -CEXIChannel::CEXIChannel(u32 ChannelId) - : m_DMAMemoryAddress(0), m_DMALength(0), m_ImmData(0), m_ChannelId(ChannelId) +CEXIChannel::CEXIChannel(u32 ChannelId) : m_ChannelId(ChannelId) { - m_Control.Hex = 0; - m_Status.Hex = 0; - if (m_ChannelId == 0 || m_ChannelId == 1) m_Status.EXTINT = 1; if (m_ChannelId == 1) diff --git a/Source/Core/Core/HW/EXI/EXI_Channel.h b/Source/Core/Core/HW/EXI/EXI_Channel.h index 4dc53b2430..e303d9a3bd 100644 --- a/Source/Core/Core/HW/EXI/EXI_Channel.h +++ b/Source/Core/Core/HW/EXI/EXI_Channel.h @@ -57,7 +57,7 @@ private: // EXI Status Register - "Channel Parameter Register" union UEXI_STATUS { - u32 Hex; + u32 Hex = 0; // DO NOT obey the warning and give this struct a name. Things will fail. struct { @@ -77,14 +77,14 @@ private: u32 ROMDIS : 1; // ROM Disable u32 : 18; }; - UEXI_STATUS() { Hex = 0; } - UEXI_STATUS(u32 _hex) { Hex = _hex; } + UEXI_STATUS() = default; + explicit UEXI_STATUS(u32 hex) : Hex{hex} {} }; // EXI Control Register union UEXI_CONTROL { - u32 Hex; + u32 Hex = 0; struct { u32 TSTART : 1; @@ -97,10 +97,10 @@ private: // STATE_TO_SAVE UEXI_STATUS m_Status; - u32 m_DMAMemoryAddress; - u32 m_DMALength; + u32 m_DMAMemoryAddress = 0; + u32 m_DMALength = 0; UEXI_CONTROL m_Control; - u32 m_ImmData; + u32 m_ImmData = 0; // Devices enum From 5a85001d308fb2797207285fab08d316a19009eb Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sat, 21 Jan 2017 18:50:35 -0500 Subject: [PATCH 3/6] EXI_Channel: Amend variable casing --- Source/Core/Core/HW/EXI/EXI_Channel.cpp | 146 ++++++++++++------------ Source/Core/Core/HW/EXI/EXI_Channel.h | 30 ++--- 2 files changed, 88 insertions(+), 88 deletions(-) diff --git a/Source/Core/Core/HW/EXI/EXI_Channel.cpp b/Source/Core/Core/HW/EXI/EXI_Channel.cpp index ff9e6ed46a..ec2bdb5b87 100644 --- a/Source/Core/Core/HW/EXI/EXI_Channel.cpp +++ b/Source/Core/Core/HW/EXI/EXI_Channel.cpp @@ -20,15 +20,15 @@ enum EXI_READWRITE }; -CEXIChannel::CEXIChannel(u32 ChannelId) : m_ChannelId(ChannelId) +CEXIChannel::CEXIChannel(u32 channel_id) : m_channel_id(channel_id) { - if (m_ChannelId == 0 || m_ChannelId == 1) - m_Status.EXTINT = 1; - if (m_ChannelId == 1) - m_Status.CHIP_SELECT = 1; + if (m_channel_id == 0 || m_channel_id == 1) + m_status.EXTINT = 1; + if (m_channel_id == 1) + m_status.CHIP_SELECT = 1; for (auto& device : m_devices) - device = EXIDevice_Create(EXIDEVICE_NONE, m_ChannelId); + device = EXIDevice_Create(EXIDEVICE_NONE, m_channel_id); } CEXIChannel::~CEXIChannel() @@ -44,115 +44,115 @@ void CEXIChannel::RegisterMMIO(MMIO::Mapping* mmio, u32 base) mmio->Register(base + EXI_STATUS, MMIO::ComplexRead([this](u32) { // check if external device is present // pretty sure it is memcard only, not entirely sure - if (m_ChannelId == 2) + if (m_channel_id == 2) { - m_Status.EXT = 0; + m_status.EXT = 0; } else { - m_Status.EXT = GetDevice(1)->IsPresent() ? 1 : 0; + m_status.EXT = GetDevice(1)->IsPresent() ? 1 : 0; } - return m_Status.Hex; + return m_status.Hex; }), MMIO::ComplexWrite([this](u32, u32 val) { - UEXI_STATUS newStatus(val); + UEXI_STATUS new_status(val); - m_Status.EXIINTMASK = newStatus.EXIINTMASK; - if (newStatus.EXIINT) - m_Status.EXIINT = 0; + m_status.EXIINTMASK = new_status.EXIINTMASK; + if (new_status.EXIINT) + m_status.EXIINT = 0; - m_Status.TCINTMASK = newStatus.TCINTMASK; - if (newStatus.TCINT) - m_Status.TCINT = 0; + m_status.TCINTMASK = new_status.TCINTMASK; + if (new_status.TCINT) + m_status.TCINT = 0; - m_Status.CLK = newStatus.CLK; + m_status.CLK = new_status.CLK; - if (m_ChannelId == 0 || m_ChannelId == 1) + if (m_channel_id == 0 || m_channel_id == 1) { - m_Status.EXTINTMASK = newStatus.EXTINTMASK; + m_status.EXTINTMASK = new_status.EXTINTMASK; - if (newStatus.EXTINT) - m_Status.EXTINT = 0; + if (new_status.EXTINT) + m_status.EXTINT = 0; } - if (m_ChannelId == 0) - m_Status.ROMDIS = newStatus.ROMDIS; + if (m_channel_id == 0) + m_status.ROMDIS = new_status.ROMDIS; - IEXIDevice* pDevice = GetDevice(m_Status.CHIP_SELECT ^ newStatus.CHIP_SELECT); - m_Status.CHIP_SELECT = newStatus.CHIP_SELECT; - if (pDevice != nullptr) - pDevice->SetCS(m_Status.CHIP_SELECT); + IEXIDevice* device = GetDevice(m_status.CHIP_SELECT ^ new_status.CHIP_SELECT); + m_status.CHIP_SELECT = new_status.CHIP_SELECT; + if (device != nullptr) + device->SetCS(m_status.CHIP_SELECT); ExpansionInterface::UpdateInterrupts(); })); - mmio->Register(base + EXI_DMAADDR, MMIO::DirectRead(&m_DMAMemoryAddress), - MMIO::DirectWrite(&m_DMAMemoryAddress)); - mmio->Register(base + EXI_DMALENGTH, MMIO::DirectRead(&m_DMALength), - MMIO::DirectWrite(&m_DMALength)); - mmio->Register(base + EXI_DMACONTROL, MMIO::DirectRead(&m_Control.Hex), + mmio->Register(base + EXI_DMA_ADDRESS, MMIO::DirectRead(&m_dma_memory_address), + MMIO::DirectWrite(&m_dma_memory_address)); + mmio->Register(base + EXI_DMA_LENGTH, MMIO::DirectRead(&m_dma_length), + MMIO::DirectWrite(&m_dma_length)); + mmio->Register(base + EXI_DMA_CONTROL, MMIO::DirectRead(&m_control.Hex), MMIO::ComplexWrite([this](u32, u32 val) { - m_Control.Hex = val; + m_control.Hex = val; - if (m_Control.TSTART) + if (m_control.TSTART) { - IEXIDevice* pDevice = GetDevice(m_Status.CHIP_SELECT); - if (pDevice == nullptr) + IEXIDevice* device = GetDevice(m_status.CHIP_SELECT); + if (device == nullptr) return; - if (m_Control.DMA == 0) + if (m_control.DMA == 0) { // immediate data - switch (m_Control.RW) + switch (m_control.RW) { case EXI_READ: - m_ImmData = pDevice->ImmRead(m_Control.TLEN + 1); + m_imm_data = device->ImmRead(m_control.TLEN + 1); break; case EXI_WRITE: - pDevice->ImmWrite(m_ImmData, m_Control.TLEN + 1); + device->ImmWrite(m_imm_data, m_control.TLEN + 1); break; case EXI_READWRITE: - pDevice->ImmReadWrite(m_ImmData, m_Control.TLEN + 1); + device->ImmReadWrite(m_imm_data, m_control.TLEN + 1); break; default: _dbg_assert_msg_(EXPANSIONINTERFACE, 0, - "EXI Imm: Unknown transfer type %i", m_Control.RW); + "EXI Imm: Unknown transfer type %i", m_control.RW); } } else { // DMA - switch (m_Control.RW) + switch (m_control.RW) { case EXI_READ: - pDevice->DMARead(m_DMAMemoryAddress, m_DMALength); + device->DMARead(m_dma_memory_address, m_dma_length); break; case EXI_WRITE: - pDevice->DMAWrite(m_DMAMemoryAddress, m_DMALength); + device->DMAWrite(m_dma_memory_address, m_dma_length); break; default: _dbg_assert_msg_(EXPANSIONINTERFACE, 0, - "EXI DMA: Unknown transfer type %i", m_Control.RW); + "EXI DMA: Unknown transfer type %i", m_control.RW); } } - m_Control.TSTART = 0; + m_control.TSTART = 0; // Check if device needs specific timing, otherwise just complete transfer // immediately - if (!pDevice->UseDelayedTransferCompletion()) + if (!device->UseDelayedTransferCompletion()) SendTransferComplete(); } })); - mmio->Register(base + EXI_IMMDATA, MMIO::DirectRead(&m_ImmData), - MMIO::DirectWrite(&m_ImmData)); + mmio->Register(base + EXI_IMM_DATA, MMIO::DirectRead(&m_imm_data), + MMIO::DirectWrite(&m_imm_data)); } void CEXIChannel::SendTransferComplete() { - m_Status.TCINT = 1; + m_status.TCINT = 1; ExpansionInterface::UpdateInterrupts(); } @@ -164,7 +164,7 @@ void CEXIChannel::RemoveDevices() void CEXIChannel::AddDevice(const TEXIDevices device_type, const int device_num) { - AddDevice(EXIDevice_Create(device_type, m_ChannelId), device_num); + AddDevice(EXIDevice_Create(device_type, m_channel_id), device_num); } void CEXIChannel::AddDevice(std::unique_ptr device, const int device_num, @@ -178,10 +178,10 @@ void CEXIChannel::AddDevice(std::unique_ptr device, const int device if (notify_presence_changed) { // This means "device presence changed", software has to check - // m_Status.EXT to see if it is now present or not - if (m_ChannelId != 2) + // m_status.EXT to see if it is now present or not + if (m_channel_id != 2) { - m_Status.EXTINT = 1; + m_status.EXTINT = 1; ExpansionInterface::UpdateInterrupts(); } } @@ -189,14 +189,14 @@ void CEXIChannel::AddDevice(std::unique_ptr device, const int device bool CEXIChannel::IsCausingInterrupt() { - if (m_ChannelId != 2 && GetDevice(1)->IsInterruptSet()) - m_Status.EXIINT = 1; // Always check memcard slots - else if (GetDevice(m_Status.CHIP_SELECT)) - if (GetDevice(m_Status.CHIP_SELECT)->IsInterruptSet()) - m_Status.EXIINT = 1; + if (m_channel_id != 2 && GetDevice(1)->IsInterruptSet()) + m_status.EXIINT = 1; // Always check memcard slots + else if (GetDevice(m_status.CHIP_SELECT)) + if (GetDevice(m_status.CHIP_SELECT)->IsInterruptSet()) + m_status.EXIINT = 1; - if ((m_Status.EXIINT & m_Status.EXIINTMASK) || (m_Status.TCINT & m_Status.TCINTMASK) || - (m_Status.EXTINT & m_Status.EXTINTMASK)) + if ((m_status.EXIINT & m_status.EXIINTMASK) || (m_status.TCINT & m_status.TCINTMASK) || + (m_status.EXTINT & m_status.EXTINTMASK)) { return true; } @@ -222,11 +222,11 @@ IEXIDevice* CEXIChannel::GetDevice(const u8 chip_select) void CEXIChannel::DoState(PointerWrap& p) { - p.DoPOD(m_Status); - p.Do(m_DMAMemoryAddress); - p.Do(m_DMALength); - p.Do(m_Control); - p.Do(m_ImmData); + p.DoPOD(m_status); + p.Do(m_dma_memory_address); + p.Do(m_dma_length); + p.Do(m_control); + p.Do(m_imm_data); for (int device_index = 0; device_index < NUM_DEVICES; ++device_index) { @@ -240,24 +240,24 @@ void CEXIChannel::DoState(PointerWrap& p) } else { - std::unique_ptr save_device = EXIDevice_Create(type, m_ChannelId); + std::unique_ptr save_device = EXIDevice_Create(type, m_channel_id); save_device->DoState(p); AddDevice(std::move(save_device), device_index, false); } } } -void CEXIChannel::PauseAndLock(bool doLock, bool unpauseOnUnlock) +void CEXIChannel::PauseAndLock(bool do_lock, bool resume_on_unlock) { for (auto& device : m_devices) - device->PauseAndLock(doLock, unpauseOnUnlock); + device->PauseAndLock(do_lock, resume_on_unlock); } -IEXIDevice* CEXIChannel::FindDevice(TEXIDevices device_type, int customIndex) +IEXIDevice* CEXIChannel::FindDevice(TEXIDevices device_type, int custom_index) { for (auto& sup : m_devices) { - IEXIDevice* device = sup->FindDevice(device_type, customIndex); + IEXIDevice* device = sup->FindDevice(device_type, custom_index); if (device) return device; } diff --git a/Source/Core/Core/HW/EXI/EXI_Channel.h b/Source/Core/Core/HW/EXI/EXI_Channel.h index e303d9a3bd..2e2bb4347d 100644 --- a/Source/Core/Core/HW/EXI/EXI_Channel.h +++ b/Source/Core/Core/HW/EXI/EXI_Channel.h @@ -19,12 +19,12 @@ class Mapping; class CEXIChannel { public: - CEXIChannel(u32 ChannelId); + CEXIChannel(u32 channel_id); ~CEXIChannel(); // get device - IEXIDevice* GetDevice(const u8 _CHIP_SELECT); - IEXIDevice* FindDevice(TEXIDevices device_type, int customIndex = -1); + IEXIDevice* GetDevice(const u8 chip_select); + IEXIDevice* FindDevice(TEXIDevices device_type, int custom_index = -1); void RegisterMMIO(MMIO::Mapping* mmio, u32 base); @@ -39,19 +39,19 @@ public: bool IsCausingInterrupt(); void DoState(PointerWrap& p); - void PauseAndLock(bool doLock, bool unpauseOnUnlock); + void PauseAndLock(bool do_lock, bool resume_on_unlock); // This should only be used to transition interrupts from SP1 to Channel 2 - void SetEXIINT(bool exiint) { m_Status.EXIINT = !!exiint; } + void SetEXIINT(bool exiint) { m_status.EXIINT = !!exiint; } private: enum { EXI_STATUS = 0x00, - EXI_DMAADDR = 0x04, - EXI_DMALENGTH = 0x08, - EXI_DMACONTROL = 0x0C, - EXI_IMMDATA = 0x10 + EXI_DMA_ADDRESS = 0x04, + EXI_DMA_LENGTH = 0x08, + EXI_DMA_CONTROL = 0x0C, + EXI_IMM_DATA = 0x10 }; // EXI Status Register - "Channel Parameter Register" @@ -96,11 +96,11 @@ private: }; // STATE_TO_SAVE - UEXI_STATUS m_Status; - u32 m_DMAMemoryAddress = 0; - u32 m_DMALength = 0; - UEXI_CONTROL m_Control; - u32 m_ImmData = 0; + UEXI_STATUS m_status; + u32 m_dma_memory_address = 0; + u32 m_dma_length = 0; + UEXI_CONTROL m_control; + u32 m_imm_data = 0; // Devices enum @@ -111,5 +111,5 @@ private: std::array, NUM_DEVICES> m_devices; // Since channels operate a bit differently from each other - u32 m_ChannelId; + u32 m_channel_id; }; From edf8a79005d31378d281a05b64c97e352c108aab Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sat, 21 Jan 2017 18:56:57 -0500 Subject: [PATCH 4/6] EXI_Channel: Make constructor explicit --- Source/Core/Core/HW/EXI/EXI_Channel.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Core/Core/HW/EXI/EXI_Channel.h b/Source/Core/Core/HW/EXI/EXI_Channel.h index 2e2bb4347d..2ba54df5be 100644 --- a/Source/Core/Core/HW/EXI/EXI_Channel.h +++ b/Source/Core/Core/HW/EXI/EXI_Channel.h @@ -19,7 +19,7 @@ class Mapping; class CEXIChannel { public: - CEXIChannel(u32 channel_id); + explicit CEXIChannel(u32 channel_id); ~CEXIChannel(); // get device From 387769b4e211aab0254e65275843e74396636a74 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sat, 21 Jan 2017 19:05:14 -0500 Subject: [PATCH 5/6] EXI_Channel: Move SetEXIINT implementation into the cpp file --- Source/Core/Core/HW/EXI/EXI_Channel.cpp | 5 +++++ Source/Core/Core/HW/EXI/EXI_Channel.h | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Source/Core/Core/HW/EXI/EXI_Channel.cpp b/Source/Core/Core/HW/EXI/EXI_Channel.cpp index ec2bdb5b87..4b68371dab 100644 --- a/Source/Core/Core/HW/EXI/EXI_Channel.cpp +++ b/Source/Core/Core/HW/EXI/EXI_Channel.cpp @@ -253,6 +253,11 @@ void CEXIChannel::PauseAndLock(bool do_lock, bool resume_on_unlock) device->PauseAndLock(do_lock, resume_on_unlock); } +void CEXIChannel::SetEXIINT(bool exiint) +{ + m_status.EXIINT = !!exiint; +} + IEXIDevice* CEXIChannel::FindDevice(TEXIDevices device_type, int custom_index) { for (auto& sup : m_devices) diff --git a/Source/Core/Core/HW/EXI/EXI_Channel.h b/Source/Core/Core/HW/EXI/EXI_Channel.h index 2ba54df5be..9ede8f2be7 100644 --- a/Source/Core/Core/HW/EXI/EXI_Channel.h +++ b/Source/Core/Core/HW/EXI/EXI_Channel.h @@ -42,7 +42,7 @@ public: void PauseAndLock(bool do_lock, bool resume_on_unlock); // This should only be used to transition interrupts from SP1 to Channel 2 - void SetEXIINT(bool exiint) { m_status.EXIINT = !!exiint; } + void SetEXIINT(bool exiint); private: enum From d2ff34e5101e9ae8a98b15f03793ec94cf2c9fcd Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sat, 21 Jan 2017 19:08:08 -0500 Subject: [PATCH 6/6] EXI_Channel: Remove const qualifiers from member function declaration parameters Const used on value types only really has a use when used within the definition. --- Source/Core/Core/HW/EXI/EXI_Channel.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/Core/Core/HW/EXI/EXI_Channel.h b/Source/Core/Core/HW/EXI/EXI_Channel.h index 9ede8f2be7..6bada2dc40 100644 --- a/Source/Core/Core/HW/EXI/EXI_Channel.h +++ b/Source/Core/Core/HW/EXI/EXI_Channel.h @@ -23,15 +23,15 @@ public: ~CEXIChannel(); // get device - IEXIDevice* GetDevice(const u8 chip_select); + IEXIDevice* GetDevice(u8 chip_select); IEXIDevice* FindDevice(TEXIDevices device_type, int custom_index = -1); void RegisterMMIO(MMIO::Mapping* mmio, u32 base); void SendTransferComplete(); - void AddDevice(const TEXIDevices device_type, const int device_num); - void AddDevice(std::unique_ptr device, const int device_num, + void AddDevice(TEXIDevices device_type, int device_num); + void AddDevice(std::unique_ptr device, int device_num, bool notify_presence_changed = true); // Remove all devices