[PATCH] usb: typec: mux: ps883x: Power the retimer off when not in use

[PATCH] usb: typec: mux: ps883x: Power the retimer off when not in use

[Konrad Dybcio]

From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

When there's nothing going through the retimer, there's no reason to
keep it online. Put it in reset when possible to save power.

Also, remove the register cache-compare optimization as it makes little
sense now that the chip resets during almost all transitions and
tracking the validity of that cache becomes a headache.

Suggested-by: Jack Pham <jack.pham@oss.qualcomm.com>
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
Note most of the diff happens to be there because I need to move
ps883x_(en/dis)able_vregs() around..
---
 drivers/usb/typec/mux/ps883x.c | 196 ++++++++++++++++++++++++-----------------
 1 file changed, 114 insertions(+), 82 deletions(-)

diff --git a/drivers/usb/typec/mux/ps883x.c b/drivers/usb/typec/mux/ps883x.c
index 1256252eceed..f52443638ee2 100644
--- a/drivers/usb/typec/mux/ps883x.c
+++ b/drivers/usb/typec/mux/ps883x.c
@@ -61,19 +61,110 @@ struct ps883x_retimer {
 	struct mutex lock; /* protect non-concurrent retimer & switch */
 
 	enum typec_orientation orientation;
-	u8 cfg0;
-	u8 cfg1;
-	u8 cfg2;
+	bool in_reset;
 };
 
+static int ps883x_enable_vregs(struct ps883x_retimer *retimer)
+{
+	struct device *dev = &retimer->client->dev;
+	int ret;
+
+	ret = regulator_enable(retimer->vdd33_supply);
+	if (ret) {
+		dev_err(dev, "cannot enable VDD 3.3V regulator: %d\n", ret);
+		return ret;
+	}
+
+	ret = regulator_enable(retimer->vdd33_cap_supply);
+	if (ret) {
+		dev_err(dev, "cannot enable VDD 3.3V CAP regulator: %d\n", ret);
+		goto err_vdd33_disable;
+	}
+
+	usleep_range(4000, 10000);
+
+	ret = regulator_enable(retimer->vdd_supply);
+	if (ret) {
+		dev_err(dev, "cannot enable VDD regulator: %d\n", ret);
+		goto err_vdd33_cap_disable;
+	}
+
+	ret = regulator_enable(retimer->vddar_supply);
+	if (ret) {
+		dev_err(dev, "cannot enable VDD AR regulator: %d\n", ret);
+		goto err_vdd_disable;
+	}
+
+	ret = regulator_enable(retimer->vddat_supply);
+	if (ret) {
+		dev_err(dev, "cannot enable VDD AT regulator: %d\n", ret);
+		goto err_vddar_disable;
+	}
+
+	ret = regulator_enable(retimer->vddio_supply);
+	if (ret) {
+		dev_err(dev, "cannot enable VDD IO regulator: %d\n", ret);
+		goto err_vddat_disable;
+	}
+
+	return 0;
+
+err_vddat_disable:
+	regulator_disable(retimer->vddat_supply);
+err_vddar_disable:
+	regulator_disable(retimer->vddar_supply);
+err_vdd_disable:
+	regulator_disable(retimer->vdd_supply);
+err_vdd33_cap_disable:
+	regulator_disable(retimer->vdd33_cap_supply);
+err_vdd33_disable:
+	regulator_disable(retimer->vdd33_supply);
+
+	return ret;
+}
+
+static void ps883x_disable_vregs(struct ps883x_retimer *retimer)
+{
+	regulator_disable(retimer->vddio_supply);
+	regulator_disable(retimer->vddat_supply);
+	regulator_disable(retimer->vddar_supply);
+	regulator_disable(retimer->vdd_supply);
+	regulator_disable(retimer->vdd33_cap_supply);
+	regulator_disable(retimer->vdd33_supply);
+}
+
+static void ps883x_reset(struct ps883x_retimer *retimer)
+{
+	if (retimer->in_reset)
+		return;
+
+	gpiod_set_value(retimer->reset_gpio, 1);
+	ps883x_disable_vregs(retimer);
+	retimer->in_reset = true;
+}
+
 static int ps883x_configure(struct ps883x_retimer *retimer, int cfg0,
-			    int cfg1, int cfg2)
+			    int cfg1, int cfg2, bool reset)
 {
 	struct device *dev = &retimer->client->dev;
 	int ret;
 
-	if (retimer->cfg0 == cfg0 && retimer->cfg1 == cfg1 && retimer->cfg2 == cfg2)
+	if (reset) {
+		ps883x_reset(retimer);
+
 		return 0;
+	} else if (retimer->in_reset) {
+		ret = ps883x_enable_vregs(retimer);
+		if (ret)
+			return ret;
+
+		gpiod_set_value(retimer->reset_gpio, 0);
+
+		/* firmware initialization delay */
+		msleep(60);
+
+		retimer->in_reset = false;
+	}
 
 	ret = regmap_write(retimer->regmap, REG_USB_PORT_CONN_STATUS_0, cfg0);
 	if (ret) {
@@ -93,10 +184,6 @@ static int ps883x_configure(struct ps883x_retimer *retimer, int cfg0,
 		return ret;
 	}
 
-	retimer->cfg0 = cfg0;
-	retimer->cfg1 = cfg1;
-	retimer->cfg2 = cfg2;
-
 	return 0;
 }
 
@@ -107,6 +194,7 @@ static int ps883x_set(struct ps883x_retimer *retimer, struct typec_retimer_state
 	int cfg0 = CONN_STATUS_0_CONNECTION_PRESENT;
 	int cfg1 = 0x00;
 	int cfg2 = 0x00;
+	bool reset = false;
 
 	if (retimer->orientation == TYPEC_ORIENTATION_REVERSE)
 		cfg0 |= CONN_STATUS_0_ORIENTATION_REVERSED;
@@ -148,9 +236,13 @@ static int ps883x_set(struct ps883x_retimer *retimer, struct typec_retimer_state
 		}
 	} else {
 		switch (state->mode) {
+		/* SAFE can be transient or point to an actual disconnect */
 		case TYPEC_STATE_SAFE:
+			reset = retimer->orientation == TYPEC_ORIENTATION_NONE;
+			break;
 		/* USB2 pins don't even go through this chip */
 		case TYPEC_MODE_USB2:
+			reset = true;
 			break;
 		case TYPEC_STATE_USB:
 		case TYPEC_MODE_USB3:
@@ -171,7 +263,7 @@ static int ps883x_set(struct ps883x_retimer *retimer, struct typec_retimer_state
 		}
 	}
 
-	return ps883x_configure(retimer, cfg0, cfg1, cfg2);
+	return ps883x_configure(retimer, cfg0, cfg1, cfg2, reset);
 }
 
 static int ps883x_sw_set(struct typec_switch_dev *sw,
@@ -184,11 +276,19 @@ static int ps883x_sw_set(struct typec_switch_dev *sw,
 	if (ret)
 		return ret;
 
-	mutex_lock(&retimer->lock);
+	guard(mutex)(&retimer->lock);
 
 	if (retimer->orientation != orientation) {
 		retimer->orientation = orientation;
 
+		/*
+		 * Orientation notifications usually come prior to mode switch
+		 * events. If the retimer is already in reset, we still want to
+		 * cache the new orientation value for the subsequent ps883x_set().
+		 */
+		if (retimer->in_reset)
+			return 0;
+
 		ret = regmap_assign_bits(retimer->regmap, REG_USB_PORT_CONN_STATUS_0,
 					 CONN_STATUS_0_ORIENTATION_REVERSED,
 					 orientation == TYPEC_ORIENTATION_REVERSE);
@@ -196,8 +296,6 @@ static int ps883x_sw_set(struct typec_switch_dev *sw,
 			dev_err(&retimer->client->dev, "failed to set orientation: %d\n", ret);
 	}
 
-	mutex_unlock(&retimer->lock);
-
 	return ret;
 }
 
@@ -222,75 +320,6 @@ static int ps883x_retimer_set(struct typec_retimer *rtmr,
 	return typec_mux_set(retimer->typec_mux, &mux_state);
 }
 
-static int ps883x_enable_vregs(struct ps883x_retimer *retimer)
-{
-	struct device *dev = &retimer->client->dev;
-	int ret;
-
-	ret = regulator_enable(retimer->vdd33_supply);
-	if (ret) {
-		dev_err(dev, "cannot enable VDD 3.3V regulator: %d\n", ret);
-		return ret;
-	}
-
-	ret = regulator_enable(retimer->vdd33_cap_supply);
-	if (ret) {
-		dev_err(dev, "cannot enable VDD 3.3V CAP regulator: %d\n", ret);
-		goto err_vdd33_disable;
-	}
-
-	usleep_range(4000, 10000);
-
-	ret = regulator_enable(retimer->vdd_supply);
-	if (ret) {
-		dev_err(dev, "cannot enable VDD regulator: %d\n", ret);
-		goto err_vdd33_cap_disable;
-	}
-
-	ret = regulator_enable(retimer->vddar_supply);
-	if (ret) {
-		dev_err(dev, "cannot enable VDD AR regulator: %d\n", ret);
-		goto err_vdd_disable;
-	}
-
-	ret = regulator_enable(retimer->vddat_supply);
-	if (ret) {
-		dev_err(dev, "cannot enable VDD AT regulator: %d\n", ret);
-		goto err_vddar_disable;
-	}
-
-	ret = regulator_enable(retimer->vddio_supply);
-	if (ret) {
-		dev_err(dev, "cannot enable VDD IO regulator: %d\n", ret);
-		goto err_vddat_disable;
-	}
-
-	return 0;
-
-err_vddat_disable:
-	regulator_disable(retimer->vddat_supply);
-err_vddar_disable:
-	regulator_disable(retimer->vddar_supply);
-err_vdd_disable:
-	regulator_disable(retimer->vdd_supply);
-err_vdd33_cap_disable:
-	regulator_disable(retimer->vdd33_cap_supply);
-err_vdd33_disable:
-	regulator_disable(retimer->vdd33_supply);
-
-	return ret;
-}
-
-static void ps883x_disable_vregs(struct ps883x_retimer *retimer)
-{
-	regulator_disable(retimer->vddio_supply);
-	regulator_disable(retimer->vddat_supply);
-	regulator_disable(retimer->vddar_supply);
-	regulator_disable(retimer->vdd_supply);
-	regulator_disable(retimer->vdd33_cap_supply);
-	regulator_disable(retimer->vdd33_supply);
-}
-
 static int ps883x_get_vregs(struct ps883x_retimer *retimer)
 {
 	struct device *dev = &retimer->client->dev;
@@ -422,6 +451,9 @@ static int ps883x_retimer_probe(struct i2c_client *client)
 		}
 	}
 
+	/* Keep the retimer in reset until a Type-C notification comes */
+	ps883x_reset(retimer);
+
 	sw_desc.drvdata = retimer;
 	sw_desc.fwnode = dev_fwnode(dev);
 	sw_desc.set = ps883x_sw_set;

---
base-commit: c7275b05bc428c7373d97aa2da02d3a7fa6b9f66
change-id: 20260420-topic-ps883x_unused_reset-9af0909cefac

Best regards,
-- 
Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Re: [PATCH] usb: typec: mux: ps883x: Power the retimer off when not in use

[Anthony Ruhier]

On Mon, Apr 20, 2026 at 01:40:28PM +0200, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
> When there's nothing going through the retimer, there's no reason to
> keep it online. Put it in reset when possible to save power.
>
> Also, remove the register cache-compare optimization as it makes little
> sense now that the chip resets during almost all transitions and
> tracking the validity of that cache becomes a headache.
>
> Suggested-by: Jack Pham <jack.pham@oss.qualcomm.com>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> --
> Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>

Tested-by: Anthony Ruhier <aruhier@mailbox.org>

Thanks a lot for this patch, on a Yoga Slim 7x (x1e80100) the power saving is
huge. The laptop would rarely idle below 6.9W, now it reaches 4.6W (same 7.0.1
kernel, the only difference is this patch). Screen off, the laptop was idling
at 3.5W, now it reaches 2W.

I tested that DP-alt and USB-C devices continued to work well with this patch,
and it works as expected.

--
Anthony Ruhier

Re: [PATCH] usb: typec: mux: ps883x: Power the retimer off when not in use

[Heikki Krogerus]

On Mon, Apr 20, 2026 at 01:40:28PM +0200, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> 
> When there's nothing going through the retimer, there's no reason to
> keep it online. Put it in reset when possible to save power.
> 
> Also, remove the register cache-compare optimization as it makes little
> sense now that the chip resets during almost all transitions and
> tracking the validity of that cache becomes a headache.
> 
> Suggested-by: Jack Pham <jack.pham@oss.qualcomm.com>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> Note most of the diff happens to be there because I need to move
> ps883x_(en/dis)able_vregs() around..
> ---
>  drivers/usb/typec/mux/ps883x.c | 196 ++++++++++++++++++++++++-----------------
>  1 file changed, 114 insertions(+), 82 deletions(-)
> 
> diff --git a/drivers/usb/typec/mux/ps883x.c b/drivers/usb/typec/mux/ps883x.c
> index 1256252eceed..f52443638ee2 100644
> --- a/drivers/usb/typec/mux/ps883x.c
> +++ b/drivers/usb/typec/mux/ps883x.c
> @@ -61,19 +61,110 @@ struct ps883x_retimer {
>  	struct mutex lock; /* protect non-concurrent retimer & switch */
>  
>  	enum typec_orientation orientation;
> -	u8 cfg0;
> -	u8 cfg1;
> -	u8 cfg2;
> +	bool in_reset;
>  };
>  
> +static int ps883x_enable_vregs(struct ps883x_retimer *retimer)
> +{
> +	struct device *dev = &retimer->client->dev;
> +	int ret;
> +
> +	ret = regulator_enable(retimer->vdd33_supply);
> +	if (ret) {
> +		dev_err(dev, "cannot enable VDD 3.3V regulator: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = regulator_enable(retimer->vdd33_cap_supply);
> +	if (ret) {
> +		dev_err(dev, "cannot enable VDD 3.3V CAP regulator: %d\n", ret);
> +		goto err_vdd33_disable;
> +	}
> +
> +	usleep_range(4000, 10000);
> +
> +	ret = regulator_enable(retimer->vdd_supply);
> +	if (ret) {
> +		dev_err(dev, "cannot enable VDD regulator: %d\n", ret);
> +		goto err_vdd33_cap_disable;
> +	}
> +
> +	ret = regulator_enable(retimer->vddar_supply);
> +	if (ret) {
> +		dev_err(dev, "cannot enable VDD AR regulator: %d\n", ret);
> +		goto err_vdd_disable;
> +	}
> +
> +	ret = regulator_enable(retimer->vddat_supply);
> +	if (ret) {
> +		dev_err(dev, "cannot enable VDD AT regulator: %d\n", ret);
> +		goto err_vddar_disable;
> +	}
> +
> +	ret = regulator_enable(retimer->vddio_supply);
> +	if (ret) {
> +		dev_err(dev, "cannot enable VDD IO regulator: %d\n", ret);
> +		goto err_vddat_disable;
> +	}
> +
> +	return 0;
> +
> +err_vddat_disable:
> +	regulator_disable(retimer->vddat_supply);
> +err_vddar_disable:
> +	regulator_disable(retimer->vddar_supply);
> +err_vdd_disable:
> +	regulator_disable(retimer->vdd_supply);
> +err_vdd33_cap_disable:
> +	regulator_disable(retimer->vdd33_cap_supply);
> +err_vdd33_disable:
> +	regulator_disable(retimer->vdd33_supply);
> +
> +	return ret;
> +}
> +
> +static void ps883x_disable_vregs(struct ps883x_retimer *retimer)
> +{
> +	regulator_disable(retimer->vddio_supply);
> +	regulator_disable(retimer->vddat_supply);
> +	regulator_disable(retimer->vddar_supply);
> +	regulator_disable(retimer->vdd_supply);
> +	regulator_disable(retimer->vdd33_cap_supply);
> +	regulator_disable(retimer->vdd33_supply);
> +}
> +
> +static void ps883x_reset(struct ps883x_retimer *retimer)
> +{
> +	if (retimer->in_reset)
> +		return;
> +
> +	gpiod_set_value(retimer->reset_gpio, 1);
> +	ps883x_disable_vregs(retimer);
> +	retimer->in_reset = true;
> +}
> +
>  static int ps883x_configure(struct ps883x_retimer *retimer, int cfg0,
> -			    int cfg1, int cfg2)
> +			    int cfg1, int cfg2, bool reset)
>  {
>  	struct device *dev = &retimer->client->dev;
>  	int ret;
>  
> -	if (retimer->cfg0 == cfg0 && retimer->cfg1 == cfg1 && retimer->cfg2 == cfg2)
> +	if (reset) {
> +		ps883x_reset(retimer);
> +
>  		return 0;
> +	} else if (retimer->in_reset) {
> +		ret = ps883x_enable_vregs(retimer);
> +		if (ret)
> +			return ret;
> +
> +		gpiod_set_value(retimer->reset_gpio, 0);
> +
> +		/* firmware initialization delay */
> +		msleep(60);
> +
> +		retimer->in_reset = false;
> +	}
>  
>  	ret = regmap_write(retimer->regmap, REG_USB_PORT_CONN_STATUS_0, cfg0);
>  	if (ret) {
> @@ -93,10 +184,6 @@ static int ps883x_configure(struct ps883x_retimer *retimer, int cfg0,
>  		return ret;
>  	}
>  
> -	retimer->cfg0 = cfg0;
> -	retimer->cfg1 = cfg1;
> -	retimer->cfg2 = cfg2;
> -
>  	return 0;
>  }
>  
> @@ -107,6 +194,7 @@ static int ps883x_set(struct ps883x_retimer *retimer, struct typec_retimer_state
>  	int cfg0 = CONN_STATUS_0_CONNECTION_PRESENT;
>  	int cfg1 = 0x00;
>  	int cfg2 = 0x00;
> +	bool reset = false;
>  
>  	if (retimer->orientation == TYPEC_ORIENTATION_REVERSE)
>  		cfg0 |= CONN_STATUS_0_ORIENTATION_REVERSED;
> @@ -148,9 +236,13 @@ static int ps883x_set(struct ps883x_retimer *retimer, struct typec_retimer_state
>  		}
>  	} else {
>  		switch (state->mode) {
> +		/* SAFE can be transient or point to an actual disconnect */
>  		case TYPEC_STATE_SAFE:
> +			reset = retimer->orientation == TYPEC_ORIENTATION_NONE;
> +			break;
>  		/* USB2 pins don't even go through this chip */
>  		case TYPEC_MODE_USB2:
> +			reset = true;
>  			break;
>  		case TYPEC_STATE_USB:
>  		case TYPEC_MODE_USB3:
> @@ -171,7 +263,7 @@ static int ps883x_set(struct ps883x_retimer *retimer, struct typec_retimer_state
>  		}
>  	}
>  
> -	return ps883x_configure(retimer, cfg0, cfg1, cfg2);
> +	return ps883x_configure(retimer, cfg0, cfg1, cfg2, reset);
>  }
>  
>  static int ps883x_sw_set(struct typec_switch_dev *sw,
> @@ -184,11 +276,19 @@ static int ps883x_sw_set(struct typec_switch_dev *sw,
>  	if (ret)
>  		return ret;
>  
> -	mutex_lock(&retimer->lock);
> +	guard(mutex)(&retimer->lock);
>  
>  	if (retimer->orientation != orientation) {
>  		retimer->orientation = orientation;
>  
> +		/*
> +		 * Orientation notifications usually come prior to mode switch
> +		 * events. If the retimer is already in reset, we still want to
> +		 * cache the new orientation value for the subsequent ps883x_set().
> +		 */
> +		if (retimer->in_reset)
> +			return 0;
> +
>  		ret = regmap_assign_bits(retimer->regmap, REG_USB_PORT_CONN_STATUS_0,
>  					 CONN_STATUS_0_ORIENTATION_REVERSED,
>  					 orientation == TYPEC_ORIENTATION_REVERSE);
> @@ -196,8 +296,6 @@ static int ps883x_sw_set(struct typec_switch_dev *sw,
>  			dev_err(&retimer->client->dev, "failed to set orientation: %d\n", ret);
>  	}
>  
> -	mutex_unlock(&retimer->lock);
> -
>  	return ret;
>  }
>  
> @@ -222,75 +320,6 @@ static int ps883x_retimer_set(struct typec_retimer *rtmr,
>  	return typec_mux_set(retimer->typec_mux, &mux_state);
>  }
>  
> -static int ps883x_enable_vregs(struct ps883x_retimer *retimer)
> -{
> -	struct device *dev = &retimer->client->dev;
> -	int ret;
> -
> -	ret = regulator_enable(retimer->vdd33_supply);
> -	if (ret) {
> -		dev_err(dev, "cannot enable VDD 3.3V regulator: %d\n", ret);
> -		return ret;
> -	}
> -
> -	ret = regulator_enable(retimer->vdd33_cap_supply);
> -	if (ret) {
> -		dev_err(dev, "cannot enable VDD 3.3V CAP regulator: %d\n", ret);
> -		goto err_vdd33_disable;
> -	}
> -
> -	usleep_range(4000, 10000);
> -
> -	ret = regulator_enable(retimer->vdd_supply);
> -	if (ret) {
> -		dev_err(dev, "cannot enable VDD regulator: %d\n", ret);
> -		goto err_vdd33_cap_disable;
> -	}
> -
> -	ret = regulator_enable(retimer->vddar_supply);
> -	if (ret) {
> -		dev_err(dev, "cannot enable VDD AR regulator: %d\n", ret);
> -		goto err_vdd_disable;
> -	}
> -
> -	ret = regulator_enable(retimer->vddat_supply);
> -	if (ret) {
> -		dev_err(dev, "cannot enable VDD AT regulator: %d\n", ret);
> -		goto err_vddar_disable;
> -	}
> -
> -	ret = regulator_enable(retimer->vddio_supply);
> -	if (ret) {
> -		dev_err(dev, "cannot enable VDD IO regulator: %d\n", ret);
> -		goto err_vddat_disable;
> -	}
> -
> -	return 0;
> -
> -err_vddat_disable:
> -	regulator_disable(retimer->vddat_supply);
> -err_vddar_disable:
> -	regulator_disable(retimer->vddar_supply);
> -err_vdd_disable:
> -	regulator_disable(retimer->vdd_supply);
> -err_vdd33_cap_disable:
> -	regulator_disable(retimer->vdd33_cap_supply);
> -err_vdd33_disable:
> -	regulator_disable(retimer->vdd33_supply);
> -
> -	return ret;
> -}
> -
> -static void ps883x_disable_vregs(struct ps883x_retimer *retimer)
> -{
> -	regulator_disable(retimer->vddio_supply);
> -	regulator_disable(retimer->vddat_supply);
> -	regulator_disable(retimer->vddar_supply);
> -	regulator_disable(retimer->vdd_supply);
> -	regulator_disable(retimer->vdd33_cap_supply);
> -	regulator_disable(retimer->vdd33_supply);
> -}
> -
>  static int ps883x_get_vregs(struct ps883x_retimer *retimer)
>  {
>  	struct device *dev = &retimer->client->dev;
> @@ -422,6 +451,9 @@ static int ps883x_retimer_probe(struct i2c_client *client)
>  		}
>  	}
>  
> +	/* Keep the retimer in reset until a Type-C notification comes */
> +	ps883x_reset(retimer);
> +
>  	sw_desc.drvdata = retimer;
>  	sw_desc.fwnode = dev_fwnode(dev);
>  	sw_desc.set = ps883x_sw_set;
> 
> ---
> base-commit: c7275b05bc428c7373d97aa2da02d3a7fa6b9f66
> change-id: 20260420-topic-ps883x_unused_reset-9af0909cefac
> 
> Best regards,
> -- 
> Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

-- 
heikki