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 28B6BC4332F for ; Mon, 18 Oct 2021 19:42:22 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 0C0AE60C51 for ; Mon, 18 Oct 2021 19:42:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233049AbhJRToc (ORCPT ); Mon, 18 Oct 2021 15:44:32 -0400 Received: from mail.kernel.org ([198.145.29.99]:60670 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230159AbhJRTob (ORCPT ); Mon, 18 Oct 2021 15:44:31 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 9485E60F57; Mon, 18 Oct 2021 19:42:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1634586139; bh=sL9bIISzFWXLmkxiyBctOfsH3HvP/CyNjDg9oXri6sw=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=swOvq8MdtbJ0gdSpPW1iV1a4gLL6xWh1VP4h+WRTK3nC930K8SL2AVILnvOEAf8Sf 5UrRJwwFlHUy3NdAHL7REtWQd6KiMW2RrZQ/QPbPCLOvJTE2tSLWppOHFa5XQrBWsh AqQeLuaUAFHIPFZWgIXEYIaLpV5VBC5PYU36O8wVqZOUzO4Yiz0HIh2FUpcKCT0Lxl 0oXObKITDoEHU+rhJLhyX14BoOYFSEdbPcNVybr3UMNWvWRJi8cYUvtVT9gXmJzf5K RszevF2H7zHLyV9Q9ev7s+YPpIcxMvNV6o4P/dhXesLlngpGwgNcw83rRBLUynRMyv 0i4jVj1xvDoqA== Date: Mon, 18 Oct 2021 14:42:18 -0500 From: Bjorn Helgaas To: Parshuram Raju Thombare Cc: kishon@ti.com, tjoseph@cadence.com, lorenzo.pieralisi@arm.com, robh@kernel.org, kw@linux.com, bhelgaas@google.com, linux-omap@vger.kernel.org, linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, mparab@cadence.com Subject: Re: [PATCH] PCI: cadence: Disable Function Level Reset support Message-ID: <20211018194218.GA2248370@bhelgaas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1634580445-89772-1-git-send-email-pthombar@cadence.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Oct 18, 2021 at 11:07:25AM -0700, Parshuram Raju Thombare wrote: > From: Parshuram Thombare > > This patch disables FLR (Function Level Reset) support on all physical > functions. > During FLR, the Margining Lane Status and Margining Lane Control > registers should not be reset, as per PCIe specification. > However, the Controller incorrectly resets these registers upon FLR. > This causes PCISIG compliance FLR test to fail. Hence disabling > FLR on all functions using quirk flag. Add blank lines between paragraphs. Write the text in imperative mood, e.g., Disable FLR (Function Level Reset) support on all functions. It looks like this patch clears PCI_EXP_DEVCAP_FLR in the Device Capabilities register. From the point of view of Linux, that means the device doesn't *advertise* FLR support. That's different from actualy *disabling* FLR support, but maybe there's internal logic in the device that ignores PCI_EXP_DEVCTL_BCR_FLR when PCI_EXP_DEVCAP_FLR is cleared? > Signed-off-by: Parshuram Thombare > --- > drivers/pci/controller/cadence/pci-j721e.c | 3 +++ > drivers/pci/controller/cadence/pcie-cadence-ep.c | 18 +++++++++++++++++- > drivers/pci/controller/cadence/pcie-cadence.h | 3 +++ > 3 files changed, 23 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c > index ffb176d..635e36c 100644 > --- a/drivers/pci/controller/cadence/pci-j721e.c > +++ b/drivers/pci/controller/cadence/pci-j721e.c > @@ -70,6 +70,7 @@ struct j721e_pcie_data { > enum j721e_pcie_mode mode; > unsigned int quirk_retrain_flag:1; > unsigned int quirk_detect_quiet_flag:1; > + unsigned int quirk_disable_flr:1; > u32 linkdown_irq_regfield; > unsigned int byte_access_allowed:1; > }; > @@ -308,6 +309,7 @@ static int cdns_ti_pcie_config_write(struct pci_bus *bus, unsigned int devfn, > static const struct j721e_pcie_data j7200_pcie_ep_data = { > .mode = PCI_MODE_EP, > .quirk_detect_quiet_flag = true, > + .quirk_disable_flr = true, > }; > > static const struct j721e_pcie_data am64_pcie_rc_data = { > @@ -510,6 +512,7 @@ static int j721e_pcie_probe(struct platform_device *pdev) > goto err_get_sync; > } > ep->quirk_detect_quiet_flag = data->quirk_detect_quiet_flag; > + ep->quirk_disable_flr = data->quirk_disable_flr; > > cdns_pcie = &ep->pcie; > cdns_pcie->dev = dev; > diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c b/drivers/pci/controller/cadence/pcie-cadence-ep.c > index 88e05b9..4b1c4bc 100644 > --- a/drivers/pci/controller/cadence/pcie-cadence-ep.c > +++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c > @@ -565,7 +565,8 @@ static int cdns_pcie_ep_start(struct pci_epc *epc) > struct cdns_pcie_ep *ep = epc_get_drvdata(epc); > struct cdns_pcie *pcie = &ep->pcie; > struct device *dev = pcie->dev; > - int ret; > + int max_epfs = sizeof(epc->function_num_map) * 8; > + int ret, value, epf; > > /* > * BIT(0) is hardwired to 1, hence function 0 is always enabled > @@ -573,6 +574,21 @@ static int cdns_pcie_ep_start(struct pci_epc *epc) > */ > cdns_pcie_writel(pcie, CDNS_PCIE_LM_EP_FUNC_CFG, epc->function_num_map); > > + if (ep->quirk_disable_flr) { > + for (epf = 0; epf < max_epfs; epf++) { > + if (!(epc->function_num_map & BIT(epf))) > + continue; > + > + value = cdns_pcie_ep_fn_readl(pcie, epf, > + CDNS_PCIE_EP_FUNC_DEV_CAP_OFFSET + > + PCI_EXP_DEVCAP); > + value &= ~PCI_EXP_DEVCAP_FLR; > + cdns_pcie_ep_fn_writel(pcie, epf, > + CDNS_PCIE_EP_FUNC_DEV_CAP_OFFSET + > + PCI_EXP_DEVCAP, value); > + } > + } > + > ret = cdns_pcie_start_link(pcie); > if (ret) { > dev_err(dev, "Failed to start link\n"); > diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h > index 262421e..e978e7c 100644 > --- a/drivers/pci/controller/cadence/pcie-cadence.h > +++ b/drivers/pci/controller/cadence/pcie-cadence.h > @@ -123,6 +123,7 @@ > > #define CDNS_PCIE_EP_FUNC_MSI_CAP_OFFSET 0x90 > #define CDNS_PCIE_EP_FUNC_MSIX_CAP_OFFSET 0xb0 > +#define CDNS_PCIE_EP_FUNC_DEV_CAP_OFFSET 0xc0 > #define CDNS_PCIE_EP_FUNC_SRIOV_CAP_OFFSET 0x200 > > /* > @@ -357,6 +358,7 @@ struct cdns_pcie_epf { > * minimize time between read and write > * @epf: Structure to hold info about endpoint function > * @quirk_detect_quiet_flag: LTSSM Detect Quiet min delay set as quirk > + * @quirk_disable_flr: Disable FLR (Function Level Reset) quirk flag > */ > struct cdns_pcie_ep { > struct cdns_pcie pcie; > @@ -372,6 +374,7 @@ struct cdns_pcie_ep { > spinlock_t lock; > struct cdns_pcie_epf *epf; > unsigned int quirk_detect_quiet_flag:1; > + unsigned int quirk_disable_flr:1; > }; > > > -- > 1.9.1 > 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 A1531C433FE for ; Mon, 18 Oct 2021 19:50:58 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 73C4D611EF for ; Mon, 18 Oct 2021 19:50:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 73C4D611EF Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:References: List-Owner; bh=f1lD+uGRfuvmj+SlHhnxxXSDtsxEvzlYIDkTEAbJNaM=; b=O4dix/wp/AgfA9 pcdl6nfUZeNsFtbAPzAIxgD+TH7YPvfLbJVDYy5xJ70/AGMftr92wRZTeUeyuYVJbfeXEj+p5bEqF UB0f9+eLwQXhv7x4dX+lqPTIgMdgoOYoxuVGVt5TCtzGFI2DmFJCXPUNzCsgSZZrYUg5Q7goHGkAf tcRyPG9Qq2w4fkHYJe6VIKZU71ksurr9jON3JUUFDIbCKd0yz/K+HqmLgMk0GVdLTl5pBWSaP9NUW FkTcKjmEVdOXvaK5AmnzAtgyVZCEkHR8zK3SNpO9X5hTAJ7K/Jt3hso926V+sY7UxpCiWYoqncpHl v1ganoWKJSeUVMHIurmw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mcYdJ-00H7xk-T8; Mon, 18 Oct 2021 19:49:19 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mcYWa-00H53e-Fk for linux-arm-kernel@lists.infradead.org; Mon, 18 Oct 2021 19:42:22 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id 9485E60F57; Mon, 18 Oct 2021 19:42:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1634586139; bh=sL9bIISzFWXLmkxiyBctOfsH3HvP/CyNjDg9oXri6sw=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=swOvq8MdtbJ0gdSpPW1iV1a4gLL6xWh1VP4h+WRTK3nC930K8SL2AVILnvOEAf8Sf 5UrRJwwFlHUy3NdAHL7REtWQd6KiMW2RrZQ/QPbPCLOvJTE2tSLWppOHFa5XQrBWsh AqQeLuaUAFHIPFZWgIXEYIaLpV5VBC5PYU36O8wVqZOUzO4Yiz0HIh2FUpcKCT0Lxl 0oXObKITDoEHU+rhJLhyX14BoOYFSEdbPcNVybr3UMNWvWRJi8cYUvtVT9gXmJzf5K RszevF2H7zHLyV9Q9ev7s+YPpIcxMvNV6o4P/dhXesLlngpGwgNcw83rRBLUynRMyv 0i4jVj1xvDoqA== Date: Mon, 18 Oct 2021 14:42:18 -0500 From: Bjorn Helgaas To: Parshuram Raju Thombare Cc: kishon@ti.com, tjoseph@cadence.com, lorenzo.pieralisi@arm.com, robh@kernel.org, kw@linux.com, bhelgaas@google.com, linux-omap@vger.kernel.org, linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, mparab@cadence.com Subject: Re: [PATCH] PCI: cadence: Disable Function Level Reset support Message-ID: <20211018194218.GA2248370@bhelgaas> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1634580445-89772-1-git-send-email-pthombar@cadence.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211018_124220_619488_42ED3806 X-CRM114-Status: GOOD ( 26.05 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Oct 18, 2021 at 11:07:25AM -0700, Parshuram Raju Thombare wrote: > From: Parshuram Thombare > > This patch disables FLR (Function Level Reset) support on all physical > functions. > During FLR, the Margining Lane Status and Margining Lane Control > registers should not be reset, as per PCIe specification. > However, the Controller incorrectly resets these registers upon FLR. > This causes PCISIG compliance FLR test to fail. Hence disabling > FLR on all functions using quirk flag. Add blank lines between paragraphs. Write the text in imperative mood, e.g., Disable FLR (Function Level Reset) support on all functions. It looks like this patch clears PCI_EXP_DEVCAP_FLR in the Device Capabilities register. From the point of view of Linux, that means the device doesn't *advertise* FLR support. That's different from actualy *disabling* FLR support, but maybe there's internal logic in the device that ignores PCI_EXP_DEVCTL_BCR_FLR when PCI_EXP_DEVCAP_FLR is cleared? > Signed-off-by: Parshuram Thombare > --- > drivers/pci/controller/cadence/pci-j721e.c | 3 +++ > drivers/pci/controller/cadence/pcie-cadence-ep.c | 18 +++++++++++++++++- > drivers/pci/controller/cadence/pcie-cadence.h | 3 +++ > 3 files changed, 23 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c > index ffb176d..635e36c 100644 > --- a/drivers/pci/controller/cadence/pci-j721e.c > +++ b/drivers/pci/controller/cadence/pci-j721e.c > @@ -70,6 +70,7 @@ struct j721e_pcie_data { > enum j721e_pcie_mode mode; > unsigned int quirk_retrain_flag:1; > unsigned int quirk_detect_quiet_flag:1; > + unsigned int quirk_disable_flr:1; > u32 linkdown_irq_regfield; > unsigned int byte_access_allowed:1; > }; > @@ -308,6 +309,7 @@ static int cdns_ti_pcie_config_write(struct pci_bus *bus, unsigned int devfn, > static const struct j721e_pcie_data j7200_pcie_ep_data = { > .mode = PCI_MODE_EP, > .quirk_detect_quiet_flag = true, > + .quirk_disable_flr = true, > }; > > static const struct j721e_pcie_data am64_pcie_rc_data = { > @@ -510,6 +512,7 @@ static int j721e_pcie_probe(struct platform_device *pdev) > goto err_get_sync; > } > ep->quirk_detect_quiet_flag = data->quirk_detect_quiet_flag; > + ep->quirk_disable_flr = data->quirk_disable_flr; > > cdns_pcie = &ep->pcie; > cdns_pcie->dev = dev; > diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c b/drivers/pci/controller/cadence/pcie-cadence-ep.c > index 88e05b9..4b1c4bc 100644 > --- a/drivers/pci/controller/cadence/pcie-cadence-ep.c > +++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c > @@ -565,7 +565,8 @@ static int cdns_pcie_ep_start(struct pci_epc *epc) > struct cdns_pcie_ep *ep = epc_get_drvdata(epc); > struct cdns_pcie *pcie = &ep->pcie; > struct device *dev = pcie->dev; > - int ret; > + int max_epfs = sizeof(epc->function_num_map) * 8; > + int ret, value, epf; > > /* > * BIT(0) is hardwired to 1, hence function 0 is always enabled > @@ -573,6 +574,21 @@ static int cdns_pcie_ep_start(struct pci_epc *epc) > */ > cdns_pcie_writel(pcie, CDNS_PCIE_LM_EP_FUNC_CFG, epc->function_num_map); > > + if (ep->quirk_disable_flr) { > + for (epf = 0; epf < max_epfs; epf++) { > + if (!(epc->function_num_map & BIT(epf))) > + continue; > + > + value = cdns_pcie_ep_fn_readl(pcie, epf, > + CDNS_PCIE_EP_FUNC_DEV_CAP_OFFSET + > + PCI_EXP_DEVCAP); > + value &= ~PCI_EXP_DEVCAP_FLR; > + cdns_pcie_ep_fn_writel(pcie, epf, > + CDNS_PCIE_EP_FUNC_DEV_CAP_OFFSET + > + PCI_EXP_DEVCAP, value); > + } > + } > + > ret = cdns_pcie_start_link(pcie); > if (ret) { > dev_err(dev, "Failed to start link\n"); > diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h > index 262421e..e978e7c 100644 > --- a/drivers/pci/controller/cadence/pcie-cadence.h > +++ b/drivers/pci/controller/cadence/pcie-cadence.h > @@ -123,6 +123,7 @@ > > #define CDNS_PCIE_EP_FUNC_MSI_CAP_OFFSET 0x90 > #define CDNS_PCIE_EP_FUNC_MSIX_CAP_OFFSET 0xb0 > +#define CDNS_PCIE_EP_FUNC_DEV_CAP_OFFSET 0xc0 > #define CDNS_PCIE_EP_FUNC_SRIOV_CAP_OFFSET 0x200 > > /* > @@ -357,6 +358,7 @@ struct cdns_pcie_epf { > * minimize time between read and write > * @epf: Structure to hold info about endpoint function > * @quirk_detect_quiet_flag: LTSSM Detect Quiet min delay set as quirk > + * @quirk_disable_flr: Disable FLR (Function Level Reset) quirk flag > */ > struct cdns_pcie_ep { > struct cdns_pcie pcie; > @@ -372,6 +374,7 @@ struct cdns_pcie_ep { > spinlock_t lock; > struct cdns_pcie_epf *epf; > unsigned int quirk_detect_quiet_flag:1; > + unsigned int quirk_disable_flr:1; > }; > > > -- > 1.9.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel