From patchwork Fri Jan 29 17:30:41 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bjorn Helgaas X-Patchwork-Id: 8165261 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: X-Original-To: patchwork-linux-pci@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id C64699F38B for ; Fri, 29 Jan 2016 17:31:02 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id DE8AE20389 for ; Fri, 29 Jan 2016 17:31:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C935820382 for ; Fri, 29 Jan 2016 17:31:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752964AbcA2Raq (ORCPT ); Fri, 29 Jan 2016 12:30:46 -0500 Received: from mail.kernel.org ([198.145.29.136]:50155 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751753AbcA2Rap (ORCPT ); Fri, 29 Jan 2016 12:30:45 -0500 Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id D24AA20381; Fri, 29 Jan 2016 17:30:43 +0000 (UTC) Received: from localhost (unknown [69.71.1.1]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 82169202FE; Fri, 29 Jan 2016 17:30:42 +0000 (UTC) Date: Fri, 29 Jan 2016 11:30:41 -0600 From: Bjorn Helgaas To: Ray Jui Cc: Bjorn Helgaas , Rafal Milecki , Hante Meuleman , Hauke Mehrtens , linux-kernel@vger.kernel.org, bcm-kernel-feedback-list@broadcom.com, linux-pci@vger.kernel.org Subject: Re: [PATCH] PCI: iproc: Remove redundant function number check for PAXC Message-ID: <20160129173041.GD12965@localhost> References: <1454024240-12270-1-git-send-email-rjui@broadcom.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1454024240-12270-1-git-send-email-rjui@broadcom.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Ray, On Thu, Jan 28, 2016 at 03:37:20PM -0800, Ray Jui wrote: > This patch removes the conditional check that limits the number of > functions to be supported by the internally emulated endpoint device > connected to PAXC. Investigation shows that the emulated PAXC endpoint > device can properly reject request for unsupported functions, which > makes this conditional check redundant > > Signed-off-by: Ray Jui > --- > drivers/pci/host/pcie-iproc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c > index 9ae43ed..b65185d 100644 > --- a/drivers/pci/host/pcie-iproc.c > +++ b/drivers/pci/host/pcie-iproc.c > @@ -204,7 +204,7 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus, > * allows only one device and supports a limited number of functions. > */ > if (pcie->type == IPROC_PCIE_PAXC) > - if (slot > 0 || fn >= MAX_NUM_PAXC_PF) > + if (slot > 0) > return NULL; > > /* EP device access */ Thanks for checking this out. I removed the now-unused MAX_NUM_PAXC_PF and folded this into the first patch, resulting in the combined patch below. I'm sorry to say that I have yet one more question. It looks somewhat hacky to have the PAXC-specific "slot > 0" test, and I'm not sure it should be necessary (again, unless there's some implementation deficiency in that PAXC embedded endpoint). I'm looking at section 7.3 in the spec, and it seems like that endpoint *should* handle a config transaction with a non-zero Device Number, i.e., "slot", as an Unsupported Request. This should be standard behavior for all PCIe endpoints -- we can generate config transactions like that on all root complexes on all systems, so all endpoints should be able to handle it. Bjorn commit 46560388c476c8471fde7712c10f9fad8d0d1875 Author: Ray Jui Date: Wed Jan 27 16:52:24 2016 -0600 PCI: iproc: Allow multiple devices except on PAXC Commit 943ebae781f5 ("PCI: iproc: Add PAXC interface support") only allowed device 0, which is a regression on BCMA-based platforms. All systems support only one device, a Root Port at 00:00.0, on the root bus. PAXC-based systems support only the Root Port (00:00.0) and a single device (with multiple functions) below it, e.g., 01:00.0, 01:00.1, etc. Non-PAXC systems support arbitrary devices below the Root Port. [bhelgaas: changelog, fold in removal of MAX_NUM_PAXC_PF check] Fixes: 943ebae781f5 ("PCI: iproc: Add PAXC interface support") Reported-by: Rafal Milecki Signed-off-by: Ray Jui Signed-off-by: Bjorn Helgaas --- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c index 5816bce..a576aee 100644 --- a/drivers/pci/host/pcie-iproc.c +++ b/drivers/pci/host/pcie-iproc.c @@ -64,7 +64,6 @@ #define OARR_SIZE_CFG BIT(OARR_SIZE_CFG_SHIFT) #define MAX_NUM_OB_WINDOWS 2 -#define MAX_NUM_PAXC_PF 4 #define IPROC_PCIE_REG_INVALID 0xffff @@ -170,20 +169,6 @@ static inline void iproc_pcie_ob_write(struct iproc_pcie *pcie, writel(val, pcie->base + offset + (window * 8)); } -static inline bool iproc_pcie_device_is_valid(struct iproc_pcie *pcie, - unsigned int slot, - unsigned int fn) -{ - if (slot > 0) - return false; - - /* PAXC can only support limited number of functions */ - if (pcie->type == IPROC_PCIE_PAXC && fn >= MAX_NUM_PAXC_PF) - return false; - - return true; -} - /** * Note access to the configuration registers are protected at the higher layer * by 'pci_lock' in drivers/pci/access.c @@ -199,11 +184,11 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus, u32 val; u16 offset; - if (!iproc_pcie_device_is_valid(pcie, slot, fn)) - return NULL; - /* root complex access */ if (busno == 0) { + if (slot > 0 || fn > 0) + return NULL; + iproc_pcie_write_reg(pcie, IPROC_PCIE_CFG_IND_ADDR, where & CFG_IND_ADDR_MASK); offset = iproc_pcie_reg_offset(pcie, IPROC_PCIE_CFG_IND_DATA); @@ -213,6 +198,14 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus, return (pcie->base + offset); } + /* + * PAXC is connected to an internally emulated EP within the SoC. It + * allows only one device. + */ + if (pcie->type == IPROC_PCIE_PAXC) + if (slot > 0) + return NULL; + /* EP device access */ val = (busno << CFG_ADDR_BUS_NUM_SHIFT) | (slot << CFG_ADDR_DEV_NUM_SHIFT) |