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

[Lucas Tsai <lucas_tsai@richtek.com>]

On Sat, Jun 06, 2026 at 09:20:34AM -0700, Badhri Jagan Sridharan wrote:
> On Fri, Jun 5, 2026 at 6:03 AM Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:
> >
> > 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);
> > > > > +}
> 
> Any reason why cant rt1711h_set_low_power_mode() be called from
> rt1711h_init_cc_params() and rt1711h_start_drp_toggling() ?
> rt1711h_start_drp_toggling() can enable low power mode.

rt1711h_start_drp_toggling() will not be called for 
non-DRP ports, e.g.: sink only or source only ports,
because the flow in tcpci.c blocks such conditions:
static int tcpci_start_toggling(...)
{
	...
	if (port_type != TYPEC_PORT_DRP)
		return -EOPNOTSUPP;
	...
}

> rt1711h_init_cc_params(), which is already aware of CC status, can
> disable low power mode when a source or sink is detected.
> This approach would avoid adding a new callback in tcpci.c.

Since rt1711h_start_drp_toggling() approach have the limitations,
a new callback seems like being inevitable,
so putting the logic of disabling low power mode in the new callback.

> 
> If the above works, you could optionally rely on runtime pm callbacks
> to call rt1711h_set_low_power_mode() and increment/decrement the usage
> count in  rt1711h_init_cc_params() and rt1711h_start_drp_toggling().
> 

The power mode of TCPC IC does not relate to system/runtime pm,
but to the USB-C port status (Attached vs. Unattached).

> > > >
> > > > 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
> 
> I believe Heikki was referring to tcpci.c here.
> 

Thanks for your clarifying.

> > sure that adding this kind of callbacks is really necessary.
> >
> > Badhri, do you have time to check this series?
> >
> > thanks,
> >
> > --
> > heikki

Thanks.
--
Lucas Tsai