* Dr. H. Nikolaus Schaller <hns@xxxxxxxxxxxxx> [150311 10:13]: > Am 11.03.2015 um 17:44 schrieb Tony Lindgren <tony@xxxxxxxxxxx>: > > * Dr. H. Nikolaus Schaller <hns@xxxxxxxxxxxxx> [150311 09:17]: > >> Am 11.03.2015 um 16:24 schrieb Tony Lindgren <tony@xxxxxxxxxxx>: > >>> > >>> 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 Linux generic. > >> 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. OK > >> 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 > fine. Oh if they are battery spicific, then ideally we'd have generic batery voltage to capacity maps property rather than a custom ti specific property. 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 long run. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html