From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by mx.groups.io with SMTP id smtpd.web10.9251.1589885207542345402 for ; Tue, 19 May 2020 03:46:48 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=pUBXcdos; spf=pass (domain: nuviainc.com, ip: 209.85.221.65, mailfrom: leif@nuviainc.com) Received: by mail-wr1-f65.google.com with SMTP id v12so15279107wrp.12 for ; Tue, 19 May 2020 03:46:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=Jhx7x6S7W7oqN+UreBloVhDfhRVun/6x8Gc5AXHnmMY=; b=pUBXcdosq9Oktj2eWrx1HipCmSnkyxZ2+l+M3I3SG3H8L2wYl87sEpcBIkWP4GHWmz senS5pyPdKf+F3h5sJq6bdiacv6NI0SGV9bjT1tzltuYnaodOFh/GpjG+bz7WX+DEOki QranB55rJWjT3ry1J+W//PzrQ0hW94v29T59pcX8apG+UKtTiJEEag7qTpzrp4bNR5Rz vgCUPQsynC+cSMI1RPi2B7O8Zs4i0zxtR6ASUZMiztQc7kzD1nZzeNjxeOgmyfi2FZUl qQr+zt1BjoMB9D6KVlwhm7q6oKeTJXQIlOl2Ob1BGNXN22dfWFKm11RlJxpyui41TKGP ygvg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=Jhx7x6S7W7oqN+UreBloVhDfhRVun/6x8Gc5AXHnmMY=; b=noAOo1FiNhy4Dq8a1jM+xuykZBteAHzkEcd32c2DQIwg+m77/3WuFxWpDHPl2eE9Nt QSxS4CkuJ6xOhWUkuEXnZGp670ekphUZ/Bbu9Ev3Vv3T0bzwtibLFn3Zderx8xbt2d+t 8X3DShzgQ5JZSrTFzcCh1Ec4yXvv09Qs6ADfxO4u9D1G7efZiJ3X3X1bkikw1nHX3isE vbbjEgqCGTiKocPy7G9FzNnswoaSwDg5Sz0Wohbokm4gQk2OznxWLtxtydEW1SnQJ3dP Kmo5YAH1owd9wHcIdxsMLPVHnc7709OwYYa2rt9EaVQec4ieUsL4GsDvCVTjI3PUzHkT 7pkQ== X-Gm-Message-State: AOAM533w8tPENBOOS7D1Uz2qDyrzYuvFMEi3Ts3Uyt4nVs3v1xzmGb0A /F8gEIFoHcmYcoKPfQmRucDcjg== X-Google-Smtp-Source: ABdhPJyoVGjDThyr3CxFXmMSV+cEEhltX9tTgE8IUntnLIxn1dCYf9LdVR5JW0QboCyByj5cLeukYw== X-Received: by 2002:adf:df0e:: with SMTP id y14mr509917wrl.6.1589885206074; Tue, 19 May 2020 03:46:46 -0700 (PDT) Return-Path: Received: from vanye ([2001:470:1f09:12f0:b26e:bfff:fea9:f1b8]) by smtp.gmail.com with ESMTPSA id a12sm20167685wro.68.2020.05.19.03.46.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 19 May 2020 03:46:45 -0700 (PDT) Date: Tue, 19 May 2020 11:46:43 +0100 From: "Leif Lindholm" To: Ard Biesheuvel Cc: devel@edk2.groups.io Subject: Re: [PATCH edk2-platforms 1/3] Platform/96Boards/96BoardsI2cDxe: connect I2C controllers at EndOfDxe Message-ID: <20200519104643.GL10467@vanye> References: <20200516175934.31148-1-ard.biesheuvel@arm.com> <20200516175934.31148-2-ard.biesheuvel@arm.com> <20200518172624.GF10467@vanye> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, May 19, 2020 at 12:23:21 +0200, Ard Biesheuvel wrote: > 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. If you submit it separately, I'll ack it - it is clearly a readability improvement. With that bit dropped, for the series: Reviewed-by: Leif Lindholm > > > > > > > > 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 > > > >