On Thu, Jan 10, 2019 at 02:44:33AM +0100, 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 <[email protected]>
> ---
> Silicon/Marvell/Include/Protocol/BoardDesc.h | 8 ++++
> Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c | 43 ++++++++++++++++++++
> 2 files changed, 51 insertions(+)
>
> diff --git a/Silicon/Marvell/Include/Protocol/BoardDesc.h
> b/Silicon/Marvell/Include/Protocol/BoardDesc.h
> index 1d57a16..02905ea 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_GPIO_DESCRIPTION_GET) (
> + IN MARVELL_BOARD_DESC_PROTOCOL *This,
> + IN OUT MV_BOARD_GPIO_DESCRIPTION **GpioDescription
> + );
> +
> +typedef
> +EFI_STATUS
> (EFIAPI *MV_BOARD_DESC_I2C_GET) (
> IN MARVELL_BOARD_DESC_PROTOCOL *This,
> IN OUT MV_BOARD_I2C_DESC **I2cDesc
> @@ -113,6 +120,7 @@ struct _MARVELL_BOARD_DESC_PROTOCOL {
> MV_BOARD_DESC_UTMI_GET BoardDescUtmiGet;
> MV_BOARD_DESC_XHCI_GET BoardDescXhciGet;
> MV_BOARD_DESC_FREE BoardDescFree;
> + MV_BOARD_GPIO_DESCRIPTION_GET GpioDescriptionGet;
> };
>
> #endif // __MARVELL_BOARD_DESC_PROTOCOL_H__
> diff --git a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
> b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
> index f71bfc4..e348b85 100644
> --- a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
> +++ b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
> @@ -100,6 +100,48 @@ MvBoardDescComPhyGet (
>
> STATIC
> EFI_STATUS
> +MvBoardGpioDescriptionGet (
> + IN MARVELL_BOARD_DESC_PROTOCOL *This,
> + IN OUT MV_BOARD_GPIO_DESCRIPTION **GpioDescription
> + )
> +{
> + MV_BOARD_GPIO_DESCRIPTION *Description;
My request on v1 was that this be refactored from a STATIC local
variable to a global variable.
> + UINTN SoCGpioCount, GpioExpanderCount;
> + MV_GPIO_EXPANDER *GpioExpanders;
> + GPIO_CONTROLLER *SoCGpio;
> + EFI_STATUS Status;
> +
> + /* Get SoC data about all available GPIO controllers */
> + Status = ArmadaSoCGpioGet (&SoCGpio, &SoCGpioCount);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + /* Get per-board information about all available I2C IO expanders */
GPIO
> + Status = ArmadaBoardGpioExpanderGet (&GpioExpanders, &GpioExpanderCount);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + /* Allocate and fill board description */
> + Description = AllocateZeroPool (sizeof (MV_BOARD_GPIO_DESCRIPTION));
Instead, this space is now dynamically allocated. But none of the call
sites actually change, leading to potential memory leaks.
I don't have a problem if you prefer an alternative solution to the
one I propose, but please discuss the change first rather than
submitting a new revision containing something I didn't ask for.
/
Leif
> + if (Description == NULL) {
> + DEBUG ((DEBUG_ERROR, "%a: Cannot allocate memory\n", __FUNCTION__));
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + Description->SoCGpio = SoCGpio;
> + Description->GpioDeviceCount = SoCGpioCount;
> + Description->GpioExpanders = GpioExpanders;
> + Description->GpioExpanderCount = GpioExpanderCount;
> +
> + *GpioDescription = Description;
> +
> + return EFI_SUCCESS;
> +}
> +
> +STATIC
> +EFI_STATUS
> MvBoardDescI2cGet (
> IN MARVELL_BOARD_DESC_PROTOCOL *This,
> IN OUT MV_BOARD_I2C_DESC **I2cDesc
> @@ -571,6 +613,7 @@ MvBoardDescInitProtocol (
> BoardDescProtocol->BoardDescUtmiGet = MvBoardDescUtmiGet;
> BoardDescProtocol->BoardDescXhciGet = MvBoardDescXhciGet;
> BoardDescProtocol->BoardDescFree = MvBoardDescFree;
> + BoardDescProtocol->GpioDescriptionGet = MvBoardGpioDescriptionGet;
>
> return EFI_SUCCESS;
> }
> --
> 2.7.4
>
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
