Message ID | 20230915120142.32987-5-ilpo.jarvinen@linux.intel.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:172:b0:3f2:4152:657d with SMTP id h50csp1090054vqi; Fri, 15 Sep 2023 07:31:15 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFy9wjpBxRoaP5ndhZ8g46xtKa6V2dx3/Wp9N5Qqq7Wsp0GGdFVGzP58d+moRtS4eQ6jqdh X-Received: by 2002:a05:6a21:819e:b0:156:dc22:96a6 with SMTP id pd30-20020a056a21819e00b00156dc2296a6mr1868024pzb.55.1694788275624; Fri, 15 Sep 2023 07:31:15 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1694788275; cv=none; d=google.com; s=arc-20160816; b=Odudep+g6mp6cKehDFjpj2KBS4jmmpJGnguwYp2dxz+43y/nYVJ/mdVAON+rEbZICE XmIf5dpDCAiwF0MVgrOJ8KgN86QPzt8XxSsgWmDYtPcRDuChzieL9nh3IdLct4sJu4d/ kw3h6JZ81o2sDpQT5XKOTwgzhWQBCXa6gNJlAC/soJXGsfOoARFumQTFx0HHQ4FLFWsJ Trs8o7Vz8AWm7rVc2A/XBj6RYEdLsGq3zWM+qDiO4H198q699+7VZDlg/Vi9vLoRgnQl aT9imCkIIO3Kv4EwnTLBYjb/IxRTYGM7ALo+37o9q2lCXW31IpT3sVlFlpI0Gu2Z8fUn zRCw== 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=pg8QQd3oUNwuGsFtCkXbHNhXBNZ8wqylw1RFmobdRio=; fh=EQ0ZbK9Vy/KsIBY82DxAU9xsXan7gkzuqiYJ8lQ6f64=; b=exM21MjUjKpUf3B8LPNiJCEGu1PRTTe/H3nuQ3dZYRhBC6sktkRfdzOFMqC95ZJrpq AIA84cA+myDUSI+Dj8AXa3aGmP2OblRpeIlm+zHY/qlUDU1PAdZUWdmrepuNYtB30ZO7 yjecxA16doR1XjyHlpB4nE5wW6oEbsIRAmMGAt4MyOil7e37SWSSQ7fali7NsVOM0zQv dAXe5kd4m1CFOgwDSV0o+7NnrVcLDWRBod9ehbwxxOa+M8sJRKooVaneTkXuEGDdKL5s 1UjyA4gfrXJO9N0F6RIa71fpK737f8aEmgUgnezOBzmyb8BMAJ2pD+HiPJetbmVomsVA 3maA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=PD0Pbphd; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from groat.vger.email (groat.vger.email. [2620:137:e000::3:5]) by mx.google.com with ESMTPS id m7-20020a056a00080700b0068fb986fcf3si3581693pfk.377.2023.09.15.07.31.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 15 Sep 2023 07:31:15 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=PD0Pbphd; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by groat.vger.email (Postfix) with ESMTP id 6755E83CB5BA; Fri, 15 Sep 2023 05:03:35 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at groat.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234432AbjIOMDK (ORCPT <rfc822;ruipengqi7@gmail.com> + 32 others); Fri, 15 Sep 2023 08:03:10 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55688 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232836AbjIOMDJ (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 15 Sep 2023 08:03:09 -0400 Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.120]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0F37B2722; Fri, 15 Sep 2023 05:02:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1694779365; x=1726315365; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=vDsDIHqtLtfNmUG3Y8t5VDuHkp8RJgeZagGcp/a8iuA=; b=PD0Pbphd+cTrXrBvdDVaWpF4myBOn9G1gLBVBDyobay5CxrmEKWRdZ2Q uNwoCAp//+s9Mu8gny17P13Q9g0MdpUipcjuqUFBn9n2LI7YoLFsy8ozP rJirOo8WNVjdm9ZRKwIlA7oQ5pxpvCnLnuge/hEqB0qSVvHWXR4wtKK8X RhJXm8RZxIlHbB+ER9GUxQDyGPe3BlsHkwXfZ3CVaKNtKlUNTq7sXpa7d f3VAKlTLy6HE9/MGAF0HZnq895dqlZJek1tMBRnX7YQ339zDQl0MyzYBz IUGpuhzuWtwDfmZyQ0C7IZiD98i4iHHZti+uNtqCG0s0QaBHAvsZmsaSL g==; X-IronPort-AV: E=McAfee;i="6600,9927,10833"; a="378146017" X-IronPort-AV: E=Sophos;i="6.02,149,1688454000"; d="scan'208";a="378146017" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Sep 2023 05:02:22 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10833"; a="774292848" X-IronPort-AV: E=Sophos;i="6.02,149,1688454000"; d="scan'208";a="774292848" Received: from srdoo-mobl1.ger.corp.intel.com (HELO ijarvine-mobl2.ger.corp.intel.com) ([10.252.38.99]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Sep 2023 05:02:14 -0700 From: =?utf-8?q?Ilpo_J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com> To: linux-pci@vger.kernel.org, Bjorn Helgaas <helgaas@kernel.org>, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, Rob Herring <robh@kernel.org>, =?utf-8?q?Krzysztof_Wilczy=C5=84ski?= <kw@linux.com>, Lukas Wunner <lukas@wunner.de>, Alexandru Gagniuc <mr.nuke.me@gmail.com>, Krishna chaitanya chundru <quic_krichai@quicinc.com>, Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>, "Rafael J . Wysocki" <rafael@kernel.org>, linux-pm@vger.kernel.org, Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>, Jason Gunthorpe <jgg@ziepe.ca>, Leon Romanovsky <leon@kernel.org>, linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Alex Deucher <alexdeucher@gmail.com>, Daniel Lezcano <daniel.lezcano@linaro.org>, Amit Kucheria <amitk@kernel.org>, Zhang Rui <rui.zhang@intel.com>, =?utf-8?q?Ilpo_J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com> Subject: [PATCH v2 04/10] drm/IB/hfi1: Use RMW accessors for changing LNKCTL2 Date: Fri, 15 Sep 2023 15:01:36 +0300 Message-Id: <20230915120142.32987-5-ilpo.jarvinen@linux.intel.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20230915120142.32987-1-ilpo.jarvinen@linux.intel.com> References: <20230915120142.32987-1-ilpo.jarvinen@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on groat.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 (groat.vger.email [0.0.0.0]); Fri, 15 Sep 2023 05:03:35 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1777114310908362983 X-GMAIL-MSGID: 1777114310908362983 |
Series |
Add PCIe Bandwidth Controller
|
|
Commit Message
Ilpo Järvinen
Sept. 15, 2023, 12:01 p.m. UTC
Don't assume that only the driver would be accessing LNKCTL2. In the
case of upstream (parent), the driver does not even own the device it's
changing the registers for.
Use RMW capability accessors which do proper locking to avoid losing
concurrent updates to the register value. This change is also useful as
a cleanup.
Suggested-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/infiniband/hw/hfi1/pcie.c | 30 ++++++++----------------------
1 file changed, 8 insertions(+), 22 deletions(-)
Comments
On 9/15/2023 7:01 AM, Ilpo Järvinen wrote: > Don't assume that only the driver would be accessing LNKCTL2. In the > case of upstream (parent), the driver does not even own the device it's > changing the registers for. > > Use RMW capability accessors which do proper locking to avoid losing > concurrent updates to the register value. This change is also useful as > a cleanup. > > Suggested-by: Lukas Wunner <lukas@wunner.de> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> I believe the subject should begin with "RDMA/hfi1:", the current expectation for all devices in drivers/infiniband. > --- > drivers/infiniband/hw/hfi1/pcie.c | 30 ++++++++---------------------- > 1 file changed, 8 insertions(+), 22 deletions(-) > > diff --git a/drivers/infiniband/hw/hfi1/pcie.c b/drivers/infiniband/hw/hfi1/pcie.c > index 08732e1ac966..60a177f52eb5 100644 > --- a/drivers/infiniband/hw/hfi1/pcie.c > +++ b/drivers/infiniband/hw/hfi1/pcie.c > @@ -1212,14 +1212,11 @@ int do_pcie_gen3_transition(struct hfi1_devdata *dd) > (u32)lnkctl2); > /* only write to parent if target is not as high as ours */ > if ((lnkctl2 & PCI_EXP_LNKCTL2_TLS) < target_vector) { > - lnkctl2 &= ~PCI_EXP_LNKCTL2_TLS; > - lnkctl2 |= target_vector; > - dd_dev_info(dd, "%s: ..new link control2: 0x%x\n", __func__, > - (u32)lnkctl2); > - ret = pcie_capability_write_word(parent, > - PCI_EXP_LNKCTL2, lnkctl2); > + ret = pcie_capability_clear_and_set_word(parent, PCI_EXP_LNKCTL2, > + PCI_EXP_LNKCTL2_TLS, > + target_vector); > if (ret) { > - dd_dev_err(dd, "Unable to write to PCI config\n"); > + dd_dev_err(dd, "Unable to change PCI target speed\n"); To differentiate from the dev_err below, add "parent", i.e. "Unable to change parent PCI target speed". > return_error = 1; > goto done; > } > @@ -1228,22 +1225,11 @@ int do_pcie_gen3_transition(struct hfi1_devdata *dd) > } > > dd_dev_info(dd, "%s: setting target link speed\n", __func__); > - ret = pcie_capability_read_word(dd->pcidev, PCI_EXP_LNKCTL2, &lnkctl2); > + ret = pcie_capability_clear_and_set_word(dd->pcidev, PCI_EXP_LNKCTL2, > + PCI_EXP_LNKCTL2_TLS, > + target_vector); > if (ret) { > - dd_dev_err(dd, "Unable to read from PCI config\n"); > - return_error = 1; > - goto done; > - } > - > - dd_dev_info(dd, "%s: ..old link control2: 0x%x\n", __func__, > - (u32)lnkctl2); > - lnkctl2 &= ~PCI_EXP_LNKCTL2_TLS; > - lnkctl2 |= target_vector; > - dd_dev_info(dd, "%s: ..new link control2: 0x%x\n", __func__, > - (u32)lnkctl2); > - ret = pcie_capability_write_word(dd->pcidev, PCI_EXP_LNKCTL2, lnkctl2); > - if (ret) { > - dd_dev_err(dd, "Unable to write to PCI config\n"); > + dd_dev_err(dd, "Unable to change PCI target speed\n"); To differentiate from the dev_err above, add "device", i.e. "Unable to change device PCI target speed". > return_error = 1; > goto done; > } External recipient
On Fri, 15 Sep 2023, Dean Luick wrote: > On 9/15/2023 7:01 AM, Ilpo Järvinen wrote: > > Don't assume that only the driver would be accessing LNKCTL2. In the > > case of upstream (parent), the driver does not even own the device it's > > changing the registers for. > > > > Use RMW capability accessors which do proper locking to avoid losing > > concurrent updates to the register value. This change is also useful as > > a cleanup. > > > > Suggested-by: Lukas Wunner <lukas@wunner.de> > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > > I believe the subject should begin with "RDMA/hfi1:", the current expectation for all devices in drivers/infiniband. Thanks, I'll update it. I hadn't realized given the shortlogs of the other commits (no idea where I managed to get that "drm" from, but it's also gone now). > > --- > > drivers/infiniband/hw/hfi1/pcie.c | 30 ++++++++---------------------- > > 1 file changed, 8 insertions(+), 22 deletions(-) > > > > diff --git a/drivers/infiniband/hw/hfi1/pcie.c b/drivers/infiniband/hw/hfi1/pcie.c > > index 08732e1ac966..60a177f52eb5 100644 > > --- a/drivers/infiniband/hw/hfi1/pcie.c > > +++ b/drivers/infiniband/hw/hfi1/pcie.c > > @@ -1212,14 +1212,11 @@ int do_pcie_gen3_transition(struct hfi1_devdata *dd) > > (u32)lnkctl2); > > /* only write to parent if target is not as high as ours */ > > if ((lnkctl2 & PCI_EXP_LNKCTL2_TLS) < target_vector) { > > - lnkctl2 &= ~PCI_EXP_LNKCTL2_TLS; > > - lnkctl2 |= target_vector; > > - dd_dev_info(dd, "%s: ..new link control2: 0x%x\n", __func__, > > - (u32)lnkctl2); > > - ret = pcie_capability_write_word(parent, > > - PCI_EXP_LNKCTL2, lnkctl2); > > + ret = pcie_capability_clear_and_set_word(parent, PCI_EXP_LNKCTL2, > > + PCI_EXP_LNKCTL2_TLS, > > + target_vector); > > if (ret) { > > - dd_dev_err(dd, "Unable to write to PCI config\n"); > > + dd_dev_err(dd, "Unable to change PCI target speed\n"); > > To differentiate from the dev_err below, add "parent", i.e. "Unable to change parent PCI target speed". > > > > return_error = 1; > > goto done; > > } > > @@ -1228,22 +1225,11 @@ int do_pcie_gen3_transition(struct hfi1_devdata *dd) > > } > > > > dd_dev_info(dd, "%s: setting target link speed\n", __func__); > > - ret = pcie_capability_read_word(dd->pcidev, PCI_EXP_LNKCTL2, &lnkctl2); > > + ret = pcie_capability_clear_and_set_word(dd->pcidev, PCI_EXP_LNKCTL2, > > + PCI_EXP_LNKCTL2_TLS, > > + target_vector); > > if (ret) { > > - dd_dev_err(dd, "Unable to read from PCI config\n"); > > - return_error = 1; > > - goto done; > > - } > > - > > - dd_dev_info(dd, "%s: ..old link control2: 0x%x\n", __func__, > > - (u32)lnkctl2); > > - lnkctl2 &= ~PCI_EXP_LNKCTL2_TLS; > > - lnkctl2 |= target_vector; > > - dd_dev_info(dd, "%s: ..new link control2: 0x%x\n", __func__, > > - (u32)lnkctl2); > > - ret = pcie_capability_write_word(dd->pcidev, PCI_EXP_LNKCTL2, lnkctl2); > > - if (ret) { > > - dd_dev_err(dd, "Unable to write to PCI config\n"); > > + dd_dev_err(dd, "Unable to change PCI target speed\n"); > > To differentiate from the dev_err above, add "device", i.e. "Unable to change device PCI target speed". Okay, I'll change those. Thanks for taking a look.
diff --git a/drivers/infiniband/hw/hfi1/pcie.c b/drivers/infiniband/hw/hfi1/pcie.c index 08732e1ac966..60a177f52eb5 100644 --- a/drivers/infiniband/hw/hfi1/pcie.c +++ b/drivers/infiniband/hw/hfi1/pcie.c @@ -1212,14 +1212,11 @@ int do_pcie_gen3_transition(struct hfi1_devdata *dd) (u32)lnkctl2); /* only write to parent if target is not as high as ours */ if ((lnkctl2 & PCI_EXP_LNKCTL2_TLS) < target_vector) { - lnkctl2 &= ~PCI_EXP_LNKCTL2_TLS; - lnkctl2 |= target_vector; - dd_dev_info(dd, "%s: ..new link control2: 0x%x\n", __func__, - (u32)lnkctl2); - ret = pcie_capability_write_word(parent, - PCI_EXP_LNKCTL2, lnkctl2); + ret = pcie_capability_clear_and_set_word(parent, PCI_EXP_LNKCTL2, + PCI_EXP_LNKCTL2_TLS, + target_vector); if (ret) { - dd_dev_err(dd, "Unable to write to PCI config\n"); + dd_dev_err(dd, "Unable to change PCI target speed\n"); return_error = 1; goto done; } @@ -1228,22 +1225,11 @@ int do_pcie_gen3_transition(struct hfi1_devdata *dd) } dd_dev_info(dd, "%s: setting target link speed\n", __func__); - ret = pcie_capability_read_word(dd->pcidev, PCI_EXP_LNKCTL2, &lnkctl2); + ret = pcie_capability_clear_and_set_word(dd->pcidev, PCI_EXP_LNKCTL2, + PCI_EXP_LNKCTL2_TLS, + target_vector); if (ret) { - dd_dev_err(dd, "Unable to read from PCI config\n"); - return_error = 1; - goto done; - } - - dd_dev_info(dd, "%s: ..old link control2: 0x%x\n", __func__, - (u32)lnkctl2); - lnkctl2 &= ~PCI_EXP_LNKCTL2_TLS; - lnkctl2 |= target_vector; - dd_dev_info(dd, "%s: ..new link control2: 0x%x\n", __func__, - (u32)lnkctl2); - ret = pcie_capability_write_word(dd->pcidev, PCI_EXP_LNKCTL2, lnkctl2); - if (ret) { - dd_dev_err(dd, "Unable to write to PCI config\n"); + dd_dev_err(dd, "Unable to change PCI target speed\n"); return_error = 1; goto done; }