Re: Allocating videobuf_buffer, but lists not being initialized
于 11/16/2010 03:37 PM, Hans Verkuil 写道: On Tuesday, November 16, 2010 02:10:39 Andrew Chew wrote: I'm looking at drivers/media/video/videobuf-dma-contig.c's __videobuf_alloc() routine. We call kzalloc() to allocate the videobuf_buffer. However, I don't see where the two lists (vb-stream and vb-queue) that are a part of struct videobuf_buffer get initialized (with, say, INIT_LIST_HEAD). Yuck. The videobuf framework doesn't initialize vb-stream at all. It relies on list_add_tail to effectively initialize it for it. It works, but it is not exactly clean programming :-( The vb-queue list has to be initialized in the driver. Never understood the reason for that either. Marek, can you make sure that videobuf2 will initialize these lists correctly? That is, vb2 should do this initialization instead of the driver. vb2 have init the list : INIT_LIST_HEAD(q-queued_list); INIT_LIST_HEAD(q-done_list); btw, queued_list re-name grabbing_list is better? -- 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: Allocating videobuf_buffer, but lists not being initialized
于 11/16/2010 03:40 PM, Hans Verkuil 写道: On Tuesday, November 16, 2010 06:29:53 Pawel Osciak wrote: On Mon, Nov 15, 2010 at 17:10, Andrew Chewac...@nvidia.com wrote: I'm looking at drivers/media/video/videobuf-dma-contig.c's __videobuf_alloc() routine. We call kzalloc() to allocate the videobuf_buffer. However, I don't see where the two lists (vb-stream and vb-queue) that are a part of struct videobuf_buffer get initialized (with, say, INIT_LIST_HEAD). Those are not lists, but list entries. Those members of videobuf_buffer struct are used to put the buffer on one of the following lists: stream is a list entry for stream list in videobuf_queue, queue is used as list entry for driver's buffer queue. So? They still should be initialized properly. It's bad form to leave invalid pointers there. it just a list-entry, it has initialized by kzalloc at __videobuf_alloc_vb(), 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: af9015 and nxp tda182128 support
On Mon, Nov 15, 2010 at 05:44:55PM +0100, Okkel Klaver wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hello everybody, I own a brandless hdtv usb dvb-t stick. lsusb identifies it as: Bus 001 Device 005: ID 15a4:9016 Afatech Technologies, Inc. AF9015 DVB-T USB2.0 stick few days ago I submeitted [PATCH] terratec cinergy t-stick RC (with TDA18218) that applies to latest normal (not new) v4l tree Just add your usb id as terratec cinergy t-stick RC, make install and try it. it might work best regards, Emard -- 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] media: video: do not clear 'driver' from an i2c_client
Hi Wolfram, Sorry for the late answer. On Mon, 15 Nov 2010 23:28:15 +0100, Wolfram Sang wrote: On Wed, Nov 10, 2010 at 03:14:13PM +0100, Wolfram Sang wrote: The i2c-core does this already. Reported-by: Jean Delvare kh...@linux-fr.org Signed-off-by: Wolfram Sang w.s...@pengutronix.de --- Not sure if this should go via i2c or media? Okay, as Jean did not pick it up in his latest pull request, I guess this means it shall go via the media-tree? :) Mauro, will you pick it up? Yes I think it should go through Mauro's tree. I was simply waiting for your other patch to get merged first (which happened yesterday), because this patch depended on that one. So: Acked-by: Jean Delvare kh...@linux-fr.org and Mauro please pick the patch in your tree. Thanks, -- Jean Delvare -- 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: [omap3isp][PATCH v2 0/9] YUV support for CCDC + cleanups
Hi Sergio, Thanks for the patches. I've reviewed and tested them and found no issue. They will be a small delay as I need to sync our internal tree with the public tree before applying them. -- Regards, Laurent Pinchart -- 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 PATCH 0/2] Apple remote support
Em 15-11-2010 02:11, Jarod Wilson escreveu: On Fri, Nov 5, 2010 at 9:27 AM, David Härdeman da...@hardeman.nu wrote: On Thu, 04 Nov 2010 15:43:33 -0400, Mauro Carvalho Chehab mche...@redhat.com wrote: Em 04-11-2010 15:38, David Härdeman escreveu: On Thu, Nov 04, 2010 at 11:54:25AM -0400, Jarod Wilson wrote: Okay, so we seem to be in agreement for an approach to handling this. I'll toss something together implementing that RSN... Though I talked with Mauro about this a bit yesterday here at LPC, and we're thinking maybe we slide this support back over into the nec decoder and make it a slightly more generic use full 32 bits NEC variant we look for and/or enable/disable somehow. I've got another remote here, for a Motorola cable box, which is NEC-ish, but always fails decode w/a checksum error (got 0x, iirc), which may also need to use the full 32 bits somehow... Probably a very important protocol variant to support, particularly once we have native transmit support, as its used by plenty of cable boxes on the major carriers here in the US. I've always found the checksum tests in the NEC decoder to be unnecessary so I'm all for using a 32 bit scancode in all cases (and still using a module param to squash the ID byte of apple remotes, defaulting to yes). This means changing all existing NEC tables to have 32 bits, and add the redundant information on all of them. Yep (though we should use macros to generate scancodes) It doesn't seem a good idea to me to add a penalty for those NEC tables that follow the standard. Which penalty? Using a 32 bit scancode won't affect keytable size or lookup speed. In some corner cases, additional keytable lookups will be performed for decoded scancodes which would otherwise be deemed invalid, but at the time that decision can be made, most of the processing (reading timing events from hardware, handing them to decoders, decoding them) has already been done. If you're referring to the pain caused by changing existing keytables (thereby breaking custom keytables), I think it's inevitable. Throwing away information is not a good solution. As this subsystem progresses, there's going to be more and more reports of remotes which, intentionally or unintentionally, do not follow the NEC standard (I use that word in the most liberal sense). Using the full 32 bits allows us to support them without any module parameters or code changes. Which solution do you suggest? Well, here's what I sent along on Friday: https://patchwork.kernel.org/patch/321592/ Gives us support for using the full 32-bit codes right now w/o having to change any tables yet, but does require a modparam to skip the checksum checks, unless its an apple remote which we already know the vendor bytes for. I do think I'm ultimately leaning towards just doing the full 32 bits for all nec extended though -- optionally, we might include a modparam to *enable* the checksum check for those that want strict compliance (but I'd still say use the full 32 bits). The only issue I see with going to the full 32 right now is that we have all these nec tables with only 24 bits, and we don't know ... oh, wait, no, nevermind... We *do* know the missing 8 bits, since they have to fulfill the checksum check for command ^ not_command. So yeah, I'd say 32-bit scancodes for all nec extended, don't enforce the checksum by default with a module option (or sysfs knob) to enable checksum compliance. A modprobe parameter for it doesn't seem right. Users should not need to do any manual hack for ther RC to work, if the keycode table is ok. Also, changing the current tables to 32 bits will break userspace API, as all userspace keytables for NEC will stop working, all due to a few vendors that decided to abuse on the NEC protocol. This doesn't sound fair. Considering that the new setkeycode/getkeycode ioctls support a variable size for scancodes, it seems to me that the proper solution is properly add support for variable-size scancode tables. By doing this, one of the properties for a scancode table is the size of the scancode. The NEC decoding logic can take the scancode size into account, when deciding to check checksum or not. 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 PATCH 0/8] V4L BKL removal: first round
Em 15-11-2010 07:49, Hans Verkuil escreveu: On Sunday 14 November 2010 23:48:51 Hans Verkuil wrote: On Sunday, November 14, 2010 22:53:29 Arnd Bergmann wrote: On Sunday 14 November 2010, Hans Verkuil wrote: This patch series converts 24 v4l drivers to unlocked_ioctl. These are low hanging fruit but you have to start somewhere :-) The first patch replaces mutex_lock in the V4L2 core by mutex_lock_interruptible for most fops. The patches all look good as far as I can tell, but I suppose the title is obsolete now that the BKL has been replaced with a v4l-wide mutex, which is what you are removing in the series. I guess I have to rename it, even though strictly speaking the branch I'm working in doesn't have your patch merged yet. BTW, replacing the BKL with a static mutex is rather scary: the BKL gives up the lock whenever you sleep, the mutex doesn't. Since sleeping is very common in V4L (calling VIDIOC_DQBUF will typically sleep while waiting for a new frame to arrive), this will make it impossible for another process to access any v4l2 device node while the ioctl is sleeping. I am not sure whether that is what you intended. Or am I missing something? I was aware that something like this could happen, but I apparently misjudged how big the impact is. The general pattern for ioctls is that those that get called frequently do not sleep, so it can almost always be called with a mutex held. True in general, but most definitely not true for V4L. The all important VIDIOC_DQBUF ioctl will almost always sleep. Mauro, I think this patch will have to be reverted and we just have to do the hard work ourselves. The VIDIOC_QBUF/VIDIOC_DQBUF ioctls are called after having the V4L device ready for stream. During the qbuf/dqbuf loop, the only other ioctls that may appear are the control change ioctl's, to adjust things like bright. I doubt that his will cause a really serious trouble. On the other hand, currently, if BKL is disabled, the entire V4L subsystem is disabled. So, IMO, the impact of having Arnd's patch applied is less than just having almost all drivers disabled if BKL is not compiled. So, I prefer to apply his patch and then fix it, driver by driver, than to disable the entire subsystem on .37. 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 PATCH 0/8] V4L BKL removal: first round
Em 15-11-2010 07:49, Hans Verkuil escreveu: On Sunday 14 November 2010 23:48:51 Hans Verkuil wrote: On Sunday, November 14, 2010 22:53:29 Arnd Bergmann wrote: On Sunday 14 November 2010, Hans Verkuil wrote: This patch series converts 24 v4l drivers to unlocked_ioctl. These are low hanging fruit but you have to start somewhere :-) The first patch replaces mutex_lock in the V4L2 core by mutex_lock_interruptible for most fops. The patches all look good as far as I can tell, but I suppose the title is obsolete now that the BKL has been replaced with a v4l-wide mutex, which is what you are removing in the series. I guess I have to rename it, even though strictly speaking the branch I'm working in doesn't have your patch merged yet. BTW, replacing the BKL with a static mutex is rather scary: the BKL gives up the lock whenever you sleep, the mutex doesn't. Since sleeping is very common in V4L (calling VIDIOC_DQBUF will typically sleep while waiting for a new frame to arrive), this will make it impossible for another process to access any v4l2 device node while the ioctl is sleeping. I am not sure whether that is what you intended. Or am I missing something? I was aware that something like this could happen, but I apparently misjudged how big the impact is. The general pattern for ioctls is that those that get called frequently do not sleep, so it can almost always be called with a mutex held. True in general, but most definitely not true for V4L. The all important VIDIOC_DQBUF ioctl will almost always sleep. Mauro, I think this patch will have to be reverted and we just have to do the hard work ourselves. The VIDIOC_QBUF/VIDIOC_DQBUF ioctls are called after having the V4L device ready for stream. During the qbuf/dqbuf loop, the only other ioctls that may appear are the control change ioctl's, to adjust things like bright. I doubt that his will cause a really serious trouble. Yes, it does. Anyone who is using multiple capture/output devices at the same time will be affected. For example, anyone who uses the davinci dm6467 driver for both input and output. And yes, that's what we use at work. And ship to thousands of customers. Or think about surveillance applications where you are capturing from many streams simultaneously. This change *does* cause serious trouble. On the other hand, currently, if BKL is disabled, the entire V4L subsystem is disabled. So, IMO, the impact of having Arnd's patch applied is less than just having almost all drivers disabled if BKL is not compiled. So, I prefer to apply his patch and then fix it, driver by driver, than to disable the entire subsystem on .37. Can't we test for CONFIG_LOCK_KERNEL and switch to lock_kernel if set? For now it is just a kernel config option. That seems much more reasonable. Regards, Hans -- Hans Verkuil - video4linux developer - sponsored by Cisco -- 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 v4 0/6] FM V4L2 drivers for WL128x
From: Manjunatha Halli manjunatha_ha...@ti.com Mauro, Hans and the list, This is the v4 version of the TI WL128x FM V4L2 drivers patchset. Texas Instrument's WL128x chipset packs BT, FM, GPS and WLAN in a single die with BT, FM and GPS being interfaced over a single UART. This driver works on top of the shared transport line discipline driver. This driver can also be made use for the WL127x version of the chip which packs BT, FM and WLAN only. Comments on the last version of the patches have been taken care, such as - New private CID for Channel Spacing added. - FM Seek Wrap around support is also added. - Few other style issues also taken care of. Also, Can you please also stage this version? Since the files are becoming big to be posted as patches? Currently there are few modifications which are supposed to go into videodev2.h which have been added to generic header file to make it self-contained. Thanks Regards, Manjunatha. Manjunatha Halli (6): drivers:staging: ti-st: fmdrv common header file drivers:staging: ti-st: fmdrv_v4l2 sources drivers:staging: ti-st: fmdrv_common sources drivers:staging: ti-st: fmdrv_rx sources drivers:staging: ti-st: fmdrv_tx sources drivers:staging: ti-st: Kconfig Makefile change drivers/staging/ti-st/Kconfig| 10 + drivers/staging/ti-st/Makefile |2 + drivers/staging/ti-st/fmdrv.h| 259 drivers/staging/ti-st/fmdrv_common.c | 2141 ++ drivers/staging/ti-st/fmdrv_common.h | 458 drivers/staging/ti-st/fmdrv_rx.c | 979 drivers/staging/ti-st/fmdrv_rx.h | 59 + drivers/staging/ti-st/fmdrv_tx.c | 461 drivers/staging/ti-st/fmdrv_tx.h | 37 + drivers/staging/ti-st/fmdrv_v4l2.c | 757 drivers/staging/ti-st/fmdrv_v4l2.h | 32 + 11 files changed, 5195 insertions(+), 0 deletions(-) create mode 100644 drivers/staging/ti-st/fmdrv.h create mode 100644 drivers/staging/ti-st/fmdrv_common.c create mode 100644 drivers/staging/ti-st/fmdrv_common.h create mode 100644 drivers/staging/ti-st/fmdrv_rx.c create mode 100644 drivers/staging/ti-st/fmdrv_rx.h create mode 100644 drivers/staging/ti-st/fmdrv_tx.c create mode 100644 drivers/staging/ti-st/fmdrv_tx.h create mode 100644 drivers/staging/ti-st/fmdrv_v4l2.c create mode 100644 drivers/staging/ti-st/fmdrv_v4l2.h -- 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 v4 5/6] drivers:staging: ti-st: fmdrv_tx sources
From: Manjunatha Halli manjunatha_ha...@ti.com
This has implementation for FM TX functionality.
It communicates with FM V4l2 module and FM common module.
Signed-off-by: Manjunatha Halli manjunatha_ha...@ti.com
---
drivers/staging/ti-st/fmdrv_tx.c | 461 ++
drivers/staging/ti-st/fmdrv_tx.h | 37 +++
2 files changed, 498 insertions(+), 0 deletions(-)
create mode 100644 drivers/staging/ti-st/fmdrv_tx.c
create mode 100644 drivers/staging/ti-st/fmdrv_tx.h
diff --git a/drivers/staging/ti-st/fmdrv_tx.c b/drivers/staging/ti-st/fmdrv_tx.c
new file mode 100644
index 000..7850d19
--- /dev/null
+++ b/drivers/staging/ti-st/fmdrv_tx.c
@@ -0,0 +1,461 @@
+/*
+ * FM Driver for Connectivity chip of Texas Instruments.
+ * This sub-module of FM driver implements FM TX functionality.
+ *
+ * Copyright (C) 2010 Texas Instruments
+ *
+ * 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., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#include linux/delay.h
+#include fmdrv.h
+#include fmdrv_common.h
+#include fmdrv_tx.h
+
+int fm_tx_set_stereo_mono(struct fmdrv_ops *fmdev, unsigned short mode)
+{
+ unsigned short payload;
+ int ret = 0;
+
+ if (fmdev-curr_fmmode != FM_MODE_TX)
+ return -EPERM;
+
+ if (fmdev-tx_data.aud_mode == mode)
+ return ret;
+
+ pr_debug(stereo mode: %d\n, mode);
+
+ /* Set Stereo/Mono mode */
+ FM_STORE_LE16_TO_BE16(payload, (1 - mode));
+ ret = fmc_send_cmd(fmdev, MONO_SET, payload, sizeof(payload),
+ fmdev-maintask_completion, NULL, NULL);
+ if (ret 0) {
+ pr_err((fmdrv): Failed to set TX stereo/mono mode - %d\n,
+ ret);
+ return ret;
+ }
+
+ fmdev-tx_data.aud_mode = mode;
+
+ return ret;
+}
+
+static int __set_rds_text(struct fmdrv_ops *fmdev, unsigned char *rds_text)
+{
+ unsigned short payload;
+ int ret;
+
+ ret = fmc_send_cmd(fmdev, RDS_DATA_SET, rds_text, strlen(rds_text),
+ fmdev-maintask_completion, NULL, NULL);
+ if (ret 0) {
+ pr_err((fmdrv): Failed to set TX RDS data - %d\n, ret);
+ return ret;
+ }
+
+ /* Scroll mode */
+ FM_STORE_LE16_TO_BE16(payload, (unsigned short)0x1);
+ ret = fmc_send_cmd(fmdev, DISPLAY_MODE_SET, payload, sizeof(payload),
+ fmdev-maintask_completion, NULL, NULL);
+ if (ret 0) {
+ pr_err((fmdrv): Failed to set RDS display mode - %d\n, ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int __set_rds_data_mode(struct fmdrv_ops *fmdev, unsigned char mode)
+{
+ unsigned short payload;
+ int ret;
+
+ /* Setting unique PI TODO: how unique? */
+ FM_STORE_LE16_TO_BE16(payload, (unsigned short)0xcafe);
+ ret = fmc_send_cmd(fmdev, PI_SET, payload, sizeof(payload),
+ fmdev-maintask_completion, NULL, NULL);
+ if (ret 0) {
+ pr_err((fmdrv): Failed to set PI - %d\n, ret);
+ return ret;
+ }
+
+ /* Set decoder id */
+ FM_STORE_LE16_TO_BE16(payload, (unsigned short)0xa);
+ ret = fmc_send_cmd(fmdev, DI_SET, payload, sizeof(payload),
+ fmdev-maintask_completion, NULL, NULL);
+ if (ret 0) {
+ pr_err((fmdrv): Failed to set DI - %d\n, ret);
+ return ret;
+ }
+
+ /* TODO: RDS_MODE_GET? */
+ return 0;
+}
+
+static int __set_rds_len(struct fmdrv_ops *fmdev, unsigned char type,
+ unsigned short len)
+{
+ unsigned short payload;
+ int ret;
+
+ len |= type 8;
+ FM_STORE_LE16_TO_BE16(payload, len);
+ ret = fmc_send_cmd(fmdev, LENGTH_SET, payload, sizeof(payload),
+ fmdev-maintask_completion, NULL, NULL);
+ if (ret 0) {
+ pr_err((fmdrv): Failed to set RDS data length - %d\n, ret);
+ return ret;
+ }
+
+ /* TODO: LENGTH_GET? */
+ return 0;
+}
+
+int fm_tx_set_rds_mode(struct fmdrv_ops *fmdev, unsigned char rds_en_dis)
+{
+ unsigned short payload;
+ int ret;
+ unsigned char rds_text[] = Zoom2\n;
+
+ if (fmdev-curr_fmmode != FM_MODE_TX)
+ return
[PATCH v4 1/6] drivers:staging: ti-st: fmdrv common header file
From: Manjunatha Halli manjunatha_ha...@ti.com
These are common headers used in FM submodules (FM V4L2, FM common,
FM Rx,and FM TX).
Signed-off-by: Manjunatha Halli manjunatha_ha...@ti.com
---
drivers/staging/ti-st/fmdrv.h | 259 +
1 files changed, 259 insertions(+), 0 deletions(-)
create mode 100644 drivers/staging/ti-st/fmdrv.h
diff --git a/drivers/staging/ti-st/fmdrv.h b/drivers/staging/ti-st/fmdrv.h
new file mode 100644
index 000..68ed44c
--- /dev/null
+++ b/drivers/staging/ti-st/fmdrv.h
@@ -0,0 +1,259 @@
+/*
+ * FM Driver for Connectivity chip of Texas Instruments.
+ *
+ * Common header for all FM driver sub-modules.
+ *
+ * Copyright (C) 2009 Texas Instruments
+ *
+ * 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., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#ifndef _FM_DRV_H
+#define _FM_DRV_H
+
+#include linux/skbuff.h
+#include linux/interrupt.h
+#include sound/core.h
+#include sound/initval.h
+#include linux/timer.h
+#include linux/version.h
+
+#define FM_DRV_VERSION0.01
+/* Should match with FM_DRV_VERSION */
+#define FM_DRV_RADIO_VERSION KERNEL_VERSION(0, 0, 1)
+#define FM_DRV_NAME ti_fmdrv
+#define FM_DRV_CARD_SHORT_NAMETI FM Radio
+#define FM_DRV_CARD_LONG_NAME Texas Instruments FM Radio
+
+/* Flag info */
+#define FM_INTTASK_RUNNING0
+#define FM_INTTASK_SCHEDULE_PENDING 1
+#define FM_FIRMWARE_DW_INPROGRESS 2
+#define FM_CORE_READY 3
+#define FM_CORE_TRANSPORT_READY 4
+#define FM_AF_SWITCH_INPROGRESS 5
+#define FM_CORE_TX_XMITING 6
+
+#define FM_TUNE_COMPLETE 0x1
+#define FM_BAND_LIMIT0x2
+
+#define FM_DRV_TX_TIMEOUT (5*HZ) /* 5 seconds */
+#define FM_DRV_RX_SEEK_TIMEOUT (20*HZ) /* 20 seconds */
+
+#define NO_OF_ENTRIES_IN_ARRAY(array) (sizeof(array) / sizeof(array[0]))
+
+enum {
+ FM_MODE_OFF,
+ FM_MODE_TX,
+ FM_MODE_RX,
+ FM_MODE_ENTRY_MAX
+};
+
+#define FM_RX_RDS_INFO_FIELD_MAX 8 /* 4 Group * 2 Bytes */
+
+/* TODO:
+ * move the following CIDs to videodev2.h upon acceptance
+ */
+#define V4L2_CTRL_CLASS_FM_RX 0x009c /* FM Tuner control class */
+/* FM Tuner class control IDs */
+#define V4L2_CID_FM_RX_CLASS_BASE (V4L2_CTRL_CLASS_FM_RX | 0x900)
+#define V4L2_CID_FM_RX_CLASS (V4L2_CTRL_CLASS_FM_RX | 1)
+#define V4L2_CID_RSSI_THRESHOLD(V4L2_CID_FM_RX_CLASS_BASE + 2)
+#define V4L2_CID_TUNE_AF (V4L2_CID_FM_RX_CLASS_BASE + 3)
+enum v4l2_tune_af {
+ V4L2_FM_AF_OFF = 0,
+ V4L2_FM_AF_ON = 1
+};
+#define V4L2_CID_FM_BAND (V4L2_CID_FM_RX_CLASS_BASE + 1)
+enum v4l2_fm_band {
+ V4L2_FM_BAND_OTHER = 0,
+ V4L2_FM_BAND_JAPAN = 1,
+ V4L2_FM_BAND_OIRT = 2
+};
+
+/*
+ * define private CIDs for V4L2
+ */
+#define V4L2_CID_CHANNEL_SPACING (V4L2_CID_PRIVATE_BASE + 0)
+
+/* RX RDS data format */
+struct fm_rdsdata_format {
+ union {
+ struct {
+ unsigned char rdsbuff[FM_RX_RDS_INFO_FIELD_MAX];
+ } groupdatabuff;
+ struct {
+ unsigned short pidata;
+ unsigned char block_b_byte1;
+ unsigned char block_b_byte2;
+ unsigned char block_c_byte1;
+ unsigned char block_c_byte2;
+ unsigned char block_d_byte1;
+ unsigned char block_d_byte2;
+ } groupgeneral;
+ struct {
+ unsigned short pidata;
+ unsigned char block_b_byte1;
+ unsigned char block_b_byte2;
+ unsigned char firstaf;
+ unsigned char secondaf;
+ unsigned char firstpsbyte;
+ unsigned char secondpsbyte;
+ } group0A;
+
+ struct {
+ unsigned short pidata;
+ unsigned char block_b_byte1;
+ unsigned char block_b_byte2;
+ unsigned short pidata2;
+ unsigned char firstpsbyte;
+ unsigned char secondpsbyte;
+ } group0B;
+ } rdsdata;
+};
+
+/* FM region (Europe/US, Japan)
[PATCH v4 4/6] drivers:staging: ti-st: fmdrv_rx sources
From: Manjunatha Halli manjunatha_ha...@ti.com
This has implementation for FM RX functionality.
It communicates with FM V4l2 module and FM common module.
Signed-off-by: Manjunatha Halli manjunatha_ha...@ti.com
---
drivers/staging/ti-st/fmdrv_rx.c | 979 ++
drivers/staging/ti-st/fmdrv_rx.h | 59 +++
2 files changed, 1038 insertions(+), 0 deletions(-)
create mode 100644 drivers/staging/ti-st/fmdrv_rx.c
create mode 100644 drivers/staging/ti-st/fmdrv_rx.h
diff --git a/drivers/staging/ti-st/fmdrv_rx.c b/drivers/staging/ti-st/fmdrv_rx.c
new file mode 100644
index 000..5f802da
--- /dev/null
+++ b/drivers/staging/ti-st/fmdrv_rx.c
@@ -0,0 +1,979 @@
+/*
+ * FM Driver for Connectivity chip of Texas Instruments.
+ * This sub-module of FM driver implements FM RX functionality.
+ *
+ * Copyright (C) 2010 Texas Instruments
+ * Author: Raja Mani raja_m...@ti.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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 fmdrv.h
+#include fmdrv_common.h
+#include fmdrv_rx.h
+
+void fm_rx_reset_rds_cache(struct fmdrv_ops *fmdev)
+{
+ fmdev-rx.rds.flag = FM_RDS_DISABLE;
+ fmdev-rx.rds.last_block_index = 0;
+ fmdev-rx.rds.wr_index = 0;
+ fmdev-rx.rds.rd_index = 0;
+
+ if (fmdev-rx.af_mode == FM_RX_RDS_AF_SWITCH_MODE_ON)
+ fmdev-irq_info.mask |= FM_LEV_EVENT;
+}
+
+void fm_rx_reset_curr_station_info(struct fmdrv_ops *fmdev)
+{
+ fmdev-rx.cur_station_info.picode = FM_NO_PI_CODE;
+ fmdev-rx.cur_station_info.no_of_items_in_afcache = 0;
+ fmdev-rx.cur_station_info.af_list_max = 0;
+}
+
+int fm_rx_set_frequency(struct fmdrv_ops *fmdev, unsigned int freq_to_set)
+{
+ unsigned long timeleft;
+ unsigned short payload, curr_frq, frq_index;
+ unsigned int curr_frq_in_khz;
+ int ret, resp_len;
+
+ if (fmdev-curr_fmmode != FM_MODE_RX)
+ return -EPERM;
+
+ if (freq_to_set fmdev-rx.region.bottom_frequency ||
+ freq_to_set fmdev-rx.region.top_frequency) {
+ pr_err((fmdrv): Invalid frequency %d\n, freq_to_set);
+ return -EINVAL;
+ }
+
+ /* Set audio enable */
+ FM_STORE_LE16_TO_BE16(payload, FM_RX_FM_AUDIO_ENABLE_I2S_AND_ANALOG);
+ ret = fmc_send_cmd(fmdev, AUDIO_ENABLE_SET, payload, sizeof(payload),
+ fmdev-maintask_completion, NULL, NULL);
+ if (ret 0) {
+ pr_err((fmdrv): Failed to set RX audio enable path - %d\n,
+ ret);
+ return ret;
+ }
+
+ /* Set hilo to automatic selection */
+ FM_STORE_LE16_TO_BE16(payload, FM_RX_IFFREQ_HILO_AUTOMATIC);
+ ret = fmc_send_cmd(fmdev, HILO_SET, payload, sizeof(payload),
+ fmdev-maintask_completion, NULL, NULL);
+ if (ret 0) {
+ pr_err((fmdrv): Failed to set HILO to automatic selection
+ - %d\n, ret);
+ return ret;
+ }
+
+ /* Calculate frequency index to write */
+ frq_index = (freq_to_set - fmdev-rx.region.bottom_frequency) /
+ FM_FREQ_MUL;
+
+ /* Set frequency index */
+ FM_STORE_LE16_TO_BE16(payload, frq_index);
+ ret = fmc_send_cmd(fmdev, FREQ_SET, payload, sizeof(payload),
+ fmdev-maintask_completion, NULL, NULL);
+ if (ret 0) {
+ pr_err((fmdrv): Failed to set frequency index - %d\n, ret);
+ return ret;
+ }
+
+ /* Read flags - just to clear any pending interrupts if we had */
+ ret = fmc_send_cmd(fmdev, FLAG_GET, NULL, 2,
+ fmdev-maintask_completion, NULL, NULL);
+ if (ret 0) {
+ pr_err((fmdrv): Failed to read interrupt flag - %d\n, ret);
+ return ret;
+ }
+
+ /* Enable FR, BL interrupts */
+ fmdev-irq_info.mask |= (FM_FR_EVENT | FM_BL_EVENT);
+ FM_STORE_LE16_TO_BE16(payload, fmdev-irq_info.mask);
+ ret = fmc_send_cmd(fmdev, INT_MASK_SET, payload, sizeof(payload),
+ fmdev-maintask_completion, NULL, NULL);
+ if (ret 0) {
+ pr_err((fmdrv): Failed to set interrupt mask - %d\n, ret);
+ return ret;
+ }
+
+ /* Start tune */
+
[PATCH v4 2/6] drivers:staging: ti-st: fmdrv_v4l2 sources
From: Manjunatha Halli manjunatha_ha...@ti.com
This module interfaces V4L2 subsystem and FM common
module. It registers itself with V4L2 as Radio module.
Signed-off-by: Manjunatha Halli manjunatha_ha...@ti.com
---
drivers/staging/ti-st/fmdrv_v4l2.c | 757
drivers/staging/ti-st/fmdrv_v4l2.h | 32 ++
2 files changed, 789 insertions(+), 0 deletions(-)
create mode 100644 drivers/staging/ti-st/fmdrv_v4l2.c
create mode 100644 drivers/staging/ti-st/fmdrv_v4l2.h
diff --git a/drivers/staging/ti-st/fmdrv_v4l2.c
b/drivers/staging/ti-st/fmdrv_v4l2.c
new file mode 100644
index 000..687d10f
--- /dev/null
+++ b/drivers/staging/ti-st/fmdrv_v4l2.c
@@ -0,0 +1,757 @@
+/*
+ * FM Driver for Connectivity chip of Texas Instruments.
+ * This file provides interfaces to V4L2 subsystem.
+ *
+ * This module registers with V4L2 subsystem as Radio
+ * data system interface (/dev/radio). During the registration,
+ * it will expose two set of function pointers.
+ *
+ *1) File operation related API (open, close, read, write, poll...etc).
+ *2) Set of V4L2 IOCTL complaint API.
+ *
+ * Copyright (C) 2010 Texas Instruments
+ * Author: Raja Mani raja_m...@ti.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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 fmdrv.h
+#include fmdrv_v4l2.h
+#include fmdrv_common.h
+#include fmdrv_rx.h
+#include fmdrv_tx.h
+
+static struct video_device *gradio_dev;
+static unsigned char radio_disconnected;
+
+/* Query control */
+static struct v4l2_queryctrl fmdrv_v4l2_queryctrl[] = {
+ {
+.id = V4L2_CID_AUDIO_VOLUME,
+.type = V4L2_CTRL_TYPE_INTEGER,
+.name = Volume,
+.minimum = FM_RX_VOLUME_MIN,
+.maximum = FM_RX_VOLUME_MAX,
+.step = 1,
+.default_value = FM_DEFAULT_RX_VOLUME,
+},
+ {
+.id = V4L2_CID_AUDIO_BALANCE,
+.flags = V4L2_CTRL_FLAG_DISABLED,
+},
+ {
+.id = V4L2_CID_AUDIO_BASS,
+.flags = V4L2_CTRL_FLAG_DISABLED,
+},
+ {
+.id = V4L2_CID_AUDIO_TREBLE,
+.flags = V4L2_CTRL_FLAG_DISABLED,
+},
+ {
+.id = V4L2_CID_AUDIO_MUTE,
+.type = V4L2_CTRL_TYPE_BOOLEAN,
+.name = Mute,
+.minimum = 0,
+.maximum = 2,
+.step = 1,
+.default_value = FM_MUTE_OFF,
+},
+ {
+.id = V4L2_CID_AUDIO_LOUDNESS,
+.flags = V4L2_CTRL_FLAG_DISABLED,
+},
+};
+
+/* -- V4L2 RADIO (/dev/radioX) device file operation interfaces --- */
+
+/* Read RX RDS data */
+static ssize_t fm_v4l2_fops_read(struct file *file, char __user * buf,
+ size_t count, loff_t *ppos)
+{
+ unsigned char rds_mode;
+ int ret;
+ struct fmdrv_ops *fmdev;
+
+ fmdev = video_drvdata(file);
+
+ if (!radio_disconnected) {
+ pr_err((fmdrv): FM device is already disconnected\n);
+ return -EIO;
+ }
+
+ /* Turn on RDS mode , if it is disabled */
+ ret = fm_rx_get_rds_mode(fmdev, rds_mode);
+ if (ret 0) {
+ pr_err((fmdrv): Unable to read current rds mode\n);
+ return ret;
+ }
+
+ if (rds_mode == FM_RDS_DISABLE) {
+ ret = fmc_set_rds_mode(fmdev, FM_RDS_ENABLE);
+ if (ret 0) {
+ pr_err((fmdrv): Failed to enable rds mode\n);
+ return ret;
+ }
+ }
+
+ /* Copy RDS data from internal buffer to user buffer */
+ ret = fmc_transfer_rds_from_internal_buff(fmdev, file, buf, count);
+
+ return ret;
+}
+
+/* Write TX RDS data */
+static ssize_t fm_v4l2_fops_write(struct file *file, const char __user * buf,
+ size_t count, loff_t *ppos)
+{
+ struct tx_rds rds;
+ int ret;
+ struct fmdrv_ops *fmdev;
+
+ ret = copy_from_user(rds, buf, sizeof(rds));
+ pr_debug((fmdrv): (%d)type: %d, text %s, af %d\n,
+ ret, rds.text_type, rds.text, rds.af_freq);
+
+ fmdev = video_drvdata(file);
+ fm_tx_set_radio_text(fmdev, rds.text, rds.text_type);
+ fm_tx_set_af(fmdev, rds.af_freq);
+
+ return 0;
+}
+
+static unsigned int fm_v4l2_fops_poll(struct file *file,
+ struct poll_table_struct *pts)
+{
+
Re: [RFC PATCH 0/8] V4L BKL removal: first round
Em 16-11-2010 10:35, Hans Verkuil escreveu: Em 15-11-2010 07:49, Hans Verkuil escreveu: On Sunday 14 November 2010 23:48:51 Hans Verkuil wrote: On Sunday, November 14, 2010 22:53:29 Arnd Bergmann wrote: On Sunday 14 November 2010, Hans Verkuil wrote: This patch series converts 24 v4l drivers to unlocked_ioctl. These are low hanging fruit but you have to start somewhere :-) The first patch replaces mutex_lock in the V4L2 core by mutex_lock_interruptible for most fops. The patches all look good as far as I can tell, but I suppose the title is obsolete now that the BKL has been replaced with a v4l-wide mutex, which is what you are removing in the series. I guess I have to rename it, even though strictly speaking the branch I'm working in doesn't have your patch merged yet. BTW, replacing the BKL with a static mutex is rather scary: the BKL gives up the lock whenever you sleep, the mutex doesn't. Since sleeping is very common in V4L (calling VIDIOC_DQBUF will typically sleep while waiting for a new frame to arrive), this will make it impossible for another process to access any v4l2 device node while the ioctl is sleeping. I am not sure whether that is what you intended. Or am I missing something? I was aware that something like this could happen, but I apparently misjudged how big the impact is. The general pattern for ioctls is that those that get called frequently do not sleep, so it can almost always be called with a mutex held. True in general, but most definitely not true for V4L. The all important VIDIOC_DQBUF ioctl will almost always sleep. Mauro, I think this patch will have to be reverted and we just have to do the hard work ourselves. The VIDIOC_QBUF/VIDIOC_DQBUF ioctls are called after having the V4L device ready for stream. During the qbuf/dqbuf loop, the only other ioctls that may appear are the control change ioctl's, to adjust things like bright. I doubt that his will cause a really serious trouble. Yes, it does. Anyone who is using multiple capture/output devices at the same time will be affected. One correction to your comment: Anyone that uses multiple capture/output devices that were not converted to unlocked ioctl will be affected. This means that devices with multiple entries need to be fixed first. For example, anyone who uses the davinci dm6467 driver for both input and output. And yes, that's what we use at work. And ship to thousands of customers. Or think about surveillance applications where you are capturing from many streams simultaneously. 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 PATCH 0/8] V4L BKL removal: first round
Em 16-11-2010 10:35, Hans Verkuil escreveu: Em 15-11-2010 07:49, Hans Verkuil escreveu: On Sunday 14 November 2010 23:48:51 Hans Verkuil wrote: On Sunday, November 14, 2010 22:53:29 Arnd Bergmann wrote: On Sunday 14 November 2010, Hans Verkuil wrote: This patch series converts 24 v4l drivers to unlocked_ioctl. These are low hanging fruit but you have to start somewhere :-) The first patch replaces mutex_lock in the V4L2 core by mutex_lock_interruptible for most fops. The patches all look good as far as I can tell, but I suppose the title is obsolete now that the BKL has been replaced with a v4l-wide mutex, which is what you are removing in the series. I guess I have to rename it, even though strictly speaking the branch I'm working in doesn't have your patch merged yet. BTW, replacing the BKL with a static mutex is rather scary: the BKL gives up the lock whenever you sleep, the mutex doesn't. Since sleeping is very common in V4L (calling VIDIOC_DQBUF will typically sleep while waiting for a new frame to arrive), this will make it impossible for another process to access any v4l2 device node while the ioctl is sleeping. I am not sure whether that is what you intended. Or am I missing something? I was aware that something like this could happen, but I apparently misjudged how big the impact is. The general pattern for ioctls is that those that get called frequently do not sleep, so it can almost always be called with a mutex held. True in general, but most definitely not true for V4L. The all important VIDIOC_DQBUF ioctl will almost always sleep. Mauro, I think this patch will have to be reverted and we just have to do the hard work ourselves. The VIDIOC_QBUF/VIDIOC_DQBUF ioctls are called after having the V4L device ready for stream. During the qbuf/dqbuf loop, the only other ioctls that may appear are the control change ioctl's, to adjust things like bright. I doubt that his will cause a really serious trouble. Yes, it does. Anyone who is using multiple capture/output devices at the same time will be affected. One correction to your comment: Anyone that uses multiple capture/output devices that were not converted to unlocked ioctl will be affected. This means that devices with multiple entries need to be fixed first. No, it will also affect e.g. two bttv cards that you capture from in parallel. Or two webcams, or... We can't just ditch the BKL yet for 2.6.37 IMHO. Perhaps for 2.6.38 if we all work really hard to convert everything. I would prefer to have those drivers that depend on the BKL to have a dependency on CONFIG_LOCK_KERNEL. Drivers should either work or not be available at all, rather than working poorly (if at all). Regards, Hans -- Hans Verkuil - video4linux developer - sponsored by Cisco -- 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
[GIT PULL REQUEST v2.] WL1273 FM Radio driver.
Hi. The radio pull request, now using http protocol. The following changes since commit f6614b7bb405a9b35dd28baea989a749492c46b2: Linus Torvalds (1): Merge git://git.kernel.org/.../sfrench/cifs-2.6 are available in the git repository at: http://git.gitorious.org/wl1273-fm-driver/wl1273-fm-driver.git master Matti J. Aaltonen (2): MFD: WL1273 FM Radio: MFD driver for the FM radio. V4L2: WL1273 FM Radio: Controls for the FM radio. drivers/media/radio/Kconfig| 16 + drivers/media/radio/Makefile |1 + drivers/media/radio/radio-wl1273.c | 1841 drivers/mfd/Kconfig|6 + drivers/mfd/Makefile |1 + drivers/mfd/wl1273-core.c | 617 include/linux/mfd/wl1273-core.h| 330 +++ 7 files changed, 2812 insertions(+), 0 deletions(-) create mode 100644 drivers/media/radio/radio-wl1273.c create mode 100644 drivers/mfd/wl1273-core.c create mode 100644 include/linux/mfd/wl1273-core.h B.R. Matti A. -- 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: [omap3isp][PATCH v2 0/9] YUV support for CCDC + cleanups
-Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Tuesday, November 16, 2010 5:45 AM To: Aguirre, Sergio Cc: linux-media@vger.kernel.org Subject: Re: [omap3isp][PATCH v2 0/9] YUV support for CCDC + cleanups Hi Sergio, Hi Laurent, Thanks for the patches. You're welcome. I've reviewed and tested them and found no issue. They will be a small delay as I need to sync our internal tree with the public tree before applying them. Not a problem. Thanks for your time. Regards, Sergio -- Regards, Laurent Pinchart -- 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 PATCH 0/8] V4L BKL removal: first round
On Tuesday 16 November 2010, Hans Verkuil wrote:
No, it will also affect e.g. two bttv cards that you capture from in
parallel. Or two webcams, or...
Would it be safe to turn the global mutex into a per-driver or per-device
mutex? That would largely mitigate the impact as far as I can tell.
What would work better is to add a mutex to struct v4l2_device (which is
the top-level struct for v4l devices) and have the core lock that.
A pointer to this struct is available in vdev-v4l2_dev. However, not all
drivers implement struct v4l2_device. But on the other hand, most relevant
drivers do. So as a fallback we would still need a static mutex.
What would be best is to add an #ifdef CONFIG_LOCK_KERNEL and use the BKL
there. In the #else we can use a v4l2_device mutex, or (if not set) a
static mutex.
Hardly elegant, but it'll have to do.
We can't just ditch the BKL yet for 2.6.37 IMHO. Perhaps for 2.6.38 if
we
all work really hard to convert everything.
Linus was pretty clear in that he wanted to make the default for the BKL
disabled for 2.6.37. That may of course change if there are significant
problems with this, but as long as there is an easier way, we could do
that instead.
I have not tested the patch below, but maybe that would solve the
immediate problem without reverting v4l2-dev back to use the BKL.
It would not work if we have drivers that need to serialize access
to multiple v4l2 devices in their ioctl functions because they access
global data, which is unlikely but possible.
It's very likely, I'm afraid :-(
Regards,
Hans
Signed-off-by: Arnd Bergmann a...@arndb.de
diff --git a/drivers/media/video/v4l2-dev.c
b/drivers/media/video/v4l2-dev.c
index 03f7f46..5873d12 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -246,12 +246,11 @@ static long v4l2_ioctl(struct file *filp, unsigned
int cmd, unsigned long arg)
mutex_unlock(vdev-lock);
} else if (vdev-fops-ioctl) {
/* TODO: convert all drivers to unlocked_ioctl */
- static DEFINE_MUTEX(v4l2_ioctl_mutex);
-
- mutex_lock(v4l2_ioctl_mutex);
- if (video_is_registered(vdev))
+ if (video_is_registered(vdev)) {
+ mutex_lock(vdev-ioctl_lock);
ret = vdev-fops-ioctl(filp, cmd, arg);
- mutex_unlock(v4l2_ioctl_mutex);
+ mutex_unlock(vdev-ioctl_lock);
+ }
} else
ret = -ENOTTY;
@@ -507,6 +506,7 @@ static int __video_register_device(struct video_device
*vdev, int type, int nr,
#endif
vdev-minor = i + minor_offset;
vdev-num = nr;
+ mutex_init(vdev-ioctl_lock);
devnode_set(vdev);
/* Should not happen since we thought this minor was free */
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index 15802a0..e8a8485 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -97,6 +97,9 @@ struct video_device
/* serialization lock */
struct mutex *lock;
+
+ /* used for the legacy locked ioctl */
+ struct mutex ioctl_lock;
};
/* dev to video-device */
--
Hans Verkuil - video4linux developer - sponsored by Cisco
--
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 PATCH 0/8] V4L BKL removal: first round
On Tuesday 16 November 2010, Hans Verkuil wrote: A pointer to this struct is available in vdev-v4l2_dev. However, not all drivers implement struct v4l2_device. But on the other hand, most relevant drivers do. So as a fallback we would still need a static mutex. Wouldn't that suffer the same problem as putting the mutex into videodev as I suggested? You said that there are probably drivers that need to serialize between multiple devices, so if we have a mutex per v4l2_device, you can still get races between multiple ioctl calls accessing the same per-driver data. To solve this, we'd have to put the lock into a per-driver structure like v4l2_file_operations or v4l2_ioctl_ops, which would add to the ugliness. Arnd -- 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 PATCH 0/8] V4L BKL removal: first round
On Tuesday 16 November 2010, Hans Verkuil wrote: A pointer to this struct is available in vdev-v4l2_dev. However, not all drivers implement struct v4l2_device. But on the other hand, most relevant drivers do. So as a fallback we would still need a static mutex. Wouldn't that suffer the same problem as putting the mutex into videodev as I suggested? You said that there are probably drivers that need to serialize between multiple devices, so if we have a mutex per v4l2_device, you can still get races between multiple ioctl calls accessing the same per-driver data. To solve this, we'd have to put the lock into a per-driver structure like v4l2_file_operations or v4l2_ioctl_ops, which would add to the ugliness. I think there is a misunderstanding. One V4L device (e.g. a TV capture card, a webcam, etc.) has one v4l2_device struct. But it can have multiple V4L device nodes (/dev/video0, /dev/radio0, etc.), each represented by a struct video_device (and I really hope I can rename that to v4l2_devnode soon since that's a very confusing name). You typically need to serialize between all the device nodes belonging to the same video hardware. A mutex in struct video_device doesn't do that, that just serializes access to that single device node. But a mutex in v4l2_device is at the right level. Hans Arnd -- Hans Verkuil - video4linux developer - sponsored by Cisco -- 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 01/10] MCDE: Add hardware abstraction layer
Hi,
Thank you Arnd for your comments.
A hardware abstraction layer is generally considered a bad thing,
you're usually better off not advertising your code as being one.
As a rule, the device driver *is* the hardware abstraction, so you
should not add another one ;-)
+static void disable_channel(struct mcde_chnl_state *chnl);
+static void enable_channel(struct mcde_chnl_state *chnl);
+static void watchdog_auto_sync_timer_function(unsigned long arg);
I generally recomment avoiding forward declarations of static
functions.
Just reorder the code so you don't need them.
Good point, will do that in the next patch
+u8 *mcdeio;
+u8 **dsiio;
+DEFINE_SPINLOCK(mcde_lock); /* REVIEW: Remove or use */
+struct platform_device *mcde_dev;
+u8 num_dsilinks;
You should try hard to avoid global variables in a well-designed
driver.
There are many ways around them, like accessor functions or splitting
the
driver into files in a more logical way where each file only accesses
its own data. If you really cannot think of a way to avoid these,
put them in a proper name space in the way that you have done for the
global functions, by prefixing each identifier with mcde_.
+static u8 hardware_version;
+
+static struct regulator *regulator;
+static struct clk *clock_dsi;
+static struct clk *clock_mcde;
+static struct clk *clock_dsi_lp;
+static u8 mcde_is_enabled;
Even static variables like these can cause problems. Ideally all of
these
are referenced through a driver private data structure that is passed
around
with the device. This way you can trivially support multiple devices if
that ever becomes necessary.
What is the general opinion about singleton drivers?
Both global and static variables could be fixed if the driver is redesigned to
support multiple devices.
+static inline u32 dsi_rreg(int i, u32 reg)
+{
+ return readl(dsiio[i] + reg);
+}
+static inline void dsi_wreg(int i, u32 reg, u32 val)
+{
+ writel(val, dsiio[i] + reg);
+}
dsiio is not marked __iomem, so there is something wrong here.
Moreover, why do you need two indexes? If you have multiple identical
dsiio structures, maybe each of them should just be a device by
itself?
We will add __iomem.
Each dsi link (dsiio[x]) is tightly coupled with mcde and it feels that they
should not be a device of their own.
We feel that it would be to many devices doing little.
+struct mcde_ovly_state {
+ bool inuse;
+ u8 idx; /* MCDE overlay index */
+ struct mcde_chnl_state *chnl; /* Owner channel */
+ u32 transactionid; /* Apply time stamp */
+ u32 transactionid_regs; /* Register update time stamp */
+ u32 transactionid_hw; /* HW completed time stamp */
+ wait_queue_head_t waitq_hw; /* Waitq for transactionid_hw */
+
+ /* Staged settings */
+ u32 paddr;
+ u16 stride;
+ enum mcde_ovly_pix_fmt pix_fmt;
+
+ u16 src_x;
+ u16 src_y;
+ u16 dst_x;
+ u16 dst_y;
+ u16 dst_z;
+ u16 w;
+ u16 h;
+
+ /* Applied settings */
+ struct ovly_regs regs;
+};
There should probably be a struct device pointer in this, so you can
pass
it around as a real object.
We will look into this in the context of the singleton driver redesign.
+ /* Handle channel irqs */
+ irq_status = mcde_rreg(MCDE_RISPP);
+ if (irq_status MCDE_RISPP_VCMPARIS_MASK) {
+ chnl = channels[MCDE_CHNL_A];
...
+ }
+ if (irq_status MCDE_RISPP_VCMPBRIS_MASK) {
+ chnl = channels[MCDE_CHNL_B];
...
+ }
+ if (irq_status MCDE_RISPP_VCMPC0RIS_MASK) {
+ chnl = channels[MCDE_CHNL_C0];
...
+ }
This looks a bit like you actually have multiple interrupt lines
multiplexed
through a private interrupt controller. Have you considered making this
controller
a separate device to multiplex the interrupt numbers?
MCDE contains several pipelines, each of them can generate interrupts.
Since each interrupt comes from the same device there is no need for separate
devices for interrupt controller.
+void wait_for_overlay(struct mcde_ovly_state *ovly)
Not an appropriate name for a global function. Either make this static
or
call it mcde_wait_for_overlay. Same for some other functions.
It is only used in here so it will be static.
+#ifdef CONFIG_AV8100_SDTV
+ /* TODO: check if these watermark levels work for HDMI as
well. */
+ pixelfetchwtrmrklevel = MCDE_PIXFETCH_SMALL_WTRMRKLVL;
+#else
+ if ((fifo == MCDE_FIFO_A || fifo == MCDE_FIFO_B)
+ regs-ppl =
fifo_size * 2)
+ pixelfetchwtrmrklevel =
MCDE_PIXFETCH_LARGE_WTRMRKLVL;
+ else
+ pixelfetchwtrmrklevel =
MCDE_PIXFETCH_MEDIUM_WTRMRKLVL;
+#endif /* CONFIG_AV8100_SDTV */
Be careful with config options like this. If you want to build a kernel
to run on all machines, the first part probably needs to check where it
is running and consider the
Re: [RFC PATCH 0/8] V4L BKL removal: first round
On Tuesday 16 November 2010, Hans Verkuil wrote: A pointer to this struct is available in vdev-v4l2_dev. However, not all drivers implement struct v4l2_device. But on the other hand, most relevant drivers do. So as a fallback we would still need a static mutex. Wouldn't that suffer the same problem as putting the mutex into videodev as I suggested? You said that there are probably drivers that need to serialize between multiple devices, so if we have a mutex per v4l2_device, you can still get races between multiple ioctl calls accessing the same per-driver data. To solve this, we'd have to put the lock into a per-driver structure like v4l2_file_operations or v4l2_ioctl_ops, which would add to the ugliness. I think there is a misunderstanding. One V4L device (e.g. a TV capture card, a webcam, etc.) has one v4l2_device struct. But it can have multiple V4L device nodes (/dev/video0, /dev/radio0, etc.), each represented by a struct video_device (and I really hope I can rename that to v4l2_devnode soon since that's a very confusing name). You typically need to serialize between all the device nodes belonging to the same video hardware. A mutex in struct video_device doesn't do that, that just serializes access to that single device node. But a mutex in v4l2_device is at the right level. A quick follow-up as I saw I didn't fully answer your question: to my knowledge there are no per-driver data structures that need a BKL for protection. It's definitely not something I am worried about. Regards, Hans -- Hans Verkuil - video4linux developer - sponsored by Cisco -- 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 PATCH 0/8] V4L BKL removal: first round
On Tuesday 16 November 2010, Hans Verkuil wrote: I think there is a misunderstanding. One V4L device (e.g. a TV capture card, a webcam, etc.) has one v4l2_device struct. But it can have multiple V4L device nodes (/dev/video0, /dev/radio0, etc.), each represented by a struct video_device (and I really hope I can rename that to v4l2_devnode soon since that's a very confusing name). You typically need to serialize between all the device nodes belonging to the same video hardware. A mutex in struct video_device doesn't do that, that just serializes access to that single device node. But a mutex in v4l2_device is at the right level. Ok, got it now. A quick follow-up as I saw I didn't fully answer your question: to my knowledge there are no per-driver data structures that need a BKL for protection. It's definitely not something I am worried about. Good. Are you preparing a patch for a per-v4l2_device then? This sounds like the right place with your explanation. I would not put in the CONFIG_BKL switch, because I tried that for two other subsystems and got called back, but I'm not going to stop you. As for the fallback to a global mutex, I guess you can set the videodev-lock pointer and use unlocked_ioctl for those drivers that do not use a v4l2_device yet, if there are only a handful of them. Arnd -- 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
[GIT PATCHES FOR 2.6.38] Fixes for driver pwc
Hi, While investigating the following bug: https://bugzilla.redhat.com/show_bug.cgi?id=624103 I found several errors in the handling of isoc transfers in the pwc driver. The fixes in this pull request fix these. Even with this fixed, the pwc driver is still far from ideal in various places, esp. plug / unplug handling. If I feel like it I may rewrite it as a gspca sub driver in the near future. The following changes since commit 552faf8580766b6fc944cb966f690ed0624a5564: [media] mfd: Add timberdale video-in driver to timberdale (2010-11-16 12:06:58 -0200) are available in the git repository at: git://linuxtv.org/hgoede/gspca.git gspca-for_v2.6.38 Hans de Goede (3): pwc: do not start isoc stream on /dev/video open pwc: Also set alt setting to alt0 when no error occured pwc: failure to submit an urb is a fatal error drivers/media/video/pwc/pwc-ctrl.c |7 +++- drivers/media/video/pwc/pwc-if.c | 65 +++ drivers/media/video/pwc/pwc-v4l.c | 13 --- drivers/media/video/pwc/pwc.h |1 - 4 files changed, 34 insertions(+), 52 deletions(-) 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: [PATCH 01/10] MCDE: Add hardware abstraction layer
On Tuesday 16 November 2010, Jimmy RUBIN wrote:
Even static variables like these can cause problems. Ideally all of
these
are referenced through a driver private data structure that is passed
around
with the device. This way you can trivially support multiple devices if
that ever becomes necessary.
What is the general opinion about singleton drivers?
Both global and static variables could be fixed if the driver is redesigned
to support multiple devices.
I don't know if there is a general rule. The reason why I don't like to have
device specific data spread across global variables is that it messes up
my mental model of the code.
Every device in Linux normally is set up by a bus probe (or as a hack,
a platform device instance) and given to a device driver, which then
allocates a private data structure that describes what the driver but
not the bus knows about this device. That data structure typically also
contains the locks for all in-memory and physical state of the device.
If you deviate from this model, you make it harder for reviewers and
other developers to understand what is going on.
+static inline u32 dsi_rreg(int i, u32 reg)
+{
+ return readl(dsiio[i] + reg);
+}
+static inline void dsi_wreg(int i, u32 reg, u32 val)
+{
+ writel(val, dsiio[i] + reg);
+}
dsiio is not marked __iomem, so there is something wrong here.
Moreover, why do you need two indexes? If you have multiple identical
dsiio structures, maybe each of them should just be a device by
itself?
We will add __iomem.
Each dsi link (dsiio[x]) is tightly coupled with mcde and it feels that they
should not be a device of their own.
We feel that it would be to many devices doing little.
Ok.
This looks a bit like you actually have multiple interrupt lines
multiplexed
through a private interrupt controller. Have you considered making this
controller
a separate device to multiplex the interrupt numbers?
MCDE contains several pipelines, each of them can generate interrupts.
Since each interrupt comes from the same device there is no need for
separate devices for interrupt controller.
Right, so this one and the one above is really a question of how to describe
a pipeline
Arnd
--
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 01/10] MCDE: Add hardware abstraction layer
sent out too early... On Tuesday 16 November 2010, Arnd Bergmann wrote: This looks a bit like you actually have multiple interrupt lines multiplexed through a private interrupt controller. Have you considered making this controller a separate device to multiplex the interrupt numbers? MCDE contains several pipelines, each of them can generate interrupts. Since each interrupt comes from the same device there is no need for separate devices for interrupt controller. Right, so this one and the one above is really a question of how to describe a pipeline: It may be good to have a source file that only deals with the pipelines and all that they have in common. If you use the same basic pipeline logic for doing multiple different things, this can be used to structure the code more logically. Not sure if this is worth trying, since it might not actually gain all that much in the end Arnd -- 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 PATCH 0/8] V4L BKL removal: first round
Em 16-11-2010 14:01, Arnd Bergmann escreveu: On Tuesday 16 November 2010, Hans Verkuil wrote: I think there is a misunderstanding. One V4L device (e.g. a TV capture card, a webcam, etc.) has one v4l2_device struct. But it can have multiple V4L device nodes (/dev/video0, /dev/radio0, etc.), each represented by a struct video_device (and I really hope I can rename that to v4l2_devnode soon since that's a very confusing name). You typically need to serialize between all the device nodes belonging to the same video hardware. A mutex in struct video_device doesn't do that, that just serializes access to that single device node. But a mutex in v4l2_device is at the right level. Ok, got it now. A quick follow-up as I saw I didn't fully answer your question: to my knowledge there are no per-driver data structures that need a BKL for protection. It's definitely not something I am worried about. Good. Are you preparing a patch for a per-v4l2_device then? This sounds like the right place with your explanation. I would not put in the CONFIG_BKL switch, because I tried that for two other subsystems and got called back, but I'm not going to stop you. As for the fallback to a global mutex, I guess you can set the videodev-lock pointer and use unlocked_ioctl for those drivers that do not use a v4l2_device yet, if there are only a handful of them. Hans, FYI, Linus already pull Arnd patch that removed BKL on -rc2 (commit 0edf2e5e2bd0ae7689ce8a57ae3c87cc1f0c6548). I've pulled back from -rc2 into linux-media.git tree. So, the patch is now visible on our main tree. 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: [PATCH 1/5] kconfig: add an option to determine a menu's visibility
Em 15-11-2010 14:57, Arnaud Lacombe escreveu: Hi all On Sat, Nov 6, 2010 at 5:30 PM, Arnaud Lacombe lacom...@gmail.com wrote: This option is aimed to add the possibility to control a menu's visibility without adding dependency to the expression to all the submenu. Signed-off-by: Arnaud Lacombe lacom...@gmail.com --- scripts/kconfig/expr.h |1 + scripts/kconfig/lkc.h |1 + scripts/kconfig/menu.c | 11 +++ scripts/kconfig/zconf.gperf |1 + scripts/kconfig/zconf.y | 21 ++--- 5 files changed, 32 insertions(+), 3 deletions(-) Michal, I don't think you commented on this ? Mauro, has it been worked around differently ? Those patches worked fine, and solved all problems we had (I just had to touch on two other menus that are used, as I answered upstream). I prefer if Michal could forward those patches upstream, so, there's my ack: Acked-by: Mauro Carvalho Chehab mche...@redhat.com Tested-by: Mauro Carvalho Chehab mche...@redhat.com 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
[cron job] v4l-dvb daily build: WARNINGS
This message is generated daily by a cron job that builds v4l-dvb for the kernels and architectures in the list below. Results of the daily build of v4l-dvb: date:Tue Nov 16 19:00:17 CET 2010 path:http://www.linuxtv.org/hg/v4l-dvb changeset: 15167:abd3aac6644e git master: 3e6dce76d99b328716b43929b9195adfee1de00c git media-master: a348e9110ddb5d494e060d989b35dd1f35359d58 gcc version: i686-linux-gcc (GCC) 4.5.1 host hardware:x86_64 host os: 2.6.32.5 linux-git-armv5: WARNINGS linux-git-armv5-davinci: WARNINGS linux-git-armv5-ixp: WARNINGS linux-git-armv5-omap2: WARNINGS linux-git-i686: WARNINGS linux-git-m32r: WARNINGS linux-git-mips: WARNINGS linux-git-powerpc64: WARNINGS linux-git-x86_64: WARNINGS spec-git: OK sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2 The V4L-DVB specification from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- 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
Fwd: [PATH] Fix rc-tbs-nec table after converting the cx88 driver to ir-core
Hi Dmitry,
This patch fixes an IR table. The patch is trivial, but there are two buttons on
this IR that are not directly supported currently (buttons 10- and 10+). In a
matter
of fact, some other IR's use other key codes (KEY_DOT, KEY_KPPLUS, KEY_DIGITS)
for a similar keyboard key (100+).
Mariusz is proposing the addition of two new keycodes for it. What do you think?
Cheers,
Mauro.
Mensagem original
Assunto: [PATH] Fix rc-tbs-nec table after converting the cx88 driver to ir-core
Data: Mon, 15 Nov 2010 19:50:13 +0100
De: Mariusz Białończyk ma...@skyboo.net
Para: Mauro Carvalho Chehab mche...@redhat.com
CC: linux-media@vger.kernel.org
The patch fixes the rc-tbs-nec table after converting
drivers/media/video/cx88 to ir-core
(commit ba7e90c9f878e0ac3c0614a5446fe5c62ccc33ec).
It is also adds two missing buttons (10- and 10+) with
its definition (KEY_10CHANNELSUP and KEY_10CHANNELSDOWN).
Signed-off-by: Mariusz Białończyk ma...@skyboo.net
---
drivers/media/rc/keymaps/rc-tbs-nec.c | 66 +
include/linux/input.h |2 +
2 files changed, 36 insertions(+), 32 deletions(-)
diff --git a/drivers/media/rc/keymaps/rc-tbs-nec.c
b/drivers/media/rc/keymaps/rc-tbs-nec.c
index 3309631..9a1d9a3 100644
--- a/drivers/media/rc/keymaps/rc-tbs-nec.c
+++ b/drivers/media/rc/keymaps/rc-tbs-nec.c
@@ -13,38 +13,40 @@
#include media/rc-map.h
static struct ir_scancode tbs_nec[] = {
- { 0x04, KEY_POWER2},/*power*/
- { 0x14, KEY_MUTE}, /*mute*/
- { 0x07, KEY_1},
- { 0x06, KEY_2},
- { 0x05, KEY_3},
- { 0x0b, KEY_4},
- { 0x0a, KEY_5},
- { 0x09, KEY_6},
- { 0x0f, KEY_7},
- { 0x0e, KEY_8},
- { 0x0d, KEY_9},
- { 0x12, KEY_0},
- { 0x16, KEY_CHANNELUP}, /*ch+*/
- { 0x11, KEY_CHANNELDOWN},/*ch-*/
- { 0x13, KEY_VOLUMEUP}, /*vol+*/
- { 0x0c, KEY_VOLUMEDOWN},/*vol-*/
- { 0x03, KEY_RECORD},/*rec*/
- { 0x18, KEY_PAUSE}, /*pause*/
- { 0x19, KEY_OK},/*ok*/
- { 0x1a, KEY_CAMERA},/* snapshot */
- { 0x01, KEY_UP},
- { 0x10, KEY_LEFT},
- { 0x02, KEY_RIGHT},
- { 0x08, KEY_DOWN},
- { 0x15, KEY_FAVORITES},
- { 0x17, KEY_SUBTITLE},
- { 0x1d, KEY_ZOOM},
- { 0x1f, KEY_EXIT},
- { 0x1e, KEY_MENU},
- { 0x1c, KEY_EPG},
- { 0x00, KEY_PREVIOUS},
- { 0x1b, KEY_MODE},
+ { 0x84, KEY_POWER2},/* power */
+ { 0x94, KEY_MUTE}, /* mute */
+ { 0x87, KEY_1},
+ { 0x86, KEY_2},
+ { 0x85, KEY_3},
+ { 0x8b, KEY_4},
+ { 0x8a, KEY_5},
+ { 0x89, KEY_6},
+ { 0x8f, KEY_7},
+ { 0x8e, KEY_8},
+ { 0x8d, KEY_9},
+ { 0x92, KEY_0},
+ { 0xc0, KEY_10CHANNELSUP}, /* 10+ */
+ { 0xd0, KEY_10CHANNELSDOWN},/* 10- */
+ { 0x96, KEY_CHANNELUP}, /* ch+ */
+ { 0x91, KEY_CHANNELDOWN}, /* ch- */
+ { 0x93, KEY_VOLUMEUP}, /* vol+ */
+ { 0x8c, KEY_VOLUMEDOWN},/* vol- */
+ { 0x83, KEY_RECORD},/* rec */
+ { 0x98, KEY_PAUSE}, /* pause, yellow */
+ { 0x99, KEY_OK},/* ok */
+ { 0x9a, KEY_CAMERA},/* snapshot */
+ { 0x81, KEY_UP},
+ { 0x90, KEY_LEFT},
+ { 0x82, KEY_RIGHT},
+ { 0x88, KEY_DOWN},
+ { 0x95, KEY_FAVORITES}, /* blue */
+ { 0x97, KEY_SUBTITLE}, /* green */
+ { 0x9d, KEY_ZOOM},
+ { 0x9f, KEY_EXIT},
+ { 0x9e, KEY_MENU},
+ { 0x9c, KEY_EPG},
+ { 0x80, KEY_PREVIOUS}, /* red */
+ { 0x9b, KEY_MODE},
};
static struct rc_keymap tbs_nec_map = {
diff --git a/include/linux/input.h b/include/linux/input.h
index 51af441..711c1307 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -623,6 +623,8 @@ struct input_keymap_entry {
#define KEY_CAMERA_FOCUS 0x210
#define KEY_WPS_BUTTON 0x211 /* WiFi Protected Setup key */
+#define KEY_10CHANNELSUP0x212 /* 10 channels up (10+) */
+#define KEY_10CHANNELSDOWN 0x213 /* 10 channels down (10-) */
#define BTN_TRIGGER_HAPPY 0x2c0
#define BTN_TRIGGER_HAPPY1 0x2c0
--
Mariusz Białończyk
jabber/e-mail: ma...@skyboo.net
http://manio.skyboo.net
--
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 PATCH 0/8] V4L BKL removal: first round
On Tuesday, November 16, 2010 17:49:05 Hans Verkuil wrote:
On Tuesday, November 16, 2010 17:01:36 Arnd Bergmann wrote:
On Tuesday 16 November 2010, Hans Verkuil wrote:
I think there is a misunderstanding. One V4L device (e.g. a TV capture
card, a webcam, etc.) has one v4l2_device struct. But it can have
multiple
V4L device nodes (/dev/video0, /dev/radio0, etc.), each represented by a
struct video_device (and I really hope I can rename that to v4l2_devnode
soon since that's a very confusing name).
You typically need to serialize between all the device nodes belonging
to
the same video hardware. A mutex in struct video_device doesn't do that,
that just serializes access to that single device node. But a mutex in
v4l2_device is at the right level.
Ok, got it now.
A quick follow-up as I saw I didn't fully answer your question: to my
knowledge there are no per-driver data structures that need a BKL for
protection. It's definitely not something I am worried about.
Good. Are you preparing a patch for a per-v4l2_device then? This sounds
like the right place with your explanation. I would not put in the
CONFIG_BKL switch, because I tried that for two other subsystems and got
called back, but I'm not going to stop you.
As for the fallback to a global mutex, I guess you can set the
videodev-lock pointer and use unlocked_ioctl for those drivers
that do not use a v4l2_device yet, if there are only a handful of them.
Arnd
I will look into it. I'll try to have something today or tomorrow.
OK, here is my patch adding a mutex to v4l2_device.
I did some tests if we merge this patch then there are three classes of
drivers:
1) Those implementing unlocked_ioctl: these work like a charm.
2) Those implementing v4l2_device: capturing works fine, but calling ioctls
at the same time from another process or thread is *exceedingly* slow. But at
least there is no interference from other drivers.
3) Those not implementing v4l2_device: using a core lock makes it simply
impossible to capture from e.g. two devices at the same time. I tried with two
uvc webcams: the capture rate is simply horrible.
Note that this is tested in blocking mode. These problems do not appear if you
capture in non-blocking mode.
I consider class 3 unacceptable for commonly seen devices. I did a quick scan
of the v4l drivers and the only common driver that falls in that class is uvc.
There is one other option, although it is very dirty: don't take the lock if
the ioctl command is VIDIOC_DQBUF. It works and reliably as well for uvc and
videobuf (I did a quick code analysis). But I don't know if it works everywhere.
I would like to get the opinion of others before I implement such a check. But
frankly, I think this may be our best bet.
So the patch below would look like this if I add the check:
- mutex_lock(v4l2_ioctl_mutex);
+ if (cmd != VIDIOC_DQBUF)
+ mutex_lock(m);
if (video_is_registered(vdev))
ret = vdev-fops-ioctl(filp, cmd, arg);
- mutex_unlock(v4l2_ioctl_mutex);
+ if (cmd != VIDIOC_DQBUF)
+ mutex_unlock(m);
Comments?
Regards,
Hans
diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
index 03f7f46..026bf38 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -247,11 +247,13 @@ static long v4l2_ioctl(struct file *filp, unsigned int
cmd, unsigned long arg)
} else if (vdev-fops-ioctl) {
/* TODO: convert all drivers to unlocked_ioctl */
static DEFINE_MUTEX(v4l2_ioctl_mutex);
+ struct mutex *m = vdev-v4l2_dev ?
+ vdev-v4l2_dev-ioctl_lock : v4l2_ioctl_mutex;
- mutex_lock(v4l2_ioctl_mutex);
+ mutex_lock(m);
if (video_is_registered(vdev))
ret = vdev-fops-ioctl(filp, cmd, arg);
- mutex_unlock(v4l2_ioctl_mutex);
+ mutex_unlock(m);
} else
ret = -ENOTTY;
diff --git a/drivers/media/video/v4l2-device.c
b/drivers/media/video/v4l2-device.c
index 0b08f96..7fe6f92 100644
--- a/drivers/media/video/v4l2-device.c
+++ b/drivers/media/video/v4l2-device.c
@@ -35,6 +35,7 @@ int v4l2_device_register(struct device *dev, struct
v4l2_device *v4l2_dev)
INIT_LIST_HEAD(v4l2_dev-subdevs);
spin_lock_init(v4l2_dev-lock);
+ mutex_init(v4l2_dev-ioctl_lock);
v4l2_dev-dev = dev;
if (dev == NULL) {
/* If dev == NULL, then name must be filled in by the caller */
diff --git a/include/media/v4l2-device.h b/include/media/v4l2-device.h
index 6648036..b16f307 100644
--- a/include/media/v4l2-device.h
+++ b/include/media/v4l2-device.h
@@ -51,6 +51,8 @@ struct v4l2_device {
unsigned int notification, void *arg);
Re: Fwd: [PATH] Fix rc-tbs-nec table after converting the cx88 driver to ir-core
On Tuesday, November 16, 2010 09:39:09 am Mauro Carvalho Chehab wrote: Hi Dmitry, This patch fixes an IR table. The patch is trivial, but there are two buttons on this IR that are not directly supported currently (buttons 10- and 10+). In a matter of fact, some other IR's use other key codes (KEY_DOT, KEY_KPPLUS, KEY_DIGITS) for a similar keyboard key (100+). Mariusz is proposing the addition of two new keycodes for it. What do you think? Sure, why not. Just move them over to 0x1b8 please (somewhat 'closer' to other media keys). Thanks. -- Dmitry -- 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 PATCH 0/8] V4L BKL removal: first round
On Tuesday 16 November 2010, Hans Verkuil wrote: I consider class 3 unacceptable for commonly seen devices. I did a quick scan of the v4l drivers and the only common driver that falls in that class is uvc. If uvc is the only important one, that should be easy enough to fix by adding a per-device mutex around uvc_v4l2_do_ioctl() or uvc_v4l2_ioctl(). There is one other option, although it is very dirty: don't take the lock if the ioctl command is VIDIOC_DQBUF. It works and reliably as well for uvc and videobuf (I did a quick code analysis). But I don't know if it works everywhere. I would like to get the opinion of others before I implement such a check. But frankly, I think this may be our best bet. So the patch below would look like this if I add the check: - mutex_lock(v4l2_ioctl_mutex); + if (cmd != VIDIOC_DQBUF) + mutex_lock(m); if (video_is_registered(vdev)) ret = vdev-fops-ioctl(filp, cmd, arg); - mutex_unlock(v4l2_ioctl_mutex); + if (cmd != VIDIOC_DQBUF) + mutex_unlock(m); I was thinking of this as well, but didn't bring it up because I considered it too hacky. The patch you posted looks good, thanks for bringing up the problem with my patch and the solution! Acked-by: Arnd Bergmann a...@arndb.de -- 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
Question about setting V4L2_CID_AUTO_WHITE_BALANCE control to FALSE
Hello, When I set V4L2_CID_AUTO_WHITE_BALANCE control to FALSE, which one of the following is the expected behavior? 1. Hold the current white balance settting. 2. Set the white balance to whatever V4L2_CID_WHITE_BALANCE_TEMPERATURE control is set to. The V4L2 API spec doesn't specify this clearly. Regards, -Shuzhen -- 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 01/10] MCDE: Add hardware abstraction layer
On Tue, 2010-11-16 at 16:29 +0100, Jimmy RUBIN wrote:
+/* Channel path */
+#define MCDE_CHNLPATH(__chnl, __fifo, __type, __ifc, __link) \
+ (((__chnl) 16) | ((__fifo) 12) | \
+ ((__type) 8) | ((__ifc) 4) | ((__link) 0))
+enum mcde_chnl_path {
+ /* Channel A */
+ MCDE_CHNLPATH_CHNLA_FIFOA_DPI_0 = MCDE_CHNLPATH(MCDE_CHNL_A,
+ MCDE_FIFO_A, MCDE_PORTTYPE_DPI, 0, 0),
+ MCDE_CHNLPATH_CHNLA_FIFOA_DSI_IFC0_0 =
MCDE_CHNLPATH(MCDE_CHNL_A,
+ MCDE_FIFO_A, MCDE_PORTTYPE_DSI, 0, 0),
+ MCDE_CHNLPATH_CHNLA_FIFOA_DSI_IFC0_1 =
MCDE_CHNLPATH(MCDE_CHNL_A,
+ MCDE_FIFO_A, MCDE_PORTTYPE_DSI, 0, 1),
A table like this would become more readable by making each entry a
single line, even if that goes beyond the 80-character limit.
Good point, we will fix this
Or the #define could be changed to do something like:
static inline u32 MCDE_channel_path(u32 chnl, u32 fifo, u32 type, u32 ifc, u32
link)
{
return ((chnl 16) |
(fifo 12) |
(type 8) |
(ifc 4) |
(link 0));
}
#define SET_ENUM_MCDE_CHNLPATH(chnl, fifo, var, type, ifc, link)\
MCDE_CHNLPATH_CHNL##chnl##_FIFO##fifo##_##var = \
MCDE_channel_path(MCDE_CHNL_##chnl, \
MCDE_FIFO_##fifo, \
MCDE_PORTTYPE_##type, \
ifc, \
link)
enum mcde_chnl_path {
/* Channel A */
SET_ENUM_MCDE_CHNLPATH(A, A, DPI_0, DPI, 0, 0),
SET_ENUM_MCDE_CHNLPATH(A, A, DSI_IFC0_0,DSI, 0, 0),
SET_ENUM_MCDE_CHNLPATH(A, A, DSI_IFC0_1,DSI, 0, 1),
SET_ENUM_MCDE_CHNLPATH(A, C, DSI_IFC0_2,DSI, 0, 2),
SET_ENUM_MCDE_CHNLPATH(A, C, DSI_IFC1_0,DSI, 1, 0),
SET_ENUM_MCDE_CHNLPATH(A, C, DSI_IFC1_1,DSI, 1, 1),
SET_ENUM_MCDE_CHNLPATH(A, A, DSI_IFC1_2,DSI, 1, 2),
/* Channel B */
SET_ENUM_MCDE_CHNLPATH(B, B, DPI_1, DPI, 0, 1),
SET_ENUM_MCDE_CHNLPATH(B, B, DSI_IFC0_0,DSI, 0, 0),
SET_ENUM_MCDE_CHNLPATH(B, B, DSI_IFC0_1,DSI, 0, 1),
SET_ENUM_MCDE_CHNLPATH(B, C, DSI_IFC0_2,DSI, 0, 2),
SET_ENUM_MCDE_CHNLPATH(B, C, DSI_IFC1_0,DSI, 1, 0),
SET_ENUM_MCDE_CHNLPATH(B, C, DSI_IFC1_1,DSI, 1, 1),
SET_ENUM_MCDE_CHNLPATH(B, B, DSI_IFC1_2,DSI, 1, 2),
/* Channel C0 */
SET_ENUM_MCDE_CHNLPATH(C0, A, DSI_IFC0_0, DSI, 0, 0),
SET_ENUM_MCDE_CHNLPATH(C0, A, DSI_IFC0_1, DSI, 0, 1),
SET_ENUM_MCDE_CHNLPATH(C0, C0, DSI_IFC0_2, DSI, 0, 2),
SET_ENUM_MCDE_CHNLPATH(C0, C0, DSI_IFC1_0, DSI, 1, 0),
SET_ENUM_MCDE_CHNLPATH(C0, C0, DSI_IFC1_1, DSI, 1, 1),
SET_ENUM_MCDE_CHNLPATH(C0, A, DSI_IFC1_2, DSI, 1, 2),
/* Channel C1 */
SET_ENUM_MCDE_CHNLPATH(C1, B, DSI_IFC0_0, DSI, 0, 0),
SET_ENUM_MCDE_CHNLPATH(C1, B, DSI_IFC0_1, DSI, 0, 1),
SET_ENUM_MCDE_CHNLPATH(C1, C1, DSI_IFC0_2, DSI, 0, 2),
SET_ENUM_MCDE_CHNLPATH(C1, C1, DSI_IFC1_0, DSI, 1, 0),
SET_ENUM_MCDE_CHNLPATH(C1, C1, DSI_IFC1_1, DSI, 1, 1),
SET_ENUM_MCDE_CHNLPATH(C1, B, DSI_IFC1_2, DSI, 1, 2),
};
It seems that long blocks of upper case make my eyes glaze
over and that many of your #defines are indistinguishable.
--
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 PATCH 0/8] V4L BKL removal: first round
On Tue, 2010-11-16 at 19:38 +0100, Hans Verkuil wrote:
On Tuesday, November 16, 2010 17:49:05 Hans Verkuil wrote:
On Tuesday, November 16, 2010 17:01:36 Arnd Bergmann wrote:
On Tuesday 16 November 2010, Hans Verkuil wrote:
I think there is a misunderstanding. One V4L device (e.g. a TV capture
card, a webcam, etc.) has one v4l2_device struct. But it can have
multiple
V4L device nodes (/dev/video0, /dev/radio0, etc.), each represented
by a
struct video_device (and I really hope I can rename that to
v4l2_devnode
soon since that's a very confusing name).
You typically need to serialize between all the device nodes
belonging to
the same video hardware. A mutex in struct video_device doesn't do
that,
that just serializes access to that single device node. But a mutex in
v4l2_device is at the right level.
Ok, got it now.
A quick follow-up as I saw I didn't fully answer your question: to my
knowledge there are no per-driver data structures that need a BKL for
protection. It's definitely not something I am worried about.
Good. Are you preparing a patch for a per-v4l2_device then? This sounds
like the right place with your explanation. I would not put in the
CONFIG_BKL switch, because I tried that for two other subsystems and got
called back, but I'm not going to stop you.
As for the fallback to a global mutex, I guess you can set the
videodev-lock pointer and use unlocked_ioctl for those drivers
that do not use a v4l2_device yet, if there are only a handful of them.
Arnd
I will look into it. I'll try to have something today or tomorrow.
OK, here is my patch adding a mutex to v4l2_device.
I did some tests if we merge this patch then there are three classes of
drivers:
1) Those implementing unlocked_ioctl: these work like a charm.
2) Those implementing v4l2_device: capturing works fine, but calling ioctls
at the same time from another process or thread is *exceedingly* slow. But at
least there is no interference from other drivers.
3) Those not implementing v4l2_device: using a core lock makes it simply
impossible to capture from e.g. two devices at the same time. I tried with two
uvc webcams: the capture rate is simply horrible.
Note that this is tested in blocking mode. These problems do not appear if you
capture in non-blocking mode.
I consider class 3 unacceptable for commonly seen devices. I did a quick scan
of the v4l drivers and the only common driver that falls in that class is uvc.
There is one other option, although it is very dirty: don't take the lock if
the ioctl command is VIDIOC_DQBUF.
Is this in addition to or instead of the mutex lock at
v4l2_device ?
It works and reliably as well for uvc and
videobuf (I did a quick code analysis). But I don't know if it works
everywhere.
I would like to get the opinion of others before I implement such a check. But
frankly, I think this may be our best bet.
Opinions? No problem! ;)
opinions
I think it is probably bad.
So the patch below would look like this if I add the check:
- mutex_lock(v4l2_ioctl_mutex);
+ if (cmd != VIDIOC_DQBUF)
+ mutex_lock(m);
if (video_is_registered(vdev))
ret = vdev-fops-ioctl(filp, cmd, arg);
- mutex_unlock(v4l2_ioctl_mutex);
+ if (cmd != VIDIOC_DQBUF)
+ mutex_unlock(m);
What happens to driver state when VIDIOC_STREAMOFF has the lock held and
VIDIOC_DQBUF comes through? I think it is legitimate design for an
application to have a playback control thread separate from a thread
that reads in the capture data.
If this quirk of infrastructure locking is going in, might I suggest
that you please document in code comments:
a. The scope of what infrastructure lock is intended to protect. That
is obvious right now, but may not be in the future.
b. Why there is an exception to taking the infrastructure lock or what
conditions necessitate having the lock ignored/dropped.
c. What code maintenance must be done to remove the exception to taking
the lock. A specific bullet-list of problem drivers might be nice.
We won't do future maintainers any favors by letting the operation,
intended behavior, intended scope, and rationale for this odd locking
semantic be lost to history. We just introduce a BKL with smaller
scope.
Comments?
Regards,
Hans
diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
index 03f7f46..026bf38 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -247,11 +247,13 @@ static long v4l2_ioctl(struct file *filp, unsigned int
cmd, unsigned long arg)
} else if (vdev-fops-ioctl) {
/* TODO: convert all drivers to unlocked_ioctl */
static DEFINE_MUTEX(v4l2_ioctl_mutex);
[PATCH] drivers/media: nuvoton: always true expression
I noticed that the second part of this conditional is always true. Would the intention be to strictly check on both chip_major and chip_minor? Signed-off-by: Nicolas Kaiser ni...@nikai.net --- drivers/media/IR/nuvoton-cir.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/IR/nuvoton-cir.c b/drivers/media/IR/nuvoton-cir.c index 301be53..896463b 100644 --- a/drivers/media/IR/nuvoton-cir.c +++ b/drivers/media/IR/nuvoton-cir.c @@ -249,8 +249,8 @@ static int nvt_hw_detect(struct nvt_dev *nvt) chip_minor = nvt_cr_read(nvt, CR_CHIP_ID_LO); nvt_dbg(%s: chip id: 0x%02x 0x%02x, chip_id, chip_major, chip_minor); - if (chip_major != CHIP_ID_HIGH - (chip_minor != CHIP_ID_LOW || chip_minor != CHIP_ID_LOW2)) + if (chip_major != CHIP_ID_HIGH || + (chip_minor != CHIP_ID_LOW chip_minor != CHIP_ID_LOW2)) ret = -ENODEV; nvt_efm_disable(nvt); -- 1.7.2.2 -- 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 PATCH 0/8] V4L BKL removal: first round
On Tuesday, November 16, 2010 20:59:41 Andy Walls wrote: On Tue, 2010-11-16 at 19:38 +0100, Hans Verkuil wrote: On Tuesday, November 16, 2010 17:49:05 Hans Verkuil wrote: On Tuesday, November 16, 2010 17:01:36 Arnd Bergmann wrote: On Tuesday 16 November 2010, Hans Verkuil wrote: I think there is a misunderstanding. One V4L device (e.g. a TV capture card, a webcam, etc.) has one v4l2_device struct. But it can have multiple V4L device nodes (/dev/video0, /dev/radio0, etc.), each represented by a struct video_device (and I really hope I can rename that to v4l2_devnode soon since that's a very confusing name). You typically need to serialize between all the device nodes belonging to the same video hardware. A mutex in struct video_device doesn't do that, that just serializes access to that single device node. But a mutex in v4l2_device is at the right level. Ok, got it now. A quick follow-up as I saw I didn't fully answer your question: to my knowledge there are no per-driver data structures that need a BKL for protection. It's definitely not something I am worried about. Good. Are you preparing a patch for a per-v4l2_device then? This sounds like the right place with your explanation. I would not put in the CONFIG_BKL switch, because I tried that for two other subsystems and got called back, but I'm not going to stop you. As for the fallback to a global mutex, I guess you can set the videodev-lock pointer and use unlocked_ioctl for those drivers that do not use a v4l2_device yet, if there are only a handful of them. Arnd I will look into it. I'll try to have something today or tomorrow. OK, here is my patch adding a mutex to v4l2_device. I did some tests if we merge this patch then there are three classes of drivers: 1) Those implementing unlocked_ioctl: these work like a charm. 2) Those implementing v4l2_device: capturing works fine, but calling ioctls at the same time from another process or thread is *exceedingly* slow. But at least there is no interference from other drivers. BTW, with 'exceedingly slow' I mean that e.g. a v4l2-ctl --list-ctrls takes 10-20 seconds if the device node is streaming at the same time. Something that normally takes less than 0.01s. 3) Those not implementing v4l2_device: using a core lock makes it simply impossible to capture from e.g. two devices at the same time. I tried with two uvc webcams: the capture rate is simply horrible. Note that this is tested in blocking mode. These problems do not appear if you capture in non-blocking mode. I consider class 3 unacceptable for commonly seen devices. I did a quick scan of the v4l drivers and the only common driver that falls in that class is uvc. There is one other option, although it is very dirty: don't take the lock if the ioctl command is VIDIOC_DQBUF. Is this in addition to or instead of the mutex lock at v4l2_device ? In addition to. It works and reliably as well for uvc and videobuf (I did a quick code analysis). But I don't know if it works everywhere. I would like to get the opinion of others before I implement such a check. But frankly, I think this may be our best bet. Opinions? No problem! ;) opinions I think it is probably bad. So the patch below would look like this if I add the check: - mutex_lock(v4l2_ioctl_mutex); + if (cmd != VIDIOC_DQBUF) + mutex_lock(m); if (video_is_registered(vdev)) ret = vdev-fops-ioctl(filp, cmd, arg); - mutex_unlock(v4l2_ioctl_mutex); + if (cmd != VIDIOC_DQBUF) + mutex_unlock(m); What happens to driver state when VIDIOC_STREAMOFF has the lock held and VIDIOC_DQBUF comes through? I think it is legitimate design for an application to have a playback control thread separate from a thread that reads in the capture data. All I can say is that it will work OK for drivers using videobuf since videobuf will take its own lock. UVC? I'm not sure. But I think that it is probably best to fix uvc for 2.6.37 regardless given the importance of this driver. It's dirty, but I actually think that it will work fine. If this quirk of infrastructure locking is going in, might I suggest that you please document in code comments: a. The scope of what infrastructure lock is intended to protect. That is obvious right now, but may not be in the future. b. Why there is an exception to taking the infrastructure lock or what conditions necessitate having the lock ignored/dropped. c. What code maintenance must be done to remove the exception to taking the lock. A specific bullet-list of problem drivers might be nice.
Re: HVR900H : IR Remote Control
Le 16/11/2010 21:32, Stefan Ringel a écrit : Am 16.11.2010 21:26, schrieb Massis Sirapian: Le 15/11/2010 22:08, Stefan Ringel a écrit : Am 15.11.2010 22:00, schrieb Massis Sirapian: Le 15/11/2010 18:43, Stefan Ringel a écrit : Am 15.11.2010 10:15, schrieb Richard Zidlicky: On Sun, Nov 14, 2010 at 08:22:44PM +0100, Massis Sirapian wrote: Thanks Stefan. I've checked the /drivers/media/IR/keymaps of the kernel source directory, but nothing seems to fit my remote, which is a DSR-0012 : http://lirc.sourceforge.net/remotes/hauppauge/DSR-0112.jpg. FYI, this remote is identical to that shipped with (most?) Haupauge Ministicks and the codes reportedly match the rc-dib0700-rc5.c keymap. However I have not figured out how to make the userspace work with the new ir-code yet. Richard With my terratec cinergy hybrid xe (equal yours hvr900h) I have this: localhost:/usr/src/src/tm6000_alsa/utils/v4l-utils # ir-keytable Found /sys/class/rc/rc0/ (/dev/input/event5) with: Driver tm6000, table rc-nec-terratec-cinergy-xs Supported protocols: NEC RC-5 Enabled protocols: NEC I can change outside the keytable. Just loading tm6000-dvb, I have this : [ 253.829422] IR NEC protocol handler initialized [ 253.846608] IR RC5(x) protocol handler initialized [ 253.883882] tm6000: module is from the staging directory, the quality is unknown, you have been warned. [ 253.886611] tm6000 v4l2 driver version 0.0.2 loaded [ 253.887558] tm6000: alt 0, interface 0, class 255 [ 253.887574] tm6000: alt 0, interface 0, class 255 [ 253.887575] tm6000: Bulk IN endpoint: 0x82 (max size=512 bytes) [ 253.887577] tm6000: alt 0, interface 0, class 255 [ 253.887578] tm6000: alt 1, interface 0, class 255 [ 253.887579] tm6000: ISOC IN endpoint: 0x81 (max size=3072 bytes) [ 253.887580] tm6000: alt 1, interface 0, class 255 [ 253.887581] tm6000: alt 1, interface 0, class 255 [ 253.887582] tm6000: INT IN endpoint: 0x83 (max size=4 bytes) [ 253.887583] tm6000: alt 2, interface 0, class 255 [ 253.887584] tm6000: alt 2, interface 0, class 255 [ 253.887586] tm6000: alt 2, interface 0, class 255 [ 253.887587] tm6000: alt 3, interface 0, class 255 [ 253.887588] tm6000: alt 3, interface 0, class 255 [ 253.887589] tm6000: alt 3, interface 0, class 255 [ 253.887590] tm6000: New video device @ 480 Mbps (2040:6600, ifnum 0) [ 253.887591] tm6000: Found Hauppauge WinTV HVR-900H / WinTV USB2-Stick [ 253.48] IR RC6 protocol handler initialized [ 253.890209] IR JVC protocol handler initialized [ 253.891515] IR Sony protocol handler initialized [ 253.893815] lirc_dev: IR Remote Control driver registered, major 250 [ 253.894722] IR LIRC bridge handler initialized [ 254.806122] Board version = 0x67980bf4 [ 255.197098] board=0x67980bf4 [ 255.320786] tm6000 #0: i2c eeprom 00: 01 59 54 45 12 01 00 02 00 00 00 40 40 20 00 66 .YTE...@@ .f [ 255.512277] tm6000 #0: i2c eeprom 10: 69 00 10 20 40 01 02 03 48 00 79 00 62 00 72 00 i.. @...H.y.b.r. [ 255.703783] tm6000 #0: i2c eeprom 20: ff 00 64 ff ff ff ff ff ff ff ff ff ff ff ff ff ..d. [ 255.895281] tm6000 #0: i2c eeprom 30: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 256.086786] tm6000 #0: i2c eeprom 40: 10 03 48 00 56 00 52 00 39 00 30 00 30 00 48 00 ..H.V.R.9.0.0.H. [ 256.278289] tm6000 #0: i2c eeprom 50: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 256.469783] tm6000 #0: i2c eeprom 60: 30 ff ff ff 0f ff ff ff ff ff 0a 03 32 00 2e 00 0...2... [ 256.661287] tm6000 #0: i2c eeprom 70: 3f 00 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ?... [ 256.852786] tm6000 #0: i2c eeprom 80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 257.044307] tm6000 #0: i2c eeprom 90: 30 ff ff ff 16 03 34 00 30 00 33 00 32 00 32 00 0.4.0.3.2.2. [ 257.235798] tm6000 #0: i2c eeprom a0: 33 00 36 00 39 00 30 00 35 00 00 00 77 00 ff ff 3.6.9.0.5...w... [ 257.427295] tm6000 #0: i2c eeprom b0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 257.618794] tm6000 #0: i2c eeprom c0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 257.810303] tm6000 #0: i2c eeprom d0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 258.001810] tm6000 #0: i2c eeprom e0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 258.193291] tm6000 #0: i2c eeprom f0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 258.372825] [ 258.378849] tuner 4-0061: chip found @ 0xc2 (tm6000 #0) [ 258.400777] xc2028 4-0061: creating new instance [ 258.400779] xc2028 4-0061: type set to XCeive xc2028/xc3028 tuner [ 258.400781] Setting firmware parameters for xc2028 [ 258.427221] xc2028 4-0061: Loading 81 firmware images from xc3028L-v36.fw, type: xc2028 firmware, ver 3.6 [ 258.668063] xc2028 4-0061: Loading firmware for type=BASE (1), id . [ 333.245767] xc2028 4-0061: Loading firmware for type=(0), id b700. [ 334.510473] SCODE (2000), id b700:
Re: HVR900H : IR Remote Control
Am 16.11.2010 21:42, schrieb Massis Sirapian: Le 16/11/2010 21:32, Stefan Ringel a écrit : Am 16.11.2010 21:26, schrieb Massis Sirapian: Le 15/11/2010 22:08, Stefan Ringel a écrit : Am 15.11.2010 22:00, schrieb Massis Sirapian: Le 15/11/2010 18:43, Stefan Ringel a écrit : Am 15.11.2010 10:15, schrieb Richard Zidlicky: On Sun, Nov 14, 2010 at 08:22:44PM +0100, Massis Sirapian wrote: Thanks Stefan. I've checked the /drivers/media/IR/keymaps of the kernel source directory, but nothing seems to fit my remote, which is a DSR-0012 : http://lirc.sourceforge.net/remotes/hauppauge/DSR-0112.jpg. FYI, this remote is identical to that shipped with (most?) Haupauge Ministicks and the codes reportedly match the rc-dib0700-rc5.c keymap. However I have not figured out how to make the userspace work with the new ir-code yet. Richard With my terratec cinergy hybrid xe (equal yours hvr900h) I have this: localhost:/usr/src/src/tm6000_alsa/utils/v4l-utils # ir-keytable Found /sys/class/rc/rc0/ (/dev/input/event5) with: Driver tm6000, table rc-nec-terratec-cinergy-xs Supported protocols: NEC RC-5 Enabled protocols: NEC I can change outside the keytable. Just loading tm6000-dvb, I have this : [ 253.829422] IR NEC protocol handler initialized [ 253.846608] IR RC5(x) protocol handler initialized [ 253.883882] tm6000: module is from the staging directory, the quality is unknown, you have been warned. [ 253.886611] tm6000 v4l2 driver version 0.0.2 loaded [ 253.887558] tm6000: alt 0, interface 0, class 255 [ 253.887574] tm6000: alt 0, interface 0, class 255 [ 253.887575] tm6000: Bulk IN endpoint: 0x82 (max size=512 bytes) [ 253.887577] tm6000: alt 0, interface 0, class 255 [ 253.887578] tm6000: alt 1, interface 0, class 255 [ 253.887579] tm6000: ISOC IN endpoint: 0x81 (max size=3072 bytes) [ 253.887580] tm6000: alt 1, interface 0, class 255 [ 253.887581] tm6000: alt 1, interface 0, class 255 [ 253.887582] tm6000: INT IN endpoint: 0x83 (max size=4 bytes) [ 253.887583] tm6000: alt 2, interface 0, class 255 [ 253.887584] tm6000: alt 2, interface 0, class 255 [ 253.887586] tm6000: alt 2, interface 0, class 255 [ 253.887587] tm6000: alt 3, interface 0, class 255 [ 253.887588] tm6000: alt 3, interface 0, class 255 [ 253.887589] tm6000: alt 3, interface 0, class 255 [ 253.887590] tm6000: New video device @ 480 Mbps (2040:6600, ifnum 0) [ 253.887591] tm6000: Found Hauppauge WinTV HVR-900H / WinTV USB2-Stick [ 253.48] IR RC6 protocol handler initialized [ 253.890209] IR JVC protocol handler initialized [ 253.891515] IR Sony protocol handler initialized [ 253.893815] lirc_dev: IR Remote Control driver registered, major 250 [ 253.894722] IR LIRC bridge handler initialized [ 254.806122] Board version = 0x67980bf4 [ 255.197098] board=0x67980bf4 [ 255.320786] tm6000 #0: i2c eeprom 00: 01 59 54 45 12 01 00 02 00 00 00 40 40 20 00 66 .YTE...@@ .f [ 255.512277] tm6000 #0: i2c eeprom 10: 69 00 10 20 40 01 02 03 48 00 79 00 62 00 72 00 i.. @...H.y.b.r. [ 255.703783] tm6000 #0: i2c eeprom 20: ff 00 64 ff ff ff ff ff ff ff ff ff ff ff ff ff ..d. [ 255.895281] tm6000 #0: i2c eeprom 30: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 256.086786] tm6000 #0: i2c eeprom 40: 10 03 48 00 56 00 52 00 39 00 30 00 30 00 48 00 ..H.V.R.9.0.0.H. [ 256.278289] tm6000 #0: i2c eeprom 50: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 256.469783] tm6000 #0: i2c eeprom 60: 30 ff ff ff 0f ff ff ff ff ff 0a 03 32 00 2e 00 0...2... [ 256.661287] tm6000 #0: i2c eeprom 70: 3f 00 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ?... [ 256.852786] tm6000 #0: i2c eeprom 80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 257.044307] tm6000 #0: i2c eeprom 90: 30 ff ff ff 16 03 34 00 30 00 33 00 32 00 32 00 0.4.0.3.2.2. [ 257.235798] tm6000 #0: i2c eeprom a0: 33 00 36 00 39 00 30 00 35 00 00 00 77 00 ff ff 3.6.9.0.5...w... [ 257.427295] tm6000 #0: i2c eeprom b0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 257.618794] tm6000 #0: i2c eeprom c0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 257.810303] tm6000 #0: i2c eeprom d0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 258.001810] tm6000 #0: i2c eeprom e0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 258.193291] tm6000 #0: i2c eeprom f0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 258.372825] [ 258.378849] tuner 4-0061: chip found @ 0xc2 (tm6000 #0) [ 258.400777] xc2028 4-0061: creating new instance [ 258.400779] xc2028 4-0061: type set to XCeive xc2028/xc3028 tuner [ 258.400781] Setting firmware parameters for xc2028 [ 258.427221] xc2028 4-0061: Loading 81 firmware images from xc3028L-v36.fw, type: xc2028 firmware, ver 3.6 [ 258.668063] xc2028 4-0061: Loading firmware for type=BASE (1), id . [ 333.245767] xc2028 4-0061: Loading firmware for type=(0), id
Re: [RFC PATCH 0/8] V4L BKL removal: first round
On Tuesday, November 16, 2010 21:29:11 Hans Verkuil wrote: On Tuesday, November 16, 2010 20:59:41 Andy Walls wrote: On Tue, 2010-11-16 at 19:38 +0100, Hans Verkuil wrote: On Tuesday, November 16, 2010 17:49:05 Hans Verkuil wrote: On Tuesday, November 16, 2010 17:01:36 Arnd Bergmann wrote: On Tuesday 16 November 2010, Hans Verkuil wrote: I think there is a misunderstanding. One V4L device (e.g. a TV capture card, a webcam, etc.) has one v4l2_device struct. But it can have multiple V4L device nodes (/dev/video0, /dev/radio0, etc.), each represented by a struct video_device (and I really hope I can rename that to v4l2_devnode soon since that's a very confusing name). You typically need to serialize between all the device nodes belonging to the same video hardware. A mutex in struct video_device doesn't do that, that just serializes access to that single device node. But a mutex in v4l2_device is at the right level. Ok, got it now. A quick follow-up as I saw I didn't fully answer your question: to my knowledge there are no per-driver data structures that need a BKL for protection. It's definitely not something I am worried about. Good. Are you preparing a patch for a per-v4l2_device then? This sounds like the right place with your explanation. I would not put in the CONFIG_BKL switch, because I tried that for two other subsystems and got called back, but I'm not going to stop you. As for the fallback to a global mutex, I guess you can set the videodev-lock pointer and use unlocked_ioctl for those drivers that do not use a v4l2_device yet, if there are only a handful of them. Arnd I will look into it. I'll try to have something today or tomorrow. OK, here is my patch adding a mutex to v4l2_device. I did some tests if we merge this patch then there are three classes of drivers: 1) Those implementing unlocked_ioctl: these work like a charm. 2) Those implementing v4l2_device: capturing works fine, but calling ioctls at the same time from another process or thread is *exceedingly* slow. But at least there is no interference from other drivers. BTW, with 'exceedingly slow' I mean that e.g. a v4l2-ctl --list-ctrls takes 10-20 seconds if the device node is streaming at the same time. Something that normally takes less than 0.01s. 3) Those not implementing v4l2_device: using a core lock makes it simply impossible to capture from e.g. two devices at the same time. I tried with two uvc webcams: the capture rate is simply horrible. Note that this is tested in blocking mode. These problems do not appear if you capture in non-blocking mode. I consider class 3 unacceptable for commonly seen devices. I did a quick scan of the v4l drivers and the only common driver that falls in that class is uvc. There is one other option, although it is very dirty: don't take the lock if the ioctl command is VIDIOC_DQBUF. Is this in addition to or instead of the mutex lock at v4l2_device ? In addition to. It works and reliably as well for uvc and videobuf (I did a quick code analysis). But I don't know if it works everywhere. I would like to get the opinion of others before I implement such a check. But frankly, I think this may be our best bet. Opinions? No problem! ;) opinions I think it is probably bad. So the patch below would look like this if I add the check: - mutex_lock(v4l2_ioctl_mutex); + if (cmd != VIDIOC_DQBUF) + mutex_lock(m); if (video_is_registered(vdev)) ret = vdev-fops-ioctl(filp, cmd, arg); - mutex_unlock(v4l2_ioctl_mutex); + if (cmd != VIDIOC_DQBUF) + mutex_unlock(m); What happens to driver state when VIDIOC_STREAMOFF has the lock held and VIDIOC_DQBUF comes through? I think it is legitimate design for an application to have a playback control thread separate from a thread that reads in the capture data. All I can say is that it will work OK for drivers using videobuf since videobuf will take its own lock. UVC? I'm not sure. But I think that it is probably best to fix uvc for 2.6.37 regardless given the importance of this driver. It's dirty, but I actually think that it will work fine. If this quirk of infrastructure locking is going in, might I suggest that you please document in code comments: a. The scope of what infrastructure lock is intended to protect. That is obvious right now, but may not be in the future. b. Why there is an exception to taking the
Re: [RFC PATCH 0/8] V4L BKL removal: first round
Hans, I've had some patches pending for a while now that affect the dsbr100 driver. The patches can be seen here: http://desource.dyndns.org/~atog/gitweb/?p=linux-media.git in the dsbr100 branch. The first patch in the series fixes locking issues throughout the driver and converts it to use the unlocked ioctl. The series is a bit old, so it doesn't make use of the v4l2 core assisted locking; but that is trivial to implement after this patch. Regards, David Ellingsworth -- 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 1/5] kconfig: add an option to determine a menu's visibility
Hi, On Tue, Nov 16, 2010 at 12:44 PM, Mauro Carvalho Chehab mche...@redhat.com wrote: Em 15-11-2010 14:57, Arnaud Lacombe escreveu: Hi all On Sat, Nov 6, 2010 at 5:30 PM, Arnaud Lacombe lacom...@gmail.com wrote: This option is aimed to add the possibility to control a menu's visibility without adding dependency to the expression to all the submenu. Signed-off-by: Arnaud Lacombe lacom...@gmail.com --- scripts/kconfig/expr.h | 1 + scripts/kconfig/lkc.h | 1 + scripts/kconfig/menu.c | 11 +++ scripts/kconfig/zconf.gperf | 1 + scripts/kconfig/zconf.y | 21 ++--- 5 files changed, 32 insertions(+), 3 deletions(-) Michal, I don't think you commented on this ? Mauro, has it been worked around differently ? Those patches worked fine, and solved all problems we had (I just had to touch on two other menus that are used, as I answered upstream). I prefer if Michal could forward those patches upstream, so, there's my ack: Acked-by: Mauro Carvalho Chehab mche...@redhat.com Tested-by: Mauro Carvalho Chehab mche...@redhat.com It would seem Michal is not around lately, his only passage on linux-kbuild@ is nearly a week old. Sam, by any chance, could you comment on these patches so that we could keep moving forward ? Thanks, - Arnaud ps: yes, I know, I did not upgrade the documentation. -- 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 PATCH 0/8] V4L BKL removal: first round
On Tuesday, November 16, 2010 22:32:57 David Ellingsworth wrote: Hans, I've had some patches pending for a while now that affect the dsbr100 driver. The patches can be seen here: http://desource.dyndns.org/~atog/gitweb/?p=linux-media.git in the dsbr100 branch. The first patch in the series fixes locking issues throughout the driver and converts it to use the unlocked ioctl. The series is a bit old, so it doesn't make use of the v4l2 core assisted locking; but that is trivial to implement after this patch. Would it be a problem for you if for 2.6.37 I just replace .ioctl by .unlocked_ioctl? And do the full conversion for 2.6.38? That way the 2.6.37 patches remain small. Regards. Hans -- Hans Verkuil - video4linux developer - sponsored by Cisco -- 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 1/5] kconfig: add an option to determine a menu's visibility
On Tue, Nov 16, 2010 at 04:41:06PM -0500, Arnaud Lacombe wrote: Hi, On Tue, Nov 16, 2010 at 12:44 PM, Mauro Carvalho Chehab mche...@redhat.com wrote: Em 15-11-2010 14:57, Arnaud Lacombe escreveu: Hi all On Sat, Nov 6, 2010 at 5:30 PM, Arnaud Lacombe lacom...@gmail.com wrote: This option is aimed to add the possibility to control a menu's visibility without adding dependency to the expression to all the submenu. Signed-off-by: Arnaud Lacombe lacom...@gmail.com --- scripts/kconfig/expr.h | 1 + scripts/kconfig/lkc.h | 1 + scripts/kconfig/menu.c | 11 +++ scripts/kconfig/zconf.gperf | 1 + scripts/kconfig/zconf.y | 21 ++--- 5 files changed, 32 insertions(+), 3 deletions(-) Michal, I don't think you commented on this ? Mauro, has it been worked around differently ? Those patches worked fine, and solved all problems we had (I just had to touch on two other menus that are used, as I answered upstream). I prefer if Michal could forward those patches upstream, so, there's my ack: Acked-by: Mauro Carvalho Chehab mche...@redhat.com Tested-by: Mauro Carvalho Chehab mche...@redhat.com It would seem Michal is not around lately, his only passage on linux-kbuild@ is nearly a week old. Sam, by any chance, could you comment on these patches so that we could keep moving forward ? I will try to take a look in the weekend - daytime job keeps me busy as usual. Thanks, - Arnaud ps: yes, I know, I did not upgrade the documentation. And I will toast you for that when I look at the patches :-) Sam -- 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] drivers/media: nuvoton: always true expression
On Tue, Nov 16, 2010 at 09:19:53PM +0100, Nicolas Kaiser wrote: I noticed that the second part of this conditional is always true. Would the intention be to strictly check on both chip_major and chip_minor? Signed-off-by: Nicolas Kaiser ni...@nikai.net Hrm, yeah, looks like I screwed that one up. You're correct, the intention was to make sure we have a matching chip id high and one or the other of the chip id low values. Acked-by: Jarod Wilson ja...@redhat.com -- Jarod Wilson ja...@redhat.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
[RFCv2 PATCH 00/15] BKL removal
This is my second patch series for the BKL removal project. While this series is against the v2.6.38 staging tree I think it would be good to target 2.6.37 instead. A lot of files are changed, but the changes are all trivial. And this will hopefully take care of most of the BKL removal problems, except for uvc. This patch also contains the improved core locking patch which doesn't lock the VIDIOC_DQBUF ioctl. Regards, Hans Hans Verkuil (15): v4l2-dev: use mutex_lock_interruptible instead of plain mutex_lock BKL: trivial BKL removal from V4L2 radio drivers cadet: use unlocked_ioctl tea5764: convert to unlocked_ioctl si4713: convert to unlocked_ioctl typhoon: convert to unlocked_ioctl. dsbr100: convert to unlocked_ioctl. BKL: trivial ioctl - unlocked_ioctl video driver conversions sn9c102: convert to unlocked_ioctl. et61x251_core: trivial conversion to unlocked_ioctl. cafe_ccic: replace ioctl by unlocked_ioctl. sh_vou: convert to unlocked_ioctl. radio-timb: convert to unlocked_ioctl. V4L: improve the BKL replacement heuristic cx18: convert to unlocked_ioctl. drivers/media/radio/dsbr100.c|2 +- drivers/media/radio/radio-aimslab.c | 16 +++--- drivers/media/radio/radio-aztech.c |6 +- drivers/media/radio/radio-cadet.c| 12 +++- drivers/media/radio/radio-gemtek-pci.c |6 +- drivers/media/radio/radio-gemtek.c | 14 ++-- drivers/media/radio/radio-maestro.c | 14 ++--- drivers/media/radio/radio-maxiradio.c|2 +- drivers/media/radio/radio-miropcm20.c|6 +- drivers/media/radio/radio-rtrack2.c | 10 ++-- drivers/media/radio/radio-sf16fmi.c |7 +- drivers/media/radio/radio-sf16fmr2.c | 11 ++-- drivers/media/radio/radio-si4713.c |3 +- drivers/media/radio/radio-tea5764.c | 49 +++- drivers/media/radio/radio-terratec.c |8 +- drivers/media/radio/radio-timb.c |5 +- drivers/media/radio/radio-trust.c| 18 +++--- drivers/media/radio/radio-typhoon.c | 16 +++--- drivers/media/radio/radio-zoltrix.c | 30 +- drivers/media/video/arv.c|2 +- drivers/media/video/bw-qcam.c|2 +- drivers/media/video/c-qcam.c |2 +- drivers/media/video/cafe_ccic.c |2 +- drivers/media/video/cx18/cx18-streams.c |2 +- drivers/media/video/et61x251/et61x251_core.c |2 +- drivers/media/video/meye.c | 14 ++-- drivers/media/video/pms.c|2 +- drivers/media/video/sh_vou.c | 13 +++-- drivers/media/video/sn9c102/sn9c102_core.c |2 +- drivers/media/video/v4l2-dev.c | 81 +- drivers/media/video/v4l2-device.c|1 + drivers/media/video/w9966.c |2 +- include/media/v4l2-dev.h |2 +- include/media/v4l2-device.h |2 + 34 files changed, 201 insertions(+), 165 deletions(-) -- 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
[RFCv2 PATCH 01/15] v4l2-dev: use mutex_lock_interruptible instead of plain mutex_lock
Where reasonable use mutex_lock_interruptible instead of mutex_lock.
Signed-off-by: Hans Verkuil hverk...@xs4all.nl
---
drivers/media/video/v4l2-dev.c | 44 +--
1 files changed, 32 insertions(+), 12 deletions(-)
diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
index 0ca7978..8eb0756 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -191,8 +191,12 @@ static ssize_t v4l2_read(struct file *filp, char __user
*buf,
if (!vdev-fops-read)
return -EINVAL;
- if (vdev-lock)
- mutex_lock(vdev-lock);
+ if (vdev-lock) {
+ int res = mutex_lock_interruptible(vdev-lock);
+
+ if (res)
+ return res;
+ }
if (video_is_registered(vdev))
ret = vdev-fops-read(filp, buf, sz, off);
if (vdev-lock)
@@ -208,8 +212,12 @@ static ssize_t v4l2_write(struct file *filp, const char
__user *buf,
if (!vdev-fops-write)
return -EINVAL;
- if (vdev-lock)
- mutex_lock(vdev-lock);
+ if (vdev-lock) {
+ int res = mutex_lock_interruptible(vdev-lock);
+
+ if (res)
+ return res;
+ }
if (video_is_registered(vdev))
ret = vdev-fops-write(filp, buf, sz, off);
if (vdev-lock)
@@ -224,8 +232,8 @@ static unsigned int v4l2_poll(struct file *filp, struct
poll_table_struct *poll)
if (!vdev-fops-poll)
return ret;
- if (vdev-lock)
- mutex_lock(vdev-lock);
+ if (vdev-lock mutex_lock_interruptible(vdev-lock))
+ return ret;
if (video_is_registered(vdev))
ret = vdev-fops-poll(filp, poll);
if (vdev-lock)
@@ -239,8 +247,12 @@ static long v4l2_ioctl(struct file *filp, unsigned int
cmd, unsigned long arg)
int ret = -ENODEV;
if (vdev-fops-unlocked_ioctl) {
- if (vdev-lock)
- mutex_lock(vdev-lock);
+ if (vdev-lock) {
+ int res = mutex_lock_interruptible(vdev-lock);
+
+ if (res)
+ return res;
+ }
if (video_is_registered(vdev))
ret = vdev-fops-unlocked_ioctl(filp, cmd, arg);
if (vdev-lock)
@@ -264,8 +276,12 @@ static int v4l2_mmap(struct file *filp, struct
vm_area_struct *vm)
if (!vdev-fops-mmap)
return ret;
- if (vdev-lock)
- mutex_lock(vdev-lock);
+ if (vdev-lock) {
+ int res = mutex_lock_interruptible(vdev-lock);
+
+ if (res)
+ return res;
+ }
if (video_is_registered(vdev))
ret = vdev-fops-mmap(filp, vm);
if (vdev-lock)
@@ -291,8 +307,11 @@ static int v4l2_open(struct inode *inode, struct file
*filp)
video_get(vdev);
mutex_unlock(videodev_lock);
if (vdev-fops-open) {
- if (vdev-lock)
- mutex_lock(vdev-lock);
+ if (vdev-lock) {
+ ret = mutex_lock_interruptible(vdev-lock);
+ if (ret)
+ goto err;
+ }
if (video_is_registered(vdev))
ret = vdev-fops-open(filp);
else
@@ -301,6 +320,7 @@ static int v4l2_open(struct inode *inode, struct file *filp)
mutex_unlock(vdev-lock);
}
+err:
/* decrease the refcount in case of an error */
if (ret)
video_put(vdev);
--
1.7.0.4
--
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
[RFCv2 PATCH 02/15] BKL: trivial BKL removal from V4L2 radio drivers
The patch converts a bunch of V4L2 radio drivers to unlocked_ioctl.
These are all simple conversions: most already had a lock and so the ioctl
fop could simply be replaced by unlocked_ioctl.
radio-miropcm20.c was converted to use the new V4L2 core lock.
While doing this work I noticed that many of these drivers initialized
some more fields or muted audio or something like that *after* creating
the device node. This should be done before the device node is created
to prevent problems. Especially hal tends to grab a device node as soon
as it is created.
In one or two cases the mutex_init was even done after the device creation!
Signed-off-by: Hans Verkuil hverk...@xs4all.nl
---
drivers/media/radio/radio-aimslab.c| 16
drivers/media/radio/radio-aztech.c |6 +++---
drivers/media/radio/radio-gemtek-pci.c |6 +++---
drivers/media/radio/radio-gemtek.c | 14 +++---
drivers/media/radio/radio-maestro.c| 14 ++
drivers/media/radio/radio-maxiradio.c |2 +-
drivers/media/radio/radio-miropcm20.c |6 --
drivers/media/radio/radio-rtrack2.c| 10 +-
drivers/media/radio/radio-sf16fmi.c|7 ---
drivers/media/radio/radio-sf16fmr2.c | 11 +--
drivers/media/radio/radio-terratec.c |8
drivers/media/radio/radio-trust.c | 18 +-
drivers/media/radio/radio-zoltrix.c| 30 +++---
13 files changed, 74 insertions(+), 74 deletions(-)
diff --git a/drivers/media/radio/radio-aimslab.c
b/drivers/media/radio/radio-aimslab.c
index 5bf4985..05e832f 100644
--- a/drivers/media/radio/radio-aimslab.c
+++ b/drivers/media/radio/radio-aimslab.c
@@ -361,7 +361,7 @@ static int vidioc_s_audio(struct file *file, void *priv,
static const struct v4l2_file_operations rtrack_fops = {
.owner = THIS_MODULE,
- .ioctl = video_ioctl2,
+ .unlocked_ioctl = video_ioctl2,
};
static const struct v4l2_ioctl_ops rtrack_ioctl_ops = {
@@ -412,13 +412,6 @@ static int __init rtrack_init(void)
rt-vdev.release = video_device_release_empty;
video_set_drvdata(rt-vdev, rt);
- if (video_register_device(rt-vdev, VFL_TYPE_RADIO, radio_nr) 0) {
- v4l2_device_unregister(rt-v4l2_dev);
- release_region(rt-io, 2);
- return -EINVAL;
- }
- v4l2_info(v4l2_dev, AIMSlab RadioTrack/RadioReveal card driver.\n);
-
/* Set up the I/O locking */
mutex_init(rt-lock);
@@ -430,6 +423,13 @@ static int __init rtrack_init(void)
sleep_delay(200); /* make sure it's totally down */
outb(0xc0, rt-io); /* steady volume, mute card */
+ if (video_register_device(rt-vdev, VFL_TYPE_RADIO, radio_nr) 0) {
+ v4l2_device_unregister(rt-v4l2_dev);
+ release_region(rt-io, 2);
+ return -EINVAL;
+ }
+ v4l2_info(v4l2_dev, AIMSlab RadioTrack/RadioReveal card driver.\n);
+
return 0;
}
diff --git a/drivers/media/radio/radio-aztech.c
b/drivers/media/radio/radio-aztech.c
index c223113..dd8a6ab 100644
--- a/drivers/media/radio/radio-aztech.c
+++ b/drivers/media/radio/radio-aztech.c
@@ -324,7 +324,7 @@ static int vidioc_s_ctrl(struct file *file, void *priv,
static const struct v4l2_file_operations aztech_fops = {
.owner = THIS_MODULE,
- .ioctl = video_ioctl2,
+ .unlocked_ioctl = video_ioctl2,
};
static const struct v4l2_ioctl_ops aztech_ioctl_ops = {
@@ -375,6 +375,8 @@ static int __init aztech_init(void)
az-vdev.ioctl_ops = aztech_ioctl_ops;
az-vdev.release = video_device_release_empty;
video_set_drvdata(az-vdev, az);
+ /* mute card - prevents noisy bootups */
+ outb(0, az-io);
if (video_register_device(az-vdev, VFL_TYPE_RADIO, radio_nr) 0) {
v4l2_device_unregister(v4l2_dev);
@@ -383,8 +385,6 @@ static int __init aztech_init(void)
}
v4l2_info(v4l2_dev, Aztech radio card driver v1.00/19990224
rkr...@exploits.org\n);
- /* mute card - prevents noisy bootups */
- outb(0, az-io);
return 0;
}
diff --git a/drivers/media/radio/radio-gemtek-pci.c
b/drivers/media/radio/radio-gemtek-pci.c
index 7903967..28fa85b 100644
--- a/drivers/media/radio/radio-gemtek-pci.c
+++ b/drivers/media/radio/radio-gemtek-pci.c
@@ -361,7 +361,7 @@ MODULE_DEVICE_TABLE(pci, gemtek_pci_id);
static const struct v4l2_file_operations gemtek_pci_fops = {
.owner = THIS_MODULE,
- .ioctl = video_ioctl2,
+ .unlocked_ioctl = video_ioctl2,
};
static const struct v4l2_ioctl_ops gemtek_pci_ioctl_ops = {
@@ -422,11 +422,11 @@ static int __devinit gemtek_pci_probe(struct pci_dev
*pdev, const struct pci_dev
card-vdev.release = video_device_release_empty;
video_set_drvdata(card-vdev, card);
+ gemtek_pci_mute(card);
+
[RFCv2 PATCH 03/15] cadet: use unlocked_ioctl
Converted from ioctl to unlocked_ioctl.
This driver already used an internal lock, but it was missing in cadet_open and
cadet_release and it was not used correctly in cadet_read.
Signed-off-by: Hans Verkuil hverk...@xs4all.nl
---
drivers/media/radio/radio-cadet.c | 12 +---
1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/media/radio/radio-cadet.c
b/drivers/media/radio/radio-cadet.c
index b701ea6..bc9ad08 100644
--- a/drivers/media/radio/radio-cadet.c
+++ b/drivers/media/radio/radio-cadet.c
@@ -328,11 +328,10 @@ static ssize_t cadet_read(struct file *file, char __user
*data, size_t count, lo
unsigned char readbuf[RDS_BUFFER];
int i = 0;
+ mutex_lock(dev-lock);
if (dev-rdsstat == 0) {
- mutex_lock(dev-lock);
dev-rdsstat = 1;
outb(0x80, dev-io);/* Select RDS fifo */
- mutex_unlock(dev-lock);
init_timer(dev-readtimer);
dev-readtimer.function = cadet_handler;
dev-readtimer.data = (unsigned long)dev;
@@ -340,12 +339,15 @@ static ssize_t cadet_read(struct file *file, char __user
*data, size_t count, lo
add_timer(dev-readtimer);
}
if (dev-rdsin == dev-rdsout) {
+ mutex_unlock(dev-lock);
if (file-f_flags O_NONBLOCK)
return -EWOULDBLOCK;
interruptible_sleep_on(dev-read_queue);
+ mutex_lock(dev-lock);
}
while (i count dev-rdsin != dev-rdsout)
readbuf[i++] = dev-rdsbuf[dev-rdsout++];
+ mutex_unlock(dev-lock);
if (copy_to_user(data, readbuf, i))
return -EFAULT;
@@ -525,9 +527,11 @@ static int cadet_open(struct file *file)
{
struct cadet *dev = video_drvdata(file);
+ mutex_lock(dev-lock);
dev-users++;
if (1 == dev-users)
init_waitqueue_head(dev-read_queue);
+ mutex_unlock(dev-lock);
return 0;
}
@@ -535,11 +539,13 @@ static int cadet_release(struct file *file)
{
struct cadet *dev = video_drvdata(file);
+ mutex_lock(dev-lock);
dev-users--;
if (0 == dev-users) {
del_timer_sync(dev-readtimer);
dev-rdsstat = 0;
}
+ mutex_unlock(dev-lock);
return 0;
}
@@ -559,7 +565,7 @@ static const struct v4l2_file_operations cadet_fops = {
.open = cadet_open,
.release= cadet_release,
.read = cadet_read,
- .ioctl = video_ioctl2,
+ .unlocked_ioctl = video_ioctl2,
.poll = cadet_poll,
};
--
1.7.0.4
--
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
[RFCv2 PATCH 04/15] tea5764: convert to unlocked_ioctl
Convert from ioctl to unlocked_ioctl using the v4l2 core lock.
Also removed the 'exclusive access' limitation. There was no need for it
and it violates the v4l2 spec as well.
Signed-off-by: Hans Verkuil hverk...@xs4all.nl
---
drivers/media/radio/radio-tea5764.c | 49 ++
1 files changed, 9 insertions(+), 40 deletions(-)
diff --git a/drivers/media/radio/radio-tea5764.c
b/drivers/media/radio/radio-tea5764.c
index 789d2ec..0e71d81 100644
--- a/drivers/media/radio/radio-tea5764.c
+++ b/drivers/media/radio/radio-tea5764.c
@@ -142,7 +142,6 @@ struct tea5764_device {
struct video_device *videodev;
struct tea5764_regs regs;
struct mutexmutex;
- int users;
};
/* I2C code related */
@@ -458,41 +457,10 @@ static int vidioc_s_audio(struct file *file, void *priv,
return 0;
}
-static int tea5764_open(struct file *file)
-{
- /* Currently we support only one device */
- struct tea5764_device *radio = video_drvdata(file);
-
- mutex_lock(radio-mutex);
- /* Only exclusive access */
- if (radio-users) {
- mutex_unlock(radio-mutex);
- return -EBUSY;
- }
- radio-users++;
- mutex_unlock(radio-mutex);
- file-private_data = radio;
- return 0;
-}
-
-static int tea5764_close(struct file *file)
-{
- struct tea5764_device *radio = video_drvdata(file);
-
- if (!radio)
- return -ENODEV;
- mutex_lock(radio-mutex);
- radio-users--;
- mutex_unlock(radio-mutex);
- return 0;
-}
-
/* File system interface */
static const struct v4l2_file_operations tea5764_fops = {
.owner = THIS_MODULE,
- .open = tea5764_open,
- .release= tea5764_close,
- .ioctl = video_ioctl2,
+ .unlocked_ioctl = video_ioctl2,
};
static const struct v4l2_ioctl_ops tea5764_ioctl_ops = {
@@ -527,7 +495,7 @@ static int __devinit tea5764_i2c_probe(struct i2c_client
*client,
int ret;
PDEBUG(probe);
- radio = kmalloc(sizeof(struct tea5764_device), GFP_KERNEL);
+ radio = kzalloc(sizeof(struct tea5764_device), GFP_KERNEL);
if (!radio)
return -ENOMEM;
@@ -555,12 +523,7 @@ static int __devinit tea5764_i2c_probe(struct i2c_client
*client,
i2c_set_clientdata(client, radio);
video_set_drvdata(radio-videodev, radio);
-
- ret = video_register_device(radio-videodev, VFL_TYPE_RADIO, radio_nr);
- if (ret 0) {
- PWARN(Could not register video device!);
- goto errrel;
- }
+ radio-videodev-lock = radio-mutex;
/* initialize and power off the chip */
tea5764_i2c_read(radio);
@@ -568,6 +531,12 @@ static int __devinit tea5764_i2c_probe(struct i2c_client
*client,
tea5764_mute(radio, 1);
tea5764_power_down(radio);
+ ret = video_register_device(radio-videodev, VFL_TYPE_RADIO, radio_nr);
+ if (ret 0) {
+ PWARN(Could not register video device!);
+ goto errrel;
+ }
+
PINFO(registered.);
return 0;
errrel:
--
1.7.0.4
--
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
[RFCv2 PATCH 08/15] BKL: trivial ioctl - unlocked_ioctl video driver conversions
These drivers could be trivially converted to unlocked_ioctl since they
already did locking.
Signed-off-by: Hans Verkuil hverk...@xs4all.nl
---
drivers/media/video/arv.c |2 +-
drivers/media/video/bw-qcam.c |2 +-
drivers/media/video/c-qcam.c |2 +-
drivers/media/video/meye.c| 14 +++---
drivers/media/video/pms.c |2 +-
drivers/media/video/w9966.c |2 +-
6 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/media/video/arv.c b/drivers/media/video/arv.c
index 31e7a12..f989f28 100644
--- a/drivers/media/video/arv.c
+++ b/drivers/media/video/arv.c
@@ -712,7 +712,7 @@ static int ar_initialize(struct ar *ar)
static const struct v4l2_file_operations ar_fops = {
.owner = THIS_MODULE,
.read = ar_read,
- .ioctl = video_ioctl2,
+ .unlocked_ioctl = video_ioctl2,
};
static const struct v4l2_ioctl_ops ar_ioctl_ops = {
diff --git a/drivers/media/video/bw-qcam.c b/drivers/media/video/bw-qcam.c
index 935e0c9..c119350 100644
--- a/drivers/media/video/bw-qcam.c
+++ b/drivers/media/video/bw-qcam.c
@@ -860,7 +860,7 @@ static ssize_t qcam_read(struct file *file, char __user
*buf,
static const struct v4l2_file_operations qcam_fops = {
.owner = THIS_MODULE,
- .ioctl = video_ioctl2,
+ .unlocked_ioctl = video_ioctl2,
.read = qcam_read,
};
diff --git a/drivers/media/video/c-qcam.c b/drivers/media/video/c-qcam.c
index 6e4b196..24fc009 100644
--- a/drivers/media/video/c-qcam.c
+++ b/drivers/media/video/c-qcam.c
@@ -718,7 +718,7 @@ static ssize_t qcam_read(struct file *file, char __user
*buf,
static const struct v4l2_file_operations qcam_fops = {
.owner = THIS_MODULE,
- .ioctl = video_ioctl2,
+ .unlocked_ioctl = video_ioctl2,
.read = qcam_read,
};
diff --git a/drivers/media/video/meye.c b/drivers/media/video/meye.c
index 2be23bc..48d2c24 100644
--- a/drivers/media/video/meye.c
+++ b/drivers/media/video/meye.c
@@ -1659,7 +1659,7 @@ static const struct v4l2_file_operations meye_fops = {
.open = meye_open,
.release= meye_release,
.mmap = meye_mmap,
- .ioctl = video_ioctl2,
+ .unlocked_ioctl = video_ioctl2,
.poll = meye_poll,
};
@@ -1831,12 +1831,6 @@ static int __devinit meye_probe(struct pci_dev *pcidev,
msleep(1);
mchip_set(MCHIP_MM_INTA, MCHIP_MM_INTA_HIC_1_MASK);
- if (video_register_device(meye.vdev, VFL_TYPE_GRABBER,
- video_nr) 0) {
- v4l2_err(v4l2_dev, video_register_device failed\n);
- goto outvideoreg;
- }
-
mutex_init(meye.lock);
init_waitqueue_head(meye.proc_list);
meye.brightness = 32 10;
@@ -1858,6 +1852,12 @@ static int __devinit meye_probe(struct pci_dev *pcidev,
sony_pic_camera_command(SONY_PIC_COMMAND_SETCAMERAPICTURE, 0);
sony_pic_camera_command(SONY_PIC_COMMAND_SETCAMERAAGC, 48);
+ if (video_register_device(meye.vdev, VFL_TYPE_GRABBER,
+ video_nr) 0) {
+ v4l2_err(v4l2_dev, video_register_device failed\n);
+ goto outvideoreg;
+ }
+
v4l2_info(v4l2_dev, Motion Eye Camera Driver v%s.\n,
MEYE_DRIVER_VERSION);
v4l2_info(v4l2_dev, mchip KL5A72002 rev. %d, base %lx, irq %d\n,
diff --git a/drivers/media/video/pms.c b/drivers/media/video/pms.c
index 7129b50..7551907 100644
--- a/drivers/media/video/pms.c
+++ b/drivers/media/video/pms.c
@@ -932,7 +932,7 @@ static ssize_t pms_read(struct file *file, char __user *buf,
static const struct v4l2_file_operations pms_fops = {
.owner = THIS_MODULE,
- .ioctl = video_ioctl2,
+ .unlocked_ioctl = video_ioctl2,
.read = pms_read,
};
diff --git a/drivers/media/video/w9966.c b/drivers/media/video/w9966.c
index 635420d..019ee20 100644
--- a/drivers/media/video/w9966.c
+++ b/drivers/media/video/w9966.c
@@ -815,7 +815,7 @@ out:
static const struct v4l2_file_operations w9966_fops = {
.owner = THIS_MODULE,
- .ioctl = video_ioctl2,
+ .unlocked_ioctl = video_ioctl2,
.read = w9966_v4l_read,
};
--
1.7.0.4
--
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
[RFCv2 PATCH 09/15] sn9c102: convert to unlocked_ioctl.
Trivial conversion, this driver used a mutex already.
Signed-off-by: Hans Verkuil hverk...@xs4all.nl
---
drivers/media/video/sn9c102/sn9c102_core.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/sn9c102/sn9c102_core.c
b/drivers/media/video/sn9c102/sn9c102_core.c
index 28e19da..f49fbfb 100644
--- a/drivers/media/video/sn9c102/sn9c102_core.c
+++ b/drivers/media/video/sn9c102/sn9c102_core.c
@@ -3238,7 +3238,7 @@ static const struct v4l2_file_operations sn9c102_fops = {
.owner = THIS_MODULE,
.open = sn9c102_open,
.release = sn9c102_release,
- .ioctl = sn9c102_ioctl,
+ .unlocked_ioctl = sn9c102_ioctl,
.read = sn9c102_read,
.poll = sn9c102_poll,
.mmap = sn9c102_mmap,
--
1.7.0.4
--
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
[RFCv2 PATCH 11/15] cafe_ccic: replace ioctl by unlocked_ioctl.
Trivial change, approved by Jonathan Corbet cor...@lwn.net.
Signed-off-by: Hans Verkuil hverk...@xs4all.nl
---
drivers/media/video/cafe_ccic.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/cafe_ccic.c b/drivers/media/video/cafe_ccic.c
index d147525..f85c933 100644
--- a/drivers/media/video/cafe_ccic.c
+++ b/drivers/media/video/cafe_ccic.c
@@ -1773,7 +1773,7 @@ static const struct v4l2_file_operations cafe_v4l_fops = {
.read = cafe_v4l_read,
.poll = cafe_v4l_poll,
.mmap = cafe_v4l_mmap,
- .ioctl = video_ioctl2,
+ .unlocked_ioctl = video_ioctl2,
};
static const struct v4l2_ioctl_ops cafe_v4l_ioctl_ops = {
--
1.7.0.4
--
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
[RFCv2 PATCH 15/15] cx18: convert to unlocked_ioctl.
Signed-off-by: Hans Verkuil hverk...@xs4all.nl
---
drivers/media/video/cx18/cx18-streams.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/cx18/cx18-streams.c
b/drivers/media/video/cx18/cx18-streams.c
index 9045f1e..ab461e2 100644
--- a/drivers/media/video/cx18/cx18-streams.c
+++ b/drivers/media/video/cx18/cx18-streams.c
@@ -41,7 +41,7 @@ static struct v4l2_file_operations cx18_v4l2_enc_fops = {
.read = cx18_v4l2_read,
.open = cx18_v4l2_open,
/* FIXME change to video_ioctl2 if serialization lock can be removed */
- .ioctl = cx18_v4l2_ioctl,
+ .unlocked_ioctl = cx18_v4l2_ioctl,
.release = cx18_v4l2_close,
.poll = cx18_v4l2_enc_poll,
};
--
1.7.0.4
--
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
[RFCv2 PATCH 05/15] si4713: convert to unlocked_ioctl
Convert ioctl to unlocked_ioctl. Note that for this driver the locking
is done inside the sub-device.
Signed-off-by: Hans Verkuil hverk...@xs4all.nl
---
drivers/media/radio/radio-si4713.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/media/radio/radio-si4713.c
b/drivers/media/radio/radio-si4713.c
index 6a43578..3a84c3d 100644
--- a/drivers/media/radio/radio-si4713.c
+++ b/drivers/media/radio/radio-si4713.c
@@ -53,7 +53,8 @@ struct radio_si4713_device {
/* radio_si4713_fops - file operations interface */
static const struct v4l2_file_operations radio_si4713_fops = {
.owner = THIS_MODULE,
- .ioctl = video_ioctl2,
+ /* Note: locking is done at the subdev level in the i2c driver. */
+ .unlocked_ioctl = video_ioctl2,
};
/* Video4Linux Interface */
--
1.7.0.4
--
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
[RFCv2 PATCH 10/15] et61x251_core: trivial conversion to unlocked_ioctl.
Signed-off-by: Hans Verkuil hverk...@xs4all.nl
---
drivers/media/video/et61x251/et61x251_core.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/et61x251/et61x251_core.c
b/drivers/media/video/et61x251/et61x251_core.c
index a5cfc76..bb16409 100644
--- a/drivers/media/video/et61x251/et61x251_core.c
+++ b/drivers/media/video/et61x251/et61x251_core.c
@@ -2530,7 +2530,7 @@ static const struct v4l2_file_operations et61x251_fops = {
.owner = THIS_MODULE,
.open =et61x251_open,
.release = et61x251_release,
- .ioctl = et61x251_ioctl,
+ .unlocked_ioctl = et61x251_ioctl,
.read =et61x251_read,
.poll =et61x251_poll,
.mmap =et61x251_mmap,
--
1.7.0.4
--
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
[RFCv2 PATCH 14/15] V4L: improve the BKL replacement heuristic
The BKL replacement mutex had some serious performance side-effects on
V4L drivers. It is replaced by a better heuristic that works around the
worst of the side-effects.
Read the v4l2-dev.c comments for the whole sorry story. This is a
temporary measure only until we can convert all v4l drivers to use
unlocked_ioctl.
Signed-off-by: Hans Verkuil hverk...@xs4all.nl
---
drivers/media/video/v4l2-dev.c| 37 ++---
drivers/media/video/v4l2-device.c |1 +
include/media/v4l2-dev.h |2 +-
include/media/v4l2-device.h |2 ++
4 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
index 8eb0756..59ef642 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -258,11 +258,42 @@ static long v4l2_ioctl(struct file *filp, unsigned int
cmd, unsigned long arg)
if (vdev-lock)
mutex_unlock(vdev-lock);
} else if (vdev-fops-ioctl) {
- /* TODO: convert all drivers to unlocked_ioctl */
- lock_kernel();
+ /* This code path is a replacement for the BKL. It is a major
+* hack but it will have to do for those drivers that are not
+* yet converted to use unlocked_ioctl.
+*
+* There are two options: if the driver implements struct
+* v4l2_device, then the lock defined there is used to
+* serialize the ioctls. Otherwise the v4l2 core lock defined
+* below is used. This lock is really bad since it serializes
+* completely independent devices.
+*
+* Both variants suffer from the same problem: if the driver
+* sleeps, then it blocks all ioctls since the lock is still
+* held. This is very common for VIDIOC_DQBUF since that
+* normally waits for a frame to arrive. As a result any other
+* ioctl calls will proceed very, very slowly since each call
+* will have to wait for the VIDIOC_QBUF to finish. Things that
+* should take 0.01s may now take 10-20 seconds.
+*
+* The workaround is to *not* take the lock for VIDIOC_DQBUF.
+* This actually works OK for videobuf-based drivers, since
+* videobuf will take its own internal lock.
+*/
+ static DEFINE_MUTEX(v4l2_ioctl_mutex);
+ struct mutex *m = vdev-v4l2_dev ?
+ vdev-v4l2_dev-ioctl_lock : v4l2_ioctl_mutex;
+
+ if (cmd != VIDIOC_DQBUF) {
+ int res = mutex_lock_interruptible(m);
+
+ if (res)
+ return res;
+ }
if (video_is_registered(vdev))
ret = vdev-fops-ioctl(filp, cmd, arg);
- unlock_kernel();
+ if (cmd != VIDIOC_DQBUF)
+ mutex_unlock(m);
} else
ret = -ENOTTY;
diff --git a/drivers/media/video/v4l2-device.c
b/drivers/media/video/v4l2-device.c
index 0b08f96..7fe6f92 100644
--- a/drivers/media/video/v4l2-device.c
+++ b/drivers/media/video/v4l2-device.c
@@ -35,6 +35,7 @@ int v4l2_device_register(struct device *dev, struct
v4l2_device *v4l2_dev)
INIT_LIST_HEAD(v4l2_dev-subdevs);
spin_lock_init(v4l2_dev-lock);
+ mutex_init(v4l2_dev-ioctl_lock);
v4l2_dev-dev = dev;
if (dev == NULL) {
/* If dev == NULL, then name must be filled in by the caller */
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index 15802a0..59dec5a 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -39,7 +39,7 @@ struct v4l2_file_operations {
ssize_t (*read) (struct file *, char __user *, size_t, loff_t *);
ssize_t (*write) (struct file *, const char __user *, size_t, loff_t *);
unsigned int (*poll) (struct file *, struct poll_table_struct *);
- long (*ioctl) (struct file *, unsigned int, unsigned long);
+ long (*ioctl __deprecated) (struct file *, unsigned int, unsigned long);
long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
int (*mmap) (struct file *, struct vm_area_struct *);
int (*open) (struct file *);
diff --git a/include/media/v4l2-device.h b/include/media/v4l2-device.h
index 6648036..b16f307 100644
--- a/include/media/v4l2-device.h
+++ b/include/media/v4l2-device.h
@@ -51,6 +51,8 @@ struct v4l2_device {
unsigned int notification, void *arg);
/* The control handler. May be NULL. */
struct v4l2_ctrl_handler *ctrl_handler;
+ /* BKL replacement mutex. Temporary solution only. */
+ struct mutex ioctl_lock;
};
/* Initialize v4l2_dev and make dev-driver_data
[RFCv2 PATCH 13/15] radio-timb: convert to unlocked_ioctl.
Signed-off-by: Hans Verkuil hverk...@xs4all.nl
---
drivers/media/radio/radio-timb.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/drivers/media/radio/radio-timb.c b/drivers/media/radio/radio-timb.c
index b8bb3ef..a185610 100644
--- a/drivers/media/radio/radio-timb.c
+++ b/drivers/media/radio/radio-timb.c
@@ -34,6 +34,7 @@ struct timbradio {
struct v4l2_subdev *sd_dsp;
struct video_device video_dev;
struct v4l2_device v4l2_dev;
+ struct mutexlock;
};
@@ -142,7 +143,7 @@ static const struct v4l2_ioctl_ops timbradio_ioctl_ops = {
static const struct v4l2_file_operations timbradio_fops = {
.owner = THIS_MODULE,
- .ioctl = video_ioctl2,
+ .unlocked_ioctl = video_ioctl2,
};
static int __devinit timbradio_probe(struct platform_device *pdev)
@@ -164,6 +165,7 @@ static int __devinit timbradio_probe(struct platform_device
*pdev)
}
tr-pdata = *pdata;
+ mutex_init(tr-lock);
strlcpy(tr-video_dev.name, Timberdale Radio,
sizeof(tr-video_dev.name));
@@ -171,6 +173,7 @@ static int __devinit timbradio_probe(struct platform_device
*pdev)
tr-video_dev.ioctl_ops = timbradio_ioctl_ops;
tr-video_dev.release = video_device_release_empty;
tr-video_dev.minor = -1;
+ tr-video_dev.lock = tr-lock;
strlcpy(tr-v4l2_dev.name, DRIVER_NAME, sizeof(tr-v4l2_dev.name));
err = v4l2_device_register(NULL, tr-v4l2_dev);
--
1.7.0.4
--
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
[RFCv2 PATCH 12/15] sh_vou: convert to unlocked_ioctl.
Signed-off-by: Hans Verkuil hverk...@xs4all.nl
---
drivers/media/video/sh_vou.c | 13 -
1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/media/video/sh_vou.c b/drivers/media/video/sh_vou.c
index 0f49061..858b2f8 100644
--- a/drivers/media/video/sh_vou.c
+++ b/drivers/media/video/sh_vou.c
@@ -75,6 +75,7 @@ struct sh_vou_device {
int pix_idx;
struct videobuf_buffer *active;
enum sh_vou_status status;
+ struct mutex fop_lock;
};
struct sh_vou_file {
@@ -235,7 +236,7 @@ static void free_buffer(struct videobuf_queue *vq, struct
videobuf_buffer *vb)
vb-state = VIDEOBUF_NEEDS_INIT;
}
-/* Locking: caller holds vq-vb_lock mutex */
+/* Locking: caller holds fop_lock mutex */
static int sh_vou_buf_setup(struct videobuf_queue *vq, unsigned int *count,
unsigned int *size)
{
@@ -257,7 +258,7 @@ static int sh_vou_buf_setup(struct videobuf_queue *vq,
unsigned int *count,
return 0;
}
-/* Locking: caller holds vq-vb_lock mutex */
+/* Locking: caller holds fop_lock mutex */
static int sh_vou_buf_prepare(struct videobuf_queue *vq,
struct videobuf_buffer *vb,
enum v4l2_field field)
@@ -306,7 +307,7 @@ static int sh_vou_buf_prepare(struct videobuf_queue *vq,
return 0;
}
-/* Locking: caller holds vq-vb_lock mutex and vq-irqlock spinlock */
+/* Locking: caller holds fop_lock mutex and vq-irqlock spinlock */
static void sh_vou_buf_queue(struct videobuf_queue *vq,
struct videobuf_buffer *vb)
{
@@ -1190,7 +1191,7 @@ static int sh_vou_open(struct file *file)
V4L2_BUF_TYPE_VIDEO_OUTPUT,
V4L2_FIELD_NONE,
sizeof(struct videobuf_buffer), vdev,
- NULL);
+ vou_dev-fop_lock);
return 0;
}
@@ -1292,7 +1293,7 @@ static const struct v4l2_file_operations sh_vou_fops = {
.owner = THIS_MODULE,
.open = sh_vou_open,
.release= sh_vou_release,
- .ioctl = video_ioctl2,
+ .unlocked_ioctl = video_ioctl2,
.mmap = sh_vou_mmap,
.poll = sh_vou_poll,
};
@@ -1331,6 +1332,7 @@ static int __devinit sh_vou_probe(struct platform_device
*pdev)
INIT_LIST_HEAD(vou_dev-queue);
spin_lock_init(vou_dev-lock);
+ mutex_init(vou_dev-fop_lock);
atomic_set(vou_dev-use_count, 0);
vou_dev-pdata = vou_pdata;
vou_dev-status = SH_VOU_IDLE;
@@ -1388,6 +1390,7 @@ static int __devinit sh_vou_probe(struct platform_device
*pdev)
vdev-tvnorms |= V4L2_STD_PAL;
vdev-v4l2_dev = vou_dev-v4l2_dev;
vdev-release = video_device_release;
+ vdev-lock = vou_dev-fop_lock;
vou_dev-vdev = vdev;
video_set_drvdata(vdev, vou_dev);
--
1.7.0.4
--
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
[RFCv2 PATCH 06/15] typhoon: convert to unlocked_ioctl.
Convert the typhoon driver from ioctl to unlocked_ioctl.
When doing this I noticed a bug where curfreq was not initialized correctly
to mutefreq (it wasn't multiplied by 16).
The initialization is now also done before the device node is created.
Signed-off-by: Hans Verkuil hverk...@xs4all.nl
---
drivers/media/radio/radio-typhoon.c | 16
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/media/radio/radio-typhoon.c
b/drivers/media/radio/radio-typhoon.c
index b1f6305..8dbbf08 100644
--- a/drivers/media/radio/radio-typhoon.c
+++ b/drivers/media/radio/radio-typhoon.c
@@ -317,7 +317,7 @@ static int vidioc_log_status(struct file *file, void *priv)
static const struct v4l2_file_operations typhoon_fops = {
.owner = THIS_MODULE,
- .ioctl = video_ioctl2,
+ .unlocked_ioctl = video_ioctl2,
};
static const struct v4l2_ioctl_ops typhoon_ioctl_ops = {
@@ -344,18 +344,18 @@ static int __init typhoon_init(void)
strlcpy(v4l2_dev-name, typhoon, sizeof(v4l2_dev-name));
dev-io = io;
- dev-curfreq = dev-mutefreq = mutefreq;
if (dev-io == -1) {
v4l2_err(v4l2_dev, You must set an I/O address with io=0x316
or io=0x336\n);
return -EINVAL;
}
- if (dev-mutefreq 87000 || dev-mutefreq 108500) {
+ if (mutefreq 87000 || mutefreq 108500) {
v4l2_err(v4l2_dev, You must set a frequency (in kHz) used when
muting the card,\n);
v4l2_err(v4l2_dev, e.g. with \mutefreq=87500\ (87000 =
mutefreq = 108500)\n);
return -EINVAL;
}
+ dev-curfreq = dev-mutefreq = mutefreq 4;
mutex_init(dev-lock);
if (!request_region(dev-io, 8, typhoon)) {
@@ -378,17 +378,17 @@ static int __init typhoon_init(void)
dev-vdev.ioctl_ops = typhoon_ioctl_ops;
dev-vdev.release = video_device_release_empty;
video_set_drvdata(dev-vdev, dev);
+
+ /* mute card - prevents noisy bootups */
+ typhoon_mute(dev);
+
if (video_register_device(dev-vdev, VFL_TYPE_RADIO, radio_nr) 0) {
v4l2_device_unregister(dev-v4l2_dev);
release_region(dev-io, 8);
return -EINVAL;
}
v4l2_info(v4l2_dev, port 0x%x.\n, dev-io);
- v4l2_info(v4l2_dev, mute frequency is %lu kHz.\n, dev-mutefreq);
- dev-mutefreq = 4;
-
- /* mute card - prevents noisy bootups */
- typhoon_mute(dev);
+ v4l2_info(v4l2_dev, mute frequency is %lu kHz.\n, mutefreq);
return 0;
}
--
1.7.0.4
--
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
An article on the media controller
I've just spent a fair while looking through the September posting of the media controller code (is there a more recent version?). The result is a high-level review which interested people can read here: http://lwn.net/SubscriberLink/415714/1e837f01b8579eb7/ Most people will not see it for another 24 hours or so; if there's something I got radically wrong, I'd appreciate hearing about it. The executive summary is that I think this code really needs some exposure outside of the V4L2 list; I'd encourage posting it to linux-kernel. That could be hard on plans for a 2.6.38 merge (or, at least, plans for any spare time between now and then), but the end result might be better for everybody. I have some low-level comments too which were not suitable for the article. I'll be posting them here, but I have to get some other things done first. Thanks, jon -- 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: Question about setting V4L2_CID_AUTO_WHITE_BALANCE control to FALSE
Nevermind. I think behavior #1 makes more sense. -Shuzhen -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media-ow...@vger.kernel.org] On Behalf Of Shuzhen Wang Sent: Tuesday, November 16, 2010 11:30 AM To: linux-media@vger.kernel.org Subject: Question about setting V4L2_CID_AUTO_WHITE_BALANCE control to FALSE Hello, When I set V4L2_CID_AUTO_WHITE_BALANCE control to FALSE, which one of the following is the expected behavior? 1. Hold the current white balance settting. 2. Set the white balance to whatever V4L2_CID_WHITE_BALANCE_TEMPERATURE control is set to. The V4L2 API spec doesn't specify this clearly. Regards, -Shuzhen -- 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 -- 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: [RFCv2 PATCH 15/15] cx18: convert to unlocked_ioctl.
On Tue, 2010-11-16 at 22:56 +0100, Hans Verkuil wrote:
Signed-off-by: Hans Verkuil hverk...@xs4all.nl
---
drivers/media/video/cx18/cx18-streams.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/cx18/cx18-streams.c
b/drivers/media/video/cx18/cx18-streams.c
index 9045f1e..ab461e2 100644
--- a/drivers/media/video/cx18/cx18-streams.c
+++ b/drivers/media/video/cx18/cx18-streams.c
@@ -41,7 +41,7 @@ static struct v4l2_file_operations cx18_v4l2_enc_fops = {
.read = cx18_v4l2_read,
.open = cx18_v4l2_open,
/* FIXME change to video_ioctl2 if serialization lock can be removed */
- .ioctl = cx18_v4l2_ioctl,
+ .unlocked_ioctl = cx18_v4l2_ioctl,
.release = cx18_v4l2_close,
.poll = cx18_v4l2_enc_poll,
};
Hans,
Because I haven't done my homework on the ALSA API, could you also add
snd_cx18_lock()/snd_cx18_unlock() in snd_cx18_pcm_ioctl()?
Devin,
Do you know off the top of your head if any other operations in
cx18-alsa-* may need additional locking?
I am an ALSA API callback idiot. :)
Regards,
Andy
--
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: An article on the media controller
On Tue, 2010-11-16 at 15:18 -0700, Jonathan Corbet wrote: I've just spent a fair while looking through the September posting of the media controller code (is there a more recent version?). The result is a high-level review which interested people can read here: http://lwn.net/SubscriberLink/415714/1e837f01b8579eb7/ As I understand it, the current code patches are a subset of the ultimate desired/planned functionality. So I would need to think about this statement: Given that the configuration interface changes a single bit at a time, there is no need for the sort of transactional functionality that can make ioctl() preferable to sysfs. It may very well apply to the current patches, but I'd have to think about if multiple items would need to be set or queried at one time. You might just want to add the qualifier current when referring to the interface changes. Regards, Andy Most people will not see it for another 24 hours or so; if there's something I got radically wrong, I'd appreciate hearing about it. The executive summary is that I think this code really needs some exposure outside of the V4L2 list; I'd encourage posting it to linux-kernel. That could be hard on plans for a 2.6.38 merge (or, at least, plans for any spare time between now and then), but the end result might be better for everybody. I have some low-level comments too which were not suitable for the article. I'll be posting them here, but I have to get some other things done first. Thanks, jon -- 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 -- 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 PATCH 0/2] Apple remote support
On Tue, Nov 16, 2010 at 10:08:29AM -0200, Mauro Carvalho Chehab wrote: Em 15-11-2010 02:11, Jarod Wilson escreveu: Gives us support for using the full 32-bit codes right now w/o having to change any tables yet, but does require a modparam to skip the checksum checks, unless its an apple remote which we already know the vendor bytes for. I do think I'm ultimately leaning towards just doing the full 32 bits for all nec extended though -- optionally, we might include a modparam to *enable* the checksum check for those that want strict compliance (but I'd still say use the full 32 bits). The only issue I see with going to the full 32 right now is that we have all these nec tables with only 24 bits, and we don't know ... oh, wait, no, nevermind... We *do* know the missing 8 bits, since they have to fulfill the checksum check for command ^ not_command. So yeah, I'd say 32-bit scancodes for all nec extended, don't enforce the checksum by default with a module option (or sysfs knob) to enable checksum compliance. A modprobe parameter for it doesn't seem right. Users should not need to do any manual hack for ther RC to work, if the keycode table is ok. Agreed. There are already too many weird driver-specific modparams in use as is. Also, changing the current tables to 32 bits will break userspace API, as all userspace keytables for NEC will stop working, all due to a few vendors that decided to abuse on the NEC protocol. This doesn't sound fair. The NEC protocol is hardly a standard. There's lot's of variations out there. And intentionally throwing away information inside the kernel doesn't sound fair either. Considering that the new setkeycode/getkeycode ioctls support a variable size for scancodes, it seems to me that the proper solution is properly add support for variable-size scancode tables. By doing this, one of the properties for a scancode table is the size of the scancode. The NEC decoding logic can take the scancode size into account, when deciding to check checksum or not. With that approach you'd have to have the same scancode mangling code in each driver which generates NEC scancodes as well as in the nec raw decoder. One suggestion would be to use a full 32-bit scancode table, but use the size from the ioctl to determine how to generate the scancode to be inserted into the table. So if the ioctl was called with a 2 byte scancode, assume NEC with addr(8 bits) + cmd(8 bits); 3 byte - NEC Extended with addr(16 bits) + cmd(8 bits); 4 byte - 32 bit scancode. That way the nec decoder and other scancode drivers can be kept simple, the scancode table has a full 32 bit scancode for all keys and userspace won't see the difference (though I still think we should use the new large scancode API to let userspace properly indicate the protocol along with the scancode in the future). -- David Härdeman -- 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
media_tree.git weirdness
Mauro, the current state of the staging/for_v2.6.38 branch seems a bit weird with regard to the three patches to use raw decoding for the Encore FM 5.3, resulting cleanups to saa7134 and removal of ir-common. I still see drivers/media/rc/ir-functions.c in my local repo (and using the web interface), and the cleanups to saa7134 do not seem to be there... -- David Härdeman -- 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
Translation faults with OMAP ISP
Laurent, I am getting iommu translation errors when I try to use the CCDC output after using the Resizer output. If I use the CCDC output to stream some video, then close it down, switch to the Resizer output and open it up and try to stream, I get the following errors spewing out: omap-iommu omap-iommu.0: omap2_iommu_fault_isr: da:00d0ef00 translation fault omap-iommu omap-iommu.0: iommu_fault_handler: da:00d0ef00 pgd:ce664034 *pgd: and the select times out. From a fresh boot, I can stream just fine from the Resizer and then switch to the CCDC output just fine. It is only when I go from the CCDC to the Resizer that I get this problem. Furthermore, when it gets into this state, then anything dev node I try to use has the translation errors and the only way to recover is to reboot. Any ideas on the problem? Thanks, Lane -- 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/PATCH v5 01/12] media: Media device node support
Here's the first of my more specific comments - probably the only file
I'll get to today...
The media_devnode structure provides support for registering and
unregistering character devices using a dynamic major number. Reference
counting is handled internally, making device drivers easier to write
without having to solve the open/disconnect race condition issue over
and over again.
My first impression is that you're creating a simplified wrapper around
chardevs - essentially a reimplementation of the miscdevice subsystem. If
we really need this, it seems like it should be made something more
generic.
I wonder, though, why this functionality isn't just folded into struct
media_device? Do you plan to use it elsewhere as well?
[...]
+static unsigned int media_poll(struct file *filp,
+struct poll_table_struct *poll)
+{
+ struct media_devnode *mdev = media_devnode_data(filp);
+
+ if (!mdev-fops-poll || !media_devnode_is_registered(mdev))
+ return DEFAULT_POLLMASK;
+ return mdev-fops-poll(filp, poll);
+}
If it's not registered, I would expect poll() to return an error.
[...]
+/**
+ * media_devnode_register - register a media device node
+ * @mdev: media device node structure we want to register
+ *
+ * The registration code assigns minor numbers and registers the new device
node
+ * with the kernel. An error is returned if no free minor number can be
found,
+ * or if the registration of the device node fails.
+ *
+ * Zero is returned on success.
+ *
+ * Note that if the media_devnode_register call fails, the release()
callback of
+ * the media_devnode structure is *not* called, so the caller is responsible
for
+ * freeing any data.
+ */
+int __must_check media_devnode_register(struct media_devnode *mdev)
+{
+ int minor;
+ int ret;
+
+ /* Part 1: Find a free minor number */
+ mutex_lock(media_devnode_lock);
+ minor = find_next_zero_bit(media_devnode_nums, 0, MEDIA_NUM_DEVICES);
+ if (minor == MEDIA_NUM_DEVICES) {
+ mutex_unlock(media_devnode_lock);
+ printk(KERN_ERR could not get a free minor\n);
+ return -ENFILE;
+ }
+
+ set_bit(mdev-minor, media_devnode_nums);
+ mutex_unlock(media_devnode_lock);
+
+ mdev-minor = minor;
+
+ /* Part 2: Initialize and register the character device */
+ cdev_init(mdev-cdev, media_devnode_fops);
+ mdev-cdev.owner = mdev-fops-owner;
+
+ ret = cdev_add(mdev-cdev, MKDEV(MAJOR(media_dev_t), mdev-minor), 1);
+ if (ret 0) {
+ printk(KERN_ERR %s: cdev_add failed\n, __func__);
+ goto error;
+ }
+
+ /* Part 3: Register the media device */
+ mdev-dev.class = media_class;
There is a real push to get rid of classes. Is there a need for this class
here? It doesn't look like you attach any attributes to it.
+ mdev-dev.devt = MKDEV(MAJOR(media_dev_t), mdev-minor);
+ mdev-dev.release = media_devnode_release;
+ if (mdev-parent)
+ mdev-dev.parent = mdev-parent;
+ dev_set_name(mdev-dev, media%d, mdev-minor);
+ ret = device_register(mdev-dev);
+ if (ret 0) {
+ printk(KERN_ERR %s: device_register failed\n, __func__);
+ goto error;
+ }
+
+ /* Part 4: Activate this minor. The char device can now be used. */
+ set_bit(MEDIA_FLAG_REGISTERED, mdev-flags);
+
+ return 0;
+
+error:
+ cdev_del(mdev-cdev);
+ mutex_lock(media_devnode_lock);
+ clear_bit(mdev-minor, media_devnode_nums);
Bitops are atomic, even, it seems, for large bitmaps.
+ mutex_unlock(media_devnode_lock);
+ return ret;
+}
+
+/**
+ * media_devnode_unregister - unregister a media device node
+ * @mdev: the device node to unregister
+ *
+ * This unregisters the passed device. Future open calls will be met with
+ * errors.
+ *
+ * This function can safely be called if the device node has never been
+ * registered or has already been unregistered.
+ */
+void media_devnode_unregister(struct media_devnode *mdev)
+{
+ /* Check if mdev was ever registered at all */
+ if (!media_devnode_is_registered(mdev))
+ return;
+
+ mutex_lock(media_devnode_lock);
+ clear_bit(MEDIA_FLAG_REGISTERED, mdev-flags);
This lock shouldn't be needed either.
+ mutex_unlock(media_devnode_lock);
+ device_unregister(mdev-dev);
+}
+
[...]
+struct media_file_operations {
+ struct module *owner;
+ ssize_t (*read) (struct file *, char __user *, size_t, loff_t *);
+ ssize_t (*write) (struct file *, const char __user *, size_t, loff_t *);
+ unsigned int (*poll) (struct file *, struct poll_table_struct *);
+ long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
+ int (*open) (struct file *);
+ int (*release) (struct file *);
+};
I'd be awfully tempted to just call it ioctl(); I suspect that
Re: [PATCH 1/1] videobuf: Initialize lists in videobuf_buffer.
diff --git a/drivers/media/video/videobuf-dma-contig.c
b/drivers/media/video/videobuf-dma-contig.c
index c969111..f7e0f86 100644
--- a/drivers/media/video/videobuf-dma-contig.c
+++ b/drivers/media/video/videobuf-dma-contig.c
@@ -193,6 +193,8 @@ static struct videobuf_buffer *__videobuf_alloc_vb(size_t
size)
if (vb) {
mem = vb-priv = ((char *)vb) + size;
mem-magic = MAGIC_DC_MEM;
+ INIT_LIST_HEAD(vb-stream);
+ INIT_LIST_HEAD(vb-queue);
i think it no need to be init, it just a list-entry.
--
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 v4 2/6] drivers:staging: ti-st: fmdrv_v4l2 sources
于 11/16/2010 09:18 PM, manjunatha_ha...@ti.com 写道: From: Manjunatha Hallimanjunatha_ha...@ti.com This module interfaces V4L2 subsystem and FM common module. It registers itself with V4L2 as Radio module. Signed-off-by: Manjunatha Hallimanjunatha_ha...@ti.com --- drivers/staging/ti-st/fmdrv_v4l2.c | 757 drivers/staging/ti-st/fmdrv_v4l2.h | 32 ++ 2 files changed, 789 insertions(+), 0 deletions(-) create mode 100644 drivers/staging/ti-st/fmdrv_v4l2.c create mode 100644 drivers/staging/ti-st/fmdrv_v4l2.h diff --git a/drivers/staging/ti-st/fmdrv_v4l2.c b/drivers/staging/ti-st/fmdrv_v4l2.c new file mode 100644 index 000..687d10f --- /dev/null +++ b/drivers/staging/ti-st/fmdrv_v4l2.c @@ -0,0 +1,757 @@ +/* + * FM Driver for Connectivity chip of Texas Instruments. + * This file provides interfaces to V4L2 subsystem. + * + * This module registers with V4L2 subsystem as Radio + * data system interface (/dev/radio). During the registration, + * it will expose two set of function pointers. + * + *1) File operation related API (open, close, read, write, poll...etc). + *2) Set of V4L2 IOCTL complaint API. + * + * Copyright (C) 2010 Texas Instruments + * Author: Raja Maniraja_m...@ti.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * 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 fmdrv.h +#include fmdrv_v4l2.h +#include fmdrv_common.h +#include fmdrv_rx.h +#include fmdrv_tx.h + +static struct video_device *gradio_dev; why are you using global variable here? -- 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 1/1] videobuf: Initialize lists in videobuf_buffer.
于 11/17/2010 09:38 AM, Andrew Chew 写道:
diff --git a/drivers/media/video/videobuf-dma-contig.c
b/drivers/media/video/videobuf-dma-contig.c
index c969111..f7e0f86 100644
--- a/drivers/media/video/videobuf-dma-contig.c
+++ b/drivers/media/video/videobuf-dma-contig.c
@@ -193,6 +193,8 @@ static struct videobuf_buffer
*__videobuf_alloc_vb(size_t size)
if (vb) {
mem = vb-priv = ((char *)vb) + size;
mem-magic = MAGIC_DC_MEM;
+ INIT_LIST_HEAD(vb-stream);
+ INIT_LIST_HEAD(vb-queue);
i think it no need to be init, it just a list-entry.
Okay, if that's really the case, then sh_mobile_ceu_camera.c, pxa_camera.c, mx1_camera.c,
mx2_camera.c, and omap1_camera.c needs to be fixed to remove that
WARN_ON(!list_empty(vb-queue)); in their videobuf_prepare() methods, because
those WARN_ON's are assuming that vb-queue is properly initialized as a list head.
Which will it be?
yes, i think those WARN_ONs are no need.
--
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] drivers/media: nuvoton: always true expression
Em 16-11-2010 19:54, Jarod Wilson escreveu: On Tue, Nov 16, 2010 at 09:19:53PM +0100, Nicolas Kaiser wrote: I noticed that the second part of this conditional is always true. Would the intention be to strictly check on both chip_major and chip_minor? Signed-off-by: Nicolas Kaiser ni...@nikai.net Hrm, yeah, looks like I screwed that one up. You're correct, the intention was to make sure we have a matching chip id high and one or the other of the chip id low values. Acked-by: Jarod Wilson ja...@redhat.com I wander if it wouldn't be good to print something if the probe fails due to the wrong chip ID. It may help if someone complain about a different revision. 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: hg pull http://202.88.242.108:8000/hg/var/www/hg/v4l-dvb/
Em 14-11-2010 18:48, Manu Abraham escreveu: Mauro, Please pull from http://202.88.242.108:8000/hg/var/www/hg/v4l-dvb/ for the following changes. changeset 15168:baa4e8008db5 Mantis, hopper: use MODULE_DEVICE_TABLE http://202.88.242.108:8000/hg/var/www/hg/v4l-dvb/rev/baa4e8008db5 changeset 15169:f04605948fdc Mantis: append tasklet maintenance for DVB stream delivery http://202.88.242.108:8000/hg/var/www/hg/v4l-dvb/rev/f04605948fdc changeset 15170:ee7a63d70f94 Mantis: use dvb_attach to avoid double dereferencing on module removal http://202.88.242.108:8000/hg/var/www/hg/v4l-dvb/rev/ee7a63d70f94 changeset 15171:3a2ece3bf184 Mantis: Rename gpio_set_bits to mantis_gpio_set_bits http://202.88.242.108:8000/hg/var/www/hg/v4l-dvb/rev/3a2ece3bf184 changeset 15172:56c20de4f697 stb6100: Improve tuner performance http://202.88.242.108:8000/hg/var/www/hg/v4l-dvb/rev/56c20de4f697 changeset 15173:5cc010e3a803 stb0899: fix diseqc messages getting lost http://202.88.242.108:8000/hg/var/www/hg/v4l-dvb/rev/5cc010e3a803 Applied, thanks. A new warning appeared: drivers/media/dvb/frontends/stb6100.c:120: warning: ‘stb6100_normalise_regs’ defined but not used I tried to remove the applied patches from the patchwork queue. There are still a few patches pending there. Not sure if they got obsoleted by the applied ones, or if they are still relevant: Jun,20 2010: [2/2] DVB/V4L: mantis: remove unused files http://patchwork.kernel.org/patch/107062 Bjørn Mork bj...@mork.no Jul,10 2010: Mantis driver patch: use interrupt for I2C traffic instead of busy reg http://patchwork.kernel.org/patch/111245 Marko Ristola marko.rist...@kolumbus.fi Jul,19 2010: Twinhan DTV Ter-CI (3030 mantis) http://patchwork.kernel.org/patch/112708 Niklas Claesson nicke.claes...@gmail.com Aug, 7 2010: Refactor Mantis DMA transfer to deliver 16Kb TS data per interrupt http://patchwork.kernel.org/patch/118173 Marko Ristola marko.rist...@kolumbus.fi Oct,10 2010: [v2] V4L/DVB: faster DVB-S lock for mantis cards using stb0899 demod http://patchwork.kernel.org/patch/244201 Tuxoholic tuxoho...@hotmail.de Please take a look and give us some feedback. Thanks, 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: [GIT PATCHES FOR 2.6.38] mantis for_2.6.38
Hi Bjørn, Em 13-11-2010 12:45, Bjørn Mork escreveu: Mauro Carvalho Chehab mche...@redhat.com writes: Em 12-11-2010 12:43, Bjørn Mork escreveu: git://git.mork.no/mantis.git for_2.6.38 Didn't work: git pull git://git.mork.no/mantis.git for_2.6.38 fatal: Couldn't find remote ref for_2.6.38 Damn, sorry about that. Was supposed to be git://git.mork.no/mantis.git for_v2.6.38 Except when drivers are not maintained anymore (or when the patch is trivial), I wait for the driver author(s) to test the patches and ask me to pull (or for them to reply that a patch is ok with his ack). Manu sent a pull request with some of the long-standing Mantis patches tested plus with some improvements for the frontend tuning. I've applied them today. Please test, and give us a feedback. Thanks, 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
[patch 1/3] [media] lirc_dev: stray unlock in lirc_dev_fop_poll()
We shouldn't unlock here. I think this was a cut and paste error.
Signed-off-by: Dan Carpenter erro...@gmail.com
diff --git a/drivers/media/IR/lirc_dev.c b/drivers/media/IR/lirc_dev.c
index 8418b14..8ab9d87 100644
--- a/drivers/media/IR/lirc_dev.c
+++ b/drivers/media/IR/lirc_dev.c
@@ -522,10 +522,8 @@ unsigned int lirc_dev_fop_poll(struct file *file,
poll_table *wait)
dev_dbg(ir-d.dev, LOGHEAD poll called\n, ir-d.name, ir-d.minor);
- if (!ir-attached) {
- mutex_unlock(ir-irctl_lock);
+ if (!ir-attached)
return POLLERR;
- }
poll_wait(file, ir-buf-wait_poll, wait);
--
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 2/3] [media] lirc_dev: add some __user annotations
Sparse complains because there are no __user annotations.
drivers/media/IR/lirc_dev.c:156:27: warning:
incorrect type in initializer (incompatible argument 2 (different
address spaces))
drivers/media/IR/lirc_dev.c:156:27:expected int ( *read )( ... )
drivers/media/IR/lirc_dev.c:156:27:got int ( extern [toplevel] *noident
)( ... )
Signed-off-by: Dan Carpenter erro...@gmail.com
diff --git a/include/media/lirc_dev.h b/include/media/lirc_dev.h
index 54780a5..630e702 100644
--- a/include/media/lirc_dev.h
+++ b/include/media/lirc_dev.h
@@ -217,9 +217,9 @@ int lirc_dev_fop_open(struct inode *inode, struct file
*file);
int lirc_dev_fop_close(struct inode *inode, struct file *file);
unsigned int lirc_dev_fop_poll(struct file *file, poll_table *wait);
long lirc_dev_fop_ioctl(struct file *file, unsigned int cmd, unsigned long
arg);
-ssize_t lirc_dev_fop_read(struct file *file, char *buffer, size_t length,
+ssize_t lirc_dev_fop_read(struct file *file, char __user *buffer, size_t
length,
loff_t *ppos);
-ssize_t lirc_dev_fop_write(struct file *file, const char *buffer, size_t
length,
- loff_t *ppos);
+ssize_t lirc_dev_fop_write(struct file *file, const char __user *buffer,
+ size_t length, loff_t *ppos);
#endif
diff --git a/drivers/media/IR/lirc_dev.c b/drivers/media/IR/lirc_dev.c
index 8ab9d87..fbca94f 100644
--- a/drivers/media/IR/lirc_dev.c
+++ b/drivers/media/IR/lirc_dev.c
@@ -627,7 +627,7 @@ long lirc_dev_fop_ioctl(struct file *file, unsigned int
cmd, unsigned long arg)
EXPORT_SYMBOL(lirc_dev_fop_ioctl);
ssize_t lirc_dev_fop_read(struct file *file,
- char *buffer,
+ char __user *buffer,
size_t length,
loff_t *ppos)
{
@@ -742,7 +742,7 @@ void *lirc_get_pdata(struct file *file)
EXPORT_SYMBOL(lirc_get_pdata);
-ssize_t lirc_dev_fop_write(struct file *file, const char *buffer,
+ssize_t lirc_dev_fop_write(struct file *file, const char __user *buffer,
size_t length, loff_t *ppos)
{
struct irctl *ir = irctls[iminor(file-f_dentry-d_inode)];
--
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 3/3] [media] lirc_dev: fixes in lirc_dev_fop_read()
This makes several changes but they're in one function and sort of
related:
buf was leaked on error. The leak if we try to read an invalid
length is the main concern because it could be triggered over and
over.
If the copy_to_user() failed, then the original code returned the
number of bytes remaining. read() is supposed to be the opposite way,
where we return the number of bytes copied. I changed it to just return
-EFAULT on errors.
Also I changed the debug output from -EFAULT to just fail because
it isn't -EFAULT necessarily. And since we go though that path if the
length is invalid now, there was another debug print that I removed.
Signed-off-by: Dan Carpenter erro...@gmail.com
diff --git a/drivers/media/IR/lirc_dev.c b/drivers/media/IR/lirc_dev.c
index fbca94f..6b9fc74 100644
--- a/drivers/media/IR/lirc_dev.c
+++ b/drivers/media/IR/lirc_dev.c
@@ -647,18 +647,18 @@ ssize_t lirc_dev_fop_read(struct file *file,
if (!buf)
return -ENOMEM;
- if (mutex_lock_interruptible(ir-irctl_lock))
- return -ERESTARTSYS;
+ if (mutex_lock_interruptible(ir-irctl_lock)) {
+ ret = -ERESTARTSYS;
+ goto out_unlocked;
+ }
if (!ir-attached) {
- mutex_unlock(ir-irctl_lock);
- return -ENODEV;
+ ret = -ENODEV;
+ goto out_locked;
}
if (length % ir-chunk_size) {
- dev_dbg(ir-d.dev, LOGHEAD read result = -EINVAL\n,
- ir-d.name, ir-d.minor);
- mutex_unlock(ir-irctl_lock);
- return -EINVAL;
+ ret = -EINVAL;
+ goto out_locked;
}
/*
@@ -709,18 +709,23 @@ ssize_t lirc_dev_fop_read(struct file *file,
lirc_buffer_read(ir-buf, buf);
ret = copy_to_user((void *)buffer+written, buf,
ir-buf-chunk_size);
- written += ir-buf-chunk_size;
+ if (!ret)
+ written += ir-buf-chunk_size;
+ else
+ ret = -EFAULT;
}
}
remove_wait_queue(ir-buf-wait_poll, wait);
set_current_state(TASK_RUNNING);
+
+out_locked:
mutex_unlock(ir-irctl_lock);
out_unlocked:
kfree(buf);
dev_dbg(ir-d.dev, LOGHEAD read result = %s (%d)\n,
- ir-d.name, ir-d.minor, ret ? -EFAULT : OK, ret);
+ ir-d.name, ir-d.minor, ret ? fail : ok, ret);
return ret ? ret : written;
}
--
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] mx2_camera: fix pixel clock polarity configuration
On Wed, 10 Nov 2010, Baruch Siach wrote: Guennadi Liakhovetski g.liakhovetski at gmx.de writes: On Wed, 27 Oct 2010, Baruch Siach wrote: When SOCAM_PCLK_SAMPLE_FALLING, just leave CSICR1_REDGE unset, otherwise we get the inverted behaviour. Seems logical to me, that if this is true, then you need the inverse: if (!(common_flags SOCAM_PCLK_SAMPLE_FALLING)) csicr1 |= CSICR1_INV_PCLK; No. Doing so you'll get the inverted behaviour of SAMPLE_RISING. When common_flags have SAMPLE_RISING set and SAMPLE_FALLING unset you get CSICR1_REDGE set, which triggers on the rising edge, and then also CSICR1_INV_PCLK set, which invert this. Thus you get the expected behaviour of SAMPLE_FALLING. Currently you get the inverted behaviour only for SAMPLE_FALLING. IMO, we should just use CSICR1_REDGE to set the sample timing, and leave CSICR1_INV_PCLK alone. Ah, right, of course, I've overlooked that CSICR1_REDGE flag. Then yes, your patch makes sense and should go in for 2.6.37. Thanks Guennadi baruch if (common_flags SOCAM_PCLK_SAMPLE_RISING) csicr1 |= CSICR1_REDGE; - if (common_flags SOCAM_PCLK_SAMPLE_FALLING) - csicr1 |= CSICR1_INV_PCLK; if (common_flags SOCAM_VSYNC_ACTIVE_HIGH) csicr1 |= CSICR1_SOF_POL; if (common_flags SOCAM_HSYNC_ACTIVE_HIGH) -- 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 --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 1/1] videobuf: Initialize lists in videobuf_buffer.
On Wednesday, November 17, 2010 02:38:09 Andrew Chew wrote:
diff --git a/drivers/media/video/videobuf-dma-contig.c
b/drivers/media/video/videobuf-dma-contig.c
index c969111..f7e0f86 100644
--- a/drivers/media/video/videobuf-dma-contig.c
+++ b/drivers/media/video/videobuf-dma-contig.c
@@ -193,6 +193,8 @@ static struct videobuf_buffer
*__videobuf_alloc_vb(size_t size)
if (vb) {
mem = vb-priv = ((char *)vb) + size;
mem-magic = MAGIC_DC_MEM;
+ INIT_LIST_HEAD(vb-stream);
+ INIT_LIST_HEAD(vb-queue);
i think it no need to be init, it just a list-entry.
Okay, if that's really the case, then sh_mobile_ceu_camera.c, pxa_camera.c,
mx1_camera.c, mx2_camera.c, and omap1_camera.c needs to be fixed to remove
that WARN_ON(!list_empty(vb-queue)); in their videobuf_prepare() methods,
because those WARN_ON's are assuming that vb-queue is properly initialized
as a list head.
Which will it be?
These list entries need to be inited. It is bad form to have uninitialized
list entries. It is not as if this is a big deal to initialize them properly.
Regards,
Hans
--
Hans Verkuil - video4linux developer - sponsored by Cisco
--
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 1/1] videobuf: Initialize lists in videobuf_buffer.
On Tuesday, November 16, 2010 21:24:43 ac...@nvidia.com wrote:
From: Andrew Chew ac...@nvidia.com
There are two struct list_head's in struct videobuf_buffer.
Prior to this fix, all we did for initialization of struct videobuf_buffer
was to zero out its memory. This does not properly initialize this struct's
two list_head members.
This patch immediately calls INIT_LIST_HEAD on both lists after the kzalloc,
so that the two lists are initialized properly.
Rather than doing this for all videobuf variants I would suggest that you
do this in videobuf-core.c, videobuf_alloc_vb().
Regards,
Hans
Signed-off-by: Andrew Chew ac...@nvidia.com
---
I thought I'd submit a patch for this anyway. Without this, the existing
camera host drivers will spew an ugly warning on every videobuf allocation,
which gets annoying really fast.
drivers/media/video/videobuf-dma-contig.c |2 ++
drivers/media/video/videobuf-dma-sg.c |2 ++
drivers/media/video/videobuf-vmalloc.c|2 ++
3 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/drivers/media/video/videobuf-dma-contig.c
b/drivers/media/video/videobuf-dma-contig.c
index c969111..f7e0f86 100644
--- a/drivers/media/video/videobuf-dma-contig.c
+++ b/drivers/media/video/videobuf-dma-contig.c
@@ -193,6 +193,8 @@ static struct videobuf_buffer *__videobuf_alloc_vb(size_t
size)
if (vb) {
mem = vb-priv = ((char *)vb) + size;
mem-magic = MAGIC_DC_MEM;
+ INIT_LIST_HEAD(vb-stream);
+ INIT_LIST_HEAD(vb-queue);
}
return vb;
diff --git a/drivers/media/video/videobuf-dma-sg.c
b/drivers/media/video/videobuf-dma-sg.c
index 20f227e..5af3217 100644
--- a/drivers/media/video/videobuf-dma-sg.c
+++ b/drivers/media/video/videobuf-dma-sg.c
@@ -430,6 +430,8 @@ static struct videobuf_buffer *__videobuf_alloc_vb(size_t
size)
mem = vb-priv = ((char *)vb) + size;
mem-magic = MAGIC_SG_MEM;
+ INIT_LIST_HEAD(vb-stream);
+ INIT_LIST_HEAD(vb-queue);
videobuf_dma_init(mem-dma);
diff --git a/drivers/media/video/videobuf-vmalloc.c
b/drivers/media/video/videobuf-vmalloc.c
index df14258..8babedd 100644
--- a/drivers/media/video/videobuf-vmalloc.c
+++ b/drivers/media/video/videobuf-vmalloc.c
@@ -146,6 +146,8 @@ static struct videobuf_buffer *__videobuf_alloc_vb(size_t
size)
mem = vb-priv = ((char *)vb) + size;
mem-magic = MAGIC_VMAL_MEM;
+ INIT_LIST_HEAD(vb-stream);
+ INIT_LIST_HEAD(vb-queue);
dprintk(1, %s: allocated at %p(%ld+%ld) %p(%ld)\n,
__func__, vb, (long)sizeof(*vb), (long)size - sizeof(*vb),
--
Hans Verkuil - video4linux developer - sponsored by Cisco
--
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 1/1] videobuf: Initialize lists in videobuf_buffer.
On 11/17/2010 03:11 PM, Hans Verkuil wrote:
On Wednesday, November 17, 2010 02:38:09 Andrew Chew wrote:
diff --git a/drivers/media/video/videobuf-dma-contig.c
b/drivers/media/video/videobuf-dma-contig.c
index c969111..f7e0f86 100644
--- a/drivers/media/video/videobuf-dma-contig.c
+++ b/drivers/media/video/videobuf-dma-contig.c
@@ -193,6 +193,8 @@ static struct videobuf_buffer
*__videobuf_alloc_vb(size_t size)
if (vb) {
mem = vb-priv = ((char *)vb) + size;
mem-magic = MAGIC_DC_MEM;
+ INIT_LIST_HEAD(vb-stream);
+ INIT_LIST_HEAD(vb-queue);
i think it no need to be init, it just a list-entry.
Okay, if that's really the case, then sh_mobile_ceu_camera.c, pxa_camera.c, mx1_camera.c,
mx2_camera.c, and omap1_camera.c needs to be fixed to remove that
WARN_ON(!list_empty(vb-queue)); in their videobuf_prepare() methods, because
those WARN_ON's are assuming that vb-queue is properly initialized as a list head.
Which will it be?
These list entries need to be inited. It is bad form to have uninitialized
list entries. It is not as if this is a big deal to initialize them properly.
in kernel source code, list entry are not often to be inited.
for example, see mm/vmscan.c register_shrinker(), no one init the
shrinker-list.
another example: see mm/swapfile.c add_swap_extent(), no one init the
new_se-list.
--
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: An article on the media controller
On Tuesday, November 16, 2010 23:18:02 Jonathan Corbet wrote: I've just spent a fair while looking through the September posting of the media controller code (is there a more recent version?). The result is a high-level review which interested people can read here: http://lwn.net/SubscriberLink/415714/1e837f01b8579eb7/ Most people will not see it for another 24 hours or so; if there's something I got radically wrong, I'd appreciate hearing about it. Just two points, really: we do not expect generic applications to use this API other than for determining the default device nodes. E.g. if a device has 5 video nodes and two alsa nodes, then one video and one alsa node will be marked as the default and applications are supposed to open those. This will allow apps to find the right device nodes for capturing. The more advanced functionality is expected to be used by custom applications specific to that hardware. I also expect to see either libv4l2 or gstreamer plugins or libraries created for specific SoCs that use this API. The hardware that the MC describes is simply too varied and unpredictable to ever be able to use it with just the MC data. The main purpose is to at least be able to expose the full functionality to userspace. The other point is the same that Andy made: we expect that the API will change a bit allowing for atomically changing multiple links at the same time. Not something that sysfs can handle (not without some horrible hacks at least). I also think sysfs is a mess and horrible to use, but that's just my opinion. The executive summary is that I think this code really needs some exposure outside of the V4L2 list; I'd encourage posting it to linux-kernel. That could be hard on plans for a 2.6.38 merge (or, at least, plans for any spare time between now and then), but the end result might be better for everybody. I agree with that. It was always designed for generic use, but on the other hand we can't postpone it for too long since V4L really needs this. Thanks for the article! Regards, Hans I have some low-level comments too which were not suitable for the article. I'll be posting them here, but I have to get some other things done first. Thanks, jon -- 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 -- Hans Verkuil - video4linux developer - sponsored by Cisco -- 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 1/1] videobuf: Initialize lists in videobuf_buffer.
On Wednesday, November 17, 2010 08:16:27 Figo.zhang wrote:
On 11/17/2010 03:11 PM, Hans Verkuil wrote:
On Wednesday, November 17, 2010 02:38:09 Andrew Chew wrote:
diff --git a/drivers/media/video/videobuf-dma-contig.c
b/drivers/media/video/videobuf-dma-contig.c
index c969111..f7e0f86 100644
--- a/drivers/media/video/videobuf-dma-contig.c
+++ b/drivers/media/video/videobuf-dma-contig.c
@@ -193,6 +193,8 @@ static struct videobuf_buffer
*__videobuf_alloc_vb(size_t size)
if (vb) {
mem = vb-priv = ((char *)vb) + size;
mem-magic = MAGIC_DC_MEM;
+INIT_LIST_HEAD(vb-stream);
+INIT_LIST_HEAD(vb-queue);
i think it no need to be init, it just a list-entry.
Okay, if that's really the case, then sh_mobile_ceu_camera.c,
pxa_camera.c, mx1_camera.c, mx2_camera.c, and omap1_camera.c needs to be
fixed to remove that WARN_ON(!list_empty(vb-queue)); in their
videobuf_prepare() methods, because those WARN_ON's are assuming that
vb-queue is properly initialized as a list head.
Which will it be?
These list entries need to be inited. It is bad form to have uninitialized
list entries. It is not as if this is a big deal to initialize them
properly.
in kernel source code, list entry are not often to be inited.
for example, see mm/vmscan.c register_shrinker(), no one init the
shrinker-list.
another example: see mm/swapfile.c add_swap_extent(), no one init the
new_se-list.
I have to think some more about this. I'll get back to this today. BTW, I do
agree that the WARN_ON's are bogus and should be removed.
And BTW, this isn't going to work either (mx1_camera.c):
static int mx1_camera_setup_dma(struct mx1_camera_dev *pcdev)
{
struct videobuf_buffer *vbuf = pcdev-active-vb;
struct device *dev = pcdev-icd-dev.parent;
int ret;
if (unlikely(!pcdev-active)) {
dev_err(dev, DMA End IRQ with no active buffer\n);
return -EFAULT;
}
The vbuf assignment should be moved after the 'if'.
Regards,
Hans
--
Hans Verkuil - video4linux developer - sponsored by Cisco
--
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
