Hi Ard, 2017-12-07 20:48 GMT+01:00 Ard Biesheuvel <[email protected]>: > Hi Marcin, > > On 7 December 2017 at 19:20, Marcin Wojtas <[email protected]> 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 <[email protected]> >> --- >> 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 >> _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
