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> 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]
> *Sent:* Friday, August 22, 2014 17:28
> *To:* 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> wrote:
>
> Good to me.
>
> Reviewed-by: Feng Tian <feng.t...@intel.com>
>
> -----Original Message-----
> From: reza.jel...@tuhh.de [mailto:reza.jel...@tuhh.de]
> Sent: Thursday, August 21, 2014 17:56
> To: edk2-devel@lists.sourceforge.net
> Cc: ag...@suse.de
> Subject: [edk2] [PATCH v2 1/6] MdeModulePkg: Check D2H register status in
> AhciPioTransfer
>
> From: Reza Jelveh <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>
> ---
> 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
> 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
> 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
> 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
https://lists.sourceforge.net/lists/listinfo/edk2-devel
