From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:4864:20::444; helo=mail-wr1-x444.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wr1-x444.google.com (mail-wr1-x444.google.com [IPv6:2a00:1450:4864:20::444]) (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 7024321B02822 for ; Thu, 17 Jan 2019 02:10:05 -0800 (PST) Received: by mail-wr1-x444.google.com with SMTP id u4so10317755wrp.3 for ; Thu, 17 Jan 2019 02:10:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=+aKotzClbIzBzGmpY83JPAU7k6Y0e1N1j/DNUuTQWTI=; b=iNQgQJL6qxXQBqGWReBSiIqPGkohqtw17Dfbe7ZFfw6NZzrOGnqDXmlrLZHOiQE5sA L2qxSVvoqYKPnOjzSv6REWTgSsUNDcCL/Wa7cw+6Uc5CAbrwJgNp3PRuji81GrQT9qpB cxGD2e77Usw7P4kjKLT+chnq42UWIZ6S4Pi9o= 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=+aKotzClbIzBzGmpY83JPAU7k6Y0e1N1j/DNUuTQWTI=; b=pOP3HAODYg8ttbjNV/DVSwzlE5DUJpNlJzUFX9CCy5pV8zx8OedJAiqFA63cJa7ZuT sNLTYmk+YH56fgXLxLGEJlYw13OAM2HwI2QYV5yBEkVwsV2L3MHqEadfENGvY4bvED+3 9p8T+fScoX6OJ+g59wYVNiQe1rLvLEcmRJBYvJE2GlkyVmiltpgflwT9MgYAtZSRJUy2 +7Opy+eRKJFo0hQkXOsuuAIOLtq+WIBxq6dyYyZyZ/IUTooyLZZ9aZwYt/wrTRtvhTMC OcLSr9VM4NhKw0PHmOkK/JKCj9j+CHkRa5+keAwpwIsbJN41yB/hKccT8oOax2+Wz3jB 3EXg== X-Gm-Message-State: AJcUukdqnBFFoHOwDT+w49L0haRFzE18fvb4x5BD5cxcJC5ew2PTj8xv v45fko3mWPQLbOlAWkCUMvK3iA== X-Google-Smtp-Source: ALg8bN7iANpYUw7RvQjJU9KyOC/hjxCEMThsAACodvmNPYqHC2eoHGlJoTO2lRgW5AVsm/N7gu0QWw== X-Received: by 2002:adf:ed46:: with SMTP id u6mr11660025wro.262.1547719803491; Thu, 17 Jan 2019 02:10:03 -0800 (PST) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id a17sm31280042wma.15.2019.01.17.02.10.02 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 17 Jan 2019 02:10:02 -0800 (PST) Date: Thu, 17 Jan 2019 10:10:00 +0000 From: Leif Lindholm To: Ard Biesheuvel Cc: edk2-devel@lists.01.org, Masahisa Kojima Message-ID: <20190117101000.hsesfat7kcie7yuu@bivouac.eciton.net> References: <20190104144336.8941-1-ard.biesheuvel@linaro.org> <20190104144336.8941-3-ard.biesheuvel@linaro.org> MIME-Version: 1.0 In-Reply-To: <20190104144336.8941-3-ard.biesheuvel@linaro.org> User-Agent: NeoMutt/20170113 (1.7.2) 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 10:10:05 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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? > #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"