Date: Thu, 16 Sep 2021 16:42:45 +0200 From: Marcin Wojtas <mw@semihalf.com> To: Jessica Clarke <jrtc27@freebsd.org> Cc: Marcin Wojtas <mw@freebsd.org>, "src-committers@freebsd.org" <src-committers@freebsd.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@freebsd.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@freebsd.org> Subject: Re: git: 2de4c7f6d087 - main - pci_host_generic: Add Synopsys Designware PCIe controller quirk Message-ID: <CAPv3WKcn8z69SyAD3FCx%2BHrL-cbkTYUBXpWs=FJt8wavCyCqiQ@mail.gmail.com> In-Reply-To: <AFE0B4C2-5479-46B9-BA53-3FC7BBF3B025@freebsd.org> References: <202109151318.18FDIppM059878@gitrepo.freebsd.org> <AFE0B4C2-5479-46B9-BA53-3FC7BBF3B025@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
=C5=9Br., 15 wrz 2021 o 15:21 Jessica Clarke <jrtc27@freebsd.org> napisa=C5= =82(a): > > On 15 Sep 2021, at 14:18, Marcin Wojtas <mw@FreeBSD.org> wrote: > > > > The branch main has been updated by mw: > > > > URL: https://cgit.FreeBSD.org/src/commit/?id=3D2de4c7f6d08798fb62695829= 07155703d1ab5ef4 > > > > commit 2de4c7f6d08798fb6269582907155703d1ab5ef4 > > Author: Pawel Anikiel <pan@semihalf.com> > > AuthorDate: 2021-09-13 14:59:40 +0000 > > Commit: Marcin Wojtas <mw@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_gene= ric.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 =3D=3D 0 && = slot > 0) > > + return (~0U); > > > > offset =3D PCIE_ADDR_OFFSET(bus - sc->bus_start, slot, func, reg)= ; > > t =3D sc->bst; > > diff --git a/sys/dev/pci/pci_host_generic.h b/sys/dev/pci/pci_host_gene= ric.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[] =3D { > > + { "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 =3D 0; pci_acpi_quirks[i].quirks; i++) { > > + if (memcmp(hdr->OemId, pci_acpi_quirks[i].oem_id, > > + ACPI_OEM_ID_SIZE) !=3D 0) > > + continue; > > + if (memcmp(hdr->OemTableId, pci_acpi_quirks[i].oem_table_= id, > > + ACPI_OEM_TABLE_ID_SIZE) !=3D 0) > > + continue; > > + sc->base.quirks |=3D 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 =3D 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 !=3D 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? > Indeed, thanks for noticing. I'll commit an update right away. Thanks, Marcin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAPv3WKcn8z69SyAD3FCx%2BHrL-cbkTYUBXpWs=FJt8wavCyCqiQ>