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

Message ID 20170830082108.7470-2-ard.biesheuvel@linaro.org
State New
Headers show
Series
  • ArmPkg EmbeddedPkg: clean up DmaLib implementations
Related show

Commit Message

Ard Biesheuvel Aug. 30, 2017, 8:21 a.m.
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(-)

-- 
2.11.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Leif Lindholm Aug. 30, 2017, 10:46 a.m. | #1
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
Ard Biesheuvel Aug. 30, 2017, 10:52 a.m. | #2
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
Leif Lindholm Aug. 30, 2017, 11:37 a.m. | #3
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

Patch

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