diff mbox

[RFC,1/4] mfd: AXP20x: Add power supply bindings documentation

Message ID 20141020223314.0484f795@neptune.home
State New
Headers show

Commit Message

Bruno Prémont Oct. 20, 2014, 8:33 p.m. UTC
---
Note: the OCV values seem to have some defaults build into the
PMIC though may need adjustment if the used battery has a different
open circuit voltage curve.
As far as understood (these values are set in vendor driver but not
mentioned in chip documentation) they represent charge percentage
for some predefined voltages.

If prefixing these values with "x-power," is preferred the following
patch should becomes a dependency:
  http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/267606.html
and users in patch 2/4, 4/4 need adjusting.


 Documentation/devicetree/bindings/mfd/axp20x.txt |   20 +
 1 files changed, 20 insertions(+), 0 deletion(-)

Comments

Lee Jones Oct. 21, 2014, 10:15 a.m. UTC | #1
On Mon, 20 Oct 2014, Bruno Prémont wrote:

> 
> ---
> Note: the OCV values seem to have some defaults build into the
> PMIC though may need adjustment if the used battery has a different
> open circuit voltage curve.
> As far as understood (these values are set in vendor driver but not
> mentioned in chip documentation) they represent charge percentage
> for some predefined voltages.
> 
> If prefixing these values with "x-power," is preferred the following
> patch should becomes a dependency:
>   http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/267606.html
> and users in patch 2/4, 4/4 need adjusting.
> 
> 
>  Documentation/devicetree/bindings/mfd/axp20x.txt |   20 +
>  1 files changed, 20 insertions(+), 0 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
> index cc9e01b..8ea681c 100644
> --- a/Documentation/devicetree/bindings/mfd/axp20x.txt
> +++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
> @@ -28,6 +28,20 @@ Required properties:
>  		      (range: 750-1875). Default: 1.5MHz
>  
>  Optional properties for DCDCs:
> +- backup: Settings for backup/RTC battery charger
> +	  (Voltage in µV, current in µA)
> +	  If not present, charger will be left untouched
> +- battery.ocv: OCV capacity curve points (16 data values)
> +- battery.resistance: internal battery resistance in mΩ
> +                      (defaults to 100mΩ)
> +- battery.capacity: Battery capacity in mAh
> +		    If this attribute is missing, charger will be disabled
> +		    unless there is a battery connected.
> +- battery.temp_sensor: Description of temperautre sensor, 3 values
> +		       - driver current (20µA, 40µA, 60µA or 80µA)
> +		       - low temperature warning level (in µV)
> +		       - high temperature warning level (in µV)
> +		       If missing, temperature sensor gets disabled
>  - x-powers,dcdc-workmode: 1 for PWM mode, 0 for AUTO mode
>  			  Default: AUTO mode
>  
> @@ -49,6 +63,12 @@ axp209: pmic@34 {
>  	ldo3in-supply = <&axp_ipsout_reg>;
>  	ldo5in-supply = <&axp_ipsout_reg>;
>  
> +	backup = <3000000 200>;
> +	battery.ocv = <0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0>;
> +	battery.resistance = <0>;
> +	battery.capacity = <2000>;
> +	battery.temp_sensor = <20 1000000 4000000>;

Since when do we use '.'s in property names?

>  	regulators {
>  		x-powers,dcdc-freq = <1500>;
>
Bruno Prémont Oct. 21, 2014, 4:09 p.m. UTC | #2
On Tue, 21 October 2014 Lee Jones <lee.jones@linaro.org> wrote:
> On Mon, 20 Oct 2014, Bruno Prémont wrote:
> > ---
> > Note: the OCV values seem to have some defaults build into the
> > PMIC though may need adjustment if the used battery has a different
> > open circuit voltage curve.
> > As far as understood (these values are set in vendor driver but not
> > mentioned in chip documentation) they represent charge percentage
> > for some predefined voltages.
> > 
> > If prefixing these values with "x-power," is preferred the following
> > patch should becomes a dependency:
> >   http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/267606.html
> > and users in patch 2/4, 4/4 need adjusting.
> > 
> > 
> >  Documentation/devicetree/bindings/mfd/axp20x.txt |   20 +
> >  1 files changed, 20 insertions(+), 0 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
> > index cc9e01b..8ea681c 100644
> > --- a/Documentation/devicetree/bindings/mfd/axp20x.txt
> > +++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
> > @@ -28,6 +28,20 @@ Required properties:
> >  		      (range: 750-1875). Default: 1.5MHz
> >  
> >  Optional properties for DCDCs:
> > +- backup: Settings for backup/RTC battery charger
> > +	  (Voltage in µV, current in µA)
> > +	  If not present, charger will be left untouched
> > +- battery.ocv: OCV capacity curve points (16 data values)
> > +- battery.resistance: internal battery resistance in mΩ
> > +                      (defaults to 100mΩ)
> > +- battery.capacity: Battery capacity in mAh
> > +		    If this attribute is missing, charger will be disabled
> > +		    unless there is a battery connected.
> > +- battery.temp_sensor: Description of temperautre sensor, 3 values
> > +		       - driver current (20µA, 40µA, 60µA or 80µA)
> > +		       - low temperature warning level (in µV)
> > +		       - high temperature warning level (in µV)
> > +		       If missing, temperature sensor gets disabled
> >  - x-powers,dcdc-workmode: 1 for PWM mode, 0 for AUTO mode
> >  			  Default: AUTO mode
> >  
> > @@ -49,6 +63,12 @@ axp209: pmic@34 {
> >  	ldo3in-supply = <&axp_ipsout_reg>;
> >  	ldo5in-supply = <&axp_ipsout_reg>;
> >  
> > +	backup = <3000000 200>;
> > +	battery.ocv = <0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0>;
> > +	battery.resistance = <0>;
> > +	battery.capacity = <2000>;
> > +	battery.temp_sensor = <20 1000000 4000000>;
> 
> Since when do we use '.'s in property names?

I've not found guidelines on this, but whatever is the right way to
name them, I'm open for suggestions.

Bruno

> >  	regulators {
> >  		x-powers,dcdc-freq = <1500>;
> >  
>
Maxime Ripard Oct. 21, 2014, 7:19 p.m. UTC | #3
Hi Bruno,

Thanks a lot for working on this!

On Tue, Oct 21, 2014 at 06:09:16PM +0200, Bruno Prémont wrote:
> On Tue, 21 October 2014 Lee Jones <lee.jones@linaro.org> wrote:
> > On Mon, 20 Oct 2014, Bruno Prémont wrote:
> > > ---
> > > Note: the OCV values seem to have some defaults build into the
> > > PMIC though may need adjustment if the used battery has a different
> > > open circuit voltage curve.
> > > As far as understood (these values are set in vendor driver but not
> > > mentioned in chip documentation) they represent charge percentage
> > > for some predefined voltages.
> > > 
> > > If prefixing these values with "x-power," is preferred the following
> > > patch should becomes a dependency:
> > >   http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/267606.html
> > > and users in patch 2/4, 4/4 need adjusting.
> > > 
> > > 
> > >  Documentation/devicetree/bindings/mfd/axp20x.txt |   20 +
> > >  1 files changed, 20 insertions(+), 0 deletion(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
> > > index cc9e01b..8ea681c 100644
> > > --- a/Documentation/devicetree/bindings/mfd/axp20x.txt
> > > +++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
> > > @@ -28,6 +28,20 @@ Required properties:
> > >  		      (range: 750-1875). Default: 1.5MHz
> > >  
> > >  Optional properties for DCDCs:
> > > +- backup: Settings for backup/RTC battery charger
> > > +	  (Voltage in µV, current in µA)
> > > +	  If not present, charger will be left untouched
> > > +- battery.ocv: OCV capacity curve points (16 data values)
> > > +- battery.resistance: internal battery resistance in mΩ
> > > +                      (defaults to 100mΩ)
> > > +- battery.capacity: Battery capacity in mAh
> > > +		    If this attribute is missing, charger will be disabled
> > > +		    unless there is a battery connected.
> > > +- battery.temp_sensor: Description of temperautre sensor, 3 values
> > > +		       - driver current (20µA, 40µA, 60µA or 80µA)
> > > +		       - low temperature warning level (in µV)
> > > +		       - high temperature warning level (in µV)
> > > +		       If missing, temperature sensor gets disabled
> > >  - x-powers,dcdc-workmode: 1 for PWM mode, 0 for AUTO mode
> > >  			  Default: AUTO mode
> > >  
> > > @@ -49,6 +63,12 @@ axp209: pmic@34 {
> > >  	ldo3in-supply = <&axp_ipsout_reg>;
> > >  	ldo5in-supply = <&axp_ipsout_reg>;
> > >  
> > > +	backup = <3000000 200>;
> > > +	battery.ocv = <0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0>;
> > > +	battery.resistance = <0>;
> > > +	battery.capacity = <2000>;
> > > +	battery.temp_sensor = <20 1000000 4000000>;
> > 
> > Since when do we use '.'s in property names?
> 
> I've not found guidelines on this, but whatever is the right way to
> name them, I'm open for suggestions.

You can take a look at the ePAPR specs. Even if it's quite outdated,
it still puts you in the right mindset.

That being said, since they are driver-specific properties, they
should be prefixed by the vendor name (here x-powers).

And I think they all belong in a sub-node, just like what's been done
for the regulators.

Maxime
Maxime Ripard Oct. 21, 2014, 8:10 p.m. UTC | #4
On Mon, Oct 20, 2014 at 10:33:14PM +0200, Bruno Prémont wrote:
> 
> ---

You're missing a commit log, and your signed-off-by.

You should go through Documentation/SubmittingPatches, and make sure
to run checkpatch.pl, and fix the warning and errors.

> Note: the OCV values seem to have some defaults build into the
> PMIC though may need adjustment if the used battery has a different
> open circuit voltage curve.
> As far as understood (these values are set in vendor driver but not
> mentioned in chip documentation) they represent charge percentage
> for some predefined voltages.
> 
> If prefixing these values with "x-power," is preferred the following
> patch should becomes a dependency:
>   http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/267606.html

Feel free to fold that patch into your serie, since it seems to have
been forgotten.

Maxime
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
index cc9e01b..8ea681c 100644
--- a/Documentation/devicetree/bindings/mfd/axp20x.txt
+++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
@@ -28,6 +28,20 @@  Required properties:
 		      (range: 750-1875). Default: 1.5MHz
 
 Optional properties for DCDCs:
+- backup: Settings for backup/RTC battery charger
+	  (Voltage in µV, current in µA)
+	  If not present, charger will be left untouched
+- battery.ocv: OCV capacity curve points (16 data values)
+- battery.resistance: internal battery resistance in mΩ
+                      (defaults to 100mΩ)
+- battery.capacity: Battery capacity in mAh
+		    If this attribute is missing, charger will be disabled
+		    unless there is a battery connected.
+- battery.temp_sensor: Description of temperautre sensor, 3 values
+		       - driver current (20µA, 40µA, 60µA or 80µA)
+		       - low temperature warning level (in µV)
+		       - high temperature warning level (in µV)
+		       If missing, temperature sensor gets disabled
 - x-powers,dcdc-workmode: 1 for PWM mode, 0 for AUTO mode
 			  Default: AUTO mode
 
@@ -49,6 +63,12 @@  axp209: pmic@34 {
 	ldo3in-supply = <&axp_ipsout_reg>;
 	ldo5in-supply = <&axp_ipsout_reg>;
 
+	backup = <3000000 200>;
+	battery.ocv = <0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0>;
+	battery.resistance = <0>;
+	battery.capacity = <2000>;
+	battery.temp_sensor = <20 1000000 4000000>;
+
 	regulators {
 		x-powers,dcdc-freq = <1500>;