On 29/11/2024 08:57, Xiangxu Yin wrote:
> Extended DP support for QCS615 USB or DP phy. Differentiated between
> USBC and DP PHY using the match table’s type, dynamically generating
> different types of cfg and layout attributes during initialization based
> on this type. Static variables are stored in cfg, while parsed values
> are organized into the layout structure.
>
> Signed-off-by: Xiangxu Yin <quic_xiangxuy@xxxxxxxxxxx>
> ---
> drivers/phy/qualcomm/phy-qcom-qmp-dp-phy.h | 1 +
> drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 1453 ++++++++++++++++++++++++----
> 2 files changed, 1254 insertions(+), 200 deletions(-)
...
> + /* program default setting first */
> + writel(0x2A, tx + QSERDES_V3_TX_TX_DRV_LVL);
> + writel(0x20, tx + QSERDES_V3_TX_TX_EMP_POST1_LVL);
> + writel(0x2A, tx2 + QSERDES_V3_TX_TX_DRV_LVL);
> + writel(0x20, tx2 + QSERDES_V3_TX_TX_EMP_POST1_LVL);
> +
> + writel(voltage_swing_cfg, tx + QSERDES_V3_TX_TX_DRV_LVL);
> + writel(pre_emphasis_cfg, tx + QSERDES_V3_TX_TX_EMP_POST1_LVL);
> + writel(voltage_swing_cfg, tx2 + QSERDES_V3_TX_TX_DRV_LVL);
> + writel(pre_emphasis_cfg, tx2 + QSERDES_V3_TX_TX_EMP_POST1_LVL);
> +
> + return 0;
> +}
> +
> +static int qcs615_qmp_configure_dp_phy(struct qmp_usbc *qmp)
> +{
> + struct qmp_phy_dp_layout *layout = to_dp_layout(qmp);
> + u32 status;
> +
> + writel(0x01, layout->dp_phy + QSERDES_DP_PHY_CFG);
> + writel(0x05, layout->dp_phy + QSERDES_DP_PHY_CFG);
> + writel(0x01, layout->dp_phy + QSERDES_DP_PHY_CFG);
> + writel(0x09, layout->dp_phy + QSERDES_DP_PHY_CFG);
> +
> + writel(0x20, layout->dp_serdes + QSERDES_COM_RESETSM_CNTRL);
> +
> + // C_READY
Use Linux coding style.
Anyway, drop all useless comments. Say something useful or don't say
anything.
> + if (readl_poll_timeout(layout->dp_serdes + QSERDES_COM_C_READY_STATUS,
> + status,
> + ((status & BIT(0)) > 0),
> + 500,
> + 10000)) {
> + dev_err(qmp->dev, "C_READY not ready\n");
> + return -ETIMEDOUT;
> + }
> +
> + // FREQ_DONE
> + if (readl_poll_timeout(layout->dp_serdes + QSERDES_COM_CMN_STATUS,
> + status,
> + ((status & BIT(0)) > 0),
> + 500,
> + 10000)){
> + dev_err(qmp->dev, "FREQ_DONE not ready\n");
> + return -ETIMEDOUT;
> + }
> +
> + // PLL_LOCKED
> + if (readl_poll_timeout(layout->dp_serdes + QSERDES_COM_CMN_STATUS,
> + status,
> + ((status & BIT(1)) > 0),
> + 500,
> + 10000)){
> + dev_err(qmp->dev, "PLL_LOCKED not ready\n");
> + return -ETIMEDOUT;
> + }
> +
> + writel(0x19, layout->dp_phy + QSERDES_DP_PHY_CFG);
> + udelay(10);
> +
> + // TSYNC_DONE
> + if (readl_poll_timeout(layout->dp_phy + QSERDES_V3_DP_PHY_STATUS,
> + status,
> + ((status & BIT(0)) > 0),
> + 500,
> + 10000)){
> + dev_err(qmp->dev, "TSYNC_DONE not ready\n");
> + return -ETIMEDOUT;
> + }
> +
> + // PHY_READY
> + if (readl_poll_timeout(layout->dp_phy + QSERDES_V3_DP_PHY_STATUS,
> + status,
> + ((status & BIT(1)) > 0),
> + 500,
> + 10000)){
> + dev_err(qmp->dev, "PHY_READY not ready\n");
> + return -ETIMEDOUT;
> + }
> +
> + writel(0x3f, layout->dp_tx + QSERDES_V3_TX_TRANSCEIVER_BIAS_EN);
> + writel(0x10, layout->dp_tx + QSERDES_V3_TX_HIGHZ_DRVR_EN);
> + writel(0x0a, layout->dp_tx + QSERDES_V3_TX_TX_POL_INV);
> + writel(0x3f, layout->dp_tx2 + QSERDES_V3_TX_TRANSCEIVER_BIAS_EN);
> + writel(0x10, layout->dp_tx2 + QSERDES_V3_TX_HIGHZ_DRVR_EN);
> + writel(0x0a, layout->dp_tx2 + QSERDES_V3_TX_TX_POL_INV);
> +
> + writel(0x18, layout->dp_phy + QSERDES_DP_PHY_CFG);
> + writel(0x19, layout->dp_phy + QSERDES_DP_PHY_CFG);
> +
> + if (readl_poll_timeout(layout->dp_phy + QSERDES_V3_DP_PHY_STATUS,
> + status,
> + ((status & BIT(1)) > 0),
> + 500,
> + 10000)){
> + dev_err(qmp->dev, "PHY_READY not ready\n");
> + return -ETIMEDOUT;
> + }
> +
> + return 0;
> +}
> +
> +static int qcs615_qmp_calibrate_dp_phy(struct qmp_usbc *qmp)
> +{
> + static const u8 cfg1_settings[] = {0x13, 0x23, 0x1d};
> + struct qmp_phy_dp_layout *layout = to_dp_layout(qmp);
> + u8 val;
> +
> + layout->dp_aux_cfg++;
> + layout->dp_aux_cfg %= ARRAY_SIZE(cfg1_settings);
> + val = cfg1_settings[layout->dp_aux_cfg];
> +
> + writel(val, layout->dp_phy + QSERDES_DP_PHY_AUX_CFG1);
> +
> + qmp_usbc_check_dp_phy(qmp, "pos_calibrate");
> +
> + return 0;
> +}
> +
> +static int qmp_usbc_com_init(struct phy *phy)
> {
> struct qmp_usbc *qmp = phy_get_drvdata(phy);
> - const struct qmp_phy_cfg *cfg = qmp->cfg;
> - void __iomem *pcs = qmp->pcs;
> + int num_vregs;
> u32 val = 0;
> int ret;
> + unsigned int reg_pwr_dn;
>
> - ret = regulator_bulk_enable(cfg->num_vregs, qmp->vregs);
> + if (qmp->type == QMP_PHY_USBC_USB) {
Sorry, all this code is unreviewable. Organize your changes in logical,
reviewable chunks.
> + struct qmp_phy_usb_cfg *cfg = to_usb_cfg(qmp);
> +
> + num_vregs = cfg->num_vregs;
> + reg_pwr_dn = cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL];
> + } else {
...
> + .compatible = "qcom,qcs615-qmp-dp-phy",
> + .data = &(struct dev_cfg) {
> + .type = QMP_PHY_USBC_DP,
> + .cfg = &qcs615_dpphy_cfg,
> + },
> }, {
> .compatible = "qcom,sdm660-qmp-usb3-phy",
> - .data = &sdm660_usb3phy_cfg,
> + .data = &(struct dev_cfg) {
> + .type = QMP_PHY_USBC_USB,
> + .cfg = &sdm660_usb3phy_cfg,
> + },
> }, {
> .compatible = "qcom,sm6115-qmp-usb3-phy",
> - .data = &qcm2290_usb3phy_cfg,
> + .data = &(struct dev_cfg) {
> + .type = QMP_PHY_USBC_USB,
> + .cfg = &qcm2290_usb3phy_cfg,
> + },
> },
> { },
> };
> +
You make some random changes all over this file. No, clean it up.
> MODULE_DEVICE_TABLE(of, qmp_usbc_of_match_table);
>
> static struct platform_driver qmp_usbc_driver = {
>
Best regards,
Krzysztof
