Re: [edk2-devel] [edk2-platforms: PATCH 05/14] Marvell/Armada7k8k: MvBoardDesc: Extend protocol with PCIE support
On Thu, 9 May 2019 at 11:54, Marcin Wojtas wrote:
>
> Introduce new callback that can provide information about PCIE
> controllers, which are used on the platform. According ArmadaSoCDescLib
> ArmadaBoardDescLib routines are used for obtaining required data.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas
> ---
> Silicon/Marvell/Include/Protocol/BoardDesc.h | 22 +
> Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c | 86
> 2 files changed, 108 insertions(+)
>
> diff --git a/Silicon/Marvell/Include/Protocol/BoardDesc.h
> b/Silicon/Marvell/Include/Protocol/BoardDesc.h
> index 02905ea..c38ad86 100644
> --- a/Silicon/Marvell/Include/Protocol/BoardDesc.h
> +++ b/Silicon/Marvell/Include/Protocol/BoardDesc.h
> @@ -90,6 +90,27 @@ EFI_STATUS
>IN OUT MV_BOARD_XHCI_DESC **XhciDesc
>);
>
> +/**
> + Return the description of PCIE controllers used on the platform.
> +
> + @param[in out] *This Pointer to board description
> protocol.
> + @param[in out] **PcieDescription Array containing PCIE controllers'
> +description.
> +
> + @retval EFI_SUCCESS The data were obtained successfully.
> + @retval EFI_NOT_FOUND None of the controllers is used.
> + @retval EFI_INVALID_PARAMETER Description wrongly defined.
> + @retval EFI_OUT_OF_RESOURCES Lack of resources.
> + @retval Other Return error status.
> +
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *MV_BOARD_PCIE_DESCRIPTION_GET) (
> + IN MARVELL_BOARD_DESC_PROTOCOL *This,
> + IN OUT MV_BOARD_PCIE_DESCRIPTION **PcieDescription
> + );
> +
> typedef
> EFI_STATUS
> (EFIAPI *MV_BOARD_DESC_PP2_GET) (
> @@ -121,6 +142,7 @@ struct _MARVELL_BOARD_DESC_PROTOCOL {
>MV_BOARD_DESC_XHCI_GET BoardDescXhciGet;
>MV_BOARD_DESC_FREE BoardDescFree;
>MV_BOARD_GPIO_DESCRIPTION_GET GpioDescriptionGet;
> + MV_BOARD_PCIE_DESCRIPTION_GET PcieDescriptionGet;
> };
>
> #endif // __MARVELL_BOARD_DESC_PROTOCOL_H__
> diff --git a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
> b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
> index 973c362..9cd95bd 100644
> --- a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
> +++ b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
> @@ -36,6 +36,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> MV_BOARD_DESC *mBoardDescInstance;
>
> STATIC MV_BOARD_GPIO_DESCRIPTION *mGpioDescription;
> +STATIC MV_BOARD_PCIE_DESCRIPTION *mPcieDescription;
>
> STATIC
> EFI_STATUS
> @@ -444,6 +445,90 @@ MvBoardDescXhciGet (
>return EFI_SUCCESS;
> }
>
> +/**
> + Return the description of PCIE controllers used on the platform.
> +
> + @param[in out] *This Pointer to board description
> protocol.
> + @param[in out] **PcieDescription Array containing PCIE controllers'
> +description.
> +
> + @retval EFI_SUCCESS The data were obtained successfully.
> + @retval EFI_NOT_FOUND None of the controllers is used.
> + @retval EFI_INVALID_PARAMETER Description wrongly defined.
> + @retval EFI_OUT_OF_RESOURCES Lack of resources.
> + @retval Other Return error status.
> +
> +**/
> +STATIC
> +EFI_STATUS
> +MvBoardPcieDescriptionGet (
> + IN MARVELL_BOARD_DESC_PROTOCOL*This,
> + IN OUT MV_BOARD_PCIE_DESCRIPTION **PcieDescription
> + )
> +{
> + UINTN SoCPcieControllerCount, BoardPcieControllerCount, SoCIndex,
> BoardIndex;
> + EFI_PHYSICAL_ADDRESS *PcieBaseAddresses;
> + MV_PCIE_CONTROLLER *PcieControllers;
> + EFI_STATUS Status;
> +
> + /* Use existing structure if already created. */
> + if (mPcieDescription != NULL) {
> +*PcieDescription = mPcieDescription;
Since you are returning a cached copy here, I'd prefer it if the
prototype of the OUT parameter was MV_BOARD_PCIE_DESCRIPTION CONST**
rather than MV_BOARD_PCIE_DESCRIPTION**, since now, you are returning
a pointer to internal state of this driver.
(I guess this might apply to other methods as well)
> +return EFI_SUCCESS;
> + }
> +
> + /* Get SoC data about all available PCIE controllers. */
> + Status = ArmadaSoCPcieGet (, );
> + if (EFI_ERROR (Status)) {
> +return Status;
> + }
> +
> + /* Get per-board information about all used PCIE controllers. */
> + Status = ArmadaBoardPcieControllerGet (,
> + );
> + if (EFI_ERROR (Status)) {
> +return Status;
> + }
> +
> + /* Sanity check of the board description. */
> + if (BoardPcieControllerCount > SoCPcieControllerCount) {
> +DEBUG ((DEBUG_ERROR, "%a: Too many controllers described\n",
> __FUNCTION__));
> +return EFI_INVALID_PARAMETER;
> + }
> +
> + for (BoardIndex = 0; BoardIndex < BoardPcieControllerCount; BoardIndex++) {
> +for (SoCIndex = 0; SoCIndex <
[edk2-devel] [edk2-platforms: PATCH 05/14] Marvell/Armada7k8k: MvBoardDesc: Extend protocol with PCIE support
Introduce new callback that can provide information about PCIE
controllers, which are used on the platform. According ArmadaSoCDescLib
ArmadaBoardDescLib routines are used for obtaining required data.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas
---
Silicon/Marvell/Include/Protocol/BoardDesc.h | 22 +
Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c | 86
2 files changed, 108 insertions(+)
diff --git a/Silicon/Marvell/Include/Protocol/BoardDesc.h
b/Silicon/Marvell/Include/Protocol/BoardDesc.h
index 02905ea..c38ad86 100644
--- a/Silicon/Marvell/Include/Protocol/BoardDesc.h
+++ b/Silicon/Marvell/Include/Protocol/BoardDesc.h
@@ -90,6 +90,27 @@ EFI_STATUS
IN OUT MV_BOARD_XHCI_DESC **XhciDesc
);
+/**
+ Return the description of PCIE controllers used on the platform.
+
+ @param[in out] *This Pointer to board description protocol.
+ @param[in out] **PcieDescription Array containing PCIE controllers'
+description.
+
+ @retval EFI_SUCCESS The data were obtained successfully.
+ @retval EFI_NOT_FOUND None of the controllers is used.
+ @retval EFI_INVALID_PARAMETER Description wrongly defined.
+ @retval EFI_OUT_OF_RESOURCES Lack of resources.
+ @retval Other Return error status.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *MV_BOARD_PCIE_DESCRIPTION_GET) (
+ IN MARVELL_BOARD_DESC_PROTOCOL *This,
+ IN OUT MV_BOARD_PCIE_DESCRIPTION **PcieDescription
+ );
+
typedef
EFI_STATUS
(EFIAPI *MV_BOARD_DESC_PP2_GET) (
@@ -121,6 +142,7 @@ struct _MARVELL_BOARD_DESC_PROTOCOL {
MV_BOARD_DESC_XHCI_GET BoardDescXhciGet;
MV_BOARD_DESC_FREE BoardDescFree;
MV_BOARD_GPIO_DESCRIPTION_GET GpioDescriptionGet;
+ MV_BOARD_PCIE_DESCRIPTION_GET PcieDescriptionGet;
};
#endif // __MARVELL_BOARD_DESC_PROTOCOL_H__
diff --git a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
index 973c362..9cd95bd 100644
--- a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
+++ b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
@@ -36,6 +36,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
MV_BOARD_DESC *mBoardDescInstance;
STATIC MV_BOARD_GPIO_DESCRIPTION *mGpioDescription;
+STATIC MV_BOARD_PCIE_DESCRIPTION *mPcieDescription;
STATIC
EFI_STATUS
@@ -444,6 +445,90 @@ MvBoardDescXhciGet (
return EFI_SUCCESS;
}
+/**
+ Return the description of PCIE controllers used on the platform.
+
+ @param[in out] *This Pointer to board description protocol.
+ @param[in out] **PcieDescription Array containing PCIE controllers'
+description.
+
+ @retval EFI_SUCCESS The data were obtained successfully.
+ @retval EFI_NOT_FOUND None of the controllers is used.
+ @retval EFI_INVALID_PARAMETER Description wrongly defined.
+ @retval EFI_OUT_OF_RESOURCES Lack of resources.
+ @retval Other Return error status.
+
+**/
+STATIC
+EFI_STATUS
+MvBoardPcieDescriptionGet (
+ IN MARVELL_BOARD_DESC_PROTOCOL*This,
+ IN OUT MV_BOARD_PCIE_DESCRIPTION **PcieDescription
+ )
+{
+ UINTN SoCPcieControllerCount, BoardPcieControllerCount, SoCIndex, BoardIndex;
+ EFI_PHYSICAL_ADDRESS *PcieBaseAddresses;
+ MV_PCIE_CONTROLLER *PcieControllers;
+ EFI_STATUS Status;
+
+ /* Use existing structure if already created. */
+ if (mPcieDescription != NULL) {
+*PcieDescription = mPcieDescription;
+return EFI_SUCCESS;
+ }
+
+ /* Get SoC data about all available PCIE controllers. */
+ Status = ArmadaSoCPcieGet (, );
+ if (EFI_ERROR (Status)) {
+return Status;
+ }
+
+ /* Get per-board information about all used PCIE controllers. */
+ Status = ArmadaBoardPcieControllerGet (,
+ );
+ if (EFI_ERROR (Status)) {
+return Status;
+ }
+
+ /* Sanity check of the board description. */
+ if (BoardPcieControllerCount > SoCPcieControllerCount) {
+DEBUG ((DEBUG_ERROR, "%a: Too many controllers described\n",
__FUNCTION__));
+return EFI_INVALID_PARAMETER;
+ }
+
+ for (BoardIndex = 0; BoardIndex < BoardPcieControllerCount; BoardIndex++) {
+for (SoCIndex = 0; SoCIndex < SoCPcieControllerCount; SoCIndex++) {
+ if (PcieControllers[BoardIndex].PcieBaseAddress ==
+ PcieBaseAddresses[SoCIndex]) {
+ /* Match found */
+ break;
+ }
+}
+if (SoCIndex == SoCPcieControllerCount) {
+ DEBUG ((DEBUG_ERROR,
+"%a: Controller #%d base address invalid: 0x%x\n",
+__FUNCTION__,
+BoardIndex,
+PcieControllers[BoardIndex].PcieBaseAddress));
+ return EFI_INVALID_PARAMETER;
+}
+ }
+
+ /* Allocate and fill board description. */
+ mPcieDescription = AllocateZeroPool (sizeof (MV_BOARD_PCIE_DESCRIPTION));
+ if
