On Tue, Dec 18, 2007 at 02:10:01AM -0500, Andres Salomon wrote:
> On Mon, 17 Dec 2007 14:24:16 +0300
> Anton Vorontsov <[email protected]> wrote:
>
> > On Mon, Dec 17, 2007 at 02:41:39AM -0500, Andres Salomon wrote:
> > [...]
> > > > > On Sun, 2007-12-16 at 21:24 -0500, Andres Salomon wrote:
> > > > > > This API has the power_supply drivers device their own device_attribute
> > > > > > list; I find this to be a lot more flexible and cleaner.
> > > >
> > > > I don't see how this is more flexible and cleaner. See below.
> > > >
> > > > > > For example,
> > > > > > rather than having a function with a huge switch statement (as olpc_battery
> > > > > > currently has), we have separate callback functions.
> > > >
> > > > Is this an improvement? Look into ds2760_battery.c. I scared to
> > > > imagine what it will look like after conversion.
> > >
> [...]
> >
> > I see your point now. Basically, now I'm encourage to think just one
> > more time: is there third (better) option in addition to current and
> > this? I still hope there is some not obvious, but elegant solution.
> > If there isn't, I'm ready to surrender and will help with everything
> > I can.
> >
>
> Hm. It occurs to me that there's nothing keeping us from having a
> single callback for the driver properties. Keeping the other patches
> the same, do you prefer the following approach versus what was originally
> in patch#3?
Why so difficult? Maybe like this:
diff --git a/drivers/power/olpc_battery.c b/drivers/power/olpc_battery.c
index c998e68..00f0b71 100644
--- a/drivers/power/olpc_battery.c
+++ b/drivers/power/olpc_battery.c
@@ -176,13 +176,13 @@ static int olpc_bat_get_property(struct power_supply *psy,
switch (ec_byte >> 4) {
case 1:
- val->strval = "Gold Peak";
+ ret = sprintf(val->strval, "%s\n", "Gold Peak");
break;
case 2:
- val->strval = "BYD";
+ ret = sprintf(val->strval, "%s\n", "BYD");
break;
default:
- val->strval = "Unknown";
+ ret = sprintf(val->strval, "%s\n", "Unknown");
break;
}
break;
diff --git a/drivers/power/power_supply_sysfs.c b/drivers/power/power_supply_sysfs.c
index 249f61b..83e127d 100644
--- a/drivers/power/power_supply_sysfs.c
+++ b/drivers/power/power_supply_sysfs.c
@@ -54,7 +54,9 @@ static ssize_t power_supply_show_property(struct device *dev,
ssize_t ret;
struct power_supply *psy = dev_get_drvdata(dev);
const ptrdiff_t off = attr - power_supply_attrs;
- union power_supply_propval value;
+ union power_supply_propval value = {
+ .strval = buf,
+ };
ret = psy->get_property(psy, off, &value);
@@ -75,7 +77,7 @@ static ssize_t power_supply_show_property(struct device *dev,
return sprintf(buf, "%s\n",
capacity_level_text[value.intval]);
else if (off >= POWER_SUPPLY_PROP_MODEL_NAME)
- return sprintf(buf, "%s\n", value.strval);
+ return ret;
return sprintf(buf, "%d\n", value.intval);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
