On Saturday, 29 August 2015, <axeld@xxxxxxxxxxxxxxxx
<javascript:_e(%7B%7D,'cvml','axeld@xxxxxxxxxxxxxxxx');>> wrote:
hrev49590 adds 2 changesets to branch 'master'
old head: 8a28f8496597f67b9e0f80c1143398763062239b
new head: df5aeb6dda2dae580e5073d0b80f0b0fd98c3dfc
overview:
http://cgit.haiku-os.org/haiku/log/?qt=range&q=df5aeb6dda2d+%5E8a28f8496597
----------------------------------------------------------------------------
5584c22fdd66: AHCI: Fix boot failures due to "Port Connect Change" IRQ
storm.
Signed-off-by: Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>
[ Sylvian Kerjean <sylvain_kerjean@xxxxxxxxxxx>
]
df5aeb6dda2d: AHCI: fixed constant mixup, minor cleanup.
* TRANSITION_... was incorrectly changed from the original patch.
* Divided it into two constants, and also prefixed the new constants with
the register fields they are valid for.
* Fixed incorrect usage of |= and removed the corresponding TODO
comments.
* Moved some reoccurring code into their own methods.
* Added check for the ST bit in the command register for the interrupt
hard reset, too.
* This closes ticket #12295, thanks Anarchos!
[ Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>
]
----------------------------------------------------------------------------
3 files changed, 72 insertions(+), 25 deletions(-)
src/add-ons/kernel/busses/scsi/ahci/ahci_defs.h | 16 +++-
.../kernel/busses/scsi/ahci/ahci_port.cpp | 79 ++++++++++++++------
src/add-ons/kernel/busses/scsi/ahci/ahci_port.h | 2 +
############################################################################
Commit: 5584c22fdd668cdd7f0193c71cabf1a23fa6d3cf
URL: http://cgit.haiku-os.org/haiku/commit/?id=5584c22fdd66
Author: Sylvian Kerjean <sylvain_kerjean@xxxxxxxxxxx>
Date: Sat Aug 8 18:43:03 2015 UTC
Committer: Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>
Commit-Date: Fri Aug 28 17:26:14 2015 UTC
AHCI: Fix boot failures due to "Port Connect Change" IRQ storm.
Signed-off-by: Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>
----------------------------------------------------------------------------
diff --git a/src/add-ons/kernel/busses/scsi/ahci/ahci_defs.h
b/src/add-ons/kernel/busses/scsi/ahci/ahci_defs.h
index c4b0101..e9d49f1 100644
--- a/src/add-ons/kernel/busses/scsi/ahci/ahci_defs.h
+++ b/src/add-ons/kernel/busses/scsi/ahci/ahci_defs.h
@@ -76,6 +76,19 @@ enum {
INT_DHR = (1 << 0), // Device to Host
Register FIS Interrupt/Enable
};
+typedef struct {
+ uint16 reserved : 12;
+ uint8 pmp : 4; // Port Multiplier Port: Not used by AHCI
+ uint8 spm : 4; // Select Power Management: Not used by
AHCI
+ uint8 ipm : 4; // Interface Power Management Transitions
Allowed
+ uint8 spd : 4; // Speed Allowed
+ uint8 det : 4; // Device Detection Initialization
+} _PACKED scontrol;
+
+#define TRANSITIONS_TO_PARTIAL_SLUMBER_DISABLED 0x300
+#define NO_INITIALIZATION 0
+#define INITIALIZATION 1
+
typedef struct {
uint32 clb; // Command List Base
Address (alignment 1024 byte)
@@ -89,7 +102,7 @@ typedef struct {
uint32 tfd; // Task File Data
uint32 sig; // Signature
uint32 ssts; // Serial ATA Status
(SCR0: SStatus)
- uint32 sctl; // Serial ATA Control
(SCR2: SControl)
+ scontrol sctl; // Serial ATA Control
(SCR2: SControl)
uint32 serr; // Serial ATA Error (SCR1:
SError) **RWC**
uint32 sact; // Serial ATA Active
(SCR3: SActive) **RW1**
uint32 ci; // Command Issue
**RW1**
diff --git a/src/add-ons/kernel/busses/scsi/ahci/ahci_port.cpp
b/src/add-ons/kernel/busses/scsi/ahci/ahci_port.cpp
index a045771..ed71461 100644
--- a/src/add-ons/kernel/busses/scsi/ahci/ahci_port.cpp
+++ b/src/add-ons/kernel/busses/scsi/ahci/ahci_port.cpp
@@ -110,7 +110,7 @@ AHCIPort::Init1()
// prdt follows after command table
// disable transitions to partial or slumber state
- fRegs->sctl |= 0x300;
+ fRegs->sctl.ipm |= TRANSITIONS_TO_PARTIAL_SLUMBER_DISABLED; /*TODO
Why "|= and not "=" ??*/
// clear IRQ status bits
fRegs->is = fRegs->is;
@@ -156,7 +156,12 @@ AHCIPort::Init2()
TRACE("is 0x%08" B_PRIx32 "\n", fRegs->is);
TRACE("cmd 0x%08" B_PRIx32 "\n", fRegs->cmd);
TRACE("ssts 0x%08" B_PRIx32 "\n", fRegs->ssts);
- TRACE("sctl 0x%08" B_PRIx32 "\n", fRegs->sctl);
+ TRACE("sctl.reserved 0x%04" B_PRIx16 "\n", fRegs->sctl.reserved);
+ TRACE("sctl.pmp 0x%02" B_PRIx8 "\n", fRegs->sctl.pmp);
+ TRACE("sctl.spm 0x%02" B_PRIx8 "\n", fRegs->sctl.spm);
+ TRACE("sctl.ipm 0x%02" B_PRIx8 "\n", fRegs->sctl.ipm);
+ TRACE("sctl.spd 0x%02" B_PRIx8 "\n", fRegs->sctl.spd);
+ TRACE("sctl.det 0x%02" B_PRIx8 "\n", fRegs->sctl.det);
TRACE("serr 0x%08" B_PRIx32 "\n", fRegs->serr);
TRACE("sact 0x%08" B_PRIx32 "\n", fRegs->sact);
TRACE("tfd 0x%08" B_PRIx32 "\n", fRegs->tfd);
@@ -212,10 +217,10 @@ AHCIPort::ResetDevice()
TRACE("AHCIPort::ResetDevice PORT_CMD_ST set, behaviour
undefined\n");
// perform a hard reset
- fRegs->sctl = (fRegs->sctl & ~0xf) | 1;
+ fRegs->sctl.det |= INITIALIZATION; //TODO Why "|=" instead of "=" ?
FlushPostedWrites();
spin(1100);
- fRegs->sctl &= ~0xf;
+ fRegs->sctl.det = NO_INITIALIZATION;
FlushPostedWrites();
if (wait_until_set(&fRegs->ssts, 0x1, 100000) < B_OK) {
@@ -364,10 +369,15 @@ AHCIPort::InterruptErrorHandler(uint32 is)
TRACE("AHCIPort::InterruptErrorHandler port %d,
fCommandsActive 0x%08"
B_PRIx32 ", is 0x%08" B_PRIx32 ", ci 0x%08"
B_PRIx32 "\n", fIndex,
fCommandsActive, is, ci);
-
- TRACE("ssts 0x%08" B_PRIx32 ", sctl 0x%08" B_PRIx32 ",
serr 0x%08"
- B_PRIx32 ", sact 0x%08" B_PRIx32 "\n",
- fRegs->ssts, fRegs->sctl, fRegs->serr,
fRegs->sact);
+ TRACE("ssts 0x%08" B_PRIx32 "\n", fRegs->ssts);
+ TRACE("sctl.reserved 0x%04" B_PRIx16 "\n",
fRegs->sctl.reserved);
+ TRACE("sctl.pmp 0x%02" B_PRIx8 "\n", fRegs->sctl.pmp);
+ TRACE("sctl.spm 0x%02" B_PRIx8 "\n", fRegs->sctl.spm);
+ TRACE("sctl.ipm 0x%02" B_PRIx8 "\n", fRegs->sctl.ipm);
+ TRACE("sctl.spd 0x%02" B_PRIx8 "\n", fRegs->sctl.spd);
+ TRACE("sctl.det 0x%02" B_PRIx8 "\n", fRegs->sctl.det);
+ TRACE("serr 0x%08" B_PRIx32 "\n", fRegs->serr);
+ TRACE("sact 0x%08" B_PRIx32 "\n", fRegs->sact);
}
// read and clear SError
@@ -413,6 +423,19 @@ AHCIPort::InterruptErrorHandler(uint32 is)
}
if (is & PORT_INT_PC) {
TRACE("Port Connect Change\n");
+ /* spec v1.3, §6.2.2.3 Recovery of Unsolicited COMINIT (a
COMINIT that is
+ * not received as a consequence of issuing a COMRESET to
the device) */
+
+ // perform a hard reset
+ fRegs->sctl.det |= INITIALIZATION; //TODO Why "|=" instead
of "=" ?
+ FlushPostedWrites();
+ spin(1100); // specification says you must wait 1ms
+ fRegs->sctl.det = NO_INITIALIZATION;
+ FlushPostedWrites();
+
+ // clear error bits to clear PxSERR.DIAG.X
+ fRegs->serr = fRegs->serr;
+ FlushPostedWrites();
// fResetPort = true;
}
if (is & PORT_INT_UF) {
############################################################################
Revision: hrev49590
Commit: df5aeb6dda2dae580e5073d0b80f0b0fd98c3dfc
URL: http://cgit.haiku-os.org/haiku/commit/?id=df5aeb6dda2d
Author: Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>
Date: Fri Aug 28 17:22:09 2015 UTC
Ticket: https://dev.haiku-os.org/ticket/12295
AHCI: fixed constant mixup, minor cleanup.
* TRANSITION_... was incorrectly changed from the original patch.
* Divided it into two constants, and also prefixed the new constants with
the register fields they are valid for.
* Fixed incorrect usage of |= and removed the corresponding TODO comments.
* Moved some reoccurring code into their own methods.
* Added check for the ST bit in the command register for the interrupt
hard reset, too.
* This closes ticket #12295, thanks Anarchos!
----------------------------------------------------------------------------
diff --git a/src/add-ons/kernel/busses/scsi/ahci/ahci_defs.h
b/src/add-ons/kernel/busses/scsi/ahci/ahci_defs.h
index e9d49f1..160cbd7 100644
--- a/src/add-ons/kernel/busses/scsi/ahci/ahci_defs.h
+++ b/src/add-ons/kernel/busses/scsi/ahci/ahci_defs.h
@@ -85,9 +85,10 @@ typedef struct {
uint8 det : 4; // Device Detection Initialization
} _PACKED scontrol;
-#define TRANSITIONS_TO_PARTIAL_SLUMBER_DISABLED 0x300
-#define NO_INITIALIZATION 0
-#define INITIALIZATION 1
+#define IPM_TRANSITIONS_TO_PARTIAL_DISABLED 0x1
+#define IPM_TRANSITIONS_TO_SLUMBER_DISABLED 0x2
+#define DET_NO_INITIALIZATION 0x0
+#define DET_INITIALIZATION 0x1
typedef struct {
diff --git a/src/add-ons/kernel/busses/scsi/ahci/ahci_port.cpp
b/src/add-ons/kernel/busses/scsi/ahci/ahci_port.cpp
index ed71461..2cd4ca5 100644
--- a/src/add-ons/kernel/busses/scsi/ahci/ahci_port.cpp
+++ b/src/add-ons/kernel/busses/scsi/ahci/ahci_port.cpp
@@ -1,5 +1,5 @@
/*
- * Copyright 2008-2014 Haiku, Inc. All rights reserved.
+ * Copyright 2008-2015 Haiku, Inc. All rights reserved.
* Copyright 2007-2009, Marcus Overhagen. All rights reserved.
* Distributed under the terms of the MIT License.
*/
@@ -110,7 +110,8 @@ AHCIPort::Init1()
// prdt follows after command table
// disable transitions to partial or slumber state
- fRegs->sctl.ipm |= TRANSITIONS_TO_PARTIAL_SLUMBER_DISABLED; /*TODO
Why "|= and not "=" ??*/
+ fRegs->sctl.ipm = IPM_TRANSITIONS_TO_PARTIAL_DISABLED
+ | IPM_TRANSITIONS_TO_SLUMBER_DISABLED;
// clear IRQ status bits
fRegs->is = fRegs->is;
@@ -213,23 +214,13 @@ AHCIPort::Uninit()
void
AHCIPort::ResetDevice()
{
- if (fRegs->cmd & PORT_CMD_ST)
- TRACE("AHCIPort::ResetDevice PORT_CMD_ST set, behaviour
undefined\n");
-
// perform a hard reset
- fRegs->sctl.det |= INITIALIZATION; //TODO Why "|=" instead of "=" ?
- FlushPostedWrites();
- spin(1100);
- fRegs->sctl.det = NO_INITIALIZATION;
- FlushPostedWrites();
+ _HardReset();
- if (wait_until_set(&fRegs->ssts, 0x1, 100000) < B_OK) {
+ if (wait_until_set(&fRegs->ssts, 0x1, 100000) < B_OK)
TRACE("AHCIPort::ResetDevice port %d no device
detected\n", fIndex);
- }
- // clear error bits
- fRegs->serr = fRegs->serr;
- FlushPostedWrites();
+ _ClearErrorRegister();
if (fRegs->ssts & 1) {
if (wait_until_set(&fRegs->ssts, 0x3, 500000) < B_OK) {
@@ -238,9 +229,7 @@ AHCIPort::ResetDevice()
}
}
- // clear error bits
- fRegs->serr = fRegs->serr;
- FlushPostedWrites();
+ _ClearErrorRegister();
}
@@ -423,20 +412,13 @@ AHCIPort::InterruptErrorHandler(uint32 is)
}
if (is & PORT_INT_PC) {
TRACE("Port Connect Change\n");
- /* spec v1.3, §6.2.2.3 Recovery of Unsolicited COMINIT (a
COMINIT that is
- * not received as a consequence of issuing a COMRESET to
the device) */
+ // Spec v1.3, §6.2.2.3 Recovery of Unsolicited COMINIT
// perform a hard reset
- fRegs->sctl.det |= INITIALIZATION; //TODO Why "|=" instead
of "=" ?
- FlushPostedWrites();
- spin(1100); // specification says you must wait 1ms
- fRegs->sctl.det = NO_INITIALIZATION;
- FlushPostedWrites();
+ _HardReset();
// clear error bits to clear PxSERR.DIAG.X
- fRegs->serr = fRegs->serr;
- FlushPostedWrites();
-// fResetPort = true;
+ _ClearErrorRegister();
}
if (is & PORT_INT_UF) {
TRACE("Unknown FIS\n");
@@ -1247,3 +1229,29 @@ AHCIPort::ScsiGetRestrictions(bool* isATAPI, bool*
noAutoSense,
"maxBlocks %" B_PRIu32 "\n", fIndex, *isATAPI,
*noAutoSense,
*maxBlocks);
}
+
+
+void
+AHCIPort::_HardReset()
+{
+ if ((fRegs->cmd & PORT_CMD_ST) != 0) {
+ // We shouldn't perform a reset, but at least document it
+ TRACE("AHCIPort::_HardReset() PORT_CMD_ST set, behaviour
undefined\n");
+ }
+
+ fRegs->sctl.det = DET_INITIALIZATION;
+ FlushPostedWrites();
+ spin(1100);
+ // You must wait 1ms at minimum
+ fRegs->sctl.det = DET_NO_INITIALIZATION;
+ FlushPostedWrites();
+}
+
+
+void
+AHCIPort::_ClearErrorRegister()
+{
+ // clear error bits
+ fRegs->serr = fRegs->serr;
+ FlushPostedWrites();
+}
diff --git a/src/add-ons/kernel/busses/scsi/ahci/ahci_port.h
b/src/add-ons/kernel/busses/scsi/ahci/ahci_port.h
index c08ea54..7ebb1fc 100644
--- a/src/add-ons/kernel/busses/scsi/ahci/ahci_port.h
+++ b/src/add-ons/kernel/busses/scsi/ahci/ahci_port.h
@@ -51,6 +51,8 @@ private:
status_t WaitForTransfer(int *tfd, bigtime_t timeout);
void FinishTransfer();
+ inline void _HardReset();
+ inline void _ClearErrorRegister();
// uint8 * SetCommandFis(volatile command_list_entry *cmd,
volatile fis *fis, const void *data, size_t dataSize);
status_t FillPrdTable(volatile prd *prdTable, int
*prdCount, int prdMax, const void *data, size_t dataSize);
