[PATCH v3] DT: Add documentation for the mfd Maxim max77693

[PATCH v3] DT: Add documentation for the mfd Maxim max77693

[Jacek Anaszewski]

This patch adds device tree binding documentation for
the flash cell of the Maxim max77693 multifunctional device.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Chanwoo Choi <cw00.choi@samsung.com>
Cc: Bryan Wu <cooloney@gmail.com>
Cc: Richard Purdie <rpurdie@rpsys.net>
Cc: devicetree@vger.kernel.org
---
 Documentation/devicetree/bindings/mfd/max77693.txt |   63 ++++++++++++++++++++
 1 file changed, 63 insertions(+)

- added range to maxim,boost-mvout and maxim,mvsys-min description

diff --git a/Documentation/devicetree/bindings/mfd/max77693.txt b/Documentation/devicetree/bindings/mfd/max77693.txt
index 38e6440..3d2103a 100644
--- a/Documentation/devicetree/bindings/mfd/max77693.txt
+++ b/Documentation/devicetree/bindings/mfd/max77693.txt
@@ -76,7 +76,55 @@ Optional properties:
     Valid values: 4300000, 4700000, 4800000, 4900000
     Default: 4300000
 
+- led : the LED submodule device node
+
+There are two LED outputs available - FLED1 and FLED2. Each of them can
+control a separate LED or they can be connected together to double
+the maximum current for a single connected LED. One LED is represented
+by one child node.
+
+Required properties:
+- compatible : Must be "maxim,max77693-led".
+
+Optional properties:
+- maxim,trigger-type : Flash trigger type.
+	Possible trigger types:
+		LEDS_TRIG_TYPE_EDGE (0) - Rising edge of the signal triggers
+			the flash,
+		LEDS_TRIG_TYPE_LEVEL (1) - Strobe pulse length controls duration
+			of the flash.
+- maxim,boost-mode :
+	In boost mode the device can produce up to 1.2A of total current
+	on both outputs. The maximum current on each output is reduced
+	to 625mA then. If not enabled explicitly, boost setting defaults to
+	LEDS_BOOST_FIXED in case both current sources are used.
+	Possible values:
+		LEDS_BOOST_OFF (0) - no boost,
+		LEDS_BOOST_ADAPTIVE (1) - adaptive mode,
+		LEDS_BOOST_FIXED (2) - fixed mode.
+- maxim,boost-mvout : Output voltage of the boost module in millivolts.
+		Range: 3300 - 5500
+- maxim,mvsys-min : Low input voltage level in millivolts. Flash is not fired
+	if chip estimates that system voltage could drop below this level due
+	to flash power consumption.
+		Range: 2400 - 3400
+
+Required properties of the LED child node:
+- led-sources : see Documentation/devicetree/bindings/leds/common.txt;
+		device current output identifiers: 0 - FLED1, 1 - FLED2
+
+Optional properties of the LED child node:
+- label : see Documentation/devicetree/bindings/leds/common.txt
+- max-microamp : see Documentation/devicetree/bindings/leds/common.txt
+		Range: 15625 - 250000
+- flash-max-microamp : see Documentation/devicetree/bindings/leds/common.txt
+		Range: 15625 - 1000000
+- flash-timeout-us : see Documentation/devicetree/bindings/leds/common.txt
+		Range: 62500 - 1000000
+
 Example:
+#include <dt-bindings/leds/common.h>
+
 	max77693@66 {
 		compatible = "maxim,max77693";
 		reg = <0x66>;
@@ -117,5 +165,20 @@ Example:
 			maxim,thermal-regulation-celsius = <75>;
 			maxim,battery-overcurrent-microamp = <3000000>;
 			maxim,charge-input-threshold-microvolt = <4300000>;
+
+		led {
+			compatible = "maxim,max77693-led";
+			maxim,trigger-type = <LEDS_TRIG_TYPE_LEVEL>;
+			maxim,boost-mode = <LEDS_BOOST_FIXED>;
+			maxim,boost-mvout = <5000>;
+			maxim,mvsys-min = <2400>;
+
+			camera_flash: flash-led {
+				label = "max77693-flash";
+				led-sources = <0>, <1>;
+				max-microamp = <500000>;
+				flash-max-microamp = <1250000>;
+				flash-timeout-us = <1000000>;
+			};
 		};
 	};
-- 
1.7.9.5

Re: [PATCH v3] DT: Add documentation for the mfd Maxim max77693

[Mark Rutland]

On Mon, Mar 30, 2015 at 10:52:28AM +0100, Jacek Anaszewski wrote:
> This patch adds device tree binding documentation for
> the flash cell of the Maxim max77693 multifunctional device.
> 
> Signed-off-by: Jacek Anaszewski <j.anaszewski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Andrzej Hajda <a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Acked-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Acked-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Cc: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Chanwoo Choi <cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Cc: Bryan Wu <cooloney-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Richard Purdie <rpurdie-Fm38FmjxZ/leoWH0uzbU5w@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
>  Documentation/devicetree/bindings/mfd/max77693.txt |   63 ++++++++++++++++++++
>  1 file changed, 63 insertions(+)
> 
> - added range to maxim,boost-mvout and maxim,mvsys-min description
> 
> diff --git a/Documentation/devicetree/bindings/mfd/max77693.txt b/Documentation/devicetree/bindings/mfd/max77693.txt
> index 38e6440..3d2103a 100644
> --- a/Documentation/devicetree/bindings/mfd/max77693.txt
> +++ b/Documentation/devicetree/bindings/mfd/max77693.txt
> @@ -76,7 +76,55 @@ Optional properties:
>      Valid values: 4300000, 4700000, 4800000, 4900000
>      Default: 4300000
>  
> +- led : the LED submodule device node
> +
> +There are two LED outputs available - FLED1 and FLED2. Each of them can
> +control a separate LED or they can be connected together to double
> +the maximum current for a single connected LED. One LED is represented
> +by one child node.
> +
> +Required properties:
> +- compatible : Must be "maxim,max77693-led".
> +
> +Optional properties:
> +- maxim,trigger-type : Flash trigger type.
> +	Possible trigger types:
> +		LEDS_TRIG_TYPE_EDGE (0) - Rising edge of the signal triggers
> +			the flash,
> +		LEDS_TRIG_TYPE_LEVEL (1) - Strobe pulse length controls duration
> +			of the flash.

Surely this is required? What should be assumed if this property isn't
present?

Otherwise this looks OK, but I'm not that familiar with LED/flash
bindings.

Mark.

> +- maxim,boost-mode :
> +	In boost mode the device can produce up to 1.2A of total current
> +	on both outputs. The maximum current on each output is reduced
> +	to 625mA then. If not enabled explicitly, boost setting defaults to
> +	LEDS_BOOST_FIXED in case both current sources are used.
> +	Possible values:
> +		LEDS_BOOST_OFF (0) - no boost,
> +		LEDS_BOOST_ADAPTIVE (1) - adaptive mode,
> +		LEDS_BOOST_FIXED (2) - fixed mode.
> +- maxim,boost-mvout : Output voltage of the boost module in millivolts.
> +		Range: 3300 - 5500
> +- maxim,mvsys-min : Low input voltage level in millivolts. Flash is not fired
> +	if chip estimates that system voltage could drop below this level due
> +	to flash power consumption.
> +		Range: 2400 - 3400
> +
> +Required properties of the LED child node:
> +- led-sources : see Documentation/devicetree/bindings/leds/common.txt;
> +		device current output identifiers: 0 - FLED1, 1 - FLED2
> +
> +Optional properties of the LED child node:
> +- label : see Documentation/devicetree/bindings/leds/common.txt
> +- max-microamp : see Documentation/devicetree/bindings/leds/common.txt
> +		Range: 15625 - 250000
> +- flash-max-microamp : see Documentation/devicetree/bindings/leds/common.txt
> +		Range: 15625 - 1000000
> +- flash-timeout-us : see Documentation/devicetree/bindings/leds/common.txt
> +		Range: 62500 - 1000000
> +
>  Example:
> +#include <dt-bindings/leds/common.h>
> +
>  	max77693@66 {
>  		compatible = "maxim,max77693";
>  		reg = <0x66>;
> @@ -117,5 +165,20 @@ Example:
>  			maxim,thermal-regulation-celsius = <75>;
>  			maxim,battery-overcurrent-microamp = <3000000>;
>  			maxim,charge-input-threshold-microvolt = <4300000>;
> +
> +		led {
> +			compatible = "maxim,max77693-led";
> +			maxim,trigger-type = <LEDS_TRIG_TYPE_LEVEL>;
> +			maxim,boost-mode = <LEDS_BOOST_FIXED>;
> +			maxim,boost-mvout = <5000>;
> +			maxim,mvsys-min = <2400>;
> +
> +			camera_flash: flash-led {
> +				label = "max77693-flash";
> +				led-sources = <0>, <1>;
> +				max-microamp = <500000>;
> +				flash-max-microamp = <1250000>;
> +				flash-timeout-us = <1000000>;
> +			};
>  		};
>  	};
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] DT: Add documentation for the mfd Maxim max77693

[Jacek Anaszewski]

On 03/30/2015 01:57 PM, Mark Rutland wrote:
> On Mon, Mar 30, 2015 at 10:52:28AM +0100, Jacek Anaszewski wrote:
>> This patch adds device tree binding documentation for
>> the flash cell of the Maxim max77693 multifunctional device.
>>
>> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>> Cc: Lee Jones <lee.jones@linaro.org>
>> Cc: Chanwoo Choi <cw00.choi@samsung.com>
>> Cc: Bryan Wu <cooloney@gmail.com>
>> Cc: Richard Purdie <rpurdie@rpsys.net>
>> Cc: devicetree@vger.kernel.org
>> ---
>>   Documentation/devicetree/bindings/mfd/max77693.txt |   63 ++++++++++++++++++++
>>   1 file changed, 63 insertions(+)
>>
>> - added range to maxim,boost-mvout and maxim,mvsys-min description
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/max77693.txt b/Documentation/devicetree/bindings/mfd/max77693.txt
>> index 38e6440..3d2103a 100644
>> --- a/Documentation/devicetree/bindings/mfd/max77693.txt
>> +++ b/Documentation/devicetree/bindings/mfd/max77693.txt
>> @@ -76,7 +76,55 @@ Optional properties:
>>       Valid values: 4300000, 4700000, 4800000, 4900000
>>       Default: 4300000
>>
>> +- led : the LED submodule device node
>> +
>> +There are two LED outputs available - FLED1 and FLED2. Each of them can
>> +control a separate LED or they can be connected together to double
>> +the maximum current for a single connected LED. One LED is represented
>> +by one child node.
>> +
>> +Required properties:
>> +- compatible : Must be "maxim,max77693-led".
>> +
>> +Optional properties:
>> +- maxim,trigger-type : Flash trigger type.
>> +	Possible trigger types:
>> +		LEDS_TRIG_TYPE_EDGE (0) - Rising edge of the signal triggers
>> +			the flash,
>> +		LEDS_TRIG_TYPE_LEVEL (1) - Strobe pulse length controls duration
>> +			of the flash.
>
> Surely this is required? What should be assumed if this property isn't
> present?

LEDS_TRIG_TYPE_LEVEL allows for an ISP to do e.g. short flash blink
before the actual strobe - it is used for eliminating photographs with
closed eyes, or can serve for probing ambient light conditions.

With LEDS_TRIG_TYPE_EDGE flash strobe is triggered on rising edge
and lasts until programmed timeout expires.

This setting is tightly related to a camera sensor, which generates
the strobe signal. Effectively it depends on board configuration.


> Otherwise this looks OK, but I'm not that familiar with LED/flash
> bindings.
>
> Mark.
>
>> +- maxim,boost-mode :
>> +	In boost mode the device can produce up to 1.2A of total current
>> +	on both outputs. The maximum current on each output is reduced
>> +	to 625mA then. If not enabled explicitly, boost setting defaults to
>> +	LEDS_BOOST_FIXED in case both current sources are used.
>> +	Possible values:
>> +		LEDS_BOOST_OFF (0) - no boost,
>> +		LEDS_BOOST_ADAPTIVE (1) - adaptive mode,
>> +		LEDS_BOOST_FIXED (2) - fixed mode.
>> +- maxim,boost-mvout : Output voltage of the boost module in millivolts.
>> +		Range: 3300 - 5500
>> +- maxim,mvsys-min : Low input voltage level in millivolts. Flash is not fired
>> +	if chip estimates that system voltage could drop below this level due
>> +	to flash power consumption.
>> +		Range: 2400 - 3400
>> +
>> +Required properties of the LED child node:
>> +- led-sources : see Documentation/devicetree/bindings/leds/common.txt;
>> +		device current output identifiers: 0 - FLED1, 1 - FLED2
>> +
>> +Optional properties of the LED child node:
>> +- label : see Documentation/devicetree/bindings/leds/common.txt
>> +- max-microamp : see Documentation/devicetree/bindings/leds/common.txt
>> +		Range: 15625 - 250000
>> +- flash-max-microamp : see Documentation/devicetree/bindings/leds/common.txt
>> +		Range: 15625 - 1000000
>> +- flash-timeout-us : see Documentation/devicetree/bindings/leds/common.txt
>> +		Range: 62500 - 1000000
>> +
>>   Example:
>> +#include <dt-bindings/leds/common.h>
>> +
>>   	max77693@66 {
>>   		compatible = "maxim,max77693";
>>   		reg = <0x66>;
>> @@ -117,5 +165,20 @@ Example:
>>   			maxim,thermal-regulation-celsius = <75>;
>>   			maxim,battery-overcurrent-microamp = <3000000>;
>>   			maxim,charge-input-threshold-microvolt = <4300000>;
>> +
>> +		led {
>> +			compatible = "maxim,max77693-led";
>> +			maxim,trigger-type = <LEDS_TRIG_TYPE_LEVEL>;
>> +			maxim,boost-mode = <LEDS_BOOST_FIXED>;
>> +			maxim,boost-mvout = <5000>;
>> +			maxim,mvsys-min = <2400>;
>> +
>> +			camera_flash: flash-led {
>> +				label = "max77693-flash";
>> +				led-sources = <0>, <1>;
>> +				max-microamp = <500000>;
>> +				flash-max-microamp = <1250000>;
>> +				flash-timeout-us = <1000000>;
>> +			};
>>   		};
>>   	};
>> --
>> 1.7.9.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Best Regards,
Jacek Anaszewski

Re: [PATCH v3] DT: Add documentation for the mfd Maxim max77693

[Mark Rutland]

Hi,

> >> +Optional properties:
> >> +- maxim,trigger-type : Flash trigger type.
> >> +	Possible trigger types:
> >> +		LEDS_TRIG_TYPE_EDGE (0) - Rising edge of the signal triggers
> >> +			the flash,
> >> +		LEDS_TRIG_TYPE_LEVEL (1) - Strobe pulse length controls duration
> >> +			of the flash.
> >
> > Surely this is required? What should be assumed if this property isn't
> > present?
> 
> LEDS_TRIG_TYPE_LEVEL allows for an ISP to do e.g. short flash blink
> before the actual strobe - it is used for eliminating photographs with
> closed eyes, or can serve for probing ambient light conditions.
> 
> With LEDS_TRIG_TYPE_EDGE flash strobe is triggered on rising edge
> and lasts until programmed timeout expires.
> 
> This setting is tightly related to a camera sensor, which generates
> the strobe signal. Effectively it depends on board configuration.

My comment wasn't to do with the semantics of eitehr option but rather
the optionality of the property.

Surely it's vital to know what this should be, and hence this property
should be required rather than optional?

If it isn't required, what would the assumed default be?

Mark.

Re: [PATCH v3] DT: Add documentation for the mfd Maxim max77693

[Jacek Anaszewski]

Hi Mark,

On 03/30/2015 03:20 PM, Mark Rutland wrote:
> Hi,
>
>>>> +Optional properties:
>>>> +- maxim,trigger-type : Flash trigger type.
>>>> +	Possible trigger types:
>>>> +		LEDS_TRIG_TYPE_EDGE (0) - Rising edge of the signal triggers
>>>> +			the flash,
>>>> +		LEDS_TRIG_TYPE_LEVEL (1) - Strobe pulse length controls duration
>>>> +			of the flash.
>>>
>>> Surely this is required? What should be assumed if this property isn't
>>> present?
>>
>> LEDS_TRIG_TYPE_LEVEL allows for an ISP to do e.g. short flash blink
>> before the actual strobe - it is used for eliminating photographs with
>> closed eyes, or can serve for probing ambient light conditions.
>>
>> With LEDS_TRIG_TYPE_EDGE flash strobe is triggered on rising edge
>> and lasts until programmed timeout expires.
>>
>> This setting is tightly related to a camera sensor, which generates
>> the strobe signal. Effectively it depends on board configuration.
>
> My comment wasn't to do with the semantics of eitehr option but rather
> the optionality of the property.
>
> Surely it's vital to know what this should be, and hence this property
> should be required rather than optional?
>
> If it isn't required, what would the assumed default be?

Default is LEDS_TRIG_TYPE_EDGE.

-- 
Best Regards,
Jacek Anaszewski

Re: [PATCH v3] DT: Add documentation for the mfd Maxim max77693

[sakari.ailus]

Hi Mark and Jacek,

On Mon, Mar 30, 2015 at 02:20:03PM +0100, Mark Rutland wrote:
> Hi,
> 
> > >> +Optional properties:
> > >> +- maxim,trigger-type : Flash trigger type.
> > >> +	Possible trigger types:
> > >> +		LEDS_TRIG_TYPE_EDGE (0) - Rising edge of the signal triggers
> > >> +			the flash,
> > >> +		LEDS_TRIG_TYPE_LEVEL (1) - Strobe pulse length controls duration
> > >> +			of the flash.
> > >
> > > Surely this is required? What should be assumed if this property isn't
> > > present?
> > 
> > LEDS_TRIG_TYPE_LEVEL allows for an ISP to do e.g. short flash blink
> > before the actual strobe - it is used for eliminating photographs with
> > closed eyes, or can serve for probing ambient light conditions.
> > 
> > With LEDS_TRIG_TYPE_EDGE flash strobe is triggered on rising edge
> > and lasts until programmed timeout expires.
> > 
> > This setting is tightly related to a camera sensor, which generates
> > the strobe signal. Effectively it depends on board configuration.
> 
> My comment wasn't to do with the semantics of eitehr option but rather
> the optionality of the property.
> 
> Surely it's vital to know what this should be, and hence this property
> should be required rather than optional?
> 
> If it isn't required, what would the assumed default be?

I wonder if there's a use case for edge triggering. In level trigger mode,
whichever component generates the trigger signal, determines also the strobe
time (up to the timeout). The sensor or (in the case of lack of the signal
from the sensor) the ISP has the most information on the sensor timing.

The existing as3654a driver only supports level triggering while the chip
can do edge, too.

I'd make level default and perhaps add a V4L2 control / sysfs file to change
this if needed.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [PATCH v3] DT: Add documentation for the mfd Maxim max77693

[Jacek Anaszewski]

Hi Sakari,

On 03/31/2015 11:14 AM, sakari.ailus@iki.fi wrote:
> Hi Mark and Jacek,
>
> On Mon, Mar 30, 2015 at 02:20:03PM +0100, Mark Rutland wrote:
>> Hi,
>>
>>>>> +Optional properties:
>>>>> +- maxim,trigger-type : Flash trigger type.
>>>>> +	Possible trigger types:
>>>>> +		LEDS_TRIG_TYPE_EDGE (0) - Rising edge of the signal triggers
>>>>> +			the flash,
>>>>> +		LEDS_TRIG_TYPE_LEVEL (1) - Strobe pulse length controls duration
>>>>> +			of the flash.
>>>>
>>>> Surely this is required? What should be assumed if this property isn't
>>>> present?
>>>
>>> LEDS_TRIG_TYPE_LEVEL allows for an ISP to do e.g. short flash blink
>>> before the actual strobe - it is used for eliminating photographs with
>>> closed eyes, or can serve for probing ambient light conditions.
>>>
>>> With LEDS_TRIG_TYPE_EDGE flash strobe is triggered on rising edge
>>> and lasts until programmed timeout expires.
>>>
>>> This setting is tightly related to a camera sensor, which generates
>>> the strobe signal. Effectively it depends on board configuration.
>>
>> My comment wasn't to do with the semantics of eitehr option but rather
>> the optionality of the property.
>>
>> Surely it's vital to know what this should be, and hence this property
>> should be required rather than optional?
>>
>> If it isn't required, what would the assumed default be?
>
> I wonder if there's a use case for edge triggering. In level trigger mode,
> whichever component generates the trigger signal, determines also the strobe
> time (up to the timeout). The sensor or (in the case of lack of the signal
> from the sensor) the ISP has the most information on the sensor timing.
>
> The existing as3654a driver only supports level triggering while the chip
> can do edge, too.
>
> I'd make level default and perhaps add a V4L2 control / sysfs file to change
> this if needed.

That sounds reasonable. I am going to come up with another version of
leds-max77693.c patch anyway, so I'll address this issue too.

-- 
Best Regards,
Jacek Anaszewski