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

Attachment: 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

Reply via email to