Re: [PATCH v4 3/6] phy: qcom: qmp: Add phy register and clk setting for x1e80100 PCIe3

Qiang Yu
2024-09-25T03:39:21Z

On 9/24/2024 11:15 PM, Johan Hovold wrote:
> 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.
PCIe3 supports 8 lanes, so in general, we have to program 8 pairs tx/rx
registers. However, most of tx/rx registers of different lanes have
same settings, so the configuration for all 8 lanes tx/rx registers is
a little repetitive.

Hence, txz/rxz registers are included. The values programmed into txz/rxz
registers by software will be "broadcasted" to all 8 lanes by hardware.
Some lanes may have different settings on several registers than txz/rxz.
In order to ensure the different values take effect, they need to be
programmed after txz/rxz programming sequences completing.
>
>> 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@quicinc.com>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> Reviewed-by: Konrad Dybcio <konradybcio@kernel.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).
OK, will follow this order.
>
>> +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).

Will follow 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.

Will add a comment here.

Thanks,
Qiang Yu
>
>> + 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