On Wed, Oct 16, 2013 at 4:54 PM, Bryan Wu <cooloney@xxxxxxxxx> wrote: > 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 Remove my old email address. -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
