public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* Re: [edk2-devel] [PATCH v2] MdeModulePkg: Enabling OS boot from SD card through UEFI payload
  2022-01-17 12:55 Aiman Rosli
@ 2022-01-18  1:05 ` Wu, Hao A
  0 siblings, 0 replies; 3+ messages in thread
From: Wu, Hao A @ 2022-01-18  1:05 UTC (permalink / raw)
  To: devel@edk2.groups.io, Rosli, Muhammad Aiman

Please refer to the inline comments below:


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Aiman
> Rosli
> Sent: Monday, January 17, 2022 8:56 PM
> To: devel@edk2.groups.io
> Cc: Rosli, Muhammad Aiman <muhammad.aiman.rosli@intel.com>
> Subject: [edk2-devel] [PATCH v2] MdeModulePkg: Enabling OS boot from SD
> card through UEFI payload
> 
> This changes is by adding 50ms delay during voltage switching from 3.3V to
> 1.8V, plus adding a goto Voltage33Retry for 3.3V checking and retrying.
> 
> Signed-off-by: Aiman Rosli <muhammad.aiman.rosli@intel.com>
> ---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 23
> ++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> index 662f9f483c..527fe7dc20 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> @@ -1213,9 +1213,14 @@ SdCardIdentification (
>    UINT32                         PresentState;
>    UINT8                          HostCtrl2;
>    UINTN                          Retry;
> +  BOOLEAN                        ForceVoltage33;
> +
> +  ForceVoltage33 = FALSE;
> 
>    PciIo    = Private->PciIo;
>    PassThru = &Private->PassThru;
> +
> +Voltage33Retry:
>    //
>    // 1. Send Cmd0 to the device
>    //
> @@ -1294,6 +1299,14 @@ SdCardIdentification (
>      return EFI_UNSUPPORTED;
>    }
> 
> +  //
> +  // 1.8V had failed in the previous run, forcing a retry with 3.3V
> + instead  //  if (ForceVoltage33 == TRUE) {
> +    S18r           = FALSE;
> +    ForceVoltage33 = FALSE;


Like I mentioned in https://github.com/tianocore/edk2/pull/2413#discussion_r785611042
I do not think "ForceVoltage33 = FALSE" is necessary, could you help to double check?


> +  }
> +
>    //
>    // 5. Repeatly send Acmd41 with supply voltage window to the device.
>    //    Note here we only support the cards complied with SD physical
> @@ -1362,13 +1375,17 @@ SdCardIdentification (
>          goto Error;
>        }
> 
> -      gBS->Stall (1000);
> +      // Workaround to add a delay of 50 ms in order for clock to stabilize
> before turning on the SD card again.
> +      gBS->Stall (50000);


Sorry for a question.
As I mentioned in:
https://github.com/tianocore/edk2/pull/2413#discussion_r785610179
https://github.com/tianocore/edk2/pull/2413#discussion_r785610742

Can this 50 milliseconds delay be put in the below 1.8V switch failure handling logic?
Enlarging the delay (from 1ms to 50ms) in the normal initialization flow will bring performance impact.


> 
>        SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_PRESENT_STATE, TRUE,
> sizeof (PresentState), &PresentState);
>        if (((PresentState >> 20) & 0xF) != 0xF) {
>          DEBUG ((DEBUG_ERROR, "SdCardIdentification: SwitchVoltage fails with
> PresentState = 0x%x, It should be 0xF\n", PresentState));
> -        Status = EFI_DEVICE_ERROR;
> -        goto Error;
> +        Status         = SdMmcHcReset (Private, Slot);
> +        Status         = SdMmcHcInitHost (Private, Slot);


Could you help to check my comment in:
https://github.com/tianocore/edk2/pull/2413#discussion_r785610742
to 1) add comments and 2) refine the error handling for the "reset and reinitialize the slot before the 3.3V retry"

Best Regards,
Hao Wu


> +        ForceVoltage33 = TRUE;
> +        DEBUG ((DEBUG_ERROR, "SdCardIdentification: Switching to 1.8V had
> failed in the previous run, forcing a retry with 3.3V instead\n"));
> +        goto Voltage33Retry;
>        }
>      }
> 
> --
> 2.34.1.windows.1
> 
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [edk2-devel] [PATCH v2] MdeModulePkg: Enabling OS boot from SD card through UEFI payload
       [not found] <16CB1013993012B3.30851@groups.io>
@ 2022-01-18 16:25 ` Aiman Rosli
  2022-01-19  0:45   ` Wu, Hao A
  0 siblings, 1 reply; 3+ messages in thread
From: Aiman Rosli @ 2022-01-18 16:25 UTC (permalink / raw)
  To: devel@edk2.groups.io, Wu, Hao A, Gao, Zhichao, Ni, Ray,
	Wang, Jian J

Hi all, 

Please review my patch, if there is no issue, can help to add push label.

Thank you.

Regards,
Aiman Rosli

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Aiman Rosli
Sent: Monday, 17 January, 2022 8:56 PM
To: devel@edk2.groups.io
Cc: Rosli, Muhammad Aiman <muhammad.aiman.rosli@intel.com>
Subject: [edk2-devel] [PATCH v2] MdeModulePkg: Enabling OS boot from SD card through UEFI payload

This changes is by adding 50ms delay during voltage switching from 3.3V to 1.8V, plus adding a goto Voltage33Retry for 3.3V checking and retrying.

Signed-off-by: Aiman Rosli <muhammad.aiman.rosli@intel.com>
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 23 ++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
index 662f9f483c..527fe7dc20 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
@@ -1213,9 +1213,14 @@ SdCardIdentification (
   UINT32                         PresentState;
   UINT8                          HostCtrl2;
   UINTN                          Retry;
+  BOOLEAN                        ForceVoltage33;
+
+  ForceVoltage33 = FALSE;
 
   PciIo    = Private->PciIo;
   PassThru = &Private->PassThru;
+
+Voltage33Retry:
   //
   // 1. Send Cmd0 to the device
   //
@@ -1294,6 +1299,14 @@ SdCardIdentification (
     return EFI_UNSUPPORTED;
   }
 
+  //
+  // 1.8V had failed in the previous run, forcing a retry with 3.3V 
+ instead  //  if (ForceVoltage33 == TRUE) {
+    S18r           = FALSE;
+    ForceVoltage33 = FALSE;
+  }
+
   //
   // 5. Repeatly send Acmd41 with supply voltage window to the device.
   //    Note here we only support the cards complied with SD physical
@@ -1362,13 +1375,17 @@ SdCardIdentification (
         goto Error;
       }
 
-      gBS->Stall (1000);
+      // Workaround to add a delay of 50 ms in order for clock to stabilize before turning on the SD card again.
+      gBS->Stall (50000);
 
       SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_PRESENT_STATE, TRUE, sizeof (PresentState), &PresentState);
       if (((PresentState >> 20) & 0xF) != 0xF) {
         DEBUG ((DEBUG_ERROR, "SdCardIdentification: SwitchVoltage fails with PresentState = 0x%x, It should be 0xF\n", PresentState));
-        Status = EFI_DEVICE_ERROR;
-        goto Error;
+        Status         = SdMmcHcReset (Private, Slot);
+        Status         = SdMmcHcInitHost (Private, Slot);
+        ForceVoltage33 = TRUE;
+        DEBUG ((DEBUG_ERROR, "SdCardIdentification: Switching to 1.8V had failed in the previous run, forcing a retry with 3.3V instead\n"));
+        goto Voltage33Retry;
       }
     }
 
--
2.34.1.windows.1







^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [edk2-devel] [PATCH v2] MdeModulePkg: Enabling OS boot from SD card through UEFI payload
  2022-01-18 16:25 ` [edk2-devel] [PATCH v2] MdeModulePkg: Enabling OS boot from SD card through UEFI payload Aiman Rosli
@ 2022-01-19  0:45   ` Wu, Hao A
  0 siblings, 0 replies; 3+ messages in thread
From: Wu, Hao A @ 2022-01-19  0:45 UTC (permalink / raw)
  To: Rosli, Muhammad Aiman, devel@edk2.groups.io, Gao, Zhichao,
	Ni, Ray, Wang, Jian J

Hello Aiman Rosli,

I have provided my feedbacks by replying your patch mail.
Please help to check if you received the mail.
Or you can check the feedbacks at: https://edk2.groups.io/g/devel/message/85764

Best Regards,
Hao Wu

> -----Original Message-----
> From: Rosli, Muhammad Aiman <muhammad.aiman.rosli@intel.com>
> Sent: Wednesday, January 19, 2022 12:26 AM
> To: devel@edk2.groups.io; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhichao
> <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>
> Subject: RE: [edk2-devel] [PATCH v2] MdeModulePkg: Enabling OS boot from
> SD card through UEFI payload
> 
> Hi all,
> 
> Please review my patch, if there is no issue, can help to add push label.
> 
> Thank you.
> 
> Regards,
> Aiman Rosli
> 
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Aiman
> Rosli
> Sent: Monday, 17 January, 2022 8:56 PM
> To: devel@edk2.groups.io
> Cc: Rosli, Muhammad Aiman <muhammad.aiman.rosli@intel.com>
> Subject: [edk2-devel] [PATCH v2] MdeModulePkg: Enabling OS boot from SD
> card through UEFI payload
> 
> This changes is by adding 50ms delay during voltage switching from 3.3V to
> 1.8V, plus adding a goto Voltage33Retry for 3.3V checking and retrying.
> 
> Signed-off-by: Aiman Rosli <muhammad.aiman.rosli@intel.com>
> ---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 23
> ++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> index 662f9f483c..527fe7dc20 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> @@ -1213,9 +1213,14 @@ SdCardIdentification (
>    UINT32                         PresentState;
>    UINT8                          HostCtrl2;
>    UINTN                          Retry;
> +  BOOLEAN                        ForceVoltage33;
> +
> +  ForceVoltage33 = FALSE;
> 
>    PciIo    = Private->PciIo;
>    PassThru = &Private->PassThru;
> +
> +Voltage33Retry:
>    //
>    // 1. Send Cmd0 to the device
>    //
> @@ -1294,6 +1299,14 @@ SdCardIdentification (
>      return EFI_UNSUPPORTED;
>    }
> 
> +  //
> +  // 1.8V had failed in the previous run, forcing a retry with 3.3V
> + instead  //  if (ForceVoltage33 == TRUE) {
> +    S18r           = FALSE;
> +    ForceVoltage33 = FALSE;
> +  }
> +
>    //
>    // 5. Repeatly send Acmd41 with supply voltage window to the device.
>    //    Note here we only support the cards complied with SD physical
> @@ -1362,13 +1375,17 @@ SdCardIdentification (
>          goto Error;
>        }
> 
> -      gBS->Stall (1000);
> +      // Workaround to add a delay of 50 ms in order for clock to stabilize
> before turning on the SD card again.
> +      gBS->Stall (50000);
> 
>        SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_PRESENT_STATE, TRUE,
> sizeof (PresentState), &PresentState);
>        if (((PresentState >> 20) & 0xF) != 0xF) {
>          DEBUG ((DEBUG_ERROR, "SdCardIdentification: SwitchVoltage fails with
> PresentState = 0x%x, It should be 0xF\n", PresentState));
> -        Status = EFI_DEVICE_ERROR;
> -        goto Error;
> +        Status         = SdMmcHcReset (Private, Slot);
> +        Status         = SdMmcHcInitHost (Private, Slot);
> +        ForceVoltage33 = TRUE;
> +        DEBUG ((DEBUG_ERROR, "SdCardIdentification: Switching to 1.8V had
> failed in the previous run, forcing a retry with 3.3V instead\n"));
> +        goto Voltage33Retry;
>        }
>      }
> 
> --
> 2.34.1.windows.1
> 
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-01-19  0:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <16CB1013993012B3.30851@groups.io>
2022-01-18 16:25 ` [edk2-devel] [PATCH v2] MdeModulePkg: Enabling OS boot from SD card through UEFI payload Aiman Rosli
2022-01-19  0:45   ` Wu, Hao A
2022-01-17 12:55 Aiman Rosli
2022-01-18  1:05 ` [edk2-devel] " Wu, Hao A

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox