From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: None (no SPF record) identity=mailfrom; client-ip=2607:f8b0:4001:c0b::241; helo=mail-it0-x241.google.com; envelope-from=mw@semihalf.com; receiver=edk2-devel@lists.01.org Received: from mail-it0-x241.google.com (mail-it0-x241.google.com [IPv6:2607:f8b0:4001:c0b::241]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 36D7422590E17 for ; Mon, 16 Apr 2018 22:15:48 -0700 (PDT) Received: by mail-it0-x241.google.com with SMTP id f6-v6so14354346ita.2 for ; Mon, 16 Apr 2018 22:15:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=768I4DAarQCxk/keclSVeqqQS6x5vV/xIMMuuhpLTFA=; b=CiVKBb0sttCWRUxckbv7LizkiPW6TKgkUcimZN+gE7Nyed2VXQ+7gswbPKHyyTVn9Q VvmFXn5P+RIX4UnXFsV9yOoGKSRh7NTFqc12DzTzbrDoxRr0r1S7Y3qBZx4P2b5zZLqC aNyfqx+rsjj1E+lBiuwTqTtTaQgTTpzi4RJ/1St5HPDm+YkDh+VIHeF6Z4irnxPTnA96 v1pXyrLv60aQ5JRCVwE3rLZgKanI/FZdRmYjTPcKyV1gIXZl63avsqDO/6Gj/4H/kgm/ N19WDpX542NHwZBkDSL/9E54ey1UqXAP1yTvVagLCNLNUuq2C7+WQxP+ebaKzw95yKad nKzQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=768I4DAarQCxk/keclSVeqqQS6x5vV/xIMMuuhpLTFA=; b=oKOpk8NpsuaBgEIbP9rE3Nt9OxBubkE2hde08xEtKp3GwYadSXNPyxilLCN+KW5QJC IQ8gFIAUTUxXOplA8uAP265wO/trYfc8HBFB8JtAUegjAN4GYowikUjmuooJFU9NgV7G y/HTaXFMZ0EqIBdUeWrBghh8v5vlMs482Y9avwzuIwgzRqXAvZnltyXbWhXiOYwl0Hi8 DXvSk7FDYyYqC2e7Hd/0txD2PwocKBchwAqPPVqP0Me+s5rM3H+q3R6lFfKgVUlz03qS pkdEzpKeFSysul0/EFoI5qGFRePshtRnBkOmGVF4Dcp/mDxShZgPQv6fEnDHRmvAdU79 BOww== X-Gm-Message-State: ALQs6tCdcTDVy6/63Ld5mtmr2kGx4hZJGD1AX2dGSNsxNqoiDzaZGxob lU6c6ogbNySeFx5HXX7G/qRxbRpQntmgwOeaIakHLA== X-Google-Smtp-Source: AIpwx49f9SUEt0elBB/0D5sW6esbR+uOgv1rVbOLGz7XDpbRmj4lQJWtbVZCm5V+m9uV374COnxTDj0UG+8tPqGLMXM= X-Received: by 2002:a24:67d7:: with SMTP id u206-v6mr1016956itc.138.1523942147330; Mon, 16 Apr 2018 22:15:47 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.139.23 with HTTP; Mon, 16 Apr 2018 22:15:46 -0700 (PDT) In-Reply-To: <487cbdc8-72f7-e990-f53b-354c88370ba8@redhat.com> References: <1523855353-9262-1-git-send-email-mw@semihalf.com> <1523855353-9262-2-git-send-email-mw@semihalf.com> <487cbdc8-72f7-e990-f53b-354c88370ba8@redhat.com> From: Marcin Wojtas Date: Tue, 17 Apr 2018 07:15:46 +0200 Message-ID: To: Laszlo Ersek Cc: Ard Biesheuvel , Hua Jing , Grzegorz Jaszczyk , "edk2-devel@lists.01.org" , Leif Lindholm , Nadav Haklai , Neta Zur Hershkovits Subject: Re: [platforms PATCH 1/2] Marvell/Drivers: MvFvbDxe: Adjust to new dependencies X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 17 Apr 2018 05:15:48 -0000 Content-Type: text/plain; charset="UTF-8" Hi Laszlo, 2018-04-16 21:41 GMT+02:00 Laszlo Ersek : > On 04/16/18 07:40, Ard Biesheuvel wrote: >> (+ Laszlo) >> >> On 16 April 2018 at 07:09, Marcin Wojtas wrote: >>> Recent changes in the EDK2 mainline resulted in breaking >>> of compilation and booting of Armada platforms. >>> This patch adjust the MvFvbDxe driver by: >>> >>> * installation of gEdkiiNvVarStoreFormattedGuid in order to signal >>> NvVarStoreFormattedLib to the generic variable runtime driver >>> >> >> Hello Marcin, >> >> Installing this GUID is only necessary if you update your platform >> .DSC to make the generic variable runtime driver depend on it by >> adding a NULL library class resolution using NvVarStoreFormattedLib. >> So I think this patch is correct, but you'll need an additional change >> to make it work as expected. (Otherwise, the variable runtime driver >> could still be dispatched early and invoked for read access before the >> variable store is reformatted) > > I agree. > > I'd also like to point out another frequent pitfall in this patch: > > > While gBS->InstallProtocolInterface() takes a *pointer* to a handle > (because it can *create* a handle, if the handle is NULL on input, and > the first protocol interface is installed on it), > gBS->UninstallProtocolInterface() takes the handle *itself*. If the last > protocol interface is uninstalled from the handle, then the handle is > destroyed, but gBS->UninstallProtocolInterface() does not attempt to > NULL the handle itself. So, here you should pass "gImageHandle", not > "&gImageHandle". > Right, I'll correct it. > There's also a bit of whitespace mangling here that's not compatible > with either multiline function call style that we like in edk2, but > perhaps edk2-platforms treats that more laxly. > We had some discussions last year - I followed the coding standards: Function ( Argument1, Argument2, Argument3 ); But was requested to place Argument1 to the function line and the last bracket to Argument3 line: Function (Argument1, Argument2, Argument3); Afair, there were some attempts to modify coding standards at the time, but I see the original version persisted. In fact I can do whatever line-breaking necessary: Ard - what style do you prefer in future patches? Best regards, Marcin > Thanks > Laszlo > >>> +ErrorInstallNvVarStoreFormatted: >>> gBS->UninstallMultipleProtocolInterfaces (&mFvbDevice->Handle, >>> &gEfiDevicePathProtocolGuid, >>> &gEfiFirmwareVolumeBlockProtocolGuid, >>> diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf >>> index 117fe8b..fd3f2f7 100644 >>> --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf >>> +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf >>> @@ -63,6 +63,7 @@ >>> UefiRuntimeServicesTableLib >>> >>> [Guids] >>> + gEdkiiNvVarStoreFormattedGuid >>> gEfiAuthenticatedVariableGuid >>> gEfiEventVirtualAddressChangeGuid >>> gEfiSystemNvDataFvGuid >>> @@ -84,8 +85,4 @@ >>> gMarvellTokenSpaceGuid.PcdSpiMemoryBase >>> >>> [Depex] >>> - # >>> - # MvFvbDxe must be loaded before VariableRuntimeDxe in case empty >>> - # flash needs populating with default values. >>> - # >>> - BEFORE gVariableRuntimeDxeFileGuid >>> + gEfiCpuArchProtocolGuid >>> -- >>> 2.7.4 >>> >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel >> >