On Wed, Mar 22, 2017 at 11:28:02AM +0100, Marcin Wojtas wrote:
> From: Konstantin Porotchkin <kostap@marvell.com>
>
> Move the platform description structure to a common header
> HwDescLib.h. Extend the structure with entries corresponding
> to interfaces located on a first CP slave device, so that both
> CP110 blocks are described. Change the platform description
> structure type to MVHW_NONDISCOVERABLE_DESC. Also add macros
> for array sizes and base addresses.
>
> This change is a beginning of Armada 7k/8k support improvement,
> that will allow to store SoC's description in one place, accessible
> for the drivers and libraries. With that, the PCD description will be
> cleaner and it will also ease adding support for multiple interfaces
> of one kind.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  Platforms/Marvell/Armada/Armada70x0.dsc         |  4 +-
>  Platforms/Marvell/Include/Library/MvHwDescLib.h | 97 +++++++++++++++++++++++++
>  Platforms/Marvell/PciEmulation/PciEmulation.c   | 44 ++---------
>  3 files changed, 104 insertions(+), 41 deletions(-)
>  create mode 100644 Platforms/Marvell/Include/Library/MvHwDescLib.h
>
> diff --git a/Platforms/Marvell/Armada/Armada70x0.dsc b/Platforms/Marvell/Armada/Armada70x0.dsc
> index 83842da..ab8fa3d 100644
> --- a/Platforms/Marvell/Armada/Armada70x0.dsc
> +++ b/Platforms/Marvell/Armada/Armada70x0.dsc
> @@ -140,8 +140,8 @@
>    gMarvellTokenSpaceGuid.PcdPp2XlgDevSize|0x1000
>
>    #PciEmulation
> -  gMarvellTokenSpaceGuid.PcdPciEXhci|{ 0x1, 0x1 }
> -  gMarvellTokenSpaceGuid.PcdPciEAhci|{ 0x1 }
> +  gMarvellTokenSpaceGuid.PcdPciEXhci|{ 0x1, 0x1, 0x0, 0x0 }
> +  gMarvellTokenSpaceGuid.PcdPciEAhci|{ 0x1, 0x0 }
>    gMarvellTokenSpaceGuid.PcdPciESdhci|{ 0x1, 0x1 }
>
>    #ResetLib
> diff --git a/Platforms/Marvell/Include/Library/MvHwDescLib.h b/Platforms/Marvell/Include/Library/MvHwDescLib.h
> new file mode 100644
> index 0000000..47bc4ce
> --- /dev/null
> +++ b/Platforms/Marvell/Include/Library/MvHwDescLib.h
> @@ -0,0 +1,97 @@
> +/********************************************************************************
> +Copyright (C) 2017 Marvell International Ltd.
> +
> +Marvell BSD License Option
> +
> +If you received this File from Marvell, you may opt to use, redistribute and/or
> +modify this File under the following licensing terms.
> +Redistribution and use in source and binary forms, with or without modification,
> +are permitted provided that the following conditions are met:
> +
> +* Redistributions of source code must retain the above copyright notice,
> +  this list of conditions and the following disclaimer.
> +
> +* Redistributions in binary form must reproduce the above copyright
> +  notice, this list of conditions and the following disclaimer in the
> +  documentation and/or other materials provided with the distribution.
> +
> +* Neither the name of Marvell nor the names of its contributors may be
> +  used to endorse or promote products derived from this software without
> +  specific prior written permission.
> +
> +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
> +ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> +WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> +DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
> +ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> +(INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> +LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> +ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
> +SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> +
> +*******************************************************************************/
> +
> +#ifndef __MVHWDESCLIB_H__
> +#define __MVHWDESCLIB_H__
> +
> +#include <Library/NonDiscoverableDeviceRegistrationLib.h>
> +
> +//
> +// NonDiscoverable devices description template definition
> +//
> +#define MVHW_MAX_XHCI_DEVS         4
> +#define MVHW_MAX_AHCI_DEVS         4
> +#define MVHW_MAX_SDHCI_DEVS        4
> +
> +typedef struct {
> +  // XHCI
> +  UINT8 XhciDevCount;
> +  UINTN XhciBaseAddresses[MVHW_MAX_XHCI_DEVS];
> +  UINTN XhciMemSize[MVHW_MAX_XHCI_DEVS];
> +  NON_DISCOVERABLE_DEVICE_DMA_TYPE XhciDmaType[MVHW_MAX_XHCI_DEVS];
> +  // AHCI
> +  UINT8 AhciDevCount;
> +  UINTN AhciBaseAddresses[MVHW_MAX_AHCI_DEVS];
> +  UINTN AhciMemSize[MVHW_MAX_AHCI_DEVS];
> +  NON_DISCOVERABLE_DEVICE_DMA_TYPE AhciDmaType[MVHW_MAX_AHCI_DEVS];
> +  // SDHCI
> +  UINT8 SdhciDevCount;
> +  UINTN SdhciBaseAddresses[MVHW_MAX_SDHCI_DEVS];
> +  UINTN SdhciMemSize[MVHW_MAX_SDHCI_DEVS];
> +  NON_DISCOVERABLE_DEVICE_DMA_TYPE SdhciDmaType[MVHW_MAX_SDHCI_DEVS];
> +} MVHW_NONDISCOVERABLE_DESC;
> +
> +//
> +// Platform description of NonDiscoverable devices
> +//
> +#define MVHW_CP0_XHCI0_BASE        0xF2500000
> +#define MVHW_CP0_XHCI1_BASE        0xF2510000
> +#define MVHW_CP1_XHCI0_BASE        0xF4500000
> +#define MVHW_CP1_XHCI1_BASE        0xF4510000
> +
> +#define MVHW_CP0_AHCI0_BASE        0xF2540000
> +#define MVHW_CP1_AHCI0_BASE        0xF4540000
> +
> +#define MVHW_AP0_SDHCI0_BASE       0xF06E0000
> +#define MVHW_CP0_SDHCI0_BASE       0xF2780000
> +
> +#define DECLARE_A7K8K_NONDISCOVERABLE_TEMPLATE   \
> +STATIC \
> +MVHW_NONDISCOVERABLE_DESC mA7k8kNonDiscoverableDescTemplate = {\
> +  4, /* XHCI */\
> +  { MVHW_CP0_XHCI0_BASE, MVHW_CP0_XHCI1_BASE, MVHW_CP1_XHCI0_BASE, MVHW_CP1_XHCI1_BASE },\
> +  { SIZE_16KB, SIZE_16KB, SIZE_16KB, SIZE_16KB },\
> +  { NonDiscoverableDeviceDmaTypeCoherent, NonDiscoverableDeviceDmaTypeCoherent,\
> +    NonDiscoverableDeviceDmaTypeCoherent, NonDiscoverableDeviceDmaTypeCoherent },\
> +  2, /* AHCI */\
> +  { MVHW_CP0_AHCI0_BASE, MVHW_CP1_AHCI0_BASE },\
> +  { SIZE_8KB, SIZE_8KB },\
> +  { NonDiscoverableDeviceDmaTypeCoherent, NonDiscoverableDeviceDmaTypeCoherent },\
> +  2, /* SDHCI */\
> +  { MVHW_AP0_SDHCI0_BASE, MVHW_CP0_SDHCI0_BASE },\
> +  { SIZE_1KB, SIZE_1KB },\
> +  { NonDiscoverableDeviceDmaTypeCoherent, NonDiscoverableDeviceDmaTypeCoherent }\
> +}
> +
> +#endif /* __MVHWDESCLIB_H__ */
> diff --git a/Platforms/Marvell/PciEmulation/PciEmulation.c b/Platforms/Marvell/PciEmulation/PciEmulation.c
> index 491b886..7e9727e 100644
> --- a/Platforms/Marvell/PciEmulation/PciEmulation.c
> +++ b/Platforms/Marvell/PciEmulation/PciEmulation.c
> @@ -35,46 +35,12 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  #include <PiDxe.h>
>
>  #include <Library/DebugLib.h>
> -#include <Library/NonDiscoverableDeviceRegistrationLib.h>

I would actually opt to keep this in here, for clarity.
The existing code still makes use of functions defined by that header.
I can fold that in as well if you agree.
If so:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
(I am being this helpful because I'm leaving for holiday in 3h :)

This version is a huge improvement in clarity b.t.w., thanks!

/
    Leif

> +#include <Library/MvHwDescLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
>
>  #include <Protocol/EmbeddedExternalDevice.h>
>
> -//
> -// Platform description
> -//
> -typedef struct {
> -  // XHCI
> -  UINT8 XhciDevCount;
> -  UINTN XhciBaseAddresses[4];
> -  UINTN XhciMemSize[4];
> -  NON_DISCOVERABLE_DEVICE_DMA_TYPE XhciDmaType[4];
> -  // AHCI
> -  UINT8 AhciDevCount;
> -  UINTN AhciBaseAddresses[4];
> -  UINTN AhciMemSize[4];
> -  NON_DISCOVERABLE_DEVICE_DMA_TYPE AhciDmaType[4];
> -  // SDHCI
> -  UINT8 SdhciDevCount;
> -  UINTN SdhciBaseAddresses[4];
> -  UINTN SdhciMemSize[4];
> -  NON_DISCOVERABLE_DEVICE_DMA_TYPE SdhciDmaType[4];
> -} PCIE_PLATFORM_DESC;
> -
> -STATIC PCIE_PLATFORM_DESC mA70x0PlatDescTemplate = {
> -  2, // XHCI
> -  { 0xF2500000, 0xF2510000 },
> -  { SIZE_16KB, SIZE_16KB },
> -  { NonDiscoverableDeviceDmaTypeCoherent, NonDiscoverableDeviceDmaTypeCoherent },
> -  1, // AHCI
> -  { 0xF2540000 },
> -  { SIZE_8KB },
> -  { NonDiscoverableDeviceDmaTypeCoherent },
> -  2, // SDHCI
> -  { 0xF06E0000, 0xF2780000 },
> -  { SIZE_1KB, SIZE_1KB },
> -  { NonDiscoverableDeviceDmaTypeCoherent, NonDiscoverableDeviceDmaTypeCoherent }
> -};
> +DECLARE_A7K8K_NONDISCOVERABLE_TEMPLATE;
>
>  //
>  // Tables with used devices
> @@ -93,7 +59,7 @@ EFI_STATUS
>  PciEmulationInitXhci (
>    )
>  {
> -  PCIE_PLATFORM_DESC *Desc = &mA70x0PlatDescTemplate;
> +  MVHW_NONDISCOVERABLE_DESC *Desc = &mA7k8kNonDiscoverableDescTemplate;
>    EFI_STATUS Status;
>    UINT8 i;
>
> @@ -130,7 +96,7 @@ EFI_STATUS
>  PciEmulationInitAhci (
>    )
>  {
> -  PCIE_PLATFORM_DESC *Desc = &mA70x0PlatDescTemplate;
> +  MVHW_NONDISCOVERABLE_DESC *Desc = &mA7k8kNonDiscoverableDescTemplate;
>    EFI_STATUS Status;
>    UINT8 i;
>
> @@ -167,7 +133,7 @@ EFI_STATUS
>  PciEmulationInitSdhci (
>    )
>  {
> -  PCIE_PLATFORM_DESC *Desc = &mA70x0PlatDescTemplate;
> +  MVHW_NONDISCOVERABLE_DESC *Desc = &mA7k8kNonDiscoverableDescTemplate;
>    EFI_STATUS Status;
>    UINT8 i;
>
> --
> 1.8.3.1
>