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

[Ard Biesheuvel]

On 6/5/20 3:27 PM, Marcin Wojtas wrote:
pt., 5 cze 2020 o 15:08 Ard Biesheuvel <ard.biesheu...@arm.com> napisał(a):

On 6/5/20 1:54 PM, Marcin Wojtas wrote:
Hi Ard,


czw., 4 cze 2020 o 23:35 Ard Biesheuvel <ard.biesheu...@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.biesheu...@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 <m...@semihalf.com>

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


Thanks Marcin.

I'll cherry pick some of those improvement, although I don't see the
point of replace 'while (TRUE)' with 'while (1)'


In the diff I did not change that:
https://github.com/Semihalf/edk2-platforms/commit/99b5775e9b1692d309ca25eac2ea86b0ca71437b#diff-044fee521a7731faf141e6c6d74fd83bR181


Fair enough.

I have just grabbed your version, and applied it as 
c8000ecccc83..ed4cc8059ec5

Thanks,


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60811): https://edk2.groups.io/g/devel/message/60811
Mute This Topic: https://groups.io/mt/74681118/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-