public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v4] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Set Marvell Yukon MAC address
@ 2017-01-09 21:20 Daniil Egranov
  2017-01-10 22:36 ` Leif Lindholm
  0 siblings, 1 reply; 4+ messages in thread
From: Daniil Egranov @ 2017-01-09 21:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: leif.lindholm, Daniil Egranov

From: Daniil Egranov <daniil.egranov@linaro.org>

The patch reads a valid MAC address form the Juno IOFPGA registers 
and pushes it into onboard Marvell Yukon NIC.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Daniil Egranov <daniil.egranov@linaro.org>
---

Hi Leif,

The Marvell control registers should be accesable through the memory
specified in BAR 0.

The new helper function should cover all 4 checks:
- We can read BAR 0 attributes. 
 We always read BAR 0
- BAR 0 resources contain at least one QWORD Address Space Descriptor.
- The first of these refers to something memory mapped.
- That it is not (ARM terminology) Normal memory.
 The BAR 0 for Marvell Yukon should always be a memory type. I am not 
 sure if it's possible to have more than one descriptor for a BAR. The 
 current implementation of PciIoGetBarAttributes() will return only one
 descriptor based on BAR index following the End Tag one. This update 
 will step through Address Space Descriptors anyway until the End Tag.  

Thanks,
Daniil

Changelog:

v4
Provided more comments on the code and corrected functions name.
Created helper function for reading base memory address from BAR.
Added stepping through ACPI address space descriptor until End Tag.

v3
Changed code structure per Leif Lindholm's request

v2
Corrected style issues and code structure.

v1
The patch reads a valid MAC address form the Juno IOFPGA registers 
and pushes it into onboard Marvell Yukon NIC.

 .../ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c     | 289 +++++++++++++++++++++
 .../Drivers/ArmJunoDxe/ArmJunoDxeInternal.h        |  13 +
 2 files changed, 302 insertions(+)

diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
index b97f044..5847d0a 100644
--- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
+++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
@@ -15,7 +15,9 @@
 #include "ArmJunoDxeInternal.h"
 #include <ArmPlatform.h>
 
+#include <IndustryStandard/Pci.h>
 #include <Protocol/DevicePathFromText.h>
+#include <Protocol/PciIo.h>
 #include <Protocol/PciRootBridgeIo.h>
 
 #include <Guid/EventGroup.h>
@@ -69,6 +71,290 @@ STATIC CONST EFI_PCI_ROOT_BRIDGE_DEVICE_PATH mPciRootComplexDevicePath = {
 EFI_EVENT mAcpiRegistration = NULL;
 
 /**
+  This function reads PCI ID of the controller.
+
+  @param[in]  PciIo   PCI IO protocol handle
+  @param[in]  PciId   Looking for specified PCI ID Vendor/Device
+**/
+STATIC
+EFI_STATUS
+ReadMarvellYoukonPciId (
+  IN EFI_PCI_IO_PROTOCOL  *PciIo,
+  IN UINT32               PciId
+  )
+{
+  UINT32      DevicePciId;
+  EFI_STATUS  Status;
+
+  Status = PciIo->Pci.Read (
+                        PciIo,
+                        EfiPciIoWidthUint32,
+                        PCI_VENDOR_ID_OFFSET,
+                        1,
+                        &DevicePciId);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  if (DevicePciId != PciId) {
+    return EFI_NOT_FOUND;
+  }
+
+  return EFI_SUCCESS;
+}
+
+/**
+  This function searches for Marvell Yukon NIC on the Juno
+  platform and returns PCI IO protocol handle for the controller.
+
+  @param[out]  PciIo   PCI IO protocol handle
+**/
+STATIC
+EFI_STATUS
+GetMarvellYukonPciIoProtocol (
+  OUT EFI_PCI_IO_PROTOCOL  **PciIo
+  )
+{
+  UINTN       HandleCount;
+  EFI_HANDLE  *HandleBuffer;
+  UINTN       HIndex;
+  EFI_STATUS  Status;
+
+  Status = gBS->LocateHandleBuffer (
+                  ByProtocol,
+                  &gEfiPciIoProtocolGuid,
+                  NULL,
+                  &HandleCount,
+                  &HandleBuffer);
+  if (EFI_ERROR (Status)) {
+    return (Status);
+  }
+
+  for (HIndex = 0; HIndex < HandleCount; ++HIndex) {
+    // If PciIo opened with EFI_OPEN_PROTOCOL_GET_PROTOCOL, the CloseProtocol() is not required
+    Status = gBS->OpenProtocol (
+                    HandleBuffer[HIndex],
+                    &gEfiPciIoProtocolGuid,
+                    (VOID **) PciIo,
+                    NULL,
+                    NULL,
+                    EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+    if (EFI_ERROR (Status)) {
+      continue;
+    }
+
+    Status = ReadMarvellYoukonPciId (*PciIo, JUNO_MARVELL_YUKON_ID);
+    if (EFI_ERROR (Status)) {
+      continue;
+    } else {
+      break;
+    }
+  }
+
+  gBS->FreePool (HandleBuffer);
+
+  return Status;
+}
+
+/**
+ This function restore the original controller attributes
+
+   @param[in]   PciIo               PCI IO protocol handle
+   @param[in]   PciAttr             PCI controller attributes.
+   @param[in]   AcpiResDescriptor   ACPI 2.0 resource descriptors for the BAR
+**/
+STATIC
+VOID
+RestorePciDev (
+  IN EFI_PCI_IO_PROTOCOL                *PciIo,
+  IN UINT64                             PciAttr
+  )
+{
+  PciIo->Attributes (
+           PciIo,
+           EfiPciIoAttributeOperationSet,
+           PciAttr,
+           NULL
+           );
+}
+
+/**
+ This function returns PCI MMIO base address for a controller
+
+   @param[in]   PciIo               PCI IO protocol handle
+   @param[out]  PciRegBase          PCI base MMIO address
+**/
+STATIC
+EFI_STATUS
+BarIsDeviceMemory (
+  IN   EFI_PCI_IO_PROTOCOL *PciIo,
+  OUT  UINT32              *PciRegBase
+  )
+{
+  EFI_STATUS                         Status;
+  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR  *AcpiResDescriptor;
+  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR  *AcpiCurrentDescriptor;
+
+  // Marvell Yukon's Bar0 provides base memory address for control registers
+  Status = PciIo->GetBarAttributes (PciIo, PCI_BAR_IDX0, NULL, (VOID**)&AcpiResDescriptor);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  AcpiCurrentDescriptor = AcpiResDescriptor;
+
+  // Search for a memory type descriptor
+  while (AcpiCurrentDescriptor->Desc != ACPI_END_TAG_DESCRIPTOR) {
+
+    // Check if Bar is memory type one and fetch a base address
+    if (AcpiCurrentDescriptor->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR &&
+        AcpiCurrentDescriptor->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM &&
+        !(AcpiCurrentDescriptor->SpecificFlag & ACPI_SPECFLAG_PREFETCHABLE)) {
+      *PciRegBase = AcpiCurrentDescriptor->AddrRangeMin;
+      break;
+    } else {
+      Status = EFI_UNSUPPORTED;
+    }
+
+    AcpiCurrentDescriptor = (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *) (AcpiCurrentDescriptor + 1);
+  }
+
+  gBS->FreePool (AcpiResDescriptor);
+
+  return Status;
+}
+
+/**
+ This function provides PCI MMIO base address, old PCI controller attributes.
+
+   @param[in]   PciIo               PCI IO protocol handle
+   @param[out]  PciRegBase          PCI base MMIO address
+   @param[out]  OldPciAttr          Old PCI controller attributes.
+**/
+STATIC
+EFI_STATUS
+InitPciDev (
+  IN EFI_PCI_IO_PROTOCOL                 *PciIo,
+  OUT UINT32                             *PciRegBase,
+  OUT UINT64                             *OldPciAttr
+  )
+{
+  UINT64      AttrSupports;
+  EFI_STATUS  Status;
+
+  // Get controller's current attributes
+  Status = PciIo->Attributes (
+                    PciIo,
+                    EfiPciIoAttributeOperationGet,
+                    0,
+                    OldPciAttr);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  // Fetch supported attributes
+  Status = PciIo->Attributes (
+                    PciIo,
+                    EfiPciIoAttributeOperationSupported,
+                    0,
+                    &AttrSupports);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  // Enable EFI_PCI_IO_ATTRIBUTE_IO, EFI_PCI_IO_ATTRIBUTE_MEMORY and
+  // EFI_PCI_IO_ATTRIBUTE_BUS_MASTER bits in the PCI Config Header
+  AttrSupports &= EFI_PCI_DEVICE_ENABLE;
+  Status = PciIo->Attributes (
+                    PciIo,
+                    EfiPciIoAttributeOperationEnable,
+                    AttrSupports,
+                    NULL);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  Status = BarIsDeviceMemory (PciIo, PciRegBase);
+  if (EFI_ERROR (Status)) {
+    RestorePciDev (PciIo, *OldPciAttr);
+  }
+
+  return Status;
+}
+
+/**
+ This function reads MAC address from IOFPGA and writes it to Marvell Yukon NIC
+
+   @param[in]   PciRegBase   PCI base MMIO address
+**/
+STATIC
+EFI_STATUS
+WriteMacAddress (
+  IN UINT32  PciRegBase
+  )
+{
+  UINT32  MacHigh;
+  UINT32  MacLow;
+
+  // Read MAC address from IOFPGA
+  MacHigh= MmioRead32 (ARM_JUNO_SYS_PCIGBE_H);
+  MacLow = MmioRead32 (ARM_JUNO_SYS_PCIGBE_L);
+
+  // Set software reset control register to protect from deactivation
+  // the config write state
+  MmioWrite16 (PciRegBase + R_CONTROL_STATUS, CS_RESET_CLR);
+
+  // Convert to Marvell MAC Address register format
+  MacHigh = SwapBytes32 ((MacHigh & 0xFFFF) << 16 |
+                         (MacLow & 0xFFFF0000) >> 16);
+  MacLow = SwapBytes32 (MacLow) >> 16;
+
+  // Set MAC Address
+  MmioWrite8 (PciRegBase + R_TST_CTRL_1, TST_CFG_WRITE_ENABLE);
+  MmioWrite32 (PciRegBase + R_MAC, MacHigh);
+  MmioWrite32 (PciRegBase + R_MAC_MAINT, MacHigh);
+  MmioWrite32 (PciRegBase + R_MAC + R_MAC_LOW, MacLow);
+  MmioWrite32 (PciRegBase + R_MAC_MAINT + R_MAC_LOW, MacLow);
+  MmioWrite8 (PciRegBase + R_TST_CTRL_1, TST_CFG_WRITE_DISABLE);
+
+  // Initiate device reset
+  MmioWrite16 (PciRegBase + R_CONTROL_STATUS, CS_RESET_SET);
+  MmioWrite16 (PciRegBase + R_CONTROL_STATUS, CS_RESET_CLR);
+
+  return EFI_SUCCESS;
+}
+
+/**
+  The function reads MAC address from Juno IOFPGA registers and writes it
+  into Marvell Yukon NIC.
+**/
+STATIC
+EFI_STATUS
+ArmJunoSetNicMacAddress ()
+{
+  UINT64                              OldPciAttr;
+  EFI_PCI_IO_PROTOCOL*                PciIo;
+  UINT32                              PciRegBase;
+  EFI_STATUS                          Status;
+
+  Status = GetMarvellYukonPciIoProtocol (&PciIo);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  Status = InitPciDev (PciIo, &PciRegBase, &OldPciAttr);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  Status = WriteMacAddress (PciRegBase);
+
+  RestorePciDev (PciIo, OldPciAttr);
+
+  return EFI_SUCCESS;
+}
+
+/**
   Notification function of the event defined as belonging to the
   EFI_END_OF_DXE_EVENT_GROUP_GUID event group that was created in
   the entry point of the driver.
@@ -106,6 +392,9 @@ OnEndOfDxe (
 
   Status = gBS->ConnectController (Handle, NULL, PciRootComplexDevicePath, FALSE);
   ASSERT_EFI_ERROR (Status);
+
+  Status = ArmJunoSetNicMacAddress ();
+  ASSERT_EFI_ERROR (Status);
 }
 
 STATIC
diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h
index 662c413..df02770 100644
--- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h
+++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h
@@ -29,6 +29,19 @@
 
 #include <IndustryStandard/Acpi.h>
 
+#define ACPI_SPECFLAG_PREFETCHABLE    0x06
+#define JUNO_MARVELL_YUKON_ID         0x438011AB /* Juno Marvell PCI Dev ID */
+#define TST_CFG_WRITE_ENABLE          0x02       /* Enable Config Write */
+#define TST_CFG_WRITE_DISABLE         0x00       /* Disable Config Write */
+#define CS_RESET_CLR                  0x02       /* SW Reset Clear */
+#define CS_RESET_SET                  0x00       /* SW Reset Set */
+#define R_CONTROL_STATUS              0x0004     /* Control/Status Register */
+#define R_MAC                         0x0100     /* MAC Address */
+#define R_MAC_MAINT                   0x0110     /* MAC Address Maintenance */
+#define R_MAC_LOW                     0x04       /* MAC Address Low Register Offset */
+#define R_TST_CTRL_1                  0x0158     /* Test Control Register 1 */
+
+
 EFI_STATUS
 PciEmulationEntryPoint (
   VOID
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Set Marvell Yukon MAC address
  2017-01-09 21:20 [PATCH v4] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Set Marvell Yukon MAC address Daniil Egranov
@ 2017-01-10 22:36 ` Leif Lindholm
  2017-01-11 11:45   ` Ryan Harkin
  0 siblings, 1 reply; 4+ messages in thread
From: Leif Lindholm @ 2017-01-10 22:36 UTC (permalink / raw)
  To: Daniil Egranov; +Cc: edk2-devel, Daniil Egranov

On Mon, Jan 09, 2017 at 03:20:51PM -0600, Daniil Egranov wrote:
> From: Daniil Egranov <daniil.egranov@linaro.org>
> 
> The patch reads a valid MAC address form the Juno IOFPGA registers 
> and pushes it into onboard Marvell Yukon NIC.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Daniil Egranov <daniil.egranov@linaro.org>

Thanks for sticking with it.

I fixed a typo in commit message an shortened the subject line
slightly, but:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Pushed as a8675a1

Regards,

Leif

> ---
> 
> Hi Leif,
> 
> The Marvell control registers should be accesable through the memory
> specified in BAR 0.
> 
> The new helper function should cover all 4 checks:
> - We can read BAR 0 attributes. 
>  We always read BAR 0
> - BAR 0 resources contain at least one QWORD Address Space Descriptor.
> - The first of these refers to something memory mapped.
> - That it is not (ARM terminology) Normal memory.
>  The BAR 0 for Marvell Yukon should always be a memory type. I am not 
>  sure if it's possible to have more than one descriptor for a BAR. The 
>  current implementation of PciIoGetBarAttributes() will return only one
>  descriptor based on BAR index following the End Tag one. This update 
>  will step through Address Space Descriptors anyway until the End Tag.  
> 
> Thanks,
> Daniil
> 
> Changelog:
> 
> v4
> Provided more comments on the code and corrected functions name.
> Created helper function for reading base memory address from BAR.
> Added stepping through ACPI address space descriptor until End Tag.
> 
> v3
> Changed code structure per Leif Lindholm's request
> 
> v2
> Corrected style issues and code structure.
> 
> v1
> The patch reads a valid MAC address form the Juno IOFPGA registers 
> and pushes it into onboard Marvell Yukon NIC.
> 
>  .../ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c     | 289 +++++++++++++++++++++
>  .../Drivers/ArmJunoDxe/ArmJunoDxeInternal.h        |  13 +
>  2 files changed, 302 insertions(+)
> 
> diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> index b97f044..5847d0a 100644
> --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> @@ -15,7 +15,9 @@
>  #include "ArmJunoDxeInternal.h"
>  #include <ArmPlatform.h>
>  
> +#include <IndustryStandard/Pci.h>
>  #include <Protocol/DevicePathFromText.h>
> +#include <Protocol/PciIo.h>
>  #include <Protocol/PciRootBridgeIo.h>
>  
>  #include <Guid/EventGroup.h>
> @@ -69,6 +71,290 @@ STATIC CONST EFI_PCI_ROOT_BRIDGE_DEVICE_PATH mPciRootComplexDevicePath = {
>  EFI_EVENT mAcpiRegistration = NULL;
>  
>  /**
> +  This function reads PCI ID of the controller.
> +
> +  @param[in]  PciIo   PCI IO protocol handle
> +  @param[in]  PciId   Looking for specified PCI ID Vendor/Device
> +**/
> +STATIC
> +EFI_STATUS
> +ReadMarvellYoukonPciId (
> +  IN EFI_PCI_IO_PROTOCOL  *PciIo,
> +  IN UINT32               PciId
> +  )
> +{
> +  UINT32      DevicePciId;
> +  EFI_STATUS  Status;
> +
> +  Status = PciIo->Pci.Read (
> +                        PciIo,
> +                        EfiPciIoWidthUint32,
> +                        PCI_VENDOR_ID_OFFSET,
> +                        1,
> +                        &DevicePciId);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  if (DevicePciId != PciId) {
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  This function searches for Marvell Yukon NIC on the Juno
> +  platform and returns PCI IO protocol handle for the controller.
> +
> +  @param[out]  PciIo   PCI IO protocol handle
> +**/
> +STATIC
> +EFI_STATUS
> +GetMarvellYukonPciIoProtocol (
> +  OUT EFI_PCI_IO_PROTOCOL  **PciIo
> +  )
> +{
> +  UINTN       HandleCount;
> +  EFI_HANDLE  *HandleBuffer;
> +  UINTN       HIndex;
> +  EFI_STATUS  Status;
> +
> +  Status = gBS->LocateHandleBuffer (
> +                  ByProtocol,
> +                  &gEfiPciIoProtocolGuid,
> +                  NULL,
> +                  &HandleCount,
> +                  &HandleBuffer);
> +  if (EFI_ERROR (Status)) {
> +    return (Status);
> +  }
> +
> +  for (HIndex = 0; HIndex < HandleCount; ++HIndex) {
> +    // If PciIo opened with EFI_OPEN_PROTOCOL_GET_PROTOCOL, the CloseProtocol() is not required
> +    Status = gBS->OpenProtocol (
> +                    HandleBuffer[HIndex],
> +                    &gEfiPciIoProtocolGuid,
> +                    (VOID **) PciIo,
> +                    NULL,
> +                    NULL,
> +                    EFI_OPEN_PROTOCOL_GET_PROTOCOL);
> +    if (EFI_ERROR (Status)) {
> +      continue;
> +    }
> +
> +    Status = ReadMarvellYoukonPciId (*PciIo, JUNO_MARVELL_YUKON_ID);
> +    if (EFI_ERROR (Status)) {
> +      continue;
> +    } else {
> +      break;
> +    }
> +  }
> +
> +  gBS->FreePool (HandleBuffer);
> +
> +  return Status;
> +}
> +
> +/**
> + This function restore the original controller attributes
> +
> +   @param[in]   PciIo               PCI IO protocol handle
> +   @param[in]   PciAttr             PCI controller attributes.
> +   @param[in]   AcpiResDescriptor   ACPI 2.0 resource descriptors for the BAR
> +**/
> +STATIC
> +VOID
> +RestorePciDev (
> +  IN EFI_PCI_IO_PROTOCOL                *PciIo,
> +  IN UINT64                             PciAttr
> +  )
> +{
> +  PciIo->Attributes (
> +           PciIo,
> +           EfiPciIoAttributeOperationSet,
> +           PciAttr,
> +           NULL
> +           );
> +}
> +
> +/**
> + This function returns PCI MMIO base address for a controller
> +
> +   @param[in]   PciIo               PCI IO protocol handle
> +   @param[out]  PciRegBase          PCI base MMIO address
> +**/
> +STATIC
> +EFI_STATUS
> +BarIsDeviceMemory (
> +  IN   EFI_PCI_IO_PROTOCOL *PciIo,
> +  OUT  UINT32              *PciRegBase
> +  )
> +{
> +  EFI_STATUS                         Status;
> +  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR  *AcpiResDescriptor;
> +  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR  *AcpiCurrentDescriptor;
> +
> +  // Marvell Yukon's Bar0 provides base memory address for control registers
> +  Status = PciIo->GetBarAttributes (PciIo, PCI_BAR_IDX0, NULL, (VOID**)&AcpiResDescriptor);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  AcpiCurrentDescriptor = AcpiResDescriptor;
> +
> +  // Search for a memory type descriptor
> +  while (AcpiCurrentDescriptor->Desc != ACPI_END_TAG_DESCRIPTOR) {
> +
> +    // Check if Bar is memory type one and fetch a base address
> +    if (AcpiCurrentDescriptor->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR &&
> +        AcpiCurrentDescriptor->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM &&
> +        !(AcpiCurrentDescriptor->SpecificFlag & ACPI_SPECFLAG_PREFETCHABLE)) {
> +      *PciRegBase = AcpiCurrentDescriptor->AddrRangeMin;
> +      break;
> +    } else {
> +      Status = EFI_UNSUPPORTED;
> +    }
> +
> +    AcpiCurrentDescriptor = (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *) (AcpiCurrentDescriptor + 1);
> +  }
> +
> +  gBS->FreePool (AcpiResDescriptor);
> +
> +  return Status;
> +}
> +
> +/**
> + This function provides PCI MMIO base address, old PCI controller attributes.
> +
> +   @param[in]   PciIo               PCI IO protocol handle
> +   @param[out]  PciRegBase          PCI base MMIO address
> +   @param[out]  OldPciAttr          Old PCI controller attributes.
> +**/
> +STATIC
> +EFI_STATUS
> +InitPciDev (
> +  IN EFI_PCI_IO_PROTOCOL                 *PciIo,
> +  OUT UINT32                             *PciRegBase,
> +  OUT UINT64                             *OldPciAttr
> +  )
> +{
> +  UINT64      AttrSupports;
> +  EFI_STATUS  Status;
> +
> +  // Get controller's current attributes
> +  Status = PciIo->Attributes (
> +                    PciIo,
> +                    EfiPciIoAttributeOperationGet,
> +                    0,
> +                    OldPciAttr);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  // Fetch supported attributes
> +  Status = PciIo->Attributes (
> +                    PciIo,
> +                    EfiPciIoAttributeOperationSupported,
> +                    0,
> +                    &AttrSupports);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  // Enable EFI_PCI_IO_ATTRIBUTE_IO, EFI_PCI_IO_ATTRIBUTE_MEMORY and
> +  // EFI_PCI_IO_ATTRIBUTE_BUS_MASTER bits in the PCI Config Header
> +  AttrSupports &= EFI_PCI_DEVICE_ENABLE;
> +  Status = PciIo->Attributes (
> +                    PciIo,
> +                    EfiPciIoAttributeOperationEnable,
> +                    AttrSupports,
> +                    NULL);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  Status = BarIsDeviceMemory (PciIo, PciRegBase);
> +  if (EFI_ERROR (Status)) {
> +    RestorePciDev (PciIo, *OldPciAttr);
> +  }
> +
> +  return Status;
> +}
> +
> +/**
> + This function reads MAC address from IOFPGA and writes it to Marvell Yukon NIC
> +
> +   @param[in]   PciRegBase   PCI base MMIO address
> +**/
> +STATIC
> +EFI_STATUS
> +WriteMacAddress (
> +  IN UINT32  PciRegBase
> +  )
> +{
> +  UINT32  MacHigh;
> +  UINT32  MacLow;
> +
> +  // Read MAC address from IOFPGA
> +  MacHigh= MmioRead32 (ARM_JUNO_SYS_PCIGBE_H);
> +  MacLow = MmioRead32 (ARM_JUNO_SYS_PCIGBE_L);
> +
> +  // Set software reset control register to protect from deactivation
> +  // the config write state
> +  MmioWrite16 (PciRegBase + R_CONTROL_STATUS, CS_RESET_CLR);
> +
> +  // Convert to Marvell MAC Address register format
> +  MacHigh = SwapBytes32 ((MacHigh & 0xFFFF) << 16 |
> +                         (MacLow & 0xFFFF0000) >> 16);
> +  MacLow = SwapBytes32 (MacLow) >> 16;
> +
> +  // Set MAC Address
> +  MmioWrite8 (PciRegBase + R_TST_CTRL_1, TST_CFG_WRITE_ENABLE);
> +  MmioWrite32 (PciRegBase + R_MAC, MacHigh);
> +  MmioWrite32 (PciRegBase + R_MAC_MAINT, MacHigh);
> +  MmioWrite32 (PciRegBase + R_MAC + R_MAC_LOW, MacLow);
> +  MmioWrite32 (PciRegBase + R_MAC_MAINT + R_MAC_LOW, MacLow);
> +  MmioWrite8 (PciRegBase + R_TST_CTRL_1, TST_CFG_WRITE_DISABLE);
> +
> +  // Initiate device reset
> +  MmioWrite16 (PciRegBase + R_CONTROL_STATUS, CS_RESET_SET);
> +  MmioWrite16 (PciRegBase + R_CONTROL_STATUS, CS_RESET_CLR);
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  The function reads MAC address from Juno IOFPGA registers and writes it
> +  into Marvell Yukon NIC.
> +**/
> +STATIC
> +EFI_STATUS
> +ArmJunoSetNicMacAddress ()
> +{
> +  UINT64                              OldPciAttr;
> +  EFI_PCI_IO_PROTOCOL*                PciIo;
> +  UINT32                              PciRegBase;
> +  EFI_STATUS                          Status;
> +
> +  Status = GetMarvellYukonPciIoProtocol (&PciIo);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  Status = InitPciDev (PciIo, &PciRegBase, &OldPciAttr);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  Status = WriteMacAddress (PciRegBase);
> +
> +  RestorePciDev (PciIo, OldPciAttr);
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/**
>    Notification function of the event defined as belonging to the
>    EFI_END_OF_DXE_EVENT_GROUP_GUID event group that was created in
>    the entry point of the driver.
> @@ -106,6 +392,9 @@ OnEndOfDxe (
>  
>    Status = gBS->ConnectController (Handle, NULL, PciRootComplexDevicePath, FALSE);
>    ASSERT_EFI_ERROR (Status);
> +
> +  Status = ArmJunoSetNicMacAddress ();
> +  ASSERT_EFI_ERROR (Status);
>  }
>  
>  STATIC
> diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h
> index 662c413..df02770 100644
> --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h
> +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h
> @@ -29,6 +29,19 @@
>  
>  #include <IndustryStandard/Acpi.h>
>  
> +#define ACPI_SPECFLAG_PREFETCHABLE    0x06
> +#define JUNO_MARVELL_YUKON_ID         0x438011AB /* Juno Marvell PCI Dev ID */
> +#define TST_CFG_WRITE_ENABLE          0x02       /* Enable Config Write */
> +#define TST_CFG_WRITE_DISABLE         0x00       /* Disable Config Write */
> +#define CS_RESET_CLR                  0x02       /* SW Reset Clear */
> +#define CS_RESET_SET                  0x00       /* SW Reset Set */
> +#define R_CONTROL_STATUS              0x0004     /* Control/Status Register */
> +#define R_MAC                         0x0100     /* MAC Address */
> +#define R_MAC_MAINT                   0x0110     /* MAC Address Maintenance */
> +#define R_MAC_LOW                     0x04       /* MAC Address Low Register Offset */
> +#define R_TST_CTRL_1                  0x0158     /* Test Control Register 1 */
> +
> +
>  EFI_STATUS
>  PciEmulationEntryPoint (
>    VOID
> -- 
> 2.7.4
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Set Marvell Yukon MAC address
  2017-01-10 22:36 ` Leif Lindholm
@ 2017-01-11 11:45   ` Ryan Harkin
  2017-01-11 12:24     ` Ryan Harkin
  0 siblings, 1 reply; 4+ messages in thread
From: Ryan Harkin @ 2017-01-11 11:45 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: Daniil Egranov, edk2-devel@lists.01.org, Daniil Egranov

On 10 January 2017 at 22:36, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Mon, Jan 09, 2017 at 03:20:51PM -0600, Daniil Egranov wrote:
>> From: Daniil Egranov <daniil.egranov@linaro.org>
>>
>> The patch reads a valid MAC address form the Juno IOFPGA registers
>> and pushes it into onboard Marvell Yukon NIC.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Daniil Egranov <daniil.egranov@linaro.org>
>
> Thanks for sticking with it.
>
> I fixed a typo in commit message an shortened the subject line
> slightly, but:
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> Pushed as a8675a1
>

Excellent, another patch in my fork that can go.

Daniil,

When you originally posted this, you posted it along with
"EmbeddedPkg: Added device configuration protocol". As Leif is happy
with this patch as-is, I assume it isn't needed any more. You haven't
chased it, you didn't respond to my ping about it and it's currently
in my tree.

Unless you get back to me about it, I'll drop it next time I rebase.

Cheers,
Ryan.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Set Marvell Yukon MAC address
  2017-01-11 11:45   ` Ryan Harkin
@ 2017-01-11 12:24     ` Ryan Harkin
  0 siblings, 0 replies; 4+ messages in thread
From: Ryan Harkin @ 2017-01-11 12:24 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: Daniil Egranov, edk2-devel@lists.01.org, Daniil Egranov

On 11 January 2017 at 11:45, Ryan Harkin <ryan.harkin@linaro.org> wrote:
> On 10 January 2017 at 22:36, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> On Mon, Jan 09, 2017 at 03:20:51PM -0600, Daniil Egranov wrote:
>>> From: Daniil Egranov <daniil.egranov@linaro.org>
>>>
>>> The patch reads a valid MAC address form the Juno IOFPGA registers
>>> and pushes it into onboard Marvell Yukon NIC.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Daniil Egranov <daniil.egranov@linaro.org>
>>
>> Thanks for sticking with it.
>>
>> I fixed a typo in commit message an shortened the subject line
>> slightly, but:
>> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>> Pushed as a8675a1
>>
>
> Excellent, another patch in my fork that can go.
>

Unfortunately, while this works on Juno R1 and R2, it hangs R0 every time:

ASSERT [ArmJunoDxe]
/linaro/platforms/uefi/edk2/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c(397):
!EFI_ERROR (Status)

I guess that before the refactor, it would return EFI_SUCCESS on R0
when the MAC configuration failed...

> Daniil,
>
> When you originally posted this, you posted it along with
> "EmbeddedPkg: Added device configuration protocol". As Leif is happy
> with this patch as-is, I assume it isn't needed any more. You haven't
> chased it, you didn't respond to my ping about it and it's currently
> in my tree.
>
> Unless you get back to me about it, I'll drop it next time I rebase.
>

And I tested with this patch removed and everything still seemed to
work, so I'm assuming it's not needed and leaving it out unless you
explain otherwise.

> Cheers,
> Ryan.


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-01-11 12:24 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-09 21:20 [PATCH v4] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Set Marvell Yukon MAC address Daniil Egranov
2017-01-10 22:36 ` Leif Lindholm
2017-01-11 11:45   ` Ryan Harkin
2017-01-11 12:24     ` Ryan Harkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox