From owner-svn-src-head@freebsd.org Mon Jul 9 16:21:42 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 40ED51035820; Mon, 9 Jul 2018 16:21:42 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from c.mail.sonic.net (c.mail.sonic.net [64.142.111.80]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id D2EFE7744F; Mon, 9 Jul 2018 16:21:41 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from comporellon.tachypleus.net (cpe-75-82-218-62.socal.res.rr.com [75.82.218.62]) (authenticated bits=0) by c.mail.sonic.net (8.15.1/8.15.1) with ESMTPSA id w69GAwuX021397 (version=TLSv1.2 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Mon, 9 Jul 2018 09:10:58 -0700 Subject: Re: svn commit: r336130 - head/sys/dev/pci To: Andrew Turner , Wojciech Macek Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org References: <201807090900.w6990GCZ081981@repo.freebsd.org> From: Nathan Whitehorn Message-ID: Date: Mon, 9 Jul 2018 09:10:57 -0700 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:52.0) Gecko/20100101 Thunderbird/52.9.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US X-Sonic-CAuth: UmFuZG9tSVaVP8i3gdygQhPdvHuLFztAh0FmWSa9UYAjRwgSjEgJZ6TXt23aSfBTl1TTUUC6y30kefmnM5pPR7qtuqsnHHQ2uY8ALt90xQI= X-Sonic-ID: C;2uJWqpKD6BGRw/Mw1zjrCw== M;/prUqpKD6BGRw/Mw1zjrCw== X-Spam-Flag: No X-Sonic-Spam-Details: 0.0/5.0 by cerberusd X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 09 Jul 2018 16:21:42 -0000 Maybe this file should be renamed and/or split anyway? It's not actually very generic and is used only on (some) ARM systems. -Nathan On 07/09/18 09:00, Andrew Turner wrote: > 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 wrote: >> >> Author: wma >> Date: Mon Jul 9 09:00:16 2018 >> New Revision: 336130 >> URL: https://svnweb.freebsd.org/changeset/base/336130 >> >> Log: >> ARM64: Add quirk mechanism to pci_host_generic_acpi >> >> Add few quirks which are necessary to use AHCI on ThX2 >> >> Submitted by: Patryk Duda >> Obtained from: Semihalf >> Sponsored by: Cavium >> Differential revision: https://reviews.freebsd.org/D15929 >> >> Modified: >> head/sys/dev/pci/pci_host_generic.c >> >> Modified: head/sys/dev/pci/pci_host_generic.c >> ============================================================================== >> --- 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)) >> >> +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 */ >> >> 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); >> >> +struct pci_host_generic_quirk_entry pci_host_generic_quirks[] = >> +{ >> + {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[] = >> +{ >> + /* 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); >> } >> >> + pci_host_generic_apply_quirks(dev); >> + >> return (0); >> } >> >> +static void >> +pci_host_generic_apply_quirks(device_t dev) >> +{ >> + struct pci_host_generic_quirk_entry *quirk; >> + >> + quirk = pci_host_generic_quirks; >> + while (1) { >> + if (quirk->impl == 0) >> + break; >> + >> + if (CPU_MATCH(CPU_IMPL_MASK | CPU_PART_MASK, >> + quirk->impl, quirk->part, quirk->var, quirk->rev) && >> + quirk->func != 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; >> >> if ((bus > PCI_BUSMAX) || (slot > PCI_SLOTMAX) || >> (func > PCI_FUNCMAX) || (reg > PCIE_REGMAX)) >> return (~0U); >> >> + block = pci_host_generic_blocked; >> + while (1) { >> + if (block->impl == 0) >> + break; >> + >> + if (CPU_MATCH(CPU_IMPL_MASK | CPU_PART_MASK, >> + block->impl, block->part, block->var, block->rev) && >> + block->bus == bus && block->slot == slot) > This is also arm64 specific. > >> + return (~0); >> + >> + block++; >> + } >> + >> sc = device_get_softc(dev); >> >> offset = PCIE_ADDR_OFFSET(bus, slot, func, reg); >> @@ -392,3 +461,20 @@ static device_method_t generic_pcie_methods[] = { >> >> 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) == 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 > > >