[Dry run PATCH 0/2] usb: typec: ucsi: psy: Fix non-PD and advanced PD sources

[Dry run PATCH 0/2] usb: typec: ucsi: psy: Fix non-PD and advanced PD sources

[Benson Leung]

Addresses two USB-C power source related problems jthies@google.com and I
discovered in the ucsi/psy driver.

1. Non-USBPD USB-C chargers (USB Type-C 3A only, 1.5A only, or Legacy USB)
2. Advanced USB PD Chargers (Supporting PPS and SPR AVS).

Benson Leung (2):
  usb: typec: ucsi: psy: Fix ucsi_psy_get_current_now in non-PD cases
  usb: typec: ucsi: psy: Fix voltage and current max for non-Fixed PDOs

 drivers/usb/typec/ucsi/psy.c | 54 +++++++++++++++++++++++++++---------
 1 file changed, 41 insertions(+), 13 deletions(-)

-- 
2.52.0.223.gf5cc29aaa4-goog

[PATCH 1/2] usb: typec: ucsi: psy: Fix ucsi_psy_get_current_now in non-PD cases

[Benson Leung]

current_now would always return 0 in for non-PD power sources, and the
negotiated current based on the Request RDO in PD mode.

For USB Type-C current or legacy Default USB cases current_now will present
the max value of those modes, as that is the equivalent of the Request RDO
in PD.

Also, current_now will now return 0 when the port is disconnected to match
the same behavior of current_max.

Signed-off-by: Benson Leung <bleung@chromium.org>
---
 drivers/usb/typec/ucsi/psy.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/typec/ucsi/psy.c b/drivers/usb/typec/ucsi/psy.c
index 3abe9370ffaa..b828719e33df 100644
--- a/drivers/usb/typec/ucsi/psy.c
+++ b/drivers/usb/typec/ucsi/psy.c
@@ -202,10 +202,28 @@ static int ucsi_psy_get_current_max(struct ucsi_connector *con,
 static int ucsi_psy_get_current_now(struct ucsi_connector *con,
 				    union power_supply_propval *val)
 {
-	if (UCSI_CONSTAT(con, PWR_OPMODE) == UCSI_CONSTAT_PWR_OPMODE_PD)
-		val->intval = rdo_op_current(con->rdo) * 1000;
-	else
+	if (!UCSI_CONSTAT(con, CONNECTED)) {
 		val->intval = 0;
+		return 0;
+	}
+
+	switch (UCSI_CONSTAT(con, PWR_OPMODE)) {
+	case UCSI_CONSTAT_PWR_OPMODE_PD:
+		val->intval = rdo_op_current(con->rdo) * 1000;
+		break;
+	case UCSI_CONSTAT_PWR_OPMODE_TYPEC1_5:
+		val->intval = UCSI_TYPEC_1_5_CURRENT * 1000;
+		break;
+	case UCSI_CONSTAT_PWR_OPMODE_TYPEC3_0:
+		val->intval = UCSI_TYPEC_3_0_CURRENT * 1000;
+		break;
+	case UCSI_CONSTAT_PWR_OPMODE_BC:
+	case UCSI_CONSTAT_PWR_OPMODE_DEFAULT:
+	/* UCSI can't tell b/w DCP/CDP or USB2/3x1/3x2 SDP chargers */
+	default:
+		val->intval = UCSI_TYPEC_DEFAULT_CURRENT * 1000;
+		break;
+	}
 	return 0;
 }
 
-- 
2.52.0.223.gf5cc29aaa4-goog

[PATCH 2/2] usb: typec: ucsi: psy: Fix voltage and current max for non-Fixed PDOs

[Benson Leung]

ucsi_psy_get_voltage_max and ucsi_psy_get_current_max are calculated
using whichever pdo is in the last position of the src_pdos array, presuming
it to be a fixed pdo, so the pdo_fixed_voltage or pdo_max_current
helpers are used on that last pdo.

However, non-Fixed PDOs such as Battery PDOs, Augmented PDOs (used for AVS and
for PPS) may exist, and are always at the end of the array if they do.
In the event one of these more advanced chargers are attached the helpers for
fixed return mangled values.

Here's an example case of a Google Pixel Flex Dual Port 67W USB-C Fast Charger
with PPS support:
POWER_SUPPLY_NAME=ucsi-source-psy-cros_ec_ucsi.4.auto2
POWER_SUPPLY_TYPE=USB
POWER_SUPPLY_CHARGE_TYPE=Standard
POWER_SUPPLY_USB_TYPE=C [PD] PD_PPS PD_DRP
POWER_SUPPLY_ONLINE=1
POWER_SUPPLY_VOLTAGE_MIN=5000000
POWER_SUPPLY_VOLTAGE_MAX=13400000
POWER_SUPPLY_VOLTAGE_NOW=20000000
POWER_SUPPLY_CURRENT_MAX=5790000
POWER_SUPPLY_CURRENT_NOW=3250000

Voltage Max is reading as 13.4V, but that's an incorrect decode of the PPS
APDO in the last position. Same goes for CURRENT_MAX. 5.79A is incorrect.

Instead, enumerate through the src_pdos and filter just for Fixed PDOs for
now, and find the one with the highest voltage and current respectively.

After, from the same charger:
POWER_SUPPLY_NAME=ucsi-source-psy-cros_ec_ucsi.4.auto2
POWER_SUPPLY_TYPE=USB
POWER_SUPPLY_CHARGE_TYPE=Standard
POWER_SUPPLY_USB_TYPE=C [PD] PD_PPS PD_DRP
POWER_SUPPLY_ONLINE=1
POWER_SUPPLY_VOLTAGE_MIN=5000000
POWER_SUPPLY_VOLTAGE_MAX=20000000
POWER_SUPPLY_VOLTAGE_NOW=20000000
POWER_SUPPLY_CURRENT_MAX=4000000
POWER_SUPPLY_CURRENT_NOW=3250000

Signed-off-by: Benson Leung <bleung@chromium.org>
---
 drivers/usb/typec/ucsi/psy.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/typec/ucsi/psy.c b/drivers/usb/typec/ucsi/psy.c
index b828719e33df..c8ebab8ba7e7 100644
--- a/drivers/usb/typec/ucsi/psy.c
+++ b/drivers/usb/typec/ucsi/psy.c
@@ -112,15 +112,20 @@ static int ucsi_psy_get_voltage_max(struct ucsi_connector *con,
 				    union power_supply_propval *val)
 {
 	u32 pdo;
+	int max_voltage = 0;
 
 	switch (UCSI_CONSTAT(con, PWR_OPMODE)) {
 	case UCSI_CONSTAT_PWR_OPMODE_PD:
-		if (con->num_pdos > 0) {
-			pdo = con->src_pdos[con->num_pdos - 1];
-			val->intval = pdo_fixed_voltage(pdo) * 1000;
-		} else {
-			val->intval = 0;
+		for (int i = 0; i < con->num_pdos; i++) {
+			int pdo_voltage = 0;
+
+			pdo = con->src_pdos[i];
+			if (pdo_type(pdo) == PDO_TYPE_FIXED)
+				pdo_voltage = pdo_fixed_voltage(pdo) * 1000;
+			max_voltage = (pdo_voltage > max_voltage) ? pdo_voltage
+								  : max_voltage;
 		}
+		val->intval = max_voltage;
 		break;
 	case UCSI_CONSTAT_PWR_OPMODE_TYPEC3_0:
 	case UCSI_CONSTAT_PWR_OPMODE_TYPEC1_5:
@@ -168,6 +173,7 @@ static int ucsi_psy_get_current_max(struct ucsi_connector *con,
 				    union power_supply_propval *val)
 {
 	u32 pdo;
+	int max_current = 0;
 
 	if (!UCSI_CONSTAT(con, CONNECTED)) {
 		val->intval = 0;
@@ -176,12 +182,16 @@ static int ucsi_psy_get_current_max(struct ucsi_connector *con,
 
 	switch (UCSI_CONSTAT(con, PWR_OPMODE)) {
 	case UCSI_CONSTAT_PWR_OPMODE_PD:
-		if (con->num_pdos > 0) {
-			pdo = con->src_pdos[con->num_pdos - 1];
-			val->intval = pdo_max_current(pdo) * 1000;
-		} else {
-			val->intval = 0;
+		for (int i = 0; i < con->num_pdos; i++) {
+			int pdo_current = 0;
+
+			pdo = con->src_pdos[i];
+			if (pdo_type(pdo) == PDO_TYPE_FIXED)
+				pdo_current = pdo_max_current(pdo) * 1000;
+			max_current = (pdo_current > max_current) ? pdo_current
+								  : max_current;
 		}
+		val->intval = max_current;
 		break;
 	case UCSI_CONSTAT_PWR_OPMODE_TYPEC1_5:
 		val->intval = UCSI_TYPEC_1_5_CURRENT * 1000;
-- 
2.52.0.223.gf5cc29aaa4-goog

Re: [PATCH 1/2] usb: typec: ucsi: psy: Fix ucsi_psy_get_current_now in non-PD cases

[Heikki Krogerus]

Mon, Dec 08, 2025 at 05:48:47PM +0000, Benson Leung kirjoitti:
> current_now would always return 0 in for non-PD power sources, and the
> negotiated current based on the Request RDO in PD mode.
> 
> For USB Type-C current or legacy Default USB cases current_now will present
> the max value of those modes, as that is the equivalent of the Request RDO
> in PD.
> 
> Also, current_now will now return 0 when the port is disconnected to match
> the same behavior of current_max.
> 
> Signed-off-by: Benson Leung <bleung@chromium.org>

Looks good to me. Shouldn't this be applied also to the stable trees?

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  drivers/usb/typec/ucsi/psy.c | 24 +++++++++++++++++++++---
>  1 file changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/typec/ucsi/psy.c b/drivers/usb/typec/ucsi/psy.c
> index 3abe9370ffaa..b828719e33df 100644
> --- a/drivers/usb/typec/ucsi/psy.c
> +++ b/drivers/usb/typec/ucsi/psy.c
> @@ -202,10 +202,28 @@ static int ucsi_psy_get_current_max(struct ucsi_connector *con,
>  static int ucsi_psy_get_current_now(struct ucsi_connector *con,
>  				    union power_supply_propval *val)
>  {
> -	if (UCSI_CONSTAT(con, PWR_OPMODE) == UCSI_CONSTAT_PWR_OPMODE_PD)
> -		val->intval = rdo_op_current(con->rdo) * 1000;
> -	else
> +	if (!UCSI_CONSTAT(con, CONNECTED)) {
>  		val->intval = 0;
> +		return 0;
> +	}
> +
> +	switch (UCSI_CONSTAT(con, PWR_OPMODE)) {
> +	case UCSI_CONSTAT_PWR_OPMODE_PD:
> +		val->intval = rdo_op_current(con->rdo) * 1000;
> +		break;
> +	case UCSI_CONSTAT_PWR_OPMODE_TYPEC1_5:
> +		val->intval = UCSI_TYPEC_1_5_CURRENT * 1000;
> +		break;
> +	case UCSI_CONSTAT_PWR_OPMODE_TYPEC3_0:
> +		val->intval = UCSI_TYPEC_3_0_CURRENT * 1000;
> +		break;
> +	case UCSI_CONSTAT_PWR_OPMODE_BC:
> +	case UCSI_CONSTAT_PWR_OPMODE_DEFAULT:
> +	/* UCSI can't tell b/w DCP/CDP or USB2/3x1/3x2 SDP chargers */
> +	default:
> +		val->intval = UCSI_TYPEC_DEFAULT_CURRENT * 1000;
> +		break;
> +	}
>  	return 0;
>  }
>  
> -- 
> 2.52.0.223.gf5cc29aaa4-goog

-- 
heikki

Re: [PATCH 2/2] usb: typec: ucsi: psy: Fix voltage and current max for non-Fixed PDOs

[Heikki Krogerus]

Mon, Dec 08, 2025 at 05:48:48PM +0000, Benson Leung kirjoitti:
> ucsi_psy_get_voltage_max and ucsi_psy_get_current_max are calculated
> using whichever pdo is in the last position of the src_pdos array, presuming
> it to be a fixed pdo, so the pdo_fixed_voltage or pdo_max_current
> helpers are used on that last pdo.
> 
> However, non-Fixed PDOs such as Battery PDOs, Augmented PDOs (used for AVS and
> for PPS) may exist, and are always at the end of the array if they do.
> In the event one of these more advanced chargers are attached the helpers for
> fixed return mangled values.
> 
> Here's an example case of a Google Pixel Flex Dual Port 67W USB-C Fast Charger
> with PPS support:
> POWER_SUPPLY_NAME=ucsi-source-psy-cros_ec_ucsi.4.auto2
> POWER_SUPPLY_TYPE=USB
> POWER_SUPPLY_CHARGE_TYPE=Standard
> POWER_SUPPLY_USB_TYPE=C [PD] PD_PPS PD_DRP
> POWER_SUPPLY_ONLINE=1
> POWER_SUPPLY_VOLTAGE_MIN=5000000
> POWER_SUPPLY_VOLTAGE_MAX=13400000
> POWER_SUPPLY_VOLTAGE_NOW=20000000
> POWER_SUPPLY_CURRENT_MAX=5790000
> POWER_SUPPLY_CURRENT_NOW=3250000
> 
> Voltage Max is reading as 13.4V, but that's an incorrect decode of the PPS
> APDO in the last position. Same goes for CURRENT_MAX. 5.79A is incorrect.
> 
> Instead, enumerate through the src_pdos and filter just for Fixed PDOs for
> now, and find the one with the highest voltage and current respectively.
> 
> After, from the same charger:
> POWER_SUPPLY_NAME=ucsi-source-psy-cros_ec_ucsi.4.auto2
> POWER_SUPPLY_TYPE=USB
> POWER_SUPPLY_CHARGE_TYPE=Standard
> POWER_SUPPLY_USB_TYPE=C [PD] PD_PPS PD_DRP
> POWER_SUPPLY_ONLINE=1
> POWER_SUPPLY_VOLTAGE_MIN=5000000
> POWER_SUPPLY_VOLTAGE_MAX=20000000
> POWER_SUPPLY_VOLTAGE_NOW=20000000
> POWER_SUPPLY_CURRENT_MAX=4000000
> POWER_SUPPLY_CURRENT_NOW=3250000
> 
> Signed-off-by: Benson Leung <bleung@chromium.org>

Stable material as well?

Instead of using the ternary operator you could have used a simple if
statement. But that's minor.

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  drivers/usb/typec/ucsi/psy.c | 30 ++++++++++++++++++++----------
>  1 file changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/usb/typec/ucsi/psy.c b/drivers/usb/typec/ucsi/psy.c
> index b828719e33df..c8ebab8ba7e7 100644
> --- a/drivers/usb/typec/ucsi/psy.c
> +++ b/drivers/usb/typec/ucsi/psy.c
> @@ -112,15 +112,20 @@ static int ucsi_psy_get_voltage_max(struct ucsi_connector *con,
>  				    union power_supply_propval *val)
>  {
>  	u32 pdo;
> +	int max_voltage = 0;
>  
>  	switch (UCSI_CONSTAT(con, PWR_OPMODE)) {
>  	case UCSI_CONSTAT_PWR_OPMODE_PD:
> -		if (con->num_pdos > 0) {
> -			pdo = con->src_pdos[con->num_pdos - 1];
> -			val->intval = pdo_fixed_voltage(pdo) * 1000;
> -		} else {
> -			val->intval = 0;
> +		for (int i = 0; i < con->num_pdos; i++) {
> +			int pdo_voltage = 0;
> +
> +			pdo = con->src_pdos[i];
> +			if (pdo_type(pdo) == PDO_TYPE_FIXED)
> +				pdo_voltage = pdo_fixed_voltage(pdo) * 1000;
> +			max_voltage = (pdo_voltage > max_voltage) ? pdo_voltage
> +								  : max_voltage;
>  		}
> +		val->intval = max_voltage;
>  		break;
>  	case UCSI_CONSTAT_PWR_OPMODE_TYPEC3_0:
>  	case UCSI_CONSTAT_PWR_OPMODE_TYPEC1_5:
> @@ -168,6 +173,7 @@ static int ucsi_psy_get_current_max(struct ucsi_connector *con,
>  				    union power_supply_propval *val)
>  {
>  	u32 pdo;
> +	int max_current = 0;
>  
>  	if (!UCSI_CONSTAT(con, CONNECTED)) {
>  		val->intval = 0;
> @@ -176,12 +182,16 @@ static int ucsi_psy_get_current_max(struct ucsi_connector *con,
>  
>  	switch (UCSI_CONSTAT(con, PWR_OPMODE)) {
>  	case UCSI_CONSTAT_PWR_OPMODE_PD:
> -		if (con->num_pdos > 0) {
> -			pdo = con->src_pdos[con->num_pdos - 1];
> -			val->intval = pdo_max_current(pdo) * 1000;
> -		} else {
> -			val->intval = 0;
> +		for (int i = 0; i < con->num_pdos; i++) {
> +			int pdo_current = 0;
> +
> +			pdo = con->src_pdos[i];
> +			if (pdo_type(pdo) == PDO_TYPE_FIXED)
> +				pdo_current = pdo_max_current(pdo) * 1000;
> +			max_current = (pdo_current > max_current) ? pdo_current
> +								  : max_current;
>  		}
> +		val->intval = max_current;
>  		break;
>  	case UCSI_CONSTAT_PWR_OPMODE_TYPEC1_5:
>  		val->intval = UCSI_TYPEC_1_5_CURRENT * 1000;
> -- 
> 2.52.0.223.gf5cc29aaa4-goog

-- 
heikki