Hi, Sava From my side, I am ok with integrating these two cases into one.
Please help review and verify if the attached patch is ok for Qemu and Marvel’s AHCI controller. Thanks Feng From: A. Sava [mailto:asava....@gmail.com] Sent: Monday, September 01, 2014 06:50 To: edk2-devel@lists.sourceforge.net Subject: Re: [edk2] [PATCH v2 1/6] MdeModulePkg: Check D2H register status in AhciPioTransfer Hi Feng, Sorry for the late reply. Yes, but in this case, I wonder why at all separate the two cases (D2H and PIO Setup)? We don't automatically treat D2H as error anymore, so isn't it enough to just check PxTFD for both cases together, instead of handling them separately? Is there any benefit to examine directly the D2H FIS Status Register when D2H is received, instead of combining the two cases? Thanks, A. Sava On Mon, Aug 25, 2014 at 3:44 AM, Tian, Feng <feng.t...@intel.com<mailto:feng.t...@intel.com>> wrote: My understanding is when D2H or PIO Setup FIS is received, the PxTFD gets updated with the content of these two FISes. So checking D2H FIS Status register is enough, am I right? Thanks Feng From: A. Sava [mailto:asava....@gmail.com<mailto:asava....@gmail.com>] Sent: Friday, August 22, 2014 17:28 To: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net> Subject: Re: [edk2] [PATCH v2 1/6] MdeModulePkg: Check D2H register status in AhciPioTransfer Regarding this, don't you think there's also need to check PxTFD for error in the case D2H is received? On Friday, August 22, 2014, Tian, Feng <feng.t...@intel.com<mailto:feng.t...@intel.com>> wrote: Good to me. Reviewed-by: Feng Tian <feng.t...@intel.com<mailto:feng.t...@intel.com>> -----Original Message----- From: reza.jel...@tuhh.de<mailto:reza.jel...@tuhh.de> [mailto:reza.jel...@tuhh.de] Sent: Thursday, August 21, 2014 17:56 To: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net> Cc: ag...@suse.de<mailto:ag...@suse.de> Subject: [edk2] [PATCH v2 1/6] MdeModulePkg: Check D2H register status in AhciPioTransfer From: Reza Jelveh <reza.jel...@tuhh.de<mailto:reza.jel...@tuhh.de>> Some AHCI controllers such as the Marvel 9230 controllers do not send PIO Setup FIS when the PIO data-in command is completed. Instead they just send a D2H FIS. To accomodate for this possibility the status code of the D2H FIS is checked. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Reza Jelveh <reza.jel...@tuhh.de<mailto:reza.jel...@tuhh.de>> --- MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c | 14 ++++++++++++-- MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h | 3 +++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c index 7fc7a28..487f516 100644 --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c @@ -703,6 +703,7 @@ AhciPioTransfer ( EFI_AHCI_COMMAND_LIST CmdList; UINT32 PortTfd; UINT32 PrdCount; + UINT8 D2HStatus; BOOLEAN InfiniteWait; if (Timeout == 0) { @@ -804,8 +805,17 @@ AhciPioTransfer ( Offset = FisBaseAddr + EFI_AHCI_D2H_FIS_OFFSET; Status = AhciCheckMemSet (Offset, EFI_AHCI_FIS_TYPE_MASK, EFI_AHCI_FIS_REGISTER_D2H, NULL); if (!EFI_ERROR (Status)) { - Status = EFI_DEVICE_ERROR; - break; + D2HStatus = *(volatile UINT8 *) (Offset + + EFI_AHCI_D2H_FIS_STATUS_OFFSET); + + if ((D2HStatus & EFI_AHCI_D2H_FIS_ERR) != 0) { + Status = EFI_DEVICE_ERROR; + break; + } + + PrdCount = *(volatile UINT32 *) (&(AhciRegisters->AhciCmdList[0].AhciCmdPrdbc)); + if (PrdCount == DataCount) { + break; + } } // diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h index 485b647..9fe1fb3 100644 --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h @@ -78,12 +78,15 @@ typedef union { #define EFI_AHCI_FIS_SET_DEVICE_LENGTH 8 #define EFI_AHCI_D2H_FIS_OFFSET 0x40 +#define EFI_AHCI_D2H_FIS_STATUS_OFFSET 0x02 +#define EFI_AHCI_D2H_FIS_ERR BIT0 #define EFI_AHCI_DMA_FIS_OFFSET 0x00 #define EFI_AHCI_PIO_FIS_OFFSET 0x20 #define EFI_AHCI_SDB_FIS_OFFSET 0x58 #define EFI_AHCI_FIS_TYPE_MASK 0xFF #define EFI_AHCI_U_FIS_OFFSET 0x60 + // // Port register // -- 1.9.2 ------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that matters. http://tv.slashdot.org/ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net> https://lists.sourceforge.net/lists/listinfo/edk2-devel ------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that matters. http://tv.slashdot.org/ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net> https://lists.sourceforge.net/lists/listinfo/edk2-devel ------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that matters. http://tv.slashdot.org/ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net> https://lists.sourceforge.net/lists/listinfo/edk2-devel
AhciMode.c.patch
Description: AhciMode.c.patch
------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that matters. http://tv.slashdot.org/
_______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel