Re: [PATCH 5/9] V4L2: Added Timberdale Logiwin driver
Hello, Richard
i have only two small suggestions.
On Fri, Jun 5, 2009 at 5:40 PM, Richard
Röjforsrichard.rojfors@mocean-labs.com wrote:
V4L2 video capture driver for the logiwin IP on the Timberdale FPGA.
The driver uses the Timberdale DMA engine
Signed-off-by: Richard Röjfors richard.rojfors@mocean-labs.com
---
Index: linux-2.6.30-rc7/drivers/media/video/timblogiw.c
===
--- linux-2.6.30-rc7/drivers/media/video/timblogiw.c (revision 0)
+++ linux-2.6.30-rc7/drivers/media/video/timblogiw.c (revision 867)
@@ -0,0 +1,949 @@
+/*
+ * timblogiw.c timberdale FPGA LogiWin Video In driver
+ * Copyright (c) 2009 Intel Corporation
+ *
+ * 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.
+ *
+ * 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., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+/* Supports:
+ * Timberdale FPGA LogiWin Video In
+ */
+
+#include linux/list.h
+#include linux/version.h
+#include linux/module.h
+#include linux/pci.h
+#include linux/dma-mapping.h
+#include media/v4l2-common.h
+#include media/v4l2-ioctl.h
+#include media/v4l2-device.h
+#include linux/platform_device.h
+#include linux/interrupt.h
+#include timblogiw.h
+#include linux/mfd/timbdma.h
+#include linux/i2c.h
+#include media/timb_video.h
+
+#define TIMBLOGIW_CTRL 0x40
+
+#define TIMBLOGIW_H_SCALE 0x20
+#define TIMBLOGIW_V_SCALE 0x28
+
+#define TIMBLOGIW_X_CROP 0x58
+#define TIMBLOGIW_Y_CROP 0x60
+
+#define TIMBLOGIW_W_CROP 0x00
+#define TIMBLOGIW_H_CROP 0x08
+
+#define TIMBLOGIW_VERSION_CODE 0x02
+
+#define TIMBLOGIW_BUF 0x04
+#define TIMBLOGIW_TBI 0x2c
+#define TIMBLOGIW_BPL 0x30
+
+#define dbg(...)
+
+#define DMA_BUFFER_SIZE (720 * 576 * 2)
+
+const struct timblogiw_tvnorm timblogiw_tvnorms[] = {
+ {
+ .std = V4L2_STD_PAL,
+ .name = PAL,
+ .width = 720,
+ .height = 576
+ },
+ {
+ .std = V4L2_STD_NTSC_M,
+ .name = NTSC,
+ .width = 720,
+ .height = 480
+ }
+};
+
+static int timblogiw_bytes_per_line(const struct timblogiw_tvnorm *norm)
+{
+ return norm-width * 2;
+}
+
+
+static int timblogiw_frame_size(const struct timblogiw_tvnorm *norm)
+{
+ return norm-height * timblogiw_bytes_per_line(norm);
+}
+
+static const struct timblogiw_tvnorm *timblogiw_get_norm(const v4l2_std_id
std)
+{
+ int i;
+ for (i = 0; i ARRAY_SIZE(timblogiw_tvnorms); i++)
+ if (timblogiw_tvnorms[i].std == std)
+ return timblogiw_tvnorms + i;
+
+ /* default to first element */
+ return timblogiw_tvnorms;
+}
+
+static void timblogiw_handleframe(unsigned long arg)
+{
+ struct timblogiw_frame *f;
+ struct timblogiw *lw = (struct timblogiw *)arg;
+
+ spin_lock_bh(lw-queue_lock);
+ if (lw-dma.filled !list_empty(lw-inqueue)) {
+ /* put the entry in the outqueue */
+ f = list_entry(lw-inqueue.next, struct timblogiw_frame,
frame);
+
+ /* copy data from the DMA buffer */
+ memcpy(f-bufmem, lw-dma.filled-buf, f-buf.length);
+ /* buffer consumed */
+ lw-dma.filled = NULL;
+
+ do_gettimeofday(f-buf.timestamp);
+ f-buf.sequence = ++lw-frame_count;
+ f-buf.field = V4L2_FIELD_NONE;
+ f-state = F_DONE;
+ f-buf.bytesused = f-buf.length;
+ list_move_tail(f-frame, lw-outqueue);
+ /* wake up any waiter */
+ wake_up(lw-wait_frame);
+ } else {
+ /* No user buffer available, consume buffer anyway
+ * who wants an old video frame?
+ */
+ lw-dma.filled = NULL;
+ }
+ spin_unlock_bh(lw-queue_lock);
+}
+
+static int timblogiw_isr(u32 flag, void *pdev)
+{
+ struct timblogiw *lw = (struct timblogiw *)pdev;
+
+ if (!lw-dma.filled (flag DMA_IRQ_VIDEO_RX)) {
+ /* Got a frame, store it, and flip to next DMA buffer */
+ lw-dma.filled = lw-dma.transfer + lw-dma.curr;
+ lw-dma.curr = !lw-dma.curr;
+
Re: [linux-dvb] SDMC DM1105N not being detected
On 5 June 2009 21:41:46 Simon Kenyon wrote: Simon Kenyon wrote: Simon Kenyon wrote: the picture seems to be breaking up badly will revert to my version and see if that fixes it [sorry for the delay. i was away on business] i've checked and your original code, modified to compile and including my changes to control the LNB works very well; the patch you posted does not. i have swapped between the two and rebooted several times to make sure. i will do a diff and see what the differences are regards -- simon the main changes seem to be a reworking of the interrupt handling and some i2c changes -- simon How fast is your system? -- Igor M. Liplianin Microsoft Windows Free Zone - Linux used for all Computing Tasks -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] zr364xx.c: vfree does its own NULL check
vfree() does it's own NULL checking, no need for explicit check before calling it. Signed-off-by: Figo.zhang figo1...@gmail.com --- drivers/media/video/zr364xx.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/media/video/zr364xx.c b/drivers/media/video/zr364xx.c index ac169c9..fc976f4 100644 --- a/drivers/media/video/zr364xx.c +++ b/drivers/media/video/zr364xx.c @@ -882,9 +882,11 @@ static void zr364xx_disconnect(struct usb_interface *intf) video_unregister_device(cam-vdev); cam-vdev = NULL; kfree(cam-buffer); - if (cam-framebuf) - vfree(cam-framebuf); + cam-buffer = NULL; + vfree(cam-framebuf); + cam-framebuf = NULL; kfree(cam); + cam = NULL; } -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv5 1 of 8] v4l2_subdev i2c: Add v4l2_i2c_new_subdev_board i2c helper function
On Friday 29 May 2009 09:33:21 Eduardo Valentin wrote:
# HG changeset patch
# User Eduardo Valentin eduardo.valen...@nokia.com
# Date 1243414605 -10800
# Branch export
# Node ID 4fb354645426f8b187c2c90cd8528b2518461005
# Parent 142fd6020df3b4d543068155e49a2618140efa49
Device drivers of v4l2_subdev devices may want to have
board specific data. This patch adds an helper function
to allow bridge drivers to pass board specific data to
v4l2_subdev drivers.
For those drivers which need to support kernel versions
bellow 2.6.26, a .s_config callback was added. The
idea of this callback is to pass board configuration
as well. In that case, subdev driver should set .s_config
properly, because v4l2_i2c_new_subdev_board will call
the .s_config directly. Of course, if we are = 2.6.26,
the same data will be passed through i2c board info as well.
Hi Eduardo,
I finally had some time to look at this. After some thought I realized that
the main problem is really that the API is becoming quite messy. Basically
there are 9 different ways of loading and initializing a subdev:
First there are three basic initialization calls: no initialization, passing
irq and platform_data, and passing the i2c_board_info struct directly
(preferred for drivers that don't need pre-2.6.26 compatibility).
And for each flavor you would like to see three different versions as well:
one with a fixed known i2c address, one where you probe for a list of
addresses, and one where you can probe for a single i2c address.
I propose to change the API as follows:
#define V4L2_I2C_ADDRS(addr, addrs...) \
((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
struct v4l2_subdev *v4l2_i2c_new_subdev(struct v4l2_device *v4l2_dev,
struct i2c_adapter *adapter,
const char *module_name, const char *client_type,
u8 addr, const unsigned short *addrs);
struct v4l2_subdev *v4l2_i2c_new_subdev_cfg(struct v4l2_device *v4l2_dev,
struct i2c_adapter *adapter,
const char *module_name, const char *client_type,
int irq, void *platform_data,
u8 addr, const unsigned short *addrs);
/* Only available for kernels = 2.6.26 */
struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
struct i2c_adapter *adapter, const char *module_name,
struct i2c_board_info *info, const unsigned short *addrs);
If you use a fixed address, then only set addr (or info.addr) and set addrs
to NULL. If you want to probe for a list of addrs, then set addrs to the
list of addrs. If you want to probe for a single addr, then use
V4L2_I2C_ADDRS(addr) as the addrs argument. This constructs an array with
just two entries. Actually, this macro can also create arrays with more
entries.
Note that v4l2_i2c_new_subdev will be an inline that calls
v4l2_i2c_new_subdev_cfg with 0, NULL for the irq and platform_data.
And for kernels = 2.6.26 v4l2_i2c_new_subdev_cfg can be an inline calling
v4l2_i2c_new_subdev_board.
This approach reduces the number of functions to just one (not counting the
inlines) and simplifies things all around. It does mean that all sources
need to be changed, but if we go this route, then now is the time before
the 2.6.31 window is closed. And I would also like to remove the '_new'
from these function names. I never thought it added anything useful.
Comments? If we decide to go this way, then I need to know soon so that I
can make the changes before the 2.6.31 window closes.
BTW, if the new s_config subdev call is present, then it should always be
called. That way the subdev driver can safely do all of its initialization
in s_config, no matter how it was initialized.
Sorry about the long delay in replying to this: it's been very hectic lately
at the expense of my v4l-dvb work.
Regards,
Hans
Signed-off-by: Eduardo Valentin eduardo.valen...@nokia.com
---
drivers/media/video/v4l2-common.c | 37
+++-- include/linux/v4l2-common.h |
8
include/linux/v4l2-subdev.h |1 +
3 files changed, 44 insertions(+), 2 deletions(-)
diff -r 142fd6020df3 -r 4fb354645426
linux/drivers/media/video/v4l2-common.c ---
a/linux/drivers/media/video/v4l2-common.c Mon May 18 02:31:55 2009 +
+++ b/linux/drivers/media/video/v4l2-common.c Wed May 27 11:56:45 2009
+0300 @@ -819,9 +819,10 @@
/* Load an i2c sub-device. */
-struct v4l2_subdev *v4l2_i2c_new_subdev(struct v4l2_device *v4l2_dev,
+static struct v4l2_subdev *__v4l2_i2c_new_subdev(struct v4l2_device
*v4l2_dev, struct i2c_adapter *adapter,
- const char *module_name, const char *client_type, u8 addr)
+ const char *module_name, const char *client_type, u8 addr,
+ int irq, void *platform_data)
{
struct v4l2_subdev *sd = NULL;
struct i2c_client *client;
@@ -840,6 +841,8 @@
memset(info, 0,
Re: [PATCHv5 1 of 8] v4l2_subdev i2c: Add v4l2_i2c_new_subdev_board i2c helper function
On Saturday 06 June 2009 13:59:19 Hans Verkuil wrote:
On Friday 29 May 2009 09:33:21 Eduardo Valentin wrote:
# HG changeset patch
# User Eduardo Valentin eduardo.valen...@nokia.com
# Date 1243414605 -10800
# Branch export
# Node ID 4fb354645426f8b187c2c90cd8528b2518461005
# Parent 142fd6020df3b4d543068155e49a2618140efa49
Device drivers of v4l2_subdev devices may want to have
board specific data. This patch adds an helper function
to allow bridge drivers to pass board specific data to
v4l2_subdev drivers.
For those drivers which need to support kernel versions
bellow 2.6.26, a .s_config callback was added. The
idea of this callback is to pass board configuration
as well. In that case, subdev driver should set .s_config
properly, because v4l2_i2c_new_subdev_board will call
the .s_config directly. Of course, if we are = 2.6.26,
the same data will be passed through i2c board info as well.
Hi Eduardo,
I finally had some time to look at this. After some thought I realized
that the main problem is really that the API is becoming quite messy.
Basically there are 9 different ways of loading and initializing a
subdev:
First there are three basic initialization calls: no initialization,
passing irq and platform_data, and passing the i2c_board_info struct
directly (preferred for drivers that don't need pre-2.6.26
compatibility).
And for each flavor you would like to see three different versions as
well: one with a fixed known i2c address, one where you probe for a list
of addresses, and one where you can probe for a single i2c address.
I propose to change the API as follows:
#define V4L2_I2C_ADDRS(addr, addrs...) \
((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
struct v4l2_subdev *v4l2_i2c_new_subdev(struct v4l2_device *v4l2_dev,
struct i2c_adapter *adapter,
const char *module_name, const char *client_type,
u8 addr, const unsigned short *addrs);
struct v4l2_subdev *v4l2_i2c_new_subdev_cfg(struct v4l2_device *v4l2_dev,
struct i2c_adapter *adapter,
const char *module_name, const char *client_type,
int irq, void *platform_data,
u8 addr, const unsigned short *addrs);
/* Only available for kernels = 2.6.26 */
struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device
*v4l2_dev, struct i2c_adapter *adapter, const char *module_name, struct
i2c_board_info *info, const unsigned short *addrs);
If you use a fixed address, then only set addr (or info.addr) and set
addrs to NULL. If you want to probe for a list of addrs, then set addrs
to the list of addrs. If you want to probe for a single addr, then use
V4L2_I2C_ADDRS(addr) as the addrs argument. This constructs an array with
just two entries. Actually, this macro can also create arrays with more
entries.
Note that v4l2_i2c_new_subdev will be an inline that calls
v4l2_i2c_new_subdev_cfg with 0, NULL for the irq and platform_data.
And for kernels = 2.6.26 v4l2_i2c_new_subdev_cfg can be an inline
calling v4l2_i2c_new_subdev_board.
This approach reduces the number of functions to just one (not counting
the inlines) and simplifies things all around. It does mean that all
sources need to be changed, but if we go this route, then now is the time
before the 2.6.31 window is closed. And I would also like to remove the
'_new' from these function names. I never thought it added anything
useful.
Comments? If we decide to go this way, then I need to know soon so that I
can make the changes before the 2.6.31 window closes.
BTW, if the new s_config subdev call is present, then it should always be
called. That way the subdev driver can safely do all of its
initialization in s_config, no matter how it was initialized.
Sorry about the long delay in replying to this: it's been very hectic
lately at the expense of my v4l-dvb work.
I've done the initial conversion to the new API (no _cfg or _board version
yet) in my ~hverkuil/v4l-dvb-subdev tree. It really simplifies things and
if nobody objects then I'd like to get this in before 2.6.31.
Regards,
Hans
--
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RFC: proposal for new i2c.h macro to initialize i2c address lists on the fly
Hi all,
For video4linux we sometimes need to probe for a single i2c address.
Normally you would do it like this:
static const unsigned short addrs[] = {
addr, I2C_CLIENT_END
};
client = i2c_new_probed_device(adapter, info, addrs);
This is a bit awkward and I came up with this macro:
#define V4L2_I2C_ADDRS(addr, addrs...) \
((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
This can construct a list of one or more i2c addresses on the fly. But this
is something that really belongs in i2c.h, renamed to I2C_ADDRS.
With this macro we can just do:
client = i2c_new_probed_device(adapter, info, I2C_ADDRS(addr));
Comments?
Regards,
Hans
--
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv5 1 of 8] v4l2_subdev i2c: Add v4l2_i2c_new_subdev_board i2c helper function
On Sat, 2009-06-06 at 14:49 +0200, Hans Verkuil wrote:
I propose to change the API as follows:
#define V4L2_I2C_ADDRS(addr, addrs...) \
((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
Comments? If we decide to go this way, then I need to know soon so that I
can make the changes before the 2.6.31 window closes.
I've done the initial conversion to the new API (no _cfg or _board version
yet) in my ~hverkuil/v4l-dvb-subdev tree. It really simplifies things and
if nobody objects then I'd like to get this in before 2.6.31.
+Alternatively, you can create the unsigned short array dynamically:
+
+struct v4l2_subdev *sd = v4l2_i2c_subdev(v4l2_dev, adapter,
+ module_foo, chipid, 0, V4L2_I2C_ADDRS(0x10, 0x12));
Strictly speaking, that's not dynamically in the sense of the
generated machine code - everything is going to come from the local
stack and the initialized data space. The compiler will probably be
smart enough to generate an unnamed array in the initialized data space
anyway, avoiding the use of local stack for the array. :)
Anyway, the macro looks fine to me.
But...
@@ -100,16 +100,16 @@ int cx18_i2c_register(struct cx18 *cx, u
if (hw == CX18_HW_TUNER) {
/* special tuner group handling */
- sd = v4l2_i2c_new_probed_subdev(cx-v4l2_dev,
- adap, mod, type, cx-card_i2c-radio);
+ sd = v4l2_i2c_subdev(cx-v4l2_dev,
+ adap, mod, type, 0, cx-card_i2c-radio);
Something happened with readability for maintenance purposes. We're in
cx18_i2c_register(), we're probing, we're allocating new objects, and
we're registering with two subsystems (i2c and v4l). However, all we
see on the surface is
foo = v4l2_i2c_subdev(blah, blah, blah, ... );
The ALSA subsystem at least uses _create for object constructor type
functions. The v4l2 subdev framework has sophisticated constructors for
convenience. I know new wasn't strcitly correct, as the function does
probe, create, register an object. However, the proposed name does
not make it obvious that it's a constructor, IMO.
Regards,
Andy
Regards,
Hans
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv5 1 of 8] v4l2_subdev i2c: Add v4l2_i2c_new_subdev_board i2c helper function
On Saturday 06 June 2009 17:19:08 Andy Walls wrote:
On Sat, 2009-06-06 at 14:49 +0200, Hans Verkuil wrote:
I propose to change the API as follows:
#define V4L2_I2C_ADDRS(addr, addrs...) \
((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
Comments? If we decide to go this way, then I need to know soon so
that I can make the changes before the 2.6.31 window closes.
I've done the initial conversion to the new API (no _cfg or _board
version yet) in my ~hverkuil/v4l-dvb-subdev tree. It really simplifies
things and if nobody objects then I'd like to get this in before
2.6.31.
+Alternatively, you can create the unsigned short array dynamically:
+
+struct v4l2_subdev *sd = v4l2_i2c_subdev(v4l2_dev, adapter,
+module_foo, chipid, 0, V4L2_I2C_ADDRS(0x10, 0x12));
Strictly speaking, that's not dynamically in the sense of the
generated machine code - everything is going to come from the local
stack and the initialized data space. The compiler will probably be
smart enough to generate an unnamed array in the initialized data space
anyway, avoiding the use of local stack for the array. :)
'on the fly' is perhaps a better term...
Anyway, the macro looks fine to me.
But...
@@ -100,16 +100,16 @@ int cx18_i2c_register(struct cx18 *cx, u
if (hw == CX18_HW_TUNER) {
/* special tuner group handling */
- sd = v4l2_i2c_new_probed_subdev(cx-v4l2_dev,
- adap, mod, type, cx-card_i2c-radio);
+ sd = v4l2_i2c_subdev(cx-v4l2_dev,
+ adap, mod, type, 0, cx-card_i2c-radio);
Something happened with readability for maintenance purposes. We're in
cx18_i2c_register(), we're probing, we're allocating new objects, and
we're registering with two subsystems (i2c and v4l). However, all we
see on the surface is
foo = v4l2_i2c_subdev(blah, blah, blah, ... );
The ALSA subsystem at least uses _create for object constructor type
functions. The v4l2 subdev framework has sophisticated constructors for
convenience. I know new wasn't strcitly correct, as the function does
probe, create, register an object. However, the proposed name does
not make it obvious that it's a constructor, IMO.
Hmm, I should probably just leave this as v4l2_i2c_new_subdev since that
corresponds to the i2c core's i2c_new_device call.
Regards,
Hans
--
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv5 1 of 8] v4l2_subdev i2c: Add v4l2_i2c_new_subdev_board i2c helper function
On Saturday 06 June 2009 14:49:46 Hans Verkuil wrote:
On Saturday 06 June 2009 13:59:19 Hans Verkuil wrote:
On Friday 29 May 2009 09:33:21 Eduardo Valentin wrote:
# HG changeset patch
# User Eduardo Valentin eduardo.valen...@nokia.com
# Date 1243414605 -10800
# Branch export
# Node ID 4fb354645426f8b187c2c90cd8528b2518461005
# Parent 142fd6020df3b4d543068155e49a2618140efa49
Device drivers of v4l2_subdev devices may want to have
board specific data. This patch adds an helper function
to allow bridge drivers to pass board specific data to
v4l2_subdev drivers.
For those drivers which need to support kernel versions
bellow 2.6.26, a .s_config callback was added. The
idea of this callback is to pass board configuration
as well. In that case, subdev driver should set .s_config
properly, because v4l2_i2c_new_subdev_board will call
the .s_config directly. Of course, if we are = 2.6.26,
the same data will be passed through i2c board info as well.
Hi Eduardo,
I finally had some time to look at this. After some thought I realized
that the main problem is really that the API is becoming quite messy.
Basically there are 9 different ways of loading and initializing a
subdev:
First there are three basic initialization calls: no initialization,
passing irq and platform_data, and passing the i2c_board_info struct
directly (preferred for drivers that don't need pre-2.6.26
compatibility).
And for each flavor you would like to see three different versions as
well: one with a fixed known i2c address, one where you probe for a
list of addresses, and one where you can probe for a single i2c
address.
I propose to change the API as follows:
#define V4L2_I2C_ADDRS(addr, addrs...) \
((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
struct v4l2_subdev *v4l2_i2c_new_subdev(struct v4l2_device *v4l2_dev,
struct i2c_adapter *adapter,
const char *module_name, const char *client_type,
u8 addr, const unsigned short *addrs);
struct v4l2_subdev *v4l2_i2c_new_subdev_cfg(struct v4l2_device
*v4l2_dev, struct i2c_adapter *adapter,
const char *module_name, const char *client_type,
int irq, void *platform_data,
u8 addr, const unsigned short *addrs);
/* Only available for kernels = 2.6.26 */
struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device
*v4l2_dev, struct i2c_adapter *adapter, const char *module_name, struct
i2c_board_info *info, const unsigned short *addrs);
If you use a fixed address, then only set addr (or info.addr) and set
addrs to NULL. If you want to probe for a list of addrs, then set addrs
to the list of addrs. If you want to probe for a single addr, then use
V4L2_I2C_ADDRS(addr) as the addrs argument. This constructs an array
with just two entries. Actually, this macro can also create arrays with
more entries.
Note that v4l2_i2c_new_subdev will be an inline that calls
v4l2_i2c_new_subdev_cfg with 0, NULL for the irq and platform_data.
And for kernels = 2.6.26 v4l2_i2c_new_subdev_cfg can be an inline
calling v4l2_i2c_new_subdev_board.
This approach reduces the number of functions to just one (not counting
the inlines) and simplifies things all around. It does mean that all
sources need to be changed, but if we go this route, then now is the
time before the 2.6.31 window is closed. And I would also like to
remove the '_new' from these function names. I never thought it added
anything useful.
Comments? If we decide to go this way, then I need to know soon so that
I can make the changes before the 2.6.31 window closes.
BTW, if the new s_config subdev call is present, then it should always
be called. That way the subdev driver can safely do all of its
initialization in s_config, no matter how it was initialized.
Sorry about the long delay in replying to this: it's been very hectic
lately at the expense of my v4l-dvb work.
I've done the initial conversion to the new API (no _cfg or _board
version yet) in my ~hverkuil/v4l-dvb-subdev tree. It really simplifies
things and if nobody objects then I'd like to get this in before 2.6.31.
I've added the new _cfg and _board fucntions as well in this tree. It needs
a bit of a cleanup before I can do a pull request (the last two patches
should be merged to one), but otherwise this is the code as I think it
should be:
/* Construct an I2C_CLIENT_END-terminated array of i2c addresses on the fly
*/
#define V4L2_I2C_ADDRS(addr, addrs...) \
((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
/* Load an i2c module and return an initialized v4l2_subdev struct.
Only call request_module if module_name != NULL.
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,
Re: [PATCHv5 1 of 8] v4l2_subdev i2c: Add v4l2_i2c_new_subdev_board i2c helper function
On Sat, 6 Jun 2009, Hans Verkuil wrote:
On Saturday 06 June 2009 13:59:19 Hans Verkuil wrote:
I propose to change the API as follows:
#define V4L2_I2C_ADDRS(addr, addrs...) \
((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
struct v4l2_subdev *v4l2_i2c_new_subdev(struct v4l2_device *v4l2_dev,
struct i2c_adapter *adapter,
const char *module_name, const char *client_type,
u8 addr, const unsigned short *addrs);
struct v4l2_subdev *v4l2_i2c_new_subdev_cfg(struct v4l2_device *v4l2_dev,
struct i2c_adapter *adapter,
const char *module_name, const char *client_type,
int irq, void *platform_data,
u8 addr, const unsigned short *addrs);
/* Only available for kernels = 2.6.26 */
struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device
*v4l2_dev, struct i2c_adapter *adapter, const char *module_name, struct
i2c_board_info *info, const unsigned short *addrs);
Maybe addrs could be changed to something like probed_addrs so it's
easier to tell that the two address fields are used differently?
Is v4l2_i2c_new_subdev() in effect just a wrapper around
v4l2_i2c_new_subdev_cfg() that calls it with NO_IRQ and NULL for irq and
platform_data?
And could v4l2_i2c_new_subdev_cfg() be done like this?
struct v4l2_subdev *v4l2_i2c_new_subdev_cfg(struct v4l2_device *v4l2_dev,
struct i2c_adapter *adapter, const char *module_name,
const char *client_type, int irq, void *platform_data,
u8 addr, const unsigned short *addrs)
{
struct i2c_board_info info = {
.addr = addr, .platform_data = platform_data, .irq = irq, };
strlcpy(info.type, client_type, sizeof(info.type));
return v4l2_i2c_new_subdev_board(v4l2_dev, adapter, module_name,
info, addrs);
}
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: proposal for new i2c.h macro to initialize i2c address lists on the fly
On Sat, 6 Jun 2009, Hans Verkuil wrote:
For video4linux we sometimes need to probe for a single i2c address.
Normally you would do it like this:
static const unsigned short addrs[] = {
addr, I2C_CLIENT_END
};
client = i2c_new_probed_device(adapter, info, addrs);
This is a bit awkward and I came up with this macro:
#define V4L2_I2C_ADDRS(addr, addrs...) \
((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
This can construct a list of one or more i2c addresses on the fly. But this
is something that really belongs in i2c.h, renamed to I2C_ADDRS.
With this macro we can just do:
client = i2c_new_probed_device(adapter, info, I2C_ADDRS(addr));
Comments?
I agree it's better placed with i2c.h. It's also possible to come up with
a more syntax more variadic function like, which can be called like this:
i2c_new_probed_device(adapter, info, addr1);
i2c_new_probed_device(adapter, info, addr1, addr2);
Using a macro like this...
#define i2c_new_probed_device(adap,info,addr,addrs...) ({ \
const unsigned short _addrs[] = {addr, ## addrs, I2C_CLIENT_END }; \
_i2c_new_probed_device(adap, info, _addrs); })
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] tuner-simple, tveeprom: Add support for the FQ1216LME MK3
Am Samstag, 6. Juni 2009 03:59:41 schrieb Andy Walls: Hi, This is version 2 of the patch that: 1. adds explicit support for the FQ1216LME MK3 2. points the tveeprom module to the FQ1216LME MK3 entry for EEPROMs claiming FQ1216LME MK3 and MK5. 3. refactors some code in simple_post_tune() because I needed to set the Auxillary Byte, as did TUNER_LG_TDVS_H06XF, so I could set the TUA6030 TOP to external AGC per the datasheet. The patch no longer twiddles the CP bit nor polls the FL bit for the FQ1216LME. That will be something for another patch. Any other comments? Hi Andy, I have tested this with a current v4l-dvb hg and kernel 2.6.29 with my PVR 150 (FQ1216LME MK5). It works like using tuner type 38, but unfortunately it does not solve the problem of ocassionally loosing audio. This happens even on the video inputs! I need to do further testings with older drivers/kernels to verify if this may be an issue of the new i2c stuff. Testing is very difficult, because sometimes it works for an hour and more, maybe depending on the temperature or the moon phase. The problem reminds me a little bit on this: http://www.gossamer-threads.com/lists/ivtv/users/39029 This is what dmesg says: [ 1864.639391] ivtv: Start initialization, version 1.4.1 [ 1864.639484] ivtv0: Initializing card 0 [ 1864.639489] ivtv0: Autodetected Hauppauge card (cx23416 based) [ 1864.641281] ivtv :01:09.0: PCI INT A - Link[APC4] - GSI 19 (level, low) - IRQ 19 [ 1864.694693] tveeprom 0-0050: Hauppauge model 26709, rev F0C1, serial# 9585403 [ 1864.694697] tveeprom 0-0050: tuner model is Philips FQ1216LME MK5 (idx 121, type 80) [ 1864.694700] tveeprom 0-0050: TV standards PAL(B/G) PAL(I) SECAM(L/L') PAL(D/D1/K) (eeprom 0x74) [ 1864.694703] tveeprom 0-0050: audio processor is CX25843 (idx 37) [ 1864.694705] tveeprom 0-0050: decoder processor is CX25843 (idx 30) [ 1864.694707] tveeprom 0-0050: has no radio [ 1864.694709] ivtv0: Autodetected Hauppauge WinTV PVR-150 [ 1864.704531] cx25840 0-0044: cx25843-24 found @ 0x88 (ivtv i2c driver #0) [ 1864.723533] tuner 0-0043: chip found @ 0x86 (ivtv i2c driver #0) [ 1864.723613] tda9887 0-0043: creating new instance [ 1864.723615] tda9887 0-0043: tda988[5/6/7] found [ 1864.730824] tuner 0-0061: chip found @ 0xc2 (ivtv i2c driver #0) [ 1864.741408] wm8775 0-001b: chip found @ 0x36 (ivtv i2c driver #0) [ 1864.749596] tuner-simple 0-0061: creating new instance [ 1864.749599] tuner-simple 0-0061: type set to 80 (Philips FQ1216LME MK3 PAL/SECAM w/active loopthrough) [ 1864.752114] IRQ 19/ivtv0: IRQF_DISABLED is not guaranteed on shared IRQs [ 1864.753099] ivtv0: Registered device video0 for encoder MPG (4096 kB) [ 1864.756347] ivtv0: Registered device video32 for encoder YUV (2048 kB) [ 1864.760023] ivtv0: Registered device vbi1 for encoder VBI (1024 kB) [ 1864.764195] ivtv0: Registered device video24 for encoder PCM (320 kB) [ 1864.764199] ivtv0: Initialized card: Hauppauge WinTV PVR-150 [ 1864.770863] ivtv: End initialization [ 1864.797718] usbcore: registered new interface driver pvrusb2 [ 1864.797727] pvrusb2: V4L in-tree version:Hauppauge WinTV-PVR-USB2 MPEG2 Encoder/Tuner [ 1864.797729] pvrusb2: Debug mask is 31 (0x1f) [ 1864.843601] saa7146: register extension 'budget_ci dvb'. [ 1864.862564] saa7146: register extension 'budget dvb'. [ 1864.891936] b2c2-flexcop: B2C2 FlexcopII/II(b)/III digital TV receiver chip loaded successfully [ 1865.423084] lirc_serial: auto-detected active low receiver [ 1865.423089] lirc_dev: lirc_register_driver: sample_rate: 0 [ 1865.424882] lirc_serial $Revision: 5.100 $ registered [ 1871.259020] ivtv :01:09.0: firmware: requesting v4l-cx2341x-enc.fw [ 1871.298034] ivtv0: Loaded v4l-cx2341x-enc.fw firmware (376836 bytes) [ 1871.498142] ivtv0: Encoder revision: 0x02060039 [ 1871.514028] cx25840 0-0044: firmware: requesting v4l-cx25840.fw [ 1874.755988] cx25840 0-0044: loaded v4l-cx25840.fw firmware (16382 bytes) [ 4426.500017] Clocksource tsc unstable (delta = -62502716 ns) Greets, Martin -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PULL] http://linuxtv.org/hg/~dougsland/em28xx/
Hello Mauro, Please pull from http://www.linuxtv.org/hg/~dougsland/em28xx for the following: - em28xx: Add Kworld 315 entry - em28xx: set up tda9887_conf in em28xx_card_setup() Thanks, Douglas -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-dvb] SDMC DM1105N not being detected
Igor M. Liplianin wrote: On 5 June 2009 21:41:46 Simon Kenyon wrote: Simon Kenyon wrote: Simon Kenyon wrote: the picture seems to be breaking up badly will revert to my version and see if that fixes it [sorry for the delay. i was away on business] i've checked and your original code, modified to compile and including my changes to control the LNB works very well; the patch you posted does not. i have swapped between the two and rebooted several times to make sure. i will do a diff and see what the differences are regards -- simon the main changes seem to be a reworking of the interrupt handling and some i2c changes -- simon How fast is your system? reasonably fast it is a dual core AMD64 X2 running at 3.1GHz /proc/cpuinfo says: processor : 0 vendor_id : AuthenticAMD cpu family : 15 model : 107 model name : AMD Athlon(tm) 64 X2 Dual Core Processor 6000+ stepping: 2 cpu MHz : 3100.268 cache size : 512 KB physical id : 0 siblings: 2 core id : 0 cpu cores : 2 apicid : 0 initial apicid : 0 fpu : yes fpu_exception : yes cpuid level : 1 wp : yes flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ht syscall nx mmxext fxsr_opt rdtscp lm 3dnowext 3dnow rep_good pni cx16 lahf_lm cmp_legacy svm extapic cr8_legacy 3dnowprefetch bogomips: 6203.89 TLB size: 1024 4K pages clflush size: 64 cache_alignment : 64 address sizes : 40 bits physical, 48 bits virtual power management: ts fid vid ttp tm stc 100mhzsteps processor : 1 vendor_id : AuthenticAMD cpu family : 15 model : 107 model name : AMD Athlon(tm) 64 X2 Dual Core Processor 6000+ stepping: 2 cpu MHz : 3100.268 cache size : 512 KB physical id : 0 siblings: 2 core id : 1 cpu cores : 2 apicid : 1 initial apicid : 1 fpu : yes fpu_exception : yes cpuid level : 1 wp : yes flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ht syscall nx mmxext fxsr_opt rdtscp lm 3dnowext 3dnow rep_good pni cx16 lahf_lm cmp_legacy svm extapic cr8_legacy 3dnowprefetch bogomips: 6203.89 TLB size: 1024 4K pages clflush size: 64 cache_alignment : 64 address sizes : 40 bits physical, 48 bits virtual power management: ts fid vid ttp tm stc 100mhzsteps -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv5 1 of 8] v4l2_subdev i2c: Add v4l2_i2c_new_subdev_board i2c helper function
Hi Hans,
On Sat, Jun 6, 2009 at 8:09 PM, Hans Verkuil hverk...@xs4all.nl wrote:
On Saturday 06 June 2009 14:49:46 Hans Verkuil wrote:
On Saturday 06 June 2009 13:59:19 Hans Verkuil wrote:
On Friday 29 May 2009 09:33:21 Eduardo Valentin wrote:
# HG changeset patch
# User Eduardo Valentin eduardo.valen...@nokia.com
# Date 1243414605 -10800
# Branch export
# Node ID 4fb354645426f8b187c2c90cd8528b2518461005
# Parent 142fd6020df3b4d543068155e49a2618140efa49
Device drivers of v4l2_subdev devices may want to have
board specific data. This patch adds an helper function
to allow bridge drivers to pass board specific data to
v4l2_subdev drivers.
For those drivers which need to support kernel versions
bellow 2.6.26, a .s_config callback was added. The
idea of this callback is to pass board configuration
as well. In that case, subdev driver should set .s_config
properly, because v4l2_i2c_new_subdev_board will call
the .s_config directly. Of course, if we are = 2.6.26,
the same data will be passed through i2c board info as well.
Hi Eduardo,
I finally had some time to look at this. After some thought I realized
that the main problem is really that the API is becoming quite messy.
Basically there are 9 different ways of loading and initializing a
subdev:
First there are three basic initialization calls: no initialization,
passing irq and platform_data, and passing the i2c_board_info struct
directly (preferred for drivers that don't need pre-2.6.26
compatibility).
And for each flavor you would like to see three different versions as
well: one with a fixed known i2c address, one where you probe for a
list of addresses, and one where you can probe for a single i2c
address.
I propose to change the API as follows:
#define V4L2_I2C_ADDRS(addr, addrs...) \
((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
struct v4l2_subdev *v4l2_i2c_new_subdev(struct v4l2_device *v4l2_dev,
struct i2c_adapter *adapter,
const char *module_name, const char *client_type,
u8 addr, const unsigned short *addrs);
struct v4l2_subdev *v4l2_i2c_new_subdev_cfg(struct v4l2_device
*v4l2_dev, struct i2c_adapter *adapter,
const char *module_name, const char *client_type,
int irq, void *platform_data,
u8 addr, const unsigned short *addrs);
/* Only available for kernels = 2.6.26 */
struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device
*v4l2_dev, struct i2c_adapter *adapter, const char *module_name, struct
i2c_board_info *info, const unsigned short *addrs);
If you use a fixed address, then only set addr (or info.addr) and set
addrs to NULL. If you want to probe for a list of addrs, then set addrs
to the list of addrs. If you want to probe for a single addr, then use
V4L2_I2C_ADDRS(addr) as the addrs argument. This constructs an array
with just two entries. Actually, this macro can also create arrays with
more entries.
Note that v4l2_i2c_new_subdev will be an inline that calls
v4l2_i2c_new_subdev_cfg with 0, NULL for the irq and platform_data.
And for kernels = 2.6.26 v4l2_i2c_new_subdev_cfg can be an inline
calling v4l2_i2c_new_subdev_board.
This approach reduces the number of functions to just one (not counting
the inlines) and simplifies things all around. It does mean that all
sources need to be changed, but if we go this route, then now is the
time before the 2.6.31 window is closed. And I would also like to
remove the '_new' from these function names. I never thought it added
anything useful.
Comments? If we decide to go this way, then I need to know soon so that
I can make the changes before the 2.6.31 window closes.
BTW, if the new s_config subdev call is present, then it should always
be called. That way the subdev driver can safely do all of its
initialization in s_config, no matter how it was initialized.
Sorry about the long delay in replying to this: it's been very hectic
lately at the expense of my v4l-dvb work.
I've done the initial conversion to the new API (no _cfg or _board
version yet) in my ~hverkuil/v4l-dvb-subdev tree. It really simplifies
things and if nobody objects then I'd like to get this in before 2.6.31.
I've added the new _cfg and _board fucntions as well in this tree. It needs
a bit of a cleanup before I can do a pull request (the last two patches
should be merged to one), but otherwise this is the code as I think it
should be:
/* Construct an I2C_CLIENT_END-terminated array of i2c addresses on the fly
*/
#define V4L2_I2C_ADDRS(addr, addrs...) \
((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
/* Load an i2c module and return an initialized v4l2_subdev struct.
Only call request_module if
Re: Probably strange bug with usb radio-mr800
Am Freitag, 5. Juni 2009 00:43:04 schrieb Alexey Klimov: Is there any ideas about different behaviour of device on 32- and 64-bit platforms with the same usb bulk messages? Any input is welcome. Are you running a 32 bit userland? If so, ioctls could be critical. If not, the driver may not be 64bit clean. Which driver is affected? Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: proposal for new i2c.h macro to initialize i2c address lists on the fly
Em Sat, 6 Jun 2009 15:00:48 +0200
Hans Verkuil hverk...@xs4all.nl escreveu:
Hi all,
For video4linux we sometimes need to probe for a single i2c address.
Normally you would do it like this:
static const unsigned short addrs[] = {
addr, I2C_CLIENT_END
};
client = i2c_new_probed_device(adapter, info, addrs);
This is a bit awkward and I came up with this macro:
#define V4L2_I2C_ADDRS(addr, addrs...) \
((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
This can construct a list of one or more i2c addresses on the fly. But this
is something that really belongs in i2c.h, renamed to I2C_ADDRS.
With this macro we can just do:
client = i2c_new_probed_device(adapter, info, I2C_ADDRS(addr));
Comments?
Seems fine for me, but Your define has V4L2_foo.
Since this has nothing to do with V4L2, IMO, it is better to declare it as:
#define I2C_ADDRS(addr, addrs...) \
((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
Cheers,
Mauro
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: proposal for new i2c.h macro to initialize i2c address lists on the fly
On Sat, Jun 6, 2009 at 9:00 AM, Hans Verkuilhverk...@xs4all.nl wrote:
Hi all,
For video4linux we sometimes need to probe for a single i2c address.
Normally you would do it like this:
Why does video4linux need to probe to find i2c devices? Can't the
address be determined by knowing the PCI ID of the board?
static const unsigned short addrs[] = {
addr, I2C_CLIENT_END
};
client = i2c_new_probed_device(adapter, info, addrs);
This is a bit awkward and I came up with this macro:
#define V4L2_I2C_ADDRS(addr, addrs...) \
((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
This can construct a list of one or more i2c addresses on the fly. But this
is something that really belongs in i2c.h, renamed to I2C_ADDRS.
With this macro we can just do:
client = i2c_new_probed_device(adapter, info, I2C_ADDRS(addr));
Comments?
Regards,
Hans
--
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Jon Smirl
jonsm...@gmail.com
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: proposal for new i2c.h macro to initialize i2c address lists on the fly
Hi, Am Samstag, den 06.06.2009, 18:20 -0400 schrieb Jon Smirl: On Sat, Jun 6, 2009 at 9:00 AM, Hans Verkuilhverk...@xs4all.nl wrote: Hi all, For video4linux we sometimes need to probe for a single i2c address. Normally you would do it like this: Why does video4linux need to probe to find i2c devices? Can't the address be determined by knowing the PCI ID of the board? NO, are you dreaming ? Even the m$ attempts over additional stuff, that _eventually_ conforms to something in the eeprom are doomed. That is also not about video4linux anymore, since decades now soon, but about v4l-dvb. For real interesting stuff it does not work anymore and never did. You should have a look at real hardware. Cheers, Hermann -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH]videobuf-core.c: add pointer check
On Sun, 2009-06-07 at 09:16 +0800, Figo.zhang wrote:
On Wed, 2009-06-03 at 10:01 +0800, Figo.zhang wrote:
add poiter check for videobuf_queue_core_init().
any guys who write a v4l driver, pass a NULL pointer or a non-inintial
pointer to the first parameter such as videobuf_queue_sg_init() , it
would be crashed.
Signed-off-by: Figo.zhang figo1...@x
---
drivers/media/video/videobuf-core.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/media/video/videobuf-core.c
b/drivers/media/video/videobuf-core.c
index b7b0584..5f41fd9 100644
--- a/drivers/media/video/videobuf-core.c
+++ b/drivers/media/video/videobuf-core.c
@@ -118,6 +118,7 @@ void videobuf_queue_core_init(struct videobuf_queue *q,
void *priv,
struct videobuf_qtype_ops *int_ops)
{
+ BUG_ON(!q);
memset(q, 0, sizeof(*q));
q-irqlock = irqlock;
q-dev = dev;
i do a test driver for it, the main code like this.
struct mydev_dev{
struct video_device *video_dev;
...
struct videobuf_queue *cap;
};
static int video_open(struct inode *inode, struct file *file)
{
...
videobuf_queue_dma_contig_init(dev-cap, video_qops,
dev-pci-dev, dev-slock,
V4L2_BUF_TYPE_VIDEO_CAPTURE,
V4L2_FIELD_ALTERNATE,
sizeof(struct mydev_buf),
dev);
...
}
i pass a non-initial pointer for the first argment, so it crashed.
BUG: unable to handle kernel NULL pointer dereference at
IP: [f8d6bd67] :videobuf_core:videobuf_queue_core_init+0x1b/0xbf
*pde = 7ed5a067
Oops: 0002 [#1] SMP
Modules linked in: mydev_drv tvp5160_vd videobuf_dma_contig videobuf_dma_sg
videobuf_core videocodec videodev v4l2_int_device v4l2_common v4l1_compat
compat_ioctl32 fuse i915 drm sco bridge stp bnep l2cap bluetooth sunrpc ipv6
cpufreq_ondemand acpi_cpufreq dm_multipath uinput snd_hda_intel snd_seq_dummy
snd_seq_oss snd_seq_midi_event snd_seq snd_seq_device ppdev snd_pcm_oss
parport_pc
snd_mixer_oss snd_pcm snd_timer snd_page_alloc snd_hwdep parport snd
i2c_i801
i2c_core pcspkr soundcore iTCO_wdt iTCO_vendor_support r8169 mii ftdi_sio
usbserial
ata_generic pata_acpi [last unloaded: microcode]
Pid: 4053, comm: capture Not tainted (2.6.27.5-117.fc10.i686 #1)
EIP: 0060:[f8d6bd67] EFLAGS: 00210246 CPU: 1
EIP is at videobuf_queue_core_init+0x1b/0xbf [videobuf_core]
EAX: EBX: ECX: 0036 EDX: 0036
ESI: f8e1e158 EDI: EBP: f15f3e28 ESP: f15f3e18
DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
Process capture (pid: 4053, ti=f15f3000 task=f155 task.ti=f15f3000)
Stack: f790905c f5fd5224 f15d8840 f5811560 f15f3e48 f8e01163 f5fd5224
0001
0007 006c f5fd5200 f8e0202c f15f3e70 f8e1b5a0 f5fd5224
0001
0007 006c f5fd5200 f8e0faa4 f15d8840 f15f3e88
f8e0c195
Call Trace:
[f8e01163] ? videobuf_queue_dma_contig_init+0x1c/0x21 [videobuf_dma_contig]
[f8e1b5a0] ? video_open+0x8b/0xb1 [kt2741drv]
[f8e0c195] ? video_open+0xc7/0x125 [videodev]
[c0492767] ? chrdev_open+0x12b/0x142
[c048edd2] ? __dentry_open+0x10e/0x1fc
[c048ef47] ? nameidata_to_filp+0x1f/0x33
[c049263c] ? chrdev_open+0x0/0x142
[c0498a3c] ? do_filp_open+0x31c/0x611
[c048a971] ? virt_to_head_page+0x22/0x2e
[c041d8ba] ? need_resched+0x18/0x22
[c048ebf0] ? do_sys_open+0x42/0xb7
[c048eca7] ? sys_open+0x1e/0x26
[c0403c76] ? syscall_call+0x7/0xb
[c06a007b] ? init_intel_cacheinfo+0x0/0x421
===
Code: d8 e8 69 ff ff ff 89 d8 e8 6d a9 93 c7 5b 5d c3 55 89 e5 57 89 c7 56 89
d6 53 ba 36 00 00 00 83 ec 04 89 c3 89 4d f0 31 c0 89 d1 f3 ab 8b 45 08 8b
4d f0 89 b3 b8 00 00 00 89 43 10 8b 45 0c 89
EIP: [f8d6bd67] videobuf_queue_core_init+0x1b/0xbf [videobuf_core] SS:ESP
0068:f15f3e18
---[ end trace 4bfe52d17b82af8c ]---
so i think it need to add pointer checking for the first argment of
videobuf_queue_core_init(),
the videobuf code would be more stronger and reliable.
hi, Mauro Hans,
at this point, would you agree with me or would you like to give me some
advice?
Best Regards,
Figo.zhang
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
