Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/55583 )
(
3 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the
submitted one.
)Change subject: dev: Clean up the IDE disk and controller classes a
little.
......................................................................
dev: Clean up the IDE disk and controller classes a little.
Fix some style issues, and replace some if () panics with panic_ifs.
Change-Id: Ic4fae016520e43d32f435bf3fc0ec37df25ca02a
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/55583
Reviewed-by: Andreas Sandberg <andreas.sandb...@arm.com>
Maintainer: Andreas Sandberg <andreas.sandb...@arm.com>
Tested-by: kokoro <noreply+kok...@google.com>
---
M src/dev/storage/ide_ctrl.cc
M src/dev/storage/ide_disk.cc
M src/dev/storage/ide_disk.hh
3 files changed, 56 insertions(+), 40 deletions(-)
Approvals:
Andreas Sandberg: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/dev/storage/ide_ctrl.cc b/src/dev/storage/ide_ctrl.cc
index 7d4ecac..701314f 100644
--- a/src/dev/storage/ide_ctrl.cc
+++ b/src/dev/storage/ide_ctrl.cc
@@ -50,10 +50,6 @@
#include "params/IdeController.hh"
#include "sim/byteswap.hh"
-// clang complains about std::set being overloaded with Packet::set if
-// we open up the entire namespace std
-using std::string;
-
namespace gem5
{
@@ -65,7 +61,7 @@
BMIDescTablePtr = 0x4
};
-IdeController::Channel::Channel(string newName) : _name(newName)
+IdeController::Channel::Channel(std::string newName) : _name(newName)
{
bmiRegs.reset();
bmiRegs.status.dmaCap0 = 1;
@@ -287,8 +283,8 @@
newVal.intStatus = 0; // clear the interrupt?
} else {
// Assigning two bitunion fields to each other does not
- // work as intended, so we need to use this temporary
variable
- // to get around the bug.
+ // work as intended, so we need to use this temporary
+ // variable to get around the bug.
uint8_t tmp = oldVal.intStatus;
newVal.intStatus = tmp;
}
@@ -309,8 +305,9 @@
break;
default:
if (size != sizeof(uint8_t) && size != sizeof(uint16_t) &&
- size != sizeof(uint32_t))
- panic("IDE controller write of invalid write size: %x\n",
size);
+ size != sizeof(uint32_t)) {
+ panic("IDE controller write of invalid size: %x\n", size);
+ }
memcpy((uint8_t *)&bmiRegs + offset, data, size);
}
}
diff --git a/src/dev/storage/ide_disk.cc b/src/dev/storage/ide_disk.cc
index a7185e4..a5e52b0 100644
--- a/src/dev/storage/ide_disk.cc
+++ b/src/dev/storage/ide_disk.cc
@@ -538,8 +538,9 @@
void
IdeDisk::dmaWriteDone()
{
- DPRINTF(IdeDisk, "doWriteDone: curPrd byte count %d, eot %#x cmd bytes
left:%d\n",
- curPrd.getByteCount(), curPrd.getEOT(), cmdBytesLeft);
+ DPRINTF(IdeDisk,
+ "doWriteDone: curPrd byte count %d, eot %#x cmd bytes
left:%d\n",
+ curPrd.getByteCount(), curPrd.getEOT(), cmdBytesLeft);
// check for the EOT
if (curPrd.getEOT()) {
assert(cmdBytesLeft == 0);
@@ -559,9 +560,9 @@
{
uint32_t bytesRead = image->read(data, sector);
- if (bytesRead != SectorSize)
- panic("Can't read from %s. Only %d of %d read. errno=%d\n",
- name(), bytesRead, SectorSize, errno);
+ panic_if(bytesRead != SectorSize,
+ "Can't read from %s. Only %d of %d read. errno=%d",
+ name(), bytesRead, SectorSize, errno);
}
void
@@ -569,9 +570,9 @@
{
uint32_t bytesWritten = image->write(data, sector);
- if (bytesWritten != SectorSize)
- panic("Can't write to %s. Only %d of %d written. errno=%d\n",
- name(), bytesWritten, SectorSize, errno);
+ panic_if(bytesWritten != SectorSize,
+ "Can't write to %s. Only %d of %d written. errno=%d",
+ name(), bytesWritten, SectorSize, errno);
}
////
@@ -581,11 +582,11 @@
void
IdeDisk::startDma(const uint32_t &prdTableBase)
{
- if (dmaState != Dma_Start)
- panic("Inconsistent DMA state, should be in Dma_Start!\n");
+ panic_if(dmaState != Dma_Start,
+ "Inconsistent DMA state, should be in Dma_Start!");
- if (devState != Transfer_Data_Dma)
- panic("Inconsistent device state for DMA start!\n");
+ panic_if(devState != Transfer_Data_Dma,
+ "Inconsistent device state for DMA start!");
// PRD base address is given by bits 31:2
curPrdAddr = pciToDma((Addr)(prdTableBase & ~0x3ULL));
@@ -599,11 +600,11 @@
void
IdeDisk::abortDma()
{
- if (dmaState == Dma_Idle)
- panic("Inconsistent DMA state, should be Start or Transfer!");
+ panic_if(dmaState == Dma_Idle,
+ "Inconsistent DMA state, should be Start or Transfer!");
- if (devState != Transfer_Data_Dma && devState != Prepare_Data_Dma)
- panic("Inconsistent device state, should be Transfer or
Prepare!\n");
+ panic_if(devState != Transfer_Data_Dma && devState != Prepare_Data_Dma,
+ "Inconsistent device state, should be Transfer or Prepare!");
updateState(ACT_CMD_ERROR);
}
@@ -652,8 +653,8 @@
case WDCC_READMULTI:
case WDCC_READ:
- if (!(cmdReg.drive & DRIVE_LBA_BIT))
- panic("Attempt to perform CHS access, only supports LBA\n");
+ panic_if(!(cmdReg.drive & DRIVE_LBA_BIT),
+ "Attempt to perform CHS access, only supports LBA");
if (cmdReg.sec_count == 0)
cmdBytes = cmdBytesLeft = (256 * SectorSize);
@@ -670,8 +671,8 @@
// Supported PIO data-out commands
case WDCC_WRITEMULTI:
case WDCC_WRITE:
- if (!(cmdReg.drive & DRIVE_LBA_BIT))
- panic("Attempt to perform CHS access, only supports LBA\n");
+ panic_if(!(cmdReg.drive & DRIVE_LBA_BIT),
+ "Attempt to perform CHS access, only supports LBA");
if (cmdReg.sec_count == 0)
cmdBytes = cmdBytesLeft = (256 * SectorSize);
@@ -689,14 +690,15 @@
dmaRead = true; // a write to the disk is a DMA read from memory
[[fallthrough]];
case WDCC_READDMA:
- if (!(cmdReg.drive & DRIVE_LBA_BIT))
- panic("Attempt to perform CHS access, only supports LBA\n");
+ panic_if(!(cmdReg.drive & DRIVE_LBA_BIT),
+ "Attempt to perform CHS access, only supports LBA");
if (cmdReg.sec_count == 0)
cmdBytes = cmdBytesLeft = (256 * SectorSize);
else
cmdBytes = cmdBytesLeft = (cmdReg.sec_count * SectorSize);
- DPRINTF(IdeDisk, "Setting cmdBytesLeft to %d in readdma\n",
cmdBytesLeft);
+ DPRINTF(IdeDisk, "Setting cmdBytesLeft to %d in readdma\n",
+ cmdBytesLeft);
curSector = getLBABase();
@@ -728,8 +730,7 @@
IdeDisk::intrPost()
{
DPRINTF(IdeDisk, "Posting Interrupt\n");
- if (intrPending)
- panic("Attempt to post an interrupt with one pending\n");
+ panic_if(intrPending, "Attempt to post an interrupt with one pending");
intrPending = true;
@@ -743,8 +744,7 @@
IdeDisk::intrClear()
{
DPRINTF(IdeDisk, "Clearing Interrupt\n");
- if (!intrPending)
- panic("Attempt to clear a non-pending interrupt\n");
+ panic_if(!intrPending, "Attempt to clear a non-pending interrupt");
intrPending = false;
diff --git a/src/dev/storage/ide_disk.hh b/src/dev/storage/ide_disk.hh
index 95eec24..96bbd4a 100644
--- a/src/dev/storage/ide_disk.hh
+++ b/src/dev/storage/ide_disk.hh
@@ -355,7 +355,8 @@
bool isIENSet() { return nIENBit; }
bool isDEVSelect();
- void setComplete()
+ void
+ setComplete()
{
// clear out the status byte
status = 0;
@@ -365,10 +366,13 @@
status |= STATUS_SEEK_BIT;
}
- uint32_t getLBABase()
+ uint32_t
+ getLBABase()
{
- return (Addr)(((cmdReg.head & 0xf) << 24) | (cmdReg.cyl_high <<
16) |
- (cmdReg.cyl_low << 8) | (cmdReg.sec_num));
+ return ((cmdReg.head & 0xf) << 24) |
+ (cmdReg.cyl_high << 16) |
+ (cmdReg.cyl_low << 8) |
+ (cmdReg.sec_num);
}
inline Addr pciToDma(Addr pciAddr);
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/55583
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Ic4fae016520e43d32f435bf3fc0ec37df25ca02a
Gerrit-Change-Number: 55583
Gerrit-PatchSet: 5
Gerrit-Owner: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Reviewer: Andreas Sandberg <andreas.sandb...@arm.com>
Gerrit-Reviewer: Daniel Carvalho <oda...@yahoo.com.br>
Gerrit-Reviewer: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Reviewer: Matt Sinclair <mattdsincl...@gmail.com>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
