From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::142; helo=mail-it1-x142.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it1-x142.google.com (mail-it1-x142.google.com [IPv6:2607:f8b0:4864:20::142]) (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 DE1D9211B6C22 for ; Thu, 17 Jan 2019 03:27:59 -0800 (PST) Received: by mail-it1-x142.google.com with SMTP id i145so795625ita.4 for ; Thu, 17 Jan 2019 03:27:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=lgyFafkJeRC0/Slsd1Q+g/TQzfdIGL+dcxaVoYsedBU=; b=Ty21usL9UH66uufoFkCIsQZdLx6FPalfB95jPEZ367eAPxfWPIhzp1Dts3mAqhkxLV 9NUzTKo91FgxesrS1CYW+0LZSCaKzUW+rR7ZSgXQD0s9wZZvmAJ7kNNU9gM8nXCZi+9o DK3e/+h1RQil8UMXc1n3feEeJAoTmb1nET53Y= 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; bh=lgyFafkJeRC0/Slsd1Q+g/TQzfdIGL+dcxaVoYsedBU=; b=bpq3KuNdMFcmqRsN9QuY8hAaTIis+vwl95ym3Zrfuwa5EDQksFOFedG30zfCNiFM3+ gZKa9gYUaaTc7x6mchg4FYHF4rzHlEWr/4apfzM+i/tnXUPX99lIqqFhM2duXue90QBj mQupjgI6EPbP4EM0CL4yjy3CLUkP0qkGxwD9+TMkUtFnI1PjMJdI+VFub+DmGojJzm3H t9NbCRaA6mz6VvYu8QLlzgYVOjXbWWI/IsL8Td92cdZGtt8X0DjH+InvlmS28VA8O5/d EQrdchOrM8YJS6dM3nWQN9GLd48jMi1zAZHzfYOzePPhCbaMBdfC/653CSHQ1VyAlWun 7MqA== X-Gm-Message-State: AJcUukd8cJOV6jIGa7z1xdrNC3t4iD0LdCel3jBOFeqrrCW0ytA6Dddr LdZJuI0OnhZ0t7ml6FExYsb4TJSvU58DcrtTztblKg== X-Google-Smtp-Source: ALg8bN4boXmngjD2J7bYijeXUyUbCHMNM7W0zxKqPffC6jJCdYR8Stz2tlpGVyHCFTla4D0c80jEZGnBCzsHTBwotqg= X-Received: by 2002:a05:660c:4b:: with SMTP id p11mr8415084itk.71.1547724478958; Thu, 17 Jan 2019 03:27:58 -0800 (PST) MIME-Version: 1.0 References: <20190104144336.8941-1-ard.biesheuvel@linaro.org> <20190104144336.8941-3-ard.biesheuvel@linaro.org> <20190117101000.hsesfat7kcie7yuu@bivouac.eciton.net> In-Reply-To: <20190117101000.hsesfat7kcie7yuu@bivouac.eciton.net> From: Ard Biesheuvel Date: Thu, 17 Jan 2019 12:27:47 +0100 Message-ID: To: Leif Lindholm Cc: "edk2-devel@lists.01.org" , Masahisa Kojima Subject: Re: [PATCH edk2-platforms 2/7] Silicon/SynQuacer/Fip006Dxe: factor out DXE specific pieces X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 17 Jan 2019 11:28:00 -0000 X-List-Received-Date: Thu, 17 Jan 2019 11:28:00 -0000 X-List-Received-Date: Thu, 17 Jan 2019 11:28:00 -0000 X-List-Received-Date: Thu, 17 Jan 2019 11:28:00 -0000 Content-Type: text/plain; charset="UTF-8" On Thu, 17 Jan 2019 at 11:10, Leif Lindholm wrote: > > On Fri, Jan 04, 2019 at 03:43:31PM +0100, Ard Biesheuvel wrote: > > In preparation of creating a SMM version of the FIP006 NOR flash > > driver, refactor the existing pieces into a core driver, the FVB > > methods and the DXE instantiation code. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Ard Biesheuvel > > I only have one nitpicky question on this patch: > > > --- > > Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf | 6 +- > > Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c | 1006 +++++++++++++++++ > > Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/{NorFlashDxe.h => NorFlash.h} | 52 +- > > Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c | 1150 +++----------------- > > Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/{NorFlashFvbDxe.c => NorFlashFvb.c} | 161 +-- > > 5 files changed, 1194 insertions(+), 1181 deletions(-) > > > > > diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h > > similarity index 88% > > rename from Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h > > rename to Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h > > index 20e74b0320ce..61b8e6a08fa0 100644 > > --- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h > > +++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h > > @@ -27,11 +27,9 @@ > > #include > > > > #include > > -#include > > #include > > +#include > > Why add this include? > I'm not going to ask to move out the existing headers not actually > used by this file, but could we avoid adding new ones? > > https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/v/release/2.20/5_source_files/53_include_files.html#534-include-files-may-include-only-those-headers-that-it-directly-depends-upon > is actually a rule I agree with. > > But also, aren't all of the users of this file already manually > including this one? > Fair enough. I will drop the include here, and add it to whichever source file requires it afterwards. > > #include > > -#include > > -#include > > > > #include "Fip006Reg.h" > > > > > diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c > > index e52ab52d8cf7..6c07799b22d8 100644 > > --- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c > > +++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c > > @@ -15,15 +15,16 @@ > > **/ > > > > #include > > +#include > > +#include > > #include > > At least this one does. > > > #include > > #include > > #include > > #include > > +#include > > > > -#include "NorFlashDxe.h" > > - > > -STATIC EFI_EVENT mNorFlashVirtualAddrChangeEvent; > > +#include "NorFlash.h"