| Message ID | 20170830082108.7470-2-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | ArmPkg EmbeddedPkg: clean up DmaLib implementations | expand |
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
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
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
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
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