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...@intel.com>
> Cc: Marcin Wojtas <m...@semihalf.com>
> Cc: Zhichao Gao <zhichao....@intel.com>
> Cc: Liming Gao <liming....@intel.com>
>
> Signed-off-by: Mateusz Albecki <mateusz.albe...@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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
