RE: [PATCH] power: bq24261_charger: Add support for TI BQ24261 charger

[Pallala, Ramakrishna]

Hi, 

> From: k.kozlowski.k@xxxxxxxxx [mailto:k.kozlowski.k@xxxxxxxxx] On Behalf
> Of Krzysztof Kozlowski
> Sent: Monday, September 7, 2015 9:28 AM
> To: Pallala, Ramakrishna
> Cc: linux-kernel@xxxxxxxxxxxxxxx; linux-pm@xxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; Sebastian Reichel; Tc, Jenny; Andreas Dannenberg
> Subject: Re: [PATCH] power: bq24261_charger: Add support for TI BQ24261
> charger
> 
> 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).
Ok.


> > +
> > +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! :)
This is being closed by Andreas and we would probably go with mA/mV

> 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.
Ok...i will check with other charger driver DT settings and fix it.

> > +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 BQ24261 has provision to add Battery Pack thermistor but it has no ADC read it.
So a HW designer would or may not add the thermistor to charger and instead he can
connect to the Fuel Gauge.

> > +- 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.
We needed a mechanism to enable the sysfs writes on certain properties.
If DT is not the place where should it go?

Thanks,
Ram

N‹§²æ¸›yú²X¬¶ÇvØ–)Þ{.nlj·¥Š{±‘êX§¶›¡Ü}©ž²ÆzÚj:+v‰¨¾«‘êZ+€Êzf£¢·hšˆ§~†­†Ûÿû®w¥¢¸?™¨è&¢)ßf”ùy§m…á«a¶Úÿ0¶ìå