[V7,1/8] mfd: add device-tree binding doc for PMIC max77620/max20024

The MAXIM PMIC MAX77620 and MAX20024 are power management IC
which supports RTC, GPIO, DCDC/LDO regulators, interrupt,
watchdog etc.

Add DT binding document for the different functionality of
this device.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
Acked-by: Rob Herring <robh@kernel.org>
---
Changes from V1: 
- Added units in some of properties.
- Change the boolean property to tristate type and detail some of
  properties.

Change from V2: 
- added unit in period related dt property.

Change from V3: None
- Added Rob's ack.

Changes from V4: 
- A- Provide more details in the dt binding doc.
- Take care of fps nodes.
- Split the submodule's DT binding doc on respective folder.
- Drop the battery charger and low battery binding and related code as
  it need to go on power driver.

Change from V5:
- None

Change from V6:
-start the patch title with mfd instead of DT: mfd:

 Documentation/devicetree/bindings/mfd/max77620.txt | 118 +++++++++++++++++++++
 include/dt-bindings/mfd/max77620.h                 |  35 ++++++
 2 files changed, 153 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/max77620.txt
 create mode 100644 include/dt-bindings/mfd/max77620.h

On Sat, 30 Jan 2016, Laxman Dewangan wrote:

> The MAXIM PMIC MAX77620 and MAX20024 are power management IC
> which supports RTC, GPIO, DCDC/LDO regulators, interrupt,
> watchdog etc.
> 
> Add DT binding document for the different functionality of
> this device.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
> Changes from V1: 
> - Added units in some of properties.
> - Change the boolean property to tristate type and detail some of
>   properties.
> 
> Change from V2: 
> - added unit in period related dt property.
> 
> Change from V3: None
> - Added Rob's ack.
> 
> Changes from V4: 
> - A- Provide more details in the dt binding doc.
> - Take care of fps nodes.
> - Split the submodule's DT binding doc on respective folder.
> - Drop the battery charger and low battery binding and related code as
>   it need to go on power driver.
> 
> Change from V5:
> - None
> 
> Change from V6:
> -start the patch title with mfd instead of DT: mfd:
> 
>  Documentation/devicetree/bindings/mfd/max77620.txt | 118 +++++++++++++++++++++
>  include/dt-bindings/mfd/max77620.h                 |  35 ++++++
>  2 files changed, 153 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/max77620.txt
>  create mode 100644 include/dt-bindings/mfd/max77620.h
> 
> diff --git a/Documentation/devicetree/bindings/mfd/max77620.txt b/Documentation/devicetree/bindings/mfd/max77620.txt
> new file mode 100644
> index 0000000..f258ce4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/max77620.txt
> @@ -0,0 +1,118 @@
> +MAX77620 Power management IC from Maxim Semiconductor.
> +
> +Required properties:
> +-------------------
> +- compatible: Must be one of
> +		"maxim,max77620"
> +		"maxim,max20024".
> +- reg: I2C device address.
> +
> +Optional properties:
> +-------------------
> +- interrupts:		the interrupt on the parent the controller is
> +			connected to

Nit: Sometimes you use full-stops and sometimes you don't.

> +- interrupt-controller: Marks the device node as an interrupt controller.
> +- #interrupt-cells:	is <2> and their usage is compliant to the 2 cells
> +			variant of <../interrupt-controller/interrupts.txt>
> +			IRQ numbers for different interrupt source of MAX77620
> +			are defined at dt-bindings/mfd/max77620.h.
> +
> +Optional subnodes and their properties:
> +=======================================
> +
> +Flexible power sequence configurations:
> +--------------------------------------
> +The PMIC has multiple control mode:

s/mode/modes/

> +	Normal mode also called as active mode on which all step-down
> +		regulators, all linear regulators, GPIOs, and the 32kHz
> +		oscillator are in normal active mode.
> +	sleep mode: Regulators/GPIOs/clock can go on OFF state based on

"can go on OFF state"?

> +		their configurations.
> +	Global Low power mode (GLPM): In this mode, step-down regulators, linear
> +		regulators, and the 32kHz oscillator are in low-power modes.

Again, please standardise your formatting.  Sometimes you use
uppercase characters, sometimes you don't.  Sometimes you use ':'
after the mode, sometimes you don't.  Etc.

I suggest the following format would read better:

Normal mode:		Also called as active mode on which all step-down
			regulators, all linear regulators, GPIOs, and the 32kHz
			oscillator are in normal active mode.
Sleep mode:		Regulators/GPIOs/clock can go on OFF state based on
			their configurations.
Global Low Power mode:	In GLPM, step-down regulators, linear
			regulators, and the 32kHz oscillator are in low-power modes.

> +	Different modes of regulators/clock/GPIOs are controlled by the their
> +FPS configurations. There is different configuration registers for each of
> +these resources. Typical configurations per resource are:
> +	FPS source: 	Attach the resource to required FPS source. When
> +			resources are attached to one of FPS source then
> +			resournce can be enable/disable when related FPS

Have you used spell check?  I suggest you do.

> +			source gets the control signal for ON and OFF.
> +	Power on slot: 	Slot number on which resource is ON once FPS source
> +			get ON signal.

Can you find another way of explaining this please?

> +	Power down slot Slot number on which resource is OFF once FPS source

Please read what you wrote to see if it makes sense before
submitting.

> +			get OFF signal.
> +
> +There is three FPS source for resources called FPS0, FPS1 and FPS2. All

s/is/are/
s/source/sources/

> +resources need to attached to one of these FPS. It can alsoi be set

Please spell check the entire document.

> +FPS source to NONE, and on this case, it is completely controlled by

s/on/in/

> +the register control rather than external input control to PMIC.
> +
> +The configuration parameters of FPS is provided through subnode "fps"
> +and their child for FPS specific.
> +The node name for FPS child are defined as "fps0", "fps1", and "fps2" for
> +the FPS0, FPS1 and FPS2 respectively.
> +
> +There is need for different FPS configuration parameters based on system
> +state like when system state changed from active to suspend or active to
> +power off (shutdown).
> +
> +Optinal properties:
> +-------------------
> +-maxim,fps-control:	u32, FPS control source like external control input
> +			to PMIC i.e. EN0, EN1 or software (SW). The macros
> +			are defined on dt-bindings/mfd/max77620.h for
> +			different control source.
> +				FPS_CONTROL_SRC_EN0 for PMIC external input EN0.
> +				FPS_CONTROL_SRC_EN1 for PMIC external input EN1.
> +				FPS_CONTROL_SRC_SW for software control.
> +
> +-maxim,shutdown-fps-time-period-us: u32, FPS time period in microseconds
> +				    when system enters to shutdown state.
> +-maxim,suspend-fps-time-period-us:  u32, FPS time period in microseconds
> +				    when system enters to suspend state.
> +
> +-maxim,enable-sleep: 		    Boolean, enable sleep state of PMIC

We already have bindings for sleeping.  Please use a generic one.

> +				    when the FPS control input become active.
> +-maxim,enable-global-lpm: 	    Boolean, enable Global Low Power Mode (GLPM)
> +				    of PMIC when the FPS control input become
> +				    active.
> +
> +Here supported time periods by device in microseconds are as follows:
> +MAX77620 supports 40, 80, 160, 320, 640, 1280, 2560 and 5120 microseconds.
> +MAX20024 supports 20, 40, 80, 160, 320, 640, 1280 and 2540 microseconds.
> +
> +For different sub modules like GPIO, pincontrol, regulator, power, please refer
> +respected device-tree binding document on respective directories.
> +
> +Example:
> +--------
> +#include <dt-bindings/mfd/max77620.h>
> +
> +max77620@3c {
> +	compatible = "maxim,max77620";
> +	reg = <0x3c>;
> +
> +	interrupt-parent = <&intc>;
> +	interrupts = <0 86 IRQ_TYPE_NONE>;
> +
> +	interrupt-controller;
> +	#interrupt-cells = <2>;
> +
> +	fps {
> +		fps0 {
> +			maxim,shutdown-fps-time-period-us = <1280>;
> +			maxim,fps-control = <FPS_CONTROL_SRC_EN1>;
> +		};
> +
> +		fps1 {
> +			maxim,shutdown-fps-time-period-us = <1280>;
> +			maxim,fps-control = <FPS_CONTROL_SRC_EN0>;
> +		};
> +
> +		fps2 {
> +			maxim,shutdown-fps-time-period-us = <1280>;
> +			maxim,fps-control = <FPS_CONTROL_SRC_SW>;
> +		};
> +	};
> +};
> diff --git a/include/dt-bindings/mfd/max77620.h b/include/dt-bindings/mfd/max77620.h
> new file mode 100644
> index 0000000..1b571d7
> --- /dev/null
> +++ b/include/dt-bindings/mfd/max77620.h
> @@ -0,0 +1,35 @@
> +/*
> + * This header provides macros for MAXIM MAX77620 device bindings.
> + *
> + * Copyright (c) 2016, NVIDIA Corporation.
> + * Author: Laxman Dewangan <ldewangan@nvidia.com>
> + */
> +
> +#ifndef _DT_BINDINGS_MFD_MAX77620_H
> +#define _DT_BINDINGS_MFD_MAX77620_H
> +
> +/* MAX77620 interrupts */
> +#define MAX77620_IRQ_TOP_GLBL		0 /* Low-Battery */
> +#define MAX77620_IRQ_TOP_SD		1 /* SD power fail */
> +#define MAX77620_IRQ_TOP_LDO		2 /* LDO power fail */
> +#define MAX77620_IRQ_TOP_GPIO		3 /* GPIO internal int to MAX77620 */
> +#define MAX77620_IRQ_TOP_RTC		4 /* RTC */
> +#define MAX77620_IRQ_TOP_32K		5 /* 32kHz oscillator */
> +#define MAX77620_IRQ_TOP_ONOFF		6 /* ON/OFF oscillator */
> +#define MAX77620_IRQ_LBT_MBATLOW	7 /* Thermal alarm status, > 120C */
> +#define MAX77620_IRQ_LBT_TJALRM1	8 /* Thermal alarm status, > 120C */
> +#define MAX77620_IRQ_LBT_TJALRM2	9 /* Thermal alarm status, > 140C */
> +
> +/* FPS control inputs */
> +#define FPS_CONTROL_SRC_EN0		0
> +#define FPS_CONTROL_SRC_EN1		1
> +#define FPS_CONTROL_SRC_SW		2
> +
> +/* FPS source */
> +#define FPS_SRC_0			0
> +#define FPS_SRC_1			1
> +#define FPS_SRC_2			2
> +#define FPS_SRC_NONE			3
> +#define FPS_SRC_DEF			4
> +
> +#endif

On Tuesday 09 February 2016 09:12 PM, Lee Jones wrote:
> On Sat, 30 Jan 2016, Laxman Dewangan wrote:
>
> +	Normal mode also called as active mode on which all step-down
> +		regulators, all linear regulators, GPIOs, and the 32kHz
> +		oscillator are in normal active mode.
> +	sleep mode: Regulators/GPIOs/clock can go on OFF state based on
> "can go on OFF state"?

Regulator/GPIO has two states, enable and disable. If sleep mode is 
configured for these resource and external signal triggers to sleep then 
this get disabled.
>> +	Different modes of regulators/clock/GPIOs are controlled by the their
>> +FPS configurations. There is different configuration registers for each of
>> +these resources. Typical configurations per resource are:
>> +	FPS source: 	Attach the resource to required FPS source. When
>> +			resources are attached to one of FPS source then
>> +			resournce can be enable/disable when related FPS
> Have you used spell check?  I suggest you do.

My bad, I did but mixed with technical ignorance. Will try best.

>
>> +			source gets the control signal for ON and OFF.
>> +	Power on slot: 	Slot number on which resource is ON once FPS source
>> +			get ON signal.
> Can you find another way of explaining this please?

Hmm..
Does it look fine:
There is 8 slots for each FPS on which resource can get enabled. This 
property provides the slot number on which resource gets enabled after 
FPS sequence started.


>
>
> +
> +-maxim,enable-sleep: 		    Boolean, enable sleep state of PMIC
> We already have bindings for sleeping.  Please use a generic one.

Which property? Saw sleep property with vendor prefix.

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Tue, 09 Feb 2016, Laxman Dewangan wrote:

> 
> On Tuesday 09 February 2016 09:12 PM, Lee Jones wrote:
> >On Sat, 30 Jan 2016, Laxman Dewangan wrote:
> >
> >+	Normal mode also called as active mode on which all step-down
> >+		regulators, all linear regulators, GPIOs, and the 32kHz
> >+		oscillator are in normal active mode.
> >+	sleep mode: Regulators/GPIOs/clock can go on OFF state based on
> >"can go on OFF state"?
> 
> Regulator/GPIO has two states, enable and disable. If sleep mode is
> configured for these resource and external signal triggers to sleep
> then this get disabled.

It's the English that I'm unhappy with.

"can go on OFF state" doesn't sound right.

> >>+			source gets the control signal for ON and OFF.
> >>+	Power on slot: 	Slot number on which resource is ON once FPS source
> >>+			get ON signal.
> >Can you find another way of explaining this please?
> 
> Hmm..
> Does it look fine:
> There is 8 slots for each FPS on which resource can get enabled.
> This property provides the slot number on which resource gets
> enabled after FPS sequence started.

It's a bit better, yes.  Although it's still a little difficult to
read.  In fact, I can't even recommend a suitable alternative, since I
still do not understand exactly what it is you're trying to say.

> >+
> >+-maxim,enable-sleep: 		    Boolean, enable sleep state of PMIC
> >We already have bindings for sleeping.  Please use a generic one.
> 
> Which property? Saw sleep property with vendor prefix.

I guess there are too many '.*sleep.*' properties now, each with
slightly different syntax and meaning.  The situation is exacerbated
by one of the key examples is using the property with cells expected
i.e. non-bool.

On Wednesday 10 February 2016 06:53 PM, Lee Jones wrote:
> On Tue, 09 Feb 2016, Laxman Dewangan wrote:
>
>> On Tuesday 09 February 2016 09:12 PM, Lee Jones wrote:
>>> On Sat, 30 Jan 2016, Laxman Dewangan wrote:
>>>
>>> +	Normal mode also called as active mode on which all step-down
>>> +		regulators, all linear regulators, GPIOs, and the 32kHz
>>> +		oscillator are in normal active mode.
>>> +	sleep mode: Regulators/GPIOs/clock can go on OFF state based on
>>> "can go on OFF state"?
>> Regulator/GPIO has two states, enable and disable. If sleep mode is
>> configured for these resource and external signal triggers to sleep
>> then this get disabled.
> It's the English that I'm unhappy with.
>
> "can go on OFF state" doesn't sound right.
>
>>>> +			source gets the control signal for ON and OFF.
>>>> +	Power on slot: 	Slot number on which resource is ON once FPS source
>>>> +			get ON signal.
>>> Can you find another way of explaining this please?
>> Hmm..
>> Does it look fine:
>> There is 8 slots for each FPS on which resource can get enabled.
>> This property provides the slot number on which resource gets
>> enabled after FPS sequence started.
> It's a bit better, yes.  Although it's still a little difficult to
> read.  In fact, I can't even recommend a suitable alternative, since I
> still do not understand exactly what it is you're trying to say.

I am sorry if I am not able to make it clear. Better I post the 
datasheet content so that it is easy to understand and then we can have 
better text here.


The Flexible Power Sequencer (FPS) allows each regulator to power up 
under hardware or software control. Additionally, each regulator can 
power on independently or among a group of other regulators with an 
adjustable power-up and power-down delays (sequencing). GPIO1, GPIO2, 
and GPIO3 can be programmed to be part of a sequence allowing external 
regulators to be sequenced along with internal regulators. nRST_IO can 
be programmed to be part of a sequence.

The flexible sequencing structure consists of two hardware enable inputs 
(EN0, EN1), and 3 master sequencing timers. Each master sequencing timer 
is programmable through its configuration register to have a hardware 
enable source or a software enable source (CNFGFPSx). When 
enabled/disabled the master sequencing timer generates eight sequencing 
events. The time period between each event is programmable within the 
configuration register.

Each regulator, GPIO1, GPIO2, GPIO3, and nRST_IO has a flexible power 
sequence slave register (FPS_x) which allows its enable source to be 
specified as a flexible power sequencer timer or a software bit. When a 
FPSSRCx specifies the enable source to be a flexible power sequencer, 
the power up and power down delays are configured by FPSPUx[2:0] and 
FPSPDx[2:0].can be specified in that regulators flexible power sequencer 
configuration register.



>>> +
>>> +-maxim,enable-sleep: 		    Boolean, enable sleep state of PMIC
>>> We already have bindings for sleeping.  Please use a generic one.
>> Which property? Saw sleep property with vendor prefix.
> I guess there are too many '.*sleep.*' properties now, each with
> slightly different syntax and meaning.  The situation is exacerbated
> by one of the key examples is using the property with cells expected
> i.e. non-bool.


My grep shows some sleep property and followig are near to which I am 
looking.

st,supports-sleepmode

pinctrl/ste,nomadik.txt:- ste,sleep: <0/1>
                                     0: sleep mode disable,
                                     1: sleep mode enable.


Should I made this as generic like

sleep: <0/1>
             0: sleep mode disable,
             1: sleep mode enable.





--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Wed, 10 Feb 2016, Laxman Dewangan wrote:

> 
> On Wednesday 10 February 2016 06:53 PM, Lee Jones wrote:
> >On Tue, 09 Feb 2016, Laxman Dewangan wrote:
> >
> >>On Tuesday 09 February 2016 09:12 PM, Lee Jones wrote:
> >>>On Sat, 30 Jan 2016, Laxman Dewangan wrote:
> >>>
> >>>+	Normal mode also called as active mode on which all step-down
> >>>+		regulators, all linear regulators, GPIOs, and the 32kHz
> >>>+		oscillator are in normal active mode.
> >>>+	sleep mode: Regulators/GPIOs/clock can go on OFF state based on
> >>>"can go on OFF state"?
> >>Regulator/GPIO has two states, enable and disable. If sleep mode is
> >>configured for these resource and external signal triggers to sleep
> >>then this get disabled.
> >It's the English that I'm unhappy with.
> >
> >"can go on OFF state" doesn't sound right.
> >
> >>>>+			source gets the control signal for ON and OFF.
> >>>>+	Power on slot: 	Slot number on which resource is ON once FPS source
> >>>>+			get ON signal.
> >>>Can you find another way of explaining this please?
> >>Hmm..
> >>Does it look fine:
> >>There is 8 slots for each FPS on which resource can get enabled.
> >>This property provides the slot number on which resource gets
> >>enabled after FPS sequence started.
> >It's a bit better, yes.  Although it's still a little difficult to
> >read.  In fact, I can't even recommend a suitable alternative, since I
> >still do not understand exactly what it is you're trying to say.
> 
> I am sorry if I am not able to make it clear. Better I post the
> datasheet content so that it is easy to understand and then we can
> have better text here.
> 
> 
> The Flexible Power Sequencer (FPS) allows each regulator to power up
> under hardware or software control. Additionally, each regulator can
> power on independently or among a group of other regulators with an
> adjustable power-up and power-down delays (sequencing). GPIO1,
> GPIO2, and GPIO3 can be programmed to be part of a sequence allowing
> external regulators to be sequenced along with internal regulators.
> nRST_IO can be programmed to be part of a sequence.
> 
> The flexible sequencing structure consists of two hardware enable
> inputs (EN0, EN1), and 3 master sequencing timers. Each master
> sequencing timer is programmable through its configuration register
> to have a hardware enable source or a software enable source
> (CNFGFPSx). When enabled/disabled the master sequencing timer
> generates eight sequencing events. The time period between each
> event is programmable within the configuration register.
> 
> Each regulator, GPIO1, GPIO2, GPIO3, and nRST_IO has a flexible
> power sequence slave register (FPS_x) which allows its enable source
> to be specified as a flexible power sequencer timer or a software
> bit. When a FPSSRCx specifies the enable source to be a flexible
> power sequencer, the power up and power down delays are configured
> by FPSPUx[2:0] and FPSPDx[2:0].can be specified in that regulators
> flexible power sequencer configuration register.

Perfect, let's go with that.

> >>>+-maxim,enable-sleep: 		    Boolean, enable sleep state of PMIC
> >>>We already have bindings for sleeping.  Please use a generic one.
> >>Which property? Saw sleep property with vendor prefix.
> >I guess there are too many '.*sleep.*' properties now, each with
> >slightly different syntax and meaning.  The situation is exacerbated
> >by one of the key examples is using the property with cells expected
> >i.e. non-bool.
> 
> 
> My grep shows some sleep property and followig are near to which I
> am looking.
> 
> st,supports-sleepmode
> 
> pinctrl/ste,nomadik.txt:- ste,sleep: <0/1>
>                                     0: sleep mode disable,
>                                     1: sleep mode enable.
> 
> 
> Should I made this as generic like
> 
> sleep: <0/1>
>             0: sleep mode disable,
>             1: sleep mode enable.

Ideally yes.  This is obviously going to be used again.

However;
 - My fear is that it will get confused with the 'sleep' property in
   Documentation/devicetree/booting-without-of.txt.
 - Secondly, you would need to get Rob to Ack it.

On Thursday 11 February 2016 02:56 PM, Lee Jones wrote:
> On Wed, 10 Feb 2016, Laxman Dewangan wrote:
>
>>
>> sleep: <0/1>
>>              0: sleep mode disable,
>>              1: sleep mode enable.
> Ideally yes.  This is obviously going to be used again.
>
> However;
>   - My fear is that it will get confused with the 'sleep' property in
>     Documentation/devicetree/booting-without-of.txt.
>   - Secondly, you would need to get Rob to Ack it.
>

Another thought, becasue this is just for enable/disable, why not we add 
property as "enable-sleep" of boolean type instead of u32 value type?

It is easy to describe the behavior with enable-sleep and 
enable-low-power-mode.

-enable-sleep:                  Boolean, when FPS event cleared
                                         (set to LOW), resources get 
disabled
                                         at the sequencing event 
corresponding
                                         to its FPS configuration register.

-enable-low-power-mode: Boolean, when FPS event cleared
                                         (set to LOW), resources sets into
                                         low power mode at the 
sequencing event
                                         corresponding to its FPS 
configuration
                                         register.

Both property can not be together.


Other approach is to club together
maxim,device-state-on-disabled-event : u32, describe the PMIC state
                                                             when FPS 
event cleared (set to LOW)
                                                             whether it 
should go to sleep state or
low-power state.
                                                                 1 = 
Device state is set to sleep
                                                                 2 = 
Device state is set to low power mode.
Absence of this property or other value will not change device state 
when FPS event
         cleared.


--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html