* [PATCH v6 1/7] v4l: Share code between video_usercopy and video_ioctl2
2011-01-27 12:28 [PATCH v6 0/7] V4L2 subdev userspace API Laurent Pinchart
@ 2011-01-27 12:28 ` Laurent Pinchart
2011-01-27 12:28 ` [PATCH v6 2/7] v4l: subdev: Don't require core operations Laurent Pinchart
` (5 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Laurent Pinchart @ 2011-01-27 12:28 UTC (permalink / raw)
To: linux-media; +Cc: sakari.ailus
The two functions are mostly identical. They handle the copy_from_user
and copy_to_user operations related with V4L2 ioctls and call the real
ioctl handler.
Create a __video_usercopy function that implements the core of
video_usercopy and video_ioctl2, and call that function from both.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/video/v4l2-ioctl.c | 218 ++++++++++++-------------------------
1 files changed, 71 insertions(+), 147 deletions(-)
diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
index dd9283f..1e01554 100644
--- a/drivers/media/video/v4l2-ioctl.c
+++ b/drivers/media/video/v4l2-ioctl.c
@@ -374,35 +374,62 @@ video_fix_command(unsigned int cmd)
}
#endif
-/*
- * Obsolete usercopy function - Should be removed soon
- */
-long
-video_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
+/* In some cases, only a few fields are used as input, i.e. when the app sets
+ * "index" and then the driver fills in the rest of the structure for the thing
+ * with that index. We only need to copy up the first non-input field. */
+static unsigned long cmd_input_size(unsigned int cmd)
+{
+ /* Size of structure up to and including 'field' */
+#define CMDINSIZE(cmd, type, field) \
+ case VIDIOC_##cmd: \
+ return offsetof(struct v4l2_##type, field) + \
+ sizeof(((struct v4l2_##type *)0)->field);
+
+ switch (cmd) {
+ CMDINSIZE(ENUM_FMT, fmtdesc, type);
+ CMDINSIZE(G_FMT, format, type);
+ CMDINSIZE(QUERYBUF, buffer, type);
+ CMDINSIZE(G_PARM, streamparm, type);
+ CMDINSIZE(ENUMSTD, standard, index);
+ CMDINSIZE(ENUMINPUT, input, index);
+ CMDINSIZE(G_CTRL, control, id);
+ CMDINSIZE(G_TUNER, tuner, index);
+ CMDINSIZE(QUERYCTRL, queryctrl, id);
+ CMDINSIZE(QUERYMENU, querymenu, index);
+ CMDINSIZE(ENUMOUTPUT, output, index);
+ CMDINSIZE(G_MODULATOR, modulator, index);
+ CMDINSIZE(G_FREQUENCY, frequency, tuner);
+ CMDINSIZE(CROPCAP, cropcap, type);
+ CMDINSIZE(G_CROP, crop, type);
+ CMDINSIZE(ENUMAUDIO, audio, index);
+ CMDINSIZE(ENUMAUDOUT, audioout, index);
+ CMDINSIZE(ENCODER_CMD, encoder_cmd, flags);
+ CMDINSIZE(TRY_ENCODER_CMD, encoder_cmd, flags);
+ CMDINSIZE(G_SLICED_VBI_CAP, sliced_vbi_cap, type);
+ CMDINSIZE(ENUM_FRAMESIZES, frmsizeenum, pixel_format);
+ CMDINSIZE(ENUM_FRAMEINTERVALS, frmivalenum, height);
+ default:
+ return _IOC_SIZE(cmd);
+ }
+}
+
+static long
+__video_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
v4l2_kioctl func)
{
char sbuf[128];
void *mbuf = NULL;
- void *parg = NULL;
+ void *parg = (void *)arg;
long err = -EINVAL;
int is_ext_ctrl;
size_t ctrls_size = 0;
void __user *user_ptr = NULL;
-#ifdef __OLD_VIDIOC_
- cmd = video_fix_command(cmd);
-#endif
is_ext_ctrl = (cmd == VIDIOC_S_EXT_CTRLS || cmd == VIDIOC_G_EXT_CTRLS ||
cmd == VIDIOC_TRY_EXT_CTRLS);
/* Copy arguments into temp kernel buffer */
- switch (_IOC_DIR(cmd)) {
- case _IOC_NONE:
- parg = NULL;
- break;
- case _IOC_READ:
- case _IOC_WRITE:
- case (_IOC_WRITE | _IOC_READ):
+ if (_IOC_DIR(cmd) != _IOC_NONE) {
if (_IOC_SIZE(cmd) <= sizeof(sbuf)) {
parg = sbuf;
} else {
@@ -414,11 +441,21 @@ video_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
}
err = -EFAULT;
- if (_IOC_DIR(cmd) & _IOC_WRITE)
- if (copy_from_user(parg, (void __user *)arg, _IOC_SIZE(cmd)))
+ if (_IOC_DIR(cmd) & _IOC_WRITE) {
+ unsigned long n = cmd_input_size(cmd);
+
+ if (copy_from_user(parg, (void __user *)arg, n))
goto out;
- break;
+
+ /* zero out anything we don't copy from userspace */
+ if (n < _IOC_SIZE(cmd))
+ memset((u8 *)parg + n, 0, _IOC_SIZE(cmd) - n);
+ } else {
+ /* read-only ioctl */
+ memset(parg, 0, _IOC_SIZE(cmd));
+ }
}
+
if (is_ext_ctrl) {
struct v4l2_ext_controls *p = parg;
@@ -440,7 +477,7 @@ video_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
}
}
- /* call driver */
+ /* Handles IOCTL */
err = func(file, cmd, parg);
if (err == -ENOIOCTLCMD)
err = -EINVAL;
@@ -469,6 +506,19 @@ out:
kfree(mbuf);
return err;
}
+
+/*
+ * Obsolete usercopy function - Should be removed soon
+ */
+long
+video_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
+ v4l2_kioctl func)
+{
+#ifdef __OLD_VIDIOC_
+ cmd = video_fix_command(cmd);
+#endif
+ return __video_usercopy(file, cmd, arg, func);
+}
EXPORT_SYMBOL(video_usercopy);
static void dbgbuf(unsigned int cmd, struct video_device *vfd,
@@ -2041,138 +2091,12 @@ static long __video_do_ioctl(struct file *file,
return ret;
}
-/* In some cases, only a few fields are used as input, i.e. when the app sets
- * "index" and then the driver fills in the rest of the structure for the thing
- * with that index. We only need to copy up the first non-input field. */
-static unsigned long cmd_input_size(unsigned int cmd)
-{
- /* Size of structure up to and including 'field' */
-#define CMDINSIZE(cmd, type, field) \
- case VIDIOC_##cmd: \
- return offsetof(struct v4l2_##type, field) + \
- sizeof(((struct v4l2_##type *)0)->field);
-
- switch (cmd) {
- CMDINSIZE(ENUM_FMT, fmtdesc, type);
- CMDINSIZE(G_FMT, format, type);
- CMDINSIZE(QUERYBUF, buffer, type);
- CMDINSIZE(G_PARM, streamparm, type);
- CMDINSIZE(ENUMSTD, standard, index);
- CMDINSIZE(ENUMINPUT, input, index);
- CMDINSIZE(G_CTRL, control, id);
- CMDINSIZE(G_TUNER, tuner, index);
- CMDINSIZE(QUERYCTRL, queryctrl, id);
- CMDINSIZE(QUERYMENU, querymenu, index);
- CMDINSIZE(ENUMOUTPUT, output, index);
- CMDINSIZE(G_MODULATOR, modulator, index);
- CMDINSIZE(G_FREQUENCY, frequency, tuner);
- CMDINSIZE(CROPCAP, cropcap, type);
- CMDINSIZE(G_CROP, crop, type);
- CMDINSIZE(ENUMAUDIO, audio, index);
- CMDINSIZE(ENUMAUDOUT, audioout, index);
- CMDINSIZE(ENCODER_CMD, encoder_cmd, flags);
- CMDINSIZE(TRY_ENCODER_CMD, encoder_cmd, flags);
- CMDINSIZE(G_SLICED_VBI_CAP, sliced_vbi_cap, type);
- CMDINSIZE(ENUM_FRAMESIZES, frmsizeenum, pixel_format);
- CMDINSIZE(ENUM_FRAMEINTERVALS, frmivalenum, height);
- default:
- return _IOC_SIZE(cmd);
- }
-}
-
long video_ioctl2(struct file *file,
unsigned int cmd, unsigned long arg)
{
- char sbuf[128];
- void *mbuf = NULL;
- void *parg = (void *)arg;
- long err = -EINVAL;
- int is_ext_ctrl;
- size_t ctrls_size = 0;
- void __user *user_ptr = NULL;
-
#ifdef __OLD_VIDIOC_
cmd = video_fix_command(cmd);
#endif
- is_ext_ctrl = (cmd == VIDIOC_S_EXT_CTRLS || cmd == VIDIOC_G_EXT_CTRLS ||
- cmd == VIDIOC_TRY_EXT_CTRLS);
-
- /* Copy arguments into temp kernel buffer */
- if (_IOC_DIR(cmd) != _IOC_NONE) {
- if (_IOC_SIZE(cmd) <= sizeof(sbuf)) {
- parg = sbuf;
- } else {
- /* too big to allocate from stack */
- mbuf = kmalloc(_IOC_SIZE(cmd), GFP_KERNEL);
- if (NULL == mbuf)
- return -ENOMEM;
- parg = mbuf;
- }
-
- err = -EFAULT;
- if (_IOC_DIR(cmd) & _IOC_WRITE) {
- unsigned long n = cmd_input_size(cmd);
-
- if (copy_from_user(parg, (void __user *)arg, n))
- goto out;
-
- /* zero out anything we don't copy from userspace */
- if (n < _IOC_SIZE(cmd))
- memset((u8 *)parg + n, 0, _IOC_SIZE(cmd) - n);
- } else {
- /* read-only ioctl */
- memset(parg, 0, _IOC_SIZE(cmd));
- }
- }
-
- if (is_ext_ctrl) {
- struct v4l2_ext_controls *p = parg;
-
- /* In case of an error, tell the caller that it wasn't
- a specific control that caused it. */
- p->error_idx = p->count;
- user_ptr = (void __user *)p->controls;
- if (p->count) {
- ctrls_size = sizeof(struct v4l2_ext_control) * p->count;
- /* Note: v4l2_ext_controls fits in sbuf[] so mbuf is still NULL. */
- mbuf = kmalloc(ctrls_size, GFP_KERNEL);
- err = -ENOMEM;
- if (NULL == mbuf)
- goto out_ext_ctrl;
- err = -EFAULT;
- if (copy_from_user(mbuf, user_ptr, ctrls_size))
- goto out_ext_ctrl;
- p->controls = mbuf;
- }
- }
-
- /* Handles IOCTL */
- err = __video_do_ioctl(file, cmd, parg);
- if (err == -ENOIOCTLCMD)
- err = -EINVAL;
- if (is_ext_ctrl) {
- struct v4l2_ext_controls *p = parg;
-
- p->controls = (void *)user_ptr;
- if (p->count && err == 0 && copy_to_user(user_ptr, mbuf, ctrls_size))
- err = -EFAULT;
- goto out_ext_ctrl;
- }
- if (err < 0)
- goto out;
-
-out_ext_ctrl:
- /* Copy results into user buffer */
- switch (_IOC_DIR(cmd)) {
- case _IOC_READ:
- case (_IOC_WRITE | _IOC_READ):
- if (copy_to_user((void __user *)arg, parg, _IOC_SIZE(cmd)))
- err = -EFAULT;
- break;
- }
-
-out:
- kfree(mbuf);
- return err;
+ return __video_usercopy(file, cmd, arg, __video_do_ioctl);
}
EXPORT_SYMBOL(video_ioctl2);
--
1.7.3.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v6 2/7] v4l: subdev: Don't require core operations
2011-01-27 12:28 [PATCH v6 0/7] V4L2 subdev userspace API Laurent Pinchart
2011-01-27 12:28 ` [PATCH v6 1/7] v4l: Share code between video_usercopy and video_ioctl2 Laurent Pinchart
@ 2011-01-27 12:28 ` Laurent Pinchart
2011-01-27 12:28 ` [PATCH v6 3/7] v4l: subdev: Merge v4l2_i2c_new_subdev_cfg and v4l2_i2c_new_subdev Laurent Pinchart
` (4 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Laurent Pinchart @ 2011-01-27 12:28 UTC (permalink / raw)
To: linux-media; +Cc: sakari.ailus
There's no reason to require subdevices to implement the core
operations. Remove the check for non-NULL core operations when
initializing the subdev.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
include/media/v4l2-subdev.h | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index b0316a7..b636444 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -466,8 +466,7 @@ static inline void v4l2_subdev_init(struct v4l2_subdev *sd,
const struct v4l2_subdev_ops *ops)
{
INIT_LIST_HEAD(&sd->list);
- /* ops->core MUST be set */
- BUG_ON(!ops || !ops->core);
+ BUG_ON(!ops);
sd->ops = ops;
sd->v4l2_dev = NULL;
sd->flags = 0;
--
1.7.3.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v6 3/7] v4l: subdev: Merge v4l2_i2c_new_subdev_cfg and v4l2_i2c_new_subdev
2011-01-27 12:28 [PATCH v6 0/7] V4L2 subdev userspace API Laurent Pinchart
2011-01-27 12:28 ` [PATCH v6 1/7] v4l: Share code between video_usercopy and video_ioctl2 Laurent Pinchart
2011-01-27 12:28 ` [PATCH v6 2/7] v4l: subdev: Don't require core operations Laurent Pinchart
@ 2011-01-27 12:28 ` Laurent Pinchart
2011-01-27 12:28 ` [PATCH v6 4/7] v4l: subdev: Add device node support Laurent Pinchart
` (3 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Laurent Pinchart @ 2011-01-27 12:28 UTC (permalink / raw)
To: linux-media; +Cc: sakari.ailus
v4l2_i2c_new_subdev is a thin wrapper around v4l2_i2c_new_subdev_cfg,
which is itself a wrapper around v4l2_i2c_new_subdev_board.
The intermediate v4l2_i2c_new_subdev_cfg function is called directly by
the ivtv and cafe-ccic drivers only. Merge it with v4l2_i2c_new_subdev
and use v4l2_i2c_new_subdev_board in the ivtv and cafe-ccic drivers.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/video/cafe_ccic.c | 11 +++++++++--
drivers/media/video/ivtv/ivtv-i2c.c | 11 +++++++++--
drivers/media/video/v4l2-common.c | 7 ++-----
include/media/v4l2-common.h | 13 +------------
4 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/drivers/media/video/cafe_ccic.c b/drivers/media/video/cafe_ccic.c
index 0dfff50..6e23add 100644
--- a/drivers/media/video/cafe_ccic.c
+++ b/drivers/media/video/cafe_ccic.c
@@ -1992,6 +1992,7 @@ static int cafe_pci_probe(struct pci_dev *pdev,
{
int ret;
struct cafe_camera *cam;
+ struct i2c_board_info info;
struct ov7670_config sensor_cfg = {
/* This controller only does SMBUS */
.use_smbus = true,
@@ -2065,8 +2066,14 @@ static int cafe_pci_probe(struct pci_dev *pdev,
sensor_cfg.clock_speed = 45;
cam->sensor_addr = 0x42;
- cam->sensor = v4l2_i2c_new_subdev_cfg(&cam->v4l2_dev, &cam->i2c_adapter,
- "ov7670", 0, &sensor_cfg, cam->sensor_addr, NULL);
+
+ memset(&info, 0, sizeof(info));
+ strlcpy(info.type, "ov7670", sizeof(info.type));
+ info.addr = cam->sensor_addr;
+ info.platform_data = &sensor_cfg;
+
+ cam->sensor = v4l2_i2c_new_subdev_board(&cam->v4l2_dev,
+ &cam->i2c_adapter, &info, NULL);
if (cam->sensor == NULL) {
ret = -ENODEV;
goto out_smbus;
diff --git a/drivers/media/video/ivtv/ivtv-i2c.c b/drivers/media/video/ivtv/ivtv-i2c.c
index 665191c..6651a6c 100644
--- a/drivers/media/video/ivtv/ivtv-i2c.c
+++ b/drivers/media/video/ivtv/ivtv-i2c.c
@@ -267,10 +267,17 @@ int ivtv_i2c_register(struct ivtv *itv, unsigned idx)
adap, type, 0, I2C_ADDRS(hw_addrs[idx]));
} else if (hw == IVTV_HW_CX25840) {
struct cx25840_platform_data pdata;
+ struct i2c_board_info info;
pdata.pvr150_workaround = itv->pvr150_workaround;
- sd = v4l2_i2c_new_subdev_cfg(&itv->v4l2_dev,
- adap, type, 0, &pdata, hw_addrs[idx], NULL);
+
+ memset(&info, 0, sizeof(info));
+ strlcpy(info.type, type, sizeof(info.type));
+ info.addr = hw_addrs[idx];
+ info.platform_data = &pdata;
+
+ sd = v4l2_i2c_new_subdev_board(&itv->v4l2_dev, adap, &info,
+ NULL);
} else {
sd = v4l2_i2c_new_subdev(&itv->v4l2_dev,
adap, type, hw_addrs[idx], NULL);
diff --git a/drivers/media/video/v4l2-common.c b/drivers/media/video/v4l2-common.c
index b5eb1f3..e007e61 100644
--- a/drivers/media/video/v4l2-common.c
+++ b/drivers/media/video/v4l2-common.c
@@ -428,9 +428,8 @@ error:
}
EXPORT_SYMBOL_GPL(v4l2_i2c_new_subdev_board);
-struct v4l2_subdev *v4l2_i2c_new_subdev_cfg(struct v4l2_device *v4l2_dev,
+struct v4l2_subdev *v4l2_i2c_new_subdev(struct v4l2_device *v4l2_dev,
struct i2c_adapter *adapter, const char *client_type,
- int irq, void *platform_data,
u8 addr, const unsigned short *probe_addrs)
{
struct i2c_board_info info;
@@ -440,12 +439,10 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_cfg(struct v4l2_device *v4l2_dev,
memset(&info, 0, sizeof(info));
strlcpy(info.type, client_type, sizeof(info.type));
info.addr = addr;
- info.irq = irq;
- info.platform_data = platform_data;
return v4l2_i2c_new_subdev_board(v4l2_dev, adapter, &info, probe_addrs);
}
-EXPORT_SYMBOL_GPL(v4l2_i2c_new_subdev_cfg);
+EXPORT_SYMBOL_GPL(v4l2_i2c_new_subdev);
/* Return i2c client address of v4l2_subdev. */
unsigned short v4l2_i2c_subdev_addr(struct v4l2_subdev *sd)
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index 239125a..565fb32 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -138,21 +138,10 @@ struct v4l2_subdev_ops;
/* Load an i2c module and return an initialized v4l2_subdev struct.
The client_type argument is the name of the chip that's on the adapter. */
-struct v4l2_subdev *v4l2_i2c_new_subdev_cfg(struct v4l2_device *v4l2_dev,
+struct v4l2_subdev *v4l2_i2c_new_subdev(struct v4l2_device *v4l2_dev,
struct i2c_adapter *adapter, const char *client_type,
- int irq, void *platform_data,
u8 addr, const unsigned short *probe_addrs);
-/* Load an i2c module and return an initialized v4l2_subdev struct.
- The client_type argument is the name of the chip that's on the adapter. */
-static inline struct v4l2_subdev *v4l2_i2c_new_subdev(struct v4l2_device *v4l2_dev,
- struct i2c_adapter *adapter, const char *client_type,
- u8 addr, const unsigned short *probe_addrs)
-{
- return v4l2_i2c_new_subdev_cfg(v4l2_dev, adapter, client_type, 0, NULL,
- addr, probe_addrs);
-}
-
struct i2c_board_info;
struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
--
1.7.3.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v6 4/7] v4l: subdev: Add device node support
2011-01-27 12:28 [PATCH v6 0/7] V4L2 subdev userspace API Laurent Pinchart
` (2 preceding siblings ...)
2011-01-27 12:28 ` [PATCH v6 3/7] v4l: subdev: Merge v4l2_i2c_new_subdev_cfg and v4l2_i2c_new_subdev Laurent Pinchart
@ 2011-01-27 12:28 ` Laurent Pinchart
2011-02-04 10:09 ` Hans Verkuil
2011-01-27 12:28 ` [PATCH v6 5/7] v4l: subdev: Uninline the v4l2_subdev_init function Laurent Pinchart
` (2 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Laurent Pinchart @ 2011-01-27 12:28 UTC (permalink / raw)
To: linux-media; +Cc: sakari.ailus
Create a device node named subdevX for every registered subdev.
As the device node is registered before the subdev core::s_config
function is called, return -EGAIN on open until initialization
completes.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Vimarsh Zutshi <vimarsh.zutshi@nokia.com>
---
Documentation/video4linux/v4l2-framework.txt | 18 +++++++
drivers/media/radio/radio-si4713.c | 2 +-
drivers/media/video/Makefile | 2 +-
drivers/media/video/cafe_ccic.c | 2 +-
drivers/media/video/davinci/vpfe_capture.c | 2 +-
drivers/media/video/davinci/vpif_capture.c | 2 +-
drivers/media/video/davinci/vpif_display.c | 2 +-
drivers/media/video/ivtv/ivtv-i2c.c | 2 +-
drivers/media/video/s5p-fimc/fimc-capture.c | 2 +-
drivers/media/video/sh_vou.c | 2 +-
drivers/media/video/soc_camera.c | 2 +-
drivers/media/video/v4l2-common.c | 15 +++++-
drivers/media/video/v4l2-dev.c | 27 ++++------
drivers/media/video/v4l2-device.c | 24 +++++++++-
drivers/media/video/v4l2-ioctl.c | 2 +-
drivers/media/video/v4l2-subdev.c | 66 ++++++++++++++++++++++++++
include/media/v4l2-common.h | 5 +-
include/media/v4l2-dev.h | 18 ++++++-
include/media/v4l2-ioctl.h | 3 +
include/media/v4l2-subdev.h | 16 ++++++-
20 files changed, 176 insertions(+), 38 deletions(-)
create mode 100644 drivers/media/video/v4l2-subdev.c
diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt
index f22f35c..4c9185a 100644
--- a/Documentation/video4linux/v4l2-framework.txt
+++ b/Documentation/video4linux/v4l2-framework.txt
@@ -319,6 +319,24 @@ controlled through GPIO pins. This distinction is only relevant when setting
up the device, but once the subdev is registered it is completely transparent.
+V4L2 sub-device userspace API
+-----------------------------
+
+Beside exposing a kernel API through the v4l2_subdev_ops structure, V4L2
+sub-devices can also be controlled directly by userspace applications.
+
+When a sub-device is registered, a device node named v4l-subdevX can be created
+in /dev. If the sub-device supports direct userspace configuration it must set
+the V4L2_SUBDEV_FL_HAS_DEVNODE flag before being registered.
+
+For I2C and SPI sub-devices, the v4l2_device driver can disable registration of
+the device node if it wants to control the sub-device on its own. In that case
+it must set the v4l2_i2c_new_subdev_board or v4l2_spi_new_subdev enable_devnode
+argument to 0. Setting the argument to 1 will only enable device node
+registration if the sub-device driver has set the V4L2_SUBDEV_FL_HAS_DEVNODE
+flag.
+
+
I2C sub-device drivers
----------------------
diff --git a/drivers/media/radio/radio-si4713.c b/drivers/media/radio/radio-si4713.c
index 726d367..f7c942f 100644
--- a/drivers/media/radio/radio-si4713.c
+++ b/drivers/media/radio/radio-si4713.c
@@ -293,7 +293,7 @@ static int radio_si4713_pdriver_probe(struct platform_device *pdev)
}
sd = v4l2_i2c_new_subdev_board(&rsdev->v4l2_dev, adapter,
- pdata->subdev_board_info, NULL);
+ pdata->subdev_board_info, NULL, 0);
if (!sd) {
dev_err(&pdev->dev, "Cannot get v4l2 subdevice\n");
rval = -ENODEV;
diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
index af79d47..adc1bd5 100644
--- a/drivers/media/video/Makefile
+++ b/drivers/media/video/Makefile
@@ -11,7 +11,7 @@ stkwebcam-objs := stk-webcam.o stk-sensor.o
omap2cam-objs := omap24xxcam.o omap24xxcam-dma.o
videodev-objs := v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
- v4l2-event.o v4l2-ctrls.o
+ v4l2-event.o v4l2-ctrls.o v4l2-subdev.o
# V4L2 core modules
diff --git a/drivers/media/video/cafe_ccic.c b/drivers/media/video/cafe_ccic.c
index 6e23add..f932da1 100644
--- a/drivers/media/video/cafe_ccic.c
+++ b/drivers/media/video/cafe_ccic.c
@@ -2073,7 +2073,7 @@ static int cafe_pci_probe(struct pci_dev *pdev,
info.platform_data = &sensor_cfg;
cam->sensor = v4l2_i2c_new_subdev_board(&cam->v4l2_dev,
- &cam->i2c_adapter, &info, NULL);
+ &cam->i2c_adapter, &info, NULL, 0);
if (cam->sensor == NULL) {
ret = -ENODEV;
goto out_smbus;
diff --git a/drivers/media/video/davinci/vpfe_capture.c b/drivers/media/video/davinci/vpfe_capture.c
index 7333a9b..bfc2a47 100644
--- a/drivers/media/video/davinci/vpfe_capture.c
+++ b/drivers/media/video/davinci/vpfe_capture.c
@@ -1987,7 +1987,7 @@ static __init int vpfe_probe(struct platform_device *pdev)
v4l2_i2c_new_subdev_board(&vpfe_dev->v4l2_dev,
i2c_adap,
&sdinfo->board_info,
- NULL);
+ NULL, 0);
if (vpfe_dev->sd[i]) {
v4l2_info(&vpfe_dev->v4l2_dev,
"v4l2 sub device %s registered\n",
diff --git a/drivers/media/video/davinci/vpif_capture.c b/drivers/media/video/davinci/vpif_capture.c
index 193abab..d2228e0 100644
--- a/drivers/media/video/davinci/vpif_capture.c
+++ b/drivers/media/video/davinci/vpif_capture.c
@@ -2014,7 +2014,7 @@ static __init int vpif_probe(struct platform_device *pdev)
v4l2_i2c_new_subdev_board(&vpif_obj.v4l2_dev,
i2c_adap,
&subdevdata->board_info,
- NULL);
+ NULL, 0);
if (!vpif_obj.sd[i]) {
vpif_err("Error registering v4l2 subdevice\n");
diff --git a/drivers/media/video/davinci/vpif_display.c b/drivers/media/video/davinci/vpif_display.c
index 412c65d..060c049 100644
--- a/drivers/media/video/davinci/vpif_display.c
+++ b/drivers/media/video/davinci/vpif_display.c
@@ -1555,7 +1555,7 @@ static __init int vpif_probe(struct platform_device *pdev)
vpif_obj.sd[i] = v4l2_i2c_new_subdev_board(&vpif_obj.v4l2_dev,
i2c_adap,
&subdevdata[i].board_info,
- NULL);
+ NULL, 0);
if (!vpif_obj.sd[i]) {
vpif_err("Error registering v4l2 subdevice\n");
goto probe_subdev_out;
diff --git a/drivers/media/video/ivtv/ivtv-i2c.c b/drivers/media/video/ivtv/ivtv-i2c.c
index 6651a6c..3d3b62d 100644
--- a/drivers/media/video/ivtv/ivtv-i2c.c
+++ b/drivers/media/video/ivtv/ivtv-i2c.c
@@ -277,7 +277,7 @@ int ivtv_i2c_register(struct ivtv *itv, unsigned idx)
info.platform_data = &pdata;
sd = v4l2_i2c_new_subdev_board(&itv->v4l2_dev, adap, &info,
- NULL);
+ NULL, 0);
} else {
sd = v4l2_i2c_new_subdev(&itv->v4l2_dev,
adap, type, hw_addrs[idx], NULL);
diff --git a/drivers/media/video/s5p-fimc/fimc-capture.c b/drivers/media/video/s5p-fimc/fimc-capture.c
index 2f50080..b237daa 100644
--- a/drivers/media/video/s5p-fimc/fimc-capture.c
+++ b/drivers/media/video/s5p-fimc/fimc-capture.c
@@ -44,7 +44,7 @@ static struct v4l2_subdev *fimc_subdev_register(struct fimc_dev *fimc,
return ERR_PTR(-ENOMEM);
sd = v4l2_i2c_new_subdev_board(&vid_cap->v4l2_dev, i2c_adap,
- isp_info->board_info, NULL);
+ isp_info->board_info, NULL, 0);
if (!sd) {
v4l2_err(&vid_cap->v4l2_dev, "failed to acquire subdev\n");
return NULL;
diff --git a/drivers/media/video/sh_vou.c b/drivers/media/video/sh_vou.c
index 07cf0c6..c50f0f5 100644
--- a/drivers/media/video/sh_vou.c
+++ b/drivers/media/video/sh_vou.c
@@ -1409,7 +1409,7 @@ static int __devinit sh_vou_probe(struct platform_device *pdev)
goto ereset;
subdev = v4l2_i2c_new_subdev_board(&vou_dev->v4l2_dev, i2c_adap,
- vou_pdata->board_info, NULL);
+ vou_pdata->board_info, NULL, 0);
if (!subdev) {
ret = -ENOMEM;
goto ei2cnd;
diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index 052bd6d..5afb601 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -896,7 +896,7 @@ static int soc_camera_init_i2c(struct soc_camera_device *icd,
icl->board_info->platform_data = icd;
subdev = v4l2_i2c_new_subdev_board(&ici->v4l2_dev, adap,
- icl->board_info, NULL);
+ icl->board_info, NULL, 0);
if (!subdev)
goto ei2cnd;
diff --git a/drivers/media/video/v4l2-common.c b/drivers/media/video/v4l2-common.c
index e007e61..ffee794 100644
--- a/drivers/media/video/v4l2-common.c
+++ b/drivers/media/video/v4l2-common.c
@@ -369,7 +369,7 @@ EXPORT_SYMBOL_GPL(v4l2_i2c_subdev_init);
/* Load an i2c sub-device. */
struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
struct i2c_adapter *adapter, struct i2c_board_info *info,
- const unsigned short *probe_addrs)
+ const unsigned short *probe_addrs, int enable_devnode)
{
struct v4l2_subdev *sd = NULL;
struct i2c_client *client;
@@ -399,9 +399,12 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
if (!try_module_get(client->driver->driver.owner))
goto error;
sd = i2c_get_clientdata(client);
+ if (!enable_devnode)
+ sd->flags &= ~V4L2_SUBDEV_FL_HAS_DEVNODE;
/* Register with the v4l2_device which increases the module's
use count as well. */
+ sd->initialized = 0;
if (v4l2_device_register_subdev(v4l2_dev, sd))
sd = NULL;
/* Decrease the module use count to match the first try_module_get. */
@@ -416,6 +419,8 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
if (err && err != -ENOIOCTLCMD) {
v4l2_device_unregister_subdev(sd);
sd = NULL;
+ } else {
+ sd->initialized = 1;
}
}
@@ -440,7 +445,8 @@ struct v4l2_subdev *v4l2_i2c_new_subdev(struct v4l2_device *v4l2_dev,
strlcpy(info.type, client_type, sizeof(info.type));
info.addr = addr;
- return v4l2_i2c_new_subdev_board(v4l2_dev, adapter, &info, probe_addrs);
+ return v4l2_i2c_new_subdev_board(v4l2_dev, adapter, &info, probe_addrs,
+ 0);
}
EXPORT_SYMBOL_GPL(v4l2_i2c_new_subdev);
@@ -510,7 +516,8 @@ void v4l2_spi_subdev_init(struct v4l2_subdev *sd, struct spi_device *spi,
EXPORT_SYMBOL_GPL(v4l2_spi_subdev_init);
struct v4l2_subdev *v4l2_spi_new_subdev(struct v4l2_device *v4l2_dev,
- struct spi_master *master, struct spi_board_info *info)
+ struct spi_master *master, struct spi_board_info *info,
+ int enable_devnode)
{
struct v4l2_subdev *sd = NULL;
struct spi_device *spi = NULL;
@@ -529,6 +536,8 @@ struct v4l2_subdev *v4l2_spi_new_subdev(struct v4l2_device *v4l2_dev,
goto error;
sd = spi_get_drvdata(spi);
+ if (!enable_devnode)
+ sd->flags &= ~V4L2_SUBDEV_FL_HAS_DEVNODE;
/* Register with the v4l2_device which increases the module's
use count as well. */
diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
index 359e232..f22bd41 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -408,13 +408,14 @@ static int get_index(struct video_device *vdev)
}
/**
- * video_register_device - register video4linux devices
+ * __video_register_device - register video4linux devices
* @vdev: video device structure we want to register
* @type: type of device to register
* @nr: which device node number (0 == /dev/video0, 1 == /dev/video1, ...
* -1 == first free)
* @warn_if_nr_in_use: warn if the desired device node number
* was already in use and another number was chosen instead.
+ * @owner: module that owns the video device node
*
* The registration code assigns minor numbers and device node numbers
* based on the requested type and registers the new device node with
@@ -431,9 +432,11 @@ static int get_index(struct video_device *vdev)
* %VFL_TYPE_VBI - Vertical blank data (undecoded)
*
* %VFL_TYPE_RADIO - A radio card
+ *
+ * %VFL_TYPE_SUBDEV - A subdevice
*/
-static int __video_register_device(struct video_device *vdev, int type, int nr,
- int warn_if_nr_in_use)
+int __video_register_device(struct video_device *vdev, int type, int nr,
+ int warn_if_nr_in_use, struct module *owner)
{
int i = 0;
int ret;
@@ -466,6 +469,9 @@ static int __video_register_device(struct video_device *vdev, int type, int nr,
case VFL_TYPE_RADIO:
name_base = "radio";
break;
+ case VFL_TYPE_SUBDEV:
+ name_base = "v4l-subdev";
+ break;
default:
printk(KERN_ERR "%s called with unknown type: %d\n",
__func__, type);
@@ -549,7 +555,7 @@ static int __video_register_device(struct video_device *vdev, int type, int nr,
goto cleanup;
}
vdev->cdev->ops = &v4l2_fops;
- vdev->cdev->owner = vdev->fops->owner;
+ vdev->cdev->owner = owner;
ret = cdev_add(vdev->cdev, MKDEV(VIDEO_MAJOR, vdev->minor), 1);
if (ret < 0) {
printk(KERN_ERR "%s: cdev_add failed\n", __func__);
@@ -598,18 +604,7 @@ cleanup:
vdev->minor = -1;
return ret;
}
-
-int video_register_device(struct video_device *vdev, int type, int nr)
-{
- return __video_register_device(vdev, type, nr, 1);
-}
-EXPORT_SYMBOL(video_register_device);
-
-int video_register_device_no_warn(struct video_device *vdev, int type, int nr)
-{
- return __video_register_device(vdev, type, nr, 0);
-}
-EXPORT_SYMBOL(video_register_device_no_warn);
+EXPORT_SYMBOL(__video_register_device);
/**
* video_unregister_device - unregister a video4linux device
diff --git a/drivers/media/video/v4l2-device.c b/drivers/media/video/v4l2-device.c
index 7fe6f92..97e84df 100644
--- a/drivers/media/video/v4l2-device.c
+++ b/drivers/media/video/v4l2-device.c
@@ -117,24 +117,43 @@ EXPORT_SYMBOL_GPL(v4l2_device_unregister);
int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,
struct v4l2_subdev *sd)
{
+ struct video_device *vdev;
int err;
/* Check for valid input */
if (v4l2_dev == NULL || sd == NULL || !sd->name[0])
return -EINVAL;
+
/* Warn if we apparently re-register a subdev */
WARN_ON(sd->v4l2_dev != NULL);
+
if (!try_module_get(sd->owner))
return -ENODEV;
+
/* This just returns 0 if either of the two args is NULL */
err = v4l2_ctrl_add_handler(v4l2_dev->ctrl_handler, sd->ctrl_handler);
if (err)
return err;
+
sd->v4l2_dev = v4l2_dev;
spin_lock(&v4l2_dev->lock);
list_add_tail(&sd->list, &v4l2_dev->subdevs);
spin_unlock(&v4l2_dev->lock);
- return 0;
+
+ /* Register the device node. */
+ vdev = &sd->devnode;
+ strlcpy(vdev->name, sd->name, sizeof(vdev->name));
+ vdev->parent = v4l2_dev->dev;
+ vdev->fops = &v4l2_subdev_fops;
+ vdev->release = video_device_release_empty;
+ if (sd->flags & V4L2_SUBDEV_FL_HAS_DEVNODE) {
+ err = __video_register_device(vdev, VFL_TYPE_SUBDEV, -1, 1,
+ sd->owner);
+ if (err < 0)
+ v4l2_device_unregister_subdev(sd);
+ }
+
+ return err;
}
EXPORT_SYMBOL_GPL(v4l2_device_register_subdev);
@@ -143,10 +162,13 @@ void v4l2_device_unregister_subdev(struct v4l2_subdev *sd)
/* return if it isn't registered */
if (sd == NULL || sd->v4l2_dev == NULL)
return;
+
spin_lock(&sd->v4l2_dev->lock);
list_del(&sd->list);
spin_unlock(&sd->v4l2_dev->lock);
sd->v4l2_dev = NULL;
+
module_put(sd->owner);
+ video_unregister_device(&sd->devnode);
}
EXPORT_SYMBOL_GPL(v4l2_device_unregister_subdev);
diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
index 1e01554..4137e4c 100644
--- a/drivers/media/video/v4l2-ioctl.c
+++ b/drivers/media/video/v4l2-ioctl.c
@@ -413,7 +413,7 @@ static unsigned long cmd_input_size(unsigned int cmd)
}
}
-static long
+long
__video_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
v4l2_kioctl func)
{
diff --git a/drivers/media/video/v4l2-subdev.c b/drivers/media/video/v4l2-subdev.c
new file mode 100644
index 0000000..00bd4b1
--- /dev/null
+++ b/drivers/media/video/v4l2-subdev.c
@@ -0,0 +1,66 @@
+/*
+ * V4L2 subdevice support.
+ *
+ * Copyright (C) 2010 Nokia Corporation
+ *
+ * Contact: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include <linux/types.h>
+#include <linux/ioctl.h>
+#include <linux/videodev2.h>
+
+#include <media/v4l2-device.h>
+#include <media/v4l2-ioctl.h>
+
+static int subdev_open(struct file *file)
+{
+ struct video_device *vdev = video_devdata(file);
+ struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
+
+ if (!sd->initialized)
+ return -EAGAIN;
+
+ return 0;
+}
+
+static int subdev_close(struct file *file)
+{
+ return 0;
+}
+
+static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
+{
+ switch (cmd) {
+ default:
+ return -ENOIOCTLCMD;
+ }
+
+ return 0;
+}
+
+static long subdev_ioctl(struct file *file, unsigned int cmd,
+ unsigned long arg)
+{
+ return __video_usercopy(file, cmd, arg, subdev_do_ioctl);
+}
+
+const struct v4l2_file_operations v4l2_subdev_fops = {
+ .owner = THIS_MODULE,
+ .open = subdev_open,
+ .unlocked_ioctl = subdev_ioctl,
+ .release = subdev_close,
+};
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index 565fb32..ef8965d 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -146,7 +146,7 @@ struct i2c_board_info;
struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
struct i2c_adapter *adapter, struct i2c_board_info *info,
- const unsigned short *probe_addrs);
+ const unsigned short *probe_addrs, int enable_devnode);
/* Initialize an v4l2_subdev with data from an i2c_client struct */
void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct i2c_client *client,
@@ -179,7 +179,8 @@ struct spi_device;
/* Load an spi module and return an initialized v4l2_subdev struct.
The client_type argument is the name of the chip that's on the adapter. */
struct v4l2_subdev *v4l2_spi_new_subdev(struct v4l2_device *v4l2_dev,
- struct spi_master *master, struct spi_board_info *info);
+ struct spi_master *master, struct spi_board_info *info,
+ int enable_devnode);
/* Initialize an v4l2_subdev with data from an spi_device struct */
void v4l2_spi_subdev_init(struct v4l2_subdev *sd, struct spi_device *spi,
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index 15802a0..4fe6831 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -21,7 +21,8 @@
#define VFL_TYPE_GRABBER 0
#define VFL_TYPE_VBI 1
#define VFL_TYPE_RADIO 2
-#define VFL_TYPE_MAX 3
+#define VFL_TYPE_SUBDEV 3
+#define VFL_TYPE_MAX 4
struct v4l2_ioctl_callbacks;
struct video_device;
@@ -102,15 +103,26 @@ struct video_device
/* dev to video-device */
#define to_video_device(cd) container_of(cd, struct video_device, dev)
+int __must_check __video_register_device(struct video_device *vdev, int type,
+ int nr, int warn_if_nr_in_use, struct module *owner);
+
/* Register video devices. Note that if video_register_device fails,
the release() callback of the video_device structure is *not* called, so
the caller is responsible for freeing any data. Usually that means that
you call video_device_release() on failure. */
-int __must_check video_register_device(struct video_device *vdev, int type, int nr);
+static inline int __must_check video_register_device(struct video_device *vdev,
+ int type, int nr)
+{
+ return __video_register_device(vdev, type, nr, 1, vdev->fops->owner);
+}
/* Same as video_register_device, but no warning is issued if the desired
device node number was already in use. */
-int __must_check video_register_device_no_warn(struct video_device *vdev, int type, int nr);
+static inline int __must_check video_register_device_no_warn(
+ struct video_device *vdev, int type, int nr)
+{
+ return __video_register_device(vdev, type, nr, 0, vdev->fops->owner);
+}
/* Unregister video devices. Will do nothing if vdev == NULL or
video_is_registered() returns false. */
diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
index 06daa6e..abb64d0 100644
--- a/include/media/v4l2-ioctl.h
+++ b/include/media/v4l2-ioctl.h
@@ -316,6 +316,9 @@ extern long v4l2_compat_ioctl32(struct file *file, unsigned int cmd,
unsigned long arg);
#endif
+extern long __video_usercopy(struct file *file, unsigned int cmd,
+ unsigned long arg, v4l2_kioctl func);
+
/* Include support for obsoleted stuff */
extern long video_usercopy(struct file *file, unsigned int cmd,
unsigned long arg, v4l2_kioctl func);
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index b636444..de181db 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -22,6 +22,7 @@
#define _V4L2_SUBDEV_H
#include <media/v4l2-common.h>
+#include <media/v4l2-dev.h>
#include <media/v4l2-mediabus.h>
/* generic v4l2_device notify callback notification values */
@@ -418,9 +419,11 @@ struct v4l2_subdev_ops {
#define V4L2_SUBDEV_NAME_SIZE 32
/* Set this flag if this subdev is a i2c device. */
-#define V4L2_SUBDEV_FL_IS_I2C (1U << 0)
+#define V4L2_SUBDEV_FL_IS_I2C (1U << 0)
/* Set this flag if this subdev is a spi device. */
-#define V4L2_SUBDEV_FL_IS_SPI (1U << 1)
+#define V4L2_SUBDEV_FL_IS_SPI (1U << 1)
+/* Set this flag if this subdev needs a device node. */
+#define V4L2_SUBDEV_FL_HAS_DEVNODE (1U << 2)
/* Each instance of a subdev driver should create this struct, either
stand-alone or embedded in a larger struct.
@@ -440,8 +443,16 @@ struct v4l2_subdev {
/* pointer to private data */
void *dev_priv;
void *host_priv;
+ /* subdev device node */
+ struct video_device devnode;
+ unsigned int initialized;
};
+#define vdev_to_v4l2_subdev(vdev) \
+ container_of(vdev, struct v4l2_subdev, devnode)
+
+extern const struct v4l2_file_operations v4l2_subdev_fops;
+
static inline void v4l2_set_subdevdata(struct v4l2_subdev *sd, void *p)
{
sd->dev_priv = p;
@@ -474,6 +485,7 @@ static inline void v4l2_subdev_init(struct v4l2_subdev *sd,
sd->grp_id = 0;
sd->dev_priv = NULL;
sd->host_priv = NULL;
+ sd->initialized = 1;
}
/* Call an ops of a v4l2_subdev, doing the right checks against
--
1.7.3.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v6 4/7] v4l: subdev: Add device node support
2011-01-27 12:28 ` [PATCH v6 4/7] v4l: subdev: Add device node support Laurent Pinchart
@ 2011-02-04 10:09 ` Hans Verkuil
2011-02-09 17:37 ` Laurent Pinchart
0 siblings, 1 reply; 13+ messages in thread
From: Hans Verkuil @ 2011-02-04 10:09 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, sakari.ailus
On Thursday, January 27, 2011 13:28:55 Laurent Pinchart wrote:
> Create a device node named subdevX for every registered subdev.
>
> As the device node is registered before the subdev core::s_config
> function is called, return -EGAIN on open until initialization
> completes.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Vimarsh Zutshi <vimarsh.zutshi@nokia.com>
> ---
> Documentation/video4linux/v4l2-framework.txt | 18 +++++++
> drivers/media/radio/radio-si4713.c | 2 +-
> drivers/media/video/Makefile | 2 +-
> drivers/media/video/cafe_ccic.c | 2 +-
> drivers/media/video/davinci/vpfe_capture.c | 2 +-
> drivers/media/video/davinci/vpif_capture.c | 2 +-
> drivers/media/video/davinci/vpif_display.c | 2 +-
> drivers/media/video/ivtv/ivtv-i2c.c | 2 +-
> drivers/media/video/s5p-fimc/fimc-capture.c | 2 +-
> drivers/media/video/sh_vou.c | 2 +-
> drivers/media/video/soc_camera.c | 2 +-
> drivers/media/video/v4l2-common.c | 15 +++++-
> drivers/media/video/v4l2-dev.c | 27 ++++------
> drivers/media/video/v4l2-device.c | 24 +++++++++-
> drivers/media/video/v4l2-ioctl.c | 2 +-
> drivers/media/video/v4l2-subdev.c | 66 ++++++++++++++++++++++++++
> include/media/v4l2-common.h | 5 +-
> include/media/v4l2-dev.h | 18 ++++++-
> include/media/v4l2-ioctl.h | 3 +
> include/media/v4l2-subdev.h | 16 ++++++-
> 20 files changed, 176 insertions(+), 38 deletions(-)
> create mode 100644 drivers/media/video/v4l2-subdev.c
>
<snip>
> diff --git a/drivers/media/video/v4l2-device.c b/drivers/media/video/v4l2-device.c
> index 7fe6f92..97e84df 100644
> --- a/drivers/media/video/v4l2-device.c
> +++ b/drivers/media/video/v4l2-device.c
> @@ -117,24 +117,43 @@ EXPORT_SYMBOL_GPL(v4l2_device_unregister);
> int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,
> struct v4l2_subdev *sd)
> {
> + struct video_device *vdev;
> int err;
>
> /* Check for valid input */
> if (v4l2_dev == NULL || sd == NULL || !sd->name[0])
> return -EINVAL;
> +
> /* Warn if we apparently re-register a subdev */
> WARN_ON(sd->v4l2_dev != NULL);
> +
> if (!try_module_get(sd->owner))
> return -ENODEV;
> +
> /* This just returns 0 if either of the two args is NULL */
> err = v4l2_ctrl_add_handler(v4l2_dev->ctrl_handler, sd->ctrl_handler);
> if (err)
> return err;
> +
> sd->v4l2_dev = v4l2_dev;
> spin_lock(&v4l2_dev->lock);
> list_add_tail(&sd->list, &v4l2_dev->subdevs);
> spin_unlock(&v4l2_dev->lock);
> - return 0;
> +
> + /* Register the device node. */
> + vdev = &sd->devnode;
> + strlcpy(vdev->name, sd->name, sizeof(vdev->name));
> + vdev->parent = v4l2_dev->dev;
> + vdev->fops = &v4l2_subdev_fops;
> + vdev->release = video_device_release_empty;
> + if (sd->flags & V4L2_SUBDEV_FL_HAS_DEVNODE) {
> + err = __video_register_device(vdev, VFL_TYPE_SUBDEV, -1, 1,
> + sd->owner);
> + if (err < 0)
> + v4l2_device_unregister_subdev(sd);
> + }
> +
> + return err;
> }
> EXPORT_SYMBOL_GPL(v4l2_device_register_subdev);
>
> @@ -143,10 +162,13 @@ void v4l2_device_unregister_subdev(struct v4l2_subdev *sd)
> /* return if it isn't registered */
> if (sd == NULL || sd->v4l2_dev == NULL)
> return;
> +
> spin_lock(&sd->v4l2_dev->lock);
> list_del(&sd->list);
> spin_unlock(&sd->v4l2_dev->lock);
> sd->v4l2_dev = NULL;
> +
> module_put(sd->owner);
> + video_unregister_device(&sd->devnode);
I'm pretty sure that the video_unregister_device should be called before the
module_put. And I think it is cleaner to test the V4L2_SUBDEV_FL_HAS_DEVNODE
flag as well. It may not be strictly necessary, but it is less confusing.
> }
> EXPORT_SYMBOL_GPL(v4l2_device_unregister_subdev);
> diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
> index 1e01554..4137e4c 100644
> --- a/drivers/media/video/v4l2-ioctl.c
> +++ b/drivers/media/video/v4l2-ioctl.c
> @@ -413,7 +413,7 @@ static unsigned long cmd_input_size(unsigned int cmd)
> }
> }
>
> -static long
> +long
> __video_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
> v4l2_kioctl func)
> {
> diff --git a/drivers/media/video/v4l2-subdev.c b/drivers/media/video/v4l2-subdev.c
> new file mode 100644
> index 0000000..00bd4b1
> --- /dev/null
> +++ b/drivers/media/video/v4l2-subdev.c
> @@ -0,0 +1,66 @@
> +/*
> + * V4L2 subdevice support.
> + *
> + * Copyright (C) 2010 Nokia Corporation
> + *
> + * Contact: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + */
> +
> +#include <linux/types.h>
> +#include <linux/ioctl.h>
> +#include <linux/videodev2.h>
> +
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-ioctl.h>
> +
> +static int subdev_open(struct file *file)
> +{
> + struct video_device *vdev = video_devdata(file);
> + struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
> +
> + if (!sd->initialized)
> + return -EAGAIN;
> +
> + return 0;
> +}
> +
> +static int subdev_close(struct file *file)
> +{
> + return 0;
> +}
> +
> +static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
> +{
> + switch (cmd) {
> + default:
> + return -ENOIOCTLCMD;
> + }
> +
> + return 0;
> +}
> +
> +static long subdev_ioctl(struct file *file, unsigned int cmd,
> + unsigned long arg)
> +{
> + return __video_usercopy(file, cmd, arg, subdev_do_ioctl);
> +}
> +
> +const struct v4l2_file_operations v4l2_subdev_fops = {
> + .owner = THIS_MODULE,
> + .open = subdev_open,
> + .unlocked_ioctl = subdev_ioctl,
> + .release = subdev_close,
> +};
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index 565fb32..ef8965d 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -146,7 +146,7 @@ struct i2c_board_info;
>
> struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
> struct i2c_adapter *adapter, struct i2c_board_info *info,
> - const unsigned short *probe_addrs);
> + const unsigned short *probe_addrs, int enable_devnode);
>
> /* Initialize an v4l2_subdev with data from an i2c_client struct */
> void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct i2c_client *client,
> @@ -179,7 +179,8 @@ struct spi_device;
> /* Load an spi module and return an initialized v4l2_subdev struct.
> The client_type argument is the name of the chip that's on the adapter. */
> struct v4l2_subdev *v4l2_spi_new_subdev(struct v4l2_device *v4l2_dev,
> - struct spi_master *master, struct spi_board_info *info);
> + struct spi_master *master, struct spi_board_info *info,
> + int enable_devnode);
>
> /* Initialize an v4l2_subdev with data from an spi_device struct */
> void v4l2_spi_subdev_init(struct v4l2_subdev *sd, struct spi_device *spi,
> diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
> index 15802a0..4fe6831 100644
> --- a/include/media/v4l2-dev.h
> +++ b/include/media/v4l2-dev.h
> @@ -21,7 +21,8 @@
> #define VFL_TYPE_GRABBER 0
> #define VFL_TYPE_VBI 1
> #define VFL_TYPE_RADIO 2
> -#define VFL_TYPE_MAX 3
> +#define VFL_TYPE_SUBDEV 3
> +#define VFL_TYPE_MAX 4
>
> struct v4l2_ioctl_callbacks;
> struct video_device;
> @@ -102,15 +103,26 @@ struct video_device
> /* dev to video-device */
> #define to_video_device(cd) container_of(cd, struct video_device, dev)
>
> +int __must_check __video_register_device(struct video_device *vdev, int type,
> + int nr, int warn_if_nr_in_use, struct module *owner);
> +
> /* Register video devices. Note that if video_register_device fails,
> the release() callback of the video_device structure is *not* called, so
> the caller is responsible for freeing any data. Usually that means that
> you call video_device_release() on failure. */
> -int __must_check video_register_device(struct video_device *vdev, int type, int nr);
> +static inline int __must_check video_register_device(struct video_device *vdev,
> + int type, int nr)
> +{
> + return __video_register_device(vdev, type, nr, 1, vdev->fops->owner);
> +}
>
> /* Same as video_register_device, but no warning is issued if the desired
> device node number was already in use. */
> -int __must_check video_register_device_no_warn(struct video_device *vdev, int type, int nr);
> +static inline int __must_check video_register_device_no_warn(
> + struct video_device *vdev, int type, int nr)
> +{
> + return __video_register_device(vdev, type, nr, 0, vdev->fops->owner);
> +}
>
> /* Unregister video devices. Will do nothing if vdev == NULL or
> video_is_registered() returns false. */
> diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
> index 06daa6e..abb64d0 100644
> --- a/include/media/v4l2-ioctl.h
> +++ b/include/media/v4l2-ioctl.h
> @@ -316,6 +316,9 @@ extern long v4l2_compat_ioctl32(struct file *file, unsigned int cmd,
> unsigned long arg);
> #endif
>
> +extern long __video_usercopy(struct file *file, unsigned int cmd,
> + unsigned long arg, v4l2_kioctl func);
> +
> /* Include support for obsoleted stuff */
> extern long video_usercopy(struct file *file, unsigned int cmd,
> unsigned long arg, v4l2_kioctl func);
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index b636444..de181db 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -22,6 +22,7 @@
> #define _V4L2_SUBDEV_H
>
> #include <media/v4l2-common.h>
> +#include <media/v4l2-dev.h>
> #include <media/v4l2-mediabus.h>
>
> /* generic v4l2_device notify callback notification values */
> @@ -418,9 +419,11 @@ struct v4l2_subdev_ops {
> #define V4L2_SUBDEV_NAME_SIZE 32
>
> /* Set this flag if this subdev is a i2c device. */
> -#define V4L2_SUBDEV_FL_IS_I2C (1U << 0)
> +#define V4L2_SUBDEV_FL_IS_I2C (1U << 0)
> /* Set this flag if this subdev is a spi device. */
> -#define V4L2_SUBDEV_FL_IS_SPI (1U << 1)
> +#define V4L2_SUBDEV_FL_IS_SPI (1U << 1)
> +/* Set this flag if this subdev needs a device node. */
> +#define V4L2_SUBDEV_FL_HAS_DEVNODE (1U << 2)
>
> /* Each instance of a subdev driver should create this struct, either
> stand-alone or embedded in a larger struct.
> @@ -440,8 +443,16 @@ struct v4l2_subdev {
> /* pointer to private data */
> void *dev_priv;
> void *host_priv;
> + /* subdev device node */
> + struct video_device devnode;
> + unsigned int initialized;
This can be a bool. Actually, thinking about this some more, is this really
necessary? The device node should be created as the very last thing anyway.
Looking at the patches it seems to be set only when creating an i2c subdev,
and not when creating a spi subdev (let alone the fact that this has to be
set manually for internal subdevs).
I think it is more hassle than anything else.
There may be situations where you don't want to create a device node when
calling v4l2_device_register_subdev(): should that happen, then it is
better to add the capability of registering a subdev without creating a
device node, and add a new function that just creates the device node at
a later (safe) stage.
The easiest way to do this is to add a new flag: V4L2_SUBDEV_FL_ALLOW_DEVNODE
The HAS_DEVNODE flag is set by the subdev driver, the ALLOW_DEVNODE flag is
set by the master driver. If both are set, then the device node is created.
So by splitting off the device node creation in a public function, the master
driver can control this nicely.
Thinking about this, we can actually implement it like this from the beginning.
I never really liked the fact that the master driver clears the HAS_DEVNODE
flag. Creating two separate flags is cleaner.
> };
>
> +#define vdev_to_v4l2_subdev(vdev) \
> + container_of(vdev, struct v4l2_subdev, devnode)
> +
> +extern const struct v4l2_file_operations v4l2_subdev_fops;
> +
> static inline void v4l2_set_subdevdata(struct v4l2_subdev *sd, void *p)
> {
> sd->dev_priv = p;
> @@ -474,6 +485,7 @@ static inline void v4l2_subdev_init(struct v4l2_subdev *sd,
> sd->grp_id = 0;
> sd->dev_priv = NULL;
> sd->host_priv = NULL;
> + sd->initialized = 1;
> }
>
> /* Call an ops of a v4l2_subdev, doing the right checks against
>
Regards,
Hans
--
Hans Verkuil - video4linux developer - sponsored by Cisco
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v6 4/7] v4l: subdev: Add device node support
2011-02-04 10:09 ` Hans Verkuil
@ 2011-02-09 17:37 ` Laurent Pinchart
0 siblings, 0 replies; 13+ messages in thread
From: Laurent Pinchart @ 2011-02-09 17:37 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, sakari.ailus
Hi Hans,
Thanks for the review.
On Friday 04 February 2011 11:09:03 Hans Verkuil wrote:
> On Thursday, January 27, 2011 13:28:55 Laurent Pinchart wrote:
[snip]
> > @@ -143,10 +162,13 @@ void v4l2_device_unregister_subdev(struct
> > v4l2_subdev *sd)
> >
> > /* return if it isn't registered */
> > if (sd == NULL || sd->v4l2_dev == NULL)
> > return;
> > +
> > spin_lock(&sd->v4l2_dev->lock);
> > list_del(&sd->list);
> > spin_unlock(&sd->v4l2_dev->lock);
> > sd->v4l2_dev = NULL;
> > +
> > module_put(sd->owner);
> > + video_unregister_device(&sd->devnode);
>
> I'm pretty sure that the video_unregister_device should be called before
> the module_put. And I think it is cleaner to test the
> V4L2_SUBDEV_FL_HAS_DEVNODE flag as well. It may not be strictly necessary,
> but it is less confusing.
OK.
[snip]
> > @@ -440,8 +443,16 @@ struct v4l2_subdev {
> >
> > /* pointer to private data */
> > void *dev_priv;
> > void *host_priv;
> >
> > + /* subdev device node */
> > + struct video_device devnode;
> > + unsigned int initialized;
>
> This can be a bool. Actually, thinking about this some more, is this really
> necessary? The device node should be created as the very last thing anyway.
> Looking at the patches it seems to be set only when creating an i2c subdev,
> and not when creating a spi subdev (let alone the fact that this has to be
> set manually for internal subdevs).
>
> I think it is more hassle than anything else.
>
> There may be situations where you don't want to create a device node when
> calling v4l2_device_register_subdev(): should that happen, then it is
> better to add the capability of registering a subdev without creating a
> device node, and add a new function that just creates the device node at
> a later (safe) stage.
>
> The easiest way to do this is to add a new flag:
> V4L2_SUBDEV_FL_ALLOW_DEVNODE
>
> The HAS_DEVNODE flag is set by the subdev driver, the ALLOW_DEVNODE flag is
> set by the master driver. If both are set, then the device node is created.
>
> So by splitting off the device node creation in a public function, the
> master driver can control this nicely.
>
> Thinking about this, we can actually implement it like this from the
> beginning. I never really liked the fact that the master driver clears the
> HAS_DEVNODE flag. Creating two separate flags is cleaner.
As discussed on IRC, I've removed automatic subdev device node registration as
it's prone to race conditions. Drivers will need to call
v4l2_device_register_subdev_nodes() explicitly. The initialized field, as well
as the enable_devnode arguments, are removed.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v6 5/7] v4l: subdev: Uninline the v4l2_subdev_init function
2011-01-27 12:28 [PATCH v6 0/7] V4L2 subdev userspace API Laurent Pinchart
` (3 preceding siblings ...)
2011-01-27 12:28 ` [PATCH v6 4/7] v4l: subdev: Add device node support Laurent Pinchart
@ 2011-01-27 12:28 ` Laurent Pinchart
2011-01-27 12:28 ` [PATCH v6 6/7] v4l: subdev: Control ioctls support Laurent Pinchart
2011-01-27 12:28 ` [PATCH v6 7/7] v4l: subdev: Events support Laurent Pinchart
6 siblings, 0 replies; 13+ messages in thread
From: Laurent Pinchart @ 2011-01-27 12:28 UTC (permalink / raw)
To: linux-media; +Cc: sakari.ailus
The function isn't small or performance sensitive enough to be inlined.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/video/v4l2-subdev.c | 42 +++++++++++++++++++++++++-----------
include/media/v4l2-subdev.h | 16 +------------
2 files changed, 31 insertions(+), 27 deletions(-)
diff --git a/drivers/media/video/v4l2-subdev.c b/drivers/media/video/v4l2-subdev.c
index 00bd4b1..0deff78 100644
--- a/drivers/media/video/v4l2-subdev.c
+++ b/drivers/media/video/v4l2-subdev.c
@@ -1,22 +1,23 @@
/*
- * V4L2 subdevice support.
+ * V4L2 sub-device
*
- * Copyright (C) 2010 Nokia Corporation
+ * Copyright (C) 2010 Nokia Corporation
*
- * Contact: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ * Contact: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ * Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
*
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation.
+ * 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.
*
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
*
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
#include <linux/types.h>
@@ -64,3 +65,18 @@ const struct v4l2_file_operations v4l2_subdev_fops = {
.unlocked_ioctl = subdev_ioctl,
.release = subdev_close,
};
+
+void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops)
+{
+ INIT_LIST_HEAD(&sd->list);
+ BUG_ON(!ops);
+ sd->ops = ops;
+ sd->v4l2_dev = NULL;
+ sd->flags = 0;
+ sd->name[0] = '\0';
+ sd->grp_id = 0;
+ sd->dev_priv = NULL;
+ sd->host_priv = NULL;
+ sd->initialized = 1;
+}
+EXPORT_SYMBOL(v4l2_subdev_init);
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index de181db..90022f5 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -473,20 +473,8 @@ static inline void *v4l2_get_subdev_hostdata(const struct v4l2_subdev *sd)
return sd->host_priv;
}
-static inline void v4l2_subdev_init(struct v4l2_subdev *sd,
- const struct v4l2_subdev_ops *ops)
-{
- INIT_LIST_HEAD(&sd->list);
- BUG_ON(!ops);
- sd->ops = ops;
- sd->v4l2_dev = NULL;
- sd->flags = 0;
- sd->name[0] = '\0';
- sd->grp_id = 0;
- sd->dev_priv = NULL;
- sd->host_priv = NULL;
- sd->initialized = 1;
-}
+void v4l2_subdev_init(struct v4l2_subdev *sd,
+ const struct v4l2_subdev_ops *ops);
/* Call an ops of a v4l2_subdev, doing the right checks against
NULL pointers.
--
1.7.3.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v6 6/7] v4l: subdev: Control ioctls support
2011-01-27 12:28 [PATCH v6 0/7] V4L2 subdev userspace API Laurent Pinchart
` (4 preceding siblings ...)
2011-01-27 12:28 ` [PATCH v6 5/7] v4l: subdev: Uninline the v4l2_subdev_init function Laurent Pinchart
@ 2011-01-27 12:28 ` Laurent Pinchart
2011-01-27 12:28 ` [PATCH v6 7/7] v4l: subdev: Events support Laurent Pinchart
6 siblings, 0 replies; 13+ messages in thread
From: Laurent Pinchart @ 2011-01-27 12:28 UTC (permalink / raw)
To: linux-media; +Cc: sakari.ailus
Pass the control-related ioctls to the subdev driver through the control
framework.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Documentation/video4linux/v4l2-framework.txt | 16 ++++++++++++++++
drivers/media/video/v4l2-subdev.c | 25 +++++++++++++++++++++++++
2 files changed, 41 insertions(+), 0 deletions(-)
diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt
index 4c9185a..f683f63 100644
--- a/Documentation/video4linux/v4l2-framework.txt
+++ b/Documentation/video4linux/v4l2-framework.txt
@@ -336,6 +336,22 @@ argument to 0. Setting the argument to 1 will only enable device node
registration if the sub-device driver has set the V4L2_SUBDEV_FL_HAS_DEVNODE
flag.
+The device node handles a subset of the V4L2 API.
+
+VIDIOC_QUERYCTRL
+VIDIOC_QUERYMENU
+VIDIOC_G_CTRL
+VIDIOC_S_CTRL
+VIDIOC_G_EXT_CTRLS
+VIDIOC_S_EXT_CTRLS
+VIDIOC_TRY_EXT_CTRLS
+
+ The controls ioctls are identical to the ones defined in V4L2. They
+ behave identically, with the only exception that they deal only with
+ controls implemented in the sub-device. Depending on the driver, those
+ controls can be also be accessed through one (or several) V4L2 device
+ nodes.
+
I2C sub-device drivers
----------------------
diff --git a/drivers/media/video/v4l2-subdev.c b/drivers/media/video/v4l2-subdev.c
index 0deff78..fc57ce7 100644
--- a/drivers/media/video/v4l2-subdev.c
+++ b/drivers/media/video/v4l2-subdev.c
@@ -24,6 +24,7 @@
#include <linux/ioctl.h>
#include <linux/videodev2.h>
+#include <media/v4l2-ctrls.h>
#include <media/v4l2-device.h>
#include <media/v4l2-ioctl.h>
@@ -45,7 +46,31 @@ static int subdev_close(struct file *file)
static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
{
+ struct video_device *vdev = video_devdata(file);
+ struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
+
switch (cmd) {
+ case VIDIOC_QUERYCTRL:
+ return v4l2_subdev_queryctrl(sd, arg);
+
+ case VIDIOC_QUERYMENU:
+ return v4l2_subdev_querymenu(sd, arg);
+
+ case VIDIOC_G_CTRL:
+ return v4l2_subdev_g_ctrl(sd, arg);
+
+ case VIDIOC_S_CTRL:
+ return v4l2_subdev_s_ctrl(sd, arg);
+
+ case VIDIOC_G_EXT_CTRLS:
+ return v4l2_subdev_g_ext_ctrls(sd, arg);
+
+ case VIDIOC_S_EXT_CTRLS:
+ return v4l2_subdev_s_ext_ctrls(sd, arg);
+
+ case VIDIOC_TRY_EXT_CTRLS:
+ return v4l2_subdev_try_ext_ctrls(sd, arg);
+
default:
return -ENOIOCTLCMD;
}
--
1.7.3.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v6 7/7] v4l: subdev: Events support
2011-01-27 12:28 [PATCH v6 0/7] V4L2 subdev userspace API Laurent Pinchart
` (5 preceding siblings ...)
2011-01-27 12:28 ` [PATCH v6 6/7] v4l: subdev: Control ioctls support Laurent Pinchart
@ 2011-01-27 12:28 ` Laurent Pinchart
2011-02-04 10:12 ` Hans Verkuil
6 siblings, 1 reply; 13+ messages in thread
From: Laurent Pinchart @ 2011-01-27 12:28 UTC (permalink / raw)
To: linux-media; +Cc: sakari.ailus
From: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
Provide v4l2_subdevs with v4l2_event support. Subdev drivers only need very
little to support events.
Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
Signed-off-by: David Cohen <david.cohen@nokia.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Documentation/video4linux/v4l2-framework.txt | 18 ++++++
drivers/media/video/v4l2-subdev.c | 75 +++++++++++++++++++++++++-
include/media/v4l2-subdev.h | 10 ++++
3 files changed, 102 insertions(+), 1 deletions(-)
diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt
index f683f63..4db1def 100644
--- a/Documentation/video4linux/v4l2-framework.txt
+++ b/Documentation/video4linux/v4l2-framework.txt
@@ -352,6 +352,24 @@ VIDIOC_TRY_EXT_CTRLS
controls can be also be accessed through one (or several) V4L2 device
nodes.
+VIDIOC_DQEVENT
+VIDIOC_SUBSCRIBE_EVENT
+VIDIOC_UNSUBSCRIBE_EVENT
+
+ The events ioctls are identical to the ones defined in V4L2. They
+ behave identically, with the only exception that they deal only with
+ events generated by the sub-device. Depending on the driver, those
+ events can also be reported by one (or several) V4L2 device nodes.
+
+ Sub-device drivers that want to use events need to set the
+ V4L2_SUBDEV_USES_EVENTS v4l2_subdev::flags and initialize
+ v4l2_subdev::nevents to events queue depth before registering the
+ sub-device. After registration events can be queued as usual on the
+ v4l2_subdev::devnode device node.
+
+ To properly support events, the poll() file operation is also
+ implemented.
+
I2C sub-device drivers
----------------------
diff --git a/drivers/media/video/v4l2-subdev.c b/drivers/media/video/v4l2-subdev.c
index fc57ce7..fbccefd 100644
--- a/drivers/media/video/v4l2-subdev.c
+++ b/drivers/media/video/v4l2-subdev.c
@@ -20,27 +20,69 @@
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
-#include <linux/types.h>
#include <linux/ioctl.h>
+#include <linux/slab.h>
+#include <linux/types.h>
#include <linux/videodev2.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-device.h>
#include <media/v4l2-ioctl.h>
+#include <media/v4l2-fh.h>
+#include <media/v4l2-event.h>
static int subdev_open(struct file *file)
{
struct video_device *vdev = video_devdata(file);
struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
+ struct v4l2_fh *vfh;
+ int ret;
if (!sd->initialized)
return -EAGAIN;
+ if (sd->flags & V4L2_SUBDEV_FL_HAS_EVENTS) {
+ vfh = kzalloc(sizeof(*vfh), GFP_KERNEL);
+ if (vfh == NULL)
+ return -ENOMEM;
+
+ ret = v4l2_fh_init(vfh, vdev);
+ if (ret)
+ goto err;
+
+ ret = v4l2_event_init(vfh);
+ if (ret)
+ goto err;
+
+ ret = v4l2_event_alloc(vfh, sd->nevents);
+ if (ret)
+ goto err;
+
+ v4l2_fh_add(vfh);
+ file->private_data = vfh;
+ }
+
return 0;
+
+err:
+ if (vfh != NULL) {
+ v4l2_fh_exit(vfh);
+ kfree(vfh);
+ }
+
+ return ret;
}
static int subdev_close(struct file *file)
{
+ struct v4l2_fh *vfh = file->private_data;
+
+ if (vfh != NULL) {
+ v4l2_fh_del(vfh);
+ v4l2_fh_exit(vfh);
+ kfree(vfh);
+ }
+
return 0;
}
@@ -48,6 +90,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
{
struct video_device *vdev = video_devdata(file);
struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
+ struct v4l2_fh *fh = file->private_data;
switch (cmd) {
case VIDIOC_QUERYCTRL:
@@ -71,6 +114,18 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
case VIDIOC_TRY_EXT_CTRLS:
return v4l2_subdev_try_ext_ctrls(sd, arg);
+ case VIDIOC_DQEVENT:
+ if (!(sd->flags & V4L2_SUBDEV_FL_HAS_EVENTS))
+ return -ENOIOCTLCMD;
+
+ return v4l2_event_dequeue(fh, arg, file->f_flags & O_NONBLOCK);
+
+ case VIDIOC_SUBSCRIBE_EVENT:
+ return v4l2_subdev_call(sd, core, subscribe_event, fh, arg);
+
+ case VIDIOC_UNSUBSCRIBE_EVENT:
+ return v4l2_subdev_call(sd, core, unsubscribe_event, fh, arg);
+
default:
return -ENOIOCTLCMD;
}
@@ -84,11 +139,29 @@ static long subdev_ioctl(struct file *file, unsigned int cmd,
return __video_usercopy(file, cmd, arg, subdev_do_ioctl);
}
+static unsigned int subdev_poll(struct file *file, poll_table *wait)
+{
+ struct video_device *vdev = video_devdata(file);
+ struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
+ struct v4l2_fh *fh = file->private_data;
+
+ if (!(sd->flags & V4L2_SUBDEV_FL_HAS_EVENTS))
+ return POLLERR;
+
+ poll_wait(file, &fh->events->wait, wait);
+
+ if (v4l2_event_pending(fh))
+ return POLLPRI;
+
+ return 0;
+}
+
const struct v4l2_file_operations v4l2_subdev_fops = {
.owner = THIS_MODULE,
.open = subdev_open,
.unlocked_ioctl = subdev_ioctl,
.release = subdev_close,
+ .poll = subdev_poll,
};
void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops)
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 90022f5..68cbe48 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -37,6 +37,8 @@
struct v4l2_device;
struct v4l2_ctrl_handler;
+struct v4l2_event_subscription;
+struct v4l2_fh;
struct v4l2_subdev;
struct tuner_setup;
@@ -165,6 +167,10 @@ struct v4l2_subdev_core_ops {
int (*s_power)(struct v4l2_subdev *sd, int on);
int (*interrupt_service_routine)(struct v4l2_subdev *sd,
u32 status, bool *handled);
+ int (*subscribe_event)(struct v4l2_subdev *sd, struct v4l2_fh *fh,
+ struct v4l2_event_subscription *sub);
+ int (*unsubscribe_event)(struct v4l2_subdev *sd, struct v4l2_fh *fh,
+ struct v4l2_event_subscription *sub);
};
/* s_mode: switch the tuner to a specific tuner mode. Replacement of s_radio.
@@ -424,6 +430,8 @@ struct v4l2_subdev_ops {
#define V4L2_SUBDEV_FL_IS_SPI (1U << 1)
/* Set this flag if this subdev needs a device node. */
#define V4L2_SUBDEV_FL_HAS_DEVNODE (1U << 2)
+/* Set this flag if this subdev generates events. */
+#define V4L2_SUBDEV_FL_HAS_EVENTS (1U << 3)
/* Each instance of a subdev driver should create this struct, either
stand-alone or embedded in a larger struct.
@@ -446,6 +454,8 @@ struct v4l2_subdev {
/* subdev device node */
struct video_device devnode;
unsigned int initialized;
+ /* number of events to be allocated on open */
+ unsigned int nevents;
};
#define vdev_to_v4l2_subdev(vdev) \
--
1.7.3.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v6 7/7] v4l: subdev: Events support
2011-01-27 12:28 ` [PATCH v6 7/7] v4l: subdev: Events support Laurent Pinchart
@ 2011-02-04 10:12 ` Hans Verkuil
2011-02-04 12:03 ` Sakari Ailus
0 siblings, 1 reply; 13+ messages in thread
From: Hans Verkuil @ 2011-02-04 10:12 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, sakari.ailus
On Thursday, January 27, 2011 13:28:58 Laurent Pinchart wrote:
> From: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
>
> Provide v4l2_subdevs with v4l2_event support. Subdev drivers only need very
> little to support events.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
> Signed-off-by: David Cohen <david.cohen@nokia.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Documentation/video4linux/v4l2-framework.txt | 18 ++++++
> drivers/media/video/v4l2-subdev.c | 75 +++++++++++++++++++++++++-
> include/media/v4l2-subdev.h | 10 ++++
> 3 files changed, 102 insertions(+), 1 deletions(-)
>
> diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt
> index f683f63..4db1def 100644
> --- a/Documentation/video4linux/v4l2-framework.txt
> +++ b/Documentation/video4linux/v4l2-framework.txt
> @@ -352,6 +352,24 @@ VIDIOC_TRY_EXT_CTRLS
> controls can be also be accessed through one (or several) V4L2 device
> nodes.
>
> +VIDIOC_DQEVENT
> +VIDIOC_SUBSCRIBE_EVENT
> +VIDIOC_UNSUBSCRIBE_EVENT
> +
> + The events ioctls are identical to the ones defined in V4L2. They
> + behave identically, with the only exception that they deal only with
> + events generated by the sub-device. Depending on the driver, those
> + events can also be reported by one (or several) V4L2 device nodes.
> +
> + Sub-device drivers that want to use events need to set the
> + V4L2_SUBDEV_USES_EVENTS v4l2_subdev::flags and initialize
> + v4l2_subdev::nevents to events queue depth before registering the
> + sub-device. After registration events can be queued as usual on the
> + v4l2_subdev::devnode device node.
> +
> + To properly support events, the poll() file operation is also
> + implemented.
> +
>
> I2C sub-device drivers
> ----------------------
> diff --git a/drivers/media/video/v4l2-subdev.c b/drivers/media/video/v4l2-subdev.c
> index fc57ce7..fbccefd 100644
> --- a/drivers/media/video/v4l2-subdev.c
> +++ b/drivers/media/video/v4l2-subdev.c
> @@ -20,27 +20,69 @@
> * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> */
>
> -#include <linux/types.h>
> #include <linux/ioctl.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> #include <linux/videodev2.h>
>
> #include <media/v4l2-ctrls.h>
> #include <media/v4l2-device.h>
> #include <media/v4l2-ioctl.h>
> +#include <media/v4l2-fh.h>
> +#include <media/v4l2-event.h>
>
> static int subdev_open(struct file *file)
> {
> struct video_device *vdev = video_devdata(file);
> struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
> + struct v4l2_fh *vfh;
> + int ret;
>
> if (!sd->initialized)
> return -EAGAIN;
>
> + if (sd->flags & V4L2_SUBDEV_FL_HAS_EVENTS) {
> + vfh = kzalloc(sizeof(*vfh), GFP_KERNEL);
> + if (vfh == NULL)
> + return -ENOMEM;
> +
> + ret = v4l2_fh_init(vfh, vdev);
> + if (ret)
> + goto err;
> +
> + ret = v4l2_event_init(vfh);
> + if (ret)
> + goto err;
> +
> + ret = v4l2_event_alloc(vfh, sd->nevents);
> + if (ret)
> + goto err;
> +
> + v4l2_fh_add(vfh);
> + file->private_data = vfh;
> + }
> +
> return 0;
> +
> +err:
> + if (vfh != NULL) {
> + v4l2_fh_exit(vfh);
> + kfree(vfh);
> + }
> +
> + return ret;
> }
>
> static int subdev_close(struct file *file)
> {
> + struct v4l2_fh *vfh = file->private_data;
> +
> + if (vfh != NULL) {
> + v4l2_fh_del(vfh);
> + v4l2_fh_exit(vfh);
> + kfree(vfh);
> + }
> +
> return 0;
> }
>
> @@ -48,6 +90,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
> {
> struct video_device *vdev = video_devdata(file);
> struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
> + struct v4l2_fh *fh = file->private_data;
>
> switch (cmd) {
> case VIDIOC_QUERYCTRL:
> @@ -71,6 +114,18 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
> case VIDIOC_TRY_EXT_CTRLS:
> return v4l2_subdev_try_ext_ctrls(sd, arg);
>
> + case VIDIOC_DQEVENT:
> + if (!(sd->flags & V4L2_SUBDEV_FL_HAS_EVENTS))
> + return -ENOIOCTLCMD;
> +
> + return v4l2_event_dequeue(fh, arg, file->f_flags & O_NONBLOCK);
> +
> + case VIDIOC_SUBSCRIBE_EVENT:
> + return v4l2_subdev_call(sd, core, subscribe_event, fh, arg);
> +
> + case VIDIOC_UNSUBSCRIBE_EVENT:
> + return v4l2_subdev_call(sd, core, unsubscribe_event, fh, arg);
> +
> default:
> return -ENOIOCTLCMD;
> }
> @@ -84,11 +139,29 @@ static long subdev_ioctl(struct file *file, unsigned int cmd,
> return __video_usercopy(file, cmd, arg, subdev_do_ioctl);
> }
>
> +static unsigned int subdev_poll(struct file *file, poll_table *wait)
> +{
> + struct video_device *vdev = video_devdata(file);
> + struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
> + struct v4l2_fh *fh = file->private_data;
> +
> + if (!(sd->flags & V4L2_SUBDEV_FL_HAS_EVENTS))
> + return POLLERR;
> +
> + poll_wait(file, &fh->events->wait, wait);
> +
> + if (v4l2_event_pending(fh))
> + return POLLPRI;
> +
> + return 0;
> +}
> +
> const struct v4l2_file_operations v4l2_subdev_fops = {
> .owner = THIS_MODULE,
> .open = subdev_open,
> .unlocked_ioctl = subdev_ioctl,
> .release = subdev_close,
> + .poll = subdev_poll,
> };
>
> void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops)
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 90022f5..68cbe48 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -37,6 +37,8 @@
>
> struct v4l2_device;
> struct v4l2_ctrl_handler;
> +struct v4l2_event_subscription;
> +struct v4l2_fh;
> struct v4l2_subdev;
> struct tuner_setup;
>
> @@ -165,6 +167,10 @@ struct v4l2_subdev_core_ops {
> int (*s_power)(struct v4l2_subdev *sd, int on);
> int (*interrupt_service_routine)(struct v4l2_subdev *sd,
> u32 status, bool *handled);
> + int (*subscribe_event)(struct v4l2_subdev *sd, struct v4l2_fh *fh,
> + struct v4l2_event_subscription *sub);
> + int (*unsubscribe_event)(struct v4l2_subdev *sd, struct v4l2_fh *fh,
> + struct v4l2_event_subscription *sub);
> };
>
> /* s_mode: switch the tuner to a specific tuner mode. Replacement of s_radio.
> @@ -424,6 +430,8 @@ struct v4l2_subdev_ops {
> #define V4L2_SUBDEV_FL_IS_SPI (1U << 1)
> /* Set this flag if this subdev needs a device node. */
> #define V4L2_SUBDEV_FL_HAS_DEVNODE (1U << 2)
> +/* Set this flag if this subdev generates events. */
> +#define V4L2_SUBDEV_FL_HAS_EVENTS (1U << 3)
Do we need this flag...
>
> /* Each instance of a subdev driver should create this struct, either
> stand-alone or embedded in a larger struct.
> @@ -446,6 +454,8 @@ struct v4l2_subdev {
> /* subdev device node */
> struct video_device devnode;
> unsigned int initialized;
> + /* number of events to be allocated on open */
> + unsigned int nevents;
...when we have this field? We could just test whether nevents > 0.
> };
>
> #define vdev_to_v4l2_subdev(vdev) \
>
Regards,
Hans
--
Hans Verkuil - video4linux developer - sponsored by Cisco
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v6 7/7] v4l: subdev: Events support
2011-02-04 10:12 ` Hans Verkuil
@ 2011-02-04 12:03 ` Sakari Ailus
2011-02-04 12:10 ` Hans Verkuil
0 siblings, 1 reply; 13+ messages in thread
From: Sakari Ailus @ 2011-02-04 12:03 UTC (permalink / raw)
To: Hans Verkuil; +Cc: Laurent Pinchart, linux-media
Hi Hans,
Thanks for the comments!
Hans Verkuil wrote:
...
>> @@ -424,6 +430,8 @@ struct v4l2_subdev_ops {
>> #define V4L2_SUBDEV_FL_IS_SPI (1U << 1)
>> /* Set this flag if this subdev needs a device node. */
>> #define V4L2_SUBDEV_FL_HAS_DEVNODE (1U << 2)
>> +/* Set this flag if this subdev generates events. */
>> +#define V4L2_SUBDEV_FL_HAS_EVENTS (1U << 3)
>
> Do we need this flag...
>
>>
>> /* Each instance of a subdev driver should create this struct, either
>> stand-alone or embedded in a larger struct.
>> @@ -446,6 +454,8 @@ struct v4l2_subdev {
>> /* subdev device node */
>> struct video_device devnode;
>> unsigned int initialized;
>> + /* number of events to be allocated on open */
>> + unsigned int nevents;
>
> ...when we have this field? We could just test whether nevents > 0.
Not necessarily. But:
- It's easy to check whether events are expected to be supported by the
driver using the flag and
- AFAIR it was agreed that as the driver is free to allocate more events
using v4l2_event_alloc(), it may choose not to allocate any at
initialisation but e.g. do it in in VIDIOC_SUBSCRIBE_EVENT only.
What do you think?
--
Sakari Ailus
sakari.ailus@maxwell.research.nokia.com
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v6 7/7] v4l: subdev: Events support
2011-02-04 12:03 ` Sakari Ailus
@ 2011-02-04 12:10 ` Hans Verkuil
0 siblings, 0 replies; 13+ messages in thread
From: Hans Verkuil @ 2011-02-04 12:10 UTC (permalink / raw)
To: Sakari Ailus; +Cc: Laurent Pinchart, linux-media
On Friday, February 04, 2011 13:03:13 Sakari Ailus wrote:
> Hi Hans,
>
> Thanks for the comments!
>
> Hans Verkuil wrote:
> ...
> >> @@ -424,6 +430,8 @@ struct v4l2_subdev_ops {
> >> #define V4L2_SUBDEV_FL_IS_SPI (1U << 1)
> >> /* Set this flag if this subdev needs a device node. */
> >> #define V4L2_SUBDEV_FL_HAS_DEVNODE (1U << 2)
> >> +/* Set this flag if this subdev generates events. */
> >> +#define V4L2_SUBDEV_FL_HAS_EVENTS (1U << 3)
> >
> > Do we need this flag...
> >
> >>
> >> /* Each instance of a subdev driver should create this struct, either
> >> stand-alone or embedded in a larger struct.
> >> @@ -446,6 +454,8 @@ struct v4l2_subdev {
> >> /* subdev device node */
> >> struct video_device devnode;
> >> unsigned int initialized;
> >> + /* number of events to be allocated on open */
> >> + unsigned int nevents;
> >
> > ...when we have this field? We could just test whether nevents > 0.
>
> Not necessarily. But:
>
> - It's easy to check whether events are expected to be supported by the
> driver using the flag and
Testing sd->nevents is even easier, but...
> - AFAIR it was agreed that as the driver is free to allocate more events
> using v4l2_event_alloc(), it may choose not to allocate any at
> initialisation but e.g. do it in in VIDIOC_SUBSCRIBE_EVENT only.
...this is a good point. So let's keep the flag.
Regards,
Hans
>
> What do you think?
>
>
--
Hans Verkuil - video4linux developer - sponsored by Cisco
^ permalink raw reply [flat|nested] 13+ messages in thread