* [edk2-platforms][PATCH 0/3] Platform/RPi4: Add Genet network driver stub
@ 2020-01-23 12:00 Pete Batard
2020-01-23 12:00 ` [edk2-platforms][PATCH 1/3] Silicon/Broadcom/Net: Add Genet stub driver to setup MAC Pete Batard
` (6 more replies)
0 siblings, 7 replies; 12+ messages in thread
From: Pete Batard @ 2020-01-23 12:00 UTC (permalink / raw)
To: devel; +Cc: ard.biesheuvel, leif, philmd, lintonrjeremy
The Raspberry Pi 4 Broadcom Genet network adapter can be made to work in ACPI
mode under high level OSes such as Linux.
To facilitate this however, it is desirable to ensure that the platform's MAC
address has properly been written to the dedicated UMAC registers during UEFI
initialization.
This series of patches achieves that by:
* Adding a generic Genet driver stub under Silicon that, for the time being,
simply performs UMAC init when a MAC Address PCD has been set.
* Adding a new PlatformPcdLib in the Raspberry Pi try, to ensure that, if
no MAC address PCD was provided for the build, we set that PCD by querying
the firmware interface.
* Enabling the Genet driver for the Pi 4 platform
Jeremy Linton (2):
Silicon/Broadcom/Net: Add Genet stub driver to setup MAC
Platform/Rpi4: Enable Broadcom Genet stub driver
Pete Batard (1):
Platform/RPi: Add PlatformPcdLib to set the Genet MAC address
Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.c | 61 +++++++++++
Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.inf | 44 ++++++++
Platform/RaspberryPi/RPi4/RPi4.dsc | 5 +
Platform/RaspberryPi/RPi4/RPi4.fdf | 1 +
Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf | 40 +++++++
Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.c | 114 ++++++++++++++++++++
Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.h | 20 ++++
Silicon/Broadcom/Drivers/Net/BcmNet.dec | 22 ++++
8 files changed, 307 insertions(+)
create mode 100644 Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.c
create mode 100644 Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.inf
create mode 100644 Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
create mode 100644 Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.c
create mode 100644 Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.h
create mode 100644 Silicon/Broadcom/Drivers/Net/BcmNet.dec
--
2.21.0.windows.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [edk2-platforms][PATCH 1/3] Silicon/Broadcom/Net: Add Genet stub driver to setup MAC
2020-01-23 12:00 [edk2-platforms][PATCH 0/3] Platform/RPi4: Add Genet network driver stub Pete Batard
@ 2020-01-23 12:00 ` Pete Batard
2020-01-23 13:44 ` Ard Biesheuvel
2020-01-23 12:00 ` [edk2-platforms][PATCH 2/3] Platform/RPi: Add PlatformPcdLib to set the Genet MAC address Pete Batard
` (5 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Pete Batard @ 2020-01-23 12:00 UTC (permalink / raw)
To: devel; +Cc: ard.biesheuvel, leif, philmd, lintonrjeremy
From: Jeremy Linton <lintonrjeremy@gmail.com>
Add a stub for the Broadcom Genet network interface, that is used
by the Raspberry Pi 4 (and that may be used by other platforms).
For the time being, the stub only performs UMAC init, using a MAC
address PCD, as, even without a working network interface in UEFI
environment, it is desirable if the hardware can describe itself
for booting in an ACPI environment, rather than pass parameters via
DSDT/DSD .
Signed-off-by: Pete Batard <pete@akeo.ie>
---
Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf | 40 +++++++
Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.c | 114 ++++++++++++++++++++
Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.h | 20 ++++
Silicon/Broadcom/Drivers/Net/BcmNet.dec | 22 ++++
4 files changed, 196 insertions(+)
diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
new file mode 100644
index 000000000000..9e9301608f24
--- /dev/null
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
@@ -0,0 +1,40 @@
+## @file
+#
+# Copyright (c) 2020, Jeremy Linton All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x0001001A
+ BASE_NAME = BcmGenetDxe
+ FILE_GUID = e2b1eaf3-50b7-4ae1-b79e-ec8020cb57ac
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 0.1
+ ENTRY_POINT = GenetEntryPoint
+
+[Sources]
+ Genet.c
+
+[Packages]
+ ArmPkg/ArmPkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+ Silicon/Broadcom/Drivers/Net/BcmNet.dec
+
+[LibraryClasses]
+ ArmLib
+ BaseLib
+ IoLib
+ UefiDriverEntryPoint
+ UefiLib
+
+[FixedPcd]
+ gBcmNetTokenSpaceGuid.PcdBcmGenetRegistersAddress
+
+[Pcd]
+ gBcmNetTokenSpaceGuid.PcdBcmGenetMacAddress
+
+[Depex]
+ TRUE
diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.c b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.c
new file mode 100644
index 000000000000..d135d889cc1f
--- /dev/null
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.c
@@ -0,0 +1,114 @@
+/** @file
+
+ Copyright (c) 2020, Jeremy Linton All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ This driver acts like a stub to set the Broadcom
+ Genet MAC address, until the actual network driver
+ is in place.
+
+**/
+
+#include <Library/ArmLib.h>
+#include <Library/DebugLib.h>
+#include <Library/IoLib.h>
+#include <Library/PcdLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiLib.h>
+
+#include <Genet.h>
+#include <PiDxe.h>
+
+STATIC
+VOID
+RMWRegister (
+ UINT32 Offset,
+ UINT32 Mask,
+ UINT32 In
+ )
+{
+ EFI_PHYSICAL_ADDRESS Addr;
+ UINT32 Data;
+ UINT32 Shift;
+
+ Addr = GENET_BASE_ADDRESS + Offset;
+ Data = 0;
+ Shift = 1;
+ if (In) {
+ while (!(Mask & Shift))
+ Shift <<= 1;
+ Data = (MmioRead32 (Addr) & ~Mask) | ((In * Shift) & Mask);
+ } else {
+ Data = MmioRead32 (Addr) & ~Mask;
+ }
+
+ MmioWrite32 (Addr, Data);
+
+ ArmDataMemoryBarrier ();
+}
+
+STATIC
+VOID
+WdRegister (
+ UINT32 Offset,
+ UINT32 In
+ )
+{
+ EFI_PHYSICAL_ADDRESS Base = GENET_BASE_ADDRESS;
+
+ MmioWrite32 (Base + Offset, In);
+
+ ArmDataMemoryBarrier ();
+}
+
+STATIC
+VOID
+SetMacAddress (
+ UINT8* MacAddr
+)
+{
+ // Bring the UMAC out of reset
+ RMWRegister (GENET_SYS_RBUF_FLUSH_CTRL, 0x2, 1);
+ gBS->Stall (10);
+ RMWRegister (GENET_SYS_RBUF_FLUSH_CTRL, 0x2, 0);
+
+ // Update the MAC
+ DEBUG ((DEBUG_INFO, "Setting MAC address %02X:%02X:%02X:%02X:%02X:%02X\n",
+ MacAddr[0], MacAddr[1], MacAddr[2], MacAddr[3], MacAddr[4], MacAddr[5]));
+
+ WdRegister (GENET_UMAC_MAC0, (MacAddr[0] << 24) | (MacAddr[1] << 16) |
+ (MacAddr[2] << 8) | MacAddr[3]);
+ WdRegister (GENET_UMAC_MAC1, (MacAddr[4] << 8) | MacAddr[5]);
+
+}
+
+/**
+ The entry point of Genet UEFI Driver.
+
+ @param ImageHandle The image handle of the UEFI Driver.
+ @param SystemTable A pointer to the EFI System Table.
+
+ @retval EFI_SUCCESS The Driver or UEFI Driver exited normally.
+ @retval EFI_INCOMPATIBLE_VERSION _gUefiDriverRevision is greater than
+ SystemTable->Hdr.Revision.
+
+**/
+EFI_STATUS
+EFIAPI
+GenetEntryPoint (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ UINT64 MacAddr;
+
+ // Read the MAC address
+ MacAddr = PcdGet64 (PcdBcmGenetMacAddress);
+
+ if (MacAddr != 0) {
+ SetMacAddress ((UINT8*)&MacAddr);
+ }
+
+ return EFI_SUCCESS;
+}
diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.h b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.h
new file mode 100644
index 000000000000..4a3827c0e0d1
--- /dev/null
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.h
@@ -0,0 +1,20 @@
+/** @file
+
+ Copyright (c) 2020, Pete Batard <pete@akeo.ie>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef BCM_GENET_H__
+#define BCM_GENET_H__
+
+#include <Library/PcdLib.h>
+
+#define GENET_BASE_ADDRESS (FixedPcdGet64 (PcdBcmGenetRegistersAddress))
+
+#define GENET_SYS_RBUF_FLUSH_CTRL 0x0008
+#define GENET_UMAC_MAC0 0x080c
+#define GENET_UMAC_MAC1 0x0810
+
+#endif /* BCM_GENET_H__ */
diff --git a/Silicon/Broadcom/Drivers/Net/BcmNet.dec b/Silicon/Broadcom/Drivers/Net/BcmNet.dec
new file mode 100644
index 000000000000..d80aac27ed9e
--- /dev/null
+++ b/Silicon/Broadcom/Drivers/Net/BcmNet.dec
@@ -0,0 +1,22 @@
+## @file
+#
+# Copyright (c) 2020, Pete Batard <pete@akeo.ie>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ DEC_SPECIFICATION = 0x0001001A
+ PACKAGE_NAME = BcmNetPkg
+ PACKAGE_GUID = 34E19823-D23A-41AB-9C09-ED1225B32DFF
+ PACKAGE_VERSION = 1.0
+
+[Guids]
+ gBcmNetTokenSpaceGuid = {0x12b97d70, 0x9149, 0x4c2f, {0x82, 0xd5, 0xad, 0xa9, 0x1e, 0x92, 0x75, 0xa1}}
+
+[PcdsFixedAtBuild]
+ gBcmNetTokenSpaceGuid.PcdBcmGenetRegistersAddress|0x0|UINT32|0x00000001
+
+[PcdsDynamic]
+ gBcmNetTokenSpaceGuid.PcdBcmGenetMacAddress|0x0|UINT64|0x00000002
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [edk2-platforms][PATCH 2/3] Platform/RPi: Add PlatformPcdLib to set the Genet MAC address
2020-01-23 12:00 [edk2-platforms][PATCH 0/3] Platform/RPi4: Add Genet network driver stub Pete Batard
2020-01-23 12:00 ` [edk2-platforms][PATCH 1/3] Silicon/Broadcom/Net: Add Genet stub driver to setup MAC Pete Batard
@ 2020-01-23 12:00 ` Pete Batard
2020-01-23 13:50 ` Ard Biesheuvel
2020-01-23 12:00 ` [edk2-platforms][PATCH 3/3] Platform/Rpi4: Enable Broadcom Genet stub driver Pete Batard
` (4 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Pete Batard @ 2020-01-23 12:00 UTC (permalink / raw)
To: devel; +Cc: ard.biesheuvel, leif, philmd, lintonrjeremy
The Genet driver stub used by the Raspberry Pi 4 platform is
designed to set the MAC address according to a PCD.
To be able to set that PCD at runtime, by using the Raspberry
Pi firmware interface, that has a dedicated call to retrieve
the MAC address, and satisfy driver dependencies in a generic
manner, we create a new PlatformPcdLib that can be referenced
by the Genet driver, to set the MAC PCD before use there.
While it is currently only tailored around MAC PCD population
for Genet, we do expect this PCD library to be extended in the
future, to provide additional PCD facilities for other drivers.
Signed-off-by: Pete Batard <pete@akeo.ie>
---
Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.c | 61 ++++++++++++++++++++
Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.inf | 44 ++++++++++++++
2 files changed, 105 insertions(+)
diff --git a/Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.c b/Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.c
new file mode 100644
index 000000000000..78f3679e2e48
--- /dev/null
+++ b/Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.c
@@ -0,0 +1,61 @@
+/** @file
+ *
+ * Copyright (c) 2020, Pete Batard <pete@akeo.ie>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause-Patent
+ *
+ **/
+
+#include <Library/DebugLib.h>
+#include <Library/PcdLib.h>
+#include <Library/PrintLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiLib.h>
+#include <Library/UefiRuntimeServicesTableLib.h>
+#include <Protocol/RpiFirmware.h>
+
+EFI_STATUS
+EFIAPI
+PlatformPcdLibConstructor (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+
+ //
+ // If Genet is in use and a MAC address PCD has not been set, do it here.
+ //
+#if (FixedPcdGet64 (PcdBcmGenetRegistersAddress) != 0)
+ EFI_STATUS Status;
+ UINT64 MacAddr;
+ RASPBERRY_PI_FIRMWARE_PROTOCOL *mFwProtocol;
+
+ if (PcdGet64 (PcdBcmGenetMacAddress) == 0) {
+ Status = gBS->LocateProtocol (&gRaspberryPiFirmwareProtocolGuid, NULL,
+ (VOID**)&mFwProtocol);
+ ASSERT_EFI_ERROR(Status);
+
+ //
+ // Get the MAC address from the firmware
+ //
+ Status = mFwProtocol->GetMacAddress ((UINT8*) &MacAddr);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_WARN, "%a: failed to retrieve MAC address\n", __FUNCTION__));
+ } else {
+ PcdSet64 (PcdBcmGenetMacAddress, MacAddr);
+ }
+ }
+#endif
+
+ return EFI_SUCCESS;
+}
+
+EFI_STATUS
+EFIAPI
+PlatformPcdLibDestructor (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ return EFI_SUCCESS;
+}
diff --git a/Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.inf b/Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.inf
new file mode 100644
index 000000000000..a564ddfbb2a3
--- /dev/null
+++ b/Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.inf
@@ -0,0 +1,44 @@
+#/** @file
+#
+# Copyright (c) 2020, Pete Batard <pete@akeo.ie>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#**/
+
+[Defines]
+ INF_VERSION = 0x0001001A
+ BASE_NAME = PlatformPcdLib
+ FILE_GUID = 3B8409D7-D3C7-4006-823B-BFB184435363
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = NULL|DXE_DRIVER UEFI_APPLICATION
+ CONSTRUCTOR = PlatformPcdLibConstructor
+ DESTRUCTOR = PlatformPcdLibDestructor
+
+[Sources]
+ PlatformPcdLib.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ Platform/RaspberryPi/RaspberryPi.dec
+ Silicon/Broadcom/Drivers/Net/BcmNet.dec
+
+[LibraryClasses]
+ UefiLib
+ DebugLib
+ PrintLib
+
+[Protocols]
+ gRaspberryPiFirmwareProtocolGuid ## CONSUMES
+
+[Pcd]
+ gBcmNetTokenSpaceGuid.PcdBcmGenetMacAddress ## SOMETIMES_PRODUCES
+
+[FixedPcd]
+ gBcmNetTokenSpaceGuid.PcdBcmGenetRegistersAddress
+
+[Depex]
+ gPcdProtocolGuid AND gRaspberryPiFirmwareProtocolGuid
+
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [edk2-platforms][PATCH 3/3] Platform/Rpi4: Enable Broadcom Genet stub driver
2020-01-23 12:00 [edk2-platforms][PATCH 0/3] Platform/RPi4: Add Genet network driver stub Pete Batard
2020-01-23 12:00 ` [edk2-platforms][PATCH 1/3] Silicon/Broadcom/Net: Add Genet stub driver to setup MAC Pete Batard
2020-01-23 12:00 ` [edk2-platforms][PATCH 2/3] Platform/RPi: Add PlatformPcdLib to set the Genet MAC address Pete Batard
@ 2020-01-23 12:00 ` Pete Batard
2020-01-23 12:00 ` [PATCH] Platform/RPi/Genet: Add Genet stub driver to setup MAC Pete Batard
` (3 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Pete Batard @ 2020-01-23 12:00 UTC (permalink / raw)
To: devel; +Cc: ard.biesheuvel, leif, philmd, lintonrjeremy
From: Jeremy Linton <lintonrjeremy@gmail.com>
This adds the required references to use the Genet stub driver in
order to dynamically populate the MAC address for OS consumption.
Signed-off-by: Pete Batard <pete@akeo.ie>
---
Platform/RaspberryPi/RPi4/RPi4.dsc | 5 +++++
Platform/RaspberryPi/RPi4/RPi4.fdf | 1 +
2 files changed, 6 insertions(+)
diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
index b3a114b6e0ed..bd3800c1d653 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -402,6 +402,7 @@ [PcdsFixedAtBuild.common]
gRaspberryPiTokenSpaceGuid.PcdExtendedMemoryBase|0x40000000
gBcm27xxTokenSpaceGuid.PcdBcm27xxRegistersAddress|0xfc000000
gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress|0xfe000000
+ gBcmNetTokenSpaceGuid.PcdBcmGenetRegistersAddress|0xfd580000
# PCIe specific addresses
gBcm27xxTokenSpaceGuid.PcdBcm27xxPciRegBase|0xfd500000
@@ -648,6 +649,10 @@ [Components.common]
# Networking stack
#
!include NetworkPkg/Network.dsc.inc
+ Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf {
+ <LibraryClasses>
+ NULL|Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.inf
+ }
#
# RNG
diff --git a/Platform/RaspberryPi/RPi4/RPi4.fdf b/Platform/RaspberryPi/RPi4/RPi4.fdf
index 2bcfdb3244f6..db393d47bcea 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.fdf
+++ b/Platform/RaspberryPi/RPi4/RPi4.fdf
@@ -269,6 +269,7 @@ [FV.FvMain]
# Networking stack
#
!include NetworkPkg/Network.fdf.inc
+ INF Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
#
# RNG
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH] Platform/RPi/Genet: Add Genet stub driver to setup MAC
2020-01-23 12:00 [edk2-platforms][PATCH 0/3] Platform/RPi4: Add Genet network driver stub Pete Batard
` (2 preceding siblings ...)
2020-01-23 12:00 ` [edk2-platforms][PATCH 3/3] Platform/Rpi4: Enable Broadcom Genet stub driver Pete Batard
@ 2020-01-23 12:00 ` Pete Batard
2020-01-23 12:00 ` [PATCH] Platform/RPi4: Enable BCM GENET stub driver Pete Batard
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Pete Batard @ 2020-01-23 12:00 UTC (permalink / raw)
To: devel; +Cc: ard.biesheuvel, leif, philmd, lintonrjeremy
From: Jeremy Linton <lintonrjeremy@gmail.com>
The RPi4 has a 1G BCM Genet platform device. In order to
assist in booting the platform in an ACPI environment, it is
desirable if the hardware can describe itself rather than
passing parameters via DSDT/DSD. One way to achieve this
is to assure that the adapter is preprogrammed with the correct
ethernet mac address as it would be if the adapter were
used during the boot process.
While we currently lack a fully functional uefi driver
for the genet, it is fairly trivial to assure that the
mac address is set. That is what this driver does.
Signed-off-by: Jeremy Linton <lintonrjeremy@gmail.com>
---
Platform/RaspberryPi/Drivers/Genet/Genet.c | 132 +++++++++++++++++++
Platform/RaspberryPi/Drivers/Genet/Genet.inf | 39 ++++++
2 files changed, 171 insertions(+)
create mode 100644 Platform/RaspberryPi/Drivers/Genet/Genet.c
create mode 100644 Platform/RaspberryPi/Drivers/Genet/Genet.inf
diff --git a/Platform/RaspberryPi/Drivers/Genet/Genet.c b/Platform/RaspberryPi/Drivers/Genet/Genet.c
new file mode 100644
index 0000000000..c4180f1d9d
--- /dev/null
+++ b/Platform/RaspberryPi/Drivers/Genet/Genet.c
@@ -0,0 +1,132 @@
+/** @file
+
+ Copyright (c) 2019, Jeremy Linton All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ This driver acts like a stub to set the BCM
+ Genet MAC address, until the actual network driver
+ is in place. This should allow us to retrieve the
+ mac address directly from the hardware in supported
+ OS's rather than passing it via DSDT (which isn't
+ ideal for a number of reasons, foremost the hardware
+ should be self describing).
+
+**/
+
+#include <Library/ArmLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DevicePathLib.h>
+#include <Library/IoLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiLib.h>
+
+#include <PiDxe.h>
+#include <Protocol/RpiFirmware.h>
+
+#define GENET_BASE 0xfd580000 // len = 0x10000
+#define GENET_SYS_RBUF_FLUSH_CTRL 0x0008
+#define GENET_UMAC_MAC0 0x080C
+#define GENET_UMAC_MAC1 0x0810
+
+STATIC
+VOID
+RMWRegister (
+ UINT32 Offset,
+ UINT32 Mask,
+ UINT32 In
+ )
+{
+ EFI_PHYSICAL_ADDRESS Addr = GENET_BASE;
+ UINT32 Data = 0;
+ UINT32 Shift;
+
+ Addr += Offset;
+ Shift = 1;
+ if (In) {
+ while (!(Mask & Shift))
+ Shift <<= 1;
+ Data = (MmioRead32 (Addr) & ~Mask) | ((In * Shift) & Mask);
+ } else {
+ Data = MmioRead32 (Addr) & ~Mask;
+ }
+
+ MmioWrite32 (Addr, Data);
+
+ ArmDataMemoryBarrier ();
+}
+
+STATIC
+VOID
+WdRegister (
+ UINT32 Offset,
+ UINT32 In
+ )
+{
+ EFI_PHYSICAL_ADDRESS Base = GENET_BASE;
+
+ MmioWrite32 (Base + Offset, In);
+
+ ArmDataMemoryBarrier ();
+}
+
+
+
+STATIC VOID
+GenetMacInit (UINT8 *addr)
+{
+
+ // bring the umac out of reset
+ RMWRegister (GENET_SYS_RBUF_FLUSH_CTRL, 0x2, 1);
+ gBS->Stall (10);
+ RMWRegister (GENET_SYS_RBUF_FLUSH_CTRL, 0x2, 0);
+
+ // Update the MAC
+
+ WdRegister(GENET_UMAC_MAC0, (addr[0] << 24) | (addr[1] << 16) | (addr[2] << 8) | addr[3]);
+ WdRegister(GENET_UMAC_MAC1, (addr[4] << 8) | addr[5]);
+}
+
+/**
+ The entry point of Genet UEFI Driver.
+
+ @param ImageHandle The image handle of the UEFI Driver.
+ @param SystemTable A pointer to the EFI System Table.
+
+ @retval EFI_SUCCESS The Driver or UEFI Driver exited normally.
+ @retval EFI_INCOMPATIBLE_VERSION _gUefiDriverRevision is greater than
+ SystemTable->Hdr.Revision.
+
+**/
+EFI_STATUS
+EFIAPI
+GenetEntryPoint (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ RASPBERRY_PI_FIRMWARE_PROTOCOL *mFwProtocol;
+ EFI_STATUS Status;
+ UINT8 MacAddr[6];
+
+ DEBUG ((DEBUG_ERROR, "GENET:%a(): Init\n", __FUNCTION__));
+
+ Status = gBS->LocateProtocol (&gRaspberryPiFirmwareProtocolGuid, NULL,
+ (VOID**)&mFwProtocol);
+ if (EFI_ERROR(Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: failed to locate RPi firmware protocol\n", __FUNCTION__));
+ return Status;
+ }
+
+ // Get the MAC address from the firmware
+ Status = mFwProtocol->GetMacAddress (MacAddr);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: failed to retrieve MAC address\n", __FUNCTION__));
+ return Status;
+ }
+
+ // Write it to the hardware
+ GenetMacInit (MacAddr);
+
+ return EFI_SUCCESS;
+}
diff --git a/Platform/RaspberryPi/Drivers/Genet/Genet.inf b/Platform/RaspberryPi/Drivers/Genet/Genet.inf
new file mode 100644
index 0000000000..3599e5a6e5
--- /dev/null
+++ b/Platform/RaspberryPi/Drivers/Genet/Genet.inf
@@ -0,0 +1,39 @@
+## @file
+#
+# Copyright (c) 2020, Jeremy Linton All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x0001001A
+ BASE_NAME = Genet
+ FILE_GUID = e2b1eaf3-50b7-4ae1-b79e-ec8020cb57ac
+ MODULE_TYPE = UEFI_DRIVER
+ VERSION_STRING = 0.1
+ ENTRY_POINT = GenetEntryPoint
+
+[Sources]
+ Genet.c
+
+[Packages]
+ ArmPkg/ArmPkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+ Platform/RaspberryPi/RaspberryPi.dec
+
+[LibraryClasses]
+ BaseLib
+ IoLib
+ SynchronizationLib
+ UefiDriverEntryPoint
+ UefiLib
+
+[Protocols]
+ gRaspberryPiFirmwareProtocolGuid ## CONSUMES
+
+
+[Depex]
+ gRaspberryPiFirmwareProtocolGuid
+
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH] Platform/RPi4: Enable BCM GENET stub driver
2020-01-23 12:00 [edk2-platforms][PATCH 0/3] Platform/RPi4: Add Genet network driver stub Pete Batard
` (3 preceding siblings ...)
2020-01-23 12:00 ` [PATCH] Platform/RPi/Genet: Add Genet stub driver to setup MAC Pete Batard
@ 2020-01-23 12:00 ` Pete Batard
[not found] ` <15EC824B064B2D10.12514@groups.io>
[not found] ` <15EC824ADEB49EC2.12514@groups.io>
6 siblings, 0 replies; 12+ messages in thread
From: Pete Batard @ 2020-01-23 12:00 UTC (permalink / raw)
To: devel; +Cc: ard.biesheuvel, leif, philmd, lintonrjeremy
From: Jeremy Linton <lintonrjeremy@gmail.com>
We have a stub driver, which retrieves the rpi's
MAC address via the mailbox interface and programs the
NIC. Lets enable it.
Signed-off-by: Jeremy Linton <lintonrjeremy@gmail.com>
---
Platform/RaspberryPi/RPi4/RPi4.dsc | 1 +
Platform/RaspberryPi/RPi4/RPi4.fdf | 1 +
2 files changed, 2 insertions(+)
diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
index 9aead4de32..97972382d5 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -660,6 +660,7 @@ DEFINE TFA_BUILD_ARTIFACT = Platform/RaspberryPi/$(PLATFORM_NAME)/TrustedFirmwar
# Networking stack
#
!include NetworkPkg/Network.dsc.inc
+ Platform/RaspberryPi/Drivers/Genet/Genet.inf
#
# RNG
diff --git a/Platform/RaspberryPi/RPi4/RPi4.fdf b/Platform/RaspberryPi/RPi4/RPi4.fdf
index e00a45b8a4..c44ec839c2 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.fdf
+++ b/Platform/RaspberryPi/RPi4/RPi4.fdf
@@ -265,6 +265,7 @@ READ_LOCK_STATUS = TRUE
# Networking stack
#
!include NetworkPkg/Network.fdf.inc
+ INF Platform/RaspberryPi/Drivers/Genet/Genet.inf
#
# RNG
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH] Platform/RPi/Genet: Add Genet stub driver to setup MAC
[not found] ` <15EC824B064B2D10.12514@groups.io>
@ 2020-01-23 12:03 ` Pete Batard
0 siblings, 0 replies; 12+ messages in thread
From: Pete Batard @ 2020-01-23 12:03 UTC (permalink / raw)
To: devel; +Cc: ard.biesheuvel, leif, philmd, lintonrjeremy
**PLEASE DISREGARD THIS PATCH**
I had a couple of extra .patch files in my repo that were inadvertently
sent with the previous series.
/Pete
On 2020.01.23 12:00, Pete Batard via Groups.Io wrote:
> From: Jeremy Linton <lintonrjeremy@gmail.com>
>
> The RPi4 has a 1G BCM Genet platform device. In order to
> assist in booting the platform in an ACPI environment, it is
> desirable if the hardware can describe itself rather than
> passing parameters via DSDT/DSD. One way to achieve this
> is to assure that the adapter is preprogrammed with the correct
> ethernet mac address as it would be if the adapter were
> used during the boot process.
>
> While we currently lack a fully functional uefi driver
> for the genet, it is fairly trivial to assure that the
> mac address is set. That is what this driver does.
>
> Signed-off-by: Jeremy Linton <lintonrjeremy@gmail.com>
> ---
> Platform/RaspberryPi/Drivers/Genet/Genet.c | 132 +++++++++++++++++++
> Platform/RaspberryPi/Drivers/Genet/Genet.inf | 39 ++++++
> 2 files changed, 171 insertions(+)
> create mode 100644 Platform/RaspberryPi/Drivers/Genet/Genet.c
> create mode 100644 Platform/RaspberryPi/Drivers/Genet/Genet.inf
>
> diff --git a/Platform/RaspberryPi/Drivers/Genet/Genet.c b/Platform/RaspberryPi/Drivers/Genet/Genet.c
> new file mode 100644
> index 0000000000..c4180f1d9d
> --- /dev/null
> +++ b/Platform/RaspberryPi/Drivers/Genet/Genet.c
> @@ -0,0 +1,132 @@
> +/** @file
> +
> + Copyright (c) 2019, Jeremy Linton All rights reserved.<BR>
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> + This driver acts like a stub to set the BCM
> + Genet MAC address, until the actual network driver
> + is in place. This should allow us to retrieve the
> + mac address directly from the hardware in supported
> + OS's rather than passing it via DSDT (which isn't
> + ideal for a number of reasons, foremost the hardware
> + should be self describing).
> +
> +**/
> +
> +#include <Library/ArmLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DevicePathLib.h>
> +#include <Library/IoLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
> +
> +#include <PiDxe.h>
> +#include <Protocol/RpiFirmware.h>
> +
> +#define GENET_BASE 0xfd580000 // len = 0x10000
> +#define GENET_SYS_RBUF_FLUSH_CTRL 0x0008
> +#define GENET_UMAC_MAC0 0x080C
> +#define GENET_UMAC_MAC1 0x0810
> +
> +STATIC
> +VOID
> +RMWRegister (
> + UINT32 Offset,
> + UINT32 Mask,
> + UINT32 In
> + )
> +{
> + EFI_PHYSICAL_ADDRESS Addr = GENET_BASE;
> + UINT32 Data = 0;
> + UINT32 Shift;
> +
> + Addr += Offset;
> + Shift = 1;
> + if (In) {
> + while (!(Mask & Shift))
> + Shift <<= 1;
> + Data = (MmioRead32 (Addr) & ~Mask) | ((In * Shift) & Mask);
> + } else {
> + Data = MmioRead32 (Addr) & ~Mask;
> + }
> +
> + MmioWrite32 (Addr, Data);
> +
> + ArmDataMemoryBarrier ();
> +}
> +
> +STATIC
> +VOID
> +WdRegister (
> + UINT32 Offset,
> + UINT32 In
> + )
> +{
> + EFI_PHYSICAL_ADDRESS Base = GENET_BASE;
> +
> + MmioWrite32 (Base + Offset, In);
> +
> + ArmDataMemoryBarrier ();
> +}
> +
> +
> +
> +STATIC VOID
> +GenetMacInit (UINT8 *addr)
> +{
> +
> + // bring the umac out of reset
> + RMWRegister (GENET_SYS_RBUF_FLUSH_CTRL, 0x2, 1);
> + gBS->Stall (10);
> + RMWRegister (GENET_SYS_RBUF_FLUSH_CTRL, 0x2, 0);
> +
> + // Update the MAC
> +
> + WdRegister(GENET_UMAC_MAC0, (addr[0] << 24) | (addr[1] << 16) | (addr[2] << 8) | addr[3]);
> + WdRegister(GENET_UMAC_MAC1, (addr[4] << 8) | addr[5]);
> +}
> +
> +/**
> + The entry point of Genet UEFI Driver.
> +
> + @param ImageHandle The image handle of the UEFI Driver.
> + @param SystemTable A pointer to the EFI System Table.
> +
> + @retval EFI_SUCCESS The Driver or UEFI Driver exited normally.
> + @retval EFI_INCOMPATIBLE_VERSION _gUefiDriverRevision is greater than
> + SystemTable->Hdr.Revision.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +GenetEntryPoint (
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_SYSTEM_TABLE *SystemTable
> + )
> +{
> + RASPBERRY_PI_FIRMWARE_PROTOCOL *mFwProtocol;
> + EFI_STATUS Status;
> + UINT8 MacAddr[6];
> +
> + DEBUG ((DEBUG_ERROR, "GENET:%a(): Init\n", __FUNCTION__));
> +
> + Status = gBS->LocateProtocol (&gRaspberryPiFirmwareProtocolGuid, NULL,
> + (VOID**)&mFwProtocol);
> + if (EFI_ERROR(Status)) {
> + DEBUG ((DEBUG_ERROR, "%a: failed to locate RPi firmware protocol\n", __FUNCTION__));
> + return Status;
> + }
> +
> + // Get the MAC address from the firmware
> + Status = mFwProtocol->GetMacAddress (MacAddr);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "%a: failed to retrieve MAC address\n", __FUNCTION__));
> + return Status;
> + }
> +
> + // Write it to the hardware
> + GenetMacInit (MacAddr);
> +
> + return EFI_SUCCESS;
> +}
> diff --git a/Platform/RaspberryPi/Drivers/Genet/Genet.inf b/Platform/RaspberryPi/Drivers/Genet/Genet.inf
> new file mode 100644
> index 0000000000..3599e5a6e5
> --- /dev/null
> +++ b/Platform/RaspberryPi/Drivers/Genet/Genet.inf
> @@ -0,0 +1,39 @@
> +## @file
> +#
> +# Copyright (c) 2020, Jeremy Linton All rights reserved.<BR>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> + INF_VERSION = 0x0001001A
> + BASE_NAME = Genet
> + FILE_GUID = e2b1eaf3-50b7-4ae1-b79e-ec8020cb57ac
> + MODULE_TYPE = UEFI_DRIVER
> + VERSION_STRING = 0.1
> + ENTRY_POINT = GenetEntryPoint
> +
> +[Sources]
> + Genet.c
> +
> +[Packages]
> + ArmPkg/ArmPkg.dec
> + MdeModulePkg/MdeModulePkg.dec
> + MdePkg/MdePkg.dec
> + Platform/RaspberryPi/RaspberryPi.dec
> +
> +[LibraryClasses]
> + BaseLib
> + IoLib
> + SynchronizationLib
> + UefiDriverEntryPoint
> + UefiLib
> +
> +[Protocols]
> + gRaspberryPiFirmwareProtocolGuid ## CONSUMES
> +
> +
> +[Depex]
> + gRaspberryPiFirmwareProtocolGuid
> +
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH] Platform/RPi4: Enable BCM GENET stub driver
[not found] ` <15EC824ADEB49EC2.12514@groups.io>
@ 2020-01-23 12:03 ` Pete Batard
0 siblings, 0 replies; 12+ messages in thread
From: Pete Batard @ 2020-01-23 12:03 UTC (permalink / raw)
To: devel; +Cc: ard.biesheuvel, leif, philmd, lintonrjeremy
**PLEASE DISREGARD THIS PATCH**
I had a couple of extra .patch files in my repo that were inadvertently
sent with the previous series.
/Pete
On 2020.01.23 12:00, Pete Batard via Groups.Io wrote:
> From: Jeremy Linton <lintonrjeremy@gmail.com>
>
> We have a stub driver, which retrieves the rpi's
> MAC address via the mailbox interface and programs the
> NIC. Lets enable it.
>
> Signed-off-by: Jeremy Linton <lintonrjeremy@gmail.com>
> ---
> Platform/RaspberryPi/RPi4/RPi4.dsc | 1 +
> Platform/RaspberryPi/RPi4/RPi4.fdf | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
> index 9aead4de32..97972382d5 100644
> --- a/Platform/RaspberryPi/RPi4/RPi4.dsc
> +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
> @@ -660,6 +660,7 @@ DEFINE TFA_BUILD_ARTIFACT = Platform/RaspberryPi/$(PLATFORM_NAME)/TrustedFirmwar
> # Networking stack
> #
> !include NetworkPkg/Network.dsc.inc
> + Platform/RaspberryPi/Drivers/Genet/Genet.inf
>
> #
> # RNG
> diff --git a/Platform/RaspberryPi/RPi4/RPi4.fdf b/Platform/RaspberryPi/RPi4/RPi4.fdf
> index e00a45b8a4..c44ec839c2 100644
> --- a/Platform/RaspberryPi/RPi4/RPi4.fdf
> +++ b/Platform/RaspberryPi/RPi4/RPi4.fdf
> @@ -265,6 +265,7 @@ READ_LOCK_STATUS = TRUE
> # Networking stack
> #
> !include NetworkPkg/Network.fdf.inc
> + INF Platform/RaspberryPi/Drivers/Genet/Genet.inf
>
> #
> # RNG
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-platforms][PATCH 1/3] Silicon/Broadcom/Net: Add Genet stub driver to setup MAC
2020-01-23 12:00 ` [edk2-platforms][PATCH 1/3] Silicon/Broadcom/Net: Add Genet stub driver to setup MAC Pete Batard
@ 2020-01-23 13:44 ` Ard Biesheuvel
2020-01-23 14:36 ` Pete Batard
0 siblings, 1 reply; 12+ messages in thread
From: Ard Biesheuvel @ 2020-01-23 13:44 UTC (permalink / raw)
To: Pete Batard
Cc: edk2-devel-groups-io, Leif Lindholm, Philippe Mathieu-Daudé,
Jeremy Linton
On Thu, 23 Jan 2020 at 13:00, Pete Batard <pete@akeo.ie> wrote:
>
> From: Jeremy Linton <lintonrjeremy@gmail.com>
>
> Add a stub for the Broadcom Genet network interface, that is used
> by the Raspberry Pi 4 (and that may be used by other platforms).
>
> For the time being, the stub only performs UMAC init, using a MAC
> address PCD, as, even without a working network interface in UEFI
> environment, it is desirable if the hardware can describe itself
> for booting in an ACPI environment, rather than pass parameters via
> DSDT/DSD .
>
> Signed-off-by: Pete Batard <pete@akeo.ie>
> ---
> Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf | 40 +++++++
> Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.c | 114 ++++++++++++++++++++
> Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.h | 20 ++++
> Silicon/Broadcom/Drivers/Net/BcmNet.dec | 22 ++++
> 4 files changed, 196 insertions(+)
>
> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
> new file mode 100644
> index 000000000000..9e9301608f24
> --- /dev/null
> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
> @@ -0,0 +1,40 @@
> +## @file
> +#
> +# Copyright (c) 2020, Jeremy Linton All rights reserved.<BR>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> + INF_VERSION = 0x0001001A
> + BASE_NAME = BcmGenetDxe
> + FILE_GUID = e2b1eaf3-50b7-4ae1-b79e-ec8020cb57ac
> + MODULE_TYPE = DXE_DRIVER
> + VERSION_STRING = 0.1
> + ENTRY_POINT = GenetEntryPoint
> +
> +[Sources]
> + Genet.c
> +
> +[Packages]
> + ArmPkg/ArmPkg.dec
> + MdeModulePkg/MdeModulePkg.dec
> + MdePkg/MdePkg.dec
> + Silicon/Broadcom/Drivers/Net/BcmNet.dec
> +
> +[LibraryClasses]
> + ArmLib
> + BaseLib
> + IoLib
> + UefiDriverEntryPoint
> + UefiLib
> +
> +[FixedPcd]
> + gBcmNetTokenSpaceGuid.PcdBcmGenetRegistersAddress
> +
> +[Pcd]
> + gBcmNetTokenSpaceGuid.PcdBcmGenetMacAddress
> +
> +[Depex]
> + TRUE
> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.c b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.c
> new file mode 100644
> index 000000000000..d135d889cc1f
> --- /dev/null
> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.c
> @@ -0,0 +1,114 @@
> +/** @file
> +
> + Copyright (c) 2020, Jeremy Linton All rights reserved.<BR>
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> + This driver acts like a stub to set the Broadcom
> + Genet MAC address, until the actual network driver
> + is in place.
> +
> +**/
> +
> +#include <Library/ArmLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/IoLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
> +
> +#include <Genet.h>
> +#include <PiDxe.h>
> +
> +STATIC
> +VOID
> +RMWRegister (
> + UINT32 Offset,
> + UINT32 Mask,
> + UINT32 In
> + )
> +{
> + EFI_PHYSICAL_ADDRESS Addr;
> + UINT32 Data;
> + UINT32 Shift;
> +
> + Addr = GENET_BASE_ADDRESS + Offset;
> + Data = 0;
> + Shift = 1;
> + if (In) {
> + while (!(Mask & Shift))
> + Shift <<= 1;
> + Data = (MmioRead32 (Addr) & ~Mask) | ((In * Shift) & Mask);
> + } else {
> + Data = MmioRead32 (Addr) & ~Mask;
> + }
> +
> + MmioWrite32 (Addr, Data);
> +
> + ArmDataMemoryBarrier ();
> +}
> +
> +STATIC
> +VOID
> +WdRegister (
> + UINT32 Offset,
> + UINT32 In
> + )
> +{
> + EFI_PHYSICAL_ADDRESS Base = GENET_BASE_ADDRESS;
> +
> + MmioWrite32 (Base + Offset, In);
> +
> + ArmDataMemoryBarrier ();
> +}
> +
> +STATIC
> +VOID
> +SetMacAddress (
> + UINT8* MacAddr
> +)
> +{
> + // Bring the UMAC out of reset
> + RMWRegister (GENET_SYS_RBUF_FLUSH_CTRL, 0x2, 1);
> + gBS->Stall (10);
> + RMWRegister (GENET_SYS_RBUF_FLUSH_CTRL, 0x2, 0);
> +
> + // Update the MAC
> + DEBUG ((DEBUG_INFO, "Setting MAC address %02X:%02X:%02X:%02X:%02X:%02X\n",
> + MacAddr[0], MacAddr[1], MacAddr[2], MacAddr[3], MacAddr[4], MacAddr[5]));
> +
> + WdRegister (GENET_UMAC_MAC0, (MacAddr[0] << 24) | (MacAddr[1] << 16) |
> + (MacAddr[2] << 8) | MacAddr[3]);
> + WdRegister (GENET_UMAC_MAC1, (MacAddr[4] << 8) | MacAddr[5]);
> +
> +}
> +
> +/**
> + The entry point of Genet UEFI Driver.
> +
> + @param ImageHandle The image handle of the UEFI Driver.
> + @param SystemTable A pointer to the EFI System Table.
> +
> + @retval EFI_SUCCESS The Driver or UEFI Driver exited normally.
> + @retval EFI_INCOMPATIBLE_VERSION _gUefiDriverRevision is greater than
> + SystemTable->Hdr.Revision.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +GenetEntryPoint (
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_SYSTEM_TABLE *SystemTable
> + )
> +{
> + UINT64 MacAddr;
> +
> + // Read the MAC address
> + MacAddr = PcdGet64 (PcdBcmGenetMacAddress);
> +
> + if (MacAddr != 0) {
> + SetMacAddress ((UINT8*)&MacAddr);
> + }
> +
> + return EFI_SUCCESS;
> +}
> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.h b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.h
> new file mode 100644
> index 000000000000..4a3827c0e0d1
> --- /dev/null
> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.h
> @@ -0,0 +1,20 @@
> +/** @file
> +
> + Copyright (c) 2020, Pete Batard <pete@akeo.ie>
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef BCM_GENET_H__
> +#define BCM_GENET_H__
> +
> +#include <Library/PcdLib.h>
> +
> +#define GENET_BASE_ADDRESS (FixedPcdGet64 (PcdBcmGenetRegistersAddress))
> +
> +#define GENET_SYS_RBUF_FLUSH_CTRL 0x0008
> +#define GENET_UMAC_MAC0 0x080c
> +#define GENET_UMAC_MAC1 0x0810
> +
> +#endif /* BCM_GENET_H__ */
> diff --git a/Silicon/Broadcom/Drivers/Net/BcmNet.dec b/Silicon/Broadcom/Drivers/Net/BcmNet.dec
> new file mode 100644
> index 000000000000..d80aac27ed9e
> --- /dev/null
> +++ b/Silicon/Broadcom/Drivers/Net/BcmNet.dec
> @@ -0,0 +1,22 @@
> +## @file
> +#
> +# Copyright (c) 2020, Pete Batard <pete@akeo.ie>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> + DEC_SPECIFICATION = 0x0001001A
> + PACKAGE_NAME = BcmNetPkg
> + PACKAGE_GUID = 34E19823-D23A-41AB-9C09-ED1225B32DFF
> + PACKAGE_VERSION = 1.0
> +
> +[Guids]
> + gBcmNetTokenSpaceGuid = {0x12b97d70, 0x9149, 0x4c2f, {0x82, 0xd5, 0xad, 0xa9, 0x1e, 0x92, 0x75, 0xa1}}
> +
> +[PcdsFixedAtBuild]
> + gBcmNetTokenSpaceGuid.PcdBcmGenetRegistersAddress|0x0|UINT32|0x00000001
> +
Can we make this UINT64 please? EFI_PHYSICAL_ADDRESSes are 64-bit in UEFI.
> +[PcdsDynamic]
> + gBcmNetTokenSpaceGuid.PcdBcmGenetMacAddress|0x0|UINT64|0x00000002
> --
> 2.21.0.windows.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-platforms][PATCH 2/3] Platform/RPi: Add PlatformPcdLib to set the Genet MAC address
2020-01-23 12:00 ` [edk2-platforms][PATCH 2/3] Platform/RPi: Add PlatformPcdLib to set the Genet MAC address Pete Batard
@ 2020-01-23 13:50 ` Ard Biesheuvel
2020-01-23 15:01 ` Pete Batard
0 siblings, 1 reply; 12+ messages in thread
From: Ard Biesheuvel @ 2020-01-23 13:50 UTC (permalink / raw)
To: Pete Batard
Cc: edk2-devel-groups-io, Leif Lindholm, Philippe Mathieu-Daudé,
Jeremy Linton
On Thu, 23 Jan 2020 at 13:00, Pete Batard <pete@akeo.ie> wrote:
>
> The Genet driver stub used by the Raspberry Pi 4 platform is
> designed to set the MAC address according to a PCD.
>
> To be able to set that PCD at runtime, by using the Raspberry
> Pi firmware interface, that has a dedicated call to retrieve
> the MAC address, and satisfy driver dependencies in a generic
> manner, we create a new PlatformPcdLib that can be referenced
> by the Genet driver, to set the MAC PCD before use there.
>
> While it is currently only tailored around MAC PCD population
> for Genet, we do expect this PCD library to be extended in the
> future, to provide additional PCD facilities for other drivers.
>
> Signed-off-by: Pete Batard <pete@akeo.ie>
> ---
> Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.c | 61 ++++++++++++++++++++
> Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.inf | 44 ++++++++++++++
> 2 files changed, 105 insertions(+)
>
> diff --git a/Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.c b/Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.c
> new file mode 100644
> index 000000000000..78f3679e2e48
> --- /dev/null
> +++ b/Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.c
> @@ -0,0 +1,61 @@
> +/** @file
> + *
> + * Copyright (c) 2020, Pete Batard <pete@akeo.ie>
> + *
> + * SPDX-License-Identifier: BSD-2-Clause-Patent
> + *
> + **/
> +
> +#include <Library/DebugLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/PrintLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
> +#include <Library/UefiRuntimeServicesTableLib.h>
> +#include <Protocol/RpiFirmware.h>
> +
> +EFI_STATUS
> +EFIAPI
> +PlatformPcdLibConstructor (
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_SYSTEM_TABLE *SystemTable
> + )
> +{
> +
> + //
> + // If Genet is in use and a MAC address PCD has not been set, do it here.
> + //
> +#if (FixedPcdGet64 (PcdBcmGenetRegistersAddress) != 0)
Can we drop this check? Why would you include this library in your
.DSC and set the fixed PCD to 0?
> + EFI_STATUS Status;
> + UINT64 MacAddr;
> + RASPBERRY_PI_FIRMWARE_PROTOCOL *mFwProtocol;
> +
> + if (PcdGet64 (PcdBcmGenetMacAddress) == 0) {
This is policy, which we don't need today, and may well be needed
elsewhere once we do need it. So I'd prefer it if we just drop this
check.
> + Status = gBS->LocateProtocol (&gRaspberryPiFirmwareProtocolGuid, NULL,
> + (VOID**)&mFwProtocol);
> + ASSERT_EFI_ERROR(Status);
> +
> + //
> + // Get the MAC address from the firmware
> + //
> + Status = mFwProtocol->GetMacAddress ((UINT8*) &MacAddr);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_WARN, "%a: failed to retrieve MAC address\n", __FUNCTION__));
> + } else {
> + PcdSet64 (PcdBcmGenetMacAddress, MacAddr);
Please use PcdSet64S () instead - the unsafe ones are deprecated AFAIR
> + }
> + }
> +#endif
> +
> + return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +PlatformPcdLibDestructor (
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_SYSTEM_TABLE *SystemTable
> + )
> +{
> + return EFI_SUCCESS;
> +}
No need for a destructor
> diff --git a/Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.inf b/Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.inf
> new file mode 100644
> index 000000000000..a564ddfbb2a3
> --- /dev/null
> +++ b/Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.inf
> @@ -0,0 +1,44 @@
> +#/** @file
> +#
> +# Copyright (c) 2020, Pete Batard <pete@akeo.ie>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#**/
> +
> +[Defines]
> + INF_VERSION = 0x0001001A
> + BASE_NAME = PlatformPcdLib
> + FILE_GUID = 3B8409D7-D3C7-4006-823B-BFB184435363
> + MODULE_TYPE = DXE_DRIVER
> + VERSION_STRING = 1.0
> + LIBRARY_CLASS = NULL|DXE_DRIVER UEFI_APPLICATION
> + CONSTRUCTOR = PlatformPcdLibConstructor
> + DESTRUCTOR = PlatformPcdLibDestructor
> +
> +[Sources]
> + PlatformPcdLib.c
> +
> +[Packages]
> + MdePkg/MdePkg.dec
> + MdeModulePkg/MdeModulePkg.dec
> + Platform/RaspberryPi/RaspberryPi.dec
> + Silicon/Broadcom/Drivers/Net/BcmNet.dec
> +
> +[LibraryClasses]
> + UefiLib
> + DebugLib
> + PrintLib
> +
> +[Protocols]
> + gRaspberryPiFirmwareProtocolGuid ## CONSUMES
> +
> +[Pcd]
> + gBcmNetTokenSpaceGuid.PcdBcmGenetMacAddress ## SOMETIMES_PRODUCES
> +
> +[FixedPcd]
> + gBcmNetTokenSpaceGuid.PcdBcmGenetRegistersAddress
> +
> +[Depex]
> + gPcdProtocolGuid AND gRaspberryPiFirmwareProtocolGuid
> +
Use the PcdLib library class instead - this will depex on the
gPcdProtocolGuid if needed.
I don't mind this approach, but in general, I'd be happier with a
specific library class to discover the MAC address.
I.e., a library class in BcmNet.dec called BcmGenetPlatformLib, which
has [for now] a single function called BcmGenetGetMacFromPlatform().
The RPi4 implementation would provide that routine, and depex on the
firmware protocol, ensuring the ordering is correct. That way, we can
get rid of the PCDs entirely.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-platforms][PATCH 1/3] Silicon/Broadcom/Net: Add Genet stub driver to setup MAC
2020-01-23 13:44 ` Ard Biesheuvel
@ 2020-01-23 14:36 ` Pete Batard
0 siblings, 0 replies; 12+ messages in thread
From: Pete Batard @ 2020-01-23 14:36 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: edk2-devel-groups-io, Leif Lindholm, Philippe Mathieu-Daudé,
Jeremy Linton
Hi Ard,
On 2020.01.23 13:44, Ard Biesheuvel wrote:
> On Thu, 23 Jan 2020 at 13:00, Pete Batard <pete@akeo.ie> wrote:
>>
>> From: Jeremy Linton <lintonrjeremy@gmail.com>
>>
>> Add a stub for the Broadcom Genet network interface, that is used
>> by the Raspberry Pi 4 (and that may be used by other platforms).
>>
>> For the time being, the stub only performs UMAC init, using a MAC
>> address PCD, as, even without a working network interface in UEFI
>> environment, it is desirable if the hardware can describe itself
>> for booting in an ACPI environment, rather than pass parameters via
>> DSDT/DSD .
>>
>> Signed-off-by: Pete Batard <pete@akeo.ie>
>> ---
>> Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf | 40 +++++++
>> Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.c | 114 ++++++++++++++++++++
>> Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.h | 20 ++++
>> Silicon/Broadcom/Drivers/Net/BcmNet.dec | 22 ++++
>> 4 files changed, 196 insertions(+)
>>
>> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
>> new file mode 100644
>> index 000000000000..9e9301608f24
>> --- /dev/null
>> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
>> @@ -0,0 +1,40 @@
>> +## @file
>> +#
>> +# Copyright (c) 2020, Jeremy Linton All rights reserved.<BR>
>> +#
>> +# SPDX-License-Identifier: BSD-2-Clause-Patent
>> +#
>> +##
>> +
>> +[Defines]
>> + INF_VERSION = 0x0001001A
>> + BASE_NAME = BcmGenetDxe
>> + FILE_GUID = e2b1eaf3-50b7-4ae1-b79e-ec8020cb57ac
>> + MODULE_TYPE = DXE_DRIVER
>> + VERSION_STRING = 0.1
>> + ENTRY_POINT = GenetEntryPoint
>> +
>> +[Sources]
>> + Genet.c
>> +
>> +[Packages]
>> + ArmPkg/ArmPkg.dec
>> + MdeModulePkg/MdeModulePkg.dec
>> + MdePkg/MdePkg.dec
>> + Silicon/Broadcom/Drivers/Net/BcmNet.dec
>> +
>> +[LibraryClasses]
>> + ArmLib
>> + BaseLib
>> + IoLib
>> + UefiDriverEntryPoint
>> + UefiLib
>> +
>> +[FixedPcd]
>> + gBcmNetTokenSpaceGuid.PcdBcmGenetRegistersAddress
>> +
>> +[Pcd]
>> + gBcmNetTokenSpaceGuid.PcdBcmGenetMacAddress
>> +
>> +[Depex]
>> + TRUE
>> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.c b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.c
>> new file mode 100644
>> index 000000000000..d135d889cc1f
>> --- /dev/null
>> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.c
>> @@ -0,0 +1,114 @@
>> +/** @file
>> +
>> + Copyright (c) 2020, Jeremy Linton All rights reserved.<BR>
>> +
>> + SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> + This driver acts like a stub to set the Broadcom
>> + Genet MAC address, until the actual network driver
>> + is in place.
>> +
>> +**/
>> +
>> +#include <Library/ArmLib.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/IoLib.h>
>> +#include <Library/PcdLib.h>
>> +#include <Library/UefiBootServicesTableLib.h>
>> +#include <Library/UefiLib.h>
>> +
>> +#include <Genet.h>
>> +#include <PiDxe.h>
>> +
>> +STATIC
>> +VOID
>> +RMWRegister (
>> + UINT32 Offset,
>> + UINT32 Mask,
>> + UINT32 In
>> + )
>> +{
>> + EFI_PHYSICAL_ADDRESS Addr;
>> + UINT32 Data;
>> + UINT32 Shift;
>> +
>> + Addr = GENET_BASE_ADDRESS + Offset;
>> + Data = 0;
>> + Shift = 1;
>> + if (In) {
>> + while (!(Mask & Shift))
>> + Shift <<= 1;
>> + Data = (MmioRead32 (Addr) & ~Mask) | ((In * Shift) & Mask);
>> + } else {
>> + Data = MmioRead32 (Addr) & ~Mask;
>> + }
>> +
>> + MmioWrite32 (Addr, Data);
>> +
>> + ArmDataMemoryBarrier ();
>> +}
>> +
>> +STATIC
>> +VOID
>> +WdRegister (
>> + UINT32 Offset,
>> + UINT32 In
>> + )
>> +{
>> + EFI_PHYSICAL_ADDRESS Base = GENET_BASE_ADDRESS;
>> +
>> + MmioWrite32 (Base + Offset, In);
>> +
>> + ArmDataMemoryBarrier ();
>> +}
>> +
>> +STATIC
>> +VOID
>> +SetMacAddress (
>> + UINT8* MacAddr
>> +)
>> +{
>> + // Bring the UMAC out of reset
>> + RMWRegister (GENET_SYS_RBUF_FLUSH_CTRL, 0x2, 1);
>> + gBS->Stall (10);
>> + RMWRegister (GENET_SYS_RBUF_FLUSH_CTRL, 0x2, 0);
>> +
>> + // Update the MAC
>> + DEBUG ((DEBUG_INFO, "Setting MAC address %02X:%02X:%02X:%02X:%02X:%02X\n",
>> + MacAddr[0], MacAddr[1], MacAddr[2], MacAddr[3], MacAddr[4], MacAddr[5]));
>> +
>> + WdRegister (GENET_UMAC_MAC0, (MacAddr[0] << 24) | (MacAddr[1] << 16) |
>> + (MacAddr[2] << 8) | MacAddr[3]);
>> + WdRegister (GENET_UMAC_MAC1, (MacAddr[4] << 8) | MacAddr[5]);
>> +
>> +}
>> +
>> +/**
>> + The entry point of Genet UEFI Driver.
>> +
>> + @param ImageHandle The image handle of the UEFI Driver.
>> + @param SystemTable A pointer to the EFI System Table.
>> +
>> + @retval EFI_SUCCESS The Driver or UEFI Driver exited normally.
>> + @retval EFI_INCOMPATIBLE_VERSION _gUefiDriverRevision is greater than
>> + SystemTable->Hdr.Revision.
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +GenetEntryPoint (
>> + IN EFI_HANDLE ImageHandle,
>> + IN EFI_SYSTEM_TABLE *SystemTable
>> + )
>> +{
>> + UINT64 MacAddr;
>> +
>> + // Read the MAC address
>> + MacAddr = PcdGet64 (PcdBcmGenetMacAddress);
>> +
>> + if (MacAddr != 0) {
>> + SetMacAddress ((UINT8*)&MacAddr);
>> + }
>> +
>> + return EFI_SUCCESS;
>> +}
>> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.h b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.h
>> new file mode 100644
>> index 000000000000..4a3827c0e0d1
>> --- /dev/null
>> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/Genet.h
>> @@ -0,0 +1,20 @@
>> +/** @file
>> +
>> + Copyright (c) 2020, Pete Batard <pete@akeo.ie>
>> +
>> + SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> +**/
>> +
>> +#ifndef BCM_GENET_H__
>> +#define BCM_GENET_H__
>> +
>> +#include <Library/PcdLib.h>
>> +
>> +#define GENET_BASE_ADDRESS (FixedPcdGet64 (PcdBcmGenetRegistersAddress))
>> +
>> +#define GENET_SYS_RBUF_FLUSH_CTRL 0x0008
>> +#define GENET_UMAC_MAC0 0x080c
>> +#define GENET_UMAC_MAC1 0x0810
>> +
>> +#endif /* BCM_GENET_H__ */
>> diff --git a/Silicon/Broadcom/Drivers/Net/BcmNet.dec b/Silicon/Broadcom/Drivers/Net/BcmNet.dec
>> new file mode 100644
>> index 000000000000..d80aac27ed9e
>> --- /dev/null
>> +++ b/Silicon/Broadcom/Drivers/Net/BcmNet.dec
>> @@ -0,0 +1,22 @@
>> +## @file
>> +#
>> +# Copyright (c) 2020, Pete Batard <pete@akeo.ie>
>> +#
>> +# SPDX-License-Identifier: BSD-2-Clause-Patent
>> +#
>> +##
>> +
>> +[Defines]
>> + DEC_SPECIFICATION = 0x0001001A
>> + PACKAGE_NAME = BcmNetPkg
>> + PACKAGE_GUID = 34E19823-D23A-41AB-9C09-ED1225B32DFF
>> + PACKAGE_VERSION = 1.0
>> +
>> +[Guids]
>> + gBcmNetTokenSpaceGuid = {0x12b97d70, 0x9149, 0x4c2f, {0x82, 0xd5, 0xad, 0xa9, 0x1e, 0x92, 0x75, 0xa1}}
>> +
>> +[PcdsFixedAtBuild]
>> + gBcmNetTokenSpaceGuid.PcdBcmGenetRegistersAddress|0x0|UINT32|0x00000001
>> +
>
> Can we make this UINT64 please? EFI_PHYSICAL_ADDRESSes are 64-bit in UEFI.
Will do.
/Pete
>
>> +[PcdsDynamic]
>> + gBcmNetTokenSpaceGuid.PcdBcmGenetMacAddress|0x0|UINT64|0x00000002
>> --
>> 2.21.0.windows.1
>>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-platforms][PATCH 2/3] Platform/RPi: Add PlatformPcdLib to set the Genet MAC address
2020-01-23 13:50 ` Ard Biesheuvel
@ 2020-01-23 15:01 ` Pete Batard
0 siblings, 0 replies; 12+ messages in thread
From: Pete Batard @ 2020-01-23 15:01 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: edk2-devel-groups-io, Leif Lindholm, Philippe Mathieu-Daudé,
Jeremy Linton
On 2020.01.23 13:50, Ard Biesheuvel wrote:
> On Thu, 23 Jan 2020 at 13:00, Pete Batard <pete@akeo.ie> wrote:
>>
>> The Genet driver stub used by the Raspberry Pi 4 platform is
>> designed to set the MAC address according to a PCD.
>>
>> To be able to set that PCD at runtime, by using the Raspberry
>> Pi firmware interface, that has a dedicated call to retrieve
>> the MAC address, and satisfy driver dependencies in a generic
>> manner, we create a new PlatformPcdLib that can be referenced
>> by the Genet driver, to set the MAC PCD before use there.
>>
>> While it is currently only tailored around MAC PCD population
>> for Genet, we do expect this PCD library to be extended in the
>> future, to provide additional PCD facilities for other drivers.
>>
>> Signed-off-by: Pete Batard <pete@akeo.ie>
>> ---
>> Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.c | 61 ++++++++++++++++++++
>> Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.inf | 44 ++++++++++++++
>> 2 files changed, 105 insertions(+)
>>
>> diff --git a/Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.c b/Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.c
>> new file mode 100644
>> index 000000000000..78f3679e2e48
>> --- /dev/null
>> +++ b/Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.c
>> @@ -0,0 +1,61 @@
>> +/** @file
>> + *
>> + * Copyright (c) 2020, Pete Batard <pete@akeo.ie>
>> + *
>> + * SPDX-License-Identifier: BSD-2-Clause-Patent
>> + *
>> + **/
>> +
>> +#include <Library/DebugLib.h>
>> +#include <Library/PcdLib.h>
>> +#include <Library/PrintLib.h>
>> +#include <Library/UefiBootServicesTableLib.h>
>> +#include <Library/UefiLib.h>
>> +#include <Library/UefiRuntimeServicesTableLib.h>
>> +#include <Protocol/RpiFirmware.h>
>> +
>> +EFI_STATUS
>> +EFIAPI
>> +PlatformPcdLibConstructor (
>> + IN EFI_HANDLE ImageHandle,
>> + IN EFI_SYSTEM_TABLE *SystemTable
>> + )
>> +{
>> +
>> + //
>> + // If Genet is in use and a MAC address PCD has not been set, do it here.
>> + //
>> +#if (FixedPcdGet64 (PcdBcmGenetRegistersAddress) != 0)
>
> Can we drop this check? Why would you include this library in your
> .DSC and set the fixed PCD to 0?
Well, the idea is that we may use this PCD library on the Pi 3 as well,
if we want to use a similar approach to this for other features, in
which case PcdBcmGenetRegistersAddress will be 0.
The fact that this library resides in Platform/RaspberryPi/Library/
rather than Platform/RaspberryPi/RPi4/Library/ means that it is designed
to be used on more than the Pi 4 platform if needed.
>
>> + EFI_STATUS Status;
>> + UINT64 MacAddr;
>> + RASPBERRY_PI_FIRMWARE_PROTOCOL *mFwProtocol;
>> +
>> + if (PcdGet64 (PcdBcmGenetMacAddress) == 0) {
>
> This is policy, which we don't need today, and may well be needed
> elsewhere once we do need it. So I'd prefer it if we just drop this
> check.
I'm not sure I understand your point.
This patchset is designed around the idea that someone may also provide
the MAC address at build time to force a specific MAC on their platform.
I can see specific scenarios for such a use (e.g. someone using Pi 4's
in a environment where they assign a specific IP according to the MAC
and planning for drop-in replacement in case of hardware failures) and
even outside of these, I can see people wanting for force a MAC that is
different than the hardware's (e.g. could also be handy for spoofing an
existing network device with an inexpensive Pi 4), without having to
touch the code. Granted, they should be able to perform the same in high
level OS, but once we add a full NIC driver in UEFI, we may announce a
different MAC prior to OS network init, and I think, since we can do it
here, and it will be picked up by the OS, we might as well do so since
it's easy to add.
As such, if someone did set the MAC PCD at build time, we shouldn't
override it.
>
>> + Status = gBS->LocateProtocol (&gRaspberryPiFirmwareProtocolGuid, NULL,
>> + (VOID**)&mFwProtocol);
>> + ASSERT_EFI_ERROR(Status);
>> +
>> + //
>> + // Get the MAC address from the firmware
>> + //
>> + Status = mFwProtocol->GetMacAddress ((UINT8*) &MacAddr);
>> + if (EFI_ERROR (Status)) {
>> + DEBUG ((DEBUG_WARN, "%a: failed to retrieve MAC address\n", __FUNCTION__));
>> + } else {
>> + PcdSet64 (PcdBcmGenetMacAddress, MacAddr);
>
> Please use PcdSet64S () instead - the unsafe ones are deprecated AFAIR
Will do.
>
>> + }
>> + }
>> +#endif
>> +
>> + return EFI_SUCCESS;
>> +}
>> +
>> +EFI_STATUS
>> +EFIAPI
>> +PlatformPcdLibDestructor (
>> + IN EFI_HANDLE ImageHandle,
>> + IN EFI_SYSTEM_TABLE *SystemTable
>> + )
>> +{
>> + return EFI_SUCCESS;
>> +}
>
> No need for a destructor
Yeah, I wasn't sure if that worked without one, and PlatformUiAppLib has
a similar empty destructor, so I preferred to ensure it was present.
I'll remove it form the patchset.
>
>> diff --git a/Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.inf b/Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.inf
>> new file mode 100644
>> index 000000000000..a564ddfbb2a3
>> --- /dev/null
>> +++ b/Platform/RaspberryPi/Library/PlatformPcdLib/PlatformPcdLib.inf
>> @@ -0,0 +1,44 @@
>> +#/** @file
>> +#
>> +# Copyright (c) 2020, Pete Batard <pete@akeo.ie>
>> +#
>> +# SPDX-License-Identifier: BSD-2-Clause-Patent
>> +#
>> +#**/
>> +
>> +[Defines]
>> + INF_VERSION = 0x0001001A
>> + BASE_NAME = PlatformPcdLib
>> + FILE_GUID = 3B8409D7-D3C7-4006-823B-BFB184435363
>> + MODULE_TYPE = DXE_DRIVER
>> + VERSION_STRING = 1.0
>> + LIBRARY_CLASS = NULL|DXE_DRIVER UEFI_APPLICATION
>> + CONSTRUCTOR = PlatformPcdLibConstructor
>> + DESTRUCTOR = PlatformPcdLibDestructor
>> +
>> +[Sources]
>> + PlatformPcdLib.c
>> +
>> +[Packages]
>> + MdePkg/MdePkg.dec
>> + MdeModulePkg/MdeModulePkg.dec
>> + Platform/RaspberryPi/RaspberryPi.dec
>> + Silicon/Broadcom/Drivers/Net/BcmNet.dec
>> +
>> +[LibraryClasses]
>> + UefiLib
>> + DebugLib
>> + PrintLib
>> +
>> +[Protocols]
>> + gRaspberryPiFirmwareProtocolGuid ## CONSUMES
>> +
>> +[Pcd]
>> + gBcmNetTokenSpaceGuid.PcdBcmGenetMacAddress ## SOMETIMES_PRODUCES
>> +
>> +[FixedPcd]
>> + gBcmNetTokenSpaceGuid.PcdBcmGenetRegistersAddress
>> +
>> +[Depex]
>> + gPcdProtocolGuid AND gRaspberryPiFirmwareProtocolGuid
>> +
>
> Use the PcdLib library class instead - this will depex on the
> gPcdProtocolGuid if needed.
>
> I don't mind this approach, but in general, I'd be happier with a
> specific library class to discover the MAC address.
>
> I.e., a library class in BcmNet.dec called BcmGenetPlatformLib, which
> has [for now] a single function called BcmGenetGetMacFromPlatform().
> The RPi4 implementation would provide that routine, and depex on the
> firmware protocol, ensuring the ordering is correct. That way, we can
> get rid of the PCDs entirely.
I get your point. But I'd rather not do that, since I do see some use
for a PCD provided MAC.
Regards,
/Pete
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-01-23 15:01 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-01-23 12:00 [edk2-platforms][PATCH 0/3] Platform/RPi4: Add Genet network driver stub Pete Batard
2020-01-23 12:00 ` [edk2-platforms][PATCH 1/3] Silicon/Broadcom/Net: Add Genet stub driver to setup MAC Pete Batard
2020-01-23 13:44 ` Ard Biesheuvel
2020-01-23 14:36 ` Pete Batard
2020-01-23 12:00 ` [edk2-platforms][PATCH 2/3] Platform/RPi: Add PlatformPcdLib to set the Genet MAC address Pete Batard
2020-01-23 13:50 ` Ard Biesheuvel
2020-01-23 15:01 ` Pete Batard
2020-01-23 12:00 ` [edk2-platforms][PATCH 3/3] Platform/Rpi4: Enable Broadcom Genet stub driver Pete Batard
2020-01-23 12:00 ` [PATCH] Platform/RPi/Genet: Add Genet stub driver to setup MAC Pete Batard
2020-01-23 12:00 ` [PATCH] Platform/RPi4: Enable BCM GENET stub driver Pete Batard
[not found] ` <15EC824B064B2D10.12514@groups.io>
2020-01-23 12:03 ` [edk2-devel] [PATCH] Platform/RPi/Genet: Add Genet stub driver to setup MAC Pete Batard
[not found] ` <15EC824ADEB49EC2.12514@groups.io>
2020-01-23 12:03 ` [edk2-devel] [PATCH] Platform/RPi4: Enable BCM GENET stub driver Pete Batard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox