linux-kernel - Re: [PATCH] power/ds2760_battery: Add rated capacity of the hx4700 3600mAh battery

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]

Message-ID: <20111125235613.GA21434@oksana.dev.rtsoft.ru>
Date:	Sat, 26 Nov 2011 03:56:13 +0400
From:	Anton Vorontsov <cbouatmailru@...il.com>
To:	Paul Parsons <lost.distance@...oo.com>
Cc:	Philipp Zabel <philipp.zabel@...il.com>,
	linux-kernel@...r.kernel.org, mad_soft@...ox.ru,
	koen@...inion.thruhere.net
Subject: Re: [PATCH] power/ds2760_battery: Add rated capacity of the hx4700
 3600mAh battery

On Thu, Nov 24, 2011 at 11:52:56PM +0000, Paul Parsons wrote:
> --- On Thu, 24/11/11, Philipp Zabel <philipp.zabel@...il.com> wrote:
> > > +#ifdef CONFIG_MACH_H4700
> > > +       0,
> > > +       3600,   /* HP iPAQ hx4700 3.7V 3600mAh
> > (359114-001) */
> > > +#endif
> > 
> > Is that #ifdef needed? I know there's another one a few
> > lines up, but
> > I think that one is already unfortunate and there is no
> > conflict here.
> 
> Yes it is needed, because the rated_capacities[] table must
> contain different values for different platforms. The #ifdefs
> preserve the original table for other platforms.
> 
> Agreed it's untidy. Ideally the table would be replaced by
> platform specific data. However it's not obvious which other
> platforms use the table, nor whether the original table is
> correct for those other platforms. I presume that the table
> was originally believed to be common for all ds2760 based
> platforms, but the hx4700 demonstrates this is not the case.

Well, the really bad thing is that ds2760 is on the hot-pluggable
bus, so basically there is no such thing as 'platform_data'. You
can basically attach any battery, with any value in the
rated_capacity 'register'. :-(

The only thing we can do is some machine-specific fixup. The
universal way is ds2760_battery.rate_capacity module (and kernel
command line) parameter that you can use to fixup the value.

But, having the MACH_HX4700 fixup is just a heuristic that
helps the driver to work on the particular machine 'out of
the box'.

The really bad thing about that patch is that it uses build-time
way for the heuristic. We should really call something like
machine_is(). But that would be architecture-specific today
(i.e. will only work on ARM). Heh.

So... I'm applying the patch as it is not a big deal, and we
have a similar machine fixup already.

Thanks!

-- 
Anton Vorontsov
Email: cbouatmailru@...il.com
--
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/