Re: [PATCH edk2-platforms 5/8] Silicon/SynQuacer: disable PCI RC #0 DT node if disabled

[Leif Lindholm <leif.lindholm@linaro.org>]

On Tue, Dec 12, 2017 at 06:09:13PM +0000, Ard Biesheuvel wrote:
> On 12 December 2017 at 17:50, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Tue, Dec 12, 2017 at 05:35:15PM +0000, Ard Biesheuvel wrote:
> >> On 12 December 2017 at 17:32, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >> > Suggested subject tweak:
> >> > Silicon/SynQuacer: disable PCI RC #0 DT node if disabled ->
> >> > Silicon/SynQuacer: disable PCI RC DT node if RC disabled.
> >> >
> >>
> >> Ack. It's out of date, as you probably suspected
> >>
> >> > The code below will disable either #0 or #1 node if not enabled by
> >> > Pcd.
> >> >
> >> > On Tue, Dec 12, 2017 at 10:38:04AM +0000, Ard Biesheuvel wrote:
> >> >> If PCIe RC #0 is not enabled (due to the fact that the slot is not
> >> >> populated), set its DT node 'status' property to 'disabled' so that
> >> >> the OS will not attempt to attach to it.
> >> >>
> >> >> This means we will need to switch from the default DtPlatformDtbLoaderLib
> >> >> to a special one for our platform.
> >> >>
> >> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> >> ---
> >> >>  Platform/Socionext/DeveloperBox/DeveloperBox.dsc                                    |  8 +-
> >> >>  Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc                        |  3 +-
> >> >>  Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c   | 94 ++++++++++++++++++++
> >> >>  Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf | 42 +++++++++
> >> >>  4 files changed, 141 insertions(+), 6 deletions(-)
> >> >>
> >> >> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> >> >> index 5ec26f9cdd34..80728fedbc20 100644
> >> >> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> >> >> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> >> >> @@ -160,7 +160,8 @@ [LibraryClasses.common.DXE_CORE]
> >> >>    PerformanceLib|MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
> >> >>
> >> >>  [LibraryClasses.common.DXE_DRIVER]
> >> >> -  DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
> >> >> +  DtPlatformDtbLoaderLib|Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
> >> >> +  FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
> >> >>    SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
> >> >>    PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
> >> >>
> >> >> @@ -611,10 +612,7 @@ [Components.common]
> >> >>    #
> >> >>    # Console preference selection
> >> >>    #
> >> >> -  EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.inf {
> >> >> -    <LibraryClasses>
> >> >> -      FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
> >> >> -  }
> >> >> +  EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.inf
> >> >>
> >> >>    #
> >> >>    # DT support
> >> >> diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
> >> >> index bc8ddd452d4b..c71425664bdc 100644
> >> >> --- a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
> >> >> +++ b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
> >> >> @@ -159,7 +159,8 @@ [LibraryClasses.common.DXE_CORE]
> >> >>    PerformanceLib|MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
> >> >>
> >> >>  [LibraryClasses.common.DXE_DRIVER]
> >> >> -  DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
> >> >> +  DtPlatformDtbLoaderLib|Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
> >> >> +  FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
> >> >>    SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
> >> >>    PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
> >> >>
> >> >> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
> >> >> new file mode 100644
> >> >> index 000000000000..a93a6027e64d
> >> >> --- /dev/null
> >> >> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
> >> >> @@ -0,0 +1,94 @@
> >> >> +/** @file
> >> >> +*
> >> >> +*  Copyright (c) 2017, Linaro, Ltd. All rights reserved.
> >> >> +*
> >> >> +*  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
> >> >> +*  http://opensource.org/licenses/bsd-license.php
> >> >> +*
> >> >> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> >> >> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> >> >> +*
> >> >> +**/
> >> >> +
> >> >> +#include <PiDxe.h>
> >> >> +
> >> >> +#include <libfdt.h>
> >> >> +#include <Library/BaseLib.h>
> >> >> +#include <Library/DebugLib.h>
> >> >> +#include <Library/DxeServicesLib.h>
> >> >> +#include <Library/MemoryAllocationLib.h>
> >> >> +
> >> >> +#define DTB_PADDING               64
> >> >
> >> > If there isn't a simplish way of determining this value
> >> > arithmetically, can you add a comment explaining why 64 is sufficient?
> >> >
> >>
> >> Something like
> >>
> >> // add enough space for two instances of 'status = "disabled"'
> >>
> >> ?
> >
> > Good enough for me.
> > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> >
> 
> Thanks. Does that cover the delta patch I added in my own reply?

Yes. Sorry, should have been explicit.