From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=66.187.233.73; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id BD70C210DA179 for ; Wed, 8 Aug 2018 11:11:52 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 0555B7A7E8; Wed, 8 Aug 2018 18:11:52 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-208.rdu2.redhat.com [10.10.120.208]) by smtp.corp.redhat.com (Postfix) with ESMTP id 14D481055460; Wed, 8 Aug 2018 18:11:50 +0000 (UTC) To: shenglei , edk2-devel@lists.01.org Cc: Eric Dong , Star Zeng References: <20180808084712.34696-1-shenglei.zhang@intel.com> <20180808084712.34696-4-shenglei.zhang@intel.com> From: Laszlo Ersek Message-ID: Date: Wed, 8 Aug 2018 20:11:50 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <20180808084712.34696-4-shenglei.zhang@intel.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Wed, 08 Aug 2018 18:11:52 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Wed, 08 Aug 2018 18:11:52 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lersek@redhat.com' RCPT:'' Subject: Re: [PATCH 03/26] MdeModulePkg AtaAtapiPassThru: Remove redundant functions X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Aug 2018 18:11:53 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 08/08/18 10:46, shenglei wrote: > The functions that are never called have been removed. > They are AhciCheckDeviceStatus and AhciPortReset. There's more. In total: AhciCheckDeviceStatus, AhciPortReset, DRDYReady, DRDYReady2, WaitForBSYClear2, AtaSoftReset. With the commit message update: Reviewed-by: Laszlo Ersek Thanks Laszlo > https://bugzilla.tianocore.org/show_bug.cgi?id=1062 > > Cc: Star Zeng > Cc: Eric Dong > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: shenglei > --- > .../Bus/Ata/AtaAtapiPassThru/AhciMode.c | 104 ------- > .../Bus/Ata/AtaAtapiPassThru/IdeMode.c | 257 ------------------ > 2 files changed, 361 deletions(-) > > diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c > index 79ae11bd20..d7a8b29666 100644 > --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c > +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c > @@ -294,41 +294,6 @@ AhciCheckMemSet ( > } > } > > -/** > - Check if the device is still on port. It also checks if the AHCI controller > - supports the address and data count will be transferred. > - > - @param PciIo The PCI IO protocol instance. > - @param Port The number of port. > - > - @retval EFI_SUCCESS The device is attached to port and the transfer data is > - supported by AHCI controller. > - @retval EFI_UNSUPPORTED The transfer address and count is not supported by AHCI > - controller. > - @retval EFI_NOT_READY The physical communication between AHCI controller and device > - is not ready. > - > -**/ > -EFI_STATUS > -EFIAPI > -AhciCheckDeviceStatus ( > - IN EFI_PCI_IO_PROTOCOL *PciIo, > - IN UINT8 Port > - ) > -{ > - UINT32 Data; > - UINT32 Offset; > - > - Offset = EFI_AHCI_PORT_START + Port * EFI_AHCI_PORT_REG_WIDTH + EFI_AHCI_PORT_SSTS; > - > - Data = AhciReadReg (PciIo, Offset) & EFI_AHCI_PORT_SSTS_DET_MASK; > - > - if (Data == EFI_AHCI_PORT_SSTS_DET_PCE) { > - return EFI_SUCCESS; > - } > - > - return EFI_NOT_READY; > -} > > /** > > @@ -1361,75 +1326,6 @@ AhciStartCommand ( > return EFI_SUCCESS; > } > > -/** > - Do AHCI port reset. > - > - @param PciIo The PCI IO protocol instance. > - @param Port The number of port. > - @param Timeout The timeout value of reset, uses 100ns as a unit. > - > - @retval EFI_DEVICE_ERROR The port reset unsuccessfully > - @retval EFI_TIMEOUT The reset operation is time out. > - @retval EFI_SUCCESS The port reset successfully. > - > -**/ > -EFI_STATUS > -EFIAPI > -AhciPortReset ( > - IN EFI_PCI_IO_PROTOCOL *PciIo, > - IN UINT8 Port, > - IN UINT64 Timeout > - ) > -{ > - EFI_STATUS Status; > - UINT32 Offset; > - > - AhciClearPortStatus (PciIo, Port); > - > - AhciStopCommand (PciIo, Port, Timeout); > - > - AhciDisableFisReceive (PciIo, Port, Timeout); > - > - AhciEnableFisReceive (PciIo, Port, Timeout); > - > - Offset = EFI_AHCI_PORT_START + Port * EFI_AHCI_PORT_REG_WIDTH + EFI_AHCI_PORT_SCTL; > - > - AhciOrReg (PciIo, Offset, EFI_AHCI_PORT_SCTL_DET_INIT); > - > - // > - // wait 5 millisecond before de-assert DET > - // > - MicroSecondDelay (5000); > - > - AhciAndReg (PciIo, Offset, (UINT32)EFI_AHCI_PORT_SCTL_MASK); > - > - // > - // wait 5 millisecond before de-assert DET > - // > - MicroSecondDelay (5000); > - > - // > - // Wait for communication to be re-established > - // > - Offset = EFI_AHCI_PORT_START + Port * EFI_AHCI_PORT_REG_WIDTH + EFI_AHCI_PORT_SSTS; > - Status = AhciWaitMmioSet ( > - PciIo, > - Offset, > - EFI_AHCI_PORT_SSTS_DET_MASK, > - EFI_AHCI_PORT_SSTS_DET_PCE, > - Timeout > - ); > - > - if (EFI_ERROR (Status)) { > - DEBUG ((EFI_D_ERROR, "Port %d COMRESET failed: %r\n", Port, Status)); > - return Status; > - } > - > - Offset = EFI_AHCI_PORT_START + Port * EFI_AHCI_PORT_REG_WIDTH + EFI_AHCI_PORT_SERR; > - AhciOrReg (PciIo, Offset, EFI_AHCI_PORT_ERR_CLEAR); > - > - return EFI_SUCCESS; > -} > > /** > Do AHCI HBA reset. > diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c > index 6478f7be07..79142c330d 100644 > --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c > +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c > @@ -627,146 +627,8 @@ DRQReady2 ( > return EFI_TIMEOUT; > } > > -/** > - This function is used to poll for the DRDY bit set in the Status Register. DRDY > - bit is set when the device is ready to accept command. Most ATA commands must be > - sent after DRDY set except the ATAPI Packet Command. > - > - @param PciIo A pointer to EFI_PCI_IO_PROTOCOL data structure. > - @param IdeRegisters A pointer to EFI_IDE_REGISTERS data structure. > - @param Timeout The time to complete the command, uses 100ns as a unit. > - > - @retval EFI_SUCCESS DRDY bit set within the time out. > - @retval EFI_TIMEOUT DRDY bit not set within the time out. > - > - @note Read Status Register will clear interrupt status. > -**/ > -EFI_STATUS > -EFIAPI > -DRDYReady ( > - IN EFI_PCI_IO_PROTOCOL *PciIo, > - IN EFI_IDE_REGISTERS *IdeRegisters, > - IN UINT64 Timeout > - ) > -{ > - UINT64 Delay; > - UINT8 StatusRegister; > - UINT8 ErrorRegister; > - BOOLEAN InfiniteWait; > - > - ASSERT (PciIo != NULL); > - ASSERT (IdeRegisters != NULL); > - > - if (Timeout == 0) { > - InfiniteWait = TRUE; > - } else { > - InfiniteWait = FALSE; > - } > - > - Delay = DivU64x32(Timeout, 1000) + 1; > - do { > - StatusRegister = IdeReadPortB (PciIo, IdeRegisters->CmdOrStatus); > - // > - // Wait for BSY == 0, then judge if DRDY is set or ERR is set > - // > - if ((StatusRegister & ATA_STSREG_BSY) == 0) { > - if ((StatusRegister & ATA_STSREG_ERR) == ATA_STSREG_ERR) { > - ErrorRegister = IdeReadPortB (PciIo, IdeRegisters->ErrOrFeature); > - > - if ((ErrorRegister & ATA_ERRREG_ABRT) == ATA_ERRREG_ABRT) { > - return EFI_ABORTED; > - } > - return EFI_DEVICE_ERROR; > - } > - > - if ((StatusRegister & ATA_STSREG_DRDY) == ATA_STSREG_DRDY) { > - return EFI_SUCCESS; > - } else { > - return EFI_DEVICE_ERROR; > - } > - } > - > - // > - // Stall for 100 microseconds. > - // > - MicroSecondDelay (100); > - > - Delay--; > - } while (InfiniteWait || (Delay > 0)); > - > - return EFI_TIMEOUT; > -} > - > -/** > - This function is used to poll for the DRDY bit set in the Alternate Status Register. > - DRDY bit is set when the device is ready to accept command. Most ATA commands must > - be sent after DRDY set except the ATAPI Packet Command. > - > - @param PciIo A pointer to EFI_PCI_IO_PROTOCOL data structure. > - @param IdeRegisters A pointer to EFI_IDE_REGISTERS data structure. > - @param Timeout The time to complete the command, uses 100ns as a unit. > - > - @retval EFI_SUCCESS DRDY bit set within the time out. > - @retval EFI_TIMEOUT DRDY bit not set within the time out. > - > - @note Read Alternate Status Register will clear interrupt status. > - > -**/ > -EFI_STATUS > -EFIAPI > -DRDYReady2 ( > - IN EFI_PCI_IO_PROTOCOL *PciIo, > - IN EFI_IDE_REGISTERS *IdeRegisters, > - IN UINT64 Timeout > - ) > -{ > - UINT64 Delay; > - UINT8 AltRegister; > - UINT8 ErrorRegister; > - BOOLEAN InfiniteWait; > - > - ASSERT (PciIo != NULL); > - ASSERT (IdeRegisters != NULL); > - > - if (Timeout == 0) { > - InfiniteWait = TRUE; > - } else { > - InfiniteWait = FALSE; > - } > - > - Delay = DivU64x32(Timeout, 1000) + 1; > - do { > - AltRegister = IdeReadPortB (PciIo, IdeRegisters->AltOrDev); > - // > - // Wait for BSY == 0, then judge if DRDY is set or ERR is set > - // > - if ((AltRegister & ATA_STSREG_BSY) == 0) { > - if ((AltRegister & ATA_STSREG_ERR) == ATA_STSREG_ERR) { > - ErrorRegister = IdeReadPortB (PciIo, IdeRegisters->ErrOrFeature); > - > - if ((ErrorRegister & ATA_ERRREG_ABRT) == ATA_ERRREG_ABRT) { > - return EFI_ABORTED; > - } > - return EFI_DEVICE_ERROR; > - } > - > - if ((AltRegister & ATA_STSREG_DRDY) == ATA_STSREG_DRDY) { > - return EFI_SUCCESS; > - } else { > - return EFI_DEVICE_ERROR; > - } > - } > - > - // > - // Stall for 100 microseconds. > - // > - MicroSecondDelay (100); > > - Delay--; > - } while (InfiniteWait || (Delay > 0)); > > - return EFI_TIMEOUT; > -} > > /** > This function is used to poll for the BSY bit clear in the Status Register. BSY > @@ -822,59 +684,6 @@ WaitForBSYClear ( > return EFI_TIMEOUT; > } > > -/** > - This function is used to poll for the BSY bit clear in the Status Register. BSY > - is clear when the device is not busy. Every command must be sent after device is not busy. > - > - @param PciIo A pointer to ATA_ATAPI_PASS_THRU_INSTANCE data structure. > - @param IdeRegisters A pointer to EFI_IDE_REGISTERS data structure. > - @param Timeout The time to complete the command, uses 100ns as a unit. > - > - @retval EFI_SUCCESS BSY bit clear within the time out. > - @retval EFI_TIMEOUT BSY bit not clear within the time out. > - > - @note Read Status Register will clear interrupt status. > -**/ > -EFI_STATUS > -EFIAPI > -WaitForBSYClear2 ( > - IN EFI_PCI_IO_PROTOCOL *PciIo, > - IN EFI_IDE_REGISTERS *IdeRegisters, > - IN UINT64 Timeout > - ) > -{ > - UINT64 Delay; > - UINT8 AltStatusRegister; > - BOOLEAN InfiniteWait; > - > - ASSERT (PciIo != NULL); > - ASSERT (IdeRegisters != NULL); > - > - if (Timeout == 0) { > - InfiniteWait = TRUE; > - } else { > - InfiniteWait = FALSE; > - } > - > - Delay = DivU64x32(Timeout, 1000) + 1; > - do { > - AltStatusRegister = IdeReadPortB (PciIo, IdeRegisters->AltOrDev); > - > - if ((AltStatusRegister & ATA_STSREG_BSY) == 0x00) { > - return EFI_SUCCESS; > - } > - > - // > - // Stall for 100 microseconds. > - // > - MicroSecondDelay (100); > - > - Delay--; > - > - } while (InfiniteWait || (Delay > 0)); > - > - return EFI_TIMEOUT; > -} > > /** > Get IDE i/o port registers' base addresses by mode. > @@ -1017,72 +826,6 @@ GetIdeRegisterIoAddr ( > return EFI_SUCCESS; > } > > -/** > - This function is used to implement the Soft Reset on the specified device. But, > - the ATA Soft Reset mechanism is so strong a reset method that it will force > - resetting on both devices connected to the same cable. > - > - It is called by IdeBlkIoReset(), a interface function of Block > - I/O protocol. > - > - This function can also be used by the ATAPI device to perform reset when > - ATAPI Reset command is failed. > - > - @param PciIo A pointer to ATA_ATAPI_PASS_THRU_INSTANCE data structure. > - @param IdeRegisters A pointer to EFI_IDE_REGISTERS data structure. > - @param Timeout The time to complete the command, uses 100ns as a unit. > - > - @retval EFI_SUCCESS Soft reset completes successfully. > - @retval EFI_DEVICE_ERROR Any step during the reset process is failed. > - > - @note The registers initial values after ATA soft reset are different > - to the ATA device and ATAPI device. > -**/ > -EFI_STATUS > -EFIAPI > -AtaSoftReset ( > - IN EFI_PCI_IO_PROTOCOL *PciIo, > - IN EFI_IDE_REGISTERS *IdeRegisters, > - IN UINT64 Timeout > - ) > -{ > - UINT8 DeviceControl; > - > - DeviceControl = 0; > - // > - // disable Interrupt and set SRST bit to initiate soft reset > - // > - DeviceControl = ATA_CTLREG_SRST | ATA_CTLREG_IEN_L; > - > - IdeWritePortB (PciIo, IdeRegisters->AltOrDev, DeviceControl); > - > - // > - // SRST should assert for at least 5 us, we use 10 us for > - // better compatibility > - // > - MicroSecondDelay (10); > - > - // > - // Enable interrupt to support UDMA, and clear SRST bit > - // > - DeviceControl = 0; > - IdeWritePortB (PciIo, IdeRegisters->AltOrDev, DeviceControl); > - > - // > - // Wait for at least 10 ms to check BSY status, we use 10 ms > - // for better compatibility > - // > - MicroSecondDelay (10000); > - > - // > - // slave device needs at most 31ms to clear BSY > - // > - if (WaitForBSYClear (PciIo, IdeRegisters, Timeout) == EFI_TIMEOUT) { > - return EFI_DEVICE_ERROR; > - } > - > - return EFI_SUCCESS; > -} > > /** > Send ATA Ext command into device with NON_DATA protocol. >