MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol

[edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol

Posted by Albecki, Mateusz 6 years, 7 months ago

To allow platform greater control over the bus settings for SD card and eMMC card we have added a new notify phase to SdMmcOverrideProtocol called GetOperatingParam.
This phase is signaled before SD card/eMMC initialization and allows platform to tweak the values in new structure called EDKII_SD_MMC_OPERATING_PARAMETERS which allows to configure bus width, clock frequency and driver strength.
Other bus parameters can be configured by overriding host controller capabilities.

Changes in v2:
- Fixed stylistic issues and documentation issues pointed out in v1 review
- Changed bus width to be UINT8
- Reordered the SD_MMC_BUS_MODE to fix problem with driver never choosing the DDR50 speed mode
- Fixed the bug with driver not switching the controller into correct driver strength for SD card slots
- Fixed bug with the driver not considering driver strength support for SD card slots
- Fixed bug with driver choosing SdMmcMmcHsSdr if card doesn't support frequencies above 26MHz
- Fixed bug with driver choosing driver strength for legacy speed modes

Changes in v3:
- Changed BusWidth field of EDKII_SD_MMC_OPERATING_PARAMETERS to UINT16

Changes in v4
- Fixed typos and ordering in SD_MMC_BUS_MODE(this time for real)
- Fixed non boolean types usage in if statements
- Fixed code that checks if there was an error in switch response for SD card. The code has been updated to check if switch failed due to function being busy

Tests:
- OS boot from eMMC without SdMmcOverride protocol installed
- OS boot from eMMC with SdMmcOverride installed and clock frequency lowered to 100MHz in HS200
- OS boot from eMMC with driver strength changed to Type1
- OS boot from eMMC in HS400 without override protocol installed
- SD card enumeration in UEFI shell on default speed and high speed(non UHS-I) with SdMmcOverride installed and UHS-I disabled in capability
- SD card enumeration in UEFI shell on default speed and high speed(non UHS-I) with no Override protocol(legacy card used)


Cc: Hao A Wu <hao.a.wu@intel.com>


Albecki, Mateusz (1):
  MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol

Mateusz Albecki (1):
  MdeModulePkg/SdMmcOverride: Add GetOperatingParam notify phase

 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c    | 512 +++++++++++++++------
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c      | 410 ++++++++++++++---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c |  52 ++-
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |  18 +-
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   |  34 ++
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h   |  19 +
 MdeModulePkg/Include/Protocol/SdMmcOverride.h      |  60 ++-
 7 files changed, 867 insertions(+), 238 deletions(-)

-- 
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 (#42895): https://edk2.groups.io/g/devel/message/42895
Mute This Topic: https://groups.io/mt/32214570/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol

Posted by Wu, Hao A 6 years, 7 months ago

Except a minor comment in patch 2/2, the series looks good to me.

Hello Ard and Marcin, please let me know if you have comments on this
series.

If no other feedbacks, I plan to push the change at early next week.

Best Regards,
Hao Wu


> -----Original Message-----
> From: Albecki, Mateusz
> Sent: Wednesday, June 26, 2019 9:10 PM
> To: devel@edk2.groups.io
> Cc: Albecki, Mateusz; Wu, Hao A
> Subject: [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision
> 3 of SdMmcOverrideProtocol
> 
> To allow platform greater control over the bus settings for SD card and
> eMMC card we have added a new notify phase to SdMmcOverrideProtocol
> called GetOperatingParam.
> This phase is signaled before SD card/eMMC initialization and allows platform
> to tweak the values in new structure called
> EDKII_SD_MMC_OPERATING_PARAMETERS which allows to configure bus
> width, clock frequency and driver strength.
> Other bus parameters can be configured by overriding host controller
> capabilities.
> 
> Changes in v2:
> - Fixed stylistic issues and documentation issues pointed out in v1 review
> - Changed bus width to be UINT8
> - Reordered the SD_MMC_BUS_MODE to fix problem with driver never
> choosing the DDR50 speed mode
> - Fixed the bug with driver not switching the controller into correct driver
> strength for SD card slots
> - Fixed bug with the driver not considering driver strength support for SD
> card slots
> - Fixed bug with driver choosing SdMmcMmcHsSdr if card doesn't support
> frequencies above 26MHz
> - Fixed bug with driver choosing driver strength for legacy speed modes
> 
> Changes in v3:
> - Changed BusWidth field of EDKII_SD_MMC_OPERATING_PARAMETERS to
> UINT16
> 
> Changes in v4
> - Fixed typos and ordering in SD_MMC_BUS_MODE(this time for real)
> - Fixed non boolean types usage in if statements
> - Fixed code that checks if there was an error in switch response for SD card.
> The code has been updated to check if switch failed due to function being
> busy
> 
> Tests:
> - OS boot from eMMC without SdMmcOverride protocol installed
> - OS boot from eMMC with SdMmcOverride installed and clock frequency
> lowered to 100MHz in HS200
> - OS boot from eMMC with driver strength changed to Type1
> - OS boot from eMMC in HS400 without override protocol installed
> - SD card enumeration in UEFI shell on default speed and high speed(non
> UHS-I) with SdMmcOverride installed and UHS-I disabled in capability
> - SD card enumeration in UEFI shell on default speed and high speed(non
> UHS-I) with no Override protocol(legacy card used)
> 
> 
> Cc: Hao A Wu <hao.a.wu@intel.com>
> 
> 
> Albecki, Mateusz (1):
>   MdeModulePkg/SdMmcHcDxe: Implement revision 3 of
> SdMmcOverrideProtocol
> 
> Mateusz Albecki (1):
>   MdeModulePkg/SdMmcOverride: Add GetOperatingParam notify phase
> 
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c    | 512
> +++++++++++++++------
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c      | 410
> ++++++++++++++---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c |  52 ++-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |  18 +-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   |  34 ++
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h   |  19 +
>  MdeModulePkg/Include/Protocol/SdMmcOverride.h      |  60 ++-
>  7 files changed, 867 insertions(+), 238 deletions(-)
> 
> --
> 2.14.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#42914): https://edk2.groups.io/g/devel/message/42914
Mute This Topic: https://groups.io/mt/32214570/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol

Posted by Marcin Wojtas 6 years, 7 months ago

Hi Mateusz,

Can you please push those patches somewhere (github?) so that I can
easily do a quick check for regression?

Thanks,
Marcin

śr., 26 cze 2019 o 15:10 Albecki, Mateusz <mateusz.albecki@intel.com>
napisał(a):
>
> To allow platform greater control over the bus settings for SD card and eMMC card we have added a new notify phase to SdMmcOverrideProtocol called GetOperatingParam.
> This phase is signaled before SD card/eMMC initialization and allows platform to tweak the values in new structure called EDKII_SD_MMC_OPERATING_PARAMETERS which allows to configure bus width, clock frequency and driver strength.
> Other bus parameters can be configured by overriding host controller capabilities.
>
> Changes in v2:
> - Fixed stylistic issues and documentation issues pointed out in v1 review
> - Changed bus width to be UINT8
> - Reordered the SD_MMC_BUS_MODE to fix problem with driver never choosing the DDR50 speed mode
> - Fixed the bug with driver not switching the controller into correct driver strength for SD card slots
> - Fixed bug with the driver not considering driver strength support for SD card slots
> - Fixed bug with driver choosing SdMmcMmcHsSdr if card doesn't support frequencies above 26MHz
> - Fixed bug with driver choosing driver strength for legacy speed modes
>
> Changes in v3:
> - Changed BusWidth field of EDKII_SD_MMC_OPERATING_PARAMETERS to UINT16
>
> Changes in v4
> - Fixed typos and ordering in SD_MMC_BUS_MODE(this time for real)
> - Fixed non boolean types usage in if statements
> - Fixed code that checks if there was an error in switch response for SD card. The code has been updated to check if switch failed due to function being busy
>
> Tests:
> - OS boot from eMMC without SdMmcOverride protocol installed
> - OS boot from eMMC with SdMmcOverride installed and clock frequency lowered to 100MHz in HS200
> - OS boot from eMMC with driver strength changed to Type1
> - OS boot from eMMC in HS400 without override protocol installed
> - SD card enumeration in UEFI shell on default speed and high speed(non UHS-I) with SdMmcOverride installed and UHS-I disabled in capability
> - SD card enumeration in UEFI shell on default speed and high speed(non UHS-I) with no Override protocol(legacy card used)
>
>
> Cc: Hao A Wu <hao.a.wu@intel.com>
>
>
> Albecki, Mateusz (1):
>   MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol
>
> Mateusz Albecki (1):
>   MdeModulePkg/SdMmcOverride: Add GetOperatingParam notify phase
>
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c    | 512 +++++++++++++++------
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c      | 410 ++++++++++++++---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c |  52 ++-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |  18 +-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   |  34 ++
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h   |  19 +
>  MdeModulePkg/Include/Protocol/SdMmcOverride.h      |  60 ++-
>  7 files changed, 867 insertions(+), 238 deletions(-)
>
> --
> 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 (#42920): https://edk2.groups.io/g/devel/message/42920
Mute This Topic: https://groups.io/mt/32214570/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol

Posted by Wu, Hao A 6 years, 7 months ago

> -----Original Message-----
> From: Marcin Wojtas [mailto:mw@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.

Best Regards,
Hao Wu


> 
> śr., 26 cze 2019 o 15:10 Albecki, Mateusz <mateusz.albecki@intel.com>
> napisał(a):
> >
> > To allow platform greater control over the bus settings for SD card and
> eMMC card we have added a new notify phase to SdMmcOverrideProtocol
> called GetOperatingParam.
> > This phase is signaled before SD card/eMMC initialization and allows
> platform to tweak the values in new structure called
> EDKII_SD_MMC_OPERATING_PARAMETERS which allows to configure bus
> width, clock frequency and driver strength.
> > Other bus parameters can be configured by overriding host controller
> capabilities.
> >
> > Changes in v2:
> > - Fixed stylistic issues and documentation issues pointed out in v1 review
> > - Changed bus width to be UINT8
> > - Reordered the SD_MMC_BUS_MODE to fix problem with driver never
> choosing the DDR50 speed mode
> > - Fixed the bug with driver not switching the controller into correct driver
> strength for SD card slots
> > - Fixed bug with the driver not considering driver strength support for SD
> card slots
> > - Fixed bug with driver choosing SdMmcMmcHsSdr if card doesn't support
> frequencies above 26MHz
> > - Fixed bug with driver choosing driver strength for legacy speed modes
> >
> > Changes in v3:
> > - Changed BusWidth field of EDKII_SD_MMC_OPERATING_PARAMETERS to
> UINT16
> >
> > Changes in v4
> > - Fixed typos and ordering in SD_MMC_BUS_MODE(this time for real)
> > - Fixed non boolean types usage in if statements
> > - Fixed code that checks if there was an error in switch response for SD card.
> The code has been updated to check if switch failed due to function being
> busy
> >
> > Tests:
> > - OS boot from eMMC without SdMmcOverride protocol installed
> > - OS boot from eMMC with SdMmcOverride installed and clock frequency
> lowered to 100MHz in HS200
> > - OS boot from eMMC with driver strength changed to Type1
> > - OS boot from eMMC in HS400 without override protocol installed
> > - SD card enumeration in UEFI shell on default speed and high speed(non
> UHS-I) with SdMmcOverride installed and UHS-I disabled in capability
> > - SD card enumeration in UEFI shell on default speed and high speed(non
> UHS-I) with no Override protocol(legacy card used)
> >
> >
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> >
> >
> > Albecki, Mateusz (1):
> >   MdeModulePkg/SdMmcHcDxe: Implement revision 3 of
> SdMmcOverrideProtocol
> >
> > Mateusz Albecki (1):
> >   MdeModulePkg/SdMmcOverride: Add GetOperatingParam notify phase
> >
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c    | 512
> +++++++++++++++------
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c      | 410
> ++++++++++++++---
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c |  52 ++-
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |  18 +-
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   |  34 ++
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h   |  19 +
> >  MdeModulePkg/Include/Protocol/SdMmcOverride.h      |  60 ++-
> >  7 files changed, 867 insertions(+), 238 deletions(-)
> >
> > --
> > 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 (#42921): https://edk2.groups.io/g/devel/message/42921
Mute This Topic: https://groups.io/mt/32214570/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol

Posted by Ard Biesheuvel 6 years, 7 months ago

(+ Sumit)

On Thu, 27 Jun 2019 at 08:29, Wu, Hao A <hao.a.wu@intel.com> wrote:
>
> > -----Original Message-----
> > From: Marcin Wojtas [mailto:mw@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!

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#42924): https://edk2.groups.io/g/devel/message/42924
Mute This Topic: https://groups.io/mt/32214570/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol

Posted by Sumit Garg 6 years, 7 months ago

On Thu, 27 Jun 2019 at 13:40, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> (+ Sumit)
>
> On Thu, 27 Jun 2019 at 08:29, Wu, Hao A <hao.a.wu@intel.com> wrote:
> >
> > > -----Original Message-----
> > > From: Marcin Wojtas [mailto:mw@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.garg@linaro.org>

-Sumit

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#42937): https://edk2.groups.io/g/devel/message/42937
Mute This Topic: https://groups.io/mt/32214570/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol

Posted by Wu, Hao A 6 years, 7 months ago

> -----Original Message-----
> From: Sumit Garg [mailto:sumit.garg@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.biesheuvel@linaro.org>
> wrote:
> >
> > (+ Sumit)
> >
> > On Thu, 27 Jun 2019 at 08:29, Wu, Hao A <hao.a.wu@intel.com> wrote:
> > >
> > > > -----Original Message-----
> > > > From: Marcin Wojtas [mailto:mw@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.garg@linaro.org>


Thanks a lot for the testing.

Best Regards,
Hao Wu


> 
> -Sumit

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#42951): https://edk2.groups.io/g/devel/message/42951
Mute This Topic: https://groups.io/mt/32214570/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol

Posted by Marcin Wojtas 6 years, 7 months ago

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.wu@intel.com> napisał(a):

> > -----Original Message-----
> > From: Sumit Garg [mailto:sumit.garg@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.biesheuvel@linaro.org>
> > wrote:
> > >
> > > (+ Sumit)
> > >
> > > On Thu, 27 Jun 2019 at 08:29, Wu, Hao A <hao.a.wu@intel.com> wrote:
> > > >
> > > > > -----Original Message-----
> > > > > From: Marcin Wojtas [mailto:mw@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.garg@linaro.org>
>
>
> Thanks a lot for the testing.
>
> Best Regards,
> Hao Wu
>
>
> >
> > -Sumit
>
> 
>
>

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#42974): https://edk2.groups.io/g/devel/message/42974
Mute This Topic: https://groups.io/mt/32214570/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol

Posted by Marcin Wojtas 6 years, 7 months ago

pt., 28 cze 2019 o 08:32 Marcin Wojtas <mw@semihalf.com> napisał(a):
>
> 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:

vanilla edk2 of course :)

>
> --- 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.wu@intel.com> napisał(a):
>>
>> > -----Original Message-----
>> > From: Sumit Garg [mailto:sumit.garg@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.biesheuvel@linaro.org>
>> > wrote:
>> > >
>> > > (+ Sumit)
>> > >
>> > > On Thu, 27 Jun 2019 at 08:29, Wu, Hao A <hao.a.wu@intel.com> wrote:
>> > > >
>> > > > > -----Original Message-----
>> > > > > From: Marcin Wojtas [mailto:mw@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.garg@linaro.org>
>>
>>
>> Thanks a lot for the testing.
>>
>> Best Regards,
>> Hao Wu
>>
>>
>> >
>> > -Sumit
>>
>> 
>>

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#42975): https://edk2.groups.io/g/devel/message/42975
Mute This Topic: https://groups.io/mt/32214570/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol

Posted by Wu, Hao A 6 years, 7 months ago

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).

Best Regards,
Hao Wu

From: Marcin Wojtas [mailto:mw@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.wu@intel.com<mailto:hao.a.wu@intel.com>> napisał(a):
> -----Original Message-----
> From: Sumit Garg [mailto:sumit.garg@linaro.org<mailto:sumit.garg@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.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>>
> wrote:
> >
> > (+ Sumit)
> >
> > On Thu, 27 Jun 2019 at 08:29, Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>> wrote:
> > >
> > > > -----Original Message-----
> > > > From: Marcin Wojtas [mailto:mw@semihalf.com<mailto:mw@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.garg@linaro.org<mailto:sumit.garg@linaro.org>>


Thanks a lot for the testing.

Best Regards,
Hao Wu


>
> -Sumit



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#42986): https://edk2.groups.io/g/devel/message/42986
Mute This Topic: https://groups.io/mt/32214570/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol

Posted by Marcin Wojtas 6 years, 7 months ago

Hi Hao,


pt., 28 cze 2019 o 09:23 Wu, Hao A <hao.a.wu@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:mw@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.wu@intel.com> napisał(a):
>
> > -----Original Message-----
> > From: Sumit Garg [mailto:sumit.garg@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.biesheuvel@linaro.org>
> > wrote:
> > >
> > > (+ Sumit)
> > >
> > > On Thu, 27 Jun 2019 at 08:29, Wu, Hao A <hao.a.wu@intel.com> wrote:
> > > >
> > > > > -----Original Message-----
> > > > > From: Marcin Wojtas [mailto:mw@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.garg@linaro.org>
>
>
> Thanks a lot for the testing.
>
> Best Regards,
> Hao Wu
>
>
> >
> > -Sumit
>
> 
>
>

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#42989): https://edk2.groups.io/g/devel/message/42989
Mute This Topic: https://groups.io/mt/32214570/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol

Posted by Albecki, Mateusz 6 years, 7 months ago

Hi,

Do you use override driver with this SD controller(if yes and it is open source could you provide the link)? 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.

Thanks,
Mateusz

From: 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.wu@intel.com>
Cc: Albecki, Mateusz <mateusz.albecki@intel.com>; Sumit Garg <sumit.garg@linaro.org>; Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel-groups-io <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.wu@intel.com<mailto:hao.a.wu@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:mw@semihalf.com<mailto:mw@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.wu@intel.com<mailto:hao.a.wu@intel.com>> napisał(a):
> -----Original Message-----
> From: Sumit Garg [mailto:sumit.garg@linaro.org<mailto:sumit.garg@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.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>>
> wrote:
> >
> > (+ Sumit)
> >
> > On Thu, 27 Jun 2019 at 08:29, Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>> wrote:
> > >
> > > > -----Original Message-----
> > > > From: Marcin Wojtas [mailto:mw@semihalf.com<mailto:mw@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.garg@linaro.org<mailto:sumit.garg@linaro.org>>

Thanks a lot for the testing.

Best Regards,
Hao Wu

>
> -Sumit

--------------------------------------------------------------------

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 (#42994): https://edk2.groups.io/g/devel/message/42994
Mute This Topic: https://groups.io/mt/32214570/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol

Posted by Marcin Wojtas 6 years, 7 months ago

Hi Mateusz,



pt., 28 cze 2019 o 10:12 Albecki, Mateusz <mateusz.albecki@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] *On Behalf Of *Marcin
> Wojtas
> *Sent:* Friday, June 28, 2019 9:42 AM
> *To:* Wu, Hao A <hao.a.wu@intel.com>
> *Cc:* Albecki, Mateusz <mateusz.albecki@intel.com>; Sumit Garg <
> sumit.garg@linaro.org>; Ard Biesheuvel <ard.biesheuvel@linaro.org>;
> edk2-devel-groups-io <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.wu@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:mw@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.wu@intel.com> napisał(a):
>
> > -----Original Message-----
> > From: Sumit Garg [mailto:sumit.garg@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.biesheuvel@linaro.org>
> > wrote:
> > >
> > > (+ Sumit)
> > >
> > > On Thu, 27 Jun 2019 at 08:29, Wu, Hao A <hao.a.wu@intel.com> wrote:
> > > >
> > > > > -----Original Message-----
> > > > > From: Marcin Wojtas [mailto:mw@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.garg@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 (#42999): https://edk2.groups.io/g/devel/message/42999
Mute This Topic: https://groups.io/mt/32214570/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol

Posted by Wu, Hao A 6 years, 7 months ago

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:mw@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.albecki@intel.com<mailto:mateusz.albecki@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.wu@intel.com<mailto:hao.a.wu@intel.com>>
Cc: Albecki, Mateusz <mateusz.albecki@intel.com<mailto:mateusz.albecki@intel.com>>; Sumit Garg <sumit.garg@linaro.org<mailto:sumit.garg@linaro.org>>; Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@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.wu@intel.com<mailto:hao.a.wu@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:mw@semihalf.com<mailto:mw@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.wu@intel.com<mailto:hao.a.wu@intel.com>> napisał(a):
> -----Original Message-----
> From: Sumit Garg [mailto:sumit.garg@linaro.org<mailto:sumit.garg@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.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>>
> wrote:
> >
> > (+ Sumit)
> >
> > On Thu, 27 Jun 2019 at 08:29, Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>> wrote:
> > >
> > > > -----Original Message-----
> > > > From: Marcin Wojtas [mailto:mw@semihalf.com<mailto:mw@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.garg@linaro.org<mailto:sumit.garg@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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol

Posted by Wu, Hao A 6 years, 7 months ago

Hello Marcin,

I just realized that the series added 2 new bus modes (SdMmcSdDs and SdMmcSdHs) for 3.3V signaling SD card:

Judging from your log:
SdCardSetBusMode: Target bus mode: bus timing = 1, bus width = 4, clock freq[MHz] = 50, driver strength = 255
which “bus timing = 1” means SdMmcSdHs (3.3V High Speed mode) is selected by the SdMmcPciHcDxe driver.

So I have attached a patch to handle the 2 added bus modes in Silicon/Marvell/Drivers/SdMmc/XenonDxe/.
Or you can get the patch at https://github.com/hwu25/edk2-platforms/tree/Marvell_XenonDxe

Please help to see if this solve your issue, thanks in advance.

Best Regards,
Hao Wu

From: Marcin Wojtas [mailto:mw@semihalf.com]
Sent: Friday, June 28, 2019 3:42 PM
To: Wu, Hao A
Cc: Albecki, Mateusz; Sumit Garg; Ard Biesheuvel; edk2-devel-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.wu@intel.com<mailto:hao.a.wu@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:mw@semihalf.com<mailto:mw@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.wu@intel.com<mailto:hao.a.wu@intel.com>> napisał(a):
> -----Original Message-----
> From: Sumit Garg [mailto:sumit.garg@linaro.org<mailto:sumit.garg@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.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>>
> wrote:
> >
> > (+ Sumit)
> >
> > On Thu, 27 Jun 2019 at 08:29, Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>> wrote:
> > >
> > > > -----Original Message-----
> > > > From: Marcin Wojtas [mailto:mw@semihalf.com<mailto:mw@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.garg@linaro.org<mailto:sumit.garg@linaro.org>>


Thanks a lot for the testing.

Best Regards,
Hao Wu


>
> -Sumit



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#42995): https://edk2.groups.io/g/devel/message/42995
Mute This Topic: https://groups.io/mt/32214570/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

From 94a64de93172b8096fe6555194705abede4e03ff Mon Sep 17 00:00:00 2001
From: Hao A Wu <hao.a.wu@intel.com>
Date: Fri, 28 Jun 2019 16:06:04 +0800
Subject: [PATCH] Marvell/XenonDxe: Add handles for SD 3.3V bus modes

---
 Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.c b/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.c
index 0b4949db28..1893d4bb09 100755
--- a/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.c
+++ b/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.c
@@ -387,6 +387,8 @@ XenonPhySlowMode (
   if (((Timing == SdMmcUhsSdr50) ||
        (Timing == SdMmcUhsSdr25) ||
        (Timing == SdMmcUhsSdr12) ||
+       (Timing == SdMmcSdHs) ||
+       (Timing == SdMmcSdDs) ||
        (Timing == SdMmcMmcHsDdr) ||
        (Timing == SdMmcMmcHsSdr) ||
        (Timing == SdMmcMmcLegacy)) && SlowMode) {
@@ -423,7 +425,7 @@ XenonSetPhy (
   Var &= ~(EMMC5_1_FC_CMD_PD | EMMC5_1_FC_DQ_PD);
   XenonHcRwMmio (PciIo, SD_BAR_INDEX, EMMC_PHY_PAD_CONTROL1, FALSE, SDHC_REG_SIZE_4B, &Var);
 
-  if (Timing == SdMmcUhsSdr12) {
+  if (Timing == SdMmcUhsSdr12 || Timing == SdMmcSdDs) {
     if (SlowMode) {
       XenonHcRwMmio (PciIo, SD_BAR_INDEX, EMMC_PHY_TIMING_ADJUST, TRUE, SDHC_REG_SIZE_4B, &Var);
       Var |= QSN_PHASE_SLOW_MODE_BIT;
@@ -442,7 +444,8 @@ XenonSetPhy (
       (Timing == SdMmcUhsDdr50) ||
       (Timing == SdMmcUhsSdr50) ||
       (Timing == SdMmcUhsSdr104) ||
-      (Timing == SdMmcUhsSdr25)) {
+      (Timing == SdMmcUhsSdr25) ||
+      (Timing == SdMmcSdHs)) {
     Var = ~OUTPUT_QSN_PHASE_SELECT;
     XenonHcAndMmio (PciIo, SD_BAR_INDEX, EMMC_PHY_TIMING_ADJUST, SDHC_REG_SIZE_4B, &Var);
   }
-- 
2.12.0.windows.1