Re: [PATCH] power_supply: Use attribute groups

From: Daniel Mack
Date: Tue May 18 2010 - 13:35:29 EST


Hi Anton,

On Mon, May 17, 2010 at 11:40:16PM +0400, Anton Vorontsov wrote:
> This fixes a race between power supply device and initial
> attributes creation, plus makes it possible to implement
> writable properties.

Thanks a lot. This works for me, but I have two minor comments below ...

[...]

> diff --git a/drivers/power/power_supply_sysfs.c b/drivers/power/power_supply_sysfs.c
> index 5b6e352..f95d934 100644
> --- a/drivers/power/power_supply_sysfs.c
> +++ b/drivers/power/power_supply_sysfs.c
> @@ -41,6 +41,9 @@ static struct device_attribute power_supply_attrs[];
> static ssize_t power_supply_show_property(struct device *dev,
> struct device_attribute *attr,
> char *buf) {
> + static char *type_text[] = {
> + "Battery", "UPS", "Mains", "USB"
> + };
> static char *status_text[] = {
> "Unknown", "Charging", "Discharging", "Not charging", "Full"
> };
> @@ -58,12 +61,15 @@ static ssize_t power_supply_show_property(struct device *dev,
> static char *capacity_level_text[] = {
> "Unknown", "Critical", "Low", "Normal", "High", "Full"
> };
> - ssize_t ret;
> + ssize_t ret = 0;
> struct power_supply *psy = dev_get_drvdata(dev);
> const ptrdiff_t off = attr - power_supply_attrs;
> union power_supply_propval value;
>
> - ret = psy->get_property(psy, off, &value);
> + if (off == POWER_SUPPLY_PROP_TYPE)
> + value.intval = psy->type;
> + else
> + ret = psy->get_property(psy, off, &value);
>
> if (ret < 0) {
> if (ret == -ENODATA)
> @@ -85,10 +91,13 @@ static ssize_t power_supply_show_property(struct device *dev,
> return sprintf(buf, "%s\n", technology_text[value.intval]);
> else if (off == POWER_SUPPLY_PROP_CAPACITY_LEVEL)
> return sprintf(buf, "%s\n", capacity_level_text[value.intval]);
> + else if (off == POWER_SUPPLY_PROP_TYPE)
> + return sprintf(buf, "%s\n", type_text[value.intval]);
> else if (off >= POWER_SUPPLY_PROP_MODEL_NAME)
> return sprintf(buf, "%s\n", value.strval);
>
> return sprintf(buf, "%d\n", value.intval);
> +return 0;

This return is superflous :)

And with this approach, we don't need to initialize the .mode field in
the POWER_SUPPLY_ATTR anmore.

I'll then rebase my patches for writeable properties and resend them.

Thanks,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/