git: 2de4c7f6d087 - main - pci_host_generic: Add Synopsys Designware PCIe controller quirk

[Jessica Clarke]

On 15 Sep 2021, at 14:18, Marcin Wojtas <mw at FreeBSD.org> wrote:
> 
> The branch main has been updated by mw:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=2de4c7f6d08798fb6269582907155703d1ab5ef4
> 
> commit 2de4c7f6d08798fb6269582907155703d1ab5ef4
> Author:     Pawel Anikiel <pan at semihalf.com>
> AuthorDate: 2021-09-13 14:59:40 +0000
> Commit:     Marcin Wojtas <mw at FreeBSD.org>
> CommitDate: 2021-09-15 13:17:40 +0000
> 
>    pci_host_generic: Add Synopsys Designware PCIe controller quirk
> 
>    Due to the quirky nature of the Synopsys Designware PCIe IP,
>    the type 0 configuration is broadcast and whatever device
>    is plugged into slot, will appear at each 32 device
>    positions of bus0. Mitigate the issue by filtering out
>    duplicated devices on this bus for both DT and ACPI cases.
> 
>    Reviewed by: mw
>    Sponsored by: Semihalf
>    MFC: after 3 weeks
>    Differential revision: https://reviews.freebsd.org/D31887
> ---
> sys/dev/pci/pci_host_generic.c      |  2 ++
> sys/dev/pci/pci_host_generic.h      |  4 ++++
> sys/dev/pci/pci_host_generic_acpi.c | 33 +++++++++++++++++++++++++++++++++
> sys/dev/pci/pci_host_generic_fdt.c  |  7 +++++++
> 4 files changed, 46 insertions(+)
> 
> diff --git a/sys/dev/pci/pci_host_generic.c b/sys/dev/pci/pci_host_generic.c
> index 0c45f5d316ed..22b3ccdc17b1 100644
> --- a/sys/dev/pci/pci_host_generic.c
> +++ b/sys/dev/pci/pci_host_generic.c
> @@ -185,6 +185,8 @@ generic_pcie_read_config(device_t dev, u_int bus, u_int slot,
> 	if ((slot > PCI_SLOTMAX) || (func > PCI_FUNCMAX) ||
> 	    (reg > PCIE_REGMAX))
> 		return (~0U);
> +	if ((sc->quirks & PCIE_ECAM_DESIGNWARE_QUIRK) && bus == 0 && slot > 0)
> +		return (~0U);
> 
> 	offset = PCIE_ADDR_OFFSET(bus - sc->bus_start, slot, func, reg);
> 	t = sc->bst;
> diff --git a/sys/dev/pci/pci_host_generic.h b/sys/dev/pci/pci_host_generic.h
> index 36a12b9559ba..20117cbe32e3 100644
> --- a/sys/dev/pci/pci_host_generic.h
> +++ b/sys/dev/pci/pci_host_generic.h
> @@ -85,8 +85,12 @@ struct generic_pcie_core_softc {
> 	device_t		dev;
> 	bus_space_handle_t	ioh;
> 	bus_dma_tag_t		dmat;
> +	uint32_t		quirks;
> };
> 
> +/* Quirks */
> +#define PCIE_ECAM_DESIGNWARE_QUIRK	(1 << 0)
> +
> DECLARE_CLASS(generic_pcie_core_driver);
> 
> int pci_host_generic_core_attach(device_t);
> diff --git a/sys/dev/pci/pci_host_generic_acpi.c b/sys/dev/pci/pci_host_generic_acpi.c
> index 763a84d2fd53..3c32abc5007a 100644
> --- a/sys/dev/pci/pci_host_generic_acpi.c
> +++ b/sys/dev/pci/pci_host_generic_acpi.c
> @@ -89,6 +89,21 @@ __FBSDID("$FreeBSD$");
> #define	PROPS_CELL_SIZE		1
> #define	PCI_ADDR_CELL_SIZE	2
> 
> +static struct {
> +	char		oem_id[ACPI_OEM_ID_SIZE + 1];
> +	char		oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
> +	uint32_t	quirks;
> +} pci_acpi_quirks[] = {
> +	{ "MRVL  ", "CN9130  ", PCIE_ECAM_DESIGNWARE_QUIRK },
> +	{ "MRVL  ", "CN913X  ", PCIE_ECAM_DESIGNWARE_QUIRK },
> +	{ "MVEBU ", "ARMADA7K", PCIE_ECAM_DESIGNWARE_QUIRK },
> +	{ "MVEBU ", "ARMADA8K", PCIE_ECAM_DESIGNWARE_QUIRK },
> +	{ "MVEBU ", "CN9130  ", PCIE_ECAM_DESIGNWARE_QUIRK },
> +	{ "MVEBU ", "CN9131  ", PCIE_ECAM_DESIGNWARE_QUIRK },
> +	{ "MVEBU ", "CN9132  ", PCIE_ECAM_DESIGNWARE_QUIRK },
> +	{ 0 },
> +};
> +
> /* Forward prototypes */
> 
> static int generic_pcie_acpi_probe(device_t dev);
> @@ -170,6 +185,23 @@ pci_host_generic_acpi_parse_resource(ACPI_RESOURCE *res, void *arg)
> 	return (AE_OK);
> }
> 
> +static void
> +pci_host_acpi_get_oem_quirks(struct generic_pcie_acpi_softc *sc,
> +    ACPI_TABLE_HEADER *hdr)
> +{
> +	int i;
> +
> +	for (i = 0; pci_acpi_quirks[i].quirks; i++) {
> +		if (memcmp(hdr->OemId, pci_acpi_quirks[i].oem_id,
> +		    ACPI_OEM_ID_SIZE) != 0)
> +			continue;
> +		if (memcmp(hdr->OemTableId, pci_acpi_quirks[i].oem_table_id,
> +		    ACPI_OEM_TABLE_ID_SIZE) != 0)
> +			continue;
> +		sc->base.quirks |= pci_acpi_quirks[i].quirks;
> +	}
> +}
> +
> static int
> pci_host_acpi_get_ecam_resource(device_t dev)
> {
> @@ -209,6 +241,7 @@ pci_host_acpi_get_ecam_resource(device_t dev)
> 			    sc->base.bus_start, sc->base.bus_end);
> 			return (ENXIO);
> 		}
> +		pci_host_acpi_get_oem_quirks(sc, hdr);
> 	} else {
> 		status = acpi_GetInteger(handle, "_CBA", &val);
> 		if (ACPI_SUCCESS(status))
> diff --git a/sys/dev/pci/pci_host_generic_fdt.c b/sys/dev/pci/pci_host_generic_fdt.c
> index 91ffaf7357b9..249637019137 100644
> --- a/sys/dev/pci/pci_host_generic_fdt.c
> +++ b/sys/dev/pci/pci_host_generic_fdt.c
> @@ -151,6 +151,13 @@ pci_host_generic_setup_fdt(device_t dev)
> 	if (error != 0)
> 		return (error);
> 
> +	if (ofw_bus_is_compatible(dev, "marvell,armada8k-pcie-ecam") ||
> +	    ofw_bus_is_compatible(dev, "socionext,synquacer-pcie-ecam") ||
> +	    ofw_bus_is_compatible(dev, "snps,dw-pcie-ecam")) {
> +		device_set_desc(dev, "Synopsys DesignWare PCIe Controller");

It seems inconsistent to set this for _fdt but not _acpi?

Jess