All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Mack <daniel@caiaq.de>
To: Anton Vorontsov <cbouatmailru@gmail.com>
Cc: linux-kernel@vger.kernel.org, Matt Reimer <mreimer@vpop.net>,
	Evgeniy Polyakov <zbr@ioremap.net>, Tejun Heo <tj@kernel.org>,
	David Woodhouse <dwmw2@infradead.org>,
	Andres Salomon <dilinger@collabora.co.uk>,
	Alexey Starikovskiy <astarikovskiy@suse.de>,
	Len Brown <len.brown@intel.com>,
	Mark Brown <broonie@opensource.wolfsonmicro.com>
Subject: Re: [PATCH 2/3] power/ds2760_battery: make charge_now and charge_full writeable
Date: Tue, 11 May 2010 19:28:59 +0200	[thread overview]
Message-ID: <20100511172859.GE30801@buzzloop.caiaq.de> (raw)
In-Reply-To: <20100511172003.GA19937@oksana.dev.rtsoft.ru>

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

  reply	other threads:[~2010-05-11 17:29 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-11 16:38 [PATCH 1/3] pda_power: add support for writeable properties Daniel Mack
2010-05-11 16:38 ` [PATCH 2/3] power/ds2760_battery: make charge_now and charge_full writeable Daniel Mack
2010-05-11 16:44   ` Daniel Mack
2010-05-11 17:20   ` Anton Vorontsov
2010-05-11 17:28     ` Daniel Mack [this message]
2010-05-11 18:05       ` Anton Vorontsov
2010-05-18 18:24         ` Daniel Mack
2010-05-11 16:38 ` [PATCH 3/3] power/ds2760_battery: use factor of 20 for rated_capacity Daniel Mack
2010-05-11 17:19   ` Anton Vorontsov
2010-05-11 17:25     ` Daniel Mack
2010-05-11 17:47       ` Anton Vorontsov
2010-05-11 17:47 ` [PATCH 1/3] pda_power: add support for writeable properties Anton Vorontsov
2010-05-11 17:58   ` Daniel Mack
2010-05-11 18:23     ` Anton Vorontsov
2010-05-11 22:28       ` Daniel Mack
2010-05-12 18:15         ` [PATCH] Introduce {sysfs,device}_create_file_mode Anton Vorontsov
2010-05-12 18:18           ` Daniel Mack
2010-05-12 18:38           ` Greg KH
2010-05-12 19:08             ` Anton Vorontsov
2010-05-12 19:12               ` Kay Sievers
2010-05-12 19:19                 ` Greg KH
2010-05-12 19:39                   ` Anton Vorontsov
2010-05-12 19:30                 ` Anton Vorontsov
2010-05-13  9:33                   ` Daniel Mack
2010-05-17 19:40                     ` [PATCH] power_supply: Use attribute groups Anton Vorontsov
2010-05-18 17:35                       ` Daniel Mack
2010-05-18 19:49                       ` [PATCH 1/3] " Daniel Mack
2010-05-18 19:49                       ` [PATCH 2/3] pda_power: add support for writeable properties Daniel Mack
2010-05-18 19:56                         ` Greg KH
2010-05-18 20:30                           ` [PATCH] power/ds2760_battery: document ABI change Daniel Mack
2010-05-19  8:34                             ` Anton Vorontsov
2010-05-18 19:49                       ` [PATCH 3/3] power/ds2760_battery: make charge_now and charge_full writeable Daniel Mack
2010-05-11 18:32     ` [PATCH 1/3] pda_power: add support for writeable properties Anton Vorontsov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100511172859.GE30801@buzzloop.caiaq.de \
    --to=daniel@caiaq.de \
    --cc=astarikovskiy@suse.de \
    --cc=broonie@opensource.wolfsonmicro.com \
    --cc=cbouatmailru@gmail.com \
    --cc=dilinger@collabora.co.uk \
    --cc=dwmw2@infradead.org \
    --cc=len.brown@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mreimer@vpop.net \
    --cc=tj@kernel.org \
    --cc=zbr@ioremap.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.