On 09/05/16 14:44, Ard Biesheuvel wrote: > On 5 September 2016 at 13:19, Laszlo Ersek <[email protected]> wrote: >> On 09/05/16 11:17, Ard Biesheuvel wrote: >>> PCI controller drivers must set the EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE >>> attribute if the controller supports 64-bit DMA addressing. >>> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Ard Biesheuvel <[email protected]> >>> --- >>> MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c | 22 +++++++++++++++++++- >>> MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h | 2 ++ >>> MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c | 2 +- >>> 3 files changed, 24 insertions(+), 2 deletions(-) >>> >>> diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c >>> b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c >>> index 4e9e05f0e431..e4c7e59526de 100644 >>> --- a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c >>> +++ b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c >>> @@ -89,7 +89,7 @@ EhcGetCapability ( >>> >>> *MaxSpeed = EFI_USB_SPEED_HIGH; >>> *PortNumber = (UINT8) (Ehc->HcStructParams & HCSP_NPORTS); >>> - *Is64BitCapable = (UINT8) (Ehc->HcCapParams & HCCP_64BIT); >>> + *Is64BitCapable = (UINT8) Ehc->Support64BitDma; >>> >>> DEBUG ((EFI_D_INFO, "EhcGetCapability: %d ports, 64 bit %d\n", >>> *PortNumber, *Is64BitCapable)); >>> >>> @@ -1877,6 +1877,26 @@ EhcDriverBindingStart ( >>> goto CLOSE_PCIIO; >>> } >>> >>> + // >>> + // Enable 64-bit DMA support in the PCI layer if this controller >>> + // supports it. >>> + // >>> + if ((Ehc->HcCapParams & HCCP_64BIT) != 0) { >> >> How about using the nice EHC_BIT_IS_SET macro here (inspired by the >> previous use in EhcInitSched(), at the bottom of this patch)? >> > > I just moved the test from the previous hunk here. I don't care either > way, so I will let the maintainers decide. Feng, Star? > >>> + Status = PciIo->Attributes ( >>> + PciIo, >>> + EfiPciIoAttributeOperationEnable, >>> + EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE, >>> + NULL >>> + ); >>> + if (!EFI_ERROR (Status)) { >>> + Ehc->Support64BitDma = TRUE; >>> + } else { >>> + DEBUG ((EFI_D_WARN, >>> + "EhcDriverBindingStart: failed to enable 64-bit DMA on 64-bit >>> capable controller @ %p (%r)\n", >>> + Controller, Status)); >> >> Same comment as for 5/7, i.e. %a and __FUNCTION__. >> > > The surrounding code never uses that. I started out using %a and > __FUNCTION__, and then removed it again to align with the existing > code. > >> Again, whether you want to change these is up to you. >> >> Reviewed-by: Laszlo Ersek <[email protected]> >> > > Thanks a lot! > >> (I won't try to review the rest of the patches.) >> > > No worries. I did build test all of them, but I currently have no way > of testing the NVM and SDHCI patches, so I am hoping they are > 'obviously correct' >
You can test the NVMe change with QEMU (x86_64) + OVMF: https://tianocore.acgmultimedia.com/show_bug.cgi?id=79 Thanks Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
