[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-id: <534F9044.6080508@samsung.com>
Date: Thu, 17 Apr 2014 10:26:44 +0200
From: Jacek Anaszewski <j.anaszewski@...sung.com>
To: Sakari Ailus <sakari.ailus@....fi>
Cc: linux-media@...r.kernel.org, linux-leds@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
s.nawrocki@...sung.com, a.hajda@...sung.com,
kyungmin.park@...sung.com
Subject: Re: [PATCH/RFC v3 5/5] media: Add registration helpers for V4L2 flash
sub-devices
Hi Sakari,
Thanks for the review.
On 04/16/2014 08:21 PM, Sakari Ailus wrote:
> Hi Jacek,
>
> Thanks for the update!
>
[...]
>> +static inline enum led_brightness v4l2_flash_intensity_to_led_brightness(
>> + struct led_ctrl *config,
>> + u32 intensity)
>
> Fits on a single line.
>
>> +{
>> + return intensity / config->step;
>
> Shouldn't you first decrement the minimum before the division?
Brightness level 0 means that led is off. Let's consider following case:
intensity - 15625
config->step - 15625
intensity / config->step = 1 (the lowest possible current level)
>> +}
>> +
>> +static inline u32 v4l2_flash_led_brightness_to_intensity(
>> + struct led_ctrl *config,
>> + enum led_brightness brightness)
>> +{
>> + return brightness * config->step;
>
> And do the opposite here?
>
>> +}
>> +
>> +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 *led_cdev = v4l2_flash->led_cdev;
>> + struct led_flash *flash = led_cdev->flash;
>> + struct v4l2_flash_ctrl *ctrl = &v4l2_flash->ctrl;
>> + u32 fault;
>> + int ret;
>> +
>> + switch (c->id) {
>> + case V4L2_CID_FLASH_TORCH_INTENSITY:
>> + if (ctrl->led_mode->val == V4L2_FLASH_LED_MODE_TORCH) {
>> + ret = v4l2_call_flash_op(brightness_update, led_cdev);
>> + if (ret < 0)
>> + return ret;
>> + ctrl->torch_intensity->val =
>> + v4l2_flash_led_brightness_to_intensity(
>> + &led_cdev->brightness_ctrl,
>> + led_cdev->brightness);
>> + }
>> + return 0;
>> + case V4L2_CID_FLASH_INTENSITY:
>> + ret = v4l2_call_flash_op(flash_brightness_update, led_cdev);
>> + if (ret < 0)
>> + return ret;
>> + /* no conversion is needed */
>> + c->val = flash->brightness.val;
>> + return 0;
>> + case V4L2_CID_FLASH_INDICATOR_INTENSITY:
>> + ret = v4l2_call_flash_op(indicator_brightness_update, led_cdev);
>> + if (ret < 0)
>> + return ret;
>> + /* no conversion is needed */
>> + c->val = flash->indicator_brightness->val;
>> + return 0;
>> + case V4L2_CID_FLASH_STROBE_STATUS:
>> + ret = v4l2_call_flash_op(strobe_get, led_cdev);
>> + if (ret < 0)
>> + return ret;
>> + c->val = !!ret;
>> + return 0;
>> + case V4L2_CID_FLASH_FAULT:
>> + /* led faults map directly to V4L2 flash faults */
>> + ret = v4l2_call_flash_op(fault_get, led_cdev, &fault);
>> + if (!ret)
>
> The return value seems to come all the way from regmap_read() and the bus
> related implementatio. I would just consider negative values errors, as
> above.
I think that ret would be returned if it wasn't equal to 0. But indeed
it should be done the same way as for the other cases.
>> + c->val = fault;
>> + return ret;
>> + default:
>> + return -EINVAL;
>> + }
>> +}
>> +
>> +static int v4l2_flash_s_ctrl(struct v4l2_ctrl *c)
>> +{
>> + struct v4l2_flash *v4l2_flash = v4l2_ctrl_to_v4l2_flash(c);
>> + struct led_classdev *led_cdev = v4l2_flash->led_cdev;
>> + struct v4l2_flash_ctrl *ctrl = &v4l2_flash->ctrl;
>> + 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:
>> + v4l2_call_flash_op(brightness_set, led_cdev, 0);
>> + return v4l2_call_flash_op(strobe_set, led_cdev, false);
>> + case V4L2_FLASH_LED_MODE_FLASH:
>> + /* Turn off torch LED */
>> + v4l2_call_flash_op(brightness_set, led_cdev, 0);
>> + external_strobe = (ctrl->source->val ==
>> + V4L2_FLASH_STROBE_SOURCE_EXTERNAL);
>> + return v4l2_call_flash_op(external_strobe_set, led_cdev,
>> + external_strobe);
>> + case V4L2_FLASH_LED_MODE_TORCH:
>> + /* Stop flash strobing */
>> + ret = v4l2_call_flash_op(strobe_set, led_cdev, false);
>> + if (ret)
>> + return ret;
>> + /* torch is always triggered by software */
>> + ret = v4l2_call_flash_op(external_strobe_set, led_cdev,
>> + false);
>
> Does the LED API not assume this at the moment? I'm not saying it should,
> though. :-)
Actually strobe source should apply only to flash leds. I will
remove this call.
>> + if (ret)
>> + return ret;
>> +
>> + torch_brightness =
>> + v4l2_flash_intensity_to_led_brightness(
>> + &led_cdev->brightness_ctrl,
>> + ctrl->torch_intensity->val);
>> + v4l2_call_flash_op(brightness_set, led_cdev,
>> + torch_brightness);
>> + return ret;
>> + }
>> + break;
>> + case V4L2_CID_FLASH_STROBE_SOURCE:
>> + return v4l2_call_flash_op(external_strobe_set, led_cdev,
>> + c->val == V4L2_FLASH_STROBE_SOURCE_EXTERNAL);
>> + case V4L2_CID_FLASH_STROBE:
>> + if (ctrl->led_mode->val != V4L2_FLASH_LED_MODE_FLASH ||
>> + ctrl->source->val != V4L2_FLASH_STROBE_SOURCE_SOFTWARE)
>
> I vote for -EBUSY instead.
I agree.
>> + return -EINVAL;
>> + return v4l2_call_flash_op(strobe_set, led_cdev, true);
>> + case V4L2_CID_FLASH_STROBE_STOP:
>> + return v4l2_call_flash_op(strobe_set, led_cdev, false);
>> + case V4L2_CID_FLASH_TIMEOUT:
>> + ret = v4l2_call_flash_op(timeout_set, led_cdev, c->val);
>> + case V4L2_CID_FLASH_INTENSITY:
>> + /* no conversion is needed */
>> + return v4l2_call_flash_op(flash_brightness_set, led_cdev,
>> + c->val);
>> + case V4L2_CID_FLASH_INDICATOR_INTENSITY:
>> + /* no conversion is needed */
>> + return v4l2_call_flash_op(indicator_brightness_set, led_cdev,
>> + c->val);
>> + case V4L2_CID_FLASH_TORCH_INTENSITY:
>> + if (ctrl->led_mode->val == V4L2_FLASH_LED_MODE_TORCH) {
>> + torch_brightness =
>> + v4l2_flash_intensity_to_led_brightness(
>> + &led_cdev->brightness_ctrl,
>> + ctrl->torch_intensity->val);
>> + v4l2_call_flash_op(brightness_set, led_cdev,
>> + torch_brightness);
>
> I could be missing something but don't torch and indicator require similar
> handling?
Why? Torch units need conversion whereas indicator units don't.
Moreover they have different LED API.
>> + }
>> + return 0;
>> + }
>> +
>> + return -EINVAL;
>> +}
>> +
>> +static const struct v4l2_ctrl_ops v4l2_flash_ctrl_ops = {
>> + .g_volatile_ctrl = v4l2_flash_g_volatile_ctrl,
>> + .s_ctrl = v4l2_flash_s_ctrl,
>> +};
>> +
>> +static int v4l2_flash_init_controls(struct v4l2_flash *v4l2_flash)
>> +
>> +{
>> + struct led_classdev *led_cdev = v4l2_flash->led_cdev;
>> + struct led_flash *flash = led_cdev->flash;
>> + bool has_indicator = flash->indicator_brightness;
>> + struct v4l2_ctrl *ctrl;
>> + struct led_ctrl *ctrl_cfg;
>> + unsigned int mask;
>> + int ret, max, num_ctrls;
>> +
>> + num_ctrls = flash->has_flash_led ? 8 : 2;
>> + if (flash->fault_flags)
>> + ++num_ctrls;
>> + if (has_indicator)
>> + ++num_ctrls;
>> +
>> + v4l2_ctrl_handler_init(&v4l2_flash->hdl, num_ctrls);
>> +
>> + mask = 1 << V4L2_FLASH_LED_MODE_NONE |
>> + 1 << V4L2_FLASH_LED_MODE_TORCH;
>> + if (flash->has_flash_led)
>> + mask |= 1 << V4L2_FLASH_LED_MODE_FLASH;
>> +
>> + /* Configure FLASH_LED_MODE ctrl */
>> + v4l2_flash->ctrl.led_mode = v4l2_ctrl_new_std_menu(
>> + &v4l2_flash->hdl,
>> + &v4l2_flash_ctrl_ops, V4L2_CID_FLASH_LED_MODE,
>> + V4L2_FLASH_LED_MODE_TORCH, ~mask,
>> + V4L2_FLASH_LED_MODE_NONE);
>> +
>> + /* Configure TORCH_INTENSITY ctrl */
>> + ctrl_cfg = &led_cdev->brightness_ctrl;
>> + ctrl = v4l2_ctrl_new_std(&v4l2_flash->hdl, &v4l2_flash_ctrl_ops,
>> + V4L2_CID_FLASH_TORCH_INTENSITY,
>> + ctrl_cfg->min, ctrl_cfg->max,
>> + ctrl_cfg->step, ctrl_cfg->val);
>> + if (ctrl)
>> + ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE;
>> + v4l2_flash->ctrl.torch_intensity = ctrl;
>> +
>> + if (flash->has_flash_led) {
>> + /* Configure FLASH_STROBE_SOURCE ctrl */
>> + mask = 1 << V4L2_FLASH_STROBE_SOURCE_SOFTWARE;
>> +
>> + if (flash->has_external_strobe) {
>> + mask |= 1 << V4L2_FLASH_STROBE_SOURCE_EXTERNAL;
>> + max = V4L2_FLASH_STROBE_SOURCE_EXTERNAL;
>> + } else {
>> + max = V4L2_FLASH_STROBE_SOURCE_SOFTWARE;
>> + }
>> +
>> + v4l2_flash->ctrl.source = v4l2_ctrl_new_std_menu(
>> + &v4l2_flash->hdl,
>> + &v4l2_flash_ctrl_ops,
>> + V4L2_CID_FLASH_STROBE_SOURCE,
>> + max,
>> + ~mask,
>> + V4L2_FLASH_STROBE_SOURCE_SOFTWARE);
>> +
>> + /* Configure FLASH_STROBE ctrl */
>> + ctrl = v4l2_ctrl_new_std(&v4l2_flash->hdl, &v4l2_flash_ctrl_ops,
>> + V4L2_CID_FLASH_STROBE, 0, 1, 1, 0);
>> +
>> + /* Configure FLASH_STROBE_STOP ctrl */
>> + ctrl = v4l2_ctrl_new_std(&v4l2_flash->hdl, &v4l2_flash_ctrl_ops,
>> + V4L2_CID_FLASH_STROBE_STOP,
>> + 0, 1, 1, 0);
>> +
>> + /* Configure FLASH_STROBE_STATUS ctrl */
>> + ctrl = v4l2_ctrl_new_std(&v4l2_flash->hdl, &v4l2_flash_ctrl_ops,
>> + V4L2_CID_FLASH_STROBE_STATUS,
>> + 0, 1, 1, 1);
>> + if (ctrl)
>> + ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE |
>> + V4L2_CTRL_FLAG_READ_ONLY;
>> +
>> + /* Configure FLASH_TIMEOUT ctrl */
>> + ctrl_cfg = &flash->timeout;
>> + ctrl = v4l2_ctrl_new_std(&v4l2_flash->hdl, &v4l2_flash_ctrl_ops,
>> + V4L2_CID_FLASH_TIMEOUT, ctrl_cfg->min,
>> + ctrl_cfg->max, ctrl_cfg->step,
>> + ctrl_cfg->val);
>> + if (ctrl)
>> + ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE;
>> +
>> + /* Configure FLASH_INTENSITY ctrl */
>> + ctrl_cfg = &flash->brightness;
>> + ctrl = v4l2_ctrl_new_std(&v4l2_flash->hdl, &v4l2_flash_ctrl_ops,
>> + V4L2_CID_FLASH_INTENSITY,
>> + ctrl_cfg->min, ctrl_cfg->max,
>> + ctrl_cfg->step, ctrl_cfg->val);
>> + if (ctrl)
>> + ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE;
>> +
>> + if (flash->fault_flags) {
>> + /* Configure FLASH_FAULT ctrl */
>> + ctrl = v4l2_ctrl_new_std(&v4l2_flash->hdl,
>> + &v4l2_flash_ctrl_ops,
>> + V4L2_CID_FLASH_FAULT, 0,
>> + flash->fault_flags,
>> + 0, 0);
>> + if (ctrl)
>> + ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE |
>> + V4L2_CTRL_FLAG_READ_ONLY;
>> + }
>> + if (has_indicator) {
>
> In theory it's possible to have an indicator without the flash. So I'd keep
> the two independent.
OK.
>> + /* Configure FLASH_INDICATOR_INTENSITY ctrl */
>> + ctrl_cfg = flash->indicator_brightness;
>> + ctrl = v4l2_ctrl_new_std(
>> + &v4l2_flash->hdl, &v4l2_flash_ctrl_ops,
>> + V4L2_CID_FLASH_INDICATOR_INTENSITY,
>> + ctrl_cfg->min, ctrl_cfg->max,
>> + ctrl_cfg->step, ctrl_cfg->val);
>> + if (ctrl)
>> + ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE;
>> + }
>> + }
>> +
>> + if (v4l2_flash->hdl.error) {
>> + ret = v4l2_flash->hdl.error;
>> + goto error_free;
>> + }
>> +
>> + ret = v4l2_ctrl_handler_setup(&v4l2_flash->hdl);
>> + if (ret < 0)
>> + goto error_free;
>> +
>> + v4l2_flash->subdev.ctrl_handler = &v4l2_flash->hdl;
>> +
>> + return 0;
>> +
>> +error_free:
>> + v4l2_ctrl_handler_free(&v4l2_flash->hdl);
>> + return ret;
>> +}
>> +
>> +/*
>> + * V4L2 subdev internal operations
>> + */
>> +
>> +static int v4l2_flash_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
>> +{
>> + struct v4l2_flash *v4l2_flash = v4l2_subdev_to_v4l2_flash(sd);
>> + struct led_classdev *led_cdev = v4l2_flash->led_cdev;
>> +
>> + mutex_lock(&led_cdev->led_lock);
>> + v4l2_call_flash_op(sysfs_lock, led_cdev);
>
> Have you thought about device power management yet?
Having in mind that the V4L2 Flash sub-device is only a wrapper
for LED driver, shouldn't power management be left to the
drivers?
> Traditionally, the existing flash controller drivers are powered when a file
> handle is opened. Many newer implementations seem to consume so little power
> that it's not worthwhile to try to power them off.
>
> Binding the power state to open file handles isn't very generic either but
> it's very simple and works.
>
> V4L2 sub-devices do not use runtime PM as of yet, so perhaps this could be
> left for later.
>
>> + mutex_unlock(&led_cdev->led_lock);
>> +
>> + return 0;
>> +}
>> +
>> +static int v4l2_flash_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
>> +{
>> + struct v4l2_flash *v4l2_flash = v4l2_subdev_to_v4l2_flash(sd);
>> + struct led_classdev *led_cdev = v4l2_flash->led_cdev;
>> +
>> + mutex_lock(&led_cdev->led_lock);
>> + v4l2_call_flash_op(sysfs_unlock, led_cdev);
>> + mutex_unlock(&led_cdev->led_lock);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct v4l2_subdev_internal_ops v4l2_flash_subdev_internal_ops = {
>> + .open = v4l2_flash_open,
>> + .close = v4l2_flash_close,
>> +};
>> +
>> +static struct v4l2_subdev_ops v4l2_flash_subdev_ops = {
>> +};
>> +
>> +int v4l2_flash_init(struct led_classdev *led_cdev,
>> + const struct v4l2_flash_ops *ops)
>> +{
>> + struct v4l2_flash *flash = &led_cdev->flash->v4l2_flash;
>> + struct v4l2_subdev *sd = &flash->subdev;
>> + int ret;
>> +
>> + if (!led_cdev || !ops)
>> + return -EINVAL;
>> +
>> + flash->led_cdev = led_cdev;
>> + sd->dev = led_cdev->dev->parent;
>> + v4l2_subdev_init(sd, &v4l2_flash_subdev_ops);
>> + sd->internal_ops = &v4l2_flash_subdev_internal_ops;
>> + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>> + snprintf(sd->name, sizeof(sd->name), led_cdev->name);
>
> It'd be nice to have the bus address in the name as well. But that would
> belong to the driver which calls v4l2_flash_init(). The existing flash
> controller drivers probably don't do this but many sensor drivers already
> do. The name is expected to be unique in the media device.
Will cover this.
>> + flash->ops = ops;
>> +
>> + ret = v4l2_flash_init_controls(flash);
>> + if (ret < 0)
>> + goto err_init_controls;
>> +
>> + ret = media_entity_init(&sd->entity, 0, NULL, 0);
>> + if (ret < 0)
>> + goto err_init_entity;
>> +
>> + sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_FLASH;
>> +
>> + ret = v4l2_async_register_subdev(sd);
>> + if (ret < 0)
>> + goto err_init_entity;
>> +
>> + return 0;
>> +
>> +err_init_entity:
>> + media_entity_cleanup(&sd->entity);
>> +err_init_controls:
>> + v4l2_ctrl_handler_free(sd->ctrl_handler);
>> + return -EINVAL;
>> +}
>> +EXPORT_SYMBOL_GPL(v4l2_flash_init);
>> +
>> +void v4l2_flash_release(struct led_classdev *led_cdev)
>> +{
>> + struct v4l2_flash *flash = &led_cdev->flash->v4l2_flash;
>> +
>> + v4l2_ctrl_handler_free(flash->subdev.ctrl_handler);
>> + v4l2_async_unregister_subdev(&flash->subdev);
>> + media_entity_cleanup(&flash->subdev.entity);
>> +}
>> +EXPORT_SYMBOL_GPL(v4l2_flash_release);
>> diff --git a/include/media/v4l2-flash.h b/include/media/v4l2-flash.h
>> new file mode 100644
>> index 0000000..fe16ddd
>> --- /dev/null
>> +++ b/include/media/v4l2-flash.h
>> @@ -0,0 +1,119 @@
>> +/*
>> + * 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>
>> +#include <media/v4l2-event.h>
>> +#include <media/v4l2-ioctl.h>
>> +
>> +#define v4l2_call_flash_op(op, args...) \
>> + ((v4l2_flash)->ops->op(args)) \
>
> Extra backslash above.
>
> I don't think you should assume the caller will have a local variable called
> v4l2_flash. :-)
I assumed that as this macro is only for the module internal use
and aim at simplifying the calling code it can be implemented that way.
But when I look at it now it indeed doesn't seem reasonable.
>> +struct led_classdev;
>> +enum led_brightness;
>> +
>> +struct v4l2_flash_ops {
>> + void (*brightness_set)(struct led_classdev *led_cdev,
>> + enum led_brightness brightness);
>> + int (*brightness_update)(struct led_classdev *led_cdev);
>> + int (*flash_brightness_set)(struct led_classdev *led_cdev,
>> + u32 brightness);
>> + int (*flash_brightness_update)(struct led_classdev *led_cdev);
>> + int (*strobe_set)(struct led_classdev *led_cdev,
>> + bool state);
>> + int (*strobe_get)(struct led_classdev *led_cdev);
>> + int (*timeout_set)(struct led_classdev *led_cdev,
>> + u32 timeout);
>> + int (*indicator_brightness_set)(struct led_classdev *led_cdev,
>> + u32 brightness);
>> + int (*indicator_brightness_update)(struct led_classdev *led_cdev);
>> + int (*external_strobe_set)(struct led_classdev *led_cdev,
>> + bool enable);
>> + int (*fault_get)(struct led_classdev *led_cdev,
>> + u32 *fault);
>> + void (*sysfs_lock)(struct led_classdev *led_cdev);
>> + void (*sysfs_unlock)(struct led_classdev *led_cdev);
>> +};
>> +
>> +/**
>> + * struct v4l2_flash_ctrl - controls that define the sub-dev's state
>> + * @source: V4L2_CID_FLASH_STROBE_SOURCE control
>> + * @led_mode: V4L2_CID_FLASH_LED_MODE control
>> + * @torch_intensity: V4L2_CID_FLASH_TORCH_INTENSITY control
>> + */
>> +struct v4l2_flash_ctrl {
>> + struct v4l2_ctrl *source;
>> + struct v4l2_ctrl *led_mode;
>> + struct v4l2_ctrl *torch_intensity;
>> +};
>> +
>> +/**
>> + * struct v4l2_flash - Flash sub-device context
>> + * @led_cdev: LED class device controlled by this sub-device
>> + * @ops: LED class device ops
>> + * @subdev: V4L2 sub-device
>> + * @hdl: flash controls handler
>> + * @ctrl: state defining controls
>> + */
>> +struct v4l2_flash {
>> + struct led_classdev *led_cdev;
>> + const struct v4l2_flash_ops *ops;
>> +
>> + struct v4l2_subdev subdev;
>> + struct v4l2_ctrl_handler hdl;
>> + struct v4l2_flash_ctrl ctrl;
>> +};
>> +
>> +static inline struct v4l2_flash *v4l2_subdev_to_v4l2_flash(struct v4l2_subdev *sd)
>
> Over 80 characters per line.
>
>> +{
>> + return container_of(sd, struct v4l2_flash, subdev);
>> +}
>> +
>> +static inline struct v4l2_flash *v4l2_ctrl_to_v4l2_flash(struct v4l2_ctrl *c)
>> +{
>> + return container_of(c->handler, struct v4l2_flash, hdl);
>> +}
>> +
>> +#ifdef CONFIG_V4L2_FLASH
>> +/**
>> + * v4l2_flash_init - initialize V4L2 flash led sub-device
>> + * @led_cdev: the LED to create subdev upon
>> + * @ops: LED subsystem callbacks
>> + *
>> + * Create V4L2 subdev wrapping given LED subsystem device.
>> + */
>> +int v4l2_flash_init(struct led_classdev *led_cdev,
>> + const struct v4l2_flash_ops *ops);
>> +
>> +/**
>> + * v4l2_flash_release - release V4L2 flash led sub-device
>> + * @flash: a structure representing V4L2 flash led device
>> + *
>> + * Release V4L2 flash led subdev.
>> + */
>> +void v4l2_flash_release(struct led_classdev *led_cdev);
>> +#else
>> +static inline int v4l2_flash_init(struct led_classdev *led_cdev,
>> + const struct v4l2_flash_ops *ops)
>> +{
>> + return 0;
>> +}
>> +
>> +static inline void v4l2_flash_release(struct led_classdev *led_cdev)
>> +{
>> +}
>
> Could you put the two to the first patch? It won't compile otherwise.
Yeah. I didn't make test build after every patch :-/.
>> +#endif /* CONFIG_V4L2_FLASH */
>> +
>> +#endif /* _V4L2_FLASH_H */
>
--
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/
Powered by blists - more mailing lists