| Message ID | 20240724201116.2094059-2-jthies@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | usb: typec: ucsi: Expand power supply support | expand |
On Wed, Jul 24, 2024 at 08:11:13PM GMT, Jameson Thies wrote: > Add status to UCSI power supply driver properties based on the port's > connection and power direction states. > > Signed-off-by: Jameson Thies <jthies@google.com> Please CC Power Supply maintainers for this patchset (added to cc). At least per the Documentation/ABI/testing/sysfs-class-power, the status property applies to batteries, while UCSI psy device is a charger. This is logical, as there might be multiple reasons why the battery is not being charging even when the supply is online. > --- > Changes in V2: > - None. > > drivers/usb/typec/ucsi/psy.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/drivers/usb/typec/ucsi/psy.c b/drivers/usb/typec/ucsi/psy.c > index e623d80e177c..d0b52cee41d2 100644 > --- a/drivers/usb/typec/ucsi/psy.c > +++ b/drivers/usb/typec/ucsi/psy.c > @@ -29,6 +29,7 @@ static enum power_supply_property ucsi_psy_props[] = { > POWER_SUPPLY_PROP_CURRENT_MAX, > POWER_SUPPLY_PROP_CURRENT_NOW, > POWER_SUPPLY_PROP_SCOPE, > + POWER_SUPPLY_PROP_STATUS, > }; > > static int ucsi_psy_get_scope(struct ucsi_connector *con, > @@ -51,6 +52,20 @@ static int ucsi_psy_get_scope(struct ucsi_connector *con, > return 0; > } > > +static int ucsi_psy_get_status(struct ucsi_connector *con, > + union power_supply_propval *val) > +{ > + val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING; > + if (con->status.flags & UCSI_CONSTAT_CONNECTED) { > + if ((con->status.flags & UCSI_CONSTAT_PWR_DIR) == TYPEC_SINK) > + val->intval = POWER_SUPPLY_STATUS_CHARGING; > + else > + val->intval = POWER_SUPPLY_STATUS_DISCHARGING; > + } > + > + return 0; > +} > + > static int ucsi_psy_get_online(struct ucsi_connector *con, > union power_supply_propval *val) > { > @@ -249,6 +264,8 @@ static int ucsi_psy_get_prop(struct power_supply *psy, > return ucsi_psy_get_current_now(con, val); > case POWER_SUPPLY_PROP_SCOPE: > return ucsi_psy_get_scope(con, val); > + case POWER_SUPPLY_PROP_STATUS: > + return ucsi_psy_get_status(con, val); > default: > return -EINVAL; > } > -- > 2.45.2.1089.g2a221341d9-goog >
Hi, On Thu, Jul 25, 2024 at 06:31:00AM GMT, Dmitry Baryshkov wrote: > On Wed, Jul 24, 2024 at 08:11:13PM GMT, Jameson Thies wrote: > > Add status to UCSI power supply driver properties based on the port's > > connection and power direction states. > > > > Signed-off-by: Jameson Thies <jthies@google.com> > > Please CC Power Supply maintainers for this patchset (added to cc). Thanks. I guess I should add something like this to MAINTAINERS considering the power-supply bits are in its own file for UCSI: UCSI POWER-SUPPLY API R: Sebastian Reichel <sre@kernel.org> L: linux-pm@vger.kernel.org F: drivers/usb/typec/ucsi/psy.c > At least per the Documentation/ABI/testing/sysfs-class-power, the status > property applies to batteries, while UCSI psy device is a charger. This > is logical, as there might be multiple reasons why the battery is not > being charging even when the supply is online. Correct. These status bits are not meant for chargers. E.g. POWER_SUPPLY_STATUS_NOT_CHARGING has a very specific meaning, that a battery is neither charged nor discharged. Since the device is running that can only happen when a charger is connected, but not charging (or the device has two batteries). For AC the power-supply API has been designed only taking the SINK role into account. At some point USB was added and some time later people added reverse mode to their USB chargers for OTG mode. So far this has been handled by registering a regulator and using that to switch the mode. This made sense for OTG, but USB-C PD makes things even more complex. I am open to ideas how to properly handle the source role in the power-supply API, but let's not overload the status property for it. Please make sure to CC me on any follow-up series. -- Sebastian > > > --- > > Changes in V2: > > - None. > > > > drivers/usb/typec/ucsi/psy.c | 17 +++++++++++++++++ > > 1 file changed, 17 insertions(+) > > > > diff --git a/drivers/usb/typec/ucsi/psy.c b/drivers/usb/typec/ucsi/psy.c > > index e623d80e177c..d0b52cee41d2 100644 > > --- a/drivers/usb/typec/ucsi/psy.c > > +++ b/drivers/usb/typec/ucsi/psy.c > > @@ -29,6 +29,7 @@ static enum power_supply_property ucsi_psy_props[] = { > > POWER_SUPPLY_PROP_CURRENT_MAX, > > POWER_SUPPLY_PROP_CURRENT_NOW, > > POWER_SUPPLY_PROP_SCOPE, > > + POWER_SUPPLY_PROP_STATUS, > > }; > > > > static int ucsi_psy_get_scope(struct ucsi_connector *con, > > @@ -51,6 +52,20 @@ static int ucsi_psy_get_scope(struct ucsi_connector *con, > > return 0; > > } > > > > +static int ucsi_psy_get_status(struct ucsi_connector *con, > > + union power_supply_propval *val) > > +{ > > + val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING; > > + if (con->status.flags & UCSI_CONSTAT_CONNECTED) { > > + if ((con->status.flags & UCSI_CONSTAT_PWR_DIR) == TYPEC_SINK) > > + val->intval = POWER_SUPPLY_STATUS_CHARGING; > > + else > > + val->intval = POWER_SUPPLY_STATUS_DISCHARGING; > > + } > > + > > + return 0; > > +} > > + > > static int ucsi_psy_get_online(struct ucsi_connector *con, > > union power_supply_propval *val) > > { > > @@ -249,6 +264,8 @@ static int ucsi_psy_get_prop(struct power_supply *psy, > > return ucsi_psy_get_current_now(con, val); > > case POWER_SUPPLY_PROP_SCOPE: > > return ucsi_psy_get_scope(con, val); > > + case POWER_SUPPLY_PROP_STATUS: > > + return ucsi_psy_get_status(con, val); > > default: > > return -EINVAL; > > } > > -- > > 2.45.2.1089.g2a221341d9-goog > > > > -- > With best wishes > Dmitry
On Fri, Jul 26, 2024 at 11:30:37PM GMT, Sebastian Reichel wrote: > Hi, > > On Thu, Jul 25, 2024 at 06:31:00AM GMT, Dmitry Baryshkov wrote: > > On Wed, Jul 24, 2024 at 08:11:13PM GMT, Jameson Thies wrote: > > > Add status to UCSI power supply driver properties based on the port's > > > connection and power direction states. > > > > > > Signed-off-by: Jameson Thies <jthies@google.com> > > > > Please CC Power Supply maintainers for this patchset (added to cc). > > Thanks. I guess I should add something like this to MAINTAINERS > considering the power-supply bits are in its own file for UCSI: > > UCSI POWER-SUPPLY API > R: Sebastian Reichel <sre@kernel.org> > L: linux-pm@vger.kernel.org > F: drivers/usb/typec/ucsi/psy.c Or maybe extract a separate USB PD PSY driver, which can get used by other Type-C port managers (I didn't evalue if it makes sense, i.e. if the TCPM drivers can provide data, I just assume they can).
diff --git a/drivers/usb/typec/ucsi/psy.c b/drivers/usb/typec/ucsi/psy.c index e623d80e177c..d0b52cee41d2 100644 --- a/drivers/usb/typec/ucsi/psy.c +++ b/drivers/usb/typec/ucsi/psy.c @@ -29,6 +29,7 @@ static enum power_supply_property ucsi_psy_props[] = { POWER_SUPPLY_PROP_CURRENT_MAX, POWER_SUPPLY_PROP_CURRENT_NOW, POWER_SUPPLY_PROP_SCOPE, + POWER_SUPPLY_PROP_STATUS, }; static int ucsi_psy_get_scope(struct ucsi_connector *con, @@ -51,6 +52,20 @@ static int ucsi_psy_get_scope(struct ucsi_connector *con, return 0; } +static int ucsi_psy_get_status(struct ucsi_connector *con, + union power_supply_propval *val) +{ + val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING; + if (con->status.flags & UCSI_CONSTAT_CONNECTED) { + if ((con->status.flags & UCSI_CONSTAT_PWR_DIR) == TYPEC_SINK) + val->intval = POWER_SUPPLY_STATUS_CHARGING; + else + val->intval = POWER_SUPPLY_STATUS_DISCHARGING; + } + + return 0; +} + static int ucsi_psy_get_online(struct ucsi_connector *con, union power_supply_propval *val) { @@ -249,6 +264,8 @@ static int ucsi_psy_get_prop(struct power_supply *psy, return ucsi_psy_get_current_now(con, val); case POWER_SUPPLY_PROP_SCOPE: return ucsi_psy_get_scope(con, val); + case POWER_SUPPLY_PROP_STATUS: + return ucsi_psy_get_status(con, val); default: return -EINVAL; }
Add status to UCSI power supply driver properties based on the port's connection and power direction states. Signed-off-by: Jameson Thies <jthies@google.com> --- Changes in V2: - None. drivers/usb/typec/ucsi/psy.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)