Re: [PATCH/RFC v4 15/21] media: Add registration helpers for V4L2 flash

2014-08-21 Thread Jacek Anaszewski

Hi Sakari,

On 08/20/2014 04:41 PM, Sakari Ailus wrote:

Hi Jacek,


[...]


LED Class Flash driver gains V4L2 Flash API when
CONFIG_V4L2_FLASH_LED_CLASS is defined. This is accomplished in
the probe function by either calling v4l2_flash_init function
or the macro of this name, when the CONFIG_V4L2_FLASH_LED_CLASS
macro isn't defined.

If the v4l2-flash.c was to call the LED API directly, then the
led-class-flash module symbols would have to be available at
v4l2-flash.o linking time.


Is this an issue? EXPORT_SYMBOL_GPL() for the relevant symbols should be
enough.


It isn't enough. If I call e.g. led_set_flash_brightness
directly from v4l2-flash.c and configure led-class-flash to be built as
a module then I am getting undefined reference to
led_set_flash_brightness error during linking phase.


You should not. You also should change the check as (unless you've changed
it already):

#if IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS)

This will evaluate to non-zero if the macro arguent or the argument
postfixed with _MODULE is defined.


I've missed this macro. Indeed, it is possible to avoid the need
for ops with it. I will fix it in the next version of the patch set.
Thanks for the hint.

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/RFC v4 15/21] media: Add registration helpers for V4L2 flash

2014-08-20 Thread Sakari Ailus
Hi Jacek,

On Thu, Aug 14, 2014 at 10:25:28AM +0200, Jacek Anaszewski wrote:
 On 08/14/2014 06:34 AM, Sakari Ailus wrote:
 Hi Jacek,
 
 On Mon, Aug 11, 2014 at 03:27:22PM +0200, Jacek Anaszewski wrote:
 
 ...
 
 diff --git a/include/media/v4l2-flash.h b/include/media/v4l2-flash.h
 new file mode 100644
 index 000..effa46b
 --- /dev/null
 +++ b/include/media/v4l2-flash.h
 @@ -0,0 +1,137 @@
 +/*
 + * V4L2 Flash LED sub-device registration helpers.
 + *
 + * Copyright (C) 2014 Samsung Electronics Co., Ltd
 + * Author: Jacek Anaszewski j.anaszew...@samsung.com
 + *
 + * 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.
 + */
 +
 +#ifndef _V4L2_FLASH_H
 +#define _V4L2_FLASH_H
 +
 +#include media/v4l2-ctrls.h
 +#include media/v4l2-device.h
 +#include media/v4l2-dev.hle
 +#include media/v4l2-event.h
 +#include media/v4l2-ioctl.h
 +
 +struct led_classdev_flash;
 +struct led_classdev;
 +enum led_brightness;
 +
 +struct v4l2_flash_ops {
 +   int (*torch_brightness_set)(struct led_classdev *led_cdev,
 +   enum led_brightness brightness);
 +   int (*torch_brightness_update)(struct led_classdev *led_cdev);
 +   int (*flash_brightness_set)(struct led_classdev_flash *flash,
 +   u32 brightness);
 +   int (*flash_brightness_update)(struct led_classdev_flash 
 *flash);
 +   int (*strobe_set)(struct led_classdev_flash *flash, bool state);
 +   int (*strobe_get)(struct led_classdev_flash *flash, bool 
 *state);
 +   int (*timeout_set)(struct led_classdev_flash *flash, u32 
 timeout);
 +   int (*indicator_brightness_set)(struct led_classdev_flash 
 *flash,
 +   u32 brightness);
 +   int (*indicator_brightness_update)(struct led_classdev_flash 
 *flash);
 +   int (*external_strobe_set)(struct led_classdev_flash *flash,
 +   bool enable);
 +   int (*fault_get)(struct led_classdev_flash *flash, u32 *fault);
 +   void (*sysfs_lock)(struct led_classdev *led_cdev);
 +   void (*sysfs_unlock)(struct led_classdev *led_cdev);
 
 These functions are not driver specific and there's going to be just one
 implementation (I suppose). Could you refresh my memory regarding why
 the LED framework functions aren't called directly?
 
 These ops are required to make possible building led-class-flash as
 a kernel module.
 
 Assuming you'd use the actual implementation directly, what would be the
 dependencies? I don't think the LED flash framework has any callbacks
 towards the V4L2 (LED) flash framework, does it? Please correct my
 understanding if I'm missing something. In Makefile format, assume all
 targets are .PHONY:
 
 led-flash-api: led-api
 
 v4l2-flash: led-flash-api
 
 driver: led-flash-api v4l2-flash
 
 LED Class Flash driver gains V4L2 Flash API when
 CONFIG_V4L2_FLASH_LED_CLASS is defined. This is accomplished in
 the probe function by either calling v4l2_flash_init function
 or the macro of this name, when the CONFIG_V4L2_FLASH_LED_CLASS
 macro isn't defined.
 
 If the v4l2-flash.c was to call the LED API directly, then the
 led-class-flash module symbols would have to be available at
 v4l2-flash.o linking time.
 
 Is this an issue? EXPORT_SYMBOL_GPL() for the relevant symbols should be
 enough.
 
 It isn't enough. If I call e.g. led_set_flash_brightness
 directly from v4l2-flash.c and configure led-class-flash to be built as
 a module then I am getting undefined reference to
 led_set_flash_brightness error during linking phase.

You should not. You also should change the check as (unless you've changed
it already):

#if IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS)

This will evaluate to non-zero if the macro arguent or the argument
postfixed with _MODULE is defined.

 It happens because the linker doesn't take into account
 led-flash-class.ko symbols. It is reasonable because initially
 the kernel boots up without led-flash-class.ko module and
 the processor wouldn't know the address to jump to in the
 result of calling a led API function.
 The led-class-flash.ko binary code is loaded into memory not
 sooner than after executing insmod led-class-flash.ko.
 
 After linking dynamically with kernel the LED API function
 addresses are relocated, and the LED Flash Class core can
 initialize the v4l2_flash_ops structure. Then every LED Flash Class
 driver can obtain the address of this structure with
 led_get_v4l2_flash_ops and pass it to the v4l2_flash_init.
 
 This requirement cannot be met if the led-class-flash is built
 as a module.
 
 Use of function pointers in the v4l2-flash.c allows to compile it
 into the kernel and enables the possibility of adding the V4L2 Flash
 support conditionally, during driver probing.
 
 I'd simply decide this during kernel compilation time. If you want
 something, 

Re: [PATCH/RFC v4 15/21] media: Add registration helpers for V4L2 flash

2014-08-14 Thread Jacek Anaszewski

On 08/14/2014 06:34 AM, Sakari Ailus wrote:

Hi Jacek,

On Mon, Aug 11, 2014 at 03:27:22PM +0200, Jacek Anaszewski wrote:

...


diff --git a/include/media/v4l2-flash.h b/include/media/v4l2-flash.h
new file mode 100644
index 000..effa46b
--- /dev/null
+++ b/include/media/v4l2-flash.h
@@ -0,0 +1,137 @@
+/*
+ * V4L2 Flash LED sub-device registration helpers.
+ *
+ * Copyright (C) 2014 Samsung Electronics Co., Ltd
+ * Author: Jacek Anaszewski j.anaszew...@samsung.com
+ *
+ * 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.
+ */
+
+#ifndef _V4L2_FLASH_H
+#define _V4L2_FLASH_H
+
+#include media/v4l2-ctrls.h
+#include media/v4l2-device.h
+#include media/v4l2-dev.hle
+#include media/v4l2-event.h
+#include media/v4l2-ioctl.h
+
+struct led_classdev_flash;
+struct led_classdev;
+enum led_brightness;
+
+struct v4l2_flash_ops {
+   int (*torch_brightness_set)(struct led_classdev *led_cdev,
+   enum led_brightness brightness);
+   int (*torch_brightness_update)(struct led_classdev *led_cdev);
+   int (*flash_brightness_set)(struct led_classdev_flash *flash,
+   u32 brightness);
+   int (*flash_brightness_update)(struct led_classdev_flash *flash);
+   int (*strobe_set)(struct led_classdev_flash *flash, bool state);
+   int (*strobe_get)(struct led_classdev_flash *flash, bool *state);
+   int (*timeout_set)(struct led_classdev_flash *flash, u32 timeout);
+   int (*indicator_brightness_set)(struct led_classdev_flash *flash,
+   u32 brightness);
+   int (*indicator_brightness_update)(struct led_classdev_flash *flash);
+   int (*external_strobe_set)(struct led_classdev_flash *flash,
+   bool enable);
+   int (*fault_get)(struct led_classdev_flash *flash, u32 *fault);
+   void (*sysfs_lock)(struct led_classdev *led_cdev);
+   void (*sysfs_unlock)(struct led_classdev *led_cdev);


These functions are not driver specific and there's going to be just one
implementation (I suppose). Could you refresh my memory regarding why
the LED framework functions aren't called directly?


These ops are required to make possible building led-class-flash as
a kernel module.


Assuming you'd use the actual implementation directly, what would be the
dependencies? I don't think the LED flash framework has any callbacks
towards the V4L2 (LED) flash framework, does it? Please correct my
understanding if I'm missing something. In Makefile format, assume all
targets are .PHONY:

led-flash-api: led-api

v4l2-flash: led-flash-api

driver: led-flash-api v4l2-flash


LED Class Flash driver gains V4L2 Flash API when
CONFIG_V4L2_FLASH_LED_CLASS is defined. This is accomplished in
the probe function by either calling v4l2_flash_init function
or the macro of this name, when the CONFIG_V4L2_FLASH_LED_CLASS
macro isn't defined.

If the v4l2-flash.c was to call the LED API directly, then the
led-class-flash module symbols would have to be available at
v4l2-flash.o linking time.


Is this an issue? EXPORT_SYMBOL_GPL() for the relevant symbols should be
enough.


It isn't enough. If I call e.g. led_set_flash_brightness
directly from v4l2-flash.c and configure led-class-flash to be built as
a module then I am getting undefined reference to
led_set_flash_brightness error during linking phase.

It happens because the linker doesn't take into account
led-flash-class.ko symbols. It is reasonable because initially
the kernel boots up without led-flash-class.ko module and
the processor wouldn't know the address to jump to in the
result of calling a led API function.
The led-class-flash.ko binary code is loaded into memory not
sooner than after executing insmod led-class-flash.ko.

After linking dynamically with kernel the LED API function
addresses are relocated, and the LED Flash Class core can
initialize the v4l2_flash_ops structure. Then every LED Flash Class
driver can obtain the address of this structure with
led_get_v4l2_flash_ops and pass it to the v4l2_flash_init.


This requirement cannot be met if the led-class-flash is built
as a module.

Use of function pointers in the v4l2-flash.c allows to compile it
into the kernel and enables the possibility of adding the V4L2 Flash
support conditionally, during driver probing.


I'd simply decide this during kernel compilation time. If you want
something, just enable it. v4l2_flash_init() is called directly by the
driver in any case, so unless that is also called through a wrapper the
driver is still directly dependent on it.


The problem is that v4l2-flash.o would have to depend on
led-class-flash.o, which when built as a module isn't available
during v4l2-flash.o linking time. In order to avoid v4l2-flash.o linking
problem, it would have to be built as a module.


Re: [PATCH/RFC v4 15/21] media: Add registration helpers for V4L2 flash

2014-08-13 Thread Sakari Ailus
Hi Jacek,

On Mon, Aug 11, 2014 at 03:27:22PM +0200, Jacek Anaszewski wrote:

...

 diff --git a/include/media/v4l2-flash.h b/include/media/v4l2-flash.h
 new file mode 100644
 index 000..effa46b
 --- /dev/null
 +++ b/include/media/v4l2-flash.h
 @@ -0,0 +1,137 @@
 +/*
 + * V4L2 Flash LED sub-device registration helpers.
 + *
 + *   Copyright (C) 2014 Samsung Electronics Co., Ltd
 + *   Author: Jacek Anaszewski j.anaszew...@samsung.com
 + *
 + * 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.
 + */
 +
 +#ifndef _V4L2_FLASH_H
 +#define _V4L2_FLASH_H
 +
 +#include media/v4l2-ctrls.h
 +#include media/v4l2-device.h
 +#include media/v4l2-dev.hle
 +#include media/v4l2-event.h
 +#include media/v4l2-ioctl.h
 +
 +struct led_classdev_flash;
 +struct led_classdev;
 +enum led_brightness;
 +
 +struct v4l2_flash_ops {
 + int (*torch_brightness_set)(struct led_classdev *led_cdev,
 + enum led_brightness brightness);
 + int (*torch_brightness_update)(struct led_classdev *led_cdev);
 + int (*flash_brightness_set)(struct led_classdev_flash *flash,
 + u32 brightness);
 + int (*flash_brightness_update)(struct led_classdev_flash *flash);
 + int (*strobe_set)(struct led_classdev_flash *flash, bool state);
 + int (*strobe_get)(struct led_classdev_flash *flash, bool *state);
 + int (*timeout_set)(struct led_classdev_flash *flash, u32 timeout);
 + int (*indicator_brightness_set)(struct led_classdev_flash *flash,
 + u32 brightness);
 + int (*indicator_brightness_update)(struct led_classdev_flash *flash);
 + int (*external_strobe_set)(struct led_classdev_flash *flash,
 + bool enable);
 + int (*fault_get)(struct led_classdev_flash *flash, u32 *fault);
 + void (*sysfs_lock)(struct led_classdev *led_cdev);
 + void (*sysfs_unlock)(struct led_classdev *led_cdev);
 
 These functions are not driver specific and there's going to be just one
 implementation (I suppose). Could you refresh my memory regarding why
 the LED framework functions aren't called directly?
 
 These ops are required to make possible building led-class-flash as
 a kernel module.
 
 Assuming you'd use the actual implementation directly, what would be the
 dependencies? I don't think the LED flash framework has any callbacks
 towards the V4L2 (LED) flash framework, does it? Please correct my
 understanding if I'm missing something. In Makefile format, assume all
 targets are .PHONY:
 
 led-flash-api: led-api
 
 v4l2-flash: led-flash-api
 
 driver: led-flash-api v4l2-flash
 
 LED Class Flash driver gains V4L2 Flash API when
 CONFIG_V4L2_FLASH_LED_CLASS is defined. This is accomplished in
 the probe function by either calling v4l2_flash_init function
 or the macro of this name, when the CONFIG_V4L2_FLASH_LED_CLASS
 macro isn't defined.
 
 If the v4l2-flash.c was to call the LED API directly, then the
 led-class-flash module symbols would have to be available at
 v4l2-flash.o linking time.

Is this an issue? EXPORT_SYMBOL_GPL() for the relevant symbols should be
enough.

 This requirement cannot be met if the led-class-flash is built
 as a module.
 
 Use of function pointers in the v4l2-flash.c allows to compile it
 into the kernel and enables the possibility of adding the V4L2 Flash
 support conditionally, during driver probing.

I'd simply decide this during kernel compilation time. If you want
something, just enable it. v4l2_flash_init() is called directly by the
driver in any case, so unless that is also called through a wrapper the
driver is still directly dependent on it.

-- 
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/RFC v4 15/21] media: Add registration helpers for V4L2 flash

2014-08-11 Thread Sakari Ailus

Hi Jacek,

On Mon, Aug 04, 2014 at 04:43:54PM +0200, Jacek Anaszewski wrote:
 Hi Sakari,
 
 Thanks for the review.

You're welcome! :)

 On 07/21/2014 01:12 PM, Sakari Ailus wrote:
 Hi Jacek,
 
 Jacek Anaszewski wrote:
 This patch adds helper functions for registering/unregistering
 LED class flash devices as V4L2 subdevs. 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 j.anaszew...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Sakari Ailus sakari.ai...@iki.fi
 Cc: Hans Verkuil hans.verk...@cisco.com
 ---
   drivers/media/v4l2-core/Kconfig  |   11 +
   drivers/media/v4l2-core/Makefile |2 +
   drivers/media/v4l2-core/v4l2-flash.c |  580 
  ++
   include/media/v4l2-flash.h   |  137 
   4 files changed, 730 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 9ca0f8d..3ae3f0f 100644
 --- a/drivers/media/v4l2-core/Kconfig
 +++ b/drivers/media/v4l2-core/Kconfig
 @@ -35,6 +35,17 @@ config V4L2_MEM2MEM_DEV
   tristate
   depends on VIDEOBUF2_CORE
 
 +# Used by LED subsystem flash drivers
 +config V4L2_FLASH_LED_CLASS
 +   bool 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..21080c6
 --- /dev/null
 +++ b/drivers/media/v4l2-core/v4l2-flash.c
 @@ -0,0 +1,580 @@
 +/*
 + * V4L2 Flash LED sub-device registration helpers.
 + *
 + * Copyright (C) 2014 Samsung Electronics Co., Ltd
 + * Author: Jacek Anaszewski j.anaszew...@samsung.com
 + *
 + * 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 linux/led-class-flash.h
 +#include linux/mutex.h
 +#include linux/of_led_flash_manager.h
 +#include linux/slab.h
 +#include media/v4l2-flash.h
 +
 +#define call_flash_op(v4l2_flash, op, args...) \
 +   (v4l2_flash-ops-op  ? \
 +   v4l2_flash-ops-op(args) : \
 +   -EINVAL)
 +
 +static struct v4l2_device *v4l2_dev;
 +static int registered_flashes;
 +
 +static inline enum led_brightness v4l2_flash_intensity_to_led_brightness(
 +   struct v4l2_ctrl_config *config,
 +   s32 intensity)
 +{
 +   return ((intensity - config-min) / config-step) + 1;
 +}
 +
 +static inline s32 v4l2_flash_led_brightness_to_intensity(
 +   struct v4l2_ctrl_config *config,
 +   enum led_brightness brightness)
 +{
 +   return ((brightness - 1) * config-step) + config-min;
 +}
 +
 +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 *flash = v4l2_flash-flash;
 +   struct led_classdev *led_cdev = flash-led_cdev;
 +   struct v4l2_flash_ctrl_config *config = v4l2_flash-config;
 +   struct v4l2_flash_ctrl *ctrl = v4l2_flash-ctrl;
 +   bool is_strobing;
 +   int ret;
 +
 +   switch (c-id) {
 +   case V4L2_CID_FLASH_TORCH_INTENSITY:
 +   /*
 +* Update torch brightness only if in TORCH_MODE,
 +* as otherwise brightness_update op returns 0,
 +* which would spuriously inform user space that
 +* V4L2_CID_FLASH_TORCH_INTENSITY control value
 +* has changed.
 +*/
 +   if (ctrl-led_mode-val == V4L2_FLASH_LED_MODE_TORCH) {
 +   ret = call_flash_op(v4l2_flash, torch_brightness_update,
 +   led_cdev);
 +   if (ret  0)
 +

Re: [PATCH/RFC v4 15/21] media: Add registration helpers for V4L2 flash

2014-08-11 Thread Jacek Anaszewski

Hi Sakari

On 08/11/2014 02:26 PM, Sakari Ailus wrote:


Hi Jacek,

On Mon, Aug 04, 2014 at 04:43:54PM +0200, Jacek Anaszewski wrote:

Hi Sakari,

Thanks for the review.


You're welcome! :)


On 07/21/2014 01:12 PM, Sakari Ailus wrote:

Hi Jacek,

Jacek Anaszewski wrote:

This patch adds helper functions for registering/unregistering
LED class flash devices as V4L2 subdevs. 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 j.anaszew...@samsung.com
Acked-by: Kyungmin Park kyungmin.p...@samsung.com
Cc: Sakari Ailus sakari.ai...@iki.fi
Cc: Hans Verkuil hans.verk...@cisco.com
---
  drivers/media/v4l2-core/Kconfig  |   11 +
  drivers/media/v4l2-core/Makefile |2 +
  drivers/media/v4l2-core/v4l2-flash.c |  580 ++
  include/media/v4l2-flash.h   |  137 
  4 files changed, 730 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 9ca0f8d..3ae3f0f 100644
--- a/drivers/media/v4l2-core/Kconfig
+++ b/drivers/media/v4l2-core/Kconfig
@@ -35,6 +35,17 @@ config V4L2_MEM2MEM_DEV
  tristate
  depends on VIDEOBUF2_CORE

+# Used by LED subsystem flash drivers
+config V4L2_FLASH_LED_CLASS
+   bool 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..21080c6
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-flash.c
@@ -0,0 +1,580 @@
+/*
+ * V4L2 Flash LED sub-device registration helpers.
+ *
+ * Copyright (C) 2014 Samsung Electronics Co., Ltd
+ * Author: Jacek Anaszewski j.anaszew...@samsung.com
+ *
+ * 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 linux/led-class-flash.h
+#include linux/mutex.h
+#include linux/of_led_flash_manager.h
+#include linux/slab.h
+#include media/v4l2-flash.h
+
+#define call_flash_op(v4l2_flash, op, args...) \
+   (v4l2_flash-ops-op  ?   \
+   v4l2_flash-ops-op(args) :   \
+   -EINVAL)
+
+static struct v4l2_device *v4l2_dev;
+static int registered_flashes;
+
+static inline enum led_brightness v4l2_flash_intensity_to_led_brightness(
+   struct v4l2_ctrl_config *config,
+   s32 intensity)
+{
+   return ((intensity - config-min) / config-step) + 1;
+}
+
+static inline s32 v4l2_flash_led_brightness_to_intensity(
+   struct v4l2_ctrl_config *config,
+   enum led_brightness brightness)
+{
+   return ((brightness - 1) * config-step) + config-min;
+}
+
+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 *flash = v4l2_flash-flash;
+   struct led_classdev *led_cdev = flash-led_cdev;
+   struct v4l2_flash_ctrl_config *config = v4l2_flash-config;
+   struct v4l2_flash_ctrl *ctrl = v4l2_flash-ctrl;
+   bool is_strobing;
+   int ret;
+
+   switch (c-id) {
+   case V4L2_CID_FLASH_TORCH_INTENSITY:
+   /*
+* Update torch brightness only if in TORCH_MODE,
+* as otherwise brightness_update op returns 0,
+* which would spuriously inform user space that
+* V4L2_CID_FLASH_TORCH_INTENSITY control value
+* has changed.
+*/
+   if (ctrl-led_mode-val == V4L2_FLASH_LED_MODE_TORCH) {
+   ret = call_flash_op(v4l2_flash, torch_brightness_update,
+

Re: [PATCH/RFC v4 15/21] media: Add registration helpers for V4L2 flash

2014-08-11 Thread Jacek Anaszewski

On 08/11/2014 02:26 PM, Sakari Ailus wrote:


Hi Jacek,



...


+static int v4l2_flash_s_ctrl(struct v4l2_ctrl *c)
+{
+struct v4l2_flash *v4l2_flash = v4l2_ctrl_to_v4l2_flash(c);
+struct led_classdev_flash *flash = v4l2_flash-flash;
+struct v4l2_flash_ctrl *ctrl = v4l2_flash-ctrl;
+struct v4l2_flash_ctrl_config *config = v4l2_flash-config;
+enum led_brightness torch_brightness;
+bool external_strobe;
+int ret;
+
+switch (c-id) {
+case V4L2_CID_FLASH_LED_MODE:
+switch (c-val) {
+case V4L2_FLASH_LED_MODE_NONE:
+call_flash_op(v4l2_flash, torch_brightness_set,
+flash-led_cdev, 0);
+return call_flash_op(v4l2_flash, strobe_set, flash,
+false);
+case V4L2_FLASH_LED_MODE_FLASH:
+/* Turn off torch LED */
+call_flash_op(v4l2_flash, torch_brightness_set,
+flash-led_cdev, 0);
+external_strobe = (ctrl-source-val ==
+V4L2_FLASH_STROBE_SOURCE_EXTERNAL);
+return call_flash_op(v4l2_flash, external_strobe_set,
+flash, external_strobe);
+case V4L2_FLASH_LED_MODE_TORCH:
+/* Stop flash strobing */
+ret = call_flash_op(v4l2_flash, strobe_set, flash,
+false);
+if (ret)
+return ret;
+
+torch_brightness =
+v4l2_flash_intensity_to_led_brightness(
+config-torch_intensity,
+ctrl-torch_intensity-val);
+call_flash_op(v4l2_flash, torch_brightness_set,
+flash-led_cdev, torch_brightness);
+return ret;
+}
+break;
+case V4L2_CID_FLASH_STROBE_SOURCE:
+external_strobe = (c-val ==
V4L2_FLASH_STROBE_SOURCE_EXTERNAL);


Is the external_strobe argument match exactly to the strobe source
control? You seem to assume that in g_volatile_ctrl() above. I think
having it the either way is fine but not both. :-)


The STROBE_SOURCE_EXTERNAL control state is volatile if a flash device
depends on muxes that route strobe signals to more then one flash
device. In such a case it behaves similarly to FLASH_STROBE control,
i.e. it activates external strobe only for the flash timeout period.
I touched this issue in the cover letter of this patch series,
paragraph 2.


I meant that flash-external_strobe is directly used as
V4L2_CID_FLASH_STROBE_SOURCE. Are the two guaranteed to be the same?


Yes, the external_strobe sysfs attribute is mapped to it.

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/RFC v4 15/21] media: Add registration helpers for V4L2 flash

2014-08-04 Thread Jacek Anaszewski

Hi Sakari,

Thanks for the review.

On 07/21/2014 01:12 PM, Sakari Ailus wrote:

Hi Jacek,

Jacek Anaszewski wrote:

This patch adds helper functions for registering/unregistering
LED class flash devices as V4L2 subdevs. 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 j.anaszew...@samsung.com
Acked-by: Kyungmin Park kyungmin.p...@samsung.com
Cc: Sakari Ailus sakari.ai...@iki.fi
Cc: Hans Verkuil hans.verk...@cisco.com
---
  drivers/media/v4l2-core/Kconfig  |   11 +
  drivers/media/v4l2-core/Makefile |2 +
  drivers/media/v4l2-core/v4l2-flash.c |  580 ++
  include/media/v4l2-flash.h   |  137 
  4 files changed, 730 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 9ca0f8d..3ae3f0f 100644
--- a/drivers/media/v4l2-core/Kconfig
+++ b/drivers/media/v4l2-core/Kconfig
@@ -35,6 +35,17 @@ config V4L2_MEM2MEM_DEV
  tristate
  depends on VIDEOBUF2_CORE

+# Used by LED subsystem flash drivers
+config V4L2_FLASH_LED_CLASS
+   bool 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..21080c6
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-flash.c
@@ -0,0 +1,580 @@
+/*
+ * V4L2 Flash LED sub-device registration helpers.
+ *
+ * Copyright (C) 2014 Samsung Electronics Co., Ltd
+ * Author: Jacek Anaszewski j.anaszew...@samsung.com
+ *
+ * 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 linux/led-class-flash.h
+#include linux/mutex.h
+#include linux/of_led_flash_manager.h
+#include linux/slab.h
+#include media/v4l2-flash.h
+
+#define call_flash_op(v4l2_flash, op, args...) \
+   (v4l2_flash-ops-op  ?   \
+   v4l2_flash-ops-op(args) :   \
+   -EINVAL)
+
+static struct v4l2_device *v4l2_dev;
+static int registered_flashes;
+
+static inline enum led_brightness v4l2_flash_intensity_to_led_brightness(
+   struct v4l2_ctrl_config *config,
+   s32 intensity)
+{
+   return ((intensity - config-min) / config-step) + 1;
+}
+
+static inline s32 v4l2_flash_led_brightness_to_intensity(
+   struct v4l2_ctrl_config *config,
+   enum led_brightness brightness)
+{
+   return ((brightness - 1) * config-step) + config-min;
+}
+
+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 *flash = v4l2_flash-flash;
+   struct led_classdev *led_cdev = flash-led_cdev;
+   struct v4l2_flash_ctrl_config *config = v4l2_flash-config;
+   struct v4l2_flash_ctrl *ctrl = v4l2_flash-ctrl;
+   bool is_strobing;
+   int ret;
+
+   switch (c-id) {
+   case V4L2_CID_FLASH_TORCH_INTENSITY:
+   /*
+* Update torch brightness only if in TORCH_MODE,
+* as otherwise brightness_update op returns 0,
+* which would spuriously inform user space that
+* V4L2_CID_FLASH_TORCH_INTENSITY control value
+* has changed.
+*/
+   if (ctrl-led_mode-val == V4L2_FLASH_LED_MODE_TORCH) {
+   ret = call_flash_op(v4l2_flash, torch_brightness_update,
+   led_cdev);
+   if (ret  0)
+   return ret;
+   

Re: [PATCH/RFC v4 15/21] media: Add registration helpers for V4L2 flash

2014-07-21 Thread Sakari Ailus
Hi Jacek,

Jacek Anaszewski wrote:
 This patch adds helper functions for registering/unregistering
 LED class flash devices as V4L2 subdevs. 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 j.anaszew...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Sakari Ailus sakari.ai...@iki.fi
 Cc: Hans Verkuil hans.verk...@cisco.com
 ---
  drivers/media/v4l2-core/Kconfig  |   11 +
  drivers/media/v4l2-core/Makefile |2 +
  drivers/media/v4l2-core/v4l2-flash.c |  580 
 ++
  include/media/v4l2-flash.h   |  137 
  4 files changed, 730 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 9ca0f8d..3ae3f0f 100644
 --- a/drivers/media/v4l2-core/Kconfig
 +++ b/drivers/media/v4l2-core/Kconfig
 @@ -35,6 +35,17 @@ config V4L2_MEM2MEM_DEV
  tristate
  depends on VIDEOBUF2_CORE
  
 +# Used by LED subsystem flash drivers
 +config V4L2_FLASH_LED_CLASS
 + bool 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..21080c6
 --- /dev/null
 +++ b/drivers/media/v4l2-core/v4l2-flash.c
 @@ -0,0 +1,580 @@
 +/*
 + * V4L2 Flash LED sub-device registration helpers.
 + *
 + *   Copyright (C) 2014 Samsung Electronics Co., Ltd
 + *   Author: Jacek Anaszewski j.anaszew...@samsung.com
 + *
 + * 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 linux/led-class-flash.h
 +#include linux/mutex.h
 +#include linux/of_led_flash_manager.h
 +#include linux/slab.h
 +#include media/v4l2-flash.h
 +
 +#define call_flash_op(v4l2_flash, op, args...)   \
 + (v4l2_flash-ops-op  ? \
 + v4l2_flash-ops-op(args) : \
 + -EINVAL)
 +
 +static struct v4l2_device *v4l2_dev;
 +static int registered_flashes;
 +
 +static inline enum led_brightness v4l2_flash_intensity_to_led_brightness(
 + struct v4l2_ctrl_config *config,
 + s32 intensity)
 +{
 + return ((intensity - config-min) / config-step) + 1;
 +}
 +
 +static inline s32 v4l2_flash_led_brightness_to_intensity(
 + struct v4l2_ctrl_config *config,
 + enum led_brightness brightness)
 +{
 + return ((brightness - 1) * config-step) + config-min;
 +}
 +
 +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 *flash = v4l2_flash-flash;
 + struct led_classdev *led_cdev = flash-led_cdev;
 + struct v4l2_flash_ctrl_config *config = v4l2_flash-config;
 + struct v4l2_flash_ctrl *ctrl = v4l2_flash-ctrl;
 + bool is_strobing;
 + int ret;
 +
 + switch (c-id) {
 + case V4L2_CID_FLASH_TORCH_INTENSITY:
 + /*
 +  * Update torch brightness only if in TORCH_MODE,
 +  * as otherwise brightness_update op returns 0,
 +  * which would spuriously inform user space that
 +  * V4L2_CID_FLASH_TORCH_INTENSITY control value
 +  * has changed.
 +  */
 + if (ctrl-led_mode-val == V4L2_FLASH_LED_MODE_TORCH) {
 + ret = call_flash_op(v4l2_flash, torch_brightness_update,
 + led_cdev);
 + if (ret  0)
 + return ret;
 + ctrl-torch_intensity-val =
 + 

[PATCH/RFC v4 15/21] media: Add registration helpers for V4L2 flash

2014-07-11 Thread Jacek Anaszewski
This patch adds helper functions for registering/unregistering
LED class flash devices as V4L2 subdevs. 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 j.anaszew...@samsung.com
Acked-by: Kyungmin Park kyungmin.p...@samsung.com
Cc: Sakari Ailus sakari.ai...@iki.fi
Cc: Hans Verkuil hans.verk...@cisco.com
---
 drivers/media/v4l2-core/Kconfig  |   11 +
 drivers/media/v4l2-core/Makefile |2 +
 drivers/media/v4l2-core/v4l2-flash.c |  580 ++
 include/media/v4l2-flash.h   |  137 
 4 files changed, 730 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 9ca0f8d..3ae3f0f 100644
--- a/drivers/media/v4l2-core/Kconfig
+++ b/drivers/media/v4l2-core/Kconfig
@@ -35,6 +35,17 @@ config V4L2_MEM2MEM_DEV
 tristate
 depends on VIDEOBUF2_CORE
 
+# Used by LED subsystem flash drivers
+config V4L2_FLASH_LED_CLASS
+   bool 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..21080c6
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-flash.c
@@ -0,0 +1,580 @@
+/*
+ * V4L2 Flash LED sub-device registration helpers.
+ *
+ * Copyright (C) 2014 Samsung Electronics Co., Ltd
+ * Author: Jacek Anaszewski j.anaszew...@samsung.com
+ *
+ * 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 linux/led-class-flash.h
+#include linux/mutex.h
+#include linux/of_led_flash_manager.h
+#include linux/slab.h
+#include media/v4l2-flash.h
+
+#define call_flash_op(v4l2_flash, op, args...) \
+   (v4l2_flash-ops-op  ? \
+   v4l2_flash-ops-op(args) : \
+   -EINVAL)
+
+static struct v4l2_device *v4l2_dev;
+static int registered_flashes;
+
+static inline enum led_brightness v4l2_flash_intensity_to_led_brightness(
+   struct v4l2_ctrl_config *config,
+   s32 intensity)
+{
+   return ((intensity - config-min) / config-step) + 1;
+}
+
+static inline s32 v4l2_flash_led_brightness_to_intensity(
+   struct v4l2_ctrl_config *config,
+   enum led_brightness brightness)
+{
+   return ((brightness - 1) * config-step) + config-min;
+}
+
+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 *flash = v4l2_flash-flash;
+   struct led_classdev *led_cdev = flash-led_cdev;
+   struct v4l2_flash_ctrl_config *config = v4l2_flash-config;
+   struct v4l2_flash_ctrl *ctrl = v4l2_flash-ctrl;
+   bool is_strobing;
+   int ret;
+
+   switch (c-id) {
+   case V4L2_CID_FLASH_TORCH_INTENSITY:
+   /*
+* Update torch brightness only if in TORCH_MODE,
+* as otherwise brightness_update op returns 0,
+* which would spuriously inform user space that
+* V4L2_CID_FLASH_TORCH_INTENSITY control value
+* has changed.
+*/
+   if (ctrl-led_mode-val == V4L2_FLASH_LED_MODE_TORCH) {
+   ret = call_flash_op(v4l2_flash, torch_brightness_update,
+   led_cdev);
+   if (ret  0)
+   return ret;
+   ctrl-torch_intensity-val =
+   v4l2_flash_led_brightness_to_intensity(
+