From: "Wu, Hao A" <hao.a.wu@intel.com>
To: "Philippe Mathieu-Daudé" <philmd@redhat.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Subject: Re: [PATCHv2 1/1] MdeModulePkg/SdMmcPciHcDxe Fix eMMC HS400 switch sequence
Date: Fri, 22 Feb 2019 00:18:35 +0000 [thread overview]
Message-ID: <B80AF82E9BFB8E4FBD8C89DA810C6A093C89A099@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <b3de5d7b-634c-ff7b-0e83-2eb2481fe913@redhat.com>
> -----Original Message-----
> From: Philippe Mathieu-Daudé [mailto:philmd@redhat.com]
> Sent: Friday, February 22, 2019 4:57 AM
> To: Wu, Hao A; edk2-devel@lists.01.org
> Cc: Albecki, Mateusz
> Subject: Re: [edk2] [PATCHv2 1/1] MdeModulePkg/SdMmcPciHcDxe Fix
> eMMC HS400 switch sequence
>
> Hi Hao A,
>
> On 2/20/19 2:11 AM, Wu, Hao A wrote:
> > Thanks Mateusz,
> >
> > Reviewed-by: Hao Wu <hao.a.wu@intel.com>
> > And pushed via commit 68c67d3a2a33261e41ff0123129b4e9759617f71.
>
> I think you mean "pushed 'after' commit
> 68c67d3a2a33261e41ff0123129b4e9759617f71".
Yes, the commit should have been 195f673f6270aaf73dd34b75f1da26451b63c316.
It is my mistake.
>
> Commit 68c67d3a2a33261e41ff0123129b4e9759617f71 is:
> "BaseTools: Fixed a code bug for Pcd Array", while
> this patch is commited as 195f673f6270aaf73dd34b75f1da26451b63c316.
>
> Now about this commit 195f673f6270aaf, I think you have an issue with
> your configuration, as the patch author was remplaced from:
> Albecki, Mateusz <mateusz.albecki@intel.com>
> to:
> Albecki, Mateusz </o=Intel/ou=Exchange Administrative Group
> (FYDIBOHF23SPDLT)/cn=Recipients/cn=Albecki, Mateusz3be>
You are right, there is something wrong with my local settings when
abstracting the patch. Thanks for pointing out.
Best Regards,
Hao Wu
>
> Regards,
>
> Phil.
>
> >
> > Best Regards,
> > Hao Wu
> >
> >
> >> -----Original Message-----
> >> From: Albecki, Mateusz
> >> Sent: Monday, February 18, 2019 7:12 PM
> >> To: edk2-devel@lists.01.org
> >> Cc: Albecki, Mateusz; Wu, Hao A
> >> Subject: [PATCHv2 1/1] MdeModulePkg/SdMmcPciHcDxe Fix eMMC
> HS400
> >> switch sequence
> >>
> >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1140
> >>
> >> In eMMC HS400 switch sequence flow eMMC driver attempted
> >> to execute SEND_STATUS just after switching bus timing to high
> >> speed and before downgrading clock frequency to 52MHz. Since link
> >> was at that time in incorrect state SEND_STATUS was failing which
> >> made driver think switch to HS400 failed.
> >> This change makes driver always change clock frequency after
> >> switching bus timing and before executing SEND_STATUS.
> >>
> >> Cc: Hao Wu <hao.a.wu@intel.com>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Albecki Mateusz <mateusz.albecki@intel.com>
> >> ---
> >> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 39
> >> +++++++++++++------------
> >> 1 file changed, 20 insertions(+), 19 deletions(-)
> >>
> >> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> >> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> >> index 4ef849fd0962..15db8a87a5c4 100644
> >> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> >> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> >> @@ -642,7 +642,7 @@ EmmcSwitchBusWidth (
> >> }
> >>
> >> /**
> >> - Switch the clock frequency to the specified value.
> >> + Switch the bus timing and clock frequency.
> >>
> >> Refer to EMMC Electrical Standard Spec 5.1 Section 6.6 and SD Host
> >> Controller
> >> Simplified Spec 3.0 Figure 3-3 for details.
> >> @@ -660,7 +660,7 @@ EmmcSwitchBusWidth (
> >>
> >> **/
> >> EFI_STATUS
> >> -EmmcSwitchClockFreq (
> >> +EmmcSwitchBusTiming (
> >> IN EFI_PCI_IO_PROTOCOL *PciIo,
> >> IN EFI_SD_MMC_PASS_THRU_PROTOCOL *PassThru,
> >> IN UINT8 Slot,
> >> @@ -689,22 +689,10 @@ EmmcSwitchClockFreq (
> >>
> >> Status = EmmcSwitch (PassThru, Slot, Access, Index, Value, CmdSet);
> >> if (EFI_ERROR (Status)) {
> >> - DEBUG ((DEBUG_ERROR, "EmmcSwitchClockFreq: Switch to
> hstiming %d
> >> fails with %r\n", HsTiming, Status));
> >> + DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: Switch to
> hstiming %d
> >> fails with %r\n", HsTiming, Status));
> >> return Status;
> >> }
> >>
> >> - Status = EmmcSendStatus (PassThru, Slot, Rca, &DevStatus);
> >> - if (EFI_ERROR (Status)) {
> >> - DEBUG ((DEBUG_ERROR, "EmmcSwitchClockFreq: Send status fails
> >> with %r\n", Status));
> >> - return Status;
> >> - }
> >> - //
> >> - // Check the switch operation is really successful or not.
> >> - //
> >> - if ((DevStatus & BIT7) != 0) {
> >> - DEBUG ((DEBUG_ERROR, "EmmcSwitchClockFreq: The switch
> operation
> >> fails as DevStatus is 0x%08x\n", DevStatus));
> >> - return EFI_DEVICE_ERROR;
> >> - }
> >> //
> >> // Convert the clock freq unit from MHz to KHz.
> >> //
> >> @@ -713,6 +701,19 @@ EmmcSwitchClockFreq (
> >> return Status;
> >> }
> >>
> >> + Status = EmmcSendStatus (PassThru, Slot, Rca, &DevStatus);
> >> + if (EFI_ERROR (Status)) {
> >> + DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: Send status fails
> >> with %r\n", Status));
> >> + return Status;
> >> + }
> >> + //
> >> + // Check the switch operation is really successful or not.
> >> + //
> >> + if ((DevStatus & BIT7) != 0) {
> >> + DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: The switch
> operation
> >> fails as DevStatus is 0x%08x\n", DevStatus));
> >> + return EFI_DEVICE_ERROR;
> >> + }
> >> +
> >> if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> >> Status = mOverride->NotifyPhase (
> >> Private->ControllerHandle,
> >> @@ -799,7 +800,7 @@ EmmcSwitchToHighSpeed (
> >> }
> >>
> >> HsTiming = 1;
> >> - Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming,
> Timing,
> >> ClockFreq);
> >> + Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming,
> >> Timing, ClockFreq);
> >>
> >> return Status;
> >> }
> >> @@ -887,7 +888,7 @@ EmmcSwitchToHS200 (
> >> Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_CLOCK_CTRL,
> sizeof
> >> (ClockCtrl), &ClockCtrl);
> >>
> >> HsTiming = 2;
> >> - Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming,
> Timing,
> >> ClockFreq);
> >> + Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming,
> >> Timing, ClockFreq);
> >> if (EFI_ERROR (Status)) {
> >> return Status;
> >> }
> >> @@ -937,7 +938,7 @@ EmmcSwitchToHS400 (
> >> // Set to Hight Speed timing and set the clock frequency to a value less
> than
> >> 52MHz.
> >> //
> >> HsTiming = 1;
> >> - Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming,
> >> SdMmcMmcHsSdr, 52);
> >> + Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming,
> >> SdMmcMmcHsSdr, 52);
> >> if (EFI_ERROR (Status)) {
> >> return Status;
> >> }
> >> @@ -957,7 +958,7 @@ EmmcSwitchToHS400 (
> >> }
> >>
> >> HsTiming = 3;
> >> - Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming,
> Timing,
> >> ClockFreq);
> >> + Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming,
> >> Timing, ClockFreq);
> >>
> >> return Status;
> >> }
> >> --
> >> 2.14.1.windows.1
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> >
prev parent reply other threads:[~2019-02-22 0:18 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <cover.1550487993.git.mateusz.albecki@intel.com>
2019-02-18 11:11 ` [PATCHv2 1/1] MdeModulePkg/SdMmcPciHcDxe Fix eMMC HS400 switch sequence Albecki, Mateusz
2019-02-20 1:11 ` Wu, Hao A
2019-02-21 20:57 ` Philippe Mathieu-Daudé
2019-02-22 0:18 ` Wu, Hao A [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=B80AF82E9BFB8E4FBD8C89DA810C6A093C89A099@SHSMSX104.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox