On Mon, 21 Jan 2019 at 17:46, Leif Lindholm <leif.lindh...@linaro.org> wrote: > > On Mon, Jan 21, 2019 at 05:16:09PM +0100, Ard Biesheuvel wrote: > > > > > diff --git > > > > > a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h > > > > > b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h > > > > > similarity index 88% > > > > > rename from > > > > > Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h > > > > > rename to Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h > > > > > index 20e74b0320ce..61b8e6a08fa0 100644 > > > > > --- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h > > > > > +++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h > > > > > @@ -27,11 +27,9 @@ > > > > > #include <Protocol/FirmwareVolumeBlock.h> > > > > > > > > > > #include <Library/DebugLib.h> > > > > > -#include <Library/DxeServicesTableLib.h> > > > > > #include <Library/IoLib.h> > > > > > +#include <Library/MemoryAllocationLib.h> > > > > > > > > Why add this include? > > > > I'm not going to ask to move out the existing headers not actually > > > > used by this file, but could we avoid adding new ones? > > > > > > > > https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/v/release/2.20/5_source_files/53_include_files.html#534-include-files-may-include-only-those-headers-that-it-directly-depends-upon > > > > is actually a rule I agree with. > > > > > > > > But also, aren't all of the users of this file already manually > > > > including this one? > > > > > > > > > > Fair enough. I will drop the include here, and add it to whichever > > > source file requires it afterwards. > > > > > > > That include turns out to be entirely redundant, so I will just drop it. > > Works for me. >
Wonderful. Can I take that as an ack? > > > > > #include <Library/NorFlashPlatformLib.h> > > > > > -#include <Library/UefiLib.h> > > > > > -#include <Library/UefiRuntimeLib.h> > > > > > > > > > > #include "Fip006Reg.h" > > > > > > > > > > > > > > diff --git > > > > > a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c > > > > > b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c > > > > > index e52ab52d8cf7..6c07799b22d8 100644 > > > > > --- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c > > > > > +++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c > > > > > @@ -15,15 +15,16 @@ > > > > > **/ > > > > > > > > > > #include <Library/BaseMemoryLib.h> > > > > > +#include <Library/DxeServicesTableLib.h> > > > > > +#include <Library/HobLib.h> > > > > > #include <Library/MemoryAllocationLib.h> > > > > > > > > At least this one does. > > > > > > > > > #include <Library/NorFlashInfoLib.h> > > > > > #include <Library/PcdLib.h> > > > > > #include <Library/UefiBootServicesTableLib.h> > > > > > #include <Library/UefiLib.h> > > > > > +#include <Library/UefiRuntimeLib.h> > > > > > > > > > > -#include "NorFlashDxe.h" > > > > > - > > > > > -STATIC EFI_EVENT mNorFlashVirtualAddrChangeEvent; > > > > > +#include "NorFlash.h" _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel