From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id E5ED61A1E05 for ; Mon, 5 Sep 2016 05:19:35 -0700 (PDT) Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5E9014E4C6; Mon, 5 Sep 2016 12:19:35 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-76.phx2.redhat.com [10.3.116.76]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u85CJX5D004297; Mon, 5 Sep 2016 08:19:33 -0400 To: Ard Biesheuvel , edk2-devel@ml01.01.org, feng.tian@intel.com, star.zeng@intel.com, liming.gao@intel.com References: <1473067049-16252-1-git-send-email-ard.biesheuvel@linaro.org> <1473067049-16252-3-git-send-email-ard.biesheuvel@linaro.org> Cc: leif.lindholm@linaro.org From: Laszlo Ersek Message-ID: <6c4c3202-81ef-c97f-11be-37b714381a5d@redhat.com> Date: Mon, 5 Sep 2016 14:19:32 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <1473067049-16252-3-git-send-email-ard.biesheuvel@linaro.org> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Mon, 05 Sep 2016 12:19:35 +0000 (UTC) Subject: Re: [PATCH 2/7] MdeModulePkg/EhciDxe: enable 64-bit PCI DMA X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 05 Sep 2016 12:19:36 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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 > --- > 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)? > + 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__. Again, whether you want to change these is up to you. Reviewed-by: Laszlo Ersek (I won't try to review the rest of the patches.) Thanks Laszlo > + } > + } > + > Status = gBS->InstallProtocolInterface ( > &Controller, > &gEfiUsb2HcProtocolGuid, > diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h > index 7177658092c3..be81bde40d9b 100644 > --- a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h > +++ b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h > @@ -173,6 +173,8 @@ struct _USB2_HC_DEV { > UINT16 DebugPortOffset; // The offset of debug port mmio register > UINT8 DebugPortBarNum; // The bar number of debug port mmio register > UINT8 DebugPortNum; // The port number of usb debug port > + > + BOOLEAN Support64BitDma; // Whether 64 bit DMA may be used with this device > }; > > > diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c b/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c > index 5594e6699ea6..036c00b32b40 100644 > --- a/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c > +++ b/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c > @@ -178,7 +178,7 @@ EhcInitSched ( > // > Ehc->MemPool = UsbHcInitMemPool ( > PciIo, > - EHC_BIT_IS_SET (Ehc->HcCapParams, HCCP_64BIT), > + Ehc->Support64BitDma, > EHC_HIGH_32BIT (PhyAddr) > ); > >