From: Sebastian Reichel <sebastian.reichel@collabora.com>
To: Mathew King <mathewk@chromium.org>
Cc: linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org
Subject: Re: [PATCH 3/4] power_supply: Add a macro that maps enum properties to text values
Date: Sun, 3 May 2020 03:20:10 +0200 [thread overview]
Message-ID: <20200503012010.i7wdh4ibsjdzpckr@earth.universe> (raw)
In-Reply-To: <20200424173533.48572-4-mathewk@chromium.org>
[-- Attachment #1: Type: text/plain, Size: 10692 bytes --]
Hi,
On Fri, Apr 24, 2020 at 11:35:32AM -0600, Mathew King wrote:
> Reduce the number of touch points to add a new enum property to the
> power_supply class by mapping the array of text values to the device
> attribute descriptor. A new enum property can now added by creating an
> array with the text values named POWER_SUPPLY_${PROPNAME}_TEXT and
> adding POWER_SUPPLY_ENUM_ATTR(${PROPNAME}) to the power_supply_attrs
> array.
>
> Signed-off-by: Mathew King <mathewk@chromium.org>
> ---
nice cleanup :)
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
-- Sebastian
> drivers/power/supply/power_supply_sysfs.c | 122 +++++++++-------------
> 1 file changed, 49 insertions(+), 73 deletions(-)
>
> diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> index 328107589770..fbb05466b9a5 100644
> --- a/drivers/power/supply/power_supply_sysfs.c
> +++ b/drivers/power/supply/power_supply_sysfs.c
> @@ -26,15 +26,25 @@ struct power_supply_attr {
> const char *upper_name;
> const char *lower_name;
> struct device_attribute dev_attr;
> + const char * const *text_values;
> + int text_values_len;
> };
>
> -#define POWER_SUPPLY_ATTR(_name) \
> -[POWER_SUPPLY_PROP_ ## _name] = \
> -{ \
> - .prop_name = #_name \
> +#define _POWER_SUPPLY_ATTR(_name, _text, _len) \
> +[POWER_SUPPLY_PROP_ ## _name] = \
> +{ \
> + .prop_name = #_name, \
> + .text_values = _text, \
> + .text_values_len = _len, \
> }
>
> -static const char * const power_supply_type_text[] = {
> +#define POWER_SUPPLY_ATTR(_name) _POWER_SUPPLY_ATTR(_name, NULL, 0)
> +#define _POWER_SUPPLY_ENUM_ATTR(_name, _text) \
> + _POWER_SUPPLY_ATTR(_name, _text, ARRAY_SIZE(_text))
> +#define POWER_SUPPLY_ENUM_ATTR(_name) \
> + _POWER_SUPPLY_ENUM_ATTR(_name, POWER_SUPPLY_ ## _name ## _TEXT)
> +
> +static const char * const POWER_SUPPLY_TYPE_TEXT[] = {
> [POWER_SUPPLY_TYPE_UNKNOWN] = "Unknown",
> [POWER_SUPPLY_TYPE_BATTERY] = "Battery",
> [POWER_SUPPLY_TYPE_UPS] = "UPS",
> @@ -62,7 +72,7 @@ static const char * const POWER_SUPPLY_USB_TYPE_TEXT[] = {
> [POWER_SUPPLY_USB_TYPE_APPLE_BRICK_ID] = "BrickID",
> };
>
> -static const char * const power_supply_status_text[] = {
> +static const char * const POWER_SUPPLY_STATUS_TEXT[] = {
> [POWER_SUPPLY_STATUS_UNKNOWN] = "Unknown",
> [POWER_SUPPLY_STATUS_CHARGING] = "Charging",
> [POWER_SUPPLY_STATUS_DISCHARGING] = "Discharging",
> @@ -70,7 +80,7 @@ static const char * const power_supply_status_text[] = {
> [POWER_SUPPLY_STATUS_FULL] = "Full",
> };
>
> -static const char * const power_supply_charge_type_text[] = {
> +static const char * const POWER_SUPPLY_CHARGE_TYPE_TEXT[] = {
> [POWER_SUPPLY_CHARGE_TYPE_UNKNOWN] = "Unknown",
> [POWER_SUPPLY_CHARGE_TYPE_NONE] = "N/A",
> [POWER_SUPPLY_CHARGE_TYPE_TRICKLE] = "Trickle",
> @@ -80,7 +90,7 @@ static const char * const power_supply_charge_type_text[] = {
> [POWER_SUPPLY_CHARGE_TYPE_CUSTOM] = "Custom",
> };
>
> -static const char * const power_supply_health_text[] = {
> +static const char * const POWER_SUPPLY_HEALTH_TEXT[] = {
> [POWER_SUPPLY_HEALTH_UNKNOWN] = "Unknown",
> [POWER_SUPPLY_HEALTH_GOOD] = "Good",
> [POWER_SUPPLY_HEALTH_OVERHEAT] = "Overheat",
> @@ -93,7 +103,7 @@ static const char * const power_supply_health_text[] = {
> [POWER_SUPPLY_HEALTH_OVERCURRENT] = "Over current",
> };
>
> -static const char * const power_supply_technology_text[] = {
> +static const char * const POWER_SUPPLY_TECHNOLOGY_TEXT[] = {
> [POWER_SUPPLY_TECHNOLOGY_UNKNOWN] = "Unknown",
> [POWER_SUPPLY_TECHNOLOGY_NiMH] = "NiMH",
> [POWER_SUPPLY_TECHNOLOGY_LION] = "Li-ion",
> @@ -103,7 +113,7 @@ static const char * const power_supply_technology_text[] = {
> [POWER_SUPPLY_TECHNOLOGY_LiMn] = "LiMn",
> };
>
> -static const char * const power_supply_capacity_level_text[] = {
> +static const char * const POWER_SUPPLY_CAPACITY_LEVEL_TEXT[] = {
> [POWER_SUPPLY_CAPACITY_LEVEL_UNKNOWN] = "Unknown",
> [POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL] = "Critical",
> [POWER_SUPPLY_CAPACITY_LEVEL_LOW] = "Low",
> @@ -112,7 +122,7 @@ static const char * const power_supply_capacity_level_text[] = {
> [POWER_SUPPLY_CAPACITY_LEVEL_FULL] = "Full",
> };
>
> -static const char * const power_supply_scope_text[] = {
> +static const char * const POWER_SUPPLY_SCOPE_TEXT[] = {
> [POWER_SUPPLY_SCOPE_UNKNOWN] = "Unknown",
> [POWER_SUPPLY_SCOPE_SYSTEM] = "System",
> [POWER_SUPPLY_SCOPE_DEVICE] = "Device",
> @@ -120,13 +130,13 @@ static const char * const power_supply_scope_text[] = {
>
> static struct power_supply_attr power_supply_attrs[] = {
> /* Properties of type `int' */
> - POWER_SUPPLY_ATTR(STATUS),
> - POWER_SUPPLY_ATTR(CHARGE_TYPE),
> - POWER_SUPPLY_ATTR(HEALTH),
> + POWER_SUPPLY_ENUM_ATTR(STATUS),
> + POWER_SUPPLY_ENUM_ATTR(CHARGE_TYPE),
> + POWER_SUPPLY_ENUM_ATTR(HEALTH),
> POWER_SUPPLY_ATTR(PRESENT),
> POWER_SUPPLY_ATTR(ONLINE),
> POWER_SUPPLY_ATTR(AUTHENTIC),
> - POWER_SUPPLY_ATTR(TECHNOLOGY),
> + POWER_SUPPLY_ENUM_ATTR(TECHNOLOGY),
> POWER_SUPPLY_ATTR(CYCLE_COUNT),
> POWER_SUPPLY_ATTR(VOLTAGE_MAX),
> POWER_SUPPLY_ATTR(VOLTAGE_MIN),
> @@ -169,7 +179,7 @@ static struct power_supply_attr power_supply_attrs[] = {
> POWER_SUPPLY_ATTR(CAPACITY),
> POWER_SUPPLY_ATTR(CAPACITY_ALERT_MIN),
> POWER_SUPPLY_ATTR(CAPACITY_ALERT_MAX),
> - POWER_SUPPLY_ATTR(CAPACITY_LEVEL),
> + POWER_SUPPLY_ENUM_ATTR(CAPACITY_LEVEL),
> POWER_SUPPLY_ATTR(TEMP),
> POWER_SUPPLY_ATTR(TEMP_MAX),
> POWER_SUPPLY_ATTR(TEMP_MIN),
> @@ -182,9 +192,9 @@ static struct power_supply_attr power_supply_attrs[] = {
> POWER_SUPPLY_ATTR(TIME_TO_EMPTY_AVG),
> POWER_SUPPLY_ATTR(TIME_TO_FULL_NOW),
> POWER_SUPPLY_ATTR(TIME_TO_FULL_AVG),
> - POWER_SUPPLY_ATTR(TYPE),
> + POWER_SUPPLY_ENUM_ATTR(TYPE),
> POWER_SUPPLY_ATTR(USB_TYPE),
> - POWER_SUPPLY_ATTR(SCOPE),
> + POWER_SUPPLY_ENUM_ATTR(SCOPE),
> POWER_SUPPLY_ATTR(PRECHARGE_CURRENT),
> POWER_SUPPLY_ATTR(CHARGE_TERM_CURRENT),
> POWER_SUPPLY_ATTR(CALIBRATE),
> @@ -197,10 +207,14 @@ static struct power_supply_attr power_supply_attrs[] = {
> static struct attribute *
> __power_supply_attrs[ARRAY_SIZE(power_supply_attrs) + 1];
>
> +static struct power_supply_attr *to_ps_attr(struct device_attribute *attr)
> +{
> + return container_of(attr, struct power_supply_attr, dev_attr);
> +}
> +
> static enum power_supply_property dev_attr_psp(struct device_attribute *attr)
> {
> - return container_of(attr, struct power_supply_attr, dev_attr) -
> - power_supply_attrs;
> + return to_ps_attr(attr) - power_supply_attrs;
> }
>
> static ssize_t power_supply_show_usb_type(struct device *dev,
> @@ -219,11 +233,11 @@ static ssize_t power_supply_show_usb_type(struct device *dev,
>
> if (value->intval == usb_type) {
> count += sprintf(buf + count, "[%s] ",
> - power_supply_usb_type_text[usb_type]);
> + POWER_SUPPLY_USB_TYPE_TEXT[usb_type]);
> match = true;
> } else {
> count += sprintf(buf + count, "%s ",
> - power_supply_usb_type_text[usb_type]);
> + POWER_SUPPLY_USB_TYPE_TEXT[usb_type]);
> }
> }
>
> @@ -243,6 +257,7 @@ static ssize_t power_supply_show_property(struct device *dev,
> char *buf) {
> ssize_t ret;
> struct power_supply *psy = dev_get_drvdata(dev);
> + struct power_supply_attr *ps_attr = to_ps_attr(attr);
> enum power_supply_property psp = dev_attr_psp(attr);
> union power_supply_propval value;
>
> @@ -263,39 +278,16 @@ static ssize_t power_supply_show_property(struct device *dev,
> }
> }
>
> + if (ps_attr->text_values_len > 0 &&
> + value.intval < ps_attr->text_values_len && value.intval >= 0) {
> + return sprintf(buf, "%s\n", ps_attr->text_values[value.intval]);
> + }
> +
> switch (psp) {
> - case POWER_SUPPLY_PROP_STATUS:
> - ret = sprintf(buf, "%s\n",
> - power_supply_status_text[value.intval]);
> - break;
> - case POWER_SUPPLY_PROP_CHARGE_TYPE:
> - ret = sprintf(buf, "%s\n",
> - power_supply_charge_type_text[value.intval]);
> - break;
> - case POWER_SUPPLY_PROP_HEALTH:
> - ret = sprintf(buf, "%s\n",
> - power_supply_health_text[value.intval]);
> - break;
> - case POWER_SUPPLY_PROP_TECHNOLOGY:
> - ret = sprintf(buf, "%s\n",
> - power_supply_technology_text[value.intval]);
> - break;
> - case POWER_SUPPLY_PROP_CAPACITY_LEVEL:
> - ret = sprintf(buf, "%s\n",
> - power_supply_capacity_level_text[value.intval]);
> - break;
> - case POWER_SUPPLY_PROP_TYPE:
> - ret = sprintf(buf, "%s\n",
> - power_supply_type_text[value.intval]);
> - break;
> case POWER_SUPPLY_PROP_USB_TYPE:
> ret = power_supply_show_usb_type(dev, psy->desc->usb_types,
> - psy->desc->num_usb_types,
> - &value, buf);
> - break;
> - case POWER_SUPPLY_PROP_SCOPE:
> - ret = sprintf(buf, "%s\n",
> - power_supply_scope_text[value.intval]);
> + psy->desc->num_usb_types,
> + &value, buf);
> break;
> case POWER_SUPPLY_PROP_MODEL_NAME ... POWER_SUPPLY_PROP_SERIAL_NUMBER:
> ret = sprintf(buf, "%s\n", value.strval);
> @@ -312,30 +304,14 @@ static ssize_t power_supply_store_property(struct device *dev,
> const char *buf, size_t count) {
> ssize_t ret;
> struct power_supply *psy = dev_get_drvdata(dev);
> + struct power_supply_attr *ps_attr = to_ps_attr(attr);
> enum power_supply_property psp = dev_attr_psp(attr);
> union power_supply_propval value;
>
> - switch (psp) {
> - case POWER_SUPPLY_PROP_STATUS:
> - ret = sysfs_match_string(power_supply_status_text, buf);
> - break;
> - case POWER_SUPPLY_PROP_CHARGE_TYPE:
> - ret = sysfs_match_string(power_supply_charge_type_text, buf);
> - break;
> - case POWER_SUPPLY_PROP_HEALTH:
> - ret = sysfs_match_string(power_supply_health_text, buf);
> - break;
> - case POWER_SUPPLY_PROP_TECHNOLOGY:
> - ret = sysfs_match_string(power_supply_technology_text, buf);
> - break;
> - case POWER_SUPPLY_PROP_CAPACITY_LEVEL:
> - ret = sysfs_match_string(power_supply_capacity_level_text, buf);
> - break;
> - case POWER_SUPPLY_PROP_SCOPE:
> - ret = sysfs_match_string(power_supply_scope_text, buf);
> - break;
> - default:
> - ret = -EINVAL;
> + ret = -EINVAL;
> + if (ps_attr->text_values_len > 0) {
> + ret = __sysfs_match_string(ps_attr->text_values,
> + ps_attr->text_values_len, buf);
> }
>
> /*
> --
> 2.26.2.303.gf8c07b1a785-goog
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2020-05-03 1:20 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-24 17:35 [PATCH 0/4] Cleanup power_supply_sysfs.c Mathew King
2020-04-24 17:35 ` [PATCH 1/4] power_supply: Cleanup power supply sysfs attribute list Mathew King
2020-05-02 0:32 ` Sebastian Reichel
2020-05-04 20:31 ` Mat King
2020-04-24 17:35 ` [PATCH 2/4] power_supply: Use designated initializer for property text arrays Mathew King
2020-05-02 0:34 ` Sebastian Reichel
2020-04-24 17:35 ` [PATCH 3/4] power_supply: Add a macro that maps enum properties to text values Mathew King
2020-05-03 1:20 ` Sebastian Reichel [this message]
2020-04-24 17:35 ` [PATCH 4/4] power_supply: Add power supply type property to uevent env Mathew King
2020-05-03 1:27 ` Sebastian Reichel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200503012010.i7wdh4ibsjdzpckr@earth.universe \
--to=sebastian.reichel@collabora.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=mathewk@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.