[PATCH v3] PCI/ACPI: xgene: Add ECAM quirk for X-Gene PCIe controller

On 12/01/2016 02:20 PM, Mark Salter wrote:

On Thu, 2016-12-01 at 12:33 -0600, Bjorn Helgaas wrote:

quoted

quoted

+	csr = &xgene_v1_csr_res[root->segment];

This makes me nervous because root->segment comes from the ACPI _SEG,
and if firmware gives us junk in _SEG, we will reference something in
the weeds.

The SoC provide some number of RC bridges, each with a different base
for some mmio registers. Even if segment is legitimate in MCFG, there
is still a problem if a platform doesn't use the segment ordering
implied by the code. But the PNP0A03 _CRS does have this base address
as the first memory resource, so we could get it from there and not
have hard-coded addresses and implied ording in the quirk code.

I have tested a modified version of these quirks using this to
get the CSR base and it works on the 3 different platforms I have
access to.

static int xgene_pcie_get_csr(struct device *dev, struct resource *r)
{
	struct acpi_device *adev = to_acpi_device(dev);
	unsigned long flags;
	struct list_head list;
	struct resource_entry *entry;
	int ret;

	INIT_LIST_HEAD(&list);
	flags = IORESOURCE_MEM;
	ret = acpi_dev_get_resources(adev, &list,
				     acpi_dev_filter_resource_type_cb,
				     (void *)flags);
	if (ret < 0) {
		dev_err(dev, "failed to parse _CRS, error: %d\n", ret);
		return ret;
	} else if (ret == 0) {
		dev_err(dev, "no memory resources present in _CRS\n");
		return -EINVAL;
	}

	entry = list_first_entry(&list, struct resource_entry, node);
	*r = *entry->res;
	acpi_dev_free_resource_list(&list);
	return 0;
}

This seems a lot safer. At some point trusting firmware to provide the
correct _CRS for the RC in use is better than hard coding for every
possible implementation configuration of an X-Gene SoC.

Jon.

-- 
Computer Architect | Sent from my Fedora powered laptop