[edk2-devel] [PATCH 4/4] MdeModulePkg/SdMmcPciHcDxe: Fix PIO transfer mode
Wu, Hao A
hao.a.wu at intel.com
Wed Feb 5 03:16:23 UTC 2020
> -----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.
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#53813): https://edk2.groups.io/g/devel/message/53813
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