On Tue, Jun 18, 2024 at 10:01:21AM -0500, Mario Limonciello wrote: > On 6/18/2024 03:15, Heikki Krogerus wrote: > > Adding support for the charge type Linux power supply class > > property (POWER_SUPPLY_PROP_CHARGE_TYPE) to the UCSI driver. > > That will make the charge type visible in the charge_type > > power supply class sysfs attribute file. > > > > UCSI has the charge type specified in the Battery Charging > > Status field of the response to the GET_CONNECTOR_STATUS > > command. > > > > Reviewed-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx> > > Signed-off-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx> > > --- > > Changed since the last version: > > The commit message is completely rewritten. The subject line was also changed. > > > > The original patch: > > https://lore.kernel.org/linux-usb/20240617105554.1677285-1-heikki.krogerus@xxxxxxxxxxxxxxx/ > > > > --- > > drivers/usb/typec/ucsi/psy.c | 27 +++++++++++++++++++++++++++ > > drivers/usb/typec/ucsi/ucsi.c | 3 +++ > > 2 files changed, 30 insertions(+) > > > > diff --git a/drivers/usb/typec/ucsi/psy.c b/drivers/usb/typec/ucsi/psy.c > > index b35c6e07911e..b3910f37e171 100644 > > --- a/drivers/usb/typec/ucsi/psy.c > > +++ b/drivers/usb/typec/ucsi/psy.c > > @@ -20,6 +20,7 @@ enum ucsi_psy_online_states { > > }; > > static enum power_supply_property ucsi_psy_props[] = { > > + POWER_SUPPLY_PROP_CHARGE_TYPE, > > POWER_SUPPLY_PROP_USB_TYPE, > > POWER_SUPPLY_PROP_ONLINE, > > POWER_SUPPLY_PROP_VOLTAGE_MIN, > > @@ -194,6 +195,30 @@ static int ucsi_psy_get_usb_type(struct ucsi_connector *con, > > return 0; > > } > > +static int ucsi_psy_get_charge_type(struct ucsi_connector *con, union power_supply_propval *val) > > +{ > > + if (!(con->status.flags & UCSI_CONSTAT_CONNECTED) || > > + (con->status.flags & UCSI_CONSTAT_PWR_DIR) != TYPEC_SINK) { > > + val->intval = POWER_SUPPLY_CHARGE_TYPE_NONE; > > The not connected state obviously makes sense for > POWER_SUPPLY_CHARGE_TYPE_NONE, but what exactly is the other situation? A > UCSI state machine failure? > > I'm mostly wondering if POWER_SUPPLY_CHARGE_TYPE_UNKNOWN makes sense for > that or not. That works for me. I'll change it to POWER_SUPPLY_CHARGE_TYPE_UNKNOWN in that case. > Besides this question the patch looks fine to me and you can add my tag with > your decision. > > Reviewed-by: Mario Limonciello <mario.limonciello@xxxxxxx> thanks, -- heikki
