On 2020.02.28 10:58, Ard Biesheuvel wrote:

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>

OK. I'll do that for v2.

/Pete

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>

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?

Regards,

/Pete

[Guids]
gBcmNetTokenSpaceGuid = {0x12b97d70, 0x9149, 0x4c2f, {0x82, 0xd5, 0xad, 0xa9, 0x1e, 0x92, 0x75, 0xa1}}

--
2.21.0.windows.1

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?

[Guids]
gBcmNetTokenSpaceGuid = {0x12b97d70, 0x9149, 0x4c2f, {0x82, 0xd5, 0xad, 0xa9, 0x1e, 0x92, 0x75, 0xa1}}

--
2.21.0.windows.1

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]
+ .
+
[Guids]
gBcmNetTokenSpaceGuid = {0x12b97d70, 0x9149, 0x4c2f, {0x82, 0xd5, 0xad, 0xa9, 0x1e, 0x92, 0x75, 0xa1}}

--
2.21.0.windows.1