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.10411.1591362483942010648 for ; Fri, 05 Jun 2020 06:08:04 -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 9346F31B; Fri, 5 Jun 2020 06:08:03 -0700 (PDT) Received: from [192.168.1.69] (unknown [10.37.8.209]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C9CA23F305; Fri, 5 Jun 2020 06:08:02 -0700 (PDT) Subject: Re: [RFT PATCH edk2-platforms] Silicon/Marvell/MvI2cDxe: connect all I2C masters at EndOfDxe To: Marcin Wojtas Cc: edk2-devel-groups-io , Leif Lindholm References: <20200604213544.279976-1-ard.biesheuvel@arm.com> From: "Ard Biesheuvel" Message-ID: Date: Fri, 5 Jun 2020 15:07:59 +0200 User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:68.0) Gecko/20100101 Thunderbird/68.8.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable On 6/5/20 1:54 PM, Marcin Wojtas wrote: > Hi Ard, >=20 >=20 > czw., 4 cze 2020 o 23:35 Ard Biesheuvel napisa= =C5=82(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 >> --- >> 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/Silic= on/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 = =3D { >> 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] =3D Bus; >> + DevicePath->Instance =3D Bus; >> >> /* if attachment succeeds, this gets freed at ExitBootServices */ >> I2cMasterContext =3D 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 =3D AllocateCopyPool (sizeof (MvI2cDevicePathProtocol), >> + &MvI2cDevicePathProtocol); >> + if (DevicePath =3D=3D NULL) { >> + DEBUG ((DEBUG_ERROR, "MvI2cDxe: I2C device path allocation failed= \n")); >> + return; >> + } >> + >> + do { >> + DevicePathPointer =3D (EFI_DEVICE_PATH_PROTOCOL *)DevicePath; >> + Status =3D gBS->LocateDevicePath (&gEfiI2cMasterProtocolGuid, >> + &DevicePathPointer, &DeviceHandle); >> + if (EFI_ERROR (Status)) { >> + break; >> + } >> + >> + Status =3D 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 =3D gBS->LocateProtocol (&gMarvellBoardDescProtocolGuid, >> @@ -177,6 +221,10 @@ MvI2cInitialise ( >> >> BoardDescProtocol->BoardDescFree (Desc); >> >> + Status =3D gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_CALLBACK, OnE= ndOfDxe, >> + NULL, &gEfiEndOfDxeEventGroupGuid, &EndOfDxeEvent); >> + ASSERT_EFI_ERROR (Status); >> + >> return EFI_SUCCESS; >> } >> >> -- >> 2.26.2 >> >=20 > This change works as planned, so you can add my: > Tested-by: Marcin Wojtas >=20 > I pushed this patch with slightly improved style to: > https://github.com/Semihalf/edk2-platforms/commits/i2c-end-of-dxe >=20 Thanks Marcin. I'll cherry pick some of those improvement, although I don't see the=20 point of replace 'while (TRUE)' with 'while (1)'