On Wed, Oct 16, 2013 at 4:36 PM, Milo Kim <milo.kim@xxxxxx> wrote: > > Hi Bryan, > > > On 10/17/2013 02:17 AM, Bryan Wu wrote: >> >> On Tue, Oct 15, 2013 at 6:49 PM, Milo Kim <milo.kim@xxxxxx> 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.pinchart@xxxxxxxxxxxxxxxx> 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. > Yeah, my proposal is to add V4L2 interface into ledtrig-camera.c. For existing chip driver like yours LM3555, I guess we don't need to big change but for future support for new chip or adding flash/torch to existing chip, we need to create or change chip driver. Because eventually those flash/torch/indicator operation happens in chip driver. Thanks, -Bryan -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
