Re: [RFT PATCH edk2-platforms] Silicon/Marvell/MvI2cDxe: connect all I2C masters at EndOfDxe

["Marcin Wojtas" <mw@semihalf.com>]

Hi Ard,


czw., 4 cze 2020 o 23:35 Ard Biesheuvel <ard.biesheuvel@arm.com> napisał(a):
>
> To ensure that platforms incorporating MvI2cDxe will keep working
> as intended once the platform BDS code stops calling ConnectAll(),
> connect the I2C masters explicitly at EndOfDxe.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
> Build tested only.
>
>  Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf |  3 ++
>  Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.h   |  1 +
>  Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c   | 54 ++++++++++++++++++--
>  3 files changed, 55 insertions(+), 3 deletions(-)
>
> diff --git a/Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf b/Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
> index e59fee0ac1b5..f631fbe797fc 100755
> --- a/Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
> +++ b/Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
> @@ -45,5 +45,8 @@ [Pcd]
>    gMarvellTokenSpaceGuid.PcdI2cBaudRate
>    gMarvellTokenSpaceGuid.PcdI2cBusCount
>
> +[Guids]
> +  gEfiEndOfDxeEventGroupGuid
> +
>  [Depex]
>    TRUE
> diff --git a/Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.h b/Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.h
> index f5c2cdd8ab3a..6caaa45cece0 100644
> --- a/Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.h
> +++ b/Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.h
> @@ -80,6 +80,7 @@ typedef struct {
>
>  typedef struct {
>    VENDOR_DEVICE_PATH            Guid;
> +  UINTN                         Instance;
>    EFI_DEVICE_PATH_PROTOCOL      End;
>  } MV_I2C_DEVICE_PATH;
>
> diff --git a/Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c b/Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c
> index b13ab8f02c99..dfe8da9891a5 100755
> --- a/Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c
> +++ b/Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c
> @@ -30,12 +30,13 @@ STATIC MV_I2C_DEVICE_PATH MvI2cDevicePathProtocol = {
>        HARDWARE_DEVICE_PATH,
>        HW_VENDOR_DP,
>        {
> -  (UINT8) (sizeof(VENDOR_DEVICE_PATH)),
> -  (UINT8) (sizeof(VENDOR_DEVICE_PATH) >> 8),
> +        (UINT8) (OFFSET_OF (MV_I2C_DEVICE_PATH, End)),
> +        (UINT8) (OFFSET_OF (MV_I2C_DEVICE_PATH, End) >> 8),
>        },
>      },
>      EFI_CALLER_ID_GUID
>    },
> +  0,  // Instance
>    {
>      END_DEVICE_PATH_TYPE,
>      END_ENTIRE_DEVICE_PATH_SUBTYPE,
> @@ -86,7 +87,7 @@ MvI2cInitialiseController (
>      DEBUG((DEBUG_ERROR, "MvI2cDxe: I2C device path allocation failed\n"));
>      return EFI_OUT_OF_RESOURCES;
>    }
> -  DevicePath->Guid.Guid.Data4[0] = Bus;
> +  DevicePath->Instance = Bus;
>
>    /* if attachment succeeds, this gets freed at ExitBootServices */
>    I2cMasterContext = AllocateZeroPool (sizeof (I2C_MASTER_CONTEXT));
> @@ -139,6 +140,47 @@ MvI2cInitialiseController (
>    return Status;
>  }
>
> +STATIC
> +VOID
> +EFIAPI
> +OnEndOfDxe (
> +  IN EFI_EVENT  Event,
> +  IN VOID       *Context
> +  )
> +{
> +  MV_I2C_DEVICE_PATH        *DevicePath;
> +  EFI_DEVICE_PATH_PROTOCOL  *DevicePathPointer;
> +  EFI_HANDLE                DeviceHandle;
> +  EFI_STATUS                Status;
> +
> +  gBS->CloseEvent (Event);
> +
> +  DevicePath = AllocateCopyPool (sizeof (MvI2cDevicePathProtocol),
> +                 &MvI2cDevicePathProtocol);
> +  if (DevicePath == NULL) {
> +    DEBUG ((DEBUG_ERROR, "MvI2cDxe: I2C device path allocation failed\n"));
> +    return;
> +  }
> +
> +  do {
> +    DevicePathPointer = (EFI_DEVICE_PATH_PROTOCOL *)DevicePath;
> +    Status = gBS->LocateDevicePath (&gEfiI2cMasterProtocolGuid,
> +                    &DevicePathPointer, &DeviceHandle);
> +    if (EFI_ERROR (Status)) {
> +      break;
> +    }
> +
> +    Status = gBS->ConnectController (DeviceHandle, NULL, NULL, TRUE);
> +    DEBUG ((DEBUG_INFO, "%a: ConnectController () returned %r\n",
> +      __FUNCTION__, Status));
> +
> +    DevicePath->Instance++;
> +  } while (TRUE);
> +
> +  gBS->FreePool (DevicePath);
> +}
> +
> +
>  EFI_STATUS
>  EFIAPI
>  MvI2cInitialise (
> @@ -150,6 +192,8 @@ MvI2cInitialise (
>    MV_BOARD_I2C_DESC *Desc;
>    EFI_STATUS Status;
>    UINTN Index;
> +  EFI_EVENT EndOfDxeEvent;
> +
>
>    /* Obtain list of available controllers */
>    Status = gBS->LocateProtocol (&gMarvellBoardDescProtocolGuid,
> @@ -177,6 +221,10 @@ MvI2cInitialise (
>
>    BoardDescProtocol->BoardDescFree (Desc);
>
> +  Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_CALLBACK, OnEndOfDxe,
> +                  NULL, &gEfiEndOfDxeEventGroupGuid, &EndOfDxeEvent);
> +  ASSERT_EFI_ERROR (Status);
> +
>    return EFI_SUCCESS;
>  }
>
> --
> 2.26.2
>

This change works as planned, so you can add my:
Tested-by: Marcin Wojtas <mw@semihalf.com>

I pushed this patch with slightly improved style to:
https://github.com/Semihalf/edk2-platforms/commits/i2c-end-of-dxe

Best regards,
Marcin