On Sat, Oct 20, 2018 at 03:57:35AM +0200, Marcin Wojtas wrote:
> Introduce new callback that can provide information
> about GPIO SoC controllers, as well as on-board
> I2C IO expanders. According ArmadaSoCDescLib
> ArmadaBoardDescLib routines are used for
> obtaining required data.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <m...@semihalf.com>
> ---
> Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf | 1 +
> Silicon/Marvell/Include/Protocol/BoardDesc.h | 8 ++++
> Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c | 48
> ++++++++++++++++++++
> 3 files changed, 57 insertions(+)
>
> diff --git a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf
> b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf
> index 41f72d6..0b93948 100644
> --- a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf
> +++ b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf
> @@ -47,6 +47,7 @@
> Silicon/Marvell/Marvell.dec
>
> [LibraryClasses]
> + ArmadaBoardDescLib
> ArmadaSoCDescLib
> DebugLib
> MemoryAllocationLib
> diff --git a/Silicon/Marvell/Include/Protocol/BoardDesc.h
> b/Silicon/Marvell/Include/Protocol/BoardDesc.h
> index 1d57a16..e06d689 100644
> --- a/Silicon/Marvell/Include/Protocol/BoardDesc.h
> +++ b/Silicon/Marvell/Include/Protocol/BoardDesc.h
> @@ -50,6 +50,13 @@ EFI_STATUS
>
> typedef
> EFI_STATUS
> +(EFIAPI *MV_BOARD_DESC_GPIO_GET) (
> + IN MARVELL_BOARD_DESC_PROTOCOL *This,
> + IN OUT MV_BOARD_GPIO_DESC **GpioDesc
> + );
> +
> +typedef
> +EFI_STATUS
> (EFIAPI *MV_BOARD_DESC_I2C_GET) (
> IN MARVELL_BOARD_DESC_PROTOCOL *This,
> IN OUT MV_BOARD_I2C_DESC **I2cDesc
> @@ -106,6 +113,7 @@ VOID
> struct _MARVELL_BOARD_DESC_PROTOCOL {
> MV_BOARD_DESC_AHCI_GET BoardDescAhciGet;
> MV_BOARD_DESC_COMPHY_GET BoardDescComPhyGet;
> + MV_BOARD_DESC_GPIO_GET BoardDescGpioGet;
> MV_BOARD_DESC_I2C_GET BoardDescI2cGet;
> MV_BOARD_DESC_MDIO_GET BoardDescMdioGet;
> MV_BOARD_DESC_PP2_GET BoardDescPp2Get;
> diff --git a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
> b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
> index 39dc06c..948a6a0 100644
> --- a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
> +++ b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
> @@ -100,6 +100,53 @@ MvBoardDescComPhyGet (
>
> STATIC
> EFI_STATUS
> +MvBoardDescGpioGet (
> + IN MARVELL_BOARD_DESC_PROTOCOL *This,
> + IN OUT MV_BOARD_GPIO_DESC **GpioDesc
> + )
> +{
> + MV_I2C_IO_EXPANDER_DESC *I2cIoExpanderDesc;
> + UINTN SoCGpioCount, I2cIoExpanderCount;
> + STATIC MV_BOARD_GPIO_DESC *BoardDesc;
> + MV_SOC_GPIO_DESC *SoCDesc;
> + EFI_STATUS Status;
> +
> + if (BoardDesc != NULL) {
> + *GpioDesc = BoardDesc;
> + return EFI_SUCCESS;
> + }
For a completely opposite comment to the previous patch:
I would much prefer for BoardDesc to be a globally visible variable
with a g prefix and proper namespace.
Even though it itself is not globally visible, the above maneuver
makes it effectively so, but not semantically.
It's not like there's a million call sites, so I would prefer if the
check was there rather than in this function.
> +
> + /* Get SoC data about all available GPIO controllers */
> + Status = ArmadaSoCDescGpioGet (&SoCDesc, &SoCGpioCount);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + /* Get per-board information about all available I2C IO expanders */
> + Status = ArmadaBoardDescGpioGet (&I2cIoExpanderDesc, &I2cIoExpanderCount);
Same I2C->GPIO comments apply here.
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + /* Allocate and fill board description */
> + BoardDesc = AllocateZeroPool (sizeof (MV_BOARD_GPIO_DESC));
> + if (BoardDesc == NULL) {
> + DEBUG ((DEBUG_ERROR, "%a: Cannot allocate memory\n", __FUNCTION__));
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + BoardDesc->SoC = SoCDesc;
> + BoardDesc->GpioDevCount = SoCGpioCount;
> + BoardDesc->I2cIoExpanderDesc = I2cIoExpanderDesc;
> + BoardDesc->I2cIoExpanderCount = I2cIoExpanderCount;
> +
> + *GpioDesc = BoardDesc;
> +
> + return EFI_SUCCESS;
> +}
> +
> +STATIC
> +EFI_STATUS
> MvBoardDescI2cGet (
> IN MARVELL_BOARD_DESC_PROTOCOL *This,
> IN OUT MV_BOARD_I2C_DESC **I2cDesc
> @@ -556,6 +603,7 @@ MvBoardDescInitProtocol (
> {
> BoardDescProtocol->BoardDescAhciGet = MvBoardDescAhciGet;
> BoardDescProtocol->BoardDescComPhyGet = MvBoardDescComPhyGet;
> + BoardDescProtocol->BoardDescGpioGet = MvBoardDescGpioGet;
> BoardDescProtocol->BoardDescI2cGet = MvBoardDescI2cGet;
Even more so because of the two lines above :)
/
Leif
> BoardDescProtocol->BoardDescMdioGet = MvBoardDescMdioGet;
> BoardDescProtocol->BoardDescPp2Get = MvBoardDescPp2Get;
> --
> 2.7.4
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
