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 0000000..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.anaszewski@xxxxxxxxxxx> > >>>>+ * > >>>>+ * 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.h>le > >>>>+#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.ailus@xxxxxx XMPP: sailus@xxxxxxxxxxxxxx -- 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