public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Set Marvell Yukon MAC address
@ 2016-10-06  1:42 Daniil Egranov
  2016-11-01 17:55 ` Ryan Harkin
  2016-12-08 16:48 ` Leif Lindholm
  0 siblings, 2 replies; 6+ messages in thread
From: Daniil Egranov @ 2016-10-06  1:42 UTC (permalink / raw)
  To: edk2-devel; +Cc: leif.lindholm, alan, Daniil Egranov

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@arm.com>
---
 .../ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c     | 141 +++++++++++++++++++++
 .../Drivers/ArmJunoDxe/ArmJunoDxeInternal.h        |  12 ++
 2 files changed, 153 insertions(+)

diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
index b97f044..0c5fbd0 100644
--- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
+++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
@@ -17,6 +17,8 @@
 
 #include <Protocol/DevicePathFromText.h>
 #include <Protocol/PciRootBridgeIo.h>
+#include <Protocol/PciIo.h>
+#include <IndustryStandard/Pci.h>
 
 #include <Guid/EventGroup.h>
 #include <Guid/GlobalVariable.h>
@@ -68,6 +70,142 @@ STATIC CONST EFI_PCI_ROOT_BRIDGE_DEVICE_PATH mPciRootComplexDevicePath = {
 
 EFI_EVENT mAcpiRegistration = NULL;
 
+UINT32 SwapUINT32(UINT32 value)
+{
+  value = ((value << 8) & 0xFF00FF00 ) | ((value >> 8) & 0xFF00FF );
+  return (value << 16) | (value >> 16);
+}
+
+/**
+  The function reads MAC address from Juno IOFPGA registers and writes it
+  into Marvell Yukon NIC.
+**/
+STATIC
+EFI_STATUS
+ArmJunoSetNetworkMAC()
+{
+
+  EFI_STATUS                          Status;
+  UINTN                               HandleCount;
+  EFI_HANDLE                          *HandleBuffer;
+  UINTN                               HIndex;
+  EFI_PCI_IO_PROTOCOL*                PciIo;
+  UINT64                              PciID;
+  UINT32                              MacHigh;
+  UINT32                              MacLow;
+  UINT32                              PciRegBase;
+  UINT64                              OldPciAttributes;
+  UINT64                              AttrSupports;
+  UINT8                               *PciBarAttributes;
+
+  Status = gBS->LocateHandleBuffer (ByProtocol,
+                                    &gEfiPciIoProtocolGuid,
+                                    NULL, &HandleCount, &HandleBuffer);
+
+  if (!EFI_ERROR (Status)) {
+    for (HIndex = 0; HIndex < HandleCount; ++HIndex) {
+      Status = gBS->OpenProtocol (
+                      HandleBuffer[HIndex],
+                      &gEfiPciIoProtocolGuid,
+                      (VOID **) &PciIo,
+                      NULL,
+                      NULL,
+                      EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+
+      if (EFI_ERROR (Status)) {
+        continue;
+      }
+
+      Status = PciIo->Pci.Read (
+            PciIo,
+            EfiPciIoWidthUint32,
+            PCI_VENDOR_ID_OFFSET,
+            1,
+            &PciID
+            );
+
+      if (EFI_ERROR (Status)) {
+        continue;
+      }
+
+      if ((PciID & 0xFFFFFFFF) == JUNO_MARVELL_YUKON_ID) {
+
+        // Read MAC address from IOFPGA
+        MacHigh= MmioRead32 (ARM_JUNO_SYS_PCIGBE_H);
+        MacLow = MmioRead32 (ARM_JUNO_SYS_PCIGBE_L);
+
+        Status = PciIo->Attributes (
+              PciIo,
+              EfiPciIoAttributeOperationGet,
+              0,
+              &OldPciAttributes
+              );
+
+        if (EFI_ERROR (Status)) {
+          continue;
+        }
+
+        Status = PciIo->Attributes (
+              PciIo,
+              EfiPciIoAttributeOperationSupported,
+              0,
+              &AttrSupports
+              );
+
+        if (!EFI_ERROR (Status)) {
+          AttrSupports &= EFI_PCI_DEVICE_ENABLE;
+          Status = PciIo->Attributes (
+                PciIo,
+                EfiPciIoAttributeOperationEnable,
+                AttrSupports,
+                NULL
+                );
+
+          Status = PciIo->GetBarAttributes (PciIo, 0, &AttrSupports, (VOID**)&PciBarAttributes);
+          if (!EFI_ERROR (Status) && (((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)PciBarAttributes)->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR)) {
+            if (((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)PciBarAttributes)->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM) {
+              if (!(((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)PciBarAttributes)->SpecificFlag & ACPI_SPECFLAG_PREFETCHABLE)) {
+                PciRegBase = ((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)PciBarAttributes)->AddrRangeMin;
+
+                // Clear Software Reset
+                MmioWrite16 (PciRegBase + R_CONTROL_STATUS, CS_RESET_CLR);
+
+                // Convert to Marvell MAC Address register format
+                MacHigh = SwapUINT32 ((MacHigh & 0xFFFF) << 16 |
+                                     (MacLow & 0xFFFF0000) >> 16);
+                MacLow = SwapUINT32 (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 + 4, MacLow);
+                MmioWrite32 (PciRegBase + R_MAC_MAINT + 4, MacLow);
+                MmioWrite8 (PciRegBase + R_TST_CTRL_1, TST_CFG_WRITE_DISABLE);
+
+                // Soft reset
+                MmioWrite16 (PciRegBase + R_CONTROL_STATUS, CS_RESET_SET);
+                MmioWrite16 (PciRegBase + R_CONTROL_STATUS, CS_RESET_CLR);
+
+                Status = EFI_SUCCESS;
+              }
+            }
+          }
+
+          PciIo->Attributes (
+                PciIo,
+                EfiPciIoAttributeOperationSet,
+                OldPciAttributes,
+                NULL
+                );
+        }
+      }
+    }
+  }
+
+  return Status;
+}
+
 /**
   Notification function of the event defined as belonging to the
   EFI_END_OF_DXE_EVENT_GROUP_GUID event group that was created in
@@ -106,6 +244,9 @@ OnEndOfDxe (
 
   Status = gBS->ConnectController (Handle, NULL, PciRootComplexDevicePath, FALSE);
   ASSERT_EFI_ERROR (Status);
+
+  Status = ArmJunoSetNetworkMAC();
+  ASSERT_EFI_ERROR (Status);
 }
 
 STATIC
diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h
index 662c413..cb8fdf6 100644
--- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h
+++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h
@@ -29,6 +29,18 @@
 
 #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_TST_CTRL_1                  0x0158     /* Test Control Register 1 */
+
+
 EFI_STATUS
 PciEmulationEntryPoint (
   VOID
-- 
2.7.4



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

* Re: [PATCH] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Set Marvell Yukon MAC address
  2016-10-06  1:42 [PATCH] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Set Marvell Yukon MAC address Daniil Egranov
@ 2016-11-01 17:55 ` Ryan Harkin
  2016-11-01 21:05   ` Leif Lindholm
  2016-12-08 16:48 ` Leif Lindholm
  1 sibling, 1 reply; 6+ messages in thread
From: Ryan Harkin @ 2016-11-01 17:55 UTC (permalink / raw)
  To: Daniil Egranov; +Cc: edk2-devel-01, alan, Leif Lindholm

Hi Daniil,

While looking for another patch, I found this on the maillist...

On 6 October 2016 at 02:42, Daniil Egranov <daniil.egranov@arm.com> wrote:
> 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@arm.com>
> ---
>  .../ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c     | 141 +++++++++++++++++++++
>  .../Drivers/ArmJunoDxe/ArmJunoDxeInternal.h        |  12 ++
>  2 files changed, 153 insertions(+)
>
> diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> index b97f044..0c5fbd0 100644
> --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> @@ -17,6 +17,8 @@
>
>  #include <Protocol/DevicePathFromText.h>
>  #include <Protocol/PciRootBridgeIo.h>
> +#include <Protocol/PciIo.h>
> +#include <IndustryStandard/Pci.h>
>
>  #include <Guid/EventGroup.h>
>  #include <Guid/GlobalVariable.h>
> @@ -68,6 +70,142 @@ STATIC CONST EFI_PCI_ROOT_BRIDGE_DEVICE_PATH mPciRootComplexDevicePath = {
>
>  EFI_EVENT mAcpiRegistration = NULL;
>
> +UINT32 SwapUINT32(UINT32 value)
> +{
> +  value = ((value << 8) & 0xFF00FF00 ) | ((value >> 8) & 0xFF00FF );
> +  return (value << 16) | (value >> 16);
> +}
> +
> +/**
> +  The function reads MAC address from Juno IOFPGA registers and writes it
> +  into Marvell Yukon NIC.
> +**/
> +STATIC
> +EFI_STATUS
> +ArmJunoSetNetworkMAC()
> +{
> +
> +  EFI_STATUS                          Status;
> +  UINTN                               HandleCount;
> +  EFI_HANDLE                          *HandleBuffer;
> +  UINTN                               HIndex;
> +  EFI_PCI_IO_PROTOCOL*                PciIo;
> +  UINT64                              PciID;
> +  UINT32                              MacHigh;
> +  UINT32                              MacLow;
> +  UINT32                              PciRegBase;
> +  UINT64                              OldPciAttributes;
> +  UINT64                              AttrSupports;
> +  UINT8                               *PciBarAttributes;
> +
> +  Status = gBS->LocateHandleBuffer (ByProtocol,
> +                                    &gEfiPciIoProtocolGuid,
> +                                    NULL, &HandleCount, &HandleBuffer);
> +
> +  if (!EFI_ERROR (Status)) {
> +    for (HIndex = 0; HIndex < HandleCount; ++HIndex) {
> +      Status = gBS->OpenProtocol (
> +                      HandleBuffer[HIndex],
> +                      &gEfiPciIoProtocolGuid,
> +                      (VOID **) &PciIo,
> +                      NULL,
> +                      NULL,
> +                      EFI_OPEN_PROTOCOL_GET_PROTOCOL);
> +
> +      if (EFI_ERROR (Status)) {
> +        continue;
> +      }
> +
> +      Status = PciIo->Pci.Read (
> +            PciIo,
> +            EfiPciIoWidthUint32,
> +            PCI_VENDOR_ID_OFFSET,
> +            1,
> +            &PciID
> +            );
> +
> +      if (EFI_ERROR (Status)) {
> +        continue;
> +      }
> +
> +      if ((PciID & 0xFFFFFFFF) == JUNO_MARVELL_YUKON_ID) {
> +
> +        // Read MAC address from IOFPGA
> +        MacHigh= MmioRead32 (ARM_JUNO_SYS_PCIGBE_H);
> +        MacLow = MmioRead32 (ARM_JUNO_SYS_PCIGBE_L);
> +
> +        Status = PciIo->Attributes (
> +              PciIo,
> +              EfiPciIoAttributeOperationGet,
> +              0,
> +              &OldPciAttributes
> +              );
> +
> +        if (EFI_ERROR (Status)) {
> +          continue;
> +        }
> +
> +        Status = PciIo->Attributes (
> +              PciIo,
> +              EfiPciIoAttributeOperationSupported,
> +              0,
> +              &AttrSupports
> +              );
> +
> +        if (!EFI_ERROR (Status)) {
> +          AttrSupports &= EFI_PCI_DEVICE_ENABLE;
> +          Status = PciIo->Attributes (
> +                PciIo,
> +                EfiPciIoAttributeOperationEnable,
> +                AttrSupports,
> +                NULL
> +                );
> +
> +          Status = PciIo->GetBarAttributes (PciIo, 0, &AttrSupports, (VOID**)&PciBarAttributes);
> +          if (!EFI_ERROR (Status) && (((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)PciBarAttributes)->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR)) {
> +            if (((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)PciBarAttributes)->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM) {
> +              if (!(((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)PciBarAttributes)->SpecificFlag & ACPI_SPECFLAG_PREFETCHABLE)) {
> +                PciRegBase = ((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)PciBarAttributes)->AddrRangeMin;
> +
> +                // Clear Software Reset
> +                MmioWrite16 (PciRegBase + R_CONTROL_STATUS, CS_RESET_CLR);
> +
> +                // Convert to Marvell MAC Address register format
> +                MacHigh = SwapUINT32 ((MacHigh & 0xFFFF) << 16 |
> +                                     (MacLow & 0xFFFF0000) >> 16);
> +                MacLow = SwapUINT32 (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 + 4, MacLow);
> +                MmioWrite32 (PciRegBase + R_MAC_MAINT + 4, MacLow);
> +                MmioWrite8 (PciRegBase + R_TST_CTRL_1, TST_CFG_WRITE_DISABLE);
> +
> +                // Soft reset
> +                MmioWrite16 (PciRegBase + R_CONTROL_STATUS, CS_RESET_SET);
> +                MmioWrite16 (PciRegBase + R_CONTROL_STATUS, CS_RESET_CLR);
> +
> +                Status = EFI_SUCCESS;
> +              }
> +            }
> +          }
> +
> +          PciIo->Attributes (
> +                PciIo,
> +                EfiPciIoAttributeOperationSet,
> +                OldPciAttributes,
> +                NULL
> +                );
> +        }
> +      }
> +    }
> +  }
> +
> +  return Status;
> +}
> +
>  /**
>    Notification function of the event defined as belonging to the
>    EFI_END_OF_DXE_EVENT_GROUP_GUID event group that was created in
> @@ -106,6 +244,9 @@ OnEndOfDxe (
>
>    Status = gBS->ConnectController (Handle, NULL, PciRootComplexDevicePath, FALSE);
>    ASSERT_EFI_ERROR (Status);
> +
> +  Status = ArmJunoSetNetworkMAC();
> +  ASSERT_EFI_ERROR (Status);
>  }
>
>  STATIC
> diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h
> index 662c413..cb8fdf6 100644
> --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h
> +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h
> @@ -29,6 +29,18 @@
>
>  #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_TST_CTRL_1                  0x0158     /* Test Control Register 1 */
> +
> +
>  EFI_STATUS
>  PciEmulationEntryPoint (
>    VOID
> --
> 2.7.4
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

This appears to be an updated version of the patch I've been using for
a few months now:

https://git.linaro.org/landing-teams/working/arm/edk2.git/commit/?id=cd9a7a18eebfd2c8683a5663ce6447d3800ccf9b

Is the intention that this one should go upstream and I drop mine from
my fork?  If so, I guess I should test it.

Cheers,
Ryan.


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

* Re: [PATCH] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Set Marvell Yukon MAC address
  2016-11-01 17:55 ` Ryan Harkin
@ 2016-11-01 21:05   ` Leif Lindholm
  2016-11-02 11:55     ` Ryan Harkin
  0 siblings, 1 reply; 6+ messages in thread
From: Leif Lindholm @ 2016-11-01 21:05 UTC (permalink / raw)
  To: Ryan Harkin; +Cc: Daniil Egranov, edk2-devel-01, alan

On Tue, Nov 01, 2016 at 05:55:11PM +0000, Ryan Harkin wrote:
> Hi Daniil,
> 
> While looking for another patch, I found this on the maillist...
> 
> On 6 October 2016 at 02:42, Daniil Egranov <daniil.egranov@arm.com> wrote:
> > 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@arm.com>
> > ---
> >  .../ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c     | 141 +++++++++++++++++++++
> >  .../Drivers/ArmJunoDxe/ArmJunoDxeInternal.h        |  12 ++
> >  2 files changed, 153 insertions(+)
> >
> > diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> > index b97f044..0c5fbd0 100644
> > --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> > +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> > @@ -17,6 +17,8 @@
> >
> >  #include <Protocol/DevicePathFromText.h>
> >  #include <Protocol/PciRootBridgeIo.h>
> > +#include <Protocol/PciIo.h>
> > +#include <IndustryStandard/Pci.h>
> >
> >  #include <Guid/EventGroup.h>
> >  #include <Guid/GlobalVariable.h>
> > @@ -68,6 +70,142 @@ STATIC CONST EFI_PCI_ROOT_BRIDGE_DEVICE_PATH mPciRootComplexDevicePath = {
> >
> >  EFI_EVENT mAcpiRegistration = NULL;
> >
> > +UINT32 SwapUINT32(UINT32 value)
> > +{
> > +  value = ((value << 8) & 0xFF00FF00 ) | ((value >> 8) & 0xFF00FF );
> > +  return (value << 16) | (value >> 16);
> > +}
> > +
> > +/**
> > +  The function reads MAC address from Juno IOFPGA registers and writes it
> > +  into Marvell Yukon NIC.
> > +**/
> > +STATIC
> > +EFI_STATUS
> > +ArmJunoSetNetworkMAC()
> > +{
> > +
> > +  EFI_STATUS                          Status;
> > +  UINTN                               HandleCount;
> > +  EFI_HANDLE                          *HandleBuffer;
> > +  UINTN                               HIndex;
> > +  EFI_PCI_IO_PROTOCOL*                PciIo;
> > +  UINT64                              PciID;
> > +  UINT32                              MacHigh;
> > +  UINT32                              MacLow;
> > +  UINT32                              PciRegBase;
> > +  UINT64                              OldPciAttributes;
> > +  UINT64                              AttrSupports;
> > +  UINT8                               *PciBarAttributes;
> > +
> > +  Status = gBS->LocateHandleBuffer (ByProtocol,
> > +                                    &gEfiPciIoProtocolGuid,
> > +                                    NULL, &HandleCount, &HandleBuffer);
> > +
> > +  if (!EFI_ERROR (Status)) {
> > +    for (HIndex = 0; HIndex < HandleCount; ++HIndex) {
> > +      Status = gBS->OpenProtocol (
> > +                      HandleBuffer[HIndex],
> > +                      &gEfiPciIoProtocolGuid,
> > +                      (VOID **) &PciIo,
> > +                      NULL,
> > +                      NULL,
> > +                      EFI_OPEN_PROTOCOL_GET_PROTOCOL);
> > +
> > +      if (EFI_ERROR (Status)) {
> > +        continue;
> > +      }
> > +
> > +      Status = PciIo->Pci.Read (
> > +            PciIo,
> > +            EfiPciIoWidthUint32,
> > +            PCI_VENDOR_ID_OFFSET,
> > +            1,
> > +            &PciID
> > +            );
> > +
> > +      if (EFI_ERROR (Status)) {
> > +        continue;
> > +      }
> > +
> > +      if ((PciID & 0xFFFFFFFF) == JUNO_MARVELL_YUKON_ID) {
> > +
> > +        // Read MAC address from IOFPGA
> > +        MacHigh= MmioRead32 (ARM_JUNO_SYS_PCIGBE_H);
> > +        MacLow = MmioRead32 (ARM_JUNO_SYS_PCIGBE_L);
> > +
> > +        Status = PciIo->Attributes (
> > +              PciIo,
> > +              EfiPciIoAttributeOperationGet,
> > +              0,
> > +              &OldPciAttributes
> > +              );
> > +
> > +        if (EFI_ERROR (Status)) {
> > +          continue;
> > +        }
> > +
> > +        Status = PciIo->Attributes (
> > +              PciIo,
> > +              EfiPciIoAttributeOperationSupported,
> > +              0,
> > +              &AttrSupports
> > +              );
> > +
> > +        if (!EFI_ERROR (Status)) {
> > +          AttrSupports &= EFI_PCI_DEVICE_ENABLE;
> > +          Status = PciIo->Attributes (
> > +                PciIo,
> > +                EfiPciIoAttributeOperationEnable,
> > +                AttrSupports,
> > +                NULL
> > +                );
> > +
> > +          Status = PciIo->GetBarAttributes (PciIo, 0, &AttrSupports, (VOID**)&PciBarAttributes);
> > +          if (!EFI_ERROR (Status) && (((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)PciBarAttributes)->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR)) {
> > +            if (((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)PciBarAttributes)->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM) {
> > +              if (!(((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)PciBarAttributes)->SpecificFlag & ACPI_SPECFLAG_PREFETCHABLE)) {
> > +                PciRegBase = ((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)PciBarAttributes)->AddrRangeMin;
> > +
> > +                // Clear Software Reset
> > +                MmioWrite16 (PciRegBase + R_CONTROL_STATUS, CS_RESET_CLR);
> > +
> > +                // Convert to Marvell MAC Address register format
> > +                MacHigh = SwapUINT32 ((MacHigh & 0xFFFF) << 16 |
> > +                                     (MacLow & 0xFFFF0000) >> 16);
> > +                MacLow = SwapUINT32 (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 + 4, MacLow);
> > +                MmioWrite32 (PciRegBase + R_MAC_MAINT + 4, MacLow);
> > +                MmioWrite8 (PciRegBase + R_TST_CTRL_1, TST_CFG_WRITE_DISABLE);
> > +
> > +                // Soft reset
> > +                MmioWrite16 (PciRegBase + R_CONTROL_STATUS, CS_RESET_SET);
> > +                MmioWrite16 (PciRegBase + R_CONTROL_STATUS, CS_RESET_CLR);
> > +
> > +                Status = EFI_SUCCESS;
> > +              }
> > +            }
> > +          }
> > +
> > +          PciIo->Attributes (
> > +                PciIo,
> > +                EfiPciIoAttributeOperationSet,
> > +                OldPciAttributes,
> > +                NULL
> > +                );
> > +        }
> > +      }
> > +    }
> > +  }
> > +
> > +  return Status;
> > +}
> > +
> >  /**
> >    Notification function of the event defined as belonging to the
> >    EFI_END_OF_DXE_EVENT_GROUP_GUID event group that was created in
> > @@ -106,6 +244,9 @@ OnEndOfDxe (
> >
> >    Status = gBS->ConnectController (Handle, NULL, PciRootComplexDevicePath, FALSE);
> >    ASSERT_EFI_ERROR (Status);
> > +
> > +  Status = ArmJunoSetNetworkMAC();
> > +  ASSERT_EFI_ERROR (Status);
> >  }
> >
> >  STATIC
> > diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h
> > index 662c413..cb8fdf6 100644
> > --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h
> > +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h
> > @@ -29,6 +29,18 @@
> >
> >  #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_TST_CTRL_1                  0x0158     /* Test Control Register 1 */
> > +
> > +
> >  EFI_STATUS
> >  PciEmulationEntryPoint (
> >    VOID
> > --
> > 2.7.4
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> 
> This appears to be an updated version of the patch I've been using for
> a few months now:
> 
> https://git.linaro.org/landing-teams/working/arm/edk2.git/commit/?id=cd9a7a18eebfd2c8683a5663ce6447d3800ccf9b
> 
> Is the intention that this one should go upstream and I drop mine from
> my fork?  If so, I guess I should test it.

That would be ideal, yes.
If you can confirm it works as expected, I'll start nitpicking on the
code :)

Regards,

Leif

> 
> Cheers,
> Ryan.


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

* Re: [PATCH] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Set Marvell Yukon MAC address
  2016-11-01 21:05   ` Leif Lindholm
@ 2016-11-02 11:55     ` Ryan Harkin
  2016-12-08 16:23       ` Ryan Harkin
  0 siblings, 1 reply; 6+ messages in thread
From: Ryan Harkin @ 2016-11-02 11:55 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: Daniil Egranov, edk2-devel-01, alan

On 1 November 2016 at 21:05, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Tue, Nov 01, 2016 at 05:55:11PM +0000, Ryan Harkin wrote:
>> Hi Daniil,
>>
>> While looking for another patch, I found this on the maillist...
>>
>> On 6 October 2016 at 02:42, Daniil Egranov <daniil.egranov@arm.com> wrote:
>> > 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@arm.com>

Tested on Juno R0/1/2.

Tested-by; Ryan Harkin <ryan.harkin@linaro.org>


>> > ---
>> >  .../ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c     | 141 +++++++++++++++++++++
>> >  .../Drivers/ArmJunoDxe/ArmJunoDxeInternal.h        |  12 ++
>> >  2 files changed, 153 insertions(+)
>> >
>> > diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
>> > index b97f044..0c5fbd0 100644
>> > --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
>> > +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
>> > @@ -17,6 +17,8 @@
>> >
>> >  #include <Protocol/DevicePathFromText.h>
>> >  #include <Protocol/PciRootBridgeIo.h>
>> > +#include <Protocol/PciIo.h>
>> > +#include <IndustryStandard/Pci.h>
>> >
>> >  #include <Guid/EventGroup.h>
>> >  #include <Guid/GlobalVariable.h>
>> > @@ -68,6 +70,142 @@ STATIC CONST EFI_PCI_ROOT_BRIDGE_DEVICE_PATH mPciRootComplexDevicePath = {
>> >
>> >  EFI_EVENT mAcpiRegistration = NULL;
>> >
>> > +UINT32 SwapUINT32(UINT32 value)
>> > +{
>> > +  value = ((value << 8) & 0xFF00FF00 ) | ((value >> 8) & 0xFF00FF );
>> > +  return (value << 16) | (value >> 16);
>> > +}
>> > +
>> > +/**
>> > +  The function reads MAC address from Juno IOFPGA registers and writes it
>> > +  into Marvell Yukon NIC.
>> > +**/
>> > +STATIC
>> > +EFI_STATUS
>> > +ArmJunoSetNetworkMAC()
>> > +{
>> > +
>> > +  EFI_STATUS                          Status;
>> > +  UINTN                               HandleCount;
>> > +  EFI_HANDLE                          *HandleBuffer;
>> > +  UINTN                               HIndex;
>> > +  EFI_PCI_IO_PROTOCOL*                PciIo;
>> > +  UINT64                              PciID;
>> > +  UINT32                              MacHigh;
>> > +  UINT32                              MacLow;
>> > +  UINT32                              PciRegBase;
>> > +  UINT64                              OldPciAttributes;
>> > +  UINT64                              AttrSupports;
>> > +  UINT8                               *PciBarAttributes;
>> > +
>> > +  Status = gBS->LocateHandleBuffer (ByProtocol,
>> > +                                    &gEfiPciIoProtocolGuid,
>> > +                                    NULL, &HandleCount, &HandleBuffer);
>> > +
>> > +  if (!EFI_ERROR (Status)) {
>> > +    for (HIndex = 0; HIndex < HandleCount; ++HIndex) {
>> > +      Status = gBS->OpenProtocol (
>> > +                      HandleBuffer[HIndex],
>> > +                      &gEfiPciIoProtocolGuid,
>> > +                      (VOID **) &PciIo,
>> > +                      NULL,
>> > +                      NULL,
>> > +                      EFI_OPEN_PROTOCOL_GET_PROTOCOL);
>> > +
>> > +      if (EFI_ERROR (Status)) {
>> > +        continue;
>> > +      }
>> > +
>> > +      Status = PciIo->Pci.Read (
>> > +            PciIo,
>> > +            EfiPciIoWidthUint32,
>> > +            PCI_VENDOR_ID_OFFSET,
>> > +            1,
>> > +            &PciID
>> > +            );
>> > +
>> > +      if (EFI_ERROR (Status)) {
>> > +        continue;
>> > +      }
>> > +
>> > +      if ((PciID & 0xFFFFFFFF) == JUNO_MARVELL_YUKON_ID) {
>> > +
>> > +        // Read MAC address from IOFPGA
>> > +        MacHigh= MmioRead32 (ARM_JUNO_SYS_PCIGBE_H);
>> > +        MacLow = MmioRead32 (ARM_JUNO_SYS_PCIGBE_L);
>> > +
>> > +        Status = PciIo->Attributes (
>> > +              PciIo,
>> > +              EfiPciIoAttributeOperationGet,
>> > +              0,
>> > +              &OldPciAttributes
>> > +              );
>> > +
>> > +        if (EFI_ERROR (Status)) {
>> > +          continue;
>> > +        }
>> > +
>> > +        Status = PciIo->Attributes (
>> > +              PciIo,
>> > +              EfiPciIoAttributeOperationSupported,
>> > +              0,
>> > +              &AttrSupports
>> > +              );
>> > +
>> > +        if (!EFI_ERROR (Status)) {
>> > +          AttrSupports &= EFI_PCI_DEVICE_ENABLE;
>> > +          Status = PciIo->Attributes (
>> > +                PciIo,
>> > +                EfiPciIoAttributeOperationEnable,
>> > +                AttrSupports,
>> > +                NULL
>> > +                );
>> > +
>> > +          Status = PciIo->GetBarAttributes (PciIo, 0, &AttrSupports, (VOID**)&PciBarAttributes);
>> > +          if (!EFI_ERROR (Status) && (((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)PciBarAttributes)->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR)) {
>> > +            if (((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)PciBarAttributes)->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM) {
>> > +              if (!(((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)PciBarAttributes)->SpecificFlag & ACPI_SPECFLAG_PREFETCHABLE)) {
>> > +                PciRegBase = ((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)PciBarAttributes)->AddrRangeMin;
>> > +
>> > +                // Clear Software Reset
>> > +                MmioWrite16 (PciRegBase + R_CONTROL_STATUS, CS_RESET_CLR);
>> > +
>> > +                // Convert to Marvell MAC Address register format
>> > +                MacHigh = SwapUINT32 ((MacHigh & 0xFFFF) << 16 |
>> > +                                     (MacLow & 0xFFFF0000) >> 16);
>> > +                MacLow = SwapUINT32 (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 + 4, MacLow);
>> > +                MmioWrite32 (PciRegBase + R_MAC_MAINT + 4, MacLow);
>> > +                MmioWrite8 (PciRegBase + R_TST_CTRL_1, TST_CFG_WRITE_DISABLE);
>> > +
>> > +                // Soft reset
>> > +                MmioWrite16 (PciRegBase + R_CONTROL_STATUS, CS_RESET_SET);
>> > +                MmioWrite16 (PciRegBase + R_CONTROL_STATUS, CS_RESET_CLR);
>> > +
>> > +                Status = EFI_SUCCESS;
>> > +              }
>> > +            }
>> > +          }
>> > +
>> > +          PciIo->Attributes (
>> > +                PciIo,
>> > +                EfiPciIoAttributeOperationSet,
>> > +                OldPciAttributes,
>> > +                NULL
>> > +                );
>> > +        }
>> > +      }
>> > +    }
>> > +  }
>> > +
>> > +  return Status;
>> > +}
>> > +
>> >  /**
>> >    Notification function of the event defined as belonging to the
>> >    EFI_END_OF_DXE_EVENT_GROUP_GUID event group that was created in
>> > @@ -106,6 +244,9 @@ OnEndOfDxe (
>> >
>> >    Status = gBS->ConnectController (Handle, NULL, PciRootComplexDevicePath, FALSE);
>> >    ASSERT_EFI_ERROR (Status);
>> > +
>> > +  Status = ArmJunoSetNetworkMAC();
>> > +  ASSERT_EFI_ERROR (Status);
>> >  }
>> >
>> >  STATIC
>> > diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h
>> > index 662c413..cb8fdf6 100644
>> > --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h
>> > +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h
>> > @@ -29,6 +29,18 @@
>> >
>> >  #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_TST_CTRL_1                  0x0158     /* Test Control Register 1 */
>> > +
>> > +
>> >  EFI_STATUS
>> >  PciEmulationEntryPoint (
>> >    VOID
>> > --
>> > 2.7.4
>> >
>> > _______________________________________________
>> > edk2-devel mailing list
>> > edk2-devel@lists.01.org
>> > https://lists.01.org/mailman/listinfo/edk2-devel
>>
>> This appears to be an updated version of the patch I've been using for
>> a few months now:
>>
>> https://git.linaro.org/landing-teams/working/arm/edk2.git/commit/?id=cd9a7a18eebfd2c8683a5663ce6447d3800ccf9b
>>
>> Is the intention that this one should go upstream and I drop mine from
>> my fork?  If so, I guess I should test it.
>
> That would be ideal, yes.
> If you can confirm it works as expected, I'll start nitpicking on the
> code :)
>
> Regards,
>
> Leif
>
>>
>> Cheers,
>> Ryan.


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

* Re: [PATCH] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Set Marvell Yukon MAC address
  2016-11-02 11:55     ` Ryan Harkin
@ 2016-12-08 16:23       ` Ryan Harkin
  0 siblings, 0 replies; 6+ messages in thread
From: Ryan Harkin @ 2016-12-08 16:23 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: Daniil Egranov, edk2-devel-01, alan

Hi Leif,

On 2 November 2016 at 11:55, Ryan Harkin <ryan.harkin@linaro.org> wrote:
> On 1 November 2016 at 21:05, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> On Tue, Nov 01, 2016 at 05:55:11PM +0000, Ryan Harkin wrote:
>>> Hi Daniil,
>>>
>>> While looking for another patch, I found this on the maillist...
>>>
>>> On 6 October 2016 at 02:42, Daniil Egranov <daniil.egranov@arm.com> wrote:
>>> > 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@arm.com>
>
> Tested on Juno R0/1/2.
>
> Tested-by; Ryan Harkin <ryan.harkin@linaro.org>
>
>
>>> > ---
>>> >  .../ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c     | 141 +++++++++++++++++++++
>>> >  .../Drivers/ArmJunoDxe/ArmJunoDxeInternal.h        |  12 ++
>>> >  2 files changed, 153 insertions(+)
>>> >
>>> > diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
>>> > index b97f044..0c5fbd0 100644
>>> > --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
>>> > +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
>>> > @@ -17,6 +17,8 @@
>>> >
>>> >  #include <Protocol/DevicePathFromText.h>
>>> >  #include <Protocol/PciRootBridgeIo.h>
>>> > +#include <Protocol/PciIo.h>
>>> > +#include <IndustryStandard/Pci.h>
>>> >
>>> >  #include <Guid/EventGroup.h>
>>> >  #include <Guid/GlobalVariable.h>
>>> > @@ -68,6 +70,142 @@ STATIC CONST EFI_PCI_ROOT_BRIDGE_DEVICE_PATH mPciRootComplexDevicePath = {
>>> >
>>> >  EFI_EVENT mAcpiRegistration = NULL;
>>> >
>>> > +UINT32 SwapUINT32(UINT32 value)
>>> > +{
>>> > +  value = ((value << 8) & 0xFF00FF00 ) | ((value >> 8) & 0xFF00FF );
>>> > +  return (value << 16) | (value >> 16);
>>> > +}
>>> > +
>>> > +/**
>>> > +  The function reads MAC address from Juno IOFPGA registers and writes it
>>> > +  into Marvell Yukon NIC.
>>> > +**/
>>> > +STATIC
>>> > +EFI_STATUS
>>> > +ArmJunoSetNetworkMAC()
>>> > +{
>>> > +
>>> > +  EFI_STATUS                          Status;
>>> > +  UINTN                               HandleCount;
>>> > +  EFI_HANDLE                          *HandleBuffer;
>>> > +  UINTN                               HIndex;
>>> > +  EFI_PCI_IO_PROTOCOL*                PciIo;
>>> > +  UINT64                              PciID;
>>> > +  UINT32                              MacHigh;
>>> > +  UINT32                              MacLow;
>>> > +  UINT32                              PciRegBase;
>>> > +  UINT64                              OldPciAttributes;
>>> > +  UINT64                              AttrSupports;
>>> > +  UINT8                               *PciBarAttributes;
>>> > +
>>> > +  Status = gBS->LocateHandleBuffer (ByProtocol,
>>> > +                                    &gEfiPciIoProtocolGuid,
>>> > +                                    NULL, &HandleCount, &HandleBuffer);
>>> > +
>>> > +  if (!EFI_ERROR (Status)) {
>>> > +    for (HIndex = 0; HIndex < HandleCount; ++HIndex) {
>>> > +      Status = gBS->OpenProtocol (
>>> > +                      HandleBuffer[HIndex],
>>> > +                      &gEfiPciIoProtocolGuid,
>>> > +                      (VOID **) &PciIo,
>>> > +                      NULL,
>>> > +                      NULL,
>>> > +                      EFI_OPEN_PROTOCOL_GET_PROTOCOL);
>>> > +
>>> > +      if (EFI_ERROR (Status)) {
>>> > +        continue;
>>> > +      }
>>> > +
>>> > +      Status = PciIo->Pci.Read (
>>> > +            PciIo,
>>> > +            EfiPciIoWidthUint32,
>>> > +            PCI_VENDOR_ID_OFFSET,
>>> > +            1,
>>> > +            &PciID
>>> > +            );
>>> > +
>>> > +      if (EFI_ERROR (Status)) {
>>> > +        continue;
>>> > +      }
>>> > +
>>> > +      if ((PciID & 0xFFFFFFFF) == JUNO_MARVELL_YUKON_ID) {
>>> > +
>>> > +        // Read MAC address from IOFPGA
>>> > +        MacHigh= MmioRead32 (ARM_JUNO_SYS_PCIGBE_H);
>>> > +        MacLow = MmioRead32 (ARM_JUNO_SYS_PCIGBE_L);
>>> > +
>>> > +        Status = PciIo->Attributes (
>>> > +              PciIo,
>>> > +              EfiPciIoAttributeOperationGet,
>>> > +              0,
>>> > +              &OldPciAttributes
>>> > +              );
>>> > +
>>> > +        if (EFI_ERROR (Status)) {
>>> > +          continue;
>>> > +        }
>>> > +
>>> > +        Status = PciIo->Attributes (
>>> > +              PciIo,
>>> > +              EfiPciIoAttributeOperationSupported,
>>> > +              0,
>>> > +              &AttrSupports
>>> > +              );
>>> > +
>>> > +        if (!EFI_ERROR (Status)) {
>>> > +          AttrSupports &= EFI_PCI_DEVICE_ENABLE;
>>> > +          Status = PciIo->Attributes (
>>> > +                PciIo,
>>> > +                EfiPciIoAttributeOperationEnable,
>>> > +                AttrSupports,
>>> > +                NULL
>>> > +                );
>>> > +
>>> > +          Status = PciIo->GetBarAttributes (PciIo, 0, &AttrSupports, (VOID**)&PciBarAttributes);
>>> > +          if (!EFI_ERROR (Status) && (((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)PciBarAttributes)->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR)) {
>>> > +            if (((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)PciBarAttributes)->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM) {
>>> > +              if (!(((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)PciBarAttributes)->SpecificFlag & ACPI_SPECFLAG_PREFETCHABLE)) {
>>> > +                PciRegBase = ((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)PciBarAttributes)->AddrRangeMin;
>>> > +
>>> > +                // Clear Software Reset
>>> > +                MmioWrite16 (PciRegBase + R_CONTROL_STATUS, CS_RESET_CLR);
>>> > +
>>> > +                // Convert to Marvell MAC Address register format
>>> > +                MacHigh = SwapUINT32 ((MacHigh & 0xFFFF) << 16 |
>>> > +                                     (MacLow & 0xFFFF0000) >> 16);
>>> > +                MacLow = SwapUINT32 (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 + 4, MacLow);
>>> > +                MmioWrite32 (PciRegBase + R_MAC_MAINT + 4, MacLow);
>>> > +                MmioWrite8 (PciRegBase + R_TST_CTRL_1, TST_CFG_WRITE_DISABLE);
>>> > +
>>> > +                // Soft reset
>>> > +                MmioWrite16 (PciRegBase + R_CONTROL_STATUS, CS_RESET_SET);
>>> > +                MmioWrite16 (PciRegBase + R_CONTROL_STATUS, CS_RESET_CLR);
>>> > +
>>> > +                Status = EFI_SUCCESS;
>>> > +              }
>>> > +            }
>>> > +          }
>>> > +
>>> > +          PciIo->Attributes (
>>> > +                PciIo,
>>> > +                EfiPciIoAttributeOperationSet,
>>> > +                OldPciAttributes,
>>> > +                NULL
>>> > +                );
>>> > +        }
>>> > +      }
>>> > +    }
>>> > +  }
>>> > +
>>> > +  return Status;
>>> > +}
>>> > +
>>> >  /**
>>> >    Notification function of the event defined as belonging to the
>>> >    EFI_END_OF_DXE_EVENT_GROUP_GUID event group that was created in
>>> > @@ -106,6 +244,9 @@ OnEndOfDxe (
>>> >
>>> >    Status = gBS->ConnectController (Handle, NULL, PciRootComplexDevicePath, FALSE);
>>> >    ASSERT_EFI_ERROR (Status);
>>> > +
>>> > +  Status = ArmJunoSetNetworkMAC();
>>> > +  ASSERT_EFI_ERROR (Status);
>>> >  }
>>> >
>>> >  STATIC
>>> > diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h
>>> > index 662c413..cb8fdf6 100644
>>> > --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h
>>> > +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h
>>> > @@ -29,6 +29,18 @@
>>> >
>>> >  #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_TST_CTRL_1                  0x0158     /* Test Control Register 1 */
>>> > +
>>> > +
>>> >  EFI_STATUS
>>> >  PciEmulationEntryPoint (
>>> >    VOID
>>> > --
>>> > 2.7.4
>>> >
>>> > _______________________________________________
>>> > edk2-devel mailing list
>>> > edk2-devel@lists.01.org
>>> > https://lists.01.org/mailman/listinfo/edk2-devel
>>>
>>> This appears to be an updated version of the patch I've been using for
>>> a few months now:
>>>
>>> https://git.linaro.org/landing-teams/working/arm/edk2.git/commit/?id=cd9a7a18eebfd2c8683a5663ce6447d3800ccf9b
>>>
>>> Is the intention that this one should go upstream and I drop mine from
>>> my fork?  If so, I guess I should test it.
>>
>> That would be ideal, yes.
>> If you can confirm it works as expected, I'll start nitpicking on the
>> code :)
>>

I see this one is not upstream and no nitpicking has taken place ;-)

Cheers,
Ryan.


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

* Re: [PATCH] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Set Marvell Yukon MAC address
  2016-10-06  1:42 [PATCH] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Set Marvell Yukon MAC address Daniil Egranov
  2016-11-01 17:55 ` Ryan Harkin
@ 2016-12-08 16:48 ` Leif Lindholm
  1 sibling, 0 replies; 6+ messages in thread
From: Leif Lindholm @ 2016-12-08 16:48 UTC (permalink / raw)
  To: Daniil Egranov; +Cc: edk2-devel, alan

Thanks to Ryan for reminding me :)

On Wed, Oct 05, 2016 at 08:42:51PM -0500, Daniil Egranov wrote:
> 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@arm.com>
> ---
>  .../ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c     | 141 +++++++++++++++++++++
>  .../Drivers/ArmJunoDxe/ArmJunoDxeInternal.h        |  12 ++
>  2 files changed, 153 insertions(+)
> 
> diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> index b97f044..0c5fbd0 100644
> --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> @@ -17,6 +17,8 @@
>  
>  #include <Protocol/DevicePathFromText.h>
>  #include <Protocol/PciRootBridgeIo.h>
> +#include <Protocol/PciIo.h>
> +#include <IndustryStandard/Pci.h>

Could you insert these alphabetically sorted please?

>  
>  #include <Guid/EventGroup.h>
>  #include <Guid/GlobalVariable.h>
> @@ -68,6 +70,142 @@ STATIC CONST EFI_PCI_ROOT_BRIDGE_DEVICE_PATH mPciRootComplexDevicePath = {
>  
>  EFI_EVENT mAcpiRegistration = NULL;
>  
> +UINT32 SwapUINT32(UINT32 value)

Use SwapBytes32 from BaseLib instead?

> +{
> +  value = ((value << 8) & 0xFF00FF00 ) | ((value >> 8) & 0xFF00FF );
> +  return (value << 16) | (value >> 16);
> +}
> +
> +/**
> +  The function reads MAC address from Juno IOFPGA registers and writes it
> +  into Marvell Yukon NIC.
> +**/
> +STATIC
> +EFI_STATUS
> +ArmJunoSetNetworkMAC()
> +{
> +
> +  EFI_STATUS                          Status;
> +  UINTN                               HandleCount;
> +  EFI_HANDLE                          *HandleBuffer;
> +  UINTN                               HIndex;
> +  EFI_PCI_IO_PROTOCOL*                PciIo;
> +  UINT64                              PciID;
> +  UINT32                              MacHigh;
> +  UINT32                              MacLow;
> +  UINT32                              PciRegBase;
> +  UINT64                              OldPciAttributes;
> +  UINT64                              AttrSupports;
> +  UINT8                               *PciBarAttributes;
> +
> +  Status = gBS->LocateHandleBuffer (ByProtocol,
> +                                    &gEfiPciIoProtocolGuid,
> +                                    NULL, &HandleCount, &HandleBuffer);
> +
> +  if (!EFI_ERROR (Status)) {

Better to bail out and reduce the indentation for the rest of the
function.

> +    for (HIndex = 0; HIndex < HandleCount; ++HIndex) {
> +      Status = gBS->OpenProtocol (
> +                      HandleBuffer[HIndex],
> +                      &gEfiPciIoProtocolGuid,
> +                      (VOID **) &PciIo,
> +                      NULL,
> +                      NULL,
> +                      EFI_OPEN_PROTOCOL_GET_PROTOCOL);
> +
> +      if (EFI_ERROR (Status)) {
> +        continue;
> +      }
> +
> +      Status = PciIo->Pci.Read (
> +            PciIo,

Inconcistent indentation - the OpenProtocol above looks better.

> +            EfiPciIoWidthUint32,
> +            PCI_VENDOR_ID_OFFSET,
> +            1,
> +            &PciID
> +            );
> +
> +      if (EFI_ERROR (Status)) {
> +        continue;
> +      }
> +
> +      if ((PciID & 0xFFFFFFFF) == JUNO_MARVELL_YUKON_ID) {

Invert test and continue, to reduce indentation.

The rest of this function would be nice to break out into a helper
function. (PciIo, 

> +
> +        // Read MAC address from IOFPGA
> +        MacHigh= MmioRead32 (ARM_JUNO_SYS_PCIGBE_H);
> +        MacLow = MmioRead32 (ARM_JUNO_SYS_PCIGBE_L);
> +
> +        Status = PciIo->Attributes (
> +              PciIo,

Indentation.

> +              EfiPciIoAttributeOperationGet,
> +              0,
> +              &OldPciAttributes
> +              );
> +
> +        if (EFI_ERROR (Status)) {
> +          continue;
> +        }
> +
> +        Status = PciIo->Attributes (
> +              PciIo,

Indentation.

> +              EfiPciIoAttributeOperationSupported,
> +              0,
> +              &AttrSupports
> +              );
> +
> +        if (!EFI_ERROR (Status)) {
> +          AttrSupports &= EFI_PCI_DEVICE_ENABLE;
> +          Status = PciIo->Attributes (
> +                PciIo,

Indentation.

> +                EfiPciIoAttributeOperationEnable,
> +                AttrSupports,
> +                NULL
> +                );
> +
> +          Status = PciIo->GetBarAttributes (PciIo, 0, &AttrSupports, (VOID**)&PciBarAttributes);
> +          if (!EFI_ERROR (Status) && (((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)PciBarAttributes)->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR)) {

First of all, it would be useful to have a temporary variable for
(EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)PciBarAttributes to make this
more readable.

Secondly, it would also be helpful to 

> +            if (((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)PciBarAttributes)->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM) {
> +              if (!(((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)PciBarAttributes)->SpecificFlag & ACPI_SPECFLAG_PREFETCHABLE)) {
> +                PciRegBase = ((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)PciBarAttributes)->AddrRangeMin;
> +
> +                // Clear Software Reset

Of what? Presumably MAC, but be explicit.

> +                MmioWrite16 (PciRegBase + R_CONTROL_STATUS, CS_RESET_CLR);
> +
> +                // Convert to Marvell MAC Address register format
> +                MacHigh = SwapUINT32 ((MacHigh & 0xFFFF) << 16 |
> +                                     (MacLow & 0xFFFF0000) >> 16);
> +                MacLow = SwapUINT32 (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 + 4, MacLow);
> +                MmioWrite32 (PciRegBase + R_MAC_MAINT + 4, MacLow);

What does 4 constitute in the above lines? (Is there not an R_MAC_LOW
and R_MAC_HIGH?)

> +                MmioWrite8 (PciRegBase + R_TST_CTRL_1, TST_CFG_WRITE_DISABLE);
> +
> +                // Soft reset

Of what? Presumably MAC, but be explicit.

> +                MmioWrite16 (PciRegBase + R_CONTROL_STATUS, CS_RESET_SET);
> +                MmioWrite16 (PciRegBase + R_CONTROL_STATUS, CS_RESET_CLR);
> +
> +                Status = EFI_SUCCESS;
> +              }
> +            }
> +          }
> +
> +          PciIo->Attributes (
> +                PciIo,

Indentation.

> +                EfiPciIoAttributeOperationSet,
> +                OldPciAttributes,
> +                NULL
> +                );
> +        }
> +      }
> +    }
> +  }
> +
> +  return Status;
> +}
> +
>  /**
>    Notification function of the event defined as belonging to the
>    EFI_END_OF_DXE_EVENT_GROUP_GUID event group that was created in
> @@ -106,6 +244,9 @@ OnEndOfDxe (
>  
>    Status = gBS->ConnectController (Handle, NULL, PciRootComplexDevicePath, FALSE);
>    ASSERT_EFI_ERROR (Status);
> +
> +  Status = ArmJunoSetNetworkMAC();
> +  ASSERT_EFI_ERROR (Status);
>  }
>  
>  STATIC
> diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h
> index 662c413..cb8fdf6 100644
> --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h
> +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxeInternal.h
> @@ -29,6 +29,18 @@
>  
>  #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_TST_CTRL_1                  0x0158     /* Test Control Register 1 */
> +
> +
>  EFI_STATUS
>  PciEmulationEntryPoint (
>    VOID
> -- 
> 2.7.4
> 


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

end of thread, other threads:[~2016-12-08 16:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-06  1:42 [PATCH] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Set Marvell Yukon MAC address Daniil Egranov
2016-11-01 17:55 ` Ryan Harkin
2016-11-01 21:05   ` Leif Lindholm
2016-11-02 11:55     ` Ryan Harkin
2016-12-08 16:23       ` Ryan Harkin
2016-12-08 16:48 ` Leif Lindholm

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