* [PATCH v2 0/2] Add support for Parade PS5169 USB Type-C linear redriver
@ 2026-07-29 6:03 Esteban Urrutia via B4 Relay
2026-07-29 6:03 ` [PATCH v2 1/2] dt-bindings: usb: Add Parade PS5169 redriver bindings Esteban Urrutia via B4 Relay
2026-07-29 6:03 ` [PATCH v2 2/2] usb: typec: Add support for Parade PS5169 USB Type-C redriver Esteban Urrutia via B4 Relay
0 siblings, 2 replies; 5+ messages in thread
From: Esteban Urrutia via B4 Relay @ 2026-07-29 6:03 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].
Switch/retimer set functions were adapted from NB7VPQ904M/PTN36502
implementations.
[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>
---
Changes in v2:
- Fix valid issues raised by Sashiko
- Link to v1: https://patch.msgid.link/20260727-ps5169-bup-v1-0-c76a8ef3b239@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 | 191 ++++++++
MAINTAINERS | 7 +
drivers/usb/typec/mux/Kconfig | 10 +
drivers/usb/typec/mux/Makefile | 1 +
drivers/usb/typec/mux/ps5169.c | 538 +++++++++++++++++++++
5 files changed, 747 insertions(+)
---
base-commit: 3652b49adac266a3d27cb41cdfdb7d8790fc3633
change-id: 20260726-ps5169-bup-8076851dc255
Best regards,
--
Esteban Urrutia <esteuwu@proton.me>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] dt-bindings: usb: Add Parade PS5169 redriver bindings
2026-07-29 6:03 [PATCH v2 0/2] Add support for Parade PS5169 USB Type-C linear redriver Esteban Urrutia via B4 Relay
@ 2026-07-29 6:03 ` Esteban Urrutia via B4 Relay
2026-07-29 6:13 ` sashiko-bot
2026-07-29 6:03 ` [PATCH v2 2/2] usb: typec: Add support for Parade PS5169 USB Type-C redriver Esteban Urrutia via B4 Relay
1 sibling, 1 reply; 5+ messages in thread
From: Esteban Urrutia via B4 Relay @ 2026-07-29 6:03 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 | 191 +++++++++++++++++++++
1 file changed, 191 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..86ea2351ee1e
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/parade,ps5169.yaml
@@ -0,0 +1,191 @@
+# 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.
+ else:
+ properties:
+ ports:
+ properties:
+ port@2: false
+
+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] 5+ messages in thread
* [PATCH v2 2/2] usb: typec: Add support for Parade PS5169 USB Type-C redriver
2026-07-29 6:03 [PATCH v2 0/2] Add support for Parade PS5169 USB Type-C linear redriver Esteban Urrutia via B4 Relay
2026-07-29 6:03 ` [PATCH v2 1/2] dt-bindings: usb: Add Parade PS5169 redriver bindings Esteban Urrutia via B4 Relay
@ 2026-07-29 6:03 ` Esteban Urrutia via B4 Relay
2026-07-29 6:12 ` sashiko-bot
1 sibling, 1 reply; 5+ messages in thread
From: Esteban Urrutia via B4 Relay @ 2026-07-29 6:03 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 | 538 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 556 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index e95fc6f2ddc6..0ef942ac8df3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -28287,6 +28287,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..74cc1504dd35
--- /dev/null
+++ b/drivers/usb/typec/mux/ps5169.c
@@ -0,0 +1,538 @@
+// 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;
+ }
+ chip_id = cpu_to_le16(chip_id);
+
+ /* Compare chip IDs */
+ if (chip_id != PS5169_CHIP_ID) {
+ dev_err(dev, "Unexpected chip ID 0x%04x\n", chip_id);
+ ret = -EINVAL;
+ 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", cpu_to_le16(chip_revision));
+
+ ret = drm_aux_bridge_register(dev);
+ if (ret) {
+ dev_err(dev, "Failed to register aux_bridge\n");
+ goto vcc_disable;
+ }
+
+ 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 vcc_disable;
+ }
+ 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 vcc_disable;
+ }
+ } 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 vcc_disable;
+ }
+ 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 vcc_disable;
+ }
+ } 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 vcc_disable;
+ }
+ 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 vcc_disable;
+ }
+ } 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;
+ }
+
+ /* 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;
+ }
+
+ /* 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" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, ps5169_of_match_table);
+
+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] 5+ messages in thread
* Re: [PATCH v2 2/2] usb: typec: Add support for Parade PS5169 USB Type-C redriver
2026-07-29 6:03 ` [PATCH v2 2/2] usb: typec: Add support for Parade PS5169 USB Type-C redriver Esteban Urrutia via B4 Relay
@ 2026-07-29 6:12 ` sashiko-bot
0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-07-29 6:12 UTC (permalink / raw)
To: Esteban Urrutia; +Cc: conor+dt, devicetree, robh
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The Type-C switch and retimer are registered before their dependencies are initialized, leading to potential crashes or lockdep splats.
- [High] Failed hardware configurations corrupt the cached software state, preventing future validation and retries.
- [Medium] Silent failures due to ignored I2C/regmap error returns during device configuration.
- [Low] Incorrect endianness macro used for memory-to-CPU conversion.
--
--- Patch [2]: [PATCH v2 2/2] usb: typec: Add support for Parade PS5169 USB Type-C redriver ---
commit 63274a3d684029a0da1c1e47cb9856c0a759ba57
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..74cc1504dd357
> --- /dev/null
> +++ b/drivers/usb/typec/mux/ps5169.c
[ ... ]
> +static void ps5169_write(struct redriver *ps5169, unsigned int reg, unsigned int val)
> +{
> + regmap_write(ps5169->regmap, reg, val);
[Severity: Medium]
Does this function need to handle errors from regmap_write? The error codes
are currently discarded, which might cause the hardware state to fall out of
sync with the software state if an I2C transaction fails.
> +
> + 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);
[ ... ]
> +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;
[Severity: Medium]
Is it intentional to return success unconditionally here even if the underlying
I2C writes in ps5169_write() failed?
[ ... ]
> +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);
> + }
[Severity: High]
If ps5169_set() fails here, ps5169->orientation is already updated. Will this
prevent future validation and retries since the driver might skip hardware
configuration on a subsequent retry with the same orientation?
> +
> + 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);
> + }
[Severity: High]
Similar to ps5169_switch_set(), if ps5169_set() fails here, ps5169->mode and
ps5169->svid are already updated. Does this need to be reverted on failure to
prevent desynchronization?
Also, is it intentional that SVID updates are completely ignored if the mode
does not change?
> +
> + mutex_unlock(&ps5169->lock);
[ ... ]
> +static int ps5169_probe(struct i2c_client *client)
> +{
[ ... ]
> + 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;
> + }
> + chip_id = cpu_to_le16(chip_id);
[Severity: Low]
Could this be le16_to_cpu() instead of cpu_to_le16()? While mathematically
identical, cpu_to_le16() assigns a little-endian type (__le16) back to a
CPU-native u16 variable, which might cause sparse warnings and semantic
confusion.
> +
> + /* Compare chip IDs */
> + if (chip_id != PS5169_CHIP_ID) {
> + dev_err(dev, "Unexpected chip ID 0x%04x\n", chip_id);
> + ret = -EINVAL;
> + 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", cpu_to_le16(chip_revision));
[Severity: Low]
Similar to the chip_id reading above, should this use le16_to_cpu() when
passing the value to the string format expecting a CPU-native unsigned int?
> +
> + ret = drm_aux_bridge_register(dev);
[ ... ]
> + /* 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;
> + }
> +
> + /* 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);
[Severity: High]
Can the subsystem invoke a state change callback (like ps5169_switch_set or
ps5169_retimer_set) as soon as the switch and retimer are registered? If so,
could those callbacks execute before mutex_init(&ps5169->lock) is called here,
leading to a crash or lockdep splat since they attempt to lock this uninitialized
mutex?
> +
> + i2c_set_clientdata(client, ps5169);
> +
> + return 0;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-ps5169-bup-v2-0-203b0a49881a@proton.me?part=2
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: usb: Add Parade PS5169 redriver bindings
2026-07-29 6:03 ` [PATCH v2 1/2] dt-bindings: usb: Add Parade PS5169 redriver bindings Esteban Urrutia via B4 Relay
@ 2026-07-29 6:13 ` sashiko-bot
0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-07-29 6:13 UTC (permalink / raw)
To: Esteban Urrutia; +Cc: devicetree, conor+dt, robh
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Using 'parade,ps5169' as a fallback compatible string for 'parade,ps5170' violates backward compatibility principles due to hardware topology differences.
- [Medium] Missing `mode-switch` property definition in the top-level `properties` block causes valid `parade,ps5170` device trees to be rejected.
- [Low] The `parade,aux-rx-data-reverse` property is erroneously allowed for the `ps5169` variant which lacks an AUX switch.
--
--- Patch [1]: [PATCH v2 1/2] dt-bindings: usb: Add Parade PS5169 redriver bindings ---
commit b10ecfdc9ec0281d0cf16967c19be130e3289a23
Author: Esteban Urrutia <esteuwu@proton.me>
dt-bindings: usb: Add Parade PS5169 redriver bindings
This commit adds bindings for the Parade PS5169 and PS5170 USB Type-C
linear redrivers. The PS5170 features an embedded AUX switch, which the
PS5169 lacks.
> diff --git a/Documentation/devicetree/bindings/usb/parade,ps5169.yaml b/Documentation/devicetree/bindings/usb/parade,ps5169.yaml
> new file mode 100644
> index 0000000000000..86ea2351ee1e8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/parade,ps5169.yaml
> @@ -0,0 +1,191 @@
> +# 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
[Severity: High]
Does this incorrectly define parade,ps5169 as a fallback for parade,ps5170?
Since the PS5170 features an embedded AUX switch for SBU lines, an older OS
binding to the fallback ps5169 driver would fail to configure the internal
AUX switch. This would leave the SBU lines disconnected and break
DisplayPort Alternate Mode.
> + - const: parade,ps5169
> +
> + reg:
> + maxItems: 1
> +
> + vcc-supply:
> + description: 1.8V power supply
> +
> + reset-gpios:
> + maxItems: 1
> +
> + orientation-switch: true
> + retimer-switch: true
[Severity: Medium]
Should mode-switch: true be included here?
Since the PS5170 supports altmode muxing with its embedded AUX switch,
valid device trees for the ps5170 may define the mode-switch property.
Because additionalProperties is set to false later in this schema,
dt_binding_check will reject any node specifying mode-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.
> + else:
> + properties:
> + ports:
> + properties:
> + port@2: false
[Severity: Low]
Can we also disable parade,aux-rx-data-reverse in this else block?
This else block applies to the ps5169 variant, which lacks an AUX switch.
Currently, invalid device trees for ps5169 can specify the AUX-specific
parade,aux-rx-data-reverse property and silently pass schema validation.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-ps5169-bup-v2-0-203b0a49881a@proton.me?part=1
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-07-29 6:13 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-29 6:03 [PATCH v2 0/2] Add support for Parade PS5169 USB Type-C linear redriver Esteban Urrutia via B4 Relay
2026-07-29 6:03 ` [PATCH v2 1/2] dt-bindings: usb: Add Parade PS5169 redriver bindings Esteban Urrutia via B4 Relay
2026-07-29 6:13 ` sashiko-bot
2026-07-29 6:03 ` [PATCH v2 2/2] usb: typec: Add support for Parade PS5169 USB Type-C redriver Esteban Urrutia via B4 Relay
2026-07-29 6:12 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox