| Message ID | 20240322-typec-fix-example-v1-0-6b01c347419e@linaro.org |
|---|---|
| Headers | show |
| Series | dt-bindings: usb: qcom,pmic-typec: OF graph corrections | expand |
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 <bryan.odonoghue@linaro.org> 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 >
On Fri, 22 Mar 2024 at 17:23, Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote: > > On 22/03/2024 15:09, neil.armstrong@linaro.org 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
On Fri, Mar 22, 2024 at 05:49:00PM +0200, Dmitry Baryshkov wrote: > On Fri, 22 Mar 2024 at 17:23, Bryan O'Donoghue > <bryan.odonoghue@linaro.org> wrote: > > > > On 22/03/2024 15:09, neil.armstrong@linaro.org 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 <robh@kernel.org> 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 > > <bryan.odonoghue@linaro.org> wrote: > > > > > > On 22/03/2024 15:09, neil.armstrong@linaro.org 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.
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 <dmitry.baryshkov@linaro.org> --- 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,