From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: jive.eu, ip: 192.87.1.114, mailfrom: kettenis@jive.eu) Received: from smtp1.astron.nl (smtp1.astron.nl [192.87.1.114]) by groups.io with SMTP; Thu, 23 May 2019 11:02:07 -0700 Received: from jop94 ([10.87.10.94]) by smtp1.astron.nl with ESMTP (TLS encrypted); Thu, 23 May 2019 20:01:55 +0200 Date: Thu, 23 May 2019 20:01:58 +0200 Message-Id: From: kettenis@jive.eu To: Leif Lindholm CC: mw@semihalf.com, devel@edk2.groups.io, ard.biesheuvel@linaro.org, jsd@semihalf.com, jaz@semihalf.com, kostap@marvell.com, Jici.Gao@arm.com, rebecca@bluestop.org, kettenis@openbsd.org In-reply-to: <20190523141426.rvuqh2nfe3vepm4p@bivouac.eciton.net> (message from Leif Lindholm on Thu, 23 May 2019 15:14:27 +0100) Subject: Re: [edk2-platforms: PATCH v2 00/14] Armada7k8k PCIE support References: <1558366047-15994-1-git-send-email-mw@semihalf.com> <20190523141426.rvuqh2nfe3vepm4p@bivouac.eciton.net> > Date: Thu, 23 May 2019 15:14:27 +0100 > From: Leif Lindholm > > On Thu, May 23, 2019 at 03:27:47PM +0200, Mark Kettenis wrote: > > > From: Marcin Wojtas > > > Date: Mon, 20 May 2019 17:27:13 +0200 > > > > > > Hi, > > > > > > Thank you for thorough review of v1. I submit second > > > version of the Armada7k8k PCIE support. I addressed > > > all comments. There is no functional change to initial > > > patchset, but mostly clean-up and improvements - please > > > refer to the changelog below. > > > > > > The patches are available in the github: > > > https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/pcie-upstream-r20190520 > > > > > > I'm looking forward to your comments or remarks. > > > > > > Best regards, > > > Marcin > > > > Tested this on my mcbin running OpenBSD. It incovers a small issue in > > our kernel which I'm fixing. Otherwise this seems to work fine. > > > > So tested-by: Mark Kettenis if that matters. > > Always helpful, thanks. > > Out of interest, what was the issue? > Could I still expect the 6.5 installer to run on this > hardware/firmware combo? That should still work fine. The issue is with mapping PCI ROMS, which currently fails with the new firmware due to an oversight in the OpenBSD code. There are only a few OpenBSD drivers that attempt to map the PCI ROM. But one of those is radeondrm(4) and I stuck an AMD graphics card into the PCIe slot on my machine. There is a bit of a firmware angle to this though. The issue happens because the PCI ROM address register has been set to 0xfffe0000. All the writable address bits in the register are set to 1. While it is possible that the hardware comes up in that state, I suspect this is done by an attempt by the firmware to determine the size of the ROM that doesn't properly restore the original contents of the register. It may be related to the following messages that are printed by the firmware: Image type X64 can't be loaded on AARCH64 UEFI system. Unloading driver at 0x00000000000 Connect: PcieRoot(0x0)/Pci(0x0,0x0): Not Found Hmm, that's actually interesting. Maybe I should play with the X86 emulator that Ard added recently to see if that gives me a framebuffer console. Cheers, Mark > > Cheers, > > > > Mark > > > > > Changelog: > > > v1->v2: > > > *All > > > - s/PcieBaseAddress/PcieDbiAdress/ > > > > > > *2/14 > > > - fix alignment in comment > > > > > > * 3/14 > > > - add CONST** to library callback > > > > > > * 4/14 > > > - add missing reset GPIO to McBin description > > > > > > * 5/15 > > > - add CONST** to protocol callback > > > > > > * 6/14 > > > - cleanup all casting in file > > > - use MAX_UINTx macros > > > - add Linaro copyright > > > - use MmioWrite8 instead of volatile in PciExpressReadBuffer > > > - correct commient in IgnoreBusDeviceFunction () > > > - fix typo in commit message > > > > > > * 7/10 > > > - correct line endings > > > - use temporary variable for memory description in PciHostBridgeResourceConflict > > > - use MAX_UINTx macros > > > - add comments around stalls and MemoryFence in GPIO reset > > > - keep the reset active for 150ms > > > - assign translation values instead of asserting > > > > > > *8/14 > > > - assign gArmTokenSpaceGuid.PcdPciIoTranslation value in .dsc > > > > > > * 9-11/14 > > > - correct line endings > > > - remove unused methods > > > - extend commit messages with 32k shift description > > > > > > Ard Biesheuvel (1): > > > Marvell/Armada7k8k: Add PciExpressLib implementation > > > > > > Marcin Wojtas (13): > > > Marvell/Library: MvGpioLib: Extend GPIO pin description > > > Marvell/Library: ArmadaSoCDescLib: Add PCIE information > > > Marvell/Library: ArmadaBoardDescLib: Add PCIE information > > > Marvell/Armada7k8k: Extend board description libraries with PCIE > > > Marvell/Armada7k8k: MvBoardDesc: Extend protocol with PCIE support > > > Marvell/Armada7k8k: Implement PciHostBridgeLib > > > Marvell/Armada7k8k: Enable PCIE support > > > Marvell/Armada80x0McBin: Enable ACPI PCIE support > > > Marvell/Armada80x0Db: Enable ACPI PCIE support > > > Marvell/Armada70x0Db: Enable ACPI PCIE support > > > Marvell/Armada80x0McBin: DeviceTree: Use pci-host-generic driver > > > Marvell/Armada7k8k: Remove duplication in .dsc files > > > Marvell/Armada7k8: Add 'acpiview' shell command to build > > > > > > Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 19 +- > > > Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc | 4 +- > > > Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc | 4 +- > > > Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc | 4 +- > > > Silicon/Marvell/Armada7k8k/Armada7k8k.fdf | 5 + > > > Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db.inf | 1 + > > > Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0Db.inf | 1 + > > > Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0McBin.inf | 1 + > > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciExpressLib/PciExpressLib.inf | 42 + > > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciHostBridgeLib/PciHostBridgeLib.inf | 52 + > > > Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Pcie.h | 26 + > > > Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0Db/Pcie.h | 26 + > > > Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0McBin/Pcie.h | 26 + > > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciHostBridgeLib/PciHostBridgeLibConstructor.h | 95 ++ > > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h | 6 + > > > Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h | 46 + > > > Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h | 20 + > > > Silicon/Marvell/Include/Library/MvGpioLib.h | 1 + > > > Silicon/Marvell/Include/Protocol/BoardDesc.h | 22 + > > > Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.c | 48 + > > > Platform/Marvell/Armada70x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.c | 4 + > > > Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.c | 48 + > > > Platform/Marvell/Armada80x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.c | 6 + > > > Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.c | 54 + > > > Platform/SolidRun/Armada80x0McBin/NonDiscoverableInitLib/NonDiscoverableInitLib.c | 1 + > > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciExpressLib/PciExpressLib.c | 1531 ++++++++++++++++++++ > > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciHostBridgeLib/PciHostBridgeLib.c | 265 ++++ > > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciHostBridgeLib/PciHostBridgeLibConstructor.c | 345 +++++ > > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c | 44 + > > > Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c | 86 ++ > > > Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Dsdt.asl | 108 ++ > > > Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Mcfg.aslc | 47 + > > > Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0Db/Dsdt.asl | 108 ++ > > > Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0Db/Mcfg.aslc | 47 + > > > Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0McBin/Dsdt.asl | 108 ++ > > > Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0McBin/Mcfg.aslc | 47 + > > > Silicon/Marvell/Armada7k8k/DeviceTree/armada-8040-mcbin.dts | 3 + > > > 37 files changed, 3290 insertions(+), 11 deletions(-) > > > create mode 100644 Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciExpressLib/PciExpressLib.inf > > > create mode 100644 Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciHostBridgeLib/PciHostBridgeLib.inf > > > create mode 100644 Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Pcie.h > > > create mode 100644 Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0Db/Pcie.h > > > create mode 100644 Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0McBin/Pcie.h > > > create mode 100644 Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciHostBridgeLib/PciHostBridgeLibConstructor.h > > > create mode 100644 Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciExpressLib/PciExpressLib.c > > > create mode 100644 Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciHostBridgeLib/PciHostBridgeLib.c > > > create mode 100644 Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciHostBridgeLib/PciHostBridgeLibConstructor.c > > > create mode 100644 Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Mcfg.aslc > > > create mode 100644 Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0Db/Mcfg.aslc > > > create mode 100644 Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0McBin/Mcfg.aslc > > > > > > -- > > > 2.7.4 > > > >