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::22a; helo=mail-it0-x22a.google.com; envelope-from=mw@semihalf.com; receiver=edk2-devel@lists.01.org Received: from mail-it0-x22a.google.com (mail-it0-x22a.google.com [IPv6:2607:f8b0:4001:c0b::22a]) (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 0790721B02820 for ; Thu, 7 Dec 2017 14:57:25 -0800 (PST) Received: by mail-it0-x22a.google.com with SMTP id x28so971672ita.0 for ; Thu, 07 Dec 2017 15:01:59 -0800 (PST) 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=bI91Gw/NFUKsflxVGXxPA1LWo/MHTcEfRKFIIwcM2iA=; b=Fetf9FzsNib+CntONaGUNRK/2o8M6u4UC1OSnKBq4TtHe16sHSOwQTpPmu7MDrxWA9 z5/liwSPJ+jCnN3ElaZUsA7n9cYX9+wgmqR1b6Ns+lexpW6MsjujH+O1aiOObL+XEpl+ JGwa4m7+2Z75MTAZyIdu2Fjtf4mscMxjxboy183Z0joZkdgSESscLH2f1kPzGDV4xbxg rBEpu9F//Ugs3DCeCJZyjiGPCej5gBzmEjujnQbwxPRijJxvWZ7j7bTMfnlrXZOBuTFq djjDkSSzWPXOPMSj0YXtzxLGRD+8N0SVKvjeuUuJah+NcwjpSAFHWK816ABOXdleCIHv jA1w== 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=bI91Gw/NFUKsflxVGXxPA1LWo/MHTcEfRKFIIwcM2iA=; b=CV5Hz5my1M7wZquQ5mS/GGiBg/DtLe6dRIGLSFUsOR2AdztNB1rIf83HRN2WzbD+/V tEKUcUQ7K3hy8XW2T1ir/jnloKt6brWWXtCdtU1Ruh/aCGyr/SFCj7p7djNuYR+DyDX4 CQB5QUYW+Es31ytQgvt7VF1vSW7ZIqLx+r40sHBeO9YT7TCKvde9wAvlBFKm3IAcvpQD HvQis3ulFykueywEhfWatCCPSYDTqqvOvcvL9HlJx5tJFel+Jzip9VjSjk9NOlKoDEtg p/wYRBNwUzFxnuIp7D1Rvh+CiSyAJv8x1mZfFr/y9GcWgBi1Z8fWspi8jyBNsetyO8US K63g== X-Gm-Message-State: AJaThX4lAyBpzWRcH6cIEJG7aGZzdSwA++F+dtr5YSbfcYiJgoOmD9jO 68/fEOvf2mhMrmBnN9x/Zp25DJcH55PzGPph+NncCg== X-Google-Smtp-Source: AGs4zMY+4Ey/xDLdl+8Zl6gXnm2NkeEhOUjjHkqPmk5wljE7vPQJpofzuVT+daMeZbcAPapuyQfsDZ45nKP/cDP72Aw= X-Received: by 10.107.8.152 with SMTP id h24mr42553368ioi.155.1512687718730; Thu, 07 Dec 2017 15:01:58 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.132.164 with HTTP; Thu, 7 Dec 2017 15:01:58 -0800 (PST) In-Reply-To: References: <1512674434-32326-1-git-send-email-mw@semihalf.com> <1512674434-32326-5-git-send-email-mw@semihalf.com> From: Marcin Wojtas Date: Fri, 8 Dec 2017 00:01:58 +0100 Message-ID: To: Ard Biesheuvel Cc: "edk2-devel@lists.01.org" , Leif Lindholm , Nadav Haklai , Neta Zur Hershkovits , Kostya Porotchkin , Hua Jing , =?UTF-8?B?SmFuIETEhWJyb8Wb?= , Konrad Adamczyk Subject: Re: [platforms: PATCH v2 4/5] Marvell/Drivers: Rename SPI master driver X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Dec 2017 22:57:26 -0000 Content-Type: text/plain; charset="UTF-8" Hi Ard, 2017-12-07 20:48 GMT+01:00 Ard Biesheuvel : > Hi Marcin, > > On 7 December 2017 at 19:20, Marcin Wojtas wrote: >> Hitherto driver name was very generic. In order to be ready >> for adding new SPI master drivers, use the controller's >> traditional name (it's called SPI Orion in Linux and >> U-Boot) for files and the entry point. >> >> Additionally, move the files to new 'Controllers' directory, >> which is paralel to existing 'Devices' and 'Variables', so >> that to make the separation more clear. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Marcin Wojtas >> --- >> Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf | 2 +- >> Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 2 +- >> Silicon/Marvell/Drivers/Spi/{MvSpiDxe.c => Controllers/MvSpiOrionDxe.c} | 4 ++-- >> Silicon/Marvell/Drivers/Spi/{MvSpiDxe.h => Controllers/MvSpiOrionDxe.h} | 0 >> Silicon/Marvell/Drivers/Spi/{MvSpiDxe.inf => Controllers/MvSpiOrionDxe.inf} | 8 ++++---- >> 5 files changed, 8 insertions(+), 8 deletions(-) >> rename Silicon/Marvell/Drivers/Spi/{MvSpiDxe.c => Controllers/MvSpiOrionDxe.c} (95%) >> rename Silicon/Marvell/Drivers/Spi/{MvSpiDxe.h => Controllers/MvSpiOrionDxe.h} (100%) >> rename Silicon/Marvell/Drivers/Spi/{MvSpiDxe.inf => Controllers/MvSpiOrionDxe.inf} (92%) > > I think this is not entirely correct. With these patches applied, we have > > Silicon/Marvell/Drivers/Spi/Variables > Silicon/Marvell/Drivers/Spi/Variables/MvFvbDxe.h > Silicon/Marvell/Drivers/Spi/Variables/MvFvbDxe.inf > Silicon/Marvell/Drivers/Spi/Variables/MvFvbDxe.c > Silicon/Marvell/Drivers/Spi/Devices > Silicon/Marvell/Drivers/Spi/Devices/MvSpiFlash.h > Silicon/Marvell/Drivers/Spi/Devices/MvSpiFlash.c > Silicon/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf > Silicon/Marvell/Drivers/Spi/Controllers > Silicon/Marvell/Drivers/Spi/Controllers/MvSpiOrionDxe.h > Silicon/Marvell/Drivers/Spi/Controllers/MvSpiOrionDxe.inf > Silicon/Marvell/Drivers/Spi/Controllers/MvSpiOrionDxe.c > > where each directory is named as if it could contain multiple drivers, > but given that each driver belongs in its own directory, we are > missing one directory level. > > To be honest (but I will let Leif comment as well), I think you can > remove the Variables/Devices/Controllers distinction, and just have > > Silicon/Marvell/Drivers/Spi/MvFvbDxe > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c > Silicon/Marvell/Drivers/Spi/MvSpiFlash > Silicon/Marvell/Drivers/Spi/MvSpiFlash/MvSpiFlash.h > Silicon/Marvell/Drivers/Spi/MvSpiFlash/MvSpiFlash.c > Silicon/Marvell/Drivers/Spi/MvSpiFlash/MvSpiFlash.inf > Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe > Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.h > Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.inf > Silicon/Marvell/Drivers/Spi/MvSpiOrionDxe/MvSpiOrionDxe.c > Sure, I can use your naming convention. MvSpiFlash and MvFvbDxe are generic in a way, that we need to only add new controller driver to support a different SoC family (this willl happen with Armada 37xx family). Best regards, Marcin > >> >> diff --git a/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf b/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf >> index 3b0646e..6d57b9a 100644 >> --- a/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf >> +++ b/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf >> @@ -110,7 +110,7 @@ FvNameGuid = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c >> INF Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf >> INF MdeModulePkg/Bus/I2c/I2cDxe/I2cDxe.inf >> INF Silicon/Marvell/Drivers/I2c/Devices/MvEeprom/MvEeprom.inf >> - INF Silicon/Marvell/Drivers/Spi/MvSpiDxe.inf >> + INF Silicon/Marvell/Drivers/Spi/Controllers/MvSpiOrionDxe.inf >> INF Silicon/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf >> INF Silicon/Marvell/Armada7k8k/Drivers/Armada7k8kRngDxe/Armada7k8kRngDxe.inf >> >> diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc >> index 7d87766..0eb3ef3 100644 >> --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc >> +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc >> @@ -411,7 +411,7 @@ >> Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf >> MdeModulePkg/Bus/I2c/I2cDxe/I2cDxe.inf >> Silicon/Marvell/Drivers/I2c/Devices/MvEeprom/MvEeprom.inf >> - Silicon/Marvell/Drivers/Spi/MvSpiDxe.inf >> + Silicon/Marvell/Drivers/Spi/Controllers/MvSpiOrionDxe.inf >> Silicon/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf >> Silicon/Marvell/Armada7k8k/Drivers/Armada7k8kRngDxe/Armada7k8kRngDxe.inf >> >> diff --git a/Silicon/Marvell/Drivers/Spi/MvSpiDxe.c b/Silicon/Marvell/Drivers/Spi/Controllers/MvSpiOrionDxe.c >> similarity index 95% >> rename from Silicon/Marvell/Drivers/Spi/MvSpiDxe.c >> rename to Silicon/Marvell/Drivers/Spi/Controllers/MvSpiOrionDxe.c >> index bab6cf4..c657daf 100755 >> --- a/Silicon/Marvell/Drivers/Spi/MvSpiDxe.c >> +++ b/Silicon/Marvell/Drivers/Spi/Controllers/MvSpiOrionDxe.c >> @@ -31,7 +31,7 @@ ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT >> SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. >> >> *******************************************************************************/ >> -#include "MvSpiDxe.h" >> +#include "MvSpiOrionDxe.h" >> >> SPI_MASTER *mSpiMasterInstance; >> >> @@ -399,7 +399,7 @@ SpiMasterInitProtocol ( >> >> EFI_STATUS >> EFIAPI >> -SpiMasterEntryPoint ( >> +SpiOrionEntryPoint ( >> IN EFI_HANDLE ImageHandle, >> IN EFI_SYSTEM_TABLE *SystemTable >> ) >> diff --git a/Silicon/Marvell/Drivers/Spi/MvSpiDxe.h b/Silicon/Marvell/Drivers/Spi/Controllers/MvSpiOrionDxe.h >> similarity index 100% >> rename from Silicon/Marvell/Drivers/Spi/MvSpiDxe.h >> rename to Silicon/Marvell/Drivers/Spi/Controllers/MvSpiOrionDxe.h >> diff --git a/Silicon/Marvell/Drivers/Spi/MvSpiDxe.inf b/Silicon/Marvell/Drivers/Spi/Controllers/MvSpiOrionDxe.inf >> similarity index 92% >> rename from Silicon/Marvell/Drivers/Spi/MvSpiDxe.inf >> rename to Silicon/Marvell/Drivers/Spi/Controllers/MvSpiOrionDxe.inf >> index e7bc170..3f85b40 100644 >> --- a/Silicon/Marvell/Drivers/Spi/MvSpiDxe.inf >> +++ b/Silicon/Marvell/Drivers/Spi/Controllers/MvSpiOrionDxe.inf >> @@ -31,15 +31,15 @@ >> >> [Defines] >> INF_VERSION = 0x00010005 >> - BASE_NAME = SpiMasterDxe >> + BASE_NAME = SpiOrionDxe >> FILE_GUID = c19dbc8a-f4f9-43b0-aee5-802e3ed03d15 >> MODULE_TYPE = DXE_RUNTIME_DRIVER >> VERSION_STRING = 1.0 >> - ENTRY_POINT = SpiMasterEntryPoint >> + ENTRY_POINT = SpiOrionEntryPoint >> >> [Sources] >> - MvSpiDxe.c >> - MvSpiDxe.h >> + MvSpiOrionDxe.c >> + MvSpiOrionDxe.h >> >> [Packages] >> EmbeddedPkg/EmbeddedPkg.dec >> -- >> 2.7.4 >>