From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web11.8967.1589883807830426661 for ; Tue, 19 May 2020 03:23:28 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: ard.biesheuvel@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id E3E16101E; Tue, 19 May 2020 03:23:26 -0700 (PDT) Received: from [192.168.1.81] (unknown [10.37.8.132]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id CADA63F305; Tue, 19 May 2020 03:23:25 -0700 (PDT) Subject: Re: [PATCH edk2-platforms 1/3] Platform/96Boards/96BoardsI2cDxe: connect I2C controllers at EndOfDxe To: Leif Lindholm Cc: devel@edk2.groups.io References: <20200516175934.31148-1-ard.biesheuvel@arm.com> <20200516175934.31148-2-ard.biesheuvel@arm.com> <20200518172624.GF10467@vanye> From: "Ard Biesheuvel" Message-ID: Date: Tue, 19 May 2020 12:23:21 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <20200518172624.GF10467@vanye> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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 >> --- >> 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 >>