Linux-PHY Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v7 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
From: sashiko-bot @ 2026-05-22 15:27 UTC (permalink / raw)
  To: Bryan O'Donoghue
  Cc: vkoul, neil.armstrong, olteanv, linux-phy, robh, conor+dt,
	devicetree
In-Reply-To: <20260522-x1e-csi2-phy-v7-2-79cb1280fad6@linaro.org>

Thank you for your contribution! Sashiko AI review found 9 potential issue(s) to consider:
- [High] Lane polarity configuration overwrites the same hardcoded register instead of applying per-lane offsets.
- [High] The driver attaches PM domains but fails to enable Runtime PM or power them on.
- [High] Unconditional dereference of `pd_list` which can be NULL.
- [High] The power-off sequence incorrectly drops the PM domain performance state before disabling clocks and dropping the OPP rate.
- [High] A zero return value from `clk_round_rate()` is incorrectly treated as success, leading to a divide-by-zero panic.
- [Medium] Missing resource cleanup in the error paths of `phy_qcom_mipi_csi2_power_on()`.
- [Medium] Integer underflow in `settle_cnt` calculation.
- [Medium] Unvalidated lane positions from Device Tree lead to undefined behavior in bitwise shifts.
- [Low] Missing bounds check on `args_count` in the PHY xlate callback.
--

commit 9960771b4e31e9a37ca7022cc7945264d506b447
Author: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Date:   Fri May 22 14:45:10 2026 +0000

phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver

Add a new MIPI CSI2 driver in DPHY mode initially. The entire set of
existing CAMSS CSI PHY init sequences are imported in order to save time
and effort in later patches.

> diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
> new file mode 100644
> index 0000000000000..1e7729d4ee5d0
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
[ ... ]
> +static u8 phy_qcom_mipi_csi2_settle_cnt_calc(s64 link_freq, u32 timer_clk_rate)
> +{
[ ... ]
> +	timer_period_ps = div_u64(PSEC_PER_SEC, timer_clk_rate);
> +	settle_cnt = t_hs_settle_ps / timer_period_ps - 6;

[Severity: Medium]
Could this underflow if timer_period_ps is large?

Because settle_cnt is an unsigned u8, an underflow here would result in a
large wrap-around value (like 251) being written to the hardware,
corrupting the PHY timing settings.

> +
> +	return settle_cnt;
> +}
[ ... ]
> +static int phy_qcom_mipi_csi2_lanes_enable(struct mipi_csi2phy_device *csi2phy,
> +					   struct mipi_csi2phy_stream_cfg *cfg)
> +{
[ ... ]
> +	settle_cnt = phy_qcom_mipi_csi2_settle_cnt_calc(cfg->link_freq, csi2phy->timer_clk_rate);
> +
> +	/* Lane position enable in common reg offset */
> +	val = BIT(csi2phy->stream_cfg.lane_cfg.clk.pos);
> +	for (i = 0; i < cfg->num_data_lanes; i++)
> +		val |= BIT(lane_cfg->data[i].pos);

[Severity: Medium]
Is it possible for unvalidated lane positions from the Device Tree to
cause undefined behavior here?

If a parsed position value is >= 32, this might result in an invalid
shift. Additionally, since val is a u8, any position >= 8 would
silently truncate to 0 and fail to enable the lane.

> +
> +	writel(val, csi2phy->base +
> +	       CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 5));
> +
> +	/* Lane configuration for polarity @ CSIPHY-base + CTRL9 */
> +	for (i = 0; i < cfg->num_data_lanes; i++) {
> +		if (lane_cfg->data[i].pol) {
> +			writel(BIT(2), csi2phy->base + CSIPHY_2PH_LN_CSI_2PHASE_CTRLn(9));

[Severity: High]
Does this overwrite the same hardcoded register for all lanes?

Since CSIPHY_2PH_LN_CSI_2PHASE_CTRLn(9) expands to a static offset, this
repeatedly writes to lane 0's polarity register regardless of which lane
is being configured, and seems to ignore the clock lane polarity setting
entirely.

> +		}
> +	}
> +
[ ... ]
> diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
> new file mode 100644
> index 0000000000000..578926bfaef45
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
[ ... ]
> +static int
> +phy_qcom_mipi_csi2_set_clock_rates(struct mipi_csi2phy_device *csi2phy,
> +				   s64 link_freq)
> +{
[ ... ]
> +	for (i = 0; i < csi2phy->pd_list->num_pds; i++) {

[Severity: High]
Can pd_list be NULL here?

If CONFIG_PM_GENERIC_DOMAINS is disabled, devm_pm_domain_attach_list()
returns 0 but leaves csi2phy->pd_list as NULL. This would lead to a
kernel panic when dereferencing pd_list->num_pds.

> +		unsigned int perf = dev_pm_opp_get_required_pstate(opp, i);
> +
> +		ret = dev_pm_genpd_set_performance_state(csi2phy->pd_list->pd_devs[i], perf);
[ ... ]
> +	timer_rate = clk_round_rate(csi2phy->timer_clk, link_freq / 4);
> +	if (timer_rate <= 0) {
> +		ret = timer_rate;
> +		goto unset_opp_rate;

[Severity: High]
If clk_round_rate() returns 0, does this incorrectly treat it as a success?

Setting ret to 0 and jumping to unset_opp_rate would leave timer_clk_rate
as 0, which later causes a divide-by-zero panic in
phy_qcom_mipi_csi2_settle_cnt_calc().

> +	}
> +
> +	ret = clk_set_rate(csi2phy->timer_clk, timer_rate);
[ ... ]
> +static int phy_qcom_mipi_csi2_power_on(struct phy *phy)
> +{
[ ... ]
> +	ret = clk_bulk_prepare_enable(csi2phy->soc_cfg->num_clk,
> +				      csi2phy->clks);
> +	if (ret) {
> +		dev_err(dev, "failed to enable clocks, %d\n", ret);
> +		goto unset_rate;
> +	}
> +
> +	ops->reset(csi2phy);
[ ... ]
> +unset_rate:
> +	dev_pm_opp_set_rate(csi2phy->dev, 0);

[Severity: Medium]
Does this error path miss cleaning up the PM domain performance states?

While it drops the OPP rate and regulator votes, it completely skips
unwinding the performance states that were elevated during
phy_qcom_mipi_csi2_set_clock_rates(), leaking the votes.

> +
> +poweroff_phy:
> +	regulator_bulk_disable(csi2phy->soc_cfg->num_supplies,
> +			       csi2phy->supplies);
> +
> +	return ret;
> +}
> +
> +static int phy_qcom_mipi_csi2_power_off(struct phy *phy)
> +{
> +	struct mipi_csi2phy_device *csi2phy = phy_get_drvdata(phy);
> +	const struct mipi_csi2phy_hw_ops *ops = csi2phy->soc_cfg->ops;
> +	int i;
> +
> +	ops->lanes_disable(csi2phy, &csi2phy->stream_cfg);
> +
> +	for (i = 0; i < csi2phy->pd_list->num_pds; i++)
> +		dev_pm_genpd_set_performance_state(csi2phy->pd_list->pd_devs[i], 0);
> +
> +	clk_bulk_disable_unprepare(csi2phy->soc_cfg->num_clk,
> +				   csi2phy->clks);

[Severity: High]
Is this sequence dropping the PM domain performance state before
disabling the clocks?

Removing the voltage (performance state) while hardware logic is still
driven by high-frequency clocks can cause logic timing failures and system
hangs.

Also, similarly to phy_qcom_mipi_csi2_set_clock_rates(), accessing
pd_list->num_pds unconditionally here could panic if pd_list is NULL.

> +	dev_pm_opp_set_rate(csi2phy->dev, 0);
> +	regulator_bulk_disable(csi2phy->soc_cfg->num_supplies,
> +			       csi2phy->supplies);
> +
> +	return 0;
> +}
[ ... ]
> +static struct phy *qcom_csi2_phy_xlate(struct device *dev,
> +				       const struct of_phandle_args *args)
> +{
> +	struct mipi_csi2phy_device *csi2phy = dev_get_drvdata(dev);
> +
> +	if (args->args[0] != PHY_TYPE_DPHY) {

[Severity: Low]
Can we add a bounds check for args->args_count here?

If a malformed Device Tree node specifies #phy-cells = <0>, this will
read uninitialized memory from the args array.

> +		dev_err(csi2phy->dev, "mode %d -EOPNOTSUPP\n", args->args[0]);
> +		return ERR_PTR(-EOPNOTSUPP);
> +	}
[ ... ]
> +static int phy_qcom_mipi_csi2_probe(struct platform_device *pdev)
> +{
[ ... ]
> +	ret = phy_qcom_mipi_csi2_attach_pm_domains(csi2phy);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Failed to attach power-domain list\n");

[Severity: High]
Are we missing Runtime PM initialization for the attached PM domains?

devm_pm_domain_attach_list() creates virtual devices, but without
pm_runtime_enable() and pm_runtime_get_sync(), these domains remain
suspended. As a result, any dev_pm_genpd_set_performance_state() votes
are just cached and never applied to the hardware, leaving the PHY
unpowered.

> +
> +	for (i = 0; i < num_clk; i++)
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260522-x1e-csi2-phy-v7-0-79cb1280fad6@linaro.org?part=2

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH V1 1/3] dt-bindings: phy: qcom,sc8280xp-qmp-ufs-phy: Add Hawi UFS PHY compatible
From: Palash Kambar @ 2026-05-22 15:18 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, alim.akhtar,
	avri.altman, bvanassche, andersson, dmitry.baryshkov, abel.vesa,
	luca.weiss, linux-arm-msm, linux-phy, devicetree, linux-kernel,
	linux-scsi
In-Reply-To: <uut3u6djc2dc23aznjxdqvkehpx3ooa7eknzjzubnlmylwvxxb@aow3lpjurhnt>



On 5/19/2026 11:35 AM, Manivannan Sadhasivam wrote:
> On Mon, May 18, 2026 at 10:23:44PM +0530, palash.kambar@oss.qualcomm.com wrote:
>> From: Palash Kambar <palash.kambar@oss.qualcomm.com>
>>
>> Document QMP UFS PHY compatible for Hawi SoC.
>>
>> Signed-off-by: Palash Kambar <palash.kambar@oss.qualcomm.com>
>> ---
>>  .../devicetree/bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml       | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml
>> index 9616c736b6d4..2326dcf38a46 100644
>> --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml
>> +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml
>> @@ -37,6 +37,7 @@ properties:
>>                - qcom,kaanapali-qmp-ufs-phy
>>            - const: qcom,sm8750-qmp-ufs-phy
>>        - enum:
>> +          - qcom,hawi-qmp-ufs-phy
> 
> Don't you need to add the Hawi compatible to the below check for clocks?
> 
> - Mani
> 

Sure Mani Will update. Thanks.


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH v7 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: sashiko-bot @ 2026-05-22 14:53 UTC (permalink / raw)
  To: Bryan O'Donoghue
  Cc: robh, devicetree, conor+dt, olteanv, neil.armstrong, vkoul,
	linux-phy
In-Reply-To: <20260522-x1e-csi2-phy-v7-1-79cb1280fad6@linaro.org>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The device tree example fails schema validation because `port@2`'s endpoint includes `data-lanes` and `clock-lanes` which are forbidden by the schema.
--

commit f15340b905d3f2f95e02027798a2eb5d4ed06bf5
Author: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema

This commit adds a new base schema for the x1e80100 MIPI CSI2 PHY devices.
It supports CPHY, DPHY, and DPHY split-mode by modeling the CSIPHY devices
as individual nodes with three distinct ports.

> diff --git a/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
> new file mode 100644
> index 0000000000000..f434fb04447f6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
[ ... ]
> +      port@2:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        description: Output to CAMSS controller.
> +
> +        properties:
> +          endpoint:
> +            $ref: /schemas/graph.yaml#/$defs/endpoint-base
> +            unevaluatedProperties: false
> +            properties:
> +              remote-endpoint: true
> +            required:
> +              - remote-endpoint
[ ... ]
> +examples:
> +  - |
[ ... ]
> +            port@2 {
> +                reg = <2>;
> +                csiphy0_out_ep: endpoint {
> +                    data-lanes = <3>;
> +                    clock-lanes = <4>;

[Severity: Low]
Will this example fail dt_binding_check schema validation?

The schema definition above for port@2's endpoint points to endpoint-base and
sets unevaluatedProperties to false, without locally defining data-lanes or
clock-lanes.

However, the example uses them here. Does the port@2 definition need a
reference to video-interfaces.yaml like the other ports, or should these
properties be removed from the example?

> +                    remote-endpoint = <&controller_in>;
> +                };
> +            };
> +        };
> +    };
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260522-x1e-csi2-phy-v7-0-79cb1280fad6@linaro.org?part=1

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* [PATCH v7 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
From: Bryan O'Donoghue @ 2026-05-22 14:45 UTC (permalink / raw)
  To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
  Cc: Bryan O'Donoghue, Vladimir Zapolskiy, linux-arm-msm,
	linux-phy, linux-media, devicetree, linux-kernel,
	Bryan O'Donoghue
In-Reply-To: <20260522-x1e-csi2-phy-v7-0-79cb1280fad6@linaro.org>

Add a new MIPI CSI2 driver in DPHY mode initially. The entire set of
existing CAMSS CSI PHY init sequences are imported in order to save time
and effort in later patches.

The following devices are supported in this drop:
"qcom,x1e80100-csi2-phy"

In-line with other PHY drivers the process node is included in the name.
Data-lane and clock lane positioning and polarity selection via newly
amended struct phy_configure_opts_mipi_dphy{} is supported.

The Qualcomm 3PH class of PHYs can do both DPHY and CPHY mode. For now only
DPHY is supported.

In porting some of the logic over from camss-csiphy*.c to here its also
possible to rationalise some of the code.

In particular use of regulator_bulk and clk_bulk as well as dropping the
seemingly useless and unused interrupt handler.

The PHY sequences and a lot of the logic that goes with them are well
proven in CAMSS and mature so the main thing to watch out for here is how
to get the right sequencing of regulators, clocks and register-writes.

The register init sequence table is imported verbatim from the existing
CAMSS csiphy driver. A follow-up series will rework the table to extract
the repetitive per-lane pattern into a loop.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 MAINTAINERS                                        |  10 +
 drivers/phy/qualcomm/Kconfig                       |  14 +
 drivers/phy/qualcomm/Makefile                      |   5 +
 drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c | 371 ++++++++++++++++++++
 drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c     | 386 +++++++++++++++++++++
 drivers/phy/qualcomm/phy-qcom-mipi-csi2.h          |  95 +++++
 6 files changed, 881 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 63389fea5d150..3b5da8a40383f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22018,6 +22018,16 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/media/qcom,*-iris.yaml
 F:	drivers/media/platform/qcom/iris/
 
+QUALCOMM MIPI CSI2 PHY DRIVER
+M:	Bryan O'Donoghue <bod@kernel.org>
+L:	linux-phy@lists.infradead.org
+L:	linux-media@vger.kernel.org
+L:	linux-arm-msm@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/phy/qcom,*-csi2-phy.yaml
+F:	drivers/phy/qualcomm/phy-qcom-mipi-csi2*.c
+F:	drivers/phy/qualcomm/phy-qcom-mipi-csi2*.h
+
 QUALCOMM NAND CONTROLLER DRIVER
 M:	Manivannan Sadhasivam <mani@kernel.org>
 L:	linux-mtd@lists.infradead.org
diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
index 60a0ead127fa9..779a3511ba852 100644
--- a/drivers/phy/qualcomm/Kconfig
+++ b/drivers/phy/qualcomm/Kconfig
@@ -28,6 +28,20 @@ config PHY_QCOM_EDP
 	  Enable this driver to support the Qualcomm eDP PHY found in various
 	  Qualcomm chipsets.
 
+config PHY_QCOM_MIPI_CSI2
+	tristate "Qualcomm MIPI CSI2 PHY driver"
+	depends on ARCH_QCOM || COMPILE_TEST
+	depends on OF
+	depends on PM
+	depends on COMMON_CLK
+	select GENERIC_PHY
+	select GENERIC_PHY_MIPI_DPHY
+	help
+	  Enable this to support the MIPI CSI2 PHY driver found in various
+	  Qualcomm chipsets. This PHY is used to connect MIPI CSI2
+	  camera sensors to the CSI Decoder in the Qualcomm Camera Subsystem
+	  CAMSS.
+
 config PHY_QCOM_IPQ4019_USB
 	tristate "Qualcomm IPQ4019 USB PHY driver"
 	depends on OF && (ARCH_QCOM || COMPILE_TEST)
diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
index b71a6a0bed3f1..382cb594b06b6 100644
--- a/drivers/phy/qualcomm/Makefile
+++ b/drivers/phy/qualcomm/Makefile
@@ -6,6 +6,11 @@ obj-$(CONFIG_PHY_QCOM_IPQ4019_USB)	+= phy-qcom-ipq4019-usb.o
 obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA)	+= phy-qcom-ipq806x-sata.o
 obj-$(CONFIG_PHY_QCOM_M31_USB)		+= phy-qcom-m31.o
 obj-$(CONFIG_PHY_QCOM_M31_EUSB)		+= phy-qcom-m31-eusb2.o
+
+phy-qcom-mipi-csi2-objs			+= phy-qcom-mipi-csi2-core.o \
+					   phy-qcom-mipi-csi2-3ph-dphy.o
+obj-$(CONFIG_PHY_QCOM_MIPI_CSI2)	+= phy-qcom-mipi-csi2.o
+
 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
diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
new file mode 100644
index 0000000000000..1e7729d4ee5d0
--- /dev/null
+++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
@@ -0,0 +1,371 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Qualcomm MSM Camera Subsystem - CSIPHY Module 3phase v1.0
+ *
+ * Copyright (c) 2011-2015, The Linux Foundation. All rights reserved.
+ * Copyright (C) 2016-2025 Linaro Ltd.
+ */
+
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/time64.h>
+
+#include "phy-qcom-mipi-csi2.h"
+
+#define CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(offset, n)	((offset) + 0x4 * (n))
+#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL0_PHY_SW_RESET	BIT(0)
+#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE	BIT(7)
+#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_COMMON_PWRDN_B	BIT(0)
+#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_SHOW_REV_ID	BIT(1)
+#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL10_IRQ_CLEAR_CMD	BIT(0)
+#define CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(offset, n)	((offset) + 0xb0 + 0x4 * (n))
+
+#define CSIPHY_2PH_LN_CSI_2PHASE_CTRLn(n)		(0x4 * (n))
+
+/*
+ * 3 phase CSI has 19 common status regs with only 0-10 being used
+ * and 11-18 being reserved.
+ */
+#define CSI_COMMON_STATUS_NUM				11
+/*
+ * There are a number of common control registers
+ * The offset to clear the CSIPHY IRQ status starts @ 22
+ * So to clear CSI_COMMON_STATUS0 this is CSI_COMMON_CONTROL22, STATUS1 is
+ * CONTROL23 and so on
+ */
+#define CSI_CTRL_STATUS_INDEX				22
+
+/*
+ * There are 43 COMMON_CTRL registers with regs after # 33 being reserved
+ */
+#define CSI_CTRL_MAX					33
+
+#define CSIPHY_DEFAULT_PARAMS				0
+#define CSIPHY_SETTLE_CNT_LOWER_BYTE			2
+#define CSIPHY_SKEW_CAL					7
+
+/* 4nm 2PH v 2.1.2 2p5Gbps 4 lane DPHY mode */
+static const struct
+mipi_csi2phy_lane_regs lane_regs_x1e80100[] = {
+	/* Power up lanes 2ph mode */
+	{.reg_addr = 0x1014, .reg_data = 0xd5, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x101c, .reg_data = 0x7a, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x1018, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+
+	{.reg_addr = 0x0094, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x00a0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0090, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0098, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0094, .reg_data = 0x07, .delay_us = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0030, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0000, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0038, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x002c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0034, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x001c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0014, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x003c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0004, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0020, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0008, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
+	{.reg_addr = 0x0010, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0094, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
+	{.reg_addr = 0x005c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
+	{.reg_addr = 0x0060, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
+	{.reg_addr = 0x0064, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
+
+	{.reg_addr = 0x0e94, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0ea0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e90, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e98, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e94, .reg_data = 0x07, .delay_us =  0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e30, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e28, .reg_data = 0x04, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e00, .reg_data = 0x80, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e0c, .reg_data = 0xff, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e38, .reg_data = 0x1f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e2c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e34, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e1c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e14, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e3c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e04, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e20, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0e08, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
+	{.reg_addr = 0x0e10, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
+
+	{.reg_addr = 0x0494, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x04a0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0490, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0498, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0494, .reg_data = 0x07, .delay_us =  0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0430, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0400, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0438, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x042c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0434, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x041c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0414, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x043c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0404, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0420, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0408, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
+	{.reg_addr = 0x0410, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0494, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
+	{.reg_addr = 0x045c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
+	{.reg_addr = 0x0460, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
+	{.reg_addr = 0x0464, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
+
+	{.reg_addr = 0x0894, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x08a0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0890, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0898, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0894, .reg_data = 0x07, .delay_us =  0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0830, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0800, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0838, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x082c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0834, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x081c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0814, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x083c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0804, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0820, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0808, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
+	{.reg_addr = 0x0810, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0894, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
+	{.reg_addr = 0x085c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
+	{.reg_addr = 0x0860, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
+	{.reg_addr = 0x0864, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
+
+	{.reg_addr = 0x0c94, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0ca0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c90, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c98, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c94, .reg_data = 0x07, .delay_us =  0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c30, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c00, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c38, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c2c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c34, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c1c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c14, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c3c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c04, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c20, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c08, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
+	{.reg_addr = 0x0c10, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
+	{.reg_addr = 0x0c94, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
+	{.reg_addr = 0x0c5c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
+	{.reg_addr = 0x0c60, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
+	{.reg_addr = 0x0c64, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
+};
+
+static inline const struct mipi_csi2phy_device_regs *
+csi2phy_dev_to_regs(struct mipi_csi2phy_device *csi2phy)
+{
+	return &csi2phy->soc_cfg->reg_info;
+}
+
+static void phy_qcom_mipi_csi2_hw_version_read(struct mipi_csi2phy_device *csi2phy)
+{
+	const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
+	u32 tmp;
+
+	writel(CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_SHOW_REV_ID, csi2phy->base +
+	       CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 6));
+
+	tmp = readl_relaxed(csi2phy->base +
+			    CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->common_regs_offset, 12));
+	csi2phy->hw_version = tmp;
+
+	tmp = readl_relaxed(csi2phy->base +
+			    CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->common_regs_offset, 13));
+	csi2phy->hw_version |= (tmp << 8) & 0xFF00;
+
+	tmp = readl_relaxed(csi2phy->base +
+			    CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->common_regs_offset, 14));
+	csi2phy->hw_version |= (tmp << 16) & 0xFF0000;
+
+	tmp = readl_relaxed(csi2phy->base +
+			    CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->common_regs_offset, 15));
+	csi2phy->hw_version |= (tmp << 24) & 0xFF000000;
+
+	dev_dbg_once(csi2phy->dev, "CSIPHY 3PH HW Version = 0x%08x\n", csi2phy->hw_version);
+}
+
+/*
+ * phy_qcom_mipi_csi2_reset - Perform software reset on CSIPHY module
+ * @phy_qcom_mipi_csi2: CSIPHY device
+ */
+static void phy_qcom_mipi_csi2_reset(struct mipi_csi2phy_device *csi2phy)
+{
+	const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
+
+	writel(CSIPHY_3PH_CMN_CSI_COMMON_CTRL0_PHY_SW_RESET,
+	       csi2phy->base + CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 0));
+	usleep_range(5000, 8000);
+	writel(0x0, csi2phy->base +
+	       CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 0));
+}
+
+/*
+ * phy_qcom_mipi_csi2_settle_cnt_calc - Calculate settle count value
+ *
+ * Helper function to calculate settle count value. This is
+ * based on the CSI2 T_hs_settle parameter which in turn
+ * is calculated based on the CSI2 transmitter link frequency.
+ *
+ * Return settle count value or 0 if the CSI2 link frequency
+ * is not available
+ */
+static u8 phy_qcom_mipi_csi2_settle_cnt_calc(s64 link_freq, u32 timer_clk_rate)
+{
+	u32 t_hs_prepare_max_ps;
+	u32 timer_period_ps;
+	u32 t_hs_settle_ps;
+	u8 settle_cnt;
+	u32 ui_ps;
+
+	if (link_freq <= 0)
+		return 0;
+
+	ui_ps = div_u64(PSEC_PER_SEC, link_freq);
+	ui_ps /= 2;
+	t_hs_prepare_max_ps = 85000 + 6 * ui_ps;
+	t_hs_settle_ps = t_hs_prepare_max_ps;
+
+	timer_period_ps = div_u64(PSEC_PER_SEC, timer_clk_rate);
+	settle_cnt = t_hs_settle_ps / timer_period_ps - 6;
+
+	return settle_cnt;
+}
+
+static void
+phy_qcom_mipi_csi2_gen2_config_lanes(struct mipi_csi2phy_device *csi2phy,
+				     u8 settle_cnt)
+{
+	const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
+	const struct mipi_csi2phy_lane_regs *r = regs->init_seq;
+	int i, array_size = regs->lane_array_size;
+	u32 val;
+
+	for (i = 0; i < array_size; i++, r++) {
+		switch (r->param_type) {
+		case CSIPHY_SETTLE_CNT_LOWER_BYTE:
+			val = settle_cnt & 0xff;
+			break;
+		case CSIPHY_SKEW_CAL:
+			/* TODO: support application of skew from dt flag */
+			continue;
+		default:
+			val = r->reg_data;
+			break;
+		}
+		writel(val, csi2phy->base + r->reg_addr);
+		if (r->delay_us)
+			udelay(r->delay_us);
+	}
+}
+
+static int phy_qcom_mipi_csi2_lanes_enable(struct mipi_csi2phy_device *csi2phy,
+					   struct mipi_csi2phy_stream_cfg *cfg)
+{
+	const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
+	struct mipi_csi2phy_lanes_cfg *lane_cfg = &cfg->lane_cfg;
+	u8 settle_cnt;
+	u8 val;
+	int i;
+
+	settle_cnt = phy_qcom_mipi_csi2_settle_cnt_calc(cfg->link_freq, csi2phy->timer_clk_rate);
+
+	/* Lane position enable in common reg offset */
+	val = BIT(csi2phy->stream_cfg.lane_cfg.clk.pos);
+	for (i = 0; i < cfg->num_data_lanes; i++)
+		val |= BIT(lane_cfg->data[i].pos);
+
+	writel(val, csi2phy->base +
+	       CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 5));
+
+	/* Lane configuration for polarity @ CSIPHY-base + CTRL9 */
+	for (i = 0; i < cfg->num_data_lanes; i++) {
+		if (lane_cfg->data[i].pol) {
+			writel(BIT(2), csi2phy->base + CSIPHY_2PH_LN_CSI_2PHASE_CTRLn(9));
+		}
+	}
+
+	val = CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_COMMON_PWRDN_B;
+	writel(val, csi2phy->base +
+	       CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 6));
+
+	val = 0x02;
+	writel(val, csi2phy->base +
+	       CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 7));
+
+	val = 0x00;
+	writel(val, csi2phy->base +
+	       CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 0));
+
+	phy_qcom_mipi_csi2_gen2_config_lanes(csi2phy, settle_cnt);
+
+	/* IRQ_MASK registers - disable all interrupts */
+	for (i = CSI_COMMON_STATUS_NUM; i < CSI_CTRL_STATUS_INDEX; i++) {
+		writel(0, csi2phy->base +
+		       CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, i));
+	}
+
+	return 0;
+}
+
+static void
+phy_qcom_mipi_csi2_lanes_disable(struct mipi_csi2phy_device *csi2phy,
+				 struct mipi_csi2phy_stream_cfg *cfg)
+{
+	const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
+
+	writel(0, csi2phy->base +
+	       CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 5));
+
+	writel(0, csi2phy->base +
+	       CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 6));
+}
+
+static const struct mipi_csi2phy_hw_ops phy_qcom_mipi_csi2_ops_3ph_1_0 = {
+	.hw_version_read = phy_qcom_mipi_csi2_hw_version_read,
+	.reset = phy_qcom_mipi_csi2_reset,
+	.lanes_enable = phy_qcom_mipi_csi2_lanes_enable,
+	.lanes_disable = phy_qcom_mipi_csi2_lanes_disable,
+};
+
+static const char * const x1e_clks[] = {
+	"core",
+	"timer"
+};
+
+static const char * const x1e_supplies[] = {
+	"vdda-0p9",
+	"vdda-1p2"
+};
+
+static const char * const x1e_genpd_names[] = {
+	"mmcx",
+	"mx",
+};
+
+const struct mipi_csi2phy_soc_cfg mipi_csi2_dphy_4nm_x1e = {
+	.ops = &phy_qcom_mipi_csi2_ops_3ph_1_0,
+	.reg_info = {
+		.init_seq = lane_regs_x1e80100,
+		.lane_array_size = ARRAY_SIZE(lane_regs_x1e80100),
+		.common_regs_offset = 0x1000,
+	},
+	.supply_names = (const char **)x1e_supplies,
+	.num_supplies = ARRAY_SIZE(x1e_supplies),
+	.clk_names = (const char **)x1e_clks,
+	.num_clk = ARRAY_SIZE(x1e_clks),
+	.opp_clk = x1e_clks[0],
+	.timer_clk = x1e_clks[1],
+	.genpd_names = (const char **)x1e_genpd_names,
+	.num_genpd_names = ARRAY_SIZE(x1e_genpd_names),
+};
diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
new file mode 100644
index 0000000000000..578926bfaef45
--- /dev/null
+++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
@@ -0,0 +1,386 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025, Linaro Ltd.
+ */
+#include <dt-bindings/phy/phy.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/pm_opp.h>
+#include <linux/phy/phy.h>
+#include <linux/phy/phy-mipi-dphy.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-mipi-csi2.h"
+
+static int
+phy_qcom_mipi_csi2_set_clock_rates(struct mipi_csi2phy_device *csi2phy,
+				   s64 link_freq)
+{
+	struct device *dev = csi2phy->dev;
+	unsigned long opp_rate = link_freq / 4;
+	struct dev_pm_opp *opp;
+	long timer_rate;
+	int i, ret;
+
+	opp = dev_pm_opp_find_freq_ceil(dev, &opp_rate);
+	if (IS_ERR(opp)) {
+		dev_err(csi2phy->dev, "Couldn't find ceiling for %lld Hz\n",
+			link_freq);
+		return PTR_ERR(opp);
+	}
+
+	for (i = 0; i < csi2phy->pd_list->num_pds; i++) {
+		unsigned int perf = dev_pm_opp_get_required_pstate(opp, i);
+
+		ret = dev_pm_genpd_set_performance_state(csi2phy->pd_list->pd_devs[i], perf);
+		if (ret) {
+			dev_err(csi2phy->dev, "Couldn't set perf state %u\n",
+				perf);
+			dev_pm_opp_put(opp);
+			goto unset_pstate;
+		}
+	}
+	dev_pm_opp_put(opp);
+
+	ret = dev_pm_opp_set_rate(dev, opp_rate);
+	if (ret) {
+		dev_err(csi2phy->dev, "dev_pm_opp_set_rate() fail\n");
+		goto unset_opp_rate;
+	}
+
+	timer_rate = clk_round_rate(csi2phy->timer_clk, link_freq / 4);
+	if (timer_rate <= 0) {
+		ret = timer_rate;
+		goto unset_opp_rate;
+	}
+
+	ret = clk_set_rate(csi2phy->timer_clk, timer_rate);
+	if (ret)
+		goto unset_opp_rate;
+
+	csi2phy->timer_clk_rate = timer_rate;
+
+	return 0;
+
+unset_opp_rate:
+	dev_pm_opp_set_rate(dev, 0);
+
+unset_pstate:
+	while (i--)
+		dev_pm_genpd_set_performance_state(csi2phy->pd_list->pd_devs[i], 0);
+
+	return ret;
+}
+
+static int phy_qcom_mipi_csi2_configure(struct phy *phy,
+					union phy_configure_opts *opts)
+{
+	struct mipi_csi2phy_device *csi2phy = phy_get_drvdata(phy);
+	struct phy_configure_opts_mipi_dphy *dphy_cfg = &opts->mipi_dphy;
+	struct mipi_csi2phy_stream_cfg *stream_cfg = &csi2phy->stream_cfg;
+	int ret;
+
+	ret = phy_mipi_dphy_config_validate(dphy_cfg);
+	if (ret)
+		return ret;
+
+	if (dphy_cfg->lanes < 1 || dphy_cfg->lanes > CSI2_MAX_DATA_LANES)
+		return -EINVAL;
+
+	stream_cfg->link_freq = dphy_cfg->hs_clk_rate;
+
+	return 0;
+}
+
+static int phy_qcom_mipi_csi2_power_on(struct phy *phy)
+{
+	struct mipi_csi2phy_device *csi2phy = phy_get_drvdata(phy);
+	const struct mipi_csi2phy_hw_ops *ops = csi2phy->soc_cfg->ops;
+	struct device *dev = &phy->dev;
+	int ret;
+
+	ret = regulator_bulk_enable(csi2phy->soc_cfg->num_supplies,
+				    csi2phy->supplies);
+	if (ret)
+		return ret;
+
+	ret = phy_qcom_mipi_csi2_set_clock_rates(csi2phy, csi2phy->stream_cfg.link_freq);
+	if (ret)
+		goto poweroff_phy;
+
+	ret = clk_bulk_prepare_enable(csi2phy->soc_cfg->num_clk,
+				      csi2phy->clks);
+	if (ret) {
+		dev_err(dev, "failed to enable clocks, %d\n", ret);
+		goto unset_rate;
+	}
+
+	ops->reset(csi2phy);
+
+	ops->hw_version_read(csi2phy);
+
+	return ops->lanes_enable(csi2phy, &csi2phy->stream_cfg);
+
+unset_rate:
+	dev_pm_opp_set_rate(csi2phy->dev, 0);
+
+poweroff_phy:
+	regulator_bulk_disable(csi2phy->soc_cfg->num_supplies,
+			       csi2phy->supplies);
+
+	return ret;
+}
+
+static int phy_qcom_mipi_csi2_power_off(struct phy *phy)
+{
+	struct mipi_csi2phy_device *csi2phy = phy_get_drvdata(phy);
+	const struct mipi_csi2phy_hw_ops *ops = csi2phy->soc_cfg->ops;
+	int i;
+
+	ops->lanes_disable(csi2phy, &csi2phy->stream_cfg);
+
+	for (i = 0; i < csi2phy->pd_list->num_pds; i++)
+		dev_pm_genpd_set_performance_state(csi2phy->pd_list->pd_devs[i], 0);
+
+	clk_bulk_disable_unprepare(csi2phy->soc_cfg->num_clk,
+				   csi2phy->clks);
+	dev_pm_opp_set_rate(csi2phy->dev, 0);
+	regulator_bulk_disable(csi2phy->soc_cfg->num_supplies,
+			       csi2phy->supplies);
+
+	return 0;
+}
+
+static const struct phy_ops phy_qcom_mipi_csi2_ops = {
+	.configure	= phy_qcom_mipi_csi2_configure,
+	.power_on	= phy_qcom_mipi_csi2_power_on,
+	.power_off	= phy_qcom_mipi_csi2_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static struct phy *qcom_csi2_phy_xlate(struct device *dev,
+				       const struct of_phandle_args *args)
+{
+	struct mipi_csi2phy_device *csi2phy = dev_get_drvdata(dev);
+
+	if (args->args[0] != PHY_TYPE_DPHY) {
+		dev_err(csi2phy->dev, "mode %d -EOPNOTSUPP\n", args->args[0]);
+		return ERR_PTR(-EOPNOTSUPP);
+	}
+
+	csi2phy->phy_mode = args->args[0];
+
+	return csi2phy->phy;
+}
+
+static int phy_qcom_mipi_csi2_attach_pm_domains(struct mipi_csi2phy_device *csi2phy)
+{
+	const struct dev_pm_domain_attach_data pd_data = {
+		.pd_names = csi2phy->soc_cfg->genpd_names,
+		.num_pd_names = csi2phy->soc_cfg->num_genpd_names,
+	};
+
+	return devm_pm_domain_attach_list(csi2phy->dev, &pd_data, &csi2phy->pd_list);
+}
+
+static int phy_qcom_mipi_csi2_parse_routing(struct mipi_csi2phy_device *csi2phy)
+{
+	struct mipi_csi2phy_stream_cfg *stream_cfg = &csi2phy->stream_cfg;
+	u32 lane_polarities[CSI2_MAX_DATA_LANES + 1];
+	u32 data_lanes[CSI2_MAX_DATA_LANES];
+	struct device *dev = csi2phy->dev;
+	struct fwnode_handle *ep;
+	int num_polarities;
+	int num_data_lanes;
+	u32 clock_lane;
+	int i, ret;
+
+	ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 1, 0,
+					     FWNODE_GRAPH_ENDPOINT_NEXT);
+	if (ep) {
+		fwnode_handle_put(ep);
+		dev_err(dev, "DPHY split mode is not supported\n");
+		return -EOPNOTSUPP;
+	}
+
+	ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 0, 0, 0);
+	if (!ep) {
+		dev_err(dev, "Missing port@0\n");
+		return -ENODEV;
+	}
+
+	num_data_lanes = fwnode_property_count_u32(ep, "data-lanes");
+	if (num_data_lanes < 1 || num_data_lanes > CSI2_MAX_DATA_LANES) {
+		ret = -EINVAL;
+		dev_err(dev, "Invalid data-lanes count: %d\n", num_data_lanes);
+		goto out_put;
+	}
+	stream_cfg->num_data_lanes = num_data_lanes;
+
+	ret = fwnode_property_read_u32_array(ep, "data-lanes", data_lanes,
+					     stream_cfg->num_data_lanes);
+	if (ret) {
+		dev_err(dev, "Failed to read data-lanes: %d\n", ret);
+		goto out_put;
+	}
+
+	ret = fwnode_property_read_u32(ep, "clock-lanes", &clock_lane);
+	if (ret) {
+		clock_lane = CSI2_DEFAULT_CLK_LN;
+		dev_info(dev, "Using default clock-lane %d\n",
+			 CSI2_DEFAULT_CLK_LN);
+	}
+
+	/* lane-polarities: optional, up to num_data_lanes + 1 entries */
+	memset(lane_polarities, 0x00, sizeof(lane_polarities));
+	num_polarities = fwnode_property_count_u32(ep, "lane-polarities");
+	if (num_polarities > 0) {
+		if (num_polarities != stream_cfg->num_data_lanes + 1) {
+			ret = -EINVAL;
+			dev_err(dev, "clock+data-lane %d/polarities %d mismatch\n",
+				stream_cfg->num_data_lanes + 1, num_polarities);
+			goto out_put;
+		}
+
+		ret = fwnode_property_read_u32_array(ep, "lane-polarities", lane_polarities,
+						     num_polarities);
+		if (ret) {
+			dev_err(dev, "Failed to read lane-polarities: %d\n", ret);
+			goto out_put;
+		}
+	}
+
+	for (i = 0; i < csi2phy->stream_cfg.num_data_lanes; i++) {
+		csi2phy->stream_cfg.lane_cfg.data[i].pos = data_lanes[i];
+		csi2phy->stream_cfg.lane_cfg.data[i].pol = lane_polarities[i + 1];
+	}
+	csi2phy->stream_cfg.lane_cfg.clk.pos = clock_lane;
+	csi2phy->stream_cfg.lane_cfg.clk.pol = lane_polarities[0];
+
+	ret = 0;
+
+out_put:
+	fwnode_handle_put(ep);
+
+	return ret;
+}
+
+static int phy_qcom_mipi_csi2_probe(struct platform_device *pdev)
+{
+	unsigned int i, num_clk, num_supplies;
+	struct mipi_csi2phy_device *csi2phy;
+	struct phy_provider *phy_provider;
+	struct device *dev = &pdev->dev;
+	struct phy *generic_phy;
+	int ret;
+
+	csi2phy = devm_kzalloc(dev, sizeof(*csi2phy), GFP_KERNEL);
+	if (!csi2phy)
+		return -ENOMEM;
+
+	csi2phy->dev = dev;
+	dev_set_drvdata(dev, csi2phy);
+
+	csi2phy->soc_cfg = device_get_match_data(&pdev->dev);
+
+	if (!csi2phy->soc_cfg)
+		return -EINVAL;
+
+	num_clk = csi2phy->soc_cfg->num_clk;
+	csi2phy->clks = devm_kzalloc(dev, sizeof(*csi2phy->clks) * num_clk, GFP_KERNEL);
+	if (!csi2phy->clks)
+		return -ENOMEM;
+
+	ret = phy_qcom_mipi_csi2_parse_routing(csi2phy);
+	if (ret)
+		return ret;
+
+	ret = phy_qcom_mipi_csi2_attach_pm_domains(csi2phy);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Failed to attach power-domain list\n");
+
+	for (i = 0; i < num_clk; i++)
+		csi2phy->clks[i].id = csi2phy->soc_cfg->clk_names[i];
+
+	ret = devm_clk_bulk_get(dev, num_clk, csi2phy->clks);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to get clocks\n");
+
+	csi2phy->timer_clk = devm_clk_get(dev, csi2phy->soc_cfg->timer_clk);
+	if (IS_ERR(csi2phy->timer_clk)) {
+		return dev_err_probe(dev, PTR_ERR(csi2phy->timer_clk),
+				     "Failed to get timer clock\n");
+	}
+
+	ret = devm_pm_opp_set_clkname(dev, csi2phy->soc_cfg->opp_clk);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to set opp clkname\n");
+
+	ret = devm_pm_opp_of_add_table(dev);
+	if (ret && ret != -ENODEV)
+		return dev_err_probe(dev, ret, "invalid OPP table in device tree\n");
+
+	num_supplies = csi2phy->soc_cfg->num_supplies;
+	csi2phy->supplies = devm_kzalloc(dev, sizeof(*csi2phy->supplies) * num_supplies,
+					 GFP_KERNEL);
+	if (!csi2phy->supplies)
+		return -ENOMEM;
+
+	for (i = 0; i < num_supplies; i++)
+		csi2phy->supplies[i].supply = csi2phy->soc_cfg->supply_names[i];
+
+	ret = devm_regulator_bulk_get(dev, num_supplies, csi2phy->supplies);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to get regulator supplies\n");
+
+	csi2phy->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(csi2phy->base))
+		return PTR_ERR(csi2phy->base);
+
+	generic_phy = devm_phy_create(dev, NULL, &phy_qcom_mipi_csi2_ops);
+	if (IS_ERR(generic_phy)) {
+		ret = PTR_ERR(generic_phy);
+		return dev_err_probe(dev, ret, "failed to create phy\n");
+	}
+	csi2phy->phy = generic_phy;
+
+	phy_set_drvdata(generic_phy, csi2phy);
+
+	phy_provider = devm_of_phy_provider_register(dev, qcom_csi2_phy_xlate);
+	if (!IS_ERR(phy_provider))
+		dev_dbg(dev, "Registered MIPI CSI2 PHY device\n");
+
+	return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static const struct of_device_id phy_qcom_mipi_csi2_of_match_table[] = {
+	{ .compatible	= "qcom,x1e80100-csi2-phy", .data = &mipi_csi2_dphy_4nm_x1e },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, phy_qcom_mipi_csi2_of_match_table);
+
+static struct platform_driver phy_qcom_mipi_csi2_driver = {
+	.probe		= phy_qcom_mipi_csi2_probe,
+	.driver = {
+		.name	= "qcom-mipi-csi2-phy",
+		.of_match_table = phy_qcom_mipi_csi2_of_match_table,
+	},
+};
+
+module_platform_driver(phy_qcom_mipi_csi2_driver);
+
+MODULE_DESCRIPTION("Qualcomm MIPI CSI2 PHY driver");
+MODULE_AUTHOR("Bryan O'Donoghue <bryan.odonoghue@linaro.org>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h b/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h
new file mode 100644
index 0000000000000..e7c1ce00916e3
--- /dev/null
+++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h
@@ -0,0 +1,95 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ *
+ * Qualcomm MIPI CSI2 CPHY/DPHY driver
+ *
+ * Copyright (C) 2025 Linaro Ltd.
+ */
+#ifndef __PHY_QCOM_MIPI_CSI2_H__
+#define __PHY_QCOM_MIPI_CSI2_H__
+
+#include <linux/phy/phy.h>
+
+#define CSI2_MAX_DATA_LANES 4
+#define CSI2_DEFAULT_CLK_LN 7
+
+struct mipi_csi2phy_lane {
+	u8 pos;
+	u8 pol;
+};
+
+struct mipi_csi2phy_lanes_cfg {
+	struct mipi_csi2phy_lane data[CSI2_MAX_DATA_LANES];
+	struct mipi_csi2phy_lane clk;
+};
+
+struct mipi_csi2phy_stream_cfg {
+	s64 link_freq;
+	u8 num_data_lanes;
+	struct mipi_csi2phy_lanes_cfg lane_cfg;
+};
+
+struct mipi_csi2phy_device;
+
+struct mipi_csi2phy_hw_ops {
+	void (*hw_version_read)(struct mipi_csi2phy_device *csi2phy_dev);
+	void (*reset)(struct mipi_csi2phy_device *csi2phy_dev);
+	int (*lanes_enable)(struct mipi_csi2phy_device *csi2phy_dev,
+			    struct mipi_csi2phy_stream_cfg *cfg);
+	void (*lanes_disable)(struct mipi_csi2phy_device *csi2phy_dev,
+			      struct mipi_csi2phy_stream_cfg *cfg);
+};
+
+struct mipi_csi2phy_lane_regs {
+	const s32 reg_addr;
+	const s32 reg_data;
+	const u32 delay_us;
+	const u32 param_type;
+};
+
+struct mipi_csi2phy_device_regs {
+	const struct mipi_csi2phy_lane_regs *init_seq;
+	const int lane_array_size;
+	const u32 common_regs_offset;
+};
+
+struct mipi_csi2phy_soc_cfg {
+	const struct mipi_csi2phy_hw_ops *ops;
+	const struct mipi_csi2phy_device_regs reg_info;
+
+	const char ** const supply_names;
+	const unsigned int num_supplies;
+
+	const char ** const clk_names;
+	const unsigned int num_clk;
+
+	const char * const opp_clk;
+	const char * const timer_clk;
+
+	const char ** const genpd_names;
+	const unsigned int num_genpd_names;
+};
+
+struct mipi_csi2phy_device {
+	struct device *dev;
+	u8 phy_mode;
+
+	struct phy *phy;
+	void __iomem *base;
+
+	struct clk_bulk_data *clks;
+	struct clk *timer_clk;
+	u32 timer_clk_rate;
+
+	struct regulator_bulk_data *supplies;
+	struct dev_pm_domain_list *pd_list;
+
+	const struct mipi_csi2phy_soc_cfg *soc_cfg;
+	struct mipi_csi2phy_stream_cfg stream_cfg;
+
+	u32 hw_version;
+};
+
+extern const struct mipi_csi2phy_soc_cfg mipi_csi2_dphy_4nm_x1e;
+
+#endif /* __PHY_QCOM_MIPI_CSI2_H__ */

-- 
2.54.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply related

* [PATCH v7 0/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
From: Bryan O'Donoghue @ 2026-05-22 14:45 UTC (permalink / raw)
  To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
  Cc: Bryan O'Donoghue, Vladimir Zapolskiy, linux-arm-msm,
	linux-phy, linux-media, devicetree, linux-kernel,
	Bryan O'Donoghue

Changes in v7:
- Made CONFIG_PM a dependency. Sashiko commented on the pd_list being NULL
  and suggested I check the pointer but, we need to ramp the rails when
  switching clock rate so we need CONFIG_PM full stop. - Sashiko.ai, Bryan
- Added unwind operation for performance state error path - Sashiko
- Made clock-lanes genuinely optional for the supported use-case. - Sashiko
- Fixed the enable of lanes. Thus far we have had forever it seems
  val |= BIT(lane.pos * 2) which I've never looked at much because it
  has always worked. But looking at how to switch on polarities I realised
  the relevant register is a linear bitmask without gaps so the correct
  method is `val |= BIT(lane.pos)`.
  This needs an update in the legacy PHY too in another series - Bryan
- I opted not to do any of the "but if DT send junk into your driver" fixes
  from Sashiko since TBH I think the code would be Spaghetti afterwards.
  We trust DT and if DT is wrong we fix it, we don't try to graph its
  relative (in)sanity.
- Fixed my example in the yaml. Sashiko
- Link to v6: https://patch.msgid.link/20260521-x1e-csi2-phy-v6-0-9d73d9bd7d20@linaro.org

Changes in v6:
- Taking feedback from lively debate added ports and
  endpoints to the PHY - Neil, Vlad
- Detection of split mode by way of which ports are declared.
  port@0 is always a sensor input.
  port@1 is optional and if present implies split-mode
  port@2 is always the output. - Dmitry, Neil, Vlad.
- Split mode is left as -ENOTSUPP unless/until someone with the appropriate
  hardware can take on responsibility to drive to completion.
- Extending phy_config_opts dropped.
  I think this is a worthwhile extension but this series no longer depends
  on it so dropped. - Bryan
- MX/MXC.
  Two OPP tables one for CSIPHY0/1/2 scaling MXC one for CSIPHY4 keeping
  MXA at LOWSVS_D1 - to be implemented in DT not here. Taniya, Konrad, Bryan
- Changed MAINTAINERS from Supported to Maintained.
  Hobby time for me right now. - Bryan
- Link to v5: https://lore.kernel.org/r/20260326-x1e-csi2-phy-v5-0-0c0fc7f5c01b@linaro.org

v5:
- Adds support to apply passed parameters for clock/data position/polarity - Neil
- Drops GEN1/GEN2 differentiation this can be reconstituted if GEN1 ever
  gets supported in this driver - Dmitry
- Drops camnoc_axi, cpas_ahb - Konrad
- Renames csiphy->core csiphy_timer->timer - Konrad
- Renames rail from 0p8 to 0p9 schematics say  VDD_A_CSI_n_0P9 - Konrad
- TITAN_TOP_GDSC dropped - Konrad
- Passes PHY_QCOM_CSI2_MODE_{DPHY|CPHY|SPLIT_DPHY} with the controller
  selecting the mode. Only DPHY mode is supported but the method to pass
  CPHY or split-mode DPHY configuration is there.
  Since split-mode is a Qualcomm specific mode the PHY modes are defined in
  our binding instead of adding a new type to include/linux/phy/phy.h - bod
- Depends-on: https://lore.kernel.org/r/20260325-dphy-params-extension-v1-0-c6df5599284a@linaro.org
- Link to v4: https://lore.kernel.org/r/20260315-x1e-csi2-phy-v4-0-90c09203888d@linaro.org

v4:
- MMCX, MCX and MX/MXA power-domains added - Dmitry, Vijay, Konrad
- power-domain-names added as required - bod
- opp-tables amended to capture RPMHPD deps - Dmitry, Vijay
- Switched to dev_pm_opp_set_rate, dev_pm_domain_attach_by_name etc
  dropped inherited CAMSS code - Dmitry
- Amended parameters structure to specify power-domain name list - bod
- Removed dead defines - Dmitry
- Noted in CSIPHY commit log intention to rework patterns of
  PHY lane configs into loops/defines/bit-fields later - Dmitry, bod
- Lowercase hex throughout - Dmitry
- The yaml and code in this driver doesn't care if the node is a
  sibling or a sub-node of CAMSS confirmed to work both ways - Dmitry, bod
- Link to v3: https://lore.kernel.org/r/20260226-x1e-csi2-phy-v3-0-11e608759410@linaro.org

v3:

- Resending this to make clear this submission is additive to x1e/Hamoa
  The existing bindings and code will continue to work 
  Bindings are added only, nothing is subtracted from existing ABI.
- Link to v2: https://lore.kernel.org/r/20260225-x1e-csi2-phy-v2-0-7756edb67ea9@linaro.org

v2:

In this updated version

- Added operating-point support
  The csiphy clock sets the OPP prior to setting the rate
  for csiphy and csiphy_timer - Konrad

- Combo mode
  Combo mode in CAMSS yaml has been added. Right now
  no code has been changed in the PHY driver to support it as
  I don't have hardware to test. In principle though it can
  be supported. - Vladimir

- CSIPHY init sequences
  I left these as their "magic number formats". With my diminished
  status as a non-qcom VPN person - I can no longer see what the bits
  map to. Moreover this is the situation any non-VPN community member
  will be in when submitting CSIPHY sequences derived from downstream.

  I think it is perfectly reasonable to take public CSIPHY init sequences
  as magic numbers. If someone with bit-level access wants to enumerate
  the bits that's fine but, it shouldn't gate in the interim. - Konrad/bod

- Sensor endpoints
  I've stuck to the format used by every other CSIPHY in upstream.
  Sensor endpoints hit the CAMSS/CSID endpoint not a endpoint in the PHY.
  Given the proposed changes to CAMSS though to support "combo mode" I
  think this should achieve the same outcome - multiple sensors on the one
  PHY without introducing endpoints into the PHY that no other CSIPHY in
  upstream currently has.

- Bitmask of enabled lanes
  Work needs to be done in the v4l2 layer to really support this.
  I propose making a separate series dedicated to non-linear bit
  interpretation after merging this so as to contain the scope of the
  series to something more bite (byte haha) sized. - Konrad/bod

- Link to v1: https://lore.kernel.org/r/20250710-x1e-csi2-phy-v1-0-74acbb5b162b@linaro.org

v1:
This short series adds a CSI2 MIPI PHY driver, initially supporting D-PHY
mode. The core logic and init sequences come directly from CAMSS and are
working on at least five separate x1e devices.

The rationale to instantiate CSI2 PHYs as standalone devices instead of as
sub-nodes of CAMSS is as follows.

1. Precedence
   CAMSS has a dedicated I2C bus called CCI Camera Control Interface.
   We model this controller as its own separate device in devicetree.
   This makes sense and CCI/I2C is a well defined bus type already modelled
   in Linux.

   MIPI CSI2 PHY devices similarly fit into a well defined separate
   bus/device structure.

   Contrast to another CAMSS component such as VFE, CSID or TPG these
   components only interact with other CAMSS inputs/outputs unlike CSIPHY
   which interacts with non-SoC components.

2. Hardware pinouts and rails
   The CSI2 PHY has its own data/clock lanes out from the SoC and indeed
   has its own incoming power-rails.

3. Other devicetree schemas
   There are several examples throughout the kernel of CSI PHYs modeled as
   standalone devices which one assumes follows the same reasoning as given
   above.

I've been working on this on-and-off since the end of April:
Link: https://lore.kernel.org/linux-media/c5cf0155-f839-4db9-b865-d39b56bb1e0a@linaro.org

There is another proposal to have the PHYs be subdevices of CAMSS but, I
believe we should go with a "full fat" PHY to match best practices in
drivers/phy/qualcomm/*.

Using the standard PHY API and the parameter passing that goes with it
allows us to move away from custom interfaces in CAMSS and to conform more
clearly to established PHY paradigms such as the QMP combo PHY.

Looking at existing compat strings I settled on
"qcom,x1e80100-mipi-csi2-combo-phy" deliberately omitting reference to the
fact the PHY is built on a four nano-meter process node, which seems to
match recent submissions to QMP PHY.

My first pass at this driver included support for the old two phase
devices:

Link: https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/a504c28d109296c93470340cfe7281231f573bcb#b6e59ed7db94c9da22e492bb03fcda6a4300983c

I realised that the device tree schema changes required to support a
comprehensive conversion of all CAMSS to this driver would be an
almost certainly be unacceptable ABI break or at the very least an enormous
amount of work and verification so I instead aimed to support just one new
SoC in the submission.

I've retained the callback indirections give us scope to add in another type of
future PHY including potentially adding in the 2PH later on.

This driver is tested and working on x1e/Hamoa and has been tested as not
breaking sc8280xp/Makena and sm8250/Kona.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
Bryan O'Donoghue (2):
      dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
      phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver

 .../bindings/phy/qcom,x1e80100-csi2-phy.yaml       | 208 +++++++++++
 MAINTAINERS                                        |  10 +
 drivers/phy/qualcomm/Kconfig                       |  14 +
 drivers/phy/qualcomm/Makefile                      |   5 +
 drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c | 371 ++++++++++++++++++++
 drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c     | 386 +++++++++++++++++++++
 drivers/phy/qualcomm/phy-qcom-mipi-csi2.h          |  95 +++++
 7 files changed, 1089 insertions(+)
---
base-commit: a1db83cc6f7e88a166c77d9060507ec01d617784
change-id: 20250710-x1e-csi2-phy-f6434b651d3a

Best regards,
--  
Bryan O'Donoghue <bryan.odonoghue@linaro.org>


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* [PATCH v7 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Bryan O'Donoghue @ 2026-05-22 14:45 UTC (permalink / raw)
  To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
  Cc: Bryan O'Donoghue, Vladimir Zapolskiy, linux-arm-msm,
	linux-phy, linux-media, devicetree, linux-kernel,
	Bryan O'Donoghue
In-Reply-To: <20260522-x1e-csi2-phy-v7-0-79cb1280fad6@linaro.org>

Add a base schema initially compatible with x1e80100 to describe MIPI CSI2
PHY devices.

The hardware can support both CPHY, DPHY and a special split-mode DPHY.

The schema here defines three ports:

port@0:
    The first input port where a sensor is always required.

port@1:
    A second optional input port which if present implies DPHY split-mode.

port@2:
    A third always required output port which connects to the controller.

The CSIPHY devices have their own pinouts on the SoC as well as their own
individual voltage rails.

The need to model voltage rails on a per-PHY basis leads us to define
CSIPHY devices as individual nodes.

Two nice outcomes in terms of schema and DT arise from this change.

1. The ability to define on a per-PHY basis voltage rails.
2. The ability to require those voltage.

We have had a complete bodge upstream for this where a single set of
voltage rail for all CSIPHYs has been buried inside of CAMSS.

Much like the I2C bus which is dedicated to Camera sensors - the CCI bus in
CAMSS parlance, the CSIPHY devices should be individually modelled.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 .../bindings/phy/qcom,x1e80100-csi2-phy.yaml       | 208 +++++++++++++++++++++
 1 file changed, 208 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
new file mode 100644
index 0000000000000..f434fb04447f6
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
@@ -0,0 +1,208 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/qcom,x1e80100-csi2-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm CSI2 PHY
+
+maintainers:
+  - Bryan O'Donoghue <bod@kernel.org>
+
+description:
+  Qualcomm MIPI CSI2 C-PHY/D-PHY combination PHY. Connects MIPI CSI2 sensors
+  to Qualcomm's Camera CSI Decoder. The PHY supports both C-PHY and D-PHY
+  modes.
+
+properties:
+  compatible:
+    const: qcom,x1e80100-csi2-phy
+
+  reg:
+    maxItems: 1
+
+  "#phy-cells":
+    const: 1
+    description:
+      The single cell specifies the PHY operating mode.
+
+  clocks:
+    maxItems: 2
+
+  clock-names:
+    items:
+      - const: core
+      - const: timer
+
+  interrupts:
+    maxItems: 1
+
+  operating-points-v2:
+    maxItems: 1
+
+  power-domains:
+    items:
+      - description: MMCX voltage rail
+      - description: MXC or MXA voltage rail
+
+  power-domain-names:
+    items:
+      - const: mmcx
+      - const: mx
+
+  vdda-0p9-supply:
+    description: Phandle to a 0.9V regulator supply to a PHY.
+
+  vdda-1p2-supply:
+    description: Phandle to 1.2V regulator supply to a PHY.
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        description: Sensor input. Always present.
+
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+            unevaluatedProperties: false
+            properties:
+              data-lanes:
+                minItems: 1
+                maxItems: 4
+              clock-lanes:
+                maxItems: 1
+              remote-endpoint: true
+            required:
+              - data-lanes
+              - remote-endpoint
+
+      port@1:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        description:
+          Second sensor input. When present, indicates DPHY split mode.
+
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+            unevaluatedProperties: false
+            properties:
+              data-lanes:
+                maxItems: 1
+              clock-lanes:
+                maxItems: 1
+              remote-endpoint: true
+            required:
+              - data-lanes
+              - clock-lanes
+              - remote-endpoint
+
+      port@2:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        description: Output to CAMSS controller.
+
+        properties:
+          endpoint:
+            $ref: /schemas/graph.yaml#/$defs/endpoint-base
+            unevaluatedProperties: false
+            properties:
+              remote-endpoint: true
+            required:
+              - remote-endpoint
+
+    required:
+      - port@0
+      - port@2
+
+required:
+  - compatible
+  - reg
+  - "#phy-cells"
+  - clocks
+  - clock-names
+  - interrupts
+  - operating-points-v2
+  - power-domains
+  - power-domain-names
+  - vdda-0p9-supply
+  - vdda-1p2-supply
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/qcom,x1e80100-camcc.h>
+    #include <dt-bindings/clock/qcom,x1e80100-gcc.h>
+    #include <dt-bindings/power/qcom,rpmhpd.h>
+
+    csiphy4: csiphy@ace4000 {
+        compatible = "qcom,x1e80100-csi2-phy";
+        reg = <0x0ace4000 0x2000>;
+        #phy-cells = <1>;
+
+        clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
+                 <&camcc CAM_CC_CSI0PHYTIMER_CLK>;
+        clock-names = "core",
+                      "timer";
+
+        operating-points-v2 = <&csiphy_opp_table>;
+
+        interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
+
+        power-domains = <&rpmhpd RPMHPD_MMCX>,
+                        <&rpmhpd RPMHPD_MX>;
+        power-domain-names = "mmcx",
+                             "mx";
+
+        vdda-0p9-supply = <&vreg_l2c_0p8>;
+        vdda-1p2-supply = <&vreg_l1c_1p2>;
+
+        ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            port@0 {
+                reg = <0>;
+                csiphy0_in_ep: endpoint {
+                    data-lanes = <0 1>;
+                    clock-lanes = <2>;
+                    remote-endpoint = <&sensor_out>;
+                };
+            };
+
+            port@2 {
+                reg = <2>;
+                csiphy0_out_ep: endpoint {
+                    data-lanes = <3>;
+                    clock-lanes = <4>;
+                    remote-endpoint = <&controller_in>;
+                };
+            };
+        };
+    };
+
+    csiphy_opp_table: opp-table {
+        compatible = "operating-points-v2";
+
+        opp-300000000 {
+            opp-hz = /bits/ 64 <300000000>;
+            required-opps = <&rpmhpd_opp_low_svs_d1>,
+                            <&rpmhpd_opp_low_svs_d1>;
+        };
+
+        opp-400000000 {
+            opp-hz = /bits/ 64 <400000000>;
+            required-opps = <&rpmhpd_opp_low_svs>,
+                            <&rpmhpd_opp_low_svs_d1>;
+        };
+
+        opp-480000000 {
+            opp-hz = /bits/ 64 <480000000>;
+            required-opps = <&rpmhpd_opp_low_svs>,
+                            <&rpmhpd_opp_low_svs_d1>;
+        };
+    };

-- 
2.54.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply related

* Re: [PATCH v2 3/5] arm64: dts: qcom: Add Shikra CQ7790M SoM platform
From: Komal Bajaj @ 2026-05-22 14:29 UTC (permalink / raw)
  To: Konrad Dybcio, Bjorn Andersson, Konrad Dybcio, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Neil Armstrong,
	Wesley Cheng, Ulf Hansson
  Cc: linux-arm-msm, devicetree, linux-kernel, linux-phy, linux-mmc,
	monish.chunara, Rakesh Kota
In-Reply-To: <0b1c66ba-d841-460c-b702-8b4530d56f73@oss.qualcomm.com>


On 5/19/2026 5:21 PM, Konrad Dybcio wrote:
> On 5/19/26 1:21 PM, Komal Bajaj wrote:
>> Add device tree include for the CQ7790M variant of the Shikra
> I don't think CQ7790M is Shikra

Thanks for catching it. CQ7790M is Eliza. Will correct it.

Thanks
Komal

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH v2 3/5] arm64: dts: qcom: Add Shikra CQ7790M SoM platform
From: Komal Bajaj @ 2026-05-22 14:27 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: robh, devicetree, olteanv, conor+dt, vkoul, neil.armstrong,
	linux-phy
In-Reply-To: <20260519120238.1C84CC2BCB3@smtp.kernel.org>

On 5/19/2026 5:32 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] The SoM device tree partially defines PMIC regulators (LDOs) but completely omits their internal parent SMPS regulators (S3, S4) and the required supply linkage properties.
> - [High] The SoM integrates the PM4125 PMIC, but the device tree include for the PMIC's SPMI components (`pm4125.dtsi`) is missing.
> --
>
> commit 1edd30ea88da411f667f786ef5494cebf1420c78
> Author: Komal Bajaj <komal.bajaj@oss.qualcomm.com>
>
> arm64: dts: qcom: Add Shikra CQ7790M SoM platform
>
> This commit adds the device tree include for the CQ7790M variant of the
> Shikra System-on-Module. It includes the common shikra.dtsi and adds
> PM4125 PMIC regulator definitions specific to this variant.
>
>> diff --git a/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi b/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
>> new file mode 100644
>> index 0000000000000..401e71720519d
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
>> @@ -0,0 +1,112 @@
>> +// SPDX-License-Identifier: BSD-3-Clause
>> +/*
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + */
>> +
>> +#include "shikra.dtsi"
> The commit message mentions the SoM integrates the PM4125 PMIC, but is
> pm4125.dtsi supposed to be included here as well?
>
> Without including the PMIC's SPMI components (like pm4125_gpios,
> pm4125_pwrkey), it seems carrier boards won't be able to address or utilize
> these peripherals.

Will add in next revision.

>
>> +
>> +&rpm_requests {
>> +	regulators {
>> +		compatible = "qcom,rpm-pm2250-regulators";
>> +
>> +		pm4125_s2: s2 {
>> +			regulator-min-microvolt = <1000000>;
>> +			regulator-max-microvolt = <1200000>;
>> +		};
> Does this regulator node need to define the internal parent SMPS regulators
> like s3 and s4, as well as the vdd_X-supply linkage properties for the child
> LDOs?
>
> The driver drivers/regulator/qcom_smd-regulator.c expects supply mappings for
> pm2250 (e.g., vdd_l1_l2_l3_l5_l6_l7_l8_l9_l10_l11_l12).
>
> Without representing this fixed internal power routing in the SoM's device
> tree, the core framework might not be able to correctly manage the parent
> supply voltages and states when child LDOs are toggled.

This can be ignored, refer discussion:
https://lore.kernel.org/all/20260519120238.1C84CC2BCB3@smtp.kernel.org/

Thanks
Komal

>
>> +
>> +		pm4125_l3: l3 {
>> +			regulator-min-microvolt = <624000>;
>> +			regulator-max-microvolt = <650000>;
>> +		};


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH v2 2/5] arm64: dts: qcom: Introduce Shikra SoC base dtsi
From: Komal Bajaj @ 2026-05-22 13:33 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: robh, devicetree, vkoul, linux-phy, olteanv, conor+dt,
	neil.armstrong
In-Reply-To: <20260519114737.70CD0C2BCB3@smtp.kernel.org>

On 5/19/2026 5:17 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> - [Critical] Incorrect PPI numbers are specified for the ARM architected timers and GIC maintenance interrupts.
> - [High] The base memory node specifies an incorrect start address, obscuring 512MB of DDR system RAM and breaking reservations.
> - [Medium] The `sdhc_1` node incorrectly references a power domain specific to a different SoC.
> - [Medium] The logical `cpu-map` splits CPUs into two clusters, contradicting physical MPIDR topology and shared cache.
> - [Medium] The `usb_1` node is missing MPM-routed interrupts for USB 2.0 wake from deep sleep.
> --
>
> commit 32b9841bdefaa72fe3d5267070711875edaaafa0
> Author: Komal Bajaj <komal.bajaj@oss.qualcomm.com>
>
> arm64: dts: qcom: Introduce Shikra SoC base dtsi
>
> Add initial device tree support for the Qualcomm Shikra SoC, an
> IoT-focused platform built around a heterogeneous CPU cluster
> (Cortex-A55 + Cortex-A78C) with RPM-based power and clock management.
>
>> diff --git a/arch/arm64/boot/dts/qcom/shikra.dtsi b/arch/arm64/boot/dts/qcom/shikra.dtsi
>> new file mode 100644
>> index 0000000000000..31d0126e5b3ee
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/shikra.dtsi
>> @@ -0,0 +1,981 @@
> [ ... ]
>
>> +		cpu-map {
>> +			cluster0 {
>> +				core0 {
>> +					cpu = <&cpu0>;
>> +				};
>> +
>> +				core1 {
>> +					cpu = <&cpu1>;
>> +				};
>> +
>> +				core2 {
>> +					cpu = <&cpu2>;
>> +				};
>> +			};
>> +
>> +			cluster1 {
>> +				core0 {
>> +					cpu = <&cpu3>;
>> +				};
>> +			};
>> +		};
> Is this logical topology misaligned with the physical MPIDR topology?
>
> The CPU reg values (0x0, 0x100, 0x200, 0x300) have their Aff2 bits set to 0,
> meaning they physically reside in the same cluster. The A55 cores and A78C
> core also share the same L3 cache.
>
> Does splitting cpu0-cpu2 into cluster0 and cpu3 into cluster1 violate
> DynamIQ specifications and confuse the scheduler's task migration and
> domains?
>
> [ ... ]

The hardware MPIDR for cpu3 is 0x300 (Aff2=0), confirmed by PSCI boot 
failure when reg was changed to 0x10000.
The cpu-map two-cluster split is intentional and correct for EAS 
scheduler topology (separate performance domains for A55 silver and A78C 
gold), even though all four cores share Aff2=0 in the same DSU.

>
>> +	memory@a0000000 {
>> +		device_type = "memory";
>> +		/* We expect the bootloader to fill in the size */
>> +		reg = <0x0 0xa0000000 0x0 0x0>;
>> +	};
> Is this base address correct?
>
> The reserved-memory list allocates memory regions well below this address,
> such as hyp@80000000 and smem@86000000.
>
> Does starting the system RAM at 0xa0000000 cause the kernel to disregard the
> 0x80000000-0x9fffffff region, effectively losing 512MB of RAM and
> potentially causing the reservations to fail?

Will fix by updating base address to memory@80000000.

>
> [ ... ]
>
>> +		sdhc_1: mmc@4744000 {
>> +			compatible = "qcom,shikra-sdhci", "qcom,sdhci-msm-v5";
>> +
>> +			reg = <0x0 0x04744000 0x0 0x1000>,
> [ ... ]
>
>> +			interconnects = <&system_noc MASTER_SDCC_1 RPM_ALWAYS_TAG
>> +					&mc_virt SLAVE_EBI_CH0 RPM_ALWAYS_TAG>,
>> +					<&mem_noc MASTER_AMPSS_M0 RPM_ACTIVE_TAG
>> +					&config_noc SLAVE_SDCC_1 RPM_ACTIVE_TAG>;
>> +			interconnect-names = "sdhc-ddr",
>> +					     "cpu-sdhc";
>> +
>> +			power-domains = <&rpmpd QCM2290_VDDCX>;
> Is this a copy-paste error?
>
> The gcc node correctly uses the generic or Shikra-specific RPMPD_VDDCX.
>
> Could referencing a QCM2290-specific power domain macro here result in the
> hardware attempting to scale the wrong voltage domain or cause an
> out-of-bounds array access in the driver?

Yes, that's a copy-paste error. Will fix it in the next version.

[ ... ]

> [ ... ]
>
>> +		intc: interrupt-controller@f200000 {
>> +			compatible = "arm,gic-v3";
>> +			reg = <0x0 0xf200000 0x0 0x10000>,
>> +			      <0x0 0xf240000 0x0 0x80000>;
>> +
>> +			interrupts = <GIC_PPI 8 IRQ_TYPE_LEVEL_HIGH 0>;
> Is this the correct PPI for the GIC maintenance interrupt?
>
> The architecturally mandated GIC maintenance interrupt is PPI 9, rather than
> PPI 8.

Will fix by changing GIC_PPI 8 to GIC_PPI 9 in the next version.

Thanks
Komal

>
> [ ... ]
>
>> +	timer {
>> +		compatible = "arm,armv8-timer";
>> +
>> +		interrupts = <GIC_PPI 1 IRQ_TYPE_LEVEL_LOW 0>,
>> +			     <GIC_PPI 2 IRQ_TYPE_LEVEL_LOW 0>,
>> +			     <GIC_PPI 3 IRQ_TYPE_LEVEL_LOW 0>,
>> +			     <GIC_PPI 0 IRQ_TYPE_LEVEL_LOW 0>;
>> +	};
>> +};
> Are these PPI numbers correct for the ARM architected timers?
>
> The architecturally hardwired ARM core timer PPIs are 13 (Secure),
> 14 (Non-Secure), 11 (Virtual), and 10 (Hypervisor).
>
> Could specifying these incorrect numbers prevent the kernel from receiving
> timer ticks and cause the system to hang during boot?
>


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH 3/5] phy: qualcomm: qmp-combo: Add preliminary USB4 support
From: Konrad Dybcio @ 2026-05-22 12:05 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Konrad Dybcio, Vinod Koul, Neil Armstrong, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, linux-kernel,
	linux-phy, linux-arm-msm, devicetree, usb4-upstream,
	Raghavendra Thoorpu, Mika Westerberg, Sven Peter
In-Reply-To: <iws2snsi3yfddich3rqqurhwt6mvi2boushkog5t5gvo2ahwmp@l2m6ays2kicf>

On 5/20/26 5:06 PM, Dmitry Baryshkov wrote:
> On Tue, May 19, 2026 at 10:12:06AM +0200, Konrad Dybcio wrote:
>> On 5/18/26 5:38 PM, Dmitry Baryshkov wrote:
>>> On Mon, May 18, 2026 at 04:15:16PM +0200, Konrad Dybcio wrote:
>>>> On 5/18/26 3:57 PM, Dmitry Baryshkov wrote:
>>>>> On Mon, May 18, 2026 at 12:29:50PM +0200, Konrad Dybcio wrote:
>>>>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>>>>
>>>>>> Some Combo PHYs (so far only on SC8280XP, X1E80100 and Glymur), come in
>>>>>> a flavor called USB43DP, which as the name implies, features USB4, USB3
>>>>>> and DP signal processing capabilities. In that architecture, USB3 and
>>>>>> USB4 PHYs share the same USB_PLL while featuring separate logic spaces.
>>>>>> The DP part is roughly the same as on the instances without USB4.
>>>>>>
>>>>>> The USB4 and USB3/DP operation modes of the PHY are mutually exclusive.
>>>>>> Only one USB protocol (and flavor of pipe clock) can be active at a
>>>>>> given moment (not to be confused with USB3 not being able to be
>>>>>> tunneled as USB4 packets - that of course remains possible).
>>>>>> The DP PLL is still used for clocking tunneled DP links. It may be
>>>>>> turned off to save power when no tunnels are active, but that's left as
>>>>>> a TODO item for now.
>>>>>>
>>>>>> Due to the nature of USB4, the Type-C handling happens entirely inside
>>>>>> the Host Router, and as such the QMPPHY's mux_set() function is
>>>>>> nullified for the period when USB4 PHY remains active. This is strictly
>>>>>> necessary, as the Host Router driver is going to excercise manual
>>>>>> control over the USB4 PHY's power state, which is needed by the suspend
>>>>>> and resume flows. Failure to control that synchronously with other
>>>>>> parts of the code results in a SoC crash by unlocked access.
>>>>>>
>>>>>> Because of that, a new struct phy is spawned to expose the USB4 mode,
>>>>>> along with a .set_mode callback to allow toggling between USB4 and TBT3
>>>>>> submodes.
>>>>>>
>>>>>> Thunderbolt 3, having a number of differences vs USB4, requires a
>>>>>> couple specific overrides, pertaining to electrical characteristics,
>>>>>> which are easily accommodated for.
>>>>>>
>>>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>>>> ---
>>>>>>  drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 392 ++++++++++++++++++++++++------
>>>>>>  1 file changed, 322 insertions(+), 70 deletions(-)
>>>>>>
>>>>>
>>>>> Overall it looks good. The major question (after looking at TODOs), do
>>>>> we need a separate submode for USB+DP / TBT+DP?
>>>>
>>>> The problem space is as follows:
>>>>
>>>> After a TBT (collectively TBT3+ and USB4) link has been established and
>>>> we have a link partner, we may (based on the HW capabilities and user
>>>> config, such as kernel params but not only) start or stop a DP tunnel at
>>>> runtime. On Qualcomm hardware, the PHY is kept in USB4 mode and its DP
>>>> AUX lines are not used (instead, the encapsulated DP AUX packets are r/w
>>>> entirely within the USB4 subsystem via a pair of FIFOs that Linux sees
>>>> as a separate DP AUX host)
>>>
>>> So far so good. But I still don't grok if having a DP-over-USB4 is a
>>> separate submode or not. I.e. I see code (and TODOs) to detect and
>>> handle DP going on and off. Would it be better if we specify that
>>> explicitly?
>>
>> I really don't want to end up in a situation like we have with:
>>
>> $ rg _USB include/linux/phy/phy.h
>> 29:     PHY_MODE_USB_HOST,
>> 30:     PHY_MODE_USB_HOST_LS,
>> 31:     PHY_MODE_USB_HOST_FS,
>> 32:     PHY_MODE_USB_HOST_HS,
>> 33:     PHY_MODE_USB_HOST_SS,
>> 34:     PHY_MODE_USB_DEVICE,
>> 35:     PHY_MODE_USB_DEVICE_LS,
>> 36:     PHY_MODE_USB_DEVICE_FS,
>> 37:     PHY_MODE_USB_DEVICE_HS,
>> 38:     PHY_MODE_USB_DEVICE_SS,
>> 39:     PHY_MODE_USB_OTG,
>>
>>>> Then, on hamoa/glymur specifically, any of the 3 USB4-capable DP hosts
>>>> can be muxed to either of the 2 DPIN ports on any of the 3 USB4 routers
>>>> (and each of these routers is hardwired to one of the PHYs).
>>>>
>>>> To underline, we have 3 DP producers and 6 consumers. If there's e.g. a
>>>> super high-res display at one of the physical ports, or a long
>>>> daisy-chain, we may need to use 2 DPTXes to service 1 receptacle. Then,
>>>> we would only need one of the PHYs (associated with the router that's
>>>> wired to that port) to provide a DP clock.
>>>>
>>>> This, along with the normal (logical or physical) present/absent status
>>>> can change at runtime. My plan is to use phy_set_opts(dp_tunelling=true)
>>>> or something along those lines to toggle that bit as necessary
>>>
>>> I don't see phy_set_opts(). So maybe a submode then...
>>
>> Sorry, I misremembered the name. The function is phy_configure(), and it
>> takes a union phy_configure_opts, hence the confusion
> 
> So, phy_configure() will be called for the DP PHY to set the DP opts,
> but how do you plan to determine if DP is on or not? Or do you plan to
> add phy_tbt_configure_opts ?
> 
> Another obvious option would be to set the flag if DP PHY is being tuned
> on / off. I don't know if that fulfills your needs.

Either this or tbt_configure_opts. We still have the muxing question to
chew through.

The bottom line is that all AUX traffic happens between the "AUX adapters"
within USB4SS, talking over thunderbolt to other AUX adapters on the LTTPRs
and the far-end device (and anything inbetween in a chained topology) meaning
we only need to engage the DP host itself (and therefore the PHY) after we've
already performed the capability negotiations

Konrad

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH 3/4] PCI: qcom: Add Support for Eliza
From: Krzysztof Kozlowski @ 2026-05-22 11:03 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Krishna Chaitanya Chundru, Vinod Koul, Neil Armstrong,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Bjorn Helgaas, Bjorn Andersson,
	linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-pci
In-Reply-To: <t6wljnlnz77dvthbohlr7v4qlhsqzvykhcm6evmtnvkgtqk34w@o7sl6nq5lkbi>

On 22/05/2026 12:48, Manivannan Sadhasivam wrote:
> On Fri, May 22, 2026 at 11:34:30AM +0200, Krzysztof Kozlowski wrote:
>> On 22/05/2026 11:31, Krishna Chaitanya Chundru wrote:
>>>
>>>
>>> On 5/22/2026 12:21 PM, Krzysztof Kozlowski wrote:
>>>> On Thu, May 21, 2026 at 07:35:31PM +0530, Krishna Chaitanya Chundru wrote:
>>>>> Add support for Eliza soc, which has two PCIe controllers capable
>>>>> of 8GT/s X1 and 8GT/s X2, using the cfg_1_9_0 configuration.
>>>>>
>>>>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>>>>> ---
>>>>>  drivers/pci/controller/dwc/pcie-qcom.c | 1 +
>>>>>  1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>>>>> index af6bf5cce65b..40f0a5f247eb 100644
>>>>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>>>>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>>>>> @@ -2123,6 +2123,7 @@ static int qcom_pcie_resume_noirq(struct device *dev)
>>>>>  static const struct of_device_id qcom_pcie_match[] = {
>>>>>  	{ .compatible = "qcom,pcie-apq8064", .data = &cfg_2_1_0 },
>>>>>  	{ .compatible = "qcom,pcie-apq8084", .data = &cfg_1_0_0 },
>>>>> +	{ .compatible = "qcom,pcie-eliza", .data = &cfg_1_9_0 },
>>>> So compatible with sm8550. Why isn't this explained in commit msg of
>>>> the binding?
>>> No, PCIe controller is not compatible with sm8550, we are just re using the boot
>>> sequence used by the sm8550.
>>
>> Then with what it is compatible? You cannot use someone else's match
>> data and claim they are not compatible. This is contradictory to itself.
>>
> 
> 'cfg_1_9_0' is the match data of base PCIe IP v1.9.0, not just SM8550. The 'ops'
> callbacks for 'cfg_1_9_0' are compatible with PCIe IPs used in many SoCs such as
> SM8550, SM8450, SM8350, SM8250, SM8150, SDX55, SC8180X, and SC7280. And Eliza is
> also falling into the same category.
> 
And if all implementations uses the same SW interface, why are you
claiming these are not compatible?

I don't care with which device this is compatible, BTW. I do care about
incomplete or incorrect hardware description.

Best regards,
Krzysztof

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH RFC v4 5/9] phy: qcom: qmp-pcie: Refactor pipe clk register and parse_dt helpers
From: Manivannan Sadhasivam @ 2026-05-22 10:57 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Qiang Yu, Vinod Koul, Neil Armstrong, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Bjorn Andersson,
	Konrad Dybcio, linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <ipzncrxi3x45fc6tz5xb7frxt62zmg4gwr25xmvzghlbt5miio@7eavln3cydfa>

On Wed, May 20, 2026 at 07:25:01PM +0300, Dmitry Baryshkov wrote:
> On Mon, May 18, 2026 at 10:47:16PM -0700, Qiang Yu wrote:
> > Some QMP PCIe PHY hardware blocks can be split into multiple sub-PHYs
> > under a single DT node, each requiring its own pipe clock registration and
> > DT resource mapping. The current helpers are tightly coupled to a single
> > qmp_pcie instance, which prevents reuse across sub-PHY instances.
> > 
> > Refactor __phy_pipe_clk_register() as a generic helper and reduce
> > phy_pipe_clk_register() to a thin wrapper around it. Similarly, extract
> > qmp_pcie_parse_dt_common() from qmp_pcie_parse_dt() to hold the register-
> > mapping and pipe-clock setup that will be shared between sub-PHY instances,
> > with pipe clock names parameterised per instance.
> > 
> > This is a preparatory step before adding multi-PHY support. No functional
> > change for existing platforms.
> > 
> > Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> > ---
> >  drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 76 ++++++++++++++++++--------------
> >  1 file changed, 44 insertions(+), 32 deletions(-)
> 
> I'd suggest splitting the Glymur PHY to a separate driver. Otherwise we
> end up having too many single-platform, single-device specifics which
> don't apply to other platforms.
> 

I don't think that's really needed. This shared PHY concept is going to be
applicable to upcoming SoCs as well. And moreover, the split won't be clean
either. We still need to reuse a lot of common logic in the 'phy-qcom-qmp-pcie'
driver and may only end up keeping very minimal code in
'phy-qcom-qmp-pcie-glymur'.

If you are concerned about the file size of 'phy-qcom-qmp-pcie', then we should
move the SoC specific 'cfg' structs into a separate file as that's what
occupying majority of the space.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH 3/4] PCI: qcom: Add Support for Eliza
From: Manivannan Sadhasivam @ 2026-05-22 10:48 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Krishna Chaitanya Chundru, Vinod Koul, Neil Armstrong,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Bjorn Helgaas, Bjorn Andersson,
	linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-pci
In-Reply-To: <c751f5f9-f9c2-4575-a029-0be4148e1aee@kernel.org>

On Fri, May 22, 2026 at 11:34:30AM +0200, Krzysztof Kozlowski wrote:
> On 22/05/2026 11:31, Krishna Chaitanya Chundru wrote:
> > 
> > 
> > On 5/22/2026 12:21 PM, Krzysztof Kozlowski wrote:
> >> On Thu, May 21, 2026 at 07:35:31PM +0530, Krishna Chaitanya Chundru wrote:
> >>> Add support for Eliza soc, which has two PCIe controllers capable
> >>> of 8GT/s X1 and 8GT/s X2, using the cfg_1_9_0 configuration.
> >>>
> >>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> >>> ---
> >>>  drivers/pci/controller/dwc/pcie-qcom.c | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> >>> index af6bf5cce65b..40f0a5f247eb 100644
> >>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> >>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> >>> @@ -2123,6 +2123,7 @@ static int qcom_pcie_resume_noirq(struct device *dev)
> >>>  static const struct of_device_id qcom_pcie_match[] = {
> >>>  	{ .compatible = "qcom,pcie-apq8064", .data = &cfg_2_1_0 },
> >>>  	{ .compatible = "qcom,pcie-apq8084", .data = &cfg_1_0_0 },
> >>> +	{ .compatible = "qcom,pcie-eliza", .data = &cfg_1_9_0 },
> >> So compatible with sm8550. Why isn't this explained in commit msg of
> >> the binding?
> > No, PCIe controller is not compatible with sm8550, we are just re using the boot
> > sequence used by the sm8550.
> 
> Then with what it is compatible? You cannot use someone else's match
> data and claim they are not compatible. This is contradictory to itself.
> 

'cfg_1_9_0' is the match data of base PCIe IP v1.9.0, not just SM8550. The 'ops'
callbacks for 'cfg_1_9_0' are compatible with PCIe IPs used in many SoCs such as
SM8550, SM8450, SM8350, SM8250, SM8150, SDX55, SC8180X, and SC7280. And Eliza is
also falling into the same category.

@Krishna: You need to reword the commit message to clearly state the fact that
PCIe controller found in Eliza SoC is compatible with base IP version 1.9.0, so
it reuses 'cfg_1_9_0' match data.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH 3/4] PCI: qcom: Add Support for Eliza
From: Krzysztof Kozlowski @ 2026-05-22  9:34 UTC (permalink / raw)
  To: Krishna Chaitanya Chundru
  Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Bjorn Helgaas, Bjorn Andersson,
	linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-pci
In-Reply-To: <5593d136-569e-4ba9-9a2f-e635125899aa@oss.qualcomm.com>

On 22/05/2026 11:31, Krishna Chaitanya Chundru wrote:
> 
> 
> On 5/22/2026 12:21 PM, Krzysztof Kozlowski wrote:
>> On Thu, May 21, 2026 at 07:35:31PM +0530, Krishna Chaitanya Chundru wrote:
>>> Add support for Eliza soc, which has two PCIe controllers capable
>>> of 8GT/s X1 and 8GT/s X2, using the cfg_1_9_0 configuration.
>>>
>>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>>> ---
>>>  drivers/pci/controller/dwc/pcie-qcom.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>>> index af6bf5cce65b..40f0a5f247eb 100644
>>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>>> @@ -2123,6 +2123,7 @@ static int qcom_pcie_resume_noirq(struct device *dev)
>>>  static const struct of_device_id qcom_pcie_match[] = {
>>>  	{ .compatible = "qcom,pcie-apq8064", .data = &cfg_2_1_0 },
>>>  	{ .compatible = "qcom,pcie-apq8084", .data = &cfg_1_0_0 },
>>> +	{ .compatible = "qcom,pcie-eliza", .data = &cfg_1_9_0 },
>> So compatible with sm8550. Why isn't this explained in commit msg of
>> the binding?
> No, PCIe controller is not compatible with sm8550, we are just re using the boot
> sequence used by the sm8550.

Then with what it is compatible? You cannot use someone else's match
data and claim they are not compatible. This is contradictory to itself.


>> Anyway, drop the change, pointless. Look how other devices handle this -
>> do you see kaanapali here? No.
> As we are going to use different dts schema for this controller we can't
> really re-use
> like how we have done in kaanapali case.  kaanpali is reusing sm8550 schema, where
> this controller can't use sm8550 schema, as some clocks are different.

OK, schema is different, that's fine and might need some adjustments.
However this commit clearly states devices are compatible and this needs
to be fixed.


Best regards,
Krzysztof

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH 3/4] PCI: qcom: Add Support for Eliza
From: Krishna Chaitanya Chundru @ 2026-05-22  9:31 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Bjorn Helgaas, Bjorn Andersson,
	linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-pci
In-Reply-To: <20260522-discerning-sympathetic-moth-daa9e7@quoll>



On 5/22/2026 12:21 PM, Krzysztof Kozlowski wrote:
> On Thu, May 21, 2026 at 07:35:31PM +0530, Krishna Chaitanya Chundru wrote:
>> Add support for Eliza soc, which has two PCIe controllers capable
>> of 8GT/s X1 and 8GT/s X2, using the cfg_1_9_0 configuration.
>>
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>> ---
>>  drivers/pci/controller/dwc/pcie-qcom.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>> index af6bf5cce65b..40f0a5f247eb 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -2123,6 +2123,7 @@ static int qcom_pcie_resume_noirq(struct device *dev)
>>  static const struct of_device_id qcom_pcie_match[] = {
>>  	{ .compatible = "qcom,pcie-apq8064", .data = &cfg_2_1_0 },
>>  	{ .compatible = "qcom,pcie-apq8084", .data = &cfg_1_0_0 },
>> +	{ .compatible = "qcom,pcie-eliza", .data = &cfg_1_9_0 },
> So compatible with sm8550. Why isn't this explained in commit msg of
> the binding?
No, PCIe controller is not compatible with sm8550, we are just re using the boot
sequence used by the sm8550.
> Anyway, drop the change, pointless. Look how other devices handle this -
> do you see kaanapali here? No.
As we are going to use different dts schema for this controller we can't
really re-use
like how we have done in kaanapali case.  kaanpali is reusing sm8550 schema, where
this controller can't use sm8550 schema, as some clocks are different.

- Krishna Chaitanya.
>
> Best regards,
> Krzysztof
>


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH 2/4] dt-bindings: PCI: qcom: Document the Eliza PCIe Controller
From: Krishna Chaitanya Chundru @ 2026-05-22  9:17 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Bjorn Helgaas, Bjorn Andersson,
	linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-pci
In-Reply-To: <20260522-calm-bug-of-kindness-2dfae2@quoll>



On 5/22/2026 12:20 PM, Krzysztof Kozlowski wrote:
> On Thu, May 21, 2026 at 07:35:30PM +0530, Krishna Chaitanya Chundru wrote:
>> +description:
>> +  Qualcomm ELIZA SoC (and compatible) PCIe root complex controller is based on
>> +  the Synopsis DesignWare PCIe IP.
>> +
>> +properties:
>> +  compatible:
>> +    const: qcom,pcie-eliza
> All new pcie compatibles moved to correct format.
Ack, I will change it to qcom,elize-pcie
>> +
>> +  reg:
>> +    minItems: 5
>> +    maxItems: 6
>> +
>> +  reg-names:
>> +    minItems: 5
>> +    items:
>> +      - const: parf # Qualcomm specific registers
>> +      - const: dbi # DesignWare PCIe registers
>> +      - const: elbi # External local bus interface registers
>> +      - const: atu # ATU address space
>> +      - const: config # PCIe configuration space
>> +      - const: mhi # MHI registers
>> +
>> +  clocks:
>> +    maxItems: 7
>> +
>> +  clock-names:
>> +    items:
>> +      - const: aux # Auxiliary clock
>> +      - const: cfg # Configuration clock
>> +      - const: bus_master # Master AXI clock
>> +      - const: bus_slave # Slave AXI clock
>> +      - const: slave_q2a # Slave Q2A clock
>> +      - const: ddrss_sf_tbu # PCIe SF TBU clock
>> +      - const: cnoc_sf_axi # Config NoC PCIe1 AXI clock
>> +
>> +  interrupts:
>> +    minItems: 8
> This should not be flexible. Neither 'reg'.
ack I will remove minItems
>> +    maxItems: 9
>> +
>> +  interrupt-names:
>> +    minItems: 8
>> +    items:
>> +      - const: msi0
>> +      - const: msi1
>> +      - const: msi2
>> +      - const: msi3
>> +      - const: msi4
>> +      - const: msi5
>> +      - const: msi6
>> +      - const: msi7
>> +      - const: global
>> +
>> +  resets:
>> +    maxItems: 2
>> +
>> +  reset-names:
>> +    items:
>> +      - const: pci # PCIe core reset
>> +      - const: link_down # PCIe link down reset
>> +
>> +required:
>> +  - power-domains
>> +  - resets
>> +  - reset-names
>> +
>> +allOf:
>> +  - $ref: qcom,pcie-common.yaml#
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/qcom,eliza-gcc.h>
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/interconnect/qcom,eliza-rpmh.h>
>> +
>> +    soc {
>> +        #address-cells = <2>;
>> +        #size-cells = <2>;
>> +
>> +        pcie@1c00000 {
>> +            compatible = "qcom,pcie-eliza";
>> +            reg = <0 0x01c00000 0 0x3000>,
>> +                  <0 0x40000000 0 0xf1d>,
>> +                  <0 0x40000f20 0 0xa8>,
>> +                  <0 0x40001000 0 0x1000>,
>> +                  <0 0x40100000 0 0x100000>;
>> +            reg-names = "parf", "dbi", "elbi", "atu", "config";
>> +            ranges = <0x01000000 0x0 0x00000000 0x0 0x40200000 0x0 0x100000>,
>> +                     <0x02000000 0x0 0x40300000 0x0 0x40300000 0x0 0x3d00000>;
>> +
>> +            bus-range = <0x00 0xff>;
>> +            device_type = "pci";
>> +            linux,pci-domain = <0>;
>> +            num-lanes = <1>;
>> +
>> +            #address-cells = <3>;
>> +            #size-cells = <2>;
>> +
>> +            clocks = <&gcc GCC_PCIE_0_AUX_CLK>,
>> +                     <&gcc GCC_PCIE_0_CFG_AHB_CLK>,
>> +                     <&gcc GCC_PCIE_0_MSTR_AXI_CLK>,
>> +                     <&gcc GCC_PCIE_0_SLV_AXI_CLK>,
>> +                     <&gcc GCC_PCIE_0_SLV_Q2A_AXI_CLK>,
>> +                     <&gcc GCC_DDRSS_PCIE_SF_QTB_CLK>,
>> +                     <&gcc GCC_CNOC_PCIE_SF_AXI_CLK>;
>> +            clock-names = "aux",
>> +                          "cfg",
>> +                          "bus_master",
>> +                          "bus_slave",
>> +                          "slave_q2a",
>> +                          "ddrss_sf_tbu",
>> +                          "cnoc_sf_axi";
>> +
>> +            dma-coherent;
>> +
>> +            interrupts = <GIC_SPI 376 IRQ_TYPE_LEVEL_HIGH>,
>> +                         <GIC_SPI 478 IRQ_TYPE_LEVEL_HIGH>,
>> +                         <GIC_SPI 479 IRQ_TYPE_LEVEL_HIGH>,
>> +                         <GIC_SPI 535 IRQ_TYPE_LEVEL_HIGH>,
>> +                         <GIC_SPI 536 IRQ_TYPE_LEVEL_HIGH>,
>> +                         <GIC_SPI 537 IRQ_TYPE_LEVEL_HIGH>,
>> +                         <GIC_SPI 540 IRQ_TYPE_LEVEL_HIGH>,
>> +                         <GIC_SPI 653 IRQ_TYPE_LEVEL_HIGH>,
>> +                         <GIC_SPI 370 IRQ_TYPE_LEVEL_HIGH>;
>> +            interrupt-names = "msi0", "msi1", "msi2", "msi3",
>> +                              "msi4", "msi5", "msi6", "msi7", "global";
>> +            #interrupt-cells = <1>;
>> +            interrupt-map-mask = <0 0 0 0x7>;
>> +            interrupt-map = <0 0 0 1 &intc 0 0 0 564 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
>> +                            <0 0 0 2 &intc 0 0 0 565 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
>> +                            <0 0 0 3 &intc 0 0 0 566 IRQ_TYPE_LEVEL_HIGH>, /* int_c */
>> +                            <0 0 0 4 &intc 0 0 0 567 IRQ_TYPE_LEVEL_HIGH>; /* int_d */
>> +
>> +            interconnects = <&pcie_noc MASTER_PCIE_0 0 &mc_virt SLAVE_EBI1 0>,
>> +                            <&gem_noc MASTER_APPSS_PROC 0 &cnoc_main SLAVE_PCIE_0 0>;
>> +            interconnect-names = "pcie-mem", "cpu-pcie";
>> +
>> +            iommu-map = <0x0 &apps_smmu 0x1480 0x1>,
>> +                        <0x100 &apps_smmu 0x1481 0x1>;
>> +
>> +            pinctrl-0 = <&pcie0_default_state>;
>> +            pinctrl-names = "default";
>> +
>> +            power-domains = <&gcc GCC_PCIE_0_GDSC>;
>> +
>> +            resets = <&gcc GCC_PCIE_0_BCR>,
>> +                     <&gcc GCC_PCIE_0_LINK_DOWN_BCR>;
>> +            reset-names = "pci",
>> +                          "link_down";
>> +
> Drop stray blank line.
ack.

- Krishna Chaitanya.
>> +        };
>> +    };
>>
>> -- 
>> 2.34.1
>>


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH v5 1/2] dt-bindings: phy: add support for NXPs TJA1145 CAN transceiver
From: Dimitri Fedrau @ 2026-05-22  8:13 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: Dimitri Fedrau via B4 Relay, linux-phy, conor+dt, devicetree,
	vkoul, olteanv, robh, neil.armstrong, krzk+dt
In-Reply-To: <20260514051738.B2D32C2BCB7@smtp.kernel.org>

Am Thu, May 14, 2026 at 05:17:38AM +0000 schrieb sashiko-bot@kernel.org:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] The dt-binding omits support for the `max-bitrate` property.
> - [Medium] The dt-binding omits support for the `interrupts` property for wake-up routing.
> --
> 
> commit 853ad7c93ebe1c2a625ddddde401c4aaad496a8e
> Author: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> 
> dt-bindings: phy: add support for NXPs TJA1145 CAN transceiver
> 
> This commit adds documentation for the NXP TJA1145 CAN transceiver, residing
> in the generic PHY subsystem bindings. It describes the required SPI
> configurations and supply properties for the device.
> 
> > diff --git a/Documentation/devicetree/bindings/phy/nxp,tja1145.yaml b/Documentation/devicetree/bindings/phy/nxp,tja1145.yaml
> > new file mode 100644
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/nxp,tja1145.yaml
> 
> [ ... ]
> 
> > +  vbat-supply:
> > +    description:
> > +      Battery supply voltage
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - "#phy-cells"
> > +  - spi-cpha
> > +
> > +additionalProperties: false
> 
> Does this binding need to include the max-bitrate property?
>
No, the max bitrate is know for the device. See:
https://lore.kernel.org/linux-phy/0b4b8291-40cf-4faf-a79f-48c56677f9a2@kernel.org/

> This property is standard for CAN transceivers to describe board-level
> capability constraints where the layout cannot support the chip's theoretical
> maximum speed. Since additionalProperties is set to false, valid device trees
> specifying a maximum bitrate will fail validation.
> 
> Also, does this device tree binding need support for the interrupts property?
> 
Not at the moment, this can be added later on when there is an
implementation for this features.

> The TJA1145 supports partial networking and wake-up functionality, which can
> route a wake-up signal to an interrupt line on the host processor. With
> additionalProperties set to false, it is impossible to describe this hardware
> interrupt routing in the device tree.
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260513-tja1145-support-v5-0-38720a7ee63e@liebherr.com?part=1
> 
> -- 
> linux-phy mailing list
> linux-phy@lists.infradead.org
> https://lists.infradead.org/mailman/listinfo/linux-phy

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH 3/4] PCI: qcom: Add Support for Eliza
From: Krzysztof Kozlowski @ 2026-05-22  6:51 UTC (permalink / raw)
  To: Krishna Chaitanya Chundru
  Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Bjorn Helgaas, Bjorn Andersson,
	linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-pci
In-Reply-To: <20260521-eliza-v1-3-97cdbe88389d@oss.qualcomm.com>

On Thu, May 21, 2026 at 07:35:31PM +0530, Krishna Chaitanya Chundru wrote:
> Add support for Eliza soc, which has two PCIe controllers capable
> of 8GT/s X1 and 8GT/s X2, using the cfg_1_9_0 configuration.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index af6bf5cce65b..40f0a5f247eb 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -2123,6 +2123,7 @@ static int qcom_pcie_resume_noirq(struct device *dev)
>  static const struct of_device_id qcom_pcie_match[] = {
>  	{ .compatible = "qcom,pcie-apq8064", .data = &cfg_2_1_0 },
>  	{ .compatible = "qcom,pcie-apq8084", .data = &cfg_1_0_0 },
> +	{ .compatible = "qcom,pcie-eliza", .data = &cfg_1_9_0 },

So compatible with sm8550. Why isn't this explained in commit msg of
the binding?

Anyway, drop the change, pointless. Look how other devices handle this -
do you see kaanapali here? No.

Best regards,
Krzysztof


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH 2/4] dt-bindings: PCI: qcom: Document the Eliza PCIe Controller
From: Krzysztof Kozlowski @ 2026-05-22  6:50 UTC (permalink / raw)
  To: Krishna Chaitanya Chundru
  Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Bjorn Helgaas, Bjorn Andersson,
	linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-pci
In-Reply-To: <20260521-eliza-v1-2-97cdbe88389d@oss.qualcomm.com>

On Thu, May 21, 2026 at 07:35:30PM +0530, Krishna Chaitanya Chundru wrote:
> +description:
> +  Qualcomm ELIZA SoC (and compatible) PCIe root complex controller is based on
> +  the Synopsis DesignWare PCIe IP.
> +
> +properties:
> +  compatible:
> +    const: qcom,pcie-eliza

All new pcie compatibles moved to correct format.

> +
> +  reg:
> +    minItems: 5
> +    maxItems: 6
> +
> +  reg-names:
> +    minItems: 5
> +    items:
> +      - const: parf # Qualcomm specific registers
> +      - const: dbi # DesignWare PCIe registers
> +      - const: elbi # External local bus interface registers
> +      - const: atu # ATU address space
> +      - const: config # PCIe configuration space
> +      - const: mhi # MHI registers
> +
> +  clocks:
> +    maxItems: 7
> +
> +  clock-names:
> +    items:
> +      - const: aux # Auxiliary clock
> +      - const: cfg # Configuration clock
> +      - const: bus_master # Master AXI clock
> +      - const: bus_slave # Slave AXI clock
> +      - const: slave_q2a # Slave Q2A clock
> +      - const: ddrss_sf_tbu # PCIe SF TBU clock
> +      - const: cnoc_sf_axi # Config NoC PCIe1 AXI clock
> +
> +  interrupts:
> +    minItems: 8

This should not be flexible. Neither 'reg'.

> +    maxItems: 9
> +
> +  interrupt-names:
> +    minItems: 8
> +    items:
> +      - const: msi0
> +      - const: msi1
> +      - const: msi2
> +      - const: msi3
> +      - const: msi4
> +      - const: msi5
> +      - const: msi6
> +      - const: msi7
> +      - const: global
> +
> +  resets:
> +    maxItems: 2
> +
> +  reset-names:
> +    items:
> +      - const: pci # PCIe core reset
> +      - const: link_down # PCIe link down reset
> +
> +required:
> +  - power-domains
> +  - resets
> +  - reset-names
> +
> +allOf:
> +  - $ref: qcom,pcie-common.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/qcom,eliza-gcc.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interconnect/qcom,eliza-rpmh.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        pcie@1c00000 {
> +            compatible = "qcom,pcie-eliza";
> +            reg = <0 0x01c00000 0 0x3000>,
> +                  <0 0x40000000 0 0xf1d>,
> +                  <0 0x40000f20 0 0xa8>,
> +                  <0 0x40001000 0 0x1000>,
> +                  <0 0x40100000 0 0x100000>;
> +            reg-names = "parf", "dbi", "elbi", "atu", "config";
> +            ranges = <0x01000000 0x0 0x00000000 0x0 0x40200000 0x0 0x100000>,
> +                     <0x02000000 0x0 0x40300000 0x0 0x40300000 0x0 0x3d00000>;
> +
> +            bus-range = <0x00 0xff>;
> +            device_type = "pci";
> +            linux,pci-domain = <0>;
> +            num-lanes = <1>;
> +
> +            #address-cells = <3>;
> +            #size-cells = <2>;
> +
> +            clocks = <&gcc GCC_PCIE_0_AUX_CLK>,
> +                     <&gcc GCC_PCIE_0_CFG_AHB_CLK>,
> +                     <&gcc GCC_PCIE_0_MSTR_AXI_CLK>,
> +                     <&gcc GCC_PCIE_0_SLV_AXI_CLK>,
> +                     <&gcc GCC_PCIE_0_SLV_Q2A_AXI_CLK>,
> +                     <&gcc GCC_DDRSS_PCIE_SF_QTB_CLK>,
> +                     <&gcc GCC_CNOC_PCIE_SF_AXI_CLK>;
> +            clock-names = "aux",
> +                          "cfg",
> +                          "bus_master",
> +                          "bus_slave",
> +                          "slave_q2a",
> +                          "ddrss_sf_tbu",
> +                          "cnoc_sf_axi";
> +
> +            dma-coherent;
> +
> +            interrupts = <GIC_SPI 376 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 478 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 479 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 535 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 536 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 537 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 540 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 653 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 370 IRQ_TYPE_LEVEL_HIGH>;
> +            interrupt-names = "msi0", "msi1", "msi2", "msi3",
> +                              "msi4", "msi5", "msi6", "msi7", "global";
> +            #interrupt-cells = <1>;
> +            interrupt-map-mask = <0 0 0 0x7>;
> +            interrupt-map = <0 0 0 1 &intc 0 0 0 564 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
> +                            <0 0 0 2 &intc 0 0 0 565 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
> +                            <0 0 0 3 &intc 0 0 0 566 IRQ_TYPE_LEVEL_HIGH>, /* int_c */
> +                            <0 0 0 4 &intc 0 0 0 567 IRQ_TYPE_LEVEL_HIGH>; /* int_d */
> +
> +            interconnects = <&pcie_noc MASTER_PCIE_0 0 &mc_virt SLAVE_EBI1 0>,
> +                            <&gem_noc MASTER_APPSS_PROC 0 &cnoc_main SLAVE_PCIE_0 0>;
> +            interconnect-names = "pcie-mem", "cpu-pcie";
> +
> +            iommu-map = <0x0 &apps_smmu 0x1480 0x1>,
> +                        <0x100 &apps_smmu 0x1481 0x1>;
> +
> +            pinctrl-0 = <&pcie0_default_state>;
> +            pinctrl-names = "default";
> +
> +            power-domains = <&gcc GCC_PCIE_0_GDSC>;
> +
> +            resets = <&gcc GCC_PCIE_0_BCR>,
> +                     <&gcc GCC_PCIE_0_LINK_DOWN_BCR>;
> +            reset-names = "pci",
> +                          "link_down";
> +

Drop stray blank line.

> +        };
> +    };
> 
> -- 
> 2.34.1
> 

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH 1/4] dt-bindings: phy: sc8280xp-qmp-pcie: Document Eliza PCIe phy
From: Krzysztof Kozlowski @ 2026-05-22  6:46 UTC (permalink / raw)
  To: Krishna Chaitanya Chundru
  Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Bjorn Helgaas, Bjorn Andersson,
	linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-pci
In-Reply-To: <20260521-eliza-v1-1-97cdbe88389d@oss.qualcomm.com>

On Thu, May 21, 2026 at 07:35:29PM +0530, Krishna Chaitanya Chundru wrote:
> Add compatibles for the Eliza PCIe QMP PHY's, which supports Gen3x1 and
> Gen3x2 configurations.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
>  .../devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml         | 6 ++++++
>  1 file changed, 6 insertions(+)

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH v9 2/5] dt-bindings: phy: Add documentation for Airoha AN7581 USB PHY
From: Krzysztof Kozlowski @ 2026-05-22  6:06 UTC (permalink / raw)
  To: Christian Marangi
  Cc: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Neil Armstrong,
	Lorenzo Bianconi, Felix Fietkau, linux-clk, devicetree,
	linux-kernel, linux-arm-kernel, linux-phy
In-Reply-To: <20260521153645.7028-3-ansuelsmth@gmail.com>

On Thu, May 21, 2026 at 05:35:53PM +0200, Christian Marangi wrote:
> Add documentation for Airoha AN7581 USB PHY that describe the USB PHY
> for the USB controller.
> 
> Airoha AN7581 SoC support a maximum of 2 USB port. The USB 2.0 mode is
> always supported. The USB 3.0 mode is optional and depends on the Serdes
> mode currently configured on the system for the relevant USB port.
> 
> To correctly calibrate, the USB 2.0 port require correct value in
> "airoha,usb2-monitor-clk-sel" property. Both the 2 USB 2.0 port permit
> selecting one of the 4 monitor clock for calibration (internal clock not
> exposed to the system) but each port have only one of the 4 actually
> connected in HW hence the correct value needs to be specified in DT
> based on board and the physical port. Normally it's monitor clock 1 for
> USB1 and monitor clock 2 for USB2.
> 
> To correctly setup the Serdes mode attached to the USB 3.0 mode, a phys
> property is required with the phandle pointing to the correct Serdes port
> provided by the SCU node. Providing the phys property is optional if USB
> 3.0 is not used.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  .../bindings/phy/airoha,an7581-usb-phy.yaml   | 62 +++++++++++++++++++
>  MAINTAINERS                                   |  6 ++
>  2 files changed, 68 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/airoha,an7581-usb-phy.yaml

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH 5/5] drm/rockchip: cdn-dp: Add multiple bridges to support PHY port selection
From: Chaoyi Chen @ 2026-05-22  2:37 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: vkoul, linux-phy, neil.armstrong, olteanv, Heiko Stuebner
In-Reply-To: <20260521062223.913FB1F000E9@smtp.kernel.org>

Oh! Interesting AI. It really did uncover some issues.

On 5/21/2026 2:22 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 11 potential issue(s) to consider:
> - [High] Prematurely dropping the reference count of the last bridge in `cdn_bridge_add()` will cause a use-after-free.
> - [High] The `prev_port` pointer is used uninitialized in `cdn_dp_bridge_edid_read()`, leading to a kernel crash.
> - [High] `cdn_dp_connected_port()` ignores connection status for extcon configurations, breaking hotplug detection.
> - [High] Missing bounds check in `cdn_dp_parse_next_bridge_dt()` allows an out-of-bounds write to the `next_bridge_list` array.
> - [High] The `lanes` stack array is read out-of-bounds or uninitialized in `cdn_dp_connected_port()`.
> - [High] DRM bridge references are leaked on error paths during DT parsing and driver binding.
> - [High] If link training fails in `cdn_dp_switch_port()`, it returns an error but leaves the DP controller erroneously marked as active.
> - [High] `cdn_dp_switch_port()` incorrectly aborts hardware switching when the DP controller is active, causing EDID reads from the wrong port.
> - [High] Disabling an inactive bridge erroneously disables the active DP controller for another bridge.
> - [Medium] Overwriting `connector->fwnode` in `cdn_bridge_add()` causes a memory leak.
> - [Medium] Incorrect check for the active port treats valid index 0 as invalid.
> --
> 
> Hi,
> 
> I have reviewed the patch and found some potential issues to share.
> 
> commit 082c705aa7547b521ad9df20ac76e0592b41864f
> Author: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> 
>     drm/rockchip: cdn-dp: Add multiple bridges to support PHY port selection
> 
> This commit adds support for multiple bridges to the Rockchip cdn-dp driver
> to allow switching between two USB/DP combo PHYs. It changes the driver to
> use the DRM AUX HPD bridge and adds DT parsing logic to register multiple
> bridges.
> 
>> @@ -192,14 +193,27 @@ static int cdn_dp_get_sink_count(struct cdn_dp_device *dp, u8 *sink_count)
>>  static struct cdn_dp_port *cdn_dp_connected_port(struct cdn_dp_device *dp)
>>  {
>>  	struct cdn_dp_port *port;
>> -	int i, lanes;
>> +	int i, lanes[MAX_PHY];
>>  
>>  	for (i = 0; i < dp->ports; i++) {
>>  		port = dp->port[i];
>> -		lanes = cdn_dp_get_port_lanes(port);
>> -		if (lanes)
>> +		lanes[i] = cdn_dp_get_port_lanes(port);
>> +		if (!dp->next_bridge_valid)
>>  			return port;
> 
> [Severity: High]
> Does this unconditionally return the first port without checking if it is
> actually connected? If dp->next_bridge_valid is false (which is the case for
> extcon setups), it appears to return port on the first iteration regardless
> of lanes[i], completely breaking hotplug detection.
> 
>>  	}

No.

>> +
>> +	if (dp->next_bridge_valid) {
>> +		/* If more than one port is available, pick the last active port */
>> +		if (dp->active_port > 0 && lanes[dp->active_port])
> 
> [Severity: Medium]
> Since dp->active_port is initialized to -1, valid port indices are 0 and 1.
> Does the check dp->active_port > 0 incorrectly evaluate to false when the
> active port is 0, skipping the active port?
> 

That's right. Will fix in v2.

>> +			return dp->port[dp->active_port];
>> +
>> +		/* If the last active port is not available, pick an available port in order */
>> +		for (i = 0; i < dp->bridge_count; i++) {
>> +			if (lanes[i])
> 
> [Severity: High]
> Can this read uninitialized stack memory? The lanes stack array is populated
> in a loop bounded by dp->ports, but accessed here in a loop bounded by
> dp->bridge_count.
> 
> If dp->bridge_count > dp->ports, this will read uninitialized values.
> Furthermore, since lanes is sized to MAX_PHY, if dp->bridge_count > MAX_PHY,
> it will perform an out-of-bounds stack read.
> 

No.

>> +				return dp->port[i];
>> +		}
>> +	}
>> +
>>  	return NULL;
>>  }
> 
> [ ... ]
> 
>> @@ -254,12 +268,45 @@ static const struct drm_edid *
>>  cdn_dp_bridge_edid_read(struct drm_bridge *bridge, struct drm_connector *connector)
>>  {
>>  	struct cdn_dp_device *dp = bridge_to_dp(bridge);
>> -	const struct drm_edid *drm_edid;
>> +	struct cdn_dp_bridge *dp_bridge = bridge_to_dp_bridge(bridge);
>> +	struct cdn_dp_port *port = dp->port[dp_bridge->id];
>> +	struct cdn_dp_port *prev_port;
>> +	const struct drm_edid *drm_edid = NULL;
>>  	int i, ret;
>>  
>>  	mutex_lock(&dp->lock);
>> +
>> +	/* More than one port is available */
>> +	if (dp->bridge_count > 1 && !port->phy_enabled) {
>> +		for (i = 0; i < dp->bridge_count; i++) {
>> +			/* Another port already enable */
>> +			if (dp->bridge_list[i] != dp_bridge && dp->bridge_list[i]->enabled)
>> +				goto get_cache;
>> +			/* Find already enabled port */
>> +			if (dp->port[i]->phy_enabled)
>> +				prev_port = dp->port[i];
>> +		}
>> +
>> +		/* Switch to current port */
>> +		if (prev_port) {
> 
> [Severity: High]
> Can this lead to a kernel crash by dereferencing an uninitialized pointer?
> The prev_port variable is not initialized. If no ports are currently enabled
> (which is common at boot), the for loop will complete without assigning
> anything to prev_port.
> 
> Then if (prev_port) evaluates garbage memory, and calling cdn_dp_switch_port
> with it will cause a kernel panic.
>

No.

>> +			ret = cdn_dp_switch_port(dp, prev_port, port);
>> +			if (ret)
>> +				goto get_cache;
>> +		}
>> +	}
> 
> [ ... ]
> 
>> @@ -551,6 +599,54 @@ static bool cdn_dp_check_link_status(struct cdn_dp_device *dp)
>>  	return drm_dp_channel_eq_ok(link_status, min(port->lanes, sink_lanes));
>>  }
>>  
>> +static int cdn_dp_switch_port(struct cdn_dp_device *dp, struct cdn_dp_port *prev_port,
>> +			      struct cdn_dp_port *port)
>> +{
>> +	int ret;
>> +
>> +	if (dp->active)
>> +		return 0;
> 
> [Severity: High]
> Does this incorrectly abort hardware switching? If the DP controller is
> already active but currently on a different port, returning 0 skips the
> actual port switch. The caller cdn_dp_bridge_edid_read() will proceed with
> reading EDID from the wrong (already-active) port and caching it for the
> new port.
>

Actually, this is a redundant check. When dp->active is true, 
this function is never called at all.

>> +
>> +	ret = cdn_dp_disable_phy(dp, prev_port);
>> +	if (ret)
>> +		goto out;
>> +	ret = cdn_dp_enable_phy(dp, port);
>> +	if (ret)
>> +		goto out;
>> +
>> +	ret = cdn_dp_get_sink_capability(dp);
>> +	if (ret) {
>> +		cdn_dp_disable_phy(dp, port);
>> +		goto out;
>> +	}
>> +
>> +	dp->active = true;
>> +	dp->lanes = port->lanes;
>> +
>> +	if (!cdn_dp_check_link_status(dp)) {
>> +		dev_info(dp->dev, "Connected with sink; re-train link\n");
>> +
>> +		ret = cdn_dp_train_link(dp);
>> +		if (ret) {
>> +			dev_err(dp->dev, "Training link failed: %d\n", ret);
>> +			goto out;
> 
> [Severity: High]
> If link training fails, this jumps to out and returns the error code, but
> leaves dp->active = true. Does this trap the port in a permanently broken,
> untrained state since subsequent calls to cdn_dp_switch_port() will
> immediately return 0?
>

The redundant assignment, as mentioned above, should be removed.

>> +		}
> 
> [ ... ]
> 
>> @@ -639,9 +740,11 @@ static void cdn_dp_bridge_atomic_enable(struct drm_bridge *bridge, struct drm_at
>>  static void cdn_dp_bridge_atomic_disable(struct drm_bridge *bridge, struct drm_atomic_state *state)
>>  {
>>  	struct cdn_dp_device *dp = bridge_to_dp(bridge);
>> +	struct cdn_dp_bridge *dp_bridge = bridge_to_dp_bridge(bridge);
>>  	int ret;
>>  
>>  	mutex_lock(&dp->lock);
>> +	dp_bridge->enabled = false;
>>  
>>  	if (dp->active) {
>>  		ret = cdn_dp_disable(dp);
> 
> [Severity: High]
> Does this accidentally disable the active DP controller when another inactive
> bridge is disabled? If dp->active is true, the entire DP controller is
> disabled, without checking if the bridge being disabled (dp_bridge) is
> actually the one driving the active DP port. This would break an active
> Bridge A if userspace disables an inactive Bridge B.
> 
> [ ... ]
>

No.

>> @@ -967,28 +1085,16 @@ static int cdn_dp_pd_event(struct notifier_block *nb,
>>  	return NOTIFY_DONE;
>>  }
>>  
>> -static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
>> +static int cdn_bridge_add(struct device *dev,
>> +			  struct drm_bridge *bridge,
>> +			  struct drm_bridge *next_bridge,
>> +			  struct drm_encoder *encoder)
>>  {
>>  	struct cdn_dp_device *dp = dev_get_drvdata(dev);
>> -	struct drm_encoder *encoder;
>> +	struct drm_device *drm_dev = dp->drm_dev;
>> +	struct drm_bridge *last_bridge __free(drm_bridge_put) = NULL;
> 
> [Severity: High]
> Will this cause a reference count underflow and a use-after-free
> vulnerability? The last_bridge pointer is populated by
> drm_bridge_chain_get_last_bridge(), which returns a raw pointer without
> incrementing the bridge's reference count. When last_bridge goes out of
> scope, the automatic drm_bridge_put() will prematurely decrement the
> refcount.
> 

No.

>>  	struct drm_connector *connector;
>> -	struct cdn_dp_port *port;
>> -	struct drm_device *drm_dev = data;
>> -	int ret, i;
>> -
> 
> [ ... ]
> 
>> +	if (next_bridge) {
>> +		ret = drm_bridge_attach(encoder, next_bridge, bridge,
>> +					DRM_BRIDGE_ATTACH_NO_CONNECTOR);
>> +		if (ret)
>> +			return ret;
>> +
>> +		last_bridge = drm_bridge_chain_get_last_bridge(bridge->encoder);
>> +	}
>> +
>>  	connector = drm_bridge_connector_init(drm_dev, encoder);
>>  	if (IS_ERR(connector)) {
>>  		ret = PTR_ERR(connector);
>> @@ -1030,8 +1145,99 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
>>  		return ret;
>>  	}
>>  
>> +	if (last_bridge)
>> +		connector->fwnode = fwnode_handle_get(of_fwnode_handle(last_bridge->of_node));
> 
> [Severity: Medium]
> Is this assignment leaking an fwnode reference? drm_bridge_connector_init()
> automatically sets connector->fwnode using the of_node of the last bridge
> and acquires a reference via fwnode_handle_get(). Overwriting it here without
> dropping the original reference will cause a memory leak.
>

Yes, the old fwnode should be put.

>> +
>>  	drm_connector_attach_encoder(connector, encoder);
>>  
>> +	return 0;
>> +}
>> +
>> +static int cdn_dp_parse_next_bridge_dt(struct cdn_dp_device *dp)
>> +{
>> +	struct device_node *np = dp->dev->of_node;
>> +	struct device_node *port __free(device_node) = of_graph_get_port_by_id(np, 1);
>> +	struct drm_bridge *bridge;
>> +	int count = 0;
>> +	int ret = 0;
>> +	int i;
>> +
>> +	/* If device use extcon, do not use hpd bridge */
>> +	for (i = 0; i < dp->ports; i++) {
>> +		if (dp->port[i]->extcon) {
>> +			dp->bridge_count = 1;
>> +			return 0;
>> +		}
>> +	}
>> +
>> +	/* One endpoint may correspond to one next bridge. */
>> +	for_each_of_graph_port_endpoint(port, dp_ep) {
>> +		struct device_node *next_bridge_node __free(device_node) =
>> +			of_graph_get_remote_port_parent(dp_ep);
>> +
>> +		bridge = of_drm_find_bridge(next_bridge_node);
>> +		if (!bridge) {
>> +			ret = -EPROBE_DEFER;
>> +			goto out;
> 
> [Severity: High]
> Does this leak the DRM bridge references acquired in earlier loop iterations?
> If of_drm_find_bridge() returns NULL on a subsequent iteration, it jumps to
> out and returns -EPROBE_DEFER without releasing the references previously
> obtained with drm_bridge_get().
>

Yes, it should be put.

>> +		}
>> +
>> +		dp->next_bridge_valid = true;
>> +		dp->next_bridge_list[count] = drm_bridge_get(bridge);
>> +		count++;
> 
> [Severity: High]
> Could this overflow the next_bridge_list array? next_bridge_list is sized to
> MAX_PHY (2). If a device tree specifies more than two endpoints, count will
> exceed the array bounds and corrupt adjacent fields in the cdn_dp_device
> struct.
>

No.

>> +	}
>> +
>> +out:
>> +	dp->bridge_count = count ? count : 1;
>> +	return ret;
>> +}
>> +
>> +static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
>> +{
> 
> [ ... ]
> 
>> +	for (i = 0; i < dp->bridge_count; i++) {
>> +		encoder = &dp->bridge_list[i]->encoder.encoder;
>> +		bridge = &dp->bridge_list[i]->bridge;
>> +		next_bridge = dp->next_bridge_list[i];
>> +		ret = cdn_bridge_add(dev, bridge, next_bridge, encoder);
>> +		if (ret)
>> +			return ret;
> 
> [Severity: High]
> Does this leak the bridge references acquired in cdn_dp_parse_next_bridge_dt
> if cdn_bridge_add() fails? Returning directly without a cleanup path means
> drm_bridge_put() is never called for dp->next_bridge_list.
> 
>> +	}
> 

Yes, it should be put.

-- 
Best, 
Chaoyi

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* Re: [PATCH v6 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
From: Bryan O'Donoghue @ 2026-05-21 15:58 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: devicetree, vkoul, robh, olteanv, linux-phy, neil.armstrong,
	conor+dt
In-Reply-To: <20260521124947.4BE4B1F000E9@smtp.kernel.org>

On 21/05/2026 13:49, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 10 potential issue(s) to consider:
> - [High] Extensive resource leaks on error paths in power_on and set_clock_rates, and missing OPP rate reset in power_off.
> - [High] phy_qcom_mipi_csi2_configure overrides the DT-parsed lane count without bounds checking against the physical layout.
> - [High] Unconditional dereference of pd_list leads to a NULL pointer dereference panic.
> - [High] Division by zero occurs if the timer clock rate resolves to 0.
> - [High] Settle count calculation can underflow to a very large positive value.
> - [Medium] The ops->lanes_disable callback is omitted in phy_qcom_mipi_csi2_power_off.
> - [Medium] Clock lane positioning and lane polarities are parsed but entirely ignored.
> - [Medium] The probe function allows the absence of an OPP table, but power_on strictly requires it.
> - [Medium] qcom_csi2_phy_xlate accesses PHY arguments without checking args_count.
> - [Medium] Unvalidated data-lanes integer from DT can cause an out-of-bounds bitwise shift.
> --
> 
> commit 3c373b3ab7783a677ec4c98f344d8b1c57e2e110
> Author: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> 
> phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
> 
> Add a new MIPI CSI2 driver in DPHY mode initially. The entire set of
> existing CAMSS CSI PHY init sequences are imported in order to save time
> and effort in later patches.
> 
>> diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
>> index 0000000000000..8cdff35be2da9
>> --- /dev/null
>> +++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
> [ ... ]
>> +static u8 phy_qcom_mipi_csi2_settle_cnt_calc(s64 link_freq, u32 timer_clk_rate)
>> +{
> [ ... ]
>> +	timer_period_ps = div_u64(PSEC_PER_SEC, timer_clk_rate);
>> +	settle_cnt = t_hs_settle_ps / timer_period_ps - 6;
> 
> [Severity: High]
> Can this subtraction underflow to a very large positive value?
> 
> If the system has a low timer clock rate resulting in a high
> timer_period_ps, the division could yield a value less than 6. Since
> settle_cnt is an unsigned 8-bit integer, wrapping around to a large
> value could severely misconfigure the PHY hardware timings.

I'm not sure it is really worthwhile writing defensive code for this case.

We audit every DT binding for clock frequency sanity so you have to 
input junk into a device tree and upstream it and we aren't generally in 
the habit of verifying those inputs.

> 
>> +
>> +	return settle_cnt;
>> +}
> [ ... ]
>> +static int phy_qcom_mipi_csi2_lanes_enable(struct mipi_csi2phy_device *csi2phy,
>> +					   struct mipi_csi2phy_stream_cfg *cfg)
>> +{
> [ ... ]
>> +	settle_cnt = phy_qcom_mipi_csi2_settle_cnt_calc(cfg->link_freq, csi2phy->timer_clk_rate);
>> +
>> +	val = CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE;
> 
> [Severity: Medium]
> The commit message states that data-lane and clock lane positioning and
> polarity selection are supported. Are the polarity fields ignored here?

Exactly the same situation as we already have for this PHY in CAMSS 
since forever.

We parse but don't program the various fields.

> It appears the clock lane enable bit is hardcoded and the polarity fields
> parsed from the DT are not programmed into the PHY registers.

>> +	for (i = 0; i < cfg->num_data_lanes; i++)
>> +		val |= BIT(lane_cfg->data[i].pos * 2);

Yeah this is longstanding behaviour but, its probably worthwhile to 
update it since we consume the data from DT anyway.
> [Severity: Medium]
> Can this result in an out-of-bounds shift?
> 
> The pos value is read from the device tree as an unvalidated 32-bit
> integer. A malformed DT with large lane positions (e.g. > 15) could
> result in a shift value >= 32, which is undefined behavior and might
> truncate the hardware configuration mask.

Still no - not going to spend time picking through what goes into DT in 
the driver.

For example we don't sanity check register values or IRQ vector numbers.
> 
>> +
>> +	writel(val, csi2phy->base +
>> +	       CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 5));
> [ ... ]
>> diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
>> index 0000000000000..d8bdb2dd3c2ac
>> --- /dev/null
>> +++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
> [ ... ]
>> +static int
>> +phy_qcom_mipi_csi2_set_clock_rates(struct mipi_csi2phy_device *csi2phy,
>> +				   s64 link_freq)
>> +{
> [ ... ]
>> +	opp = dev_pm_opp_find_freq_ceil(dev, &opp_rate);
>> +	if (IS_ERR(opp)) {
> 
> [Severity: Medium]
> Since the probe function tolerates -ENODEV for the OPP table, won't
> power_on unconditionally fail here if the table is actually missing?
> 
>> +		dev_err(csi2phy->dev, "Couldn't find ceiling for %lld Hz\n",
>> +			link_freq);
>> +		return PTR_ERR(opp);
>> +	}
>> +
>> +	for (int i = 0; i < csi2phy->pd_list->num_pds; i++) {
> 
> [Severity: High]
> What happens to pd_list if CONFIG_PM is disabled?
> 
> devm_pm_domain_attach_list() returns 0 when CONFIG_PM is disabled, but
> pd_list will remain NULL. This would cause a NULL pointer dereference
> here and in phy_qcom_mipi_csi2_power_off().

No idea, lets find out.

>> +		unsigned int perf = dev_pm_opp_get_required_pstate(opp, i);
>> +
>> +		ret = dev_pm_genpd_set_performance_state(csi2phy->pd_list->pd_devs[i], perf);
>> +		if (ret) {
>> +			dev_err(csi2phy->dev, "Couldn't set perf state %u\n",
>> +				perf);
>> +			dev_pm_opp_put(opp);
>> +			return ret;
> 
> [Severity: High]
> Does this leak the performance state for previously set domains?
> 
> If the loop fails on a later iteration, it appears to return without resetting
> the domains that were already successfully configured.

True we need to unwind.

>> +		}
>> +	}
>> +	dev_pm_opp_put(opp);
>> +
>> +	ret = dev_pm_opp_set_rate(dev, opp_rate);
>> +	if (ret) {
>> +		dev_err(csi2phy->dev, "dev_pm_opp_set_rate() fail\n");
>> +		return ret;
>> +	}
>> +
>> +	timer_rate = clk_round_rate(csi2phy->timer_clk, link_freq / 4);
>> +	if (timer_rate < 0)
>> +		return timer_rate;
>> +
>> +	ret = clk_set_rate(csi2phy->timer_clk, timer_rate);
>> +	if (ret)
>> +		return ret;
>> +
>> +	csi2phy->timer_clk_rate = timer_rate;
> 
> [Severity: High]
> What if clk_round_rate() returns 0?
> 
> If the clock is disabled or misconfigured, timer_rate could be 0. Later,
> phy_qcom_mipi_csi2_settle_cnt_calc() calls
> div_u64(PSEC_PER_SEC, timer_clk_rate), which would result in a division
> by zero and a kernel panic.

An esoteric ask since you again have to have bugs in your DT but, for 
the sake of not being spammed by a static analysis tool, sure lets check 
for zero.

>> +
>> +	return 0;
>> +}
>> +
>> +static int phy_qcom_mipi_csi2_configure(struct phy *phy,
>> +					union phy_configure_opts *opts)
>> +{
> [ ... ]
>> +	stream_cfg->link_freq = dphy_cfg->hs_clk_rate;
>> +	stream_cfg->num_data_lanes = dphy_cfg->lanes;
> 
> [Severity: High]
> Does this blindly overwrite the DT-parsed lane count without bounds
> checking against the physical layout?

It takes the lanes_count from the controller DT field.

> If a consumer requests more lanes than are physically mapped in the
> device tree, the hardware configuration could iterate out-of-bounds of
> the initialized array elements, repeatedly applying bitmasks for physical
> position 0.

No the value comes from the controller DT data-lanes field but since we 
are consuming data-lanes from the PHY its actually a worthwhile test to 
validate what the controller says to what we have an -EINVAL if they 
don't agree.

> 
>> +
>> +	return 0;
>> +}
>> +
>> +static int phy_qcom_mipi_csi2_power_on(struct phy *phy)
>> +{
> [ ... ]
>> +	ret = clk_bulk_prepare_enable(csi2phy->soc_cfg->num_clk,
>> +				      csi2phy->clks);
>> +	if (ret) {
>> +		dev_err(dev, "failed to enable clocks, %d\n", ret);
>> +		goto poweroff_phy;
>> +	}
>> +
> [ ... ]
>> +poweroff_phy:
>> +	regulator_bulk_disable(csi2phy->soc_cfg->num_supplies,
>> +			       csi2phy->supplies);
>> +
>> +	return ret;
> 
> [Severity: High]
> Does this error path leak the performance state and OPP rate?
> 
> It seems to only disable regulators, completely missing the cleanup for
> dev_pm_genpd_set_performance_state() and dev_pm_opp_set_rate().
> 

When the clocks disable the perf state _should_ drop back but, I will check.

>> +}
>> +
>> +static int phy_qcom_mipi_csi2_power_off(struct phy *phy)
>> +{
>> +	struct mipi_csi2phy_device *csi2phy = phy_get_drvdata(phy);
>> +	int i;
>> +
>> +	for (i = 0; i < csi2phy->pd_list->num_pds; i++)
>> +		dev_pm_genpd_set_performance_state(csi2phy->pd_list->pd_devs[i], 0);
> 
> [Severity: High]
> Does this leave the OPP frequency vote elevated?
> 
> It doesn't look like dev_pm_opp_set_rate(dev, 0) is called to release
> the OPP frequency vote.
> 
> [Severity: Medium]
> Is the ops->lanes_disable() callback intentionally omitted here?
> 
> Disabling the clocks and regulators without invoking the callback
> bypasses the hardware-specific teardown sequence (like clearing
> CLK_ENABLE and COMMON_PWRDN_B). Could this leave the PHY in an invalid
> logical state for the next power-on cycle?

I mean, when power goes off you loose the state in the regs ... but for 
the sake of completeness sure, I will add this.

>> +
>> +	clk_bulk_disable_unprepare(csi2phy->soc_cfg->num_clk,
>> +				   csi2phy->clks);
>> +	regulator_bulk_disable(csi2phy->soc_cfg->num_supplies,
>> +			       csi2phy->supplies);
>> +
>> +	return 0;
>> +}
> [ ... ]
>> +static struct phy *qcom_csi2_phy_xlate(struct device *dev,
>> +				       const struct of_phandle_args *args)
>> +{
>> +	struct mipi_csi2phy_device *csi2phy = dev_get_drvdata(dev);
>> +
>> +	if (args->args[0] != PHY_TYPE_DPHY) {
> 
> [Severity: Medium]
> Can this access uninitialized stack memory?
> 
> If the device tree node has #phy-cells = <0>, args_count will be 0, and
> reading args->args[0] might yield uninitialized memory, leading to
> unpredictable behavior or the incorrect rejection of valid PHY bindings.
> Should args->args_count be checked first?

I feel the answer is "no" but I'm not averse to checking.

> 
>> +		dev_err(csi2phy->dev, "mode %d -EOPNOTSUPP\n", args->args[0]);
>> +		return ERR_PTR(-EOPNOTSUPP);
>> +	}
> 


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply

* [PATCH v9 5/5] phy: airoha: Add support for Airoha AN7581 USB PHY
From: Christian Marangi @ 2026-05-21 15:35 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Christian Marangi, Vinod Koul,
	Neil Armstrong, Lorenzo Bianconi, Felix Fietkau, linux-clk,
	devicetree, linux-kernel, linux-arm-kernel, linux-phy
In-Reply-To: <20260521153645.7028-1-ansuelsmth@gmail.com>

Add support for Airoha AN7581 USB PHY driver. AN7581 supports up to 2
USB port with USB 2.0 mode always supported and USB 3.0 mode available
only if the Serdes port is correctly configured for USB 3.0.

If the USB 3.0 mode is not configured, the modes needs to be also
disabled in the xHCI node or the driver will report unsable clock and
fail probe.

For USB 2.0 Slew Rate calibration, airoha,usb2-monitor-clk-sel is
mandatory and is used to select the monitor clock for calibration.

Normally it's 1 for USB port 1 and 2 for USB port 2.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 MAINTAINERS                         |   1 +
 drivers/phy/airoha/Kconfig          |  11 +
 drivers/phy/airoha/Makefile         |   1 +
 drivers/phy/airoha/phy-an7581-usb.c | 559 ++++++++++++++++++++++++++++
 4 files changed, 572 insertions(+)
 create mode 100644 drivers/phy/airoha/phy-an7581-usb.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 7bea8c620da8..2f05faa44503 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -776,6 +776,7 @@ M:	Christian Marangi <ansuelsmth@gmail.com>
 L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
 S:	Maintained
 F:	Documentation/devicetree/bindings/phy/airoha,an7581-usb-phy.yaml
+F:	drivers/phy/airoha/phy-an7581-usb.c
 
 AIRSPY MEDIA DRIVER
 L:	linux-media@vger.kernel.org
diff --git a/drivers/phy/airoha/Kconfig b/drivers/phy/airoha/Kconfig
index 9a1b625a7701..634448ee39b5 100644
--- a/drivers/phy/airoha/Kconfig
+++ b/drivers/phy/airoha/Kconfig
@@ -11,3 +11,14 @@ config PHY_AIROHA_AN7581_PCIE
 	  Say Y here to add support for Airoha AN7581 PCIe PHY driver.
 	  This driver create the basic PHY instance and provides initialize
 	  callback for PCIe GEN3 port.
+
+config PHY_AIROHA_AN7581_USB
+	tristate "Airoha AN7581 USB PHY Driver"
+	depends on ARCH_AIROHA || COMPILE_TEST
+	depends on OF
+	select GENERIC_PHY
+	select REGMAP_MMIO
+	help
+	  Say 'Y' here to add support for Airoha AN7581 USB PHY driver.
+	  This driver create the basic PHY instance and provides initialize
+	  callback for USB port.
diff --git a/drivers/phy/airoha/Makefile b/drivers/phy/airoha/Makefile
index 912f3e11a061..944bf842deba 100644
--- a/drivers/phy/airoha/Makefile
+++ b/drivers/phy/airoha/Makefile
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
 
 obj-$(CONFIG_PHY_AIROHA_AN7581_PCIE)	+= phy-an7581-pcie.o
+obj-$(CONFIG_PHY_AIROHA_AN7581_USB)	+= phy-an7581-usb.o
diff --git a/drivers/phy/airoha/phy-an7581-usb.c b/drivers/phy/airoha/phy-an7581-usb.c
new file mode 100644
index 000000000000..92c5e5c2fbf3
--- /dev/null
+++ b/drivers/phy/airoha/phy-an7581-usb.c
@@ -0,0 +1,559 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Author: Christian Marangi <ansuelsmth@gmail.com>
+ */
+
+#include <dt-bindings/phy/phy.h>
+#include <dt-bindings/soc/airoha,scu-ssr.h>
+#include <linux/bitfield.h>
+#include <linux/math.h>
+#include <linux/module.h>
+#include <linux/mfd/syscon.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+/* U2PHY */
+#define AIROHA_USB_PHY_FMCR0			0x100
+#define   AIROHA_USB_PHY_MONCLK_SEL		GENMASK(27, 26)
+#define   AIROHA_USB_PHY_MONCLK_SEL0		FIELD_PREP_CONST(AIROHA_USB_PHY_MONCLK_SEL, 0x0)
+#define   AIROHA_USB_PHY_MONCLK_SEL1		FIELD_PREP_CONST(AIROHA_USB_PHY_MONCLK_SEL, 0x1)
+#define   AIROHA_USB_PHY_MONCLK_SEL2		FIELD_PREP_CONST(AIROHA_USB_PHY_MONCLK_SEL, 0x2)
+#define   AIROHA_USB_PHY_MONCLK_SEL3		FIELD_PREP_CONST(AIROHA_USB_PHY_MONCLK_SEL, 0x3)
+#define   AIROHA_USB_PHY_FREQDET_EN		BIT(24)
+#define   AIROHA_USB_PHY_CYCLECNT		GENMASK(23, 0)
+#define AIROHA_USB_PHY_FMMONR0			0x10c
+#define   AIROHA_USB_PHY_USB_FM_OUT		GENMASK(31, 0)
+#define AIROHA_USB_PHY_FMMONR1			0x110
+#define   AIROHA_USB_PHY_FRCK_EN		BIT(8)
+
+#define AIROHA_USB_PHY_USBPHYACR4		0x310
+#define   AIROHA_USB_PHY_USB20_FS_CR		GENMASK(10, 8)
+#define   AIROHA_USB_PHY_USB20_FS_CR_MAX	FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_FS_CR, 0x0)
+#define   AIROHA_USB_PHY_USB20_FS_CR_NORMAL	FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_FS_CR, 0x2)
+#define   AIROHA_USB_PHY_USB20_FS_CR_SMALLER	FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_FS_CR, 0x4)
+#define   AIROHA_USB_PHY_USB20_FS_CR_MIN	FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_FS_CR, 0x6)
+#define   AIROHA_USB_PHY_USB20_FS_SR		GENMASK(2, 0)
+#define   AIROHA_USB_PHY_USB20_FS_SR_MAX	FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_FS_SR, 0x0)
+#define   AIROHA_USB_PHY_USB20_FS_SR_NORMAL	FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_FS_SR, 0x2)
+#define   AIROHA_USB_PHY_USB20_FS_SR_SMALLER	FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_FS_SR, 0x4)
+#define   AIROHA_USB_PHY_USB20_FS_SR_MIN	FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_FS_SR, 0x6)
+#define AIROHA_USB_PHY_USBPHYACR5		0x314
+#define   AIROHA_USB_PHY_USB20_HSTX_SRCAL_EN	BIT(15)
+#define   AIROHA_USB_PHY_USB20_HSTX_SRCTRL	GENMASK(14, 12)
+#define AIROHA_USB_PHY_USBPHYACR6		0x318
+#define   AIROHA_USB_PHY_USB20_BC11_SW_EN	BIT(23)
+#define   AIROHA_USB_PHY_USB20_DISCTH		GENMASK(7, 4)
+#define   AIROHA_USB_PHY_USB20_DISCTH_400	FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_DISCTH, 0x0)
+#define   AIROHA_USB_PHY_USB20_DISCTH_420	FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_DISCTH, 0x1)
+#define   AIROHA_USB_PHY_USB20_DISCTH_440	FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_DISCTH, 0x2)
+#define   AIROHA_USB_PHY_USB20_DISCTH_460	FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_DISCTH, 0x3)
+#define   AIROHA_USB_PHY_USB20_DISCTH_480	FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_DISCTH, 0x4)
+#define   AIROHA_USB_PHY_USB20_DISCTH_500	FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_DISCTH, 0x5)
+#define   AIROHA_USB_PHY_USB20_DISCTH_520	FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_DISCTH, 0x6)
+#define   AIROHA_USB_PHY_USB20_DISCTH_540	FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_DISCTH, 0x7)
+#define   AIROHA_USB_PHY_USB20_DISCTH_560	FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_DISCTH, 0x8)
+#define   AIROHA_USB_PHY_USB20_DISCTH_580	FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_DISCTH, 0x9)
+#define   AIROHA_USB_PHY_USB20_DISCTH_600	FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_DISCTH, 0xa)
+#define   AIROHA_USB_PHY_USB20_DISCTH_620	FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_DISCTH, 0xb)
+#define   AIROHA_USB_PHY_USB20_DISCTH_640	FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_DISCTH, 0xc)
+#define   AIROHA_USB_PHY_USB20_DISCTH_660	FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_DISCTH, 0xd)
+#define   AIROHA_USB_PHY_USB20_DISCTH_680	FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_DISCTH, 0xe)
+#define   AIROHA_USB_PHY_USB20_DISCTH_700	FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_DISCTH, 0xf)
+#define   AIROHA_USB_PHY_USB20_SQTH		GENMASK(3, 0)
+#define   AIROHA_USB_PHY_USB20_SQTH_85		FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_SQTH, 0x0)
+#define   AIROHA_USB_PHY_USB20_SQTH_90		FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_SQTH, 0x1)
+#define   AIROHA_USB_PHY_USB20_SQTH_95		FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_SQTH, 0x2)
+#define   AIROHA_USB_PHY_USB20_SQTH_100		FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_SQTH, 0x3)
+#define   AIROHA_USB_PHY_USB20_SQTH_105		FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_SQTH, 0x4)
+#define   AIROHA_USB_PHY_USB20_SQTH_110		FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_SQTH, 0x5)
+#define   AIROHA_USB_PHY_USB20_SQTH_115		FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_SQTH, 0x6)
+#define   AIROHA_USB_PHY_USB20_SQTH_120		FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_SQTH, 0x7)
+#define   AIROHA_USB_PHY_USB20_SQTH_125		FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_SQTH, 0x8)
+#define   AIROHA_USB_PHY_USB20_SQTH_130		FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_SQTH, 0x9)
+#define   AIROHA_USB_PHY_USB20_SQTH_135		FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_SQTH, 0xa)
+#define   AIROHA_USB_PHY_USB20_SQTH_140		FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_SQTH, 0xb)
+#define   AIROHA_USB_PHY_USB20_SQTH_145		FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_SQTH, 0xc)
+#define   AIROHA_USB_PHY_USB20_SQTH_150		FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_SQTH, 0xd)
+#define   AIROHA_USB_PHY_USB20_SQTH_155		FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_SQTH, 0xe)
+#define   AIROHA_USB_PHY_USB20_SQTH_160		FIELD_PREP_CONST(AIROHA_USB_PHY_USB20_SQTH, 0xf)
+
+#define AIROHA_USB_PHY_U2PHYDTM1		0x36c
+#define   AIROHA_USB_PHY_FORCE_IDDIG		BIT(9)
+#define   AIROHA_USB_PHY_IDDIG			BIT(1)
+
+#define AIROHA_USB_PHY_GPIO_CTLD		0x80c
+#define   AIROHA_USB_PHY_C60802_GPIO_CTLD	GENMASK(31, 0)
+#define     AIROHA_USB_PHY_SSUSB_IP_SW_RST	BIT(31)
+#define     AIROHA_USB_PHY_MCU_BUS_CK_GATE_EN	BIT(30)
+#define     AIROHA_USB_PHY_FORCE_SSUSB_IP_SW_RST BIT(29)
+#define     AIROHA_USB_PHY_SSUSB_SW_RST		BIT(28)
+
+#define AIROHA_USB_PHY_U3_PHYA_REG0		0xb00
+#define   AIROHA_USB_PHY_SSUSB_BG_DIV		GENMASK(29, 28)
+#define   AIROHA_USB_PHY_SSUSB_BG_DIV_2		FIELD_PREP_CONST(AIROHA_USB_PHY_SSUSB_BG_DIV, 0x0)
+#define   AIROHA_USB_PHY_SSUSB_BG_DIV_4		FIELD_PREP_CONST(AIROHA_USB_PHY_SSUSB_BG_DIV, 0x1)
+#define   AIROHA_USB_PHY_SSUSB_BG_DIV_8		FIELD_PREP_CONST(AIROHA_USB_PHY_SSUSB_BG_DIV, 0x2)
+#define   AIROHA_USB_PHY_SSUSB_BG_DIV_16	FIELD_PREP_CONST(AIROHA_USB_PHY_SSUSB_BG_DIV, 0x3)
+#define AIROHA_USB_PHY_U3_PHYA_REG1		0xb04
+#define   AIROHA_USB_PHY_SSUSB_XTAL_TOP_RESERVE	GENMASK(25, 10)
+#define AIROHA_USB_PHY_U3_PHYA_REG6		0xb18
+#define   AIROHA_USB_PHY_SSUSB_CDR_RESERVE	GENMASK(31, 24)
+#define AIROHA_USB_PHY_U3_PHYA_REG8		0xb20
+#define   AIROHA_USB_PHY_SSUSB_CDR_RST_DLY	GENMASK(7, 6)
+#define   AIROHA_USB_PHY_SSUSB_CDR_RST_DLY_32	FIELD_PREP_CONST(AIROHA_USB_PHY_SSUSB_CDR_RST_DLY, 0x0)
+#define   AIROHA_USB_PHY_SSUSB_CDR_RST_DLY_64	FIELD_PREP_CONST(AIROHA_USB_PHY_SSUSB_CDR_RST_DLY, 0x1)
+#define   AIROHA_USB_PHY_SSUSB_CDR_RST_DLY_128	FIELD_PREP_CONST(AIROHA_USB_PHY_SSUSB_CDR_RST_DLY, 0x2)
+#define   AIROHA_USB_PHY_SSUSB_CDR_RST_DLY_216	FIELD_PREP_CONST(AIROHA_USB_PHY_SSUSB_CDR_RST_DLY, 0x3)
+
+#define AIROHA_USB_PHY_U3_PHYA_DA_REG19		0xc38
+#define   AIROHA_USB_PHY_SSUSB_PLL_SSC_DELTA1_U3 GENMASK(15, 0)
+
+#define AIROHA_USB_PHY_U2_FM_DET_CYCLE_CNT	1024
+#define AIROHA_USB_PHY_REF_CK			20
+#define AIROHA_USB_PHY_U2_SR_COEF		28
+#define AIROHA_USB_PHY_U2_SR_COEF_DIVISOR	1000
+
+#define AIROHA_USB_PHY_DEFAULT_SR_CALIBRATION	0x5
+#define AIROHA_USB_PHY_FREQDET_SLEEP		1000 /* 1ms */
+#define AIROHA_USB_PHY_FREQDET_TIMEOUT		(AIROHA_USB_PHY_FREQDET_SLEEP * 10)
+
+struct an7581_usb_phy_instance {
+	struct phy *phy;
+	u32 type;
+};
+
+enum an7581_usb_phy_instance_type {
+	AIROHA_PHY_USB2,
+	AIROHA_PHY_USB3,
+
+	AIROHA_PHY_USB_MAX,
+};
+
+struct an7581_usb_phy_priv {
+	struct device *dev;
+	struct regmap *regmap;
+
+	unsigned int monclk_sel;
+
+	struct phy *serdes_phy;
+	struct an7581_usb_phy_instance *phys[AIROHA_PHY_USB_MAX];
+};
+
+static void an7581_usb_phy_u2_slew_rate_calibration(struct an7581_usb_phy_priv *priv)
+{
+	u32 fm_out = 0;
+	u32 srctrl;
+
+	/* Enable HS TX SR calibration */
+	regmap_set_bits(priv->regmap, AIROHA_USB_PHY_USBPHYACR5,
+			AIROHA_USB_PHY_USB20_HSTX_SRCAL_EN);
+
+	usleep_range(1000, 1500);
+
+	/* Enable Free run clock */
+	regmap_set_bits(priv->regmap, AIROHA_USB_PHY_FMMONR1,
+			AIROHA_USB_PHY_FRCK_EN);
+
+	/* Select Monitor Clock */
+	regmap_update_bits(priv->regmap, AIROHA_USB_PHY_FMCR0,
+			   AIROHA_USB_PHY_MONCLK_SEL,
+			   FIELD_PREP(AIROHA_USB_PHY_MONCLK_SEL,
+				      priv->monclk_sel));
+
+	/* Set cyclecnt */
+	regmap_update_bits(priv->regmap, AIROHA_USB_PHY_FMCR0,
+			   AIROHA_USB_PHY_CYCLECNT,
+			   FIELD_PREP(AIROHA_USB_PHY_CYCLECNT,
+				      AIROHA_USB_PHY_U2_FM_DET_CYCLE_CNT));
+
+	/* Enable Frequency meter */
+	regmap_set_bits(priv->regmap, AIROHA_USB_PHY_FMCR0,
+			AIROHA_USB_PHY_FREQDET_EN);
+
+	/* Timeout can happen and we will apply workaround at the end */
+	regmap_read_poll_timeout(priv->regmap, AIROHA_USB_PHY_FMMONR0, fm_out,
+				 fm_out, AIROHA_USB_PHY_FREQDET_SLEEP,
+				 AIROHA_USB_PHY_FREQDET_TIMEOUT);
+
+	/* Disable Frequency meter */
+	regmap_clear_bits(priv->regmap, AIROHA_USB_PHY_FMCR0,
+			  AIROHA_USB_PHY_FREQDET_EN);
+
+	/* Disable Free run clock */
+	regmap_clear_bits(priv->regmap, AIROHA_USB_PHY_FMMONR1,
+			  AIROHA_USB_PHY_FRCK_EN);
+
+	/* Disable HS TX SR calibration */
+	regmap_clear_bits(priv->regmap, AIROHA_USB_PHY_USBPHYACR5,
+			  AIROHA_USB_PHY_USB20_HSTX_SRCAL_EN);
+
+	usleep_range(1000, 1500);
+
+	/* Frequency was not detected, use default SR calibration value */
+	if (!fm_out) {
+		srctrl = AIROHA_USB_PHY_DEFAULT_SR_CALIBRATION;
+		dev_err(priv->dev, "Frequency not detected, using default SR calibration.\n");
+	} else {
+		/* (1024 / FM_OUT) * REF_CK * U2_SR_COEF (round to the nearest digits) */
+		srctrl = AIROHA_USB_PHY_REF_CK * AIROHA_USB_PHY_U2_SR_COEF;
+		srctrl = (srctrl * AIROHA_USB_PHY_U2_FM_DET_CYCLE_CNT) / fm_out;
+		srctrl = DIV_ROUND_CLOSEST(srctrl, AIROHA_USB_PHY_U2_SR_COEF_DIVISOR);
+		dev_dbg(priv->dev, "SR calibration applied: %x\n", srctrl);
+	}
+
+	regmap_update_bits(priv->regmap, AIROHA_USB_PHY_USBPHYACR5,
+			   AIROHA_USB_PHY_USB20_HSTX_SRCTRL,
+			   FIELD_PREP(AIROHA_USB_PHY_USB20_HSTX_SRCTRL, srctrl));
+}
+
+static void an7581_usb_phy_u2_init(struct an7581_usb_phy_priv *priv)
+{
+	regmap_update_bits(priv->regmap, AIROHA_USB_PHY_USBPHYACR4,
+			   AIROHA_USB_PHY_USB20_FS_CR,
+			   AIROHA_USB_PHY_USB20_FS_CR_MIN);
+
+	regmap_update_bits(priv->regmap, AIROHA_USB_PHY_USBPHYACR4,
+			   AIROHA_USB_PHY_USB20_FS_SR,
+			   AIROHA_USB_PHY_USB20_FS_SR_NORMAL);
+
+	/* FIXME: evaluate if needed */
+	regmap_update_bits(priv->regmap, AIROHA_USB_PHY_USBPHYACR6,
+			   AIROHA_USB_PHY_USB20_SQTH,
+			   AIROHA_USB_PHY_USB20_SQTH_130);
+
+	regmap_update_bits(priv->regmap, AIROHA_USB_PHY_USBPHYACR6,
+			   AIROHA_USB_PHY_USB20_DISCTH,
+			   AIROHA_USB_PHY_USB20_DISCTH_600);
+
+	/* Enable the USB port and then disable after calibration */
+	regmap_clear_bits(priv->regmap, AIROHA_USB_PHY_USBPHYACR6,
+			  AIROHA_USB_PHY_USB20_BC11_SW_EN);
+
+	an7581_usb_phy_u2_slew_rate_calibration(priv);
+
+	regmap_set_bits(priv->regmap, AIROHA_USB_PHY_USBPHYACR6,
+			AIROHA_USB_PHY_USB20_BC11_SW_EN);
+
+	usleep_range(1000, 1500);
+}
+
+/*
+ * USB 3.0 mode can only work if USB serdes is correctly set.
+ * This is validated in xLate function.
+ */
+static void an7581_usb_phy_u3_init(struct an7581_usb_phy_priv *priv)
+{
+	regmap_update_bits(priv->regmap, AIROHA_USB_PHY_U3_PHYA_REG8,
+			   AIROHA_USB_PHY_SSUSB_CDR_RST_DLY,
+			   AIROHA_USB_PHY_SSUSB_CDR_RST_DLY_32);
+
+	regmap_update_bits(priv->regmap, AIROHA_USB_PHY_U3_PHYA_REG6,
+			   AIROHA_USB_PHY_SSUSB_CDR_RESERVE,
+			   FIELD_PREP(AIROHA_USB_PHY_SSUSB_CDR_RESERVE, 0xe));
+
+	regmap_update_bits(priv->regmap, AIROHA_USB_PHY_U3_PHYA_REG0,
+			   AIROHA_USB_PHY_SSUSB_BG_DIV,
+			   AIROHA_USB_PHY_SSUSB_BG_DIV_4);
+
+	regmap_set_bits(priv->regmap, AIROHA_USB_PHY_U3_PHYA_REG1,
+			FIELD_PREP(AIROHA_USB_PHY_SSUSB_XTAL_TOP_RESERVE, 0x600));
+
+	regmap_update_bits(priv->regmap, AIROHA_USB_PHY_U3_PHYA_DA_REG19,
+			   AIROHA_USB_PHY_SSUSB_PLL_SSC_DELTA1_U3,
+			   FIELD_PREP(AIROHA_USB_PHY_SSUSB_PLL_SSC_DELTA1_U3, 0x43));
+}
+
+static int an7581_usb_phy_init(struct phy *phy)
+{
+	struct an7581_usb_phy_instance *instance = phy_get_drvdata(phy);
+	struct an7581_usb_phy_priv *priv = dev_get_drvdata(phy->dev.parent);
+	int ret;
+
+	switch (instance->type) {
+	case PHY_TYPE_USB2:
+		an7581_usb_phy_u2_init(priv);
+		break;
+	case PHY_TYPE_USB3:
+		ret = phy_set_mode(priv->serdes_phy, PHY_MODE_USB_DEVICE_SS);
+		if (ret)
+			return ret;
+
+		an7581_usb_phy_u3_init(priv);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int an7581_usb_phy_u2_power_on(struct an7581_usb_phy_priv *priv)
+{
+	regmap_clear_bits(priv->regmap, AIROHA_USB_PHY_USBPHYACR6,
+			  AIROHA_USB_PHY_USB20_BC11_SW_EN);
+
+	usleep_range(1000, 1500);
+
+	return 0;
+}
+
+static int an7581_usb_phy_u3_power_on(struct an7581_usb_phy_priv *priv)
+{
+	regmap_clear_bits(priv->regmap, AIROHA_USB_PHY_GPIO_CTLD,
+			  AIROHA_USB_PHY_SSUSB_IP_SW_RST |
+			  AIROHA_USB_PHY_MCU_BUS_CK_GATE_EN |
+			  AIROHA_USB_PHY_FORCE_SSUSB_IP_SW_RST |
+			  AIROHA_USB_PHY_SSUSB_SW_RST);
+
+	usleep_range(1000, 1500);
+
+	return 0;
+}
+
+static int an7581_usb_phy_power_on(struct phy *phy)
+{
+	struct an7581_usb_phy_instance *instance = phy_get_drvdata(phy);
+	struct an7581_usb_phy_priv *priv = dev_get_drvdata(phy->dev.parent);
+
+	switch (instance->type) {
+	case PHY_TYPE_USB2:
+		an7581_usb_phy_u2_power_on(priv);
+		break;
+	case PHY_TYPE_USB3:
+		an7581_usb_phy_u3_power_on(priv);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int an7581_usb_phy_u2_power_off(struct an7581_usb_phy_priv *priv)
+{
+	regmap_set_bits(priv->regmap, AIROHA_USB_PHY_USBPHYACR6,
+			AIROHA_USB_PHY_USB20_BC11_SW_EN);
+
+	usleep_range(1000, 1500);
+
+	return 0;
+}
+
+static int an7581_usb_phy_u3_power_off(struct an7581_usb_phy_priv *priv)
+{
+	regmap_set_bits(priv->regmap, AIROHA_USB_PHY_GPIO_CTLD,
+			AIROHA_USB_PHY_SSUSB_IP_SW_RST |
+			AIROHA_USB_PHY_FORCE_SSUSB_IP_SW_RST);
+
+	usleep_range(1000, 1500);
+
+	return 0;
+}
+
+static int an7581_usb_phy_power_off(struct phy *phy)
+{
+	struct an7581_usb_phy_instance *instance = phy_get_drvdata(phy);
+	struct an7581_usb_phy_priv *priv = dev_get_drvdata(phy->dev.parent);
+
+	switch (instance->type) {
+	case PHY_TYPE_USB2:
+		an7581_usb_phy_u2_power_off(priv);
+		break;
+	case PHY_TYPE_USB3:
+		an7581_usb_phy_u3_power_off(priv);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int an7581_usb_phy_u2_set_mode(struct an7581_usb_phy_priv *priv,
+				      enum phy_mode mode)
+{
+	u32 val;
+
+	/*
+	 * For Device and Host mode, enable force IDDIG.
+	 * For Device set IDDIG, for Host clear IDDIG.
+	 * For OTG disable force and clear IDDIG bit while at it.
+	 */
+	switch (mode) {
+	case PHY_MODE_USB_DEVICE:
+		val = AIROHA_USB_PHY_FORCE_IDDIG |
+		      AIROHA_USB_PHY_IDDIG;
+		break;
+	case PHY_MODE_USB_HOST:
+		val = AIROHA_USB_PHY_FORCE_IDDIG;
+		break;
+	case PHY_MODE_USB_OTG:
+		val = 0;
+		break;
+	default:
+		return 0;
+	}
+
+	regmap_update_bits(priv->regmap, AIROHA_USB_PHY_U2PHYDTM1,
+			   AIROHA_USB_PHY_FORCE_IDDIG |
+			   AIROHA_USB_PHY_IDDIG, val);
+
+	return 0;
+}
+
+static int an7581_usb_phy_set_mode(struct phy *phy, enum phy_mode mode, int submode)
+{
+	struct an7581_usb_phy_instance *instance = phy_get_drvdata(phy);
+	struct an7581_usb_phy_priv *priv = dev_get_drvdata(phy->dev.parent);
+
+	switch (instance->type) {
+	case PHY_TYPE_USB2:
+		return an7581_usb_phy_u2_set_mode(priv, mode);
+	default:
+		return 0;
+	}
+}
+
+static struct phy *an7581_usb_phy_xlate(struct device *dev,
+					const struct of_phandle_args *args)
+{
+	struct an7581_usb_phy_priv *priv = dev_get_drvdata(dev);
+	struct an7581_usb_phy_instance *instance = NULL;
+	unsigned int index, phy_type;
+
+	if (args->args_count != 1) {
+		dev_err(dev, "invalid number of cells in 'phy' property\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	phy_type = args->args[0];
+	if (!(phy_type == PHY_TYPE_USB2 || phy_type == PHY_TYPE_USB3)) {
+		dev_err(dev, "unsupported device type: %d\n", phy_type);
+		return ERR_PTR(-EINVAL);
+	}
+
+	for (index = 0; index < AIROHA_PHY_USB_MAX; index++)
+		if (priv->phys[index] &&
+		    phy_type == priv->phys[index]->type) {
+			instance = priv->phys[index];
+			break;
+		}
+
+	if (!instance) {
+		dev_err(dev, "failed to find appropriate phy\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (instance->type == PHY_TYPE_USB3 && !priv->serdes_phy) {
+		dev_err(dev, "missing serdes phy for USB 3.0\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	return instance->phy;
+}
+
+static const struct phy_ops airoha_phy = {
+	.init		= an7581_usb_phy_init,
+	.power_on	= an7581_usb_phy_power_on,
+	.power_off	= an7581_usb_phy_power_off,
+	.set_mode	= an7581_usb_phy_set_mode,
+	.owner		= THIS_MODULE,
+};
+
+static const struct regmap_config an7581_usb_phy_regmap_config = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+};
+
+static int an7581_usb_phy_probe(struct platform_device *pdev)
+{
+	struct phy_provider *phy_provider;
+	struct an7581_usb_phy_priv *priv;
+	struct device *dev = &pdev->dev;
+	unsigned int index;
+	void __iomem *base;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = dev;
+
+	ret = of_property_read_u32(dev->of_node, "airoha,usb2-monitor-clk-sel",
+				   &priv->monclk_sel);
+	if (ret)
+		return dev_err_probe(dev, ret, "Monitor clock selection is mandatory for USB PHY calibration\n");
+
+	if (priv->monclk_sel > 3)
+		return dev_err_probe(dev, -EINVAL, "only 4 Monitor clock are selectable on the SoC\n");
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	priv->regmap = devm_regmap_init_mmio(dev, base, &an7581_usb_phy_regmap_config);
+	if (IS_ERR(priv->regmap))
+		return PTR_ERR(priv->regmap);
+
+	platform_set_drvdata(pdev, priv);
+
+	for (index = 0; index < AIROHA_PHY_USB_MAX; index++) {
+		struct an7581_usb_phy_instance *instance;
+		u32 phy_type;
+
+		switch (index) {
+		case AIROHA_PHY_USB2:
+			phy_type = PHY_TYPE_USB2;
+			break;
+		case AIROHA_PHY_USB3:
+			phy_type = PHY_TYPE_USB3;
+			break;
+		}
+
+		if (phy_type == PHY_TYPE_USB3) {
+			priv->serdes_phy = devm_phy_optional_get(dev, NULL);
+			if (IS_ERR(priv->serdes_phy))
+				return dev_err_probe(dev, PTR_ERR(priv->serdes_phy), "error on serdes phy for USB 3.0\n");
+		}
+
+		instance = devm_kzalloc(dev, sizeof(*instance), GFP_KERNEL);
+		if (!instance)
+			return -ENOMEM;
+
+		instance->type = phy_type;
+		priv->phys[index] = instance;
+
+		instance->phy = devm_phy_create(dev, NULL, &airoha_phy);
+		if (IS_ERR(instance->phy))
+			return dev_err_probe(dev, PTR_ERR(instance->phy), "failed to create phy\n");
+
+		phy_set_drvdata(instance->phy, instance);
+	}
+
+	phy_provider = devm_of_phy_provider_register(&pdev->dev, an7581_usb_phy_xlate);
+
+	return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static const struct of_device_id airoha_phy_id_table[] = {
+	{ .compatible = "airoha,an7581-usb-phy" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, airoha_phy_id_table);
+
+static struct platform_driver an7581_usb_driver = {
+	.probe		= an7581_usb_phy_probe,
+	.driver		= {
+		.name	= "airoha-an7581-usb-phy",
+		.of_match_table = airoha_phy_id_table,
+	},
+};
+
+module_platform_driver(an7581_usb_driver);
+
+MODULE_DESCRIPTION("Airoha AN7581 USB PHY driver");
+MODULE_AUTHOR("Christian Marangi <ansuelsmth@gmail.com>");
+MODULE_LICENSE("GPL");
-- 
2.53.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply related

* [PATCH v9 4/5] phy: move and rename Airoha PCIe PHY driver to dedicated directory
From: Christian Marangi @ 2026-05-21 15:35 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Christian Marangi, Vinod Koul,
	Neil Armstrong, Lorenzo Bianconi, Felix Fietkau, linux-clk,
	devicetree, linux-kernel, linux-arm-kernel, linux-phy
In-Reply-To: <20260521153645.7028-1-ansuelsmth@gmail.com>

To keep the generic PHY directory tidy, move the PCIe PHY driver for
Airoha AN7581 SoC to a dedicated directory.

Also rename the driver and add the relevant SoC name to the .c and .h
file in preparation for support of PCIe and USB PHY driver for Airoha
AN7583 SoC that use a completely different implementation and
calibration for PHYs and will have their own dedicated drivers.

The rename permits to better identify the specific usage of the driver
in the future once the airoha PHY directory will have multiple driver
for multiple SoC.

The config is changed from PHY_AIROHA_PCIE to PHY_AIROHA_AN7581_PCIE.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 MAINTAINERS                                         |  4 ++--
 drivers/phy/Kconfig                                 | 11 +----------
 drivers/phy/Makefile                                |  4 ++--
 drivers/phy/airoha/Kconfig                          | 13 +++++++++++++
 drivers/phy/airoha/Makefile                         |  3 +++
 .../phy-an7581-pcie-regs.h}                         |  2 +-
 .../{phy-airoha-pcie.c => airoha/phy-an7581-pcie.c} |  6 +++---
 7 files changed, 25 insertions(+), 18 deletions(-)
 create mode 100644 drivers/phy/airoha/Kconfig
 create mode 100644 drivers/phy/airoha/Makefile
 rename drivers/phy/{phy-airoha-pcie-regs.h => airoha/phy-an7581-pcie-regs.h} (99%)
 rename drivers/phy/{phy-airoha-pcie.c => airoha/phy-an7581-pcie.c} (99%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 932044785a39..7bea8c620da8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -759,8 +759,8 @@ M:	Lorenzo Bianconi <lorenzo@kernel.org>
 L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
 S:	Maintained
 F:	Documentation/devicetree/bindings/phy/airoha,en7581-pcie-phy.yaml
-F:	drivers/phy/phy-airoha-pcie-regs.h
-F:	drivers/phy/phy-airoha-pcie.c
+F:	drivers/phy/airoha/phy-an7581-pcie-regs.h
+F:	drivers/phy/airoha/phy-an7581-pcie.c
 
 AIROHA SPI SNFI DRIVER
 M:	Lorenzo Bianconi <lorenzo@kernel.org>
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 227b9a4c612e..f9cd765a3ccc 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -46,16 +46,6 @@ config GENERIC_PHY_MIPI_DPHY
 	  Provides a number of helpers a core functions for MIPI D-PHY
 	  drivers to us.
 
-config PHY_AIROHA_PCIE
-	tristate "Airoha PCIe-PHY Driver"
-	depends on ARCH_AIROHA || COMPILE_TEST
-	depends on OF
-	select GENERIC_PHY
-	help
-	  Say Y here to add support for Airoha PCIe PHY driver.
-	  This driver create the basic PHY instance and provides initialize
-	  callback for PCIe GEN3 port.
-
 config PHY_CAN_TRANSCEIVER
 	tristate "CAN transceiver PHY"
 	select GENERIC_PHY
@@ -133,6 +123,7 @@ config PHY_XGENE
 	help
 	  This option enables support for APM X-Gene SoC multi-purpose PHY.
 
+source "drivers/phy/airoha/Kconfig"
 source "drivers/phy/allwinner/Kconfig"
 source "drivers/phy/amlogic/Kconfig"
 source "drivers/phy/apple/Kconfig"
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index f49d83f00a3d..84062279fa63 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -7,7 +7,6 @@ obj-$(CONFIG_PHY_COMMON_PROPS)		+= phy-common-props.o
 obj-$(CONFIG_PHY_COMMON_PROPS_TEST)	+= phy-common-props-test.o
 obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
 obj-$(CONFIG_GENERIC_PHY_MIPI_DPHY)	+= phy-core-mipi-dphy.o
-obj-$(CONFIG_PHY_AIROHA_PCIE)		+= phy-airoha-pcie.o
 obj-$(CONFIG_PHY_CAN_TRANSCEIVER)	+= phy-can-transceiver.o
 obj-$(CONFIG_PHY_GOOGLE_USB)		+= phy-google-usb.o
 obj-$(CONFIG_USB_LGM_PHY)		+= phy-lgm-usb.o
@@ -17,7 +16,8 @@ obj-$(CONFIG_PHY_PISTACHIO_USB)		+= phy-pistachio-usb.o
 obj-$(CONFIG_PHY_SNPS_EUSB2)		+= phy-snps-eusb2.o
 obj-$(CONFIG_PHY_XGENE)			+= phy-xgene.o
 
-obj-$(CONFIG_GENERIC_PHY)		+= allwinner/	\
+obj-$(CONFIG_GENERIC_PHY)		+= airoha/	\
+					   allwinner/	\
 					   amlogic/	\
 					   apple/	\
 					   broadcom/	\
diff --git a/drivers/phy/airoha/Kconfig b/drivers/phy/airoha/Kconfig
new file mode 100644
index 000000000000..9a1b625a7701
--- /dev/null
+++ b/drivers/phy/airoha/Kconfig
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Phy drivers for Airoha devices
+#
+config PHY_AIROHA_AN7581_PCIE
+	tristate "Airoha AN7581 PCIe-PHY Driver"
+	depends on ARCH_AIROHA || COMPILE_TEST
+	depends on OF
+	select GENERIC_PHY
+	help
+	  Say Y here to add support for Airoha AN7581 PCIe PHY driver.
+	  This driver create the basic PHY instance and provides initialize
+	  callback for PCIe GEN3 port.
diff --git a/drivers/phy/airoha/Makefile b/drivers/phy/airoha/Makefile
new file mode 100644
index 000000000000..912f3e11a061
--- /dev/null
+++ b/drivers/phy/airoha/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_PHY_AIROHA_AN7581_PCIE)	+= phy-an7581-pcie.o
diff --git a/drivers/phy/phy-airoha-pcie-regs.h b/drivers/phy/airoha/phy-an7581-pcie-regs.h
similarity index 99%
rename from drivers/phy/phy-airoha-pcie-regs.h
rename to drivers/phy/airoha/phy-an7581-pcie-regs.h
index 58572c793722..b938a7b468fe 100644
--- a/drivers/phy/phy-airoha-pcie-regs.h
+++ b/drivers/phy/airoha/phy-an7581-pcie-regs.h
@@ -1,4 +1,4 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
+// SPDX-License-Identifier: GPL-2.0-only
 /*
  * Copyright (c) 2024 AIROHA Inc
  * Author: Lorenzo Bianconi <lorenzo@kernel.org>
diff --git a/drivers/phy/phy-airoha-pcie.c b/drivers/phy/airoha/phy-an7581-pcie.c
similarity index 99%
rename from drivers/phy/phy-airoha-pcie.c
rename to drivers/phy/airoha/phy-an7581-pcie.c
index 56e9ade8a9fd..81ddf0e7638b 100644
--- a/drivers/phy/phy-airoha-pcie.c
+++ b/drivers/phy/airoha/phy-an7581-pcie.c
@@ -13,7 +13,7 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 
-#include "phy-airoha-pcie-regs.h"
+#include "phy-an7581-pcie-regs.h"
 
 #define LEQ_LEN_CTRL_MAX_VAL	7
 #define FREQ_LOCK_MAX_ATTEMPT	10
@@ -1279,12 +1279,12 @@ MODULE_DEVICE_TABLE(of, airoha_pcie_phy_of_match);
 static struct platform_driver airoha_pcie_phy_driver = {
 	.probe	= airoha_pcie_phy_probe,
 	.driver	= {
-		.name = "airoha-pcie-phy",
+		.name = "airoha-an7581-pcie-phy",
 		.of_match_table = airoha_pcie_phy_of_match,
 	},
 };
 module_platform_driver(airoha_pcie_phy_driver);
 
-MODULE_DESCRIPTION("Airoha PCIe PHY driver");
+MODULE_DESCRIPTION("Airoha AN7581 PCIe PHY driver");
 MODULE_AUTHOR("Lorenzo Bianconi <lorenzo@kernel.org>");
 MODULE_LICENSE("GPL");
-- 
2.53.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox