Message ID | 20231010155914.9516-2-manivannan.sadhasivam@linaro.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:2908:b0:403:3b70:6f57 with SMTP id ib8csp306928vqb; Tue, 10 Oct 2023 09:01:04 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHInbvNo0uNny3Xj8FhS8qmb59Xz683JqROv2BCYHa/lOEgXOeKWZ35LK/E/TOSlx14yOSE X-Received: by 2002:a05:6a00:24c4:b0:68e:3eab:9e1b with SMTP id d4-20020a056a0024c400b0068e3eab9e1bmr19502152pfv.22.1696953664272; Tue, 10 Oct 2023 09:01:04 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1696953664; cv=none; d=google.com; s=arc-20160816; b=mEpEUFPrDsP4kNqFn58ihhCeEGNg/DtuSytFQJXbgfyLZ2RIfF5DsAOGX6PHs1mzFV 9UMM/vdc/zEvelZIPCATeUQEQTJFb/gbOnguCgTnmfSfXowAloywLp6QSAakANgOBlxS PobH9PnxpactFbBrdfx1qX4EtrSZ8mT1T5tRs3e19mjzc/aOXvS4XkU39KY6Y4iIAPxY SGL9n1DqioY7oC53xenwo6K2ng81C1KDWsaXnyCUAzVWIaTnJVbwYMVsvVGx3lZr/8AF OWpO9BiksNhcGnDzfB9v7kDCtlZtWwJeTapOaHfO3VFwqq4ms+5YcYyQxc4Bg8h7hvVZ 3N5Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=3XHRCY63hQtzpYUIBv5+mXIDtPmwcujXauM/JNVB8nw=; fh=+V2vYZH5xq7Tr40jqwUShSH8vZSpdfr/dj2bN908Pc4=; b=G21WLNU1dPhV09Sl0XMu1hZQ+b/KN/ZxBNRS64sy1REPsmf04/pdX/Zs/pe6OcFMn5 hobo80NukgJc2pRKPlJhsYSXSDt+25sDjFKDGBSULgnYWkPOADbWIKJdp/31uBmBKVos lTQQvTgj5aFc/jQVOMLOuCc+h5vf3i9UMLo6FtBErkO/qNI9AUiisRDK0Fu8Py10VudT ecq44WOuQ8O2Ox/rRj8+esm/7tmF2wFjBZLc546MRHdc9HdtQYf4SsNe7lJReGqS8wxg rWs25iEzBqrtPxqWcTh1BG1P2n8t9GUShnt16xe3yhJyJCTP5mgsxJmbBLVKEDNb35wD U2qg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=SDf7MCmQ; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from fry.vger.email (fry.vger.email. [23.128.96.38]) by mx.google.com with ESMTPS id d1-20020a63a701000000b0059cdf90b9b2si1145988pgf.685.2023.10.10.09.01.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 10 Oct 2023 09:01:04 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=SDf7MCmQ; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by fry.vger.email (Postfix) with ESMTP id 7EB1882D87A1; Tue, 10 Oct 2023 09:00:01 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at fry.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233619AbjJJP7l (ORCPT <rfc822;rua109.linux@gmail.com> + 20 others); Tue, 10 Oct 2023 11:59:41 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59130 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233597AbjJJP7h (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 10 Oct 2023 11:59:37 -0400 Received: from mail-il1-x131.google.com (mail-il1-x131.google.com [IPv6:2607:f8b0:4864:20::131]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CAF5BAF for <linux-kernel@vger.kernel.org>; Tue, 10 Oct 2023 08:59:34 -0700 (PDT) Received: by mail-il1-x131.google.com with SMTP id e9e14a558f8ab-35135f69de2so19711805ab.2 for <linux-kernel@vger.kernel.org>; Tue, 10 Oct 2023 08:59:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1696953574; x=1697558374; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=3XHRCY63hQtzpYUIBv5+mXIDtPmwcujXauM/JNVB8nw=; b=SDf7MCmQAEga71pshauObsxutTVz/eEpcgeWbSTX4ZcfhwDPBsfKPGDQYgjFXkDfhX A6mjI6wRkp1LfpNm2GTezfYQ7F967XhMFaK+8fCUN6rStRmb3QgtMjqBpp7RaqMeOMHy qY5sw2cfcHX+kLGtCU0SSw6kP+CD9GVwub+cMkoU5tGd2+4gRaI0KcetpEgTzh05RcH5 7y39wsrrbEUr3WLU6fZ0wAu0psYY0hKw6xw1quZPVQeQlE7G+uD0nrLj/J6YvXEZM73W BkGpqw6haT22QcjsKOh0exh0caENmVwW3UGHHEUdbo3rFK+teUEKX95nzZAU3QGM1jDo lnvg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696953574; x=1697558374; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=3XHRCY63hQtzpYUIBv5+mXIDtPmwcujXauM/JNVB8nw=; b=nIM3/cmLfBkJ7HawPL9FMx793wTHusouW8u5kaFcQZG1GzRXyjqBN5dDh7MeUd2Brn oQozrTHFjvR/6wuS8OUSrJNjc5tWfPOxIchPiqO5WD/azBI28JUWcLG0nQWJ68n4uHSN M4PNnEOdciuf45uiCC0lvDkrmk311CP0lhVWLyXD8eSYhUvU7IaGFj2skQ568JjMHuZc FU5zlHC1fyOUz//6frEbN9NvOwVSt2swzpfrAo+b28o5vXVrZoE8eXNMGAqE11ZpTjDE c7trJlYit9UxsR61A0sSCwWtiCxnfiFfDjVnJ3DyNHg5vlk5TDYLSzBpivSp2jxSu68x 0b/w== X-Gm-Message-State: AOJu0YweQ+yCBREttYXAJLj6l/2l2D743/ZJupcG5mOxhus6tP7v/TDD u/VBTC4CpJkwG3Nu0y34nrUu X-Received: by 2002:a05:6e02:ecc:b0:34f:c9b4:5f9c with SMTP id i12-20020a056e020ecc00b0034fc9b45f9cmr16813737ilk.31.1696953574071; Tue, 10 Oct 2023 08:59:34 -0700 (PDT) Received: from localhost.localdomain ([117.217.182.234]) by smtp.gmail.com with ESMTPSA id c24-20020a637258000000b0055c178a8df1sm6537023pgn.94.2023.10.10.08.59.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 10 Oct 2023 08:59:33 -0700 (PDT) From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> To: lpieralisi@kernel.org, kw@linux.com, bhelgaas@google.com Cc: robh@kernel.org, gustavo.pimentel@synopsys.com, jingoohan1@gmail.com, andersson@kernel.org, konrad.dybcio@linaro.org, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Subject: [PATCH v2 1/2] PCI: dwc: Add host_post_init() callback Date: Tue, 10 Oct 2023 21:29:13 +0530 Message-Id: <20231010155914.9516-2-manivannan.sadhasivam@linaro.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20231010155914.9516-1-manivannan.sadhasivam@linaro.org> References: <20231010155914.9516-1-manivannan.sadhasivam@linaro.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=2.7 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, RCVD_IN_SBL_CSS,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Tue, 10 Oct 2023 09:00:01 -0700 (PDT) X-Spam-Level: ** X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1779384885483145934 X-GMAIL-MSGID: 1779384885483145934 |
Series |
PCI: qcom: Enable ASPM on host bridge and devices
|
|
Commit Message
Manivannan Sadhasivam
Oct. 10, 2023, 3:59 p.m. UTC
This callback can be used by the platform drivers to do configuration once
all the devices are scanned. Like changing LNKCTL of all downstream devices
to enable ASPM etc...
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
drivers/pci/controller/dwc/pcie-designware-host.c | 3 +++
drivers/pci/controller/dwc/pcie-designware.h | 1 +
2 files changed, 4 insertions(+)
Comments
On Tue, Oct 10, 2023 at 09:29:13PM +0530, Manivannan Sadhasivam wrote: > This callback can be used by the platform drivers to do configuration once > all the devices are scanned. Like changing LNKCTL of all downstream devices > to enable ASPM etc... > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > --- > drivers/pci/controller/dwc/pcie-designware-host.c | 3 +++ > drivers/pci/controller/dwc/pcie-designware.h | 1 + > 2 files changed, 4 insertions(+) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > index a7170fd0e847..7991f0e179b2 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > @@ -502,6 +502,9 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) > if (ret) > goto err_stop_link; > > + if (pp->ops->host_post_init) > + pp->ops->host_post_init(pp); I know we talked about this a little bit in the context of enabling ASPM for devices below qcom 1.9.0 controllers at https://lore.kernel.org/r/20231011050058.GC3508@thinkpad But I didn't realize at the time that this callback adds a fundamental difference between devices present at boot-time (these devices can be affected by this callback) and any devices hot-added later (we never run this callback again, so anything done by .host_post_init() never applies to them). We merged this for now, and it helps enable ASPM for builtin devices on qcom, but I don't feel good about this from a larger DWC perspective. If other drivers use this and they support hot-add, I think we're going to have problems. > return 0; > > err_stop_link: > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > index ef0b2efa9f93..efb4d4754fc8 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.h > +++ b/drivers/pci/controller/dwc/pcie-designware.h > @@ -301,6 +301,7 @@ enum dw_pcie_ltssm { > struct dw_pcie_host_ops { > int (*host_init)(struct dw_pcie_rp *pp); > void (*host_deinit)(struct dw_pcie_rp *pp); > + void (*host_post_init)(struct dw_pcie_rp *pp); > int (*msi_host_init)(struct dw_pcie_rp *pp); > void (*pme_turn_off)(struct dw_pcie_rp *pp); > }; > -- > 2.25.1 >
On Mon, Oct 16, 2023 at 03:58:49PM -0500, Bjorn Helgaas wrote: > On Tue, Oct 10, 2023 at 09:29:13PM +0530, Manivannan Sadhasivam wrote: > > This callback can be used by the platform drivers to do configuration once > > all the devices are scanned. Like changing LNKCTL of all downstream devices > > to enable ASPM etc... > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > --- > > drivers/pci/controller/dwc/pcie-designware-host.c | 3 +++ > > drivers/pci/controller/dwc/pcie-designware.h | 1 + > > 2 files changed, 4 insertions(+) > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > > index a7170fd0e847..7991f0e179b2 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > > @@ -502,6 +502,9 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) > > if (ret) > > goto err_stop_link; > > > > + if (pp->ops->host_post_init) > > + pp->ops->host_post_init(pp); > > I know we talked about this a little bit in the context of enabling > ASPM for devices below qcom 1.9.0 controllers at > https://lore.kernel.org/r/20231011050058.GC3508@thinkpad > > But I didn't realize at the time that this callback adds a fundamental > difference between devices present at boot-time (these devices can be > affected by this callback) and any devices hot-added later (we never > run this callback again, so anything done by .host_post_init() never > applies to them). > > We merged this for now, and it helps enable ASPM for builtin devices > on qcom, but I don't feel good about this from a larger DWC > perspective. If other drivers use this and they support hot-add, I > think we're going to have problems. > If someone is going to add same ASPM code in host_post_init() callback, they will most likely aware of the hotplug issue. I see this as an interim solution overall and we should fix the PCI core to handle this. But I do not see any straightforward way to enable ASPM by default in PCI core as the misbehaving devices can pull the system down (atleast in some x86 cases). - Mani
On Tue, Oct 17, 2023 at 01:29:52PM +0530, Manivannan Sadhasivam wrote: > On Mon, Oct 16, 2023 at 03:58:49PM -0500, Bjorn Helgaas wrote: > > On Tue, Oct 10, 2023 at 09:29:13PM +0530, Manivannan Sadhasivam wrote: > > > This callback can be used by the platform drivers to do configuration once > > > all the devices are scanned. Like changing LNKCTL of all downstream devices > > > to enable ASPM etc... > > > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > > --- > > > drivers/pci/controller/dwc/pcie-designware-host.c | 3 +++ > > > drivers/pci/controller/dwc/pcie-designware.h | 1 + > > > 2 files changed, 4 insertions(+) > > > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > > > index a7170fd0e847..7991f0e179b2 100644 > > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > > > @@ -502,6 +502,9 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) > > > if (ret) > > > goto err_stop_link; > > > > > > + if (pp->ops->host_post_init) > > > + pp->ops->host_post_init(pp); > > > > I know we talked about this a little bit in the context of enabling > > ASPM for devices below qcom 1.9.0 controllers at > > https://lore.kernel.org/r/20231011050058.GC3508@thinkpad > > > > But I didn't realize at the time that this callback adds a fundamental > > difference between devices present at boot-time (these devices can be > > affected by this callback) and any devices hot-added later (we never > > run this callback again, so anything done by .host_post_init() never > > applies to them). > > > > We merged this for now, and it helps enable ASPM for builtin devices > > on qcom, but I don't feel good about this from a larger DWC > > perspective. If other drivers use this and they support hot-add, I > > think we're going to have problems. > > If someone is going to add same ASPM code in host_post_init() > callback, they will most likely aware of the hotplug issue. I see > this as an interim solution overall and we should fix the PCI core > to handle this. But I do not see any straightforward way to enable > ASPM by default in PCI core as the misbehaving devices can pull the > system down (atleast in some x86 cases). Definitely an interim solution, but the interim is going to be long :) I don't see the PCI core ASPM issue being resolved very soon; it's been this way ever since the beginning. I guess there's no point in me whining without any proposals. Bjorn
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c index a7170fd0e847..7991f0e179b2 100644 --- a/drivers/pci/controller/dwc/pcie-designware-host.c +++ b/drivers/pci/controller/dwc/pcie-designware-host.c @@ -502,6 +502,9 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) if (ret) goto err_stop_link; + if (pp->ops->host_post_init) + pp->ops->host_post_init(pp); + return 0; err_stop_link: diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h index ef0b2efa9f93..efb4d4754fc8 100644 --- a/drivers/pci/controller/dwc/pcie-designware.h +++ b/drivers/pci/controller/dwc/pcie-designware.h @@ -301,6 +301,7 @@ enum dw_pcie_ltssm { struct dw_pcie_host_ops { int (*host_init)(struct dw_pcie_rp *pp); void (*host_deinit)(struct dw_pcie_rp *pp); + void (*host_post_init)(struct dw_pcie_rp *pp); int (*msi_host_init)(struct dw_pcie_rp *pp); void (*pme_turn_off)(struct dw_pcie_rp *pp); };