On Thu, Jan 16, 2025 at 7:25 PM Heikki Krogerus
<heikki.krogerus@xxxxxxxxxxxxxxx> wrote:
>
> On Tue, Jan 14, 2025 at 10:24:35PM +0800, Kyle Tso wrote:
> > The Source can drop its output voltage to the minimum of the requested
> > PPS APDO voltage range when it is in Current Limit Mode. If this voltage
> > falls within the range of vPpsShutdown, the Source initiates a Hard
> > Reset and discharges Vbus. However, currently the Sink may disconnect
> > before the voltage reaches vPpsShutdown, leading to unexpected behavior.
> >
> > Prevent premature disconnection by setting the Sink's disconnect
> > threshold to the minimum vPpsShutdown value. Additionally, consider the
> > voltage drop due to IR drop when calculating the appropriate threshold.
> > This ensures a robust and reliable interaction between the Source and
> > Sink during SPR PPS Current Limit Mode operation.
> >
> > Fixes: 4288debeaa4e ("usb: typec: tcpci: Fix up sink disconnect thresholds for PD")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Kyle Tso <kyletso@xxxxxxxxxx>
>
> You've resend this, right? So is this v2 (or v1)?
>
> Reviewed-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
>
Hello Heikki,
Thank you for the review.
Apologies for the resend. This is indeed the v1 patch. The previous
email was accidentally sent with an incomplete recipient list.
Thanks,
Kyle
> > ---
> > drivers/usb/typec/tcpm/tcpci.c | 13 +++++++++----
> > drivers/usb/typec/tcpm/tcpm.c | 8 +++++---
> > include/linux/usb/tcpm.h | 3 ++-
> > 3 files changed, 16 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> > index 48762508cc86..19ab6647af70 100644
> > --- a/drivers/usb/typec/tcpm/tcpci.c
> > +++ b/drivers/usb/typec/tcpm/tcpci.c
> > @@ -27,6 +27,7 @@
> > #define VPPS_NEW_MIN_PERCENT 95
> > #define VPPS_VALID_MIN_MV 100
> > #define VSINKDISCONNECT_PD_MIN_PERCENT 90
> > +#define VPPS_SHUTDOWN_MIN_PERCENT 85
> >
> > struct tcpci {
> > struct device *dev;
> > @@ -366,7 +367,8 @@ static int tcpci_enable_auto_vbus_discharge(struct tcpc_dev *dev, bool enable)
> > }
> >
> > static int tcpci_set_auto_vbus_discharge_threshold(struct tcpc_dev *dev, enum typec_pwr_opmode mode,
> > - bool pps_active, u32 requested_vbus_voltage_mv)
> > + bool pps_active, u32 requested_vbus_voltage_mv,
> > + u32 apdo_min_voltage_mv)
> > {
> > struct tcpci *tcpci = tcpc_to_tcpci(dev);
> > unsigned int pwr_ctrl, threshold = 0;
> > @@ -388,9 +390,12 @@ static int tcpci_set_auto_vbus_discharge_threshold(struct tcpc_dev *dev, enum ty
> > threshold = AUTO_DISCHARGE_DEFAULT_THRESHOLD_MV;
> > } else if (mode == TYPEC_PWR_MODE_PD) {
> > if (pps_active)
> > - threshold = ((VPPS_NEW_MIN_PERCENT * requested_vbus_voltage_mv / 100) -
> > - VSINKPD_MIN_IR_DROP_MV - VPPS_VALID_MIN_MV) *
> > - VSINKDISCONNECT_PD_MIN_PERCENT / 100;
> > + /*
> > + * To prevent disconnect when the source is in Current Limit Mode.
> > + * Set the threshold to the lowest possible voltage vPpsShutdown (min)
> > + */
> > + threshold = VPPS_SHUTDOWN_MIN_PERCENT * apdo_min_voltage_mv / 100 -
> > + VSINKPD_MIN_IR_DROP_MV;
> > else
> > threshold = ((VSRC_NEW_MIN_PERCENT * requested_vbus_voltage_mv / 100) -
> > VSINKPD_MIN_IR_DROP_MV - VSRC_VALID_MIN_MV) *
> > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> > index 460dbde9fe22..e4b85a09c3ae 100644
> > --- a/drivers/usb/typec/tcpm/tcpm.c
> > +++ b/drivers/usb/typec/tcpm/tcpm.c
> > @@ -2973,10 +2973,12 @@ static int tcpm_set_auto_vbus_discharge_threshold(struct tcpm_port *port,
> > return 0;
> >
> > ret = port->tcpc->set_auto_vbus_discharge_threshold(port->tcpc, mode, pps_active,
> > - requested_vbus_voltage);
> > + requested_vbus_voltage,
> > + port->pps_data.min_volt);
> > tcpm_log_force(port,
> > - "set_auto_vbus_discharge_threshold mode:%d pps_active:%c vbus:%u ret:%d",
> > - mode, pps_active ? 'y' : 'n', requested_vbus_voltage, ret);
> > + "set_auto_vbus_discharge_threshold mode:%d pps_active:%c vbus:%u pps_apdo_min_volt:%u ret:%d",
> > + mode, pps_active ? 'y' : 'n', requested_vbus_voltage,
> > + port->pps_data.min_volt, ret);
> >
> > return ret;
> > }
> > diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
> > index 061da9546a81..b22e659f81ba 100644
> > --- a/include/linux/usb/tcpm.h
> > +++ b/include/linux/usb/tcpm.h
> > @@ -163,7 +163,8 @@ struct tcpc_dev {
> > void (*frs_sourcing_vbus)(struct tcpc_dev *dev);
> > int (*enable_auto_vbus_discharge)(struct tcpc_dev *dev, bool enable);
> > int (*set_auto_vbus_discharge_threshold)(struct tcpc_dev *dev, enum typec_pwr_opmode mode,
> > - bool pps_active, u32 requested_vbus_voltage);
> > + bool pps_active, u32 requested_vbus_voltage,
> > + u32 pps_apdo_min_voltage);
> > bool (*is_vbus_vsafe0v)(struct tcpc_dev *dev);
> > void (*set_partner_usb_comm_capable)(struct tcpc_dev *dev, bool enable);
> > void (*check_contaminant)(struct tcpc_dev *dev);
> > --
> > 2.47.1.688.g23fc6f90ad-goog
>
> --
> heikki
