Re: [platforms: PATCH v2 14/25] Marvell/Library: ComPhyLib: Get AHCI data with MARVELL_BOARD_DESC

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

On Sun, Jun 17, 2018 at 10:11:54PM +0200, Marcin Wojtas wrote:
> ComPhy Library used to get Armada7k8k AHCI/SDMMC/XHCI controller
> description from hardcoded values stored in the header file
> MvHwDescLib.h. As a result it is very hard to support other
> Armada SoC families with this library.
> 
> This patch updates the driver to get AHCI controller
> description from newly introduced MARVELL_BOARD_DESC protocol,
> and removes the dependency on the hardcoded structures.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf |  1 -
>  Silicon/Marvell/Library/ComPhyLib/ComPhyLib.inf                    |  6 +-
>  Silicon/Marvell/Include/Library/MvHwDescLib.h                      | 60 --------------------
>  Silicon/Marvell/Library/ComPhyLib/ComPhyLib.h                      |  4 ++
>  Silicon/Marvell/Library/ComPhyLib/ComPhyCp110.c                    | 50 ++++++++--------
>  5 files changed, 35 insertions(+), 86 deletions(-)
> 
> diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf
> index f2c173c..e888566 100644
> --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf
> +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf
> @@ -47,7 +47,6 @@
>  
>  [LibraryClasses]
>    ArmLib
> -  ComPhyLib
>    DebugLib
>    MemoryAllocationLib
>    MppLib
> diff --git a/Silicon/Marvell/Library/ComPhyLib/ComPhyLib.inf b/Silicon/Marvell/Library/ComPhyLib/ComPhyLib.inf
> index ce0af54..f36c701 100644
> --- a/Silicon/Marvell/Library/ComPhyLib/ComPhyLib.inf
> +++ b/Silicon/Marvell/Library/ComPhyLib/ComPhyLib.inf
> @@ -52,12 +52,16 @@
>    PcdLib
>    SampleAtResetLib
>    IoLib
> +  UefiBootServicesTableLib
>  
>  [Sources.common]
>    ComPhyLib.c
>    ComPhyCp110.c
>    ComPhyMux.c
>  
> +[Protocols]
> +  gMarvellBoardDescProtocolGuid  ## CONSUMES
> +
>  [FixedPcd]
>    gMarvellTokenSpaceGuid.PcdComPhyDevices
>  
> @@ -80,5 +84,3 @@
>    gMarvellTokenSpaceGuid.PcdChip3ComPhyTypes
>    gMarvellTokenSpaceGuid.PcdChip3ComPhySpeeds
>    gMarvellTokenSpaceGuid.PcdChip3ComPhyInvFlags
> -
> -  gMarvellTokenSpaceGuid.PcdPciEAhci
> diff --git a/Silicon/Marvell/Include/Library/MvHwDescLib.h b/Silicon/Marvell/Include/Library/MvHwDescLib.h
> index 5fd514c..9f383f4 100644
> --- a/Silicon/Marvell/Include/Library/MvHwDescLib.h
> +++ b/Silicon/Marvell/Include/Library/MvHwDescLib.h
> @@ -36,7 +36,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  #define __MVHWDESCLIB_H__
>  
>  #include <Library/MvComPhyLib.h>
> -#include <Library/NonDiscoverableDeviceRegistrationLib.h>
>  
>  //
>  // Helper macros
> @@ -80,31 +79,6 @@ typedef struct {
>  } MVHW_MDIO_DESC;
>  
>  //
> -// 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 CommonPhy devices
>  //
>  #define MVHW_CP0_COMPHY_BASE       0xF2441000
> @@ -155,38 +129,4 @@ MVHW_MDIO_DESC mA7k8kMdioDescTemplate = {\
>    { MVHW_CP0_MDIO_BASE, MVHW_CP1_MDIO_BASE }\
>  }
>  
> -//
> -// 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_CP0_AHCI0_ID          0
> -#define MVHW_CP1_AHCI0_BASE        0xF4540000
> -#define MVHW_CP1_AHCI0_ID          1
> -
> -#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/Silicon/Marvell/Library/ComPhyLib/ComPhyLib.h b/Silicon/Marvell/Library/ComPhyLib/ComPhyLib.h
> index c675d74..090116d 100644
> --- a/Silicon/Marvell/Library/ComPhyLib/ComPhyLib.h
> +++ b/Silicon/Marvell/Library/ComPhyLib/ComPhyLib.h
> @@ -35,6 +35,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  #ifndef __COMPHY_H__
>  #define __COMPHY_H__
>  
> +#include <Uefi.h>
>  #include <Library/ArmLib.h>
>  #include <Library/ArmPlatformLib.h>
>  #include <Library/DebugLib.h>
> @@ -43,6 +44,9 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  #include <Library/MvComPhyLib.h>
>  #include <Library/IoLib.h>
>  #include <Library/TimerLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +
> +#include <Protocol/BoardDesc.h>
>  
>  #define MAX_LANE_OPTIONS          10
>  
> diff --git a/Silicon/Marvell/Library/ComPhyLib/ComPhyCp110.c b/Silicon/Marvell/Library/ComPhyLib/ComPhyCp110.c
> index 09994ca..5e0ebf6 100755
> --- a/Silicon/Marvell/Library/ComPhyLib/ComPhyCp110.c
> +++ b/Silicon/Marvell/Library/ComPhyLib/ComPhyCp110.c
> @@ -33,7 +33,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  *******************************************************************************/
>  
>  #include "ComPhyLib.h"
> -#include <Library/MvHwDescLib.h>
>  #include <Library/SampleAtResetLib.h>
>  
>  #define SD_LANE_ADDR_WIDTH          0x1000
> @@ -46,8 +45,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  #define CP110_PCIE_REF_CLK_TYPE0    0
>  #define CP110_PCIE_REF_CLK_TYPE12   1
>  
> -DECLARE_A7K8K_NONDISCOVERABLE_TEMPLATE;
> -
>  /*
>   * For CP-110 we have 2 Selector registers "PHY Selectors"
>   * and " PIPE Selectors".
> @@ -1138,36 +1135,23 @@ ComPhySataCheckPll (
>  STATIC
>  UINTN
>  ComPhySataPowerUp (
> +  IN UINTN ChipId,
>    IN UINT32 Lane,
>    IN EFI_PHYSICAL_ADDRESS HpipeBase,
>    IN EFI_PHYSICAL_ADDRESS ComPhyBase,
> -  IN UINT8 SataHostId
> +  IN MV_BOARD_AHCI_DESC *Desc
>    )
>  {
>    EFI_STATUS Status;
> -  UINT8 *SataDeviceTable;
> -  MVHW_NONDISCOVERABLE_DESC *Desc = &mA7k8kNonDiscoverableDescTemplate;
>    EFI_PHYSICAL_ADDRESS HpipeAddr = HPIPE_ADDR(HpipeBase, Lane);
>    EFI_PHYSICAL_ADDRESS SdIpAddr = SD_ADDR(HpipeBase, Lane);
>    EFI_PHYSICAL_ADDRESS ComPhyAddr = COMPHY_ADDR(ComPhyBase, Lane);
>  
> -  SataDeviceTable = (UINT8 *) PcdGetPtr (PcdPciEAhci);
> -
> -  if (SataDeviceTable == NULL || SataHostId >= PcdGetSize (PcdPciEAhci)) {
> -    DEBUG ((DEBUG_ERROR, "ComPhySata: Sata host %d is undefined\n", SataHostId));
> -    return EFI_INVALID_PARAMETER;
> -  }
> -
> -  if (!MVHW_DEV_ENABLED (Sata, SataHostId)) {
> -    DEBUG ((DEBUG_ERROR, "ComPhySata: Sata host %d is disabled\n", SataHostId));
> -    return EFI_INVALID_PARAMETER;
> -  }
> -
>    DEBUG ((DEBUG_INFO, "ComPhySata: Initialize SATA PHYs\n"));
>  
>    DEBUG((DEBUG_INFO, "ComPhySataPowerUp: stage: MAC configuration - power down ComPhy\n"));
>  
> -  ComPhySataMacPowerDown (Desc->AhciBaseAddresses[SataHostId]);
> +  ComPhySataMacPowerDown (Desc[ChipId].SoC->AhciBaseAddress);
>  
>    DEBUG((DEBUG_INFO, "ComPhy: stage: RFU configurations - hard reset ComPhy\n"));
>  
> @@ -1183,7 +1167,7 @@ ComPhySataPowerUp (
>  
>    DEBUG((DEBUG_INFO, "ComPhy: stage: ComPhy power up\n"));
>  
> -  ComPhySataPhyPowerUp (Desc->AhciBaseAddresses[SataHostId]);
> +  ComPhySataPhyPowerUp (Desc[ChipId].SoC->AhciBaseAddress);
>  
>    DEBUG((DEBUG_INFO, "ComPhy: stage: Check PLL\n"));
>  
> @@ -1884,6 +1868,8 @@ ComPhyCp110Init (
>    EFI_STATUS Status;
>    COMPHY_MAP *PtrComPhyMap, *SerdesMap;
>    EFI_PHYSICAL_ADDRESS ComPhyBaseAddr, HpipeBaseAddr;
> +  MARVELL_BOARD_DESC_PROTOCOL *BoardDescProtocol;
> +  MV_BOARD_AHCI_DESC *AhciBoardDesc;
>    UINT32 ComPhyMaxCount, Lane;
>    UINT32 PcieWidth = 0;
>    UINT8 ChipId;
> @@ -1927,11 +1913,29 @@ ComPhyCp110Init (
>        break;
>      case COMPHY_TYPE_SATA0:
>      case COMPHY_TYPE_SATA1:
> -      Status = ComPhySataPowerUp (Lane, HpipeBaseAddr, ComPhyBaseAddr, MVHW_CP0_AHCI0_ID);
> -      break;
>      case COMPHY_TYPE_SATA2:
>      case COMPHY_TYPE_SATA3:
> -      Status = ComPhySataPowerUp (Lane, HpipeBaseAddr, ComPhyBaseAddr, MVHW_CP1_AHCI0_ID);
> +      /* Obtain AHCI board description */
> +      Status = gBS->LocateProtocol (&gMarvellBoardDescProtocolGuid,
> +                      NULL,
> +                      (VOID **)&BoardDescProtocol);
> +      if (EFI_ERROR (Status)) {
> +        break;
> +      }
> +
> +      Status = BoardDescProtocol->BoardDescAhciGet (BoardDescProtocol,
> +                                    &AhciBoardDesc);
> +      if (EFI_ERROR (Status)) {
> +        break;
> +      }
> +
> +      Status = ComPhySataPowerUp (ChipId,
> +                 Lane,
> +                 HpipeBaseAddr,
> +                 ComPhyBaseAddr,
> +                 AhciBoardDesc);
> +
> +      BoardDescProtocol->BoardDescFree (AhciBoardDesc);
>        break;
>      case COMPHY_TYPE_USB3_HOST0:
>      case COMPHY_TYPE_USB3_HOST1:
> -- 
> 2.7.4
>