From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga06.intel.com (mga06.intel.com []) by mx.groups.io with SMTP id smtpd.web09.1801.1580872579718317387 for ; Tue, 04 Feb 2020 19:16:22 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=fail (domain: intel.com, ip: , mailfrom: hao.a.wu@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Feb 2020 19:16:21 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,404,1574150400"; d="scan'208";a="231586037" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga003.jf.intel.com with ESMTP; 04 Feb 2020 19:16:21 -0800 Received: from FMSMSX110.amr.corp.intel.com (10.18.116.10) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 4 Feb 2020 19:16:21 -0800 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by fmsmsx110.amr.corp.intel.com (10.18.116.10) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 4 Feb 2020 19:16:20 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.5]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.222]) with mapi id 14.03.0439.000; Wed, 5 Feb 2020 11:16:18 +0800 From: "Wu, Hao A" To: "devel@edk2.groups.io" , "Albecki, Mateusz" CC: Marcin Wojtas , "Gao, Zhichao" , "Gao, Liming" Subject: Re: [edk2-devel] [PATCH 2/4] MdeModulePkg/SdMmcPciHcDxe: Read response on command completion Thread-Topic: [edk2-devel] [PATCH 2/4] MdeModulePkg/SdMmcPciHcDxe: Read response on command completion Thread-Index: AQHV2pz0Z0HJcHftHUCDGei1gWPEhagKinwg Date: Wed, 5 Feb 2020 03:16:18 +0000 Message-ID: References: <20200203141858.3236-1-mateusz.albecki@intel.com> <20200203141858.3236-3-mateusz.albecki@intel.com> In-Reply-To: <20200203141858.3236-3-mateusz.albecki@intel.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: hao.a.wu@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable One question below: > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > 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, Limin= g > Subject: [edk2-devel] [PATCH 2/4] MdeModulePkg/SdMmcPciHcDxe: Read > response on command completion >=20 > SdMmcPciHcDxe driver used to read response only after > command and data transfer completed. According to SDHCI > specification response data is ready after the command > complete status is set by the host controller. Getting > the response data early will help debugging the cases > when command completed but data transfer timed out. >=20 > Cc: Hao A Wu > Cc: Marcin Wojtas > Cc: Zhichao Gao > Cc: Liming Gao >=20 > Signed-off-by: Mateusz Albecki > --- > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 1 + > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 201 > +++++++++++++++------ > 2 files changed, 144 insertions(+), 58 deletions(-) >=20 > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h > index 5bc3577ba2..15b7d12596 100644 > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h > @@ -153,6 +153,7 @@ typedef struct { >=20 > EFI_EVENT Event; > BOOLEAN Started; > + BOOLEAN CommandComplete; > UINT64 Timeout; > UINT32 Retries; >=20 > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c > index 959645bf26..3dfaae8542 100644 > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c > @@ -1708,6 +1708,7 @@ SdMmcPrintTrb ( > DEBUG ((DebugLevel, "AdmaLengthMode: %d\n", Trb->AdmaLengthMode)); > DEBUG ((DebugLevel, "Event: %d\n", Trb->Event)); > DEBUG ((DebugLevel, "Started: %d\n", Trb->Started)); > + DEBUG ((DebugLevel, "CommandComplete: %d\n", Trb- > >CommandComplete)); > DEBUG ((DebugLevel, "Timeout: %d\n", Trb->Timeout)); > DEBUG ((DebugLevel, "Retries: %d\n", Trb->Retries)); > DEBUG ((DebugLevel, "Adma32Desc: %X\n", Trb->Adma32Desc)); > @@ -1758,6 +1759,7 @@ SdMmcCreateTrb ( > Trb->Packet =3D Packet; > Trb->Event =3D Event; > Trb->Started =3D FALSE; > + Trb->CommandComplete =3D FALSE; > Trb->Timeout =3D Packet->Timeout; > Trb->Retries =3D SD_MMC_TRB_RETRIES; > Trb->Private =3D Private; > @@ -2352,6 +2354,99 @@ SdMmcCheckAndRecoverErrors ( > return ErrorStatus; > } >=20 > +/** > + Reads the response data into the TRB buffer. > + This function assumes that caller made sure that > + command has completed. > + > + @param[in] Private A pointer to the SD_MMC_HC_PRIVATE_DATA > instance. > + @param[in] Trb The pointer to the SD_MMC_HC_TRB instance. > + > + @retval EFI_SUCCESS Response read successfully. > + @retval Others Failed to get response. > +**/ > +EFI_STATUS > +SdMmcGetResponse ( > + IN SD_MMC_HC_PRIVATE_DATA *Private, > + IN SD_MMC_HC_TRB *Trb > + ) > +{ > + EFI_SD_MMC_PASS_THRU_COMMAND_PACKET *Packet; > + UINT8 Index; > + UINT32 Response[4]; > + EFI_STATUS Status; > + > + Packet =3D Trb->Packet; > + > + if (Packet->SdMmcCmdBlk->CommandType =3D=3D SdMmcCommandTypeBc) { > + return EFI_SUCCESS; > + } > + > + for (Index =3D 0; Index < 4; Index++) { > + Status =3D SdMmcHcRwMmio ( > + Private->PciIo, > + Trb->Slot, > + SD_MMC_HC_RESPONSE + Index * 4, > + TRUE, > + sizeof (UINT32), > + &Response[Index] > + ); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + } > + CopyMem (Packet->SdMmcStatusBlk, Response, sizeof (Response)); > + > + return EFI_SUCCESS; > +} > + > +/** > + Checks if the command completed. If the command > + completed it gets the response and records the > + command completion in the TRB. > + > + @param[in] Private A pointer to the SD_MMC_HC_PRIVATE_DATA > instance. > + @param[in] Trb The pointer to the SD_MMC_HC_TRB instance. > + @param[in] IntStatus Snapshot of the normal interrupt status registe= r. > + > + @retval EFI_SUCCESS Command completed successfully. > + @retval EFI_NOT_READY Command completion still pending. > + @retval Others Command failed to complete. > +**/ > +EFI_STATUS > +SdMmcCheckCommandComplete ( > + IN SD_MMC_HC_PRIVATE_DATA *Private, > + IN SD_MMC_HC_TRB *Trb, > + IN UINT16 IntStatus > + ) > +{ > + UINT16 Data16; > + EFI_STATUS Status; > + > + if ((IntStatus & BIT0) !=3D 0) { > + Data16 =3D BIT0; > + Status =3D SdMmcHcRwMmio ( > + Private->PciIo, > + Trb->Slot, > + SD_MMC_HC_NOR_INT_STS, > + FALSE, > + sizeof (Data16), > + &Data16 > + ); Previously, the driver cleans the Command Complete (BIT0) at the beginning= of the execution of the next TRB in function SdMmcExecTrb(). Now, the patch w= ill clean this bit just after checking it. So the behavior in the patch is more aligned with the SD Host Controller S= pec, right? Best Regards, Hao Wu > + if (EFI_ERROR (Status)) { > + return Status; > + } > + Status =3D SdMmcGetResponse (Private, Trb); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + Trb->CommandComplete =3D TRUE; > + return EFI_SUCCESS; > + } > + > + return EFI_NOT_READY; > +} > + > /** > Check the TRB execution result. >=20 > @@ -2372,9 +2467,7 @@ SdMmcCheckTrbResult ( > EFI_STATUS Status; > EFI_SD_MMC_PASS_THRU_COMMAND_PACKET *Packet; > UINT16 IntStatus; > - UINT32 Response[4]; > UINT64 SdmaAddr; > - UINT8 Index; > UINT32 PioLength; >=20 > Packet =3D Trb->Packet; > @@ -2402,6 +2495,54 @@ SdMmcCheckTrbResult ( > goto Done; > } >=20 > + // > + // Tuning commands are the only ones that do not generate command > + // complete interrupt. Process them here before entering the code > + // that waits for command completion. > + // > + if (((Private->Slot[Trb->Slot].CardType =3D=3D EmmcCardType) && > + (Packet->SdMmcCmdBlk->CommandIndex =3D=3D > EMMC_SEND_TUNING_BLOCK)) || > + ((Private->Slot[Trb->Slot].CardType =3D=3D SdCardType) && > + (Packet->SdMmcCmdBlk->CommandIndex =3D=3D > SD_SEND_TUNING_BLOCK))) { > + // > + // When performing tuning procedure (Execute Tuning is set to 1) th= rough > PIO mode, > + // wait Buffer Read Ready bit of Normal Interrupt Status Register t= o be 1. > + // Refer to SD Host Controller Simplified Specification 3.0 figure = 2-29 for > details. > + // > + if ((IntStatus & BIT5) =3D=3D BIT5) { > + // > + // Clear Buffer Read Ready interrupt at first. > + // > + IntStatus =3D BIT5; > + SdMmcHcRwMmio (Private->PciIo, Trb->Slot, > SD_MMC_HC_NOR_INT_STS, FALSE, sizeof (IntStatus), &IntStatus); > + // > + // Read data out from Buffer Port register > + // > + for (PioLength =3D 0; PioLength < Trb->DataLen; PioLength +=3D 4)= { > + SdMmcHcRwMmio (Private->PciIo, Trb->Slot, > SD_MMC_HC_BUF_DAT_PORT, TRUE, 4, (UINT8*)Trb->Data + PioLength); > + } > + Status =3D EFI_SUCCESS; > + goto Done; > + } > + } > + > + if (!Trb->CommandComplete) { > + Status =3D SdMmcCheckCommandComplete (Private, Trb, IntStatus); > + if (EFI_ERROR (Status)) { > + goto Done; > + } else { > + // > + // If the command doesn't require data transfer skip the transfer > + // complete checking. > + // > + if ((Packet->SdMmcCmdBlk->CommandType !=3D > SdMmcCommandTypeAdtc) && > + (Packet->SdMmcCmdBlk->ResponseType !=3D SdMmcResponseTypeR1b) > && > + (Packet->SdMmcCmdBlk->ResponseType !=3D SdMmcResponseTypeR5b)= ) > { > + goto Done; > + } > + } > + } > + > // > // Check Transfer Complete bit is set or not. > // > @@ -2459,65 +2600,9 @@ SdMmcCheckTrbResult ( > Trb->DataPhy =3D (UINT64)(UINTN)SdmaAddr; > } >=20 > - if ((Packet->SdMmcCmdBlk->CommandType !=3D SdMmcCommandTypeAdtc) > && > - (Packet->SdMmcCmdBlk->ResponseType !=3D SdMmcResponseTypeR1b) > && > - (Packet->SdMmcCmdBlk->ResponseType !=3D SdMmcResponseTypeR5b)) { > - if ((IntStatus & BIT0) =3D=3D BIT0) { > - Status =3D EFI_SUCCESS; > - goto Done; > - } > - } > - > - if (((Private->Slot[Trb->Slot].CardType =3D=3D EmmcCardType) && > - (Packet->SdMmcCmdBlk->CommandIndex =3D=3D > EMMC_SEND_TUNING_BLOCK)) || > - ((Private->Slot[Trb->Slot].CardType =3D=3D SdCardType) && > - (Packet->SdMmcCmdBlk->CommandIndex =3D=3D > SD_SEND_TUNING_BLOCK))) { > - // > - // When performing tuning procedure (Execute Tuning is set to 1) th= rough > PIO mode, > - // wait Buffer Read Ready bit of Normal Interrupt Status Register t= o be 1. > - // Refer to SD Host Controller Simplified Specification 3.0 figure = 2-29 for > details. > - // > - if ((IntStatus & BIT5) =3D=3D BIT5) { > - // > - // Clear Buffer Read Ready interrupt at first. > - // > - IntStatus =3D BIT5; > - SdMmcHcRwMmio (Private->PciIo, Trb->Slot, > SD_MMC_HC_NOR_INT_STS, FALSE, sizeof (IntStatus), &IntStatus); > - // > - // Read data out from Buffer Port register > - // > - for (PioLength =3D 0; PioLength < Trb->DataLen; PioLength +=3D 4)= { > - SdMmcHcRwMmio (Private->PciIo, Trb->Slot, > SD_MMC_HC_BUF_DAT_PORT, TRUE, 4, (UINT8*)Trb->Data + PioLength); > - } > - Status =3D EFI_SUCCESS; > - goto Done; > - } > - } >=20 > Status =3D EFI_NOT_READY; > Done: > - // > - // Get response data when the cmd is executed successfully. > - // > - if (!EFI_ERROR (Status)) { > - if (Packet->SdMmcCmdBlk->CommandType !=3D SdMmcCommandTypeBc) { > - for (Index =3D 0; Index < 4; Index++) { > - Status =3D SdMmcHcRwMmio ( > - Private->PciIo, > - Trb->Slot, > - SD_MMC_HC_RESPONSE + Index * 4, > - TRUE, > - sizeof (UINT32), > - &Response[Index] > - ); > - if (EFI_ERROR (Status)) { > - SdMmcHcLedOnOff (Private->PciIo, Trb->Slot, FALSE); > - return Status; > - } > - } > - CopyMem (Packet->SdMmcStatusBlk, Response, sizeof (Response)); > - } > - } >=20 > if (Status !=3D EFI_NOT_READY) { > SdMmcHcLedOnOff (Private->PciIo, Trb->Slot, FALSE); > -- > 2.14.1.windows.1 >=20 > -------------------------------------------------------------------- >=20 > 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. >=20 > 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 th= e > sole use of the intended recipient(s). If you are not the intended recip= ient, > please contact the sender and delete all copies; any review or distribut= ion by > others is strictly prohibited. >=20 >=20 >=20