Re: [PATCH v6 2/6] phy: qcom: qmp-pcie: Add QMP PCIe Multi-PHY driver
From: Manivannan Sadhasivam
Date: Thu Jul 23 2026 - 09:35:16 EST
On Wed, Jul 22, 2026 at 07:30:19PM -0700, Qiang Yu wrote:
> Some QMP PCIe PHY hardware blocks support multiple link topologies (e.g.
> x8 or x4+x4) selected via a TCSR register. The existing single-instance
> QMP PCIe PHY driver has no way to model this: it assumes a single cfg per
> DT node and instantiates exactly one PHY.
>
> Add a dedicated driver for this class of PHY. Match data carries a
> per-mode cfg table; qmp_pcie_multiphy_probe() reads the current link
> mode from the TCSR register pointed to by "qcom,link-mode", looks up the
> corresponding cfg array, and instantiates one qmp_pcie per sub-PHY
> required by that link mode, registering the clock and #phy-cells = <1> phy
> providers so consumers can address individual sub-PHYs by index.
>
> The driver inherits the phy setting and link-mode programmed by firmware,
> so only the no_csr reset is used and no phy setting tables are provided.
>
> Add the first match data and compatible, qcom,glymur-qmp-gen5x8-pcie-phy,
> for the Glymur Gen5 PCIe PHY that can bifurcate into two x4 links or
> operate as a single x8 link.
>
> Signed-off-by: Qiang Yu <qiang.yu@xxxxxxxxxxxxxxxx>
Some nits below. But overall, the driver looks good to me:
Reviewed-by: Manivannan Sadhasivam <mani@xxxxxxxxxx>
> ---
> drivers/phy/qualcomm/Kconfig | 11 +
> drivers/phy/qualcomm/Makefile | 1 +
> drivers/phy/qualcomm/phy-qcom-qmp-pcie-multiphy.c | 770 ++++++++++++++++++++++
> 3 files changed, 782 insertions(+)
>
> diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
> index 60a0ead127fa..31241d1bbef2 100644
> --- a/drivers/phy/qualcomm/Kconfig
> +++ b/drivers/phy/qualcomm/Kconfig
> @@ -77,6 +77,17 @@ config PHY_QCOM_QMP_PCIE
> Enable this to support the QMP PCIe PHY transceiver that is used
> with PCIe controllers on Qualcomm chips.
>
> +config PHY_QCOM_QMP_PCIE_MULTIPHY
> + tristate "Qualcomm QMP PCIe Multiple Link-mode PHY Driver"
s/"Multiple Link-mode PHY"/"Multi PHY"? link-mode is just a TCSR register name.
> + depends on PCI || COMPILE_TEST
> + select GENERIC_PHY
> + default PHY_QCOM_QMP
> + help
> + Enable this to support the QMP PCIe PHY transceiver that is used
> + with PCIe controllers on Qualcomm chips. This PHY is a single
> + multi-lane QMP block that can be configured either as one wide
> + link or as multiple narrower independent links (bifurcation).
> +
> config PHY_QCOM_QMP_PCIE_8996
> tristate "Qualcomm QMP PCIe 8996 PHY Driver"
> depends on PCI || COMPILE_TEST
> diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
> index b71a6a0bed3f..8bf887d58ee4 100644
> --- a/drivers/phy/qualcomm/Makefile
> +++ b/drivers/phy/qualcomm/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_PHY_QCOM_PCIE2) += phy-qcom-pcie2.o
>
> obj-$(CONFIG_PHY_QCOM_QMP_COMBO) += phy-qcom-qmp-combo.o phy-qcom-qmp-usbc.o
> obj-$(CONFIG_PHY_QCOM_QMP_PCIE) += phy-qcom-qmp-pcie.o
> +obj-$(CONFIG_PHY_QCOM_QMP_PCIE_MULTIPHY) += phy-qcom-qmp-pcie-multiphy.o
> obj-$(CONFIG_PHY_QCOM_QMP_PCIE_8996) += phy-qcom-qmp-pcie-msm8996.o
> obj-$(CONFIG_PHY_QCOM_QMP_UFS) += phy-qcom-qmp-ufs.o
> obj-$(CONFIG_PHY_QCOM_QMP_USB) += phy-qcom-qmp-usb.o
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie-multiphy.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie-multiphy.c
> new file mode 100644
> index 000000000000..4531570b6fb1
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie-multiphy.c
> @@ -0,0 +1,770 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>
> +#include <linux/slab.h>
> +
> +#include "phy-qcom-qmp.h"
> +
> +#define PHY_INIT_COMPLETE_TIMEOUT 10000
s/PHY_INIT_COMPLETE_TIMEOUT/PHY_INIT_COMPLETE_TIMEOUT_US
> +
> +enum qmp_pcie_glymur_link_mode {
> + QMP_PCIE_GLYMUR_MODE_X8,
> + QMP_PCIE_GLYMUR_MODE_X4X4,
> +};
> +
> +enum qphy_reg_layout {
> + QPHY_PCS_STATUS,
> + QPHY_LAYOUT_SIZE
> +};
> +
> +static const unsigned int pciephy_v8_50_regs_layout[QPHY_LAYOUT_SIZE] = {
> + [QPHY_PCS_STATUS] = QPHY_V8_50_PCS_STATUS1,
> +};
> +
> +struct qmp_pcie_offsets {
> + u16 pcs;
> +};
> +
> +struct qmp_phy_cfg {
> + const struct qmp_pcie_offsets *offsets;
> +
> + const char * const *reg_names;
> + int num_regs;
> +
> + const char * const *pd_names;
> + int num_pds;
> +
> + const char * const *nocsr_reset_list;
> + int num_nocsr_resets;
> +
> + const char * const *vreg_list;
> + int num_vregs;
> +
> + const unsigned int *regs;
> +
> + unsigned int phy_status;
> +
> + const char * const *clk_list;
> + int num_clks;
> + const char * const *pipe_clk_list;
> +
> + int num_pipe_clks;
Remove newlines between the members, here and below.
> +};
> +
> +struct qmp_pcie {
> + struct device *dev;
> +
> + const struct qmp_phy_cfg *cfg;
> +
> + void __iomem **base;
> +
> + struct clk_bulk_data *clks;
> + struct clk_bulk_data *pipe_clks;
> +
> + struct reset_control_bulk_data *nocsr_resets;
> +
> + struct regulator_bulk_data *vregs;
> +
> + struct device **pd_devs;
> +
> + struct clk_fixed_rate pipe_clk_fixed;
> +};
> +
> +struct qmp_pcie_multiphy {
> + struct phy **phys;
> + const struct qmp_pcie_link_mode_cfg *mode_cfg;
> +
> + int num_pipe_outputs;
> + struct clk_fixed_rate *pipe_out_clks;
> +};
> +
> +struct qmp_pcie_link_mode_cfg {
> + const struct qmp_phy_cfg * const *cfgs;
> + u32 num_phys;
> +};
> +
> +struct qmp_pcie_match_data {
> + const struct qmp_pcie_link_mode_cfg *mode_cfgs;
> + u32 num_modes;
> +};
> +
> +static const char * const glymur_pciephy_clk_l[] = {
> + "aux", "cfg_ahb", "ref", "rchng", "phy_b_aux",
> +};
> +
> +static const char * const glymur_pciephy_a_clk_l[] = {
> + "aux", "cfg_ahb", "ref", "rchng",
> +};
> +
> +static const char * const glymur_pciephy_b_clk_l[] = {
> + "phy_b_aux", "cfg_ahb_b", "ref", "rchng_b",
> +};
> +
> +static const char * const glymur_pciephy_pipeclk_l[] = {
> + "pipe",
> +};
> +
> +static const char * const glymur_pipephy_b_pipeclk_l[] = {
> + "pipe_b", "pipediv2_b",
> +};
> +
> +static const char * const glymur_vreg_l[] = {
> + "vdda-phy", "vdda-pll", "vdda-refgen0p9", "vdda-refgen1p2",
> +};
> +
> +static const char * const glymur_pciephy_a_reg_l[] = {
> + "port_a",
> +};
> +
> +static const char * const glymur_pciephy_b_reg_l[] = {
> + "port_b",
> +};
> +
> +static const char * const glymur_pciephy_reg_l[] = {
> + "port_a", "port_b",
> +};
> +
> +static const char * const glymur_pciephy_a_pd_l[] = {
> + "port_a",
> +};
> +
> +static const char * const glymur_pciephy_b_pd_l[] = {
> + "port_b",
> +};
> +
> +static const char * const glymur_pciephy_pd_l[] = {
> + "port_a", "port_b",
> +};
> +
> +static const char * const glymur_pciephy_a_nocsr_reset_l[] = {
> + "port_a_nocsr",
> +};
> +
> +static const char * const glymur_pciephy_nocsr_reset_l[] = {
> + "port_a_nocsr", "port_b_nocsr",
> +};
> +
> +static const char * const glymur_pciephy_b_nocsr_reset_l[] = {
> + "port_b_nocsr",
> +};
> +
> +static const struct qmp_pcie_offsets glymur_pcie_offsets_v8_50 = {
> + .pcs = 0x9000,
> +};
> +
> +static const struct qmp_phy_cfg glymur_qmp_gen5x4_pciephy_a_cfg = {
> + .offsets = &glymur_pcie_offsets_v8_50,
> + .reg_names = glymur_pciephy_a_reg_l,
> + .num_regs = ARRAY_SIZE(glymur_pciephy_a_reg_l),
> + .pd_names = glymur_pciephy_a_pd_l,
> + .num_pds = ARRAY_SIZE(glymur_pciephy_a_pd_l),
> + .nocsr_reset_list = glymur_pciephy_a_nocsr_reset_l,
> + .num_nocsr_resets = ARRAY_SIZE(glymur_pciephy_a_nocsr_reset_l),
> + .vreg_list = glymur_vreg_l,
> + .num_vregs = ARRAY_SIZE(glymur_vreg_l),
> + .regs = pciephy_v8_50_regs_layout,
> + .phy_status = PHYSTATUS_4_20,
> + .pipe_clk_list = glymur_pciephy_pipeclk_l,
> + .num_pipe_clks = ARRAY_SIZE(glymur_pciephy_pipeclk_l),
> + .clk_list = glymur_pciephy_a_clk_l,
> + .num_clks = ARRAY_SIZE(glymur_pciephy_a_clk_l),
> +};
> +
> +static const struct qmp_phy_cfg glymur_qmp_gen5x4_pciephy_b_cfg = {
> + .offsets = &glymur_pcie_offsets_v8_50,
> + .reg_names = glymur_pciephy_b_reg_l,
> + .num_regs = ARRAY_SIZE(glymur_pciephy_b_reg_l),
> + .pd_names = glymur_pciephy_b_pd_l,
> + .num_pds = ARRAY_SIZE(glymur_pciephy_b_pd_l),
> + .nocsr_reset_list = glymur_pciephy_b_nocsr_reset_l,
> + .num_nocsr_resets = ARRAY_SIZE(glymur_pciephy_b_nocsr_reset_l),
> + .vreg_list = glymur_vreg_l,
> + .num_vregs = ARRAY_SIZE(glymur_vreg_l),
> + .regs = pciephy_v8_50_regs_layout,
> + .phy_status = PHYSTATUS_4_20,
> + .pipe_clk_list = glymur_pipephy_b_pipeclk_l,
> + .num_pipe_clks = ARRAY_SIZE(glymur_pipephy_b_pipeclk_l),
> + .clk_list = glymur_pciephy_b_clk_l,
> + .num_clks = ARRAY_SIZE(glymur_pciephy_b_clk_l),
> +};
> +
> +static const struct qmp_phy_cfg glymur_qmp_gen5x8_pciephy_cfg = {
> + .offsets = &glymur_pcie_offsets_v8_50,
> + .reg_names = glymur_pciephy_reg_l,
> + .num_regs = ARRAY_SIZE(glymur_pciephy_reg_l),
> + .pd_names = glymur_pciephy_pd_l,
> + .num_pds = ARRAY_SIZE(glymur_pciephy_pd_l),
> + .nocsr_reset_list = glymur_pciephy_nocsr_reset_l,
> + .num_nocsr_resets = ARRAY_SIZE(glymur_pciephy_nocsr_reset_l),
> + .vreg_list = glymur_vreg_l,
> + .num_vregs = ARRAY_SIZE(glymur_vreg_l),
> + .regs = pciephy_v8_50_regs_layout,
> + .phy_status = PHYSTATUS_4_20,
> + .pipe_clk_list = glymur_pciephy_pipeclk_l,
> + .num_pipe_clks = ARRAY_SIZE(glymur_pciephy_pipeclk_l),
> + .clk_list = glymur_pciephy_clk_l,
> + .num_clks = ARRAY_SIZE(glymur_pciephy_clk_l),
> +};
> +
> +static const struct qmp_phy_cfg * const glymur_qmp_gen5x8_mode_x8_cfgs[] = {
> + &glymur_qmp_gen5x8_pciephy_cfg,
> +};
> +
> +static const struct qmp_phy_cfg * const glymur_qmp_gen5x8_mode_x4x4_cfgs[] = {
> + &glymur_qmp_gen5x4_pciephy_a_cfg,
> + &glymur_qmp_gen5x4_pciephy_b_cfg,
> +};
> +
> +static const struct qmp_pcie_link_mode_cfg glymur_qmp_gen5x8_mode_cfgs[] = {
> + [QMP_PCIE_GLYMUR_MODE_X8] = {
> + .cfgs = glymur_qmp_gen5x8_mode_x8_cfgs,
> + .num_phys = ARRAY_SIZE(glymur_qmp_gen5x8_mode_x8_cfgs),
> + },
> + [QMP_PCIE_GLYMUR_MODE_X4X4] = {
> + .cfgs = glymur_qmp_gen5x8_mode_x4x4_cfgs,
> + .num_phys = ARRAY_SIZE(glymur_qmp_gen5x8_mode_x4x4_cfgs),
> + },
> +};
> +
> +static const struct qmp_pcie_match_data glymur_qmp_gen5x8_match_data = {
> + .mode_cfgs = glymur_qmp_gen5x8_mode_cfgs,
> + .num_modes = ARRAY_SIZE(glymur_qmp_gen5x8_mode_cfgs),
> +};
> +
> +static int qmp_pcie_pd_power_on(struct qmp_pcie *qmp)
> +{
> + const struct qmp_phy_cfg *cfg = qmp->cfg;
> + int i, ret;
> +
> + for (i = 0; i < cfg->num_pds; i++) {
> + ret = pm_runtime_resume_and_get(qmp->pd_devs[i]);
> + if (ret < 0) {
> + dev_err(qmp->dev, "failed to power on %s domain\n",
> + cfg->pd_names[i]);
> + goto err_power_off;
> + }
> + }
> +
> + return 0;
> +
> +err_power_off:
> + while (--i >= 0)
> + pm_runtime_put(qmp->pd_devs[i]);
> +
> + return ret;
> +}
> +
> +static void qmp_pcie_pd_power_off(struct qmp_pcie *qmp)
> +{
> + const struct qmp_phy_cfg *cfg = qmp->cfg;
> + int i;
> +
> + for (i = cfg->num_pds - 1; i >= 0; i--)
> + pm_runtime_put(qmp->pd_devs[i]);
> +}
> +
> +static int qmp_pcie_init(struct phy *phy)
> +{
> + struct qmp_pcie *qmp = phy_get_drvdata(phy);
> + const struct qmp_phy_cfg *cfg = qmp->cfg;
> + int ret;
> +
> + ret = qmp_pcie_pd_power_on(qmp);
> + if (ret)
> + return ret;
> +
> + ret = regulator_bulk_enable(cfg->num_vregs, qmp->vregs);
> + if (ret) {
> + dev_err(qmp->dev, "failed to enable regulators, err=%d\n", ret);
Drop 'err=':
"failed to enable regulators: %d\n"
> + goto err_pd_power_off;
> + }
> +
> + ret = reset_control_bulk_assert(qmp->cfg->num_nocsr_resets, qmp->nocsr_resets);
> + if (ret) {
> + dev_err(qmp->dev, "no-csr reset assert failed\n");
Print errno in all error prints.
> + goto err_disable_regulators;
> + }
> +
> + usleep_range(200, 300);
> +
> + ret = clk_bulk_prepare_enable(qmp->cfg->num_clks, qmp->clks);
> + if (ret)
> + goto err_disable_regulators;
> +
> + return 0;
> +
> +err_disable_regulators:
> + regulator_bulk_disable(cfg->num_vregs, qmp->vregs);
> +err_pd_power_off:
> + qmp_pcie_pd_power_off(qmp);
> +
> + return ret;
> +}
> +
> +static int qmp_pcie_exit(struct phy *phy)
> +{
> + struct qmp_pcie *qmp = phy_get_drvdata(phy);
> + const struct qmp_phy_cfg *cfg = qmp->cfg;
> +
> + reset_control_bulk_assert(qmp->cfg->num_nocsr_resets, qmp->nocsr_resets);
> +
> + clk_bulk_disable_unprepare(qmp->cfg->num_clks, qmp->clks);
> + regulator_bulk_disable(cfg->num_vregs, qmp->vregs);
> + qmp_pcie_pd_power_off(qmp);
> +
> + return 0;
> +}
> +
> +static int qmp_pcie_power_on(struct phy *phy)
> +{
> + struct qmp_pcie *qmp = phy_get_drvdata(phy);
> + const struct qmp_phy_cfg *cfg = qmp->cfg;
> + const struct qmp_pcie_offsets *offs = cfg->offsets;
> + void __iomem *status;
> + unsigned int val;
> + int i, ret;
> +
> + ret = clk_bulk_prepare_enable(qmp->cfg->num_pipe_clks, qmp->pipe_clks);
> + if (ret)
> + return ret;
> +
> + ret = reset_control_bulk_deassert(qmp->cfg->num_nocsr_resets, qmp->nocsr_resets);
> + if (ret) {
> + dev_err(qmp->dev, "no-csr reset deassert failed\n");
> + goto err_disable_pipe_clk;
> + }
> +
> + for (i = 0; i < cfg->num_regs; i++) {
> + status = qmp->base[i] + offs->pcs + cfg->regs[QPHY_PCS_STATUS];
> + ret = readl_poll_timeout(status, val, !(val & cfg->phy_status), 200,
> + PHY_INIT_COMPLETE_TIMEOUT);
> + if (ret) {
> + dev_err(qmp->dev, "phy initialization timed-out (%s)\n",
"PHY power on timed-out: %d"
> + cfg->reg_names[i]);
> + goto err_disable_pipe_clk;
> + }
> + }
> +
> + return 0;
> +
> +err_disable_pipe_clk:
> + clk_bulk_disable_unprepare(qmp->cfg->num_pipe_clks, qmp->pipe_clks);
> +
> + return ret;
> +}
> +
> +
> +static int qmp_pcie_power_off(struct phy *phy)
> +{
> + struct qmp_pcie *qmp = phy_get_drvdata(phy);
> +
> + clk_bulk_disable_unprepare(qmp->cfg->num_pipe_clks, qmp->pipe_clks);
> +
> + return 0;
> +}
> +
> +static int qmp_pcie_enable(struct phy *phy)
> +{
> + int ret;
> +
> + ret = qmp_pcie_init(phy);
> + if (ret)
> + return ret;
> +
> + ret = qmp_pcie_power_on(phy);
> + if (ret)
> + qmp_pcie_exit(phy);
> +
> + return ret;
return 0;
> +}
> +
[...]
> +static int qmp_pcie_multiphy_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct phy_provider *phy_provider;
> + struct qmp_pcie_multiphy *qmp_data;
> + const struct qmp_pcie_match_data *match_data;
> + struct qmp_pcie *qmp;
> + struct phy **phys;
> + unsigned int link_mode;
> + int phy_index;
> + int ret;
Reverse Xmas order please.
> +
> + qmp_data = devm_kzalloc(dev, sizeof(*qmp_data), GFP_KERNEL);
> +
> + match_data = of_device_get_match_data(dev);
> + if (!match_data)
> + return -EINVAL;
> +
> + if (!qmp_data)
> + return -ENOMEM;
> +
> + ret = qmp_pcie_read_link_mode(dev, &link_mode);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to read qcom,link-mode\n");
> +
> + if (link_mode >= match_data->num_modes)
> + return dev_err_probe(dev, -EINVAL, "invalid qcom,link-mode: %u\n",
> + link_mode);
> +
> + qmp_data->mode_cfg = &match_data->mode_cfgs[link_mode];
> +
> + qmp = devm_kcalloc(dev, qmp_data->mode_cfg->num_phys, sizeof(*qmp), GFP_KERNEL);
> + if (!qmp)
> + return -ENOMEM;
> +
> + phys = devm_kcalloc(dev, qmp_data->mode_cfg->num_phys, sizeof(*phys), GFP_KERNEL);
> + if (!phys)
> + return -ENOMEM;
> +
> + qmp_data->phys = phys;
> + dev_set_drvdata(dev, qmp_data);
> +
> + for (phy_index = 0; phy_index < qmp_data->mode_cfg->num_phys; phy_index++) {
> + qmp[phy_index].dev = dev;
> + qmp[phy_index].cfg = qmp_data->mode_cfg->cfgs[phy_index];
> + ret = qmp_pcie_probe_phy(&qmp[phy_index], dev->of_node, &phys[phy_index]);
> + if (ret)
> + return ret;
> + }
> +
> + ret = qmp_pcie_multiphy_register_clocks(dev, dev->of_node, qmp_data);
> + if (ret)
> + return ret;
> +
> + phy_provider = devm_of_phy_provider_register(dev, qmp_pcie_multiphy_xlate);
> +
> + return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static const struct of_device_id qmp_pcie_multiphy_of_match_table[] = {
> + {
> + .compatible = "qcom,glymur-qmp-gen5x8-pcie-phy",
> + .data = &glymur_qmp_gen5x8_match_data,
> + },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, qmp_pcie_multiphy_of_match_table);
> +
> +static struct platform_driver qmp_pcie_multiphy_driver = {
> + .probe = qmp_pcie_multiphy_probe,
> + .driver = {
> + .name = "qcom-qmp-pcie-multiphy",
> + .of_match_table = qmp_pcie_multiphy_of_match_table,
> + },
> +};
> +module_platform_driver(qmp_pcie_multiphy_driver);
> +
> +MODULE_AUTHOR("Qiang Yu <qiang.yu@xxxxxxxxxxxxxxxx>");
> +MODULE_DESCRIPTION("Qualcomm QMP PCIe PHY driver for Glymur");
s/PHY/Multi-PHY
- Mani
--
மணிவண்ணன் சதாசிவம்