Messages in this thread | | | Date | Thu, 20 Nov 2014 13:38:09 +0100 | From | Jacek Anaszewski <> | Subject | Re: [RFC] adp1653: Add device tree bindings for LED controller |
| |
On 11/19/2014 10:45 AM, Jacek Anaszewski wrote: > Hi Pavel, Sakari, > > On 11/18/2014 05:51 PM, Pavel Machek wrote: >> Hi! >> >>>> If the hardware LED changes with one that needs different current, the >>>> block for the adp1653 stays the same, but white LED block should be >>>> updated with different value. >>> >>> I think that you are talking about sub nodes. Indeed I am leaning >>> towards this type of design. >> >> I think I am :-). >> >>>>> I agree that flash-timeout should be per led - an array, similarly >>>>> as in case of iout's. >>>> >>>> Agreed about per-led, disagreed about the array. As all the fields >>>> would need arrays, and as LED system currently does not use arrays for >>>> label and linux,default-trigger, I believe we should follow existing >>>> design and model it as three devices. (It _is_ physically three >>>> devices.) >>> >>> Right, I missed that the leds/common.txt describes child node. >>> >>> I propose following modifications to the binding: >>> >>> Optional properties for child nodes: >>> - iout-mode-led : maximum intensity in microamperes of the LED >>> (torch LED for flash devices) >>> - iout-mode-flash : initial intensity in microamperes of the >>> flash LED; it is required to enable support >>> for the flash led >>> - iout-mode-indicator : initial intensity in microamperes of the >>> indicator LED; it is required to enable support >>> for the indicator led >>> - max-iout-mode-led : maximum intensity in microamperes of the LED >>> (torch LED for flash devices) >>> - max-iout-mode-flash : maximum intensity in microamperes of the >>> flash LED >>> - max-iout-mode-indicator : maximum intensity in microamperes of the >>> indicator LED >>> - flash-timeout : timeout in microseconds after which flash >>> led is turned off >> >> Ok, I took a look, and "iout" is notation I understand, but people may >> have trouble with and I don't see it used anywhere else. >> >> Also... do we need both "current" and "max-current" properties? >> >> But regulators already have "regulator-max-microamp" property. So what >> about: >> >> max-microamp : maximum intensity in microamperes of the LED >> (torch LED for flash devices) >> max-flash-microamp : initial intensity in microamperes of the >> flash LED; it is required to enable support >> for the flash led >> flash-timeout-microseconds : timeout in microseconds after which flash >> led is turned off >> >> If you had indicator on the same led, I guess >> >> indicator-microamp : recommended intensity in microamperes of the LED >> for indication >> >> ...would do? > > > Ongoing discussion allowed me for taking a look at the indicator issue > from different perspective. This is also vital for the issue of > whether a v4l2-flash sub-device should be created per device or > per sub-led [1]. > > Currently each sub-led is represented as a separate device tree > sub node and the led drivers create separate LED class device for the > sub nodes. What this implies is that indicator led also must be > represented by the separate LED class device. > > This is contrary to the way how V4L2 Flash API approaches this issue, > as it considers a flash device as a regulator chip driven through > a bus. The API allows to set the led in torch or flash mode and > implicitly assumes that there can be additional indicator led > supported, which can't be turned on separately, but the drivers apply > the indicator current to the indicator led when the torch or flash led > is activated. > > I propose to create separate v4l2-flash device for the indicator led, > and treat it as a regular sub-led similarly like it is done in the > LED subsystem. LED Flash class driver would only add a flag > LED_DEV_CAP_INDICATOR and basing on it the v4l2-flash sub-device > would create only V4L2_CID_FLASH_INDICATOR_INTENSITY control for it. > There could ba also additional control added: > V4L2_CID_FLASH_INDICATOR_PATTERN to support the feature > supported by some LED class drivers. > > From the media device perspective such an approach would > be harmful, as the indicator led could be turned on right
I intended here "wouldn't be harmful".
> before strobing the flash or turning the torch on, by > separate calls to different v4l2-flash sub-devices. > > The design described above would allow for avoiding issues I touched > in the message [1]. > > Regarding DT documentation: > > I would also swap the segments of a property name to follow the > convention as in case of "regulator-max-microamp". > > Updated version: > > ========================================================== > > Optional properties for child nodes: > - max-microamp : maximum intensity in microamperes of the LED > (torch LED for flash devices) > - flash-max-microamp : maximum intensity in microamperes of the > flash LED; it is mandatory if the led should > support the flash mode > - flash-timeout-microsec : timeout in microseconds after which the flash > led is turned off > - indicator-pattern : identifier of the blinking pattern for the > indicator led > > ========================================================== > > Regards, > Jacek > > [1] > http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/84643 >
| |