* [PATCH v1] usb: typec: tcpci: Prevent Sink disconnection before vPpsShutdown in SPR PPS
@ 2025-01-14 14:24 Kyle Tso
2025-01-15 5:06 ` Badhri Jagan Sridharan
2025-01-16 11:25 ` Heikki Krogerus
0 siblings, 2 replies; 6+ messages in thread
From: Kyle Tso @ 2025-01-14 14:24 UTC (permalink / raw)
To: heikki.krogerus, gregkh, andre.draszik, rdbabiera, m.felsch,
xu.yang_2, u.kleine-koenig, emanuele.ghidoli, badhri, amitsd
Cc: linux-kernel, linux-usb, Kyle Tso, stable
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@vger.kernel.org
Signed-off-by: Kyle Tso <kyletso@google.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v1] usb: typec: tcpci: Prevent Sink disconnection before vPpsShutdown in SPR PPS
2025-01-14 14:24 [PATCH v1] usb: typec: tcpci: Prevent Sink disconnection before vPpsShutdown in SPR PPS Kyle Tso
@ 2025-01-15 5:06 ` Badhri Jagan Sridharan
2025-01-16 11:25 ` Heikki Krogerus
1 sibling, 0 replies; 6+ messages in thread
From: Badhri Jagan Sridharan @ 2025-01-15 5:06 UTC (permalink / raw)
To: Kyle Tso
Cc: heikki.krogerus, gregkh, andre.draszik, rdbabiera, m.felsch,
xu.yang_2, u.kleine-koenig, emanuele.ghidoli, amitsd,
linux-kernel, linux-usb, stable
On Tue, Jan 14, 2025 at 6:25 AM Kyle Tso <kyletso@google.com> 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@vger.kernel.org
> Signed-off-by: Kyle Tso <kyletso@google.com>
Reviewed-by: Badhri Jagan Sridharan <badhri@google.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v1] usb: typec: tcpci: Prevent Sink disconnection before vPpsShutdown in SPR PPS
2025-01-14 14:24 [PATCH v1] usb: typec: tcpci: Prevent Sink disconnection before vPpsShutdown in SPR PPS Kyle Tso
2025-01-15 5:06 ` Badhri Jagan Sridharan
@ 2025-01-16 11:25 ` Heikki Krogerus
2025-01-16 11:41 ` Kyle Tso
1 sibling, 1 reply; 6+ messages in thread
From: Heikki Krogerus @ 2025-01-16 11:25 UTC (permalink / raw)
To: Kyle Tso
Cc: gregkh, andre.draszik, rdbabiera, m.felsch, xu.yang_2,
u.kleine-koenig, emanuele.ghidoli, badhri, amitsd, linux-kernel,
linux-usb, stable
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@vger.kernel.org
> Signed-off-by: Kyle Tso <kyletso@google.com>
You've resend this, right? So is this v2 (or v1)?
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v1] usb: typec: tcpci: Prevent Sink disconnection before vPpsShutdown in SPR PPS
2025-01-16 11:25 ` Heikki Krogerus
@ 2025-01-16 11:41 ` Kyle Tso
2025-01-17 11:22 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Kyle Tso @ 2025-01-16 11:41 UTC (permalink / raw)
To: Heikki Krogerus
Cc: gregkh, andre.draszik, rdbabiera, m.felsch, xu.yang_2,
u.kleine-koenig, emanuele.ghidoli, badhri, amitsd, linux-kernel,
linux-usb, stable
On Thu, Jan 16, 2025 at 7:25 PM Heikki Krogerus
<heikki.krogerus@linux.intel.com> 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@vger.kernel.org
> > Signed-off-by: Kyle Tso <kyletso@google.com>
>
> You've resend this, right? So is this v2 (or v1)?
>
> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v1] usb: typec: tcpci: Prevent Sink disconnection before vPpsShutdown in SPR PPS
2025-01-16 11:41 ` Kyle Tso
@ 2025-01-17 11:22 ` Greg KH
0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2025-01-17 11:22 UTC (permalink / raw)
To: Kyle Tso
Cc: Heikki Krogerus, andre.draszik, rdbabiera, m.felsch, xu.yang_2,
u.kleine-koenig, emanuele.ghidoli, badhri, amitsd, linux-kernel,
linux-usb, stable
On Thu, Jan 16, 2025 at 07:41:16PM +0800, Kyle Tso wrote:
> On Thu, Jan 16, 2025 at 7:25 PM Heikki Krogerus
> <heikki.krogerus@linux.intel.com> 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@vger.kernel.org
> > > Signed-off-by: Kyle Tso <kyletso@google.com>
> >
> > You've resend this, right? So is this v2 (or v1)?
> >
> > Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> >
>
> 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.
Our tools play havoc when we have duplicates like this, always increment
the version number when resending as obviously you did the resend for
some reason. Also, it let's us know which ones to review, what would
you do if you saw both of these in your inbox?
I'll try to fix this up by hand this time..
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v1] usb: typec: tcpci: Prevent Sink disconnection before vPpsShutdown in SPR PPS
@ 2025-01-14 14:23 Kyle Tso
0 siblings, 0 replies; 6+ messages in thread
From: Kyle Tso @ 2025-01-14 14:23 UTC (permalink / raw)
To: heikki.krogerus, gregkh, andre.draszik, rdbabiera, m.felsch,
xu.yang_2, u.kleine-koenig, emanuele.ghidoli
Cc: linux-kernel, linux-usb, Kyle Tso, stable
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@vger.kernel.org
Signed-off-by: Kyle Tso <kyletso@google.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-01-17 11:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-14 14:24 [PATCH v1] usb: typec: tcpci: Prevent Sink disconnection before vPpsShutdown in SPR PPS Kyle Tso
2025-01-15 5:06 ` Badhri Jagan Sridharan
2025-01-16 11:25 ` Heikki Krogerus
2025-01-16 11:41 ` Kyle Tso
2025-01-17 11:22 ` Greg KH
-- strict thread matches above, loose matches on Subject: below --
2025-01-14 14:23 Kyle Tso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).