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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 9 Sep 2015 18:11:15 +0000
From:	"Pallala, Ramakrishna" <ramakrishna.pallala@...el.com>
To:	Krzysztof Kozlowski <k.kozlowski@...sung.com>
CC:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Sebastian Reichel <sre@...nel.org>,
	"Tc, Jenny" <jenny.tc@...el.com>,
	Andreas Dannenberg <dannenberg@...com>
Subject: RE: [PATCH] power: bq24261_charger: Add support for TI BQ24261
 charger

Hi, 

> From: k.kozlowski.k@...il.com [mailto:k.kozlowski.k@...il.com] On Behalf
> Of Krzysztof Kozlowski
> Sent: Monday, September 7, 2015 9:28 AM
> To: Pallala, Ramakrishna
> Cc: linux-kernel@...r.kernel.org; linux-pm@...r.kernel.org;
> devicetree@...r.kernel.org; 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@...el.com>:
> >
> > 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@...el.com>
> > Signed-off-by: Jennt TC <jenny.tc@...el.com>
> > ---
> >  .../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