Re: [platforms: PATCH v2 5/7] Marvell/Armada80x0Db: Introduce board description library

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 8 October 2018 at 15:28, Marcin Wojtas <mw@semihalf.com> wrote:
> Hi Ard,
>
> pon., 8 paź 2018 o 14:52 Ard Biesheuvel <ard.biesheuvel@linaro.org> napisał(a):
>>
>> On 5 October 2018 at 15:26, Marcin Wojtas <mw@semihalf.com> wrote:
>> > From: Tomasz Michalec <tm@semihalf.com>
>> >
>> > This patch implements ArmadaBoarDescLib library for
>> > Armada8040 Development Board and add to it ArmadaBoardDescSdMmcGet
>> > function with description of connected Xenon host controllers.
>> >
>> > Contributed-under: TianoCore Contribution Agreement 1.1
>> > Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>> > ---
>> >  Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc                                      |  3 +
>> >  Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.inf | 34 ++++++++++
>> >  Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.c   | 66 ++++++++++++++++++++
>> >  3 files changed, 103 insertions(+)
>> >  create mode 100644 Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.inf
>> >  create mode 100644 Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.c
>> >
>> > diff --git a/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc b/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc
>> > index 92e2dc8..42f7bd3 100644
>> > --- a/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc
>> > +++ b/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc
>> > @@ -54,6 +54,9 @@
>> >  [Components.AARCH64]
>> >    Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0Db.inf
>> >
>> > +[LibraryClasses.common]
>> > +  ArmadaBoardDescLib|Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.inf
>> > +
>> >  ################################################################################
>> >  #
>> >  # Pcd Section - list of all EDK II PCD Entries defined by this Platform
>> > diff --git a/Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.inf b/Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.inf
>> > new file mode 100644
>> > index 0000000..2d39d96
>> > --- /dev/null
>> > +++ b/Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.inf
>> > @@ -0,0 +1,34 @@
>> > +## @file
>> > +#
>> > +#  Copyright (C) 2018, Marvell International Ltd. and its affiliates<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
>> > +#  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.
>> > +#
>> > +#
>> > +##
>> > +
>> > +[Defines]
>> > +  INF_VERSION                    = 0x0001001A
>> > +  BASE_NAME                      = Armada80x0DbBoardDescLib
>> > +  FILE_GUID                      = fee9e874-1481-4b4f-9882-966bd0d1310f
>> > +  MODULE_TYPE                    = BASE
>> > +  VERSION_STRING                 = 1.0
>> > +  LIBRARY_CLASS                  = ArmadaBoardDescLib
>> > +
>> > +[Sources]
>> > +  Armada80x0DbBoardDescLib.c
>> > +
>> > +[Packages]
>> > +  MdeModulePkg/MdeModulePkg.dec
>> > +  MdePkg/MdePkg.dec
>> > +  Silicon/Marvell/Marvell.dec
>> > +
>> > +[LibraryClasses]
>> > +  DebugLib
>> > +  IoLib
>> > diff --git a/Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.c b/Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.c
>> > new file mode 100644
>> > index 0000000..00d696d
>> > --- /dev/null
>> > +++ b/Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.c
>> > @@ -0,0 +1,66 @@
>> > +/**
>> > +*
>> > +*  Copyright (C) 2018, Marvell International Ltd. and its affiliates.
>> > +*
>> > +*  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 <Uefi.h>
>> > +
>> > +#include <Library/ArmadaBoardDescLib.h>
>> > +#include <Library/BaseMemoryLib.h>
>> > +#include <Library/DebugLib.h>
>> > +#include <Library/IoLib.h>
>> > +#include <Library/MemoryAllocationLib.h>
>> > +#include <Library/UefiBootServicesTableLib.h>
>> > +
>> > +//
>> > +// Order of devices in SdMmcDescTemplate has to be in par with ArmadaSoCDescLib
>> > +//
>> > +STATIC
>> > +MV_BOARD_SDMMC_DESC mSdMmcDescTemplate[] = {
>> > +  { /* eMMC 0xF06E0000 */
>> > +    0,     /* SOC will be filled by MvBoardDescDxe */
>> > +    0,     /* SdMmcDevCount will be filled by MvBoardDescDxe */
>> > +    TRUE,  /* Xenon1v8Enabled */
>> > +    TRUE,  /* Xenon8BitBusEnabled */
>> > +    TRUE,  /* XenonSlowModeEnabled */
>> > +    0x40,  /* XenonTuningStepDivisor */
>> > +    EmbeddedSlot /* SlotType */
>> > +  },
>> > +  { /* SD/MMC 0xF2780000 */
>> > +    0,     /* SOC will be filled by MvBoardDescDxe */
>> > +    0,     /* SdMmcDevCount will be filled by MvBoardDescDxe */
>> > +    FALSE, /* Xenon1v8Enabled */
>> > +    FALSE, /* Xenon8BitBusEnabled */
>> > +    FALSE, /* XenonSlowModeEnabled */
>> > +    0x19,  /* XenonTuningStepDivisor */
>> > +    EmbeddedSlot /* SlotType */
>>
>> Isn't the SD removable on MacchiatoBin? Or are we not able to support
>> that in UEFI?
>>
>>
>
> UEFI CardDetect routine does not support CD via GPIO. On MacchiatoBin,
> in case we define SD as 'RemovableSlot', it remains undetected.
>

OK, fair enough. We could look into adding support for this to the
override protocol in the future, but I don't think it really matters
tbh.