Aw: Re: [PATCH 2/2] media: si2157: Add optional firmware download

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Von: "Mauro Carvalho Chehab" <mchehab+huawei@xxxxxxxxxx>

> See, while the technical docs for this device are not publicity
> available, the block diagram for this device on its "advertising"
> datasheet:
> https://media.digikey.com/pdf/Data%20Sheets/Silicon%20Laboratories%20PDFs/Si2157.pdf
> 
> Doesn't show any internal ROM/eeprom. So, it sounds to me that
> either some external rom chip is needed or its firmware should
> load via I2C.

I am very certain that the Silabs Si21xx tuners and demodulators
all have an embedded firmware ROM. Unfortunately, I cannot find a
public datasheet that explicitly mentions this, so I can only
provide "circumstantial evidence":

- There are very simple USB TV sticks out there e.g. with only a
  Cypress FX2LP on them and SiLabs tuner/demod. Those work in
  Linux without the Linux driver downloading a firmware onto the
  tuner. Where else would the tuner firmware then come from? The
  FX2LP firmware does not have such function, the demod does not
  do that either, and the datasheet you found also does not show
  that the tuner would be capable of downloading it from an
  attached EEPROM either.

- The last two digits of the part number are actually the ROM
  ROM firmware version (e.g. Si2157-A30 has firmware 3.0[.5],
  or Si2183-B60 has firmware 6.0[.2]).

- And one little hint "officialy" from SiLabs, in their driver
  code at:

  https://github.com/SiliconLabs/video_si21xx_superset/blob/master/SILABS_SUPERSET/TER/Si2157/Si2157_Commands.h

  Note that Si2157_PART_INFO_CMD_REPLY_struct has a field
  "rom_id" in it. So it must have some sort of ROM.

> So, my main concern here is with regards to other devices that
> are using si2157 driver. Among those:
> 
> - Some may have no eeproms;
> - Some may have an eeprom with compatible firmware;
> - Some may have an eeprom with a too old firmware.

I think it's very unlikely that ANY device out there actually has
an EEPROM with Si2157 firmware on it.

> On other words, the only way to ensure that the device will
> be in sane state and be fully supported by the driver is to
> load a known-to work firmware.

Ah, that's actually a different point, which I agree is valid:
The driver code must match the firmware API version running on
the tuner. So if a very different firmware was running on the
tuner, the Linux driver might not be compatible with it.

There are two ways to go about this:

1. Support only one specific firmware version in the driver
   and error out in init if a potentially required firmware
   file is not available.

2. Being more tolerant about this and only outputting a
   warning that the firmware running on the tuner does not
   match the version the driver was developed/tested against
   and might not work right, and that providing a firmware
   patch file might fix that.

I would prefer option 2 as it is more user-friendly.

> Do you have access to all the technical datasheet and
> application notes for all chips supported by this driver?

I wish. AFAIK, Antti developed the Linux drivers using
reverse engineering of the Windows drivers.

The situation is a bit better now, as SiLabs has now
published their own driver source code:

https://github.com/SiliconLabs/video_si21xx_superset

Ideally, someone would have a lot of spare time to
shuffle through that source code and e.g. correct some
incorrect command or parameter descriptions in the
Linux driver code...

> If you have, could you please describe why only SI2157_A30
> is safe with regards to firmware?
> 
> If not, then checking for chip_id == SI2157_A30 makes no
> sense.

The _existing_ logic is that if chip_id == SI2157_A30, no
firmware will ever be downloaded into the chip. My change
made it possible to _optionally_ download firmware into the
chip, but also proceeding without firmware, behaving
exactly as before. So it is "safe" with regards of ensuring
there are no regressions introduced in the Linux driver.

It is not "safe" with regards to _improving_ the existing
firmware handling, but that was not my goal. If you want to
expand the scope, you're welcome.

But I think what you are proposing is much more risky than
my patch, if only because you're touching far more code
lines.

> #define FIRMWARE_MIN_VERSION 0x123456 // FIXME: add something here

No, this will have to be far more complex:

1. The driver supports several Si21xx tuners (note that the
   SiLabs code has one driver per model), mostly having
   different firmware versions, so you will have to provide
   firmware versions for every tuner model supported by the
   driver.

2. You might also want to provide at least a "range" of
   supported/tested firmware versions, if not even a full
   list of firmware versions.

However, I consider that an almost impossible undertaking.
Where are you going to get that list of firmware versions
from? Do you plan to have many, many contributors filling
that list patch by patch?

I'm not sure the benefit would be worth the effort.

Best Regards,
-Robert Schlabbach




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux