Re: [platforms: PATCH v2 4/5] Marvell/Drivers: Rename SPI master driver

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

Hi Marcin,

On 7 December 2017 at 19:20, Marcin Wojtas <mw@semihalf.com> 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 <mw@semihalf.com>
> ---
>  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


>
> 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
>