From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A5EB4C433FE for ; Fri, 8 Oct 2021 06:21:20 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id F0C4A60F6E for ; Fri, 8 Oct 2021 06:21:19 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org F0C4A60F6E Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 58AE0835B0; Fri, 8 Oct 2021 08:21:17 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1633674078; bh=PZpu+byurY1+moulVEHPxaRM2OkQpQFn+Mzbh8CE5/o=; h=Subject:To:Cc:References:From:Date:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=SFviVJc22bUI/3qTAtrrFN9hUvBc3KUzqUBMa0LCrNROaL8csdKSwtYchfV6YlZ4x U/1E+rPP+3jYSdotdGLJ26GxE5H0zycjzA266C9QBIOo8+jkhbJaiZPs9Zt2f2lucF 8p6OwVgtGSNSj/uL0wi1BCGkEEUv7MAtVlMBC18Pf44bbyPbuWSrC+gWsr3QP7NCa5 NU7/c2hnjtaIMc4H/DPuaSGNILBHZSadG5xOgPKbitNMFNqojWFpMXygF3decIEdfF ZI3hh5Q3GVGrl792SHTaUlq6s+E/gYp5pO/+ZoqE62XYFv2h0VNbeE9nrFtLujK2XC QAxZC7Im3ZzwA== Received: by phobos.denx.de (Postfix, from userid 109) id 7681C835AA; Fri, 8 Oct 2021 08:21:14 +0200 (CEST) Received: from mout-u-205.mailbox.org (mout-u-205.mailbox.org [IPv6:2001:67c:2050:1::465:205]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 3A93F835A4 for ; Fri, 8 Oct 2021 08:21:10 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: phobos.denx.de; spf=fail smtp.mailfrom=sr@denx.de Received: from smtp102.mailbox.org (smtp102.mailbox.org [80.241.60.233]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-384) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-u-205.mailbox.org (Postfix) with ESMTPS id 4HQdP21BHTzQkwj; Fri, 8 Oct 2021 08:21:10 +0200 (CEST) Subject: Re: [PATCH u-boot-marvell v2 2/6] arm: a37xx: pci: Add support for accessing PCI Bridge on root bus To: =?UTF-8?Q?Marek_Beh=c3=ban?= Cc: u-boot@lists.denx.de, pali@kernel.org, =?UTF-8?Q?Marek_Beh=c3=ban?= References: <20210925225446.1872-1-kabel@kernel.org> <20210925225446.1872-3-kabel@kernel.org> From: Stefan Roese Message-ID: Date: Fri, 8 Oct 2021 08:21:06 +0200 MIME-Version: 1.0 In-Reply-To: <20210925225446.1872-3-kabel@kernel.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: de-DE Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 590F337F X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean On 26.09.21 00:54, Marek Behún wrote: > From: Pali Rohár > > Aardvark does not have a real PCIe Root Port device on the root bus. > Instead it has PCIe registers of PCIe Root Port device mapped in > internal Aardvark memory space starting at offset 0xc0. > > The PCIe Root Port itself is normally available as a PCI Bridge device > on the root bus with bus number zero. Aardvark instead has the > configuration registers of this PCI Bridge at offset 0x00 of Aardvark's > memory space, but the class code of this device is Mass Storage > Controller (0x010400), instead of PCI Bridge (0x600400), which causes > U-Boot to fail to recognize it as a P2P Bridge > > Add a hook into the pcie_advk_read_config() / pcie_advk_write_config() > functions to redirect access for root bus from PIO transfer to this > internal Aardvark memory space. This will allow U-Boot to access > configuration space of this PCI Bridge which represents PCIe Root Port. > > Redirect access to PCI Bridge registers in range 0x10 - 0x34 to driver's > internal buffer (cfgcache[]). This is because at those addresses > Aardvark has different registers, incompatible with config space of a > PCI Bridge. > > Redirect access to PCI Bridge register PCI_ROM_ADDRESS1 (0x38) to > Aardvark internal address for that register (0x30). > > When reading PCI Bridge register PCI_HEADER_TYPE, set it explicitly to > value Type 1 (PCI_HEADER_TYPE_BRIDGE) as PCI Bridge must be of Type 1. > > When writing to PCI_PRIMARY_BUS or PCI_SECONDARY_BUS registers on this > PCI Bridge, correctly update driver's first_busno and sec_busno > variables, so that pcie_advk_addr_valid() function can check if address > of any device behind the root bus is valid and that PIO transfers are > started with correct config type (1 vs 0), which is required for > accessing devices behind some PCI bridge after the root bus. > > U-Boot's PCI_PNP code sets primary and secondary bus numbers as relative > to the configured bus number of the root bus. This is done so that > U-Boot can support multiple PCIe host bridges or multiple root port > buses, when internal bus numbers are different. > > Now that root bus is available, update code in pcie_advk_read_config() > and pcie_advk_write_config() functions to correctly calculate real > Aardvark bus number of the target device from U-Boot's bus number as: > busno = PCI_BUS(bdf) - dev_seq(bus) > > Signed-off-by: Pali Rohár > Reviewed-by: Marek Behún Reviewed-by: Stefan Roese Thanks, Stefan > --- > drivers/pci/pci-aardvark.c | 143 ++++++++++++++++++++++++++++++++----- > 1 file changed, 124 insertions(+), 19 deletions(-) > > diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c > index 741e0431e1..692210ded9 100644 > --- a/drivers/pci/pci-aardvark.c > +++ b/drivers/pci/pci-aardvark.c > @@ -39,6 +39,8 @@ > #define PCIE_CORE_CMD_IO_ACCESS_EN BIT(0) > #define PCIE_CORE_CMD_MEM_ACCESS_EN BIT(1) > #define PCIE_CORE_CMD_MEM_IO_REQ_EN BIT(2) > +#define PCIE_CORE_DEV_REV_REG 0x8 > +#define PCIE_CORE_EXP_ROM_BAR_REG 0x30 > #define PCIE_CORE_DEV_CTRL_STATS_REG 0xc8 > #define PCIE_CORE_DEV_CTRL_STATS_RELAX_ORDER_DISABLE (0 << 4) > #define PCIE_CORE_DEV_CTRL_STATS_SNOOP_DISABLE (0 << 11) > @@ -163,7 +165,7 @@ > #define PCIE_CONFIG_WR_TYPE1 0xb > > /* PCI_BDF shifts 8bit, so we need extra 4bit shift */ > -#define PCIE_BDF(dev) (dev << 4) > +#define PCIE_BDF(b, d, f) (PCI_BDF(b, d, f) << 4) > #define PCIE_CONF_BUS(bus) (((bus) & 0xff) << 20) > #define PCIE_CONF_DEV(dev) (((dev) & 0x1f) << 15) > #define PCIE_CONF_FUNC(fun) (((fun) & 0x7) << 12) > @@ -188,13 +190,17 @@ > * first_busno stores the bus number of the PCIe root-port > * number which may vary depending on the PCIe setup > * (PEX switches etc). > + * @sec_busno: sec_busno stores the bus number for the device behind > + * the PCIe root-port > * @device: The pointer to PCI uclass device. > */ > struct pcie_advk { > void *base; > int first_busno; > + int sec_busno; > struct udevice *dev; > struct gpio_desc reset_gpio; > + u32 cfgcache[0x34 - 0x10]; > }; > > static inline void advk_writel(struct pcie_advk *pcie, uint val, uint reg) > @@ -210,22 +216,30 @@ static inline uint advk_readl(struct pcie_advk *pcie, uint reg) > /** > * pcie_advk_addr_valid() - Check for valid bus address > * > + * @pcie: Pointer to the PCI bus > + * @busno: Bus number of PCI device > + * @dev: Device number of PCI device > + * @func: Function number of PCI device > * @bdf: The PCI device to access > - * @first_busno: Bus number of the PCIe controller root complex > * > - * Return: 1 on valid, 0 on invalid > + * Return: true on valid, false on invalid > */ > -static int pcie_advk_addr_valid(pci_dev_t bdf, int first_busno) > +static bool pcie_advk_addr_valid(struct pcie_advk *pcie, > + int busno, u8 dev, u8 func) > { > + /* On the primary (local) bus there is only one PCI Bridge */ > + if (busno == pcie->first_busno && (dev != 0 || func != 0)) > + return false; > + > /* > - * In PCIE-E only a single device (0) can exist > - * on the local bus. Beyound the local bus, there might be > - * a Switch and everything is possible. > + * In PCI-E only a single device (0) can exist on the secondary bus. > + * Beyond the secondary bus, there might be a Switch and anything is > + * possible. > */ > - if ((PCI_BUS(bdf) == first_busno) && (PCI_DEV(bdf) > 0)) > - return 0; > + if (busno == pcie->sec_busno && dev != 0) > + return false; > > - return 1; > + return true; > } > > /** > @@ -355,20 +369,55 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf, > enum pci_size_t size) > { > struct pcie_advk *pcie = dev_get_priv(bus); > + int busno = PCI_BUS(bdf) - dev_seq(bus); > int retry_count; > bool allow_crs; > + ulong data; > uint reg; > int ret; > > dev_dbg(pcie->dev, "PCIE CFG read: (b,d,f)=(%2d,%2d,%2d) ", > PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf)); > > - if (!pcie_advk_addr_valid(bdf, pcie->first_busno)) { > + if (!pcie_advk_addr_valid(pcie, busno, PCI_DEV(bdf), PCI_FUNC(bdf))) { > dev_dbg(pcie->dev, "- out of range\n"); > *valuep = pci_get_ff(size); > return 0; > } > > + /* > + * The configuration space of the PCI Bridge on primary (local) bus is > + * not accessible via PIO transfers like all other PCIe devices. PCI > + * Bridge config registers are available directly in Aardvark memory > + * space starting at offset zero. Moreover PCI Bridge registers in the > + * range 0x10 - 0x34 are not available and register 0x38 (Expansion ROM > + * Base Address) is at offset 0x30. > + * We therefore read configuration space content of the primary PCI > + * Bridge from our virtual cache. > + */ > + if (busno == pcie->first_busno) { > + if (offset >= 0x10 && offset < 0x34) > + data = pcie->cfgcache[(offset - 0x10) / 4]; > + else if ((offset & ~3) == PCI_ROM_ADDRESS1) > + data = advk_readl(pcie, PCIE_CORE_EXP_ROM_BAR_REG); > + else > + data = advk_readl(pcie, offset & ~3); > + > + if ((offset & ~3) == (PCI_HEADER_TYPE & ~3)) { > + /* > + * Change Header Type of PCI Bridge device to Type 1 > + * (0x01, used by PCI Bridges) because hardwired value > + * is Type 0 (0x00, used by Endpoint devices). > + */ > + data &= ~0x00ff0000; > + data |= PCI_HEADER_TYPE_BRIDGE << 16; > + } > + > + *valuep = pci_conv_32_to_size(data, offset, size); > + > + return 0; > + } > + > /* > * Returning fabricated CRS value (0xFFFF0001) by PCIe Root Complex to > * OS is allowed only for 4-byte PCI_VENDOR_ID config read request and > @@ -396,14 +445,14 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf, > /* Program the control register */ > reg = advk_readl(pcie, PIO_CTRL); > reg &= ~PIO_CTRL_TYPE_MASK; > - if (PCI_BUS(bdf) == pcie->first_busno) > + if (busno == pcie->sec_busno) > reg |= PCIE_CONFIG_RD_TYPE0; > else > reg |= PCIE_CONFIG_RD_TYPE1; > advk_writel(pcie, reg, PIO_CTRL); > > /* Program the address registers */ > - reg = PCIE_BDF(bdf) | PCIE_CONF_REG(offset); > + reg = PCIE_BDF(busno, PCI_DEV(bdf), PCI_FUNC(bdf)) | PCIE_CONF_REG(offset); > advk_writel(pcie, reg, PIO_ADDR_LS); > advk_writel(pcie, 0, PIO_ADDR_MS); > > @@ -491,7 +540,9 @@ static int pcie_advk_write_config(struct udevice *bus, pci_dev_t bdf, > enum pci_size_t size) > { > struct pcie_advk *pcie = dev_get_priv(bus); > + int busno = PCI_BUS(bdf) - dev_seq(bus); > int retry_count; > + ulong data; > uint reg; > int ret; > > @@ -500,11 +551,41 @@ static int pcie_advk_write_config(struct udevice *bus, pci_dev_t bdf, > dev_dbg(pcie->dev, "(addr,size,val)=(0x%04x, %d, 0x%08lx)\n", > offset, size, value); > > - if (!pcie_advk_addr_valid(bdf, pcie->first_busno)) { > + if (!pcie_advk_addr_valid(pcie, busno, PCI_DEV(bdf), PCI_FUNC(bdf))) { > dev_dbg(pcie->dev, "- out of range\n"); > return 0; > } > > + /* > + * As explained in pcie_advk_read_config(), for the configuration > + * space of the primary PCI Bridge, we write the content into virtual > + * cache. > + */ > + if (busno == pcie->first_busno) { > + if (offset >= 0x10 && offset < 0x34) { > + data = pcie->cfgcache[(offset - 0x10) / 4]; > + data = pci_conv_size_to_32(data, value, offset, size); > + pcie->cfgcache[(offset - 0x10) / 4] = data; > + } else if ((offset & ~3) == PCI_ROM_ADDRESS1) { > + data = advk_readl(pcie, PCIE_CORE_EXP_ROM_BAR_REG); > + data = pci_conv_size_to_32(data, value, offset, size); > + advk_writel(pcie, data, PCIE_CORE_EXP_ROM_BAR_REG); > + } else { > + data = advk_readl(pcie, offset & ~3); > + data = pci_conv_size_to_32(data, value, offset, size); > + advk_writel(pcie, data, offset & ~3); > + } > + > + if (offset == PCI_PRIMARY_BUS) > + pcie->first_busno = data & 0xff; > + > + if (offset == PCI_SECONDARY_BUS || > + (offset == PCI_PRIMARY_BUS && size != PCI_SIZE_8)) > + pcie->sec_busno = (data >> 8) & 0xff; > + > + return 0; > + } > + > if (advk_readl(pcie, PIO_START)) { > dev_err(pcie->dev, > "Previous PIO read/write transfer is still running\n"); > @@ -514,14 +595,14 @@ static int pcie_advk_write_config(struct udevice *bus, pci_dev_t bdf, > /* Program the control register */ > reg = advk_readl(pcie, PIO_CTRL); > reg &= ~PIO_CTRL_TYPE_MASK; > - if (PCI_BUS(bdf) == pcie->first_busno) > + if (busno == pcie->sec_busno) > reg |= PCIE_CONFIG_WR_TYPE0; > else > reg |= PCIE_CONFIG_WR_TYPE1; > advk_writel(pcie, reg, PIO_CTRL); > > /* Program the address registers */ > - reg = PCIE_BDF(bdf) | PCIE_CONF_REG(offset); > + reg = PCIE_BDF(busno, PCI_DEV(bdf), PCI_FUNC(bdf)) | PCIE_CONF_REG(offset); > advk_writel(pcie, reg, PIO_ADDR_LS); > advk_writel(pcie, 0, PIO_ADDR_MS); > dev_dbg(pcie->dev, "\tPIO req. - addr = 0x%08x\n", reg); > @@ -590,14 +671,14 @@ static int pcie_advk_wait_for_link(struct pcie_advk *pcie) > /* check if the link is up or not */ > for (retries = 0; retries < LINK_MAX_RETRIES; retries++) { > if (pcie_advk_link_up(pcie)) { > - printf("PCIE-%d: Link up\n", pcie->first_busno); > + printf("PCIe: Link up\n"); > return 0; > } > > udelay(LINK_WAIT_TIMEOUT); > } > > - printf("PCIE-%d: Link down\n", pcie->first_busno); > + printf("PCIe: Link down\n"); > > return -ETIMEDOUT; > } > @@ -716,6 +797,25 @@ static int pcie_advk_setup_hw(struct pcie_advk *pcie) > */ > advk_writel(pcie, 0x11ab11ab, VENDOR_ID_REG); > > + /* > + * Change Class Code of PCI Bridge device to PCI Bridge (0x600400), > + * because default value is Mass Storage Controller (0x010400), causing > + * U-Boot to fail to recognize it as P2P Bridge. > + * > + * Note that this Aardvark PCI Bridge does not have a compliant Type 1 > + * Configuration Space and it even cannot be accessed via Aardvark's > + * PCI config space access method. Something like config space is > + * available in internal Aardvark registers starting at offset 0x0 > + * and is reported as Type 0. In range 0x10 - 0x34 it has totally > + * different registers. So our driver reports Header Type as Type 1 and > + * for the above mentioned range redirects access to the virtual > + * cfgcache[] buffer, which avoids changing internal Aardvark registers. > + */ > + reg = advk_readl(pcie, PCIE_CORE_DEV_REV_REG); > + reg &= ~0xffffff00; > + reg |= (PCI_CLASS_BRIDGE_PCI << 8) << 8; > + advk_writel(pcie, reg, PCIE_CORE_DEV_REV_REG); > + > /* Set Advanced Error Capabilities and Control PF0 register */ > reg = PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX | > PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX_EN | > @@ -857,9 +957,14 @@ static int pcie_advk_probe(struct udevice *dev) > dev_warn(dev, "PCIE Reset on GPIO support is missing\n"); > } > > - pcie->first_busno = dev_seq(dev); > pcie->dev = pci_get_controller(dev); > > + /* PCI Bridge support 32-bit I/O and 64-bit prefetch mem addressing */ > + pcie->cfgcache[(PCI_IO_BASE - 0x10) / 4] = > + PCI_IO_RANGE_TYPE_32 | (PCI_IO_RANGE_TYPE_32 << 8); > + pcie->cfgcache[(PCI_PREF_MEMORY_BASE - 0x10) / 4] = > + PCI_PREF_RANGE_TYPE_64 | (PCI_PREF_RANGE_TYPE_64 << 16); > + > return pcie_advk_setup_hw(pcie); > } > > Viele Grüße, Stefan -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de