* [edk2-platform][PATCH 1/1] MdeModulePkg: SdMmcPciHcDxe: Fix issue that SD1.0 cards can't be recognized
@ 2022-12-06 6:48 Chevron Li
2022-12-06 8:28 ` Wu, Hao A
0 siblings, 1 reply; 3+ messages in thread
From: Chevron Li @ 2022-12-06 6:48 UTC (permalink / raw)
To: devel
Cc: hao.a.wu, ray.ni, jian.j.wang, gaoliming, shirley.her, shaper.liu,
xiaoguang.yu, Chevron Li (WH)
From: "Chevron Li (WH)" <chevron.li@bayhubtech.com>
SD1.0 cards don't support CMD8 and CMD6
CMD8 result can be used to distinguish the card is SD1.0 or not.
CMD8 result can be used to decide following CMD6 is sent or skip.
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Chevron Li <chevron.li@bayhubtech.com>
---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 30 ++++++++++++-------
1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
index f5a3607e47..281aa38170 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
@@ -1085,7 +1085,8 @@ SdCardSetBusMode (
IN EFI_SD_MMC_PASS_THRU_PROTOCOL *PassThru,
IN UINT8 Slot,
IN UINT16 Rca,
- IN BOOLEAN S18A
+ IN BOOLEAN S18A,
+ IN BOOLEAN SdVersion1
)
{
EFI_STATUS Status;
@@ -1117,10 +1118,13 @@ SdCardSetBusMode (
//
// Get the supported bus speed from SWITCH cmd return data group #1.
+ // SdVersion1 don't support the SWITCH cmd
//
- Status = SdCardSwitch (PassThru, Slot, 0xFF, 0xF, SdDriverStrengthIgnore, 0xF, FALSE, SwitchResp);
- if (EFI_ERROR (Status)) {
- return Status;
+ if(SdVersion1 == FALSE) {
+ Status = SdCardSwitch (PassThru, Slot, 0xFF, 0xF, SdDriverStrengthIgnore, 0xF, FALSE, SwitchResp);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
}
SdGetTargetBusMode (Private, Slot, SwitchResp, S18A, &BusMode);
@@ -1141,9 +1145,14 @@ SdCardSetBusMode (
}
}
- Status = SdCardSwitch (PassThru, Slot, BusMode.BusTiming, 0xF, BusMode.DriverStrength.Sd, 0xF, TRUE, SwitchResp);
- if (EFI_ERROR (Status)) {
- return Status;
+ //
+ // SdVersion1 don't support the SWITCH cmd
+ //
+ if(SdVersion1 == FALSE){
+ Status = SdCardSwitch (PassThru, Slot, BusMode.BusTiming, 0xF, BusMode.DriverStrength.Sd, 0xF, TRUE, SwitchResp);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
}
Status = SdMmcSetDriverStrength (Private->PciIo, Slot, BusMode.DriverStrength.Sd);
@@ -1214,6 +1223,7 @@ SdCardIdentification (
UINT8 HostCtrl2;
UINTN Retry;
BOOLEAN ForceVoltage33;
+ BOOLEAN SdVersion1;
ForceVoltage33 = FALSE;
@@ -1231,12 +1241,12 @@ Voltage33Retry:
}
//
- // 2. Send Cmd8 to the device
+ // 2. Send Cmd8 to the device, the command will fail for SdVersion1;
//
Status = SdCardVoltageCheck (PassThru, Slot, 0x1, 0xFF);
if (EFI_ERROR (Status)) {
+ SdVersion1 = 1;
DEBUG ((DEBUG_INFO, "SdCardIdentification: Executing Cmd8 fails with %r\n", Status));
- return Status;
}
//
@@ -1426,7 +1436,7 @@ Voltage33Retry:
DEBUG ((DEBUG_INFO, "SdCardIdentification: Found a SD device at slot [%d]\n", Slot));
Private->Slot[Slot].CardType = SdCardType;
- Status = SdCardSetBusMode (PciIo, PassThru, Slot, Rca, ((Ocr & BIT24) != 0));
+ Status = SdCardSetBusMode (PciIo, PassThru, Slot, Rca, ((Ocr & BIT24) != 0), SdVersion1);
return Status;
base-commit: 7bee2498910a9034faaf90802c49188afb7582dc
--
2.18.0.windows.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [edk2-platform][PATCH 1/1] MdeModulePkg: SdMmcPciHcDxe: Fix issue that SD1.0 cards can't be recognized
2022-12-06 6:48 [edk2-platform][PATCH 1/1] MdeModulePkg: SdMmcPciHcDxe: Fix issue that SD1.0 cards can't be recognized Chevron Li
@ 2022-12-06 8:28 ` Wu, Hao A
2022-12-06 9:51 ` 回复: " Chevron Li (WH)
0 siblings, 1 reply; 3+ messages in thread
From: Wu, Hao A @ 2022-12-06 8:28 UTC (permalink / raw)
To: Chevron Li, devel@edk2.groups.io
Cc: Ni, Ray, Wang, Jian J, Gao, Liming, shirley.her@bayhubtech.com,
shaper.liu@bayhubtech.com, xiaoguang.yu@bayhubtech.com
Hello,
The proposed patch will cause CI test failure: https://github.com/tianocore/edk2/pull/3724. Could you help to resolve them?
May I know what kind of tests have been performed for this patch? Have you tried with non SD1.0 cards and make sure that they still work fine after the patch?
Also, serveral more inline comments below:
> -----Original Message-----
> From: Chevron Li <chevron.li@bayhubtech.com>
> Sent: Tuesday, December 6, 2022 2:48 PM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Wang,
> Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>;
> shirley.her@bayhubtech.com; shaper.liu@bayhubtech.com;
> xiaoguang.yu@bayhubtech.com; Chevron Li (WH)
> <chevron.li@bayhubtech.com>
> Subject: [edk2-platform][PATCH 1/1] MdeModulePkg: SdMmcPciHcDxe: Fix
> issue that SD1.0 cards can't be recognized
>
> From: "Chevron Li (WH)" <chevron.li@bayhubtech.com>
>
>
> SD1.0 cards don't support CMD8 and CMD6
>
> CMD8 result can be used to distinguish the card is SD1.0 or not.
>
> CMD8 result can be used to decide following CMD6 is sent or skip.
>
>
>
> Cc: Hao A Wu <hao.a.wu@intel.com>
>
> Cc: Ray Ni <ray.ni@intel.com>
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>
> Signed-off-by: Chevron Li <chevron.li@bayhubtech.com>
>
> ---
>
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 30 ++++++++++++---
> ----
>
> 1 file changed, 20 insertions(+), 10 deletions(-)
>
>
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
>
> index f5a3607e47..281aa38170 100644
>
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
>
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
>
> @@ -1085,7 +1085,8 @@ SdCardSetBusMode (
>
> IN EFI_SD_MMC_PASS_THRU_PROTOCOL *PassThru,
>
> IN UINT8 Slot,
>
> IN UINT16 Rca,
>
> - IN BOOLEAN S18A
>
> + IN BOOLEAN S18A,
>
> + IN BOOLEAN SdVersion1
For the newly added input parameter 'SdVersion1', please help to update the function description comments to reflect it.
>
> )
>
> {
>
> EFI_STATUS Status;
>
> @@ -1117,10 +1118,13 @@ SdCardSetBusMode (
>
>
>
> //
>
> // Get the supported bus speed from SWITCH cmd return data group #1.
>
> + // SdVersion1 don't support the SWITCH cmd
>
> //
>
> - Status = SdCardSwitch (PassThru, Slot, 0xFF, 0xF, SdDriverStrengthIgnore,
> 0xF, FALSE, SwitchResp);
>
> - if (EFI_ERROR (Status)) {
>
> - return Status;
>
> + if(SdVersion1 == FALSE) {
>
> + Status = SdCardSwitch (PassThru, Slot, 0xFF, 0xF, SdDriverStrengthIgnore,
> 0xF, FALSE, SwitchResp);
The local variable 'SwitchResp' is an output parameter for SdCardSwitch().
When SdVersion1 is TRUE, SdCardSwitch() call will be skipped and 'SwitchResp' will have uninitialized value.
This will probably cause issues when 'SwitchResp' later being used as an input parameter for SdGetTargetBusMode() function call.
Could you help to check on this?
>
> + if (EFI_ERROR (Status)) {
>
> + return Status;
>
> + }
>
> }
>
>
>
> SdGetTargetBusMode (Private, Slot, SwitchResp, S18A, &BusMode);
>
> @@ -1141,9 +1145,14 @@ SdCardSetBusMode (
>
> }
>
> }
>
>
>
> - Status = SdCardSwitch (PassThru, Slot, BusMode.BusTiming, 0xF,
> BusMode.DriverStrength.Sd, 0xF, TRUE, SwitchResp);
>
> - if (EFI_ERROR (Status)) {
>
> - return Status;
>
> + //
>
> + // SdVersion1 don't support the SWITCH cmd
>
> + //
>
> + if(SdVersion1 == FALSE){
>
> + Status = SdCardSwitch (PassThru, Slot, BusMode.BusTiming, 0xF,
> BusMode.DriverStrength.Sd, 0xF, TRUE, SwitchResp);
>
> + if (EFI_ERROR (Status)) {
>
> + return Status;
>
> + }
>
> }
>
>
>
> Status = SdMmcSetDriverStrength (Private->PciIo, Slot,
> BusMode.DriverStrength.Sd);
>
> @@ -1214,6 +1223,7 @@ SdCardIdentification (
>
> UINT8 HostCtrl2;
>
> UINTN Retry;
>
> BOOLEAN ForceVoltage33;
>
> + BOOLEAN SdVersion1;
>
>
>
> ForceVoltage33 = FALSE;
Please help to assign a initial value for the newly added local variable 'SdVersion1'.
If the card supports cmd8, this local variable will have uninitialized value when calling SdCardSetBusMode().
>
>
>
> @@ -1231,12 +1241,12 @@ Voltage33Retry:
>
> }
>
>
>
> //
>
> - // 2. Send Cmd8 to the device
>
> + // 2. Send Cmd8 to the device, the command will fail for SdVersion1;
>
> //
>
> Status = SdCardVoltageCheck (PassThru, Slot, 0x1, 0xFF);
>
> if (EFI_ERROR (Status)) {
>
> + SdVersion1 = 1;
Please use "SdVersion1 = TRUE;".
Best Regards,
Hao Wu
>
> DEBUG ((DEBUG_INFO, "SdCardIdentification: Executing Cmd8 fails
> with %r\n", Status));
>
> - return Status;
>
> }
>
>
>
> //
>
> @@ -1426,7 +1436,7 @@ Voltage33Retry:
>
> DEBUG ((DEBUG_INFO, "SdCardIdentification: Found a SD device at slot
> [%d]\n", Slot));
>
> Private->Slot[Slot].CardType = SdCardType;
>
>
>
> - Status = SdCardSetBusMode (PciIo, PassThru, Slot, Rca, ((Ocr & BIT24) !=
> 0));
>
> + Status = SdCardSetBusMode (PciIo, PassThru, Slot, Rca, ((Ocr & BIT24) != 0),
> SdVersion1);
>
>
>
> return Status;
>
>
>
>
>
> base-commit: 7bee2498910a9034faaf90802c49188afb7582dc
>
> --
>
> 2.18.0.windows.1
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* 回复: [edk2-platform][PATCH 1/1] MdeModulePkg: SdMmcPciHcDxe: Fix issue that SD1.0 cards can't be recognized
2022-12-06 8:28 ` Wu, Hao A
@ 2022-12-06 9:51 ` Chevron Li (WH)
0 siblings, 0 replies; 3+ messages in thread
From: Chevron Li (WH) @ 2022-12-06 9:51 UTC (permalink / raw)
To: Wu, Hao A, devel@edk2.groups.io
Cc: Ni, Ray, Wang, Jian J, Gao, Liming, Shirley Her(SC),
Shaper Liu (WH), XiaoGuang Yu (WH)
Thanks for your response.
I will update patch later according to your suggestions.
Answer your questions below.
BR,
Chevron
> -----邮件原件-----
> 发件人: Wu, Hao A <hao.a.wu@intel.com>
> 发送时间: Tuesday, December 6, 2022 16:28
> 收件人: Chevron Li (WH) <chevron.li@bayhubtech.com>;
> devel@edk2.groups.io
> 抄送: Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Gao,
> Liming <gaoliming@byosoft.com.cn>; Shirley Her(SC)
> <shirley.her@bayhubtech.com>; Shaper Liu (WH)
> <shaper.liu@bayhubtech.com>; XiaoGuang Yu (WH)
> <xiaoguang.yu@bayhubtech.com>
> 主题: RE: [edk2-platform][PATCH 1/1] MdeModulePkg: SdMmcPciHcDxe: Fix
> issue that SD1.0 cards can't be recognized
>
> Hello,
>
> The proposed patch will cause CI test failure:
> https://github.com/tianocore/edk2/pull/3724. Could you help to resolve
> them?
Yes, I will update according to your suggestions.
> May I know what kind of tests have been performed for this patch? Have
> you tried with non SD1.0 cards and make sure that they still work fine after
> the patch?
>
We have tested this patch with SD1.0 cards and SD2.0, SD3.0 cards, all of them can work right.
Test items are application level:
Such as card identify, create file, delete file.
> Also, serveral more inline comments below:
>
>
> > -----Original Message-----
> > From: Chevron Li <chevron.li@bayhubtech.com>
> > Sent: Tuesday, December 6, 2022 2:48 PM
> > To: devel@edk2.groups.io
> > Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Wang,
> > Jian J <jian.j.wang@intel.com>; Gao, Liming
> > <gaoliming@byosoft.com.cn>; shirley.her@bayhubtech.com;
> > shaper.liu@bayhubtech.com; xiaoguang.yu@bayhubtech.com; Chevron Li
> > (WH) <chevron.li@bayhubtech.com>
> > Subject: [edk2-platform][PATCH 1/1] MdeModulePkg: SdMmcPciHcDxe:
> Fix
> > issue that SD1.0 cards can't be recognized
> >
> > From: "Chevron Li (WH)" <chevron.li@bayhubtech.com>
> >
> >
> > SD1.0 cards don't support CMD8 and CMD6
> >
> > CMD8 result can be used to distinguish the card is SD1.0 or not.
> >
> > CMD8 result can be used to decide following CMD6 is sent or skip.
> >
> >
> >
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> >
> > Cc: Ray Ni <ray.ni@intel.com>
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> >
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> >
> > Signed-off-by: Chevron Li <chevron.li@bayhubtech.com>
> >
> > ---
> >
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 30
> ++++++++++++---
> > ----
> >
> > 1 file changed, 20 insertions(+), 10 deletions(-)
> >
> >
> >
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> >
> > index f5a3607e47..281aa38170 100644
> >
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> >
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> >
> > @@ -1085,7 +1085,8 @@ SdCardSetBusMode (
> >
> > IN EFI_SD_MMC_PASS_THRU_PROTOCOL *PassThru,
> >
> > IN UINT8 Slot,
> >
> > IN UINT16 Rca,
> >
> > - IN BOOLEAN S18A
> >
> > + IN BOOLEAN S18A,
> >
> > + IN BOOLEAN SdVersion1
>
>
> For the newly added input parameter 'SdVersion1', please help to update
> the function description comments to reflect it.
>
>
> >
> > )
> >
> > {
> >
> > EFI_STATUS Status;
> >
> > @@ -1117,10 +1118,13 @@ SdCardSetBusMode (
> >
> >
> >
> > //
> >
> > // Get the supported bus speed from SWITCH cmd return data group #1.
> >
> > + // SdVersion1 don't support the SWITCH cmd
> >
> > //
> >
> > - Status = SdCardSwitch (PassThru, Slot, 0xFF, 0xF,
> > SdDriverStrengthIgnore, 0xF, FALSE, SwitchResp);
> >
> > - if (EFI_ERROR (Status)) {
> >
> > - return Status;
> >
> > + if(SdVersion1 == FALSE) {
> >
> > + Status = SdCardSwitch (PassThru, Slot, 0xFF, 0xF,
> > + SdDriverStrengthIgnore,
> > 0xF, FALSE, SwitchResp);
>
>
> The local variable 'SwitchResp' is an output parameter for SdCardSwitch().
> When SdVersion1 is TRUE, SdCardSwitch() call will be skipped and
> 'SwitchResp' will have uninitialized value.
> This will probably cause issues when 'SwitchResp' later being used as an input
> parameter for SdGetTargetBusMode() function call.
> Could you help to check on this?
>
>
> >
> > + if (EFI_ERROR (Status)) {
> >
> > + return Status;
> >
> > + }
> >
> > }
> >
> >
> >
> > SdGetTargetBusMode (Private, Slot, SwitchResp, S18A, &BusMode);
> >
> > @@ -1141,9 +1145,14 @@ SdCardSetBusMode (
> >
> > }
> >
> > }
> >
> >
> >
> > - Status = SdCardSwitch (PassThru, Slot, BusMode.BusTiming, 0xF,
> > BusMode.DriverStrength.Sd, 0xF, TRUE, SwitchResp);
> >
> > - if (EFI_ERROR (Status)) {
> >
> > - return Status;
> >
> > + //
> >
> > + // SdVersion1 don't support the SWITCH cmd
> >
> > + //
> >
> > + if(SdVersion1 == FALSE){
> >
> > + Status = SdCardSwitch (PassThru, Slot, BusMode.BusTiming, 0xF,
> > BusMode.DriverStrength.Sd, 0xF, TRUE, SwitchResp);
> >
> > + if (EFI_ERROR (Status)) {
> >
> > + return Status;
> >
> > + }
> >
> > }
> >
> >
> >
> > Status = SdMmcSetDriverStrength (Private->PciIo, Slot,
> > BusMode.DriverStrength.Sd);
> >
> > @@ -1214,6 +1223,7 @@ SdCardIdentification (
> >
> > UINT8 HostCtrl2;
> >
> > UINTN Retry;
> >
> > BOOLEAN ForceVoltage33;
> >
> > + BOOLEAN SdVersion1;
> >
> >
> >
> > ForceVoltage33 = FALSE;
>
>
> Please help to assign a initial value for the newly added local variable
> 'SdVersion1'.
> If the card supports cmd8, this local variable will have uninitialized value
> when calling SdCardSetBusMode().
>
>
> >
> >
> >
> > @@ -1231,12 +1241,12 @@ Voltage33Retry:
> >
> > }
> >
> >
> >
> > //
> >
> > - // 2. Send Cmd8 to the device
> >
> > + // 2. Send Cmd8 to the device, the command will fail for
> > + SdVersion1;
> >
> > //
> >
> > Status = SdCardVoltageCheck (PassThru, Slot, 0x1, 0xFF);
> >
> > if (EFI_ERROR (Status)) {
> >
> > + SdVersion1 = 1;
>
>
> Please use "SdVersion1 = TRUE;".
>
> Best Regards,
> Hao Wu
>
>
> >
> > DEBUG ((DEBUG_INFO, "SdCardIdentification: Executing Cmd8 fails
> > with %r\n", Status));
> >
> > - return Status;
> >
> > }
> >
> >
> >
> > //
> >
> > @@ -1426,7 +1436,7 @@ Voltage33Retry:
> >
> > DEBUG ((DEBUG_INFO, "SdCardIdentification: Found a SD device at
> > slot [%d]\n", Slot));
> >
> > Private->Slot[Slot].CardType = SdCardType;
> >
> >
> >
> > - Status = SdCardSetBusMode (PciIo, PassThru, Slot, Rca, ((Ocr &
> > BIT24) != 0));
> >
> > + Status = SdCardSetBusMode (PciIo, PassThru, Slot, Rca, ((Ocr &
> > + BIT24) != 0),
> > SdVersion1);
> >
> >
> >
> > return Status;
> >
> >
> >
> >
> >
> > base-commit: 7bee2498910a9034faaf90802c49188afb7582dc
> >
> > --
> >
> > 2.18.0.windows.1
> >
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-12-06 9:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-06 6:48 [edk2-platform][PATCH 1/1] MdeModulePkg: SdMmcPciHcDxe: Fix issue that SD1.0 cards can't be recognized Chevron Li
2022-12-06 8:28 ` Wu, Hao A
2022-12-06 9:51 ` 回复: " Chevron Li (WH)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox