Re: [platforms: PATCH 6/6] Marvell/Drivers: MvSpiDxe: Keep data in SPI_DEVICE structure

[Leif Lindholm <leif.lindholm@linaro.org>]

On Tue, Oct 31, 2017 at 04:59:35AM +0100, Marcin Wojtas wrote:
> In the MvSpiDxe driver obtaining host register base address,
> controller clock and device maximum frequency directly from PCDs
> was done all over the code. This patch cleans up the parameters'
> handling and enables accessing them from SPI_DEVICE structure fields.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  Platform/Marvell/Drivers/Spi/MvSpiDxe.c | 48 ++++++++++++--------
>  Platform/Marvell/Include/Protocol/Spi.h |  2 +
>  2 files changed, 31 insertions(+), 19 deletions(-)
> 
> diff --git a/Platform/Marvell/Drivers/Spi/MvSpiDxe.c b/Platform/Marvell/Drivers/Spi/MvSpiDxe.c
> index a7db5f2..c60a520 100755
> --- a/Platform/Marvell/Drivers/Spi/MvSpiDxe.c
> +++ b/Platform/Marvell/Drivers/Spi/MvSpiDxe.c
> @@ -38,12 +38,13 @@ SPI_MASTER *mSpiMasterInstance;
>  STATIC
>  EFI_STATUS
>  SpiSetBaudRate (
> +  IN SPI_DEVICE *Slave,
>    IN UINT32 CpuClock,
>    IN UINT32 MaxFreq
>    )
>  {
>    UINT32 Spr, BestSpr, Sppr, BestSppr, ClockDivider, Match, Reg, MinBaudDiff;
> -  UINTN SpiRegBase = PcdGet32 (PcdSpiRegBase);
> +  UINTN SpiRegBase = Slave->HostRegisterBaseAddress;
>  
>    MinBaudDiff = 0xFFFFFFFF;
>    BestSppr = 0;
> @@ -93,26 +94,28 @@ SpiSetBaudRate (
>  STATIC
>  VOID
>  SpiSetCs (
> -  UINT8 CsId
> +  IN SPI_DEVICE *Slave
>    )
>  {
> -  UINT32 Reg, SpiRegBase = PcdGet32 (PcdSpiRegBase);
> +  UINT32 Reg;
> +  UINTN  SpiRegBase = Slave->HostRegisterBaseAddress;
>  
>    Reg = MmioRead32 (SpiRegBase + SPI_CTRL_REG);
>    Reg &= ~SPI_CS_NUM_MASK;
> -  Reg |= (CsId << SPI_CS_NUM_OFFSET);
> +  Reg |= (Slave->Cs << SPI_CS_NUM_OFFSET);
>    MmioWrite32 (SpiRegBase + SPI_CTRL_REG, Reg);
>  }
>  
>  STATIC
>  VOID
>  SpiActivateCs (
> -  UINT8 IN CsId
> +  IN SPI_DEVICE *Slave
>    )
>  {
> -  UINT32  Reg, SpiRegBase = PcdGet32 (PcdSpiRegBase);
> +  UINT32 Reg;
> +  UINTN  SpiRegBase = Slave->HostRegisterBaseAddress;
>  
> -  SpiSetCs(CsId);
> +  SpiSetCs(Slave);
>    Reg = MmioRead32 (SpiRegBase + SPI_CTRL_REG);
>    Reg |= SPI_CS_EN_MASK;
>    MmioWrite32(SpiRegBase + SPI_CTRL_REG, Reg);
> @@ -121,10 +124,11 @@ SpiActivateCs (
>  STATIC
>  VOID
>  SpiDeactivateCs (
> -  VOID
> +  IN SPI_DEVICE *Slave
>    )
>  {
> -  UINT32  Reg, SpiRegBase = PcdGet32 (PcdSpiRegBase);
> +  UINT32 Reg;
> +  UINTN  SpiRegBase = Slave->HostRegisterBaseAddress;
>  
>    Reg = MmioRead32 (SpiRegBase + SPI_CTRL_REG);
>    Reg &= ~SPI_CS_EN_MASK;
> @@ -139,14 +143,15 @@ SpiSetupTransfer (
>    )
>  {
>    SPI_MASTER *SpiMaster;
> -  UINT32 Reg, SpiRegBase, CoreClock, SpiMaxFreq;
> +  UINT32 Reg, CoreClock, SpiMaxFreq;
> +  UINTN SpiRegBase;
>  
>    SpiMaster = SPI_MASTER_FROM_SPI_MASTER_PROTOCOL (This);
>  
>    // Initialize values from PCDs
> -  SpiRegBase  = PcdGet32 (PcdSpiRegBase);
> -  CoreClock   = PcdGet32 (PcdSpiClockFrequency);
> -  SpiMaxFreq  = PcdGet32 (PcdSpiMaxFrequency);
> +  SpiRegBase  = Slave->HostRegisterBaseAddress;
> +  CoreClock   = Slave->CoreClock;
> +  SpiMaxFreq  = Slave->MaxFreq;
>  
>    EfiAcquireLock (&SpiMaster->Lock);
>  
> @@ -154,9 +159,9 @@ SpiSetupTransfer (
>    Reg |= SPI_BYTE_LENGTH;
>    MmioWrite32 (SpiRegBase + SPI_CONF_REG, Reg);
>  
> -  SpiSetCs(Slave->Cs);
> +  SpiSetCs(Slave);
>  
> -  SpiSetBaudRate (CoreClock, SpiMaxFreq);
> +  SpiSetBaudRate (Slave, CoreClock, SpiMaxFreq);
>  
>    Reg = MmioRead32 (SpiRegBase + SPI_CONF_REG);
>    Reg &= ~(SPI_CPOL_MASK | SPI_CPHA_MASK | SPI_TXLSBF_MASK | SPI_RXLSBF_MASK);
> @@ -194,21 +199,22 @@ MvSpiTransfer (
>  {
>    SPI_MASTER *SpiMaster;
>    UINT64  Length;
> -  UINT32  Iterator, Reg, SpiRegBase;
> +  UINT32  Iterator, Reg;
>    UINT8   *DataOutPtr = (UINT8 *)DataOut;
>    UINT8   *DataInPtr  = (UINT8 *)DataIn;
>    UINT8   DataToSend  = 0;
> +  UINTN   SpiRegBase;
>  
>    SpiMaster = SPI_MASTER_FROM_SPI_MASTER_PROTOCOL (This);
>  
> -  SpiRegBase = PcdGet32 (PcdSpiRegBase);
> +  SpiRegBase = Slave->HostRegisterBaseAddress;
>  
>    Length = 8 * DataByteCount;
>  
>    EfiAcquireLock (&SpiMaster->Lock);
>  
>    if (Flag & SPI_TRANSFER_BEGIN) {
> -    SpiActivateCs (Slave->Cs);
> +    SpiActivateCs (Slave);
>    }
>  
>    // Set 8-bit mode
> @@ -245,7 +251,7 @@ MvSpiTransfer (
>    }
>  
>    if (Flag & SPI_TRANSFER_END) {
> -    SpiDeactivateCs ();
> +    SpiDeactivateCs (Slave);
>    }
>  
>    EfiReleaseLock (&SpiMaster->Lock);
> @@ -312,6 +318,10 @@ MvSpiSetupSlave (
>      Slave->Mode = Mode;
>    }
>  
> +  Slave->HostRegisterBaseAddress = PcdGet32 (PcdSpiRegBase);
> +  Slave->CoreClock = PcdGet32 (PcdSpiClockFrequency);
> +  Slave->MaxFreq = PcdGet32 (PcdSpiMaxFrequency);
> +
>    SpiSetupTransfer (This, Slave);
>  
>    return Slave;
> diff --git a/Platform/Marvell/Include/Protocol/Spi.h b/Platform/Marvell/Include/Protocol/Spi.h
> index 0cf7914..b8981f3 100644
> --- a/Platform/Marvell/Include/Protocol/Spi.h
> +++ b/Platform/Marvell/Include/Protocol/Spi.h
> @@ -52,6 +52,8 @@ typedef struct {
>    INTN MaxFreq;
>    SPI_MODE Mode;
>    NOR_FLASH_INFO *Info;
> +  UINTN HostRegisterBaseAddress;
> +  UINTN CoreClock;
>  } SPI_DEVICE;
>  
>  typedef
> -- 
> 2.7.4
>