Re: [edk2-platforms][PATCH 04/15] Silicon/BcmGenet: Add missing I/O mapping length and clean up

["Ard Biesheuvel" <ard.biesheuvel@linaro.org>]

On Fri, 28 Feb 2020 at 11:51, Pete Batard <pete@akeo.ie> wrote:
>
> Hi Ard,
>
> On 2020.02.28 10:45, Ard Biesheuvel wrote:
> > On Fri, 28 Feb 2020 at 11:39, Pete Batard <pete@akeo.ie> wrote:
> >>
> >> Remove unneeded extra parenthesis on PCD, which can cause problems
> >> when used with ACPI ASL macros and add an [Includes] section to the
> >> .inf, so that the Genet.h header can be referenced where required.
> >>
> >> Signed-off-by: Pete Batard <pete@akeo.ie>
> >> ---
> >>   Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.h | 3 ++-
> >>   Silicon/Broadcom/Drivers/Net/BcmNet.dec          | 3 +++
> >>   2 files changed, 5 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.h b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.h
> >> index 4a3827c0e0d1..f56fb2977422 100644
> >> --- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.h
> >> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.h
> >> @@ -11,7 +11,8 @@
> >>
> >>   #include <Library/PcdLib.h>
> >>
> >> -#define GENET_BASE_ADDRESS         (FixedPcdGet64 (PcdBcmGenetRegistersAddress))
> >> +#define GENET_BASE_ADDRESS         FixedPcdGet64 (PcdBcmGenetRegistersAddress)
> >> +#define GENET_LENGTH               0x00010000
> >>
> >>   #define GENET_SYS_RBUF_FLUSH_CTRL  0x0008
> >>   #define GENET_UMAC_MAC0            0x080c
> >> diff --git a/Silicon/Broadcom/Drivers/Net/BcmNet.dec b/Silicon/Broadcom/Drivers/Net/BcmNet.dec
> >> index 2a8688cb09a7..483b033af51c 100644
> >> --- a/Silicon/Broadcom/Drivers/Net/BcmNet.dec
> >> +++ b/Silicon/Broadcom/Drivers/Net/BcmNet.dec
> >> @@ -12,6 +12,9 @@ [Defines]
> >>     PACKAGE_GUID                   = 34E19823-D23A-41AB-9C09-ED1225B32DFF
> >>     PACKAGE_VERSION                = 1.0
> >>
> >> +[Includes]
> >> +  .
> >> +
> >
> > This looks fishy. Won't this cause *every* module that incorporates
> > this .dec to add . to its include path?
>
> Yeah, I don't like it either. I kind of expected you guys to comment on
> it, so that we can discuss what you think the better approach should be.
>
> Do you think it'd make sense to create a Drivers/Include/ section in
> Silicon/Broadcom/ and move the header there?
>
> And if we do that, do you think the header should go to something like
> Include/Net or just reside at the top level of Include/?
>
> What would be your preferred approach?
>

If the contents of the header need to be visible outside of the
module, then the header needs to be moved outside of the module.

So move the header to

Silicon/Broadcom/Drivers/Include/Net/

and add Include under the [Includes] section

Then, any component that includes the .dec can access the header via

#include <Net/Genet.h>