diff mbox series

[v5,4/4] power: supply: cros_charge-control: implement a power supply extension

Message ID 20241205-power-supply-extensions-v5-4-f0f996db4347@weissschuh.net (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series power: supply: extension API | expand

Commit Message

Thomas Weißschuh Dec. 5, 2024, 8:46 p.m. UTC 
Power supply extensions provide an easier mechanism to implement
additional properties for existing power supplies.
Use that instead of reimplementing the sysfs attributes manually.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/power/supply/cros_charge-control.c | 217 ++++++++++++-----------------
 1 file changed, 88 insertions(+), 129 deletions(-)

Comments

Sebastian Reichel Dec. 6, 2024, 1:15 a.m. UTC | #1 
Hi,

On Thu, Dec 05, 2024 at 09:46:38PM +0100, Thomas Weißschuh wrote:
> Power supply extensions provide an easier mechanism to implement
> additional properties for existing power supplies.
> Use that instead of reimplementing the sysfs attributes manually.
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---

LGTM.

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian

>  drivers/power/supply/cros_charge-control.c | 217 ++++++++++++-----------------
>  1 file changed, 88 insertions(+), 129 deletions(-)
> 
> diff --git a/drivers/power/supply/cros_charge-control.c b/drivers/power/supply/cros_charge-control.c
> index 17c53591ce197d08d97c94d3d4359a282026dd7d..7f914a65f88f8ea57bf01c334d25d9b8ebaa2de7 100644
> --- a/drivers/power/supply/cros_charge-control.c
> +++ b/drivers/power/supply/cros_charge-control.c
> @@ -18,13 +18,6 @@
>  					 BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE)   | \
>  					 BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_FORCE_DISCHARGE))
>  
> -enum CROS_CHCTL_ATTR {
> -	CROS_CHCTL_ATTR_START_THRESHOLD,
> -	CROS_CHCTL_ATTR_END_THRESHOLD,
> -	CROS_CHCTL_ATTR_CHARGE_BEHAVIOUR,
> -	_CROS_CHCTL_ATTR_COUNT
> -};
> -
>  /*
>   * Semantics of data *returned* from the EC API and Linux sysfs differ
>   * slightly, also the v1 API can not return any data.
> @@ -41,13 +34,7 @@ struct cros_chctl_priv {
>  	struct power_supply *hooked_battery;
>  	u8 cmd_version;
>  
> -	/* The callbacks need to access this priv structure.
> -	 * As neither the struct device nor power_supply are under the drivers
> -	 * control, embed the attributes within priv to use with container_of().
> -	 */
> -	struct device_attribute device_attrs[_CROS_CHCTL_ATTR_COUNT];
> -	struct attribute *attributes[_CROS_CHCTL_ATTR_COUNT];
> -	struct attribute_group group;
> +	const struct power_supply_ext *psy_ext;
>  
>  	enum power_supply_charge_behaviour current_behaviour;
>  	u8 current_start_threshold, current_end_threshold;
> @@ -114,124 +101,104 @@ static int cros_chctl_configure_ec(struct cros_chctl_priv *priv)
>  	return cros_chctl_send_charge_control_cmd(priv->cros_ec, priv->cmd_version, &req);
>  }
>  
> -static struct cros_chctl_priv *cros_chctl_attr_to_priv(struct attribute *attr,
> -						       enum CROS_CHCTL_ATTR idx)
> -{
> -	struct device_attribute *dev_attr = container_of(attr, struct device_attribute, attr);
> -
> -	return container_of(dev_attr, struct cros_chctl_priv, device_attrs[idx]);
> -}
> +static const enum power_supply_property cros_chctl_psy_ext_props[] = {
> +	POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR, /* has to be first for cros_chctl_psy_ext_v1 */
> +	POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD,
> +	POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD,
> +};
>  
> -static ssize_t cros_chctl_store_threshold(struct device *dev, struct cros_chctl_priv *priv,
> -					  int is_end_threshold, const char *buf, size_t count)
> +static int cros_chctl_psy_ext_get_prop(struct power_supply *psy,
> +				       const struct power_supply_ext *ext,
> +				       void *data,
> +				       enum power_supply_property psp,
> +				       union power_supply_propval *val)
>  {
> -	int ret, val;
> +	struct cros_chctl_priv *priv = data;
>  
> -	ret = kstrtoint(buf, 10, &val);
> -	if (ret < 0)
> -		return ret;
> -	if (val < 0 || val > 100)
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD:
> +		val->intval = priv->current_start_threshold;
> +		return 0;
> +	case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD:
> +		val->intval = priv->current_end_threshold;
> +		return 0;
> +	case POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR:
> +		val->intval = priv->current_behaviour;
> +		return 0;
> +	default:
>  		return -EINVAL;
> -
> -	if (is_end_threshold) {
> -		if (val <= priv->current_start_threshold)
> -			return -EINVAL;
> -		priv->current_end_threshold = val;
> -	} else {
> -		if (val >= priv->current_end_threshold)
> -			return -EINVAL;
> -		priv->current_start_threshold = val;
>  	}
> -
> -	if (priv->current_behaviour == POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO) {
> -		ret = cros_chctl_configure_ec(priv);
> -		if (ret < 0)
> -			return ret;
> -	}
> -
> -	return count;
> -}
> -
> -static ssize_t charge_control_start_threshold_show(struct device *dev,
> -						   struct device_attribute *attr,
> -						   char *buf)
> -{
> -	struct cros_chctl_priv *priv = cros_chctl_attr_to_priv(&attr->attr,
> -							       CROS_CHCTL_ATTR_START_THRESHOLD);
> -
> -	return sysfs_emit(buf, "%u\n", (unsigned int)priv->current_start_threshold);
> -}
> -
> -static ssize_t charge_control_start_threshold_store(struct device *dev,
> -						    struct device_attribute *attr,
> -						    const char *buf, size_t count)
> -{
> -	struct cros_chctl_priv *priv = cros_chctl_attr_to_priv(&attr->attr,
> -							       CROS_CHCTL_ATTR_START_THRESHOLD);
> -
> -	return cros_chctl_store_threshold(dev, priv, 0, buf, count);
> -}
> -
> -static ssize_t charge_control_end_threshold_show(struct device *dev, struct device_attribute *attr,
> -						 char *buf)
> -{
> -	struct cros_chctl_priv *priv = cros_chctl_attr_to_priv(&attr->attr,
> -							       CROS_CHCTL_ATTR_END_THRESHOLD);
> -
> -	return sysfs_emit(buf, "%u\n", (unsigned int)priv->current_end_threshold);
> -}
> -
> -static ssize_t charge_control_end_threshold_store(struct device *dev, struct device_attribute *attr,
> -						  const char *buf, size_t count)
> -{
> -	struct cros_chctl_priv *priv = cros_chctl_attr_to_priv(&attr->attr,
> -							       CROS_CHCTL_ATTR_END_THRESHOLD);
> -
> -	return cros_chctl_store_threshold(dev, priv, 1, buf, count);
>  }
>  
> -static ssize_t charge_behaviour_show(struct device *dev, struct device_attribute *attr, char *buf)
> -{
> -	struct cros_chctl_priv *priv = cros_chctl_attr_to_priv(&attr->attr,
> -							       CROS_CHCTL_ATTR_CHARGE_BEHAVIOUR);
> -
> -	return power_supply_charge_behaviour_show(dev, EC_CHARGE_CONTROL_BEHAVIOURS,
> -						  priv->current_behaviour, buf);
> -}
>  
> -static ssize_t charge_behaviour_store(struct device *dev, struct device_attribute *attr,
> -				      const char *buf, size_t count)
> +static int cros_chctl_psy_ext_set_prop(struct power_supply *psy,
> +				       const struct power_supply_ext *ext,
> +				       void *data,
> +				       enum power_supply_property psp,
> +				       const union power_supply_propval *val)
>  {
> -	struct cros_chctl_priv *priv = cros_chctl_attr_to_priv(&attr->attr,
> -							       CROS_CHCTL_ATTR_CHARGE_BEHAVIOUR);
> +	struct cros_chctl_priv *priv = data;
>  	int ret;
>  
> -	ret = power_supply_charge_behaviour_parse(EC_CHARGE_CONTROL_BEHAVIOURS, buf);
> -	if (ret < 0)
> -		return ret;
> -
> -	priv->current_behaviour = ret;
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD:
> +	case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD:
> +		if (val->intval < 0 || val->intval > 100)
> +			return -EINVAL;
>  
> -	ret = cros_chctl_configure_ec(priv);
> -	if (ret < 0)
> -		return ret;
> +		if (psp == POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD) {
> +			if (val->intval <= priv->current_start_threshold)
> +				return -EINVAL;
> +			priv->current_end_threshold = val->intval;
> +		} else {
> +			if (val->intval >= priv->current_end_threshold)
> +				return -EINVAL;
> +			priv->current_start_threshold = val->intval;
> +		}
> +
> +		if (priv->current_behaviour == POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO) {
> +			ret = cros_chctl_configure_ec(priv);
> +			if (ret < 0)
> +				return ret;
> +		}
>  
> -	return count;
> +		return 0;
> +	case POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR:
> +		priv->current_behaviour = val->intval;
> +		ret = cros_chctl_configure_ec(priv);
> +		if (ret < 0)
> +			return ret;
> +		return 0;
> +	default:
> +		return -EINVAL;
> +	}
>  }
>  
> -static umode_t cros_chtl_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n)
> +static int cros_chctl_psy_prop_is_writeable(struct power_supply *psy,
> +					    const struct power_supply_ext *ext,
> +					    void *data,
> +					    enum power_supply_property psp)
>  {
> -	struct cros_chctl_priv *priv = cros_chctl_attr_to_priv(attr, n);
> +	return true;
> +}
>  
> -	if (priv->cmd_version < 2) {
> -		if (n == CROS_CHCTL_ATTR_START_THRESHOLD)
> -			return 0;
> -		if (n == CROS_CHCTL_ATTR_END_THRESHOLD)
> -			return 0;
> -	}
> +static const struct power_supply_ext cros_chctl_psy_ext = {
> +	.properties = cros_chctl_psy_ext_props,
> +	.num_properties = ARRAY_SIZE(cros_chctl_psy_ext_props),
> +	.charge_behaviours = EC_CHARGE_CONTROL_BEHAVIOURS,
> +	.get_property = cros_chctl_psy_ext_get_prop,
> +	.set_property = cros_chctl_psy_ext_set_prop,
> +	.property_is_writeable = cros_chctl_psy_prop_is_writeable,
> +};
>  
> -	return attr->mode;
> -}
> +static const struct power_supply_ext cros_chctl_psy_ext_v1 = {
> +	.properties = cros_chctl_psy_ext_props,
> +	.num_properties = 1,
> +	.charge_behaviours = EC_CHARGE_CONTROL_BEHAVIOURS,
> +	.get_property = cros_chctl_psy_ext_get_prop,
> +	.set_property = cros_chctl_psy_ext_set_prop,
> +	.property_is_writeable = cros_chctl_psy_prop_is_writeable,
> +};
>  
>  static int cros_chctl_add_battery(struct power_supply *battery, struct acpi_battery_hook *hook)
>  {
> @@ -241,7 +208,7 @@ static int cros_chctl_add_battery(struct power_supply *battery, struct acpi_batt
>  		return 0;
>  
>  	priv->hooked_battery = battery;
> -	return device_add_group(&battery->dev, &priv->group);
> +	return power_supply_register_extension(battery, priv->psy_ext, priv);
>  }
>  
>  static int cros_chctl_remove_battery(struct power_supply *battery, struct acpi_battery_hook *hook)
> @@ -249,7 +216,7 @@ static int cros_chctl_remove_battery(struct power_supply *battery, struct acpi_b
>  	struct cros_chctl_priv *priv = container_of(hook, struct cros_chctl_priv, battery_hook);
>  
>  	if (priv->hooked_battery == battery) {
> -		device_remove_group(&battery->dev, &priv->group);
> +		power_supply_unregister_extension(battery, priv->psy_ext);
>  		priv->hooked_battery = NULL;
>  	}
>  
> @@ -275,7 +242,6 @@ static int cros_chctl_probe(struct platform_device *pdev)
>  	struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent);
>  	struct cros_ec_device *cros_ec = ec_dev->ec_dev;
>  	struct cros_chctl_priv *priv;
> -	size_t i;
>  	int ret;
>  
>  	ret = cros_chctl_fwk_charge_control_versions(cros_ec);
> @@ -305,18 +271,11 @@ static int cros_chctl_probe(struct platform_device *pdev)
>  	dev_dbg(dev, "Command version: %u\n", (unsigned int)priv->cmd_version);
>  
>  	priv->cros_ec = cros_ec;
> -	priv->device_attrs[CROS_CHCTL_ATTR_START_THRESHOLD] =
> -		(struct device_attribute)__ATTR_RW(charge_control_start_threshold);
> -	priv->device_attrs[CROS_CHCTL_ATTR_END_THRESHOLD] =
> -		(struct device_attribute)__ATTR_RW(charge_control_end_threshold);
> -	priv->device_attrs[CROS_CHCTL_ATTR_CHARGE_BEHAVIOUR] =
> -		(struct device_attribute)__ATTR_RW(charge_behaviour);
> -	for (i = 0; i < _CROS_CHCTL_ATTR_COUNT; i++) {
> -		sysfs_attr_init(&priv->device_attrs[i].attr);
> -		priv->attributes[i] = &priv->device_attrs[i].attr;
> -	}
> -	priv->group.is_visible = cros_chtl_attr_is_visible;
> -	priv->group.attrs = priv->attributes;
> +
> +	if (priv->cmd_version == 1)
> +		priv->psy_ext = &cros_chctl_psy_ext_v1;
> +	else
> +		priv->psy_ext = &cros_chctl_psy_ext;
>  
>  	priv->battery_hook.name = dev_name(dev);
>  	priv->battery_hook.add_battery = cros_chctl_add_battery;
> 
> -- 
> 2.47.1
> 
>
diff mbox series

Patch

diff --git a/drivers/power/supply/cros_charge-control.c b/drivers/power/supply/cros_charge-control.c
index 17c53591ce197d08d97c94d3d4359a282026dd7d..7f914a65f88f8ea57bf01c334d25d9b8ebaa2de7 100644
--- a/drivers/power/supply/cros_charge-control.c
+++ b/drivers/power/supply/cros_charge-control.c
@@ -18,13 +18,6 @@ 
 					 BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE)   | \
 					 BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_FORCE_DISCHARGE))
 
-enum CROS_CHCTL_ATTR {
-	CROS_CHCTL_ATTR_START_THRESHOLD,
-	CROS_CHCTL_ATTR_END_THRESHOLD,
-	CROS_CHCTL_ATTR_CHARGE_BEHAVIOUR,
-	_CROS_CHCTL_ATTR_COUNT
-};
-
 /*
  * Semantics of data *returned* from the EC API and Linux sysfs differ
  * slightly, also the v1 API can not return any data.
@@ -41,13 +34,7 @@  struct cros_chctl_priv {
 	struct power_supply *hooked_battery;
 	u8 cmd_version;
 
-	/* The callbacks need to access this priv structure.
-	 * As neither the struct device nor power_supply are under the drivers
-	 * control, embed the attributes within priv to use with container_of().
-	 */
-	struct device_attribute device_attrs[_CROS_CHCTL_ATTR_COUNT];
-	struct attribute *attributes[_CROS_CHCTL_ATTR_COUNT];
-	struct attribute_group group;
+	const struct power_supply_ext *psy_ext;
 
 	enum power_supply_charge_behaviour current_behaviour;
 	u8 current_start_threshold, current_end_threshold;
@@ -114,124 +101,104 @@  static int cros_chctl_configure_ec(struct cros_chctl_priv *priv)
 	return cros_chctl_send_charge_control_cmd(priv->cros_ec, priv->cmd_version, &req);
 }
 
-static struct cros_chctl_priv *cros_chctl_attr_to_priv(struct attribute *attr,
-						       enum CROS_CHCTL_ATTR idx)
-{
-	struct device_attribute *dev_attr = container_of(attr, struct device_attribute, attr);
-
-	return container_of(dev_attr, struct cros_chctl_priv, device_attrs[idx]);
-}
+static const enum power_supply_property cros_chctl_psy_ext_props[] = {
+	POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR, /* has to be first for cros_chctl_psy_ext_v1 */
+	POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD,
+	POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD,
+};
 
-static ssize_t cros_chctl_store_threshold(struct device *dev, struct cros_chctl_priv *priv,
-					  int is_end_threshold, const char *buf, size_t count)
+static int cros_chctl_psy_ext_get_prop(struct power_supply *psy,
+				       const struct power_supply_ext *ext,
+				       void *data,
+				       enum power_supply_property psp,
+				       union power_supply_propval *val)
 {
-	int ret, val;
+	struct cros_chctl_priv *priv = data;
 
-	ret = kstrtoint(buf, 10, &val);
-	if (ret < 0)
-		return ret;
-	if (val < 0 || val > 100)
+	switch (psp) {
+	case POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD:
+		val->intval = priv->current_start_threshold;
+		return 0;
+	case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD:
+		val->intval = priv->current_end_threshold;
+		return 0;
+	case POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR:
+		val->intval = priv->current_behaviour;
+		return 0;
+	default:
 		return -EINVAL;
-
-	if (is_end_threshold) {
-		if (val <= priv->current_start_threshold)
-			return -EINVAL;
-		priv->current_end_threshold = val;
-	} else {
-		if (val >= priv->current_end_threshold)
-			return -EINVAL;
-		priv->current_start_threshold = val;
 	}
-
-	if (priv->current_behaviour == POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO) {
-		ret = cros_chctl_configure_ec(priv);
-		if (ret < 0)
-			return ret;
-	}
-
-	return count;
-}
-
-static ssize_t charge_control_start_threshold_show(struct device *dev,
-						   struct device_attribute *attr,
-						   char *buf)
-{
-	struct cros_chctl_priv *priv = cros_chctl_attr_to_priv(&attr->attr,
-							       CROS_CHCTL_ATTR_START_THRESHOLD);
-
-	return sysfs_emit(buf, "%u\n", (unsigned int)priv->current_start_threshold);
-}
-
-static ssize_t charge_control_start_threshold_store(struct device *dev,
-						    struct device_attribute *attr,
-						    const char *buf, size_t count)
-{
-	struct cros_chctl_priv *priv = cros_chctl_attr_to_priv(&attr->attr,
-							       CROS_CHCTL_ATTR_START_THRESHOLD);
-
-	return cros_chctl_store_threshold(dev, priv, 0, buf, count);
-}
-
-static ssize_t charge_control_end_threshold_show(struct device *dev, struct device_attribute *attr,
-						 char *buf)
-{
-	struct cros_chctl_priv *priv = cros_chctl_attr_to_priv(&attr->attr,
-							       CROS_CHCTL_ATTR_END_THRESHOLD);
-
-	return sysfs_emit(buf, "%u\n", (unsigned int)priv->current_end_threshold);
-}
-
-static ssize_t charge_control_end_threshold_store(struct device *dev, struct device_attribute *attr,
-						  const char *buf, size_t count)
-{
-	struct cros_chctl_priv *priv = cros_chctl_attr_to_priv(&attr->attr,
-							       CROS_CHCTL_ATTR_END_THRESHOLD);
-
-	return cros_chctl_store_threshold(dev, priv, 1, buf, count);
 }
 
-static ssize_t charge_behaviour_show(struct device *dev, struct device_attribute *attr, char *buf)
-{
-	struct cros_chctl_priv *priv = cros_chctl_attr_to_priv(&attr->attr,
-							       CROS_CHCTL_ATTR_CHARGE_BEHAVIOUR);
-
-	return power_supply_charge_behaviour_show(dev, EC_CHARGE_CONTROL_BEHAVIOURS,
-						  priv->current_behaviour, buf);
-}
 
-static ssize_t charge_behaviour_store(struct device *dev, struct device_attribute *attr,
-				      const char *buf, size_t count)
+static int cros_chctl_psy_ext_set_prop(struct power_supply *psy,
+				       const struct power_supply_ext *ext,
+				       void *data,
+				       enum power_supply_property psp,
+				       const union power_supply_propval *val)
 {
-	struct cros_chctl_priv *priv = cros_chctl_attr_to_priv(&attr->attr,
-							       CROS_CHCTL_ATTR_CHARGE_BEHAVIOUR);
+	struct cros_chctl_priv *priv = data;
 	int ret;
 
-	ret = power_supply_charge_behaviour_parse(EC_CHARGE_CONTROL_BEHAVIOURS, buf);
-	if (ret < 0)
-		return ret;
-
-	priv->current_behaviour = ret;
+	switch (psp) {
+	case POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD:
+	case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD:
+		if (val->intval < 0 || val->intval > 100)
+			return -EINVAL;
 
-	ret = cros_chctl_configure_ec(priv);
-	if (ret < 0)
-		return ret;
+		if (psp == POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD) {
+			if (val->intval <= priv->current_start_threshold)
+				return -EINVAL;
+			priv->current_end_threshold = val->intval;
+		} else {
+			if (val->intval >= priv->current_end_threshold)
+				return -EINVAL;
+			priv->current_start_threshold = val->intval;
+		}
+
+		if (priv->current_behaviour == POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO) {
+			ret = cros_chctl_configure_ec(priv);
+			if (ret < 0)
+				return ret;
+		}
 
-	return count;
+		return 0;
+	case POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR:
+		priv->current_behaviour = val->intval;
+		ret = cros_chctl_configure_ec(priv);
+		if (ret < 0)
+			return ret;
+		return 0;
+	default:
+		return -EINVAL;
+	}
 }
 
-static umode_t cros_chtl_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n)
+static int cros_chctl_psy_prop_is_writeable(struct power_supply *psy,
+					    const struct power_supply_ext *ext,
+					    void *data,
+					    enum power_supply_property psp)
 {
-	struct cros_chctl_priv *priv = cros_chctl_attr_to_priv(attr, n);
+	return true;
+}
 
-	if (priv->cmd_version < 2) {
-		if (n == CROS_CHCTL_ATTR_START_THRESHOLD)
-			return 0;
-		if (n == CROS_CHCTL_ATTR_END_THRESHOLD)
-			return 0;
-	}
+static const struct power_supply_ext cros_chctl_psy_ext = {
+	.properties = cros_chctl_psy_ext_props,
+	.num_properties = ARRAY_SIZE(cros_chctl_psy_ext_props),
+	.charge_behaviours = EC_CHARGE_CONTROL_BEHAVIOURS,
+	.get_property = cros_chctl_psy_ext_get_prop,
+	.set_property = cros_chctl_psy_ext_set_prop,
+	.property_is_writeable = cros_chctl_psy_prop_is_writeable,
+};
 
-	return attr->mode;
-}
+static const struct power_supply_ext cros_chctl_psy_ext_v1 = {
+	.properties = cros_chctl_psy_ext_props,
+	.num_properties = 1,
+	.charge_behaviours = EC_CHARGE_CONTROL_BEHAVIOURS,
+	.get_property = cros_chctl_psy_ext_get_prop,
+	.set_property = cros_chctl_psy_ext_set_prop,
+	.property_is_writeable = cros_chctl_psy_prop_is_writeable,
+};
 
 static int cros_chctl_add_battery(struct power_supply *battery, struct acpi_battery_hook *hook)
 {
@@ -241,7 +208,7 @@  static int cros_chctl_add_battery(struct power_supply *battery, struct acpi_batt
 		return 0;
 
 	priv->hooked_battery = battery;
-	return device_add_group(&battery->dev, &priv->group);
+	return power_supply_register_extension(battery, priv->psy_ext, priv);
 }
 
 static int cros_chctl_remove_battery(struct power_supply *battery, struct acpi_battery_hook *hook)
@@ -249,7 +216,7 @@  static int cros_chctl_remove_battery(struct power_supply *battery, struct acpi_b
 	struct cros_chctl_priv *priv = container_of(hook, struct cros_chctl_priv, battery_hook);
 
 	if (priv->hooked_battery == battery) {
-		device_remove_group(&battery->dev, &priv->group);
+		power_supply_unregister_extension(battery, priv->psy_ext);
 		priv->hooked_battery = NULL;
 	}
 
@@ -275,7 +242,6 @@  static int cros_chctl_probe(struct platform_device *pdev)
 	struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent);
 	struct cros_ec_device *cros_ec = ec_dev->ec_dev;
 	struct cros_chctl_priv *priv;
-	size_t i;
 	int ret;
 
 	ret = cros_chctl_fwk_charge_control_versions(cros_ec);
@@ -305,18 +271,11 @@  static int cros_chctl_probe(struct platform_device *pdev)
 	dev_dbg(dev, "Command version: %u\n", (unsigned int)priv->cmd_version);
 
 	priv->cros_ec = cros_ec;
-	priv->device_attrs[CROS_CHCTL_ATTR_START_THRESHOLD] =
-		(struct device_attribute)__ATTR_RW(charge_control_start_threshold);
-	priv->device_attrs[CROS_CHCTL_ATTR_END_THRESHOLD] =
-		(struct device_attribute)__ATTR_RW(charge_control_end_threshold);
-	priv->device_attrs[CROS_CHCTL_ATTR_CHARGE_BEHAVIOUR] =
-		(struct device_attribute)__ATTR_RW(charge_behaviour);
-	for (i = 0; i < _CROS_CHCTL_ATTR_COUNT; i++) {
-		sysfs_attr_init(&priv->device_attrs[i].attr);
-		priv->attributes[i] = &priv->device_attrs[i].attr;
-	}
-	priv->group.is_visible = cros_chtl_attr_is_visible;
-	priv->group.attrs = priv->attributes;
+
+	if (priv->cmd_version == 1)
+		priv->psy_ext = &cros_chctl_psy_ext_v1;
+	else
+		priv->psy_ext = &cros_chctl_psy_ext;
 
 	priv->battery_hook.name = dev_name(dev);
 	priv->battery_hook.add_battery = cros_chctl_add_battery;