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

Pete Batard posted 15 patches 5 years, 11 months ago
[edk2-devel] [edk2-platforms][PATCH 04/15] Silicon/BcmGenet: Add missing I/O mapping length and clean up
Posted by Pete Batard 5 years, 11 months ago
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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55067): https://edk2.groups.io/g/devel/message/55067
Mute This Topic: https://groups.io/mt/71605842/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-platforms][PATCH 04/15] Silicon/BcmGenet: Add missing I/O mapping length and clean up
Posted by Ard Biesheuvel 5 years, 11 months ago
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
>

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55079): https://edk2.groups.io/g/devel/message/55079
Mute This Topic: https://groups.io/mt/71605842/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-platforms][PATCH 04/15] Silicon/BcmGenet: Add missing I/O mapping length and clean up
Posted by Pete Batard 5 years, 11 months ago
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
>>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55081): https://edk2.groups.io/g/devel/message/55081
Mute This Topic: https://groups.io/mt/71605842/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-platforms][PATCH 04/15] Silicon/BcmGenet: Add missing I/O mapping length and clean up
Posted by Ard Biesheuvel 5 years, 11 months ago
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>

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55082): https://edk2.groups.io/g/devel/message/55082
Mute This Topic: https://groups.io/mt/71605842/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-platforms][PATCH 04/15] Silicon/BcmGenet: Add missing I/O mapping length and clean up
Posted by Pete Batard 5 years, 11 months ago
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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55083): https://edk2.groups.io/g/devel/message/55083
Mute This Topic: https://groups.io/mt/71605842/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-