Hi, On Mon, Apr 20, 2026 at 01:40:28PM +0200, Konrad Dybcio wrote: > From: Konrad Dybcio > > 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 > Signed-off-by: Konrad Dybcio > --- > Note most of the diff happens to be there because I need to move > ps883x_(en/dis)able_vregs() around.. > --- Tested-by: Sebastian Reichel # T14s Gen6 Snapdragon Greetings, -- Sebastian > 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 > >