Re: [PATCH v5 06/10] media: Add registration helpers for V4L2 flash sub-devices

2015-04-24 Thread Sakari Ailus
Hi Jacek,

On Fri, Apr 24, 2015 at 12:29:17PM +0200, Jacek Anaszewski wrote:
> Hi Sakari,
> 
> On Fri, 24 Apr 2015 00:52:12 +0300
> Sakari Ailus  wrote:
> 
> > Hi Jacek,
> > 
> > Jacek Anaszewski wrote:
> > ...
> > >>> +#define call_flash_op(v4l2_flash, op,
> > >>> arg)\
> > >>> +   (has_flash_op(v4l2_flash,
> > >>> op) ?   \
> > >>> +   v4l2_flash->ops->op(v4l2_flash,
> > >>> arg) :  \
> > >>> +   -EINVAL)
> > >>> +
> > >>> +static enum led_brightness __intensity_to_led_brightness(
> > >>> +   struct v4l2_ctrl *ctrl,
> > >>> +   s32 intensity)
> > >>
> > >> Fits on previous line.
> > >>
> > >>> +{
> > >>> +   s64 intensity64 = intensity - ctrl->minimum;
> > >>
> > >> intensity, ctrl->step and ctrl->minimum are 32-bit signed integers.
> > >> Do you need a 64-bit integer here?
> > > 
> > > step is u64.
> > 
> > Nevertheless integer controls will not have values outside the s32
> > range, using a step value that's outside the range makes no sense
> > either. I think you should use s32 instead.
> 
> I infer that local u32 variable should be assigned ctrl->step,
> and then used as a divisor.

You could cast explicitly as well. Either is fine for me.

> 
> > > 
> > >>
> > >>> +
> > >>> +   do_div(intensity64, ctrl->step);
> > >>> +
> > >>> +   /*
> > >>> +* Indicator LEDs, unlike torch LEDs, are turned on/off
> > >>> basing on
> > >>> +* the state of V4L2_CID_FLASH_INDICATOR_INTENSITY
> > >>> control only.
> > >>> +* Therefore it must be possible to set it to 0 level
> > >>> which in
> > >>> +* the LED subsystem reflects LED_OFF state.
> > >>> +*/
> > >>> +   if (ctrl->id != V4L2_CID_FLASH_INDICATOR_INTENSITY)
> > >>> +   ++intensity64;
> > >>
> > >> I think the condition could simply be ctrl->minimum instead, that
> > >> way I find it easier to understand what's happening here. I'd
> > >> expect the minimum for non-intensity controls always to be
> > >> non-zero, though, so the end result is the same. Up to you.
> > > 
> > > Minimum for indicator control must be 0 to make possible
> > > turning the indicator LED off only with this control.
> > 
> > Would torch be still on if the minimum torch current was 0 mA? I'd
> > say no.
> > 
> > Although in that case I'd expect the driver to use a different range,
> > and selecting the off mode would then turn it off, I still think
> > that's a better condition than relying on the control id.
> 
> I didn't catch your point previously. Probably you was thinking
> about somethig like this:
> 
> if (ctrl->minimum)
>   ++intensity;
> 
> If so, I agree.

Yes, that's what I meant.

> 
> > ...
> > 
> > >>> +static int v4l2_flash_g_volatile_ctrl(struct v4l2_ctrl *c)
> > >>> +{
> > >>> +   struct v4l2_flash *v4l2_flash =
> > >>> v4l2_ctrl_to_v4l2_flash(c);
> > >>> +   struct led_classdev_flash *fled_cdev =
> > >>> v4l2_flash->fled_cdev;
> > >>> +   bool is_strobing;
> > >>> +   int ret;
> > >>> +
> > >>> +   switch (c->id) {
> > >>> +   case V4L2_CID_FLASH_TORCH_INTENSITY:
> > >>> +   case V4L2_CID_FLASH_INDICATOR_INTENSITY:
> > >>> +   return
> > >>> v4l2_flash_update_led_brightness(v4l2_flash, c);
> > >>> +   case V4L2_CID_FLASH_INTENSITY:
> > >>> +   ret = led_update_flash_brightness(fled_cdev);
> > >>> +   if (ret < 0)
> > >>> +   return ret;
> > >>> +   /* no conversion is needed */
> > >>
> > >> Maybe a stupid question, but why is it not needed?
> > > 
> > > Because LED Flash class also uses microamperes.
> > 
> > Right, I had missed that. It'd be nice if that was said in the
> > comment, it might not be obvious to others either.
> 
> OK, I will add the comment.

Thanks!

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
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


Re: [PATCH v5 06/10] media: Add registration helpers for V4L2 flash sub-devices

2015-04-24 Thread Jacek Anaszewski
Hi Sakari,

On Fri, 24 Apr 2015 00:52:12 +0300
Sakari Ailus  wrote:

> Hi Jacek,
> 
> Jacek Anaszewski wrote:
> ...
> >>> +#define call_flash_op(v4l2_flash, op,
> >>> arg)  \
> >>> + (has_flash_op(v4l2_flash,
> >>> op) ? \
> >>> + v4l2_flash->ops->op(v4l2_flash,
> >>> arg) :\
> >>> + -EINVAL)
> >>> +
> >>> +static enum led_brightness __intensity_to_led_brightness(
> >>> + struct v4l2_ctrl *ctrl,
> >>> + s32 intensity)
> >>
> >> Fits on previous line.
> >>
> >>> +{
> >>> + s64 intensity64 = intensity - ctrl->minimum;
> >>
> >> intensity, ctrl->step and ctrl->minimum are 32-bit signed integers.
> >> Do you need a 64-bit integer here?
> > 
> > step is u64.
> 
> Nevertheless integer controls will not have values outside the s32
> range, using a step value that's outside the range makes no sense
> either. I think you should use s32 instead.

I infer that local u32 variable should be assigned ctrl->step,
and then used as a divisor.

> > 
> >>
> >>> +
> >>> + do_div(intensity64, ctrl->step);
> >>> +
> >>> + /*
> >>> +  * Indicator LEDs, unlike torch LEDs, are turned on/off
> >>> basing on
> >>> +  * the state of V4L2_CID_FLASH_INDICATOR_INTENSITY
> >>> control only.
> >>> +  * Therefore it must be possible to set it to 0 level
> >>> which in
> >>> +  * the LED subsystem reflects LED_OFF state.
> >>> +  */
> >>> + if (ctrl->id != V4L2_CID_FLASH_INDICATOR_INTENSITY)
> >>> + ++intensity64;
> >>
> >> I think the condition could simply be ctrl->minimum instead, that
> >> way I find it easier to understand what's happening here. I'd
> >> expect the minimum for non-intensity controls always to be
> >> non-zero, though, so the end result is the same. Up to you.
> > 
> > Minimum for indicator control must be 0 to make possible
> > turning the indicator LED off only with this control.
> 
> Would torch be still on if the minimum torch current was 0 mA? I'd
> say no.
> 
> Although in that case I'd expect the driver to use a different range,
> and selecting the off mode would then turn it off, I still think
> that's a better condition than relying on the control id.

I didn't catch your point previously. Probably you was thinking
about somethig like this:

if (ctrl->minimum)
++intensity;

If so, I agree.

> ...
> 
> >>> +static int v4l2_flash_g_volatile_ctrl(struct v4l2_ctrl *c)
> >>> +{
> >>> + struct v4l2_flash *v4l2_flash =
> >>> v4l2_ctrl_to_v4l2_flash(c);
> >>> + struct led_classdev_flash *fled_cdev =
> >>> v4l2_flash->fled_cdev;
> >>> + bool is_strobing;
> >>> + int ret;
> >>> +
> >>> + switch (c->id) {
> >>> + case V4L2_CID_FLASH_TORCH_INTENSITY:
> >>> + case V4L2_CID_FLASH_INDICATOR_INTENSITY:
> >>> + return
> >>> v4l2_flash_update_led_brightness(v4l2_flash, c);
> >>> + case V4L2_CID_FLASH_INTENSITY:
> >>> + ret = led_update_flash_brightness(fled_cdev);
> >>> + if (ret < 0)
> >>> + return ret;
> >>> + /* no conversion is needed */
> >>
> >> Maybe a stupid question, but why is it not needed?
> > 
> > Because LED Flash class also uses microamperes.
> 
> Right, I had missed that. It'd be nice if that was said in the
> comment, it might not be obvious to others either.

OK, I will add the comment.

-- 
Best Regards,
Jacek Anaszewski
--
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


Re: [PATCH v5 06/10] media: Add registration helpers for V4L2 flash sub-devices

2015-04-23 Thread Sakari Ailus
Hi Jacek,

Jacek Anaszewski wrote:
...
>>> +#define call_flash_op(v4l2_flash, op, arg) \
>>> +   (has_flash_op(v4l2_flash,
>>> op) ?   \
>>> +   v4l2_flash->ops->op(v4l2_flash,
>>> arg) :  \
>>> +   -EINVAL)
>>> +
>>> +static enum led_brightness __intensity_to_led_brightness(
>>> +   struct v4l2_ctrl *ctrl,
>>> +   s32 intensity)
>>
>> Fits on previous line.
>>
>>> +{
>>> +   s64 intensity64 = intensity - ctrl->minimum;
>>
>> intensity, ctrl->step and ctrl->minimum are 32-bit signed integers.
>> Do you need a 64-bit integer here?
> 
> step is u64.

Nevertheless integer controls will not have values outside the s32
range, using a step value that's outside the range makes no sense
either. I think you should use s32 instead.

> 
>>
>>> +
>>> +   do_div(intensity64, ctrl->step);
>>> +
>>> +   /*
>>> +* Indicator LEDs, unlike torch LEDs, are turned on/off
>>> basing on
>>> +* the state of V4L2_CID_FLASH_INDICATOR_INTENSITY control
>>> only.
>>> +* Therefore it must be possible to set it to 0 level
>>> which in
>>> +* the LED subsystem reflects LED_OFF state.
>>> +*/
>>> +   if (ctrl->id != V4L2_CID_FLASH_INDICATOR_INTENSITY)
>>> +   ++intensity64;
>>
>> I think the condition could simply be ctrl->minimum instead, that way
>> I find it easier to understand what's happening here. I'd expect the
>> minimum for non-intensity controls always to be non-zero, though, so
>> the end result is the same. Up to you.
> 
> Minimum for indicator control must be 0 to make possible
> turning the indicator LED off only with this control.

Would torch be still on if the minimum torch current was 0 mA? I'd say no.

Although in that case I'd expect the driver to use a different range,
and selecting the off mode would then turn it off, I still think that's
a better condition than relying on the control id.

...

>>> +static int v4l2_flash_g_volatile_ctrl(struct v4l2_ctrl *c)
>>> +{
>>> +   struct v4l2_flash *v4l2_flash = v4l2_ctrl_to_v4l2_flash(c);
>>> +   struct led_classdev_flash *fled_cdev =
>>> v4l2_flash->fled_cdev;
>>> +   bool is_strobing;
>>> +   int ret;
>>> +
>>> +   switch (c->id) {
>>> +   case V4L2_CID_FLASH_TORCH_INTENSITY:
>>> +   case V4L2_CID_FLASH_INDICATOR_INTENSITY:
>>> +   return
>>> v4l2_flash_update_led_brightness(v4l2_flash, c);
>>> +   case V4L2_CID_FLASH_INTENSITY:
>>> +   ret = led_update_flash_brightness(fled_cdev);
>>> +   if (ret < 0)
>>> +   return ret;
>>> +   /* no conversion is needed */
>>
>> Maybe a stupid question, but why is it not needed?
> 
> Because LED Flash class also uses microamperes.

Right, I had missed that. It'd be nice if that was said in the comment,
it might not be obvious to others either.

-- 
Kind regards,

Sakari Ailus
sakari.ai...@iki.fi
--
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


Re: [PATCH v5 06/10] media: Add registration helpers for V4L2 flash sub-devices

2015-04-23 Thread Jacek Anaszewski
On Thu, 23 Apr 2015 10:40:09 +0300
Hi Sakari,

Thanks for the review.

Sakari Ailus  wrote:

> Hi Jacek,
> 
> On Wed, Apr 15, 2015 at 08:48:36AM +0200, Jacek Anaszewski wrote:
> > This patch adds helper functions for registering/unregistering
> > LED Flash class devices as V4L2 sub-devices. The functions should
> > be called from the LED subsystem device driver. In case the
> > support for V4L2 Flash sub-devices is disabled in the kernel
> > config the functions' empty versions will be used.
> > 
> > Signed-off-by: Jacek Anaszewski 
> > Acked-by: Kyungmin Park 
> > Cc: Sakari Ailus 
> > Cc: Hans Verkuil 
> > ---
> >  drivers/media/v4l2-core/Kconfig  |   11 +
> >  drivers/media/v4l2-core/Makefile |2 +
> >  drivers/media/v4l2-core/v4l2-flash.c |  619
> > ++
> > include/media/v4l2-flash.h   |  145  4 files
> > changed, 777 insertions(+) create mode 100644
> > drivers/media/v4l2-core/v4l2-flash.c create mode 100644
> > include/media/v4l2-flash.h
> > 
> > diff --git a/drivers/media/v4l2-core/Kconfig
> > b/drivers/media/v4l2-core/Kconfig index ba7e21a..f034f1a 100644
> > --- a/drivers/media/v4l2-core/Kconfig
> > +++ b/drivers/media/v4l2-core/Kconfig
> > @@ -44,6 +44,17 @@ config V4L2_MEM2MEM_DEV
> >  tristate
> >  depends on VIDEOBUF2_CORE
> >  
> > +# Used by LED subsystem flash drivers
> > +config V4L2_FLASH_LED_CLASS
> > +   tristate "Enable support for Flash sub-devices"
> 
> How about: "V4L2 flash API for LED flash class devices"?

OK.

> > +   depends on VIDEO_V4L2_SUBDEV_API
> > +   depends on LEDS_CLASS_FLASH
> > +   ---help---
> > + Say Y here to enable support for Flash sub-devices,
> > which allow
> > + to control LED class devices with use of V4L2 Flash
> > controls.
> 
> How about this: "Say Y here to enable V4L2 flash API support for LED
> flash class drivers".

OK.

> > +
> > + When in doubt, say N.
> > +
> >  # Used by drivers that need Videobuf modules
> >  config VIDEOBUF_GEN
> > tristate
> > diff --git a/drivers/media/v4l2-core/Makefile
> > b/drivers/media/v4l2-core/Makefile index 63d29f2..44e858c 100644
> > --- a/drivers/media/v4l2-core/Makefile
> > +++ b/drivers/media/v4l2-core/Makefile
> > @@ -22,6 +22,8 @@ obj-$(CONFIG_VIDEO_TUNER) += tuner.o
> >  
> >  obj-$(CONFIG_V4L2_MEM2MEM_DEV) += v4l2-mem2mem.o
> >  
> > +obj-$(CONFIG_V4L2_FLASH_LED_CLASS) += v4l2-flash.o
> > +
> >  obj-$(CONFIG_VIDEOBUF_GEN) += videobuf-core.o
> >  obj-$(CONFIG_VIDEOBUF_DMA_SG) += videobuf-dma-sg.o
> >  obj-$(CONFIG_VIDEOBUF_DMA_CONTIG) += videobuf-dma-contig.o
> > diff --git a/drivers/media/v4l2-core/v4l2-flash.c
> > b/drivers/media/v4l2-core/v4l2-flash.c new file mode 100644
> > index 000..bed2036
> > --- /dev/null
> > +++ b/drivers/media/v4l2-core/v4l2-flash.c
> 
> I might rename this as v4l2-flash-led-class.c to match the Kconfig
> option.

Agreed as well.

> > @@ -0,0 +1,619 @@
> > +/*
> > + * V4L2 Flash LED sub-device registration helpers.
> > + *
> > + * Copyright (C) 2015 Samsung Electronics Co., Ltd
> > + * Author: Jacek Anaszewski 
> > + *
> > + * This program is free software; you can redistribute it and/or
> > modify
> > + * it under the terms of the GNU General Public License version 2
> > as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#define has_flash_op(v4l2_flash,
> > op) \
> > +   (v4l2_flash && v4l2_flash->ops->op)
> > +
> > +#define call_flash_op(v4l2_flash, op, arg) \
> > +   (has_flash_op(v4l2_flash,
> > op) ?   \
> > +   v4l2_flash->ops->op(v4l2_flash,
> > arg) :  \
> > +   -EINVAL)
> > +
> > +static enum led_brightness __intensity_to_led_brightness(
> > +   struct v4l2_ctrl *ctrl,
> > +   s32 intensity)
> 
> Fits on previous line.
> 
> > +{
> > +   s64 intensity64 = intensity - ctrl->minimum;
> 
> intensity, ctrl->step and ctrl->minimum are 32-bit signed integers.
> Do you need a 64-bit integer here?

step is u64.

> 
> > +
> > +   do_div(intensity64, ctrl->step);
> > +
> > +   /*
> > +* Indicator LEDs, unlike torch LEDs, are turned on/off
> > basing on
> > +* the state of V4L2_CID_FLASH_INDICATOR_INTENSITY control
> > only.
> > +* Therefore it must be possible to set it to 0 level
> > which in
> > +* the LED subsystem reflects LED_OFF state.
> > +*/
> > +   if (ctrl->id != V4L2_CID_FLASH_INDICATOR_INTENSITY)
> > +   ++intensity64;
> 
> I think the condition could simply be ctrl->minimum instead, that way
> I find it easier to understand what's happening here. I'd expect the
> minimum for non-intensity controls always to be non-zero, though, so
> the end result is the same. Up to you.

Minimum for indicator control must be 0 to make possible
turning the indicator LED of

Re: [PATCH v5 06/10] media: Add registration helpers for V4L2 flash sub-devices

2015-04-23 Thread Sakari Ailus
Hi Jacek,

On Wed, Apr 15, 2015 at 08:48:36AM +0200, Jacek Anaszewski wrote:
> This patch adds helper functions for registering/unregistering
> LED Flash class devices as V4L2 sub-devices. The functions should
> be called from the LED subsystem device driver. In case the
> support for V4L2 Flash sub-devices is disabled in the kernel
> config the functions' empty versions will be used.
> 
> Signed-off-by: Jacek Anaszewski 
> Acked-by: Kyungmin Park 
> Cc: Sakari Ailus 
> Cc: Hans Verkuil 
> ---
>  drivers/media/v4l2-core/Kconfig  |   11 +
>  drivers/media/v4l2-core/Makefile |2 +
>  drivers/media/v4l2-core/v4l2-flash.c |  619 
> ++
>  include/media/v4l2-flash.h   |  145 
>  4 files changed, 777 insertions(+)
>  create mode 100644 drivers/media/v4l2-core/v4l2-flash.c
>  create mode 100644 include/media/v4l2-flash.h
> 
> diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
> index ba7e21a..f034f1a 100644
> --- a/drivers/media/v4l2-core/Kconfig
> +++ b/drivers/media/v4l2-core/Kconfig
> @@ -44,6 +44,17 @@ config V4L2_MEM2MEM_DEV
>  tristate
>  depends on VIDEOBUF2_CORE
>  
> +# Used by LED subsystem flash drivers
> +config V4L2_FLASH_LED_CLASS
> + tristate "Enable support for Flash sub-devices"

How about: "V4L2 flash API for LED flash class devices"?

> + depends on VIDEO_V4L2_SUBDEV_API
> + depends on LEDS_CLASS_FLASH
> + ---help---
> +   Say Y here to enable support for Flash sub-devices, which allow
> +   to control LED class devices with use of V4L2 Flash controls.

How about this: "Say Y here to enable V4L2 flash API support for LED flash
class drivers".

> +
> +   When in doubt, say N.
> +
>  # Used by drivers that need Videobuf modules
>  config VIDEOBUF_GEN
>   tristate
> diff --git a/drivers/media/v4l2-core/Makefile 
> b/drivers/media/v4l2-core/Makefile
> index 63d29f2..44e858c 100644
> --- a/drivers/media/v4l2-core/Makefile
> +++ b/drivers/media/v4l2-core/Makefile
> @@ -22,6 +22,8 @@ obj-$(CONFIG_VIDEO_TUNER) += tuner.o
>  
>  obj-$(CONFIG_V4L2_MEM2MEM_DEV) += v4l2-mem2mem.o
>  
> +obj-$(CONFIG_V4L2_FLASH_LED_CLASS) += v4l2-flash.o
> +
>  obj-$(CONFIG_VIDEOBUF_GEN) += videobuf-core.o
>  obj-$(CONFIG_VIDEOBUF_DMA_SG) += videobuf-dma-sg.o
>  obj-$(CONFIG_VIDEOBUF_DMA_CONTIG) += videobuf-dma-contig.o
> diff --git a/drivers/media/v4l2-core/v4l2-flash.c 
> b/drivers/media/v4l2-core/v4l2-flash.c
> new file mode 100644
> index 000..bed2036
> --- /dev/null
> +++ b/drivers/media/v4l2-core/v4l2-flash.c

I might rename this as v4l2-flash-led-class.c to match the Kconfig option.

> @@ -0,0 +1,619 @@
> +/*
> + * V4L2 Flash LED sub-device registration helpers.
> + *
> + *   Copyright (C) 2015 Samsung Electronics Co., Ltd
> + *   Author: Jacek Anaszewski 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define has_flash_op(v4l2_flash, op) \
> + (v4l2_flash && v4l2_flash->ops->op)
> +
> +#define call_flash_op(v4l2_flash, op, arg)   \
> + (has_flash_op(v4l2_flash, op) ? \
> + v4l2_flash->ops->op(v4l2_flash, arg) :  \
> + -EINVAL)
> +
> +static enum led_brightness __intensity_to_led_brightness(
> + struct v4l2_ctrl *ctrl,
> + s32 intensity)

Fits on previous line.

> +{
> + s64 intensity64 = intensity - ctrl->minimum;

intensity, ctrl->step and ctrl->minimum are 32-bit signed integers. Do you
need a 64-bit integer here?

> +
> + do_div(intensity64, ctrl->step);
> +
> + /*
> +  * Indicator LEDs, unlike torch LEDs, are turned on/off basing on
> +  * the state of V4L2_CID_FLASH_INDICATOR_INTENSITY control only.
> +  * Therefore it must be possible to set it to 0 level which in
> +  * the LED subsystem reflects LED_OFF state.
> +  */
> + if (ctrl->id != V4L2_CID_FLASH_INDICATOR_INTENSITY)
> + ++intensity64;

I think the condition could simply be ctrl->minimum instead, that way I find
it easier to understand what's happening here. I'd expect the minimum for
non-intensity controls always to be non-zero, though, so the end result is
the same. Up to you.

> +
> + return intensity64;
> +}
> +
> +static s32 __led_brightness_to_intensity(struct v4l2_ctrl *ctrl,
> +  enum led_brightness brightness)
> +{
> + /*
> +  * Indicator LEDs, unlike torch LEDs, are turned on/off basing on
> +  * the state of V4L2_CID_FLASH_INDICATOR_INTENSITY control only.
> +  * Do not decrement brightness read from the LED subsystem for
> +  * indicator LED as it may equal 0.

[PATCH v5 06/10] media: Add registration helpers for V4L2 flash sub-devices

2015-04-14 Thread Jacek Anaszewski
This patch adds helper functions for registering/unregistering
LED Flash class devices as V4L2 sub-devices. The functions should
be called from the LED subsystem device driver. In case the
support for V4L2 Flash sub-devices is disabled in the kernel
config the functions' empty versions will be used.

Signed-off-by: Jacek Anaszewski 
Acked-by: Kyungmin Park 
Cc: Sakari Ailus 
Cc: Hans Verkuil 
---
 drivers/media/v4l2-core/Kconfig  |   11 +
 drivers/media/v4l2-core/Makefile |2 +
 drivers/media/v4l2-core/v4l2-flash.c |  619 ++
 include/media/v4l2-flash.h   |  145 
 4 files changed, 777 insertions(+)
 create mode 100644 drivers/media/v4l2-core/v4l2-flash.c
 create mode 100644 include/media/v4l2-flash.h

diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
index ba7e21a..f034f1a 100644
--- a/drivers/media/v4l2-core/Kconfig
+++ b/drivers/media/v4l2-core/Kconfig
@@ -44,6 +44,17 @@ config V4L2_MEM2MEM_DEV
 tristate
 depends on VIDEOBUF2_CORE
 
+# Used by LED subsystem flash drivers
+config V4L2_FLASH_LED_CLASS
+   tristate "Enable support for Flash sub-devices"
+   depends on VIDEO_V4L2_SUBDEV_API
+   depends on LEDS_CLASS_FLASH
+   ---help---
+ Say Y here to enable support for Flash sub-devices, which allow
+ to control LED class devices with use of V4L2 Flash controls.
+
+ When in doubt, say N.
+
 # Used by drivers that need Videobuf modules
 config VIDEOBUF_GEN
tristate
diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index 63d29f2..44e858c 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -22,6 +22,8 @@ obj-$(CONFIG_VIDEO_TUNER) += tuner.o
 
 obj-$(CONFIG_V4L2_MEM2MEM_DEV) += v4l2-mem2mem.o
 
+obj-$(CONFIG_V4L2_FLASH_LED_CLASS) += v4l2-flash.o
+
 obj-$(CONFIG_VIDEOBUF_GEN) += videobuf-core.o
 obj-$(CONFIG_VIDEOBUF_DMA_SG) += videobuf-dma-sg.o
 obj-$(CONFIG_VIDEOBUF_DMA_CONTIG) += videobuf-dma-contig.o
diff --git a/drivers/media/v4l2-core/v4l2-flash.c 
b/drivers/media/v4l2-core/v4l2-flash.c
new file mode 100644
index 000..bed2036
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-flash.c
@@ -0,0 +1,619 @@
+/*
+ * V4L2 Flash LED sub-device registration helpers.
+ *
+ * Copyright (C) 2015 Samsung Electronics Co., Ltd
+ * Author: Jacek Anaszewski 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define has_flash_op(v4l2_flash, op)   \
+   (v4l2_flash && v4l2_flash->ops->op)
+
+#define call_flash_op(v4l2_flash, op, arg) \
+   (has_flash_op(v4l2_flash, op) ? \
+   v4l2_flash->ops->op(v4l2_flash, arg) :  \
+   -EINVAL)
+
+static enum led_brightness __intensity_to_led_brightness(
+   struct v4l2_ctrl *ctrl,
+   s32 intensity)
+{
+   s64 intensity64 = intensity - ctrl->minimum;
+
+   do_div(intensity64, ctrl->step);
+
+   /*
+* Indicator LEDs, unlike torch LEDs, are turned on/off basing on
+* the state of V4L2_CID_FLASH_INDICATOR_INTENSITY control only.
+* Therefore it must be possible to set it to 0 level which in
+* the LED subsystem reflects LED_OFF state.
+*/
+   if (ctrl->id != V4L2_CID_FLASH_INDICATOR_INTENSITY)
+   ++intensity64;
+
+   return intensity64;
+}
+
+static s32 __led_brightness_to_intensity(struct v4l2_ctrl *ctrl,
+enum led_brightness brightness)
+{
+   /*
+* Indicator LEDs, unlike torch LEDs, are turned on/off basing on
+* the state of V4L2_CID_FLASH_INDICATOR_INTENSITY control only.
+* Do not decrement brightness read from the LED subsystem for
+* indicator LED as it may equal 0. For torch LEDs this function
+* is called only when V4L2_FLASH_LED_MODE_TORCH is set and the
+* brightness read is guaranteed to be greater than 0. In the mode
+* V4L2_FLASH_LED_MODE_NONE the cached torch intensity value is used.
+*/
+   if (ctrl->id != V4L2_CID_FLASH_INDICATOR_INTENSITY)
+   --brightness;
+
+   return (brightness * ctrl->step) + ctrl->minimum;
+}
+
+static void v4l2_flash_set_led_brightness(struct v4l2_flash *v4l2_flash,
+   struct v4l2_ctrl *ctrl)
+{
+   struct v4l2_ctrl **ctrls = v4l2_flash->ctrls;
+   enum led_brightness brightness;
+
+   if (has_flash_op(v4l2_flash, intensity_to_led_brightness))
+   brightness = call_flash_op(v4l2_flash,
+   intensity_to_led_brightness,
+