Re: [PATCH] MdeModulePkg/NonDiscoverable: Compare SIZE_4GB with address type

["Wu, Hao A" <hao.a.wu@intel.com>]

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Wednesday, January 11, 2017 3:55 PM
> To: Ni, Ruiyu; Laszlo Ersek
> Cc: Wu, Hao A; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] MdeModulePkg/NonDiscoverable: Compare
> SIZE_4GB with address type
> 
> On 11 January 2017 at 02:05, Ni, Ruiyu <ruiyu.ni@intel.com> wrote:
> > One minor comments: put one space after (EFI_PHYSICAL_ADDRESS) and
> (UINTN) as below.
> >> +            (EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress + *NumberOfBytes >
> SIZE_4GB
> >
> 
> I disagree. The EDK2 coding style does not mandate a space after a
> cast, and given the tight binding of the () cast operator in C, it is
> counterinituitive.

Thanks Ard, I agree with your point. I will leave the patch unchanged.

Best Regards,
Hao Wu

> 
> > Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
> >
> >
> >> -----Original Message-----
> >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> >> Hao Wu
> >> Sent: Wednesday, January 11, 2017 10:01 AM
> >> To: edk2-devel@lists.01.org
> >> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Ard
> >> Biesheuvel <ard.biesheuvel@linaro.org>
> >> Subject: [edk2] [PATCH] MdeModulePkg/NonDiscoverable: Compare
> >> SIZE_4GB with address type
> >>
> >> Refine the codes to compare the definition 'SIZE_4GB' with type
> >> EFI_PHYSICAL_ADDRESS.
> >>
> >> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> >> ---
> >>  .../Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c  |
> >> 4 ++--
> >>  .../NonDiscoverableDeviceRegistrationLib.c                            | 2 +-
> >>  2 files changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git
> >> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable
> >> PciDeviceIo.c
> >> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable
> >> PciDeviceIo.c
> >> index b07c129..c836ad6 100644
> >> ---
> >> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable
> >> PciDeviceIo.c
> >> +++
> >> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable
> >> Pc
> >> +++ iDeviceIo.c
> >> @@ -598,7 +598,7 @@ CoherentPciIoMap (
> >>    //
> >>    Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
> >>    if ((Dev->Attributes & EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE) ==
> >> 0 &&
> >> -      (UINTN)HostAddress + *NumberOfBytes > SIZE_4GB) {
> >> +      (EFI_PHYSICAL_ADDRESS)(UINTN)HostAddress + *NumberOfBytes >
> >> + SIZE_4GB) {
> >>
> >>      //
> >>      // Bounce buffering is not possible for consistent mappings @@ -1006,7
> >> +1006,7 @@ NonCoherentPciIoMap (
> >>    // a bounce buffer and copy over the data in case HostAddress >= 4 GB.
> >>    //
> >>    Bounce = ((Dev->Attributes &
> >> EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE) == 0 &&
> >> -            (UINTN)HostAddress + *NumberOfBytes > SIZE_4GB);
> >> +            (EFI_PHYSICAL_ADDRESS)(UINTN)HostAddress + *NumberOfBytes >
> >> + SIZE_4GB);
> >>
> >>    if (!Bounce) {
> >>      switch (Operation) {
> >> diff --git
> >> a/MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDisco
> >> verableDeviceRegistrationLib.c
> >> b/MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDisco
> >> verableDeviceRegistrationLib.c
> >> index 6f46dfa..4180b0a 100644
> >> ---
> >> a/MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDisco
> >> verableDeviceRegistrationLib.c
> >> +++
> >> b/MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDisco
> >> +++ verableDeviceRegistrationLib.c
> >> @@ -163,7 +163,7 @@ RegisterNonDiscoverableMmioDevice (
> >>      Desc->AddrLen               = Size;
> >>      Desc->AddrRangeMax          = Base + Size - 1;
> >>      Desc->ResType               = ACPI_ADDRESS_SPACE_TYPE_MEM;
> >> -    Desc->AddrSpaceGranularity  = (Base + Size > SIZE_4GB) ? 64 : 32;
> >> +    Desc->AddrSpaceGranularity  = ((EFI_PHYSICAL_ADDRESS) Base + Size >
> >> + SIZE_4GB) ? 64 : 32;
> >>      Desc->AddrTranslationOffset = 0;
> >>    }
> >>    VA_END (Args);
> >> --
> >> 1.9.5.msysgit.0
> >>
> >> _______________________________________________
> >> edk2-devel mailing list
> >> edk2-devel@lists.01.org
> >> https://lists.01.org/mailman/listinfo/edk2-devel