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

Jacek Anaszewski
2015-03-31T10:58:30Z

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