* [PATCH 0/2] Add support for Parade PS5169 USB Type-C linear redriver
@ 2026-07-28 1:19 ` Esteban Urrutia
0 siblings, 0 replies; 8+ messages in thread
From: Esteban Urrutia via B4 Relay @ 2026-07-28 1:19 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heikki Krogerus
Cc: linux-usb, devicetree, linux-kernel, phone-devel, Esteban Urrutia
The Parade PS5169 is an USB Type-C linear redriver capable of handling:
- Two-lane USB 3.2 Gen2x2, at 10Gb/s
- Four-lane DP 1.4, at HBR3 (8.1Gb/s)
- Two-lane USB 3.2 Gen2x2, at 10Gb/s AND two-lane DP 1.4, at HBR3
(8.1Gb/s)
This redriver features programmable equalization and gain settings.
This series also future-proofs the binding and adds support for the
Parade PS5170.
Unlike PS5169, PS5170 embeds the AUX switch into the redriver.
The PS5169 redriver is found in the Motorola ThinkPhone by motorola,
device in which this driver was tested on.
Given Parade does not publish datasheets for their redrivers/retimers,
this implementation is based on downstream drivers.
Most of my work is based on [1], while registers were decoded from other
implementations, such as [2] and [3].
[1] https://github.com/LineageOS/android_kernel_motorola_sm8475/blob/lineage-22.2/drivers/usb/misc/ssusb-redriver-ps5169.c
[2] https://github.com/ilsubyeega/tb321fu-zui16/blob/main/kernel-6.1.68/kernel/drivers/usb/redriver/ps5169.c
[3] https://github.com/ferstar/xiaomi_xaga_kernel/blob/mglru/drivers/misc/mediatek/typec/mux/ps5170.c
Signed-off-by: Esteban Urrutia <esteuwu@proton.me>
---
Esteban Urrutia (2):
dt-bindings: usb: Add Parade PS5169 redriver bindings
usb: typec: Add support for Parade PS5169 USB Type-C redriver
.../devicetree/bindings/usb/parade,ps5169.yaml | 186 +++++++
MAINTAINERS | 7 +
drivers/usb/typec/mux/Kconfig | 10 +
drivers/usb/typec/mux/Makefile | 1 +
drivers/usb/typec/mux/ps5169.c | 535 +++++++++++++++++++++
5 files changed, 739 insertions(+)
---
base-commit: 9eebf259d5352b87080d67758f483583d9e763d7
change-id: 20260726-ps5169-bup-8076851dc255
Best regards,
--
Esteban Urrutia <esteuwu@proton.me>
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 0/2] Add support for Parade PS5169 USB Type-C linear redriver @ 2026-07-28 1:19 ` Esteban Urrutia 0 siblings, 0 replies; 8+ messages in thread From: Esteban Urrutia @ 2026-07-28 1:19 UTC (permalink / raw) To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heikki Krogerus Cc: linux-usb, devicetree, linux-kernel, phone-devel, Esteban Urrutia The Parade PS5169 is an USB Type-C linear redriver capable of handling: - Two-lane USB 3.2 Gen2x2, at 10Gb/s - Four-lane DP 1.4, at HBR3 (8.1Gb/s) - Two-lane USB 3.2 Gen2x2, at 10Gb/s AND two-lane DP 1.4, at HBR3 (8.1Gb/s) This redriver features programmable equalization and gain settings. This series also future-proofs the binding and adds support for the Parade PS5170. Unlike PS5169, PS5170 embeds the AUX switch into the redriver. The PS5169 redriver is found in the Motorola ThinkPhone by motorola, device in which this driver was tested on. Given Parade does not publish datasheets for their redrivers/retimers, this implementation is based on downstream drivers. Most of my work is based on [1], while registers were decoded from other implementations, such as [2] and [3]. [1] https://github.com/LineageOS/android_kernel_motorola_sm8475/blob/lineage-22.2/drivers/usb/misc/ssusb-redriver-ps5169.c [2] https://github.com/ilsubyeega/tb321fu-zui16/blob/main/kernel-6.1.68/kernel/drivers/usb/redriver/ps5169.c [3] https://github.com/ferstar/xiaomi_xaga_kernel/blob/mglru/drivers/misc/mediatek/typec/mux/ps5170.c Signed-off-by: Esteban Urrutia <esteuwu@proton.me> --- Esteban Urrutia (2): dt-bindings: usb: Add Parade PS5169 redriver bindings usb: typec: Add support for Parade PS5169 USB Type-C redriver .../devicetree/bindings/usb/parade,ps5169.yaml | 186 +++++++ MAINTAINERS | 7 + drivers/usb/typec/mux/Kconfig | 10 + drivers/usb/typec/mux/Makefile | 1 + drivers/usb/typec/mux/ps5169.c | 535 +++++++++++++++++++++ 5 files changed, 739 insertions(+) --- base-commit: 9eebf259d5352b87080d67758f483583d9e763d7 change-id: 20260726-ps5169-bup-8076851dc255 Best regards, -- Esteban Urrutia <esteuwu@proton.me> ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] dt-bindings: usb: Add Parade PS5169 redriver bindings 2026-07-28 1:19 ` Esteban Urrutia @ 2026-07-28 1:19 ` Esteban Urrutia -1 siblings, 0 replies; 8+ messages in thread From: Esteban Urrutia via B4 Relay @ 2026-07-28 1:19 UTC (permalink / raw) To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heikki Krogerus Cc: linux-usb, devicetree, linux-kernel, phone-devel, Esteban Urrutia From: Esteban Urrutia <esteuwu@proton.me> Add bindings for the Parade PS5169 and PS5170 USB Type-C linear redrivers. The difference between PS5169 and PS5170 is that PS5170 features an embedded AUX switch, while PS5169 does not. Future-proof the binding by adding PS5170 as well. Signed-off-by: Esteban Urrutia <esteuwu@proton.me> --- .../devicetree/bindings/usb/parade,ps5169.yaml | 186 +++++++++++++++++++++ 1 file changed, 186 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/parade,ps5169.yaml b/Documentation/devicetree/bindings/usb/parade,ps5169.yaml new file mode 100644 index 000000000000..23ca8e6c0491 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/parade,ps5169.yaml @@ -0,0 +1,186 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/usb/parade,ps5169.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Parade PS5169 USB Type-C linear redriver + +maintainers: + - Esteban Urrutia <esteuwu@proton.me> + +properties: + compatible: + oneOf: + - items: + - const: parade,ps5170 + - const: parade,ps5169 + - const: parade,ps5169 + + reg: + maxItems: 1 + + vcc-supply: + description: 1.8V power supply + + reset-gpios: + maxItems: 1 + + orientation-switch: true + retimer-switch: true + + parade,dp-eq-level: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Equalization level for DisplayPort channels which is mapped to decibels. + # 0: 2 dB + # 1: 5.5 dB + # 2: 6.5 dB + # 3: 7.5 dB + # 4: 8 dB + # 5: 8.5 dB + # 6: 9.5 dB + # 7: 10 dB + enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ] + + parade,usb-rx-eq-level: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Equalization level for USB receive channels which is mapped to decibels. + # 0: 5.2 dB + # 1: 6 dB + # 2: 7 dB + # 3: 8 dB + # 4: 8.8 dB + # 5: 9.6 dB + # 6: 10.4 dB + # 7: 11.2 dB + enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ] + + parade,usb-tx-eq-level: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Equalization level for USB transmit channels which is mapped to decibels. + # 0: 2 dB + # 1: 5.5 dB + # 2: 6.5 dB + # 3: 7.5 dB + # 4: 8 dB + # 5: 8.5 dB + # 6: 9.5 dB + # 7: 10 dB + enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ] + + parade,aux-rx-data-reverse: + type: boolean + description: + Please check the driver before specifying this property. + Usage of this property is design-specific. + + parade,fine-tune-lfps-swing: + type: boolean + description: + If this property is specified a 50 ohm termination will be set on the USB receive channels. + Usage of this property is design-specific. + + parade,dp-set-gain: + type: boolean + description: + If this property is specified a gain of -0.9 dB will be applied on the DisplayPort channels. + + parade,usb-rx-set-gain: + type: boolean + description: + If this property is specified a gain of -0.9 dB will be applied on the USB receive channels. + + parade,usb-tx-set-gain: + type: boolean + description: + If this property is specified a gain of -0.9 dB will be applied on the USB transmit channels. + + ports: + $ref: /schemas/graph.yaml#/properties/ports + properties: + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: Super Speed (SS) Output endpoint to the Type-C connector + + port@1: + $ref: /schemas/graph.yaml#/$defs/port-base + description: Super Speed (SS) Input endpoint from the Super-Speed PHY + unevaluatedProperties: false + +required: + - compatible + - reg + - vcc-supply + - reset-gpios + - orientation-switch + - retimer-switch + +allOf: + - $ref: usb-switch.yaml# + - $ref: usb-switch-ports.yaml# + - if: + properties: + compatible: + contains: + const: parade,ps5170 + then: + properties: + ports: + properties: + port@2: + $ref: /schemas/graph.yaml#/properties/port + description: + Sideband Use (SBU) AUX lines endpoint to the Type-C connector for the purpose of + handling altmode muxing and orientation switching. + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + + i2c { + #address-cells = <1>; + #size-cells = <0>; + + typec-retimer@30 { + compatible = "parade,ps5169"; + reg = <0x30>; + + vcc-supply = <&vreg_s10b_1p8>; + reset-gpios = <&tlmm 54 GPIO_ACTIVE_LOW>; + + orientation-switch; + retimer-switch; + + parade,dp-eq-level = <0>; /* 2 dB */ + parade,usb-rx-eq-level = <0>; /* 5.2 dB */ + parade,usb-tx-eq-level = <0>; /* 2 dB */ + + parade,aux-rx-data-reverse; + parade,fine-tune-lfps-swing; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + usb_con_ss: endpoint { + remote-endpoint = <&typec_con_ss>; + }; + }; + + port@1 { + reg = <1>; + phy_con_ss: endpoint { + remote-endpoint = <&usb_phy_ss>; + }; + }; + }; + }; + }; +... -- 2.55.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 1/2] dt-bindings: usb: Add Parade PS5169 redriver bindings @ 2026-07-28 1:19 ` Esteban Urrutia 0 siblings, 0 replies; 8+ messages in thread From: Esteban Urrutia @ 2026-07-28 1:19 UTC (permalink / raw) To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heikki Krogerus Cc: linux-usb, devicetree, linux-kernel, phone-devel, Esteban Urrutia Add bindings for the Parade PS5169 and PS5170 USB Type-C linear redrivers. The difference between PS5169 and PS5170 is that PS5170 features an embedded AUX switch, while PS5169 does not. Future-proof the binding by adding PS5170 as well. Signed-off-by: Esteban Urrutia <esteuwu@proton.me> --- .../devicetree/bindings/usb/parade,ps5169.yaml | 186 +++++++++++++++++++++ 1 file changed, 186 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/parade,ps5169.yaml b/Documentation/devicetree/bindings/usb/parade,ps5169.yaml new file mode 100644 index 000000000000..23ca8e6c0491 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/parade,ps5169.yaml @@ -0,0 +1,186 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/usb/parade,ps5169.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Parade PS5169 USB Type-C linear redriver + +maintainers: + - Esteban Urrutia <esteuwu@proton.me> + +properties: + compatible: + oneOf: + - items: + - const: parade,ps5170 + - const: parade,ps5169 + - const: parade,ps5169 + + reg: + maxItems: 1 + + vcc-supply: + description: 1.8V power supply + + reset-gpios: + maxItems: 1 + + orientation-switch: true + retimer-switch: true + + parade,dp-eq-level: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Equalization level for DisplayPort channels which is mapped to decibels. + # 0: 2 dB + # 1: 5.5 dB + # 2: 6.5 dB + # 3: 7.5 dB + # 4: 8 dB + # 5: 8.5 dB + # 6: 9.5 dB + # 7: 10 dB + enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ] + + parade,usb-rx-eq-level: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Equalization level for USB receive channels which is mapped to decibels. + # 0: 5.2 dB + # 1: 6 dB + # 2: 7 dB + # 3: 8 dB + # 4: 8.8 dB + # 5: 9.6 dB + # 6: 10.4 dB + # 7: 11.2 dB + enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ] + + parade,usb-tx-eq-level: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Equalization level for USB transmit channels which is mapped to decibels. + # 0: 2 dB + # 1: 5.5 dB + # 2: 6.5 dB + # 3: 7.5 dB + # 4: 8 dB + # 5: 8.5 dB + # 6: 9.5 dB + # 7: 10 dB + enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ] + + parade,aux-rx-data-reverse: + type: boolean + description: + Please check the driver before specifying this property. + Usage of this property is design-specific. + + parade,fine-tune-lfps-swing: + type: boolean + description: + If this property is specified a 50 ohm termination will be set on the USB receive channels. + Usage of this property is design-specific. + + parade,dp-set-gain: + type: boolean + description: + If this property is specified a gain of -0.9 dB will be applied on the DisplayPort channels. + + parade,usb-rx-set-gain: + type: boolean + description: + If this property is specified a gain of -0.9 dB will be applied on the USB receive channels. + + parade,usb-tx-set-gain: + type: boolean + description: + If this property is specified a gain of -0.9 dB will be applied on the USB transmit channels. + + ports: + $ref: /schemas/graph.yaml#/properties/ports + properties: + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: Super Speed (SS) Output endpoint to the Type-C connector + + port@1: + $ref: /schemas/graph.yaml#/$defs/port-base + description: Super Speed (SS) Input endpoint from the Super-Speed PHY + unevaluatedProperties: false + +required: + - compatible + - reg + - vcc-supply + - reset-gpios + - orientation-switch + - retimer-switch + +allOf: + - $ref: usb-switch.yaml# + - $ref: usb-switch-ports.yaml# + - if: + properties: + compatible: + contains: + const: parade,ps5170 + then: + properties: + ports: + properties: + port@2: + $ref: /schemas/graph.yaml#/properties/port + description: + Sideband Use (SBU) AUX lines endpoint to the Type-C connector for the purpose of + handling altmode muxing and orientation switching. + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + + i2c { + #address-cells = <1>; + #size-cells = <0>; + + typec-retimer@30 { + compatible = "parade,ps5169"; + reg = <0x30>; + + vcc-supply = <&vreg_s10b_1p8>; + reset-gpios = <&tlmm 54 GPIO_ACTIVE_LOW>; + + orientation-switch; + retimer-switch; + + parade,dp-eq-level = <0>; /* 2 dB */ + parade,usb-rx-eq-level = <0>; /* 5.2 dB */ + parade,usb-tx-eq-level = <0>; /* 2 dB */ + + parade,aux-rx-data-reverse; + parade,fine-tune-lfps-swing; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + usb_con_ss: endpoint { + remote-endpoint = <&typec_con_ss>; + }; + }; + + port@1 { + reg = <1>; + phy_con_ss: endpoint { + remote-endpoint = <&usb_phy_ss>; + }; + }; + }; + }; + }; +... -- 2.55.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] dt-bindings: usb: Add Parade PS5169 redriver bindings 2026-07-28 1:19 ` Esteban Urrutia (?) @ 2026-07-28 1:34 ` sashiko-bot -1 siblings, 0 replies; 8+ messages in thread From: sashiko-bot @ 2026-07-28 1:34 UTC (permalink / raw) To: Esteban Urrutia; +Cc: conor+dt, robh, devicetree Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] The schema incorrectly uses `additionalProperties: false` at the top level, rejecting inherited properties from referenced schemas and breaking altmode support for the PS5170 variant. - [Medium] The `ports` object lacks an `unevaluatedProperties: false` constraint, rendering the `if-then` restriction ineffective and silently allowing `port@2` on the PS5169 variant. -- --- Patch [1]: [PATCH 1/2] dt-bindings: usb: Add Parade PS5169 redriver bindings --- commit dc85f2d66c6d67f68f982e513db1687e5793eecf Author: Esteban Urrutia <esteuwu@proton.me> dt-bindings: usb: Add Parade PS5169 redriver bindings Add bindings for the Parade PS5169 and PS5170 USB Type-C linear redrivers. The difference between PS5169 and PS5170 is that PS5170 features an embedded AUX switch, while PS5169 does not. Future-proof the binding by adding PS5170 as well. diff --git a/Documentation/devicetree/bindings/usb/parade,ps5169.yaml b/Documentation/devicetree/bindings/usb/parade,ps5169.yaml new file mode 100644 index 0000000000000..23ca8e6c0491f --- /dev/null +++ b/Documentation/devicetree/bindings/usb/parade,ps5169.yaml [ ... ] > + parade,usb-tx-set-gain: > + type: boolean > + description: > + If this property is specified a gain of -0.9 dB will be applied on the USB transmit channels. > + > + ports: > + $ref: /schemas/graph.yaml#/properties/ports > + properties: [Severity: Medium] Should this ports definition include unevaluatedProperties: false? Without it, any unlisted extra properties are permitted by default. This means the if-then block later in the schema will successfully define port@2 for PS5170, but it won't prevent port@2 from being silently accepted for the base PS5169 device that lacks an AUX switch. > + port@0: > + $ref: /schemas/graph.yaml#/properties/port > + description: Super Speed (SS) Output endpoint to the Type-C connector [ ... ] > +allOf: > + - $ref: usb-switch.yaml# > + - $ref: usb-switch-ports.yaml# > + - if: > + properties: > + compatible: > + contains: > + const: parade,ps5170 > + then: > + properties: > + ports: > + properties: > + port@2: > + $ref: /schemas/graph.yaml#/properties/port > + description: > + Sideband Use (SBU) AUX lines endpoint to the Type-C connector for the purpose of > + handling altmode muxing and orientation switching. > + > +additionalProperties: false [Severity: Medium] Does this need to be unevaluatedProperties: false instead of additionalProperties: false? In JSON schema, additionalProperties only considers the explicitly listed local properties and ignores properties imported via $ref in allOf. Because mode-switch (inherited from usb-switch.yaml) is omitted from the local properties list, additionalProperties: false will reject it. This might break validation for the PS5170 variant acting as an altmode switch. > + > +examples: > + - | [ ... ] -- Sashiko AI review · https://sashiko.dev/#/patchset/20260727-ps5169-bup-v1-0-c76a8ef3b239@proton.me?part=1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] usb: typec: Add support for Parade PS5169 USB Type-C redriver 2026-07-28 1:19 ` Esteban Urrutia @ 2026-07-28 1:19 ` Esteban Urrutia -1 siblings, 0 replies; 8+ messages in thread From: Esteban Urrutia via B4 Relay @ 2026-07-28 1:19 UTC (permalink / raw) To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heikki Krogerus Cc: linux-usb, devicetree, linux-kernel, phone-devel, Esteban Urrutia From: Esteban Urrutia <esteuwu@proton.me> Add driver for the Parade PS5169, an USB Type-C redriver capable of handling 10Gb/s USB signals and 8.1Gb/s DisplayPort signals. Signed-off-by: Esteban Urrutia <esteuwu@proton.me> --- MAINTAINERS | 7 + drivers/usb/typec/mux/Kconfig | 10 + drivers/usb/typec/mux/Makefile | 1 + drivers/usb/typec/mux/ps5169.c | 535 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 553 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index cc3c01eecfa2..3fa3a795c29b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -28277,6 +28277,13 @@ F: drivers/usb/typec/tcpm/tcpm.c F: include/linux/usb/tcpci.h F: include/linux/usb/tcpm.h +USB TYPEC PS5169 MUX DRIVER +M: Esteban Urrutia <esteuwu@proton.me> +L: linux-usb@vger.kernel.org +S: Maintained +F: Documentation/devicetree/bindings/usb/parade,ps5169.yaml +F: drivers/usb/typec/mux/ps5169.c + USB TYPEC TUSB1046 MUX DRIVER M: Romain Gantois <romain.gantois@bootlin.com> L: linux-usb@vger.kernel.org diff --git a/drivers/usb/typec/mux/Kconfig b/drivers/usb/typec/mux/Kconfig index 6dd8f961b593..2838a53ce564 100644 --- a/drivers/usb/typec/mux/Kconfig +++ b/drivers/usb/typec/mux/Kconfig @@ -56,6 +56,16 @@ config TYPEC_MUX_NB7VPQ904M Say Y or M if your system has a On Semiconductor NB7VPQ904M Type-C redriver chip found on some devices with a Type-C port. +config TYPEC_MUX_PS5169 + tristate "Parade PS5169 Type-C redriver driver" + depends on I2C + depends on DRM || DRM=n + select DRM_AUX_BRIDGE if DRM_BRIDGE && OF + select REGMAP_I2C + help + Say Y or M if your system has a Parade PS5169 Type-C redriver chip + found on some devices with a Type-C port. + config TYPEC_MUX_PS883X tristate "Parade PS883x Type-C retimer driver" depends on I2C diff --git a/drivers/usb/typec/mux/Makefile b/drivers/usb/typec/mux/Makefile index b4f599eb5053..c775bfa12068 100644 --- a/drivers/usb/typec/mux/Makefile +++ b/drivers/usb/typec/mux/Makefile @@ -6,6 +6,7 @@ obj-$(CONFIG_TYPEC_MUX_PI3USB30532) += pi3usb30532.o obj-$(CONFIG_TYPEC_MUX_INTEL_PMC) += intel_pmc_mux.o obj-$(CONFIG_TYPEC_MUX_IT5205) += it5205.o obj-$(CONFIG_TYPEC_MUX_NB7VPQ904M) += nb7vpq904m.o +obj-$(CONFIG_TYPEC_MUX_PS5169) += ps5169.o obj-$(CONFIG_TYPEC_MUX_PS883X) += ps883x.o obj-$(CONFIG_TYPEC_MUX_PTN36502) += ptn36502.o obj-$(CONFIG_TYPEC_MUX_TUSB1046) += tusb1046.o diff --git a/drivers/usb/typec/mux/ps5169.c b/drivers/usb/typec/mux/ps5169.c new file mode 100644 index 000000000000..3ee04843fa2d --- /dev/null +++ b/drivers/usb/typec/mux/ps5169.c @@ -0,0 +1,535 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Driver for Parade PS5169 USB Type-C linear redriver + * Registers and bits decoded from downstream drivers + * + * Copyright (c) 2026 Esteban Urrutia <esteuwu@proton.me> + */ + +#include <drm/bridge/aux-bridge.h> + +#include <linux/i2c.h> +#include <linux/regmap.h> + +#include <linux/usb/typec_altmode.h> +#include <linux/usb/typec_dp.h> +#include <linux/usb/typec_mux.h> +#include <linux/usb/typec_retimer.h> + +/* Useful constants */ +#define PS5169_DP_EQ_REG_COUNT 2 +#define PS5169_USB_RX_EQ_REG_COUNT 4 +#define PS5169_USB_TX_EQ_REG_COUNT 3 + +#define PS5169_DP_EQ_LEVEL_COUNT 8 +#define PS5169_USB_RX_EQ_LEVEL_COUNT 8 +#define PS5169_USB_TX_EQ_LEVEL_COUNT 8 + +/* PS5169 registers */ +#define PS5169_CONFIG_REG 0x40 +#define PS5169_GAIN_REG 0x5c +#define PS5169_USB_RX1_LFPS_REG 0x8d +#define PS5169_USB_RX2_LFPS_REG 0x90 +#define PS5169_AUX_RX_DATA_REG 0x9f +#define PS5169_AUX_MONITORING_REG 0xa0 +#define PS5169_HPD_LEVEL_REG 0xa1 +#define PS5169_CHIP_ID_REG 0xac +#define PS5169_CHIP_REVISION_REG 0xae +/* Chip revision is split in two registers */ +#define PS5169_MAX_REG (PS5169_CHIP_REVISION_REG + 1) + +/* PS5169 register bits */ +#define PS5169_CONFIG_REVERSE BIT(4) +#define PS5169_CONFIG_DP_EN BIT(5) +#define PS5169_CONFIG_USB3_EN BIT(6) +#define PS5169_CONFIG_BASE BIT(7) + +#define PS5169_GAIN_DP BIT(4) +#define PS5169_GAIN_USB_RX BIT(2) +#define PS5169_GAIN_USB_TX BIT(0) + +#define PS5169_USB_RX_LFPS_50_OHM BIT(0) + +#define PS5169_AUX_RX_DATA_REVERSE BIT(1) + +#define PS5169_AUX_MONITORING_BIT BIT(1) +#define PS5169_HPD_LEVEL_BIT BIT(2) + +/* PS5169 register values */ +#define PS5169_CHIP_ID 0x6987 + +/* PS5169 register tables */ +static const int ps5169_dp_eq_reg[PS5169_DP_EQ_REG_COUNT] = {0x52, 0x5e}; +static const int ps5169_usb_rx_eq_reg[PS5169_USB_RX_EQ_REG_COUNT] = {0x51, 0x77, 0x54, 0x78}; +static const int ps5169_usb_tx_eq_reg[PS5169_USB_TX_EQ_REG_COUNT] = {0x50, 0x5d, 0x54}; + +/* PS5169 register mask tables */ +static const int ps5169_dp_eq_mask[PS5169_DP_EQ_REG_COUNT] = {0x70, 0x07}; +static const int ps5169_usb_rx_eq_mask[PS5169_USB_RX_EQ_REG_COUNT] = {0xf6, 0xf0, 0x0f, 0xe1}; +static const int ps5169_usb_tx_eq_mask[PS5169_USB_TX_EQ_REG_COUNT] = {0x70, 0x70, 0xf0}; + +/* PS5169 register value tables */ +static const int ps5169_dp_eq[PS5169_DP_EQ_LEVEL_COUNT][PS5169_DP_EQ_REG_COUNT] = { + {0x00, 0x04}, /* 2 dB */ + {0x10, 0x05}, /* 5.5 dB */ + {0x20, 0x06}, /* 6.5 dB */ + {0x30, 0x06}, /* 7.5 dB */ + {0x40, 0x06}, /* 8 dB */ + {0x50, 0x07}, /* 8.5 dB */ + {0x60, 0x07}, /* 9.5 dB */ + {0x70, 0x07}, /* 10 dB, supposedly */ +}; + +static const int ps5169_usb_rx_eq[PS5169_USB_RX_EQ_LEVEL_COUNT][PS5169_USB_RX_EQ_REG_COUNT] = { + {0x86, 0x00, 0x00, 0x20}, /* 5.2 dB */ + {0x96, 0x00, 0x01, 0x20}, /* 6 dB */ + {0xa6, 0x50, 0x01, 0x40}, /* 7 dB */ + {0xb6, 0x50, 0x05, 0x40}, /* 8 dB */ + {0xc6, 0xb0, 0x0c, 0x80}, /* 8.8 dB */ + {0xd6, 0xf0, 0x05, 0x80}, /* 9.6 dB */ + {0xe6, 0xf0, 0x0f, 0x80}, /* 10.4 dB */ + {0xf6, 0x30, 0x0f, 0xa1}, /* 11.2 dB */ +}; + +static const int ps5169_usb_tx_eq[PS5169_USB_TX_EQ_LEVEL_COUNT][PS5169_USB_TX_EQ_REG_COUNT] = { + {0x00, 0x40, 0x00}, /* 2 dB */ + {0x10, 0x50, 0x10}, /* 5.5 dB */ + {0x20, 0x60, 0x10}, /* 6.5 dB */ + {0x30, 0x60, 0x50}, /* 7.5 dB */ + {0x40, 0x60, 0xc0}, /* 8 dB */ + {0x50, 0x70, 0x50}, /* 8.5 dB */ + {0x60, 0x70, 0xf0}, /* 9.5 dB */ + {0x70, 0x70, 0xf0}, /* 10 dB, supposedly */ +}; + +struct redriver { + struct regulator *vcc; + struct gpio_desc *reset_gpio; + + struct regmap *regmap; + + struct typec_switch *typec_switch; + struct typec_mux *typec_mux; + + struct typec_switch_dev *sw; + struct typec_retimer *retimer; + + unsigned long mode; + enum typec_orientation orientation; + + u32 dp_eq; + u32 usb_rx_eq; + u32 usb_tx_eq; + + bool aux_rx_data_reverse; + bool tune_lfps; + + bool dp_set_gain; + bool usb_rx_set_gain; + bool usb_tx_set_gain; + + struct mutex lock; /* protect non-concurrent retimer & switch */ + + unsigned int svid; +}; + +static const struct regmap_config ps5169_regmap = { + .max_register = PS5169_MAX_REG, + .reg_bits = 8, + .val_bits = 8, + /* Accesses only done under ps5169->lock */ + .disable_locking = true, +}; + +static void ps5169_write(struct redriver *ps5169, unsigned int reg, unsigned int val) +{ + regmap_write(ps5169->regmap, reg, val); + + if (val & PS5169_CONFIG_DP_EN) { + /* Clearing this bit will enable the AUX channel */ + regmap_clear_bits(ps5169->regmap, PS5169_AUX_MONITORING_REG, + PS5169_AUX_MONITORING_BIT); + /* Setting this bit will set a high HPD level */ + regmap_set_bits(ps5169->regmap, PS5169_HPD_LEVEL_REG, PS5169_HPD_LEVEL_BIT); + } else { + /* Setting this bit will disable the AUX channel */ + regmap_set_bits(ps5169->regmap, PS5169_AUX_MONITORING_REG, + PS5169_AUX_MONITORING_BIT); + /* Clearing this bit will set a low HPD level */ + regmap_clear_bits(ps5169->regmap, PS5169_HPD_LEVEL_REG, PS5169_HPD_LEVEL_BIT); + } + + if (ps5169->tune_lfps) { + if (val & PS5169_CONFIG_USB3_EN) { + /* Setting these bits will enable a 50 ohm termination on USB RX */ + regmap_set_bits(ps5169->regmap, PS5169_USB_RX1_LFPS_REG, + PS5169_USB_RX_LFPS_50_OHM); + regmap_set_bits(ps5169->regmap, PS5169_USB_RX2_LFPS_REG, + PS5169_USB_RX_LFPS_50_OHM); + } else { + /* Clearing these bits will disable a 50 ohm termination on USB RX */ + regmap_clear_bits(ps5169->regmap, PS5169_USB_RX1_LFPS_REG, + PS5169_USB_RX_LFPS_50_OHM); + regmap_clear_bits(ps5169->regmap, PS5169_USB_RX2_LFPS_REG, + PS5169_USB_RX_LFPS_50_OHM); + } + } +} + +static int ps5169_set(struct redriver *ps5169) +{ + bool reverse = (ps5169->orientation == TYPEC_ORIENTATION_REVERSE); + unsigned int val = PS5169_CONFIG_BASE; + + switch (ps5169->mode) { + case TYPEC_STATE_SAFE: + ps5169_write(ps5169, PS5169_CONFIG_REG, val); + + return 0; + + case TYPEC_STATE_USB: + val |= PS5169_CONFIG_USB3_EN; + if (reverse) + val |= PS5169_CONFIG_REVERSE; + ps5169_write(ps5169, PS5169_CONFIG_REG, val); + + return 0; + + default: + if (ps5169->svid != USB_TYPEC_DP_SID) + return -EINVAL; + + break; + } + + switch (ps5169->mode) { + case TYPEC_DP_STATE_C: + case TYPEC_DP_STATE_E: + val |= PS5169_CONFIG_DP_EN; + if (reverse) + val |= PS5169_CONFIG_REVERSE; + ps5169_write(ps5169, PS5169_CONFIG_REG, val); + + break; + + case TYPEC_DP_STATE_D: + val |= PS5169_CONFIG_DP_EN | PS5169_CONFIG_USB3_EN; + if (reverse) + val |= PS5169_CONFIG_REVERSE; + ps5169_write(ps5169, PS5169_CONFIG_REG, val); + + break; + + default: + return -EOPNOTSUPP; + } + + return 0; +} + +static int ps5169_switch_set(struct typec_switch_dev *sw, enum typec_orientation orientation) +{ + struct redriver *ps5169 = typec_switch_get_drvdata(sw); + int ret; + + ret = typec_switch_set(ps5169->typec_switch, orientation); + if (ret) + return ret; + + mutex_lock(&ps5169->lock); + + if (ps5169->orientation != orientation) { + ps5169->orientation = orientation; + + ret = ps5169_set(ps5169); + } + + mutex_unlock(&ps5169->lock); + + return ret; +} + +static int ps5169_retimer_set(struct typec_retimer *retimer, struct typec_retimer_state *state) +{ + struct redriver *ps5169 = typec_retimer_get_drvdata(retimer); + struct typec_mux_state mux_state; + int ret = 0; + + mutex_lock(&ps5169->lock); + + if (ps5169->mode != state->mode) { + ps5169->mode = state->mode; + + if (state->alt) + ps5169->svid = state->alt->svid; + else + ps5169->svid = 0; // No SVID + + ret = ps5169_set(ps5169); + } + + mutex_unlock(&ps5169->lock); + + if (ret) + return ret; + + mux_state.alt = state->alt; + mux_state.data = state->data; + mux_state.mode = state->mode; + + return typec_mux_set(ps5169->typec_mux, &mux_state); +} + +static void ps5169_init(struct redriver *ps5169) +{ + int i; + + /* DP equalization */ + if (ps5169->dp_eq != U32_MAX) + for (i = 0; i < PS5169_DP_EQ_REG_COUNT; i++) + regmap_update_bits(ps5169->regmap, ps5169_dp_eq_reg[i], + ps5169_dp_eq_mask[i], ps5169_dp_eq[ps5169->dp_eq][i]); + + /* USB RX equalization */ + if (ps5169->usb_rx_eq != U32_MAX) + for (i = 0; i < PS5169_USB_RX_EQ_REG_COUNT; i++) + regmap_update_bits(ps5169->regmap, ps5169_usb_rx_eq_reg[i], + ps5169_usb_rx_eq_mask[i], + ps5169_usb_rx_eq[ps5169->usb_rx_eq][i]); + + /* USB TX equalization */ + if (ps5169->usb_tx_eq != U32_MAX) + for (i = 0; i < PS5169_USB_TX_EQ_REG_COUNT; i++) + regmap_update_bits(ps5169->regmap, ps5169_usb_tx_eq_reg[i], + ps5169_usb_tx_eq_mask[i], + ps5169_usb_tx_eq[ps5169->usb_tx_eq][i]); + + /* Some devices set this bit in this register, but its effect is unknown */ + regmap_assign_bits(ps5169->regmap, PS5169_AUX_RX_DATA_REG, PS5169_AUX_RX_DATA_REVERSE, + ps5169->aux_rx_data_reverse); + + /* If these bits are assigned a -0.9 dB gain will be set on the corresponding channels */ + regmap_assign_bits(ps5169->regmap, PS5169_GAIN_REG, PS5169_GAIN_DP, ps5169->dp_set_gain); + regmap_assign_bits(ps5169->regmap, PS5169_GAIN_REG, PS5169_GAIN_USB_RX, + ps5169->usb_rx_set_gain); + regmap_assign_bits(ps5169->regmap, PS5169_GAIN_REG, PS5169_GAIN_USB_TX, + ps5169->usb_tx_set_gain); +} + +static int ps5169_probe(struct i2c_client *client) +{ + struct redriver *ps5169; + struct device *dev = &client->dev; + int ret; + u16 chip_id; + u16 chip_revision; + struct typec_switch_desc switch_desc = { }; + struct typec_retimer_desc retimer_desc = { }; + + ps5169 = devm_kzalloc(dev, sizeof(*ps5169), GFP_KERNEL); + if (!ps5169) + return -ENOMEM; + + ps5169->vcc = devm_regulator_get(dev, "vcc"); + if (IS_ERR(ps5169->vcc)) + return dev_err_probe(dev, PTR_ERR(ps5169->vcc), "Failed to get vcc-supply\n"); + + ps5169->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW); + if (IS_ERR(ps5169->reset_gpio)) + return dev_err_probe(dev, PTR_ERR(ps5169->reset_gpio), + "Failed to get reset-gpios\n"); + + ps5169->regmap = devm_regmap_init_i2c(client, &ps5169_regmap); + if (IS_ERR(ps5169->regmap)) + return dev_err_probe(dev, PTR_ERR(ps5169->regmap), "Failed to initialize regmap\n"); + + ps5169->typec_switch = typec_switch_get(dev); + if (IS_ERR(ps5169->typec_switch)) + return dev_err_probe(dev, PTR_ERR(ps5169->typec_switch), + "Failed to get orientation-switch\n"); + + ps5169->typec_mux = typec_mux_get(dev); + if (IS_ERR(ps5169->typec_mux)) { + ret = dev_err_probe(dev, PTR_ERR(ps5169->typec_mux), "Failed to get mode-switch\n"); + goto switch_put; + } + + ret = regulator_enable(ps5169->vcc); + if (ret) { + dev_err(dev, "Failed to enable vcc-supply\n"); + goto mux_put; + } + + /* Reset the retimer */ + gpiod_set_value(ps5169->reset_gpio, 1); + usleep_range(1000, 1500); + gpiod_set_value(ps5169->reset_gpio, 0); + usleep_range(10000, 11000); + + ret = regmap_raw_read(ps5169->regmap, PS5169_CHIP_ID_REG, &chip_id, sizeof(chip_id)); + if (ret) { + dev_err(dev, "Failed to read chip ID\n"); + goto vcc_disable; + } + + /* Compare chip IDs */ + if (chip_id != PS5169_CHIP_ID) { + dev_err(dev, "Unexpected chip ID 0x%04x\n", chip_id); + goto vcc_disable; + } + + /* Read chip revision */ + ret = regmap_raw_read(ps5169->regmap, PS5169_CHIP_REVISION_REG, &chip_revision, + sizeof(chip_revision)); + if (ret) { + dev_err(dev, "Failed to read chip revision\n"); + goto vcc_disable; + } + dev_dbg(dev, "Found PS5169 with chip revision 0x%04x\n", chip_revision); + + ret = drm_aux_bridge_register(dev); + if (ret) { + dev_err(dev, "Failed to register aux_bridge\n"); + goto vcc_disable; + } + + /* orientation-switch */ + switch_desc.drvdata = ps5169; + switch_desc.fwnode = dev_fwnode(dev); + switch_desc.set = ps5169_switch_set; + + ps5169->sw = typec_switch_register(dev, &switch_desc); + if (IS_ERR(ps5169->sw)) { + ret = dev_err_probe(dev, PTR_ERR(ps5169->sw), "Failed to register switch\n"); + goto vcc_disable; + } + + /* retimer-switch */ + retimer_desc.drvdata = ps5169; + retimer_desc.fwnode = dev_fwnode(dev); + retimer_desc.set = ps5169_retimer_set; + + ps5169->retimer = typec_retimer_register(dev, &retimer_desc); + if (IS_ERR(ps5169->retimer)) { + ret = dev_err_probe(dev, PTR_ERR(ps5169->retimer), "Failed to register retimer\n"); + goto switch_unregister; + } + + ps5169->mode = TYPEC_STATE_SAFE; + ps5169->orientation = TYPEC_ORIENTATION_NONE; + + /* DP equalization */ + if (device_property_present(dev, "parade,dp-eq-level")) { + ret = device_property_read_u32(dev, "parade,dp-eq-level", &ps5169->dp_eq); + if (ret) { + dev_err(dev, "Failed to read parade,dp-eq-level\n"); + goto switch_unregister; + } + if (ps5169->dp_eq > PS5169_DP_EQ_LEVEL_COUNT - 1) { + dev_err(dev, "parade,dp-eq-level exceeds maximum of %d\n", + PS5169_DP_EQ_LEVEL_COUNT - 1); + ret = -EINVAL; + goto switch_unregister; + } + } else { + dev_warn(dev, "parade,dp-eq-level is not present, leaving DP EQ as is\n"); + ps5169->dp_eq = U32_MAX; + } + + /* USB RX equalization */ + if (device_property_present(dev, "parade,usb-rx-eq-level")) { + ret = device_property_read_u32(dev, "parade,usb-rx-eq-level", &ps5169->usb_rx_eq); + if (ret) { + dev_err(dev, "Failed to read parade,usb-rx-eq-level\n"); + goto switch_unregister; + } + if (ps5169->usb_rx_eq > PS5169_USB_RX_EQ_LEVEL_COUNT - 1) { + dev_err(dev, "parade,usb-rx-eq-level exceeds maximum of %d\n", + PS5169_USB_RX_EQ_LEVEL_COUNT - 1); + ret = -EINVAL; + goto switch_unregister; + } + } else { + dev_warn(dev, "parade,usb-rx-eq-level is not present, leaving USB RX EQ as is\n"); + ps5169->usb_rx_eq = U32_MAX; + } + + /* USB TX equalization */ + if (device_property_present(dev, "parade,usb-tx-eq-level")) { + ret = device_property_read_u32(dev, "parade,usb-tx-eq-level", &ps5169->usb_tx_eq); + if (ret) { + dev_err(dev, "Failed to read parade,usb-tx-eq-level\n"); + goto switch_unregister; + } + if (ps5169->usb_tx_eq > PS5169_USB_TX_EQ_LEVEL_COUNT - 1) { + dev_err(dev, "parade,usb-tx-eq-level exceeds maximum of %d\n", + PS5169_USB_TX_EQ_LEVEL_COUNT - 1); + ret = -EINVAL; + goto switch_unregister; + } + } else { + dev_warn(dev, "parade,usb-tx-eq-level is not present, leaving USB TX EQ as is\n"); + ps5169->usb_tx_eq = U32_MAX; + } + + /* Read properties from DT */ + ps5169->aux_rx_data_reverse = device_property_read_bool(dev, "parade,aux-rx-data-reverse"); + ps5169->tune_lfps = device_property_read_bool(dev, "parade,fine-tune-lfps-swing"); + ps5169->dp_set_gain = device_property_read_bool(dev, "parade,dp-set-gain"); + ps5169->usb_rx_set_gain = device_property_read_bool(dev, "parade,usb-rx-set-gain"); + ps5169->usb_tx_set_gain = device_property_read_bool(dev, "parade,usb-tx-set-gain"); + + ps5169_init(ps5169); + + mutex_init(&ps5169->lock); + + i2c_set_clientdata(client, ps5169); + + return 0; + +switch_unregister: + typec_switch_unregister(ps5169->sw); + +vcc_disable: + regulator_disable(ps5169->vcc); + +mux_put: + typec_mux_put(ps5169->typec_mux); +switch_put: + typec_switch_put(ps5169->typec_switch); + + return ret; +} + +static void ps5169_remove(struct i2c_client *client) +{ + struct redriver *ps5169 = i2c_get_clientdata(client); + + typec_retimer_unregister(ps5169->retimer); + typec_switch_unregister(ps5169->sw); + + regulator_disable(ps5169->vcc); + + typec_mux_put(ps5169->typec_mux); + typec_switch_put(ps5169->typec_switch); +} + +static const struct of_device_id ps5169_of_match_table[] = { + { .compatible = "parade,ps5169" }, + { }, +}; + +static struct i2c_driver ps5169_driver = { + .driver = { + .name = "ps5169", + .of_match_table = ps5169_of_match_table, + }, + .probe = ps5169_probe, + .remove = ps5169_remove, +}; + +module_i2c_driver(ps5169_driver); + +MODULE_AUTHOR("Esteban Urrutia <esteuwu@proton.me>"); +MODULE_DESCRIPTION("Driver for Parade PS5169 USB Type-C linear redriver"); +MODULE_LICENSE("GPL"); -- 2.55.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] usb: typec: Add support for Parade PS5169 USB Type-C redriver @ 2026-07-28 1:19 ` Esteban Urrutia 0 siblings, 0 replies; 8+ messages in thread From: Esteban Urrutia @ 2026-07-28 1:19 UTC (permalink / raw) To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heikki Krogerus Cc: linux-usb, devicetree, linux-kernel, phone-devel, Esteban Urrutia Add driver for the Parade PS5169, an USB Type-C redriver capable of handling 10Gb/s USB signals and 8.1Gb/s DisplayPort signals. Signed-off-by: Esteban Urrutia <esteuwu@proton.me> --- MAINTAINERS | 7 + drivers/usb/typec/mux/Kconfig | 10 + drivers/usb/typec/mux/Makefile | 1 + drivers/usb/typec/mux/ps5169.c | 535 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 553 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index cc3c01eecfa2..3fa3a795c29b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -28277,6 +28277,13 @@ F: drivers/usb/typec/tcpm/tcpm.c F: include/linux/usb/tcpci.h F: include/linux/usb/tcpm.h +USB TYPEC PS5169 MUX DRIVER +M: Esteban Urrutia <esteuwu@proton.me> +L: linux-usb@vger.kernel.org +S: Maintained +F: Documentation/devicetree/bindings/usb/parade,ps5169.yaml +F: drivers/usb/typec/mux/ps5169.c + USB TYPEC TUSB1046 MUX DRIVER M: Romain Gantois <romain.gantois@bootlin.com> L: linux-usb@vger.kernel.org diff --git a/drivers/usb/typec/mux/Kconfig b/drivers/usb/typec/mux/Kconfig index 6dd8f961b593..2838a53ce564 100644 --- a/drivers/usb/typec/mux/Kconfig +++ b/drivers/usb/typec/mux/Kconfig @@ -56,6 +56,16 @@ config TYPEC_MUX_NB7VPQ904M Say Y or M if your system has a On Semiconductor NB7VPQ904M Type-C redriver chip found on some devices with a Type-C port. +config TYPEC_MUX_PS5169 + tristate "Parade PS5169 Type-C redriver driver" + depends on I2C + depends on DRM || DRM=n + select DRM_AUX_BRIDGE if DRM_BRIDGE && OF + select REGMAP_I2C + help + Say Y or M if your system has a Parade PS5169 Type-C redriver chip + found on some devices with a Type-C port. + config TYPEC_MUX_PS883X tristate "Parade PS883x Type-C retimer driver" depends on I2C diff --git a/drivers/usb/typec/mux/Makefile b/drivers/usb/typec/mux/Makefile index b4f599eb5053..c775bfa12068 100644 --- a/drivers/usb/typec/mux/Makefile +++ b/drivers/usb/typec/mux/Makefile @@ -6,6 +6,7 @@ obj-$(CONFIG_TYPEC_MUX_PI3USB30532) += pi3usb30532.o obj-$(CONFIG_TYPEC_MUX_INTEL_PMC) += intel_pmc_mux.o obj-$(CONFIG_TYPEC_MUX_IT5205) += it5205.o obj-$(CONFIG_TYPEC_MUX_NB7VPQ904M) += nb7vpq904m.o +obj-$(CONFIG_TYPEC_MUX_PS5169) += ps5169.o obj-$(CONFIG_TYPEC_MUX_PS883X) += ps883x.o obj-$(CONFIG_TYPEC_MUX_PTN36502) += ptn36502.o obj-$(CONFIG_TYPEC_MUX_TUSB1046) += tusb1046.o diff --git a/drivers/usb/typec/mux/ps5169.c b/drivers/usb/typec/mux/ps5169.c new file mode 100644 index 000000000000..3ee04843fa2d --- /dev/null +++ b/drivers/usb/typec/mux/ps5169.c @@ -0,0 +1,535 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Driver for Parade PS5169 USB Type-C linear redriver + * Registers and bits decoded from downstream drivers + * + * Copyright (c) 2026 Esteban Urrutia <esteuwu@proton.me> + */ + +#include <drm/bridge/aux-bridge.h> + +#include <linux/i2c.h> +#include <linux/regmap.h> + +#include <linux/usb/typec_altmode.h> +#include <linux/usb/typec_dp.h> +#include <linux/usb/typec_mux.h> +#include <linux/usb/typec_retimer.h> + +/* Useful constants */ +#define PS5169_DP_EQ_REG_COUNT 2 +#define PS5169_USB_RX_EQ_REG_COUNT 4 +#define PS5169_USB_TX_EQ_REG_COUNT 3 + +#define PS5169_DP_EQ_LEVEL_COUNT 8 +#define PS5169_USB_RX_EQ_LEVEL_COUNT 8 +#define PS5169_USB_TX_EQ_LEVEL_COUNT 8 + +/* PS5169 registers */ +#define PS5169_CONFIG_REG 0x40 +#define PS5169_GAIN_REG 0x5c +#define PS5169_USB_RX1_LFPS_REG 0x8d +#define PS5169_USB_RX2_LFPS_REG 0x90 +#define PS5169_AUX_RX_DATA_REG 0x9f +#define PS5169_AUX_MONITORING_REG 0xa0 +#define PS5169_HPD_LEVEL_REG 0xa1 +#define PS5169_CHIP_ID_REG 0xac +#define PS5169_CHIP_REVISION_REG 0xae +/* Chip revision is split in two registers */ +#define PS5169_MAX_REG (PS5169_CHIP_REVISION_REG + 1) + +/* PS5169 register bits */ +#define PS5169_CONFIG_REVERSE BIT(4) +#define PS5169_CONFIG_DP_EN BIT(5) +#define PS5169_CONFIG_USB3_EN BIT(6) +#define PS5169_CONFIG_BASE BIT(7) + +#define PS5169_GAIN_DP BIT(4) +#define PS5169_GAIN_USB_RX BIT(2) +#define PS5169_GAIN_USB_TX BIT(0) + +#define PS5169_USB_RX_LFPS_50_OHM BIT(0) + +#define PS5169_AUX_RX_DATA_REVERSE BIT(1) + +#define PS5169_AUX_MONITORING_BIT BIT(1) +#define PS5169_HPD_LEVEL_BIT BIT(2) + +/* PS5169 register values */ +#define PS5169_CHIP_ID 0x6987 + +/* PS5169 register tables */ +static const int ps5169_dp_eq_reg[PS5169_DP_EQ_REG_COUNT] = {0x52, 0x5e}; +static const int ps5169_usb_rx_eq_reg[PS5169_USB_RX_EQ_REG_COUNT] = {0x51, 0x77, 0x54, 0x78}; +static const int ps5169_usb_tx_eq_reg[PS5169_USB_TX_EQ_REG_COUNT] = {0x50, 0x5d, 0x54}; + +/* PS5169 register mask tables */ +static const int ps5169_dp_eq_mask[PS5169_DP_EQ_REG_COUNT] = {0x70, 0x07}; +static const int ps5169_usb_rx_eq_mask[PS5169_USB_RX_EQ_REG_COUNT] = {0xf6, 0xf0, 0x0f, 0xe1}; +static const int ps5169_usb_tx_eq_mask[PS5169_USB_TX_EQ_REG_COUNT] = {0x70, 0x70, 0xf0}; + +/* PS5169 register value tables */ +static const int ps5169_dp_eq[PS5169_DP_EQ_LEVEL_COUNT][PS5169_DP_EQ_REG_COUNT] = { + {0x00, 0x04}, /* 2 dB */ + {0x10, 0x05}, /* 5.5 dB */ + {0x20, 0x06}, /* 6.5 dB */ + {0x30, 0x06}, /* 7.5 dB */ + {0x40, 0x06}, /* 8 dB */ + {0x50, 0x07}, /* 8.5 dB */ + {0x60, 0x07}, /* 9.5 dB */ + {0x70, 0x07}, /* 10 dB, supposedly */ +}; + +static const int ps5169_usb_rx_eq[PS5169_USB_RX_EQ_LEVEL_COUNT][PS5169_USB_RX_EQ_REG_COUNT] = { + {0x86, 0x00, 0x00, 0x20}, /* 5.2 dB */ + {0x96, 0x00, 0x01, 0x20}, /* 6 dB */ + {0xa6, 0x50, 0x01, 0x40}, /* 7 dB */ + {0xb6, 0x50, 0x05, 0x40}, /* 8 dB */ + {0xc6, 0xb0, 0x0c, 0x80}, /* 8.8 dB */ + {0xd6, 0xf0, 0x05, 0x80}, /* 9.6 dB */ + {0xe6, 0xf0, 0x0f, 0x80}, /* 10.4 dB */ + {0xf6, 0x30, 0x0f, 0xa1}, /* 11.2 dB */ +}; + +static const int ps5169_usb_tx_eq[PS5169_USB_TX_EQ_LEVEL_COUNT][PS5169_USB_TX_EQ_REG_COUNT] = { + {0x00, 0x40, 0x00}, /* 2 dB */ + {0x10, 0x50, 0x10}, /* 5.5 dB */ + {0x20, 0x60, 0x10}, /* 6.5 dB */ + {0x30, 0x60, 0x50}, /* 7.5 dB */ + {0x40, 0x60, 0xc0}, /* 8 dB */ + {0x50, 0x70, 0x50}, /* 8.5 dB */ + {0x60, 0x70, 0xf0}, /* 9.5 dB */ + {0x70, 0x70, 0xf0}, /* 10 dB, supposedly */ +}; + +struct redriver { + struct regulator *vcc; + struct gpio_desc *reset_gpio; + + struct regmap *regmap; + + struct typec_switch *typec_switch; + struct typec_mux *typec_mux; + + struct typec_switch_dev *sw; + struct typec_retimer *retimer; + + unsigned long mode; + enum typec_orientation orientation; + + u32 dp_eq; + u32 usb_rx_eq; + u32 usb_tx_eq; + + bool aux_rx_data_reverse; + bool tune_lfps; + + bool dp_set_gain; + bool usb_rx_set_gain; + bool usb_tx_set_gain; + + struct mutex lock; /* protect non-concurrent retimer & switch */ + + unsigned int svid; +}; + +static const struct regmap_config ps5169_regmap = { + .max_register = PS5169_MAX_REG, + .reg_bits = 8, + .val_bits = 8, + /* Accesses only done under ps5169->lock */ + .disable_locking = true, +}; + +static void ps5169_write(struct redriver *ps5169, unsigned int reg, unsigned int val) +{ + regmap_write(ps5169->regmap, reg, val); + + if (val & PS5169_CONFIG_DP_EN) { + /* Clearing this bit will enable the AUX channel */ + regmap_clear_bits(ps5169->regmap, PS5169_AUX_MONITORING_REG, + PS5169_AUX_MONITORING_BIT); + /* Setting this bit will set a high HPD level */ + regmap_set_bits(ps5169->regmap, PS5169_HPD_LEVEL_REG, PS5169_HPD_LEVEL_BIT); + } else { + /* Setting this bit will disable the AUX channel */ + regmap_set_bits(ps5169->regmap, PS5169_AUX_MONITORING_REG, + PS5169_AUX_MONITORING_BIT); + /* Clearing this bit will set a low HPD level */ + regmap_clear_bits(ps5169->regmap, PS5169_HPD_LEVEL_REG, PS5169_HPD_LEVEL_BIT); + } + + if (ps5169->tune_lfps) { + if (val & PS5169_CONFIG_USB3_EN) { + /* Setting these bits will enable a 50 ohm termination on USB RX */ + regmap_set_bits(ps5169->regmap, PS5169_USB_RX1_LFPS_REG, + PS5169_USB_RX_LFPS_50_OHM); + regmap_set_bits(ps5169->regmap, PS5169_USB_RX2_LFPS_REG, + PS5169_USB_RX_LFPS_50_OHM); + } else { + /* Clearing these bits will disable a 50 ohm termination on USB RX */ + regmap_clear_bits(ps5169->regmap, PS5169_USB_RX1_LFPS_REG, + PS5169_USB_RX_LFPS_50_OHM); + regmap_clear_bits(ps5169->regmap, PS5169_USB_RX2_LFPS_REG, + PS5169_USB_RX_LFPS_50_OHM); + } + } +} + +static int ps5169_set(struct redriver *ps5169) +{ + bool reverse = (ps5169->orientation == TYPEC_ORIENTATION_REVERSE); + unsigned int val = PS5169_CONFIG_BASE; + + switch (ps5169->mode) { + case TYPEC_STATE_SAFE: + ps5169_write(ps5169, PS5169_CONFIG_REG, val); + + return 0; + + case TYPEC_STATE_USB: + val |= PS5169_CONFIG_USB3_EN; + if (reverse) + val |= PS5169_CONFIG_REVERSE; + ps5169_write(ps5169, PS5169_CONFIG_REG, val); + + return 0; + + default: + if (ps5169->svid != USB_TYPEC_DP_SID) + return -EINVAL; + + break; + } + + switch (ps5169->mode) { + case TYPEC_DP_STATE_C: + case TYPEC_DP_STATE_E: + val |= PS5169_CONFIG_DP_EN; + if (reverse) + val |= PS5169_CONFIG_REVERSE; + ps5169_write(ps5169, PS5169_CONFIG_REG, val); + + break; + + case TYPEC_DP_STATE_D: + val |= PS5169_CONFIG_DP_EN | PS5169_CONFIG_USB3_EN; + if (reverse) + val |= PS5169_CONFIG_REVERSE; + ps5169_write(ps5169, PS5169_CONFIG_REG, val); + + break; + + default: + return -EOPNOTSUPP; + } + + return 0; +} + +static int ps5169_switch_set(struct typec_switch_dev *sw, enum typec_orientation orientation) +{ + struct redriver *ps5169 = typec_switch_get_drvdata(sw); + int ret; + + ret = typec_switch_set(ps5169->typec_switch, orientation); + if (ret) + return ret; + + mutex_lock(&ps5169->lock); + + if (ps5169->orientation != orientation) { + ps5169->orientation = orientation; + + ret = ps5169_set(ps5169); + } + + mutex_unlock(&ps5169->lock); + + return ret; +} + +static int ps5169_retimer_set(struct typec_retimer *retimer, struct typec_retimer_state *state) +{ + struct redriver *ps5169 = typec_retimer_get_drvdata(retimer); + struct typec_mux_state mux_state; + int ret = 0; + + mutex_lock(&ps5169->lock); + + if (ps5169->mode != state->mode) { + ps5169->mode = state->mode; + + if (state->alt) + ps5169->svid = state->alt->svid; + else + ps5169->svid = 0; // No SVID + + ret = ps5169_set(ps5169); + } + + mutex_unlock(&ps5169->lock); + + if (ret) + return ret; + + mux_state.alt = state->alt; + mux_state.data = state->data; + mux_state.mode = state->mode; + + return typec_mux_set(ps5169->typec_mux, &mux_state); +} + +static void ps5169_init(struct redriver *ps5169) +{ + int i; + + /* DP equalization */ + if (ps5169->dp_eq != U32_MAX) + for (i = 0; i < PS5169_DP_EQ_REG_COUNT; i++) + regmap_update_bits(ps5169->regmap, ps5169_dp_eq_reg[i], + ps5169_dp_eq_mask[i], ps5169_dp_eq[ps5169->dp_eq][i]); + + /* USB RX equalization */ + if (ps5169->usb_rx_eq != U32_MAX) + for (i = 0; i < PS5169_USB_RX_EQ_REG_COUNT; i++) + regmap_update_bits(ps5169->regmap, ps5169_usb_rx_eq_reg[i], + ps5169_usb_rx_eq_mask[i], + ps5169_usb_rx_eq[ps5169->usb_rx_eq][i]); + + /* USB TX equalization */ + if (ps5169->usb_tx_eq != U32_MAX) + for (i = 0; i < PS5169_USB_TX_EQ_REG_COUNT; i++) + regmap_update_bits(ps5169->regmap, ps5169_usb_tx_eq_reg[i], + ps5169_usb_tx_eq_mask[i], + ps5169_usb_tx_eq[ps5169->usb_tx_eq][i]); + + /* Some devices set this bit in this register, but its effect is unknown */ + regmap_assign_bits(ps5169->regmap, PS5169_AUX_RX_DATA_REG, PS5169_AUX_RX_DATA_REVERSE, + ps5169->aux_rx_data_reverse); + + /* If these bits are assigned a -0.9 dB gain will be set on the corresponding channels */ + regmap_assign_bits(ps5169->regmap, PS5169_GAIN_REG, PS5169_GAIN_DP, ps5169->dp_set_gain); + regmap_assign_bits(ps5169->regmap, PS5169_GAIN_REG, PS5169_GAIN_USB_RX, + ps5169->usb_rx_set_gain); + regmap_assign_bits(ps5169->regmap, PS5169_GAIN_REG, PS5169_GAIN_USB_TX, + ps5169->usb_tx_set_gain); +} + +static int ps5169_probe(struct i2c_client *client) +{ + struct redriver *ps5169; + struct device *dev = &client->dev; + int ret; + u16 chip_id; + u16 chip_revision; + struct typec_switch_desc switch_desc = { }; + struct typec_retimer_desc retimer_desc = { }; + + ps5169 = devm_kzalloc(dev, sizeof(*ps5169), GFP_KERNEL); + if (!ps5169) + return -ENOMEM; + + ps5169->vcc = devm_regulator_get(dev, "vcc"); + if (IS_ERR(ps5169->vcc)) + return dev_err_probe(dev, PTR_ERR(ps5169->vcc), "Failed to get vcc-supply\n"); + + ps5169->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW); + if (IS_ERR(ps5169->reset_gpio)) + return dev_err_probe(dev, PTR_ERR(ps5169->reset_gpio), + "Failed to get reset-gpios\n"); + + ps5169->regmap = devm_regmap_init_i2c(client, &ps5169_regmap); + if (IS_ERR(ps5169->regmap)) + return dev_err_probe(dev, PTR_ERR(ps5169->regmap), "Failed to initialize regmap\n"); + + ps5169->typec_switch = typec_switch_get(dev); + if (IS_ERR(ps5169->typec_switch)) + return dev_err_probe(dev, PTR_ERR(ps5169->typec_switch), + "Failed to get orientation-switch\n"); + + ps5169->typec_mux = typec_mux_get(dev); + if (IS_ERR(ps5169->typec_mux)) { + ret = dev_err_probe(dev, PTR_ERR(ps5169->typec_mux), "Failed to get mode-switch\n"); + goto switch_put; + } + + ret = regulator_enable(ps5169->vcc); + if (ret) { + dev_err(dev, "Failed to enable vcc-supply\n"); + goto mux_put; + } + + /* Reset the retimer */ + gpiod_set_value(ps5169->reset_gpio, 1); + usleep_range(1000, 1500); + gpiod_set_value(ps5169->reset_gpio, 0); + usleep_range(10000, 11000); + + ret = regmap_raw_read(ps5169->regmap, PS5169_CHIP_ID_REG, &chip_id, sizeof(chip_id)); + if (ret) { + dev_err(dev, "Failed to read chip ID\n"); + goto vcc_disable; + } + + /* Compare chip IDs */ + if (chip_id != PS5169_CHIP_ID) { + dev_err(dev, "Unexpected chip ID 0x%04x\n", chip_id); + goto vcc_disable; + } + + /* Read chip revision */ + ret = regmap_raw_read(ps5169->regmap, PS5169_CHIP_REVISION_REG, &chip_revision, + sizeof(chip_revision)); + if (ret) { + dev_err(dev, "Failed to read chip revision\n"); + goto vcc_disable; + } + dev_dbg(dev, "Found PS5169 with chip revision 0x%04x\n", chip_revision); + + ret = drm_aux_bridge_register(dev); + if (ret) { + dev_err(dev, "Failed to register aux_bridge\n"); + goto vcc_disable; + } + + /* orientation-switch */ + switch_desc.drvdata = ps5169; + switch_desc.fwnode = dev_fwnode(dev); + switch_desc.set = ps5169_switch_set; + + ps5169->sw = typec_switch_register(dev, &switch_desc); + if (IS_ERR(ps5169->sw)) { + ret = dev_err_probe(dev, PTR_ERR(ps5169->sw), "Failed to register switch\n"); + goto vcc_disable; + } + + /* retimer-switch */ + retimer_desc.drvdata = ps5169; + retimer_desc.fwnode = dev_fwnode(dev); + retimer_desc.set = ps5169_retimer_set; + + ps5169->retimer = typec_retimer_register(dev, &retimer_desc); + if (IS_ERR(ps5169->retimer)) { + ret = dev_err_probe(dev, PTR_ERR(ps5169->retimer), "Failed to register retimer\n"); + goto switch_unregister; + } + + ps5169->mode = TYPEC_STATE_SAFE; + ps5169->orientation = TYPEC_ORIENTATION_NONE; + + /* DP equalization */ + if (device_property_present(dev, "parade,dp-eq-level")) { + ret = device_property_read_u32(dev, "parade,dp-eq-level", &ps5169->dp_eq); + if (ret) { + dev_err(dev, "Failed to read parade,dp-eq-level\n"); + goto switch_unregister; + } + if (ps5169->dp_eq > PS5169_DP_EQ_LEVEL_COUNT - 1) { + dev_err(dev, "parade,dp-eq-level exceeds maximum of %d\n", + PS5169_DP_EQ_LEVEL_COUNT - 1); + ret = -EINVAL; + goto switch_unregister; + } + } else { + dev_warn(dev, "parade,dp-eq-level is not present, leaving DP EQ as is\n"); + ps5169->dp_eq = U32_MAX; + } + + /* USB RX equalization */ + if (device_property_present(dev, "parade,usb-rx-eq-level")) { + ret = device_property_read_u32(dev, "parade,usb-rx-eq-level", &ps5169->usb_rx_eq); + if (ret) { + dev_err(dev, "Failed to read parade,usb-rx-eq-level\n"); + goto switch_unregister; + } + if (ps5169->usb_rx_eq > PS5169_USB_RX_EQ_LEVEL_COUNT - 1) { + dev_err(dev, "parade,usb-rx-eq-level exceeds maximum of %d\n", + PS5169_USB_RX_EQ_LEVEL_COUNT - 1); + ret = -EINVAL; + goto switch_unregister; + } + } else { + dev_warn(dev, "parade,usb-rx-eq-level is not present, leaving USB RX EQ as is\n"); + ps5169->usb_rx_eq = U32_MAX; + } + + /* USB TX equalization */ + if (device_property_present(dev, "parade,usb-tx-eq-level")) { + ret = device_property_read_u32(dev, "parade,usb-tx-eq-level", &ps5169->usb_tx_eq); + if (ret) { + dev_err(dev, "Failed to read parade,usb-tx-eq-level\n"); + goto switch_unregister; + } + if (ps5169->usb_tx_eq > PS5169_USB_TX_EQ_LEVEL_COUNT - 1) { + dev_err(dev, "parade,usb-tx-eq-level exceeds maximum of %d\n", + PS5169_USB_TX_EQ_LEVEL_COUNT - 1); + ret = -EINVAL; + goto switch_unregister; + } + } else { + dev_warn(dev, "parade,usb-tx-eq-level is not present, leaving USB TX EQ as is\n"); + ps5169->usb_tx_eq = U32_MAX; + } + + /* Read properties from DT */ + ps5169->aux_rx_data_reverse = device_property_read_bool(dev, "parade,aux-rx-data-reverse"); + ps5169->tune_lfps = device_property_read_bool(dev, "parade,fine-tune-lfps-swing"); + ps5169->dp_set_gain = device_property_read_bool(dev, "parade,dp-set-gain"); + ps5169->usb_rx_set_gain = device_property_read_bool(dev, "parade,usb-rx-set-gain"); + ps5169->usb_tx_set_gain = device_property_read_bool(dev, "parade,usb-tx-set-gain"); + + ps5169_init(ps5169); + + mutex_init(&ps5169->lock); + + i2c_set_clientdata(client, ps5169); + + return 0; + +switch_unregister: + typec_switch_unregister(ps5169->sw); + +vcc_disable: + regulator_disable(ps5169->vcc); + +mux_put: + typec_mux_put(ps5169->typec_mux); +switch_put: + typec_switch_put(ps5169->typec_switch); + + return ret; +} + +static void ps5169_remove(struct i2c_client *client) +{ + struct redriver *ps5169 = i2c_get_clientdata(client); + + typec_retimer_unregister(ps5169->retimer); + typec_switch_unregister(ps5169->sw); + + regulator_disable(ps5169->vcc); + + typec_mux_put(ps5169->typec_mux); + typec_switch_put(ps5169->typec_switch); +} + +static const struct of_device_id ps5169_of_match_table[] = { + { .compatible = "parade,ps5169" }, + { }, +}; + +static struct i2c_driver ps5169_driver = { + .driver = { + .name = "ps5169", + .of_match_table = ps5169_of_match_table, + }, + .probe = ps5169_probe, + .remove = ps5169_remove, +}; + +module_i2c_driver(ps5169_driver); + +MODULE_AUTHOR("Esteban Urrutia <esteuwu@proton.me>"); +MODULE_DESCRIPTION("Driver for Parade PS5169 USB Type-C linear redriver"); +MODULE_LICENSE("GPL"); -- 2.55.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] usb: typec: Add support for Parade PS5169 USB Type-C redriver 2026-07-28 1:19 ` Esteban Urrutia (?) @ 2026-07-28 1:36 ` sashiko-bot -1 siblings, 0 replies; 8+ messages in thread From: sashiko-bot @ 2026-07-28 1:36 UTC (permalink / raw) To: Esteban Urrutia; +Cc: conor+dt, devicetree, robh Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider: - [High] Race condition due to early device registration before hardware and mutex initialization. - [High] Type-C retimer device is leaked in the `ps5169_probe` error paths, leading to Use-After-Free. - [High] Probe returns 0 (success) on invalid chip ID, causing double-frees during removal. - [Medium] Software state cache is updated before hardware programming and validation, leading to state desynchronization. - [Medium] Reading raw I2C bytes into a native `u16` using `regmap_raw_read` causes endianness mismatches on big-endian architectures. - [Medium] The driver is missing `MODULE_DEVICE_TABLE` for device tree matching, preventing auto-loading. -- --- Patch [2]: [PATCH 2/2] usb: typec: Add support for Parade PS5169 USB Type-C redriver --- commit 09d82f46ea2293b7e9f9ce40ffd1668b1a047cd2 Author: Esteban Urrutia <esteuwu@proton.me> usb: typec: Add support for Parade PS5169 USB Type-C redriver Add driver for the Parade PS5169, an USB Type-C redriver capable of handling 10Gb/s USB signals and 8.1Gb/s DisplayPort signals. > diff --git a/drivers/usb/typec/mux/ps5169.c b/drivers/usb/typec/mux/ps5169.c > index 0000000000000..3ee04843fa2dc > --- /dev/null > +++ b/drivers/usb/typec/mux/ps5169.c [ ... ] > @@ -242,28 +242,21 @@ static int ps5169_retimer_set(struct typec_retimer *retimer, struct typec_retimer_state *state) > struct redriver *ps5169 = typec_retimer_get_drvdata(retimer); > struct typec_mux_state mux_state; > int ret = 0; > > mutex_lock(&ps5169->lock); > > if (ps5169->mode != state->mode) { > ps5169->mode = state->mode; > > if (state->alt) > ps5169->svid = state->alt->svid; > else > ps5169->svid = 0; // No SVID > > ret = ps5169_set(ps5169); > } [Severity: Medium] Does this risk desynchronizing the software state from the hardware? If ps5169_set() fails and returns an error, the internal state (ps5169->mode) remains updated. If the caller subsequently requests the exact same mode again, the equality check evaluates to false. This causes the driver to bypass validation and falsely report success while leaving the hardware unconfigured. Should the state update be rolled back on error, or deferred until after a successful hardware update? > mutex_unlock(&ps5169->lock); [ ... ] > @@ -361,54 +354,47 @@ static int ps5169_probe(struct i2c_client *client) > ret = regulator_enable(ps5169->vcc); > if (ret) { > dev_err(dev, "Failed to enable vcc-supply\n"); > goto mux_put; > } > > /* Reset the retimer */ > gpiod_set_value(ps5169->reset_gpio, 1); > usleep_range(1000, 1500); > gpiod_set_value(ps5169->reset_gpio, 0); > usleep_range(10000, 11000); > > ret = regmap_raw_read(ps5169->regmap, PS5169_CHIP_ID_REG, &chip_id, sizeof(chip_id)); [Severity: Medium] Does reading raw I2C bytes directly into a native u16 variable cause endianness mismatches on big-endian architectures? regmap_raw_read() performs a raw byte copy, so reading a 2-byte register directly into chip_id produces a byte order mismatch on big-endian systems. Could a standard regmap read or explicit endianness handling be used here? > if (ret) { > dev_err(dev, "Failed to read chip ID\n"); > goto vcc_disable; > } > > /* Compare chip IDs */ > if (chip_id != PS5169_CHIP_ID) { > dev_err(dev, "Unexpected chip ID 0x%04x\n", chip_id); > goto vcc_disable; > } [Severity: High] Is it intended to return success when the chip ID is invalid? The code jumps to vcc_disable without updating the ret variable. Since the preceding regmap_raw_read() just succeeded, ret equals 0. The probe function falsely returns success to the driver core while releasing resources, leaving a crippled device object that might be double-freed on removal. Should ret be set to -ENODEV before the goto? > /* Read chip revision */ > ret = regmap_raw_read(ps5169->regmap, PS5169_CHIP_REVISION_REG, &chip_revision, [ ... ] > @@ -395,21 +381,14 @@ static int ps5169_probe(struct i2c_client *client) > > /* orientation-switch */ > switch_desc.drvdata = ps5169; > switch_desc.fwnode = dev_fwnode(dev); > switch_desc.set = ps5169_switch_set; > > ps5169->sw = typec_switch_register(dev, &switch_desc); > if (IS_ERR(ps5169->sw)) { > ret = dev_err_probe(dev, PTR_ERR(ps5169->sw), "Failed to register switch\n"); > goto vcc_disable; > } > > /* retimer-switch */ > retimer_desc.drvdata = ps5169; > retimer_desc.fwnode = dev_fwnode(dev); > retimer_desc.set = ps5169_retimer_set; > > ps5169->retimer = typec_retimer_register(dev, &retimer_desc); > if (IS_ERR(ps5169->retimer)) { > ret = dev_err_probe(dev, PTR_ERR(ps5169->retimer), "Failed to register retimer\n"); > goto switch_unregister; > } [ ... ] > ps5169_init(ps5169); > > mutex_init(&ps5169->lock); > > i2c_set_clientdata(client, ps5169); > > return 0; [Severity: High] Does registering the typec_switch and typec_retimer before initializing the hardware and mutex expose a race window? The subsystem can immediately invoke the .set callbacks (ps5169_switch_set, ps5169_retimer_set) before probe() finishes, causing the callbacks to attempt to lock an uninitialized mutex and write to unconfigured hardware. Should ps5169_init() and mutex_init() happen before device registration? > switch_unregister: > typec_switch_unregister(ps5169->sw); > > vcc_disable: > regulator_disable(ps5169->vcc); [Severity: High] Does this error path leak the retimer device? If property parsing (like parade,dp-eq-level) fails after typec_retimer_register() succeeds, the function jumps to the switch_unregister label. The cleanup path unregisters the switch but omits calling typec_retimer_unregister(). The retimer device object remains registered with the Type-C framework while the parent memory block is freed, leading to a potential use-after-free. > mux_put: > typec_mux_put(ps5169->typec_mux); [ ... ] > @@ -518,17 +490,10 @@ static void ps5169_remove(struct i2c_client *client) > typec_mux_put(ps5169->typec_mux); > typec_switch_put(ps5169->typec_switch); > } > > static const struct of_device_id ps5169_of_match_table[] = { > { .compatible = "parade,ps5169" }, > { }, > }; [Severity: Medium] Is the driver missing MODULE_DEVICE_TABLE for device tree matching? The kernel build system will not extract the device tree match information into the module aliases without it, which might prevent user-space tools like udev from auto-loading this driver when the hardware is discovered. > static struct i2c_driver ps5169_driver = { > .driver = { > .name = "ps5169", > .of_match_table = ps5169_of_match_table, > }, -- Sashiko AI review · https://sashiko.dev/#/patchset/20260727-ps5169-bup-v1-0-c76a8ef3b239@proton.me?part=2 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-07-28 1:36 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-28 1:19 [PATCH 0/2] Add support for Parade PS5169 USB Type-C linear redriver Esteban Urrutia via B4 Relay 2026-07-28 1:19 ` Esteban Urrutia 2026-07-28 1:19 ` [PATCH 1/2] dt-bindings: usb: Add Parade PS5169 redriver bindings Esteban Urrutia via B4 Relay 2026-07-28 1:19 ` Esteban Urrutia 2026-07-28 1:34 ` sashiko-bot 2026-07-28 1:19 ` [PATCH 2/2] usb: typec: Add support for Parade PS5169 USB Type-C redriver Esteban Urrutia via B4 Relay 2026-07-28 1:19 ` Esteban Urrutia 2026-07-28 1:36 ` sashiko-bot
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.