Andrew, Ard,

 

In that case then I will update the patch to address Mike’s concerns and send a new version up.

 

Thanks!

 

From: Andrew Fish <afish@apple.com>
Sent: Wednesday, April 28, 2021 10:58 PM
To: Ard Biesheuvel <ardb@kernel.org>
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel-groups-io <devel@edk2.groups.io>; Loh, Tien Hock <tien.hock.loh@intel.com>; thloh85@gmail.com; Leif Lindholm <leif@nuviainc.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>
Subject: Re: [edk2-devel] [PATCH V5 1/1] EmbeddedPkg: DwMmcHcDxe: Add support for Designware SDMMC driver

 

 



On Apr 28, 2021, at 6:03 AM, Ard Biesheuvel <ardb@kernel.org> wrote:

 

On Tue, 27 Apr 2021 at 21:31, Kinney, Michael D
<michael.d.kinney@intel.com> wrote:


This is an example of another approach.  This module uses PCI I/O or IoLib based on the type of device.



https://github.com/tianocore/edk2/tree/master/MdeModulePkg/Bus/Pci/PciSioSerialDxe



No additional protocols or lib classes/instances.  Instead, the register access APIs are included in the module and based on the type of device detected, it uses PCI I/O or IoLib:



https://github.com/tianocore/edk2/blob/5b90b8abb4049e2d98040f548ad23b6ab22d5d19/MdeModulePkg/Bus/Pci/PciSioSerialDxe/SerialIo.c#L1327

https://github.com/tianocore/edk2/blob/5b90b8abb4049e2d98040f548ad23b6ab22d5d19/MdeModulePkg/Bus/Pci/PciSioSerialDxe/SerialIo.c#L1358



This technique could for PCI I/O vs MMIO register access.  You would need to add more APIs for the use of PCI I/O or DmaLib for DMA access.


Is it really worth the effort to rewrite this code?

This patch has been circulating for a while now, and I fail to see the
point of refactoring and splitting up this code, given how unlikely it
is that DesignWare will ever put a real PCI frontend on this IP. The
SD/MMC override protocol was intended for implementations that are
almost SDHCI compliant, but have some quirks that need to be worked
around.

 

Ard,

 

If we don’t see any potential value for making it more portable I’m OK with going with the current patch style. 

 

Thanks,

 

Andrew Fish