Re: Afatech AF9013
I have a problem that may be related to the issues on this thread and it's driving me nuts. I have two dual tuner Afatech based cards, they are both Leadtek 2000DS cards, one made by Leadtek and the other branded as KWorld but they are otherwise identical in spite of different VID:PID. On each card tuner A is an AF9015 and tuner B is an AF9013. The KWorld card worked just fine for about 18 months in Mythbuntu 10.04 with the rebuilt and patched modules as described in the Wiki entry on the 2000DS. A few weeks ago tuner A started giving errors making the viewing unwatchable so figuring the card had died I bought the Leadtek. To my surprise it gave the same problem as the KWorld when using tuner A. It seems Tuner A is OK until Tuner B is used and then Tuner A gets a lot of errors. Tuner B never has errors. I did try using the latest "media_build" from V4L but that didn't help. So, I installed Mythbuntu 11.04 and with both cards I still get the same problem. Watching live TV with MythTV or with mplayer on tuner A gives errors and tuner B is always flawless even with "media_build" updates. I honestly can't recall if when the failure first occurred if I had done a routine kernel update at that time - though it would have just been the usual 2.6.32 update that is in line with 10.04 maintenance. I have tried everything imaginable to nail down the problem but can't seem to fix it. Even "options dvb-usb force_pid_filter_usage=1" seems to improve the problem somewhat but the errors are still there. I have tried every firmware from 4.65 to 5.10, adjusting the PCI latency from 32 to 96, fed each card directly from the antenna (taking the splitter out of the loop), one card fitted, both cards fitted, kernel and system upgrades (Mythbuntu 10.04 to 11.04), mplayer vs MythTV but the results are always the same. Tuner B is perfect, tuner A corrupts when Tuner B is used. There are no errors or warnings in syslog or dmesg to suggest anything has failed. I'd appreciate any suggestions at this point as I am pretty unhappy with the situation considering it *used* to work. -- 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/1] EM28xx - fix deadlock when unplugging and replugging a DVB adapter
Here's the new patch for the deadlock problem, which releases the device mutex before calling em28xx_init_extension() and then reacquires it afterwards. The locking in dvb_init() is now left alone. Signed-off-by: Chris Rankin --- linux-3.0/drivers/media/video/em28xx/em28xx-cards.c.orig 2011-08-19 00:45:48.0 +0100 +++ linux-3.0/drivers/media/video/em28xx/em28xx-cards.c 2011-08-20 23:28:39.0 +0100 @@ -2929,7 +2929,9 @@ goto fail_reg_analog_devices; } + mutex_unlock(&dev->lock); em28xx_init_extension(dev); + mutex_lock(&dev->lock); /* Save some power by putting tuner to sleep */ v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_power, 0);
Re: RFC: Negotiating frame buffer size between sensor subdevs and bridge devices
On 08/17/2011 07:43 PM, Guennadi Liakhovetski wrote: > On Wed, 17 Aug 2011, Sakari Ailus wrote: >> On Thu, Jul 28, 2011 at 07:04:11PM +0200, Sylwester Nawrocki wrote: ... >>> In order to let the host drivers query or configure subdevs with required >>> frame buffer size one of the following changes could be done at V4L2 API: >>> >>> 1. Add a 'sizeimage' field in struct v4l2_mbus_framefmt and make subdev >>> drivers optionally set/adjust it when setting or getting the format with >>> set_fmt/get_fmt pad level ops (and s/g_mbus_fmt ?) >>> There could be two situations: >>> - effective required frame buffer size is specified by the sensor and the >>> host driver relies on that value when allocating a buffer; >>> - the host driver forces some arbitrary buffer size and the sensor >>> performs >>> any required action to limit transmitted amount of data to that amount >>> of data; >>> Both cases could be covered similarly as it's done with VIDIOC_S_FMT. >>> >>> Introducing 'sizeimage' field is making the media bus format struct looking >>> more similar to struct v4l2_pix_format and not quite in line with media bus >>> format meaning, i.e. describing data on a physical bus, not in the memory. >>> The other option I can think of is to create separate subdev video ops. >>> 2. Add new s/g_sizeimage subdev video operations ... >> I prefer this second approach over the first once since the maxiumu size of >> the image in bytes really isn't a property of the bus. > > Call that field framesamples and already it fits quite well with the > notion of data on the bus and not in memory. Wouldn't this work? Hmm...that might be exactly what we need. That was also an initial Hans' proposal when we recently discussed it. At least such an information should be sufficient for handling JPEG, where the effective buffer size might be calculated from a media bus pixel code and a number of samples per frame. -- Regards, Sylwester -- 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
flag DVB_CA_EN50221_POLL_CAM_READY
Why this flag is defined? I don't see how interface driver can know CAM is ready (after plug or reset) unless it reads that info from CAM itself. Thus, I think correct behaviour should be move that detection functionality to the EN50221 CA core. Looking from existing drivers can confirm that. Those just returns that flag when CAM is present (plugged in slot) with DVB_CA_EN50221_POLL_CAM_PRESENT flag. OR read directly CAM memory to see it answers and set flag according to that (which should be job of EN50221 CA core IMHO). regards Antti -- http://palosaari.fi/ -- 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:Sat Aug 20 19:00:37 CEST 2011 git hash:9bed77ee2fb46b74782d0d9d14b92e9d07f3df6e gcc version: i686-linux-gcc (GCC) 4.6.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: OK linux-git-mips: WARNINGS linux-git-powerpc64: WARNINGS linux-git-x86_64: WARNINGS linux-2.6.31.12-i686: WARNINGS linux-2.6.32.6-i686: WARNINGS linux-2.6.33-i686: WARNINGS linux-2.6.34-i686: WARNINGS linux-2.6.35.3-i686: WARNINGS linux-2.6.36-i686: WARNINGS linux-2.6.37-i686: WARNINGS linux-2.6.38.2-i686: WARNINGS linux-2.6.39.1-i686: WARNINGS linux-3.0-i686: WARNINGS linux-3.1-rc1-i686: WARNINGS linux-2.6.31.12-x86_64: WARNINGS linux-2.6.32.6-x86_64: WARNINGS linux-2.6.33-x86_64: WARNINGS linux-2.6.34-x86_64: WARNINGS linux-2.6.35.3-x86_64: WARNINGS linux-2.6.36-x86_64: WARNINGS linux-2.6.37-x86_64: WARNINGS linux-2.6.38.2-x86_64: WARNINGS linux-2.6.39.1-x86_64: WARNINGS linux-3.0-x86_64: WARNINGS linux-3.1-rc1-x86_64: WARNINGS spec-git: WARNINGS sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Saturday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Saturday.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
Re: [PATCH 6/6] EM28xx - don't sleep on disconnect
Here's the updated patch that checks whether the frontend exists before
disabling the sleep operations.
Cheers,
Chris
--- linux-3.0/drivers/media/common/tuners/tda18271-fe.c.orig2011-08-20
18:53:48.0 +0100
+++ linux-3.0/drivers/media/common/tuners/tda18271-fe.c 2011-08-20
19:21:34.0 +0100
@@ -1230,7 +1230,7 @@
return 0;
}
-static struct dvb_tuner_ops tda18271_tuner_ops = {
+static const struct dvb_tuner_ops tda18271_tuner_ops = {
.info = {
.name = "NXP TDA18271HD",
.frequency_min = 4500,
--- linux-3.0/drivers/media/dvb/frontends/cxd2820r_core.c.orig 2011-08-20
18:54:09.0 +0100
+++ linux-3.0/drivers/media/dvb/frontends/cxd2820r_core.c 2011-08-20
19:22:21.0 +0100
@@ -778,7 +778,7 @@
}
EXPORT_SYMBOL(cxd2820r_get_tuner_i2c_adapter);
-static struct dvb_frontend_ops cxd2820r_ops[2];
+static const struct dvb_frontend_ops cxd2820r_ops[2];
struct dvb_frontend *cxd2820r_attach(const struct cxd2820r_config *cfg,
struct i2c_adapter *i2c, struct dvb_frontend *fe)
@@ -844,7 +844,7 @@
}
EXPORT_SYMBOL(cxd2820r_attach);
-static struct dvb_frontend_ops cxd2820r_ops[2] = {
+static const struct dvb_frontend_ops cxd2820r_ops[2] = {
{
/* DVB-T/T2 */
.info = {
--- linux-3.0/drivers/media/video/em28xx/em28xx-dvb.c.orig 2011-08-20
18:53:25.0 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-dvb.c 2011-08-20
19:26:51.0 +0100
@@ -720,6 +720,13 @@
goto ret;
}
+static inline void prevent_sleep(struct dvb_frontend_ops *ops)
+{
+ ops->set_voltage = NULL;
+ ops->sleep = NULL;
+ ops->tuner_ops.sleep = NULL;
+}
+
static int dvb_fini(struct em28xx *dev)
{
if (!dev->board.has_dvb) {
@@ -728,8 +735,19 @@
}
if (dev->dvb) {
- unregister_dvb(dev->dvb);
- kfree(dev->dvb);
+ struct em28xx_dvb *dvb = dev->dvb;
+
+ if (dev->state & DEV_DISCONNECTED) {
+ /* We cannot tell the device to sleep
+* once it has been unplugged. */
+ if (dvb->fe[0])
+ prevent_sleep(&dvb->fe[0]->ops);
+ if (dvb->fe[1])
+ prevent_sleep(&dvb->fe[1]->ops);
+ }
+
+ unregister_dvb(dvb);
+ kfree(dvb);
dev->dvb = NULL;
}
Re: RFC: Negotiating frame buffer size between sensor subdevs and bridge devices
On Wed, 17 Aug 2011, Sakari Ailus wrote: > On Thu, Jul 28, 2011 at 07:04:11PM +0200, Sylwester Nawrocki wrote: > > Hello, > > Hi Sylwester, > > > Trying to capture images in JPEG format with regular "image sensor -> > > mipi-csi receiver -> host interface" H/W configuration I've found there > > is no standard way to communicate between the sensor subdev and the host > > driver what is exactly a required maximum buffer size to capture a frame. > > > > For the raw formats there is no issue as the buffer size can be easily > > determined from the pixel format and resolution (or sizeimage set on > > a video node). > > However compressed data formats are a bit more complicated, the required > > memory buffer size depends on multiple factors, like compression ratio, > > exact file header structure etc. > > > > Often it is at the sensor driver where all information required to > > determine size of the allocated memory is present. Bridge/host devices > > just do plain DMA without caring much what is transferred. I know of > > hardware which, for some pixel formats, once data capture is started, > > writes to memory whatever amount of data the sensor is transmitting, > > without any means to interrupt on the host side. So it is critical > > to assure the buffer allocation is done right, according to the sensor > > requirements, to avoid buffer overrun. > > > > > > Here is a link to somehow related discussion I could find: > > [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg27138.html > > > > > > In order to let the host drivers query or configure subdevs with required > > frame buffer size one of the following changes could be done at V4L2 API: > > > > 1. Add a 'sizeimage' field in struct v4l2_mbus_framefmt and make subdev > > drivers optionally set/adjust it when setting or getting the format with > > set_fmt/get_fmt pad level ops (and s/g_mbus_fmt ?) > > There could be two situations: > > - effective required frame buffer size is specified by the sensor and the > >host driver relies on that value when allocating a buffer; > > - the host driver forces some arbitrary buffer size and the sensor performs > >any required action to limit transmitted amount of data to that amount > >of data; > > Both cases could be covered similarly as it's done with VIDIOC_S_FMT. > > > > Introducing 'sizeimage' field is making the media bus format struct looking > > more similar to struct v4l2_pix_format and not quite in line with media bus > > format meaning, i.e. describing data on a physical bus, not in the memory. > > The other option I can think of is to create separate subdev video ops. > > 2. Add new s/g_sizeimage subdev video operations > > > > The best would be to make this an optional callback, not sure if it makes > > sense > > though. It has an advantage of not polluting the user space API. Although > > 'sizeimage' in user space might be useful for some purposes I rather tried > > to > > focus on "in-kernel" calls. > > I prefer this second approach over the first once since the maxiumu size of > the image in bytes really isn't a property of the bus. Call that field framesamples and already it fits quite well with the notion of data on the bus and not in memory. Wouldn't this work? Thanks Guennadi > > How about a regular V4L2 control? You would also have minimum and maximum > values, I'm not quite sure whather this is a plus, though. :) > > Btw. how does v4l2_mbus_framefmt suit for compressed formats in general? > > -- > Sakari Ailus > sakari.ai...@iki.fi --- 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 2/2] EM28xx - fix deadlock when unplugging and replugging a DVB adapter
Em 20-08-2011 07:40, Chris Rankin escreveu: > --- On Sat, 20/8/11, Mauro Carvalho Chehab wrote: >> No. The extension load can happen after the usb probe >> phase. In practice, the only case where the extension init will happen >> together with the usb probe phase is when the em28xx modules are >> compiled builtin > > It also happens when someone plugs an adapter into the machine when the > modules are already loaded. E.g. someone plugging a second adapter in, or > unplugging and then replugging the same one. Yes. >> Maybe the proper fix would be to change the logic under >> em28xx_usb_probe() to not hold dev->lock anymore when the device is >> loading the extensions. > > I could certainly write such a patch, although I only have a PCTV 290e > adapter to test with. We can test it on more devices. > Is this problem unique to the em28xx-dvb module? How does the em28xx-alsa > module get > away with creating ALSA devices without causing a similar race condition? It might also affect alsa. Pulseaudio has the bad habit of opening the device. However, the device lock is hold when the driver is changing the lock. It should be noticed that the current mutex lock strategy is a workaround. The proper solution is to work on resource lock library that could be used when the access of a device via one API blocks the access of the same device using another API. We had some discussions about that during the USB mini-summit on last Monday. We'll probably discuss more about that during the Kernel Summit/media workshop. 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: [PATCH 2/2] EM28xx - fix deadlock when unplugging and replugging a DVB adapter
--- On Sat, 20/8/11, Mauro Carvalho Chehab wrote: > No. The extension load can happen after the usb probe > phase. In practice, the only case where the extension init will happen > together with the usb probe phase is when the em28xx modules are > compiled builtin It also happens when someone plugs an adapter into the machine when the modules are already loaded. E.g. someone plugging a second adapter in, or unplugging and then replugging the same one. > Maybe the proper fix would be to change the logic under > em28xx_usb_probe() to not hold dev->lock anymore when the device is > loading the extensions. I could certainly write such a patch, although I only have a PCTV 290e adapter to test with. Is this problem unique to the em28xx-dvb module? How does the em28xx-alsa module get away with creating ALSA devices without causing a similar race condition? Cheers, Chris -- 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 6/6] EM28xx - don't sleep on disconnect
Em 20-08-2011 06:46, Chris Rankin escreveu: > --- On Sat, 20/8/11, Mauro Carvalho Chehab wrot >> >> This will cause an OOPS if dvb->fe[n] == NULL. >> > > OK, that's trivially fixable. I'll send you an updated patch. Is it safe to > assume that dvb->fe[0] at least will always be non-NULL? No, it isn't. The dvb initialization may fail or the device can be analog only, but somebody might manually load em28xx-dvb (or two devices were plugged). Regards, 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 6/6] EM28xx - don't sleep on disconnect
--- On Sat, 20/8/11, Mauro Carvalho Chehab wrot > > This will cause an OOPS if dvb->fe[n] == NULL. > OK, that's trivially fixable. I'll send you an updated patch. Is it safe to assume that dvb->fe[0] at least will always be non-NULL? Cheers, Chris -- 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
Updated DVB-T file fr-Bourges
Since the cessation of analog TV in my area, 5 of the 6 DTT multiplexes have had their frequency changed. You will find attached the updated file for the DVB-T transmitter of Bourges in France. I tested it successfully with the software "Me TV" Thank you kindly update. Sincerely, Superpat # Bourges - France (DVB-T transmitter of Bourges ( CollinesduSancerrois ) ) # Bourges - France (signal DVB-T transmis depuis l'émetteur de CollinesduSancerrois ) # # ATTENTION ! Ce fichier a ete construit automatiquement a partir # des frequences obtenues sur : http://www.tvnt.net/multiplex_frequences.htm # en Avril 2006. Si vous constatez des problemes et voulez apporter des # modifications au fichier, envoyez le fichier modifie a # l'adresse linux-...@linuxtv.org (depot des fichiers d'init dvb) # ou a l'auteur du fichier : # Nicolas Estre # # T freq bw fec_hi fec_lo mod transmission-mode guard-interval hierarchy Bourges - CollinesduSancerrois #R1 T 75400 8MHz AUTO NONE QAM64 8k AUTO NONE #R2 T 67400 8MHz AUTO NONE QAM64 8k AUTO NONE #R3 T 65000 8MHz AUTO NONE QAM64 8k AUTO NONE #R4 T 49800 8MHz AUTO NONE QAM64 8k AUTO NONE #R5 T 62600 8MHz AUTO NONE QAM64 8k AUTO NONE #R6 T 59400 8MHz AUTO NONE QAM64 8k AUTO NONE
Updated DVB-T file fr-Bourges
Since the cessation of analog TV in my area, 5 of the 6 DTT multiplexes have had their frequency changed. You will find attached the updated file for the DVB-T transmitter of Bourges in France. I tested it successfully with the software "Me TV" Thank you kindly update. Sincerely, Superpat # Bourges - France (DVB-T transmitter of Bourges ( CollinesduSancerrois ) ) # Bourges - France (signal DVB-T transmis depuis l'émetteur de CollinesduSancerrois ) # # ATTENTION ! Ce fichier a ete construit automatiquement a partir # des frequences obtenues sur : http://www.tvnt.net/multiplex_frequences.htm # en Avril 2006. Si vous constatez des problemes et voulez apporter des # modifications au fichier, envoyez le fichier modifie a # l'adresse linux-...@linuxtv.org (depot des fichiers d'init dvb) # ou a l'auteur du fichier : # Nicolas Estre # # T freq bw fec_hi fec_lo mod transmission-mode guard-interval hierarchy Bourges - CollinesduSancerrois #R1 T 75400 8MHz AUTO NONE QAM64 8k AUTO NONE #R2 T 67400 8MHz AUTO NONE QAM64 8k AUTO NONE #R3 T 65000 8MHz AUTO NONE QAM64 8k AUTO NONE #R4 T 49800 8MHz AUTO NONE QAM64 8k AUTO NONE #R5 T 62600 8MHz AUTO NONE QAM64 8k AUTO NONE #R6 T 59400 8MHz AUTO NONE QAM64 8k AUTO NONE
Re: [PATCH 1/2] EM28xx - fix race on disconnect
Hi Chris, On 08/20/2011 01:42 PM, Chris Rankin wrote: > *Sigh* I overlooked two patches in the original numbering... How about using git format-patch and git send-email ? http://www.kernel.org/pub/software/scm/git/docs/git-send-email.html It's easier to review when patches are inlined rather than sent as an attachment. git format-patch will make you a nice series of patches. For instance, when you have 3 of your patches applied on top of some branch and it's checked out, to generate the patches the simple command: $ git format-patch -3 --cover-letter will do. Then you just need to pass the files to 'git send-email'. Also, the patch description should be wrapped around 75 characters. So there is no need for text wrapping with 'git log', etc. Unfortunately git send-email won't work with free e-email accounts on yahoo.com. The SMTP server throws an error like: "Access denied : Free users cannot access this server." Gmail works for me. -- Regards, Sylwester -- 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 2/2] EM28xx - fix deadlock when unplugging and replugging a DVB adapter
Em 20-08-2011 04:46, Chris Rankin escreveu:
> The final patch removes the unplug/replug deadlock by not holding the device
> mutex during dvb_init(). However, this mutex has already been locked during
> device initialisation by em28xx_usb_probe() and is not released again until
> all extensions have been initialised successfully.
No. The extension load can happen after the usb probe phase. In practice,
the only case where the extension init will happen together with the
usb probe phase is when the em28xx modules are compiled builtin, as the
module load is done asynchronously, and generally happens after the em28xx
core to load.
> The device mutex is not held during either em28xx_register_extension() or
> em28xx_unregister_extension() any more. More importantly, I don't believe it
> can safely be held by these functions because they must both - by their
> nature - acquire the device list mutex before they can iterate through the
> device list. In other words, while usb_probe() and usb_disconnect() acquire
> the device mutex followed by the device list mutex, the
> register/unregister_extension() functions would need to acquire these mutexes
> in the opposite order. And that sounds like a potential deadlock.
>
> On the other hand, the new situation is a definite improvement :-).
No, it is a regression for a known bug.
This patch can cause troubles. The point is that, after initializing the
analog part, the device can be accessed via the V4L2 API, while it is
still initializing the DVB part. This actually happens in practice, as
when udev detects a new device, it opens it and calls VIDIOC_QUERYCAP.
So, it ends by having a race issue, as at V4L2 open, or at some analog mode
ioctl's, there are calls for em28xx_set_mode(dev, EM28XX_ANALOG_MODE).
In order words, if the DVB initialization is still happening, the driver
should not allow any V4L2 call, otherwise the DVB detection breaks.
Maybe the proper fix would be to change the logic under em28xx_usb_probe()
to not hold dev->lock anymore when the device is loading the extensions.
>
> Signed-off-by: Chris Rankin
>
>
> EM28xx-replug-deadlock.diff
>
>
> --- linux-3.0/drivers/media/video/em28xx/em28xx-dvb.c.orig2011-08-19
> 00:50:41.0 +0100
> +++ linux-3.0/drivers/media/video/em28xx/em28xx-dvb.c 2011-08-19
> 00:51:03.0 +0100
> @@ -542,7 +542,6 @@
> dev->dvb = dvb;
> dvb->fe[0] = dvb->fe[1] = NULL;
>
> - mutex_lock(&dev->lock);
> em28xx_set_mode(dev, EM28XX_DIGITAL_MODE);
> /* init frontend */
> switch (dev->model) {
> @@ -711,7 +710,6 @@
> em28xx_info("Successfully loaded em28xx-dvb\n");
> ret:
> em28xx_set_mode(dev, EM28XX_SUSPEND);
> - mutex_unlock(&dev->lock);
> return result;
>
> out_free:
--
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 6/6] EM28xx - don't sleep on disconnect
Em 20-08-2011 04:37, Chris Rankin escreveu:
> +
> + if (dev->state & DEV_DISCONNECTED) {
> + /* We cannot tell the device to sleep
> + * once it has been unplugged. */
> + prevent_sleep(&dvb->fe[0]->ops);
> + prevent_sleep(&dvb->fe[1]->ops);
This will cause an OOPS if dvb->fe[n] == NULL.
> + }
> +
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Embedded device and the V4L2 API support - Was: [GIT PATCHES FOR 3.1] s5p-fimc and noon010pc30 driver updates
Em 20-08-2011 04:27, Sylwester Nawrocki escreveu: > Hi Mauro, > > On 08/17/2011 08:13 AM, Mauro Carvalho Chehab wrote: >> It seems that there are too many miss understandings or maybe we're just >> talking the same thing on different ways. >> >> So, instead of answering again, let's re-start this discussion on a >> different way. >> >> One of the requirements that it was discussed a lot on both mailing >> lists and on the Media Controllers meetings that we had (or, at least >> in the ones where I've participated) is that: >> >> "A pure V4L2 userspace application, knowing about video device >> nodes only, can still use the driver. Not all advanced features >> will be available." > > What does a term "a pure V4L2 userspace application" mean here ? The above quotation are exactly the Laurent's words that I took from one of his replies. > Does it also account an application which is linked to libv4l2 and uses > calls specific to a particular hardware which are included there? That's a good question. We need to properly define what it means, to avoid having libv4l abuses. In other words, it seems ok to use libv4l to set pipelines via the MC API at open(), but it isn't ok to have an open() binary only libv4l plugin that will hook open and do the complete device initialization on userspace (I remember that one vendor once proposed a driver like that). Also, from my side, I'd like to see both libv4l and kernel drivers being submitted together, if the new driver depends on a special libv4l support for it to work. Regards, 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 2/2] EM28xx - fix deadlock when unplugging and replugging a DVB adapter
The final patch removes the unplug/replug deadlock by not holding the device
mutex during dvb_init(). However, this mutex has already been locked during
device initialisation by em28xx_usb_probe() and is not released again until all
extensions have been initialised successfully.
The device mutex is not held during either em28xx_register_extension() or
em28xx_unregister_extension() any more. More importantly, I don't believe it can
safely be held by these functions because they must both - by their nature -
acquire the device list mutex before they can iterate through the device list.
In other words, while usb_probe() and usb_disconnect() acquire the device mutex
followed by the device list mutex, the register/unregister_extension() functions
would need to acquire these mutexes in the opposite order. And that sounds like
a potential deadlock.
On the other hand, the new situation is a definite improvement :-).
Signed-off-by: Chris Rankin
--- linux-3.0/drivers/media/video/em28xx/em28xx-dvb.c.orig 2011-08-19 00:50:41.0 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-dvb.c 2011-08-19 00:51:03.0 +0100
@@ -542,7 +542,6 @@
dev->dvb = dvb;
dvb->fe[0] = dvb->fe[1] = NULL;
- mutex_lock(&dev->lock);
em28xx_set_mode(dev, EM28XX_DIGITAL_MODE);
/* init frontend */
switch (dev->model) {
@@ -711,7 +710,6 @@
em28xx_info("Successfully loaded em28xx-dvb\n");
ret:
em28xx_set_mode(dev, EM28XX_SUSPEND);
- mutex_unlock(&dev->lock);
return result;
out_free:
[PATCH 1/2] EM28xx - fix race on disconnect
*Sigh* I overlooked two patches in the original numbering...
This patch closes the race on the device and extension lists at USB disconnect
time. Previously, the device was removed from the device list during
em28xx_release_resources(), and then passed to the em28xx_close_extension()
function so that all extensions could run their fini() operations. However, this
left a (brief, theoretical, highly unlikely ;-)) window between these two calls
during which a new module could call em28xx_register_extension(). The result
would have been that the em28xx_usb_disconnect() function would also have passed
the device to the new extension's fini() function, despite never having called
the extension's init() function.
This patch also restores em28xx_close_extension()'s symmetry with
em28xx_init_extension(), and establishes the property that every device in the
device list must have been initialised for every extension in the extension list.
Signed-of-by: Chris Rankin
--- linux-3.0/drivers/media/video/em28xx/em28xx-cards.c.orig 2011-08-19 00:23:17.0 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-cards.c 2011-08-19 00:32:40.0 +0100
@@ -2738,9 +2738,9 @@
#endif /* CONFIG_MODULES */
/*
- * em28xx_realease_resources()
+ * em28xx_release_resources()
* unregisters the v4l2,i2c and usb devices
- * called when the device gets disconected or at module unload
+ * called when the device gets disconnected or at module unload
*/
void em28xx_release_resources(struct em28xx *dev)
{
@@ -2754,8 +2754,6 @@
em28xx_release_analog_resources(dev);
- em28xx_remove_from_devlist(dev);
-
em28xx_i2c_unregister(dev);
v4l2_device_unregister(&dev->v4l2_dev);
@@ -3152,7 +3150,7 @@
/*
* em28xx_usb_disconnect()
- * called when the device gets diconencted
+ * called when the device gets disconnected
* video device will be unregistered on v4l2_close in case it is still open
*/
static void em28xx_usb_disconnect(struct usb_interface *interface)
--- linux-3.0/drivers/media/video/em28xx/em28xx-core.c.orig 2011-08-18 23:07:51.0 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-core.c 2011-08-19 00:27:00.0 +0100
@@ -1160,18 +1160,6 @@
static DEFINE_MUTEX(em28xx_devlist_mutex);
/*
- * em28xx_realease_resources()
- * unregisters the v4l2,i2c and usb devices
- * called when the device gets disconected or at module unload
-*/
-void em28xx_remove_from_devlist(struct em28xx *dev)
-{
- mutex_lock(&em28xx_devlist_mutex);
- list_del(&dev->devlist);
- mutex_unlock(&em28xx_devlist_mutex);
-};
-
-/*
* Extension interface
*/
@@ -1221,14 +1209,13 @@
void em28xx_close_extension(struct em28xx *dev)
{
- struct em28xx_ops *ops = NULL;
+ const struct em28xx_ops *ops = NULL;
mutex_lock(&em28xx_devlist_mutex);
- if (!list_empty(&em28xx_extension_devlist)) {
- list_for_each_entry(ops, &em28xx_extension_devlist, next) {
- if (ops->fini)
-ops->fini(dev);
- }
+ list_for_each_entry(ops, &em28xx_extension_devlist, next) {
+ if (ops->fini)
+ ops->fini(dev);
}
+ list_del(&dev->devlist);
mutex_unlock(&em28xx_devlist_mutex);
}
[PATCH 6/6] EM28xx - don't sleep on disconnect
The DVB framework will try to power-down an adapter that no-one is using any
more, but this assumes that the adapter is still connected to the machine.
That's not always true for a USB adapter, so disable the sleep operations when
the adapter has been physically unplugged.
This prevents I2C write failures with error -19 from appearing occasionally in
the dmesg log.
Signed-off-by: Chris Rankin
--- linux-3.0/drivers/media/common/tuners/tda18271-fe.c.orig 2011-08-18 16:55:53.0 +0100
+++ linux-3.0/drivers/media/common/tuners/tda18271-fe.c 2011-08-18 23:12:55.0 +0100
@@ -1230,7 +1230,7 @@
return 0;
}
-static struct dvb_tuner_ops tda18271_tuner_ops = {
+static const struct dvb_tuner_ops tda18271_tuner_ops = {
.info = {
.name = "NXP TDA18271HD",
.frequency_min = 4500,
--- linux-3.0/drivers/media/dvb/frontends/cxd2820r_core.c.orig 2011-08-18 16:56:02.0 +0100
+++ linux-3.0/drivers/media/dvb/frontends/cxd2820r_core.c 2011-08-18 23:14:06.0 +0100
@@ -778,7 +778,7 @@
}
EXPORT_SYMBOL(cxd2820r_get_tuner_i2c_adapter);
-static struct dvb_frontend_ops cxd2820r_ops[2];
+static const struct dvb_frontend_ops cxd2820r_ops[2];
struct dvb_frontend *cxd2820r_attach(const struct cxd2820r_config *cfg,
struct i2c_adapter *i2c, struct dvb_frontend *fe)
@@ -844,7 +844,7 @@
}
EXPORT_SYMBOL(cxd2820r_attach);
-static struct dvb_frontend_ops cxd2820r_ops[2] = {
+static const struct dvb_frontend_ops cxd2820r_ops[2] = {
{
/* DVB-T/T2 */
.info = {
--- linux-3.0/drivers/media/video/em28xx/em28xx-dvb.c.orig 2011-08-17 08:52:30.0 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-dvb.c 2011-08-18 23:17:42.0 +0100
@@ -720,6 +720,12 @@
goto ret;
}
+static inline void prevent_sleep(struct dvb_frontend_ops *ops) {
+ ops->set_voltage = NULL;
+ ops->sleep = NULL;
+ ops->tuner_ops.sleep = NULL;
+}
+
static int dvb_fini(struct em28xx *dev)
{
if (!dev->board.has_dvb) {
@@ -728,8 +734,17 @@
}
if (dev->dvb) {
- unregister_dvb(dev->dvb);
- kfree(dev->dvb);
+ struct em28xx_dvb *dvb = dev->dvb;
+
+ if (dev->state & DEV_DISCONNECTED) {
+ /* We cannot tell the device to sleep
+ * once it has been unplugged. */
+ prevent_sleep(&dvb->fe[0]->ops);
+ prevent_sleep(&dvb->fe[1]->ops);
+ }
+
+ unregister_dvb(dvb);
+ kfree(dvb);
dev->dvb = NULL;
}
[PATCH 5/6] EM28xx - move printk lines outside mutex lock
There's no reason to still be holding the device list mutex for either of these
printk statements.
Signed-off-by: Chris Rankin
--- linux-3.0/drivers/media/video/em28xx/em28xx-core.c.orig 2011-08-18 23:05:50.0 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-core.c 2011-08-18 23:07:02.0 +0100
@@ -1186,8 +1186,8 @@
list_for_each_entry(dev, &em28xx_devlist, devlist) {
ops->init(dev);
}
- printk(KERN_INFO "Em28xx: Initialized (%s) extension\n", ops->name);
mutex_unlock(&em28xx_devlist_mutex);
+ printk(KERN_INFO "Em28xx: Initialized (%s) extension\n", ops->name);
return 0;
}
EXPORT_SYMBOL(em28xx_register_extension);
@@ -1200,9 +1200,9 @@
list_for_each_entry(dev, &em28xx_devlist, devlist) {
ops->fini(dev);
}
- printk(KERN_INFO "Em28xx: Removed (%s) extension\n", ops->name);
list_del(&ops->next);
mutex_unlock(&em28xx_devlist_mutex);
+ printk(KERN_INFO "Em28xx: Removed (%s) extension\n", ops->name);
}
EXPORT_SYMBOL(em28xx_unregister_extension);
[PATCH 4/6] EM28xx - clean up resources should init fail
This patch ensures that the em28xx_init_dev() function cleans up after itself,
in the event that it fails. This isimportant because the struct em28xx will be
deallocated if em28xx_init_dev() returns an error.
Signed-off-by: Chris Rankin
--- linux-3.0/drivers/media/video/em28xx/em28xx-cards.c.orig 2011-08-18 22:42:03.0 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-cards.c 2011-08-18 22:55:52.0 +0100
@@ -2776,7 +2776,6 @@
{
struct em28xx *dev = *devhandle;
int retval;
- int errCode;
dev->udev = udev;
mutex_init(&dev->ctrl_urb_lock);
@@ -2858,7 +2857,7 @@
/* Resets I2C speed */
em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, dev->board.i2c_speed);
if (retval < 0) {
- em28xx_errdev("%s: em28xx_write_regs_req failed!"
+ em28xx_errdev("%s: em28xx_write_reg failed!"
" retval [%d]\n",
__func__, retval);
return retval;
@@ -2872,12 +2871,11 @@
}
/* register i2c bus */
- errCode = em28xx_i2c_register(dev);
- if (errCode < 0) {
- v4l2_device_unregister(&dev->v4l2_dev);
+ retval = em28xx_i2c_register(dev);
+ if (retval < 0) {
em28xx_errdev("%s: em28xx_i2c_register - errCode [%d]!\n",
- __func__, errCode);
- return errCode;
+ __func__, retval);
+ goto fail_reg_i2c;
}
/*
@@ -2891,11 +2889,11 @@
em28xx_card_setup(dev);
/* Configure audio */
- errCode = em28xx_audio_setup(dev);
- if (errCode < 0) {
- v4l2_device_unregister(&dev->v4l2_dev);
+ retval = em28xx_audio_setup(dev);
+ if (retval < 0) {
em28xx_errdev("%s: Error while setting audio - errCode [%d]!\n",
- __func__, errCode);
+ __func__, retval);
+ goto fail_setup_audio;
}
/* wake i2c devices */
@@ -2909,31 +2907,28 @@
if (dev->board.has_msp34xx) {
/* Send a reset to other chips via gpio */
- errCode = em28xx_write_reg(dev, EM28XX_R08_GPIO, 0xf7);
- if (errCode < 0) {
- em28xx_errdev("%s: em28xx_write_regs_req - "
+ retval = em28xx_write_reg(dev, EM28XX_R08_GPIO, 0xf7);
+ if (retval < 0) {
+ em28xx_errdev("%s: em28xx_write_reg - "
"msp34xx(1) failed! errCode [%d]\n",
- __func__, errCode);
- return errCode;
+ __func__, retval);
+ goto fail_write_reg;
}
msleep(3);
- errCode = em28xx_write_reg(dev, EM28XX_R08_GPIO, 0xff);
- if (errCode < 0) {
- em28xx_errdev("%s: em28xx_write_regs_req - "
+ retval = em28xx_write_reg(dev, EM28XX_R08_GPIO, 0xff);
+ if (retval < 0) {
+ em28xx_errdev("%s: em28xx_write_reg - "
"msp34xx(2) failed! errCode [%d]\n",
- __func__, errCode);
- return errCode;
+ __func__, retval);
+ goto fail_write_reg;
}
msleep(3);
}
- em28xx_add_into_devlist(dev);
-
retval = em28xx_register_analog_devices(dev);
if (retval < 0) {
- em28xx_release_resources(dev);
- goto fail_reg_devices;
+ goto fail_reg_analog_devices;
}
em28xx_init_extension(dev);
@@ -2943,7 +2938,14 @@
return 0;
-fail_reg_devices:
+fail_setup_audio:
+fail_write_reg:
+fail_reg_analog_devices:
+ em28xx_i2c_unregister(dev);
+
+fail_reg_i2c:
+ v4l2_device_unregister(&dev->v4l2_dev);
+
return retval;
}
--- linux-3.0/drivers/media/video/em28xx/em28xx-core.c.orig 2011-08-17 08:52:25.0 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-core.c 2011-08-18 22:51:59.0 +0100
@@ -1171,13 +1171,6 @@
mutex_unlock(&em28xx_devlist_mutex);
};
-void em28xx_add_into_devlist(struct em28xx *dev)
-{
- mutex_lock(&em28xx_devlist_mutex);
- list_add_tail(&dev->devlist, &em28xx_devlist);
- mutex_unlock(&em28xx_devlist_mutex);
-};
-
/*
* Extension interface
*/
@@ -1215,14 +1208,13 @@
void em28xx_init_extension(struct em28xx *dev)
{
- struct em28xx_ops *ops = NULL;
+ const struct em28xx_ops *ops = NULL;
mutex_lock(&em28xx_devlist_mutex);
- if (!list_empty(&em28xx_extension_devlist)) {
- list_for_each_entry(ops, &em28xx_extension_devlist, next) {
- if (ops->init)
-ops->init(dev);
- }
+ list_add_tail(&dev->devlist, &em28xx_devlist);
+ list_for_each_entry(ops, &em28xx_extension_devlist, next) {
+ if (ops->init)
+ ops->init(dev);
}
mutex_unlock(&em28xx_devlist_mutex);
}
Re: Embedded device and the V4L2 API support - Was: [GIT PATCHES FOR 3.1] s5p-fimc and noon010pc30 driver updates
Hi Mauro, On 08/17/2011 08:13 AM, Mauro Carvalho Chehab wrote: > It seems that there are too many miss understandings or maybe we're just > talking the same thing on different ways. > > So, instead of answering again, let's re-start this discussion on a > different way. > > One of the requirements that it was discussed a lot on both mailing > lists and on the Media Controllers meetings that we had (or, at least > in the ones where I've participated) is that: > > "A pure V4L2 userspace application, knowing about video device >nodes only, can still use the driver. Not all advanced features >will be available." What does a term "a pure V4L2 userspace application" mean here ? Does it also account an application which is linked to libv4l2 and uses calls specific to a particular hardware which are included there? > > This is easily said than done. Also, different understandings can be > obtained by a simple phrase like that. -- Regards, Sylwester -- 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/6] EM28xx - use atomic bit operations for devices-in-use mask
Use atomic bit operations for the em28xx_devused mask, to prevent an unlikely
race condition should two adapters be plugged in simultaneously. The operations
also clearer than explicit bit manipulation anyway.
Signed-off-by: Chris Rankin
--- linux-3.0/drivers/media/video/em28xx/em28xx-cards.c.orig 2011-08-18 22:24:12.0 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-cards.c 2011-08-18 22:22:14.0 +0100
@@ -60,7 +60,7 @@
module_param_array(card, int, NULL, 0444);
MODULE_PARM_DESC(card, "card type");
-/* Bitmask marking allocated devices from 0 to EM28XX_MAXBOARDS */
+/* Bitmask marking allocated devices from 0 to EM28XX_MAXBOARDS - 1 */
static unsigned long em28xx_devused;
struct em28xx_hash_table {
@@ -2763,7 +2763,7 @@
usb_put_dev(dev->udev);
/* Mark device as unused */
- em28xx_devused &= ~(1 << dev->devno);
+ clear_bit(dev->devno, &em28xx_devused);
};
/*
@@ -2967,8 +2967,16 @@
ifnum = interface->altsetting[0].desc.bInterfaceNumber;
/* Check to see next free device and mark as used */
- nr = find_first_zero_bit(&em28xx_devused, EM28XX_MAXBOARDS);
- em28xx_devused |= 1<= EM28XX_MAXBOARDS) {
+ /* No free device slots */
+ printk(DRIVER_NAME ": Supports only %i em28xx boards.\n",
+ EM28XX_MAXBOARDS);
+ retval = -ENOMEM;
+ goto err_no_slot;
+ }
+ } while (test_and_set_bit(nr, &em28xx_devused));
/* Don't register audio interfaces */
if (interface->altsetting[0].desc.bInterfaceClass == USB_CLASS_AUDIO) {
@@ -2979,7 +2987,6 @@
ifnum,
interface->altsetting[0].desc.bInterfaceClass);
- em28xx_devused &= ~(1<= EM28XX_MAXBOARDS) {
- printk(DRIVER_NAME ": Supports only %i em28xx boards.\n",
-EM28XX_MAXBOARDS);
- em28xx_devused &= ~(1alt_max_pkt_size);
mutex_unlock(&dev->lock);
kfree(dev);
@@ -3147,6 +3141,10 @@
return 0;
err:
+ clear_bit(nr, &em28xx_devused);
+
+err_no_slot:
+ usb_put_dev(udev);
return retval;
}
[PATCH 2/6] Fix memory leak on disconnect or error.
Release the dev->alt_max_pkt_size buffer in all cases.
Signed-off-by: Chris Rankin
--- linux-3.0/drivers/media/video/em28xx/em28xx-video.c.orig 2011-08-18 17:20:10.0 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-video.c 2011-08-18 17:20:33.0 +0100
@@ -2202,6 +2202,7 @@
free the remaining resources */
if (dev->state & DEV_DISCONNECTED) {
em28xx_release_resources(dev);
+ kfree(dev->alt_max_pkt_size);
kfree(dev);
return 0;
}
--- linux-3.0/drivers/media/video/em28xx/em28xx-cards.c.orig 2011-08-17 08:52:19.0 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-cards.c 2011-08-18 22:09:32.0 +0100
@@ -3128,6 +3128,7 @@
retval = em28xx_init_dev(&dev, udev, interface, nr);
if (retval) {
em28xx_devused &= ~(1alt_max_pkt_size);
mutex_unlock(&dev->lock);
kfree(dev);
goto err;
[PATCH 1/6 ] EM28xx - pass correct buffer size to snprintf
snprintf()'s size parameter includes space for the terminating '\0' character. Signed-off-by: Chris Rankin --- linux-3.0/drivers/media/video/em28xx/em28xx-cards.c.orig 2011-08-17 08:52:19.0 +0100 +++ linux-3.0/drivers/media/video/em28xx/em28xx-cards.c 2011-08-18 22:03:05.0 +0100 @@ -3085,7 +3085,7 @@ goto err; } - snprintf(dev->name, 29, "em28xx #%d", nr); + snprintf(dev->name, sizeof(dev->name), "em28xx #%d", nr); dev->devno = nr; dev->model = id->driver_info; dev->alt = -1;
