On Tuesday, 16 of December 2008, Henrique de Moraes Holschuh wrote: > On Tue, 16 Dec 2008, Alexey Starikovskiy wrote: > > Henrique de Moraes Holschuh wrote: > >> On Tue, 16 Dec 2008, Alexey Starikovskiy wrote: > >>> @@ -224,10 +224,12 @@ static int acpi_sbs_battery_get_property(struct power_supply *psy, > >>> acpi_battery_vscale(battery) * 1000; > >>> break; > >>> case POWER_SUPPLY_PROP_CURRENT_NOW: > >>> + case POWER_SUPPLY_PROP_POWER_NOW: > >>> val->intval = abs(battery->current_now) * > >>> acpi_battery_ipscale(battery) * 1000; > >>> break; > >>> case POWER_SUPPLY_PROP_CURRENT_AVG: > >>> + case POWER_SUPPLY_PROP_POWER_AVG: > >>> val->intval = abs(battery->current_avg) * > >>> acpi_battery_ipscale(battery) * 1000; > >>> break; > >> > >> Excuse me if I am talking nonsense (I have looked over just the patch, > >> not the entire file), but how can that be correct? It is either power > >> or current, it cannot be both, so the CURRENT case should be dropped. > > file name under /sys depends on property, so if we want some variable to > > be named as current_now, it should be returned by case POWER_SUPPLY_PROP_CURRENT_NOW. > > Then we register with power_supply, we say which set of properties (either charge or energy) we support, > > so for one battery we will receive request for either CURRENT_AVG or POWER_AVG, not both. > >> And if it is power, why have fields named current_now... or is > >> ipscale() a voltage, and not a scaling factor? > > ipscale stands for I/P scaling, as opposed to V scaling -- it depends on units returned by actual battery. > > All energy/charge fields are reused, so battery->current_now contains either power_now or current_now from battery. > > I see. But that's a loaded-spring trap waiting for the unaware. Can it be > called something else that is neutral re. power or current, pretty please? > even "current_or_power_now" would be less confusing... As I said in my recent reply to Alex, I'd prefer it if there were separate fields called 'power_now' and 'power_avg' under 'battery'. Then, there won't be any confustion and people reading the source will clearly understand what is what. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
