10 Jun
2016
10 Jun
'16
8:27 a.m.
Author: apriyadarshi
Date: Fri Jun 10 08:27:20 2016
New Revision: 71607
URL: http://svn.reactos.org/svn/reactos?rev=71607&view=rev
Log:
Code Review #2
Modified:
branches/GSoC_2016/AHCI/drivers/storage/storahci/storahci.c
branches/GSoC_2016/AHCI/drivers/storage/storahci/storahci.h
Modified: branches/GSoC_2016/AHCI/drivers/storage/storahci/storahci.c
URL:
http://svn.reactos.org/svn/reactos/branches/GSoC_2016/AHCI/drivers/storage/…
==============================================================================
--- branches/GSoC_2016/AHCI/drivers/storage/storahci/storahci.c [iso-8859-1] (original)
+++ branches/GSoC_2016/AHCI/drivers/storage/storahci/storahci.c [iso-8859-1] Fri Jun 10
08:27:20 2016
@@ -13,49 +13,49 @@
*
* Initialize port by setting up PxCLB & PxFB Registers
*
- * @param portExtension
+ * @param PortExtension
*
* @return
* Return true if intialization was successful
*/
BOOLEAN
AhciPortInitialize (
- __in PAHCI_PORT_EXTENSION portExtension
- )
-{
- ULONG mappedLength;
+ __in PAHCI_PORT_EXTENSION PortExtension
+ )
+{
+ ULONG mappedLength, portNumber;
PAHCI_MEMORY_REGISTERS abar;
PAHCI_ADAPTER_EXTENSION adapterExtension;
STOR_PHYSICAL_ADDRESS commandListPhysical, receivedFISPhysical;
DebugPrint("AhciPortInitialize()\n");
- NT_ASSERT(portExtension != NULL);
-
- adapterExtension = portExtension->AdapterExtension;
+ adapterExtension = PortExtension->AdapterExtension;
abar = adapterExtension->ABAR_Address;
+ portNumber = PortExtension->PortNumber;
NT_ASSERT(abar != NULL);
-
- portExtension->Port = &abar->PortList[portExtension->PortNumber];
-
- commandListPhysical = StorPortGetPhysicalAddress( adapterExtension,
- NULL,
- portExtension->CommandList,
- &mappedLength);
-
- if ((mappedLength) == 0 || ((commandListPhysical.LowPart % 1024) != 0))
+ NT_ASSERT(portNumber < MAXIMUM_AHCI_PORT_COUNT);
+
+ PortExtension->Port = &abar->PortList[portNumber];
+
+ commandListPhysical = StorPortGetPhysicalAddress(adapterExtension,
+ NULL,
+ PortExtension->CommandList,
+ &mappedLength);
+
+ if ((mappedLength == 0) || ((commandListPhysical.LowPart % 1024) != 0))
{
DebugPrint("\tcommandListPhysical mappedLength:%d\n", mappedLength);
return FALSE;
}
- receivedFISPhysical = StorPortGetPhysicalAddress( adapterExtension,
- NULL,
- portExtension->ReceivedFIS,
- &mappedLength);
-
- if ((mappedLength) == 0 || ((receivedFISPhysical.LowPart % 1024) != 0))
+ receivedFISPhysical = StorPortGetPhysicalAddress(adapterExtension,
+ NULL,
+ PortExtension->ReceivedFIS,
+ &mappedLength);
+
+ if ((mappedLength == 0) || ((receivedFISPhysical.LowPart % 256) != 0))
{
DebugPrint("\treceivedFISPhysical mappedLength:%d\n", mappedLength);
return FALSE;
@@ -71,16 +71,16 @@
// ï· PxCLB and PxCLBU (if CAP.S64A is set to â1â)
// ï· PxFB and PxFBU (if CAP.S64A is set to â1â)
// Note: Assuming 32bit support only
- StorPortWriteRegisterUlong(adapterExtension, &portExtension->Port->CLB,
commandListPhysical.LowPart);
- StorPortWriteRegisterUlong(adapterExtension, &portExtension->Port->FB,
receivedFISPhysical.LowPart);
+ StorPortWriteRegisterUlong(adapterExtension, &PortExtension->Port->CLB,
commandListPhysical.LowPart);
+ StorPortWriteRegisterUlong(adapterExtension, &PortExtension->Port->FB,
receivedFISPhysical.LowPart);
// set device power state flag to D0
- portExtension->DevicePowerState = StorPowerDeviceD0;
+ PortExtension->DevicePowerState = StorPowerDeviceD0;
// clear pending interrupts
- StorPortWriteRegisterUlong(adapterExtension, &portExtension->Port->SERR,
(ULONG)-1);
- StorPortWriteRegisterUlong(adapterExtension, &portExtension->Port->IS,
(ULONG)-1);
- StorPortWriteRegisterUlong(adapterExtension,
portExtension->AdapterExtension->IS, (1 << portExtension->PortNumber));
+ StorPortWriteRegisterUlong(adapterExtension, &PortExtension->Port->SERR,
(ULONG)-1);
+ StorPortWriteRegisterUlong(adapterExtension, &PortExtension->Port->IS,
(ULONG)-1);
+ StorPortWriteRegisterUlong(adapterExtension,
PortExtension->AdapterExtension->IS, (1 << PortExtension->PortNumber));
return TRUE;
}// -- AhciPortInitialize();
@@ -91,7 +91,7 @@
*
* Allocate memory from poll for required pointers
*
- * @param adapterExtension
+ * @param AdapterExtension
* @param ConfigInfo
*
* @return
@@ -99,8 +99,8 @@
*/
BOOLEAN
AhciAllocateResourceForAdapter (
- __in PAHCI_ADAPTER_EXTENSION adapterExtension,
- __in PPORT_CONFIGURATION_INFORMATION ConfigInfo
+ __in PAHCI_ADAPTER_EXTENSION AdapterExtension,
+ __in PPORT_CONFIGURATION_INFORMATION ConfigInfo
)
{
PVOID portsExtension = NULL;
@@ -110,19 +110,23 @@
DebugPrint("AhciAllocateResourceForAdapter()\n");
- NCS = AHCI_Global_Port_CAP_NCS(adapterExtension->CAP);
+ NCS = AHCI_Global_Port_CAP_NCS(AdapterExtension->CAP);
AlignedNCS = ROUND_UP(NCS, 8);
- // get port count -- Number of set bits in `adapterExtension->PortImplemented`
+ // get port count -- Number of set bits in `AdapterExtension->PortImplemented`
portCount = 0;
- portImplemented = adapterExtension->PortImplemented;
+ portImplemented = AdapterExtension->PortImplemented;
+
+ // make sure we don't allocate too much memory for the ports we have not
implemented
+ // LOGIC: AND with all MAXIMUM_AHCI_PORT_COUNT (low significant) bits set
+ portImplemented = portImplemented & ((1 << MAXIMUM_AHCI_PORT_COUNT) - 1);
while (portImplemented > 0)
{
portCount++;
portImplemented &= (portImplemented - 1);
}
- NT_ASSERT(portCount != 0);
+ NT_ASSERT(portCount <= MAXIMUM_AHCI_PORT_COUNT);
DebugPrint("\tPort Count: %d\n", portCount);
nonCachedExtensionSize = sizeof(AHCI_COMMAND_HEADER) * AlignedNCS + //should be 1K
aligned
@@ -131,29 +135,29 @@
// align nonCachedExtensionSize to 1024
nonCachedExtensionSize = ROUND_UP(nonCachedExtensionSize, 1024);
- adapterExtension->NonCachedExtension = StorPortGetUncachedExtension(
adapterExtension,
- ConfigInfo,
-
nonCachedExtensionSize * portCount);
-
- if (adapterExtension->NonCachedExtension == NULL)
+ AdapterExtension->NonCachedExtension =
StorPortGetUncachedExtension(AdapterExtension,
+ ConfigInfo,
+
nonCachedExtensionSize * portCount);
+
+ if (AdapterExtension->NonCachedExtension == NULL)
{
DebugPrint("\tadapterExtension->NonCachedExtension == NULL\n");
return FALSE;
}
- nonCachedExtension = adapterExtension->NonCachedExtension;
+ nonCachedExtension = AdapterExtension->NonCachedExtension;
AhciZeroMemory(nonCachedExtension, nonCachedExtensionSize * portCount);
for (index = 0; index < MAXIMUM_AHCI_PORT_COUNT; index++)
{
- adapterExtension->PortExtension[index].IsActive = FALSE;
- if ((adapterExtension->PortImplemented & (1 << index)) != 0)
+ AdapterExtension->PortExtension[index].IsActive = FALSE;
+ if ((AdapterExtension->PortImplemented & (1 << index)) != 0)
{
- adapterExtension->PortExtension[index].PortNumber = index;
- adapterExtension->PortExtension[index].IsActive = TRUE;
- adapterExtension->PortExtension[index].AdapterExtension =
adapterExtension;
- adapterExtension->PortExtension[index].CommandList = nonCachedExtension;
- adapterExtension->PortExtension[index].ReceivedFIS =
(PAHCI_RECEIVED_FIS)(nonCachedExtension + sizeof(AHCI_COMMAND_HEADER) * AlignedNCS);
+ AdapterExtension->PortExtension[index].PortNumber = index;
+ AdapterExtension->PortExtension[index].IsActive = TRUE;
+ AdapterExtension->PortExtension[index].AdapterExtension =
AdapterExtension;
+ AdapterExtension->PortExtension[index].CommandList = nonCachedExtension;
+ AdapterExtension->PortExtension[index].ReceivedFIS =
(PAHCI_RECEIVED_FIS)(nonCachedExtension + sizeof(AHCI_COMMAND_HEADER) * AlignedNCS);
nonCachedExtension += nonCachedExtensionSize;
}
}
@@ -174,7 +178,7 @@
*/
BOOLEAN
AhciHwInitialize (
- __in PVOID AdapterExtension
+ __in PVOID AdapterExtension
)
{
ULONG ghc, messageCount, status;
@@ -205,18 +209,18 @@
* @name AhciInterruptHandler
* @implemented
*
- * Interrupt Handler for portExtension
- *
- * @param portExtension
+ * Interrupt Handler for PortExtension
+ *
+ * @param PortExtension
*
*/
VOID
AhciInterruptHandler (
- __in PAHCI_PORT_EXTENSION portExtension
+ __in PAHCI_PORT_EXTENSION PortExtension
)
{
DebugPrint("AhciInterruptHandler()\n");
- DebugPrint("\tPort Number: %d\n", portExtension->PortNumber);
+ DebugPrint("\tPort Number: %d\n", PortExtension->PortNumber);
}// -- AhciInterruptHandler();
@@ -234,7 +238,7 @@
*/
BOOLEAN
AhciHwInterrupt(
- __in PVOID AdapterExtension
+ __in PVOID AdapterExtension
)
{
ULONG portPending, nextPort, i;
@@ -265,8 +269,8 @@
if ((portPending & (0x1 << nextPort)) == 0)
continue;
- if ((nextPort == adapterExtension->LastInterruptPort)
- || (adapterExtension->PortExtension[nextPort].IsActive == FALSE))
+ if ((nextPort == adapterExtension->LastInterruptPort) ||
+ (adapterExtension->PortExtension[nextPort].IsActive == FALSE))
{
return FALSE;
}
@@ -296,8 +300,8 @@
*/
BOOLEAN
AhciHwStartIo (
- __in PVOID AdapterExtension,
- __in PSCSI_REQUEST_BLOCK Srb
+ __in PVOID AdapterExtension,
+ __in PSCSI_REQUEST_BLOCK Srb
)
{
UCHAR function, pathId;
@@ -404,8 +408,8 @@
*/
BOOLEAN
AhciHwResetBus (
- __in PVOID AdapterExtension,
- __in ULONG PathId
+ __in PVOID AdapterExtension,
+ __in ULONG PathId
)
{
PAHCI_ADAPTER_EXTENSION adapterExtension;
@@ -451,12 +455,12 @@
*/
ULONG
AhciHwFindAdapter (
- __in PVOID AdapterExtension,
- __in PVOID HwContext,
- __in PVOID BusInformation,
- __in PVOID ArgumentString,
- __inout PPORT_CONFIGURATION_INFORMATION ConfigInfo,
- __in PBOOLEAN Reserved3
+ __in PVOID AdapterExtension,
+ __in PVOID HwContext,
+ __in PVOID BusInformation,
+ __in PVOID ArgumentString,
+ __inout PPORT_CONFIGURATION_INFORMATION ConfigInfo,
+ __in PBOOLEAN Reserved3
)
{
ULONG ghc;
@@ -498,7 +502,9 @@
// The last PCI base address register (BAR[5], header offset 0x24) points to the AHCI
base memory, itâs called ABAR (AHCI Base Memory Register).
adapterExtension->AhciBaseAddress = pciConfigData->u.type0.BaseAddresses[5]
& (0xFFFFFFF0);
- DebugPrint("\tVendorID:%d DeviceID:%d RevisionID:%d\n",
adapterExtension->VendorID, adapterExtension->DeviceID,
adapterExtension->RevisionID);
+ DebugPrint("\tVendorID:%d DeviceID:%d RevisionID:%d\n",
adapterExtension->VendorID,
+
adapterExtension->DeviceID,
+
adapterExtension->RevisionID);
// 2.1.11
abar = NULL;
@@ -542,7 +548,8 @@
{
// reset controller to have it in known state
DebugPrint("\tAE Already set, Reset()\n");
- if (!AhciAdapterReset(adapterExtension)){
+ if (!AhciAdapterReset(adapterExtension))
+ {
DebugPrint("\tReset Failed!\n");
return SP_RETURN_ERROR;// reset failed
}
@@ -603,8 +610,8 @@
*/
ULONG
DriverEntry (
- __in PVOID DriverObject,
- __in PVOID RegistryPath
+ __in PVOID DriverObject,
+ __in PVOID RegistryPath
)
{
HW_INITIALIZATION_DATA hwInitializationData;
@@ -664,22 +671,22 @@
* If the HBA has not cleared GHC.HR to â0â within 1 second of software setting
GHC.HR to â1â, the HBA is in
* a hung or locked state.
*
- * @param adapterExtension
+ * @param AdapterExtension
*
* @return
* TRUE in case AHCI Controller RESTARTED successfully. i.e GHC.HR == 0
*/
BOOLEAN
AhciAdapterReset (
- __in PAHCI_ADAPTER_EXTENSION adapterExtension
- )
-{
- ULONG ghc, ticks;
+ __in PAHCI_ADAPTER_EXTENSION AdapterExtension
+ )
+{
+ ULONG ghc, ticks, ghcStatus;
PAHCI_MEMORY_REGISTERS abar = NULL;
DebugPrint("AhciAdapterReset()\n");
- abar = adapterExtension->ABAR_Address;
+ abar = AdapterExtension->ABAR_Address;
if (abar == NULL) // basic sanity
{
return FALSE;
@@ -687,11 +694,12 @@
// HR -- Very first bit (lowest significant)
ghc = AHCI_Global_HBA_CONTROL_HR;
- StorPortWriteRegisterUlong(adapterExtension, &abar->GHC, ghc);
+ StorPortWriteRegisterUlong(AdapterExtension, &abar->GHC, ghc);
for (ticks = 0; ticks < 50; ++ticks)
{
- if ((StorPortReadRegisterUlong(adapterExtension, &abar->GHC) &
AHCI_Global_HBA_CONTROL_HR) == 0)
+ ghcStatus = StorPortReadRegisterUlong(AdapterExtension, &abar->GHC);
+ if ((ghcStatus & AHCI_Global_HBA_CONTROL_HR) == 0)
{
break;
}
@@ -713,18 +721,21 @@
*
* Clear buffer by filling zeros
*
- * @param buffer
+ * @param Buffer
+ * @param BufferSize
*/
__inline
VOID
AhciZeroMemory (
- __out PCHAR buffer,
- __in ULONG bufferSize
+ __out PCHAR Buffer,
+ __in ULONG BufferSize
)
{
ULONG i;
- for (i = 0; i < bufferSize; i++)
- buffer[i] = 0;
+ for (i = 0; i < BufferSize; i++)
+ {
+ Buffer[i] = 0;
+ }
}// -- AhciZeroMemory();
/**
@@ -733,7 +744,7 @@
*
* Tells wheather given port is implemented or not
*
- * @param adapterExtension
+ * @param AdapterExtension
* @param PathId
*
* @return
@@ -742,8 +753,8 @@
__inline
BOOLEAN
IsPortValid (
- __in PAHCI_ADAPTER_EXTENSION adapterExtension,
- __in UCHAR pathId
+ __in PAHCI_ADAPTER_EXTENSION AdapterExtension,
+ __in UCHAR pathId
)
{
NT_ASSERT(pathId >= 0);
@@ -753,7 +764,7 @@
return FALSE;
}
- return adapterExtension->PortExtension[pathId].IsActive;
+ return AdapterExtension->PortExtension[pathId].IsActive;
}// -- IsPortValid()
/**
@@ -762,7 +773,7 @@
*
* Tells wheather given port is implemented or not
*
- * @param adapterExtension
+ * @param AdapterExtension
* @param Srb
* @param Cdb
*
@@ -774,9 +785,9 @@
*/
ULONG
DeviceInquiryRequest (
- __in PAHCI_ADAPTER_EXTENSION adapterExtension,
- __in PSCSI_REQUEST_BLOCK Srb,
- __in PCDB Cdb
+ __in PAHCI_ADAPTER_EXTENSION AdapterExtension,
+ __in PSCSI_REQUEST_BLOCK Srb,
+ __in PCDB Cdb
)
{
PVOID DataBuffer;
@@ -798,7 +809,9 @@
DataBufferLength = Srb->DataTransferLength;
if (DataBuffer == NULL)
+ {
return SRB_STATUS_INVALID_REQUEST;
+ }
AhciZeroMemory(DataBuffer, DataBufferLength);
}
Modified: branches/GSoC_2016/AHCI/drivers/storage/storahci/storahci.h
URL:
http://svn.reactos.org/svn/reactos/branches/GSoC_2016/AHCI/drivers/storage/…
==============================================================================
--- branches/GSoC_2016/AHCI/drivers/storage/storahci/storahci.h [iso-8859-1] (original)
+++ branches/GSoC_2016/AHCI/drivers/storage/storahci/storahci.h [iso-8859-1] Fri Jun 10
08:27:20 2016
@@ -250,26 +250,26 @@
BOOLEAN
AhciAdapterReset (
- __in PAHCI_ADAPTER_EXTENSION adapterExtension
+ __in PAHCI_ADAPTER_EXTENSION AdapterExtension
);
__inline
VOID
AhciZeroMemory (
- __out PCHAR buffer,
- __in ULONG bufferSize
+ __out PCHAR Buffer,
+ __in ULONG BufferSize
);
__inline
BOOLEAN
IsPortValid (
- __in PAHCI_ADAPTER_EXTENSION adapterExtension,
- __in UCHAR pathId
+ __in PAHCI_ADAPTER_EXTENSION AdapterExtension,
+ __in UCHAR pathId
);
ULONG
DeviceInquiryRequest (
- __in PAHCI_ADAPTER_EXTENSION adapterExtension,
- __in PSCSI_REQUEST_BLOCK Srb,
- __in PCDB Cdb
- );
+ __in PAHCI_ADAPTER_EXTENSION AdapterExtension,
+ __in PSCSI_REQUEST_BLOCK Srb,
+ __in PCDB Cdb
+ );
3075
days inactive
3075
days old
0 comments
1 participants
participants (1)
-
apriyadarshi＠svn.reactos.org