Linux-PHY Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
From: Bryan O'Donoghue @ 2026-05-21 12:20 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 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       | 205 ++++++++++++
 MAINTAINERS                                        |  10 +
 drivers/phy/qualcomm/Kconfig                       |  13 +
 drivers/phy/qualcomm/Makefile                      |   5 +
 drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c | 361 ++++++++++++++++++++
 drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c     | 368 +++++++++++++++++++++
 drivers/phy/qualcomm/phy-qcom-mipi-csi2.h          |  94 ++++++
 7 files changed, 1056 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

* Re: [PATCH v2 4/4] arm64: dts: qcom: x1-dell-thena: bump linux,cma to 256 MiB
From: Bryan O'Donoghue @ 2026-05-21 12:04 UTC (permalink / raw)
  To: Michael Scott, linux-arm-msm
  Cc: vkoul, neil.armstrong, dmitry.baryshkov, wesley.cheng, abelvesa,
	faisal.hassan, linux-phy, andersson, konradybcio, robh, krzk+dt,
	conor+dt, devicetree, val, laurentiu.tudor1, alex.vinarskis,
	linux-kernel
In-Reply-To: <20260521010935.1333494-5-mike.scott@oss.qualcomm.com>

On 21/05/2026 02:09, Michael Scott wrote:
> The 128 MiB linux,cma reserved-memory pool on dell-thena is too small
> to support the camera pipeline in parallel with the normal Linux
> desktop. On a freshly-booted system with GNOME running, the typical
> runtime consumers — msm DRM framebuffers (Wayland triple buffering on
> the eDP panel), qcom_iris video codec buffers, qcom_camss VFE
> pre-allocated buffers — already occupy ~100 MiB of the pool, leaving
> only ~25 MiB free.
> 
> The libcamera "simple" pipeline handler used by /dev/media0 on
> dell-thena allocates four ABGR8888 frames at 1920×1088 = 32 MiB total.
> That request fails on the fourth frame:
> 
>      ERROR DmaBufAllocator: dma-heap allocation failure for frame-3
>      ERROR Allocator: Stream is not part of /base/.../camera@10 active configuration
>      Can't allocate buffers
>      Failed to start camera session
> 
> resulting in gnome-snapshot's "Could not play camera stream" and any
> other libcamera-mediated app being unable to actually stream.
> 
> Bumping linux,cma to 256 MiB (a 0.9% reservation on these laptops'
> typical 27 GiB RAM) leaves ~150 MiB free at runtime — sufficient for
> the libcamera buffer set plus headroom for video playback or other
> CMA-hungry workloads in parallel.
> 
> Tested on Dell Latitude 7455: with the 256 MiB pool, CmaFree at
> GNOME-desktop idle is ~150 MiB, gnome-snapshot streams the OV02E10
> camera cleanly, and `cam -c 1 --capture=2` succeeds.
> 
> The companion board files dell-inspiron-14-plus-7441 and the upstream
> .dts variants inherit from x1-dell-thena.dtsi, so this changes the
> pool size for every dell-thena-based laptop in one place.
> 
> Signed-off-by: Michael Scott <mike.scott@oss.qualcomm.com>
> ---
>   arch/arm64/boot/dts/qcom/x1-dell-thena.dtsi | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/x1-dell-thena.dtsi b/arch/arm64/boot/dts/qcom/x1-dell-thena.dtsi
> index d6de4da02dcd..714988a81384 100644
> --- a/arch/arm64/boot/dts/qcom/x1-dell-thena.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1-dell-thena.dtsi
> @@ -167,7 +167,7 @@ led-camera-indicator {
>   	reserved-memory {
>   		linux,cma {
>   			compatible = "shared-dma-pool";
> -			size = <0x0 0x8000000>;
> +			size = <0x0 0x10000000>;
>   			reusable;
>   			linux,cma-default;
>   		};

How old is your version of libcamera ?

With CONFIG_UDMA=y you don't need a contiguous memory area at all and 
you will get juicy and delicious GPUISP.

Instead of allocating in the kernel just use a better version of libcamera

┌─[deckard@inspiron14p-linux] - [~/Development/libcamera] - [Thu May 21, 
13:03]
└─[$] <git:(0.7.0-multipass-v0*)> zcat /proc/config.gz | grep UDMA
CONFIG_UDMABUF=y
┌─[deckard@inspiron14p-linux] - [~/Development/libcamera] - [Thu May 21, 
13:03]
└─[$] <git:(0.7.0-multipass-v0*)> cam -v
libcamera version v0.7.1
┌─[deckard@inspiron14p-linux] - [~/Development/libcamera] - [Thu May 21, 
13:03]
└─[$] <git:(0.7.0-multipass-v0*)> qcam
[68:50:10.493478857] [438859]  INFO Camera camera_manager.cpp:340 
libcamera v0.7.1
[68:50:10.511134091] [438863] ERROR V4L2 v4l2_subdevice.cpp:1192 
'ov02e10 10-0010': Unable to get rectangle 2 on pad 0/0: Inappropriate 
ioctl for device
[68:50:10.511201590] [438863]  WARN CameraSensor 
camera_sensor_legacy.cpp:402 'ov02e10 10-0010': The PixelArraySize 
property has been defaulted to 1928x1088
[68:50:10.511206069] [438863] ERROR V4L2 v4l2_subdevice.cpp:1192 
'ov02e10 10-0010': Unable to get rectangle 1 on pad 0/0: Inappropriate 
ioctl for device
[68:50:10.511209559] [438863]  WARN CameraSensor 
camera_sensor_legacy.cpp:413 'ov02e10 10-0010': The 
PixelArrayActiveAreas property has been defaulted to (0, 0)/1928x1088
[68:50:10.511213778] [438863] ERROR V4L2 v4l2_subdevice.cpp:1192 
'ov02e10 10-0010': Unable to get rectangle 0 on pad 0/0: Inappropriate 
ioctl for device
[68:50:10.511216590] [438863]  WARN CameraSensor 
camera_sensor_legacy.cpp:421 'ov02e10 10-0010': Failed to retrieve the 
sensor crop rectangle
[68:50:10.511219559] [438863]  WARN CameraSensor 
camera_sensor_legacy.cpp:427 'ov02e10 10-0010': The sensor kernel driver 
needs to be fixed
[68:50:10.511221746] [438863]  WARN CameraSensor 
camera_sensor_legacy.cpp:429 'ov02e10 10-0010': See 
Documentation/sensor_driver_requirements.rst in the libcamera sources 
for more information
[68:50:10.511327474] [438863]  WARN CameraSensorProperties 
camera_sensor_properties.cpp:538 No static properties available for 
'ov02e10'
[68:50:10.511330599] [438863]  WARN CameraSensorProperties 
camera_sensor_properties.cpp:540 Please consider updating the camera 
sensor properties database
[68:50:10.511334089] [438863]  WARN CameraSensor 
camera_sensor_legacy.cpp:617 'ov02e10 10-0010': Rotation control not 
available, default to 0 degrees
[68:50:10.511340912] [438863]  WARN CameraSensor 
camera_sensor_legacy.cpp:502 'ov02e10 10-0010': No sensor delays found 
in static properties. Assuming unverified defaults.
[68:50:10.512362985] [438863]  WARN IPAProxy ipa_proxy.cpp:196 
Configuration file 'ov02e10.yaml' not found for IPA module 'simple', 
falling back to '/usr/share/libcamera/ipa/simple/uncalibrated.yaml'
[68:50:10.512372828] [438863] ERROR V4L2 v4l2_subdevice.cpp:1192 
'ov02e10 10-0010': Unable to get rectangle 0 on pad 0/0: Inappropriate 
ioctl for device
[68:50:10.512377464] [438863]  WARN CameraSensor 
camera_sensor_legacy.cpp:881 'ov02e10 10-0010': The analogue crop 
rectangle has been defaulted to the active area size
[68:50:10.512386578] [438863]  WARN IPASoft soft_simple.cpp:104 IPASoft: 
Failed to create camera sensor helper for ov02e10
[68:50:10.512505275] [438863]  INFO Camera camera_manager.cpp:223 Adding 
camera '/base/soc@0/cci@ac16000/i2c-bus@1/camera@10' for pipeline 
handler simple
[68:50:10.548026157] [438859]  INFO Camera camera.cpp:1216 configuring 
streams: (0) 1920x1088-ABGR8888/sRGB
[68:50:10.548323081] [438863]  INFO IPASoft soft_simple.cpp:258 IPASoft: 
Exposure 1-2242, gain 16-248 (1)
[68:50:10.548402247] [438863]  INFO SoftwareIsp software_isp.cpp:278 
Input 1928x1088-GRBG-10-CSI2P stride 2416
Zero-copy enabled
[68:50:10.636862424] [438866]  INFO eGL egl.cpp:288 EGL: EGL_VERSION: 1.5
[68:50:10.636899299] [438866]  INFO eGL egl.cpp:289 EGL: EGL_VENDOR: 
Mesa Project
[68:50:10.636902112] [438866]  INFO eGL egl.cpp:290 EGL: 
EGL_CLIENT_APIS: OpenGL OpenGL_ES
[68:50:10.636904768] [438866]  INFO eGL egl.cpp:291 EGL: EGL_EXTENSIONS: 
EGL_ANDROID_blob_cache EGL_ANDROID_native_fence_sync 
EGL_EXT_config_select_group EGL_EXT_create_context_robustness 
EGL_EXT_image_dma_buf_import EGL_EXT_image_dma_buf_import_modifiers 
EGL_EXT_query_reset_notification_strategy EGL_EXT_surface_compression 
EGL_IMG_context_priority EGL_KHR_cl_event2 EGL_KHR_config_attribs 
EGL_KHR_context_flush_control EGL_KHR_create_context 
EGL_KHR_create_context_no_error EGL_KHR_fence_sync 
EGL_KHR_get_all_proc_addresses EGL_KHR_gl_colorspace 
EGL_KHR_gl_renderbuffer_image EGL_KHR_gl_texture_2D_image 
EGL_KHR_gl_texture_3D_image EGL_KHR_gl_texture_cubemap_image 
EGL_KHR_image_base EGL_KHR_no_config_context EGL_KHR_reusable_sync 
EGL_KHR_surfaceless_context EGL_EXT_pixel_format_float EGL_KHR_wait_sync 
EGL_MESA_configless_context EGL_MESA_gl_interop 
EGL_MESA_image_dma_buf_export EGL_MESA_query_driver 
EGL_MESA_x11_native_visual_id EGL_NV_context_priority_realtime
[68:50:10.643064652] [438866]  INFO eGL egl.cpp:332 EGL: GL_VERSION: 
OpenGL ES 3.2 Mesa 26.0.6-arch1.1
[68:50:12.667202273] [438866]  INFO Benchmark benchmark.cpp:89 Debayer 
processed 30 frames in 228802us, 7626 us/frame
┌─[deckard@inspiron14p-linux] - [~/Development/libcamera] - [Thu May 21, 
13:03]
└─[$] <git:(0.7.0-multipass-v0*)>

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

^ permalink raw reply

* Re: [PATCH v2 2/4] phy: qcom: qmp-combo: track whether the cached typec_mux mode was committed to hardware
From: Bryan O'Donoghue @ 2026-05-21 12:00 UTC (permalink / raw)
  To: Michael Scott, linux-arm-msm
  Cc: vkoul, neil.armstrong, dmitry.baryshkov, wesley.cheng, abelvesa,
	faisal.hassan, linux-phy, andersson, konradybcio, robh, krzk+dt,
	conor+dt, devicetree, val, laurentiu.tudor1, alex.vinarskis,
	linux-kernel
In-Reply-To: <20260521010935.1333494-3-mike.scott@oss.qualcomm.com>

On 21/05/2026 02:09, Michael Scott wrote:
> qmp_combo_typec_mux_set() updates qmp->qmpphy_mode (the cached state)
> unconditionally, but only reprograms hardware when qmp->init_count is
> non-zero. If pmic_glink_altmode (or any other typec_mux consumer)
> calls into the PHY before DWC3 has performed phy_init() -- a real
> ordering observed during testing of USB-C role-switch enablement on
> Snapdragon X (X1E80100) -- the cache transitions away from the
> probe default QMPPHY_MODE_USB3DP but the hardware is never touched.
> 
> Subsequent calls (for example on partner detach, where TYPEC_STATE_SAFE
> also resolves to QMPPHY_MODE_USB3_ONLY in the !DP-SVID branch) then
> match the cached mode and the function bails out early with:
> 
>    qcom-qmp-combo-phy faXX000.phy: typec_mux_set: same qmpphy mode, bail out
> 
> leaving the lane mux in whatever configuration it powered up in. On
> the Dell Latitude 7455 this manifests as the SS lanes being left in
> the default state when the first altmode notification arrives during
> DWC3 probe, with the function bailing out on every subsequent attach.
> 
> Track separately whether the cached mode has actually been committed
> to hardware. The bail-out optimization is only safe when the cache
> truly reflects the hardware:
> 
>    - qmp_combo_typec_mux_set(): bail only when the cached mode matches
>      and was committed; clear the committed flag whenever the cache is
>      updated, set it again after a successful reprogram inside the
>      init_count-guarded block.
> 
>    - qmp_combo_com_init(): set the committed flag at the end of a
>      successful init, since com_init() programs registers from the
>      cached qmpphy_mode.
> 
> No behavioural change on platforms where typec_mux_set never fires
> before phy_init -- committed remains true through normal operation.
> 
> Signed-off-by: Michael Scott <mike.scott@oss.qualcomm.com>
> ---
>   drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 25 +++++++++++++++++++++--
>   1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 0db200292642..e28bc1cc7a78 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -2295,6 +2295,7 @@ struct qmp_combo {
>   	struct mutex phy_mutex;
>   	int init_count;
>   	enum qmpphy_mode qmpphy_mode;
> +	bool qmpphy_mode_committed;
>   
>   	struct phy *usb_phy;
>   	enum phy_mode phy_mode;
> @@ -3754,6 +3755,9 @@ static int qmp_combo_com_init(struct qmp_combo *qmp, bool force)
>   	qphy_setbits(qmp->pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL],
>   			SW_PWRDN);
>   
> +	/* com_init() just programmed registers from qmp->qmpphy_mode. */
> +	qmp->qmpphy_mode_committed = true;
> +
>   	return 0;
>   
>   err_disable_clocks:
> @@ -4509,9 +4513,22 @@ static int qmp_combo_typec_mux_set(struct typec_mux_dev *mux, struct typec_mux_s
>   		new_mode = QMPPHY_MODE_USB3_ONLY;
>   	}
>   
> +	/*
> +	 * Fast-path bail only when the cached mode is also known to be
> +	 * committed to hardware. The cache may be ahead of the hardware
> +	 * if a typec_mux_set arrived while the PHY had not yet been
> +	 * initialised (init_count == 0); in that case the cache update
> +	 * below was the only thing that ran, and we still need to drive
> +	 * the registers when the PHY does come up.
> +	 */
>   	if (new_mode == qmp->qmpphy_mode) {
> -		dev_dbg(qmp->dev, "typec_mux_set: same qmpphy mode, bail out\n");
> -		return 0;
> +		if (qmp->qmpphy_mode_committed) {
> +			dev_dbg(qmp->dev,
> +				"typec_mux_set: same qmpphy mode (committed), bail out\n");
> +			return 0;
> +		}
> +		dev_dbg(qmp->dev,
> +			"typec_mux_set: same qmpphy mode but uncommitted; reprogramming\n");
>   	}
>   
>   	if (qmp->qmpphy_mode != QMPPHY_MODE_USB3_ONLY && qmp->dp_powered_on) {
> @@ -4523,6 +4540,7 @@ static int qmp_combo_typec_mux_set(struct typec_mux_dev *mux, struct typec_mux_s
>   		qmp->qmpphy_mode, new_mode);
>   
>   	qmp->qmpphy_mode = new_mode;
> +	qmp->qmpphy_mode_committed = false;
>   
>   	if (qmp->init_count) {
>   		if (qmp->usb_init_count)
> @@ -4551,6 +4569,9 @@ static int qmp_combo_typec_mux_set(struct typec_mux_dev *mux, struct typec_mux_s
>   			if (qmp->dp_init_count)
>   				cfg->dp_aux_init(qmp);
>   		}
> +
> +		/* Reprogram complete; cache now reflects hardware. */
> +		qmp->qmpphy_mode_committed = true;
>   	}
>   
>   	return 0;

Can we not make the commit to hardware atomic from the perspective of 
the caller ?

i.e. use a workqueue and a completion timeout when setting ?

---
bod

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

^ permalink raw reply

* Re: [PATCH v2 1/4] phy: qcom: qmp-combo: skip USB power_off/exit after device teardown
From: Bryan O'Donoghue @ 2026-05-21 11:58 UTC (permalink / raw)
  To: Michael Scott, linux-arm-msm
  Cc: vkoul, neil.armstrong, dmitry.baryshkov, wesley.cheng, abelvesa,
	faisal.hassan, linux-phy, andersson, konradybcio, robh, krzk+dt,
	conor+dt, devicetree, val, laurentiu.tudor1, alex.vinarskis,
	linux-kernel
In-Reply-To: <20260521010935.1333494-2-mike.scott@oss.qualcomm.com>

On 21/05/2026 02:09, Michael Scott wrote:
> qmp_combo_usb_power_off() is reachable from an external consumer
> (notably dwc3 via phy_exit() during driver unbind) after this device's
> backing resources have already been released along a separate teardown
> chain. The dereference of qmp->pcs (whose ioremap mapping has been
> freed by devm cleanup) then takes a level-3 translation fault and
> oopses.
> 
> Easily reproducible during testing of USB-C role-switch enablement on
> Dell Latitude 7455 (X1E80100), by writing "none" to a USB-C DWC3's
> usb_role_switch role attribute, e.g.
> 
>    echo none > /sys/class/usb_role/a800000.usb-role-switch/role
> 
> which triggers the chain:
> 
>    Unable to handle kernel paging request at virtual address ffff8000876c5400
>    pc : qmp_combo_usb_power_off.isra.0+0x58/0x470 [phy_qcom_qmp_combo]
>    Call trace:
>      qmp_combo_usb_power_off+0x58/0x470 [phy_qcom_qmp_combo]
>      qmp_combo_usb_exit+0x38/0x90 [phy_qcom_qmp_combo]
>      phy_exit
>      dwc3_phy_exit [dwc3]
>      dwc3_core_remove [dwc3]
>      dwc3_remove [dwc3]
>      platform_remove
>      device_release_driver_internal
>      device_driver_detach
>      unbind_store
>      sysfs_kf_write
>      vfs_write
>      ksys_write
>      __arm64_sys_write
>      el0_svc
> 
> Two WARNs precede the oops from the same teardown chain, confirming
> the resource ordering:
> 
>    WARNING: drivers/clk/clk.c:4494 at clk_nodrv_disable_unprepare+0x8/0x18
>    WARNING: drivers/regulator/core.c:2657 at _regulator_put+0x84/0x98
> 
> i.e. the pipe clock provider has been unregistered and the regulators
> released before qmp_combo_usb_power_off() runs.
> 
> The proper long-term fix is a teardown-ordering rework so the QMP
> PHY's backing resources outlive any consumer that may still call its
> phy_ops. Pending that, guard the power_off/exit paths with the
> existing usb_init_count balance so re-entry after teardown does not
> oops. usb_init_count tracks the balance of usb_power_on/off; if it
> is zero we have either never powered on or have already powered off,
> and there is nothing to do.
> 
> The same guard is added to qmp_combo_usb_exit() since it is the entry
> point used by external consumers via phy_exit().
> 
> Signed-off-by: Michael Scott <mike.scott@oss.qualcomm.com>

Something like this requires a Fixes: tag

> ---
>   drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index cdcfad2e86b1..0db200292642 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -3926,6 +3926,17 @@ static int qmp_combo_usb_power_off(struct phy *phy)
>   	struct qmp_combo *qmp = phy_get_drvdata(phy);
>   	const struct qmp_phy_cfg *cfg = qmp->cfg;
>   
> +	/*
> +	 * Reachable as ->exit from external consumers (notably dwc3) after
> +	 * this device's backing resources have already been released along
> +	 * a teardown chain. Refuse to touch registers in that case.
> +	 */
> +	if (!qmp->usb_init_count) {
> +		dev_dbg(qmp->dev, "%s: PHY not powered on, skipping\n",
> +			__func__);
> +		return 0;
> +	}
> +
>   	/* PHY reset */
>   	qphy_setbits(qmp->pcs, cfg->regs[QPHY_SW_RESET], SW_RESET);
>   
> @@ -3968,6 +3979,17 @@ static int qmp_combo_usb_exit(struct phy *phy)
>   	struct qmp_combo *qmp = phy_get_drvdata(phy);
>   	int ret;
>   
> +	/*
> +	 * See qmp_combo_usb_power_off(): an external consumer may call
> +	 * phy_exit() after the QMP device's resources have been torn
> +	 * down. usb_init_count tracks usb_init/usb_exit balance.
> +	 */
> +	if (!qmp->usb_init_count) {
> +		dev_dbg(qmp->dev, "%s: PHY not initialised, skipping\n",
> +			__func__);
> +		return 0;
> +	}
> +
>   	mutex_lock(&qmp->phy_mutex);
>   	ret = qmp_combo_usb_power_off(phy);

This can't be right - you check usb_init_count before the mutex and then 
again inside the mutex @ qmp_combo_usb_power_off();

It seems like an error to even get to this function with !usb_init_count 
also check if that is a signed or an unsigned value as usb_init_count = 
-1 will evaluate true.

>   	if (ret)
> --
> 2.53.0
> 


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

^ permalink raw reply

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


[-- Attachment #1.1: Type: text/plain, Size: 6921 bytes --]

> 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
> 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 112 bytes --]

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

^ permalink raw reply

* Re: [PATCH v4 2/2] phy: rockchip: inno-hdmi: Remove deprecated way to configure TMDS rate
From: Heiko Stuebner @ 2026-05-21  9:11 UTC (permalink / raw)
  To: Vinod Koul, Neil Armstrong, Jonas Karlman
  Cc: linux-phy, linux-rockchip, linux-arm-kernel, linux-kernel,
	Jonas Karlman
In-Reply-To: <20260518180722.2480799-3-jonas@kwiboo.se>

Am Montag, 18. Mai 2026, 20:07:21 Mitteleuropäische Sommerzeit schrieb Jonas Karlman:
> The TMDS character rate of this PHY is configured using PHY bus width
> in downstream vendor kernel and out-of-tree patches, however no in-tree
> consumer of this PHY has ever called phy_set_bus_width() to change the
> TMDS character rate as currently only 8-bit RGB output is supported by
> the HDMI display driver.
> 
> The series "Split Generic PHY consumer and provider" clarifies that
> phy_set_bus_width() is intended as a provider-only function.
> 
> Remove the deprecated unused fallback way to configure TMDS character
> rate now that this HDMI PHY support using phy_configure() to configure
> the TMDS character rate.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>


Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Heiko Stuebner <heiko@sntech.de> #rk3328




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

^ permalink raw reply

* Re: [PATCH v4 1/2] phy: rockchip: inno-hdmi: Add configure() and validate() ops
From: Heiko Stuebner @ 2026-05-21  9:10 UTC (permalink / raw)
  To: Vinod Koul, Neil Armstrong, Jonas Karlman
  Cc: linux-phy, linux-rockchip, linux-arm-kernel, linux-kernel,
	Jonas Karlman
In-Reply-To: <20260518180722.2480799-2-jonas@kwiboo.se>

Am Montag, 18. Mai 2026, 20:07:20 Mitteleuropäische Sommerzeit schrieb Jonas Karlman:
> The commit 10ed34d6eaaf ("phy: Add HDMI configuration options")
> introduced a way for HDMI PHYs to be configured through the generic
> phy_configure() function.
> 
> This driver derives the TMDS character rate from the pixel clock and the
> PHY bus width setting. However, no in-tree consumer of this PHY has ever
> called phy_set_bus_width() to change the TMDS character rate as only
> 8-bit RGB output is supported by the HDMI display driver.
> 
> Add configure() and validate() ops to allow consumers to configure the
> TMDS character rate using phy_configure(). Fallback to the deprecated
> way of using the PHY bus width to configure the TMDS character rate.
> 
> A typical call chain during DRM modeset on a RK3328 device:
> 
>   dw_hdmi_rockchip_encoder_atomic_check():
>   - inno_hdmi_phy_validate(): pixclock 148500000 tmdsclock 594000000
> 
>   dw_hdmi_rockchip_encoder_atomic_mode_set():
>   - inno_hdmi_phy_configure(): pixclock 148500000
>     - inno_hdmi_phy_validate(): pixclock 148500000 tmdsclock 594000000
> 
>   vop_crtc_atomic_enable():
>   - inno_hdmi_phy_rk3328_clk_set_rate(): rate 594000000 tmdsclk 594000000
>     inno_hdmi_phy_rk3328_clk_set_rate(): pixclock 594000000 tmdsclock 594000000
>   - inno_hdmi_phy_rk3328_clk_recalc_rate(): pixclock 594000000 vco 594000000
> 
>   dw_hdmi_rockchip_encoder_enable():
>   - inno_hdmi_phy_power_on(): Inno HDMI PHY Power On
>     - inno_hdmi_phy_rk3328_clk_set_rate(): rate 594000000 tmdsclk 594000000
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Heiko Stuebner <heiko@sntech.de> #rk3328




-- 
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: rockchip: phy-rockchip-typec: Add DRM AUX bridge
From: Heiko Stuebner @ 2026-05-21  9:06 UTC (permalink / raw)
  To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Sandy Huang,
	Andy Yan, Vinod Koul, Chaoyi Chen
  Cc: Heikki Krogerus, Dmitry Baryshkov, Luca Ceresoli, linux-kernel,
	dri-devel, linux-arm-kernel, linux-rockchip, linux-phy,
	Chaoyi Chen
In-Reply-To: <20260521032854.103-4-kernel@airkyi.com>

Hi,

Am Donnerstag, 21. Mai 2026, 05:28:52 Mitteleuropäische Sommerzeit schrieb Chaoyi Chen:
> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> 
> Using the DRM_AUX_BRIDGE helper to create the transparent DRM bridge
> device.
> 
> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>


@Vinod:
could you give this patch an Ack to go through the DRM tree please?

It is independent of any other phy changes, but needs the drm-patches 1+2
from this series, so ideally would go through the drm tree together with
them.

Thanks a lot
Heiko


> ---
>  drivers/phy/rockchip/Kconfig              |  2 ++
>  drivers/phy/rockchip/phy-rockchip-typec.c | 13 +++++++++++--
>  2 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/phy/rockchip/Kconfig b/drivers/phy/rockchip/Kconfig
> index 14698571b607..9173d3b4fef4 100644
> --- a/drivers/phy/rockchip/Kconfig
> +++ b/drivers/phy/rockchip/Kconfig
> @@ -119,6 +119,8 @@ config PHY_ROCKCHIP_SNPS_PCIE3
>  config PHY_ROCKCHIP_TYPEC
>  	tristate "Rockchip TYPEC PHY Driver"
>  	depends on OF && (ARCH_ROCKCHIP || COMPILE_TEST)
> +	depends on DRM || DRM=n
> +	select DRM_AUX_BRIDGE if DRM_BRIDGE
>  	select EXTCON
>  	select GENERIC_PHY
>  	select RESET_CONTROLLER
> diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c
> index d9701b6106d5..48070b50416e 100644
> --- a/drivers/phy/rockchip/phy-rockchip-typec.c
> +++ b/drivers/phy/rockchip/phy-rockchip-typec.c
> @@ -54,6 +54,7 @@
>  
>  #include <linux/mfd/syscon.h>
>  #include <linux/phy/phy.h>
> +#include <drm/bridge/aux-bridge.h>
>  
>  #define CMN_SSM_BANDGAP			(0x21 << 2)
>  #define CMN_SSM_BIAS			(0x22 << 2)
> @@ -1162,16 +1163,24 @@ static int rockchip_typec_phy_probe(struct platform_device *pdev)
>  
>  	for_each_available_child_of_node(np, child_np) {
>  		struct phy *phy;
> +		ret = 0;
>  
> -		if (of_node_name_eq(child_np, "dp-port"))
> +		if (of_node_name_eq(child_np, "dp-port")) {
>  			phy = devm_phy_create(dev, child_np,
>  					      &rockchip_dp_phy_ops);
> -		else if (of_node_name_eq(child_np, "usb3-port"))
> +			ret = drm_aux_bridge_register_from_node(dev, child_np);
> +		} else if (of_node_name_eq(child_np, "usb3-port"))
>  			phy = devm_phy_create(dev, child_np,
>  					      &rockchip_usb3_phy_ops);
>  		else
>  			continue;
>  
> +		if (ret) {
> +			pm_runtime_disable(dev);
> +			of_node_put(child_np);
> +			return ret;
> +		}
> +
>  		if (IS_ERR(phy)) {
>  			dev_err(dev, "failed to create phy: %pOFn\n",
>  				child_np);
> 





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

^ permalink raw reply

* Re: [PATCH v8 2/5] dt-bindings: phy: Add documentation for Airoha AN7581 USB PHY
From: Christian Marangi @ 2026-05-21  8:22 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  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: <20260521-ambrosial-abiding-lyrebird-9dc86f@quoll>

On Thu, May 21, 2026 at 09:44:16AM +0200, Krzysztof Kozlowski wrote:
> On Wed, May 20, 2026 at 05:09:07PM +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.
> 
> 
> ^^^ here - required but:
>

I think I have to rephrase it. It's required if the USB3 is used/wanted. In
the other case for the USB2 phy is not needed.

I will bettet describe this in commit description and on the phys property
description.
 
> > +  phys:
> > +    items:
> > +      - description: phandle to Serdes PHY
> > +
> > +  '#phy-cells':
> > +    description: The cell contains the mode, PHY_TYPE_USB2 or PHY_TYPE_USB3,
> > +      as defined in dt-bindings/phy/phy.h.
> > +    const: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - airoha,usb2-monitor-clk-sel
> 
> 'phys' is not required? I think you need it to configure the serdes
> correctly, no?
> 
> > +  - '#phy-cells'
> > +
> > +additionalProperties: false
> 
> Best regards,
> Krzysztof
> 

-- 
	Ansuel

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

^ permalink raw reply

* Re: [PATCH v8 2/5] dt-bindings: phy: Add documentation for Airoha AN7581 USB PHY
From: Krzysztof Kozlowski @ 2026-05-21  7:44 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: <20260520150912.11614-3-ansuelsmth@gmail.com>

On Wed, May 20, 2026 at 05:09:07PM +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.


^^^ here - required but:

> +  phys:
> +    items:
> +      - description: phandle to Serdes PHY
> +
> +  '#phy-cells':
> +    description: The cell contains the mode, PHY_TYPE_USB2 or PHY_TYPE_USB3,
> +      as defined in dt-bindings/phy/phy.h.
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - airoha,usb2-monitor-clk-sel

'phys' is not required? I think you need it to configure the serdes
correctly, no?

> +  - '#phy-cells'
> +
> +additionalProperties: false

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 v8 1/5] dt-bindings: clock: airoha: Add PHY binding for Serdes port
From: Krzysztof Kozlowski @ 2026-05-21  7:41 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: <20260520150912.11614-2-ansuelsmth@gmail.com>

On Wed, May 20, 2026 at 05:09:06PM +0200, Christian Marangi wrote:
> Add PHY cell property for Serdes port selection. Currently supported only
> for Airoha AN7581 SoC, that support up to 4 Serdes port.
> 
> The Serdes port can support both PCIe, USB3 or Ethernet mode.
> 
> - PCIe1 Serdes can support PCIe or Ethernet mode.
> - PCIe2 Serdes can support PCIe or Ethernet mode.
> - USB1 Serdes can support USB3 or HSGMII mode.
> - USB2 Serdes can support USB3 or PCIe mode.
> 
> Add bindings to permit correct reference of the Serdes ports in DT.
> Values are just symbolic and enumerates the Serdes port with a specific
> number for precise reference.
> 
> The available Serdes port can be selected following the dt-binding header
> in [2].
> 
> [2] <include/dt-bindings/soc/airoha,scu-ssr.h>
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  .../devicetree/bindings/clock/airoha,en7523-scu.yaml  |  9 +++++++++
>  include/dt-bindings/soc/airoha,scu-ssr.h              | 11 +++++++++++
>  2 files changed, 20 insertions(+)
>  create mode 100644 include/dt-bindings/soc/airoha,scu-ssr.h

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: sashiko-bot @ 2026-05-21  6:22 UTC (permalink / raw)
  To: Chaoyi Chen; +Cc: vkoul, linux-phy, neil.armstrong, olteanv
In-Reply-To: <20260521032854.103-6-kernel@airkyi.com>

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.

>  	}
> +
> +	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?

> +			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.

> +				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.

> +			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.

> +
> +	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?

> +		}

[ ... ]

> @@ -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.

[ ... ]

> @@ -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.

>  	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.

> +
>  	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().

> +		}
> +
> +		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.

> +	}
> +
> +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.

> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521032854.103-1-kernel@airkyi.com?part=5

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

^ permalink raw reply

* Re: [PATCH 4/5] drm/rockchip: cdn-dp: Support handle lane info without extcon
From: sashiko-bot @ 2026-05-21  5:49 UTC (permalink / raw)
  To: Chaoyi Chen; +Cc: olteanv, linux-phy, vkoul, neil.armstrong
In-Reply-To: <20260521032854.103-5-kernel@airkyi.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The condition `PTR_ERR(extcon) != -ENODEV` in `cdn_dp_probe()` evaluates to true for valid `extcon` pointers, breaking initialization for devices with `extcon`.
--

commit 1c12cc989734f906a15b7d45d90d57d21f55c2d3
Author: Chaoyi Chen <chaoyi.chen@rock-chips.com>

drm/rockchip: cdn-dp: Support handle lane info without extcon

This commit adds support for getting PHY lane information directly from the
PHY instead of relying on extcon when a Type-C controller is present. It
updates the probe, bind, and phy enable paths to allow operation without
an extcon device, while intending to keep extcon supported for older setups.

> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> index 177e30445ee83..9068118859e21 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c

[ ... ]

> @@ -1121,14 +1130,14 @@ static int cdn_dp_probe(struct platform_device *pdev)
>  		    PTR_ERR(phy) == -EPROBE_DEFER)
>  			return -EPROBE_DEFER;
>  
> -		if (IS_ERR(extcon) || IS_ERR(phy))
> +		if (IS_ERR(phy) || PTR_ERR(extcon) != -ENODEV)

[Severity: High]
Does this condition inadvertently skip port initialization for valid extcon
devices?

When extcon_get_edev_by_phandle() successfully returns a valid pointer,
PTR_ERR(extcon) evaluates to the pointer's memory address. Since a valid
pointer address is not equal to -ENODEV, this condition will evaluate to
true and trigger the continue statement.

This appears to break enumeration for all existing devices that use an extcon
by unconditionally skipping their setup. 

Would it be better to ensure we only check the error code when the pointer
is actually an error?

		if (IS_ERR(phy) || (IS_ERR(extcon) && PTR_ERR(extcon) != -ENODEV))

>  			continue;
>  
>  		port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
>  		if (!port)
>  			return -ENOMEM;
>  
> -		port->extcon = extcon;
> +		port->extcon = IS_ERR(extcon) ? NULL : extcon;
>  		port->phy = phy;
>  		port->dp = dp;
>  		port->id = i;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521032854.103-1-kernel@airkyi.com?part=4

-- 
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: rockchip: phy-rockchip-typec: Add DRM AUX bridge
From: sashiko-bot @ 2026-05-21  5:29 UTC (permalink / raw)
  To: Chaoyi Chen; +Cc: olteanv, vkoul, neil.armstrong, linux-phy
In-Reply-To: <20260521032854.103-4-kernel@airkyi.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Double-free of the `child_np` device node reference leading to a Use-After-Free.
- [Medium] Incorrect error check ordering masks `devm_phy_create` failures and attempts bridge registration on invalid PHY pointers.
--

commit 83430d0b883e6ab3238d19fd9d398fef26aa6ab7
Author: Chaoyi Chen <chaoyi.chen@rock-chips.com>

phy: rockchip: phy-rockchip-typec: Add DRM AUX bridge

Using the DRM_AUX_BRIDGE helper to create the transparent DRM bridge
device.

> diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c
> index d9701b6106d55..48070b50416e5 100644
> --- a/drivers/phy/rockchip/phy-rockchip-typec.c
> +++ b/drivers/phy/rockchip/phy-rockchip-typec.c
[ ... ]
> @@ -1162,16 +1163,24 @@ static int rockchip_typec_phy_probe(struct platform_device *pdev)
>  
>  	for_each_available_child_of_node(np, child_np) {
>  		struct phy *phy;
> +		ret = 0;
>  
> -		if (of_node_name_eq(child_np, "dp-port"))
> +		if (of_node_name_eq(child_np, "dp-port")) {
>  			phy = devm_phy_create(dev, child_np,
>  					      &rockchip_dp_phy_ops);
> -		else if (of_node_name_eq(child_np, "usb3-port"))
> +			ret = drm_aux_bridge_register_from_node(dev, child_np);

[Severity: High]
Does this code result in a double-free of the child_np device node
reference?

The macro for_each_available_child_of_node() automatically manages the
reference count for child_np, acquiring it for the current iteration and
dropping it when advancing.

drm_aux_bridge_register_from_node() assigns the node to the auxiliary bridge
device, and the auxiliary bridge assumes ownership of a reference (calling
of_node_put() in its error paths and release callback).

Because an additional reference is not acquired via of_node_get()
before passing it, the reference is decremented twice, which can lead to a
use-after-free.

> +		} else if (of_node_name_eq(child_np, "usb3-port"))
>  			phy = devm_phy_create(dev, child_np,
>  					      &rockchip_usb3_phy_ops);
>  		else
>  			continue;
>  
> +		if (ret) {
> +			pm_runtime_disable(dev);
> +			of_node_put(child_np);
> +			return ret;
> +		}
> +
>  		if (IS_ERR(phy)) {

[Severity: Medium]
Will this error check ordering mask devm_phy_create() failures?

The check for bridge registration failure (if (ret)) happens before the
check for PHY creation failure (if (IS_ERR(phy))). If devm_phy_create()
fails (e.g., returning -EPROBE_DEFER) and the subsequent bridge registration
also fails, the function will return the bridge error and mask the PHY error.

Additionally, this attempts to register the auxiliary bridge even after the
underlying PHY creation has already failed. Should IS_ERR(phy) be checked
before calling drm_aux_bridge_register_from_node(), or at least before
checking ret?

>  			dev_err(dev, "failed to create phy: %pOFn\n",
>  				child_np);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521032854.103-1-kernel@airkyi.com?part=3

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

^ permalink raw reply

* Re: [PATCH 2/5] drm/bridge: aux: Add drm_aux_bridge_register_from_node()
From: sashiko-bot @ 2026-05-21  4:53 UTC (permalink / raw)
  To: Chaoyi Chen; +Cc: vkoul, linux-phy, olteanv, neil.armstrong
In-Reply-To: <20260521032854.103-3-kernel@airkyi.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] The `drm_aux_bridge_register_from_node()` function fails to acquire a reference to the provided `device_node` (`np`), leading to a reference count underflow and potential use-after-free during cleanup.
- [Low] The kernel-doc comment for `drm_aux_bridge_register_from_node()` retains the old function name, causing a documentation build warning.
--

commit a4484cec194d2731c92e629a32b7e17a725601d7
Author: Chaoyi Chen <chaoyi.chen@rock-chips.com>

drm/bridge: aux: Add drm_aux_bridge_register_from_node()

This patch adds drm_aux_bridge_register_from_node() to allow
specifying the of_node corresponding to the bridge.

> diff --git a/drivers/gpu/drm/bridge/aux-bridge.c b/drivers/gpu/drm/bridge/aux-bridge.c
> index 1ed21a8713bfd..f50283abed5f6 100644
> --- a/drivers/gpu/drm/bridge/aux-bridge.c
> +++ b/drivers/gpu/drm/bridge/aux-bridge.c
> @@ -35,6 +35,7 @@ static void drm_aux_bridge_unregister_adev(void *_adev)
>  /**
>   * drm_aux_bridge_register - Create a simple bridge device to link the chain

[Severity: Low]
This isn't a bug, but there's a mismatch between the kernel-doc identifier
and the new function name. Should this be updated to
drm_aux_bridge_register_from_node?

>   * @parent: device instance providing this bridge
> + * @np: device node pointer corresponding to this bridge instance

[ ... ]

> @@ -62,7 +63,10 @@ int drm_aux_bridge_register(struct device *parent)
>  	adev->dev.parent = parent;
>  	adev->dev.release = drm_aux_bridge_release;
>  
> -	device_set_of_node_from_dev(&adev->dev, parent);
> +	if (np)
> +		device_set_node(&adev->dev, of_fwnode_handle(np));

[Severity: Critical]
Will this cause a reference count underflow on the device node?

If np is provided, device_set_node() assigns the fwnode without incrementing
the device node's reference count.

However, both the auxiliary_device_init() error path (shown below) and the
device's release callback drm_aux_bridge_release() unconditionally call
of_node_put(adev->dev.of_node).

Since there is no matching of_node_get(), this could corrupt the reference
count and lead to a use-after-free. Should we call of_node_get(np) here?

> +	else
> +		device_set_of_node_from_dev(&adev->dev, parent);
>  
>  	ret = auxiliary_device_init(adev);
>  	if (ret) {
>  		of_node_put(adev->dev.of_node);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521032854.103-1-kernel@airkyi.com?part=2

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

^ permalink raw reply

* Re: [PATCH v2 0/4] phy: qcom: qmp-combo fixes + x1-dell-thena DT maintenance
From: Val Packett @ 2026-05-21  4:39 UTC (permalink / raw)
  To: Michael Scott, linux-arm-msm
  Cc: vkoul, neil.armstrong, dmitry.baryshkov, wesley.cheng, abelvesa,
	faisal.hassan, linux-phy, andersson, konradybcio, robh, krzk+dt,
	conor+dt, devicetree, bryan.odonoghue, laurentiu.tudor1,
	alex.vinarskis, linux-kernel
In-Reply-To: <20260521010935.1333494-1-mike.scott@oss.qualcomm.com>


On 5/20/26 10:09 PM, Michael Scott wrote:
> Four patches:
>
>    * Two pre-existing bug fixes in phy-qcom-qmp-combo that are reachable
>      today on any board which registers a usb_role_switch on this PHY,
>      and will become reachable on more X1E boards as their Type-C
>      support matures (patches 1-2).
> [..]

Thanks for digging into this!

I have successfully used automatic role-switching (even switching 
between USB2 gadget and USB3+DP mostly worked fine) but I do remember 
manual sysfs switching causing more issues..

Would be great to see no more dr_mode = "host" upstream. Though I 
vaguely remember some objections related to booting from USB (?)

~val


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

^ permalink raw reply

* Re: [PATCH 1/5] drm/bridge: Implement generic USB Type-C DP HPD bridge
From: sashiko-bot @ 2026-05-21  4:35 UTC (permalink / raw)
  To: Chaoyi Chen; +Cc: olteanv, vkoul, linux-phy, neil.armstrong
In-Reply-To: <20260521032854.103-2-kernel@airkyi.com>

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Module initialization misses Type-C ports that are already registered.
- [High] Missing `BUS_NOTIFY_DEL_DEVICE` handling and incorrect `devm` usage leads to memory leaks and use-after-free on module unload.
- [High] Generic notifier blindly creates duplicate bridges for controllers that already register them natively.
- [High] Blind use of `alt->dev.parent->parent` causes NULL pointer dereference in devm_ functions if the port lacks a parent.
- [Medium] Kconfig entry lacks a prompt string and has an incorrect dependency structure.
--

commit 6278338190ad225c12a2042f8bb6669eb188e362
Author: Chaoyi Chen <chaoyi.chen@rock-chips.com>

drm/bridge: Implement generic USB Type-C DP HPD bridge

This patch implements a generic DP HPD bridge based on aux-hpd-bridge.c.
It monitors Type-C bus events, and when a Type-C port device containing
the DP svid is registered, it automatically creates an HPD bridge.

> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index c3209b0f46786..d92e938757931 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -30,6 +30,16 @@ config DRM_AUX_HPD_BRIDGE
>  	  Simple bridge that terminates the bridge chain and provides HPD
>  	  support.
>  
> +if DRM_AUX_HPD_BRIDGE
> +config DRM_AUX_HPD_TYPEC_BRIDGE
> +	tristate
> +	depends on TYPEC || !TYPEC

[Severity: Medium]
Is there a missing prompt string for this tristate? Without it, the option
is invisible in menuconfig.

Also, since this module directly uses symbols like typec_bus, won't
depends on TYPEC || !TYPEC cause a build failure when CONFIG_TYPEC=n since
the dependency evaluates to true?

> +	default TYPEC
> +	help
> +	  Simple bridge that terminates the bridge chain and provides HPD
> +	  support. It build bridge on each USB-C connector device node.
> +endif

[ ... ]

> diff --git a/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c b/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c
> new file mode 100644
> index 0000000000000..d915e0fb0668c
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c
> @@ -0,0 +1,49 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +#include <linux/of.h>
> +#include <linux/usb/typec_altmode.h>
> +#include <linux/usb/typec_dp.h>
> +
> +#include <drm/bridge/aux-bridge.h>
> +
> +static int drm_typec_bus_event(struct notifier_block *nb,
> +			       unsigned long action, void *data)
> +{
> +	struct device *dev = (struct device *)data;
> +	struct typec_altmode *alt = to_typec_altmode(dev);
> +
> +	if (action != BUS_NOTIFY_ADD_DEVICE)
> +		goto done;

[Severity: High]
What happens when a port is unregistered? Since drm_dp_hpd_bridge_register()
allocates resources using devm tied to the controller device, ignoring
BUS_NOTIFY_DEL_DEVICE means the bridge won't be freed if the port altmode
is dynamically unregistered, leading to duplicate bridges and memory leaks on
re-registration.

If this module is unloaded, the devres callbacks remain attached to the
controller. Will that cause a use-after-free kernel panic when the controller
is unbound later?

> +
> +	/*
> +	 * alt->dev.parent->parent : USB-C controller device
> +	 * alt->dev.parent         : USB-C connector device
> +	 */
> +	if (is_typec_port_altmode(&alt->dev) && alt->svid == USB_TYPEC_DP_SID)
> +		drm_dp_hpd_bridge_register(alt->dev.parent->parent,
> +					   to_of_node(alt->dev.parent->fwnode));

[Severity: High]
Can alt->dev.parent be NULL here? The Type-C subsystem allows ports to be
registered without a parent. If so, dereferencing alt->dev.parent->parent
will cause a NULL pointer dereference.

[Severity: High]
Several Type-C controller drivers already register their own DP HPD bridges.
Does this unconditionally create a second bridge for those ports?

If two bridges are registered for the same device node, the controller driver
will route HPD events to its native bridge, while the DRM driver might bind
to this generic one, breaking hotplug detection.

> +
> +done:
> +	return NOTIFY_OK;
> +}
> +
> +static struct notifier_block drm_typec_event_nb = {
> +	.notifier_call = drm_typec_bus_event,
> +};
> +
> +static void drm_aux_hpd_typec_dp_bridge_module_exit(void)
> +{
> +	bus_unregister_notifier(&typec_bus, &drm_typec_event_nb);
> +}
> +
> +static int __init drm_aux_hpd_typec_dp_bridge_module_init(void)
> +{
> +	bus_register_notifier(&typec_bus, &drm_typec_event_nb);
> +
> +	return 0;
> +}

[Severity: High]
If this module is loaded after Type-C port controllers have already probed,
will it miss the existing ports?

Calling bus_register_notifier() only catches new BUS_NOTIFY_ADD_DEVICE events,
so bridges wouldn't be created for devices already on the bus.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521032854.103-1-kernel@airkyi.com?part=1

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

^ permalink raw reply

* Re: [PATCH v2 3/4] arm64: dts: qcom: x1-dell-thena: mark l12b and l15b always-on
From: Val Packett @ 2026-05-21  4:28 UTC (permalink / raw)
  To: Michael Scott, linux-arm-msm
  Cc: vkoul, neil.armstrong, dmitry.baryshkov, wesley.cheng, abelvesa,
	faisal.hassan, linux-phy, andersson, konradybcio, robh, krzk+dt,
	conor+dt, devicetree, bryan.odonoghue, laurentiu.tudor1,
	alex.vinarskis, linux-kernel, stable
In-Reply-To: <20260521010935.1333494-4-mike.scott@oss.qualcomm.com>


On 5/20/26 10:09 PM, Michael Scott wrote:
> The l12b and l15b supplies are used by components that are not (fully)
> described (and some never will be) and must never be disabled.
>
> Mark the regulators as always-on to prevent them from being disabled,
> for example, when consumers probe defer or suspend.
>
> Note that these supplies currently have no consumers described in
> mainline for dell-thena beyond the audio codec (vdd-buck/vdd-rxtx/
> vdd-io on wcd938x), which can release them when the codec goes idle.
> The board-level gpio-fixed regulators that feed the Type-C retimer's
> VDDIO and other rails are not described with a vin-supply link, so
> the kernel cannot keep their parent LDOs alive on its own.
>
> This mirrors the same change Johan Hovold applied to every other
> X1E80100 board in a March 2025 series; commit 63169c07d740
> ("arm64: dts: qcom: x1e80100-dell-xps13-9345: mark l12b and l15b always-on")
> is representative. The dell-thena board file was introduced four months
> later and did not inherit that change; this patch closes the gap.

Acked-by: Val Packett <val@packett.cool>


Asked a friend who has totally legitimate access to the XPS schematics 
about what those components are:

L12B: IO_1P2/275mA
- MOSFETs and pullups on a bunch of I2C, PCIe CLKREQ/WAKE, etc.
- VDDIO for WSA speakers
- NVME_PLN_N_1P2 (Power Loss Notification?)
- bunch of VDD_PX pins on the SoC


L15B: IO_1P8/1.09A
- MOSFETs and pullups on eDP HPD, bunch of I2C, PCIe CLKREQ/WAKE, some 
INTR#/RESET#, SSD Load Switch, etc.
- VIN for Load Switch outputting retimer's VDDIO
- VDD_1P8 for WSA speakers
- VDDIO for power monitor ICs on IR_I2C


Yeahhh.. I think the load switches could potentially be modeled but the 
huge bunch of random pullups not so much. I wonder how Windows handles 
this. Maybe it could be in low-power mode when speakers are idle?

~val



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

^ permalink raw reply

* Re: [PATCH v2 4/4] arm64: dts: qcom: x1-dell-thena: bump linux,cma to 256 MiB
From: Val Packett @ 2026-05-21  3:52 UTC (permalink / raw)
  To: Michael Scott, linux-arm-msm
  Cc: vkoul, neil.armstrong, dmitry.baryshkov, wesley.cheng, abelvesa,
	faisal.hassan, linux-phy, andersson, konradybcio, robh, krzk+dt,
	conor+dt, devicetree, bryan.odonoghue, laurentiu.tudor1,
	alex.vinarskis, linux-kernel
In-Reply-To: <20260521010935.1333494-5-mike.scott@oss.qualcomm.com>


On 5/20/26 10:09 PM, Michael Scott wrote:
> The 128 MiB linux,cma reserved-memory pool on dell-thena is too small
> to support the camera pipeline in parallel with the normal Linux
> desktop. On a freshly-booted system with GNOME running, the typical
> runtime consumers — msm DRM framebuffers (Wayland triple buffering on
> the eDP panel), qcom_iris video codec buffers, qcom_camss VFE
> pre-allocated buffers — already occupy ~100 MiB of the pool, leaving
> only ~25 MiB free.

Huh, I'm surprised that drm framebuffers use CMA… IIRC, msm drm can work 
fine without a cma node present at all.

Indeed, with a desktop on a 4K monitor I'm seeing..

CmaTotal:         131072 kB
CmaFree:            1704 kB

> The libcamera "simple" pipeline handler used by /dev/media0 on
> dell-thena allocates four ABGR8888 frames at 1920×1088 = 32 MiB total.
> That request fails on the fourth frame:
>
>      ERROR DmaBufAllocator: dma-heap allocation failure for frame-3
>      ERROR Allocator: Stream is not part of /base/.../camera@10 active configuration
>      Can't allocate buffers
>      Failed to start camera session
>
> resulting in gnome-snapshot's "Could not play camera stream" and any
> other libcamera-mediated app being unable to actually stream.

..however I couldn't reproduce any failures, Snapshot started up just 
fine, lowering CmaFree to 300 kB.

I have even launched both Snapshot and ffplay with a 4K AV1 video 
through av1_v4l2m2m, CmaFree went all the way down to zero but there 
were no errors whatsoever, both worked simultaneously just fine. o_0

I think drm buffers might just get evicted from that area or something?

> Bumping linux,cma to 256 MiB (a 0.9% reservation on these laptops'
> typical 27 GiB RAM) leaves ~150 MiB free at runtime — sufficient for
> the libcamera buffer set plus headroom for video playback or other
> CMA-hungry workloads in parallel.
>
> Tested on Dell Latitude 7455: with the 256 MiB pool, CmaFree at
> GNOME-desktop idle is ~150 MiB, gnome-snapshot streams the OV02E10
> camera cleanly, and `cam -c 1 --capture=2` succeeds.
>
> The companion board files dell-inspiron-14-plus-7441 and the upstream
> .dts variants inherit from x1-dell-thena.dtsi, so this changes the
> pool size for every dell-thena-based laptop in one place.


In any case, that's not an objection of course, just wondering why it's 
working fine for me and not for you..

Acked-by: Val Packett <val@packett.cool>

~val


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

^ permalink raw reply

* [PATCH 4/5] drm/rockchip: cdn-dp: Support handle lane info without extcon
From: Chaoyi Chen @ 2026-05-21  3:28 UTC (permalink / raw)
  To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Sandy Huang,
	Heiko Stübner, Andy Yan, Vinod Koul
  Cc: Heikki Krogerus, Dmitry Baryshkov, Luca Ceresoli, linux-kernel,
	dri-devel, linux-arm-kernel, linux-rockchip, linux-phy,
	Chaoyi Chen
In-Reply-To: <20260521032854.103-1-kernel@airkyi.com>

From: Chaoyi Chen <chaoyi.chen@rock-chips.com>

This patch add support for get PHY lane info without help of extcon.

There is no extcon needed if the Type-C controller is present. In this
case, the lane info can be get from PHY instead of extcon.

The extcon device should still be supported if Type-C controller is
not present.

Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/gpu/drm/rockchip/cdn-dp-core.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
index 177e30445ee8..9068118859e2 100644
--- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
+++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
@@ -157,6 +157,9 @@ static int cdn_dp_get_port_lanes(struct cdn_dp_port *port)
 	int dptx;
 	u8 lanes;
 
+	if (!edev)
+		return phy_get_bus_width(port->phy);
+
 	dptx = extcon_get_state(edev, EXTCON_DISP_DP);
 	if (dptx > 0) {
 		extcon_get_property(edev, EXTCON_DISP_DP,
@@ -220,7 +223,7 @@ static bool cdn_dp_check_sink_connection(struct cdn_dp_device *dp)
 	 * some docks need more time to power up.
 	 */
 	while (time_before(jiffies, timeout)) {
-		if (!extcon_get_state(port->extcon, EXTCON_DISP_DP))
+		if (port->extcon && !extcon_get_state(port->extcon, EXTCON_DISP_DP))
 			return false;
 
 		if (!cdn_dp_get_sink_count(dp, &sink_count))
@@ -386,11 +389,14 @@ static int cdn_dp_enable_phy(struct cdn_dp_device *dp, struct cdn_dp_port *port)
 		goto err_power_on;
 	}
 
-	ret = extcon_get_property(port->extcon, EXTCON_DISP_DP,
-				  EXTCON_PROP_USB_TYPEC_POLARITY, &property);
-	if (ret) {
-		DRM_DEV_ERROR(dp->dev, "get property failed\n");
-		goto err_power_on;
+	property.intval = 0;
+	if (port->extcon) {
+		ret = extcon_get_property(port->extcon, EXTCON_DISP_DP,
+					  EXTCON_PROP_USB_TYPEC_POLARITY, &property);
+		if (ret) {
+			DRM_DEV_ERROR(dp->dev, "get property failed\n");
+			goto err_power_on;
+		}
 	}
 
 	port->lanes = cdn_dp_get_port_lanes(port);
@@ -1029,6 +1035,9 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
 	for (i = 0; i < dp->ports; i++) {
 		port = dp->port[i];
 
+		if (!port->extcon)
+			continue;
+
 		port->event_nb.notifier_call = cdn_dp_pd_event;
 		ret = devm_extcon_register_notifier(dp->dev, port->extcon,
 						    EXTCON_DISP_DP,
@@ -1121,14 +1130,14 @@ static int cdn_dp_probe(struct platform_device *pdev)
 		    PTR_ERR(phy) == -EPROBE_DEFER)
 			return -EPROBE_DEFER;
 
-		if (IS_ERR(extcon) || IS_ERR(phy))
+		if (IS_ERR(phy) || PTR_ERR(extcon) != -ENODEV)
 			continue;
 
 		port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
 		if (!port)
 			return -ENOMEM;
 
-		port->extcon = extcon;
+		port->extcon = IS_ERR(extcon) ? NULL : extcon;
 		port->phy = phy;
 		port->dp = dp;
 		port->id = i;
-- 
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 3/5] phy: rockchip: phy-rockchip-typec: Add DRM AUX bridge
From: Chaoyi Chen @ 2026-05-21  3:28 UTC (permalink / raw)
  To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Sandy Huang,
	Heiko Stübner, Andy Yan, Vinod Koul
  Cc: Heikki Krogerus, Dmitry Baryshkov, Luca Ceresoli, linux-kernel,
	dri-devel, linux-arm-kernel, linux-rockchip, linux-phy,
	Chaoyi Chen
In-Reply-To: <20260521032854.103-1-kernel@airkyi.com>

From: Chaoyi Chen <chaoyi.chen@rock-chips.com>

Using the DRM_AUX_BRIDGE helper to create the transparent DRM bridge
device.

Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/phy/rockchip/Kconfig              |  2 ++
 drivers/phy/rockchip/phy-rockchip-typec.c | 13 +++++++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/rockchip/Kconfig b/drivers/phy/rockchip/Kconfig
index 14698571b607..9173d3b4fef4 100644
--- a/drivers/phy/rockchip/Kconfig
+++ b/drivers/phy/rockchip/Kconfig
@@ -119,6 +119,8 @@ config PHY_ROCKCHIP_SNPS_PCIE3
 config PHY_ROCKCHIP_TYPEC
 	tristate "Rockchip TYPEC PHY Driver"
 	depends on OF && (ARCH_ROCKCHIP || COMPILE_TEST)
+	depends on DRM || DRM=n
+	select DRM_AUX_BRIDGE if DRM_BRIDGE
 	select EXTCON
 	select GENERIC_PHY
 	select RESET_CONTROLLER
diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c
index d9701b6106d5..48070b50416e 100644
--- a/drivers/phy/rockchip/phy-rockchip-typec.c
+++ b/drivers/phy/rockchip/phy-rockchip-typec.c
@@ -54,6 +54,7 @@
 
 #include <linux/mfd/syscon.h>
 #include <linux/phy/phy.h>
+#include <drm/bridge/aux-bridge.h>
 
 #define CMN_SSM_BANDGAP			(0x21 << 2)
 #define CMN_SSM_BIAS			(0x22 << 2)
@@ -1162,16 +1163,24 @@ static int rockchip_typec_phy_probe(struct platform_device *pdev)
 
 	for_each_available_child_of_node(np, child_np) {
 		struct phy *phy;
+		ret = 0;
 
-		if (of_node_name_eq(child_np, "dp-port"))
+		if (of_node_name_eq(child_np, "dp-port")) {
 			phy = devm_phy_create(dev, child_np,
 					      &rockchip_dp_phy_ops);
-		else if (of_node_name_eq(child_np, "usb3-port"))
+			ret = drm_aux_bridge_register_from_node(dev, child_np);
+		} else if (of_node_name_eq(child_np, "usb3-port"))
 			phy = devm_phy_create(dev, child_np,
 					      &rockchip_usb3_phy_ops);
 		else
 			continue;
 
+		if (ret) {
+			pm_runtime_disable(dev);
+			of_node_put(child_np);
+			return ret;
+		}
+
 		if (IS_ERR(phy)) {
 			dev_err(dev, "failed to create phy: %pOFn\n",
 				child_np);
-- 
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 5/5] drm/rockchip: cdn-dp: Add multiple bridges to support PHY port selection
From: Chaoyi Chen @ 2026-05-21  3:28 UTC (permalink / raw)
  To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Sandy Huang,
	Heiko Stübner, Andy Yan, Vinod Koul
  Cc: Heikki Krogerus, Dmitry Baryshkov, Luca Ceresoli, linux-kernel,
	dri-devel, linux-arm-kernel, linux-rockchip, linux-phy,
	Chaoyi Chen
In-Reply-To: <20260521032854.103-1-kernel@airkyi.com>

From: Chaoyi Chen <chaoyi.chen@rock-chips.com>

The RK3399 has two USB/DP combo PHY and one CDN-DP controller. And
the CDN-DP can be switched to output to one of the PHYs. If both ports
are plugged into DP, DP will select the first port for output.

This patch adds support for multiple bridges, enabling users to flexibly
select the output port. For each PHY port, a separate encoder and bridge
are registered.

The change is based on the DRM AUX HPD bridge, rather than the
extcon approach. This requires the DT to correctly describe the
connections between the first bridge in bridge chain and DP
controller. For example, the bridge chain may be like this:

PHY aux birdge -> fsa4480 analog audio switch bridge ->
onnn,nb7vpq904m USB reminder bridge -> USB-C controller AUX HPD bridge

In this case, the connection relationships among the PHY aux bridge
and the DP contorller need to be described in DT.

In addition, the cdn_dp_parse_next_bridge_dt() will parses it and
determines whether to register one or two bridges.

Since there is only one DP controller, only one of the PHY ports can
output at a time. The key is how to switch between different PHYs,
which is handled by cdn_dp_switch_port() and cdn_dp_enable().

There are two cases:

1. Neither bridge is enabled. In this case, both bridges can
independently read the EDID, and the PHY port may switch before
reading the EDID.

2. One bridge is already enabled. In this case, other bridges are not
allowed to read the EDID. So we will try to return the cached EDID.

Since the scenario of two ports plug in at the same time is rare,
I don't have a board which support two TypeC connector to test this.
Therefore, I tested forced switching on a single PHY port, as well as
output using a fake PHY port alongside a real PHY port.

Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/gpu/drm/rockchip/Kconfig       |   1 +
 drivers/gpu/drm/rockchip/cdn-dp-core.c | 324 ++++++++++++++++++++-----
 drivers/gpu/drm/rockchip/cdn-dp-core.h |  18 +-
 3 files changed, 286 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index 1479b8c4ed40..cb97690c5a5d 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -59,6 +59,7 @@ config ROCKCHIP_CDN_DP
 	select DRM_DISPLAY_HELPER
 	select DRM_BRIDGE_CONNECTOR
 	select DRM_DISPLAY_DP_HELPER
+	select DRM_AUX_HPD_BRIDGE
 	help
 	  This selects support for Rockchip SoC specific extensions
 	  for the cdn DP driver. If you want to enable Dp on
diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
index 9068118859e2..b9ba279ca653 100644
--- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
+++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
@@ -28,16 +28,17 @@
 #include "cdn-dp-core.h"
 #include "cdn-dp-reg.h"
 
-static inline struct cdn_dp_device *bridge_to_dp(struct drm_bridge *bridge)
+static int cdn_dp_switch_port(struct cdn_dp_device *dp, struct cdn_dp_port *prev_port,
+			      struct cdn_dp_port *port);
+
+static inline struct cdn_dp_bridge *bridge_to_dp_bridge(struct drm_bridge *bridge)
 {
-	return container_of(bridge, struct cdn_dp_device, bridge);
+	return container_of(bridge, struct cdn_dp_bridge, bridge);
 }
 
-static inline struct cdn_dp_device *encoder_to_dp(struct drm_encoder *encoder)
+static inline struct cdn_dp_device *bridge_to_dp(struct drm_bridge *bridge)
 {
-	struct rockchip_encoder *rkencoder = to_rockchip_encoder(encoder);
-
-	return container_of(rkencoder, struct cdn_dp_device, encoder);
+	return bridge_to_dp_bridge(bridge)->parent;
 }
 
 #define GRF_SOC_CON9		0x6224
@@ -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;
 	}
+
+	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])
+			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])
+				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) {
+			ret = cdn_dp_switch_port(dp, prev_port, port);
+			if (ret)
+				goto get_cache;
+		}
+	}
+
 	drm_edid = drm_edid_read_custom(connector, cdn_dp_get_edid_block, dp);
+	/* replace edid cache */
+	if (dp->edid_cache[dp_bridge->id])
+		drm_edid_free(dp->edid_cache[dp_bridge->id]);
+	dp->edid_cache[dp_bridge->id] = drm_edid_dup(drm_edid);
+
 	mutex_unlock(&dp->lock);
+	return drm_edid;
 
+get_cache:
+	drm_edid = drm_edid_dup(dp->edid_cache[dp_bridge->id]);
+	mutex_unlock(&dp->lock);
 	return drm_edid;
 }
 
@@ -268,12 +315,13 @@ cdn_dp_bridge_mode_valid(struct drm_bridge *bridge,
 			 const struct drm_display_info *display_info,
 			 const struct drm_display_mode *mode)
 {
+	struct cdn_dp_bridge *dp_bridge = bridge_to_dp_bridge(bridge);
 	struct cdn_dp_device *dp = bridge_to_dp(bridge);
 	u32 requested, actual, rate, sink_max, source_max = 0;
 	u8 lanes, bpc;
 
 	/* If DP is disconnected, every mode is invalid */
-	if (!dp->connected)
+	if (!dp_bridge->connected || !dp->connected)
 		return MODE_BAD;
 
 	switch (display_info->bpc) {
@@ -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;
+
+	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;
+		}
+
+		ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_IDLE);
+		if (ret) {
+			dev_err(dp->dev, "Failed to idle video %d\n", ret);
+			goto out;
+		}
+
+		ret = cdn_dp_config_video(dp);
+		if (ret)
+			dev_err(dp->dev, "Failed to configure video: %d\n", ret);
+	}
+
+out:
+	return ret;
+}
+
 static void cdn_dp_display_info_update(struct cdn_dp_device *dp,
 				       struct drm_display_info *display_info)
 {
@@ -572,6 +668,7 @@ static void cdn_dp_display_info_update(struct cdn_dp_device *dp,
 static void cdn_dp_bridge_atomic_enable(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);
 	struct drm_connector *connector;
 	int ret, val;
 
@@ -581,7 +678,7 @@ static void cdn_dp_bridge_atomic_enable(struct drm_bridge *bridge, struct drm_at
 
 	cdn_dp_display_info_update(dp, &connector->display_info);
 
-	ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, &dp->encoder.encoder);
+	ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, &dp_bridge->encoder.encoder);
 	if (ret < 0) {
 		DRM_DEV_ERROR(dp->dev, "Could not get vop id, %d", ret);
 		return;
@@ -600,6 +697,9 @@ static void cdn_dp_bridge_atomic_enable(struct drm_bridge *bridge, struct drm_at
 
 	mutex_lock(&dp->lock);
 
+	if (dp->next_bridge_valid)
+		dp->active_port = dp_bridge->id;
+
 	ret = cdn_dp_enable(dp);
 	if (ret) {
 		DRM_DEV_ERROR(dp->dev, "Failed to enable bridge %d\n",
@@ -632,6 +732,7 @@ static void cdn_dp_bridge_atomic_enable(struct drm_bridge *bridge, struct drm_at
 		goto out;
 	}
 
+	dp_bridge->enabled = true;
 out:
 	mutex_unlock(&dp->lock);
 }
@@ -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);
@@ -828,6 +931,16 @@ static int cdn_dp_audio_mute_stream(struct drm_bridge *bridge,
 	return ret;
 }
 
+static void cdn_dp_bridge_hpd_notify(struct drm_bridge *bridge,
+			   enum drm_connector_status status)
+{
+	struct cdn_dp_bridge *dp_bridge = bridge_to_dp_bridge(bridge);
+	struct cdn_dp_device *dp = bridge_to_dp(bridge);
+
+	dp->bridge_list[dp_bridge->id]->connected = status == connector_status_connected;
+	schedule_work(&dp->event_work);
+}
+
 static const struct drm_bridge_funcs cdn_dp_bridge_funcs = {
 	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
@@ -838,6 +951,7 @@ static const struct drm_bridge_funcs cdn_dp_bridge_funcs = {
 	.atomic_disable = cdn_dp_bridge_atomic_disable,
 	.mode_valid = cdn_dp_bridge_mode_valid,
 	.mode_set = cdn_dp_bridge_mode_set,
+	.hpd_notify = cdn_dp_bridge_hpd_notify,
 
 	.dp_audio_prepare = cdn_dp_audio_prepare,
 	.dp_audio_mute_stream = cdn_dp_audio_mute_stream,
@@ -886,7 +1000,8 @@ static void cdn_dp_pd_event_work(struct work_struct *work)
 {
 	struct cdn_dp_device *dp = container_of(work, struct cdn_dp_device,
 						event_work);
-	int ret;
+	bool connected;
+	int i, ret;
 
 	mutex_lock(&dp->lock);
 
@@ -945,9 +1060,12 @@ static void cdn_dp_pd_event_work(struct work_struct *work)
 
 out:
 	mutex_unlock(&dp->lock);
-	drm_bridge_hpd_notify(&dp->bridge,
-			      dp->connected ? connector_status_connected
-					    : connector_status_disconnected);
+	for (i = 0; i < dp->bridge_count; i++) {
+		connected = dp->connected && dp->bridge_list[i]->connected;
+		drm_bridge_hpd_notify(&dp->bridge_list[i]->bridge,
+				      connected ? connector_status_connected
+						: connector_status_disconnected);
+	}
 }
 
 static int cdn_dp_pd_event(struct notifier_block *nb,
@@ -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;
 	struct drm_connector *connector;
-	struct cdn_dp_port *port;
-	struct drm_device *drm_dev = data;
-	int ret, i;
-
-	ret = cdn_dp_parse_dt(dp);
-	if (ret < 0)
-		return ret;
-
-	dp->drm_dev = drm_dev;
-	dp->connected = false;
-	dp->active = false;
-	dp->active_port = -1;
-	dp->fw_loaded = false;
-
-	INIT_WORK(&dp->event_work, cdn_dp_pd_event_work);
-
-	encoder = &dp->encoder.encoder;
+	int ret;
 
 	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm_dev,
 							     dev->of_node);
@@ -1003,26 +1109,35 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
 
 	drm_encoder_helper_add(encoder, &cdn_dp_encoder_helper_funcs);
 
-	dp->bridge.ops =
-			DRM_BRIDGE_OP_DETECT |
-			DRM_BRIDGE_OP_EDID |
-			DRM_BRIDGE_OP_HPD |
-			DRM_BRIDGE_OP_DP_AUDIO;
-	dp->bridge.of_node = dp->dev->of_node;
-	dp->bridge.type = DRM_MODE_CONNECTOR_DisplayPort;
-	dp->bridge.hdmi_audio_dev = dp->dev;
-	dp->bridge.hdmi_audio_max_i2s_playback_channels = 8;
-	dp->bridge.hdmi_audio_spdif_playback = 1;
-	dp->bridge.hdmi_audio_dai_port = -1;
-
-	ret = devm_drm_bridge_add(dev, &dp->bridge);
+	bridge->ops =
+		DRM_BRIDGE_OP_DETECT |
+		DRM_BRIDGE_OP_EDID |
+		DRM_BRIDGE_OP_HPD |
+		DRM_BRIDGE_OP_DP_AUDIO;
+	bridge->of_node = dp->dev->of_node;
+	bridge->type = DRM_MODE_CONNECTOR_DisplayPort;
+	bridge->hdmi_audio_dev = dp->dev;
+	bridge->hdmi_audio_max_i2s_playback_channels = 8;
+	bridge->hdmi_audio_spdif_playback = 1;
+	bridge->hdmi_audio_dai_port = -1;
+
+	ret = devm_drm_bridge_add(dev, bridge);
 	if (ret)
 		return ret;
 
-	ret = drm_bridge_attach(encoder, &dp->bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+	ret = drm_bridge_attach(encoder, bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
 	if (ret)
 		return ret;
 
+	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));
+
 	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;
+		}
+
+		dp->next_bridge_valid = true;
+		dp->next_bridge_list[count] = drm_bridge_get(bridge);
+		count++;
+	}
+
+out:
+	dp->bridge_count = count ? count : 1;
+	return ret;
+}
+
+static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
+{
+	struct cdn_dp_device *dp = dev_get_drvdata(dev);
+	struct drm_bridge *bridge, *next_bridge;
+	struct drm_encoder *encoder;
+	struct cdn_dp_port *port;
+	struct drm_device *drm_dev = data;
+	struct cdn_dp_bridge *dp_bridge;
+	int ret, i;
+
+	ret = cdn_dp_parse_dt(dp);
+	if (ret < 0)
+		return ret;
+
+	ret = cdn_dp_parse_next_bridge_dt(dp);
+	if (ret)
+		return ret;
+
+	dp->drm_dev = drm_dev;
+	dp->connected = false;
+	dp->active = false;
+	dp->active_port = -1;
+	dp->fw_loaded = false;
+
+	for (i = 0; i < dp->bridge_count; i++) {
+		dp_bridge = devm_drm_bridge_alloc(dev, struct cdn_dp_bridge, bridge,
+						    &cdn_dp_bridge_funcs);
+		if (IS_ERR(dp_bridge))
+			return PTR_ERR(dp_bridge);
+		dp_bridge->id = i;
+		dp_bridge->parent = dp;
+		if (!dp->next_bridge_valid)
+			dp_bridge->connected = true;
+		dp->bridge_list[i] = dp_bridge;
+	}
+
+	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;
+	}
+
+	INIT_WORK(&dp->event_work, cdn_dp_pd_event_work);
+
 	for (i = 0; i < dp->ports; i++) {
 		port = dp->port[i];
 
@@ -1059,10 +1265,18 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
 static void cdn_dp_unbind(struct device *dev, struct device *master, void *data)
 {
 	struct cdn_dp_device *dp = dev_get_drvdata(dev);
-	struct drm_encoder *encoder = &dp->encoder.encoder;
+	struct drm_encoder *encoder;
+	int i;
 
 	cancel_work_sync(&dp->event_work);
-	encoder->funcs->destroy(encoder);
+	for (i = 0; i < dp->bridge_count; i++) {
+		encoder = &dp->bridge_list[i]->encoder.encoder;
+		encoder->funcs->destroy(encoder);
+		drm_bridge_put(dp->next_bridge_list[i]);
+	}
+
+	for (i = 0; i < MAX_PHY; i++)
+		drm_edid_free(dp->edid_cache[i]);
 
 	pm_runtime_disable(dev);
 	if (dp->fw_loaded)
@@ -1113,10 +1327,10 @@ static int cdn_dp_probe(struct platform_device *pdev)
 	int ret;
 	int i;
 
-	dp = devm_drm_bridge_alloc(dev, struct cdn_dp_device, bridge,
-				   &cdn_dp_bridge_funcs);
-	if (IS_ERR(dp))
-		return PTR_ERR(dp);
+	dp = devm_kzalloc(dev, sizeof(*dp), GFP_KERNEL);
+	if (!dp)
+		return -ENOMEM;
+
 	dp->dev = dev;
 
 	match = of_match_node(cdn_dp_dt_ids, pdev->dev.of_node);
diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.h b/drivers/gpu/drm/rockchip/cdn-dp-core.h
index e9c30b9fd543..c10e423bbf06 100644
--- a/drivers/gpu/drm/rockchip/cdn-dp-core.h
+++ b/drivers/gpu/drm/rockchip/cdn-dp-core.h
@@ -38,6 +38,8 @@ enum vic_pxl_encoding_format {
 	Y_ONLY = 0x10,
 };
 
+struct cdn_dp_device;
+
 struct video_info {
 	bool h_sync_polarity;
 	bool v_sync_polarity;
@@ -63,16 +65,28 @@ struct cdn_dp_port {
 	u8 id;
 };
 
+struct cdn_dp_bridge {
+	struct cdn_dp_device *parent;
+	struct drm_bridge bridge;
+	struct rockchip_encoder encoder;
+	bool connected;
+	bool enabled;
+	int id;
+};
+
 struct cdn_dp_device {
 	struct device *dev;
 	struct drm_device *drm_dev;
-	struct drm_bridge bridge;
-	struct rockchip_encoder encoder;
+	int bridge_count;
+	struct cdn_dp_bridge *bridge_list[MAX_PHY];
+	struct drm_bridge *next_bridge_list[MAX_PHY];
+	const struct drm_edid *edid_cache[MAX_PHY];
 	struct drm_display_mode mode;
 	struct platform_device *audio_pdev;
 	struct work_struct event_work;
 
 	struct mutex lock;
+	bool next_bridge_valid;
 	bool connected;
 	bool active;
 	bool suspended;
-- 
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 1/5] drm/bridge: Implement generic USB Type-C DP HPD bridge
From: Chaoyi Chen @ 2026-05-21  3:28 UTC (permalink / raw)
  To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Sandy Huang,
	Heiko Stübner, Andy Yan, Vinod Koul
  Cc: Heikki Krogerus, Dmitry Baryshkov, Luca Ceresoli, linux-kernel,
	dri-devel, linux-arm-kernel, linux-rockchip, linux-phy,
	Chaoyi Chen
In-Reply-To: <20260521032854.103-1-kernel@airkyi.com>

From: Chaoyi Chen <chaoyi.chen@rock-chips.com>

The HPD function of Type-C DP is implemented through
drm_connector_oob_hotplug_event(). For embedded DP, it is required
that the DRM connector fwnode corresponds to the Type-C port fwnode.

To describe the relationship between the DP controller and the Type-C
port device, we usually using drm_bridge to build a bridge chain.

Now several USB-C controller drivers have already implemented the DP
HPD bridge function provided by aux-hpd-bridge.c, it will build a DP
HPD bridge on USB-C connector port device.

But this requires the USB-C controller driver to manually register the
HPD bridge. If the driver does not implement this feature, the bridge
will not be create.

So this patch implements a generic DP HPD bridge based on
aux-hpd-bridge.c. It will monitor Type-C bus events, and when a
Type-C port device containing the DP svid is registered, it will
create an HPD bridge for it without the need for the USB-C controller
driver to implement it.

Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/gpu/drm/bridge/Kconfig                | 10 ++++
 drivers/gpu/drm/bridge/Makefile               |  1 +
 .../gpu/drm/bridge/aux-hpd-typec-dp-bridge.c  | 49 +++++++++++++++++++
 3 files changed, 60 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index c3209b0f4678..d92e93875793 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -30,6 +30,16 @@ config DRM_AUX_HPD_BRIDGE
 	  Simple bridge that terminates the bridge chain and provides HPD
 	  support.
 
+if DRM_AUX_HPD_BRIDGE
+config DRM_AUX_HPD_TYPEC_BRIDGE
+	tristate
+	depends on TYPEC || !TYPEC
+	default TYPEC
+	help
+	  Simple bridge that terminates the bridge chain and provides HPD
+	  support. It build bridge on each USB-C connector device node.
+endif
+
 menu "Display Interface Bridges"
 	depends on DRM && DRM_BRIDGE
 
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index beab5b695a6e..c4761526ba0a 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_DRM_AUX_BRIDGE) += aux-bridge.o
 obj-$(CONFIG_DRM_AUX_HPD_BRIDGE) += aux-hpd-bridge.o
+obj-$(CONFIG_DRM_AUX_HPD_TYPEC_BRIDGE) += aux-hpd-typec-dp-bridge.o
 obj-$(CONFIG_DRM_CHIPONE_ICN6211) += chipone-icn6211.o
 obj-$(CONFIG_DRM_CHRONTEL_CH7033) += chrontel-ch7033.o
 obj-$(CONFIG_DRM_CROS_EC_ANX7688) += cros-ec-anx7688.o
diff --git a/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c b/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c
new file mode 100644
index 000000000000..d915e0fb0668
--- /dev/null
+++ b/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0+
+#include <linux/of.h>
+#include <linux/usb/typec_altmode.h>
+#include <linux/usb/typec_dp.h>
+
+#include <drm/bridge/aux-bridge.h>
+
+static int drm_typec_bus_event(struct notifier_block *nb,
+			       unsigned long action, void *data)
+{
+	struct device *dev = (struct device *)data;
+	struct typec_altmode *alt = to_typec_altmode(dev);
+
+	if (action != BUS_NOTIFY_ADD_DEVICE)
+		goto done;
+
+	/*
+	 * alt->dev.parent->parent : USB-C controller device
+	 * alt->dev.parent         : USB-C connector device
+	 */
+	if (is_typec_port_altmode(&alt->dev) && alt->svid == USB_TYPEC_DP_SID)
+		drm_dp_hpd_bridge_register(alt->dev.parent->parent,
+					   to_of_node(alt->dev.parent->fwnode));
+
+done:
+	return NOTIFY_OK;
+}
+
+static struct notifier_block drm_typec_event_nb = {
+	.notifier_call = drm_typec_bus_event,
+};
+
+static void drm_aux_hpd_typec_dp_bridge_module_exit(void)
+{
+	bus_unregister_notifier(&typec_bus, &drm_typec_event_nb);
+}
+
+static int __init drm_aux_hpd_typec_dp_bridge_module_init(void)
+{
+	bus_register_notifier(&typec_bus, &drm_typec_event_nb);
+
+	return 0;
+}
+
+module_init(drm_aux_hpd_typec_dp_bridge_module_init);
+module_exit(drm_aux_hpd_typec_dp_bridge_module_exit);
+
+MODULE_DESCRIPTION("DRM TYPEC DP HPD BRIDGE");
+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 2/5] drm/bridge: aux: Add drm_aux_bridge_register_from_node()
From: Chaoyi Chen @ 2026-05-21  3:28 UTC (permalink / raw)
  To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Sandy Huang,
	Heiko Stübner, Andy Yan, Vinod Koul
  Cc: Heikki Krogerus, Dmitry Baryshkov, Luca Ceresoli, linux-kernel,
	dri-devel, linux-arm-kernel, linux-rockchip, linux-phy,
	Chaoyi Chen
In-Reply-To: <20260521032854.103-1-kernel@airkyi.com>

From: Chaoyi Chen <chaoyi.chen@rock-chips.com>

The drm_aux_bridge_register() uses the device->of_node as the
bridge->of_node.

This patch adds drm_aux_bridge_register_from_node() to allow
specifying the of_node corresponding to the bridge.

Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/gpu/drm/bridge/aux-bridge.c | 24 ++++++++++++++++++++++--
 include/drm/bridge/aux-bridge.h     |  6 ++++++
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/aux-bridge.c b/drivers/gpu/drm/bridge/aux-bridge.c
index 1ed21a8713bf..f50283abed5f 100644
--- a/drivers/gpu/drm/bridge/aux-bridge.c
+++ b/drivers/gpu/drm/bridge/aux-bridge.c
@@ -35,6 +35,7 @@ static void drm_aux_bridge_unregister_adev(void *_adev)
 /**
  * drm_aux_bridge_register - Create a simple bridge device to link the chain
  * @parent: device instance providing this bridge
+ * @np: device node pointer corresponding to this bridge instance
  *
  * Creates a simple DRM bridge that doesn't implement any drm_bridge
  * operations. Such bridges merely fill a place in the bridge chain linking
@@ -42,7 +43,7 @@ static void drm_aux_bridge_unregister_adev(void *_adev)
  *
  * Return: zero on success, negative error code on failure
  */
-int drm_aux_bridge_register(struct device *parent)
+int drm_aux_bridge_register_from_node(struct device *parent, struct device_node *np)
 {
 	struct auxiliary_device *adev;
 	int ret;
@@ -62,7 +63,10 @@ int drm_aux_bridge_register(struct device *parent)
 	adev->dev.parent = parent;
 	adev->dev.release = drm_aux_bridge_release;
 
-	device_set_of_node_from_dev(&adev->dev, parent);
+	if (np)
+		device_set_node(&adev->dev, of_fwnode_handle(np));
+	else
+		device_set_of_node_from_dev(&adev->dev, parent);
 
 	ret = auxiliary_device_init(adev);
 	if (ret) {
@@ -80,6 +84,22 @@ int drm_aux_bridge_register(struct device *parent)
 
 	return devm_add_action_or_reset(parent, drm_aux_bridge_unregister_adev, adev);
 }
+EXPORT_SYMBOL_GPL(drm_aux_bridge_register_from_node);
+
+/**
+ * drm_aux_bridge_register - Create a simple bridge device to link the chain
+ * @parent: device instance providing this bridge
+ *
+ * Creates a simple DRM bridge that doesn't implement any drm_bridge
+ * operations. Such bridges merely fill a place in the bridge chain linking
+ * surrounding DRM bridges.
+ *
+ * Return: zero on success, negative error code on failure
+ */
+int drm_aux_bridge_register(struct device *parent)
+{
+	return drm_aux_bridge_register_from_node(parent, NULL);
+}
 EXPORT_SYMBOL_GPL(drm_aux_bridge_register);
 
 struct drm_aux_bridge_data {
diff --git a/include/drm/bridge/aux-bridge.h b/include/drm/bridge/aux-bridge.h
index c2f5a855512f..7dd1f17a1354 100644
--- a/include/drm/bridge/aux-bridge.h
+++ b/include/drm/bridge/aux-bridge.h
@@ -13,11 +13,17 @@ struct auxiliary_device;
 
 #if IS_ENABLED(CONFIG_DRM_AUX_BRIDGE)
 int drm_aux_bridge_register(struct device *parent);
+int drm_aux_bridge_register_from_node(struct device *parent, struct device_node *np);
 #else
 static inline int drm_aux_bridge_register(struct device *parent)
 {
 	return 0;
 }
+
+static inline int drm_aux_bridge_register_from_node(struct device *parent, struct device_node *np)
+{
+	return 0;
+}
 #endif
 
 #if IS_ENABLED(CONFIG_DRM_AUX_HPD_BRIDGE)
-- 
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 0/5] drm/bridge: Implement generic USB Type-C DP HPD bridge
From: Chaoyi Chen @ 2026-05-21  3:28 UTC (permalink / raw)
  To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Sandy Huang,
	Heiko Stübner, Andy Yan, Vinod Koul
  Cc: Heikki Krogerus, Dmitry Baryshkov, Luca Ceresoli, linux-kernel,
	dri-devel, linux-arm-kernel, linux-rockchip, linux-phy,
	Chaoyi Chen

From: Chaoyi Chen <chaoyi.chen@rock-chips.com>

This series is split from the v15 "Add Type-C DP support for RK3399 EVB
IND board" series [1]. It focuses on the DRM bridge and Rockchip
platform CDN-DP controller changes.

[1] https://lore.kernel.org/all/20260304094152.92-1-kernel@airkyi.com/

====
1. Generic Type-C DP HPD bridge

Currently, several USB-C controller drivers register their own DP HPD
bridge via aux-hpd-bridge.c, each duplicating the same logic. For
devicetree based platforms, the USB-C controller may vary across boards,
and not every USB-C controller driver implements this feature. Patch 1
implements a generic DP HPD bridge that monitors Type-C bus events and
automatically creates an HPD bridge when a Type-C port device with DP
SVID is registered.

2. Multiple bridge model for CDN-DP

The RK3399 has two USB/DP combo PHY and one CDN-DP controller. Patch 5
introduces a multi-bridge model where each PHY port gets a separate
encoder and bridge, allowing flexible selection of the output PHY port.
This is based on the DRM AUX HPD bridge rather than extcon.

====
Patch 1 adds generic USB Type-C DP HPD bridge (Dmitry, Heikki).
Patch 2 adds new API drm_aux_bridge_register_from_node() (Neil).
Patch 3 adds DRM AUX bridge support for RK3399 USBDP PHY (Neil).
Patch 4 drops CDN-DP's extcon dependency when Type-C is present (Dmitry).
Patch 5 adds multiple bridges to support PHY port selection (Dmitry, Luca).

Chaoyi Chen (5):
  drm/bridge: Implement generic USB Type-C DP HPD bridge
  drm/bridge: aux: Add drm_aux_bridge_register_from_node()
  phy: rockchip: phy-rockchip-typec: Add DRM AUX bridge
  drm/rockchip: cdn-dp: Support handle lane info without extcon
  drm/rockchip: cdn-dp: Add multiple bridges to support PHY port
    selection

 drivers/gpu/drm/bridge/Kconfig                |  10 +
 drivers/gpu/drm/bridge/Makefile               |   1 +
 drivers/gpu/drm/bridge/aux-bridge.c           |  24 +-
 .../gpu/drm/bridge/aux-hpd-typec-dp-bridge.c  |  49 +++
 drivers/gpu/drm/rockchip/Kconfig              |   1 +
 drivers/gpu/drm/rockchip/cdn-dp-core.c        | 349 ++++++++++++++----
 drivers/gpu/drm/rockchip/cdn-dp-core.h        |  18 +-
 drivers/phy/rockchip/Kconfig                  |   2 +
 drivers/phy/rockchip/phy-rockchip-typec.c     |  13 +-
 include/drm/bridge/aux-bridge.h               |   6 +
 10 files changed, 404 insertions(+), 69 deletions(-)
 create mode 100644 drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c

-- 
2.53.0


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

^ permalink raw reply


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