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
