Hi Bryan,

On 10/17/2013 02:17 AM, Bryan Wu wrote:
On Tue, Oct 15, 2013 at 6:49 PM, Milo Kim <milo....@ti.com> wrote:
Hi Bryan,


On 10/16/2013 03:37 AM, Bryan Wu wrote:

On Fri, Oct 11, 2013 at 12:38 AM, Laurent Pinchart
<laurent.pinch...@ideasonboard.com> wrote:

Hi Bryan,

On Thursday 10 October 2013 17:02:18 Bryan Wu wrote:

On Mon, Oct 7, 2013 at 3:24 PM, Laurent Pinchart wrote:

On Tuesday 08 October 2013 00:06:23 Sakari Ailus wrote:

On Tue, Sep 24, 2013 at 11:20:53AM +0200, Thierry Reding wrote:

On Mon, Sep 23, 2013 at 10:27:06PM +0200, Sylwester Nawrocki wrote:

On 09/23/2013 06:37 PM, Oliver Schinagl wrote:

On 09/23/13 16:45, Sylwester Nawrocki wrote:

Hi,

I would like to have a short discussion on LED flash devices
support
in the kernel. Currently there are two APIs: the V4L2 and LED
class
API exposed by the kernel, which I believe is not good from user
space POV. Generic applications will need to implement both APIs.
I
think we should decide whether to extend the led class API to add
support for more advanced LED controllers there or continue to use
the both APIs with overlapping functionality. There has been some
discussion about this on the ML, but without any consensus reached
[1].


What about the linux-pwm framework and its support for the
backlight
via dts?

Or am I talking way to uninformed here. Copying backlight to
flashlight with some minor modification sounds sensible in a way...


I'd assume we don't need yet another user interface for the LEDs ;)
AFAICS the PWM subsystem exposes pretty much raw interface in sysfs.
The PWM LED controllers are already handled in the leds-class API,
there is the leds_pwm driver (drivers/leds/leds-pwm.c).

I'm adding linux-pwm and linux-leds maintainers at Cc so someone may
correct me if I got anything wrong.


The PWM subsystem is most definitely not a good fit for this. The
only
thing it provides is a way for other drivers to access a PWM device
and
use it for some specific purpose (pwm-backlight, leds-pwm).

The sysfs support is a convenience for people that needs to use a PWM
in a way for which no driver framework exists, or for which it
doesn't
make sense to write a driver. Or for testing.

Presumably, what we need is a few enhancements to support in a
standard way devices like MAX77693, LM3560 or MAX8997.  There is
already a led class driver for the MAX8997 LED controller
(drivers/leds/leds-max8997.c), but it uses some device-specific
sysfs
attributes.

Thus similar devices are currently being handled by different
subsystems. The split between the V4L2 Flash and the leds class API
WRT to Flash LED controller drivers is included in RFC [1], it seems
still up to date.

[1] http://www.spinics.net/lists/linux-leds/msg00899.html


Perhaps it would make sense for V4L2 to be able to use a LED as
exposed
by the LED subsystem and wrap it so that it can be integrated with
V4L2? If functionality is missing from the LED subsystem I suppose
that
could be added.


The V4L2 flash API supports also xenon flashes, not only LED ones.
That
said, I agree there's a common subset of functionality most LED flash
controllers implement.

If I understand correctly, the V4L2 subsystem uses LEDs as flashes
for
camera devices. I can easily imagine that there are devices out there
which provide functionality beyond what a regular LED will provide.
So
perhaps for things such as mobile phones, which typically use a plain
LED to illuminate the surroundings, an LED wrapped into something
that
emulates the flash functionality could work. But I doubt that the LED
subsystem is a good fit for anything beyond that.


I originally thought one way to do this could be to make it as easy as
possible to support both APIs in driver which some aregued, to which I
agree, is rather poor desing.

Does the LED API have a user space interface library like libv4l2? If
yes, one option oculd be to implement the wrapper between the V4L2 and
LED APIs there so that the applications using the LED API could also
access those devices that implement the V4L2 flash API. Torch mode
functionality is common between the two right now AFAIU,

The V4L2 flash API also provides a way to strobe the flash using an
external trigger which typically connected to the sensor (and the user
can choose between that and software strobe). I guess that and Xenon
flashes aren't currently covered by the LED API.


The issue is that we have a LED API targetted at controlling LEDs, a
V4L2
flash API targetted at controlling flashes, and hardware devices
somewhere
in the middle that can be used to provide LED or flash function.
Merging
the two APIs on the kernel side, with a compatibility layer for both
kernel space and user space APIs, might be an idea worth investigating.


I'm so sorry for jumping in the discussion so late. Some how the
emails from linux-media was archived in my Gmail and I haven't
checkout this for several weeks.

I agree right now LED API doesn't  quite fit for the usage of V4L2
Flash API. But I'd also like to see a unified API.

Currently, LED API are exported to user space as sysfs interface,
while V4L2 Flash APIs are like IOCTL and user space library. We also
merged some LED Flash trigger into LED subsystem. My basic idea is
what about creating or expanding the LED Flash trigger driver and
provide a well defined sysfs interface, which can be wrapped into user
space libv4l2.


The biggest reason why we're not fond of sysfs-based APIs for media
devices is
that they can't provide atomicity. There's no way to set multiple
parameters
in a single operation.

We can't get rid of the sysfs LEDs API, but maybe we could have a unified
kernel LED/flash subsystem that would provide both a sysfs-based API to
ensure
compatibility with current userspace software and an ioctl-based API
(possibly
through V4L2 controls). That way LED/flash devices would be registered
with a
single subsystem, and the corresponding drivers won't have to care about
the
API exposed to userspace. That would require a major refactoring of the
in-
kernel APIs though.


I agree this. I'm thinking about expanding the ledtrig-camera.c
created by Milo Kim. This trigger will provide flashing and strobing
control of a LED device and for sure the LED device driver like
drivers/leds/leds-lm355x.c.

So we basically can do this:
1. add V4L2 Flash subdev into ledtrig-camera.c. So this trigger driver
can provide trigger API to kernel drivers as well as V4L2 Flash API to
userspace.
2. add the real flash torch functions into LED device driver like
leds-lm355x.c, this driver will still provide sysfs interface and
extended flash/torch control sysfs interface as well.

I'm not sure about whether we need some change in V4L2 internally. But
actually Andrzej Hajda's patchset is quite straightforward, but we
just need put those V4L2 Flash API into a LED trigger driver and the
real flash/torch operation in a LED device driver.

Milo, could you please give some comments here?


General LED trigger APIs were created not for the application interface but
for any kernel space driver.
The LED camera trigger APIs are used by a camera driver, not application.


That's basically correct, but trigger sometime can also provide sysfs
interface which might be used by user space app.

Actually this camera flash/torch trigger API can also be used by V4L2
Flash subdev.
We create a V4L2 Flash subdev in the driver, which will expose V4L2
API to user space. And this V4L2 Flash subdev will use this
flash/torch trigger API to talk with our LED core and it really
doesn't need to know the details about the LED flash/torch chip, if we
can provide a good interface between trigger and LED device driver.

So benefits are
a) one trigger/V4L2 Flash subdev driver can be used by multiple LED chips
b) LED chip driver just need to provide standard or extended LED API
to support flash/torch
c) LED chip driver still keep those LED sysfs interface to user space
and won't break user space application

Some LED devices provide basic LED functionalities and high current features
like a flash and a torch.(eg. LM3554, LM3642)
The reason why I added the LED camera trigger is
   "for providing multiple operations(LEDs, flash and torch) by one LED
device driver".

For example,
A LED indicator is controlled via the LED sysfs.
And flash and torch are controlled by a camera driver - calls exported LED
trigger function, ledtrig_flash_ctrl().

My understanding is the V4L2 subsystem provides rich IOCTLs for the media
device.
I agree that the V4L2 is more proper interface for camera *application*.

So, my suggestion is:
   - If a device has only flash/torch functionalities, then register the
driver as the V4L2 sub-device.
   - If a device provides not only flash/torch but also LED features, then
create the driver as the MFD.


We really don't need to separate them, one LED device driver can
provide flash/torch/normal functions in on driver. I think LED device
driver is trying to provide the LED chip's hardware functions, like
flash/torch/indicator etc. how to use it, we can choose different
trigger. That gives us the maxim flexibility.

For example, LM3555 (and AS3645A) is used only for the camera.
Then, this driver is registered as the V4L2 sub-device.
(drivers/media/i2c/as3645a.c) - no change at all.


That's current solution, we plan to unify this two API since those
chip are basically LED.

On the other hands, LM3642 has an indicator mode with flash/torch.
Then, it will consist of 3 parts - MFD core, LED(indicator) and
V4L2(flash/torch).


So if one LED device driver can support that, we don't need these 3 parts.

Let me clarify our discussion briefly.

For the flash and torch, there are scattered user-space APIs.
We need to unify them.

We are considering supporting V4L2 structures in the LED camera trigger.
Then, camera application controls the flash/torch via not the LED sysfs but the V4L2 ioctl interface.
So, changing point is the ledtrig-camera.c. No chip driver changes at all.

Is it correct?

Best regards,
Milo

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

Reply via email to