Date: Mon, 9 Jul 2018 17:00:48 +0100 From: Andrew Turner <andrew@fubar.geek.nz> To: Wojciech Macek <wma@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r336130 - head/sys/dev/pci Message-ID: <A92C6AB0-81A3-4A90-91BF-60C8AC28D3FC@fubar.geek.nz> In-Reply-To: <201807090900.w6990GCZ081981@repo.freebsd.org> References: <201807090900.w6990GCZ081981@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
This breaks at least armv7. Please either fix, or revert this. I have some comments below I was planning on adding to the review, but = you committed before I had the time to do so. > On 9 Jul 2018, at 10:00, Wojciech Macek <wma@freebsd.org> wrote: >=20 > Author: wma > Date: Mon Jul 9 09:00:16 2018 > New Revision: 336130 > URL: https://svnweb.freebsd.org/changeset/base/336130 >=20 > Log: > ARM64: Add quirk mechanism to pci_host_generic_acpi >=20 > Add few quirks which are necessary to use AHCI on ThX2 >=20 > Submitted by: Patryk Duda <pdk@semihalf.com> > Obtained from: Semihalf > Sponsored by: Cavium > Differential revision: https://reviews.freebsd.org/D15929 >=20 > Modified: > head/sys/dev/pci/pci_host_generic.c >=20 > Modified: head/sys/dev/pci/pci_host_generic.c > = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D > --- head/sys/dev/pci/pci_host_generic.c Mon Jul 9 08:55:07 2018 = (r336129) > +++ head/sys/dev/pci/pci_host_generic.c Mon Jul 9 09:00:16 2018 = (r336130) > @@ -69,6 +69,25 @@ __FBSDID("$FreeBSD$"); > (((func) & PCIE_FUNC_MASK) << PCIE_FUNC_SHIFT) | \ > ((reg) & PCIE_REG_MASK)) >=20 > +typedef void (*pci_host_generic_quirk_function)(device_t); > + > +struct pci_host_generic_quirk_entry { > + int impl; > + int part; > + int var; > + int rev; > + pci_host_generic_quirk_function func; > +}; > + > +struct pci_host_generic_block_entry { > + int impl; > + int part; > + int var; > + int rev; > + int bus; > + int slot; > +}; > + > /* Forward prototypes */ >=20 > static uint32_t generic_pcie_read_config(device_t dev, u_int bus, = u_int slot, > @@ -80,7 +99,22 @@ static int generic_pcie_read_ivar(device_t dev, = device > uintptr_t *result); > static int generic_pcie_write_ivar(device_t dev, device_t child, int = index, > uintptr_t value); > +static void pci_host_generic_apply_quirks(device_t); > +static void thunderx2_ahci_bar_quirk(device_t); >=20 > +struct pci_host_generic_quirk_entry pci_host_generic_quirks[] =3D > +{ > + {CPU_IMPL_CAVIUM, CPU_PART_THUNDERX2, 0, 0, = thunderx2_ahci_bar_quirk}, > + {0, 0, 0, 0, NULL} This breaks non-arm64, please fix. > +}; > + > +struct pci_host_generic_block_entry pci_host_generic_blocked[] =3D > +{ > + /* ThunderX2 AHCI on second socket */ > + {CPU_IMPL_CAVIUM, CPU_PART_THUNDERX2, 0, 0, 0x80, 0x10}, > + {0, 0, 0, 0, 0, 0} And this. > +}; > + > int > pci_host_generic_core_attach(device_t dev) > { > @@ -134,9 +168,30 @@ pci_host_generic_core_attach(device_t dev) > return (error); > } >=20 > + pci_host_generic_apply_quirks(dev); > + > return (0); > } >=20 > +static void > +pci_host_generic_apply_quirks(device_t dev) > +{ > + struct pci_host_generic_quirk_entry *quirk; > + > + quirk =3D pci_host_generic_quirks; > + while (1) { > + if (quirk->impl =3D=3D 0) > + break; > + > + if (CPU_MATCH(CPU_IMPL_MASK | CPU_PART_MASK, > + quirk->impl, quirk->part, quirk->var, quirk->rev) && > + quirk->func !=3D NULL) This is arm64 specific. > + quirk->func(dev); > + > + quirk++; > + } > +} > + > static uint32_t > generic_pcie_read_config(device_t dev, u_int bus, u_int slot, > u_int func, u_int reg, int bytes) > @@ -146,11 +201,25 @@ generic_pcie_read_config(device_t dev, u_int = bus, u_in > bus_space_tag_t t; > uint64_t offset; > uint32_t data; > + struct pci_host_generic_block_entry *block; >=20 > if ((bus > PCI_BUSMAX) || (slot > PCI_SLOTMAX) || > (func > PCI_FUNCMAX) || (reg > PCIE_REGMAX)) > return (~0U); >=20 > + block =3D pci_host_generic_blocked; > + while (1) { > + if (block->impl =3D=3D 0) > + break; > + > + if (CPU_MATCH(CPU_IMPL_MASK | CPU_PART_MASK, > + block->impl, block->part, block->var, block->rev) && > + block->bus =3D=3D bus && block->slot =3D=3D slot) This is also arm64 specific. > + return (~0); > + > + block++; > + } > + > sc =3D device_get_softc(dev); >=20 > offset =3D PCIE_ADDR_OFFSET(bus, slot, func, reg); > @@ -392,3 +461,20 @@ static device_method_t generic_pcie_methods[] =3D = { >=20 > DEFINE_CLASS_0(pcib, generic_pcie_core_driver, > generic_pcie_methods, sizeof(struct generic_pcie_core_softc)); > + > +static void thunderx2_ahci_bar_quirk(device_t dev) This should be dependent on arm64 and an appropriate SOC_ check. > +{ > + > + /* > + * XXX: > + * On ThunderX2, AHCI BAR2 address is wrong. It needs to = precisely > + * match the one described in datasheet. Fixup it = unconditionally. > + */ > + if (device_get_unit(dev) =3D=3D 0) { > + device_printf(dev, "running AHCI BAR fixup\n"); > + PCIB_WRITE_CONFIG(dev, 0, 16, 0, 0x18, 0x01440000, 4); > + PCIB_WRITE_CONFIG(dev, 0, 16, 0, 0x1c, 0x40, 4); > + PCIB_WRITE_CONFIG(dev, 0, 16, 1, 0x18, 0x01450000, 4); > + PCIB_WRITE_CONFIG(dev, 0, 16, 1, 0x1c, 0x40, 4); > + } > +} Andrew
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?A92C6AB0-81A3-4A90-91BF-60C8AC28D3FC>