Re: [PATCH 1/8] get_dvb_firmware: Add firmware extractor for si2165

[Matthias Schwarzott <zzam@gentoo.org>]

On 29.07.2014 21:45, Antti Palosaari wrote:
> 
> Do you need to know whole firmware version?
There is only 1 byte to be used and it is called patch version.
> How did you obtain it, from
> sniff?
Yes - but it also is visible in code near crc version (see below).

> What happens if you don't tell fw version to chip at all?
>
In other places it is read to verify a fw was uploaded (compare to be
not equal 0x00).
I guess the exact value is never needed (so just for information).
But I did not try it.

> Usually, almost 100%, firmware version as well all the other needed
> information, is included to firmware image itself. I don't remember many
> cases where special handling is needed. One (only one?) of such case is
> af9013, where I resolved issues by calculating fw checksum by the
> driver. IIRC chip didn't boot if there was wrong checksum for fw.

The checksum is not needed to get the device working.
The chip itself only calculates it when uploading data - and the driver
reads out the calculated checksum and compares it to the expected value.
It is only a verification of the correct upload.

> 
> Own headers and checksums causes troubles if I someone would like to
> extract different firmwares from various windows binaries to test.
> 
> If windows driver needs to know that kind of things, those are usually
> found very near firmware image from the driver binary. Most often just
> dump 32 bytes after firmware image and it is somewhere there. Or before
> firmware image. That is because those are values are stored to same
> source code file => compiler puts that stuff ~same location.
> 
I had a look at the driver - the code itself has the constants compiled
in - they are really mixed with the assembly code.

Rewritten in C it is code that has fixed values as parameters to functions.

ret = load_firmware(firmware,
  0x12, /* patch version */
  48, /* block count */
  0xaa0c /* crc */
);

I also would prefer your version with static const variables near the data.

> static const unsigned char firmware[] = {
>   0x00,0x11,0x22,0x33,0x44,0x55,0x66,0x77,
>   0x88,0x99,0xaa,0xbb,0xcc,0xdd,0xee,0xff,
> };
> 
> static const unsigned int firmware_checksum = 0x01234567;
> static const unsigned int firmware_version = 0x0000002b;
> 
Regards
Matthias