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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140814043436.GM16460@valkosipuli.retiisi.org.uk>
Date:	Thu, 14 Aug 2014 07:34:36 +0300
From:	Sakari Ailus <sakari.ailus@....fi>
To:	Jacek Anaszewski <j.anaszewski@...sung.com>
Cc:	linux-leds@...r.kernel.org, devicetree@...r.kernel.org,
	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
	kyungmin.park@...sung.com, b.zolnierkie@...sung.com,
	Hans Verkuil <hans.verkuil@...co.com>
Subject: Re: [PATCH/RFC v4 15/21] media: Add registration helpers for V4L2
 flash

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@...sung.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.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@....fi	XMPP: sailus@...iisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/