From patchwork Thu Apr 7 11:42:59 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jisheng Zhang X-Patchwork-Id: 8771301 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 CEA2D9F36E for ; Thu, 7 Apr 2016 11:47:16 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B945D2020F for ; Thu, 7 Apr 2016 11:47:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 8530A201BB for ; Thu, 7 Apr 2016 11:47:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755758AbcDGLrN (ORCPT ); Thu, 7 Apr 2016 07:47:13 -0400 Received: from mx0a-0016f401.pphosted.com ([67.231.148.174]:44833 "EHLO mx0a-0016f401.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755623AbcDGLrM (ORCPT ); Thu, 7 Apr 2016 07:47:12 -0400 Received: from pps.filterd (m0045849.ppops.net [127.0.0.1]) by mx0a-0016f401.pphosted.com (8.16.0.11/8.16.0.11) with SMTP id u37BdgFi016859; Thu, 7 Apr 2016 04:46:44 -0700 Received: from sc-exch03.marvell.com ([199.233.58.183]) by mx0a-0016f401.pphosted.com with ESMTP id 222bekuygd-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Thu, 07 Apr 2016 04:46:44 -0700 Received: from SC-EXCH02.marvell.com (10.93.176.82) by SC-EXCH03.marvell.com (10.93.176.83) with Microsoft SMTP Server (TLS) id 15.0.1104.5; Thu, 7 Apr 2016 04:46:42 -0700 Received: from maili.marvell.com (10.93.176.43) by SC-EXCH02.marvell.com (10.93.176.82) with Microsoft SMTP Server id 15.0.1104.5 via Frontend Transport; Thu, 7 Apr 2016 04:46:42 -0700 Received: from xhacker (unknown [10.37.135.134]) by maili.marvell.com (Postfix) with ESMTP id C5F1F3F703F; Thu, 7 Apr 2016 04:46:41 -0700 (PDT) Date: Thu, 7 Apr 2016 19:42:59 +0800 From: Jisheng Zhang To: Gabriele Paoloni , "jingoohan1@gmail.com" , "pratyush.anand@gmail.com" , "bhelgaas@google.com" CC: "linux-pci@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" Subject: Re: [PATCH v2] PCI: designware: move remaining rc setup code to dw_pcie_setup_rc() Message-ID: <20160407194259.5c13742c@xhacker> In-Reply-To: References: <1458128433-3020-1-git-send-email-jszhang@marvell.com> <20160407103734.55e72da7@xhacker> <20160407163443.291fbd49@xhacker> X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2016-04-07_08:, , signatures=0 X-Proofpoint-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1601100000 definitions=main-1604070170 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Spam-Status: No, score=-7.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham 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 On Thu, 7 Apr 2016 10:06:45 +0000 Gabriele Paoloni wrote: > Hi Jisheng > .. > > > > > > > > > > > Is it acceptable that pcie-hisi adds a call to dw_pcie_setup_rc() > > in > > > > hisi_add_pcie_port()? > > > > > > I don't think so...that would try to overwrite what is already set by > > > the bootloader; so it is wrong in principle and maybe it can lead to > > > undefined behaviours... > > > > make sense! This commit is intend to re-setup the rc when waken from > > s2ram (in > > s2ram state, the host lost power) > > > > I have no good solution but to introduce one function e.g > > dw_pcie_setup_rc_after_linkup(), then move related code from > > dw_pcie_host_init > > to it, then let my host driver resume hook to call. > > > > Hi Pratyush, Jingoo and Bjorn etc. > > > > any suggestions are appreciated! > > What about: > > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c > index a4cccd3..e461f5d 100644 > --- a/drivers/pci/host/pcie-designware.c > +++ b/drivers/pci/host/pcie-designware.c > @@ -434,7 +434,6 @@ int dw_pcie_host_init(struct pcie_port *pp) > struct platform_device *pdev = to_platform_device(pp->dev); > struct pci_bus *bus, *child; > struct resource *cfg_res; > - u32 val; > int i, ret; > LIST_HEAD(res); > struct resource_entry *win; > @@ -544,25 +543,6 @@ int dw_pcie_host_init(struct pcie_port *pp) > if (pp->ops->host_init) > pp->ops->host_init(pp); > > - /* > - * If the platform provides ->rd_other_conf, it means the platform > - * uses its own address translation component rather than ATU, so > - * we should not program the ATU here. > - */ > - if (!pp->ops->rd_other_conf) > - dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX1, > - PCIE_ATU_TYPE_MEM, pp->mem_base, > - pp->mem_bus_addr, pp->mem_size); > - > - dw_pcie_wr_own_conf(pp, PCI_BASE_ADDRESS_0, 4, 0); > - > - /* program correct class for RC */ > - dw_pcie_wr_own_conf(pp, PCI_CLASS_DEVICE, 2, PCI_CLASS_BRIDGE_PCI); > - > - dw_pcie_rd_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, &val); > - val |= PORT_LOGIC_SPEED_CHANGE; > - dw_pcie_wr_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, val); If we introduced one global function, is it better to keep the RC registers program sequence not changed at all? And ask each pcie dw users to call this new function in its resume hook if necessary. Either is fine, and can provide what I want, let's wait maintainers' comments. Something like: --- 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-designware.c b/drivers/pci/host/pcie-designware.c index a4cccd3..53c6176 100644 --- a/drivers/pci/host/pcie-designware.c +++ b/drivers/pci/host/pcie-designware.c @@ -434,7 +434,6 @@ int dw_pcie_host_init(struct pcie_port *pp) struct platform_device *pdev = to_platform_device(pp->dev); struct pci_bus *bus, *child; struct resource *cfg_res; - u32 val; int i, ret; LIST_HEAD(res); struct resource_entry *win; @@ -544,24 +543,7 @@ int dw_pcie_host_init(struct pcie_port *pp) if (pp->ops->host_init) pp->ops->host_init(pp); - /* - * If the platform provides ->rd_other_conf, it means the platform - * uses its own address translation component rather than ATU, so - * we should not program the ATU here. - */ - if (!pp->ops->rd_other_conf) - dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX1, - PCIE_ATU_TYPE_MEM, pp->mem_base, - pp->mem_bus_addr, pp->mem_size); - - dw_pcie_wr_own_conf(pp, PCI_BASE_ADDRESS_0, 4, 0); - - /* program correct class for RC */ - dw_pcie_wr_own_conf(pp, PCI_CLASS_DEVICE, 2, PCI_CLASS_BRIDGE_PCI); - - dw_pcie_rd_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, &val); - val |= PORT_LOGIC_SPEED_CHANGE; - dw_pcie_wr_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, val); + dw_pcie_setup_rc_after_linkup(pp); pp->root_bus_nr = pp->busn->start; if (IS_ENABLED(CONFIG_PCI_MSI)) { @@ -725,6 +707,30 @@ static struct pci_ops dw_pcie_ops = { .write = dw_pcie_wr_conf, }; +void dw_pcie_setup_rc_after_linkup(struct pcie_port *pp) +{ + u32 val; + + /* + * If the platform provides ->rd_other_conf, it means the platform + * uses its own address translation component rather than ATU, so + * we should not program the ATU here. + */ + if (!pp->ops->rd_other_conf) + dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX1, + PCIE_ATU_TYPE_MEM, pp->mem_base, + pp->mem_bus_addr, pp->mem_size); + + dw_pcie_wr_own_conf(pp, PCI_BASE_ADDRESS_0, 4, 0); + + /* program correct class for RC */ + dw_pcie_wr_own_conf(pp, PCI_CLASS_DEVICE, 2, PCI_CLASS_BRIDGE_PCI); + + dw_pcie_rd_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, &val); + val |= PORT_LOGIC_SPEED_CHANGE; + dw_pcie_wr_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, val); +} + void dw_pcie_setup_rc(struct pcie_port *pp) { u32 val; diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h index f437f9b..f7746bf 100644 --- a/drivers/pci/host/pcie-designware.h +++ b/drivers/pci/host/pcie-designware.h @@ -85,5 +85,6 @@ int dw_pcie_wait_for_link(struct pcie_port *pp); int dw_pcie_link_up(struct pcie_port *pp); void dw_pcie_setup_rc(struct pcie_port *pp); int dw_pcie_host_init(struct pcie_port *pp); +void dw_pcie_setup_rc_after_linkup(struct pcie_port *pp); #endif /* _PCIE_DESIGNWARE_H */