Re: [platforms: PATCH v2 24/25] Marvell/Drivers: MvI2cDxe: Switch driver to use MARVELL_BOARD_DESC

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

On Sun, Jun 17, 2018 at 10:12:04PM +0200, Marcin Wojtas wrote:
> MvI2cDxe driver used to get Armada7k8k controller description
> from hardcoded values stored in the header file MvHwDescLib.h.
> As a result it is very hard to support other
> Armada SoC families with this driver.
> 
> This patch updates the driver, so that it can obtain the
> description from newly introduced MARVELL_BOARD_DESC protocol,
> and removes the dependency on the hardcoded structures.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>

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

> ---
>  Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf |  1 +
>  Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c   | 37 +++++++++-----------
>  2 files changed, 18 insertions(+), 20 deletions(-)
> 
> diff --git a/Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf b/Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
> index a7cf52e..0eef350 100755
> --- a/Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
> +++ b/Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
> @@ -61,6 +61,7 @@
>    gEfiDevicePathProtocolGuid
>    gEfiI2cEnumerateProtocolGuid
>    gEfiI2cBusConfigurationManagementProtocolGuid
> +  gMarvellBoardDescProtocolGuid
>  
>  [Pcd]
>    gMarvellTokenSpaceGuid.PcdI2cSlaveAddresses
> diff --git a/Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c b/Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c
> index d6f590d..9ec4929 100755
> --- a/Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c
> +++ b/Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c
> @@ -32,6 +32,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  
>  *******************************************************************************/
>  
> +#include <Protocol/BoardDesc.h>
>  #include <Protocol/I2cMaster.h>
>  #include <Protocol/I2cEnumerate.h>
>  #include <Protocol/I2cBusConfigurationManagement.h>
> @@ -43,13 +44,10 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  #include <Library/PcdLib.h>
>  #include <Library/UefiLib.h>
>  #include <Library/MemoryAllocationLib.h>
> -#include <Library/MvHwDescLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
>  
>  #include "MvI2cDxe.h"
>  
> -DECLARE_A7K8K_I2C_TEMPLATE;
> -
>  STATIC MV_I2C_BAUD_RATE baud_rate;
>  
>  STATIC MV_I2C_DEVICE_PATH MvI2cDevicePathProtocol = {
> @@ -174,38 +172,37 @@ MvI2cInitialise (
>    IN EFI_SYSTEM_TABLE  *SystemTable
>    )
>  {
> -  MVHW_I2C_DESC *Desc = &mA7k8kI2cDescTemplate;
> -  UINT8 *I2cDeviceTable, Index;
> +  MARVELL_BOARD_DESC_PROTOCOL *BoardDescProtocol;
> +  MV_BOARD_I2C_DESC *Desc;
>    EFI_STATUS Status;
> +  UINTN Index;
>  
> -  /* Obtain table with enabled I2c devices */
> -  I2cDeviceTable = (UINT8 *)PcdGetPtr (PcdI2cControllersEnabled);
> -  if (I2cDeviceTable == NULL) {
> -    DEBUG ((DEBUG_ERROR, "Missing PcdI2cControllersEnabled\n"));
> -    return EFI_INVALID_PARAMETER;
> +  /* Obtain list of available controllers */
> +  Status = gBS->LocateProtocol (&gMarvellBoardDescProtocolGuid,
> +                  NULL,
> +                  (VOID **)&BoardDescProtocol);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
>    }
>  
> -  if (PcdGetSize (PcdI2cControllersEnabled) > MVHW_MAX_I2C_DEVS) {
> -    DEBUG ((DEBUG_ERROR, "Wrong PcdI2cControllersEnabled format\n"));
> -    return EFI_INVALID_PARAMETER;
> +  Status = BoardDescProtocol->BoardDescI2cGet (BoardDescProtocol, &Desc);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
>    }
>  
>    /* Initialize enabled chips */
> -  for (Index = 0; Index < PcdGetSize (PcdI2cControllersEnabled); Index++) {
> -    if (!MVHW_DEV_ENABLED (I2c, Index)) {
> -      DEBUG ((DEBUG_ERROR, "Skip I2c chip %d\n", Index));
> -      continue;
> -    }
> -
> +  for (Index = 0; Index < Desc->I2cDevCount; Index++) {
>      Status = MvI2cInitialiseController(
>          ImageHandle,
>          SystemTable,
> -        Desc->I2cBaseAddresses[Index]
> +        Desc[Index].SoC->I2cBaseAddress
>          );
>      if (EFI_ERROR(Status))
>        return Status;
>    }
>  
> +  BoardDescProtocol->BoardDescFree (Desc);
> +
>    return EFI_SUCCESS;
>  }
>  
> -- 
> 2.7.4
>