Re: [edk2] [PATCH] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Set Marvell Yukon MAC address

2016-12-08 Thread Leif Lindholm
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 
> ---
>  .../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 
>  #include 
> +#include 
> +#include 

Could you insert these alphabetically sorted please?

>  
>  #include 
>  #include 
> @@ -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,
> +,
> +NULL, , );
> +
> +  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],
> +  ,
> +  (VOID **) ,
> +  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,
> +
> +);
> +
> +  if (EFI_ERROR (Status)) {
> +continue;
> +  }
> +
> +  if ((PciID & 0x) == 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,
> +  
> +  );
> +
> +if (EFI_ERROR (Status)) {
> +  continue;
> +}
> +
> +Status = PciIo->Attributes (
> +  PciIo,

Indentation.

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

Indentation.

> +EfiPciIoAttributeOperationEnable,
> +AttrSupports,
> +NULL
> +);
> +
> +  Status = PciIo->GetBarAttributes (PciIo, 0, , 
> (VOID**));
> +  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 
> 

Re: [edk2] [PATCH] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Set Marvell Yukon MAC address

2016-12-08 Thread Ryan Harkin
Hi Leif,

On 2 November 2016 at 11:55, Ryan Harkin  wrote:
> On 1 November 2016 at 21:05, Leif Lindholm  wrote:
>> On Tue, Nov 01, 2016 at 05:55:11PM +, 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  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 
>
> Tested on Juno R0/1/2.
>
> Tested-by; Ryan Harkin 
>
>
>>> > ---
>>> >  .../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 
>>> >  #include 
>>> > +#include 
>>> > +#include 
>>> >
>>> >  #include 
>>> >  #include 
>>> > @@ -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,
>>> > +,
>>> > +NULL, , );
>>> > +
>>> > +  if (!EFI_ERROR (Status)) {
>>> > +for (HIndex = 0; HIndex < HandleCount; ++HIndex) {
>>> > +  Status = gBS->OpenProtocol (
>>> > +  HandleBuffer[HIndex],
>>> > +  ,
>>> > +  (VOID **) ,
>>> > +  NULL,
>>> > +  NULL,
>>> > +  EFI_OPEN_PROTOCOL_GET_PROTOCOL);
>>> > +
>>> > +  if (EFI_ERROR (Status)) {
>>> > +continue;
>>> > +  }
>>> > +
>>> > +  Status = PciIo->Pci.Read (
>>> > +PciIo,
>>> > +EfiPciIoWidthUint32,
>>> > +PCI_VENDOR_ID_OFFSET,
>>> > +1,
>>> > +
>>> > +);
>>> > +
>>> > +  if (EFI_ERROR (Status)) {
>>> > +continue;
>>> > +  }
>>> > +
>>> > +  if ((PciID & 0x) == 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,
>>> > +  
>>> > +  );
>>> > +
>>> > +if (EFI_ERROR (Status)) {
>>> > +  continue;
>>> > +}
>>> > +
>>> > +Status = PciIo->Attributes (
>>> > +  PciIo,
>>> > +  EfiPciIoAttributeOperationSupported,
>>> > +  0,
>>> > +  
>>> > +  );
>>> > +
>>> > +if (!EFI_ERROR (Status)) {
>>> > +  AttrSupports &= EFI_PCI_DEVICE_ENABLE;
>>> > +  Status = PciIo->Attributes (
>>> > +PciIo,
>>> > +EfiPciIoAttributeOperationEnable,
>>> > +AttrSupports,
>>> > +NULL
>>> > +);
>>> > +
>>> > +  Status = PciIo->GetBarAttributes (PciIo, 0, , 
>>> > (VOID**));
>>> > +  if (!EFI_ERROR (Status) && 
>>> > (((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)PciBarAttributes)->Desc == 
>>> > ACPI_ADDRESS_SPACE_DESCRIPTOR)) {
>>> > +if 

Re: [edk2] [PATCH] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Set Marvell Yukon MAC address

2016-11-02 Thread Ryan Harkin
On 1 November 2016 at 21:05, Leif Lindholm  wrote:
> On Tue, Nov 01, 2016 at 05:55:11PM +, 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  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 

Tested on Juno R0/1/2.

Tested-by; Ryan Harkin 


>> > ---
>> >  .../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 
>> >  #include 
>> > +#include 
>> > +#include 
>> >
>> >  #include 
>> >  #include 
>> > @@ -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,
>> > +,
>> > +NULL, , );
>> > +
>> > +  if (!EFI_ERROR (Status)) {
>> > +for (HIndex = 0; HIndex < HandleCount; ++HIndex) {
>> > +  Status = gBS->OpenProtocol (
>> > +  HandleBuffer[HIndex],
>> > +  ,
>> > +  (VOID **) ,
>> > +  NULL,
>> > +  NULL,
>> > +  EFI_OPEN_PROTOCOL_GET_PROTOCOL);
>> > +
>> > +  if (EFI_ERROR (Status)) {
>> > +continue;
>> > +  }
>> > +
>> > +  Status = PciIo->Pci.Read (
>> > +PciIo,
>> > +EfiPciIoWidthUint32,
>> > +PCI_VENDOR_ID_OFFSET,
>> > +1,
>> > +
>> > +);
>> > +
>> > +  if (EFI_ERROR (Status)) {
>> > +continue;
>> > +  }
>> > +
>> > +  if ((PciID & 0x) == 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,
>> > +  
>> > +  );
>> > +
>> > +if (EFI_ERROR (Status)) {
>> > +  continue;
>> > +}
>> > +
>> > +Status = PciIo->Attributes (
>> > +  PciIo,
>> > +  EfiPciIoAttributeOperationSupported,
>> > +  0,
>> > +  
>> > +  );
>> > +
>> > +if (!EFI_ERROR (Status)) {
>> > +  AttrSupports &= EFI_PCI_DEVICE_ENABLE;
>> > +  Status = PciIo->Attributes (
>> > +PciIo,
>> > +EfiPciIoAttributeOperationEnable,
>> > +AttrSupports,
>> > +NULL
>> > +);
>> > +
>> > +  Status = PciIo->GetBarAttributes (PciIo, 0, , 
>> > (VOID**));
>> > +  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)) {
>> > +

Re: [edk2] [PATCH] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Set Marvell Yukon MAC address

2016-11-01 Thread Leif Lindholm
On Tue, Nov 01, 2016 at 05:55:11PM +, 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  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 
> > ---
> >  .../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 
> >  #include 
> > +#include 
> > +#include 
> >
> >  #include 
> >  #include 
> > @@ -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,
> > +,
> > +NULL, , );
> > +
> > +  if (!EFI_ERROR (Status)) {
> > +for (HIndex = 0; HIndex < HandleCount; ++HIndex) {
> > +  Status = gBS->OpenProtocol (
> > +  HandleBuffer[HIndex],
> > +  ,
> > +  (VOID **) ,
> > +  NULL,
> > +  NULL,
> > +  EFI_OPEN_PROTOCOL_GET_PROTOCOL);
> > +
> > +  if (EFI_ERROR (Status)) {
> > +continue;
> > +  }
> > +
> > +  Status = PciIo->Pci.Read (
> > +PciIo,
> > +EfiPciIoWidthUint32,
> > +PCI_VENDOR_ID_OFFSET,
> > +1,
> > +
> > +);
> > +
> > +  if (EFI_ERROR (Status)) {
> > +continue;
> > +  }
> > +
> > +  if ((PciID & 0x) == 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,
> > +  
> > +  );
> > +
> > +if (EFI_ERROR (Status)) {
> > +  continue;
> > +}
> > +
> > +Status = PciIo->Attributes (
> > +  PciIo,
> > +  EfiPciIoAttributeOperationSupported,
> > +  0,
> > +  
> > +  );
> > +
> > +if (!EFI_ERROR (Status)) {
> > +  AttrSupports &= EFI_PCI_DEVICE_ENABLE;
> > +  Status = PciIo->Attributes (
> > +PciIo,
> > +EfiPciIoAttributeOperationEnable,
> > +AttrSupports,
> > +NULL
> > +);
> > +
> > +  Status = PciIo->GetBarAttributes (PciIo, 0, , 
> > (VOID**));
> > +  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 

Re: [edk2] [PATCH] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Set Marvell Yukon MAC address

2016-11-01 Thread Ryan Harkin
Hi Daniil,

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

On 6 October 2016 at 02:42, 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 
> ---
>  .../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 
>  #include 
> +#include 
> +#include 
>
>  #include 
>  #include 
> @@ -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,
> +,
> +NULL, , );
> +
> +  if (!EFI_ERROR (Status)) {
> +for (HIndex = 0; HIndex < HandleCount; ++HIndex) {
> +  Status = gBS->OpenProtocol (
> +  HandleBuffer[HIndex],
> +  ,
> +  (VOID **) ,
> +  NULL,
> +  NULL,
> +  EFI_OPEN_PROTOCOL_GET_PROTOCOL);
> +
> +  if (EFI_ERROR (Status)) {
> +continue;
> +  }
> +
> +  Status = PciIo->Pci.Read (
> +PciIo,
> +EfiPciIoWidthUint32,
> +PCI_VENDOR_ID_OFFSET,
> +1,
> +
> +);
> +
> +  if (EFI_ERROR (Status)) {
> +continue;
> +  }
> +
> +  if ((PciID & 0x) == 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,
> +  
> +  );
> +
> +if (EFI_ERROR (Status)) {
> +  continue;
> +}
> +
> +Status = PciIo->Attributes (
> +  PciIo,
> +  EfiPciIoAttributeOperationSupported,
> +  0,
> +  
> +  );
> +
> +if (!EFI_ERROR (Status)) {
> +  AttrSupports &= EFI_PCI_DEVICE_ENABLE;
> +  Status = PciIo->Attributes (
> +PciIo,
> +EfiPciIoAttributeOperationEnable,
> +AttrSupports,
> +NULL
> +);
> +
> +  Status = PciIo->GetBarAttributes (PciIo, 0, , 
> (VOID**));
> +  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 & 0x) << 16 |
> + (MacLow & 0x) >> 16);
> +MacLow = SwapUINT32 (MacLow) >> 16;
> +
> +// Set MAC Address
> +MmioWrite8 (PciRegBase + R_TST_CTRL_1, TST_CFG_WRITE_ENABLE);
> +

[edk2] [PATCH] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Set Marvell Yukon MAC address

2016-10-05 Thread 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 
---
 .../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 
 #include 
+#include 
+#include 
 
 #include 
 #include 
@@ -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,
+,
+NULL, , );
+
+  if (!EFI_ERROR (Status)) {
+for (HIndex = 0; HIndex < HandleCount; ++HIndex) {
+  Status = gBS->OpenProtocol (
+  HandleBuffer[HIndex],
+  ,
+  (VOID **) ,
+  NULL,
+  NULL,
+  EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+
+  if (EFI_ERROR (Status)) {
+continue;
+  }
+
+  Status = PciIo->Pci.Read (
+PciIo,
+EfiPciIoWidthUint32,
+PCI_VENDOR_ID_OFFSET,
+1,
+
+);
+
+  if (EFI_ERROR (Status)) {
+continue;
+  }
+
+  if ((PciID & 0x) == 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,
+  
+  );
+
+if (EFI_ERROR (Status)) {
+  continue;
+}
+
+Status = PciIo->Attributes (
+  PciIo,
+  EfiPciIoAttributeOperationSupported,
+  0,
+  
+  );
+
+if (!EFI_ERROR (Status)) {
+  AttrSupports &= EFI_PCI_DEVICE_ENABLE;
+  Status = PciIo->Attributes (
+PciIo,
+EfiPciIoAttributeOperationEnable,
+AttrSupports,
+NULL
+);
+
+  Status = PciIo->GetBarAttributes (PciIo, 0, , 
(VOID**));
+  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 & 0x) << 16 |
+ (MacLow & 0x) >> 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);
+