public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v1 0/7] Add linked list iteration macros to BaseLib.h
@ 2020-04-09 20:05 Michael Kubacki
  2020-04-10  1:43 ` Wu, Hao A
  0 siblings, 1 reply; 3+ messages in thread
From: Michael Kubacki @ 2020-04-09 20:05 UTC (permalink / raw)
  To: devel
  Cc: Chao Zhang, Dandan Bi, Hao A Wu, Jian J Wang, Liming Gao,
	Michael D Kinney, Ray Ni, Sean Brogan

From: Michael Kubacki <michael.kubacki@microsoft.com>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1959

The macros EFI_LIST_FOR_EACH and EFI_LIST_FOR_EACH_SAFE have been
duplicated across several drivers such as:

* EhciPei
* EhciDxe
* HddPasswordDxe
* RamDiskDxe
* UfsPassThruDxe
* XhciDxe

These macros have proven useful and established a commonly used pattern
for linked list iteration.

This patch series consolidates the definitions to a single definition in
BaseLib.h so they are maintained alongside other pre-existing generic
linked list macros and functions.

Note: Another commonly used generic linked list macro is EFI_LIST_CONTAINER.
I'm considering consolidating that to BaseLib.h as well but that is not
done in this patch series since it satisfies a separate use case than the
iteration macros and the change can be made independently.

On a separate note, shallow threading might not work on this patch series
due to changes made by the SMTP server. Please bear with me while I am
investigating if this can be changed.

Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>

Michael Kubacki (7):
  MdePkg/BaseLib: Add linked list iteration macros
  MdeModulePkg/EhciDxe: Use BaseLib linked list iteration macros
  MdeModulePkg/EhciPei: Use BaseLib linked list iteration macros
  MdeModulePkg/XhciDxe: Use BaseLib linked list iteration macros
  MdeModulePkg/UfsPassThruDxe: Use BaseLib linked list iteration macros
  MdeModulePkg/RamDiskDxe: Use BaseLib linked list iteration macros
  SecurityPkg/HddPassword: Use BaseLib linked list iteration macros

 MdeModulePkg/Bus/Pci/EhciDxe/EhciDebug.c                 |  3 ++-
 MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c                 | 11 ++++----
 MdeModulePkg/Bus/Pci/EhciDxe/EhciUrb.c                   |  5 ++--
 MdeModulePkg/Bus/Pci/EhciPei/EhciSched.c                 |  3 ++-
 MdeModulePkg/Bus/Pci/EhciPei/EhciUrb.c                   |  5 ++--
 MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c                 |  9 ++++---
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c        |  3 ++-
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c     |  3 ++-
 MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDriver.c   |  3 ++-
 MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.c     |  9 ++++---
 MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c |  5 ++--
 SecurityPkg/HddPassword/HddPasswordDxe.c                 | 13 +++++-----
 MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h                      | 15 +----------
 MdeModulePkg/Bus/Pci/EhciPei/EhcPeim.h                   | 15 +----------
 MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h                      |  9 +------
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h        |  9 +------
 MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h     | 15 +----------
 MdePkg/Include/Library/BaseLib.h                         | 27 ++++++++++++++++++++
 SecurityPkg/HddPassword/HddPasswordDxe.h                 |  7 +----
 19 files changed, 75 insertions(+), 94 deletions(-)

-- 
2.16.3.windows.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v1 0/7] Add linked list iteration macros to BaseLib.h
  2020-04-09 20:05 [PATCH v1 0/7] Add linked list iteration macros to BaseLib.h Michael Kubacki
@ 2020-04-10  1:43 ` Wu, Hao A
  2020-04-10  1:48   ` Michael Kubacki
  0 siblings, 1 reply; 3+ messages in thread
From: Wu, Hao A @ 2020-04-10  1:43 UTC (permalink / raw)
  To: michael.kubacki@outlook.com, devel@edk2.groups.io
  Cc: Zhang, Chao B, Bi, Dandan, Wang, Jian J, Gao, Liming,
	Kinney, Michael D, Ni, Ray, Sean Brogan

> -----Original Message-----
> From: michael.kubacki@outlook.com [mailto:michael.kubacki@outlook.com]
> Sent: Friday, April 10, 2020 4:05 AM
> To: devel@edk2.groups.io
> Cc: Zhang, Chao B; Bi, Dandan; Wu, Hao A; Wang, Jian J; Gao, Liming; Kinney,
> Michael D; Ni, Ray; Sean Brogan
> Subject: [PATCH v1 0/7] Add linked list iteration macros to BaseLib.h
> 
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1959
> 
> The macros EFI_LIST_FOR_EACH and EFI_LIST_FOR_EACH_SAFE have been
> duplicated across several drivers such as:
> 
> * EhciPei
> * EhciDxe
> * HddPasswordDxe
> * RamDiskDxe
> * UfsPassThruDxe
> * XhciDxe
> 
> These macros have proven useful and established a commonly used pattern
> for linked list iteration.
> 
> This patch series consolidates the definitions to a single definition in
> BaseLib.h so they are maintained alongside other pre-existing generic
> linked list macros and functions.
> 
> Note: Another commonly used generic linked list macro is
> EFI_LIST_CONTAINER.
> I'm considering consolidating that to BaseLib.h as well but that is not
> done in this patch series since it satisfies a separate use case than the
> iteration macros and the change can be made independently.
> 
> On a separate note, shallow threading might not work on this patch series
> due to changes made by the SMTP server. Please bear with me while I am
> investigating if this can be changed.
> 
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> Michael Kubacki (7):
>   MdePkg/BaseLib: Add linked list iteration macros
>   MdeModulePkg/EhciDxe: Use BaseLib linked list iteration macros
>   MdeModulePkg/EhciPei: Use BaseLib linked list iteration macros
>   MdeModulePkg/XhciDxe: Use BaseLib linked list iteration macros
>   MdeModulePkg/UfsPassThruDxe: Use BaseLib linked list iteration macros
>   MdeModulePkg/RamDiskDxe: Use BaseLib linked list iteration macros


Hello Michael,

I found that for EhciPei, it does not have explicit dependency on the BaseLib,
could you help to add it?

With this handled, for the MdeModulePkg changes (patch 2~6):
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu


>   SecurityPkg/HddPassword: Use BaseLib linked list iteration macros
> 
>  MdeModulePkg/Bus/Pci/EhciDxe/EhciDebug.c                 |  3 ++-
>  MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c                 | 11 ++++----
>  MdeModulePkg/Bus/Pci/EhciDxe/EhciUrb.c                   |  5 ++--
>  MdeModulePkg/Bus/Pci/EhciPei/EhciSched.c                 |  3 ++-
>  MdeModulePkg/Bus/Pci/EhciPei/EhciUrb.c                   |  5 ++--
>  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c                 |  9 ++++---
>  MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c        |  3 ++-
>  MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c     |  3 ++-
>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDriver.c   |  3 ++-
>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.c     |  9 ++++---
>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c |  5 ++--
>  SecurityPkg/HddPassword/HddPasswordDxe.c                 | 13 +++++-----
>  MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h                      | 15 +----------
>  MdeModulePkg/Bus/Pci/EhciPei/EhcPeim.h                   | 15 +----------
>  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h                      |  9 +------
>  MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h        |  9 +------
>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h     | 15 +---------
> -
>  MdePkg/Include/Library/BaseLib.h                         | 27
> ++++++++++++++++++++
>  SecurityPkg/HddPassword/HddPasswordDxe.h                 |  7 +----
>  19 files changed, 75 insertions(+), 94 deletions(-)
> 
> --
> 2.16.3.windows.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v1 0/7] Add linked list iteration macros to BaseLib.h
  2020-04-10  1:43 ` Wu, Hao A
@ 2020-04-10  1:48   ` Michael Kubacki
  0 siblings, 0 replies; 3+ messages in thread
From: Michael Kubacki @ 2020-04-10  1:48 UTC (permalink / raw)
  To: Wu, Hao A, devel@edk2.groups.io
  Cc: Zhang, Chao B, Bi, Dandan, Wang, Jian J, Gao, Liming,
	Kinney, Michael D, Ni, Ray, Sean Brogan

I agree it should be added. I'll leave the series up until tomorrow to 
collect further feedback and then I will send out v2 with R-bs and 
modifications included.

Thanks,
Michael

On 4/9/2020 6:43 PM, Wu, Hao A wrote:
>> -----Original Message-----
>> From: michael.kubacki@outlook.com [mailto:michael.kubacki@outlook.com]
>> Sent: Friday, April 10, 2020 4:05 AM
>> To: devel@edk2.groups.io
>> Cc: Zhang, Chao B; Bi, Dandan; Wu, Hao A; Wang, Jian J; Gao, Liming; Kinney,
>> Michael D; Ni, Ray; Sean Brogan
>> Subject: [PATCH v1 0/7] Add linked list iteration macros to BaseLib.h
>>
>> From: Michael Kubacki <michael.kubacki@microsoft.com>
>>
>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1959
>>
>> The macros EFI_LIST_FOR_EACH and EFI_LIST_FOR_EACH_SAFE have been
>> duplicated across several drivers such as:
>>
>> * EhciPei
>> * EhciDxe
>> * HddPasswordDxe
>> * RamDiskDxe
>> * UfsPassThruDxe
>> * XhciDxe
>>
>> These macros have proven useful and established a commonly used pattern
>> for linked list iteration.
>>
>> This patch series consolidates the definitions to a single definition in
>> BaseLib.h so they are maintained alongside other pre-existing generic
>> linked list macros and functions.
>>
>> Note: Another commonly used generic linked list macro is
>> EFI_LIST_CONTAINER.
>> I'm considering consolidating that to BaseLib.h as well but that is not
>> done in this patch series since it satisfies a separate use case than the
>> iteration macros and the change can be made independently.
>>
>> On a separate note, shallow threading might not work on this patch series
>> due to changes made by the SMTP server. Please bear with me while I am
>> investigating if this can be changed.
>>
>> Cc: Chao Zhang <chao.b.zhang@intel.com>
>> Cc: Dandan Bi <dandan.bi@intel.com>
>> Cc: Hao A Wu <hao.a.wu@intel.com>
>> Cc: Jian J Wang <jian.j.wang@intel.com>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Ray Ni <ray.ni@intel.com>
>> Cc: Sean Brogan <sean.brogan@microsoft.com>
>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
>>
>> Michael Kubacki (7):
>>    MdePkg/BaseLib: Add linked list iteration macros
>>    MdeModulePkg/EhciDxe: Use BaseLib linked list iteration macros
>>    MdeModulePkg/EhciPei: Use BaseLib linked list iteration macros
>>    MdeModulePkg/XhciDxe: Use BaseLib linked list iteration macros
>>    MdeModulePkg/UfsPassThruDxe: Use BaseLib linked list iteration macros
>>    MdeModulePkg/RamDiskDxe: Use BaseLib linked list iteration macros
> 
> 
> Hello Michael,
> 
> I found that for EhciPei, it does not have explicit dependency on the BaseLib,
> could you help to add it?
> 
> With this handled, for the MdeModulePkg changes (patch 2~6):
> Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
> 
> Best Regards,
> Hao Wu
> 
> 
>>    SecurityPkg/HddPassword: Use BaseLib linked list iteration macros
>>
>>   MdeModulePkg/Bus/Pci/EhciDxe/EhciDebug.c                 |  3 ++-
>>   MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c                 | 11 ++++----
>>   MdeModulePkg/Bus/Pci/EhciDxe/EhciUrb.c                   |  5 ++--
>>   MdeModulePkg/Bus/Pci/EhciPei/EhciSched.c                 |  3 ++-
>>   MdeModulePkg/Bus/Pci/EhciPei/EhciUrb.c                   |  5 ++--
>>   MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c                 |  9 ++++---
>>   MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c        |  3 ++-
>>   MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c     |  3 ++-
>>   MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDriver.c   |  3 ++-
>>   MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.c     |  9 ++++---
>>   MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c |  5 ++--
>>   SecurityPkg/HddPassword/HddPasswordDxe.c                 | 13 +++++-----
>>   MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h                      | 15 +----------
>>   MdeModulePkg/Bus/Pci/EhciPei/EhcPeim.h                   | 15 +----------
>>   MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h                      |  9 +------
>>   MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h        |  9 +------
>>   MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h     | 15 +---------
>> -
>>   MdePkg/Include/Library/BaseLib.h                         | 27
>> ++++++++++++++++++++
>>   SecurityPkg/HddPassword/HddPasswordDxe.h                 |  7 +----
>>   19 files changed, 75 insertions(+), 94 deletions(-)
>>
>> --
>> 2.16.3.windows.1
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-04-10  1:48 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-04-09 20:05 [PATCH v1 0/7] Add linked list iteration macros to BaseLib.h Michael Kubacki
2020-04-10  1:43 ` Wu, Hao A
2020-04-10  1:48   ` Michael Kubacki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox