This patch contains seperate driver for ThunderX2 PCIe, which contains necesseary fixups (eg. AHCI BAR fixup)
Details
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
You should restructure the pci_host_generic_acpi code to handle this so you don't need to copy and paste so much of it in this driver, or even better handle ThunderX2 in the existing driver with quirks.
sys/arm64/cavium/thunderx2_pcie_acpi.c | ||
---|---|---|
2 ↗ | (On Diff #41680) | There is code below that looks like it was copied from an existing driver, however you didn't include the copyright. |
243 ↗ | (On Diff #41680) | Why? |
It is good idea to provide support using quirks, however pci_host_generic_acpi still needs some resource allocation changes. I have limited ability to check if it doesn't break another devices, so from my point of view it is safe to provide separate driver.
Of course I will follow your advices, but I'm depending on you to perform review very carefully.
sys/arm64/cavium/thunderx2_pcie_acpi.c | ||
---|---|---|
2 ↗ | (On Diff #41680) | Just an oversight |
243 ↗ | (On Diff #41680) | There is no SATA connectors on board, it requires bar fixing too. |
Hi, if we limit usecase to the the explicit quirks and enable proper host ecam generic operation, we can test solution on Marvell Armada 7k/8k as well (it works without quirks, when booting with ACPI).
This is not final version!
I've uploaded this so I can get some feedback from you
Changes:
- pci_host_generic_acpi adapted to work with ThunderX2
- Added quirk mechanism
It works on ThunderX2 but I think it needs to replace memory regions adding (in attach) with something more flexible
sys/dev/acpica/acpi.c | ||
---|---|---|
2096 ↗ | (On Diff #41814) | It is still not clear to me why this code is here rather than in the attach routine of the relevant bridge driver. |
2105 ↗ | (On Diff #41814) | Why is this moved? |
sys/dev/pci/pci_host_generic_acpi.c | ||
171 ↗ | (On Diff #41814) | Are these encoded as Producer resources instead of Consumer resources? |
sys/dev/acpica/acpi.c | ||
---|---|---|
2096 ↗ | (On Diff #41814) | I haven't written this code, but pci_host_generic (for fdt and acpi) depends on resource which is set by this code. I think one wanted to keep it symetric with fdt. I think it is possible to move it to attach routine. |
2105 ↗ | (On Diff #41814) | Because acpi_enable_pcie always overwrite first resource (rid 0), so if we call acpi_parse_resources earlier we lose 1 resource. |
sys/dev/pci/pci_host_generic_acpi.c | ||
171 ↗ | (On Diff #41814) | They are encoded as Producer resources. |
sys/dev/acpica/acpi.c | ||
---|---|---|
2096 ↗ | (On Diff #41814) | I've checked and it can't be moved to attach routine because in attach we can't edit our resources. |
sys/arm64/include/cpu.h | ||
---|---|---|
95 ↗ | (On Diff #43643) | Can you split this out into a new review, and include the needed updates to identcpu.c? I have moved some of these macros around so you'll also need to rebase. |
sys/dev/pci/pci_host_generic_acpi.c | ||
126–131 ↗ | (On Diff #43643) | Can you split the quirks out into a new review? It is mostly separate from the other changes. |
447–449 ↗ | (On Diff #43643) | Is there an erratum for this? I don't see anything in Linux, or my copy of the ThunderX2 known issues document. |
No functional changes. This is the previous patch with quirks and macros moved, as andrew suggested
Links to patches emerged
Macros: https://reviews.freebsd.org/D15928
Quirks: https://reviews.freebsd.org/D15929
sys/dev/pci/pci_host_generic.c | ||
---|---|---|
229 ↗ | (On Diff #44205) | Every 0x80 we have _BBN number of root complex. I can create analogue function in pci_host_generic_acpi, and obtain value directly from acpi |
sys/dev/pci/pci_host_generic_acpi.c | ||
213 ↗ | (On Diff #44205) | In pci_host_generic_acpi_parse_resource we add memory regions to rman. Here we are allocating memory from it. |
Added obtaining ecam and bus number from ACPI. This allows us to remove ThunderX2 quirk connected with ecam.