LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Tony Lindgren <email@example.com>
To: "Dr. H. Nikolaus Schaller" <firstname.lastname@example.org>
Cc: Marek Belisko <email@example.com>,
Benoit Cousson <firstname.lastname@example.org>,
Sebastian Reichel <email@example.com>,
Dmitry Eremin-Solenikov <firstname.lastname@example.org>,
David Woodhouse <email@example.com>,
Rob Herring <firstname.lastname@example.org>, Pawel Moll <email@example.com>,
Mark Rutland <firstname.lastname@example.org>,
Ian Campbell <email@example.com>,
Kumar Gala <firstname.lastname@example.org>,
email@example.com, LKML <firstname.lastname@example.org>,
Subject: Re: [PATCH v4 3/6] Documentation: DT: Document twl4030-madc-battery bindings
Date: Wed, 11 Mar 2015 10:43:17 -0700 [thread overview]
Message-ID: <20150311174316.GD5264@atomide.com> (raw)
* Dr. H. Nikolaus Schaller <email@example.com> [150311 10:13]:
> Am 11.03.2015 um 17:44 schrieb Tony Lindgren <firstname.lastname@example.org>:
> > * Dr. H. Nikolaus Schaller <email@example.com> [150311 09:17]:
> >> Am 11.03.2015 um 16:24 schrieb Tony Lindgren <firstname.lastname@example.org>:
> >>> Rather than just making platform_data into device tree properties..
> >>> Can't you hide the these custom properties behind the compatible flag?
> >>> You can initialize that data in the driver based on the compatible
> >>> flag and the match data.
> >>> This makes sense if you can group things to similar configurations.
> >> Maybe I have not completely understood your proposal.
> >> Do you mean to go back to have big parameter tables for each device/battery
> >> combination in the driver code and the compatible flag (e.g. compatible = “board17”)
> >> chooses the right data set for the charging map and channels?
> > If you can somehow group them, then yes.
> I don’t see how to group them. Could you make a proposal?
Sorry no idea about that :) I though you may have some ideas based on
dealing with the driver.
> >> I thought this is what the DT was introduced for - to have the same driver
> >> code but adapt to different boards depending on hardware variations.
> > Yeah but you also need to consider the issues related to introducing
> > new device tree properties. The device tree properties introduced
> > should be generic where possible.
> Yes, that was discussed for a while for this driver’s bindings leading to v4.
> Which ones do you think are not generic enough?
It seems maps is the only one then, assuming the cpacity-uah can be made
> >> And batteries have very different characteristics and vary between devices…
> > Right. Maybe that has been already agreed on to use capacity-uah for
> > batteries in general?
> Ah, do you mean with generic/not generic the distinction between a “ti,” prefix
> and no prefix?
> Well, I don’t know if there is such an agreement and I would have no argument
> against calling it “ti,capacity-uah”.
No no, "capacity-uah" is what we should use, but you need an ack from
the battery and device tree people that this is OK. Let's not add
"ti,capacity-uah” as that can obviously be a generic property.
> > In that case I have not problem with that as
> > it’s a generic property :)
> Well, many batteries and systems have a fuel gauge chip (e.g. bq27000). This
> chip “knows” the capacity. But therefore it is not needed to specify
> it anywhere because it can be read out (usually in uAh).
> This driver is to solve the issue that there is no such factory-programmed
> battery or fuel gauge chip connected to a twl4030 driver. Nobody can program
> that capacity value - except someone matching the device tree with real hardware.
> And, by doing and averaging some charge-discharge cycles the fuel gauge
> mapping is calibrated.
> >> The charging maps are depending on the battery type connected to the twl4030
> >> and which madc channel is which value is also a little hardware dependent
> >> (although the twl4030 doesn’t give much choice).
> > Just to consider alternatives before introducing driver specific
> > property for the maps.. Maybe here you could have few different type
> > of maps and select something safe by default? Of course it could be this
> > is higly board specific, I think some devices may be able to run below
> > 3.3V for example..
> Every battery setup has a different map (which also might depend on the
> series resistance of the battery and the wiring).
> >> And moving this information into the driver for each board that uses it
> >> would blow up the code.
> > Right, I'm not saying we should build databases into the kernel drivers.
> > Just trying to avoid introducing driver specific properties unless
> > really needed :)
> They are not really driver specific, they are battery specific. They
> describe properties of the battery:
> * capacity - depends on battery
> * voltage depending on current charging level - depends on battery (and differs between charging and discharging)
> * wiring of MADC inputs to the twl4030 is board dependent.
> So all these properties are not driver specific, but describe hardware.
> And therefore they had been introduced exactly this way into the old
> platform_data driver.
> So if you want to see a “ti," prefix to the capacity property I would be
Oh if they are battery spicific, then ideally we'd have generic batery
voltage to capacity maps property rather than a custom ti specific
To avoid extra hassles later on, maybe you could submit a generic
binding patch only documenting it to the battery people and the device
tree people? That will make it easier to maintain this driver in the
next prev parent reply other threads:[~2015-03-11 17:48 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-10 21:27 [PATCH v4 0/6] Convert twl4030_madc_battery to IIO consumer and add DT aupport Marek Belisko
2015-03-10 21:27 ` [PATCH v4 1/6] power: twl4030-madc-battery: Convert to iio consumer Marek Belisko
2015-04-06 17:39 ` Sebastian Reichel
2015-03-10 21:27 ` [PATCH v4 2/6] power: twl4030_madc_battery: Add device tree support Marek Belisko
2015-03-10 21:27 ` [PATCH v4 3/6] Documentation: DT: Document twl4030-madc-battery bindings Marek Belisko
2015-03-11 15:24 ` Tony Lindgren
2015-03-11 16:16 ` Dr. H. Nikolaus Schaller
2015-03-11 16:44 ` Tony Lindgren
2015-03-11 17:13 ` Dr. H. Nikolaus Schaller
2015-03-11 17:43 ` Tony Lindgren [this message]
2015-03-11 19:36 ` Sebastian Reichel
2015-03-11 19:37 ` Tony Lindgren
2015-03-31 7:26 ` Pavel Machek
2015-04-01 8:18 ` Dr. H. Nikolaus Schaller
2015-04-01 20:16 ` Pavel Machek
2015-04-01 20:39 ` Dr. H. Nikolaus Schaller
2015-04-04 8:16 ` Pavel Machek
2015-04-04 9:46 ` Dr. H. Nikolaus Schaller
2015-04-01 16:30 ` Rob Herring
2015-04-01 16:46 ` Dr. H. Nikolaus Schaller
2015-03-10 21:27 ` [PATCH v4 4/6] ARM: dts: omap3-gta04: Add battery support Marek Belisko
2015-03-16 20:53 ` Tony Lindgren
2015-03-16 21:04 ` Tony Lindgren
2015-03-10 21:27 ` [PATCH v4 5/6] power: twl4030_madc_battery: Add of_twl4030_madc_match to MODULE_DEVICE_TABLE Marek Belisko
2015-03-10 21:27 ` [PATCH v4 6/6] power: twl4030_madc_battery: Add missing MODULE_ALIAS Marek Belisko
2015-04-06 17:45 ` Sebastian Reichel
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:
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--subject='Re: [PATCH v4 3/6] Documentation: DT: Document twl4030-madc-battery bindings' \
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).