[PATCH][media] SI2168: Resolve unknown chip version errors with different HVR22x5 models

[PATCH][media] SI2168: Resolve unknown chip version errors with different HVR22x5 models

[Steven Toth]

[-- Attachment #1: Type: text/plain, Size: 582 bytes --]

We're seeing a mix of SI2168 demodulators appearing on HVR2205 and
HVR2215 cards, the chips are stamped with different build dates,
verified on my cards.

The si2168 driver detects some cards fine, others not at all. I can
reproduce the working and non-working case. The fix, if we detect a
newer card (D40) load the B firmware.

This fix works well for me and properly enables DVB-T tuning behavior
using tzap.

Thanks to Peter Faulkner-Ball for describing his workaround.

Signed-off-By: Steven Toth <stoth@kernellabs.com>

-- 
Steven Toth - Kernel Labs
http://www.kernellabs.com

[-- Attachment #2: d40.patch --]
[-- Type: application/octet-stream, Size: 784 bytes --]

diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
index 5db588e..6682323 100644
--- a/drivers/media/dvb-frontends/si2168.c
+++ b/drivers/media/dvb-frontends/si2168.c
@@ -407,6 +407,7 @@ static int si2168_init(struct dvb_frontend *fe)
 	#define SI2168_A20 ('A' << 24 | 68 << 16 | '2' << 8 | '0' << 0)
 	#define SI2168_A30 ('A' << 24 | 68 << 16 | '3' << 8 | '0' << 0)
 	#define SI2168_B40 ('B' << 24 | 68 << 16 | '4' << 8 | '0' << 0)
+	#define SI2168_D40 (  0 << 24 |'D' << 16 | '4' << 8 | '0' << 0)
 
 	switch (chip_id) {
 	case SI2168_A20:
@@ -416,6 +417,7 @@ static int si2168_init(struct dvb_frontend *fe)
 		fw_name = SI2168_A30_FIRMWARE;
 		break;
 	case SI2168_B40:
+	case SI2168_D40:
 		fw_name = SI2168_B40_FIRMWARE;
 		break;
 	default:

Re: [PATCH][media] SI2168: Resolve unknown chip version errors with different HVR22x5 models

[Antti Palosaari]

On 06/04/2015 03:38 PM, Steven Toth wrote:
> We're seeing a mix of SI2168 demodulators appearing on HVR2205 and
> HVR2215 cards, the chips are stamped with different build dates,
> verified on my cards.
>
> The si2168 driver detects some cards fine, others not at all. I can
> reproduce the working and non-working case. The fix, if we detect a
> newer card (D40) load the B firmware.
>
> This fix works well for me and properly enables DVB-T tuning behavior
> using tzap.
>
> Thanks to Peter Faulkner-Ball for describing his workaround.

hymm, I am not sure that patch at all. It is Olli who has been 
responsible adding support for multiple chip revisions, so I will leave 
that for Olli. I have only 2 Si2168 devices and both are B40 version.

Anyhow, for me it looks like firmware major version is always increased 
when new major revision of chip is made. Due to that I expected 5.0 
after B version 5.0.
A 1.0
A 2.0
A 3.0
B 4.0
C 5.0 ?
D 6.0 ?


And how we could explain situation Olli has device that had been working 
earlier, but now it does not? Could you Olli look back you old git tree 
and test if it still works? One possible reason could be also PCIe 
interface I2C adapter bug. Or timing issue.


regards
Antti


-- 
http://palosaari.fi/

Re: [PATCH][media] SI2168: Resolve unknown chip version errors with different HVR22x5 models

[Steven Toth]

>> This fix works well for me and properly enables DVB-T tuning behavior
>> using tzap.
>>
>> Thanks to Peter Faulkner-Ball for describing his workaround.
>
>
> hymm, I am not sure that patch at all. It is Olli who has been responsible
> adding support for multiple chip revisions, so I will leave that for Olli. I
> have only 2 Si2168 devices and both are B40 version.
>
> Anyhow, for me it looks like firmware major version is always increased when
> new major revision of chip is made. Due to that I expected 5.0 after B
> version 5.0.
> A 1.0
> A 2.0
> A 3.0
> B 4.0
> C 5.0 ?
> D 6.0 ?

The other email describes the I2C reply from the part, its cleared a
D40, not a D 6.0.

> And how we could explain situation Olli has device that had been working
> earlier, but now it does not? Could you Olli look back you old git tree and
> test if it still works? One possible reason could be also PCIe interface I2C
> adapter bug. Or timing issue.

Unlikely a timing issues or an i2c bug.

In my case I have multiple cards. The second card was acquired later
and never tested, and exhibited the issue.

Olli would need to speak for himself.

-- 
Steven Toth - Kernel Labs
http://www.kernellabs.com

Re: [PATCH][media] SI2168: Resolve unknown chip version errors with different HVR22x5 models

[Olli Salonen]

I'll test it with my old code I should have hanging around still
somewhere. I'm sure the chip on my card has been previously been
identified as Si2168-B40 by the code (I posted the logs already
earlier) and it definitely has not turned into a Si2168-D40 chip
somehow.

I don't think there's version D here. The third byte in the answer
from the demod indicates which Si21xx chip is being used. For Si2168
there should be decimal value 68, for Si2157 there's value 57, etc.
This how every single Silabs chip I've seen so far indicates it. I
think it is just the fact that the ASCII value of letter D is 68 that
caused you to assume that there's revision D now.

In addition there's the firmware version numbering that Antti points
out. I do have Si2168 devices that have the A20, A30 and B40
firmwares. Also, for all these chips I can find some references in the
internet. There's nothing regarding a Si2168-D40 (which is not a
conclusive proof that one would not exist, of course).

Cheers,
-olli


On 4 June 2015 at 14:47, Antti Palosaari <crope@iki.fi> wrote:
> On 06/04/2015 03:38 PM, Steven Toth wrote:
>>
>> We're seeing a mix of SI2168 demodulators appearing on HVR2205 and
>> HVR2215 cards, the chips are stamped with different build dates,
>> verified on my cards.
>>
>> The si2168 driver detects some cards fine, others not at all. I can
>> reproduce the working and non-working case. The fix, if we detect a
>> newer card (D40) load the B firmware.
>>
>> This fix works well for me and properly enables DVB-T tuning behavior
>> using tzap.
>>
>> Thanks to Peter Faulkner-Ball for describing his workaround.
>
>
> hymm, I am not sure that patch at all. It is Olli who has been responsible
> adding support for multiple chip revisions, so I will leave that for Olli. I
> have only 2 Si2168 devices and both are B40 version.
>
> Anyhow, for me it looks like firmware major version is always increased when
> new major revision of chip is made. Due to that I expected 5.0 after B
> version 5.0.
> A 1.0
> A 2.0
> A 3.0
> B 4.0
> C 5.0 ?
> D 6.0 ?
>
>
> And how we could explain situation Olli has device that had been working
> earlier, but now it does not? Could you Olli look back you old git tree and
> test if it still works? One possible reason could be also PCIe interface I2C
> adapter bug. Or timing issue.
>
>
> regards
> Antti
>
>
> --
> http://palosaari.fi/

Re: [PATCH][media] SI2168: Resolve unknown chip version errors with different HVR22x5 models

[Olli Salonen]

I compiled an old HVR-2205 driver from my git tree:
https://github.com/trsqr/media_tree/tree/hvr2205

And kaboom, the device is identified correctly and correct firmware is loaded:

[  882.227379] si2168 1-0064: found a 'Silicon Labs Si2168-B40'
[  882.227763] si2168 1-0064: downloading firmware from file
'dvb-demod-si2168-b40-01.fw'
[  884.784024] si2168 1-0064: firmware version: 4.0.11
[  884.822457] si2157 3-0060: found a 'Silicon Labs Si2157-A30'
[  884.898296] si2157 3-0060: firmware version: 3.0.5

I also added some additional debug to printout the chip id string
returned by command 02:
[ 1169.056353] si2168 1-0064: chip id string: 80 42 44 34 30 02 00 00
00 00 00 00 01

However, the current media_tree driver makes the card return the
following value:
80 00 44 34 30 02 00 00 00 00 00 00 00

There is something wrong here. Why is the second byte 00 with the
current driver and 42 with my old driver? I don't think we should work
around this issue with the patch suggested.

Cheers,
-olli

On 4 June 2015 at 14:57, Olli Salonen <olli.salonen@iki.fi> wrote:
> I'll test it with my old code I should have hanging around still
> somewhere. I'm sure the chip on my card has been previously been
> identified as Si2168-B40 by the code (I posted the logs already
> earlier) and it definitely has not turned into a Si2168-D40 chip
> somehow.
>
> I don't think there's version D here. The third byte in the answer
> from the demod indicates which Si21xx chip is being used. For Si2168
> there should be decimal value 68, for Si2157 there's value 57, etc.
> This how every single Silabs chip I've seen so far indicates it. I
> think it is just the fact that the ASCII value of letter D is 68 that
> caused you to assume that there's revision D now.
>
> In addition there's the firmware version numbering that Antti points
> out. I do have Si2168 devices that have the A20, A30 and B40
> firmwares. Also, for all these chips I can find some references in the
> internet. There's nothing regarding a Si2168-D40 (which is not a
> conclusive proof that one would not exist, of course).
>
> Cheers,
> -olli
>
>
> On 4 June 2015 at 14:47, Antti Palosaari <crope@iki.fi> wrote:
>> On 06/04/2015 03:38 PM, Steven Toth wrote:
>>>
>>> We're seeing a mix of SI2168 demodulators appearing on HVR2205 and
>>> HVR2215 cards, the chips are stamped with different build dates,
>>> verified on my cards.
>>>
>>> The si2168 driver detects some cards fine, others not at all. I can
>>> reproduce the working and non-working case. The fix, if we detect a
>>> newer card (D40) load the B firmware.
>>>
>>> This fix works well for me and properly enables DVB-T tuning behavior
>>> using tzap.
>>>
>>> Thanks to Peter Faulkner-Ball for describing his workaround.
>>
>>
>> hymm, I am not sure that patch at all. It is Olli who has been responsible
>> adding support for multiple chip revisions, so I will leave that for Olli. I
>> have only 2 Si2168 devices and both are B40 version.
>>
>> Anyhow, for me it looks like firmware major version is always increased when
>> new major revision of chip is made. Due to that I expected 5.0 after B
>> version 5.0.
>> A 1.0
>> A 2.0
>> A 3.0
>> B 4.0
>> C 5.0 ?
>> D 6.0 ?
>>
>>
>> And how we could explain situation Olli has device that had been working
>> earlier, but now it does not? Could you Olli look back you old git tree and
>> test if it still works? One possible reason could be also PCIe interface I2C
>> adapter bug. Or timing issue.
>>
>>
>> regards
>> Antti
>>
>>
>> --
>> http://palosaari.fi/

Re: [PATCH][media] SI2168: Resolve unknown chip version errors with different HVR22x5 models

[Antti Palosaari]

I suspect saa7164 driver. It has some strange looking I2C hacks. It has 
even some logic to split register and data - which I2C adapter should 
not be aware at all - it just should transfer bulk bytes.

regards
Antti

On 06/04/2015 04:22 PM, Olli Salonen wrote:
> I compiled an old HVR-2205 driver from my git tree:
> https://github.com/trsqr/media_tree/tree/hvr2205
>
> And kaboom, the device is identified correctly and correct firmware is loaded:
>
> [  882.227379] si2168 1-0064: found a 'Silicon Labs Si2168-B40'
> [  882.227763] si2168 1-0064: downloading firmware from file
> 'dvb-demod-si2168-b40-01.fw'
> [  884.784024] si2168 1-0064: firmware version: 4.0.11
> [  884.822457] si2157 3-0060: found a 'Silicon Labs Si2157-A30'
> [  884.898296] si2157 3-0060: firmware version: 3.0.5
>
> I also added some additional debug to printout the chip id string
> returned by command 02:
> [ 1169.056353] si2168 1-0064: chip id string: 80 42 44 34 30 02 00 00
> 00 00 00 00 01
>
> However, the current media_tree driver makes the card return the
> following value:
> 80 00 44 34 30 02 00 00 00 00 00 00 00
>
> There is something wrong here. Why is the second byte 00 with the
> current driver and 42 with my old driver? I don't think we should work
> around this issue with the patch suggested.
>
> Cheers,
> -olli
>
> On 4 June 2015 at 14:57, Olli Salonen <olli.salonen@iki.fi> wrote:
>> I'll test it with my old code I should have hanging around still
>> somewhere. I'm sure the chip on my card has been previously been
>> identified as Si2168-B40 by the code (I posted the logs already
>> earlier) and it definitely has not turned into a Si2168-D40 chip
>> somehow.
>>
>> I don't think there's version D here. The third byte in the answer
>> from the demod indicates which Si21xx chip is being used. For Si2168
>> there should be decimal value 68, for Si2157 there's value 57, etc.
>> This how every single Silabs chip I've seen so far indicates it. I
>> think it is just the fact that the ASCII value of letter D is 68 that
>> caused you to assume that there's revision D now.
>>
>> In addition there's the firmware version numbering that Antti points
>> out. I do have Si2168 devices that have the A20, A30 and B40
>> firmwares. Also, for all these chips I can find some references in the
>> internet. There's nothing regarding a Si2168-D40 (which is not a
>> conclusive proof that one would not exist, of course).
>>
>> Cheers,
>> -olli
>>
>>
>> On 4 June 2015 at 14:47, Antti Palosaari <crope@iki.fi> wrote:
>>> On 06/04/2015 03:38 PM, Steven Toth wrote:
>>>>
>>>> We're seeing a mix of SI2168 demodulators appearing on HVR2205 and
>>>> HVR2215 cards, the chips are stamped with different build dates,
>>>> verified on my cards.
>>>>
>>>> The si2168 driver detects some cards fine, others not at all. I can
>>>> reproduce the working and non-working case. The fix, if we detect a
>>>> newer card (D40) load the B firmware.
>>>>
>>>> This fix works well for me and properly enables DVB-T tuning behavior
>>>> using tzap.
>>>>
>>>> Thanks to Peter Faulkner-Ball for describing his workaround.
>>>
>>>
>>> hymm, I am not sure that patch at all. It is Olli who has been responsible
>>> adding support for multiple chip revisions, so I will leave that for Olli. I
>>> have only 2 Si2168 devices and both are B40 version.
>>>
>>> Anyhow, for me it looks like firmware major version is always increased when
>>> new major revision of chip is made. Due to that I expected 5.0 after B
>>> version 5.0.
>>> A 1.0
>>> A 2.0
>>> A 3.0
>>> B 4.0
>>> C 5.0 ?
>>> D 6.0 ?
>>>
>>>
>>> And how we could explain situation Olli has device that had been working
>>> earlier, but now it does not? Could you Olli look back you old git tree and
>>> test if it still works? One possible reason could be also PCIe interface I2C
>>> adapter bug. Or timing issue.
>>>
>>>
>>> regards
>>> Antti
>>>
>>>
>>> --
>>> http://palosaari.fi/

-- 
http://palosaari.fi/

Re: [PATCH][media] SI2168: Resolve unknown chip version errors with different HVR22x5 models

[Steven Toth]

On Thu, Jun 4, 2015 at 9:22 AM, Olli Salonen <olli.salonen@iki.fi> wrote:
> I compiled an old HVR-2205 driver from my git tree:
> https://github.com/trsqr/media_tree/tree/hvr2205

https://github.com/trsqr/media_tree/commit/61c2ef874b8a9620f498c9a4ab4138e97119462b

That's the difference perhaps.

-- 
Steven Toth - Kernel Labs
http://www.kernellabs.com

Re: [PATCH][media] SI2168: Resolve unknown chip version errors with different HVR22x5 models

[Olli Salonen]

Hi Steven,

It seems to me that that part of the code is identical to your driver, no?

The media_tree driver:

retval = saa7164_api_i2c_read(bus,
                     msgs[i].addr,
                     0 /* reglen */,
                     NULL /* reg */, msgs[i].len, msgs[i].buf);

It's exactly the same with a little bit different formatting.

Cheers,
-olli


On 4 June 2015 at 16:03, Steven Toth <stoth@kernellabs.com> wrote:
> On Thu, Jun 4, 2015 at 9:22 AM, Olli Salonen <olli.salonen@iki.fi> wrote:
>> I compiled an old HVR-2205 driver from my git tree:
>> https://github.com/trsqr/media_tree/tree/hvr2205
>
> https://github.com/trsqr/media_tree/commit/61c2ef874b8a9620f498c9a4ab4138e97119462b
>
> That's the difference perhaps.
>
> --
> Steven Toth - Kernel Labs
> http://www.kernellabs.com

Re: [PATCH][media] SI2168: Resolve unknown chip version errors with different HVR22x5 models

[Antti Palosaari]

On 06/05/2015 04:40 PM, Olli Salonen wrote:
> Hi Steven,
>
> It seems to me that that part of the code is identical to your driver, no?
>
> The media_tree driver:
>
> retval = saa7164_api_i2c_read(bus,
>                       msgs[i].addr,
>                       0 /* reglen */,
>                       NULL /* reg */, msgs[i].len, msgs[i].buf);
>
> It's exactly the same with a little bit different formatting.

And that looks correct.

But the patch which does not look correct, or is at least unclear, is that
[media] saa7164: Improvements for I2C handling
http://permalink.gmane.org/gmane.comp.video.linuxtv.scm/22211

First change does not have any effect as len should be zero in any case 
and memcpy() should do nothing.

Second change looks something that is likely wrong. There is some hack 
which increases data len. All that register len stuff is logically wrong 
- I2C adapter handles just bytes and should not know nothing about 
client register layout. OK, there is some exceptions (like af9035) where 
I2C firmware actually knows register layout for some strange reason.

So could you remove that patch and test?

Antti

-- 
http://palosaari.fi/

Re: [PATCH][media] SI2168: Resolve unknown chip version errors with different HVR22x5 models

[Olli Salonen]

Hi,

Indeed, the HVR-2205 I have works fine with that patch reverted and
after setting REGLEN_0bit for the Si2168 chips in the saa7164-cards.

The chip detection and firmware load is correct now.

[ 2046.684246] si2168 2-0064: found a 'Silicon Labs Si2168-B40'
[ 2046.684278] si2168 2-0064: downloading firmware from file
'dvb-demod-si2168-b40-01.fw'
[ 2049.242810] si2168 2-0064: firmware version: 4.0.11
[ 2049.261896] si2157 0-0060: found a 'Silicon Labs Si2157-A30'
[ 2049.294328] si2157 0-0060: firmware version: 3.0.5

I'll send the patches on to linux-media so Steven can evaluate the
impact on other boards, if any.

Cheers,
-olli


On 5 June 2015 at 16:18, Antti Palosaari <crope@iki.fi> wrote:
> On 06/05/2015 04:40 PM, Olli Salonen wrote:
>>
>> Hi Steven,
>>
>> It seems to me that that part of the code is identical to your driver, no?
>>
>> The media_tree driver:
>>
>> retval = saa7164_api_i2c_read(bus,
>>                       msgs[i].addr,
>>                       0 /* reglen */,
>>                       NULL /* reg */, msgs[i].len, msgs[i].buf);
>>
>> It's exactly the same with a little bit different formatting.
>
>
> And that looks correct.
>
> But the patch which does not look correct, or is at least unclear, is that
> [media] saa7164: Improvements for I2C handling
> http://permalink.gmane.org/gmane.comp.video.linuxtv.scm/22211
>
> First change does not have any effect as len should be zero in any case and
> memcpy() should do nothing.
>
> Second change looks something that is likely wrong. There is some hack which
> increases data len. All that register len stuff is logically wrong - I2C
> adapter handles just bytes and should not know nothing about client register
> layout. OK, there is some exceptions (like af9035) where I2C firmware
> actually knows register layout for some strange reason.
>
> So could you remove that patch and test?
>
> Antti
>
> --
> http://palosaari.fi/

Re: [PATCH][media] SI2168: Resolve unknown chip version errors with different HVR22x5 models

[Steven Toth]

On Fri, Jun 5, 2015 at 9:40 AM, Olli Salonen <olli.salonen@iki.fi> wrote:
> Hi Steven,
>
> It seems to me that that part of the code is identical to your driver, no?

Sorry for the noise. I'm been swamped with a couple of things and just
getting back to the ML now.

You are correct.

-- 
Steven Toth - Kernel Labs
http://www.kernellabs.com