From df5aeb6dda2dae580e5073d0b80f0b0fd98c3dfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Axel=20D=C3=B6rfler?= Date: Fri, 28 Aug 2015 19:22:09 +0200 Subject: [PATCH] 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! --- .../kernel/busses/scsi/ahci/ahci_defs.h | 7 +- .../kernel/busses/scsi/ahci/ahci_port.cpp | 64 +++++++++++-------- .../kernel/busses/scsi/ahci/ahci_port.h | 2 + 3 files changed, 42 insertions(+), 31 deletions(-) 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 e9d49f1703..160cbd7a47 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 ed71461694..2cd4ca5369 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 c08ea54a1f..7ebb1fcc49 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);