Hi,
I submit v2 of Armada 7k/8k improvements. It addresses all comments from review and add some minor improvements. Details can be found in the changelog and commit logs.
The commits are also available in the github: https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/opp-...
Any remarks or comments will be very welcome.
Best regards, Marcin
Changelog v1 -> v2: * 1/4: Change type of COMPHY_CHIP_INIT callback to VOID and rework Status usage * 2/4: Add 'Reviewed-by' * 3/4: - Sort includes in header - Rename 'storage' to 'nondiscoverable' in all macros and names - Add defines for array sizes in definition - Add defines for base addresses, so that they describe HW block and device - Add comments for sections * 4/4: - Remove 'Ptr' suffixes - Move and rename DEV_ENABLED macro from PciEmulation driver - use it in ComPhy code - Simplify obtaining enabled Sata ports and do not return EFI_SUCCESS on PCD mismatch (lane enabled in ComPhy, despite the controller disabled) - Add macros for controller ID's - Remove unrelated style fixes - Rewrite error print information in ComPhyCp110Init, because from now we have more root causes than PLL
Konstantin Porotchkin (4): Platforms/Marvell: ComPhyLib: Fix compilation warning Platforms/Marvell: Add support for COMPHY on CP slaves Platform/Marvell: Extend and share the platform description Platform/Marvell: ComPhyLib: Enable SATA PHY init for multiple devices
Platforms/Marvell/Armada/Armada70x0.dsc | 9 +- Platforms/Marvell/Include/Library/MvHwDescLib.h | 106 ++++++++++++++++++++++ Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c | 46 ++++++---- Platforms/Marvell/Library/ComPhyLib/ComPhyLib.c | 8 +- Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h | 4 +- Platforms/Marvell/Library/ComPhyLib/ComPhyLib.inf | 3 +- Platforms/Marvell/Marvell.dec | 4 - Platforms/Marvell/PciEmulation/PciEmulation.c | 52 ++--------- 8 files changed, 156 insertions(+), 76 deletions(-) create mode 100644 Platforms/Marvell/Include/Library/MvHwDescLib.h
From: Konstantin Porotchkin kostap@marvell.com
Fix compilation warning triggered by GCC 4.9. On the occasion change type of COMPHY_CHIP_INIT callback to VOID - the function is responsible for initializing all PHY's independently and returning overall Status does not make sense. For DEBUG build add ASSERT for switch default in case the SerDes lane type is not properly defined.
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/Library/ComPhyLib/ComPhyCp110.c | 7 +++---- Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c b/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c index df8731d..bee3bbf 100755 --- a/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c +++ b/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c @@ -975,12 +975,12 @@ ComPhyMuxCp110 ( SerdesMap[Lane].Type = PHY_TYPE_UNCONNECTED; }
-EFI_STATUS +VOID ComPhyCp110Init ( IN CHIP_COMPHY_CONFIG *PtrChipCfg ) { - EFI_STATUS Status; + EFI_STATUS Status = EFI_SUCCESS; COMPHY_MAP *PtrComPhyMap, *SerdesMap; EFI_PHYSICAL_ADDRESS ComPhyBaseAddr, HpipeBaseAddr; UINT32 ComPhyMaxCount, Lane; @@ -1036,12 +1036,11 @@ ComPhyCp110Init ( default: DEBUG((DEBUG_ERROR, "Unknown SerDes Type, skip initialize SerDes %d\n", Lane)); + ASSERT (FALSE); break; } if (EFI_ERROR(Status)) DEBUG((DEBUG_ERROR, "PLL is not locked - Failed to initialize Lane %d\n", Lane)); } - - return EFI_SUCCESS; } diff --git a/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h b/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h index 48cafc9..945f266 100644 --- a/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h +++ b/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h @@ -394,7 +394,7 @@ typedef struct { } PCD_LANE_MAP;
typedef -EFI_STATUS +VOID (*COMPHY_CHIP_INIT) ( IN CHIP_COMPHY_CONFIG *PtrChipCfg ); @@ -417,7 +417,7 @@ ComPhyMuxInit ( IN EFI_PHYSICAL_ADDRESS SelectorBase );
-EFI_STATUS +VOID ComPhyCp110Init ( IN CHIP_COMPHY_CONFIG * First );
On Wed, Mar 22, 2017 at 11:28:00AM +0100, Marcin Wojtas wrote:
From: Konstantin Porotchkin kostap@marvell.com
Fix compilation warning triggered by GCC 4.9. On the occasion change type of COMPHY_CHIP_INIT callback to VOID - the function is responsible for initializing all PHY's independently and returning overall Status does not make sense. For DEBUG build add ASSERT for switch default in case the SerDes lane type is not properly defined.
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/Library/ComPhyLib/ComPhyCp110.c | 7 +++---- Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c b/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c index df8731d..bee3bbf 100755 --- a/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c +++ b/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c @@ -975,12 +975,12 @@ ComPhyMuxCp110 ( SerdesMap[Lane].Type = PHY_TYPE_UNCONNECTED; } -EFI_STATUS +VOID ComPhyCp110Init ( IN CHIP_COMPHY_CONFIG *PtrChipCfg ) {
- EFI_STATUS Status;
- EFI_STATUS Status = EFI_SUCCESS;
This assignment shouldn't be necessary with the rest of the changes? I can fold that in on commit if you agree.
If so: Reviewed-by: Leif Lindholm leif.lindholm@linaro.org
COMPHY_MAP *PtrComPhyMap, *SerdesMap; EFI_PHYSICAL_ADDRESS ComPhyBaseAddr, HpipeBaseAddr; UINT32 ComPhyMaxCount, Lane; @@ -1036,12 +1036,11 @@ ComPhyCp110Init ( default: DEBUG((DEBUG_ERROR, "Unknown SerDes Type, skip initialize SerDes %d\n", Lane));
ASSERT (FALSE); break;
- return EFI_SUCCESS;
} diff --git a/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h b/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h index 48cafc9..945f266 100644 --- a/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h +++ b/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h @@ -394,7 +394,7 @@ typedef struct { } PCD_LANE_MAP; typedef -EFI_STATUS +VOID (*COMPHY_CHIP_INIT) ( IN CHIP_COMPHY_CONFIG *PtrChipCfg ); @@ -417,7 +417,7 @@ ComPhyMuxInit ( IN EFI_PHYSICAL_ADDRESS SelectorBase ); -EFI_STATUS +VOID ComPhyCp110Init ( IN CHIP_COMPHY_CONFIG * First ); -- 1.8.3.1
Leif,
2017-03-22 13:36 GMT+01:00 Leif Lindholm leif.lindholm@linaro.org:
On Wed, Mar 22, 2017 at 11:28:00AM +0100, Marcin Wojtas wrote:
From: Konstantin Porotchkin kostap@marvell.com
Fix compilation warning triggered by GCC 4.9. On the occasion change type of COMPHY_CHIP_INIT callback to VOID - the function is responsible for initializing all PHY's independently and returning overall Status does not make sense. For DEBUG build add ASSERT for switch default in case the SerDes lane type is not properly defined.
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/Library/ComPhyLib/ComPhyCp110.c | 7 +++---- Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c
b/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c
index df8731d..bee3bbf 100755 --- a/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c +++ b/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c @@ -975,12 +975,12 @@ ComPhyMuxCp110 ( SerdesMap[Lane].Type = PHY_TYPE_UNCONNECTED; }
-EFI_STATUS +VOID ComPhyCp110Init ( IN CHIP_COMPHY_CONFIG *PtrChipCfg ) {
- EFI_STATUS Status;
- EFI_STATUS Status = EFI_SUCCESS;
This assignment shouldn't be necessary with the rest of the changes? I can fold that in on commit if you agree.
In for loop we check for Status (if (EFI_ERROR(Status))), so unitialized value condition will still be there. I'll remove assginment from here and do 'Status = EFI_INVALID_PARAMETER' in switch default, so that all paths are covered, ok?
If so: Reviewed-by: Leif Lindholm leif.lindholm@linaro.org
COMPHY_MAP *PtrComPhyMap, *SerdesMap; EFI_PHYSICAL_ADDRESS ComPhyBaseAddr, HpipeBaseAddr; UINT32 ComPhyMaxCount, Lane; @@ -1036,12 +1036,11 @@ ComPhyCp110Init ( default: DEBUG((DEBUG_ERROR, "Unknown SerDes Type, skip initialize SerDes
%d\n",
Lane));
ASSERT (FALSE); break;Lane %d\n",
Lane));}
- return EFI_SUCCESS;
} diff --git a/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h
b/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h
index 48cafc9..945f266 100644 --- a/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h +++ b/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h @@ -394,7 +394,7 @@ typedef struct { } PCD_LANE_MAP;
typedef -EFI_STATUS +VOID (*COMPHY_CHIP_INIT) ( IN CHIP_COMPHY_CONFIG *PtrChipCfg ); @@ -417,7 +417,7 @@ ComPhyMuxInit ( IN EFI_PHYSICAL_ADDRESS SelectorBase );
-EFI_STATUS +VOID ComPhyCp110Init ( IN CHIP_COMPHY_CONFIG * First ); -- 1.8.3.1
On Wed, Mar 22, 2017 at 02:01:24PM +0100, Marcin Wojtas wrote:
Leif,
2017-03-22 13:36 GMT+01:00 Leif Lindholm leif.lindholm@linaro.org:
On Wed, Mar 22, 2017 at 11:28:00AM +0100, Marcin Wojtas wrote:
From: Konstantin Porotchkin kostap@marvell.com
Fix compilation warning triggered by GCC 4.9. On the occasion change type of COMPHY_CHIP_INIT callback to VOID - the function is responsible for initializing all PHY's independently and returning overall Status does not make sense. For DEBUG build add ASSERT for switch default in case the SerDes lane type is not properly defined.
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/Library/ComPhyLib/ComPhyCp110.c | 7 +++---- Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c
b/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c
index df8731d..bee3bbf 100755 --- a/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c +++ b/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c @@ -975,12 +975,12 @@ ComPhyMuxCp110 ( SerdesMap[Lane].Type = PHY_TYPE_UNCONNECTED; }
-EFI_STATUS +VOID ComPhyCp110Init ( IN CHIP_COMPHY_CONFIG *PtrChipCfg ) {
- EFI_STATUS Status;
- EFI_STATUS Status = EFI_SUCCESS;
This assignment shouldn't be necessary with the rest of the changes? I can fold that in on commit if you agree.
In for loop we check for Status (if (EFI_ERROR(Status))), so unitialized value condition will still be there. I'll remove assginment from here and do 'Status = EFI_INVALID_PARAMETER' in switch default, so that all paths are covered, ok?
I think that will look neater, yes. Thanks.
/ Leif
If so: Reviewed-by: Leif Lindholm leif.lindholm@linaro.org
COMPHY_MAP *PtrComPhyMap, *SerdesMap; EFI_PHYSICAL_ADDRESS ComPhyBaseAddr, HpipeBaseAddr; UINT32 ComPhyMaxCount, Lane; @@ -1036,12 +1036,11 @@ ComPhyCp110Init ( default: DEBUG((DEBUG_ERROR, "Unknown SerDes Type, skip initialize SerDes
%d\n",
Lane));
ASSERT (FALSE); break;Lane %d\n",
Lane));}
- return EFI_SUCCESS;
} diff --git a/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h
b/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h
index 48cafc9..945f266 100644 --- a/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h +++ b/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h @@ -394,7 +394,7 @@ typedef struct { } PCD_LANE_MAP;
typedef -EFI_STATUS +VOID (*COMPHY_CHIP_INIT) ( IN CHIP_COMPHY_CONFIG *PtrChipCfg ); @@ -417,7 +417,7 @@ ComPhyMuxInit ( IN EFI_PHYSICAL_ADDRESS SelectorBase );
-EFI_STATUS +VOID ComPhyCp110Init ( IN CHIP_COMPHY_CONFIG * First ); -- 1.8.3.1
From: Konstantin Porotchkin kostap@marvell.com
Extend the COMPHY configuration table by CP-slave entry. Change the chip type to be different for CP-master and CP-slave (Cp110m, Cp110s). This is a preparation commit for adding support for new SoC - Armada 80x0, which comprises two CP110 HW blocks with peripherals.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Konstantin Porotchkin kostap@marvell.com Signed-off-by: Marcin Wojtas mw@semihalf.com Reviewed-by: Leif Lindholm leif.lindholm@linaro.org --- Platforms/Marvell/Armada/Armada70x0.dsc | 2 +- Platforms/Marvell/Library/ComPhyLib/ComPhyLib.c | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/Platforms/Marvell/Armada/Armada70x0.dsc b/Platforms/Marvell/Armada/Armada70x0.dsc index 134ab71..83842da 100644 --- a/Platforms/Marvell/Armada/Armada70x0.dsc +++ b/Platforms/Marvell/Armada/Armada70x0.dsc @@ -103,7 +103,7 @@ gMarvellTokenSpaceGuid.PcdChip0ComPhyBaseAddress|0xF2441000 gMarvellTokenSpaceGuid.PcdChip0Hpipe3BaseAddress|0xF2120000 gMarvellTokenSpaceGuid.PcdChip0ComPhyMuxBitCount|4 - gMarvellTokenSpaceGuid.PcdChip0Compatible|L"Cp110" + gMarvellTokenSpaceGuid.PcdChip0Compatible|L"Cp110m"
#UtmiPhy gMarvellTokenSpaceGuid.PcdUtmiPhyCount|2 diff --git a/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.c b/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.c index 1b45bb7..9efefb2 100644 --- a/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.c +++ b/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.c @@ -46,8 +46,12 @@ CHAR16 * SpeedStringTable [] = {L"-", L"1.25 Gbps", L"1.5 Gbps", L"2.5 Gbps", L"6 Gbps", L"6.25 Gbps", L"10.31 Gbps"};
CHIP_COMPHY_CONFIG ChipCfgTbl[] = { - { - .ChipType = L"Cp110", + { /* CP master */ + .ChipType = L"Cp110m", + .Init = ComPhyCp110Init + }, + { /* CP slave */ + .ChipType = L"Cp110s", .Init = ComPhyCp110Init } };
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> +#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;
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
Agree, quickly correcting it.
2017-03-22 13:41 GMT+01:00 Leif Lindholm leif.lindholm@linaro.org:
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
From: Konstantin Porotchkin kostap@marvell.com
Add support for multiple SATA host controllers to SERDES library. Remove single SATA controller base address from the description files and use shared platform description structure. This allows to reduce redundancy and pick existing PCD, used by PciEmulation.
Move and rename DEV_ENABLED macro from PciEmulation driver, so that it can be reused in other places, where description templates are used.
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 | 3 -- Platforms/Marvell/Include/Library/MvHwDescLib.h | 9 ++++++ Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c | 39 ++++++++++++++++------- Platforms/Marvell/Library/ComPhyLib/ComPhyLib.inf | 3 +- Platforms/Marvell/Marvell.dec | 4 --- Platforms/Marvell/PciEmulation/PciEmulation.c | 8 ++--- 6 files changed, 40 insertions(+), 26 deletions(-)
diff --git a/Platforms/Marvell/Armada/Armada70x0.dsc b/Platforms/Marvell/Armada/Armada70x0.dsc index ab8fa3d..126d324 100644 --- a/Platforms/Marvell/Armada/Armada70x0.dsc +++ b/Platforms/Marvell/Armada/Armada70x0.dsc @@ -147,6 +147,3 @@ #ResetLib gMarvellTokenSpaceGuid.PcdResetRegAddress|0xf06f0084 gMarvellTokenSpaceGuid.PcdResetRegMask|0x1 - - #SATA - gMarvellTokenSpaceGuid.PcdSataBaseAddress|0xF2540000 diff --git a/Platforms/Marvell/Include/Library/MvHwDescLib.h b/Platforms/Marvell/Include/Library/MvHwDescLib.h index 47bc4ce..73e629d 100644 --- a/Platforms/Marvell/Include/Library/MvHwDescLib.h +++ b/Platforms/Marvell/Include/Library/MvHwDescLib.h @@ -38,6 +38,13 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include <Library/NonDiscoverableDeviceRegistrationLib.h>
// +// Helper macros +// + +// Check if device is enabled - it expects PCD to be read to '<type>DeviceTable' array +#define MVHW_DEV_ENABLED(type, index) (type ## DeviceTable[index]) + +// // NonDiscoverable devices description template definition // #define MVHW_MAX_XHCI_DEVS 4 @@ -71,7 +78,9 @@ typedef struct { #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 diff --git a/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c b/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c index bee3bbf..5a620a1 100755 --- a/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c +++ b/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c @@ -33,6 +33,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. *******************************************************************************/
#include "ComPhyLib.h" +#include <Library/MvHwDescLib.h>
#define SD_LANE_ADDR_WIDTH 0x1000 #define HPIPE_ADDR_OFFSET 0x800 @@ -41,6 +42,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #define HPIPE_ADDR(base, Lane) (SD_ADDR(base, Lane) + HPIPE_ADDR_OFFSET) #define COMPHY_ADDR(base, Lane) (base + COMPHY_ADDR_LANE_WIDTH * Lane)
+DECLARE_A7K8K_NONDISCOVERABLE_TEMPLATE; + /* * For CP-110 we have 2 Selector registers "PHY Selectors" * and " PIPE Selectors". @@ -702,24 +705,34 @@ UINTN ComPhySataPowerUp ( IN UINT32 Lane, IN EFI_PHYSICAL_ADDRESS HpipeBase, - IN EFI_PHYSICAL_ADDRESS ComPhyBase + IN EFI_PHYSICAL_ADDRESS ComPhyBase, + IN UINT8 SataHostId ) { 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); - EFI_PHYSICAL_ADDRESS SataBase;
- SataBase = PcdGet32 (PcdSataBaseAddress); - if (SataBase == 0) { - DEBUG((DEBUG_INFO, "ComPhy: SATA address not defined\n")); - return EFI_D_ERROR; + 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 (SataBase); + ComPhySataMacPowerDown (Desc->AhciBaseAddresses[SataHostId]);
DEBUG((DEBUG_INFO, "ComPhy: stage: RFU configurations - hard reset ComPhy\n"));
@@ -735,7 +748,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"));
@@ -1018,9 +1031,11 @@ ComPhyCp110Init ( break; case PHY_TYPE_SATA0: case PHY_TYPE_SATA1: + Status = ComPhySataPowerUp (Lane, HpipeBaseAddr, ComPhyBaseAddr, MVHW_CP0_AHCI0_ID); + break; case PHY_TYPE_SATA2: case PHY_TYPE_SATA3: - Status = ComPhySataPowerUp(Lane, HpipeBaseAddr, ComPhyBaseAddr); + Status = ComPhySataPowerUp (Lane, HpipeBaseAddr, ComPhyBaseAddr, MVHW_CP1_AHCI0_ID); break; case PHY_TYPE_USB3_HOST0: case PHY_TYPE_USB3_HOST1: @@ -1039,8 +1054,8 @@ ComPhyCp110Init ( ASSERT (FALSE); break; } - if (EFI_ERROR(Status)) - DEBUG((DEBUG_ERROR, "PLL is not locked - Failed to initialize Lane %d\n", - Lane)); + if (EFI_ERROR(Status)) { + DEBUG ((DEBUG_ERROR, "Failed to initialize Lane %d\n with Status = 0x%x", Lane, Status)); + } } } 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 }} - diff --git a/Platforms/Marvell/PciEmulation/PciEmulation.c b/Platforms/Marvell/PciEmulation/PciEmulation.c index 7e9727e..427835e 100644 --- a/Platforms/Marvell/PciEmulation/PciEmulation.c +++ b/Platforms/Marvell/PciEmulation/PciEmulation.c @@ -49,8 +49,6 @@ STATIC UINT8 * CONST XhciDeviceTable = FixedPcdGetPtr (PcdPciEXhci); STATIC UINT8 * CONST AhciDeviceTable = FixedPcdGetPtr (PcdPciEAhci); STATIC UINT8 * CONST SdhciDeviceTable = FixedPcdGetPtr (PcdPciESdhci);
-#define DEV_ENABLED(type, index) (type ## DeviceTable[index]) - // // NonDiscoverable devices registration // @@ -69,7 +67,7 @@ PciEmulationInitXhci ( }
for (i = 0; i < Desc->XhciDevCount; i++) { - if (!DEV_ENABLED(Xhci, i)) { + if (!MVHW_DEV_ENABLED(Xhci, i)) { continue; }
@@ -106,7 +104,7 @@ PciEmulationInitAhci ( }
for (i = 0; i < Desc->AhciDevCount; i++) { - if (!DEV_ENABLED(Ahci, i)) { + if (!MVHW_DEV_ENABLED(Ahci, i)) { continue; }
@@ -143,7 +141,7 @@ PciEmulationInitSdhci ( }
for (i = 0; i < Desc->SdhciDevCount; i++) { - if (!DEV_ENABLED(Sdhci, i)) { + if (!MVHW_DEV_ENABLED(Sdhci, i)) { continue; }
On Wed, Mar 22, 2017 at 11:28:03AM +0100, Marcin Wojtas wrote:
From: Konstantin Porotchkin kostap@marvell.com
Add support for multiple SATA host controllers to SERDES library. Remove single SATA controller base address from the description files and use shared platform description structure. This allows to reduce redundancy and pick existing PCD, used by PciEmulation.
Move and rename DEV_ENABLED macro from PciEmulation driver, so that it can be reused in other places, where description templates are used.
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 | 3 -- Platforms/Marvell/Include/Library/MvHwDescLib.h | 9 ++++++ Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c | 39 ++++++++++++++++------- Platforms/Marvell/Library/ComPhyLib/ComPhyLib.inf | 3 +- Platforms/Marvell/Marvell.dec | 4 --- Platforms/Marvell/PciEmulation/PciEmulation.c | 8 ++--- 6 files changed, 40 insertions(+), 26 deletions(-)
diff --git a/Platforms/Marvell/Armada/Armada70x0.dsc b/Platforms/Marvell/Armada/Armada70x0.dsc index ab8fa3d..126d324 100644 --- a/Platforms/Marvell/Armada/Armada70x0.dsc +++ b/Platforms/Marvell/Armada/Armada70x0.dsc @@ -147,6 +147,3 @@ #ResetLib gMarvellTokenSpaceGuid.PcdResetRegAddress|0xf06f0084 gMarvellTokenSpaceGuid.PcdResetRegMask|0x1
- #SATA
- gMarvellTokenSpaceGuid.PcdSataBaseAddress|0xF2540000
diff --git a/Platforms/Marvell/Include/Library/MvHwDescLib.h b/Platforms/Marvell/Include/Library/MvHwDescLib.h index 47bc4ce..73e629d 100644 --- a/Platforms/Marvell/Include/Library/MvHwDescLib.h +++ b/Platforms/Marvell/Include/Library/MvHwDescLib.h @@ -38,6 +38,13 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include <Library/NonDiscoverableDeviceRegistrationLib.h> // +// Helper macros +//
+// Check if device is enabled - it expects PCD to be read to '<type>DeviceTable' array +#define MVHW_DEV_ENABLED(type, index) (type ## DeviceTable[index])
+// // NonDiscoverable devices description template definition // #define MVHW_MAX_XHCI_DEVS 4 @@ -71,7 +78,9 @@ typedef struct { #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 diff --git a/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c b/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c index bee3bbf..5a620a1 100755 --- a/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c +++ b/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c @@ -33,6 +33,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. *******************************************************************************/ #include "ComPhyLib.h" +#include <Library/MvHwDescLib.h> #define SD_LANE_ADDR_WIDTH 0x1000 #define HPIPE_ADDR_OFFSET 0x800 @@ -41,6 +42,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #define HPIPE_ADDR(base, Lane) (SD_ADDR(base, Lane) + HPIPE_ADDR_OFFSET) #define COMPHY_ADDR(base, Lane) (base + COMPHY_ADDR_LANE_WIDTH * Lane) +DECLARE_A7K8K_NONDISCOVERABLE_TEMPLATE;
/*
- For CP-110 we have 2 Selector registers "PHY Selectors"
- and " PIPE Selectors".
@@ -702,24 +705,34 @@ UINTN ComPhySataPowerUp ( IN UINT32 Lane, IN EFI_PHYSICAL_ADDRESS HpipeBase,
- IN EFI_PHYSICAL_ADDRESS ComPhyBase
- IN EFI_PHYSICAL_ADDRESS ComPhyBase,
- IN UINT8 SataHostId )
{ 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);
- EFI_PHYSICAL_ADDRESS SataBase;
- SataBase = PcdGet32 (PcdSataBaseAddress);
- if (SataBase == 0) {
- DEBUG((DEBUG_INFO, "ComPhy: SATA address not defined\n"));
- return EFI_D_ERROR;
- 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"));
Space before "((".
- 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 +748,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")); @@ -1018,9 +1031,11 @@ ComPhyCp110Init ( break; case PHY_TYPE_SATA0: case PHY_TYPE_SATA1:
Status = ComPhySataPowerUp (Lane, HpipeBaseAddr, ComPhyBaseAddr, MVHW_CP0_AHCI0_ID);break;
Status = ComPhySataPowerUp(Lane, HpipeBaseAddr, ComPhyBaseAddr);
Status = ComPhySataPowerUp (Lane, HpipeBaseAddr, ComPhyBaseAddr, MVHW_CP1_AHCI0_ID); break;@@ -1039,8 +1054,8 @@ ComPhyCp110Init ( ASSERT (FALSE); break; }
- if (EFI_ERROR(Status))
DEBUG((DEBUG_ERROR, "PLL is not locked - Failed to initialize Lane %d\n",Lane));
- if (EFI_ERROR(Status)) {
DEBUG ((DEBUG_ERROR, "Failed to initialize Lane %d\n with Status = 0x%x", Lane, Status));- } }
} 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 }}
No random line deletions, please.
diff --git a/Platforms/Marvell/PciEmulation/PciEmulation.c b/Platforms/Marvell/PciEmulation/PciEmulation.c index 7e9727e..427835e 100644 --- a/Platforms/Marvell/PciEmulation/PciEmulation.c +++ b/Platforms/Marvell/PciEmulation/PciEmulation.c @@ -49,8 +49,6 @@ STATIC UINT8 * CONST XhciDeviceTable = FixedPcdGetPtr (PcdPciEXhci); STATIC UINT8 * CONST AhciDeviceTable = FixedPcdGetPtr (PcdPciEAhci); STATIC UINT8 * CONST SdhciDeviceTable = FixedPcdGetPtr (PcdPciESdhci); -#define DEV_ENABLED(type, index) (type ## DeviceTable[index])
// // NonDiscoverable devices registration // @@ -69,7 +67,7 @@ PciEmulationInitXhci ( } for (i = 0; i < Desc->XhciDevCount; i++) {
- if (!DEV_ENABLED(Xhci, i)) {
- if (!MVHW_DEV_ENABLED(Xhci, i)) {
Spaces after macro name, please.
/ Leif
continue; }@@ -106,7 +104,7 @@ PciEmulationInitAhci ( } for (i = 0; i < Desc->AhciDevCount; i++) {
- if (!DEV_ENABLED(Ahci, i)) {
- if (!MVHW_DEV_ENABLED(Ahci, i)) { continue; }
@@ -143,7 +141,7 @@ PciEmulationInitSdhci ( } for (i = 0; i < Desc->SdhciDevCount; i++) {
- if (!DEV_ENABLED(Sdhci, i)) {
- if (!MVHW_DEV_ENABLED(Sdhci, i)) { continue; }
1.8.3.1
On Wed, Mar 22, 2017 at 11:27:59AM +0100, Marcin Wojtas wrote:
Hi,
I submit v2 of Armada 7k/8k improvements. It addresses all comments from review and add some minor improvements. Details can be found in the changelog and commit logs.
The commits are also available in the github: https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/opp-...
Any remarks or comments will be very welcome.
Thanks - nearly there. If you can just respin 4/4 with the whitespace fixups, I can push this quickly.
/ Leif
Best regards, Marcin
Changelog v1 -> v2:
- 1/4: Change type of COMPHY_CHIP_INIT callback to VOID and rework Status usage
- 2/4: Add 'Reviewed-by'
- 3/4:
- Sort includes in header
- Rename 'storage' to 'nondiscoverable' in all macros and names
- Add defines for array sizes in definition
- Add defines for base addresses, so that they describe HW block and device
- Add comments for sections
- 4/4:
- Remove 'Ptr' suffixes
- Move and rename DEV_ENABLED macro from PciEmulation driver - use it in ComPhy code
- Simplify obtaining enabled Sata ports and do not return EFI_SUCCESS on PCD mismatch (lane enabled in ComPhy, despite the controller disabled)
- Add macros for controller ID's
- Remove unrelated style fixes
- Rewrite error print information in ComPhyCp110Init, because from now we have more root causes than PLL
Konstantin Porotchkin (4): Platforms/Marvell: ComPhyLib: Fix compilation warning Platforms/Marvell: Add support for COMPHY on CP slaves Platform/Marvell: Extend and share the platform description Platform/Marvell: ComPhyLib: Enable SATA PHY init for multiple devices
Platforms/Marvell/Armada/Armada70x0.dsc | 9 +- Platforms/Marvell/Include/Library/MvHwDescLib.h | 106 ++++++++++++++++++++++ Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c | 46 ++++++---- Platforms/Marvell/Library/ComPhyLib/ComPhyLib.c | 8 +- Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h | 4 +- Platforms/Marvell/Library/ComPhyLib/ComPhyLib.inf | 3 +- Platforms/Marvell/Marvell.dec | 4 - Platforms/Marvell/PciEmulation/PciEmulation.c | 52 ++--------- 8 files changed, 156 insertions(+), 76 deletions(-) create mode 100644 Platforms/Marvell/Include/Library/MvHwDescLib.h
-- 1.8.3.1
Hi,
Saw your vacation plans info, so preparing quickly v3.
Thanks, Marcin
2017-03-22 13:45 GMT+01:00 Leif Lindholm leif.lindholm@linaro.org:
On Wed, Mar 22, 2017 at 11:27:59AM +0100, Marcin Wojtas wrote:
Hi,
I submit v2 of Armada 7k/8k improvements. It addresses all comments from review and add some minor improvements. Details can be found in the changelog and commit logs.
The commits are also available in the github: https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/
opp-20170322-upstream
Any remarks or comments will be very welcome.
Thanks - nearly there. If you can just respin 4/4 with the whitespace fixups, I can push this quickly.
/ Leif
Best regards, Marcin
Changelog v1 -> v2:
- 1/4: Change type of COMPHY_CHIP_INIT callback to VOID and rework
Status usage
- 2/4: Add 'Reviewed-by'
- 3/4:
- Sort includes in header
- Rename 'storage' to 'nondiscoverable' in all macros and names
- Add defines for array sizes in definition
- Add defines for base addresses, so that they describe HW block and
device
- Add comments for sections
- 4/4:
- Remove 'Ptr' suffixes
- Move and rename DEV_ENABLED macro from PciEmulation driver - use
it in
ComPhy code - Simplify obtaining enabled Sata ports and do not returnEFI_SUCCESS on
PCD mismatch (lane enabled in ComPhy, despite the controllerdisabled)
- Add macros for controller ID's - Remove unrelated style fixes - Rewrite error print information in ComPhyCp110Init, because fromnow we
have more root causes than PLLKonstantin Porotchkin (4): Platforms/Marvell: ComPhyLib: Fix compilation warning Platforms/Marvell: Add support for COMPHY on CP slaves Platform/Marvell: Extend and share the platform description Platform/Marvell: ComPhyLib: Enable SATA PHY init for multiple devices
Platforms/Marvell/Armada/Armada70x0.dsc | 9 +- Platforms/Marvell/Include/Library/MvHwDescLib.h | 106
++++++++++++++++++++++
Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c | 46 ++++++---- Platforms/Marvell/Library/ComPhyLib/ComPhyLib.c | 8 +- Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h | 4 +- Platforms/Marvell/Library/ComPhyLib/ComPhyLib.inf | 3 +- Platforms/Marvell/Marvell.dec | 4 - Platforms/Marvell/PciEmulation/PciEmulation.c | 52 ++--------- 8 files changed, 156 insertions(+), 76 deletions(-) create mode 100644 Platforms/Marvell/Include/Library/MvHwDescLib.h
-- 1.8.3.1
-
Leif Lindholm -
Marcin Wojtas