Hi Sakari,
On 09/22/2014 05:21 PM, 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.anaszewski@xxxxxxxxxxx>
Acked-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
Cc: Sakari Ailus <sakari.ailus@xxxxxx>
Cc: Hans Verkuil <hans.verkuil@xxxxxxxxx>
Cc: Bryan Wu <cooloney@xxxxxxxxx>
Cc: Richard Purdie <rpurdie@xxxxxxxxx>
---
drivers/media/v4l2-core/Kconfig | 11 +
drivers/media/v4l2-core/Makefile | 2 +
drivers/media/v4l2-core/v4l2-flash.c | 502 ++++++++++++++++++++++++++++++++++
include/media/v4l2-flash.h | 135 +++++++++
4 files changed, 650 insertions(+)
create mode 100644 drivers/media/v4l2-core/v4l2-flash.c
create mode 100644 include/media/v4l2-flash.h
[...]
After discussing on IRC the way of using compound controls for
v4l2-flash sub-device I started to re-implement the patch but
encountered subsequent issues, which make my inclination for
abiding by the current version of the patch (separate v4l2-flash
device for each sub-led) even stronger.
Let's list arguments for both options:
1. Single v4l2-flash sub-device for a flash device that can control
several sub-leds:
a) a flash device driver has one related i2c device
b) there exist hardware designs where some registers are
shared between sub-leds (e.g. flash timeout, flash status)
2. Separate v4l2-flash sub-device for each sub-led of a flash device
a) LED Flash class drivers create separate LED Flash device for
sub-leds (enforced by led-triggers design). This way there is
a simple one-to-one "LED Flash device" - "v4l2-flash sub-device"
relation.
b) if a single v4l2-flash sub-device was to control several
LED Flash devices then array controls would have to be
used for accessing the settings of every LED Flash device.
This poses following issues:
- the type of each V4L2 Flash control would have to be
set to the compound one (e.g. V4L2_CTRL_TYPE_U32), which in
turn would make the menu controls unavailable for querying
and displaying e.g. in qv4l2. Also the types as bitmask, button
would have to be avoided.
- All elements of an array control have to have the same
constraints and it would make impossible setting different min,
max values (e.g. current, timeout, external strobe) for each
sub-led. All the advantageous v4l2-ctrl mechanism related
to validating and caching controls would have to be avoided
and the user space would only get feedback in the form of
failing ioctl when the value to be set is not properly aligned
- it is not possible to set only one element of the control
array and thus the settings of each sub-led would have to be
cached to avoid superfluous device register access
(functionality already secured by non-array v4l2-controls)
- the flash devices supporting single led could be provided
with standard non-array controls, but it would produce
cumbersome v4l2-flash code and inconsistent user space interface
From the above it looks like the option 2. has much more advantages.
The argument 1.a doesn't seem to be so vital in view of the fact
that LED subsystem already breaks it. The argument 1.b can be obviated
by caching the relevant values in the driver as it is for max77693-led.
I think that choosing option 2. would allow for avoiding much work
that is already done in v4l2-ctrls.c. Moreover it would keep the
V4L2 Flash controls maintainable with qv4l2.
Best Regards,
Jacek Anaszewski
--
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
