[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZvLXjdpBpUS3lLn-@hovoldconsulting.com>
Date: Tue, 24 Sep 2024 17:15:25 +0200
From: Johan Hovold <johan@...nel.org>
To: Qiang Yu <quic_qianyu@...cinc.com>
Cc: manivannan.sadhasivam@...aro.org, vkoul@...nel.org, kishon@...nel.org,
robh@...nel.org, andersson@...nel.org, konradybcio@...nel.org,
krzk+dt@...nel.org, conor+dt@...nel.org, mturquette@...libre.com,
sboyd@...nel.org, abel.vesa@...aro.org, quic_msarkar@...cinc.com,
quic_devipriy@...cinc.com, dmitry.baryshkov@...aro.org,
kw@...ux.com, lpieralisi@...nel.org, neil.armstrong@...aro.org,
linux-arm-msm@...r.kernel.org, linux-phy@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
devicetree@...r.kernel.org, linux-clk@...r.kernel.org
Subject: Re: [PATCH v4 3/6] phy: qcom: qmp: Add phy register and clk setting
for x1e80100 PCIe3
On Tue, Sep 24, 2024 at 03:14:41AM -0700, Qiang Yu wrote:
> Currently driver supports only x4 lane based functionality using tx/rx and
> tx2/rx2 pair of register sets. To support 8 lane functionality with PCIe3,
> PCIe3 related QMP PHY provides additional programming which are available
> as txz and rxz based register set. Hence adds txz and rxz based registers
> usage and programming sequences.
> Phy register setting for txz and rxz will
> be applied to all 8 lanes. Some lanes may have different settings on
> several registers than txz/rxz, these registers should be programmed after
> txz/rxz programming sequences completing.
Please expand and clarify what you mean by this.
> Besides, x1e80100 SoC uses QMP phy with version v6.30 for PCIe Gen4 x8.
> Add the new register offsets in a dedicated header file.
>
> Signed-off-by: Qiang Yu <quic_qianyu@...cinc.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> Reviewed-by: Konrad Dybcio <konradybcio@...nel.org>
> ---
> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 211 ++++++++++++++++++
> .../qualcomm/phy-qcom-qmp-pcs-pcie-v6_30.h | 25 +++
> drivers/phy/qualcomm/phy-qcom-qmp-pcs-v6_30.h | 19 ++
> 3 files changed, 255 insertions(+)
> create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_30.h
> create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-pcs-v6_30.h
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> index f71787fb4d7e..d7bbd9df11d7 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> @@ -1344,6 +1346,155 @@ static const struct qmp_phy_init_tbl x1e80100_qmp_gen4x2_pcie_pcs_misc_tbl[] = {
> QMP_PHY_INIT_CFG(QPHY_PCIE_V6_20_PCS_G4_FOM_EQ_CONFIG5, 0x8a),
> };
>
> +static const struct qmp_phy_init_tbl x1e80100_qmp_gen4x8_pcie_txz_tbl[] = {
Please try to follow the sort order used for the other platforms for
these tables (e.g. serdes, tx, rx, pcr, pcr_misc).
> +static const struct qmp_phy_init_tbl x1e80100_qmp_gen4x8_pcie_pcs_misc_tbl[] = {
> + QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_ENDPOINT_REFCLK_DRIVE, 0xc1),
> + QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_OSC_DTCT_ACTIONS, 0x00),
> + QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_EQ_CONFIG1, 0x16),
> + QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_G4_EQ_CONFIG5, 0x02),
> + QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_G4_PRE_GAIN, 0x2e),
> + QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_RX_MARGINING_CONFIG1, 0x03),
> + QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_RX_MARGINING_CONFIG3, 0x28),
> + QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_RX_MARGINING_CONFIG5, 0x18),
> + QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_G3_FOM_EQ_CONFIG5, 0x7a),
> + QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_G4_FOM_EQ_CONFIG5, 0x8a),
> + QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_G3_RXEQEVAL_TIME, 0x27),
> + QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_G4_RXEQEVAL_TIME, 0x27),
> + QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_TX_RX_CONFIG, 0xc0),
> + QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_POWER_STATE_CONFIG2, 0x1d),
> +
Stray newline.
> +};
> +
> +static const struct qmp_phy_cfg x1e80100_qmp_gen4x8_pciephy_cfg = {
> + .lanes = 8,
> +
> + .offsets = &qmp_pcie_offsets_v6_30,
> + .tbls = {
> + .serdes = x1e80100_qmp_gen4x8_pcie_serdes_tbl,
> + .serdes_num = ARRAY_SIZE(x1e80100_qmp_gen4x8_pcie_serdes_tbl),
> + .rx = x1e80100_qmp_gen4x8_pcie_rx_tbl,
> + .rx_num = ARRAY_SIZE(x1e80100_qmp_gen4x8_pcie_rx_tbl),
> + .pcs = x1e80100_qmp_gen4x8_pcie_pcs_tbl,
> + .pcs_num = ARRAY_SIZE(x1e80100_qmp_gen4x8_pcie_pcs_tbl),
> + .pcs_misc = x1e80100_qmp_gen4x8_pcie_pcs_misc_tbl,
> + .pcs_misc_num = ARRAY_SIZE(x1e80100_qmp_gen4x8_pcie_pcs_misc_tbl),
> + .ln_shrd = x1e80100_qmp_gen4x8_pcie_ln_shrd_tbl,
> + .ln_shrd_num = ARRAY_SIZE(x1e80100_qmp_gen4x8_pcie_ln_shrd_tbl),
> + .txz = x1e80100_qmp_gen4x8_pcie_txz_tbl,
> + .txz_num = ARRAY_SIZE(x1e80100_qmp_gen4x8_pcie_txz_tbl),
> + .rxz = x1e80100_qmp_gen4x8_pcie_rxz_tbl,
> + .rxz_num = ARRAY_SIZE(x1e80100_qmp_gen4x8_pcie_rxz_tbl),
Try follow the order of the other SoCs here as well (e.g. use the order
defined in struct qmp_phy_cfg_tbls).
> + },
> static void qmp_pcie_init_port_b(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tbls *tbls)
> {
> const struct qmp_phy_cfg *cfg = qmp->cfg;
> @@ -3751,6 +3953,9 @@ static void qmp_pcie_init_registers(struct qmp_pcie *qmp, const struct qmp_phy_c
>
> qmp_configure(qmp->dev, serdes, tbls->serdes, tbls->serdes_num);
If your comment in the commit message implies that txz/rxz must be
programmed before tx/rx then you need to add a comment here as well.
> + qmp_configure(qmp->dev, qmp->txz, tbls->txz, tbls->txz_num);
> + qmp_configure(qmp->dev, qmp->rxz, tbls->rxz, tbls->rxz_num);
> +
> qmp_configure_lane(qmp->dev, tx, tbls->tx, tbls->tx_num, 1);
> qmp_configure_lane(qmp->dev, rx, tbls->rx, tbls->rx_num, 1);
Johan
Powered by blists - more mailing lists