public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [RFT PATCH edk2-platforms] Silicon/Marvell/MvI2cDxe: connect all I2C masters at EndOfDxe
@ 2020-06-04 21:35 Ard Biesheuvel
  2020-06-05 11:54 ` Marcin Wojtas
  0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2020-06-04 21:35 UTC (permalink / raw)
  To: devel; +Cc: mw, leif, Ard Biesheuvel

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [RFT PATCH edk2-platforms] Silicon/Marvell/MvI2cDxe: connect all I2C masters at EndOfDxe
  2020-06-04 21:35 [RFT PATCH edk2-platforms] Silicon/Marvell/MvI2cDxe: connect all I2C masters at EndOfDxe Ard Biesheuvel
@ 2020-06-05 11:54 ` Marcin Wojtas
  2020-06-05 13:07   ` Ard Biesheuvel
  0 siblings, 1 reply; 6+ messages in thread
From: Marcin Wojtas @ 2020-06-05 11:54 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel-groups-io, Leif Lindholm

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFT PATCH edk2-platforms] Silicon/Marvell/MvI2cDxe: connect all I2C masters at EndOfDxe
  2020-06-05 11:54 ` Marcin Wojtas
@ 2020-06-05 13:07   ` Ard Biesheuvel
  2020-06-05 13:27     ` Marcin Wojtas
  0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2020-06-05 13:07 UTC (permalink / raw)
  To: Marcin Wojtas; +Cc: edk2-devel-groups-io, Leif Lindholm

On 6/5/20 1:54 PM, Marcin Wojtas wrote:
> 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
> 

Thanks Marcin.

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFT PATCH edk2-platforms] Silicon/Marvell/MvI2cDxe: connect all I2C masters at EndOfDxe
  2020-06-05 13:07   ` Ard Biesheuvel
@ 2020-06-05 13:27     ` Marcin Wojtas
  2020-06-05 14:46       ` Ard Biesheuvel
  0 siblings, 1 reply; 6+ messages in thread
From: Marcin Wojtas @ 2020-06-05 13:27 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel-groups-io, Leif Lindholm

pt., 5 cze 2020 o 15:08 Ard Biesheuvel <ard.biesheuvel@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.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
> >
>
> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFT PATCH edk2-platforms] Silicon/Marvell/MvI2cDxe: connect all I2C masters at EndOfDxe
  2020-06-05 13:27     ` Marcin Wojtas
@ 2020-06-05 14:46       ` Ard Biesheuvel
  2020-06-05 15:07         ` Marcin Wojtas
  0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2020-06-05 14:46 UTC (permalink / raw)
  To: Marcin Wojtas; +Cc: edk2-devel-groups-io, Leif Lindholm

On 6/5/20 3:27 PM, Marcin Wojtas wrote:
> pt., 5 cze 2020 o 15:08 Ard Biesheuvel <ard.biesheuvel@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.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
>>>
>>
>> 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,


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFT PATCH edk2-platforms] Silicon/Marvell/MvI2cDxe: connect all I2C masters at EndOfDxe
  2020-06-05 14:46       ` Ard Biesheuvel
@ 2020-06-05 15:07         ` Marcin Wojtas
  0 siblings, 0 replies; 6+ messages in thread
From: Marcin Wojtas @ 2020-06-05 15:07 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel-groups-io, Leif Lindholm

pt., 5 cze 2020 o 16:46 Ard Biesheuvel <ard.biesheuvel@arm.com> napisał(a):
>
> On 6/5/20 3:27 PM, Marcin Wojtas wrote:
> > pt., 5 cze 2020 o 15:08 Ard Biesheuvel <ard.biesheuvel@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.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
> >>>
> >>
> >> 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
>

Great, thanks!

Marcin

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-06-05 15:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-06-04 21:35 [RFT PATCH edk2-platforms] Silicon/Marvell/MvI2cDxe: connect all I2C masters at EndOfDxe Ard Biesheuvel
2020-06-05 11:54 ` Marcin Wojtas
2020-06-05 13:07   ` Ard Biesheuvel
2020-06-05 13:27     ` Marcin Wojtas
2020-06-05 14:46       ` Ard Biesheuvel
2020-06-05 15:07         ` Marcin Wojtas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox