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:[email protected]]
Sent: Monday, September 01, 2014 06:50
To: [email protected]
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 
<[email protected]<mailto:[email protected]>> 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:[email protected]<mailto:[email protected]>]
Sent: Friday, August 22, 2014 17:28
To: [email protected]<mailto:[email protected]>
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 
<[email protected]<mailto:[email protected]>> wrote:
Good to me.

Reviewed-by: Feng Tian <[email protected]<mailto:[email protected]>>

-----Original Message-----
From: [email protected]<mailto:[email protected]> 
[mailto:[email protected]]
Sent: Thursday, August 21, 2014 17:56
To: [email protected]<mailto:[email protected]>
Cc: [email protected]<mailto:[email protected]>
Subject: [edk2] [PATCH v2 1/6] MdeModulePkg: Check D2H register status in 
AhciPioTransfer

From: Reza Jelveh <[email protected]<mailto:[email protected]>>

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 <[email protected]<mailto:[email protected]>>
---
 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
[email protected]<mailto:[email protected]>
https://lists.sourceforge.net/lists/listinfo/edk2-devel

------------------------------------------------------------------------------
Slashdot TV.
Video for Nerds.  Stuff that matters.
http://tv.slashdot.org/
_______________________________________________
edk2-devel mailing list
[email protected]<mailto:[email protected]>
https://lists.sourceforge.net/lists/listinfo/edk2-devel

------------------------------------------------------------------------------
Slashdot TV.
Video for Nerds.  Stuff that matters.
http://tv.slashdot.org/
_______________________________________________
edk2-devel mailing list
[email protected]<mailto:[email protected]>
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to