* [PATCH v5 1/2] dt-bindings: usb: snps,dwc3: tighten the two-connector shared HS bus
@ 2026-11-02 5:41 Qianhui Xu
2026-11-02 8:07 ` Krzysztof Kozlowski
0 siblings, 1 reply; 3+ messages in thread
From: Qianhui Xu @ 2026-11-02 5:41 UTC (permalink / raw)
To: devicetree
Cc: Rob Herring, Conor Dooley, Krzysztof Kozlowski,
Greg Kroah-Hartman, linux-usb, linux-arm-msm, linux-kernel
Resending only 1/2 of this series.
The earlier post of this patch (Message-ID
<20261031150154.1106455-2-xiaqinghuan@mail.ustc.edu.cn>, 0/2 cover
<20261031150154.1106455-1-xiaqinghuan@mail.ustc.edu.cn>, 2026-10-31) used the
wrong device-tree list address: it was sent to
linux-devicetree@vger.kernel.org, which vger rejects with
550 5.1.1 <linux-devicetree@vger.kernel.org>: Recipient address rejected:
User unknown in virtual alias table
so it never reached devicetree@vger.kernel.org. The patch text below is
unchanged, byte for byte, and is the same text as in the 0/2 cover. The
sibling 2/2 went to linux-usb and linux-arm-msm and is archived there; this
resend is only about putting 1/2 into the devicetree archive. The subject is
kept as "[PATCH v5 1/2]" so it still lines up with that cover.
AI-assistance disclosure: this patch is drafted with an AI assistant and
edited by me; the author reviewed, built and tested it, and the content and
the measurements are the author's own.
Two connectors behind one controller only take the mux path when the
controller node itself carries "usb-role-switch": dwc3_get_dr_mode()
otherwise forces dr_mode back to peripheral, dwc3_get_extcon() skips the
role-switch branch and dwc3_drd_init() never calls dwc3_role_mux_init().
The binding accepted such a DT, so a DT that passed the schema check did
not describe a DT the driver could act on. Require the property whenever
port-select-gpios is present.
Two more holes in the endpoint schema are fixed at the same time:
- the hs-port patternProperties only covered endpoint@0 and endpoint@1,
so an unnumbered endpoint or endpoint@2+ escaped the
unevaluatedProperties: false rule of the graph endpoint. Use the same
endpoint name pattern as graph.yaml so every endpoint is validated.
- the mux-state complementarity was expressed with two independent "if
const: 0" blocks, which only rejected two zeroes; two endpoints both
claiming mux-state = <1> passed validation. Express the pair as oneOf
so exactly one of the two assignments is accepted.
Signed-off-by: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
---
.../bindings/usb/snps,dwc3-common.yaml | 105 +++++++++---------
1 file changed, 55 insertions(+), 50 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
index 6b06b0027b..b23d5d8d74 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
@@ -31,7 +31,9 @@ $defs:
enum: [0, 1]
description:
Logical level of the port-select GPIO which connects this
- connector to the shared High Speed data bus.
+ connector to the shared High Speed data bus. The two connectors of
+ one controller must use different levels; the driver rejects a
+ device tree where two connectors claim the same one.
dependentRequired:
usb-role-switch: [mux-state]
@@ -42,7 +44,7 @@ $defs:
unevaluatedProperties: false
patternProperties:
- '^endpoint@[01]$':
+ '^endpoint(@[0-9a-f]+)?$':
$ref: '#/$defs/hs-endpoint'
allOf:
@@ -61,70 +63,74 @@ allOf:
- if:
required: [port-select-gpios]
then:
- required: [dr_mode, ports]
+ # The mux path is only taken when the controller itself carries
+ # "usb-role-switch": dwc3_get_dr_mode() forces dr_mode back to
+ # peripheral, dwc3_get_extcon() skips the role-switch branch and
+ # dwc3_drd_init() never calls dwc3_role_mux_init() without it.
+ required: [dr_mode, ports, usb-role-switch]
properties:
dr_mode:
const: otg
ports:
- required: [port@0]
+ # Each connector is one port with a single endpoint: the mux-state
+ # property on that endpoint says which level of the port-select GPIO
+ # reaches it, so two connectors are two ports, not two endpoints of
+ # one port. port@1 is the SuperSpeed lane of the PHY and is already
+ # described by snps,dwc3.yaml, so the second High Speed connector
+ # lands on port@2.
+ required: [port@0, port@2]
properties:
port@0:
- required: ['#address-cells', '#size-cells', endpoint@0, endpoint@1]
+ $ref: '#/$defs/hs-port'
+ port@2:
+ $ref: '#/$defs/hs-port'
+
+ # The two connectors must claim opposite levels of the port-select
+ # GPIO; requiring both ports in every branch is what makes the two
+ # cases mutually exclusive for oneOf.
+ oneOf:
+ - required: [port@0, port@2]
properties:
- endpoint@0:
- required: [usb-role-switch, mux-state]
- endpoint@1:
- required: [usb-role-switch, mux-state]
- else:
- properties:
- ports:
- properties:
- port@0:
- patternProperties:
- '^endpoint@[01]$':
+ port@0:
+ $ref: '#/$defs/hs-port'
properties:
- usb-role-switch: false
- mux-state: false
- - if:
- properties:
- ports:
- properties:
- port@0:
- properties:
- endpoint@0:
+ endpoint:
+ properties:
+ mux-state:
+ const: 0
+ port@2:
+ $ref: '#/$defs/hs-port'
properties:
- mux-state:
- const: 0
- then:
- properties:
- ports:
- properties:
- port@0:
+ endpoint:
+ properties:
+ mux-state:
+ const: 1
+ - required: [port@0, port@2]
properties:
- endpoint@1:
+ port@0:
+ $ref: '#/$defs/hs-port'
properties:
- mux-state:
- const: 1
- - if:
- properties:
- ports:
- properties:
- port@0:
- properties:
- endpoint@1:
+ endpoint:
+ properties:
+ mux-state:
+ const: 1
+ port@2:
+ $ref: '#/$defs/hs-port'
properties:
- mux-state:
- const: 0
- then:
+ endpoint:
+ properties:
+ mux-state:
+ const: 0
+ else:
properties:
ports:
properties:
port@0:
- properties:
- endpoint@0:
+ patternProperties:
+ '^endpoint(@[0-9a-f]+)?$':
properties:
- mux-state:
- const: 1
+ usb-role-switch: false
+ mux-state: false
properties:
port-select-gpios:
@@ -536,4 +542,3 @@ required:
additionalProperties: true
...
-
base-commit: fd69bd54e328c407b1853378b25d15803d3453fa
--
2.53.0
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v5 1/2] dt-bindings: usb: snps,dwc3: tighten the two-connector shared HS bus
2026-11-02 5:41 [PATCH v5 1/2] dt-bindings: usb: snps,dwc3: tighten the two-connector shared HS bus Qianhui Xu
@ 2026-11-02 8:07 ` Krzysztof Kozlowski
0 siblings, 0 replies; 3+ messages in thread
From: Krzysztof Kozlowski @ 2026-11-02 8:07 UTC (permalink / raw)
To: Qianhui Xu
Cc: devicetree, Rob Herring, Conor Dooley, Krzysztof Kozlowski,
Greg Kroah-Hartman, linux-usb, linux-arm-msm, linux-kernel
On Mon, Nov 02, 2026 at 01:41:09PM +0800, Qianhui Xu wrote:
> Resending only 1/2 of this series.
>
> The earlier post of this patch (Message-ID
> <20261031150154.1106455-2-xiaqinghuan@mail.ustc.edu.cn>, 0/2 cover
> <20261031150154.1106455-1-xiaqinghuan@mail.ustc.edu.cn>, 2026-10-31) used the
> wrong device-tree list address: it was sent to
> linux-devicetree@vger.kernel.org, which vger rejects with
>
> 550 5.1.1 <linux-devicetree@vger.kernel.org>: Recipient address rejected:
> User unknown in virtual alias table
>
> so it never reached devicetree@vger.kernel.org. The patch text below is
> unchanged, byte for byte, and is the same text as in the 0/2 cover. The
> sibling 2/2 went to linux-usb and linux-arm-msm and is archived there; this
> resend is only about putting 1/2 into the devicetree archive. The subject is
> kept as "[PATCH v5 1/2]" so it still lines up with that cover.
>
> AI-assistance disclosure: this patch is drafted with an AI assistant and
> edited by me; the author reviewed, built and tested it, and the content and
> the measurements are the author's own.
I don't see how any of above is useful in git log.
Anyway, this is misthreaded - missing rest of patchset. Please read
submitting patches document and switch to b4 for sending your
contributions.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v5 0/2] usb: dwc3: fixes for two connectors sharing one HS bus
@ 2026-10-31 15:01 Qianhui Xu
2026-10-31 15:01 ` [PATCH v5 1/2] dt-bindings: usb: snps,dwc3: tighten the two-connector shared " Qianhui Xu
0 siblings, 1 reply; 3+ messages in thread
From: Qianhui Xu @ 2026-10-31 15:01 UTC (permalink / raw)
To: Greg Kroah-Hartman, Thinh Nguyen
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-usb,
linux-devicetree, linux-arm-msm, linux-kernel
*** AI-assistance disclosure ***
This cover letter and the accompanying patches were prepared with the help
of an LLM coding assistant; the author reviewed, tested and takes full
responsibility for the content. All measurements cited below come from real
hardware runs and are reproducible with the commands referenced in our
public reports.
Greetings,
This is v5. It is a fixup series that applies *on top of* v4 [1] -- it
does not replace v4 1/3..3/3, it fixes them in place:
- 1/2 tightens the binding of v4 1/3 (snps,dwc3-common.yaml),
- 2/2 fixes the driver of v4 3/3 (usb: dwc3: support two connectors
sharing one HS data bus) and also corrects
usb_role_switch_at_endpoint() introduced by v4 2/3.
Base: v4 3/3 (fd69bd54e328, "usb: dwc3: support two connectors sharing
one HS data bus"). Please apply v4 1/3..3/3 from lore [1] before this
series; the `base-commit:` trailer in this posting is informational only,
since it names a commit that is not in any public tree yet.
Hardware: Lenovo Legion Y700 (4th gen) tablet, SM8750. Both High Speed
connectors share one DWC3 controller; a port-select GPIO picks which
connector owns the bus, and each connector has its own typec port with its
own role switch, described per graph endpoint.
Changes since v4 (all points are replies to the automated review by
Sashiko [2][3][4]; no human maintainer replied in the v4 thread yet):
Binding (Sashiko on v4 1/3, three Medium findings [2]):
* "hs-port patternProperties does not match what the driver expects /
escapes unevaluatedProperties". Fixed: the endpoint name pattern is
now the same one graph.yaml uses, so an unnumbered endpoint or
endpoint@2+ is validated too instead of silently escaping the
unevaluatedProperties: false rule (v5 1/2).
* "usb-role-switch should not be required only indirectly". Fixed: the
controller node must now carry "usb-role-switch" whenever
port-select-gpios is present. This is a driver-fact fix, not just
schema cosmetics: without that property dwc3_get_dr_mode() forces
dr_mode back to peripheral and dwc3_drd_init() never runs, so a DT
that passed the old schema did not describe a DT the driver could act
on (v5 1/2).
* "mux-state complementarity is not expressed". Fixed: the two
independent `if: const: 0` blocks only rejected two zeroes; two
endpoints both claiming mux-state = <1> passed validation. The pair
is now a single oneOf so exactly one assignment is accepted (v5 1/2).
* On the standing question whether this should use the standard
mux-controls/mux-states properties instead of per-endpoint role
switches: kept the graph-endpoint shape for now. Reason: the
arbitration is "earliest request wins" between two *typec connectors*
that must each be a usb role switch consumer, and the endpoint is the
fwnode the connector is attached to; a mux-control consumer does not
give us that per-connector identity. drivers/usb/ has no
mux_control consumer today to follow as precedent. Happy to reshape
if the DT maintainers prefer the standard property.
usb: roles (Sashiko on v4 2/3, one High finding [3]):
* "-EPROBE_DEFER is swallowed by !IS_ERR_OR_NULL(sw)". Fixed: pass the
error up, as suggested (v5 2/2, last hunk). This was also the root
cause we hit on real hardware: the UCSI glue kept a NULL switch and
the connectors never bound.
usb: dwc3 (Sashiko on v4 3/3, five High + three Medium findings [4]):
* "dwc3_set_mode() under dwc->lock self-deadlocks". Fixed: request
decision and issue are now paired by mux->lock; dwc->lock is only held
while updating the individual fields (v5 2/2).
* "glue layer is never notified". Fixed: dwc3_pre_set_role() is called
with the effective role, outside spinlocks, before dwc3_set_mode(),
mirroring the single-connector path (v5 2/2).
* "same-role request that moves the data path to the other connector".
Fixed: dwc3_set_prtcap() only clears SUSPHY when PRTCAP changes, so
that case now clears it explicitly (v5 2/2).
* "registration earlier than the initial mode request races with a
connector driver". Fixed: register and request the initial mode under
mux->lock, tear down in reverse order on failure (v5 2/2).
* "fwnode references leak on error paths". Fixed: dwc->role_mux is
published before the error paths that used to leak the endpoint
fwnodes, so dwc3_role_mux_exit() can actually unwind (v5 2/2).
* "sticky mux->error is reported to the next caller after it already
committed its own request". Fixed: the sticky value is dropped; a
failed data-path switch is logged and counted per connector (v5 2/2).
* (found during on-device testing, same area) dwc3_mux_role_get()
returned USB_ROLE_NONE for a connector that did not own the data path,
hiding its pending request; the connector driver then never sent the
set_role(NONE) that would clear it, and the stale request kept winning
arbitration after unplug. get() now reports the requested role
(v5 2/2).
Test evidence (all on the SM8750 tablet, kernel built from this series):
- `git am --3way` of the whole series on top of v4 3/3 succeeds;
checkpatch --strict reports 0 errors / 0 warnings / 0 checks for both
patches.
- `dt-doc-validate` on snps,dwc3-common.yaml: clean.
- `make W=1 drivers/usb/dwc3/ drivers/usb/roles/` (arm64): 0 warnings.
- On device: after the -EPROBE_DEFER fix, /sys/class/typec shows port0,
port1 and a bound partner; with function_graph tracing, 12 consecutive
plug/unplug cycles each went usb_role_switch_set_role ->
dwc3_mux_role_set [dwc3] -> glue notifier -> dwc3_set_mode with zero
WARNING/BUG/lockdep splats, USB data and charging both stable
throughout.
- The binding topology (two HS connectors on port@0 and port@2 of the
controller's port node, one endpoint each, mux-state on the endpoint)
matches what the board DTS uses; port@1 is occupied by the SuperSpeed
path to the QMP PHY, which is why the two HS connectors are not
siblings under one port.
Known limitation, stated up front: the role *arbitration* detail (which
connector wins a simultaneous request) is verified only by code reading and
the stale-request test above; a dedicated per-port role-value trace is
instrumented but its data set is not yet complete. Everything else listed
above has been exercised on hardware.
Thanks,
Qianhui Xu
[1] v4: https://lore.kernel.org/linux-usb/20261026123211.243224-1-xiaqinghuan@mail.ustc.edu.cn/
[2] Sashiko on v4 1/3: <sashiko-outbox-185247@kernel.org>
[3] Sashiko on v4 2/3: <sashiko-outbox-185254@kernel.org>
[4] Sashiko on v4 3/3: <sashiko-outbox-185299@kernel.org>
Qianhui Xu (2):
dt-bindings: usb: snps,dwc3: tighten the two-connector shared HS bus
usb: dwc3: fix the two-connector shared HS bus role handling
.../bindings/usb/snps,dwc3-common.yaml | 105 +++++++++---------
drivers/usb/dwc3/core.c | 36 ++++--
drivers/usb/dwc3/core.h | 4 +-
drivers/usb/dwc3/drd.c | 65 ++++++++---
drivers/usb/roles/class.c | 7 +-
5 files changed, 140 insertions(+), 77 deletions(-)
base-commit: fd69bd54e328c407b1853378b25d15803d3453fa
--
2.53.0
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH v5 1/2] dt-bindings: usb: snps,dwc3: tighten the two-connector shared HS bus
2026-10-31 15:01 [PATCH v5 0/2] usb: dwc3: fixes for two connectors sharing one " Qianhui Xu
@ 2026-10-31 15:01 ` Qianhui Xu
0 siblings, 0 replies; 3+ messages in thread
From: Qianhui Xu @ 2026-10-31 15:01 UTC (permalink / raw)
To: Greg Kroah-Hartman, Thinh Nguyen
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-usb,
linux-devicetree, linux-arm-msm, linux-kernel
Two connectors behind one controller only take the mux path when the
controller node itself carries "usb-role-switch": dwc3_get_dr_mode()
otherwise forces dr_mode back to peripheral, dwc3_get_extcon() skips the
role-switch branch and dwc3_drd_init() never calls dwc3_role_mux_init().
The binding accepted such a DT, so a DT that passed the schema check did
not describe a DT the driver could act on. Require the property whenever
port-select-gpios is present.
Two more holes in the endpoint schema are fixed at the same time:
- the hs-port patternProperties only covered endpoint@0 and endpoint@1,
so an unnumbered endpoint or endpoint@2+ escaped the
unevaluatedProperties: false rule of the graph endpoint. Use the same
endpoint name pattern as graph.yaml so every endpoint is validated.
- the mux-state complementarity was expressed with two independent "if
const: 0" blocks, which only rejected two zeroes; two endpoints both
claiming mux-state = <1> passed validation. Express the pair as oneOf
so exactly one of the two assignments is accepted.
Signed-off-by: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
---
.../bindings/usb/snps,dwc3-common.yaml | 105 +++++++++---------
1 file changed, 55 insertions(+), 50 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
index 6b06b0027b..b23d5d8d74 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
@@ -31,7 +31,9 @@ $defs:
enum: [0, 1]
description:
Logical level of the port-select GPIO which connects this
- connector to the shared High Speed data bus.
+ connector to the shared High Speed data bus. The two connectors of
+ one controller must use different levels; the driver rejects a
+ device tree where two connectors claim the same one.
dependentRequired:
usb-role-switch: [mux-state]
@@ -42,7 +44,7 @@ $defs:
unevaluatedProperties: false
patternProperties:
- '^endpoint@[01]$':
+ '^endpoint(@[0-9a-f]+)?$':
$ref: '#/$defs/hs-endpoint'
allOf:
@@ -61,70 +63,74 @@ allOf:
- if:
required: [port-select-gpios]
then:
- required: [dr_mode, ports]
+ # The mux path is only taken when the controller itself carries
+ # "usb-role-switch": dwc3_get_dr_mode() forces dr_mode back to
+ # peripheral, dwc3_get_extcon() skips the role-switch branch and
+ # dwc3_drd_init() never calls dwc3_role_mux_init() without it.
+ required: [dr_mode, ports, usb-role-switch]
properties:
dr_mode:
const: otg
ports:
- required: [port@0]
+ # Each connector is one port with a single endpoint: the mux-state
+ # property on that endpoint says which level of the port-select GPIO
+ # reaches it, so two connectors are two ports, not two endpoints of
+ # one port. port@1 is the SuperSpeed lane of the PHY and is already
+ # described by snps,dwc3.yaml, so the second High Speed connector
+ # lands on port@2.
+ required: [port@0, port@2]
properties:
port@0:
- required: ['#address-cells', '#size-cells', endpoint@0, endpoint@1]
+ $ref: '#/$defs/hs-port'
+ port@2:
+ $ref: '#/$defs/hs-port'
+
+ # The two connectors must claim opposite levels of the port-select
+ # GPIO; requiring both ports in every branch is what makes the two
+ # cases mutually exclusive for oneOf.
+ oneOf:
+ - required: [port@0, port@2]
properties:
- endpoint@0:
- required: [usb-role-switch, mux-state]
- endpoint@1:
- required: [usb-role-switch, mux-state]
- else:
- properties:
- ports:
- properties:
- port@0:
- patternProperties:
- '^endpoint@[01]$':
+ port@0:
+ $ref: '#/$defs/hs-port'
properties:
- usb-role-switch: false
- mux-state: false
- - if:
- properties:
- ports:
- properties:
- port@0:
- properties:
- endpoint@0:
+ endpoint:
+ properties:
+ mux-state:
+ const: 0
+ port@2:
+ $ref: '#/$defs/hs-port'
properties:
- mux-state:
- const: 0
- then:
- properties:
- ports:
- properties:
- port@0:
+ endpoint:
+ properties:
+ mux-state:
+ const: 1
+ - required: [port@0, port@2]
properties:
- endpoint@1:
+ port@0:
+ $ref: '#/$defs/hs-port'
properties:
- mux-state:
- const: 1
- - if:
- properties:
- ports:
- properties:
- port@0:
- properties:
- endpoint@1:
+ endpoint:
+ properties:
+ mux-state:
+ const: 1
+ port@2:
+ $ref: '#/$defs/hs-port'
properties:
- mux-state:
- const: 0
- then:
+ endpoint:
+ properties:
+ mux-state:
+ const: 0
+ else:
properties:
ports:
properties:
port@0:
- properties:
- endpoint@0:
+ patternProperties:
+ '^endpoint(@[0-9a-f]+)?$':
properties:
- mux-state:
- const: 1
+ usb-role-switch: false
+ mux-state: false
properties:
port-select-gpios:
@@ -536,4 +542,3 @@ required:
additionalProperties: true
...
-
base-commit: fd69bd54e328c407b1853378b25d15803d3453fa
--
2.53.0
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-11-02 8:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-02 5:41 [PATCH v5 1/2] dt-bindings: usb: snps,dwc3: tighten the two-connector shared HS bus Qianhui Xu
2026-11-02 8:07 ` Krzysztof Kozlowski
-- strict thread matches above, loose matches on Subject: below --
2026-10-31 15:01 [PATCH v5 0/2] usb: dwc3: fixes for two connectors sharing one " Qianhui Xu
2026-10-31 15:01 ` [PATCH v5 1/2] dt-bindings: usb: snps,dwc3: tighten the two-connector shared " Qianhui Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®