No Ptr suffixes, please.> +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;
> + 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?
> + /* 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.
> + 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]); On the other hand, here is a whole series of whitespace-only changes.
>
> 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 +
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.
> 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?
> + break;
> case PHY_TYPE_SATA2:
> case PHY_TYPE_SATA3:
> - Status = ComPhySataPowerUp(Lane, HpipeBaseAddr, ComPhyBaseAddr);
> + Status = ComPhySataPowerUp (Lane, HpipeBaseAddr, ComPhyBaseAddr, 1);
#define for that 1?
Umm, no.
> 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 }}
> -