Re: [PATCH edk2-platforms 23/41] Platform/NXP: Add Platform driver for LS1046 RDB board

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

Add a commit message please.

Like - what is going on with the handling of multiple I2c masters?


On Wed, Nov 28, 2018 at 08:31:37PM +0530, Meenakshi Aggarwal wrote:
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> ---
>  .../Drivers/PlatformDxe/PlatformDxe.c              | 119 +++++++++++++++++++++
>  .../Drivers/PlatformDxe/PlatformDxe.inf            |  58 ++++++++++
>  2 files changed, 177 insertions(+)
>  create mode 100644 Platform/NXP/LS1046aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c
>  create mode 100644 Platform/NXP/LS1046aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
> 
> diff --git a/Platform/NXP/LS1046aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c b/Platform/NXP/LS1046aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c
> new file mode 100644
> index 0000000..b74818e
> --- /dev/null
> +++ b/Platform/NXP/LS1046aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c
> @@ -0,0 +1,119 @@
> +/** @file
> +  LS1046 RDB board DXE platform driver.
> +
> +  Copyright 2018 NXP
> +
> +  This program and the accompanying materials are licensed and made available
> +  under the terms and conditions of the BSD License which accompanies this
> +  distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +**/
> +
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
> +
> +#include <Protocol/NonDiscoverableDevice.h>
> +
> +typedef struct {
> +  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR StartDesc;
> +  UINT8 EndDesc;
> +} ADDRESS_SPACE_DESCRIPTOR;
> +
> +STATIC ADDRESS_SPACE_DESCRIPTOR mI2cDesc[FixedPcdGet64 (PcdNumI2cController)];
> +
> +STATIC
> +EFI_STATUS
> +RegisterDevice (
> +  IN  EFI_GUID                        *TypeGuid,
> +  IN  ADDRESS_SPACE_DESCRIPTOR        *Desc,
> +  OUT EFI_HANDLE                      *Handle
> +  )
> +{
> +  NON_DISCOVERABLE_DEVICE             *Device;
> +  EFI_STATUS                          Status;
> +
> +  Device = (NON_DISCOVERABLE_DEVICE *)AllocateZeroPool (sizeof (*Device));
> +  if (Device == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  Device->Type = TypeGuid;
> +  Device->DmaType = NonDiscoverableDeviceDmaTypeNonCoherent;
> +  Device->Resources = (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)Desc;
> +
> +  Status = gBS->InstallMultipleProtocolInterfaces (Handle,
> +                  &gEdkiiNonDiscoverableDeviceProtocolGuid, Device,
> +                  NULL);
> +  if (EFI_ERROR (Status)) {
> +    goto FreeDevice;
> +  }
> +  return EFI_SUCCESS;
> +
> +FreeDevice:
> +  FreePool (Device);
> +
> +  return Status;
> +}
> +
> +VOID
> +PopulateI2cInformation (
> +  IN VOID
> +  )
> +{
> +  UINT32 Index;
> +
> +  for (Index = 0; Index < FixedPcdGet32 (PcdNumI2cController); Index++) {
> +    mI2cDesc[Index].StartDesc.Desc = ACPI_ADDRESS_SPACE_DESCRIPTOR;
> +    mI2cDesc[Index].StartDesc.Len = sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) - 3;
> +    mI2cDesc[Index].StartDesc.ResType = ACPI_ADDRESS_SPACE_TYPE_MEM;
> +    mI2cDesc[Index].StartDesc.GenFlag = 0;
> +    mI2cDesc[Index].StartDesc.SpecificFlag = 0;
> +    mI2cDesc[Index].StartDesc.AddrSpaceGranularity = 32;
> +    mI2cDesc[Index].StartDesc.AddrRangeMin = FixedPcdGet64 (PcdI2c0BaseAddr) +
> +                                             (Index * FixedPcdGet32 (PcdI2cSize));
> +    mI2cDesc[Index].StartDesc.AddrRangeMax = mI2cDesc[Index].StartDesc.AddrRangeMin +
> +                                             FixedPcdGet32 (PcdI2cSize) - 1;
> +    mI2cDesc[Index].StartDesc.AddrTranslationOffset = 0;
> +    mI2cDesc[Index].StartDesc.AddrLen = FixedPcdGet32 (PcdI2cSize);
> +
> +    mI2cDesc[Index].EndDesc = ACPI_END_TAG_DESCRIPTOR;

This confuses me. Does this not add an end descriptor for every entry
- ensuring that only one controller would ever be properly registered?

> +  }
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +PlatformDxeEntryPoint (
> +  IN EFI_HANDLE         ImageHandle,
> +  IN EFI_SYSTEM_TABLE   *SystemTable
> +  )
> +{
> +  EFI_STATUS                      Status;
> +  EFI_HANDLE                      Handle;
> +
> +  Handle = NULL;
> +
> +  PopulateI2cInformation ();
> +
> +  Status = RegisterDevice (&gNxpNonDiscoverableI2cMasterGuid,
> +             &mI2cDesc[3], &Handle);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  //
> +  // Install the DS1307 I2C Master protocol on this handle so the RTC driver
> +  // can identify it as the I2C master it can invoke directly.
> +  //
> +  Status = gBS->InstallProtocolInterface (&Handle,
> +                  &gPcf2129RealTimeClockLibI2cMasterProtocolGuid,
> +                  EFI_NATIVE_INTERFACE, NULL);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  return EFI_SUCCESS;
> +}
> diff --git a/Platform/NXP/LS1046aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf b/Platform/NXP/LS1046aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
> new file mode 100644
> index 0000000..2556af2
> --- /dev/null
> +++ b/Platform/NXP/LS1046aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
> @@ -0,0 +1,58 @@
> +## @file
> +#
> +#  Component description file for LS1046 DXE platform driver.
> +#
> +#  Copyright 2018 NXP
> +#
> +#  This program and the accompanying materials
> +#  are licensed and made available under the terms and conditions of the BSD License
> +#  which accompanies this distribution.  The full text of the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001A
> +  BASE_NAME                      = PlatformDxe
> +  FILE_GUID                      = 5bf02256-a7d2-4bfd-9934-2055358c6a6c
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = PlatformDxeEntryPoint
> +
> +[Sources]
> +  PlatformDxe.c
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  Silicon/NXP/Library/Pcf2129RtcLib/Pcf2129RtcLib.dec
> +  Silicon/NXP/NxpQoriqLs.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  BaseMemoryLib
> +  DebugLib
> +  MemoryAllocationLib
> +  PcdLib
> +  UefiBootServicesTableLib
> +  UefiDriverEntryPoint
> +  UefiLib
> +
> +[Guids]
> +  gNxpNonDiscoverableI2cMasterGuid
> +
> +[Protocols]
> +  gEdkiiNonDiscoverableDeviceProtocolGuid        ## PRODUCES
> +  gPcf2129RealTimeClockLibI2cMasterProtocolGuid   ## PRODUCES

Align the ## PRODUCES comments please.

/
    Leif

> +
> +[FixedPcd]
> +  gNxpQoriqLsTokenSpaceGuid.PcdI2c0BaseAddr
> +  gNxpQoriqLsTokenSpaceGuid.PcdI2cSize
> +  gNxpQoriqLsTokenSpaceGuid.PcdNumI2cController
> +
> +[Depex]
> +  TRUE
> -- 
> 1.9.1
>