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.
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.
svn diff
Index: ivtv-firmware.c
===================================================================
--- ivtv-firmware.c (revision 3507)
+++ ivtv-firmware.c (working copy)
@@ -86,26 +86,44 @@
const struct firmware *fw = NULL;
int retval = -ENOMEM;
- if (request_firmware(&fw, fn, FWDEV(itv)) == 0) {
- int i;
+ if ( (retval = request_firmware(&fw, fn, FWDEV(itv))) == 0 ) {
u32 *dst = (u32 *)mem;
const u32 *src = (const u32 *)fw->data;
+ if (fw->size <= 150000) {
+ IVTV_ERR("firmware %s is invalid. It is way too
small.\n", fn);
+ release_firmware(fw);
+ return -EINVAL;
+ }
+
+ // Under Linux 2.6.18.2, using udev 0.75, for a given
+ // firmware file, fw->size seems to fluctuates randomly
+ // between the file's real size and 1024*1024 bytes. With
+ // udev 0.95 fw->size is more accurate, but we can't assume
+ // that is present so we shouldn't blindly use fw->size for
+ // firmware loading, or we will have bad memcpys etc.
if (fw->size >= size) {
+ // If there are at least "size" bytes of valid firmware
load only "size" worth.
retval = size;
+ IVTV_INFO("loading %s firmware (size=%ld
bytes;fw->size=%d bytes)\n", fn, size, fw->size);
+ memcpy(dst, src, retval);
} else {
- retval = fw->size;
+ // Otherwise fail loudly
+ IVTV_ERR("reported size of firmware %s is %d. It
should be at least %ld, loading aborting!\n", fn, fw->size, size);
+ retval = -EINVAL;
}
- for (i = 0; i < retval; i += 4) {
- writel(*src, dst);
- dst++;
- src++;
+ IVTV_DEBUG_INFO("releasing %s firmware (%d bytes)\n", fn,
retval);
+ release_firmware(fw);
+
+ if (retval >= 0) {
+ IVTV_INFO("loaded %s firmware (%d bytes)\n", fn,
retval);
}
- release_firmware(fw);
- IVTV_INFO("loaded %s firmware (%d bytes)\n", fn, retval);
} else {
- IVTV_INFO("unable to open firmware %s\n", fn);
- IVTV_INFO("did you put the firmware in the hotplug firmware
directory?\n");
+ if (retval == -ENOENT) {
+ IVTV_ERR("unable to open firmware %s: file not
found.\n", fn);
+ IVTV_ERR("did you put the firmware in the hotplug
firmware directory?\n");
+ } else
+ IVTV_ERR("cannot request firmware %s: (error %i)\n",
fn, retval);
}
return retval;
@@ -565,6 +583,7 @@
data[2] = itv->params.height;
data[3] = itv->params.audio_properties; /* Audio settings to use,
bitmap. see docs. */
+ IVTV_DEBUG_INFO("DEBUG ivtv_init_mpeg_decoder 1: data[2]=%u\n",
data[2]);
if ( ivtv_api(itv, itv->dec_mbox,
&itv->dec_msem,
CX2341X_DEC_SET_DECODER_SOURCE, &result, 4, &data[0]) !=
0)
@@ -572,6 +591,7 @@
IVTV_ERR("ivtv_init_mpeg_decoder failed to set decoder
source\n");
return;
}
+ IVTV_DEBUG_INFO("DEBUG ivtv_init_mpeg_decoder 2: data[2]=%u\n",
data[2]);
if (ivtv_vapi(itv, CX2341X_DEC_START_PLAYBACK, 2,
itv->dec_options.gop_offset,
@@ -580,10 +600,20 @@
IVTV_ERR("ivtv_init_mpeg_decoder failed to start playback\n");
return;
}
+ IVTV_DEBUG_INFO("DEBUG ivtv_init_mpeg_decoder 3: data[2]=%u\n",
data[2]);
ivtv_api_getresult_nosleep(itv, &itv->dec_mbox[9], &result, &data[0]);
+
+ // data[2] changes from 480 to 1048576 after the call immediately above
+ IVTV_DEBUG_INFO("DEBUG ivtv_init_mpeg_decoder 4: data[2]=%u\n",
data[2]);
mem_offset = itv->dec_mem+ data[1];
- if ((readbytes = load_fw_direct(IVTV_DECODE_INIT_MPEG_FILENAME, (char
*)mem_offset, itv,data[2])) <= 0)
+ IVTV_DEBUG_INFO("DEBUG ivtv_init_mpeg_decoder 5: data[2]=%u\n",
data[2]);
+
+ // Using data[2] instead of IVTV_DECODE_INIT_MPEG_SIZE in
+ // load_fw_direct below results in an attempt to load 1048576 bytes
+ // of firmware from a 155648 byte file, causing all sorts of module
+ // init failures and system lockups.
+ if ((readbytes = load_fw_direct(IVTV_DECODE_INIT_MPEG_FILENAME, (char
*)mem_offset, itv, IVTV_DECODE_INIT_MPEG_SIZE)) <= 0)
{
IVTV_INFO("failed to read mpeg decoder initialisation file
%s\n",IVTV_DECODE_INIT_MPEG_FILENAME);
} else
Index: ivtv-driver.h
===================================================================
--- ivtv-driver.h (revision 3507)
+++ ivtv-driver.h (working copy)
@@ -474,6 +474,7 @@
/*Used for locating the firmware mailboxes*/
#define IVTV_DECODE_INIT_MPEG_FILENAME "v4l-cx2341x-init.mpg"
+#define IVTV_DECODE_INIT_MPEG_SIZE 152*1024
#define IVTV_FIRM_IMAGE_SIZE 256*1024
#define IVTV_FIRM_SEARCH_ENCODER_START 0x00000000
ivtv-0.8.0_svn3507-fix_firmware_loading.patch.bz2
Description: BZip2 compressed data
_______________________________________________ ivtv-devel mailing list [email protected] http://ivtvdriver.org/mailman/listinfo/ivtv-devel
