Drop the extra port definition: it is not used by the DT files and
there is no correponding physical signal.
Update examples to follow usb-c-connector schema wrt. ports definitions.
Signed-off-by: Dmitry Baryshkov <[email protected]>
---
Dmitry Baryshkov (2):
dt-bindings: usb: qcom,pmic-typec: drop port description
dt-bindings: usb: qcom,pmic-typec: update example to follow connector schema
.../devicetree/bindings/usb/qcom,pmic-typec.yaml | 39 ++++++++++++++--------
1 file changed, 26 insertions(+), 13 deletions(-)
---
base-commit: 226d3c72fcde130a99d760895ebdd20e78e02cb5
change-id: 20240322-typec-fix-example-3d9b1eca853d
Best regards,
--
Dmitry Baryshkov <[email protected]>
The PMIC Type-C controller doesn't have separate role-switching signal.
Instead it has an HS signal connection between embedded USB-C connector
node and the HS port of the USB controller.
Fixes: 00bb478b829e ("dt-bindings: usb: Add Qualcomm PMIC Type-C")
Signed-off-by: Dmitry Baryshkov <[email protected]>
---
Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml | 5 -----
1 file changed, 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml b/Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml
index d9694570c419..63020a88a355 100644
--- a/Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml
+++ b/Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml
@@ -84,11 +84,6 @@ properties:
vdd-pdphy-supply:
description: VDD regulator supply to the PDPHY.
- port:
- $ref: /schemas/graph.yaml#/properties/port
- description:
- Contains a port which produces data-role switching messages.
-
required:
- compatible
- reg
--
2.39.2
On 22/03/2024 11:52, Dmitry Baryshkov wrote:
> The PMIC Type-C controller doesn't have separate role-switching signal.
> Instead it has an HS signal connection between embedded USB-C connector
> node and the HS port of the USB controller.
I take your point on port as a signal but the way type-c determines
data-role is via the DR_Swap message.
https://www.embedded.com/usb-type-c-and-power-delivery-101-power-delivery-protocol/
We receive an IRQ which is a packet containing DR_Swap - TCPM consumes
that data and does a data-role switch.
The port then establishes the link between typec-port and redriver or PHY.
So, I think HS should be dropped from the commit logs and names in both
series.
BTW for the GLINK devices I think the adsp firmware just notifies the
APSS of the data-role switch so, these types of devices probably should
have an epdoint with "usb_role_switch" in the name.
---
bod
On Fri, 22 Mar 2024 at 14:35, Bryan O'Donoghue
<[email protected]> wrote:
>
> On 22/03/2024 11:52, Dmitry Baryshkov wrote:
> > The PMIC Type-C controller doesn't have separate role-switching signal.
> > Instead it has an HS signal connection between embedded USB-C connector
> > node and the HS port of the USB controller.
>
> I take your point on port as a signal but the way type-c determines
> data-role is via the DR_Swap message.
>
> https://www.embedded.com/usb-type-c-and-power-delivery-101-power-delivery-protocol/
>
> We receive an IRQ which is a packet containing DR_Swap - TCPM consumes
> that data and does a data-role switch.
>
> The port then establishes the link between typec-port and redriver or PHY.
>
> So, I think HS should be dropped from the commit logs and names in both
> series.
Then the actual usage doesn't match the schema. usb-c-connector
clearly defines HS, SS and SBU ports
The snps,dwc3.yaml describes ports as ones handling usb-role-switch,
but then clearly writes that port@0 is HS and port@1 is SS. As such, I
think, the correct name for the ports is to have _hs_ in the name
We have pmic-typec/port, separate graph port for role-switching
(supported by TCPM code), but we didn't use it at all on our platforms
(nor do we need it, as we use the HS port).
>
> BTW for the GLINK devices I think the adsp firmware just notifies the
> APSS of the data-role switch so, these types of devices probably should
> have an epdoint with "usb_role_switch" in the name.
>
> ---
> bod
>
--
With best wishes
Dmitry
On 22/03/2024 13:28, Dmitry Baryshkov wrote:
> Then the actual usage doesn't match the schema. usb-c-connector
> clearly defines HS, SS and SBU ports
Its a bit restrictive IMO, data-role and power-role switching is not
limited to HS and in fact can be done with a GPIO for example.
/Looks in Documentation/devicetree/bindings/connector/usb-connector.yaml
Yeah I mean this just doesn't cover all use-cases ..
ports:
$ref: /schemas/graph.yaml#/properties/ports
description: OF graph bindings modeling any data bus to the connector
unless the bus is between parent node and the connector. Since a
single
connector can have multiple data buses every bus has an assigned
OF graph
port number as described below.
properties:
port@0:
$ref: /schemas/graph.yaml#/properties/port
description: High Speed (HS), present in all connectors.
port@1:
$ref: /schemas/graph.yaml#/properties/port
description: Super Speed (SS), present in SS capable connectors.
port@2:
$ref: /schemas/graph.yaml#/properties/port
description: Sideband Use (SBU), present in USB-C. This
describes the
alternate mode connection of which SBU is a part.
TBH I think we should drop this HS, SS stuff from the connector
definition - there's nothing to say in a h/w definition anywhere HS must
be a port or indeed SS - not all hardware knows or cares about different
HS/SS signalling.
Documentation bit-rot
---
bod
On 22/03/2024 15:52, Bryan O'Donoghue wrote:
> On 22/03/2024 13:28, Dmitry Baryshkov wrote:
>> Then the actual usage doesn't match the schema. usb-c-connector
>> clearly defines HS, SS and SBU ports
>
> Its a bit restrictive IMO, data-role and power-role switching is not limited to HS and in fact can be done with a GPIO for example.
>
> /Looks in Documentation/devicetree/bindings/connector/usb-connector.yaml
>
> Yeah I mean this just doesn't cover all use-cases ..
>
> ports:
> $ref: /schemas/graph.yaml#/properties/ports
> description: OF graph bindings modeling any data bus to the connector
> unless the bus is between parent node and the connector. Since a single
> connector can have multiple data buses every bus has an assigned OF graph
> port number as described below.
>
> properties:
> port@0:
> $ref: /schemas/graph.yaml#/properties/port
> description: High Speed (HS), present in all connectors.
>
> port@1:
> $ref: /schemas/graph.yaml#/properties/port
> description: Super Speed (SS), present in SS capable connectors.
>
> port@2:
> $ref: /schemas/graph.yaml#/properties/port
> description: Sideband Use (SBU), present in USB-C. This describes the
> alternate mode connection of which SBU is a part.
>
> TBH I think we should drop this HS, SS stuff from the connector definition - there's nothing to say in a h/w definition anywhere HS must be a port or indeed SS - not all hardware knows or cares about different HS/SS signalling.
It matches the USB-C connector electrical characteristics, which by spec has, at least:
- High-Speed USB Line
- up to 4 differential high-speed lanes that can be switched to DP, USB2 or PCIe
- SideBand line (SBU)
And those 3 components can be handled by 3 different HW in the SoC, so each one has a dedicated port.
Remember DT describes the HW, not the SW implementation.
Neil
>
> Documentation bit-rot
>
> ---
> bod
>
On 22/03/2024 15:09, [email protected] wrote:
>> TBH I think we should drop this HS, SS stuff from the connector
>> definition - there's nothing to say in a h/w definition anywhere HS
>> must be a port or indeed SS - not all hardware knows or cares about
>> different HS/SS signalling.
>
> It matches the USB-C connector electrical characteristics, which by spec
> has, at least:
> - High-Speed USB Line
> - up to 4 differential high-speed lanes that can be switched to DP, USB2
> or PCIe
> - SideBand line (SBU)
>
> And those 3 components can be handled by 3 different HW in the SoC, so
> each one has a dedicated port.
>
> Remember DT describes the HW, not the SW implementation.
>
> Neil
Yes, you're right about that.
I suppose
1. Orientation switches should be defined as coming from a port on the
connector associated with the CC pins.
port@3:
orientation-switch port name goes here
2. Data-role switches...
Again the CC pins
https://community.silabs.com/s/article/what-s-the-role-of-cc-pin-in-type-c-solution?language=en_US
Maybe the right-thing-to-do is to add another port for the CC pins -
which would still describe the hardware characteristics but would
_accurately_ name the thing which does the data-role/orientation switching
CC1/CC2
Then we would not be abusing HS/SS/SBU for the port names - we'd be
extending the connector definition but also naming the ports/endpoints
appropriately associated with the data over the hw
---
bod
On Fri, 22 Mar 2024 at 17:23, Bryan O'Donoghue
<[email protected]> wrote:
>
> On 22/03/2024 15:09, [email protected] wrote:
> >> TBH I think we should drop this HS, SS stuff from the connector
> >> definition - there's nothing to say in a h/w definition anywhere HS
> >> must be a port or indeed SS - not all hardware knows or cares about
> >> different HS/SS signalling.
> >
> > It matches the USB-C connector electrical characteristics, which by spec
> > has, at least:
> > - High-Speed USB Line
> > - up to 4 differential high-speed lanes that can be switched to DP, USB2
> > or PCIe
> > - SideBand line (SBU)
> >
> > And those 3 components can be handled by 3 different HW in the SoC, so
> > each one has a dedicated port.
> >
> > Remember DT describes the HW, not the SW implementation.
> >
> > Neil
>
> Yes, you're right about that.
>
> I suppose
>
> 1. Orientation switches should be defined as coming from a port on the
> connector associated with the CC pins.
> port@3:
> orientation-switch port name goes here
>
> 2. Data-role switches...
> Again the CC pins
>
> https://community.silabs.com/s/article/what-s-the-role-of-cc-pin-in-type-c-solution?language=en_US
>
> Maybe the right-thing-to-do is to add another port for the CC pins -
> which would still describe the hardware characteristics but would
> _accurately_ name the thing which does the data-role/orientation switching
It's true that we don't describe CC lines. However In most of the
cases CC lines are handled by the Type-C port manager directly. So
there will be a connection from usb-c-connector to the pmic-typec
itself (which looks pretty redundant).
The TCPM then sends these events to the interested parties. SS and SBU
chains really care only about orientation (to be able to remux SS
lanes and SBU polarity). USB data role is only relevant for the USB
controller itself.
So either we should add special role-switching link from the TCPM to
USB-controller, or just reuse the HS link.
>
> CC1/CC2
>
> Then we would not be abusing HS/SS/SBU for the port names - we'd be
> extending the connector definition but also naming the ports/endpoints
> appropriately associated with the data over the hw
--
With best wishes
Dmitry
On Fri, Mar 22, 2024 at 05:49:00PM +0200, Dmitry Baryshkov wrote:
> On Fri, 22 Mar 2024 at 17:23, Bryan O'Donoghue
> <[email protected]> wrote:
> >
> > On 22/03/2024 15:09, [email protected] wrote:
> > >> TBH I think we should drop this HS, SS stuff from the connector
> > >> definition - there's nothing to say in a h/w definition anywhere HS
> > >> must be a port or indeed SS - not all hardware knows or cares about
> > >> different HS/SS signalling.
> > >
> > > It matches the USB-C connector electrical characteristics, which by spec
> > > has, at least:
> > > - High-Speed USB Line
> > > - up to 4 differential high-speed lanes that can be switched to DP, USB2
> > > or PCIe
> > > - SideBand line (SBU)
> > >
> > > And those 3 components can be handled by 3 different HW in the SoC, so
> > > each one has a dedicated port.
> > >
> > > Remember DT describes the HW, not the SW implementation.
> > >
> > > Neil
> >
> > Yes, you're right about that.
> >
> > I suppose
> >
> > 1. Orientation switches should be defined as coming from a port on the
> > connector associated with the CC pins.
> > port@3:
> > orientation-switch port name goes here
> >
> > 2. Data-role switches...
> > Again the CC pins
> >
> > https://community.silabs.com/s/article/what-s-the-role-of-cc-pin-in-type-c-solution?language=en_US
> >
> > Maybe the right-thing-to-do is to add another port for the CC pins -
> > which would still describe the hardware characteristics but would
> > _accurately_ name the thing which does the data-role/orientation switching
>
> It's true that we don't describe CC lines. However In most of the
> cases CC lines are handled by the Type-C port manager directly. So
> there will be a connection from usb-c-connector to the pmic-typec
> itself (which looks pretty redundant).
The thought at the time this was designed was that would be the parent
node of the connector. That's perhaps too simple.
Rob
On Fri, 22 Mar 2024 at 20:36, Rob Herring <[email protected]> wrote:
>
> On Fri, Mar 22, 2024 at 05:49:00PM +0200, Dmitry Baryshkov wrote:
> > On Fri, 22 Mar 2024 at 17:23, Bryan O'Donoghue
> > <[email protected]> wrote:
> > >
> > > On 22/03/2024 15:09, [email protected] wrote:
> > > >> TBH I think we should drop this HS, SS stuff from the connector
> > > >> definition - there's nothing to say in a h/w definition anywhere HS
> > > >> must be a port or indeed SS - not all hardware knows or cares about
> > > >> different HS/SS signalling.
> > > >
> > > > It matches the USB-C connector electrical characteristics, which by spec
> > > > has, at least:
> > > > - High-Speed USB Line
> > > > - up to 4 differential high-speed lanes that can be switched to DP, USB2
> > > > or PCIe
> > > > - SideBand line (SBU)
> > > >
> > > > And those 3 components can be handled by 3 different HW in the SoC, so
> > > > each one has a dedicated port.
> > > >
> > > > Remember DT describes the HW, not the SW implementation.
> > > >
> > > > Neil
> > >
> > > Yes, you're right about that.
> > >
> > > I suppose
> > >
> > > 1. Orientation switches should be defined as coming from a port on the
> > > connector associated with the CC pins.
> > > port@3:
> > > orientation-switch port name goes here
> > >
> > > 2. Data-role switches...
> > > Again the CC pins
> > >
> > > https://community.silabs.com/s/article/what-s-the-role-of-cc-pin-in-type-c-solution?language=en_US
> > >
> > > Maybe the right-thing-to-do is to add another port for the CC pins -
> > > which would still describe the hardware characteristics but would
> > > _accurately_ name the thing which does the data-role/orientation switching
> >
> > It's true that we don't describe CC lines. However In most of the
> > cases CC lines are handled by the Type-C port manager directly. So
> > there will be a connection from usb-c-connector to the pmic-typec
> > itself (which looks pretty redundant).
>
> The thought at the time this was designed was that would be the parent
> node of the connector. That's perhaps too simple.
Yep. In both our cases the TCPM is a parent of the usb-c-connector.
--
With best wishes
Dmitry
On 22/03/2024 15:49, Dmitry Baryshkov wrote:
> It's true that we don't describe CC lines. However In most of the
> cases CC lines are handled by the Type-C port manager directly. So
> there will be a connection from usb-c-connector to the pmic-typec
> itself (which looks pretty redundant).
I think it more logical to associate the role-switch event with the CC
lines which actually handle the messaging than the HS PHY which does not
to be honest.
If we predicate a name change on fixing the namespace then we should fix
the namespace instead of reuse existing for expediency.
$0.02
---
bod
On Fri, 22 Mar 2024 at 22:44, Bryan O'Donoghue
<[email protected]> wrote:
>
> On 22/03/2024 15:49, Dmitry Baryshkov wrote:
> > It's true that we don't describe CC lines. However In most of the
> > cases CC lines are handled by the Type-C port manager directly. So
> > there will be a connection from usb-c-connector to the pmic-typec
> > itself (which looks pretty redundant).
>
> I think it more logical to associate the role-switch event with the CC
> lines which actually handle the messaging than the HS PHY which does not
> to be honest.
>
> If we predicate a name change on fixing the namespace then we should fix
> the namespace instead of reuse existing for expediency.
It's not an HS PHY. It is an HS host = DWC3. Anyway, CC lines do not
go to the DWC3. They go directly to the PMIC.
--
With best wishes
Dmitry