Hi Leif,


> +DECLARE_A7K8K_STORAGE_TEMPLATE;
> +
>  /*
>   * For CP-110 we have 2 Selector registers "PHY Selectors"
>   * and " PIPE Selectors".
> @@ -702,24 +705,37 @@ UINTN
>  ComPhySataPowerUp (
>    IN UINT32 Lane,
>    IN EFI_PHYSICAL_ADDRESS HpipeBase,
> -  IN EFI_PHYSICAL_ADDRESS ComPhyBase
> +  IN EFI_PHYSICAL_ADDRESS ComPhyBase,
> +  IN UINT32 SataHostId
>    )
>  {
>    EFI_STATUS Status;
> +  UINTN  SataArrSize;
> +  UINT8 *SataArrPtr;

No Ptr suffixes, please.

Ok.
 

> +  MVHW_STORAGE_DESC *Desc = &mA7k8kStorageDescTemplate;
>    EFI_PHYSICAL_ADDRESS HpipeAddr = HPIPE_ADDR(HpipeBase, Lane);
>    EFI_PHYSICAL_ADDRESS SdIpAddr = SD_ADDR(HpipeBase, Lane);
>    EFI_PHYSICAL_ADDRESS ComPhyAddr = COMPHY_ADDR(ComPhyBase, Lane);
> -  EFI_PHYSICAL_ADDRESS SataBase;
>
> -  SataBase = PcdGet32 (PcdSataBaseAddress);
> -  if (SataBase == 0) {
> -    DEBUG((DEBUG_INFO, "ComPhy: SATA address not defined\n"));
> -    return EFI_D_ERROR;
> +  SataArrSize = PcdGetSize (PcdPciEAhci);
> +  SataArrPtr = (UINT8 *) PcdGetPtr (PcdPciEAhci);
> +
> +  if (SataArrPtr == NULL || SataHostId >= SataArrSize) {
> +    DEBUG((DEBUG_ERROR, "ComPhySata: Sata host %d is undefined\n", SataHostId));
> +    return EFI_INVALID_PARAMETER;
>    }
>
> +  if (SataArrPtr[SataHostId] != 1) {

What is "1"? Can we have a descriptive #define instead?

How about adding:
#define DEV_ENABLED   1
in the MvHwDescLib.h?
 

> +    /* This SATA host is not enabled in PCD */
> +    DEBUG((DEBUG_WARN, "ComPhySata: Sata host %d is disabled\n", SataHostId));

Coding-style wise, there sould be a space between DEBUG and ((. Don't
need to change other occurences, but please add it in modified lines.


Ok.
 
> +    return EFI_SUCCESS;
> +  }
> +
> +  DEBUG((DEBUG_INFO, "ComPhySata: Initialize SATA PHYs\n"));
> +
>    DEBUG((DEBUG_INFO, "ComPhySataPowerUp: stage: MAC configuration - power down ComPhy\n"));
>
> -  ComPhySataMacPowerDown (SataBase);
> +  ComPhySataMacPowerDown (Desc->AhciBaseAddresses[SataHostId]);
>
>    DEBUG((DEBUG_INFO, "ComPhy: stage: RFU configurations - hard reset ComPhy\n"));
>
> @@ -735,7 +751,7 @@ ComPhySataPowerUp (
>
>    DEBUG((DEBUG_INFO, "ComPhy: stage: ComPhy power up\n"));
>
> -  ComPhySataPhyPowerUp (SataBase);
> +  ComPhySataPhyPowerUp (Desc->AhciBaseAddresses[SataHostId]);
>
>    DEBUG((DEBUG_INFO, "ComPhy: stage: Check PLL\n"));
>
> @@ -961,11 +977,11 @@ ComPhyMuxCp110 (
>      ComPhyMapPhyData[Lane].Speed = SerdesMap[Lane].Speed;
>    }
>    PtrChipCfg->MuxData = Cp110ComPhyMuxData;
> -  ComPhyMuxInit(PtrChipCfg, ComPhyMapPhyData, ComPhyBaseAddr +
> +  ComPhyMuxInit (PtrChipCfg, ComPhyMapPhyData, ComPhyBaseAddr +

On the other hand, here is a whole series of whitespace-only changes.
Please leave these out of this patch.
If you're happy to do all these fixups as a separate patch preceding
this one, that'd be great. If not, just leave them be - it makes it
much harder to spot the actual functional changes.

They will be removed from here.
 

>      COMMON_SELECTOR_PHY_OFFSET);
>
>    PtrChipCfg->MuxData = Cp110ComPhyPipeMuxData;
> -  ComPhyMuxInit(PtrChipCfg, ComPhyMapPipeData, ComPhyBaseAddr +
> +  ComPhyMuxInit (PtrChipCfg, ComPhyMapPipeData, ComPhyBaseAddr +
>      COMMON_SELECTOR_PIPE_OFFSET);
>
>    /* Fix the Type after check the PHY and PIPE configuration */
> @@ -992,7 +1008,7 @@ ComPhyCp110Init (
>    SerdesMap = PtrChipCfg->MapData;
>
>    /* Config Comphy mux configuration */
> -  ComPhyMuxCp110(PtrChipCfg, SerdesMap);
> +  ComPhyMuxCp110 (PtrChipCfg, SerdesMap);
>
>    /* Check if the first 4 Lanes configured as By-4 */
>    for (Lane = 0, PtrComPhyMap = SerdesMap; Lane < 4; Lane++, PtrComPhyMap++) {
> @@ -1014,23 +1030,25 @@ ComPhyCp110Init (
>      case PHY_TYPE_PCIE1:
>      case PHY_TYPE_PCIE2:
>      case PHY_TYPE_PCIE3:
> -      Status = ComPhyPciePowerUp(Lane, PcieBy4, HpipeBaseAddr, ComPhyBaseAddr);
> +      Status = ComPhyPciePowerUp (Lane, PcieBy4, HpipeBaseAddr, ComPhyBaseAddr);
>        break;
>      case PHY_TYPE_SATA0:
>      case PHY_TYPE_SATA1:
> +      Status = ComPhySataPowerUp (Lane, HpipeBaseAddr, ComPhyBaseAddr, 0);

#define for that 0?

Ok.
 

> +      break;
>      case PHY_TYPE_SATA2:
>      case PHY_TYPE_SATA3:
> -      Status = ComPhySataPowerUp(Lane, HpipeBaseAddr, ComPhyBaseAddr);
> +      Status = ComPhySataPowerUp (Lane, HpipeBaseAddr, ComPhyBaseAddr, 1);

#define for that 1?

Ok.
 

>        break;
>      case PHY_TYPE_USB3_HOST0:
>      case PHY_TYPE_USB3_HOST1:
> -      Status = ComphyUsb3PowerUp(Lane, HpipeBaseAddr, ComPhyBaseAddr);
> +      Status = ComphyUsb3PowerUp (Lane, HpipeBaseAddr, ComPhyBaseAddr);
>        break;
>      case PHY_TYPE_SGMII0:
>      case PHY_TYPE_SGMII1:
>      case PHY_TYPE_SGMII2:
>      case PHY_TYPE_SGMII3:
> -      Status = ComPhySgmiiPowerUp(Lane, PtrComPhyMap->Speed, HpipeBaseAddr,
> +      Status = ComPhySgmiiPowerUp (Lane, PtrComPhyMap->Speed, HpipeBaseAddr,
>          ComPhyBaseAddr);
>        break;
>      default:
> diff --git a/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.inf b/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.inf
> index a7ee1f8..02905a5 100644
> --- a/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.inf
> +++ b/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.inf
> @@ -105,5 +105,4 @@
>    gMarvellTokenSpaceGuid.PcdChip3ComPhySpeeds
>    gMarvellTokenSpaceGuid.PcdChip3ComPhyInvFlags
>
> -  #SATA
> -  gMarvellTokenSpaceGuid.PcdSataBaseAddress
> +  gMarvellTokenSpaceGuid.PcdPciEAhci
> diff --git a/Platforms/Marvell/Marvell.dec b/Platforms/Marvell/Marvell.dec
> index 313eaa6..267b655 100644
> --- a/Platforms/Marvell/Marvell.dec
> +++ b/Platforms/Marvell/Marvell.dec
> @@ -176,9 +176,6 @@
>    gMarvellTokenSpaceGuid.PcdChip3ComPhySpeeds|{ 0x0 }|VOID*|0x30000176
>    gMarvellTokenSpaceGuid.PcdChip3ComPhyInvFlags|{ 0x0 }|VOID*|0x30000177
>
> -#SATA
> -  gMarvellTokenSpaceGuid.PcdSataBaseAddress|0|UINT32|0x4000052
> -
>  #UtmiPhy
>    gMarvellTokenSpaceGuid.PcdUtmiPhyCount|0|UINT32|0x30000205
>    gMarvellTokenSpaceGuid.PcdUtmiPhyRegUsbCfg|{ 0x0 }|VOID*|0x30000206
> @@ -225,4 +222,3 @@
>    gMarvellPhyProtocolGuid                  = { 0x32f48a43, 0x37e3, 0x4acf, { 0x93, 0xc4, 0x3e, 0x57, 0xa7, 0xb0, 0xfb, 0xdc }}
>    gMarvellSpiMasterProtocolGuid            = { 0x23de66a3, 0xf666, 0x4b3e, { 0xaa, 0xa2, 0x68, 0x9b, 0x18, 0xae, 0x2e, 0x19 }}
>    gMarvellSpiFlashProtocolGuid             = { 0x9accb423, 0x5bd2, 0x4fca, { 0x9b, 0x4c, 0x2e, 0x65, 0xfc, 0x25, 0xdf, 0x21 }}
> -

Umm, no.

Ok.

Best regards,
Marcin