From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qk1-f194.google.com (mail-qk1-f194.google.com [209.85.222.194]) by mx.groups.io with SMTP id smtpd.web12.13205.1591369679064543111 for ; Fri, 05 Jun 2020 08:07:59 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@semihalf-com.20150623.gappssmtp.com header.s=20150623 header.b=EfN54Zug; spf=none, err=SPF record not found (domain: semihalf.com, ip: 209.85.222.194, mailfrom: mw@semihalf.com) Received: by mail-qk1-f194.google.com with SMTP id n141so9994624qke.2 for ; Fri, 05 Jun 2020 08:07:58 -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=H3ji/9AgLnoFHTPRCQjxsLohETsdtw8Br/XDwLvn1dE=; b=EfN54Zugob7932cu/ieJyYRKkKRI+iORV0YgiyYU2QIqVDdDo0hLGZisU3Pkm4WDkl UokML+nqEpKU44ztV/oo+foPJn1z6MWiQ187tbcmQJ8BaOtRtF2kjuRvAN27YnAXEgYQ IHisaFq7PJAEGlKENecovxGT2zth0IOMHJXYXW2a+zoBI3DDDjQw+/35+uA6iyG/FKlG HqRtXbClZKZoCi1QeXuzJW6de9fYLOj+lgyDzQNvlolvcPcZYvKHaBn4vNlVJdotDB+q lB8srt38hwFsiHdapBkE9STl3BA3/3EJf5zBo/1maXmSQNXuRvdfIdHjMuKnjXDTMv64 RtDw== 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=H3ji/9AgLnoFHTPRCQjxsLohETsdtw8Br/XDwLvn1dE=; b=EqdOwEyuWKJuOSzpf5rKjZi2KZK2OQF4rTuepnZSXX40sukVlYK7d6Hv/xYOpSqDLr xD52XvPGWBMFw3Lxza9y3lBc9iOJGMqRHLr0JuubnTik1cWav1H/KCWi+w3yXvcH61go ZlqAfnaruyZt8xgEQ5mSpy3ODYrxtV7PCtjG40J+3fTnHBcZPgLtZaCKrg79hDyBz+AT ottJehnt6lWpUm0bdd4FTwEQQ8kx86zyuA4s3WRGwGPEjnrfv+0Yu30ErJCTOM488Tnz p+OhJu2mMEGkPnvITGaJH0JRt/YQ3H6W5pY4ijx17as9kXEau+C35sOFzUrTa0olppXO +YpA== X-Gm-Message-State: AOAM530glUuu4j3Pmt/1FttFUZ6Ilq2w+FnOab9ztLtwoMIJ7JFiENrK n9/dnyaTjtVtzKo0JT0RAAJe0dI7Ucpkww0Fyb6iNg== X-Google-Smtp-Source: ABdhPJyXFTOaPnAvbhVPekgZAa+7mt+QbWe6lvifzKmLDMRrLvcE+hR84xHG85bFlJJ3lhfKtQFilor9620Wk4rVjKQ= X-Received: by 2002:a37:9c57:: with SMTP id f84mr10117145qke.128.1591369677779; Fri, 05 Jun 2020 08:07:57 -0700 (PDT) MIME-Version: 1.0 References: <20200604213544.279976-1-ard.biesheuvel@arm.com> <5903c3ba-8869-5b7e-73e0-cd3a90217e98@arm.com> In-Reply-To: <5903c3ba-8869-5b7e-73e0-cd3a90217e98@arm.com> From: "Marcin Wojtas" Date: Fri, 5 Jun 2020 17:07:46 +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 16:46 Ard Biesheuvel napisa=C5= =82(a): > > On 6/5/20 3:27 PM, Marcin Wojtas wrote: > > 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 napi= sa=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/Sil= icon/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/Silic= on/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/Silic= on/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 MvI2cDevicePathProtoco= l =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 fai= led\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_CONTE= XT)); > >>>> @@ -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 fail= ed\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, TR= UE); > >>>> + 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, O= nEndOfDxe, > >>>> + 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/99b5775e9b1692d309ca2= 5eac2ea86b0ca71437b#diff-044fee521a7731faf141e6c6d74fd83bR181 > > > > Fair enough. > > I have just grabbed your version, and applied it as > c8000ecccc83..ed4cc8059ec5 > Great, thanks! Marcin