Re: [PATCH 2/3] power/ds2760_battery: make charge_now andcharge_full writeable

[Daniel Mack]

On Tue, May 11, 2010 at 09:20:03PM +0400, Anton Vorontsov wrote:
> 
> On Tue, May 11, 2010 at 06:38:45PM +0200, Daniel Mack wrote:
> [...]
> > +static int ds2760_battery_set_property(struct power_supply *psy,
> > +				       enum power_supply_property psp,
> > +				       const union power_supply_propval *val)
> > +{
> > +	int ret = -EPERM;
> > +	unsigned char tmp;
> > +	struct ds2760_device_info *di = to_ds2760_device_info(psy);
> > +
> > +	switch (psp) {
> > +	case POWER_SUPPLY_PROP_CHARGE_FULL:
> > +		/* the interface counts in uAh, convert the value */
> > +		ds2760_battery_write_rated_capacity(di, (val->intval / 1000L) /
> > +							RATED_CAPACITY_FACTOR);
> 
> OK, Mark hinted that you should not change rated (design)
> capacity, ever. You've made CHARGE_FULL writable, which now
> actually changes CHARGE_FULL_DESIGN (which reports
> rated_capacity). Obviously, this isn't acceptable.

Of course, sorry.

> I understand that you might want to change rated capacity for
> your custom battery setups, and that's fine. After all, it's
> your hardware, and you have a right to break it. :-)

Well, if it was a hack purely for my hardware, I wouldn't share it beat
it upstream ;) I believe that this feature is something all users of
battery-driven devices need, as all batteries age.

> But to do so,
> 
> 1. Could you at least change this back to
>    POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN, so it'll be clear what
>    exactly this prop is actually changing, and

Do you see any other way to store the actual rated capacity (not the one
the battery had in its good days) inside the chip?

> 2. Can we introduce Kconfig symbol BATTERY_DS2760_UNSAFE_OPS,
>    and put an #ifdefs for this property? It should be
>    'default n', of course. Plus we can #ifdef the rated_capacity
>    module param as well.

Hmm, I see that this is one way. I can do this, but I'm hessitant and
would rather go a cleaer way ...


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/