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/s... ============================================================================== --- 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/s... ============================================================================== --- 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 + );
-
apriyadarshi＠svn.reactos.org