On Mon, Sep 07, 2015 at 12:57:56PM +0900, Krzysztof Kozlowski wrote: > 2015-09-07 2:23 GMT+09:00 Ramakrishna Pallala <ramakrishna.pallala@xxxxxxxxx>: > > > > Add new charger driver support for BQ24261 charger IC. > > > > BQ24261 charger driver relies on extcon notifications to get the > > charger cable type and based on that it will set the charging parameters. > > > > Signed-off-by: Ramakrishna Pallala <ramakrishna.pallala@xxxxxxxxx> > > Signed-off-by: Jennt TC <jenny.tc@xxxxxxxxx> > > --- > > .../devicetree/bindings/power/bq24261.txt | 37 + > > drivers/power/Kconfig | 6 + > > drivers/power/Makefile | 1 + > > drivers/power/bq24261_charger.c | 1208 ++++++++++++++++++++ > > include/linux/power/bq24261_charger.h | 27 + > > 5 files changed, 1279 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/power/bq24261.txt > > create mode 100644 drivers/power/bq24261_charger.c > > create mode 100644 include/linux/power/bq24261_charger.h > > > > diff --git a/Documentation/devicetree/bindings/power/bq24261.txt b/Documentation/devicetree/bindings/power/bq24261.txt > > new file mode 100644 > > index 0000000..25fc5c4 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/power/bq24261.txt > > @@ -0,0 +1,37 @@ > > +Binding for TI bq24261 Li-Ion Charger > > Please split the bindings into separate patch (the first patch in patchset). > > > + > > +Required properties: > > +- compatible: Should contain one of the following: > > + * "ti,bq24261" > > +- reg: integer, i2c address of the device. > > +- ti,charge-current: integer, default charging current (in mA); > > +- ti,charge-voltage: integer, default charging voltage (in mV); > > +- ti,termination-current: integer, charge will be terminated when current in > > + constant-voltage phase drops below this value (in mA); > > +- ti,max-charge-current: integer, maximum charging current (in mA); > > +- ti,max-charge-voltage: integer, maximum charging voltage (in mV); > > +- ti,min-charge-temperature: integer, minimum charging temperature (in DegC); > > +- ti,max-charge-temperature: integer, maximum charging temperature (in DegC). > > Before accepting "[PATCH 13/13] dt: power: bq24257-charger: Cover > additional devices" > http://www.spinics.net/lists/devicetree/msg92134.html > > could you and Andreas figure out common bindings? Look at this: > > +- ti,charge-current: integer, maximum charging current in uA. > +- ti,charge-current: integer, default charging current (in mA); > > Different meaning and different units. This is madness! :) > > In the same time you are adding TI-common bindings (not device > specific, there is no prefix) so I would expect exactly the same > bindings if it is possible. Krzysztof, good observation! In bq2425x_charger.c (formerly known as bq24257_charger.c :) that I worked on the unit used was uA. At that time I did a quick check and there didn't seem to be a clear standard whether to use the "micro" or "milli" units - different drivers use different units. However there seems to be a tendency for the TI drivers to prefer "milli" (bq2415x_charger.c, bq24735-charger.c) Personally I think "milli" units are more appropriate for chargers since they provide sufficient granularity and the numbers don't become too big (try typing a voltage in the Volt-range in uV, it's very easy to get the number of 0s wrong). However since the driver was already there I left that aspect alone to preserve compatibility. > > + > > +Optional properties: > > +- ti,thermal-sensing: boolean, if present thermal regulation will be enabled; > > What is the requirement for thermal-sensing? Can it be enabled always? > If yes, then this is not really a hardware property. > > > +- ti,enable-user-write: boolean, if present driver will allow the user space > > + to control the charging current and voltage through sysfs; > > This is not DT property. It does not describe hardware. I take responsibility for this one :) In an earlier thread we were discussing that it will be better to prevent sysfs write access to "dangerous" charger parameters (charge voltage, current, ...) but I argued that such access might still be desirable during development and debug, and that a DT property could be used to allow such write access (with the default being the charger properties being made read-only) - this way giving the best of both worlds. However then when I updated the bq24157_charger.c driver I ended up not really needing and implementing this feature. Out of curiosity, if it against best-practice to expose non hardware properties, how would one best address the above use case? Compile time switch? A sysfs property that allows write-enabling the other sysfs properties (doesn't sound right). Or...? -- Andreas Dannenberg Texas Instruments Inc > Best regards, > Krzysztof > > > + > > +Example: > > + > > +bq25890 { > > + compatible = "ti,bq24261 > > + reg = <0x6b>; > > + > > + ti,charge-current = <1000>; > > + ti,charge-voltage = <4200>; > > + ti,termination-current = <128>; > > + ti,max-charge-current = <3000>; > > + ti,max-charge-voltage = <4350>; > > + ti,min-charge-temperature = <0>; > > + ti,max-charge-temperature = <60>; > > + > > + ti,thermal-sensing; > > + ti,enable-user-write; > > +}; > > diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig > > index f8758d6..cd47d0d 100644 > > --- a/drivers/power/Kconfig > > +++ b/drivers/power/Kconfig > > @@ -396,6 +396,12 @@ config CHARGER_BQ24190 > > help > > Say Y to enable support for the TI BQ24190 battery charger. > > > > +config CHARGER_BQ24261 > > + tristate "TI BQ24261 charger driver" > > + depends on I2C && EXTCON > > + help > > + Say Y here to enable support for TI BQ24261 battery charger. > > + > > config CHARGER_BQ24257 > > tristate "TI BQ24257 battery charger driver" > > depends on I2C > > diff --git a/drivers/power/Makefile b/drivers/power/Makefile > > index 5752ce8..bec8409 100644 > > --- a/drivers/power/Makefile > > +++ b/drivers/power/Makefile > > @@ -59,6 +59,7 @@ obj-$(CONFIG_CHARGER_MAX8997) += max8997_charger.o > > obj-$(CONFIG_CHARGER_MAX8998) += max8998_charger.o > > obj-$(CONFIG_CHARGER_BQ2415X) += bq2415x_charger.o > > obj-$(CONFIG_CHARGER_BQ24190) += bq24190_charger.o > > +obj-$(CONFIG_CHARGER_BQ24261) += bq24261_charger.o > > obj-$(CONFIG_CHARGER_BQ24257) += bq24257_charger.o > > obj-$(CONFIG_CHARGER_BQ24735) += bq24735-charger.o > > obj-$(CONFIG_CHARGER_BQ25890) += bq25890_charger.o -- 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