Messages in this thread | | | Date | Mon, 30 Aug 2021 09:27:26 -0700 | Subject | Re: [PATCH v3 2/4] PCI: brcmstb: Add ACPI config space quirk | From | Florian Fainelli <> |
| |
On 8/30/2021 9:23 AM, Jeremy Linton wrote: > Hi, > > On 8/30/21 3:36 AM, nicolas saenz julienne wrote: >> Hi Jeremy, >> sorry for the late reply, I've been on vacation. >> >> On Thu, 2021-08-26 at 02:15 -0500, Jeremy Linton wrote: >> >> [...] >> >>> +static void __iomem *brcm_pcie_map_conf2(struct pci_bus *bus, >>> + unsigned int devfn, int where) >>> +{ >>> + struct pci_config_window *cfg = bus->sysdata; >>> + void __iomem *base = cfg->win; >>> + int idx; >>> + u32 up; >>> + >>> + /* Accesses to the RC go right to the RC registers if slot==0 */ >>> + if (pci_is_root_bus(bus)) >>> + return PCI_SLOT(devfn) ? NULL : base + where; >>> + >>> + /* >>> + * Assure the link is up before sending requests downstream. >>> This is done >>> + * to avoid sending transactions to EPs that don't exist. Link flap >>> + * conditions/etc make this race more probable. The resulting >>> unrecoverable >>> + * SERRORs will result in the machine crashing. >>> + */ >>> + up = readl(base + PCIE_MISC_PCIE_STATUS); >>> + if (!(up & PCIE_MISC_PCIE_STATUS_PCIE_DL_ACTIVE_MASK)) >>> + return NULL; >>> + >>> + if (!(up & PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_MASK)) >>> + return NULL; >> >> Couldn't this be integrated in the original brcm_pcie_map_conf()? IIUC >> there is >> nothing ACPI specific about it. It'd also make for less code duplication. > > That is where I started with this, but it wasn't the linkup check/etc > which caused me to hoist it but the fact that if ACPI quirks are enabled > they end up statically built into the kernel. While if this host bridge > is enabled, it can end up being a module, and the resulting mess I > created trying to satisfy the CONFIG variations. I'm not much of a fan > of copy/paste programming, but that IMHO ended up being the cleanest here. >
Agreed, the open coding that is being done is reasonable IHMO, although we may have to update the link up code in both pcie-brcmstb.c and this file in the future if offsets/bits do change, nothing impossible though. -- Florian
| |