Re: [RFC 0/2] V4L2 API for exposing flash subdevs as LED class device

[Sakari Ailus <sakari.ailus@iki.fi>]

Hi all,

On Wed, May 08, 2013 at 09:32:17AM +0200, Andrzej Hajda wrote:
> On 07.05.2013 17:07, Laurent Pinchart wrote:
> > On Tuesday 07 May 2013 02:11:27 Kim, Milo wrote:
> >> On Monday, May 06, 2013 6:34 PM Andrzej Hajda wrote:
> >>> This RFC proposes generic API for exposing flash subdevices via LED
> >>> framework.
> >>>
> >>> Rationale
> >>>
> >>> Currently there are two frameworks which are used for exposing LED
> >>> flash to user space:
> >>> - V4L2 flash controls,
> >>> - LED framework(with custom sysfs attributes).
> >>>
> >>> The list below shows flash drivers in mainline kernel with initial
> >>> commit date and typical chip application (according to producer):
> >>>
> >>> LED API:
> >>>     lm3642: 2012-09-12, Cameras
> >>>     lm355x: 2012-09-05, Cameras
> >>>     max8997: 2011-12-14, Cameras (?)
> >>>     lp3944: 2009-06-19, Cameras, Lights, Indicators, Toys
> >>>     pca955x: 2008-07-16, Cameras, Indicators (?)
> >>>
> >>> V4L2 API:
> >>>     as3645a:  2011-05-05, Cameras
> >>>     adp1653: 2011-05-05, Cameras
> >>>
> >>> V4L2 provides richest functionality, but there is often demand from
> >>> application developers to provide already established LED API. We would
> >>> like to have an unified user interface for flash devices. Some of devices
> >>> already have the LED API driver exposing limited set of a Flash IC
> >>> functionality. In order to support all required features the LED API
> >>> would have to be extended or the V4L2 API would need to be used. However
> >>> when switching from a LED to a V4L2 Flash driver existing LED API
> >>> interface would need to be retained.
> >>>
> >>> Proposed solution
> >>>
> >>> This patch adds V4L2 helper functions to register existing V4L2 flash
> >>> subdev as LED class device. After registration via v4l2_leddev_register
> >>> appropriate entry in /sys/class/leds/ is created. During registration all
> >>> V4L2 flash controls are enumerated and corresponding attributes are added.
> >>>
> >>> I have attached also patch with new max77693-led driver using v4l2_leddev.
> >>> This patch requires presence of the patch "max77693: added device tree
> >>> support": https://patchwork.kernel.org/patch/2414351/ .
> >>>
> >>> Additional features
> >>>
> >>> - simple API to access all V4L2 flash controls via sysfs,
> >>> - V4L2 subdevice should not be registered by V4L2 device to use it,
> >>> - LED triggers API can be used to control the device,
> >>> - LED device is optional - it will be created only if V4L2_LEDDEV
> >>>   configuration option is enabled and the subdev driver calls
> >>>   v4l2_leddev_register.
> >>>
> >>> Doubts
> >>>
> >>> This RFC is a result of a uncertainty which API developers should expose
> >>> by their flash drivers. It is a try to gluing together both APIs. I am not
> >>> sure if it is the best solution, but I hope there will be some discussion
> >>> and hopefully some decisions will be taken which way we should follow.
> >> The LED subsystem provides similar APIs for the Camera driver.
> >> With LED trigger event, flash and torch are enabled/disabled.
> >> I'm not sure this is applicable for you.
> >> Could you take a look at LED camera trigger feature?
> >>
> >> For the camera LED trigger,
> >> https://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git/commit/
> >> ?h=f or-next&id=48a1d032c954b9b06c3adbf35ef4735dd70ab757
> >>
> >> Example of camera flash driver,
> >> https://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git/commit/
> >> ?h=f or-next&id=313bf0b1a0eaeaac17ea8c4b748f16e28fce8b7a
> > I think we should decide on one API. Implementing two APIs for a single device 
> > is usually messy, and will result in different feature sets (and different 
> > bugs) being implemented through each API, depending on the driver. 
> > Interactions between the APIs are also a pain point on the kernel side to 
> > properly synchronize calls.

I don't like having two APIs either. Especially we shouldn't have multiple
drivers implementing different APIs for the same device.

That said, I wonder if it's possible to support camera-related use cases
using the LED API: it's originally designed for quite different devices.
Even if you could handle flash strobing using the LED API, the functionality
provided by the Media controller and subdev APIs will always be missing:
device enumeration and association with the right camera.

The strobe functionality is connected to the sensor (hardware strobe pin)
which also is why it's natural for the flash to be accessible over the V4L2
API.

This was briefly discussed some time ago:

<URL:http://www.spinics.net/lists/linux-media/msg53645.html>

> > The LED API is too limited for torch and flash usage, but I'm definitely open 
> > to moving flash devices to the LED API is we can extend it in a way that it 
> > covers all the use cases.
> >
> Extending LED API IMHO seems to be quite straightforward - by adding
> attributes for supported functionalities. We just need a specification for
> standard flash/torch attributes.
> I could prepare an RFC about it if there is a will to explore this
> direction.

I'm leaning towards providing a wrapper that provides torch functionality
using V4L2 flash API unless it's really proven to be insane. ;-) The code
supporting that in an individual flash driver should be minimal --- which is
what the patchset essentially already does.

There are caveats though, such as if powering the flash controller requires
a regulator, then power management also must be done. Albeit I still
wouldn't expect this to be a major issue.

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