* 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
* Re: [PATCH net-next v5] net: phy: sfp: probe for RollBall I2C-to-MDIO bridge in mdio-i2c
From: Petr Wozniak @ 2026-05-21 3:12 UTC (permalink / raw)
To: jan; +Cc: netdev, linux-phy, maxime.chevallier, bjorn, andrew, kuba
In-Reply-To: <20260519162007.12345-1-petr.wozniak@gmail.com>
On 2026-05-20 at 23:19, Jan wrote:
> The patch seems to be malformed. I had to do some manual changes to get
> it applied.
Thanks for testing and for taking the time to apply it manually. v6 has
been sent just now with the formatting fixed.
> Using this patch, the module works right away, without waiting several
> minutes for the PHY probing retry loop to finish.
Great to hear it works on your ATS SFP-10G-T module as well.
> I also have a 2.5G module (XikeStor SKT-2.5G-100M). It supports
> Rollball, but does not respond to commands immediately. With this patch,
> it only works with a small delay in "module_t_wait".
v6 replaces the fixed 70 ms sleep with a 10 x 20 ms polling loop
(200 ms total), matching the tolerance used by the existing
i2c_rollball_mii_poll(). This should give slow RollBall bridges a
better chance to assert CMD_DONE in time.
The XikeStor module is not yet in the quirk table so it won't be
affected by this patch. When support for it is added later, an
appropriate module_t_wait can be set in the quirk entry if needed.
> I am a bit confused by this change. This is the same model name which
> is used by my non-Rollball ATS module when it is linked at 1G on the
> copper side. Are there other modules actually supporting Rollball which
> also use the model name including "-I"?
The "OEM"/"SFP-10G-T-I" quirk uses sfp_fixup_rollball (not
sfp_fixup_rollball_cc), which routes the module through the new probe
path rather than forcing MDIO_I2C_ROLLBALL unconditionally. If the
module has no RollBall bridge -- as is the case for your ATS module --
i2c_mii_probe_rollball() returns -ENODEV and sfp_sm_add_mdio_bus()
transitions mdio_protocol back to MDIO_I2C_NONE, so PHY probing is
skipped entirely. Non-bridge modules are handled correctly regardless
of whether they respond at 1G or 10G.
Thanks again for the detailed feedback.
Petr
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH net-next v6] net: phy: sfp: probe for RollBall I2C-to-MDIO bridge in mdio-i2c
From: Petr Wozniak @ 2026-05-21 2:57 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, andrew, hkallweit1, linux, davem, edumazet, kuba,
pabeni, maxime.chevallier, bjorn, linux-phy, jan
The "OEM"/"SFP-10G-T" quirk entry in sfp_fixup_rollball_cc()
unconditionally forces MDIO_I2C_ROLLBALL for all modules matching that
vendor/part-number combination. This works for modules that genuinely
implement a RollBall I2C-to-MDIO bridge, but silently breaks modules
that share the same EEPROM strings without having such a bridge.
The Realtek RTL8261BE-CG is one such module: a pure copper 10G SFP+
media converter with no I2C-to-MDIO bridge. Its EEPROM reports
vendor="OEM", part="SFP-10G-T-I", and -- critically -- Vendor OUI
00:00:00, making OUI-based differentiation impossible. With
MDIO_I2C_ROLLBALL forced, the module silently ACKs the unlock password
write, the MDIO bus is created, but no PHY responds; the SFP state
machine cycles through the RollBall PHY-probe retry window before
reporting no PHY.
Move the probe into i2c_mii_init_rollball() in mdio-i2c.c, where the
RollBall protocol constants are already defined. After sending the
unlock password, issue a CMD_READ and poll for CMD_DONE up to 200 ms
(10 x 20 ms, matching the existing rollball poll tolerance). A genuine
RollBall bridge asserts CMD_DONE within that window; modules without a
bridge never do, so i2c_mii_init_rollball() returns -ENODEV.
mdio_i2c_alloc() propagates -ENODEV to the caller to signal that no
bridge is present and PHY probing should be skipped.
sfp_sm_add_mdio_bus() catches -ENODEV and transitions
sfp->mdio_protocol to MDIO_I2C_NONE so the rest of the state machine
skips PHY probing for this module.
Any I2C-level error (NACK, timeout) during the probe is also treated as
-ENODEV: if the module does not respond at I2C address 0x51 at all,
there is certainly no RollBall bridge there, and SFP initialization
should not abort.
The probe writes are safe with respect to SFP EEPROM integrity: only
modules explicitly listed in the quirk table enter this path, and the
RollBall password unlock write to 0x51 was already issued by
i2c_mii_init_rollball() before the probe for all such modules. Any
module without a device at 0x51 NACKs the transfer and is treated as
-ENODEV.
Add "OEM"/"SFP-10G-T-I" to the quirk table so RTL8261BE modules enter
the probe path; genuine RollBall modules continue to work as before.
Signed-off-by: Petr Wozniak <petr.wozniak@gmail.com>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
Changes since v5 (Sashiko AI review):
- Treat I2C NACK/errors in i2c_mii_init_rollball() as -ENODEV so
modules without a 0x51 EEPROM do not abort SFP initialization
- Replace fixed 70 ms wait with 10 x 20 ms poll (total 200 ms),
matching the existing i2c_rollball_mii_poll() tolerance and
preventing false -ENODEV on slow RollBall bridges
Changes since v4 (feedback from Maxime Chevallier):
- Fix commit message: replace "stalls" with accurate description of
the RollBall PHY-probe retry window
- Fix variable declaration order in i2c_mii_probe_rollball() to
follow reverse-xmas tree (descending line length)
- Remove spurious alignment space on "SFP-10G-T" quirk entry
- Document that -ENODEV from mdio_i2c_alloc() means no bridge present,
PHY probing should be skipped
Changes since v3 (feedback from Jakub Kicinski):
- Drop spurious Tested-by: tag -- author and tester are the same person
- Use PATCH net-next subject prefix
- Move -ENODEV handling from sfp_i2c_mdiobus_create() into
sfp_sm_add_mdio_bus() so bus-creation code does not mutate
sfp->mdio_protocol; the state machine is the correct place for
protocol-state transitions
- Split combined variable declaration for clarity
Changes since v2:
- Compile-tested and hardware-tested on BPI-R4 (MT7988A, 6.12.87)
- RTL8261BE (OEM/SFP-10G-T-I): probes MDIO_I2C_NONE, link Up 10Gbps
- Genuine RollBall (OEM/SFP-10G-T): bridge detected, link Up 10Gbps
drivers/net/mdio/mdio-i2c.c | 65 ++++++++++++++++++++++++++++++------
drivers/net/phy/sfp.c | 16 ++++++++--
2 files changed, 68 insertions(+), 13 deletions(-)
--- a/drivers/net/mdio/mdio-i2c.c
+++ b/drivers/net/mdio/mdio-i2c.c
@@ -352,6 +352,54 @@
return 0;
}
+static int i2c_mii_probe_rollball(struct i2c_adapter *i2c)
+{
+ u8 data_buf[] = { ROLLBALL_DATA_ADDR, 0x01, 0x00, 0x00 };
+ u8 cmd_buf[] = { ROLLBALL_CMD_ADDR, ROLLBALL_CMD_READ };
+ u8 cmd_addr = ROLLBALL_CMD_ADDR;
+ struct i2c_msg msgs[2];
+ u8 result;
+ int ret;
+ int i;
+
+ msgs[0].addr = ROLLBALL_PHY_I2C_ADDR;
+ msgs[0].flags = 0;
+ msgs[0].len = sizeof(data_buf);
+ msgs[0].buf = data_buf;
+ msgs[1].addr = ROLLBALL_PHY_I2C_ADDR;
+ msgs[1].flags = 0;
+ msgs[1].len = sizeof(cmd_buf);
+ msgs[1].buf = cmd_buf;
+
+ ret = i2c_transfer_rollball(i2c, msgs, ARRAY_SIZE(msgs));
+ if (ret < 0)
+ return -ENODEV;
+ if (ret)
+ return ret;
+
+ msgs[0].addr = ROLLBALL_PHY_I2C_ADDR;
+ msgs[0].flags = 0;
+ msgs[0].len = 1;
+ msgs[0].buf = &cmd_addr;
+ msgs[1].addr = ROLLBALL_PHY_I2C_ADDR;
+ msgs[1].flags = I2C_M_RD;
+ msgs[1].len = 1;
+ msgs[1].buf = &result;
+
+ for (i = 0; i < 10; i++) {
+ msleep(20);
+ ret = i2c_transfer_rollball(i2c, msgs, ARRAY_SIZE(msgs));
+ if (ret < 0)
+ return -ENODEV;
+ if (ret)
+ return ret;
+ if (result == ROLLBALL_CMD_DONE)
+ return 0;
+ }
+
+ return -ENODEV;
+}
+
static int i2c_mii_init_rollball(struct i2c_adapter *i2c)
{
struct i2c_msg msg;
@@ -371,11 +419,11 @@
ret = i2c_transfer(i2c, &msg, 1);
if (ret < 0)
- return ret;
- else if (ret != 1)
+ return -ENODEV;
+ if (ret != 1)
return -EIO;
- else
- return 0;
+
+ return i2c_mii_probe_rollball(i2c);
}
struct mii_bus *mdio_i2c_alloc(struct device *parent, struct i2c_adapter *i2c,
@@ -399,9 +447,12 @@
case MDIO_I2C_ROLLBALL:
ret = i2c_mii_init_rollball(i2c);
if (ret < 0) {
- dev_err(parent,
- "Cannot initialize RollBall MDIO I2C protocol: %d\n",
- ret);
+ if (ret != -ENODEV)
+ dev_err(parent,
+ "Cannot initialize RollBall MDIO I2C protocol: %d\n",
+ ret);
+ /* -ENODEV propagates to caller: no bridge present,
+ * PHY probing should be skipped for this module. */
mdiobus_free(mii);
return ERR_PTR(ret);
}
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -607,6 +607,7 @@
SFP_QUIRK_S("TP-LINK", "TL-SM410U", sfp_quirk_oem_2_5g),
SFP_QUIRK_F("ETU", "ESP-T5-R", sfp_fixup_rollball_cc),
+ SFP_QUIRK_F("OEM", "SFP-10G-T-I", sfp_fixup_rollball),
SFP_QUIRK_F("OEM", "SFP-10G-T", sfp_fixup_rollball_cc),
SFP_QUIRK_S("OEM", "SFP-2.5G-T", sfp_quirk_oem_2_5g),
SFP_QUIRK_S("OEM", "SFP-2.5G-BX10-D", sfp_quirk_2500basex),
@@ -2029,10 +2030,18 @@
dev_info(sfp->dev, "probing phy device through the [%s] protocol\n",
mdio_i2c_proto_type(sfp->mdio_protocol));
- if (sfp->mdio_protocol != MDIO_I2C_NONE)
- return sfp_i2c_mdiobus_create(sfp);
+ int ret;
- return 0;
+ if (sfp->mdio_protocol == MDIO_I2C_NONE)
+ return 0;
+
+ ret = sfp_i2c_mdiobus_create(sfp);
+ if (ret == -ENODEV) {
+ /* Probe confirmed no bridge present; skip PHY discovery. */
+ sfp->mdio_protocol = MDIO_I2C_NONE;
+ return 0;
+ }
+ return ret;
}
/* Probe a SFP for a PHY device if the module supports copper - the PHY
--
2.51.0
--
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: sashiko-bot @ 2026-05-21 2:37 UTC (permalink / raw)
To: Michael Scott
Cc: devicetree, robh, conor+dt, vkoul, neil.armstrong, olteanv,
linux-phy
In-Reply-To: <20260521010935.1333494-3-mike.scott@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] The patch fails to resolve the unprogrammed hardware state because the reprogramming sequence is still completely bypassed when `init_count == 0`.
Pre-existing issues:
- [High] The error handling path in `qmp_combo_com_init()` incorrectly decrements `init_count` when called with `force == true`.
- [High] Type-C MUX set operations modify `usb_init_count` independently of `init_count`, leading to resource leaks.
- [High] Lockless access to `qmp->usb_init_count` in `qmp_combo_usb_exit()` causes a Time-Of-Check to Time-Of-Use race condition.
--
commit c4bd0ad144568b18cf6ccfdc50c09f8d088db9c4
Author: Michael Scott <mike.scott@oss.qualcomm.com>
phy: qcom: qmp-combo: track whether the cached typec_mux mode was committed to hardware
This commit introduces a qmpphy_mode_committed flag to track whether the
cached typec_mux mode has been applied to hardware. It aims to prevent an
early bail-out in qmp_combo_typec_mux_set() when the PHY has not yet been
initialized and the hardware state is out of sync with the cache.
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 0db200292642e..e28bc1cc7a78b 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
[ ... ]
> @@ -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:
> clk_bulk_disable_unprepare(qmp->num_clks, qmp->clks);
> err_assert_reset:
> reset_control_bulk_assert(cfg->num_resets, qmp->resets);
> err_disable_regulators:
> regulator_bulk_disable(cfg->num_vregs, qmp->vregs);
> err_decrement_count:
> qmp->init_count--;
[Severity: High]
This is a pre-existing issue, but does this error path permanently break the
PHY's power management lifecycle?
When qmp_combo_com_init() is called with force == true, the init_count is not
incremented at the start of the function. However, if a hardware
initialization step fails and execution jumps to err_decrement_count, it will
decrement qmp->init_count, leading to an integer underflow.
> return ret;
> }
[ ... ]
> @@ -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) {
[Severity: High]
Does this actually fix the unprogrammed hardware state?
When init_count == 0, the early bailout is bypassed, but the hardware
programming block inside this if (qmp->init_count) condition is also skipped.
Once phy_init() is called, it executes qmp_combo_com_init(), which sets
qmpphy_mode_committed = true. Would subsequent calls to typec_mux_set() then
see committed == true and hit the early bailout just as they did before this
patch?
> if (qmp->usb_init_count)
[Severity: High]
This isn't a bug introduced by this patch, but can the interaction between
usb_init_count and init_count lead to a resource leak?
When transitioning to DP-only mode in qmp_combo_typec_mux_set(),
qmp->usb_init_count is decremented while using qmp_combo_com_exit(qmp, true),
which bypasses decrementing the master qmp->init_count.
If usb_init_count drops to 0, would a subsequent phy_exit() by the USB consumer
return early in qmp_combo_usb_exit() and skip calling
qmp_combo_com_exit(qmp, false)? This would leave the USB consumer's original
contribution to init_count unaccounted for, keeping regulators and clocks
enabled forever.
[Severity: High]
This is a pre-existing issue, but does lockless access to qmp->usb_init_count
in qmp_combo_usb_exit() cause a Time-Of-Check to Time-Of-Use race condition?
If qmp_combo_usb_exit() checks usb_init_count without holding qmp->phy_mutex,
could it drop to 0 concurrently via a MUX switch to DP-only in
qmp_combo_typec_mux_set()? When qmp_combo_usb_exit() subsequently acquires the
lock and continues, it would decrement usb_init_count causing an underflow to
UINT_MAX, allowing future exits to bypass the zero check and trigger unbalanced
teardowns.
> @@ -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;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260521010935.1333494-1-mike.scott@oss.qualcomm.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 2/4] phy: qcom: qmp-combo: track whether the cached typec_mux mode was committed to hardware
From: sashiko-bot @ 2026-05-21 2:20 UTC (permalink / raw)
To: Michael Scott
Cc: devicetree, olteanv, conor+dt, neil.armstrong, linux-phy, robh,
vkoul
In-Reply-To: <20260521003615.1260844-3-mike.scott@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [High] The patch is a complete logical no-op and fails to alter the hardware programming flow in the described failure scenario.
- [Medium] The patch unconditionally marks `qmpphy_mode_committed = true` in `qmp_combo_typec_mux_set()`, ignoring hardware reprogramming failures.
Pre-existing issues:
- [High] `qmp_combo_com_init()` incorrectly decrements `init_count` on failure when `force == true`, corrupting reference counts.
- [High] TOCTOU race condition in `qmp_combo_usb_exit()` causes `usb_init_count` integer underflow.
- [High] Delayed PHY mode switches are permanently dropped if the DP PHY is actively powered on.
--
commit 6892dbad473d7036463b8a81f539f3ea5bbf55d4
Author: Michael Scott <mike.scott@oss.qualcomm.com>
Subject: phy: qcom: qmp-combo: track whether the cached typec_mux mode was committed to hardware
This patch attempts to fix an issue where the typec_mux_set function
bails out early and fails to reprogram hardware if called before
phy_init. It adds a committed flag to track if the cached mode matches
the actual hardware state.
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 4057583c0cfeb..9b6da783ce607 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -3747,6 +3748,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_assert_reset:
> reset_control_bulk_assert(cfg->num_resets, qmp->resets);
> err_disable_regulators:
> regulator_bulk_disable(cfg->num_vregs, qmp->vregs);
> err_decrement_count:
> qmp->init_count--;
>
> return ret;
> }
[Severity: High]
This is a pre-existing issue, but does this error path corrupt the reference
count when called with force == true?
If qmp_combo_com_init() is called with force = true, the short-circuit
evaluation bypasses incrementing qmp->init_count:
if (!force && qmp->init_count++)
return 0;
However, if a subsequent operation like regulator_bulk_enable() fails, the
error path jumps to err_decrement_count and unconditionally decrements it.
Could this underflow init_count to -1, permanently breaking the initialization
state machine since subsequent checks like `if (qmp->init_count++)` will see
truthy values?
[ ... ]
> @@ -4507,9 +4511,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) {
> dev_err(qmp->dev, "DP PHY is still in use, delaying switch\n");
> return 0;
> }
[Severity: High]
This is a pre-existing issue, but does this early return permanently drop
delayed PHY mode switches?
If a switch to QMPPHY_MODE_USB3_ONLY is requested but the DP PHY is actively
powered on, this branch bails out early, logs a message, and returns 0.
Because it returns success without updating qmp->qmpphy_mode or scheduling a
delayed work to apply the switch later, the Type-C framework assumes the mode
was successfully applied.
Does this cause a permanent desynchronization between software state and
hardware, keeping the DP PHY powered when it should be disabled?
> @@ -4521,6 +4538,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) {
[Severity: High]
Is this patch effectively a logical no-op in the failure scenario it
describes?
If typec_mux_set() is called before phy_init() (when init_count is 0), the
early bail-out is bypassed because qmpphy_mode_committed is false. However,
the code then reaches the `if (qmp->init_count)` check here, which evaluates
to false, causing the function to return without touching the hardware anyway.
When phy_init() eventually executes, qmp_combo_com_init() correctly syncs
hardware to the cached mode and unconditionally sets
qmpphy_mode_committed = true.
Subsequent calls to typec_mux_set() with the same mode will see
qmpphy_mode_committed == true and bail out early, exactly as they did before
this patch. Does this mean the hardware state remains unchanged from the
pre-patch behavior?
> if (qmp->usb_init_count)
[ ... ]
> if (qmp->dp_init_count)
> cfg->dp_aux_init(qmp);
> }
> +
> + /* Reprogram complete; cache now reflects hardware. */
> + qmp->qmpphy_mode_committed = true;
> }
[Severity: Medium]
Is it safe to unconditionally mark the hardware state as committed here?
The code calls qmp_combo_com_init(qmp, true) earlier in this block to apply
hardware changes, but ignores its return value. If qmp_combo_com_init() fails,
the hardware state remains unconfigured, but this flag is blindly set to true.
Would this prevent any subsequent attempts to correct the hardware state,
since future calls will trigger the early bail-out on
qmp->qmpphy_mode_committed?
[Severity: High]
This isn't a bug introduced by this patch, but is there a time-of-check to
time-of-use race condition in qmp_combo_usb_exit() that can underflow
usb_init_count?
If qmp->usb_init_count is checked outside the phy_mutex in
qmp_combo_usb_exit(), a concurrent Type-C mode switch could occur:
Thread 1: qmp_combo_usb_exit()
if (qmp->usb_init_count == 0)
return;
// Preempted before acquiring phy_mutex
Thread 2: qmp_combo_typec_mux_set()
mutex_lock(&qmp->phy_mutex);
// Switches to QMPPHY_MODE_DP_ONLY
qmp->usb_init_count--; // Now 0
mutex_unlock(&qmp->phy_mutex);
Thread 1 resumes:
mutex_lock(&qmp->phy_mutex);
...
qmp->usb_init_count--; // Underflows to UINT_MAX
Could this permanently break subsequent power management logic and allow
unclocked hardware accesses?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260521003615.1260844-1-mike.scott@oss.qualcomm.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 1/4] phy: qcom: qmp-combo: skip USB power_off/exit after device teardown
From: sashiko-bot @ 2026-05-21 1:52 UTC (permalink / raw)
To: Michael Scott
Cc: vkoul, linux-phy, robh, conor+dt, neil.armstrong, devicetree,
olteanv
In-Reply-To: <20260521010935.1333494-2-mike.scott@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] The lockless check of `qmp->usb_init_count` in `qmp_combo_usb_exit()` introduces a Time-of-Check to Time-of-Use (TOCTOU) race condition that can lead to an integer underflow and unbalanced resource management.
Pre-existing issues:
- [Critical] The patch attempts to prevent a crash by checking `qmp->usb_init_count` after the device's backing resources are released, but the `qmp` structure itself has already been freed via `devm` cleanup, resulting in a Use-After-Free.
--
commit 639386ffdb78db9a5c3e78a1f3ada8713f38acd7
Author: Michael Scott <mike.scott@oss.qualcomm.com>
phy: qcom: qmp-combo: skip USB power_off/exit after device teardown
The commit attempts to prevent crashes when external consumers like dwc3
call phy_exit() after the QMP PHY's backing resources have been released,
by guarding the power_off and exit paths with a check on the
usb_init_count balance.
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index cdcfad2e86b1d..0db200292642e 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) {
[Severity: Critical]
This is a pre-existing issue, but the qmp structure itself is allocated
using devm_kzalloc(), so it is freed during the device's devres cleanup.
If dwc3 calls phy_exit() after qmp_combo has been unbound, won't accessing
qmp->usb_init_count result in a Use-After-Free (UAF) read?
If the freed memory happens to be zeroed, the read evaluates to 0 and
successfully masks the bug by skipping the remainder of the function. If
SLUB poisoning or KASAN is enabled, this will trigger a UAF splat or read a
non-zero value, proceeding to call mutex_lock(&qmp->phy_mutex) on freed
memory.
Does this patch replace a vmap UAF on qmp->pcs with a slab UAF on the freed
qmp structure instead of fixing the underlying lifecycle bug?
[ ... ]
> @@ -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) {
[Severity: High]
Does checking qmp->usb_init_count outside of qmp->phy_mutex introduce a
Time-of-Check to Time-of-Use (TOCTOU) race?
If a concurrent thread (such as qmp_combo_typec_mux_set() reacting to a
Type-C event) acquires the mutex and decrements usb_init_count to zero
immediately after this lockless check succeeds, qmp_combo_usb_exit() will
proceed to acquire the lock.
It will then call qmp_combo_usb_power_off(), which will safely check the
count under the lock and return 0 for success.
Because qmp_combo_usb_power_off() returns success, qmp_combo_usb_exit()
mistakenly proceeds to call qmp_combo_com_exit() and unconditionally
executes qmp->usb_init_count--.
Since the counter is an unsigned int and is already 0, could this wrap
around to UINT_MAX and cause unbalanced regulator and clock disables
which can crash the system?
> + 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);
> if (ret)
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260521010935.1333494-1-mike.scott@oss.qualcomm.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 1/4] phy: qcom: qmp-combo: skip USB power_off/exit after device teardown
From: sashiko-bot @ 2026-05-21 1:32 UTC (permalink / raw)
To: Michael Scott
Cc: neil.armstrong, olteanv, conor+dt, vkoul, robh, linux-phy,
devicetree
In-Reply-To: <20260521003615.1260844-2-mike.scott@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Bailing out early in `qmp_combo_usb_exit()` completely bypasses the release of shared common resources, causing a permanent reference leak when a Type-C switch to `DP_ONLY` occurs.
- [High] A lockless Time-of-Check to Time-of-Use (TOCTOU) race condition on `qmp->usb_init_count` in `qmp_combo_usb_exit()` leads to integer underflow and state corruption.
Pre-existing issues:
- [High] The patch attempts to fix an MMIO crash by introducing a Use-After-Free read on the `qmp` driver data structure.
--
commit 35c4e56f462fac540e8354624e2048c6bd3ecd9e
Author: Michael Scott <mike.scott@oss.qualcomm.com>
phy: qcom: qmp-combo: skip USB power_off/exit after device teardown
This commit adds checks to qmp_combo_usb_power_off() and qmp_combo_usb_exit()
to guard against being called by an external consumer after the device's
backing resources have been released. It uses the usb_init_count variable
to determine if the PHY is powered on, skipping the operations to prevent
an oops.
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 93f1aa10d4008..4057583c0cfeb 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -3922,6 +3922,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) {
[Severity: High]
This isn't a bug introduced by this patch, but does this check introduce
a use-after-free read on the qmp driver data structure?
The commit message notes that this path runs after the device's backing
resources are released. Since qmp is allocated using devm_kzalloc(), it is
also freed during devres teardown.
When dwc3 calls phy_exit() later, phy_get_drvdata() returns a freed pointer.
Dereferencing qmp->usb_init_count accesses this freed memory. If the memory
was reallocated and non-zero, or if KASAN is enabled, wouldn't this still
trigger a use-after-free and crash instead of masking the teardown order bug?
> + dev_dbg(qmp->dev, "%s: PHY not powered on, skipping\n",
> + __func__);
> + return 0;
> + }
> +
> clk_disable_unprepare(qmp->pipe_clk);
[ ... ]
> @@ -3966,6 +3977,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) {
[Severity: High]
Does bailing out early here bypass the release of shared common resources,
causing a reference leak when a Type-C switch to DP_ONLY occurs?
When the Type-C mux switches to QMPPHY_MODE_DP_ONLY,
qmp_combo_typec_mux_set() decrements usb_init_count to 0 but leaves
init_count unchanged. If the consumer unbinds and calls phy_exit() in
this state, this early return bypasses the qmp_combo_com_exit(qmp, false)
call.
Doesn't this mean qmp->init_count is never decremented, permanently
leaking shared resources like clocks and regulators?
[Severity: High]
Is there a lockless Time-of-Check to Time-of-Use race condition here?
This check is performed before acquiring qmp->phy_mutex. If another path
like qmp_combo_typec_mux_set() holds the mutex and decrements
usb_init_count to 0:
CPU1
qmp_combo_usb_exit()
if (!qmp->usb_init_count) // observes > 0
mutex_lock(&qmp->phy_mutex); // blocks
CPU2
qmp_combo_typec_mux_set()
mutex_lock(&qmp->phy_mutex);
qmp->usb_init_count--; // becomes 0
mutex_unlock(&qmp->phy_mutex);
When CPU1 acquires the lock, it proceeds to decrement qmp->usb_init_count
without re-verifying the state. Doesn't decrementing 0 cause an integer
underflow to UINT_MAX, breaking the PHY's power management balance?
> + 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);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260521003615.1260844-1-mike.scott@oss.qualcomm.com?part=1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH v2 4/4] arm64: dts: qcom: x1-dell-thena: bump linux,cma to 256 MiB
From: Michael Scott @ 2026-05-21 1:09 UTC (permalink / raw)
To: 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, bryan.odonoghue, laurentiu.tudor1,
alex.vinarskis, linux-kernel, Michael Scott
In-Reply-To: <20260521010935.1333494-1-mike.scott@oss.qualcomm.com>
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;
};
--
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 v2 3/4] arm64: dts: qcom: x1-dell-thena: mark l12b and l15b always-on
From: Michael Scott @ 2026-05-21 1:09 UTC (permalink / raw)
To: 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, bryan.odonoghue, laurentiu.tudor1,
alex.vinarskis, linux-kernel, Michael Scott, stable
In-Reply-To: <20260521010935.1333494-1-mike.scott@oss.qualcomm.com>
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.
Fixes: e7733b42111c ("arm64: dts: qcom: Add support for Dell Inspiron 7441 / Latitude 7455")
Cc: stable@vger.kernel.org
Signed-off-by: Michael Scott <mike.scott@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/x1-dell-thena.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/x1-dell-thena.dtsi b/arch/arm64/boot/dts/qcom/x1-dell-thena.dtsi
index db291730130c..d6de4da02dcd 100644
--- a/arch/arm64/boot/dts/qcom/x1-dell-thena.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1-dell-thena.dtsi
@@ -589,6 +589,7 @@ vreg_l12b_1p2: ldo12 {
regulator-min-microvolt = <1200000>;
regulator-max-microvolt = <1200000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ regulator-always-on;
};
vreg_l13b_3p0: ldo13 {
@@ -610,6 +611,7 @@ vreg_l15b_1p8: ldo15 {
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ regulator-always-on;
};
};
--
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 v2 2/4] phy: qcom: qmp-combo: track whether the cached typec_mux mode was committed to hardware
From: Michael Scott @ 2026-05-21 1:09 UTC (permalink / raw)
To: 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, bryan.odonoghue, laurentiu.tudor1,
alex.vinarskis, linux-kernel, Michael Scott
In-Reply-To: <20260521010935.1333494-1-mike.scott@oss.qualcomm.com>
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;
--
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 v2 1/4] phy: qcom: qmp-combo: skip USB power_off/exit after device teardown
From: Michael Scott @ 2026-05-21 1:09 UTC (permalink / raw)
To: 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, bryan.odonoghue, laurentiu.tudor1,
alex.vinarskis, linux-kernel, Michael Scott
In-Reply-To: <20260521010935.1333494-1-mike.scott@oss.qualcomm.com>
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>
---
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);
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 related
* [PATCH v2 0/4] phy: qcom: qmp-combo fixes + x1-dell-thena DT maintenance
From: Michael Scott @ 2026-05-21 1:09 UTC (permalink / raw)
To: 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, bryan.odonoghue, laurentiu.tudor1,
alex.vinarskis, linux-kernel, Michael Scott
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).
* Two dell-thena DT maintenance items: one mirrors a regulator
always-on change Hovold applied to the rest of the X1E80100
family but that dell-thena missed, and one bumps the linux,cma
reserved-memory pool so the camera pipeline can actually
allocate buffers alongside a normal desktop (patches 3-4).
== Changes since v1 ==
* Rebased from v7.1-rc4 onto linux-next (next-20260520). v1 did not
apply on top of commit f546912bcac6 ("phy: qcom: qmp-combo: Move
pipe_clk on/off to common"), which landed in phy/next after
v7.1-rc4; patch 1's hunk context is adjusted for that refactor.
No functional change to any patch.
v1: https://lore.kernel.org/linux-arm-msm/20260521003615.1260844-1-mike.scott@oss.qualcomm.com/
== phy-qcom-qmp-combo fixes (patches 1-2) ==
Both bugs were found by exercising the typec_mux + role-switch code
paths on Dell Latitude 7455 (X1E80100, dell-thena). In mainline
today the bugs are reachable on x1e001de-devkit, which registers a
usb_role_switch on one USB-C port; they would also fire on any
future board that opts into the same DT pattern.
Patch 1: qmp_combo_usb_power_off() / qmp_combo_usb_exit() can be
re-entered as ->exit from an external consumer (dwc3 phy_exit during
driver unbind) after this device's backing devm resources have
already been released along a separate teardown chain. The
dereference of qmp->pcs (whose ioremap has been freed) then oopses
with a level-3 translation fault. The patch adds a usb_init_count
guard so the re-entry is a no-op. The proper long-term fix is a
teardown-ordering rework so the QMP PHY outlives any consumer that
may still call its phy_ops; until then, this guard prevents the
oops.
Patch 2: qmp_combo_typec_mux_set() updates the cached qmpphy_mode
unconditionally, but only reprograms hardware when init_count is
non-zero. So a typec_mux_set arriving before phy_init updates the
cache without programming hardware; subsequent calls then see a
"match" against the cached mode and bail out early, leaving the
lane mux in whatever state it powered up in. The patch tracks
separately whether the cache has been committed to hardware, so
the fast-path bail only happens when the cache truly reflects the
hardware.
== DT maintenance (patches 3-4) ==
Patch 3 marks vreg_l12b_1p2 and vreg_l15b_1p8 always-on. Hovold did
this for every other X1E80100 board in March 2025; dell-thena landed
four months later (commit e7733b42111c) and missed the change, which
leaves the kernel free to disable those LDOs even though several
board-level fixed regulators have no described vin-supply link back
to them.
Patch 4 raises linux,cma from 128 MiB to 256 MiB. The 128 MiB pool
is too small to support libcamera's buffer set in parallel with the
normal desktop: msm DRM framebuffers, qcom_iris codec buffers, and
qcom_camss VFE pre-allocations occupy ~100 MiB at GNOME idle,
leaving ~25 MiB free. libcamera's "simple" pipeline asks for four
8.35 MiB ABGR8888 frames (32 MiB total) and the fourth allocation
fails with "dma-heap allocation failure". At 256 MiB, ~150 MiB is
free at idle -- comfortable headroom.
Note for other X1E maintainers: every other X1E80100 / X1E78100 /
X1P42100 board in mainline is still on the 128 MiB default, and
several of them carry camera nodes (Dell XPS 13 9345, Medion
Sprchrgd-14, ASUS Zenbook A14, Microsoft Romulus, Microsoft Denali,
Lenovo ThinkBook 16). Those boards are likely to hit the same
allocation failure once libcamera enablement lands on them, and
should probably take a similar bump. I limited this patch to
dell-thena because I do not have the other boards on hand to
verify the resulting CmaFree numbers under a real workload --
applying the same change blindly across boards I cannot test would
just shift the guesswork.
== Patch summary ==
1/4 phy: qcom: qmp-combo: skip USB power_off/exit after device
teardown
2/4 phy: qcom: qmp-combo: track whether the cached typec_mux mode
was committed to hardware
3/4 arm64: dts: qcom: x1-dell-thena: mark l12b and l15b always-on
4/4 arm64: dts: qcom: x1-dell-thena: bump linux,cma to 256 MiB
== Testing ==
Hardware: Dell Latitude 7455 (X1E80100), running Ubuntu 26.04.
Test kernel: a local build of Ubuntu's 7.0.0-15-generic source with
this series applied on top. The series in this submission is rebased
onto linux-next (next-20260520) so that it applies cleanly on top of
Val Packett's recent qmp-combo refactor (commit f546912bcac6, "phy:
qcom: qmp-combo: Move pipe_clk on/off to common"), which is queued in
phy/next and reaches mainline at the next merge window.
- Without this series:
* Writing "none" to a manually-bound usb_role_switch reliably
oopses the kernel. vmcore captured via kdump-tools; crash(1)
analysis confirms qmp->pcs UAF in qmp_combo_usb_power_off,
reached via dwc3_remove -> dwc3_phy_exit -> phy_exit ->
qmp_combo_usb_exit.
* Without patch 2, the first typec_mux_set arriving before
phy_init updates the cache but not the hardware; the next
call hits "same qmpphy mode, bail out" and the lane mux
stays in its default configuration.
* libcamera-mediated camera apps (gnome-snapshot, etc.) fail
to start with "dma-heap allocation failure for frame-3".
- With this series:
* Role-switch teardown no longer oopses (patch 1's guard).
* QMP PHY is reprogrammed on first altmode notification after
phy_init (patch 2's committed-state tracking).
* CmaFree at GNOME idle is ~150 MiB (was ~25 MiB).
* gnome-snapshot opens with a live preview from the OV02E10
sensor.
Patches 1-2 were exercised by manually wiring up a usb_role_switch
on dell-thena and driving the role-switch path; the DT change that
makes that wiring permanent is not part of this series.
A personal note: it has been a while since I last sent patches
upstream -- as you may have gathered from v1 being based on v7.1-rc4
rather than linux-next. Apologies for the extra round-trip; if I have
missed any other recent process changes, corrections are welcome.
Michael Scott (4):
phy: qcom: qmp-combo: skip USB power_off/exit after device teardown
phy: qcom: qmp-combo: track whether the cached typec_mux mode was
committed to hardware
arm64: dts: qcom: x1-dell-thena: mark l12b and l15b always-on
arm64: dts: qcom: x1-dell-thena: bump linux,cma to 256 MiB
arch/arm64/boot/dts/qcom/x1-dell-thena.dtsi | 4 ++-
drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 47 +++++++++++++++++++++++++++--
2 files changed, 48 insertions(+), 3 deletions(-)
base-commit: 687da68900cd1a46549f7d9430c7d40346cb86a0
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH 4/4] arm64: dts: qcom: x1-dell-thena: bump linux,cma to 256 MiB
From: Michael Scott @ 2026-05-21 0:36 UTC (permalink / raw)
To: 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, bryan.odonoghue, laurentiu.tudor1,
alex.vinarskis, linux-kernel, Michael Scott
In-Reply-To: <20260521003615.1260844-1-mike.scott@oss.qualcomm.com>
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 d93b704872b5..7894d96d34ee 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;
};
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox