Hello Marcin,

Thanks for the verification. The series has been pushed via commits 
a37e18f6fc..adec1f5deb.
Please help to raise if there is any help needed.

Best Regards,
Hao Wu

From: Marcin Wojtas [mailto:m...@semihalf.com]
Sent: Friday, June 28, 2019 4:58 PM
To: Albecki, Mateusz
Cc: devel@edk2.groups.io; Wu, Hao A; Sumit Garg; Ard Biesheuvel
Subject: Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement 
revision 3 of SdMmcOverrideProtocol

Hi Mateusz,



pt., 28 cze 2019 o 10:12 Albecki, Mateusz 
<mateusz.albe...@intel.com<mailto:mateusz.albe...@intel.com>> napisał(a):
Hi,

Do you use override driver with this SD controller(if yes and it is open source 
could you provide the link)?

[MW] Of course it's open source 
https://github.com/tianocore/edk2-platforms/tree/master/Silicon/Marvell/Drivers/SdMmc/XenonDxe

The platform is Armada70x0Db.dsc from the same edk2-platforms repo.


There is one change introduced in this patch that might require changes in the 
override driver. We have added enumeration for SdMmcSdDs and SdMmcSdHs modes 
which were so far indicated to override drivers as SdMmcUhsSdr12 and 
SdMmcUhsSdr25 respectively so if there were any actions that were necessary for 
high speed to work and those were done only for SdMmcUhsSdr25 then that might 
be the reason for regression.


[MW] Now, that was the reason. This interface required special handling for 
high speed. This patch fixed it:
https://pastebin.com/rdRe9wAh

I will submit it after your patchset is merged.

Best regards,
Marcin

Thanks,
Mateusz

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> 
[mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io>] On Behalf Of Marcin 
Wojtas
Sent: Friday, June 28, 2019 9:42 AM
To: Wu, Hao A <hao.a...@intel.com<mailto:hao.a...@intel.com>>
Cc: Albecki, Mateusz 
<mateusz.albe...@intel.com<mailto:mateusz.albe...@intel.com>>; Sumit Garg 
<sumit.g...@linaro.org<mailto:sumit.g...@linaro.org>>; Ard Biesheuvel 
<ard.biesheu...@linaro.org<mailto:ard.biesheu...@linaro.org>>; 
edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
Subject: Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement 
revision 3 of SdMmcOverrideProtocol

Hi Hao,

pt., 28 cze 2019 o 09:23 Wu, Hao A 
<hao.a...@intel.com<mailto:hao.a...@intel.com>> napisał(a):
Hello Marcin,

Do you mean by only reverting as below:
-  SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((CommandSystem & 0xF) << 
4) | \
-                                ((DriverStrength & 0xF) << 8) | ((PowerLimit & 
0xF) << 12) | \
+  SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((PowerLimit & 0xF) << 4) 
| \^M
+                                ((DriverStrength & 0xF) << 8) | 
((DriverStrength & 0xF) << 12) | \^M
                                 ModeValue;

All your devices work fine?

Since the origin code is clearly not correct (DriveStrength used 2 times,
the offset of PowerLimit is not 4, should be 12 according to SD spec).

Ok, just rechecked. It doesn't help for my 1 problematic case. Anyway for the 
next time I think it may be worth to split some improvements out of such big 
patches.

I won't be able to debug my board until second week of July (at best), so in 
order not to block you please go ahead with merging (the most important board 
(MacchiatoBin) seems not suffer any regression).

Best regards,
Marcin


Best Regards,
Hao Wu

From: Marcin Wojtas [mailto:m...@semihalf.com<mailto:m...@semihalf.com>]
Sent: Friday, June 28, 2019 2:33 PM
To: Wu, Hao A; Albecki, Mateusz
Cc: Sumit Garg; Ard Biesheuvel; edk2-devel-groups-io
Subject: Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement 
revision 3 of SdMmcOverrideProtocol

Hi,

I was almost happily sending you email with 'tested-by' information, but I 
checked 3 boards:
Board 1 (out of tree): SD - OK, MMC - OK
Board 2: (Armada80x0McBin): SD - OK, MMC - OK
Board 3: (Armada70x0Db): SD - problems, MMC - OK

In the latter case I get stall and booting takes around 3 minutes.
Without "MdeModulePkg/SdMmcHcDxe: Implement revision 3 of 
SdMmcOverrideProtocol" patch it works as before.

I enabled debugs, and in theory everything seems fine, the DriverStrength is 
set to EDKII_SD_MMC_DRIVER_STRENGTH_IGNORE.
SdCardIdentification: Found a SD device at slot [0]
SdCardSetBusMode: Target bus mode: bus timing = 1, bus width = 4, clock 
freq[MHz] = 50, driver strength = 255

However right after Csd register dump the booting stalls until printing 
following and continuing:
FatOpenDevice: read of part_lba failed Time out

This is absent from the prints I dumped from vanilla kernel. Despite I 
currently really have no time to additional debug, I checked and with following 
diff, everything works as before:

--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
@@ -536,8 +536,8 @@ SdCardSwitch (
       AccessMode = 0xF;
   }

-  SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((CommandSystem & 0xF) << 
4) | \
-                                ((DriverStrength & 0xF) << 8) | ((PowerLimit & 
0xF) << 12) | \
+  SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((PowerLimit & 0xF) << 4) 
| \^M
+                                ((DriverStrength & 0xF) << 8) | 
((DriverStrength & 0xF) << 12) | \^M
                                 ModeValue;

Above is restoring SdMmcCmdBlk.CommandArgument to the state from before the 
patch in question. Now the question - why the layout of the command changed? 
CommandSystem was unused before, and PowerLimit changed its position. Is this 
change really related to the rest of the patch? What is the justification?

Best regards,
Marcin


pt., 28 cze 2019 o 02:57 Wu, Hao A 
<hao.a...@intel.com<mailto:hao.a...@intel.com>> napisał(a):
> -----Original Message-----
> From: Sumit Garg [mailto:sumit.g...@linaro.org<mailto:sumit.g...@linaro.org>]
> Sent: Thursday, June 27, 2019 9:39 PM
> To: Ard Biesheuvel
> Cc: edk2-devel-groups-io; Wu, Hao A; Marcin Wojtas; Albecki, Mateusz
> Subject: Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe:
> Implement revision 3 of SdMmcOverrideProtocol
>
> On Thu, 27 Jun 2019 at 13:40, Ard Biesheuvel 
> <ard.biesheu...@linaro.org<mailto:ard.biesheu...@linaro.org>>
> wrote:
> >
> > (+ Sumit)
> >
> > On Thu, 27 Jun 2019 at 08:29, Wu, Hao A 
> > <hao.a...@intel.com<mailto:hao.a...@intel.com>> wrote:
> > >
> > > > -----Original Message-----
> > > > From: Marcin Wojtas [mailto:m...@semihalf.com<mailto:m...@semihalf.com>]
> > > > Sent: Thursday, June 27, 2019 2:25 PM
> > > > To: Albecki, Mateusz
> > > > Cc: edk2-devel-groups-io; Wu, Hao A
> > > > Subject: Re: [edk2-devel] [PATCH v4 0/2]
> MdeModulePkg/SdMmcHcDxe:
> > > > Implement revision 3 of SdMmcOverrideProtocol
> > > >
> > > > Hi Mateusz,
> > > >
> > > > Can you please push those patches somewhere (github?) so that I can
> > > > easily do a quick check for regression?
> > > >
> > > > Thanks,
> > > > Marcin
> > >
> > >
> > > Hello Marcin,
> > >
> > > I have just pushed the series at:
> > > https://github.com/hwu25/edk2/tree/sdmmc_override_extend_v4
> > >
> > > Please help to check.
> > >
> >
> > I have cc'ed my colleague Sumit, who has kindly agreed to regression
> > test this branch on Socionext SynQuacer, which also uses the SD/MMC
> > override infrastructure.
> >
> > Sumit, please reply here with your results. And thanks again!
>
> I did picked this patch-set and applied on top of edk2 master branch.
> It works well on SynQuacer with eMMC device enumerated properly and
> all three eMMC partitions are visible:
>
>      BLK4: Alias(s):
>           VenHw(0D51905B-B77E-452A-A2C0-
> ECA0CC8D514A,000030520000000000)/eMMC(0x
> 0)/Ctrl(0x0)
>      BLK5: Alias(s):
>           VenHw(0D51905B-B77E-452A-A2C0-
> ECA0CC8D514A,000030520000000000)/eMMC(0x
> 0)/Ctrl(0x1)
>      BLK6: Alias(s):
>           VenHw(0D51905B-B77E-452A-A2C0-
> ECA0CC8D514A,000030520000000000)/eMMC(0x
> 0)/Ctrl(0x2)
>
> Shell> devices
> <snip>
>   E9 D - -  1  1   0 VenHw(0D51905B-B77E-452A-A2C0-
> ECA0CC8D514A,0000305200000000
> 00)/eMMC(0x0)/Ctrl(0x0)
>   EA D - -  1  1   0 VenHw(0D51905B-B77E-452A-A2C0-
> ECA0CC8D514A,0000305200000000
> 00)/eMMC(0x0)/Ctrl(0x1)
>   EB D - -  1  1   0 VenHw(0D51905B-B77E-452A-A2C0-
> ECA0CC8D514A,0000305200000000
> 00)/eMMC(0x0)/Ctrl(0x2)
>
> So you can add following:
>
> Regression-tested-by: Sumit Garg 
> <sumit.g...@linaro.org<mailto:sumit.g...@linaro.org>>


Thanks a lot for the testing.

Best Regards,
Hao Wu


>
> -Sumit


---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. S&#322owackiego 173 | 80-298 Gda&#324sk | S&#261d Rejonowy Gda&#324sk 
P&#243&#322noc | VII Wydzia&#322 Gospodarczy Krajowego Rejestru S&#261dowego - 
KRS 101882 | NIP 957-07-52-316 | Kapita&#322 zak&#322adowy 200.000 PLN.

Ta wiadomo&#347&#263 wraz z za&#322&#261cznikami jest przeznaczona dla 
okre&#347lonego adresata i mo&#380e zawiera&#263 informacje poufne. W razie 
przypadkowego otrzymania tej wiadomo&#347ci, prosimy o powiadomienie nadawcy 
oraz trwa&#322e jej usuni&#281cie; jakiekolwiek przegl&#261danie 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 (#43052): https://edk2.groups.io/g/devel/message/43052
Mute This Topic: https://groups.io/mt/32214570/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to