1. Patchew
  2. EDK2
  3. [edk2] [PATCH 0/6] ArmPkg EmbeddedPkg: clean up DmaLib implementations
  4. View patch

[edk2] [PATCH 1/6] EmbeddedPkg: rename NullDmaLib to CoherentDmaLib

Ard Biesheuvel posted 6 patches 7 years, 3 months ago
[edk2] [PATCH 1/6] EmbeddedPkg: rename NullDmaLib to CoherentDmaLib
Posted by Ard Biesheuvel 7 years, 3 months ago 
Rename NullDmaLib to CoherentDmaLib to better reflect its nature, and
to better convey the fact that it actually serves a useful purpose,
i.e., as a DmaLib library class resolution for drivers that control
hardware that may only be cache coherent or in some cases (i.e., on
some platforms but not on others).

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 EmbeddedPkg/EmbeddedPkg.dsc                                                          |  2 +-
 EmbeddedPkg/Library/{NullDmaLib/NullDmaLib.c => CoherentDmaLib/CoherentDmaLib.c}     |  0
 EmbeddedPkg/Library/{NullDmaLib/NullDmaLib.inf => CoherentDmaLib/CoherentDmaLib.inf} | 20 ++++++--------------
 3 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/EmbeddedPkg/EmbeddedPkg.dsc b/EmbeddedPkg/EmbeddedPkg.dsc
index 4a34e34843ad..84c5a842e37e 100644
--- a/EmbeddedPkg/EmbeddedPkg.dsc
+++ b/EmbeddedPkg/EmbeddedPkg.dsc
@@ -250,7 +250,7 @@ [Components.common]
   EmbeddedPkg/Library/TemplateResetSystemLib/TemplateResetSystemLib.inf
   EmbeddedPkg/Library/TemplateRealTimeClockLib/TemplateRealTimeClockLib.inf
   EmbeddedPkg/Library/LzmaHobCustomDecompressLib/LzmaHobCustomDecompressLib.inf
-  EmbeddedPkg/Library/NullDmaLib/NullDmaLib.inf
+  EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
   EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
 
   EmbeddedPkg/Ebl/Ebl.inf
diff --git a/EmbeddedPkg/Library/NullDmaLib/NullDmaLib.c b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
similarity index 100%
rename from EmbeddedPkg/Library/NullDmaLib/NullDmaLib.c
rename to EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
diff --git a/EmbeddedPkg/Library/NullDmaLib/NullDmaLib.inf b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
similarity index 78%
rename from EmbeddedPkg/Library/NullDmaLib/NullDmaLib.inf
rename to EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
index 38261d5ede2b..c40a600cf6a3 100644
--- a/EmbeddedPkg/Library/NullDmaLib/NullDmaLib.inf
+++ b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
@@ -1,6 +1,8 @@
 #/** @file
 #
 #  Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
+#  Copyright (c) 2017, Linaro, Ltd. All rights reserved.<BR>
+#
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD License
 #  which accompanies this distribution.  The full text of the license may be found at
@@ -12,15 +14,15 @@
 #**/
 
 [Defines]
-  INF_VERSION                    = 0x00010005
-  BASE_NAME                      = NullDmaLib
+  INF_VERSION                    = 0x00010019
+  BASE_NAME                      = CoherentDmaLib
   FILE_GUID                      = 0F2A0816-D319-4ee7-A6B8-D58524E4428F
   MODULE_TYPE                    = BASE
   VERSION_STRING                 = 1.0
   LIBRARY_CLASS                  = DmaLib
 
-[Sources.common]
-  NullDmaLib.c
+[Sources]
+  CoherentDmaLib.c
 
 [Packages]
   MdePkg/MdePkg.dec
@@ -29,13 +31,3 @@ [Packages]
 [LibraryClasses]
   DebugLib
   MemoryAllocationLib
-
-
-[Protocols]
-
-[Guids]
-
-[Pcd]
-
-[Depex]
-  TRUE
-- 
2.11.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/6] EmbeddedPkg: rename NullDmaLib to CoherentDmaLib
Posted by Leif Lindholm 7 years, 3 months ago 
On Wed, Aug 30, 2017 at 09:21:03AM +0100, Ard Biesheuvel wrote:
> Rename NullDmaLib to CoherentDmaLib to better reflect its nature, and
> to better convey the fact that it actually serves a useful purpose,
> i.e., as a DmaLib library class resolution for drivers that control
> hardware that may only be cache coherent or in some cases (i.e., on
> some platforms but not on others).

The above doesn't read very well (and I'm not 100% certain what it's
trying to say, so can't really propose an improvement).
No other issues with patch.

/
    Leif

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  EmbeddedPkg/EmbeddedPkg.dsc                                                          |  2 +-
>  EmbeddedPkg/Library/{NullDmaLib/NullDmaLib.c => CoherentDmaLib/CoherentDmaLib.c}     |  0
>  EmbeddedPkg/Library/{NullDmaLib/NullDmaLib.inf => CoherentDmaLib/CoherentDmaLib.inf} | 20 ++++++--------------
>  3 files changed, 7 insertions(+), 15 deletions(-)
> 
> diff --git a/EmbeddedPkg/EmbeddedPkg.dsc b/EmbeddedPkg/EmbeddedPkg.dsc
> index 4a34e34843ad..84c5a842e37e 100644
> --- a/EmbeddedPkg/EmbeddedPkg.dsc
> +++ b/EmbeddedPkg/EmbeddedPkg.dsc
> @@ -250,7 +250,7 @@ [Components.common]
>    EmbeddedPkg/Library/TemplateResetSystemLib/TemplateResetSystemLib.inf
>    EmbeddedPkg/Library/TemplateRealTimeClockLib/TemplateRealTimeClockLib.inf
>    EmbeddedPkg/Library/LzmaHobCustomDecompressLib/LzmaHobCustomDecompressLib.inf
> -  EmbeddedPkg/Library/NullDmaLib/NullDmaLib.inf
> +  EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
>    EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
>  
>    EmbeddedPkg/Ebl/Ebl.inf
> diff --git a/EmbeddedPkg/Library/NullDmaLib/NullDmaLib.c b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
> similarity index 100%
> rename from EmbeddedPkg/Library/NullDmaLib/NullDmaLib.c
> rename to EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
> diff --git a/EmbeddedPkg/Library/NullDmaLib/NullDmaLib.inf b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
> similarity index 78%
> rename from EmbeddedPkg/Library/NullDmaLib/NullDmaLib.inf
> rename to EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
> index 38261d5ede2b..c40a600cf6a3 100644
> --- a/EmbeddedPkg/Library/NullDmaLib/NullDmaLib.inf
> +++ b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
> @@ -1,6 +1,8 @@
>  #/** @file
>  #
>  #  Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
> +#  Copyright (c) 2017, Linaro, Ltd. All rights reserved.<BR>
> +#
>  #  This program and the accompanying materials
>  #  are licensed and made available under the terms and conditions of the BSD License
>  #  which accompanies this distribution.  The full text of the license may be found at
> @@ -12,15 +14,15 @@
>  #**/
>  
>  [Defines]
> -  INF_VERSION                    = 0x00010005
> -  BASE_NAME                      = NullDmaLib
> +  INF_VERSION                    = 0x00010019
> +  BASE_NAME                      = CoherentDmaLib
>    FILE_GUID                      = 0F2A0816-D319-4ee7-A6B8-D58524E4428F
>    MODULE_TYPE                    = BASE
>    VERSION_STRING                 = 1.0
>    LIBRARY_CLASS                  = DmaLib
>  
> -[Sources.common]
> -  NullDmaLib.c
> +[Sources]
> +  CoherentDmaLib.c
>  
>  [Packages]
>    MdePkg/MdePkg.dec
> @@ -29,13 +31,3 @@ [Packages]
>  [LibraryClasses]
>    DebugLib
>    MemoryAllocationLib
> -
> -
> -[Protocols]
> -
> -[Guids]
> -
> -[Pcd]
> -
> -[Depex]
> -  TRUE
> -- 
> 2.11.0
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/6] EmbeddedPkg: rename NullDmaLib to CoherentDmaLib
Posted by Ard Biesheuvel 7 years, 3 months ago 
On 30 August 2017 at 11:46, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Wed, Aug 30, 2017 at 09:21:03AM +0100, Ard Biesheuvel wrote:
>> Rename NullDmaLib to CoherentDmaLib to better reflect its nature, and
>> to better convey the fact that it actually serves a useful purpose,
>> i.e., as a DmaLib library class resolution for drivers that control
>> hardware that may only be cache coherent or in some cases (i.e., on
>> some platforms but not on others).
>
> The above doesn't read very well (and I'm not 100% certain what it's
> trying to say, so can't really propose an improvement).
> No other issues with patch.
>

How about

"""
The name NullDmaLib suggests that this library is a placeholder that
only exists to fulfil formal dependencies on the DmaLib library class
without providing an actual implementation (*). This is not the case,
though: NullDmaLib does implement DmaLib fully, but doing so simply
requires very little effort on a cache coherent platform. So let's
rename it to CoherentDmaLib instead.
"""

* there are such instances in MdeModulePkg that do nothing and
ASSERT() in every function.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/6] EmbeddedPkg: rename NullDmaLib to CoherentDmaLib
Posted by Leif Lindholm 7 years, 3 months ago 
On Wed, Aug 30, 2017 at 11:52:26AM +0100, Ard Biesheuvel wrote:
> On 30 August 2017 at 11:46, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Wed, Aug 30, 2017 at 09:21:03AM +0100, Ard Biesheuvel wrote:
> >> Rename NullDmaLib to CoherentDmaLib to better reflect its nature, and
> >> to better convey the fact that it actually serves a useful purpose,
> >> i.e., as a DmaLib library class resolution for drivers that control
> >> hardware that may only be cache coherent or in some cases (i.e., on
> >> some platforms but not on others).
> >
> > The above doesn't read very well (and I'm not 100% certain what it's
> > trying to say, so can't really propose an improvement).
> > No other issues with patch.
> >
> 
> How about
> 
> """
> The name NullDmaLib suggests that this library is a placeholder that
> only exists to fulfil formal dependencies on the DmaLib library class
> without providing an actual implementation (*). This is not the case,
> though: NullDmaLib does implement DmaLib fully, but doing so simply
> requires very little effort on a cache coherent platform. So let's
> rename it to CoherentDmaLib instead.
> """
> 
> * there are such instances in MdeModulePkg that do nothing and
> ASSERT() in every function.

Indeed.
Yes, this looks fine to me:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patchew 2.1-devel-8c64711

© 2016 - 2024 Red Hat, Inc.