Re: [PATCH 3/3] usb: typec: tcpci_rt1711h: add low power mode support

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

On Wed, Jun 03, 2026 at 11:18:27AM +0800, lucas_tsai@richtek.com wrote:
> On Mon, Jun 01, 2026 at 04:36:02PM +0300, Heikki Krogerus wrote:
> > Hi,
> > 
> > I'm sorry to keep you waiting.
> > 
> > On Mon, May 18, 2026 at 05:15:14PM +0800, cy_huang@richtek.com wrote:
> > > From: Lucas Tsai <lucas_tsai@richtek.com>
> > > 
> > > Add low power mode support,
> > > add the op to enter and exit low power mode,
> > > this mode reduce RT1711H/RT1715 VDD Iq to 1 of 10,
> > 
> > What is VDD Iq?
> > 
> 
> VDD pin is RT1715's power input,
> and Iq is the quiescent current,
> so the low power mode reduces the baseline power used by RT1715.

Got it, thanks. Please add that explanation to the commit message.

> > > while disabling VBUS detection and PD BMC
> > > but keeping CC detection and not affecting DRP toggling.
> > > 
> > > Signed-off-by: Lucas Tsai <lucas_tsai@richtek.com>
> > > ---
> > >  drivers/usb/typec/tcpm/tcpci_rt1711h.c | 14 ++++++++++++++
> > >  1 file changed, 14 insertions(+)
> > > 
> > > diff --git a/drivers/usb/typec/tcpm/tcpci_rt1711h.c b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
> > > index 4b3e4e22a82e..48d6a6823ab9 100644
> > > --- a/drivers/usb/typec/tcpm/tcpci_rt1711h.c
> > > +++ b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
> > > @@ -20,6 +20,7 @@
> > >  
> > >  #define RT1711H_PHYCTRL1	0x80
> > >  #define RT1711H_PHYCTRL2	0x81
> > > +#define RT1711H_BMCCTRL		0x90
> > >  
> > >  #define RT1711H_RTCTRL4		0x93
> > >  /* rx threshold of rd/rp: 1b0 for level 0.4V/0.7V, 1b1 for 0.35V/0.75V */
> > > @@ -254,6 +255,18 @@ static int rt1711h_start_drp_toggling(struct tcpci *tcpci,
> > >  	return 0;
> > >  }
> > >  
> > > +static void rt1711h_set_low_power_mode(struct tcpci *tcpci,
> > > +				       struct tcpci_data *tdata, bool enable)
> > > +{
> > > +	int ret;
> > > +	struct rt1711h_chip *chip = tdata_to_rt1711h(tdata);
> > > +
> > > +	ret = rt1711h_write8(chip, RT1711H_BMCCTRL, enable ? 0x08 : 0x07);
> > > +	if (ret < 0)
> > > +		dev_err(chip->dev, "%s lpm fail(%d)\n",
> > > +			enable ? "enter" : "exit", ret);
> > > +}
> > 
> > Why couldn't this just be done in the PM suspend and resume
> > callbacks for this driver?
> 
> Entering low power mode or not relates directly to the status of USB-C port:
> enter low power mode when USB-C port attached nothing (unattached),
> and exit low power mode when attached with cable or device.
> 
> In the other word, it is possible to have system suspended and USB-C
> port attached at the same time, so it is not suitable to bind the flow
> of low power mode to the PM suspend and resume.

You don't have to do that in the system suspend callback, but you
still should do this in the runtime suspend callback, no?

This is not a major thing, but mctp.c is a bit bloated, I want to make
sure that adding this kind of callbacks is really necessary.

Badhri, do you have time to check this series?

thanks,

-- 
heikki