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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3d4a8243-5c2f-41c4-85ce-6e072331f4f3@quicinc.com>
Date: Wed, 25 Sep 2024 11:38:46 +0800
From: Qiang Yu <quic_qianyu@...cinc.com>
To: Johan Hovold <johan@...nel.org>
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 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@...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).
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