Given that we're unlikely to ever see Cellos with the MAC programmed correctly, implement a driver that programs a MAC in a volatile manner instead. This still does not allow us to boot from the network, but at least we no longer have to care about this at the OS level.
Ard Biesheuvel (2): Drivers/Net: add MAC override driver for Realtek 8169 Platforms/AMD/Cello: add Realtek MAC override driver
Drivers/Net/Realtek8169MacOverride/Realtek8169MacOverride.c | 262 ++++++++++++++++++++ Drivers/Net/Realtek8169MacOverride/Realtek8169MacOverride.inf | 44 ++++ OpenPlatformPkg.dec | 3 + Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc | 7 + Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf | 4 + 5 files changed, 320 insertions(+) create mode 100644 Drivers/Net/Realtek8169MacOverride/Realtek8169MacOverride.c create mode 100644 Drivers/Net/Realtek8169MacOverride/Realtek8169MacOverride.inf
Sadly, the Cellos have been delivered with a Realtek NIC that lacks a MAC address in its OTP. This can be worked around from the OS, but this confuses some installers, and it is much better to deal with this in the firmware.
Since network boot using the full Realtek 8169 driver is impossible anyway in this case, for the same reason, let's provide a driver that programs a MAC into the volatile NIC registers in a way that allows the OS to run unmodified.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org --- Drivers/Net/Realtek8169MacOverride/Realtek8169MacOverride.c | 262 ++++++++++++++++++++ Drivers/Net/Realtek8169MacOverride/Realtek8169MacOverride.inf | 44 ++++ OpenPlatformPkg.dec | 3 + 3 files changed, 309 insertions(+)
diff --git a/Drivers/Net/Realtek8169MacOverride/Realtek8169MacOverride.c b/Drivers/Net/Realtek8169MacOverride/Realtek8169MacOverride.c new file mode 100644 index 000000000000..9b2925ba35d1 --- /dev/null +++ b/Drivers/Net/Realtek8169MacOverride/Realtek8169MacOverride.c @@ -0,0 +1,262 @@ +/** @file + Realtek 8169 MAC override driver + +Copyright (c) 2017, Linaro Limited. All rights reserved. + +This program and the accompanying materials are licensed +and made available under the terms and conditions of the BSD License which +accompanies this distribution. The full text of the license may be found at +http://opensource.org/licenses/bsd-license.php + +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. + +**/ + +#include <PiDxe.h> + +#include <Library/DebugLib.h> +#include <Library/DxeServicesLib.h> +#include <Library/UefiBootServicesTableLib.h> +#include <Library/UefiLib.h> + +#include <IndustryStandard/Pci22.h> + +#include <Protocol/PciIo.h> + +#define PCI_VENDOR_ID_REALTEK 0x10EC +#define PCI_DEVICE_ID_REALTEK_8169 0x8168 + +STATIC CONST UINT8 *mMacOverride; + +#define MAC 0x0 +#define CFG9346 0x50 + +STATIC CONST UINT8 Cfg9346_Unlock = 0xc0; +STATIC CONST UINT8 Cfg9346_Lock = 0x0; + +STATIC BOOLEAN Done; + +/** + Test to see if this driver supports ControllerHandle. This service + is called by the EFI boot service ConnectController(). In + order to make drivers as small as possible, there are a few calling + restrictions for this service. ConnectController() must + follow these calling restrictions. If any other agent wishes to call + Supported() it must also follow these calling restrictions. + + @param This Protocol instance pointer. + @param ControllerHandle Handle of device to test. + @param RemainingDevicePath Optional parameter use to pick a specific child + device to start. + + @retval EFI_SUCCESS This driver supports this device. + @retval EFI_ALREADY_STARTED This driver is already running on this device. + @retval other This driver does not support this device. + +**/ +STATIC +EFI_STATUS +EFIAPI +Realtek8169MacOverrideDriverSupported ( + IN EFI_DRIVER_BINDING_PROTOCOL *This, + IN EFI_HANDLE Controller, + IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath + ) +{ + EFI_STATUS Status; + EFI_PCI_IO_PROTOCOL *PciIo; + UINT32 PciID; + UINT64 Supports; + + // + // Execute only once + // + if (Done) { + return EFI_UNSUPPORTED; + } + + // + // Check for the PCI IO Protocol + // + Status = gBS->OpenProtocol (Controller, &gEfiPciIoProtocolGuid, + (VOID **)&PciIo, This->DriverBindingHandle, Controller, + EFI_OPEN_PROTOCOL_BY_DRIVER); + + if (EFI_ERROR (Status)) { + return Status; + } + + Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint32, PCI_VENDOR_ID_OFFSET, + 1, &PciID); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, + "%a: Pci->Pci.Read() of vendor/device id failed (Status == %r)\n", + __FUNCTION__, Status)); + goto CloseProtocol; + } + + if ((PciID & 0xffff) != PCI_VENDOR_ID_REALTEK || + (PciID >> 16) != PCI_DEVICE_ID_REALTEK_8169) { + DEBUG ((DEBUG_INFO, "%a: ignoring unsupported PCI device 0x%04x:0x%04x\n", + __FUNCTION__, PciID & 0xffff, PciID >> 16)); + goto CloseProtocol; + } + + // + // Enable the device so we can poke at its MMIO registers + // + Status = PciIo->Attributes (PciIo, EfiPciIoAttributeOperationSupported, + 0, &Supports); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_WARN, + "%a: failed to get PCI attributes, aborting (Status == %r)...\n", + __FUNCTION__, Status)); + goto CloseProtocol; + } + + Supports &= EFI_PCI_DEVICE_ENABLE; + Status = PciIo->Attributes (PciIo, EfiPciIoAttributeOperationEnable, + Supports, NULL); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_WARN, + "%a: failed to set PCI attributes, aborting (Status == %r)...\n", + __FUNCTION__, Status)); + goto CloseProtocol; + } + + // + // Program the MAC address + // + Status = PciIo->Mem.Write (PciIo, EfiPciIoWidthUint8, 2, CFG9346, 0x1, + (VOID *)&Cfg9346_Unlock); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_WARN, "%a: unlock failed, aborting (Status == %r)...\n", __FUNCTION__, Status)); + goto CloseProtocol; + } + + Status = PciIo->Mem.Write (PciIo, EfiPciIoWidthUint32, 2, MAC + 4, 0x1, + (UINT8 *)mMacOverride + 4); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_WARN, "%a: failed to set MAC address (#1) ...\n", __FUNCTION__)); + } + + Status = PciIo->Mem.Write (PciIo, EfiPciIoWidthUint32, 2, MAC, 0x1, + (UINT8 *)mMacOverride); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_WARN, "%a: failed to set MAC address (#2) ...\n", __FUNCTION__)); + } + + Status = PciIo->Mem.Write (PciIo, EfiPciIoWidthUint8, 2, CFG9346, 0x1, + (VOID *)&Cfg9346_Lock); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_WARN, "%a: relock failed ...\n", __FUNCTION__)); + goto CloseProtocol; + } + + Done = TRUE; + + PciIo->Attributes (PciIo, EfiPciIoAttributeOperationDisable, Supports, NULL); + +CloseProtocol: + gBS->CloseProtocol (Controller, &gEfiPciIoProtocolGuid, + This->DriverBindingHandle, Controller); + + // + // Always return unsupported: we are not interested in driving the device, + // only in having the opportunity to install the firmware before the real + // driver attaches to it. + // + return EFI_UNSUPPORTED; +} + +/** + Start this driver on Controller. Not used. + + @param [in] This Protocol instance pointer. + @param [in] Controller Handle of device to work with. + @param [in] RemainingDevicePath Not used, always produce all possible children. + + @retval EFI_SUCCESS This driver is added to Controller. + @retval other This driver does not support this device. + +**/ +STATIC +EFI_STATUS +EFIAPI +Realtek8169MacOverrideDriverStart ( + IN EFI_DRIVER_BINDING_PROTOCOL *This, + IN EFI_HANDLE Controller, + IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath + ) +{ + // + // We are not interested in driving the device, we only poke the firmware + // in the .Supported() callback. + // + ASSERT (FALSE); + return EFI_INVALID_PARAMETER; +} + +/** + Stop this driver on Controller. Not used. + + @param [in] This Protocol instance pointer. + @param [in] Controller Handle of device to stop driver on. + @param [in] NumberOfChildren How many children need to be stopped. + @param [in] ChildHandleBuffer Not used. + + @retval EFI_SUCCESS This driver is removed Controller. + @retval EFI_DEVICE_ERROR The device could not be stopped due to a device error. + @retval other This driver was not removed from this device. + +**/ +STATIC +EFI_STATUS +EFIAPI +Realtek8169MacOverrideDriverStop ( + IN EFI_DRIVER_BINDING_PROTOCOL *This, + IN EFI_HANDLE Controller, + IN UINTN NumberOfChildren, + IN EFI_HANDLE *ChildHandleBuffer + ) +{ + ASSERT (FALSE); + return EFI_SUCCESS; +} + +// +// UEFI Driver Model entry point +// +STATIC EFI_DRIVER_BINDING_PROTOCOL Realtek8169MacOverrideDriverBinding = { + Realtek8169MacOverrideDriverSupported, + Realtek8169MacOverrideDriverStart, + Realtek8169MacOverrideDriverStop, + + // Version values of 0xfffffff0-0xffffffff are reserved for platform/OEM + // specific drivers. Protocol instances with higher 'Version' properties + // will be used before lower 'Version' ones. XhciDxe uses version 0x30, + // so this driver will be called in preference, and XhciDxe will be invoked + // after Realtek8169MacOverrideDriverSupported returns EFI_UNSUPPORTED. + 0xfffffff0, + NULL, + NULL +}; + +EFI_STATUS +EFIAPI +InitializeRealtek8169MacOverride ( + IN EFI_HANDLE ImageHandle, + IN EFI_SYSTEM_TABLE *SystemTable + ) +{ + mMacOverride = PcdGetPtr (PcdMacOverride); + + DEBUG ((DEBUG_WARN, "%a: using MAC override value %X:%X:%X:%X:%X:%X\n", + __FUNCTION__, mMacOverride[0], mMacOverride[1], mMacOverride[2], + mMacOverride[3], mMacOverride[4], mMacOverride[5] + )); + + return EfiLibInstallDriverBinding (ImageHandle, SystemTable, + &Realtek8169MacOverrideDriverBinding, NULL); +} diff --git a/Drivers/Net/Realtek8169MacOverride/Realtek8169MacOverride.inf b/Drivers/Net/Realtek8169MacOverride/Realtek8169MacOverride.inf new file mode 100644 index 000000000000..baf333562ca1 --- /dev/null +++ b/Drivers/Net/Realtek8169MacOverride/Realtek8169MacOverride.inf @@ -0,0 +1,44 @@ +## <at> file +# Component description file for Realtek 8169 MAC override driver +# +# Copyright (c) 2017, Linaro Ltd. All rights reserved.<BR> +# +# This program and the accompanying materials are licensed +# and made available under the terms and conditions of the BSD License which +# accompanies this distribution. The full text of the license may be found at +# http://opensource.org/licenses/bsd-license.php +# +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. +# +## + +[Defines] + INF_VERSION = 0x00010005 + BASE_NAME = Realtek8169MacOverride + FILE_GUID = 8d97e056-777c-4850-ab61-8166b1777f2d + MODULE_TYPE = UEFI_DRIVER + VERSION_STRING = 1.0 + ENTRY_POINT = InitializeRealtek8169MacOverride + +[Sources] + Realtek8169MacOverride.c + +[Packages] + MdeModulePkg/MdeModulePkg.dec + MdePkg/MdePkg.dec + OpenPlatformPkg/OpenPlatformPkg.dec + +[LibraryClasses] + DebugLib + DxeServicesLib + PcdLib + UefiBootServicesTableLib + UefiLib + UefiDriverEntryPoint + +[Protocols] + gEfiPciIoProtocolGuid + +[Pcd] + gOpenPlatformTokenSpaceGuid.PcdMacOverride diff --git a/OpenPlatformPkg.dec b/OpenPlatformPkg.dec index 2db143d637b3..34323bc90b9a 100644 --- a/OpenPlatformPkg.dec +++ b/OpenPlatformPkg.dec @@ -41,3 +41,6 @@ gOpenPlatformTokenSpaceGuid.PcdRamDiskMaxSize|0|UINT32|0x00000001
[PcdsFeatureFlag] + +[PcdsFixedAtBuild,PcdsDynamic] + gOpenPlatformTokenSpaceGuid.PcdMacOverride|{0x0,0x0,0x0,0x0,0x0,0x0}|VOID*|0x00000002
Deal with the missing MAC address on the Cello Realtek NIC by incorporating an alternative driver that programs a hardcoded MAC address.
The intention was to combine this driver with a PcdsDynamicHii PCD which gets set automatically from a UEFI var, but I could not get this to work, so for now, the MAC is hardcoded in the image.
E.g.,
build -p OpenPlatformPkg/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc \ -D RENESAS_XHCI_FW_DIR=~/Downloads \ -D RTK8169_MAC_OVERRIDE=0x0,0xE0,0x4C,0x97,0x3C,0xDC
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org --- Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc | 7 +++++++ Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf | 4 ++++ 2 files changed, 11 insertions(+)
diff --git a/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc b/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc index 90cda24ae49d..7e50ac179dc6 100644 --- a/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc +++ b/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc @@ -698,3 +698,10 @@ DEFINE DO_FLASHER = FALSE ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf } !endif + +!ifdef $(RTK8169_MAC_OVERRIDE) + OpenPlatformPkg/Drivers/Net/Realtek8169MacOverride/Realtek8169MacOverride.inf + +[PcdsFixedAtBuild] + gOpenPlatformTokenSpaceGuid.PcdMacOverride|{$(RTK8169_MAC_OVERRIDE)} +!endif diff --git a/Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf b/Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf index 6f7428f0c4ca..99becb445301 100644 --- a/Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf +++ b/Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf @@ -240,6 +240,10 @@ READ_LOCK_STATUS = TRUE } !endif
+!ifdef $(RTK8169_MAC_OVERRIDE) + INF OpenPlatformPkg/Drivers/Net/Realtek8169MacOverride/Realtek8169MacOverride.inf +!endif + [FV.STYX_EFI] FvAlignment = 16 ERASE_POLARITY = 1
On Tue, Jun 27, 2017 at 11:56 AM, Ard Biesheuvel ard.biesheuvel@linaro.org wrote:
Deal with the missing MAC address on the Cello Realtek NIC by incorporating an alternative driver that programs a hardcoded MAC address.
The intention was to combine this driver with a PcdsDynamicHii PCD which gets set automatically from a UEFI var, but I could not get this to work, so for now, the MAC is hardcoded in the image.
E.g.,
build -p OpenPlatformPkg/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc \ -D RENESAS_XHCI_FW_DIR=~/Downloads \ -D RTK8169_MAC_OVERRIDE=0x0,0xE0,0x4C,0x97,0x3C,0xDC
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org
Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc | 7 +++++++ Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf | 4 ++++ 2 files changed, 11 insertions(+)
diff --git a/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc b/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc index 90cda24ae49d..7e50ac179dc6 100644 --- a/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc +++ b/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc @@ -698,3 +698,10 @@ DEFINE DO_FLASHER = FALSE ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf } !endif
+!ifdef $(RTK8169_MAC_OVERRIDE)
- OpenPlatformPkg/Drivers/Net/Realtek8169MacOverride/Realtek8169MacOverride.inf
+[PcdsFixedAtBuild]
- gOpenPlatformTokenSpaceGuid.PcdMacOverride|{$(RTK8169_MAC_OVERRIDE)}
+!endif diff --git a/Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf b/Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf index 6f7428f0c4ca..99becb445301 100644 --- a/Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf +++ b/Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf @@ -240,6 +240,10 @@ READ_LOCK_STATUS = TRUE } !endif
+!ifdef $(RTK8169_MAC_OVERRIDE)
- INF OpenPlatformPkg/Drivers/Net/Realtek8169MacOverride/Realtek8169MacOverride.inf
+!endif
[FV.STYX_EFI] FvAlignment = 16 ERASE_POLARITY = 1 -- 2.9.3
This works, and I get the supplied MAC address in Linux.
If I leave off the "-D RTK8169_MAC_OVERRIDE" option, I get a build failure with: build.py... /home/rfranz/cavium/cello/firmware-build/edk2/OpenPlatformPkg/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc(706): error 3000: Invalid expression: This must be HEX value for NList or Array: [0]. gOpenPlatformTokenSpaceGuid PcdMacOverride {$(RTK8169_MAC_OVERRIDE)}
- Failed -
I see this line protected by the by the !ifdef, but that doesn't seem to be working.
Roy
On 27 June 2017 at 21:28, Roy Franz rfranz@cavium.com wrote:
On Tue, Jun 27, 2017 at 11:56 AM, Ard Biesheuvel ard.biesheuvel@linaro.org wrote:
Deal with the missing MAC address on the Cello Realtek NIC by incorporating an alternative driver that programs a hardcoded MAC address.
The intention was to combine this driver with a PcdsDynamicHii PCD which gets set automatically from a UEFI var, but I could not get this to work, so for now, the MAC is hardcoded in the image.
E.g.,
build -p OpenPlatformPkg/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc \ -D RENESAS_XHCI_FW_DIR=~/Downloads \ -D RTK8169_MAC_OVERRIDE=0x0,0xE0,0x4C,0x97,0x3C,0xDC
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org
Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc | 7 +++++++ Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf | 4 ++++ 2 files changed, 11 insertions(+)
diff --git a/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc b/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc index 90cda24ae49d..7e50ac179dc6 100644 --- a/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc +++ b/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc @@ -698,3 +698,10 @@ DEFINE DO_FLASHER = FALSE ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf } !endif
+!ifdef $(RTK8169_MAC_OVERRIDE)
- OpenPlatformPkg/Drivers/Net/Realtek8169MacOverride/Realtek8169MacOverride.inf
+[PcdsFixedAtBuild]
- gOpenPlatformTokenSpaceGuid.PcdMacOverride|{$(RTK8169_MAC_OVERRIDE)}
+!endif diff --git a/Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf b/Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf index 6f7428f0c4ca..99becb445301 100644 --- a/Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf +++ b/Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf @@ -240,6 +240,10 @@ READ_LOCK_STATUS = TRUE } !endif
+!ifdef $(RTK8169_MAC_OVERRIDE)
- INF OpenPlatformPkg/Drivers/Net/Realtek8169MacOverride/Realtek8169MacOverride.inf
+!endif
[FV.STYX_EFI] FvAlignment = 16 ERASE_POLARITY = 1 -- 2.9.3
This works, and I get the supplied MAC address in Linux.
If I leave off the "-D RTK8169_MAC_OVERRIDE" option, I get a build failure with: build.py... /home/rfranz/cavium/cello/firmware-build/edk2/OpenPlatformPkg/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc(706): error 3000: Invalid expression: This must be HEX value for NList or Array: [0]. gOpenPlatformTokenSpaceGuid PcdMacOverride {$(RTK8169_MAC_OVERRIDE)}
- Failed -
I see this line protected by the by the !ifdef, but that doesn't seem to be working.
How annoying :-(
I should be able to work around that quite easily, but annoying nonetheless...
On Tue, Jun 27, 2017 at 06:56:17PM +0000, Ard Biesheuvel wrote:
Given that we're unlikely to ever see Cellos with the MAC programmed correctly, implement a driver that programs a MAC in a volatile manner instead. This still does not allow us to boot from the network, but at least we no longer have to care about this at the OS level.
For OpenPlatformPkg: Reviewed-by: Leif Lindholm leif.lindholm@linaro.org
But ... this does not fit well with the intent for edk2-platforms. So I can't really migrate this across. Hopefully, we can reach some sort of understanding over this sort of not-quite-drivers (like the Renesas USB firmware loader) so that we can get them into edk2. I'll bring it up at the next stewards sync, next week.
/ Leif
Ard Biesheuvel (2): Drivers/Net: add MAC override driver for Realtek 8169 Platforms/AMD/Cello: add Realtek MAC override driver
Drivers/Net/Realtek8169MacOverride/Realtek8169MacOverride.c | 262 ++++++++++++++++++++ Drivers/Net/Realtek8169MacOverride/Realtek8169MacOverride.inf | 44 ++++ OpenPlatformPkg.dec | 3 + Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc | 7 + Platforms/AMD/Styx/CelloBoard/CelloBoard.fdf | 4 + 5 files changed, 320 insertions(+) create mode 100644 Drivers/Net/Realtek8169MacOverride/Realtek8169MacOverride.c create mode 100644 Drivers/Net/Realtek8169MacOverride/Realtek8169MacOverride.inf
-- 2.9.3
On 27 June 2017 at 19:42, Leif Lindholm leif.lindholm@linaro.org wrote:
On Tue, Jun 27, 2017 at 06:56:17PM +0000, Ard Biesheuvel wrote:
Given that we're unlikely to ever see Cellos with the MAC programmed correctly, implement a driver that programs a MAC in a volatile manner instead. This still does not allow us to boot from the network, but at least we no longer have to care about this at the OS level.
For OpenPlatformPkg: Reviewed-by: Leif Lindholm leif.lindholm@linaro.org
But ... this does not fit well with the intent for edk2-platforms. So I can't really migrate this across. Hopefully, we can reach some sort of understanding over this sort of not-quite-drivers (like the Renesas USB firmware loader) so that we can get them into edk2. I'll bring it up at the next stewards sync, next week.
Well, given how both these drivers are tightly coupled to Cello*, I'd be perfectly happy making them a part of the platform in this case.
* the renesas driver requires an FV section with a certain GUID to be present, and this driver depends on a PCD, which in both cases means that the drivers cannot be built in a generic way anyway
On Tue, Jun 27, 2017 at 08:06:58PM +0000, Ard Biesheuvel wrote:
On 27 June 2017 at 19:42, Leif Lindholm leif.lindholm@linaro.org wrote:
On Tue, Jun 27, 2017 at 06:56:17PM +0000, Ard Biesheuvel wrote:
Given that we're unlikely to ever see Cellos with the MAC programmed correctly, implement a driver that programs a MAC in a volatile manner instead. This still does not allow us to boot from the network, but at least we no longer have to care about this at the OS level.
For OpenPlatformPkg: Reviewed-by: Leif Lindholm leif.lindholm@linaro.org
But ... this does not fit well with the intent for edk2-platforms. So I can't really migrate this across. Hopefully, we can reach some sort of understanding over this sort of not-quite-drivers (like the Renesas USB firmware loader) so that we can get them into edk2. I'll bring it up at the next stewards sync, next week.
Well, given how both these drivers are tightly coupled to Cello*, I'd be perfectly happy making them a part of the platform in this case.
Well, they're not really. Sure, that may be the only platform we have in the tree currently using it, but the minnowboard max uses the same Realtek controller (only it currently requires a separate binary driver, and probably actually has a MAC address programmed).
- the renesas driver requires an FV section with a certain GUID to be
present, and this driver depends on a PCD, which in both cases means that the drivers cannot be built in a generic way anyway
Both that GUID and that PCD can live just as happily in an OptionRomPkg/ (or Drivers/) .dec as they can in an OpenPlatformPkg one. Or am I missing something?
If we have something that is truly a one-off (or superseded), I agree it makes sense to integrate it into the platform code. But as far as I know, both of those components are still on the market, and some manufacturers still fail to program on-device config areas.
/ Leif
On 28 June 2017 at 13:24, Leif Lindholm leif.lindholm@linaro.org wrote:
On Tue, Jun 27, 2017 at 08:06:58PM +0000, Ard Biesheuvel wrote:
On 27 June 2017 at 19:42, Leif Lindholm leif.lindholm@linaro.org wrote:
On Tue, Jun 27, 2017 at 06:56:17PM +0000, Ard Biesheuvel wrote:
Given that we're unlikely to ever see Cellos with the MAC programmed correctly, implement a driver that programs a MAC in a volatile manner instead. This still does not allow us to boot from the network, but at least we no longer have to care about this at the OS level.
For OpenPlatformPkg: Reviewed-by: Leif Lindholm leif.lindholm@linaro.org
But ... this does not fit well with the intent for edk2-platforms. So I can't really migrate this across. Hopefully, we can reach some sort of understanding over this sort of not-quite-drivers (like the Renesas USB firmware loader) so that we can get them into edk2. I'll bring it up at the next stewards sync, next week.
Well, given how both these drivers are tightly coupled to Cello*, I'd be perfectly happy making them a part of the platform in this case.
Well, they're not really. Sure, that may be the only platform we have in the tree currently using it, but the minnowboard max uses the same Realtek controller (only it currently requires a separate binary driver, and probably actually has a MAC address programmed).
- the renesas driver requires an FV section with a certain GUID to be
present, and this driver depends on a PCD, which in both cases means that the drivers cannot be built in a generic way anyway
Both that GUID and that PCD can live just as happily in an OptionRomPkg/ (or Drivers/) .dec as they can in an OpenPlatformPkg one. Or am I missing something?
True. My point was rather that you can't build this driver from, say, EmbeddedPkg/EmbeddedPkg.dsc in a generic manner, and deploy it as a standalone binary.
If we have something that is truly a one-off (or superseded), I agree it makes sense to integrate it into the platform code. But as far as I know, both of those components are still on the market, and some manufacturers still fail to program on-device config areas.
I will let you fight this battle, if you don't mind. I do agree with the reasoning, but it is not something I care deeply about, and I sure as hell don't intend to get involved with other boards that use these parts. The devboard market is not cost driven, so it makes no sense whatsoever to get the cheapest variant of the part, like they did with the XHCI controller. And for an on-board PCI NIC, we're better off using a Marvell Yukon like SoftIron did.
On Wed, Jun 28, 2017 at 09:12:04PM +0000, Ard Biesheuvel wrote:
Well, given how both these drivers are tightly coupled to Cello*, I'd be perfectly happy making them a part of the platform in this case.
Well, they're not really. Sure, that may be the only platform we have in the tree currently using it, but the minnowboard max uses the same Realtek controller (only it currently requires a separate binary driver, and probably actually has a MAC address programmed).
- the renesas driver requires an FV section with a certain GUID to be
present, and this driver depends on a PCD, which in both cases means that the drivers cannot be built in a generic way anyway
Both that GUID and that PCD can live just as happily in an OptionRomPkg/ (or Drivers/) .dec as they can in an OpenPlatformPkg one. Or am I missing something?
True. My point was rather that you can't build this driver from, say, EmbeddedPkg/EmbeddedPkg.dsc in a generic manner, and deploy it as a standalone binary.
Oh, indeed. Hmm. Is there any way to express such a thing? Dummy Pcd dependency or Depex or something?
If we have something that is truly a one-off (or superseded), I agree it makes sense to integrate it into the platform code. But as far as I know, both of those components are still on the market, and some manufacturers still fail to program on-device config areas.
I will let you fight this battle, if you don't mind. I do agree with the reasoning, but it is not something I care deeply about, and I sure as hell don't intend to get involved with other boards that use these parts.
Yeah, this one is mine. I just don't see this kind of thing not happening again (and again), so having a precedent with regards to how we upstream it would be useful.
The devboard market is not cost driven, so it makes no sense whatsoever to get the cheapest variant of the part, like they did with the XHCI controller. And for an on-board PCI NIC, we're better off using a Marvell Yukon like SoftIron did.
/ Leif
On 28 June 2017 at 21:48, Leif Lindholm leif.lindholm@linaro.org wrote:
On Wed, Jun 28, 2017 at 09:12:04PM +0000, Ard Biesheuvel wrote:
Well, given how both these drivers are tightly coupled to Cello*, I'd be perfectly happy making them a part of the platform in this case.
Well, they're not really. Sure, that may be the only platform we have in the tree currently using it, but the minnowboard max uses the same Realtek controller (only it currently requires a separate binary driver, and probably actually has a MAC address programmed).
- the renesas driver requires an FV section with a certain GUID to be
present, and this driver depends on a PCD, which in both cases means that the drivers cannot be built in a generic way anyway
Both that GUID and that PCD can live just as happily in an OptionRomPkg/ (or Drivers/) .dec as they can in an OpenPlatformPkg one. Or am I missing something?
True. My point was rather that you can't build this driver from, say, EmbeddedPkg/EmbeddedPkg.dsc in a generic manner, and deploy it as a standalone binary.
Oh, indeed. Hmm. Is there any way to express such a thing? Dummy Pcd dependency or Depex or something?
I don't think so. PCDs simply assume their default values if the DSC does not specify a value for them.
If we have something that is truly a one-off (or superseded), I agree it makes sense to integrate it into the platform code. But as far as I know, both of those components are still on the market, and some manufacturers still fail to program on-device config areas.
I will let you fight this battle, if you don't mind. I do agree with the reasoning, but it is not something I care deeply about, and I sure as hell don't intend to get involved with other boards that use these parts.
Yeah, this one is mine. I just don't see this kind of thing not happening again (and again), so having a precedent with regards to how we upstream it would be useful.
The devboard market is not cost driven, so it makes no sense whatsoever to get the cheapest variant of the part, like they did with the XHCI controller. And for an on-board PCI NIC, we're better off using a Marvell Yukon like SoftIron did.
/ Leif
-
Ard Biesheuvel -
Leif Lindholm -
Roy Franz