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

[Marcin Wojtas]

śr., 15 wrz 2021 o 15:21 Jessica Clarke <jrtc27 at freebsd.org> napisał(a):
>
> 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?
>

Indeed, thanks for noticing. I'll commit an update right away.

Thanks,
Marcin