| Message ID | 20221019113552.22353-9-johan+linaro@kernel.org |
|---|---|
| State | New |
| Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org>
Delivered-To: ouuuleilei@gmail.com
Received: by 2002:a5d:4ac7:0:0:0:0:0 with SMTP id y7csp283731wrs;
Wed, 19 Oct 2022 05:05:09 -0700 (PDT)
X-Google-Smtp-Source:
AMsMyM7eyfygFcI3DdW92H2BDVy3nToCB4q3DpzA2OSz9j17iGXx0hyT4ihjdftFQcl54w7ayFwQ
X-Received: by 2002:a17:902:868b:b0:185:be8:b316 with SMTP id
g11-20020a170902868b00b001850be8b316mr7966388plo.157.1666181109354;
Wed, 19 Oct 2022 05:05:09 -0700 (PDT)
ARC-Seal: i=1; a=rsa-sha256; t=1666181109; cv=none;
d=google.com; s=arc-20160816;
b=T+qzWZSLSaKFlFsqaLZC9UtRdfLzHjwSpeANNbYSD5v2lopMDSRzz/mQehqWEjHMwT
oNCHJ3kd1XNqB9lfaoVxI8aTb4gSwaDZcdYLJfCTTTDfiJAB7u3lJkThQf4Q0jHcnQqD
uq/6B0UQ4ui0eCxvHw1XOwNmEFj4ci6xqOMeWZsmYaACp/n0kjl2k3ALtrhJqSunqScd
YgQnKWcLt639cTL6iJsuhYF413LNKftX+aiY1oIM7bks1torTAMW7D+zybM/zpYSGIXb
Mmev3a1a92xr7FuwEiZPUX3InRdlP0tRAwCrQBM2Ubq19yo165Bm+an26avErOJM0KMU
/kNg==
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=ISjdUCt69z1OIDsskx9CpaZ65I85hFZSMnzuvdZS/LM=;
b=sSLCWTHDRcDZ8saNz8VH9JaiepvsvMnyKzWN+AiHPAKLFdYAOjWQN9qzdh+xCUCsFN
iU18nQESJ0iCwmfYcVskDYa7jJaPYH2v2LGNfBFCuG750HrFyCiOI/3w2WlwFasul5ll
BKAl3Pk2yEOkS5PbMQ1fUun99V0iYUHkfS/IMC0ghw3fBPJzZ0rjs+x2sP3RWuqW/BvC
dyF185FS07tH1Hs2RKAwKWyxHSyqM418p3OYH6dMPxZvRLHZme3/9/QHlvZVC4y6P65s
K0fbklaTeeu5TJg2zcVYT6NTpPzsBt2EciEcH0zkV/nu8WL2THSMBd9K2po++GSSQJA1
SnBg==
ARC-Authentication-Results: i=1; mx.google.com;
dkim=pass header.i=@kernel.org header.s=k20201202 header.b=WlQPUSmf;
spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
smtp.mailfrom=linux-kernel-owner@vger.kernel.org;
dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org
Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20])
by mx.google.com with ESMTP id
73-20020a63014c000000b0045b96ba46desi19022691pgb.792.2022.10.19.05.04.54;
Wed, 19 Oct 2022 05:05:09 -0700 (PDT)
Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
client-ip=2620:137:e000::1:20;
Authentication-Results: mx.google.com;
dkim=pass header.i=@kernel.org header.s=k20201202 header.b=WlQPUSmf;
spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
smtp.mailfrom=linux-kernel-owner@vger.kernel.org;
dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
id S232035AbiJSMBC (ORCPT <rfc822;samuel.l.nystrom@gmail.com>
+ 99 others); Wed, 19 Oct 2022 08:01:02 -0400
Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44490 "EHLO
lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
with ESMTP id S232228AbiJSL7o (ORCPT
<rfc822;linux-kernel@vger.kernel.org>);
Wed, 19 Oct 2022 07:59:44 -0400
Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217])
by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 466B01BF235;
Wed, 19 Oct 2022 04:37:59 -0700 (PDT)
Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(No client certificate requested)
by dfw.source.kernel.org (Postfix) with ESMTPS id 5EC9460A67;
Wed, 19 Oct 2022 11:36:20 +0000 (UTC)
Received: by smtp.kernel.org (Postfix) with ESMTPSA id 460FCC43159;
Wed, 19 Oct 2022 11:36:19 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;
s=k20201202; t=1666179379;
bh=MJlxcxMDosqe41yr9HR9iZCzCceKjo0OJDOMdw8Qp4o=;
h=From:To:Cc:Subject:Date:In-Reply-To:References:From;
b=WlQPUSmf8PQUTXLdt47ojgHBDnPlEkGFs1yTMWQUTkBVCe8YKm0sba+yCvswuK8An
kjSjkEhoDK4I0OCvhJYNff3ZQb2G7ak/wY/KwQAVovGuMUCpsLL01KN4YSmZjuse1Z
wIlHGRzbEmcpyrYCUAeMQSJNd/TWtt7ofJVWHm+XyIo/b2vx/vC1CQPdqNAhcWbCgH
LpfS3DLoVQkHuyaIU8KBBY63Yut5J/TRfF3dsK1e3PQXPbXkegg4v9sA1X7hxGJxrY
/UD2l7X7tcIgWioupQfBFkIUClV3jYKHzv5vhN+h9TlkHKysi1opRGQtFc0ZWaNHXG
1WBDP0Ofc3P3A==
Received: from johan by xi.lan with local (Exim 4.94.2)
(envelope-from <johan+linaro@kernel.org>)
id 1ol7Ml-0005pY-TW; Wed, 19 Oct 2022 13:36:07 +0200
From: Johan Hovold <johan+linaro@kernel.org>
To: Vinod Koul <vkoul@kernel.org>
Cc: Andy Gross <agross@kernel.org>,
Bjorn Andersson <andersson@kernel.org>,
Konrad Dybcio <konrad.dybcio@somainline.org>,
Rob Herring <robh+dt@kernel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
linux-arm-msm@vger.kernel.org, linux-phy@lists.infradead.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
Johan Hovold <johan+linaro@kernel.org>
Subject: [PATCH v2 08/15] phy: qcom-qmp-pcie: add register init helper
Date: Wed, 19 Oct 2022 13:35:45 +0200
Message-Id: <20221019113552.22353-9-johan+linaro@kernel.org>
X-Mailer: git-send-email 2.37.3
In-Reply-To: <20221019113552.22353-1-johan+linaro@kernel.org>
References: <20221019113552.22353-1-johan+linaro@kernel.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI,
SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?=
X-GMAIL-THRID: =?utf-8?q?1747117523173884776?=
X-GMAIL-MSGID: =?utf-8?q?1747117523173884776?=
|
| Series |
phy: qcom-qmp-pcie: add support for sc8280xp
|
|
Commit Message
Johan Hovold
Oct. 19, 2022, 11:35 a.m. UTC
Generalise the serdes initialisation helper so that it can be used to
initialise all the PHY registers (e.g. serdes, tx, rx, pcs).
Note that this defers the ungating of the PIPE clock somewhat, which is
fine as it isn't needed until starting the PHY.
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 51 +++++++-----------------
1 file changed, 15 insertions(+), 36 deletions(-)
Comments
On 19/10/2022 14:35, Johan Hovold wrote: > Generalise the serdes initialisation helper so that it can be used to > initialise all the PHY registers (e.g. serdes, tx, rx, pcs). > > Note that this defers the ungating of the PIPE clock somewhat, which is > fine as it isn't needed until starting the PHY. > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > --- > drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 51 +++++++----------------- > 1 file changed, 15 insertions(+), 36 deletions(-) > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > index dd7e72424fc0..f57d10f20277 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > @@ -1820,46 +1820,32 @@ static void qmp_pcie_configure(void __iomem *base, > qmp_pcie_configure_lane(base, tbl, num, 0xff); > } > > -static void qmp_pcie_serdes_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tables) > -{ > - void __iomem *serdes = qmp->serdes; > - > - if (!tables) > - return; > - > - qmp_pcie_configure(serdes, tables->serdes, tables->serdes_num); > -} > - > -static void qmp_pcie_lanes_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tables) > +static void qmp_pcie_init_registers(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tbls) > { > const struct qmp_phy_cfg *cfg = qmp->cfg; > + void __iomem *serdes = qmp->serdes; > void __iomem *tx = qmp->tx; > void __iomem *rx = qmp->rx; > void __iomem *tx2 = qmp->tx2; > void __iomem *rx2 = qmp->rx2; > + void __iomem *pcs = qmp->pcs; > + void __iomem *pcs_misc = qmp->pcs_misc; > > - if (!tables) > + if (!tbls) > return; > > - qmp_pcie_configure_lane(tx, tables->tx, tables->tx_num, 1); > - qmp_pcie_configure_lane(rx, tables->rx, tables->rx_num, 1); > + qmp_pcie_configure(serdes, tbls->serdes, tbls->serdes_num); > + > + qmp_pcie_configure_lane(tx, tbls->tx, tbls->tx_num, 1); > + qmp_pcie_configure_lane(rx, tbls->rx, tbls->rx_num, 1); > > if (cfg->lanes >= 2) { > - qmp_pcie_configure_lane(tx2, tables->tx, tables->tx_num, 2); > - qmp_pcie_configure_lane(rx2, tables->rx, tables->rx_num, 2); > + qmp_pcie_configure_lane(tx2, tbls->tx, tbls->tx_num, 2); > + qmp_pcie_configure_lane(rx2, tbls->rx, tbls->rx_num, 2); > } > -} > - > -static void qmp_pcie_pcs_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tables) > -{ > - void __iomem *pcs = qmp->pcs; > - void __iomem *pcs_misc = qmp->pcs_misc; > - > - if (!tables) > - return; > > - qmp_pcie_configure(pcs, tables->pcs, tables->pcs_num); > - qmp_pcie_configure(pcs_misc, tables->pcs_misc, tables->pcs_misc_num); > + qmp_pcie_configure(pcs, tbls->pcs, tbls->pcs_num); > + qmp_pcie_configure(pcs_misc, tbls->pcs_misc, tbls->pcs_misc_num); Nit: could we please keep it as `tables'? > } > > static int qmp_pcie_init(struct phy *phy) > @@ -1932,8 +1918,8 @@ static int qmp_pcie_power_on(struct phy *phy) > else > mode_tables = cfg->tables_ep; > > - qmp_pcie_serdes_init(qmp, &cfg->tables); > - qmp_pcie_serdes_init(qmp, mode_tables); > + qmp_pcie_init_registers(qmp, &cfg->tables); > + qmp_pcie_init_registers(qmp, mode_tables); > > ret = clk_prepare_enable(qmp->pipe_clk); > if (ret) { > @@ -1941,13 +1927,6 @@ static int qmp_pcie_power_on(struct phy *phy) > return ret; > } > > - /* Tx, Rx, and PCS configurations */ > - qmp_pcie_lanes_init(qmp, &cfg->tables); > - qmp_pcie_lanes_init(qmp, mode_tables); > - > - qmp_pcie_pcs_init(qmp, &cfg->tables); > - qmp_pcie_pcs_init(qmp, mode_tables); > - > /* Pull PHY out of reset state */ > qphy_clrbits(pcs, cfg->regs[QPHY_SW_RESET], SW_RESET); >
On Wed, Oct 19, 2022 at 04:12:02PM +0300, Dmitry Baryshkov wrote: > On 19/10/2022 14:35, Johan Hovold wrote: > > Generalise the serdes initialisation helper so that it can be used to > > initialise all the PHY registers (e.g. serdes, tx, rx, pcs). > > > > Note that this defers the ungating of the PIPE clock somewhat, which is > > fine as it isn't needed until starting the PHY. > > > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > > --- > > drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 51 +++++++----------------- > > 1 file changed, 15 insertions(+), 36 deletions(-) > > > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > > index dd7e72424fc0..f57d10f20277 100644 > > --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > > @@ -1820,46 +1820,32 @@ static void qmp_pcie_configure(void __iomem *base, > > qmp_pcie_configure_lane(base, tbl, num, 0xff); > > } > > > > -static void qmp_pcie_serdes_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tables) > > -{ > > - void __iomem *serdes = qmp->serdes; > > - > > - if (!tables) > > - return; > > - > > - qmp_pcie_configure(serdes, tables->serdes, tables->serdes_num); > > -} > > - > > -static void qmp_pcie_lanes_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tables) > > +static void qmp_pcie_init_registers(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tbls) > > { > > const struct qmp_phy_cfg *cfg = qmp->cfg; > > + void __iomem *serdes = qmp->serdes; > > void __iomem *tx = qmp->tx; > > void __iomem *rx = qmp->rx; > > void __iomem *tx2 = qmp->tx2; > > void __iomem *rx2 = qmp->rx2; > > + void __iomem *pcs = qmp->pcs; > > + void __iomem *pcs_misc = qmp->pcs_misc; > > > > - if (!tables) > > + if (!tbls) > > return; > > > > - qmp_pcie_configure_lane(tx, tables->tx, tables->tx_num, 1); > > - qmp_pcie_configure_lane(rx, tables->rx, tables->rx_num, 1); > > + qmp_pcie_configure(serdes, tbls->serdes, tbls->serdes_num); > > + > > + qmp_pcie_configure_lane(tx, tbls->tx, tbls->tx_num, 1); > > + qmp_pcie_configure_lane(rx, tbls->rx, tbls->rx_num, 1); > > > > if (cfg->lanes >= 2) { > > - qmp_pcie_configure_lane(tx2, tables->tx, tables->tx_num, 2); > > - qmp_pcie_configure_lane(rx2, tables->rx, tables->rx_num, 2); > > + qmp_pcie_configure_lane(tx2, tbls->tx, tbls->tx_num, 2); > > + qmp_pcie_configure_lane(rx2, tbls->rx, tbls->rx_num, 2); > > } > > -} > > - > > -static void qmp_pcie_pcs_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tables) > > -{ > > - void __iomem *pcs = qmp->pcs; > > - void __iomem *pcs_misc = qmp->pcs_misc; > > - > > - if (!tables) > > - return; > > > > - qmp_pcie_configure(pcs, tables->pcs, tables->pcs_num); > > - qmp_pcie_configure(pcs_misc, tables->pcs_misc, tables->pcs_misc_num); > > + qmp_pcie_configure(pcs, tbls->pcs, tbls->pcs_num); > > + qmp_pcie_configure(pcs_misc, tbls->pcs_misc, tbls->pcs_misc_num); > > Nit: could we please keep it as `tables'? I considered that but found that the longer identifier hurt readability so I prefer to use "tbls" here. Compare qmp_pcie_configure(serdes, tbls->serdes, tbls->serdes_num); qmp_pcie_configure_lane(tx, tbls->tx, tbls->tx_num, 1); qmp_pcie_configure_lane(rx, tbls->rx, tbls->rx_num, 1); if (cfg->lanes >= 2) { qmp_pcie_configure_lane(tx2, tbls->tx, tbls->tx_num, 2); qmp_pcie_configure_lane(rx2, tbls->rx, tbls->rx_num, 2); } qmp_pcie_configure(pcs, tbls->pcs, tbls->pcs_num); qmp_pcie_configure(pcs_misc, tbls->pcs_misc, tbls->pcs_misc_num); with qmp_pcie_configure(serdes, tables->serdes, tables->serdes_num); qmp_pcie_configure_lane(tx, tables->tx, tables->tx_num, 1); qmp_pcie_configure_lane(rx, tables->rx, tables->rx_num, 1); if (cfg->lanes >= 2) { qmp_pcie_configure_lane(tx2, tables->tx, tables->tx_num, 2); qmp_pcie_configure_lane(rx2, tables->rx, tables->rx_num, 2); } qmp_pcie_configure(pcs, tables->pcs, tables->pcs_num); qmp_pcie_configure(pcs_misc, tables->pcs_misc, tables->pcs_misc_num); Johan
On 19/10/2022 16:25, Johan Hovold wrote: > On Wed, Oct 19, 2022 at 04:12:02PM +0300, Dmitry Baryshkov wrote: >> On 19/10/2022 14:35, Johan Hovold wrote: >>> Generalise the serdes initialisation helper so that it can be used to >>> initialise all the PHY registers (e.g. serdes, tx, rx, pcs). >>> >>> Note that this defers the ungating of the PIPE clock somewhat, which is >>> fine as it isn't needed until starting the PHY. >>> >>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org> >>> --- >>> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 51 +++++++----------------- >>> 1 file changed, 15 insertions(+), 36 deletions(-) >>> >>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c >>> index dd7e72424fc0..f57d10f20277 100644 >>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c >>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c >>> @@ -1820,46 +1820,32 @@ static void qmp_pcie_configure(void __iomem *base, >>> qmp_pcie_configure_lane(base, tbl, num, 0xff); >>> } >>> >>> -static void qmp_pcie_serdes_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tables) >>> -{ >>> - void __iomem *serdes = qmp->serdes; >>> - >>> - if (!tables) >>> - return; >>> - >>> - qmp_pcie_configure(serdes, tables->serdes, tables->serdes_num); >>> -} >>> - >>> -static void qmp_pcie_lanes_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tables) >>> +static void qmp_pcie_init_registers(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tbls) >>> { >>> const struct qmp_phy_cfg *cfg = qmp->cfg; >>> + void __iomem *serdes = qmp->serdes; >>> void __iomem *tx = qmp->tx; >>> void __iomem *rx = qmp->rx; >>> void __iomem *tx2 = qmp->tx2; >>> void __iomem *rx2 = qmp->rx2; >>> + void __iomem *pcs = qmp->pcs; >>> + void __iomem *pcs_misc = qmp->pcs_misc; >>> >>> - if (!tables) >>> + if (!tbls) >>> return; >>> >>> - qmp_pcie_configure_lane(tx, tables->tx, tables->tx_num, 1); >>> - qmp_pcie_configure_lane(rx, tables->rx, tables->rx_num, 1); >>> + qmp_pcie_configure(serdes, tbls->serdes, tbls->serdes_num); >>> + >>> + qmp_pcie_configure_lane(tx, tbls->tx, tbls->tx_num, 1); >>> + qmp_pcie_configure_lane(rx, tbls->rx, tbls->rx_num, 1); >>> >>> if (cfg->lanes >= 2) { >>> - qmp_pcie_configure_lane(tx2, tables->tx, tables->tx_num, 2); >>> - qmp_pcie_configure_lane(rx2, tables->rx, tables->rx_num, 2); >>> + qmp_pcie_configure_lane(tx2, tbls->tx, tbls->tx_num, 2); >>> + qmp_pcie_configure_lane(rx2, tbls->rx, tbls->rx_num, 2); >>> } >>> -} >>> - >>> -static void qmp_pcie_pcs_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tables) >>> -{ >>> - void __iomem *pcs = qmp->pcs; >>> - void __iomem *pcs_misc = qmp->pcs_misc; >>> - >>> - if (!tables) >>> - return; >>> >>> - qmp_pcie_configure(pcs, tables->pcs, tables->pcs_num); >>> - qmp_pcie_configure(pcs_misc, tables->pcs_misc, tables->pcs_misc_num); >>> + qmp_pcie_configure(pcs, tbls->pcs, tbls->pcs_num); >>> + qmp_pcie_configure(pcs_misc, tbls->pcs_misc, tbls->pcs_misc_num); >> >> Nit: could we please keep it as `tables'? > > I considered that but found that the longer identifier hurt > readability so I prefer to use "tbls" here. > > Compare > > qmp_pcie_configure(serdes, tbls->serdes, tbls->serdes_num); > > qmp_pcie_configure_lane(tx, tbls->tx, tbls->tx_num, 1); > qmp_pcie_configure_lane(rx, tbls->rx, tbls->rx_num, 1); > > if (cfg->lanes >= 2) { > qmp_pcie_configure_lane(tx2, tbls->tx, tbls->tx_num, 2); > qmp_pcie_configure_lane(rx2, tbls->rx, tbls->rx_num, 2); > } > > qmp_pcie_configure(pcs, tbls->pcs, tbls->pcs_num); > qmp_pcie_configure(pcs_misc, tbls->pcs_misc, tbls->pcs_misc_num); > > with > > qmp_pcie_configure(serdes, tables->serdes, tables->serdes_num); > > qmp_pcie_configure_lane(tx, tables->tx, tables->tx_num, 1); > qmp_pcie_configure_lane(rx, tables->rx, tables->rx_num, 1); > > if (cfg->lanes >= 2) { > qmp_pcie_configure_lane(tx2, tables->tx, tables->tx_num, 2); > qmp_pcie_configure_lane(rx2, tables->rx, tables->rx_num, 2); > } > > qmp_pcie_configure(pcs, tables->pcs, tables->pcs_num); > qmp_pcie_configure(pcs_misc, tables->pcs_misc, tables->pcs_misc_num); I'd say, it's up to Vinod. I'd prefer the second version.
On Wed, Oct 19, 2022 at 04:44:33PM +0300, Dmitry Baryshkov wrote: > On 19/10/2022 16:25, Johan Hovold wrote: > > On Wed, Oct 19, 2022 at 04:12:02PM +0300, Dmitry Baryshkov wrote: > >> On 19/10/2022 14:35, Johan Hovold wrote: > >>> Generalise the serdes initialisation helper so that it can be used to > >>> initialise all the PHY registers (e.g. serdes, tx, rx, pcs). > >>> > >>> Note that this defers the ungating of the PIPE clock somewhat, which is > >>> fine as it isn't needed until starting the PHY. > >>> > >>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > >>> --- > >>> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 51 +++++++----------------- > >>> 1 file changed, 15 insertions(+), 36 deletions(-) > >>> > >>> -static void qmp_pcie_pcs_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tables) > >>> -{ > >>> - void __iomem *pcs = qmp->pcs; > >>> - void __iomem *pcs_misc = qmp->pcs_misc; > >>> - > >>> - if (!tables) > >>> - return; > >>> > >>> - qmp_pcie_configure(pcs, tables->pcs, tables->pcs_num); > >>> - qmp_pcie_configure(pcs_misc, tables->pcs_misc, tables->pcs_misc_num); > >>> + qmp_pcie_configure(pcs, tbls->pcs, tbls->pcs_num); > >>> + qmp_pcie_configure(pcs_misc, tbls->pcs_misc, tbls->pcs_misc_num); > >> > >> Nit: could we please keep it as `tables'? > > > > I considered that but found that the longer identifier hurt > > readability so I prefer to use "tbls" here. > I'd say, it's up to Vinod. I'd prefer the second version. Ultimately yes, but as the author of these patches I do have a say here. Johan
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c index dd7e72424fc0..f57d10f20277 100644 --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c @@ -1820,46 +1820,32 @@ static void qmp_pcie_configure(void __iomem *base, qmp_pcie_configure_lane(base, tbl, num, 0xff); } -static void qmp_pcie_serdes_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tables) -{ - void __iomem *serdes = qmp->serdes; - - if (!tables) - return; - - qmp_pcie_configure(serdes, tables->serdes, tables->serdes_num); -} - -static void qmp_pcie_lanes_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tables) +static void qmp_pcie_init_registers(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tbls) { const struct qmp_phy_cfg *cfg = qmp->cfg; + void __iomem *serdes = qmp->serdes; void __iomem *tx = qmp->tx; void __iomem *rx = qmp->rx; void __iomem *tx2 = qmp->tx2; void __iomem *rx2 = qmp->rx2; + void __iomem *pcs = qmp->pcs; + void __iomem *pcs_misc = qmp->pcs_misc; - if (!tables) + if (!tbls) return; - qmp_pcie_configure_lane(tx, tables->tx, tables->tx_num, 1); - qmp_pcie_configure_lane(rx, tables->rx, tables->rx_num, 1); + qmp_pcie_configure(serdes, tbls->serdes, tbls->serdes_num); + + qmp_pcie_configure_lane(tx, tbls->tx, tbls->tx_num, 1); + qmp_pcie_configure_lane(rx, tbls->rx, tbls->rx_num, 1); if (cfg->lanes >= 2) { - qmp_pcie_configure_lane(tx2, tables->tx, tables->tx_num, 2); - qmp_pcie_configure_lane(rx2, tables->rx, tables->rx_num, 2); + qmp_pcie_configure_lane(tx2, tbls->tx, tbls->tx_num, 2); + qmp_pcie_configure_lane(rx2, tbls->rx, tbls->rx_num, 2); } -} - -static void qmp_pcie_pcs_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tables) -{ - void __iomem *pcs = qmp->pcs; - void __iomem *pcs_misc = qmp->pcs_misc; - - if (!tables) - return; - qmp_pcie_configure(pcs, tables->pcs, tables->pcs_num); - qmp_pcie_configure(pcs_misc, tables->pcs_misc, tables->pcs_misc_num); + qmp_pcie_configure(pcs, tbls->pcs, tbls->pcs_num); + qmp_pcie_configure(pcs_misc, tbls->pcs_misc, tbls->pcs_misc_num); } static int qmp_pcie_init(struct phy *phy) @@ -1932,8 +1918,8 @@ static int qmp_pcie_power_on(struct phy *phy) else mode_tables = cfg->tables_ep; - qmp_pcie_serdes_init(qmp, &cfg->tables); - qmp_pcie_serdes_init(qmp, mode_tables); + qmp_pcie_init_registers(qmp, &cfg->tables); + qmp_pcie_init_registers(qmp, mode_tables); ret = clk_prepare_enable(qmp->pipe_clk); if (ret) { @@ -1941,13 +1927,6 @@ static int qmp_pcie_power_on(struct phy *phy) return ret; } - /* Tx, Rx, and PCS configurations */ - qmp_pcie_lanes_init(qmp, &cfg->tables); - qmp_pcie_lanes_init(qmp, mode_tables); - - qmp_pcie_pcs_init(qmp, &cfg->tables); - qmp_pcie_pcs_init(qmp, mode_tables); - /* Pull PHY out of reset state */ qphy_clrbits(pcs, cfg->regs[QPHY_SW_RESET], SW_RESET);