On Wednesday 08 November 2006 22:53, Robert Hardy wrote:
> Here is an updated version of my patch using memcpy_toio.
> I also trimmed a bunch of extra debugging that I had added.
> This version also work fine for me here.
>
> http://webcon.ca/~rhardy/patches/ivtv-0.8.0_svn3507-fix_firmware_load
>ingV2.patch.bz2
>
> On Wed, 8 Nov 2006, Tyler Trafford wrote:
> > > The third call to load_fw_direct, was passing data[2] which was
> > > in theory seemed to be coming out of some decoder mailbox which,
> > > given the results, can obviously have garbage in it... Hard
> > > coding the third call too prevents this kind of problem.
> >
> > Ah, I was unaware of that- that is a real problem.
>
> Agreed :)
For the record: the ivtv_api_getresult_nosleep() call sets data[2] to
the maximum amount of data the MPEG decoder will accept. That however
bears no relationship to the actual init-mpeg file. So this is a pure
bug.
Regarding the firmware load: from your tests it is clear that the size
can sometimes be the size of an earlier firmware load. My question is:
it is only the size or is it also the actual contents?
The reason I ask is that the two encoder and decoder firmwares are
identical in size (you seem to be using an older encoder firmware that
was actually padded with garbage bytes, on ivtvdriver.org the
firmware.tar.gz contains a version that's just 256 kB). Is it possible
that instead of loading the decoder firmware you are actually loading
the encoding firmware? So not only the fw->size is wrong, but the whole
file is wrong. There is no way to actually detect that (except that you
probably cannot find a decoder mailbox).
It's not clear from the discussion whether the problem is just fw->size
or if that just tells you that the wrong firmware was loaded. I
consider this to be quite a horrible kernel bug, BTW.
Also, your patch is quite big for what basically amounts to:
if (fw->size < size) {
IVTV_ERR("invalid firmware size\n");
return -EINVAL;
}
retval = size;
The warning can be improved upon and some comments are needed inside
the 'if', but otherwise that's all you need as far as I can tell.
Thanks,
Hans
_______________________________________________
ivtv-devel mailing list
[email protected]
http://ivtvdriver.org/mailman/listinfo/ivtv-devel
