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 X-Spam-Level: X-Spam-Status: No, score=-15.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5FD10C07E95 for ; Wed, 7 Jul 2021 12:02:52 +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 2EE5A61C78 for ; Wed, 7 Jul 2021 12:02:52 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2EE5A61C78 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@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=02UrsxFhwoPQFf/xwrwO5SP2nCaasf2kyKB0v/P/ZFU=; b=UEOLf6KrLFaMAX 72ZLektvtylM9l9Id5vB0biR9bxAFSmZnDdcYb96eDjoR4PSVIY9kGTHcmLI4w/NVWbgXE08ru2Ih hanRFv1IoBlflmIkgu4CcTJ8u6A7h5e+PODeR8ks4p1z8IhN4rAweif/FB+/pny/1a86L2KkHNfmp Q8eULzFeUpQHKLLk+f1VaZVZ3Uxvv/2r725AdTG1ObhMJ8o1Zq0BGrUbZI4t/3Y+DfMxKRClBjtEi DkAQaYQoWUIQm6wghL3M5EgxBW/EAKEUUrqMkJUC5ZYBb8HOgx//OK5t3N+Orzyj9TRjOUTCk26yx WGaEzO7zFR/JK7pXu0BQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1m16Ep-00Eiib-PF; Wed, 07 Jul 2021 12:01:11 +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 1m16Em-00Eihh-25 for linux-arm-kernel@lists.infradead.org; Wed, 07 Jul 2021 12:01:09 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id 22BB161C78; Wed, 7 Jul 2021 12:01:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1625659267; bh=x3B4x8BU2OkCmrAavlTqYXzWR4EYU/t1YZNFwkkeelk=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=dv7cAsiHvsbf6rsHT5HoZGhUKAwVKSZrNZu3MJ+IAd8WSSbTTUz0aHgIrk7WyCRkv /RxlmQNWkovTNL93o8QSCXvJzA//EU4Dx7IfrXLdYjx1ckI1v7HZwqWXpTkKuu/4jf y1//8ZQ2Nj2Gprbm8c3qvH45a1gUTHcTAg6ZfZ/ilYyiJw4viYQLtKO2ICtUorE7aY g52rhmunhzNNtN0Om3toPXbqwJHZ/NfUNwcvjpgAoURtDUTpA4/vXkmpNqcbeeyisl 2ulujxgkChNpY8GqwKzbyqHJo4A1zS1VKOrAeiWxHVb3h28l8UXCsQHoUhrO/8MPVE fGCDtlaVsVt2w== Date: Wed, 7 Jul 2021 07:01:05 -0500 From: Bjorn Helgaas To: Kishon Vijay Abraham I Cc: Lorenzo Pieralisi , Arnd Bergmann , Rob Herring , Bjorn Helgaas , Lokesh Vutla , Greg Kroah-Hartman , Tom Joseph , linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Nadeem Athani Subject: Re: [PATCH 2/5] PCI: j721e: Add PCIe support for J7200 Message-ID: <20210707120105.GA886562@bjorn-Precision-5520> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210706105035.9915-3-kishon@ti.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210707_050108_187050_D1AB63B3 X-CRM114-Status: GOOD ( 25.10 ) 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 Tue, Jul 06, 2021 at 04:20:32PM +0530, Kishon Vijay Abraham I wrote: > J7200 has the same PCIe IP as in J721E with minor changes in the > wrapper. J7200 allows byte access of bridge configuration space > registers and the register field for LINK_DOWN interrupt is different. > J7200 also requires "quirk_detect_quiet_flag" to be set. Configure these > changes as part of driver data applicable only to J7200. > > Signed-off-by: Kishon Vijay Abraham I > --- > drivers/pci/controller/cadence/pci-j721e.c | 44 ++++++++++++++++++++-- > 1 file changed, 40 insertions(+), 4 deletions(-) > > diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c > index 35e61048e133..803da33b86d8 100644 > --- a/drivers/pci/controller/cadence/pci-j721e.c > +++ b/drivers/pci/controller/cadence/pci-j721e.c > @@ -27,6 +27,7 @@ > #define STATUS_REG_SYS_2 0x508 > #define STATUS_CLR_REG_SYS_2 0x708 > #define LINK_DOWN BIT(1) > +#define J7200_LINK_DOWN BIT(10) > > #define J721E_PCIE_USER_CMD_STATUS 0x4 > #define LINK_TRAINING_ENABLE BIT(0) > @@ -57,6 +58,7 @@ struct j721e_pcie { > struct cdns_pcie *cdns_pcie; > void __iomem *user_cfg_base; > void __iomem *intd_cfg_base; > + u32 link_irq_reg_field; This seems to be a device-specific "link down" bit, so maybe a more descriptive name that suggests "link down"? > }; > > enum j721e_pcie_mode { > @@ -67,6 +69,9 @@ enum j721e_pcie_mode { > struct j721e_pcie_data { > enum j721e_pcie_mode mode; > bool quirk_retrain_flag; > + bool quirk_detect_quiet_flag; > + u32 link_irq_reg_field; > + bool byte_access_allowed; Maybe re-indent quirk_retrain_flag so the struct is consistent? I know there's a pattern of using "bool" in these structs, but I don't think it's really any better than "unsigned int :1" and it takes more space. https://lore.kernel.org/r/CA+55aFzKQ6Pj18TB8p4Yr0M4t+S+BsiHH=BJNmn=76-NcjTj-g@mail.gmail.com/ https://lore.kernel.org/r/CA+55aFxnePDimkVKVtv3gNmRGcwc8KQ5mHYvUxY8sAQg6yvVYg@mail.gmail.com/ > }; > > static inline u32 j721e_pcie_user_readl(struct j721e_pcie *pcie, u32 offset) > @@ -98,12 +103,12 @@ static irqreturn_t j721e_pcie_link_irq_handler(int irq, void *priv) > u32 reg; > > reg = j721e_pcie_intd_readl(pcie, STATUS_REG_SYS_2); > - if (!(reg & LINK_DOWN)) > + if (!(reg & pcie->link_irq_reg_field)) > return IRQ_NONE; > > dev_err(dev, "LINK DOWN!\n"); > > - j721e_pcie_intd_writel(pcie, STATUS_CLR_REG_SYS_2, LINK_DOWN); > + j721e_pcie_intd_writel(pcie, STATUS_CLR_REG_SYS_2, pcie->link_irq_reg_field); > return IRQ_HANDLED; > } > > @@ -112,7 +117,7 @@ static void j721e_pcie_config_link_irq(struct j721e_pcie *pcie) > u32 reg; > > reg = j721e_pcie_intd_readl(pcie, ENABLE_REG_SYS_2); > - reg |= LINK_DOWN; > + reg |= pcie->link_irq_reg_field; > j721e_pcie_intd_writel(pcie, ENABLE_REG_SYS_2, reg); > } > > @@ -284,10 +289,25 @@ static struct pci_ops cdns_ti_pcie_host_ops = { > static const struct j721e_pcie_data j721e_pcie_rc_data = { > .mode = PCI_MODE_RC, > .quirk_retrain_flag = true, > + .byte_access_allowed = false, > + .link_irq_reg_field = LINK_DOWN, > }; > > static const struct j721e_pcie_data j721e_pcie_ep_data = { > .mode = PCI_MODE_EP, > + .link_irq_reg_field = LINK_DOWN, > +}; > + > +static const struct j721e_pcie_data j7200_pcie_rc_data = { > + .mode = PCI_MODE_RC, > + .quirk_detect_quiet_flag = true, > + .link_irq_reg_field = J7200_LINK_DOWN, > + .byte_access_allowed = true, > +}; > + > +static const struct j721e_pcie_data j7200_pcie_ep_data = { > + .mode = PCI_MODE_EP, > + .quirk_detect_quiet_flag = true, > }; > > static const struct of_device_id of_j721e_pcie_match[] = { > @@ -299,6 +319,14 @@ static const struct of_device_id of_j721e_pcie_match[] = { > .compatible = "ti,j721e-pcie-ep", > .data = &j721e_pcie_ep_data, > }, > + { > + .compatible = "ti,j7200-pcie-host", > + .data = &j7200_pcie_rc_data, > + }, > + { > + .compatible = "ti,j7200-pcie-ep", > + .data = &j7200_pcie_ep_data, > + }, > {}, > }; > > @@ -309,10 +337,12 @@ static int j721e_pcie_probe(struct platform_device *pdev) > struct pci_host_bridge *bridge; > struct j721e_pcie_data *data; > struct cdns_pcie *cdns_pcie; > + bool byte_access_allowed; > struct j721e_pcie *pcie; > struct cdns_pcie_rc *rc; > struct cdns_pcie_ep *ep; > struct gpio_desc *gpiod; > + u32 link_irq_reg_field; > void __iomem *base; > struct clk *clk; > u32 num_lanes; > @@ -325,6 +355,8 @@ static int j721e_pcie_probe(struct platform_device *pdev) > return -EINVAL; > > mode = (u32)data->mode; > + byte_access_allowed = data->byte_access_allowed; > + link_irq_reg_field = data->link_irq_reg_field; > > pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL); > if (!pcie) > @@ -332,6 +364,7 @@ static int j721e_pcie_probe(struct platform_device *pdev) > > pcie->dev = dev; > pcie->mode = mode; > + pcie->link_irq_reg_field = link_irq_reg_field; > > base = devm_platform_ioremap_resource_byname(pdev, "intd_cfg"); > if (IS_ERR(base)) > @@ -391,9 +424,11 @@ static int j721e_pcie_probe(struct platform_device *pdev) > goto err_get_sync; > } > > - bridge->ops = &cdns_ti_pcie_host_ops; > + if (!byte_access_allowed) Why bother with the "byte_access_allowed" local variable? Could just use "data->byte_access_allowed" here, as the code below uses "data->quirk_retrain_flag", etc. Same with "link_irq_reg_field" above, I guess. > + bridge->ops = &cdns_ti_pcie_host_ops; > rc = pci_host_bridge_priv(bridge); > rc->quirk_retrain_flag = data->quirk_retrain_flag; > + rc->quirk_detect_quiet_flag = data->quirk_detect_quiet_flag; > > cdns_pcie = &rc->pcie; > cdns_pcie->dev = dev; > @@ -459,6 +494,7 @@ static int j721e_pcie_probe(struct platform_device *pdev) > ret = -ENOMEM; > goto err_get_sync; > } > + ep->quirk_detect_quiet_flag = data->quirk_detect_quiet_flag; > > cdns_pcie = &ep->pcie; > cdns_pcie->dev = dev; > -- > 2.17.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel