From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qk1-f193.google.com (mail-qk1-f193.google.com [209.85.222.193]) by mx.groups.io with SMTP id smtpd.web12.10839.1591363676295812769 for ; Fri, 05 Jun 2020 06:27:56 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@semihalf-com.20150623.gappssmtp.com header.s=20150623 header.b=qdugL/db; spf=none, err=SPF record not found (domain: semihalf.com, ip: 209.85.222.193, mailfrom: mw@semihalf.com) Received: by mail-qk1-f193.google.com with SMTP id b27so9601614qka.4 for ; Fri, 05 Jun 2020 06:27:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=up+rReyqaRATRtTdBkmyzudhW+mT+Skhc4Pn1xRYyQI=; b=qdugL/dbDe4e6l2Uilu7M+27uOlaZOC66rMR6NPyjPJdkdcWsp3/Wn6c9RMQ4Rjtfl ayUmO1CXsTmvunT+5wCHIfBAQ7x9LI5ITPIWUBQV6Q3J/I52k+kVG3R6D7GLJNSX+pVY S7ESpIVC4u9OJqjqyjy/vAdpTRpk6DyVv8/3O0pahzpVEVmdg6aTMvMUUqsxLAAWiH2J e5/CtSNDvvBJvu2wgbqHiKiC7Yzjeg1nyHO9ES97YhYcJO5VyRKjshJQ7S4O1fQMr4eY M9AXYv7yxCrdlX8iPU7pFgAZ+n+r9i+WGysUA7FWwI0/bIY3ClPntOSha2j1MVGVlEI5 S7oA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=up+rReyqaRATRtTdBkmyzudhW+mT+Skhc4Pn1xRYyQI=; b=n5nVEpKeLn/8Vs4TdJqEx9/j7I9LpWF4BNCk9II0Y7C9gEj4FLKJBzf4p54jPp5pt9 OB25DK3bfg6nneKVwtxbYy0EDSszvGf+mF991VjuYgVO8LGVPzupNuu2/yu0zGL2bkMI N80qvSyY761NYQxeuBx0eZ0eND/oH62j9l7LycVK99TO2IctFUQ6VP8H4zZ4ICrwYRcw vCmyOx8udIywWGA5l4UtahigRlcdBodUVEAFAWEH20WYxRpbm5GvFOGJieMqfS0IUAA7 bZnq5QbC2dStAIK50lX5y66mFpVWRJECKsrUVF5PWYJ7ZL71410owifMeNp4ZopNfSrv 3l2Q== X-Gm-Message-State: AOAM532ZeA4UDyKDYuy19KO058hOdO80TytkibOwiGui+qc8/Kiiqys8 1or6EUavpgWQktpKQO8yRGv3Fb+UXGuUTZjmlafOQA== X-Google-Smtp-Source: ABdhPJy0Q+pbGGiIN+u/16SooNMFCatAmJcNXVjU1gIKTj+VSfU1U6WH4qou6oC269SpdpJT9CnFyRam4bRgI6KBNkA= X-Received: by 2002:a37:49d1:: with SMTP id w200mr9333491qka.153.1591363675294; Fri, 05 Jun 2020 06:27:55 -0700 (PDT) MIME-Version: 1.0 References: <20200604213544.279976-1-ard.biesheuvel@arm.com> In-Reply-To: From: "Marcin Wojtas" Date: Fri, 5 Jun 2020 15:27:43 +0200 Message-ID: Subject: Re: [RFT PATCH edk2-platforms] Silicon/Marvell/MvI2cDxe: connect all I2C masters at EndOfDxe To: Ard Biesheuvel Cc: edk2-devel-groups-io , Leif Lindholm Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable pt., 5 cze 2020 o 15:08 Ard Biesheuvel napisa=C5= =82(a): > > On 6/5/20 1:54 PM, Marcin Wojtas wrote: > > Hi Ard, > > > > > > 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 > >> > > > > This change works as planned, so you can add my: > > Tested-by: Marcin Wojtas > > > > 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/99b5775e9b1692d309ca25eac= 2ea86b0ca71437b#diff-044fee521a7731faf141e6c6d74fd83bR181