From a5c04090503989dbe44ea53cd86c8a416ae92f1b Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 22 Jan 2017 02:30:10 -0500 Subject: [PATCH 1/6] EXI_Device: Move private details below the public interface --- Source/Core/Core/HW/EXI/EXI_Device.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Source/Core/Core/HW/EXI/EXI_Device.h b/Source/Core/Core/HW/EXI/EXI_Device.h index c32ba5edd5..061d5597a5 100644 --- a/Source/Core/Core/HW/EXI/EXI_Device.h +++ b/Source/Core/Core/HW/EXI/EXI_Device.h @@ -30,9 +30,6 @@ enum TEXIDevices : int class IEXIDevice { -private: - // Byte transfer function for this device - virtual void TransferByte(u8&) {} public: // Immediate copy functions virtual void ImmWrite(u32 _uData, u32 _uSize); @@ -59,6 +56,10 @@ public: // type. // I know this class is set up like an interface, but no code requires it to be strictly such. TEXIDevices m_deviceType; + +private: + // Byte transfer function for this device + virtual void TransferByte(u8&) {} }; std::unique_ptr EXIDevice_Create(const TEXIDevices device_type, const int channel_num); From 4115d93c71ca49203e90210eeae44635aae392a6 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 22 Jan 2017 02:35:18 -0500 Subject: [PATCH 2/6] EXI_Device: Move destructor to beginning of public section Constructors and destructors should be the first thing shown in a public interface. --- Source/Core/Core/HW/EXI/EXI_Device.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Source/Core/Core/HW/EXI/EXI_Device.h b/Source/Core/Core/HW/EXI/EXI_Device.h index 061d5597a5..a2228a07f9 100644 --- a/Source/Core/Core/HW/EXI/EXI_Device.h +++ b/Source/Core/Core/HW/EXI/EXI_Device.h @@ -31,6 +31,8 @@ enum TEXIDevices : int class IEXIDevice { public: + virtual ~IEXIDevice() = default; + // Immediate copy functions virtual void ImmWrite(u32 _uData, u32 _uSize); virtual u32 ImmRead(u32 _uSize); @@ -51,7 +53,6 @@ public: // Is generating interrupt ? virtual bool IsInterruptSet() { return false; } - virtual ~IEXIDevice() {} // for savestates. storing it here seemed cleaner than requiring each implementation to report its // type. // I know this class is set up like an interface, but no code requires it to be strictly such. From e41a6ac9a3905e877c831276f1796affa591ab7d Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 22 Jan 2017 04:06:51 -0500 Subject: [PATCH 3/6] EXI_Device: Amend variable naming --- Source/Core/Core/HW/EXI/EXI_Channel.cpp | 4 +- Source/Core/Core/HW/EXI/EXI_Device.cpp | 48 +++++++++---------- Source/Core/Core/HW/EXI/EXI_Device.h | 18 +++---- .../Core/Core/HW/EXI/EXI_DeviceMemoryCard.cpp | 2 +- 4 files changed, 35 insertions(+), 37 deletions(-) diff --git a/Source/Core/Core/HW/EXI/EXI_Channel.cpp b/Source/Core/Core/HW/EXI/EXI_Channel.cpp index 4b68371dab..1305ffd253 100644 --- a/Source/Core/Core/HW/EXI/EXI_Channel.cpp +++ b/Source/Core/Core/HW/EXI/EXI_Channel.cpp @@ -231,10 +231,10 @@ void CEXIChannel::DoState(PointerWrap& p) for (int device_index = 0; device_index < NUM_DEVICES; ++device_index) { std::unique_ptr& device = m_devices[device_index]; - TEXIDevices type = device->m_deviceType; + TEXIDevices type = device->m_device_type; p.Do(type); - if (type == device->m_deviceType) + if (type == device->m_device_type) { device->DoState(p); } diff --git a/Source/Core/Core/HW/EXI/EXI_Device.cpp b/Source/Core/Core/HW/EXI/EXI_Device.cpp index bddf057c80..06df862990 100644 --- a/Source/Core/Core/HW/EXI/EXI_Device.cpp +++ b/Source/Core/Core/HW/EXI/EXI_Device.cpp @@ -17,47 +17,45 @@ #include "Core/HW/EXI/EXI_DeviceMic.h" #include "Core/HW/Memmap.h" -void IEXIDevice::ImmWrite(u32 _uData, u32 _uSize) +void IEXIDevice::ImmWrite(u32 data, u32 size) { - while (_uSize--) + while (size--) { - u8 uByte = _uData >> 24; - TransferByte(uByte); - _uData <<= 8; + u8 byte = data >> 24; + TransferByte(byte); + data <<= 8; } } -u32 IEXIDevice::ImmRead(u32 _uSize) +u32 IEXIDevice::ImmRead(u32 size) { - u32 uResult = 0; - u32 uPosition = 0; - while (_uSize--) + u32 result = 0; + u32 position = 0; + while (size--) { - u8 uByte = 0; - TransferByte(uByte); - uResult |= uByte << (24 - (uPosition++ * 8)); + u8 byte = 0; + TransferByte(byte); + result |= byte << (24 - (position++ * 8)); } - return uResult; + return result; } -void IEXIDevice::DMAWrite(u32 _uAddr, u32 _uSize) +void IEXIDevice::DMAWrite(u32 address, u32 size) { - // _dbg_assert_(EXPANSIONINTERFACE, 0); - while (_uSize--) + while (size--) { - u8 uByte = Memory::Read_U8(_uAddr++); - TransferByte(uByte); + u8 byte = Memory::Read_U8(address++); + TransferByte(byte); } } -void IEXIDevice::DMARead(u32 _uAddr, u32 _uSize) +void IEXIDevice::DMARead(u32 address, u32 size) { - // _dbg_assert_(EXPANSIONINTERFACE, 0); - while (_uSize--) + while (size--) { - u8 uByte = 0; - TransferByte(uByte); - Memory::Write_U8(uByte, _uAddr++); + u8 byte = 0; + TransferByte(byte); + Memory::Write_U8(byte, address++); } } @@ -111,7 +109,7 @@ std::unique_ptr EXIDevice_Create(TEXIDevices device_type, const int } if (result != nullptr) - result->m_deviceType = device_type; + result->m_device_type = device_type; return result; } diff --git a/Source/Core/Core/HW/EXI/EXI_Device.h b/Source/Core/Core/HW/EXI/EXI_Device.h index a2228a07f9..3b83cecb09 100644 --- a/Source/Core/Core/HW/EXI/EXI_Device.h +++ b/Source/Core/Core/HW/EXI/EXI_Device.h @@ -34,21 +34,21 @@ public: virtual ~IEXIDevice() = default; // Immediate copy functions - virtual void ImmWrite(u32 _uData, u32 _uSize); - virtual u32 ImmRead(u32 _uSize); - virtual void ImmReadWrite(u32& /*_uData*/, u32 /*_uSize*/) {} + virtual void ImmWrite(u32 data, u32 size); + virtual u32 ImmRead(u32 size); + virtual void ImmReadWrite(u32& /*data*/, u32 /*size*/) {} // DMA copy functions - virtual void DMAWrite(u32 _uAddr, u32 _uSize); - virtual void DMARead(u32 _uAddr, u32 _uSize); + virtual void DMAWrite(u32 address, u32 size); + virtual void DMARead(u32 address, u32 size); virtual bool UseDelayedTransferCompletion() const { return false; } virtual bool IsPresent() const { return false; } virtual void SetCS(int) {} virtual void DoState(PointerWrap&) {} - virtual void PauseAndLock(bool doLock, bool unpauseOnUnlock = true) {} - virtual IEXIDevice* FindDevice(TEXIDevices device_type, int customIndex = -1) + virtual void PauseAndLock(bool do_lock, bool resume_on_unlock = true) {} + virtual IEXIDevice* FindDevice(TEXIDevices device_type, int custom_index = -1) { - return (device_type == m_deviceType) ? this : nullptr; + return (device_type == m_device_type) ? this : nullptr; } // Is generating interrupt ? @@ -56,7 +56,7 @@ public: // for savestates. storing it here seemed cleaner than requiring each implementation to report its // type. // I know this class is set up like an interface, but no code requires it to be strictly such. - TEXIDevices m_deviceType; + TEXIDevices m_device_type; private: // Byte transfer function for this device diff --git a/Source/Core/Core/HW/EXI/EXI_DeviceMemoryCard.cpp b/Source/Core/Core/HW/EXI/EXI_DeviceMemoryCard.cpp index b02345060c..9eb20b6ade 100644 --- a/Source/Core/Core/HW/EXI/EXI_DeviceMemoryCard.cpp +++ b/Source/Core/Core/HW/EXI/EXI_DeviceMemoryCard.cpp @@ -488,7 +488,7 @@ void CEXIMemoryCard::DoState(PointerWrap& p) IEXIDevice* CEXIMemoryCard::FindDevice(TEXIDevices device_type, int customIndex) { - if (device_type != m_deviceType) + if (device_type != m_device_type) return nullptr; if (customIndex != card_index) return nullptr; From 07a61b0d15d054f542fcc00b7ca30574e2af031e Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 22 Jan 2017 04:16:25 -0500 Subject: [PATCH 4/6] EXI_Device: Move implementation details into the cpp file Any change to the default behavior of any device methods now won't require the recompilation of all EXI devices. --- Source/Core/Core/HW/EXI/EXI_Device.cpp | 40 ++++++++++++++++++++++++++ Source/Core/Core/HW/EXI/EXI_Device.h | 30 +++++++++---------- 2 files changed, 55 insertions(+), 15 deletions(-) diff --git a/Source/Core/Core/HW/EXI/EXI_Device.cpp b/Source/Core/Core/HW/EXI/EXI_Device.cpp index 06df862990..05d0a4818f 100644 --- a/Source/Core/Core/HW/EXI/EXI_Device.cpp +++ b/Source/Core/Core/HW/EXI/EXI_Device.cpp @@ -40,6 +40,10 @@ u32 IEXIDevice::ImmRead(u32 size) return result; } +void IEXIDevice::ImmReadWrite(u32& data, u32 size) +{ +} + void IEXIDevice::DMAWrite(u32 address, u32 size) { while (size--) @@ -59,6 +63,42 @@ void IEXIDevice::DMARead(u32 address, u32 size) } } +IEXIDevice* IEXIDevice::FindDevice(TEXIDevices device_type, int custom_index) +{ + return (device_type == m_device_type) ? this : nullptr; +} + +bool IEXIDevice::UseDelayedTransferCompletion() const +{ + return false; +} + +bool IEXIDevice::IsPresent() const +{ + return false; +} + +void IEXIDevice::SetCS(int cs) +{ +} + +void IEXIDevice::DoState(PointerWrap& p) +{ +} + +void IEXIDevice::PauseAndLock(bool do_lock, bool resume_on_unlock) +{ +} + +bool IEXIDevice::IsInterruptSet() +{ + return false; +} + +void IEXIDevice::TransferByte(u8& byte) +{ +} + // F A C T O R Y std::unique_ptr EXIDevice_Create(TEXIDevices device_type, const int channel_num) { diff --git a/Source/Core/Core/HW/EXI/EXI_Device.h b/Source/Core/Core/HW/EXI/EXI_Device.h index 3b83cecb09..c660d4098b 100644 --- a/Source/Core/Core/HW/EXI/EXI_Device.h +++ b/Source/Core/Core/HW/EXI/EXI_Device.h @@ -36,31 +36,31 @@ public: // Immediate copy functions virtual void ImmWrite(u32 data, u32 size); virtual u32 ImmRead(u32 size); - virtual void ImmReadWrite(u32& /*data*/, u32 /*size*/) {} + virtual void ImmReadWrite(u32& data, u32 size); + // DMA copy functions virtual void DMAWrite(u32 address, u32 size); virtual void DMARead(u32 address, u32 size); - virtual bool UseDelayedTransferCompletion() const { return false; } - virtual bool IsPresent() const { return false; } - virtual void SetCS(int) {} - virtual void DoState(PointerWrap&) {} - virtual void PauseAndLock(bool do_lock, bool resume_on_unlock = true) {} - virtual IEXIDevice* FindDevice(TEXIDevices device_type, int custom_index = -1) - { - return (device_type == m_device_type) ? this : nullptr; - } + virtual IEXIDevice* FindDevice(TEXIDevices device_type, int custom_index = -1); + + virtual bool UseDelayedTransferCompletion() const; + virtual bool IsPresent() const; + virtual void SetCS(int cs); + virtual void DoState(PointerWrap& p); + virtual void PauseAndLock(bool do_lock, bool resume_on_unlock = true); // Is generating interrupt ? - virtual bool IsInterruptSet() { return false; } - // for savestates. storing it here seemed cleaner than requiring each implementation to report its - // type. - // I know this class is set up like an interface, but no code requires it to be strictly such. + virtual bool IsInterruptSet(); + + // For savestates. storing it here seemed cleaner than requiring each implementation to report its + // type. I know this class is set up like an interface, but no code requires it to be strictly + // such. TEXIDevices m_device_type; private: // Byte transfer function for this device - virtual void TransferByte(u8&) {} + virtual void TransferByte(u8& byte); }; std::unique_ptr EXIDevice_Create(const TEXIDevices device_type, const int channel_num); From 2e85ddef6032e3e9a5bcd15952834c13538a5ffe Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 22 Jan 2017 04:17:44 -0500 Subject: [PATCH 5/6] EXI_Device: Remove unnecessary const on EXIDevice_Create declaration parameters These are only relevant on its definition. --- Source/Core/Core/HW/EXI/EXI_Device.cpp | 2 +- Source/Core/Core/HW/EXI/EXI_Device.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/Core/Core/HW/EXI/EXI_Device.cpp b/Source/Core/Core/HW/EXI/EXI_Device.cpp index 05d0a4818f..69f7447db0 100644 --- a/Source/Core/Core/HW/EXI/EXI_Device.cpp +++ b/Source/Core/Core/HW/EXI/EXI_Device.cpp @@ -100,7 +100,7 @@ void IEXIDevice::TransferByte(u8& byte) } // F A C T O R Y -std::unique_ptr EXIDevice_Create(TEXIDevices device_type, const int channel_num) +std::unique_ptr EXIDevice_Create(const TEXIDevices device_type, const int channel_num) { std::unique_ptr result; diff --git a/Source/Core/Core/HW/EXI/EXI_Device.h b/Source/Core/Core/HW/EXI/EXI_Device.h index c660d4098b..89be2ce30c 100644 --- a/Source/Core/Core/HW/EXI/EXI_Device.h +++ b/Source/Core/Core/HW/EXI/EXI_Device.h @@ -63,4 +63,4 @@ private: virtual void TransferByte(u8& byte); }; -std::unique_ptr EXIDevice_Create(const TEXIDevices device_type, const int channel_num); +std::unique_ptr EXIDevice_Create(TEXIDevices device_type, int channel_num); From 6cddc1be9525681dd259353e6fb5b8df49cf6158 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 22 Jan 2017 04:18:52 -0500 Subject: [PATCH 6/6] EXI_Device: Get rid of an unnecesary cast --- Source/Core/Core/HW/EXI/EXI_Device.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Core/Core/HW/EXI/EXI_Device.h b/Source/Core/Core/HW/EXI/EXI_Device.h index 89be2ce30c..ee622324b1 100644 --- a/Source/Core/Core/HW/EXI/EXI_Device.h +++ b/Source/Core/Core/HW/EXI/EXI_Device.h @@ -25,7 +25,7 @@ enum TEXIDevices : int // Converted to EXIDEVICE_MEMORYCARD internally. EXIDEVICE_MEMORYCARDFOLDER, EXIDEVICE_AGP, - EXIDEVICE_NONE = (u8)-1 + EXIDEVICE_NONE = 0xFF }; class IEXIDevice