Re: [PATCH edk2-platforms 1/3] Platform/96Boards/96BoardsI2cDxe: connect I2C controllers at EndOfDxe

["Ard Biesheuvel" <ard.biesheuvel@arm.com>]

On 5/18/20 7:26 PM, Leif Lindholm wrote:
> On Sat, May 16, 2020 at 19:59:32 +0200, Ard Biesheuvel wrote:
>> The 96boards I2C driver currently relies on the platform to connect
>> all controllers, or I2C peripherals will not be exposed if they are
>> not the active boot target. Since I2C peripherals are not boot targets
>> in the first place, but are used to expose things like random number
>> generators, let's connect the I2C controllers specifically at EndOfDxe
>> so that the devices living on it will be available regardless of the
>> boot policy.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
>> ---
>>   Platform/96Boards/96BoardsI2cDxe/96BoardsI2cDxe.inf |  5 ++++-
>>   Platform/96Boards/96BoardsI2cDxe/96BoardsI2cDxe.c   | 18 ++++++++++++++++++
>>   2 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/Platform/96Boards/96BoardsI2cDxe/96BoardsI2cDxe.inf b/Platform/96Boards/96BoardsI2cDxe/96BoardsI2cDxe.inf
>> index ae69f0933e93..3d9ca559e60b 100644
>> --- a/Platform/96Boards/96BoardsI2cDxe/96BoardsI2cDxe.inf
>> +++ b/Platform/96Boards/96BoardsI2cDxe/96BoardsI2cDxe.inf
>> @@ -36,10 +36,13 @@ [Protocols]
>>   [Guids]
>>     g96BoardsI2c0MasterGuid
>>     g96BoardsI2c1MasterGuid
>> +  gEfiEndOfDxeEventGroupGuid
>>   
>>   [FixedPcd]
>>     g96BoardsTokenSpaceGuid.PcdI2c0BusFrequencyHz
>>     g96BoardsTokenSpaceGuid.PcdI2c1BusFrequencyHz
>>   
>>   [Depex]
>> -  g96BoardsMezzanineProtocolGuid AND g96BoardsI2c0MasterGuid OR g96BoardsI2c1MasterGuid
>> +  g96BoardsMezzanineProtocolGuid AND (
>> +    g96BoardsI2c0MasterGuid OR g96BoardsI2c1MasterGuid
>> +  )
> 
> Is this change actually a bugfix?
> It appears unrelated to the patch description as such, although
> clearly an improvement.
> 

Apologies, I should have dropped that. I though it was a bug, but when i 
looked at the resulting DEPEX, they are actually the same.



> 
>> diff --git a/Platform/96Boards/96BoardsI2cDxe/96BoardsI2cDxe.c b/Platform/96Boards/96BoardsI2cDxe/96BoardsI2cDxe.c
>> index e4ecbca62c0c..a751769cf691 100644
>> --- a/Platform/96Boards/96BoardsI2cDxe/96BoardsI2cDxe.c
>> +++ b/Platform/96Boards/96BoardsI2cDxe/96BoardsI2cDxe.c
>> @@ -179,6 +179,19 @@ RegisterI2cBus (
>>     ASSERT_EFI_ERROR (Status);
>>   }
>>   
>> +STATIC
>> +VOID
>> +EFIAPI
>> +OnEndOfDxe (
>> +  IN EFI_EVENT  Event,
>> +  IN VOID       *Context
>> +  )
>> +{
>> +  gBS->CloseEvent (Event);
>> +  gBS->ConnectController (mI2cBus0.I2cMasterHandle, NULL, NULL, TRUE);
>> +  gBS->ConnectController (mI2cBus1.I2cMasterHandle, NULL, NULL, TRUE);
>> +}
>> +
>>   EFI_STATUS
>>   EFIAPI
>>   EntryPoint (
>> @@ -187,6 +200,7 @@ EntryPoint (
>>     )
>>   {
>>     EFI_STATUS    Status;
>> +  EFI_EVENT     EndOfDxeEvent;
>>   
>>     Status = gBS->LocateProtocol (&g96BoardsMezzanineProtocolGuid, NULL,
>>                     (VOID **)&mMezzanine);
>> @@ -197,5 +211,9 @@ EntryPoint (
>>     RegisterI2cBus (&g96BoardsI2c1MasterGuid, &mI2cBus1,
>>       mMezzanine->I2c1NumDevices, mMezzanine->I2c1DeviceArray);
>>   
>> +  Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_CALLBACK, OnEndOfDxe,
>> +                  NULL, &gEfiEndOfDxeEventGroupGuid, &EndOfDxeEvent);
>> +  ASSERT_EFI_ERROR (Status);
>> +
>>     return EFI_SUCCESS;
>>   }
>> -- 
>> 2.17.1
>>