linux-kernel - Re: [PATCH 2/3] power/ds2760_battery: make charge_now and charge

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 11 May 2010 19:28:59 +0200
From:	Daniel Mack <daniel@...aq.de>
To:	Anton Vorontsov <cbouatmailru@...il.com>
Cc:	linux-kernel@...r.kernel.org, Matt Reimer <mreimer@...p.net>,
	Evgeniy Polyakov <zbr@...emap.net>, Tejun Heo <tj@...nel.org>,
	David Woodhouse <dwmw2@...radead.org>,
	Andres Salomon <dilinger@...labora.co.uk>,
	Alexey Starikovskiy <astarikovskiy@...e.de>,
	Len Brown <len.brown@...el.com>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>
Subject: Re: [PATCH 2/3] power/ds2760_battery: make charge_now and
 charge_full writeable

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/