On Tue, 7 Nov 2006, Hans Verkuil wrote: > On Tuesday 07 November 2006 21:44, Robert Hardy wrote: > > hverkuil: Please consider commiting this patch (or an adaption of > > it.) > > > > After a recent upgrade to a 2.6.18.2 based kernel with ivtv-0.8.0 > > SVN, my delay patch (which I posted a couple months back) stopped > > working. I was forced to dig deeper into how firmware was being > > handled in the ivtv driver. > > > > While debugging I found that fw->size tends to fluctuate between > > reboots and in certain instances up to an extra 700KB of random data > > was being read and writen after some firmware files (especially > > common in > > IVTV_DECODE_INIT_MPEG_FILENAME's case.) > > > > The following code rewrites the ivtv-firmware code so that it has > > more checks and balances and more closely matches recently discussed > > proper firmware_class usage examples provided by a new firmware_class > > maintainer as seen on the kernel mailing list. > > Can you give me some links to the relevant postings on the kernel > mailing list? Before I apply a patch like this I'd like to learn > something about the background.
There are several different threads on the kernel mailing list about this. The unpatched documentation in 2.6.18.x vanilla is bad/wrong in places and the examples apparently won't build. Unfortunately, as sometimes happens on the list, there was lots of discussion but nothing was actually committed to replace the current broken examples. Here is a search for the discussion: http://www.gossamer-threads.com/lists/engine?list=linux;do=search_results;search_forum=forum_1;search_string=request_firmware;search_type=AND&sb=post_time Here is one of various attempts at getting the docs fixed (patch won't apply): http://www.gossamer-threads.com/lists/linux/kernel/684433?search_string=request_firmware;#684433 Here is my patch which applies to 2.6.18.x which takes into account all the discussion on the list: http://webcon.ca/~rhardy/patches/linux-2.6.18-request_firmware_examples.patch.bz2 > > The most important modification is the code now no longer blindly > > writes all the data fw->size returns if it makes no sense to! > > > > With udev-0.95 and the patch below I haven't seen any firmware > > loading issues. Modprobe ivtv also seems to go faster but I didn't > > benchmark it. > > Does this mean that you occasionally see retries of the firmware > loading? BTW, reading through your patch it is clear that the handling > of the initial mpeg file is wrong and should be fixed, but the loading > of the firmwares just adds checks and returns an error in case of a > failure. So I'd expect retries of the firmware loading. I'm not sure what you mean about retrying. Any given modprobe ivtv will trigger three load_fw_direct calls here. Any given load_fw_direct call will do one of: load a firmware properly, fail and output an error in the logs or locks up the system (i.e. total lockup even the keyboard is dead.) The first two calls to load_fw_direct almost always load properly, in vanilla 0.8.0 code the last call almost always failed here. Recently an attempt to load v4l-cx2341x-init.mpg almost invariably resulted in a lockup and the loading attempt would print an erroneous size for the firmware as seen below (this was the last syslog entry before lockup): Nov 2 13:19:30 vortex kernel: ivtv0: loaded v4l-cx2341x-init.mpg firmware (376836 bytes) I've been using ivtv for years and many times I've been in a situation where initialization was unreliable, but once completely loaded the ivtv driver would work well. To get it to initialize fully one would have to either reload the driver repeatedly until it succeeded, or in some cases power cycle the box repeated until the driver would initialize properly. Occasionally I would get lucky and find a really stable kernel/driver combination or find a good delay which caused it to almost always initialize properly, as my patch below did until recently (it only seemed to work for some people though): http://webcon.ca/~rhardy/patches/ivtv-0.6.0-delay_request_firmware.patch.bz2 When I added some debugging to ivtv's output, without really changing the function of the code, I started to see this for the third firmware: Nov 6 18:33:37 vortex kernel: ivtv0: loading v4l-cx2341x-init.mpg firmware (size=1048576 bytes;fw->size=376836 bytes) Obviously a size=1MB is wrong and so because the old load_fw_direct code used "fw->size" for it's re-implemented memcpy, unless "size" was smaller, it was likely scribbling random data over something important.... Try as I might using data[2] as an argument to load_fw_direct in ivtv_init_mpeg_decoder in ivtv-firmware.c made no sense to me. data[2] started as itv->params.height (480 usually here) until ivtv_api_getresult_nosleep was called then it got overwritten with 1048576 which still made no sense. The code was ugly, so I avoided the problems much the same way the first two calls to load_fw_direct had, my using a defined constant, in this case IVTV_DECODE_INIT_MPEG_SIZE. Hope that helps. Regards, Rob -- ---------------------"Happiness is understanding."---------------------- Robert Hardy, B.Eng Computer Systems C.E.O. Webcon Inc. rhardy <at> webcon <dot> ca GPG Key available (613) 276-7327 _______________________________________________ ivtv-devel mailing list ivtv-devel@ivtvdriver.org http://ivtvdriver.org/mailman/listinfo/ivtv-devel
