Re: [PATCH v6 2/6] phy: qcom: qmp-pcie: Add QMP PCIe Multi-PHY driver

[Neil Armstrong]

On 7/23/26 04:30, 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>
> ---
>   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"
> +	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.
> + */
> +

<snip>

> +
> +static int qmp_pcie_pd_init(struct qmp_pcie *qmp)
> +{
> +	const struct qmp_phy_cfg *cfg = qmp->cfg;
> +	struct device *dev = qmp->dev;
> +	int i, ret;
> +
> +	if (!cfg->num_pds)
> +		return 0;
> +
> +	qmp->pd_devs = devm_kcalloc(dev, cfg->num_pds, sizeof(*qmp->pd_devs),
> +				    GFP_KERNEL);
> +	if (!qmp->pd_devs)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < cfg->num_pds; i++) {
> +		qmp->pd_devs[i] = dev_pm_domain_attach_by_name(dev,
> +							       cfg->pd_names[i]);
> +		if (IS_ERR(qmp->pd_devs[i])) {

Sashiko's review seems legitimate, just use IS_ERR_OR_NULL() here and return -ENODATA if NULL as in
https://elixir.bootlin.com/linux/v7.1.4/source/drivers/remoteproc/qcom_q6v5_mss.c#L1955

> +			ret = PTR_ERR(qmp->pd_devs[i]);
> +			goto err_detach;
> +		}
> +	}
> +
> +	return devm_add_action_or_reset(dev, qmp_pcie_pd_detach, qmp);
> +
> +err_detach:
> +	while (--i >= 0)
> +		dev_pm_domain_detach(qmp->pd_devs[i], false);
> +
> +	return ret;
> +}

<snip>

Otherwise it looks good for me, the design is much better even if
it still shared a lot of common helpers with the normal qmp phy
driver so I guess at some point if could be optimized to avoid
having some divergence between the drivers.

Thanks,
Neil