Re: [platforms: PATCH v4 6/7] Marvell/Drivers: MvBoardDesc: Extend information for SdMmc

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

On Sat, 10 Nov 2018 at 00:02, Marcin Wojtas <mw@semihalf.com> wrote:
>
> From: Tomasz Michalec <tm@semihalf.com>
>
> Extend MvBoardDescSdMmcGet function to fill MV_BOARD_SDMMC_DESC
> with Xenon specific info obtained from ArmadaBoardDescLib.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---
>  Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf |  1 +
>  Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c   | 24 +++++++++++++-------
>  2 files changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf
> index 41f72d6..0b93948 100644
> --- a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf
> +++ b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf
> @@ -47,6 +47,7 @@
>    Silicon/Marvell/Marvell.dec
>
>  [LibraryClasses]
> +  ArmadaBoardDescLib
>    ArmadaSoCDescLib
>    DebugLib
>    MemoryAllocationLib
> diff --git a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
> index 39dc06c..f71bfc4 100644
> --- a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
> +++ b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
> @@ -270,6 +270,7 @@ MvBoardDescSdMmcGet (
>  {
>    UINT8 *SdMmcDeviceEnabled;
>    UINTN SdMmcCount, SdMmcDeviceTableSize, SdMmcIndex, Index;
> +  UINTN SdMmcDevCount;
>    MV_BOARD_SDMMC_DESC *BoardDesc;
>    MV_SOC_SDMMC_DESC *SoCDesc;
>    EFI_STATUS Status;
> @@ -280,6 +281,13 @@ MvBoardDescSdMmcGet (
>      return Status;
>    }
>
> +  /* Get per-board configuration of the controllers */
> +  Status = ArmadaBoardDescSdMmcGet (&SdMmcDevCount, &BoardDesc);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "%a: ArmadaBoardDescSdMmcGet filed\n", __FUNCTION__));
> +    return Status;
> +  }
> +
>    /*
>     * Obtain table with enabled SDMMC controllers
>     * which is represented as an array of UINT8 values
> @@ -294,18 +302,12 @@ MvBoardDescSdMmcGet (
>    SdMmcDeviceTableSize = PcdGetSize (PcdPciESdhci);
>
>    /* Check if PCD with SDMMC controllers is correctly defined */
> -  if (SdMmcDeviceTableSize > SdMmcCount) {
> +  if ((SdMmcDeviceTableSize > SdMmcCount) ||
> +      (SdMmcDeviceTableSize < SdMmcDevCount)) {
>      DEBUG ((DEBUG_ERROR, "%a: Wrong PcdPciESdhci format\n", __FUNCTION__));
>      return EFI_INVALID_PARAMETER;
>    }
>
> -  /* Allocate and fill board description */
> -  BoardDesc = AllocateZeroPool (SdMmcDeviceTableSize * sizeof (MV_BOARD_SDMMC_DESC));
> -  if (BoardDesc == NULL) {
> -    DEBUG ((DEBUG_ERROR, "%a: Cannot allocate memory\n", __FUNCTION__));
> -    return EFI_OUT_OF_RESOURCES;
> -  }
> -
>    SdMmcIndex = 0;
>    for (Index = 0; Index < SdMmcDeviceTableSize; Index++) {
>      if (!SdMmcDeviceEnabled[Index]) {
> @@ -313,6 +315,12 @@ MvBoardDescSdMmcGet (
>        continue;
>      }
>
> +    if (SdMmcIndex >= SdMmcDevCount) {
> +      DEBUG ((DEBUG_ERROR,
> +        "%a: More enabled devices than returned by ArmadaBoardDescSdMmcGet\n",
> +        __FUNCTION__));
> +      return EFI_INVALID_PARAMETER;
> +    }
>      BoardDesc[SdMmcIndex].SoC = &SoCDesc[Index];
>      SdMmcIndex++;
>    }
> --
> 2.7.4
>