On Thu, Sep 10, 2015 at 09:15:09AM +0900, Krzysztof Kozlowski wrote: > On 10.09.2015 05:15, Andreas Dannenberg wrote: > > On Wed, Sep 09, 2015 at 01:58:09PM +0900, Krzysztof Kozlowski wrote: > >> > >> So we have different bindings. Existing ones: > >> bq2415x.txt - ti,charge-current - maximum charging current in mA > >> bq24257.txt - ti,charge-current - maximum charging current in uA > >> bq25890.txt - ti,charge-current - maximum charging current in uA > >> bq24735.txt - ti,charge-current - charge current (?) in mA > > > > I just spent some time with the bq24735 datasheet and the way that > > device appears to work is putting the user in control of the charging > > process rather than implementing a fully automatic control loop, but > > either way I still think it's valid to call the property > > ti,charge-current and use a description of "maximum charging current" > > for that device (there is no DT bindings doc however). And yes the unit > > is mA as one can see from reverse-engineering the register settings. > > Just for the record: the units used by device registers are not related > to units used in DT binding. You can use whatever you want. The driver > should just convert them. > > > > > On a related note the datasheet for that device says you have to > > periodically re-send the charge current setting every 44...175s to keep > > charging with the configured current or disable the device's watchdog > > timer. Neither of which the driver seems to do. I can probably go back > > and get some HW and test if that driver actually works as advertised. > > (also see http://www.ti.com/lit/ds/symlink/bq24735.pdf, Page 21) > > I mentioned existing bindings for reference. To show that it's a mess. > However you cannot change them now (at least easily). You could add new > bindings and mark old as deprecated (still they would have to be > supported by the driver)... > > > > >> bq2415x.txt - ti,current-limit - maximum current to be drawn in mA > >> > >> New bindings: > >> ti,charge-current (bq24261) - default charge current in mA > >> ti,max-charge-current (bq24261) - maximum charging current in mA > > > > This ti,max-charge-current is actually an interesting one. It's not a > > device setting as it does not impact any of the device registers at all. > > Instead, it's an artificial limit that can be set through DT that > > prevents somebody from going into sysfs and configuring a charge current > > higher than ti,max-charge-current. In other drivers I have seen that the > > sysfs property reflecting that max charge current is just read-only and > > gives you the maximum the HW is capable of. From a device point of view > > there is nothing configurable about this property. > > Hmmm, so now I wonder whether this should be a DT binding. The purpose > of DT isn't the control of the driver like enable some stuff, set some > value used by the driver. The DT provides information about hardware so > the driver could properly configure the device and work with it. > > Reason to put this into DT would be: > For example one device configuration (device, board, connected battery) > could handle one maximum value and the other configuration would require > lower one. The device and its capabilities (bq24257 for example) are the > same but configuration changes. > > > If all configurations of bq24261 device have the same maximum value then > it should not be a DT property but it should be hard-coded in the driver > instead. > > Ramakrishna, > Could you describe the reason behind "ti,max-charge-current" and > "pdata.max_cc" in bq24261 driver? > > > > >> ti,current-limit (bq2425x) - maximum current to be drawn in uA > >> > >> > >> Damn it... It's a mess. And there is no device prefixes... > > > > ACK :) Let's see how we can bring a little sense into it. > > > >> The bq24261's bindings look most sensible (max prefix for max charge > >> current) but they are not compatible with existing bindings for > >> different devices. > > > > Hmm I think they are compatible, it's just a question making the DT > > bindings description for the bq24261 fit better into what's already > > there. For example, like this: > > > > (1) ti,charge-current: integer, maximum charging current in mA. For this > > device as for others this setting controls the max current the device > > uses to charge the battery so the established description is good > > however the general use of this property name itself is not 100% > > accurate (too late for that). > > I agree. > > > (2) ti,max-charge-current: Unless there is a good reason to keep it, > > REMOVE this property (alongside ti,max-charge-voltage). Instead, have > > the charger directly report back the maximum device charge current > > (BQ24261_MAX_CC) via sysfs like most other charger drivers do (bq24190, > > bq24257, bq25890, rt9455). > > I agree but wait for some more data from Ramakrishna. > > > > >> There is no way to unify or make consistent all of these bindings. > >> However one could try to add new stuff in a more sensible way. For > >> example how about (for bq2425x-charger and bq24261_charger... BTW notice > >> even the difference in using underscore and hyphen!): > > > > :( You are talking about the driver .name, right? I saw that too. If > > the bq24261 charger was to change it's device name to use a hyphen at > > least it would be consistent with bq2415x and bq2425x. > > The name of file and name of driver. Most of existing bq* drivers have > "_" in file name and "-" in driver name. So maybe use it in > bq2425x-charger (file name: bq2425x_charger)? > > To prevent future issues in naming and bindings consistency maybe there > should be a dedicated maintainer for bq/TI charger devices? Pali Rohar > recently took care for devices related to Nokia N900 but maybe someone > from TI should also take care of rest or all of them (if TI is > interested in this)? > > > > >> ti,charge-current - maximum charging current in uA > >> (that one must be supported, it's for existing bq24257 devices) > > > > Agreed. As discussed earlier this one is pretty established -- but in > > mA (not uA). > > Okay. > > > > >> ti,default-charge-current (bq24261) - default charge current in *uA* > > > > We don't need that. If you look where it goes (registers) this should be > > called ti,charge-current for the bq24261 (like it already is). Exactly > > the same name/usage as the other bqxxxx chargers. We just need to update > > the description. > > Okay. > > > > >> ti,current-limit (bq2425x) - maximum current to be drawn in *uA* > > > > Ok. Again here my preference would be mA. Like already on the bq2415x. > > If we can change the bq2425x driver to mA (see separate thread) we'd be > > closer to a more unified set of properties. Otherwise we would have > > properties with the same name but different units (is this even > > possible?). > > mA would be nice... but bq2425x driver must support existing device > which means it must support existing bindings. Unfortunately the > existing binding for bq24257 is in uA. Oh boy... apparently this unit discrepancy mess for TI chargers was my doing. :/ I replied on the other thread already on my bindings choice... As I said there, all we can do now is agree on the binding names to be consistent. I'm afraid the units (as Krzysztof pointed) are pretty much settled... :| laurentiu -- 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
