[edk2-devel] [PATCH 4/4] MdeModulePkg/SdMmcPciHcDxe: Fix PIO transfer mode
Albecki, Mateusz
mateusz.albecki at intel.com
Mon Feb 10 13:11:36 UTC 2020
I will write a test code and update the results in next patch series.
Regarding the buffer data port read. Reading the spec I am not entirely sure on the behavior of the host when the data transfer is not aligned to DWORD boundary. I will test it with width set to 32bit and if that works I will fix it in v2.
Thanks,
Mateusz
> -----Original Message-----
> From: Wu, Hao A <hao.a.wu at intel.com>
> Sent: Wednesday, February 5, 2020 4:16 AM
> To: devel at edk2.groups.io; Albecki, Mateusz <mateusz.albecki at intel.com>
> Cc: Marcin Wojtas <mw at semihalf.com>; Gao, Zhichao
> <zhichao.gao at intel.com>; Gao, Liming <liming.gao at intel.com>
> Subject: RE: [edk2-devel] [PATCH 4/4] MdeModulePkg/SdMmcPciHcDxe: Fix
> PIO transfer mode
>
> > -----Original Message-----
> > From: devel at edk2.groups.io [mailto:devel at edk2.groups.io] On Behalf Of
> > Albecki, Mateusz
> > Sent: Monday, February 03, 2020 10:19 PM
> > To: devel at edk2.groups.io
> > Cc: Albecki, Mateusz; Wu, Hao A; Marcin Wojtas; Gao, Zhichao; Gao,
> > Liming
> > Subject: [edk2-devel] [PATCH 4/4] MdeModulePkg/SdMmcPciHcDxe: Fix
> PIO
> > transfer mode
> >
> > Current driver does not support PIO transfer mode for commands other
> > then tuning. This change adds the code to transfer PIO data.
>
>
> Hello Mateusz,
>
> Try to provide some feedbacks before I can test the patch.
>
> One test request, is it possible for you to test the asynchronous transfer for
> the PIO mode?
>
> A possible method can be using an UEFI application to locate the BlockIO 2
> protocol from a specific SD or eMMC device (which forced to PIO transfer
> mode).
> And test with the WriteBlocksEx() & ReadBlocksEx() services to see if the RW
> is successful.
>
> Also, one more inline comment below:
>
>
> >
> > Cc: Hao A Wu <hao.a.wu at intel.com>
> > Cc: Marcin Wojtas <mw at semihalf.com>
> > Cc: Zhichao Gao <zhichao.gao at intel.com>
> > Cc: Liming Gao <liming.gao at intel.com>
> >
> > Signed-off-by: Mateusz Albecki <mateusz.albecki at intel.com>
> > ---
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 3 +
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 113
> > +++++++++++++++++----
> > 2 files changed, 95 insertions(+), 21 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> > index 15b7d12596..fd89306fab 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> > @@ -157,6 +157,9 @@ typedef struct {
> > UINT64 Timeout;
> > UINT32 Retries;
> >
> > + BOOLEAN PioModeTransferCompleted;
> > + UINT32 PioBlockIndex;
> > +
> > SD_MMC_HC_ADMA_32_DESC_LINE *Adma32Desc;
> > SD_MMC_HC_ADMA_64_V3_DESC_LINE *Adma64V3Desc;
> > SD_MMC_HC_ADMA_64_V4_DESC_LINE *Adma64V4Desc;
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > index 480a1664ea..43703974f7 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > @@ -1711,6 +1711,8 @@ SdMmcPrintTrb (
> > DEBUG ((DebugLevel, "CommandComplete: %d\n", Trb-
> > >CommandComplete));
> > DEBUG ((DebugLevel, "Timeout: %d\n", Trb->Timeout));
> > DEBUG ((DebugLevel, "Retries: %d\n", Trb->Retries));
> > + DEBUG ((DebugLevel, "PioModeTransferCompleted: %d\n", Trb-
> > >PioModeTransferCompleted));
> > + DEBUG ((DebugLevel, "PioBlockIndex: %d\n", Trb->PioBlockIndex));
> > DEBUG ((DebugLevel, "Adma32Desc: %X\n", Trb->Adma32Desc));
> > DEBUG ((DebugLevel, "Adma64V3Desc: %X\n", Trb->Adma64V3Desc));
> > DEBUG ((DebugLevel, "Adma64V4Desc: %X\n", Trb->Adma64V4Desc));
> > @@ -1762,6 +1764,8 @@ SdMmcCreateTrb (
> > Trb->CommandComplete = FALSE;
> > Trb->Timeout = Packet->Timeout;
> > Trb->Retries = SD_MMC_TRB_RETRIES;
> > + Trb->PioModeTransferCompleted = FALSE;
> > + Trb->PioBlockIndex = 0;
> > Trb->Private = Private;
> >
> > if ((Packet->InTransferLength != 0) && (Packet->InDataBuffer != NULL)) {
> > @@ -2447,6 +2451,85 @@ SdMmcCheckCommandComplete (
> > return EFI_NOT_READY;
> > }
> >
> > +/**
> > + Transfers data from card using PIO method.
> > +
> > + @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 PIO transfer completed successfully.
> > + @retval EFI_NOT_READY PIO transfer completion still pending.
> > + @retval Others PIO transfer failed to complete.
> > +**/
> > +EFI_STATUS
> > +SdMmcTransferDataWithPio (
> > + IN SD_MMC_HC_PRIVATE_DATA *Private,
> > + IN SD_MMC_HC_TRB *Trb,
> > + IN UINT16 IntStatus
> > + )
> > +{
> > + EFI_STATUS Status;
> > + UINT16 Data16;
> > + UINT32 BlockCount;
> > +
> > + BlockCount = (Trb->DataLen / Trb->BlockSize);
> > + if (Trb->DataLen % Trb->BlockSize != 0) {
> > + BlockCount += 1;
> > + }
> > +
> > + if (Trb->PioBlockIndex >= BlockCount) {
> > + return EFI_SUCCESS;
> > + }
> > +
> > + if (Trb->Read) {
> > + if ((IntStatus & BIT5) == 0) {
> > + return EFI_NOT_READY;
> > + }
> > + Data16 = BIT5;
> > + SdMmcHcRwMmio (Private->PciIo, Trb->Slot,
> > SD_MMC_HC_NOR_INT_STS, FALSE, sizeof (Data16), &Data16);
> > +
> > + Status = Private->PciIo->Mem.Read (
> > + Private->PciIo,
> > + EfiPciIoWidthFifoUint8,
> > + Trb->Slot,
> > + SD_MMC_HC_BUF_DAT_PORT,
> > + Trb->BlockSize,
> > + (VOID*)((UINT8*)Trb->Data + (Trb->BlockSize * Trb-
> > >PioBlockIndex))
> > + );
>
>
> The read (write) process will be:
>
> 1. Wait for the Buffer Read (Write) Ready to set;
> 2. Clear the Buffer Read (Write) Ready bit;
> 3. Access the Buffer Data Port register 'Trb->BlockSize' times, each time
> consuming 1 byte to get all the data in a block.
>
> Since we are accessing the Buffer Data Port register, same BAR offset, so
> 'EfiPciIoWidthFifoUint8' is used here.
>
> Is my understanding correct?
>
> If so, I am thinking is it less efficient during the read/write of the data?
> Since the Buffer Data Port register is 4-byte in width, data can be accessed
> at most 4 bytes a time. Not sure if doing so can save time for the PIO transfer.
>
> Best Regards,
> Hao Wu
>
>
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > + Trb->PioBlockIndex++;
> > + } else {
> > + if ((IntStatus & BIT4) == 0) {
> > + return EFI_NOT_READY;
> > + }
> > + Data16 = BIT4;
> > + SdMmcHcRwMmio (Private->PciIo, Trb->Slot,
> > SD_MMC_HC_NOR_INT_STS, FALSE, sizeof (Data16), &Data16);
> > +
> > + Status = Private->PciIo->Mem.Write (
> > + Private->PciIo,
> > + EfiPciIoWidthFifoUint8,
> > + Trb->Slot,
> > + SD_MMC_HC_BUF_DAT_PORT,
> > + Trb->BlockSize,
> > + (VOID*)((UINT8*)Trb->Data + (Trb->BlockSize * Trb-
> > >PioBlockIndex))
> > + );
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > + Trb->PioBlockIndex++;
> > + }
> > +
> > + if (Trb->PioBlockIndex >= BlockCount) {
> > + Trb->PioModeTransferCompleted = TRUE;
> > + return EFI_SUCCESS;
> > + } else {
> > + return EFI_NOT_READY;
> > + }
> > +}
> > +
> > /**
> > Update the SDMA address on the SDMA buffer boundary interrupt.
> >
> > @@ -2531,6 +2614,13 @@ SdMmcCheckDataTransfer (
> > return Status;
> > }
> >
> > + if (Trb->Mode == SdMmcPioMode && !Trb-
> >PioModeTransferCompleted) {
> > + Status = SdMmcTransferDataWithPio (Private, Trb, IntStatus);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > + }
> > +
> > if ((Trb->Mode == SdMmcSdmaMode) && ((IntStatus & BIT3) != 0)) {
> > Data16 = BIT3;
> > Status = SdMmcHcRwMmio (
> > @@ -2573,7 +2663,6 @@ SdMmcCheckTrbResult (
> > EFI_STATUS Status;
> > EFI_SD_MMC_PASS_THRU_COMMAND_PACKET *Packet;
> > UINT16 IntStatus;
> > - UINT32 PioLength;
> >
> > Packet = Trb->Packet;
> > //
> > @@ -2609,26 +2698,8 @@ SdMmcCheckTrbResult (
> > (Packet->SdMmcCmdBlk->CommandIndex ==
> > EMMC_SEND_TUNING_BLOCK)) ||
> > ((Private->Slot[Trb->Slot].CardType == SdCardType) &&
> > (Packet->SdMmcCmdBlk->CommandIndex ==
> > SD_SEND_TUNING_BLOCK))) {
> > - //
> > - // When performing tuning procedure (Execute Tuning is set to 1)
> through
> > PIO mode,
> > - // wait Buffer Read Ready bit of Normal Interrupt Status Register to be 1.
> > - // Refer to SD Host Controller Simplified Specification 3.0 figure 2-29 for
> > details.
> > - //
> > - if ((IntStatus & BIT5) == BIT5) {
> > - //
> > - // Clear Buffer Read Ready interrupt at first.
> > - //
> > - IntStatus = 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 = 0; PioLength < Trb->DataLen; PioLength += 4) {
> > - SdMmcHcRwMmio (Private->PciIo, Trb->Slot,
> > SD_MMC_HC_BUF_DAT_PORT, TRUE, 4, (UINT8*)Trb->Data + PioLength);
> > - }
> > - Status = EFI_SUCCESS;
> > - goto Done;
> > - }
> > + Status = SdMmcTransferDataWithPio (Private, Trb, IntStatus);
> > + goto Done;
> > }
> >
> > if (!Trb->CommandComplete) {
> > --
> > 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.
> >
> >
> >
>
--------------------------------------------------------------------
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 (#54136): https://edk2.groups.io/g/devel/message/54136
Mute This Topic: https://groups.io/mt/70947190/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list