Messages in this thread Patch in this message | | | Date | Wed, 19 Dec 2007 15:35:46 +0300 | From | Anton Vorontsov <> | Subject | Re: [PATCH 1/5] power: RFC: introduce a new power API |
| |
On Tue, Dec 18, 2007 at 02:10:01AM -0500, Andres Salomon wrote: > On Mon, 17 Dec 2007 14:24:16 +0300 > Anton Vorontsov <cbou@mail.ru> 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); }
| |