To allow for easier debug of failing commands we
have added a capability to print TRB and command
packet when we start execution of the TRB(on
DEBUG_VERBOSE level) and when the TRB failed to
execute correctly(on DEBUG_ERROR level). Additionally
we will also print error interrupt status and interrupt
status register on failed SD command.

Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Marcin Wojtas <mw@semihalf.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>

Signed-off-by: Mateusz Albecki <mateusz.albecki@intel.com>
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 87 ++++++++++++++++++++++++
 1 file changed, 87 insertions(+)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index b05c818462..959645bf26 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -1643,6 +1643,82 @@ BuildAdmaDescTable (
   return EFI_SUCCESS;
 }
 
+/**
+  Prints the contents of the command packet to the debug port.
+
+  @param[in] DebugLevel  Debug level at which the packet should be printed.
+  @param[in] Packet      Pointer to packet to print.
+**/
+VOID
+SdMmcPrintPacket (
+  IN UINT32                               DebugLevel,
+  IN EFI_SD_MMC_PASS_THRU_COMMAND_PACKET  *Packet
+  )
+{
+  if (Packet == NULL) {
+    return;
+  }
+
+  DEBUG ((DebugLevel, "Printing EFI_SD_MMC_PASS_THRU_COMMAND_PACKET\n"));
+  if (Packet->SdMmcCmdBlk != NULL) {
+    DEBUG ((DebugLevel, "Command index: %d, argument: %X\n", Packet->SdMmcCmdBlk->CommandIndex, Packet->SdMmcCmdBlk->CommandArgument));
+    DEBUG ((DebugLevel, "Command type: %d, response type: %d\n", Packet->SdMmcCmdBlk->CommandType, Packet->SdMmcCmdBlk->ResponseType));
+  }
+  if (Packet->SdMmcStatusBlk != NULL) {
+    DEBUG ((DebugLevel, "Response 0: %X, 1: %X, 2: %X, 3: %X\n",
+                           Packet->SdMmcStatusBlk->Resp0,
+                           Packet->SdMmcStatusBlk->Resp1,
+                           Packet->SdMmcStatusBlk->Resp2,
+                           Packet->SdMmcStatusBlk->Resp3
+                           ));
+  }
+  DEBUG ((DebugLevel, "Timeout: %d\n", Packet->Timeout));
+  DEBUG ((DebugLevel, "InDataBuffer: %X\n", Packet->InDataBuffer));
+  DEBUG ((DebugLevel, "OutDataBuffer: %X\n", Packet->OutDataBuffer));
+  DEBUG ((DebugLevel, "InTransferLength: %d\n", Packet->InTransferLength));
+  DEBUG ((DebugLevel, "OutTransferLength: %d\n", Packet->OutTransferLength));
+  DEBUG ((DebugLevel, "TransactionStatus: %r\n", Packet->TransactionStatus));
+}
+
+/**
+  Prints the contents of the TRB to the debug port.
+
+  @param[in] DebugLevel  Debug level at which the TRB should be printed.
+  @param[in] Trb         Pointer to the TRB structure.
+**/
+VOID
+SdMmcPrintTrb (
+  IN UINT32         DebugLevel,
+  IN SD_MMC_HC_TRB  *Trb
+  )
+{
+  if (Trb == NULL) {
+    return;
+  }
+
+  DEBUG ((DebugLevel, "Printing SD_MMC_HC_TRB\n"));
+  DEBUG ((DebugLevel, "Slot: %d\n", Trb->Slot));
+  DEBUG ((DebugLevel, "BlockSize: %d\n", Trb->BlockSize));
+  DEBUG ((DebugLevel, "Data: %X\n", Trb->Data));
+  DEBUG ((DebugLevel, "DataLen: %d\n", Trb->DataLen));
+  DEBUG ((DebugLevel, "Read: %d\n", Trb->Read));
+  DEBUG ((DebugLevel, "DataPhy: %X\n", Trb->DataPhy));
+  DEBUG ((DebugLevel, "DataMap: %X\n", Trb->DataMap));
+  DEBUG ((DebugLevel, "Mode: %d\n", Trb->Mode));
+  DEBUG ((DebugLevel, "AdmaLengthMode: %d\n", Trb->AdmaLengthMode));
+  DEBUG ((DebugLevel, "Event: %d\n", Trb->Event));
+  DEBUG ((DebugLevel, "Started: %d\n", Trb->Started));
+  DEBUG ((DebugLevel, "Timeout: %d\n", Trb->Timeout));
+  DEBUG ((DebugLevel, "Retries: %d\n", Trb->Retries));
+  DEBUG ((DebugLevel, "Adma32Desc: %X\n", Trb->Adma32Desc));
+  DEBUG ((DebugLevel, "Adma64V3Desc: %X\n", Trb->Adma64V3Desc));
+  DEBUG ((DebugLevel, "Adma64V4Desc: %X\n", Trb->Adma64V4Desc));
+  DEBUG ((DebugLevel, "AdmaMap: %X\n", Trb->AdmaMap));
+  DEBUG ((DebugLevel, "AdmaPages: %X\n", Trb->AdmaPages));
+
+  SdMmcPrintPacket (DebugLevel, Trb->Packet);
+}
+
 /**
   Create a new TRB for the SD/MMC cmd request.
 
@@ -1963,6 +2039,9 @@ SdMmcExecTrb (
   UINT64                              AdmaAddr;
   BOOLEAN                             AddressingMode64;
 
+  DEBUG ((DEBUG_VERBOSE, "Starting TRB execution\n"));
+  SdMmcPrintTrb (DEBUG_VERBOSE, Trb);
+
   AddressingMode64 = FALSE;
 
   Packet = Trb->Packet;
@@ -2235,6 +2314,10 @@ SdMmcCheckAndRecoverErrors (
     return Status;
   }
 
+  DEBUG ((DEBUG_ERROR, "Error reported by SDHCI\n"));
+  DEBUG ((DEBUG_ERROR, "Interrupt status = %X\n", IntStatus));
+  DEBUG ((DEBUG_ERROR, "Error interrupt status = %X\n", ErrIntStatus));
+
   //
   // If the data timeout error is reported
   // but data transfer is signaled as completed we
@@ -2438,6 +2521,10 @@ Done:
 
   if (Status != EFI_NOT_READY) {
     SdMmcHcLedOnOff (Private->PciIo, Trb->Slot, FALSE);
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_ERROR, "TRB failed with %r\n", Status));
+      SdMmcPrintTrb (DEBUG_ERROR, Trb);
+    }
   }
 
   return Status;
-- 
2.14.1.windows.1

--------------------------------------------------------------------

Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53668): https://edk2.groups.io/g/devel/message/53668
Mute This Topic: https://groups.io/mt/70947182/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Hello Mateusz,

Try to provide some feedbacks before I can test the patch.

Some inline comments below:


> -----Original Message-----
> From: Albecki, Mateusz
> Sent: Monday, February 03, 2020 10:19 PM
> To: devel@edk2.groups.io
> Cc: Albecki, Mateusz; Wu, Hao A; Marcin Wojtas; Gao, Zhichao; Gao, Liming
> Subject: [PATCH 1/4] MdeModulePkg/SdMmcPciHcDxe: Enhance driver
> traces
> 
> To allow for easier debug of failing commands we
> have added a capability to print TRB and command
> packet when we start execution of the TRB(on
> DEBUG_VERBOSE level) and when the TRB failed to
> execute correctly(on DEBUG_ERROR level). Additionally
> we will also print error interrupt status and interrupt
> status register on failed SD command.
> 
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Marcin Wojtas <mw@semihalf.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> 
> Signed-off-by: Mateusz Albecki <mateusz.albecki@intel.com>
> ---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 87
> ++++++++++++++++++++++++
>  1 file changed, 87 insertions(+)
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index b05c818462..959645bf26 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -1643,6 +1643,82 @@ BuildAdmaDescTable (
>    return EFI_SUCCESS;
>  }
> 
> +/**
> +  Prints the contents of the command packet to the debug port.
> +
> +  @param[in] DebugLevel  Debug level at which the packet should be
> printed.
> +  @param[in] Packet      Pointer to packet to print.
> +**/
> +VOID
> +SdMmcPrintPacket (
> +  IN UINT32                               DebugLevel,
> +  IN EFI_SD_MMC_PASS_THRU_COMMAND_PACKET  *Packet
> +  )
> +{
> +  if (Packet == NULL) {
> +    return;
> +  }
> +
> +  DEBUG ((DebugLevel, "Printing
> EFI_SD_MMC_PASS_THRU_COMMAND_PACKET\n"));
> +  if (Packet->SdMmcCmdBlk != NULL) {
> +    DEBUG ((DebugLevel, "Command index: %d, argument: %X\n", Packet-
> >SdMmcCmdBlk->CommandIndex, Packet->SdMmcCmdBlk-
> >CommandArgument));
> +    DEBUG ((DebugLevel, "Command type: %d, response type: %d\n",
> Packet->SdMmcCmdBlk->CommandType, Packet->SdMmcCmdBlk-
> >ResponseType));
> +  }
> +  if (Packet->SdMmcStatusBlk != NULL) {
> +    DEBUG ((DebugLevel, "Response 0: %X, 1: %X, 2: %X, 3: %X\n",
> +                           Packet->SdMmcStatusBlk->Resp0,
> +                           Packet->SdMmcStatusBlk->Resp1,
> +                           Packet->SdMmcStatusBlk->Resp2,
> +                           Packet->SdMmcStatusBlk->Resp3
> +                           ));
> +  }
> +  DEBUG ((DebugLevel, "Timeout: %d\n", Packet->Timeout));


Please update to use %ld for UINT64 here.


> +  DEBUG ((DebugLevel, "InDataBuffer: %X\n", Packet->InDataBuffer));
> +  DEBUG ((DebugLevel, "OutDataBuffer: %X\n", Packet->OutDataBuffer));


Please update to use %p for InDataBuffer/OutDataBuffer (VOID *).


> +  DEBUG ((DebugLevel, "InTransferLength: %d\n", Packet-
> >InTransferLength));
> +  DEBUG ((DebugLevel, "OutTransferLength: %d\n", Packet-
> >OutTransferLength));
> +  DEBUG ((DebugLevel, "TransactionStatus: %r\n", Packet-
> >TransactionStatus));
> +}
> +
> +/**
> +  Prints the contents of the TRB to the debug port.
> +
> +  @param[in] DebugLevel  Debug level at which the TRB should be printed.
> +  @param[in] Trb         Pointer to the TRB structure.
> +**/
> +VOID
> +SdMmcPrintTrb (
> +  IN UINT32         DebugLevel,
> +  IN SD_MMC_HC_TRB  *Trb
> +  )
> +{
> +  if (Trb == NULL) {
> +    return;
> +  }
> +
> +  DEBUG ((DebugLevel, "Printing SD_MMC_HC_TRB\n"));
> +  DEBUG ((DebugLevel, "Slot: %d\n", Trb->Slot));
> +  DEBUG ((DebugLevel, "BlockSize: %d\n", Trb->BlockSize));
> +  DEBUG ((DebugLevel, "Data: %X\n", Trb->Data));


Please update to use %p for Trb->Data (VOID *).


> +  DEBUG ((DebugLevel, "DataLen: %d\n", Trb->DataLen));
> +  DEBUG ((DebugLevel, "Read: %d\n", Trb->Read));
> +  DEBUG ((DebugLevel, "DataPhy: %X\n", Trb->DataPhy));


Please update to use %lX for Trb->DataPhy (EFI_PHYSICAL_ADDRESS).


> +  DEBUG ((DebugLevel, "DataMap: %X\n", Trb->DataMap));


Please update to use %p for Trb->DataMap (VOID *).


> +  DEBUG ((DebugLevel, "Mode: %d\n", Trb->Mode));
> +  DEBUG ((DebugLevel, "AdmaLengthMode: %d\n", Trb-
> >AdmaLengthMode));
> +  DEBUG ((DebugLevel, "Event: %d\n", Trb->Event));


Please update to use %p for Trb->Event (EFI_EVENT).


> +  DEBUG ((DebugLevel, "Started: %d\n", Trb->Started));
> +  DEBUG ((DebugLevel, "Timeout: %d\n", Trb->Timeout));


Please update to use %ld for Trb->Timeout (UINT64).


> +  DEBUG ((DebugLevel, "Retries: %d\n", Trb->Retries));
> +  DEBUG ((DebugLevel, "Adma32Desc: %X\n", Trb->Adma32Desc));
> +  DEBUG ((DebugLevel, "Adma64V3Desc: %X\n", Trb->Adma64V3Desc));
> +  DEBUG ((DebugLevel, "Adma64V4Desc: %X\n", Trb->Adma64V4Desc));
> +  DEBUG ((DebugLevel, "AdmaMap: %X\n", Trb->AdmaMap));


Please update to use %p for:
Trb->Adma32Desc
Trb->Adma64V3Desc
Trb->Adma64V4Desc
Trb->AdmaMap
(pointers)

Best Regards,
Hao Wu

> +  DEBUG ((DebugLevel, "AdmaPages: %X\n", Trb->AdmaPages));
> +
> +  SdMmcPrintPacket (DebugLevel, Trb->Packet);
> +}
> +
>  /**
>    Create a new TRB for the SD/MMC cmd request.
> 
> @@ -1963,6 +2039,9 @@ SdMmcExecTrb (
>    UINT64                              AdmaAddr;
>    BOOLEAN                             AddressingMode64;
> 
> +  DEBUG ((DEBUG_VERBOSE, "Starting TRB execution\n"));
> +  SdMmcPrintTrb (DEBUG_VERBOSE, Trb);
> +
>    AddressingMode64 = FALSE;
> 
>    Packet = Trb->Packet;
> @@ -2235,6 +2314,10 @@ SdMmcCheckAndRecoverErrors (
>      return Status;
>    }
> 
> +  DEBUG ((DEBUG_ERROR, "Error reported by SDHCI\n"));
> +  DEBUG ((DEBUG_ERROR, "Interrupt status = %X\n", IntStatus));
> +  DEBUG ((DEBUG_ERROR, "Error interrupt status = %X\n", ErrIntStatus));
> +
>    //
>    // If the data timeout error is reported
>    // but data transfer is signaled as completed we
> @@ -2438,6 +2521,10 @@ Done:
> 
>    if (Status != EFI_NOT_READY) {
>      SdMmcHcLedOnOff (Private->PciIo, Trb->Slot, FALSE);
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_ERROR, "TRB failed with %r\n", Status));
> +      SdMmcPrintTrb (DEBUG_ERROR, Trb);
> +    }
>    }
> 
>    return Status;
> --
> 2.14.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53810): https://edk2.groups.io/g/devel/message/53810
Mute This Topic: https://groups.io/mt/70947182/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-