Re: [edk2-devel] [PATCH V5 1/1] EmbeddedPkg: DwMmcHcDxe: Add support for Designware SDMMC driver

["Andrew Fish" <afish@apple.com>]

[-- Attachment #1: Type: text/plain, Size: 1667 bytes --]



> 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 <mailto: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

[-- Attachment #2: Type: text/html, Size: 10492 bytes --]