Just a similar question to PATCH 2/4 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, Liming > Subject: [edk2-devel] [PATCH 3/4] MdeModulePkg/SdMmcPciHcDxe: > Refactor data transfer completion > > This patch refactors the way in which the driver will check > the data transfer completion. Data transfer related > functionalities have been moved to separate function. > > 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 | 181 > ++++++++++++++--------- > 1 file changed, 112 insertions(+), 69 deletions(-) > > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c > index 3dfaae8542..480a1664ea 100644 > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c > @@ -2447,6 +2447,112 @@ SdMmcCheckCommandComplete ( > return EFI_NOT_READY; > } > > +/** > + Update the SDMA address on the SDMA buffer boundary interrupt. > + > + @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 Updated SDMA buffer address. > + @retval Others Failed to update SDMA buffer address. > +**/ > +EFI_STATUS > +SdMmcUpdateSdmaAddress ( > + IN SD_MMC_HC_PRIVATE_DATA *Private, > + IN SD_MMC_HC_TRB *Trb > + ) > +{ > + UINT64 SdmaAddr; > + EFI_STATUS Status; > + > + SdmaAddr = SD_MMC_SDMA_ROUND_UP ((UINTN)Trb->DataPhy, > SD_MMC_SDMA_BOUNDARY); > + > + if (Private->ControllerVersion[Trb->Slot] >= SD_MMC_HC_CTRL_VER_400) > { > + Status = SdMmcHcRwMmio ( > + Private->PciIo, > + Trb->Slot, > + SD_MMC_HC_ADMA_SYS_ADDR, > + FALSE, > + sizeof (UINT64), > + &SdmaAddr > + ); > + } else { > + Status = SdMmcHcRwMmio ( > + Private->PciIo, > + Trb->Slot, > + SD_MMC_HC_SDMA_ADDR, > + FALSE, > + sizeof (UINT32), > + &SdmaAddr > + ); > + } > + > + if (EFI_ERROR (Status)) { > + return Status; > + } > + > + Trb->DataPhy = (UINT64)(UINTN)SdmaAddr; > + return EFI_SUCCESS; > +} > + > +/** > + Checks if the data transfer completed and performs any actions > + neccessary to continue the data transfer such as SDMA system > + address fixup or PIO data transfer. > + > + @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 register. > + > + @retval EFI_SUCCESS Data transfer completed successfully. > + @retval EFI_NOT_READY Data transfer completion still pending. > + @retval Others Data transfer failed to complete. > +**/ > +EFI_STATUS > +SdMmcCheckDataTransfer ( > + IN SD_MMC_HC_PRIVATE_DATA *Private, > + IN SD_MMC_HC_TRB *Trb, > + IN UINT16 IntStatus > + ) > +{ > + UINT16 Data16; > + EFI_STATUS Status; > + > + if ((IntStatus & BIT1) != 0) { > + Data16 = BIT1; > + Status = SdMmcHcRwMmio ( > + Private->PciIo, > + Trb->Slot, > + SD_MMC_HC_NOR_INT_STS, > + FALSE, > + sizeof (Data16), > + &Data16 > + ); Cleaning the Transfer Complete (BIT1) right after checking it is more aligned with the spec, right? Best Regards, Hao Wu > + return Status; > + } > + > + if ((Trb->Mode == SdMmcSdmaMode) && ((IntStatus & BIT3) != 0)) { > + Data16 = BIT3; > + Status = SdMmcHcRwMmio ( > + Private->PciIo, > + Trb->Slot, > + SD_MMC_HC_NOR_INT_STS, > + FALSE, > + sizeof (Data16), > + &Data16 > + ); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + Status = SdMmcUpdateSdmaAddress (Private, Trb); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + } > + > + return EFI_NOT_READY; > +} > + > /** > Check the TRB execution result. > > @@ -2467,7 +2573,6 @@ SdMmcCheckTrbResult ( > EFI_STATUS Status; > EFI_SD_MMC_PASS_THRU_COMMAND_PACKET *Packet; > UINT16 IntStatus; > - UINT64 SdmaAddr; > UINT32 PioLength; > > Packet = Trb->Packet; > @@ -2530,80 +2635,18 @@ SdMmcCheckTrbResult ( > Status = 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 != > SdMmcCommandTypeAdtc) && > - (Packet->SdMmcCmdBlk->ResponseType != SdMmcResponseTypeR1b) > && > - (Packet->SdMmcCmdBlk->ResponseType != SdMmcResponseTypeR5b)) > { > - goto Done; > - } > } > } > > - // > - // Check Transfer Complete bit is set or not. > - // > - if ((IntStatus & BIT1) == BIT1) { > - goto Done; > - } > - > - // > - // Check if DMA interrupt is signalled for the SDMA transfer. > - // > - if ((Trb->Mode == SdMmcSdmaMode) && ((IntStatus & BIT3) == BIT3)) { > - // > - // Clear DMA interrupt bit. > - // > - IntStatus = BIT3; > - Status = SdMmcHcRwMmio ( > - Private->PciIo, > - Trb->Slot, > - SD_MMC_HC_NOR_INT_STS, > - FALSE, > - sizeof (IntStatus), > - &IntStatus > - ); > - if (EFI_ERROR (Status)) { > - goto Done; > - } > - // > - // Update SDMA Address register. > - // > - SdmaAddr = SD_MMC_SDMA_ROUND_UP ((UINTN)Trb->DataPhy, > SD_MMC_SDMA_BOUNDARY); > - > - if (Private->ControllerVersion[Trb->Slot] >= SD_MMC_HC_CTRL_VER_400) > { > - Status = SdMmcHcRwMmio ( > - Private->PciIo, > - Trb->Slot, > - SD_MMC_HC_ADMA_SYS_ADDR, > - FALSE, > - sizeof (UINT64), > - &SdmaAddr > - ); > - } else { > - Status = SdMmcHcRwMmio ( > - Private->PciIo, > - Trb->Slot, > - SD_MMC_HC_SDMA_ADDR, > - FALSE, > - sizeof (UINT32), > - &SdmaAddr > - ); > - } > - > - if (EFI_ERROR (Status)) { > - goto Done; > - } > - Trb->DataPhy = (UINT64)(UINTN)SdmaAddr; > + if (Packet->SdMmcCmdBlk->CommandType == SdMmcCommandTypeAdtc > || > + Packet->SdMmcCmdBlk->ResponseType == SdMmcResponseTypeR1b || > + Packet->SdMmcCmdBlk->ResponseType == SdMmcResponseTypeR5b) { > + Status = SdMmcCheckDataTransfer (Private, Trb, IntStatus); > + } else { > + Status = EFI_SUCCESS; > } > > - > - Status = EFI_NOT_READY; > Done: > - > if (Status != EFI_NOT_READY) { > SdMmcHcLedOnOff (Private->PciIo, Trb->Slot, FALSE); > if (EFI_ERROR (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 (#53812): https://edk2.groups.io/g/devel/message/53812 Mute This Topic: https://groups.io/mt/70947188/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
