From: "Ard Biesheuvel" <ard.biesheuvel@arm.com>
To: Marcin Wojtas <mw@semihalf.com>
Cc: edk2-devel-groups-io <devel@edk2.groups.io>,
Leif Lindholm <leif@nuviainc.com>
Subject: Re: [RFT PATCH edk2-platforms] Silicon/Marvell/MvI2cDxe: connect all I2C masters at EndOfDxe
Date: Fri, 5 Jun 2020 16:46:41 +0200 [thread overview]
Message-ID: <5903c3ba-8869-5b7e-73e0-cd3a90217e98@arm.com> (raw)
In-Reply-To: <CAPv3WKde-ZpNiZVBqeU7LH_RYVCvgNMWEwu3Rk0GsSjh2tu7wg@mail.gmail.com>
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,
next prev parent reply other threads:[~2020-06-05 14:46 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2020-06-05 15:07 ` Marcin Wojtas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5903c3ba-8869-5b7e-73e0-cd3a90217e98@arm.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox