public inbox for linux-pm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] power: supply: Change usb_types from an array into a bitmask
@ 2024-08-31 14:20 Hans de Goede
  2024-08-31 14:20 ` [PATCH 1/6] power: supply: "usb_type" property may be written to Hans de Goede
                   ` (7 more replies)
  0 siblings, 8 replies; 14+ messages in thread
From: Hans de Goede @ 2024-08-31 14:20 UTC (permalink / raw)
  To: Sebastian Reichel, Thomas Weißschuh, MyungJoo Ham,
	Chanwoo Choi, Greg Kroah-Hartman, Heikki Krogerus, Vinod Koul,
	Kishon Vijay Abraham I
  Cc: Hans de Goede, Enric Balletbo Serra, Andrey Smirnov, linux-pm,
	linux-kernel, linux-usb, linux-phy

Hi All,

When support for the "charge_behaviour" property was added the list of
available values was made a bitmask in power_supply_desc.

"usb_types" is very similar in that:
1. It is an enum
2. The list of available values is stored in power_supply_desc
3. When shown it shows all available values, with the active one surrounded
   by square brackets.

But "usb_types" uses an array with valid enum values instead of a bitmask.
This uses more memory then the bitmap approach and it makes it impossible
to have a shared generic show() function for properties which show
available values, with the active one surrounded by square brackets.

This patch-set moves "usb_types" over to a bitmask in power_supply_desc
to indicate the available values.

Patches 1 - 3:

It turns out that the ucs1002-power driver contained a surprise in that
it supports writing to "usb_type" even though the ABI doc says it is
read-only. Since we cannot break shipped userspace API, the ship has sailed
on this one. The first patch documents that writing "usb_type" is allowed,
but only for power-supply devices which provide USB power rather then
consume it.

Enum properties accept writing the FOO_TEXT[] string values, passing
the enum value matching the FOO_TEXT entry to set_property(), the second
patch adjusts ucs1002_set_usb_type() to directly accept enum values.

The rt9467 driver was another driver which allowed writing to "usb_type"
but there the use made no sense, so it is simply dropped.

Patches 4 - 6:

These patches implement the actual moving of usb_types to a bitmask.

Patch 6 is a bit of a bigbang patch moving all drivers over in one go,
touching a couple of drivers outside drivers/power/supply: 1 in
drivers/extcon/ 1 in drivers/phy/ and 5 in drivers/usb/typec/ since
the changes outside of drivers/power/supply are small I've chosen to
make all the changes in one go rather then have some sort of
intermediate state where both ways are supported.

For merging this I believe it would be best for an immutable branch / tag
to be created on the linux-power-supply tree and then send a pull-request
to the extcon, phy and usb-typec maintainers to merge the tag.

extcon, phy and typec maintainers can you please give your Acked-by for
patch 6/6 for merging these changes through the linux-power-supply tree?

This set is based on top of the latest linux-power-supply/for-next.

Regards,

Hans


Hans de Goede (6):
  power: supply: "usb_type" property may be written to
  power: supply: ucs1002: Adjust ucs1002_set_usb_type() to accept string
    values
  power: supply: rt9467-charger: Remove "usb_type" property write
    support
  power: supply: sysfs: Add power_supply_show_enum_with_available()
    helper
  power: supply: sysfs: Move power_supply_show_enum_with_available() up
  power: supply: Change usb_types from an array into a bitmask

 Documentation/ABI/testing/sysfs-class-power   |  7 +-
 drivers/extcon/extcon-intel-cht-wc.c          | 15 ++---
 drivers/phy/ti/phy-tusb1210.c                 | 11 +---
 drivers/power/supply/axp20x_usb_power.c       | 13 ++--
 drivers/power/supply/bq256xx_charger.c        | 15 ++---
 drivers/power/supply/cros_usbpd-charger.c     | 22 +++----
 .../power/supply/lenovo_yoga_c630_battery.c   |  7 +-
 drivers/power/supply/mp2629_charger.c         | 15 ++---
 drivers/power/supply/mt6360_charger.c         | 13 ++--
 drivers/power/supply/mt6370-charger.c         | 13 ++--
 drivers/power/supply/power_supply_core.c      |  4 --
 drivers/power/supply/power_supply_sysfs.c     | 66 ++++++-------------
 drivers/power/supply/qcom_battmgr.c           | 37 ++++++-----
 drivers/power/supply/qcom_pmi8998_charger.c   | 13 ++--
 drivers/power/supply/rk817_charger.c          |  9 +--
 drivers/power/supply/rn5t618_power.c          | 13 ++--
 drivers/power/supply/rt9467-charger.c         | 16 ++---
 drivers/power/supply/rt9471.c                 | 15 ++---
 drivers/power/supply/ucs1002_power.c          | 26 ++++----
 drivers/usb/typec/anx7411.c                   | 11 +---
 drivers/usb/typec/rt1719.c                    | 11 +---
 drivers/usb/typec/tcpm/tcpm.c                 | 11 +---
 drivers/usb/typec/tipd/core.c                 |  9 +--
 drivers/usb/typec/ucsi/psy.c                  | 11 +---
 include/linux/power_supply.h                  |  3 +-
 25 files changed, 132 insertions(+), 254 deletions(-)

-- 
2.46.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/6] power: supply: "usb_type" property may be written to
  2024-08-31 14:20 [PATCH 0/6] power: supply: Change usb_types from an array into a bitmask Hans de Goede
@ 2024-08-31 14:20 ` Hans de Goede
  2024-09-03  7:04   ` Greg Kroah-Hartman
  2024-08-31 14:20 ` [PATCH 2/6] power: supply: ucs1002: Adjust ucs1002_set_usb_type() to accept string values Hans de Goede
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Hans de Goede @ 2024-08-31 14:20 UTC (permalink / raw)
  To: Sebastian Reichel, Thomas Weißschuh, MyungJoo Ham,
	Chanwoo Choi, Greg Kroah-Hartman, Heikki Krogerus, Vinod Koul,
	Kishon Vijay Abraham I
  Cc: Hans de Goede, Enric Balletbo Serra, Andrey Smirnov, linux-pm,
	linux-kernel, linux-usb, linux-phy

According to Documentation/ABI/testing/sysfs-class-power the "usb_type"
property is Read-Only.

For power-supplies which consume USB power such as battery charger chips,
this is correct.

But the UCS1002 USB Port Power Controller driver which is a driver
for a chip which is a power-source for USB-A charging ports "usb_type"
is actually writable to configure the type of USB charger emulated
by the USB-A port.

Adjust the docs and the power_supply_sysfs.c code to adjust for this
new writeable use of "usb_type":

1. Update Documentation/ABI/testing/sysfs-class-power to document that
"usb_type" may be writable

2. Change the power_supply_attr type in power_supply_sysfs.c from
POWER_SUPPLY_ATTR() into POWER_SUPPLY_ENUM_ATTR() so that the various
usb_type string values from POWER_SUPPLY_TYPE_TEXT[] such as e.g.
"SDP" and "USB_PD" can be written to the "usb_type" attribute instead
of only accepting integer values.

Cc: Enric Balletbo Serra <enric.balletbo@collabora.com>
Cc: Andrey Smirnov <andrew.smirnov@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 Documentation/ABI/testing/sysfs-class-power | 7 ++++++-
 drivers/power/supply/power_supply_sysfs.c   | 2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
index 84973f66b42c..45180b62d426 100644
--- a/Documentation/ABI/testing/sysfs-class-power
+++ b/Documentation/ABI/testing/sysfs-class-power
@@ -608,7 +608,12 @@ Description:
 		the supply, for example it can show if USB-PD capable source
 		is attached.
 
-		Access: Read-Only
+		Access: For power-supplies which consume USB power such
+		as battery charger chips, this indicates the type of
+		the connected USB power source and is Read-Only.
+
+		For power-supplies which act as a USB power-source such as
+		e.g. the UCS1002 USB Port Power Controller this is writable.
 
 		Valid values:
 			      "Unknown", "SDP", "DCP", "CDP", "ACA", "C", "PD",
diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
index 3e63d165b2f7..ff7e423edd57 100644
--- a/drivers/power/supply/power_supply_sysfs.c
+++ b/drivers/power/supply/power_supply_sysfs.c
@@ -209,7 +209,7 @@ static struct power_supply_attr power_supply_attrs[] = {
 	POWER_SUPPLY_ATTR(TIME_TO_FULL_NOW),
 	POWER_SUPPLY_ATTR(TIME_TO_FULL_AVG),
 	POWER_SUPPLY_ENUM_ATTR(TYPE),
-	POWER_SUPPLY_ATTR(USB_TYPE),
+	POWER_SUPPLY_ENUM_ATTR(USB_TYPE),
 	POWER_SUPPLY_ENUM_ATTR(SCOPE),
 	POWER_SUPPLY_ATTR(PRECHARGE_CURRENT),
 	POWER_SUPPLY_ATTR(CHARGE_TERM_CURRENT),
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/6] power: supply: ucs1002: Adjust ucs1002_set_usb_type() to accept string values
  2024-08-31 14:20 [PATCH 0/6] power: supply: Change usb_types from an array into a bitmask Hans de Goede
  2024-08-31 14:20 ` [PATCH 1/6] power: supply: "usb_type" property may be written to Hans de Goede
@ 2024-08-31 14:20 ` Hans de Goede
  2024-08-31 14:20 ` [PATCH 3/6] power: supply: rt9467-charger: Remove "usb_type" property write support Hans de Goede
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Hans de Goede @ 2024-08-31 14:20 UTC (permalink / raw)
  To: Sebastian Reichel, Thomas Weißschuh, MyungJoo Ham,
	Chanwoo Choi, Greg Kroah-Hartman, Heikki Krogerus, Vinod Koul,
	Kishon Vijay Abraham I
  Cc: Hans de Goede, Enric Balletbo Serra, Andrey Smirnov, linux-pm,
	linux-kernel, linux-usb, linux-phy

power_supply_sysfs.c accept wrrites of strings to "usb_type" for strings
values matching an entry in POWER_SUPPLY_USB_TYPE_TEXT[]. If such a
string value is written then the int value passed to ucs1002_set_property()
will be an enum power_supply_usb_type value.

Before this change ucs1002_set_usb_type() expected the value to be an index
into ucs1002_usb_types[]. Adjust ucs1002_set_usb_type() to use the enum
value directly so that writing string values works.

The list of supported types in ucs1002_usb_types[] is: PD, SDP, DCP, CDP.
The [POWER_SUPPLY_USB_TYPE_]SDP, DCP and CDP enum labels have a value of
1, 2 and 3. So userspace selecting SDP, DCP or CDP by writing 1, 2 or 3
will keep working. POWER_SUPPLY_USB_TYPE_PD which is mapped to the ucs1002
dedicated mode however has a value of 6. Before this change writing 0 would
select the dedicated mode. To preserve userspace API compatibility also map
POWER_SUPPLY_USB_TYPE_UNKNOWN (which is 0) to the dedicated mode.

Cc: Enric Balletbo Serra <enric.balletbo@collabora.com>
Cc: Andrey Smirnov <andrew.smirnov@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/ucs1002_power.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/power/supply/ucs1002_power.c b/drivers/power/supply/ucs1002_power.c
index 7970843a4f48..b67d5b03d93e 100644
--- a/drivers/power/supply/ucs1002_power.c
+++ b/drivers/power/supply/ucs1002_power.c
@@ -308,10 +308,13 @@ static int ucs1002_set_usb_type(struct ucs1002_info *info, int val)
 {
 	unsigned int mode;
 
-	if (val < 0 || val >= ARRAY_SIZE(ucs1002_usb_types))
-		return -EINVAL;
-
-	switch (ucs1002_usb_types[val]) {
+	switch (val) {
+	/*
+	 * POWER_SUPPLY_USB_TYPE_UNKNOWN == 0, map this to dedicated for
+	 * userspace API compatibility with older versions of this driver
+	 * which mapped 0 to dedicated.
+	 */
+	case POWER_SUPPLY_USB_TYPE_UNKNOWN:
 	case POWER_SUPPLY_USB_TYPE_PD:
 		mode = V_SET_ACTIVE_MODE_DEDICATED;
 		break;
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/6] power: supply: rt9467-charger: Remove "usb_type" property write support
  2024-08-31 14:20 [PATCH 0/6] power: supply: Change usb_types from an array into a bitmask Hans de Goede
  2024-08-31 14:20 ` [PATCH 1/6] power: supply: "usb_type" property may be written to Hans de Goede
  2024-08-31 14:20 ` [PATCH 2/6] power: supply: ucs1002: Adjust ucs1002_set_usb_type() to accept string values Hans de Goede
@ 2024-08-31 14:20 ` Hans de Goede
  2024-08-31 14:20 ` [PATCH 4/6] power: supply: sysfs: Add power_supply_show_enum_with_available() helper Hans de Goede
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Hans de Goede @ 2024-08-31 14:20 UTC (permalink / raw)
  To: Sebastian Reichel, Thomas Weißschuh, MyungJoo Ham,
	Chanwoo Choi, Greg Kroah-Hartman, Heikki Krogerus, Vinod Koul,
	Kishon Vijay Abraham I
  Cc: Hans de Goede, Enric Balletbo Serra, Andrey Smirnov, linux-pm,
	linux-kernel, linux-usb, linux-phy

The "usb_type" property must be read-only for charger power-supply devices,
see: Documentation/ABI/testing/sysfs-class-power.

But the rt9467 driver allows writing 0/1 to it to disable/enable charging.
Other charger drivers use the "status" property for this and the rt9467
code also allows writing 0/1 to its "status" property and this does
the exact same thing as writing 0/1 to its "usb_type" property.

Drop write support for the "usb_type" property making it readonly to match
the ABI documentation. If userspace wants to disable/enable charging it
can use the "status" property for this.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/rt9467-charger.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/power/supply/rt9467-charger.c b/drivers/power/supply/rt9467-charger.c
index fdfdc83ab045..f935bd761ac1 100644
--- a/drivers/power/supply/rt9467-charger.c
+++ b/drivers/power/supply/rt9467-charger.c
@@ -745,8 +745,6 @@ static int rt9467_psy_set_property(struct power_supply *psy,
 						    RT9467_RANGE_IPREC, val->intval);
 	case POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT:
 		return rt9467_psy_set_ieoc(data, val->intval);
-	case POWER_SUPPLY_PROP_USB_TYPE:
-		return regmap_field_write(data->rm_field[F_USBCHGEN], val->intval);
 	default:
 		return -EINVAL;
 	}
@@ -764,7 +762,6 @@ static int rt9467_chg_prop_is_writeable(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT:
 	case POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT:
 	case POWER_SUPPLY_PROP_PRECHARGE_CURRENT:
-	case POWER_SUPPLY_PROP_USB_TYPE:
 		return 1;
 	default:
 		return 0;
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/6] power: supply: sysfs: Add power_supply_show_enum_with_available() helper
  2024-08-31 14:20 [PATCH 0/6] power: supply: Change usb_types from an array into a bitmask Hans de Goede
                   ` (2 preceding siblings ...)
  2024-08-31 14:20 ` [PATCH 3/6] power: supply: rt9467-charger: Remove "usb_type" property write support Hans de Goede
@ 2024-08-31 14:20 ` Hans de Goede
  2024-08-31 14:20 ` [PATCH 5/6] power: supply: sysfs: Move power_supply_show_enum_with_available() up Hans de Goede
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Hans de Goede @ 2024-08-31 14:20 UTC (permalink / raw)
  To: Sebastian Reichel, Thomas Weißschuh, MyungJoo Ham,
	Chanwoo Choi, Greg Kroah-Hartman, Heikki Krogerus, Vinod Koul,
	Kishon Vijay Abraham I
  Cc: Hans de Goede, Enric Balletbo Serra, Andrey Smirnov, linux-pm,
	linux-kernel, linux-usb, linux-phy

Turn power_supply_charge_behaviour_show() into a generic function for
showing enum values with their available (for writing) values shown
and the current value shown surrounded by sqaure-brackets like
the show() output for "usb_type" and "charge_behaviour".

This is a preparation patch for refactoring the "usb_type" property
handling to use a bitmask indicating available usb-types + this new
generic function.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/power_supply_sysfs.c | 32 ++++++++++++++---------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
index ff7e423edd57..9f21b0b54caf 100644
--- a/drivers/power/supply/power_supply_sysfs.c
+++ b/drivers/power/supply/power_supply_sysfs.c
@@ -518,31 +518,28 @@ int power_supply_uevent(const struct device *dev, struct kobj_uevent_env *env)
 	return ret;
 }
 
-ssize_t power_supply_charge_behaviour_show(struct device *dev,
-					   unsigned int available_behaviours,
-					   enum power_supply_charge_behaviour current_behaviour,
-					   char *buf)
+static ssize_t power_supply_show_enum_with_available(
+			struct device *dev, const char * const labels[], int label_count,
+			unsigned int available_values, int value, char *buf)
 {
 	bool match = false, available, active;
 	ssize_t count = 0;
 	int i;
 
-	for (i = 0; i < ARRAY_SIZE(POWER_SUPPLY_CHARGE_BEHAVIOUR_TEXT); i++) {
-		available = available_behaviours & BIT(i);
-		active = i == current_behaviour;
+	for (i = 0; i < label_count; i++) {
+		available = available_values & BIT(i);
+		active = i == value;
 
 		if (available && active) {
-			count += sysfs_emit_at(buf, count, "[%s] ",
-					       POWER_SUPPLY_CHARGE_BEHAVIOUR_TEXT[i]);
+			count += sysfs_emit_at(buf, count, "[%s] ", labels[i]);
 			match = true;
 		} else if (available) {
-			count += sysfs_emit_at(buf, count, "%s ",
-					       POWER_SUPPLY_CHARGE_BEHAVIOUR_TEXT[i]);
+			count += sysfs_emit_at(buf, count, "%s ", labels[i]);
 		}
 	}
 
 	if (!match) {
-		dev_warn(dev, "driver reporting unsupported charge behaviour\n");
+		dev_warn(dev, "driver reporting unavailable enum value %d\n", value);
 		return -EINVAL;
 	}
 
@@ -551,6 +548,17 @@ ssize_t power_supply_charge_behaviour_show(struct device *dev,
 
 	return count;
 }
+
+ssize_t power_supply_charge_behaviour_show(struct device *dev,
+					   unsigned int available_behaviours,
+					   enum power_supply_charge_behaviour current_behaviour,
+					   char *buf)
+{
+	return power_supply_show_enum_with_available(
+				dev, POWER_SUPPLY_CHARGE_BEHAVIOUR_TEXT,
+				ARRAY_SIZE(POWER_SUPPLY_CHARGE_BEHAVIOUR_TEXT),
+				available_behaviours, current_behaviour, buf);
+}
 EXPORT_SYMBOL_GPL(power_supply_charge_behaviour_show);
 
 int power_supply_charge_behaviour_parse(unsigned int available_behaviours, const char *buf)
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 5/6] power: supply: sysfs: Move power_supply_show_enum_with_available() up
  2024-08-31 14:20 [PATCH 0/6] power: supply: Change usb_types from an array into a bitmask Hans de Goede
                   ` (3 preceding siblings ...)
  2024-08-31 14:20 ` [PATCH 4/6] power: supply: sysfs: Add power_supply_show_enum_with_available() helper Hans de Goede
@ 2024-08-31 14:20 ` Hans de Goede
  2024-08-31 14:20 ` [PATCH 6/6] power: supply: Change usb_types from an array into a bitmask Hans de Goede
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Hans de Goede @ 2024-08-31 14:20 UTC (permalink / raw)
  To: Sebastian Reichel, Thomas Weißschuh, MyungJoo Ham,
	Chanwoo Choi, Greg Kroah-Hartman, Heikki Krogerus, Vinod Koul,
	Kishon Vijay Abraham I
  Cc: Hans de Goede, Enric Balletbo Serra, Andrey Smirnov, linux-pm,
	linux-kernel, linux-usb, linux-phy

Move power_supply_show_enum_with_available() higher up in
the power_supply_sysfs.c file.

This is a preparation patch to avoid needing a forward declaration
when replacing power_supply_show_usb_type() with it later on.

This commit only moves the function, there are no changes to it.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/power_supply_sysfs.c | 62 +++++++++++------------
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
index 9f21b0b54caf..c98a6de59d3b 100644
--- a/drivers/power/supply/power_supply_sysfs.c
+++ b/drivers/power/supply/power_supply_sysfs.c
@@ -237,6 +237,37 @@ static enum power_supply_property dev_attr_psp(struct device_attribute *attr)
 	return  to_ps_attr(attr) - power_supply_attrs;
 }
 
+static ssize_t power_supply_show_enum_with_available(
+			struct device *dev, const char * const labels[], int label_count,
+			unsigned int available_values, int value, char *buf)
+{
+	bool match = false, available, active;
+	ssize_t count = 0;
+	int i;
+
+	for (i = 0; i < label_count; i++) {
+		available = available_values & BIT(i);
+		active = i == value;
+
+		if (available && active) {
+			count += sysfs_emit_at(buf, count, "[%s] ", labels[i]);
+			match = true;
+		} else if (available) {
+			count += sysfs_emit_at(buf, count, "%s ", labels[i]);
+		}
+	}
+
+	if (!match) {
+		dev_warn(dev, "driver reporting unavailable enum value %d\n", value);
+		return -EINVAL;
+	}
+
+	if (count)
+		buf[count - 1] = '\n';
+
+	return count;
+}
+
 static ssize_t power_supply_show_usb_type(struct device *dev,
 					  const struct power_supply_desc *desc,
 					  union power_supply_propval *value,
@@ -518,37 +549,6 @@ int power_supply_uevent(const struct device *dev, struct kobj_uevent_env *env)
 	return ret;
 }
 
-static ssize_t power_supply_show_enum_with_available(
-			struct device *dev, const char * const labels[], int label_count,
-			unsigned int available_values, int value, char *buf)
-{
-	bool match = false, available, active;
-	ssize_t count = 0;
-	int i;
-
-	for (i = 0; i < label_count; i++) {
-		available = available_values & BIT(i);
-		active = i == value;
-
-		if (available && active) {
-			count += sysfs_emit_at(buf, count, "[%s] ", labels[i]);
-			match = true;
-		} else if (available) {
-			count += sysfs_emit_at(buf, count, "%s ", labels[i]);
-		}
-	}
-
-	if (!match) {
-		dev_warn(dev, "driver reporting unavailable enum value %d\n", value);
-		return -EINVAL;
-	}
-
-	if (count)
-		buf[count - 1] = '\n';
-
-	return count;
-}
-
 ssize_t power_supply_charge_behaviour_show(struct device *dev,
 					   unsigned int available_behaviours,
 					   enum power_supply_charge_behaviour current_behaviour,
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 6/6] power: supply: Change usb_types from an array into a bitmask
  2024-08-31 14:20 [PATCH 0/6] power: supply: Change usb_types from an array into a bitmask Hans de Goede
                   ` (4 preceding siblings ...)
  2024-08-31 14:20 ` [PATCH 5/6] power: supply: sysfs: Move power_supply_show_enum_with_available() up Hans de Goede
@ 2024-08-31 14:20 ` Hans de Goede
  2024-09-03  7:04   ` Greg Kroah-Hartman
  2024-09-03 21:57 ` [PATCH 0/6] " Sebastian Reichel
  2024-09-03 22:04 ` Sebastian Reichel
  7 siblings, 1 reply; 14+ messages in thread
From: Hans de Goede @ 2024-08-31 14:20 UTC (permalink / raw)
  To: Sebastian Reichel, Thomas Weißschuh, MyungJoo Ham,
	Chanwoo Choi, Greg Kroah-Hartman, Heikki Krogerus, Vinod Koul,
	Kishon Vijay Abraham I
  Cc: Hans de Goede, Enric Balletbo Serra, Andrey Smirnov, linux-pm,
	linux-kernel, linux-usb, linux-phy

The bit_types array just hold a list of valid enum power_supply_usb_type
values which map to 0 - 9. This can easily be represented as a bitmap.

This reduces the size of struct power_supply_desc and further reduces
the data section size by drivers no longer needing to store the array.

This also unifies how usb_types are handled with charge_behaviours,
which allows power_supply_show_usb_type() to be removed.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/extcon/extcon-intel-cht-wc.c          | 15 +++----
 drivers/phy/ti/phy-tusb1210.c                 | 11 ++---
 drivers/power/supply/axp20x_usb_power.c       | 13 ++----
 drivers/power/supply/bq256xx_charger.c        | 15 +++----
 drivers/power/supply/cros_usbpd-charger.c     | 22 ++++------
 .../power/supply/lenovo_yoga_c630_battery.c   |  7 +---
 drivers/power/supply/mp2629_charger.c         | 15 +++----
 drivers/power/supply/mt6360_charger.c         | 13 ++----
 drivers/power/supply/mt6370-charger.c         | 13 ++----
 drivers/power/supply/power_supply_core.c      |  4 --
 drivers/power/supply/power_supply_sysfs.c     | 40 ++-----------------
 drivers/power/supply/qcom_battmgr.c           | 37 +++++++++--------
 drivers/power/supply/qcom_pmi8998_charger.c   | 13 ++----
 drivers/power/supply/rk817_charger.c          |  9 +----
 drivers/power/supply/rn5t618_power.c          | 13 ++----
 drivers/power/supply/rt9467-charger.c         | 13 ++----
 drivers/power/supply/rt9471.c                 | 15 +++----
 drivers/power/supply/ucs1002_power.c          | 15 +++----
 drivers/usb/typec/anx7411.c                   | 11 ++---
 drivers/usb/typec/rt1719.c                    | 11 ++---
 drivers/usb/typec/tcpm/tcpm.c                 | 11 ++---
 drivers/usb/typec/tipd/core.c                 |  9 +----
 drivers/usb/typec/ucsi/psy.c                  | 11 ++---
 include/linux/power_supply.h                  |  3 +-
 24 files changed, 102 insertions(+), 237 deletions(-)

diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c
index 733c470c3102..93552dc3c895 100644
--- a/drivers/extcon/extcon-intel-cht-wc.c
+++ b/drivers/extcon/extcon-intel-cht-wc.c
@@ -461,14 +461,6 @@ static int cht_wc_extcon_psy_get_prop(struct power_supply *psy,
 	return 0;
 }
 
-static const enum power_supply_usb_type cht_wc_extcon_psy_usb_types[] = {
-	POWER_SUPPLY_USB_TYPE_SDP,
-	POWER_SUPPLY_USB_TYPE_CDP,
-	POWER_SUPPLY_USB_TYPE_DCP,
-	POWER_SUPPLY_USB_TYPE_ACA,
-	POWER_SUPPLY_USB_TYPE_UNKNOWN,
-};
-
 static const enum power_supply_property cht_wc_extcon_psy_props[] = {
 	POWER_SUPPLY_PROP_USB_TYPE,
 	POWER_SUPPLY_PROP_ONLINE,
@@ -477,8 +469,11 @@ static const enum power_supply_property cht_wc_extcon_psy_props[] = {
 static const struct power_supply_desc cht_wc_extcon_psy_desc = {
 	.name = "cht_wcove_pwrsrc",
 	.type = POWER_SUPPLY_TYPE_USB,
-	.usb_types = cht_wc_extcon_psy_usb_types,
-	.num_usb_types = ARRAY_SIZE(cht_wc_extcon_psy_usb_types),
+	.usb_types = BIT(POWER_SUPPLY_USB_TYPE_SDP) |
+		     BIT(POWER_SUPPLY_USB_TYPE_CDP) |
+		     BIT(POWER_SUPPLY_USB_TYPE_DCP) |
+		     BIT(POWER_SUPPLY_USB_TYPE_ACA) |
+		     BIT(POWER_SUPPLY_USB_TYPE_UNKNOWN),
 	.properties = cht_wc_extcon_psy_props,
 	.num_properties = ARRAY_SIZE(cht_wc_extcon_psy_props),
 	.get_property = cht_wc_extcon_psy_get_prop,
diff --git a/drivers/phy/ti/phy-tusb1210.c b/drivers/phy/ti/phy-tusb1210.c
index 751fecd466e3..c3ae9d7948d7 100644
--- a/drivers/phy/ti/phy-tusb1210.c
+++ b/drivers/phy/ti/phy-tusb1210.c
@@ -411,12 +411,6 @@ static int tusb1210_psy_get_prop(struct power_supply *psy,
 	return 0;
 }
 
-static const enum power_supply_usb_type tusb1210_psy_usb_types[] = {
-	POWER_SUPPLY_USB_TYPE_SDP,
-	POWER_SUPPLY_USB_TYPE_DCP,
-	POWER_SUPPLY_USB_TYPE_UNKNOWN,
-};
-
 static const enum power_supply_property tusb1210_psy_props[] = {
 	POWER_SUPPLY_PROP_ONLINE,
 	POWER_SUPPLY_PROP_USB_TYPE,
@@ -426,8 +420,9 @@ static const enum power_supply_property tusb1210_psy_props[] = {
 static const struct power_supply_desc tusb1210_psy_desc = {
 	.name = "tusb1211-charger-detect",
 	.type = POWER_SUPPLY_TYPE_USB,
-	.usb_types = tusb1210_psy_usb_types,
-	.num_usb_types = ARRAY_SIZE(tusb1210_psy_usb_types),
+	.usb_types = BIT(POWER_SUPPLY_USB_TYPE_SDP) |
+		     BIT(POWER_SUPPLY_USB_TYPE_DCP) |
+		     BIT(POWER_SUPPLY_USB_TYPE_UNKNOWN),
 	.properties = tusb1210_psy_props,
 	.num_properties = ARRAY_SIZE(tusb1210_psy_props),
 	.get_property = tusb1210_psy_get_prop,
diff --git a/drivers/power/supply/axp20x_usb_power.c b/drivers/power/supply/axp20x_usb_power.c
index 69fbb5861934..7b1bf6766ff7 100644
--- a/drivers/power/supply/axp20x_usb_power.c
+++ b/drivers/power/supply/axp20x_usb_power.c
@@ -456,13 +456,6 @@ static enum power_supply_property axp813_usb_power_properties[] = {
 	POWER_SUPPLY_PROP_USB_TYPE,
 };
 
-static enum power_supply_usb_type axp813_usb_types[] = {
-	POWER_SUPPLY_USB_TYPE_SDP,
-	POWER_SUPPLY_USB_TYPE_DCP,
-	POWER_SUPPLY_USB_TYPE_CDP,
-	POWER_SUPPLY_USB_TYPE_UNKNOWN,
-};
-
 static const struct power_supply_desc axp20x_usb_power_desc = {
 	.name = "axp20x-usb",
 	.type = POWER_SUPPLY_TYPE_USB,
@@ -491,8 +484,10 @@ static const struct power_supply_desc axp813_usb_power_desc = {
 	.property_is_writeable = axp20x_usb_power_prop_writeable,
 	.get_property = axp20x_usb_power_get_property,
 	.set_property = axp20x_usb_power_set_property,
-	.usb_types = axp813_usb_types,
-	.num_usb_types = ARRAY_SIZE(axp813_usb_types),
+	.usb_types = BIT(POWER_SUPPLY_USB_TYPE_SDP) |
+		     BIT(POWER_SUPPLY_USB_TYPE_CDP) |
+		     BIT(POWER_SUPPLY_USB_TYPE_DCP) |
+		     BIT(POWER_SUPPLY_USB_TYPE_UNKNOWN),
 };
 
 static const char * const axp20x_irq_names[] = {
diff --git a/drivers/power/supply/bq256xx_charger.c b/drivers/power/supply/bq256xx_charger.c
index 1a935bc88510..5514d1896bb8 100644
--- a/drivers/power/supply/bq256xx_charger.c
+++ b/drivers/power/supply/bq256xx_charger.c
@@ -334,14 +334,6 @@ static const int bq25618_619_ichg_values[] = {
 	1290000, 1360000, 1430000, 1500000
 };
 
-static enum power_supply_usb_type bq256xx_usb_type[] = {
-	POWER_SUPPLY_USB_TYPE_SDP,
-	POWER_SUPPLY_USB_TYPE_CDP,
-	POWER_SUPPLY_USB_TYPE_DCP,
-	POWER_SUPPLY_USB_TYPE_UNKNOWN,
-	POWER_SUPPLY_USB_TYPE_ACA,
-};
-
 static int bq256xx_array_parse(int array_size, int val, const int array[])
 {
 	int i = 0;
@@ -1252,8 +1244,11 @@ static int bq256xx_property_is_writeable(struct power_supply *psy,
 static const struct power_supply_desc bq256xx_power_supply_desc = {
 	.name = "bq256xx-charger",
 	.type = POWER_SUPPLY_TYPE_USB,
-	.usb_types = bq256xx_usb_type,
-	.num_usb_types = ARRAY_SIZE(bq256xx_usb_type),
+	.usb_types = BIT(POWER_SUPPLY_USB_TYPE_SDP) |
+		     BIT(POWER_SUPPLY_USB_TYPE_CDP) |
+		     BIT(POWER_SUPPLY_USB_TYPE_DCP) |
+		     BIT(POWER_SUPPLY_USB_TYPE_ACA) |
+		     BIT(POWER_SUPPLY_USB_TYPE_UNKNOWN),
 	.properties = bq256xx_power_supply_props,
 	.num_properties = ARRAY_SIZE(bq256xx_power_supply_props),
 	.get_property = bq256xx_get_charger_property,
diff --git a/drivers/power/supply/cros_usbpd-charger.c b/drivers/power/supply/cros_usbpd-charger.c
index 8008e31c0c09..bed3e2e9bfea 100644
--- a/drivers/power/supply/cros_usbpd-charger.c
+++ b/drivers/power/supply/cros_usbpd-charger.c
@@ -73,17 +73,6 @@ static enum power_supply_property cros_usbpd_dedicated_charger_props[] = {
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
 };
 
-static enum power_supply_usb_type cros_usbpd_charger_usb_types[] = {
-	POWER_SUPPLY_USB_TYPE_UNKNOWN,
-	POWER_SUPPLY_USB_TYPE_SDP,
-	POWER_SUPPLY_USB_TYPE_DCP,
-	POWER_SUPPLY_USB_TYPE_CDP,
-	POWER_SUPPLY_USB_TYPE_C,
-	POWER_SUPPLY_USB_TYPE_PD,
-	POWER_SUPPLY_USB_TYPE_PD_DRP,
-	POWER_SUPPLY_USB_TYPE_APPLE_BRICK_ID
-};
-
 /* Input voltage/current limit in mV/mA. Default to none. */
 static u16 input_voltage_limit = EC_POWER_LIMIT_NONE;
 static u16 input_current_limit = EC_POWER_LIMIT_NONE;
@@ -643,9 +632,14 @@ static int cros_usbpd_charger_probe(struct platform_device *pd)
 			psy_desc->properties = cros_usbpd_charger_props;
 			psy_desc->num_properties =
 				ARRAY_SIZE(cros_usbpd_charger_props);
-			psy_desc->usb_types = cros_usbpd_charger_usb_types;
-			psy_desc->num_usb_types =
-				ARRAY_SIZE(cros_usbpd_charger_usb_types);
+			psy_desc->usb_types = BIT(POWER_SUPPLY_USB_TYPE_UNKNOWN) |
+					      BIT(POWER_SUPPLY_USB_TYPE_SDP)     |
+					      BIT(POWER_SUPPLY_USB_TYPE_DCP)     |
+					      BIT(POWER_SUPPLY_USB_TYPE_CDP)     |
+					      BIT(POWER_SUPPLY_USB_TYPE_C)       |
+					      BIT(POWER_SUPPLY_USB_TYPE_PD)      |
+					      BIT(POWER_SUPPLY_USB_TYPE_PD_DRP)  |
+					      BIT(POWER_SUPPLY_USB_TYPE_APPLE_BRICK_ID);
 		}
 
 		psy_desc->name = port->name;
diff --git a/drivers/power/supply/lenovo_yoga_c630_battery.c b/drivers/power/supply/lenovo_yoga_c630_battery.c
index d4d422cc5353..f98f65e00831 100644
--- a/drivers/power/supply/lenovo_yoga_c630_battery.c
+++ b/drivers/power/supply/lenovo_yoga_c630_battery.c
@@ -353,15 +353,10 @@ static enum power_supply_property yoga_c630_psy_adpt_properties[] = {
 	POWER_SUPPLY_PROP_USB_TYPE,
 };
 
-static const enum power_supply_usb_type yoga_c630_psy_adpt_usb_type[] = {
-	POWER_SUPPLY_USB_TYPE_C,
-};
-
 static const struct power_supply_desc yoga_c630_psy_adpt_psy_desc = {
 	.name = "yoga-c630-adapter",
 	.type = POWER_SUPPLY_TYPE_USB,
-	.usb_types = yoga_c630_psy_adpt_usb_type,
-	.num_usb_types = ARRAY_SIZE(yoga_c630_psy_adpt_usb_type),
+	.usb_types = BIT(POWER_SUPPLY_USB_TYPE_C),
 	.properties = yoga_c630_psy_adpt_properties,
 	.num_properties = ARRAY_SIZE(yoga_c630_psy_adpt_properties),
 	.get_property = yoga_c630_psy_adpt_get_property,
diff --git a/drivers/power/supply/mp2629_charger.c b/drivers/power/supply/mp2629_charger.c
index 3a2a28fbba73..d281c1059629 100644
--- a/drivers/power/supply/mp2629_charger.c
+++ b/drivers/power/supply/mp2629_charger.c
@@ -94,14 +94,6 @@ struct mp2629_prop {
 	int shift;
 };
 
-static enum power_supply_usb_type mp2629_usb_types[] = {
-	POWER_SUPPLY_USB_TYPE_SDP,
-	POWER_SUPPLY_USB_TYPE_DCP,
-	POWER_SUPPLY_USB_TYPE_CDP,
-	POWER_SUPPLY_USB_TYPE_PD_DRP,
-	POWER_SUPPLY_USB_TYPE_UNKNOWN
-};
-
 static enum power_supply_property mp2629_charger_usb_props[] = {
 	POWER_SUPPLY_PROP_ONLINE,
 	POWER_SUPPLY_PROP_USB_TYPE,
@@ -487,8 +479,11 @@ static irqreturn_t mp2629_irq_handler(int irq, void *dev_id)
 static const struct power_supply_desc mp2629_usb_desc = {
 	.name		= "mp2629_usb",
 	.type		= POWER_SUPPLY_TYPE_USB,
-	.usb_types      = mp2629_usb_types,
-	.num_usb_types  = ARRAY_SIZE(mp2629_usb_types),
+	.usb_types	= BIT(POWER_SUPPLY_USB_TYPE_SDP) |
+			  BIT(POWER_SUPPLY_USB_TYPE_CDP) |
+			  BIT(POWER_SUPPLY_USB_TYPE_DCP) |
+			  BIT(POWER_SUPPLY_USB_TYPE_PD_DRP) |
+			  BIT(POWER_SUPPLY_USB_TYPE_UNKNOWN),
 	.properties	= mp2629_charger_usb_props,
 	.num_properties	= ARRAY_SIZE(mp2629_charger_usb_props),
 	.get_property	= mp2629_charger_usb_get_prop,
diff --git a/drivers/power/supply/mt6360_charger.c b/drivers/power/supply/mt6360_charger.c
index aca123783efc..e99e55148976 100644
--- a/drivers/power/supply/mt6360_charger.c
+++ b/drivers/power/supply/mt6360_charger.c
@@ -154,13 +154,6 @@ enum mt6360_pmu_chg_type {
 	MT6360_CHG_TYPE_MAX,
 };
 
-static enum power_supply_usb_type mt6360_charger_usb_types[] = {
-	POWER_SUPPLY_USB_TYPE_UNKNOWN,
-	POWER_SUPPLY_USB_TYPE_SDP,
-	POWER_SUPPLY_USB_TYPE_DCP,
-	POWER_SUPPLY_USB_TYPE_CDP,
-};
-
 static int mt6360_get_chrdet_ext_stat(struct mt6360_chg_info *mci,
 					     bool *pwr_rdy)
 {
@@ -574,8 +567,10 @@ static const struct power_supply_desc mt6360_charger_desc = {
 	.get_property		= mt6360_charger_get_property,
 	.set_property		= mt6360_charger_set_property,
 	.property_is_writeable	= mt6360_charger_property_is_writeable,
-	.usb_types		= mt6360_charger_usb_types,
-	.num_usb_types		= ARRAY_SIZE(mt6360_charger_usb_types),
+	.usb_types		= BIT(POWER_SUPPLY_USB_TYPE_SDP) |
+				  BIT(POWER_SUPPLY_USB_TYPE_CDP) |
+				  BIT(POWER_SUPPLY_USB_TYPE_DCP) |
+				  BIT(POWER_SUPPLY_USB_TYPE_UNKNOWN),
 };
 
 static const struct regulator_ops mt6360_chg_otg_ops = {
diff --git a/drivers/power/supply/mt6370-charger.c b/drivers/power/supply/mt6370-charger.c
index e24fce087d80..ad8793bf997e 100644
--- a/drivers/power/supply/mt6370-charger.c
+++ b/drivers/power/supply/mt6370-charger.c
@@ -624,13 +624,6 @@ static enum power_supply_property mt6370_chg_properties[] = {
 	POWER_SUPPLY_PROP_USB_TYPE,
 };
 
-static enum power_supply_usb_type mt6370_chg_usb_types[] = {
-	POWER_SUPPLY_USB_TYPE_UNKNOWN,
-	POWER_SUPPLY_USB_TYPE_SDP,
-	POWER_SUPPLY_USB_TYPE_CDP,
-	POWER_SUPPLY_USB_TYPE_DCP,
-};
-
 static const struct power_supply_desc mt6370_chg_psy_desc = {
 	.name = "mt6370-charger",
 	.type = POWER_SUPPLY_TYPE_USB,
@@ -639,8 +632,10 @@ static const struct power_supply_desc mt6370_chg_psy_desc = {
 	.get_property = mt6370_chg_get_property,
 	.set_property = mt6370_chg_set_property,
 	.property_is_writeable = mt6370_chg_property_is_writeable,
-	.usb_types = mt6370_chg_usb_types,
-	.num_usb_types = ARRAY_SIZE(mt6370_chg_usb_types),
+	.usb_types = BIT(POWER_SUPPLY_USB_TYPE_SDP) |
+		     BIT(POWER_SUPPLY_USB_TYPE_CDP) |
+		     BIT(POWER_SUPPLY_USB_TYPE_DCP) |
+		     BIT(POWER_SUPPLY_USB_TYPE_UNKNOWN),
 };
 
 static const struct regulator_ops mt6370_chg_otg_ops = {
diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 3614d263ddad..0417fb34e846 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -1360,10 +1360,6 @@ __power_supply_register(struct device *parent,
 		pr_warn("%s: Expected proper parent device for '%s'\n",
 			__func__, desc->name);
 
-	if (psy_has_property(desc, POWER_SUPPLY_PROP_USB_TYPE) &&
-	    (!desc->usb_types || !desc->num_usb_types))
-		return ERR_PTR(-EINVAL);
-
 	psy = kzalloc(sizeof(*psy), GFP_KERNEL);
 	if (!psy)
 		return ERR_PTR(-ENOMEM);
diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
index c98a6de59d3b..16b3c5880cd8 100644
--- a/drivers/power/supply/power_supply_sysfs.c
+++ b/drivers/power/supply/power_supply_sysfs.c
@@ -268,40 +268,6 @@ static ssize_t power_supply_show_enum_with_available(
 	return count;
 }
 
-static ssize_t power_supply_show_usb_type(struct device *dev,
-					  const struct power_supply_desc *desc,
-					  union power_supply_propval *value,
-					  char *buf)
-{
-	enum power_supply_usb_type usb_type;
-	ssize_t count = 0;
-	bool match = false;
-	int i;
-
-	for (i = 0; i < desc->num_usb_types; ++i) {
-		usb_type = desc->usb_types[i];
-
-		if (value->intval == usb_type) {
-			count += sysfs_emit_at(buf, count, "[%s] ",
-					 POWER_SUPPLY_USB_TYPE_TEXT[usb_type]);
-			match = true;
-		} else {
-			count += sysfs_emit_at(buf, count, "%s ",
-					 POWER_SUPPLY_USB_TYPE_TEXT[usb_type]);
-		}
-	}
-
-	if (!match) {
-		dev_warn(dev, "driver reporting unsupported connected type\n");
-		return -EINVAL;
-	}
-
-	if (count)
-		buf[count - 1] = '\n';
-
-	return count;
-}
-
 static ssize_t power_supply_show_property(struct device *dev,
 					  struct device_attribute *attr,
 					  char *buf) {
@@ -331,8 +297,10 @@ static ssize_t power_supply_show_property(struct device *dev,
 
 	switch (psp) {
 	case POWER_SUPPLY_PROP_USB_TYPE:
-		ret = power_supply_show_usb_type(dev, psy->desc,
-						&value, buf);
+		ret = power_supply_show_enum_with_available(
+				dev, POWER_SUPPLY_USB_TYPE_TEXT,
+				ARRAY_SIZE(POWER_SUPPLY_USB_TYPE_TEXT),
+				psy->desc->usb_types, value.intval, buf);
 		break;
 	case POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR:
 		ret = power_supply_charge_behaviour_show(dev, psy->desc->charge_behaviours,
diff --git a/drivers/power/supply/qcom_battmgr.c b/drivers/power/supply/qcom_battmgr.c
index 46f36dcb185c..56c8021383da 100644
--- a/drivers/power/supply/qcom_battmgr.c
+++ b/drivers/power/supply/qcom_battmgr.c
@@ -786,19 +786,6 @@ static int qcom_battmgr_usb_get_property(struct power_supply *psy,
 	return 0;
 }
 
-static const enum power_supply_usb_type usb_psy_supported_types[] = {
-	POWER_SUPPLY_USB_TYPE_UNKNOWN,
-	POWER_SUPPLY_USB_TYPE_SDP,
-	POWER_SUPPLY_USB_TYPE_DCP,
-	POWER_SUPPLY_USB_TYPE_CDP,
-	POWER_SUPPLY_USB_TYPE_ACA,
-	POWER_SUPPLY_USB_TYPE_C,
-	POWER_SUPPLY_USB_TYPE_PD,
-	POWER_SUPPLY_USB_TYPE_PD_DRP,
-	POWER_SUPPLY_USB_TYPE_PD_PPS,
-	POWER_SUPPLY_USB_TYPE_APPLE_BRICK_ID,
-};
-
 static const enum power_supply_property sc8280xp_usb_props[] = {
 	POWER_SUPPLY_PROP_ONLINE,
 };
@@ -809,8 +796,16 @@ static const struct power_supply_desc sc8280xp_usb_psy_desc = {
 	.properties = sc8280xp_usb_props,
 	.num_properties = ARRAY_SIZE(sc8280xp_usb_props),
 	.get_property = qcom_battmgr_usb_get_property,
-	.usb_types = usb_psy_supported_types,
-	.num_usb_types = ARRAY_SIZE(usb_psy_supported_types),
+	.usb_types = BIT(POWER_SUPPLY_USB_TYPE_UNKNOWN) |
+		     BIT(POWER_SUPPLY_USB_TYPE_SDP)     |
+		     BIT(POWER_SUPPLY_USB_TYPE_DCP)     |
+		     BIT(POWER_SUPPLY_USB_TYPE_CDP)     |
+		     BIT(POWER_SUPPLY_USB_TYPE_ACA)     |
+		     BIT(POWER_SUPPLY_USB_TYPE_C)       |
+		     BIT(POWER_SUPPLY_USB_TYPE_PD)      |
+		     BIT(POWER_SUPPLY_USB_TYPE_PD_DRP)  |
+		     BIT(POWER_SUPPLY_USB_TYPE_PD_PPS)  |
+		     BIT(POWER_SUPPLY_USB_TYPE_APPLE_BRICK_ID),
 };
 
 static const enum power_supply_property sm8350_usb_props[] = {
@@ -829,8 +824,16 @@ static const struct power_supply_desc sm8350_usb_psy_desc = {
 	.properties = sm8350_usb_props,
 	.num_properties = ARRAY_SIZE(sm8350_usb_props),
 	.get_property = qcom_battmgr_usb_get_property,
-	.usb_types = usb_psy_supported_types,
-	.num_usb_types = ARRAY_SIZE(usb_psy_supported_types),
+	.usb_types = BIT(POWER_SUPPLY_USB_TYPE_UNKNOWN) |
+		     BIT(POWER_SUPPLY_USB_TYPE_SDP)     |
+		     BIT(POWER_SUPPLY_USB_TYPE_DCP)     |
+		     BIT(POWER_SUPPLY_USB_TYPE_CDP)     |
+		     BIT(POWER_SUPPLY_USB_TYPE_ACA)     |
+		     BIT(POWER_SUPPLY_USB_TYPE_C)       |
+		     BIT(POWER_SUPPLY_USB_TYPE_PD)      |
+		     BIT(POWER_SUPPLY_USB_TYPE_PD_DRP)  |
+		     BIT(POWER_SUPPLY_USB_TYPE_PD_PPS)  |
+		     BIT(POWER_SUPPLY_USB_TYPE_APPLE_BRICK_ID),
 };
 
 static const u8 sm8350_wls_prop_map[] = {
diff --git a/drivers/power/supply/qcom_pmi8998_charger.c b/drivers/power/supply/qcom_pmi8998_charger.c
index 9bb777406013..81acbd8b2169 100644
--- a/drivers/power/supply/qcom_pmi8998_charger.c
+++ b/drivers/power/supply/qcom_pmi8998_charger.c
@@ -411,13 +411,6 @@ static enum power_supply_property smb2_properties[] = {
 	POWER_SUPPLY_PROP_USB_TYPE,
 };
 
-static enum power_supply_usb_type smb2_usb_types[] = {
-	POWER_SUPPLY_USB_TYPE_UNKNOWN,
-	POWER_SUPPLY_USB_TYPE_SDP,
-	POWER_SUPPLY_USB_TYPE_DCP,
-	POWER_SUPPLY_USB_TYPE_CDP,
-};
-
 static int smb2_get_prop_usb_online(struct smb2_chip *chip, int *val)
 {
 	unsigned int stat;
@@ -775,8 +768,10 @@ static irqreturn_t smb2_handle_wdog_bark(int irq, void *data)
 static const struct power_supply_desc smb2_psy_desc = {
 	.name = "pmi8998_charger",
 	.type = POWER_SUPPLY_TYPE_USB,
-	.usb_types = smb2_usb_types,
-	.num_usb_types = ARRAY_SIZE(smb2_usb_types),
+	.usb_types = BIT(POWER_SUPPLY_USB_TYPE_SDP) |
+		     BIT(POWER_SUPPLY_USB_TYPE_CDP) |
+		     BIT(POWER_SUPPLY_USB_TYPE_DCP) |
+		     BIT(POWER_SUPPLY_USB_TYPE_UNKNOWN),
 	.properties = smb2_properties,
 	.num_properties = ARRAY_SIZE(smb2_properties),
 	.get_property = smb2_get_property,
diff --git a/drivers/power/supply/rk817_charger.c b/drivers/power/supply/rk817_charger.c
index 7ca91739c6cc..a3d377a32b49 100644
--- a/drivers/power/supply/rk817_charger.c
+++ b/drivers/power/supply/rk817_charger.c
@@ -673,11 +673,6 @@ static enum power_supply_property rk817_chg_props[] = {
 	POWER_SUPPLY_PROP_VOLTAGE_AVG,
 };
 
-static enum power_supply_usb_type rk817_usb_type[] = {
-	POWER_SUPPLY_USB_TYPE_DCP,
-	POWER_SUPPLY_USB_TYPE_UNKNOWN,
-};
-
 static const struct power_supply_desc rk817_bat_desc = {
 	.name = "rk817-battery",
 	.type = POWER_SUPPLY_TYPE_BATTERY,
@@ -689,8 +684,8 @@ static const struct power_supply_desc rk817_bat_desc = {
 static const struct power_supply_desc rk817_chg_desc = {
 	.name = "rk817-charger",
 	.type = POWER_SUPPLY_TYPE_USB,
-	.usb_types = rk817_usb_type,
-	.num_usb_types = ARRAY_SIZE(rk817_usb_type),
+	.usb_types = BIT(POWER_SUPPLY_USB_TYPE_DCP) |
+		     BIT(POWER_SUPPLY_USB_TYPE_UNKNOWN),
 	.properties = rk817_chg_props,
 	.num_properties = ARRAY_SIZE(rk817_chg_props),
 	.get_property = rk817_chg_get_prop,
diff --git a/drivers/power/supply/rn5t618_power.c b/drivers/power/supply/rn5t618_power.c
index ebea3522a2ac..40dec55a9f73 100644
--- a/drivers/power/supply/rn5t618_power.c
+++ b/drivers/power/supply/rn5t618_power.c
@@ -70,13 +70,6 @@ struct rn5t618_power_info {
 	int irq;
 };
 
-static enum power_supply_usb_type rn5t618_usb_types[] = {
-	POWER_SUPPLY_USB_TYPE_SDP,
-	POWER_SUPPLY_USB_TYPE_DCP,
-	POWER_SUPPLY_USB_TYPE_CDP,
-	POWER_SUPPLY_USB_TYPE_UNKNOWN
-};
-
 static enum power_supply_property rn5t618_usb_props[] = {
 	/* input current limit is not very accurate */
 	POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
@@ -681,8 +674,10 @@ static const struct power_supply_desc rn5t618_adp_desc = {
 static const struct power_supply_desc rn5t618_usb_desc = {
 	.name                   = "rn5t618-usb",
 	.type                   = POWER_SUPPLY_TYPE_USB,
-	.usb_types		= rn5t618_usb_types,
-	.num_usb_types		= ARRAY_SIZE(rn5t618_usb_types),
+	.usb_types		= BIT(POWER_SUPPLY_USB_TYPE_SDP) |
+				  BIT(POWER_SUPPLY_USB_TYPE_CDP) |
+				  BIT(POWER_SUPPLY_USB_TYPE_DCP) |
+				  BIT(POWER_SUPPLY_USB_TYPE_UNKNOWN),
 	.properties             = rn5t618_usb_props,
 	.num_properties         = ARRAY_SIZE(rn5t618_usb_props),
 	.get_property           = rn5t618_usb_get_property,
diff --git a/drivers/power/supply/rt9467-charger.c b/drivers/power/supply/rt9467-charger.c
index f935bd761ac1..235169c85c5d 100644
--- a/drivers/power/supply/rt9467-charger.c
+++ b/drivers/power/supply/rt9467-charger.c
@@ -630,13 +630,6 @@ static int rt9467_psy_set_ieoc(struct rt9467_chg_data *data, int microamp)
 	return ret;
 }
 
-static const enum power_supply_usb_type rt9467_chg_usb_types[] = {
-	POWER_SUPPLY_USB_TYPE_UNKNOWN,
-	POWER_SUPPLY_USB_TYPE_SDP,
-	POWER_SUPPLY_USB_TYPE_DCP,
-	POWER_SUPPLY_USB_TYPE_CDP,
-};
-
 static const enum power_supply_property rt9467_chg_properties[] = {
 	POWER_SUPPLY_PROP_STATUS,
 	POWER_SUPPLY_PROP_ONLINE,
@@ -771,8 +764,10 @@ static int rt9467_chg_prop_is_writeable(struct power_supply *psy,
 static const struct power_supply_desc rt9467_chg_psy_desc = {
 	.name = "rt9467-charger",
 	.type = POWER_SUPPLY_TYPE_USB,
-	.usb_types = rt9467_chg_usb_types,
-	.num_usb_types = ARRAY_SIZE(rt9467_chg_usb_types),
+	.usb_types = BIT(POWER_SUPPLY_USB_TYPE_SDP) |
+		     BIT(POWER_SUPPLY_USB_TYPE_CDP) |
+		     BIT(POWER_SUPPLY_USB_TYPE_DCP) |
+		     BIT(POWER_SUPPLY_USB_TYPE_UNKNOWN),
 	.properties = rt9467_chg_properties,
 	.num_properties = ARRAY_SIZE(rt9467_chg_properties),
 	.property_is_writeable = rt9467_chg_prop_is_writeable,
diff --git a/drivers/power/supply/rt9471.c b/drivers/power/supply/rt9471.c
index 868b0703d15c..c04af1ee89c6 100644
--- a/drivers/power/supply/rt9471.c
+++ b/drivers/power/supply/rt9471.c
@@ -333,14 +333,6 @@ static enum power_supply_property rt9471_charger_properties[] = {
 	POWER_SUPPLY_PROP_MANUFACTURER,
 };
 
-static enum power_supply_usb_type rt9471_charger_usb_types[] = {
-	POWER_SUPPLY_USB_TYPE_UNKNOWN,
-	POWER_SUPPLY_USB_TYPE_SDP,
-	POWER_SUPPLY_USB_TYPE_DCP,
-	POWER_SUPPLY_USB_TYPE_CDP,
-	POWER_SUPPLY_USB_TYPE_APPLE_BRICK_ID,
-};
-
 static int rt9471_charger_property_is_writeable(struct power_supply *psy,
 						enum power_supply_property psp)
 {
@@ -726,8 +718,11 @@ static int rt9471_register_psy(struct rt9471_chip *chip)
 
 	desc->name = psy_name;
 	desc->type = POWER_SUPPLY_TYPE_USB;
-	desc->usb_types = rt9471_charger_usb_types;
-	desc->num_usb_types = ARRAY_SIZE(rt9471_charger_usb_types);
+	desc->usb_types = BIT(POWER_SUPPLY_USB_TYPE_SDP) |
+			  BIT(POWER_SUPPLY_USB_TYPE_CDP) |
+			  BIT(POWER_SUPPLY_USB_TYPE_DCP) |
+			  BIT(POWER_SUPPLY_USB_TYPE_APPLE_BRICK_ID) |
+			  BIT(POWER_SUPPLY_USB_TYPE_UNKNOWN);
 	desc->properties = rt9471_charger_properties;
 	desc->num_properties = ARRAY_SIZE(rt9471_charger_properties);
 	desc->get_property = rt9471_charger_get_property;
diff --git a/drivers/power/supply/ucs1002_power.c b/drivers/power/supply/ucs1002_power.c
index b67d5b03d93e..7382bec6a43c 100644
--- a/drivers/power/supply/ucs1002_power.c
+++ b/drivers/power/supply/ucs1002_power.c
@@ -296,14 +296,6 @@ static int ucs1002_set_max_current(struct ucs1002_info *info, u32 val)
 	return 0;
 }
 
-static enum power_supply_usb_type ucs1002_usb_types[] = {
-	POWER_SUPPLY_USB_TYPE_PD,
-	POWER_SUPPLY_USB_TYPE_SDP,
-	POWER_SUPPLY_USB_TYPE_DCP,
-	POWER_SUPPLY_USB_TYPE_CDP,
-	POWER_SUPPLY_USB_TYPE_UNKNOWN,
-};
-
 static int ucs1002_set_usb_type(struct ucs1002_info *info, int val)
 {
 	unsigned int mode;
@@ -431,8 +423,11 @@ static int ucs1002_property_is_writeable(struct power_supply *psy,
 static const struct power_supply_desc ucs1002_charger_desc = {
 	.name			= "ucs1002",
 	.type			= POWER_SUPPLY_TYPE_USB,
-	.usb_types		= ucs1002_usb_types,
-	.num_usb_types		= ARRAY_SIZE(ucs1002_usb_types),
+	.usb_types		= BIT(POWER_SUPPLY_USB_TYPE_SDP) |
+				  BIT(POWER_SUPPLY_USB_TYPE_CDP) |
+				  BIT(POWER_SUPPLY_USB_TYPE_DCP) |
+				  BIT(POWER_SUPPLY_USB_TYPE_PD)  |
+				  BIT(POWER_SUPPLY_USB_TYPE_UNKNOWN),
 	.get_property		= ucs1002_get_property,
 	.set_property		= ucs1002_set_property,
 	.property_is_writeable	= ucs1002_property_is_writeable,
diff --git a/drivers/usb/typec/anx7411.c b/drivers/usb/typec/anx7411.c
index 5a5bf3532ad7..31e3e9544bc0 100644
--- a/drivers/usb/typec/anx7411.c
+++ b/drivers/usb/typec/anx7411.c
@@ -1339,12 +1339,6 @@ static void anx7411_get_gpio_irq(struct anx7411_data *ctx)
 		dev_err(dev, "failed to get GPIO IRQ\n");
 }
 
-static enum power_supply_usb_type anx7411_psy_usb_types[] = {
-	POWER_SUPPLY_USB_TYPE_C,
-	POWER_SUPPLY_USB_TYPE_PD,
-	POWER_SUPPLY_USB_TYPE_PD_PPS,
-};
-
 static enum power_supply_property anx7411_psy_props[] = {
 	POWER_SUPPLY_PROP_USB_TYPE,
 	POWER_SUPPLY_PROP_ONLINE,
@@ -1422,8 +1416,9 @@ static int anx7411_psy_register(struct anx7411_data *ctx)
 
 	psy_desc->name = psy_name;
 	psy_desc->type = POWER_SUPPLY_TYPE_USB;
-	psy_desc->usb_types = anx7411_psy_usb_types;
-	psy_desc->num_usb_types = ARRAY_SIZE(anx7411_psy_usb_types);
+	psy_desc->usb_types = BIT(POWER_SUPPLY_USB_TYPE_C)  |
+			      BIT(POWER_SUPPLY_USB_TYPE_PD) |
+			      BIT(POWER_SUPPLY_USB_TYPE_PD_PPS);
 	psy_desc->properties = anx7411_psy_props;
 	psy_desc->num_properties = ARRAY_SIZE(anx7411_psy_props);
 
diff --git a/drivers/usb/typec/rt1719.c b/drivers/usb/typec/rt1719.c
index be02d420920e..0b0c23a0b014 100644
--- a/drivers/usb/typec/rt1719.c
+++ b/drivers/usb/typec/rt1719.c
@@ -109,12 +109,6 @@ struct rt1719_data {
 	u16 conn_stat;
 };
 
-static const enum power_supply_usb_type rt1719_psy_usb_types[] = {
-	POWER_SUPPLY_USB_TYPE_C,
-	POWER_SUPPLY_USB_TYPE_PD,
-	POWER_SUPPLY_USB_TYPE_PD_PPS
-};
-
 static const enum power_supply_property rt1719_psy_properties[] = {
 	POWER_SUPPLY_PROP_ONLINE,
 	POWER_SUPPLY_PROP_USB_TYPE,
@@ -572,8 +566,9 @@ static int devm_rt1719_psy_register(struct rt1719_data *data)
 
 	data->psy_desc.name = psy_name;
 	data->psy_desc.type = POWER_SUPPLY_TYPE_USB;
-	data->psy_desc.usb_types = rt1719_psy_usb_types;
-	data->psy_desc.num_usb_types = ARRAY_SIZE(rt1719_psy_usb_types);
+	data->psy_desc.usb_types = BIT(POWER_SUPPLY_USB_TYPE_C)  |
+				   BIT(POWER_SUPPLY_USB_TYPE_PD) |
+				   BIT(POWER_SUPPLY_USB_TYPE_PD_PPS);
 	data->psy_desc.properties = rt1719_psy_properties;
 	data->psy_desc.num_properties = ARRAY_SIZE(rt1719_psy_properties);
 	data->psy_desc.get_property = rt1719_psy_get_property;
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 26f9006e95e1..0bd9c9569fb7 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -7484,12 +7484,6 @@ static int tcpm_psy_prop_writeable(struct power_supply *psy,
 	}
 }
 
-static enum power_supply_usb_type tcpm_psy_usb_types[] = {
-	POWER_SUPPLY_USB_TYPE_C,
-	POWER_SUPPLY_USB_TYPE_PD,
-	POWER_SUPPLY_USB_TYPE_PD_PPS,
-};
-
 static const char *tcpm_psy_name_prefix = "tcpm-source-psy-";
 
 static int devm_tcpm_psy_register(struct tcpm_port *port)
@@ -7510,8 +7504,9 @@ static int devm_tcpm_psy_register(struct tcpm_port *port)
 		 port_dev_name);
 	port->psy_desc.name = psy_name;
 	port->psy_desc.type = POWER_SUPPLY_TYPE_USB;
-	port->psy_desc.usb_types = tcpm_psy_usb_types;
-	port->psy_desc.num_usb_types = ARRAY_SIZE(tcpm_psy_usb_types);
+	port->psy_desc.usb_types = BIT(POWER_SUPPLY_USB_TYPE_C)  |
+				   BIT(POWER_SUPPLY_USB_TYPE_PD) |
+				   BIT(POWER_SUPPLY_USB_TYPE_PD_PPS);
 	port->psy_desc.properties = tcpm_psy_props;
 	port->psy_desc.num_properties = ARRAY_SIZE(tcpm_psy_props);
 	port->psy_desc.get_property = tcpm_psy_get_prop;
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index ea768b19a7f1..7512f0c3f6cb 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -150,11 +150,6 @@ static enum power_supply_property tps6598x_psy_props[] = {
 	POWER_SUPPLY_PROP_ONLINE,
 };
 
-static enum power_supply_usb_type tps6598x_psy_usb_types[] = {
-	POWER_SUPPLY_USB_TYPE_C,
-	POWER_SUPPLY_USB_TYPE_PD,
-};
-
 static const char *tps6598x_psy_name_prefix = "tps6598x-source-psy-";
 
 /*
@@ -827,8 +822,8 @@ static int devm_tps6598_psy_register(struct tps6598x *tps)
 
 	tps->psy_desc.name = psy_name;
 	tps->psy_desc.type = POWER_SUPPLY_TYPE_USB;
-	tps->psy_desc.usb_types = tps6598x_psy_usb_types;
-	tps->psy_desc.num_usb_types = ARRAY_SIZE(tps6598x_psy_usb_types);
+	tps->psy_desc.usb_types = BIT(POWER_SUPPLY_USB_TYPE_C) |
+				  BIT(POWER_SUPPLY_USB_TYPE_PD);
 	tps->psy_desc.properties = tps6598x_psy_props;
 	tps->psy_desc.num_properties = ARRAY_SIZE(tps6598x_psy_props);
 	tps->psy_desc.get_property = tps6598x_psy_get_prop;
diff --git a/drivers/usb/typec/ucsi/psy.c b/drivers/usb/typec/ucsi/psy.c
index e623d80e177c..1c631c7855a9 100644
--- a/drivers/usb/typec/ucsi/psy.c
+++ b/drivers/usb/typec/ucsi/psy.c
@@ -254,12 +254,6 @@ static int ucsi_psy_get_prop(struct power_supply *psy,
 	}
 }
 
-static enum power_supply_usb_type ucsi_psy_usb_types[] = {
-	POWER_SUPPLY_USB_TYPE_C,
-	POWER_SUPPLY_USB_TYPE_PD,
-	POWER_SUPPLY_USB_TYPE_PD_PPS,
-};
-
 int ucsi_register_port_psy(struct ucsi_connector *con)
 {
 	struct power_supply_config psy_cfg = {};
@@ -276,8 +270,9 @@ int ucsi_register_port_psy(struct ucsi_connector *con)
 
 	con->psy_desc.name = psy_name;
 	con->psy_desc.type = POWER_SUPPLY_TYPE_USB;
-	con->psy_desc.usb_types = ucsi_psy_usb_types;
-	con->psy_desc.num_usb_types = ARRAY_SIZE(ucsi_psy_usb_types);
+	con->psy_desc.usb_types = BIT(POWER_SUPPLY_USB_TYPE_C)  |
+				  BIT(POWER_SUPPLY_USB_TYPE_PD) |
+				  BIT(POWER_SUPPLY_USB_TYPE_PD_PPS);
 	con->psy_desc.properties = ucsi_psy_props;
 	con->psy_desc.num_properties = ARRAY_SIZE(ucsi_psy_props);
 	con->psy_desc.get_property = ucsi_psy_get_prop;
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 72dc7e45c90c..910d407ebe63 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -243,8 +243,7 @@ struct power_supply_desc {
 	const char *name;
 	enum power_supply_type type;
 	u8 charge_behaviours;
-	const enum power_supply_usb_type *usb_types;
-	size_t num_usb_types;
+	u32 usb_types;
 	const enum power_supply_property *properties;
 	size_t num_properties;
 
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/6] power: supply: "usb_type" property may be written to
  2024-08-31 14:20 ` [PATCH 1/6] power: supply: "usb_type" property may be written to Hans de Goede
@ 2024-09-03  7:04   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 14+ messages in thread
From: Greg Kroah-Hartman @ 2024-09-03  7:04 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Sebastian Reichel, Thomas Weißschuh, MyungJoo Ham,
	Chanwoo Choi, Heikki Krogerus, Vinod Koul, Kishon Vijay Abraham I,
	Enric Balletbo Serra, Andrey Smirnov, linux-pm, linux-kernel,
	linux-usb, linux-phy

On Sat, Aug 31, 2024 at 04:20:34PM +0200, Hans de Goede wrote:
> According to Documentation/ABI/testing/sysfs-class-power the "usb_type"
> property is Read-Only.
> 
> For power-supplies which consume USB power such as battery charger chips,
> this is correct.
> 
> But the UCS1002 USB Port Power Controller driver which is a driver
> for a chip which is a power-source for USB-A charging ports "usb_type"
> is actually writable to configure the type of USB charger emulated
> by the USB-A port.

Ick, crazy hardware :(

> 
> Adjust the docs and the power_supply_sysfs.c code to adjust for this
> new writeable use of "usb_type":
> 
> 1. Update Documentation/ABI/testing/sysfs-class-power to document that
> "usb_type" may be writable
> 
> 2. Change the power_supply_attr type in power_supply_sysfs.c from
> POWER_SUPPLY_ATTR() into POWER_SUPPLY_ENUM_ATTR() so that the various
> usb_type string values from POWER_SUPPLY_TYPE_TEXT[] such as e.g.
> "SDP" and "USB_PD" can be written to the "usb_type" attribute instead
> of only accepting integer values.
> 
> Cc: Enric Balletbo Serra <enric.balletbo@collabora.com>
> Cc: Andrey Smirnov <andrew.smirnov@gmail.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 6/6] power: supply: Change usb_types from an array into a bitmask
  2024-08-31 14:20 ` [PATCH 6/6] power: supply: Change usb_types from an array into a bitmask Hans de Goede
@ 2024-09-03  7:04   ` Greg Kroah-Hartman
  2024-09-03  8:27     ` Hans de Goede
  0 siblings, 1 reply; 14+ messages in thread
From: Greg Kroah-Hartman @ 2024-09-03  7:04 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Sebastian Reichel, Thomas Weißschuh, MyungJoo Ham,
	Chanwoo Choi, Heikki Krogerus, Vinod Koul, Kishon Vijay Abraham I,
	Enric Balletbo Serra, Andrey Smirnov, linux-pm, linux-kernel,
	linux-usb, linux-phy

On Sat, Aug 31, 2024 at 04:20:39PM +0200, Hans de Goede wrote:
> The bit_types array just hold a list of valid enum power_supply_usb_type
> values which map to 0 - 9. This can easily be represented as a bitmap.
> 
> This reduces the size of struct power_supply_desc and further reduces
> the data section size by drivers no longer needing to store the array.
> 
> This also unifies how usb_types are handled with charge_behaviours,
> which allows power_supply_show_usb_type() to be removed.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Nice!

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 6/6] power: supply: Change usb_types from an array into a bitmask
  2024-09-03  7:04   ` Greg Kroah-Hartman
@ 2024-09-03  8:27     ` Hans de Goede
  2024-09-03  8:32       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 14+ messages in thread
From: Hans de Goede @ 2024-09-03  8:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Sebastian Reichel, Thomas Weißschuh, MyungJoo Ham,
	Chanwoo Choi, Heikki Krogerus, Vinod Koul, Kishon Vijay Abraham I,
	Enric Balletbo Serra, Andrey Smirnov, linux-pm, linux-kernel,
	linux-usb, linux-phy

Hi Greg,

Thank you for the reviews.

On 9/3/24 9:04 AM, Greg Kroah-Hartman wrote:
> On Sat, Aug 31, 2024 at 04:20:39PM +0200, Hans de Goede wrote:
>> The bit_types array just hold a list of valid enum power_supply_usb_type
>> values which map to 0 - 9. This can easily be represented as a bitmap.
>>
>> This reduces the size of struct power_supply_desc and further reduces
>> the data section size by drivers no longer needing to store the array.
>>
>> This also unifies how usb_types are handled with charge_behaviours,
>> which allows power_supply_show_usb_type() to be removed.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> 
> Nice!
> 
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

I assume this also counts as an ack for merging this through
the linux-power-supply tree ?

Regards,

Hans




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 6/6] power: supply: Change usb_types from an array into a bitmask
  2024-09-03  8:27     ` Hans de Goede
@ 2024-09-03  8:32       ` Greg Kroah-Hartman
  0 siblings, 0 replies; 14+ messages in thread
From: Greg Kroah-Hartman @ 2024-09-03  8:32 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Sebastian Reichel, Thomas Weißschuh, MyungJoo Ham,
	Chanwoo Choi, Heikki Krogerus, Vinod Koul, Kishon Vijay Abraham I,
	Enric Balletbo Serra, Andrey Smirnov, linux-pm, linux-kernel,
	linux-usb, linux-phy

On Tue, Sep 03, 2024 at 10:27:08AM +0200, Hans de Goede wrote:
> Hi Greg,
> 
> Thank you for the reviews.
> 
> On 9/3/24 9:04 AM, Greg Kroah-Hartman wrote:
> > On Sat, Aug 31, 2024 at 04:20:39PM +0200, Hans de Goede wrote:
> >> The bit_types array just hold a list of valid enum power_supply_usb_type
> >> values which map to 0 - 9. This can easily be represented as a bitmap.
> >>
> >> This reduces the size of struct power_supply_desc and further reduces
> >> the data section size by drivers no longer needing to store the array.
> >>
> >> This also unifies how usb_types are handled with charge_behaviours,
> >> which allows power_supply_show_usb_type() to be removed.
> >>
> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > 
> > Nice!
> > 
> > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> I assume this also counts as an ack for merging this through
> the linux-power-supply tree ?

Yes please.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/6] power: supply: Change usb_types from an array into a bitmask
  2024-08-31 14:20 [PATCH 0/6] power: supply: Change usb_types from an array into a bitmask Hans de Goede
                   ` (5 preceding siblings ...)
  2024-08-31 14:20 ` [PATCH 6/6] power: supply: Change usb_types from an array into a bitmask Hans de Goede
@ 2024-09-03 21:57 ` Sebastian Reichel
  2024-09-03 22:04 ` Sebastian Reichel
  7 siblings, 0 replies; 14+ messages in thread
From: Sebastian Reichel @ 2024-09-03 21:57 UTC (permalink / raw)
  To: Sebastian Reichel, Thomas Weißschuh, MyungJoo Ham,
	Chanwoo Choi, Greg Kroah-Hartman, Heikki Krogerus, Vinod Koul,
	Kishon Vijay Abraham I, Hans de Goede
  Cc: Andrey Smirnov, linux-pm, linux-kernel, linux-usb, linux-phy,
	Enric Balletbo i Serra


On Sat, 31 Aug 2024 16:20:33 +0200, Hans de Goede wrote:
> When support for the "charge_behaviour" property was added the list of
> available values was made a bitmask in power_supply_desc.
> 
> "usb_types" is very similar in that:
> 1. It is an enum
> 2. The list of available values is stored in power_supply_desc
> 3. When shown it shows all available values, with the active one surrounded
>    by square brackets.
> 
> [...]

Applied, thanks!

[1/6] power: supply: "usb_type" property may be written to
      commit: 0d9af1e1c93b6a89f3fb6dcbafa5bc78892cb94f
[2/6] power: supply: ucs1002: Adjust ucs1002_set_usb_type() to accept string values
      commit: 83a4c42df75e8f6ff671fa9fbe7d4c79b98626de
[3/6] power: supply: rt9467-charger: Remove "usb_type" property write support
      commit: 03ec41c1670aedfbd126f541c4acbb8e69d4cd0c
[4/6] power: supply: sysfs: Add power_supply_show_enum_with_available() helper
      commit: a6456d43e9abebb5d7866e5edae3024188273306
[5/6] power: supply: sysfs: Move power_supply_show_enum_with_available() up
      commit: 322900ac7d82be0475466f81946b6484cd1936bd
[6/6] power: supply: Change usb_types from an array into a bitmask
      commit: 364ea7ccaef917a3068236a19a4b31a0623b561a

Best regards,
-- 
Sebastian Reichel <sebastian.reichel@collabora.com>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/6] power: supply: Change usb_types from an array into a bitmask
  2024-08-31 14:20 [PATCH 0/6] power: supply: Change usb_types from an array into a bitmask Hans de Goede
                   ` (6 preceding siblings ...)
  2024-09-03 21:57 ` [PATCH 0/6] " Sebastian Reichel
@ 2024-09-03 22:04 ` Sebastian Reichel
  2024-09-04  9:26   ` Hans de Goede
  7 siblings, 1 reply; 14+ messages in thread
From: Sebastian Reichel @ 2024-09-03 22:04 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Thomas Weißschuh, MyungJoo Ham, Chanwoo Choi,
	Greg Kroah-Hartman, Heikki Krogerus, Vinod Koul,
	Kishon Vijay Abraham I, Enric Balletbo Serra, Andrey Smirnov,
	linux-pm, linux-kernel, linux-usb, linux-phy

[-- Attachment #1: Type: text/plain, Size: 5758 bytes --]

Hi,

On Sat, Aug 31, 2024 at 04:20:33PM GMT, Hans de Goede wrote:
> When support for the "charge_behaviour" property was added the list of
> available values was made a bitmask in power_supply_desc.
> 
> "usb_types" is very similar in that:
> 1. It is an enum
> 2. The list of available values is stored in power_supply_desc
> 3. When shown it shows all available values, with the active one surrounded
>    by square brackets.
> 
> But "usb_types" uses an array with valid enum values instead of a bitmask.
> This uses more memory then the bitmap approach and it makes it impossible
> to have a shared generic show() function for properties which show
> available values, with the active one surrounded by square brackets.
> 
> This patch-set moves "usb_types" over to a bitmask in power_supply_desc
> to indicate the available values.
> 
> Patches 1 - 3:
> 
> It turns out that the ucs1002-power driver contained a surprise in that
> it supports writing to "usb_type" even though the ABI doc says it is
> read-only. Since we cannot break shipped userspace API, the ship has sailed
> on this one. The first patch documents that writing "usb_type" is allowed,
> but only for power-supply devices which provide USB power rather then
> consume it.
> 
> Enum properties accept writing the FOO_TEXT[] string values, passing
> the enum value matching the FOO_TEXT entry to set_property(), the second
> patch adjusts ucs1002_set_usb_type() to directly accept enum values.
> 
> The rt9467 driver was another driver which allowed writing to "usb_type"
> but there the use made no sense, so it is simply dropped.
> 
> Patches 4 - 6:
> 
> These patches implement the actual moving of usb_types to a bitmask.
> 
> Patch 6 is a bit of a bigbang patch moving all drivers over in one go,
> touching a couple of drivers outside drivers/power/supply: 1 in
> drivers/extcon/ 1 in drivers/phy/ and 5 in drivers/usb/typec/ since
> the changes outside of drivers/power/supply are small I've chosen to
> make all the changes in one go rather then have some sort of
> intermediate state where both ways are supported.
> 
> For merging this I believe it would be best for an immutable branch / tag
> to be created on the linux-power-supply tree and then send a pull-request
> to the extcon, phy and usb-typec maintainers to merge the tag.
> 
> extcon, phy and typec maintainers can you please give your Acked-by for
> patch 6/6 for merging these changes through the linux-power-supply tree?
> 
> This set is based on top of the latest linux-power-supply/for-next.

As we are quite close before the merge window I just took it with
just the Ack from Greg. The changes in PHY and Extcon are quite
small. I did prepare an immutable branch in case it is needed by
any of the other involved trees:

The following changes since commit 8400291e289ee6b2bf9779ff1c83a291501f017b:

  Linux 6.11-rc1 (2024-07-28 14:19:55 -0700)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git tags/ib-psy-usb-types-signed

for you to fetch changes up to 364ea7ccaef917a3068236a19a4b31a0623b561a:

  power: supply: Change usb_types from an array into a bitmask (2024-09-03 23:20:28 +0200)

----------------------------------------------------------------
Immutable branch for usb_types change for v6.12

Changing usb_types type from array to bitmap in the power_supply_desc
struct requires updating power-supply drivers living in different
subsystem, so it is handled via an immutable branch.

----------------------------------------------------------------
Hans de Goede (6):
      power: supply: "usb_type" property may be written to
      power: supply: ucs1002: Adjust ucs1002_set_usb_type() to accept string values
      power: supply: rt9467-charger: Remove "usb_type" property write support
      power: supply: sysfs: Add power_supply_show_enum_with_available() helper
      power: supply: sysfs: Move power_supply_show_enum_with_available() up
      power: supply: Change usb_types from an array into a bitmask

 Documentation/ABI/testing/sysfs-class-power     |  7 ++-
 drivers/extcon/extcon-intel-cht-wc.c            | 15 ++----
 drivers/phy/ti/phy-tusb1210.c                   | 11 ++---
 drivers/power/supply/axp20x_usb_power.c         | 13 ++---
 drivers/power/supply/bq256xx_charger.c          | 15 ++----
 drivers/power/supply/cros_usbpd-charger.c       | 22 +++------
 drivers/power/supply/lenovo_yoga_c630_battery.c |  7 +--
 drivers/power/supply/mp2629_charger.c           | 15 ++----
 drivers/power/supply/mt6360_charger.c           | 13 ++---
 drivers/power/supply/mt6370-charger.c           | 13 ++---
 drivers/power/supply/power_supply_core.c        |  4 --
 drivers/power/supply/power_supply_sysfs.c       | 66 ++++++++-----------------
 drivers/power/supply/qcom_battmgr.c             | 37 +++++++-------
 drivers/power/supply/qcom_pmi8998_charger.c     | 13 ++---
 drivers/power/supply/rk817_charger.c            |  9 +---
 drivers/power/supply/rn5t618_power.c            | 13 ++---
 drivers/power/supply/rt9467-charger.c           | 16 ++----
 drivers/power/supply/rt9471.c                   | 15 ++----
 drivers/power/supply/ucs1002_power.c            | 26 +++++-----
 drivers/usb/typec/anx7411.c                     | 11 ++---
 drivers/usb/typec/rt1719.c                      | 11 ++---
 drivers/usb/typec/tcpm/tcpm.c                   | 11 ++---
 drivers/usb/typec/tipd/core.c                   |  9 +---
 drivers/usb/typec/ucsi/psy.c                    | 11 ++---
 include/linux/power_supply.h                    |  3 +-
 25 files changed, 132 insertions(+), 254 deletions(-)

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/6] power: supply: Change usb_types from an array into a bitmask
  2024-09-03 22:04 ` Sebastian Reichel
@ 2024-09-04  9:26   ` Hans de Goede
  0 siblings, 0 replies; 14+ messages in thread
From: Hans de Goede @ 2024-09-04  9:26 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Thomas Weißschuh, MyungJoo Ham, Chanwoo Choi,
	Greg Kroah-Hartman, Heikki Krogerus, Vinod Koul,
	Kishon Vijay Abraham I, Enric Balletbo Serra, Andrey Smirnov,
	linux-pm, linux-kernel, linux-usb, linux-phy

Hi,

On 9/4/24 12:04 AM, Sebastian Reichel wrote:
> Hi,
> 
> On Sat, Aug 31, 2024 at 04:20:33PM GMT, Hans de Goede wrote:
>> When support for the "charge_behaviour" property was added the list of
>> available values was made a bitmask in power_supply_desc.
>>
>> "usb_types" is very similar in that:
>> 1. It is an enum
>> 2. The list of available values is stored in power_supply_desc
>> 3. When shown it shows all available values, with the active one surrounded
>>    by square brackets.
>>
>> But "usb_types" uses an array with valid enum values instead of a bitmask.
>> This uses more memory then the bitmap approach and it makes it impossible
>> to have a shared generic show() function for properties which show
>> available values, with the active one surrounded by square brackets.
>>
>> This patch-set moves "usb_types" over to a bitmask in power_supply_desc
>> to indicate the available values.
>>
>> Patches 1 - 3:
>>
>> It turns out that the ucs1002-power driver contained a surprise in that
>> it supports writing to "usb_type" even though the ABI doc says it is
>> read-only. Since we cannot break shipped userspace API, the ship has sailed
>> on this one. The first patch documents that writing "usb_type" is allowed,
>> but only for power-supply devices which provide USB power rather then
>> consume it.
>>
>> Enum properties accept writing the FOO_TEXT[] string values, passing
>> the enum value matching the FOO_TEXT entry to set_property(), the second
>> patch adjusts ucs1002_set_usb_type() to directly accept enum values.
>>
>> The rt9467 driver was another driver which allowed writing to "usb_type"
>> but there the use made no sense, so it is simply dropped.
>>
>> Patches 4 - 6:
>>
>> These patches implement the actual moving of usb_types to a bitmask.
>>
>> Patch 6 is a bit of a bigbang patch moving all drivers over in one go,
>> touching a couple of drivers outside drivers/power/supply: 1 in
>> drivers/extcon/ 1 in drivers/phy/ and 5 in drivers/usb/typec/ since
>> the changes outside of drivers/power/supply are small I've chosen to
>> make all the changes in one go rather then have some sort of
>> intermediate state where both ways are supported.
>>
>> For merging this I believe it would be best for an immutable branch / tag
>> to be created on the linux-power-supply tree and then send a pull-request
>> to the extcon, phy and usb-typec maintainers to merge the tag.
>>
>> extcon, phy and typec maintainers can you please give your Acked-by for
>> patch 6/6 for merging these changes through the linux-power-supply tree?
>>
>> This set is based on top of the latest linux-power-supply/for-next.
> 
> As we are quite close before the merge window I just took it with
> just the Ack from Greg. The changes in PHY and Extcon are quite
> small.

Great, thank you.

Regards,

Hans




> I did prepare an immutable branch in case it is needed by
> any of the other involved trees:
> 
> The following changes since commit 8400291e289ee6b2bf9779ff1c83a291501f017b:
> 
>   Linux 6.11-rc1 (2024-07-28 14:19:55 -0700)
> 
> are available in the Git repository at:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git tags/ib-psy-usb-types-signed
> 
> for you to fetch changes up to 364ea7ccaef917a3068236a19a4b31a0623b561a:
> 
>   power: supply: Change usb_types from an array into a bitmask (2024-09-03 23:20:28 +0200)
> 
> ----------------------------------------------------------------
> Immutable branch for usb_types change for v6.12
> 
> Changing usb_types type from array to bitmap in the power_supply_desc
> struct requires updating power-supply drivers living in different
> subsystem, so it is handled via an immutable branch.
> 
> ----------------------------------------------------------------
> Hans de Goede (6):
>       power: supply: "usb_type" property may be written to
>       power: supply: ucs1002: Adjust ucs1002_set_usb_type() to accept string values
>       power: supply: rt9467-charger: Remove "usb_type" property write support
>       power: supply: sysfs: Add power_supply_show_enum_with_available() helper
>       power: supply: sysfs: Move power_supply_show_enum_with_available() up
>       power: supply: Change usb_types from an array into a bitmask
> 
>  Documentation/ABI/testing/sysfs-class-power     |  7 ++-
>  drivers/extcon/extcon-intel-cht-wc.c            | 15 ++----
>  drivers/phy/ti/phy-tusb1210.c                   | 11 ++---
>  drivers/power/supply/axp20x_usb_power.c         | 13 ++---
>  drivers/power/supply/bq256xx_charger.c          | 15 ++----
>  drivers/power/supply/cros_usbpd-charger.c       | 22 +++------
>  drivers/power/supply/lenovo_yoga_c630_battery.c |  7 +--
>  drivers/power/supply/mp2629_charger.c           | 15 ++----
>  drivers/power/supply/mt6360_charger.c           | 13 ++---
>  drivers/power/supply/mt6370-charger.c           | 13 ++---
>  drivers/power/supply/power_supply_core.c        |  4 --
>  drivers/power/supply/power_supply_sysfs.c       | 66 ++++++++-----------------
>  drivers/power/supply/qcom_battmgr.c             | 37 +++++++-------
>  drivers/power/supply/qcom_pmi8998_charger.c     | 13 ++---
>  drivers/power/supply/rk817_charger.c            |  9 +---
>  drivers/power/supply/rn5t618_power.c            | 13 ++---
>  drivers/power/supply/rt9467-charger.c           | 16 ++----
>  drivers/power/supply/rt9471.c                   | 15 ++----
>  drivers/power/supply/ucs1002_power.c            | 26 +++++-----
>  drivers/usb/typec/anx7411.c                     | 11 ++---
>  drivers/usb/typec/rt1719.c                      | 11 ++---
>  drivers/usb/typec/tcpm/tcpm.c                   | 11 ++---
>  drivers/usb/typec/tipd/core.c                   |  9 +---
>  drivers/usb/typec/ucsi/psy.c                    | 11 ++---
>  include/linux/power_supply.h                    |  3 +-
>  25 files changed, 132 insertions(+), 254 deletions(-)
> 
> -- Sebastian


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2024-09-04  9:26 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-31 14:20 [PATCH 0/6] power: supply: Change usb_types from an array into a bitmask Hans de Goede
2024-08-31 14:20 ` [PATCH 1/6] power: supply: "usb_type" property may be written to Hans de Goede
2024-09-03  7:04   ` Greg Kroah-Hartman
2024-08-31 14:20 ` [PATCH 2/6] power: supply: ucs1002: Adjust ucs1002_set_usb_type() to accept string values Hans de Goede
2024-08-31 14:20 ` [PATCH 3/6] power: supply: rt9467-charger: Remove "usb_type" property write support Hans de Goede
2024-08-31 14:20 ` [PATCH 4/6] power: supply: sysfs: Add power_supply_show_enum_with_available() helper Hans de Goede
2024-08-31 14:20 ` [PATCH 5/6] power: supply: sysfs: Move power_supply_show_enum_with_available() up Hans de Goede
2024-08-31 14:20 ` [PATCH 6/6] power: supply: Change usb_types from an array into a bitmask Hans de Goede
2024-09-03  7:04   ` Greg Kroah-Hartman
2024-09-03  8:27     ` Hans de Goede
2024-09-03  8:32       ` Greg Kroah-Hartman
2024-09-03 21:57 ` [PATCH 0/6] " Sebastian Reichel
2024-09-03 22:04 ` Sebastian Reichel
2024-09-04  9:26   ` Hans de Goede

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox