* [Dry run PATCH 0/2] usb: typec: ucsi: psy: Fix non-PD and advanced PD sources @ 2025-12-08 17:48 Benson Leung 2025-12-08 17:48 ` [PATCH 1/2] usb: typec: ucsi: psy: Fix ucsi_psy_get_current_now in non-PD cases Benson Leung 2025-12-08 17:48 ` [PATCH 2/2] usb: typec: ucsi: psy: Fix voltage and current max for non-Fixed PDOs Benson Leung 0 siblings, 2 replies; 5+ messages in thread From: Benson Leung @ 2025-12-08 17:48 UTC (permalink / raw) To: heikki.krogerus, sebastian.reichel, gregkh, jthies Cc: bleung, linux-usb, linux-kernel, 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] usb: typec: ucsi: psy: Fix ucsi_psy_get_current_now in non-PD cases 2025-12-08 17:48 [Dry run PATCH 0/2] usb: typec: ucsi: psy: Fix non-PD and advanced PD sources Benson Leung @ 2025-12-08 17:48 ` Benson Leung 2025-12-11 13:54 ` Heikki Krogerus 2025-12-08 17:48 ` [PATCH 2/2] usb: typec: ucsi: psy: Fix voltage and current max for non-Fixed PDOs Benson Leung 1 sibling, 1 reply; 5+ messages in thread From: Benson Leung @ 2025-12-08 17:48 UTC (permalink / raw) To: heikki.krogerus, sebastian.reichel, gregkh, jthies Cc: bleung, linux-usb, linux-kernel, 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] usb: typec: ucsi: psy: Fix ucsi_psy_get_current_now in non-PD cases 2025-12-08 17:48 ` [PATCH 1/2] usb: typec: ucsi: psy: Fix ucsi_psy_get_current_now in non-PD cases Benson Leung @ 2025-12-11 13:54 ` Heikki Krogerus 0 siblings, 0 replies; 5+ messages in thread From: Heikki Krogerus @ 2025-12-11 13:54 UTC (permalink / raw) To: Benson Leung Cc: sebastian.reichel, gregkh, jthies, bleung, linux-usb, linux-kernel 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] usb: typec: ucsi: psy: Fix voltage and current max for non-Fixed PDOs 2025-12-08 17:48 [Dry run PATCH 0/2] usb: typec: ucsi: psy: Fix non-PD and advanced PD sources Benson Leung 2025-12-08 17:48 ` [PATCH 1/2] usb: typec: ucsi: psy: Fix ucsi_psy_get_current_now in non-PD cases Benson Leung @ 2025-12-08 17:48 ` Benson Leung 2025-12-11 14:00 ` Heikki Krogerus 1 sibling, 1 reply; 5+ messages in thread From: Benson Leung @ 2025-12-08 17:48 UTC (permalink / raw) To: heikki.krogerus, sebastian.reichel, gregkh, jthies Cc: bleung, linux-usb, linux-kernel, 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] usb: typec: ucsi: psy: Fix voltage and current max for non-Fixed PDOs 2025-12-08 17:48 ` [PATCH 2/2] usb: typec: ucsi: psy: Fix voltage and current max for non-Fixed PDOs Benson Leung @ 2025-12-11 14:00 ` Heikki Krogerus 0 siblings, 0 replies; 5+ messages in thread From: Heikki Krogerus @ 2025-12-11 14:00 UTC (permalink / raw) To: Benson Leung Cc: sebastian.reichel, gregkh, jthies, bleung, linux-usb, linux-kernel 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-12-11 14:01 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-12-08 17:48 [Dry run PATCH 0/2] usb: typec: ucsi: psy: Fix non-PD and advanced PD sources Benson Leung 2025-12-08 17:48 ` [PATCH 1/2] usb: typec: ucsi: psy: Fix ucsi_psy_get_current_now in non-PD cases Benson Leung 2025-12-11 13:54 ` Heikki Krogerus 2025-12-08 17:48 ` [PATCH 2/2] usb: typec: ucsi: psy: Fix voltage and current max for non-Fixed PDOs Benson Leung 2025-12-11 14:00 ` Heikki Krogerus
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox