Re: [PATCH] si2157: Add support for Logilink VG0022A.
Em Fri, 4 Oct 2019 15:50:18 +0200
JP escreveu:
> On 10/4/19 2:08 PM, Mauro Carvalho Chehab wrote:
> > Em Fri, 4 Oct 2019 13:50:43 +0200
> > JP escreveu:
> >
> >> On 10/3/19 10:03 PM, Mauro Carvalho Chehab wrote:
> >>> Em Thu, 3 Oct 2019 21:51:35 +0200
> >>> Gonsolo escreveu:
> >>>
> > 1) The firmware file is likely at the Windows driver for this device
> > (probably using a different format). It should be possible to get
> > it from there.
> If you tell me how I'm willing to do this. :)
> >>> I don't know. I was not the one that extracted the firmware. I guess
> >>> Antti did it.
> >>>
> >>> I suspect that there are some comments about that in the past at the
> >>> ML. seek at lore.kernel.org.
> >>>
> > 2) Another possibility would be to add a way to tell the si2168 driver
> > to not try to load a firmware, using the original one. That would
> > require adding a field at si2168_config to allow signalizing to it
> > that it should not try to load a firmware file, and add a quirk at
> > the af9035 that would set such flag for Logilink VG0022A.
> I don't get this. Which firmware, si2168 or si2157?
> >>> The one that it is causing the problem. If I understood well, the
> >>> culprit was the si2168 firmware.
> >>>
> I'm still for option 3: If there is a bogus chip revision number it's
> likely the VG0022A and we can safely set fw to NULL, in which case
> everything works.
> All already working devices will continue to work as before.
> With a low probability there are other devices that will return 0x
> but a) they didn't work until now and b) they receive a clear message
> that they return bogus numbers and this works just for the VG0022A, in
> which case this hardware can be tested.
> At last, *my* VG0022A will work without a custom kernel which I'm a
> big fan of. :))
>
> Are there any counterarguments except that it is not the cleanest
> solution in the universe? ;)
> >>> That's a really bad solution. Returning 0xff is what happens when
> >>> things go wrong during I2C transfers. Several problems can cause it,
> >>> including device misfunction. Every time someone comes with a patch
> >>> trying to ignore it, things go sideways for other devices (existing
> >>> or future ones).
> >>>
> >>> Ignoring errors is always a bad idea.
> >> add module param say 'gonso_hack_vg0022a'
> >> if true, act on error by setting a flag
> >> if this flag is set don't load firmware
> > Adding a module param should be the last resort, only when there's
> > no way for the driver to autodetect.
> Remember the guy reported the hw fix? Could be that
> only some receiver units are affected. Therefore the
> module param.
>
> The hw fix was original 4k7 and 10k added. That looks
> like 3k3 total and all 3 chips on the bus work. 10k per
> chip. Now Gon reported that said bus works with 2 chips
> active on a faulty device with 4k7 resistor, which is 2
> times 10k. It looks same hw error to me.
I'm not so sure. From the reports from Gonsolo, in the case of
this specific issue with VG0022A, the device is not unstable. It is
just that it works fine with the vendor-provided firmware, while it
breaks with the new one.
We don't know that the same thing would happen with the original
reported bug. The only way to be sure would be to obtain the same
hardware from the original post and test it to check if the device
has issues without replacing the resistor.
Even the original reporter can't help, as his device was modified,
and the issue won't be there anymore.
Btw, if we end by noticing this bug happening on other it931x
device models, we could simply disable firmware load for all of them,
but we need more tests and reports before changing the behavior for
other models, as older firmwares may have other problems.
> > Making af9035 to detect vg0022a is quite simple.
> >
> > Considering this device's entry:
> >
> > { DVB_USB_DEVICE(USB_VID_DEXATEK, 0x0100,
> > &it930x_props, "Logilink VG0022A", NULL) },
> >
> > the check, at af9035 would be:
> >
> > if (le16_to_cpu(d->udev->descriptor.idVendor) == USB_VID_DEXATEK &&
> > le16_to_cpu(d->udev->descriptor.idProduct) == 0x0100)
> > /* do something to disable firmware load */
> >
> > So, no need to add any load time parameter.
> >
> > It should be noticed that a change just at af9035 won't work, as the
> > firmware is updated by si2168 driver. So, the caller code needs to
> > pass a config parameter to si2168 driver.
> If it is a failing pull-up resistor on only some individual receiver
> units, this seems overkill to me. In my proposal I did not realized
> this change in the demod driver was needed.
Agreed, but we have no means to know that until someone buys other
units of the VG0022A and do tests with and without the patch.
Thanks,
Mauro
Re: [PATCH] si2157: Add support for Logilink VG0022A.
On 10/4/19 2:08 PM, Mauro Carvalho Chehab wrote:
Em Fri, 4 Oct 2019 13:50:43 +0200
JP escreveu:
On 10/3/19 10:03 PM, Mauro Carvalho Chehab wrote:
Em Thu, 3 Oct 2019 21:51:35 +0200
Gonsolo escreveu:
1) The firmware file is likely at the Windows driver for this device
(probably using a different format). It should be possible to get
it from there.
If you tell me how I'm willing to do this. :)
I don't know. I was not the one that extracted the firmware. I guess
Antti did it.
I suspect that there are some comments about that in the past at the
ML. seek at lore.kernel.org.
2) Another possibility would be to add a way to tell the si2168 driver
to not try to load a firmware, using the original one. That would
require adding a field at si2168_config to allow signalizing to it
that it should not try to load a firmware file, and add a quirk at
the af9035 that would set such flag for Logilink VG0022A.
I don't get this. Which firmware, si2168 or si2157?
The one that it is causing the problem. If I understood well, the
culprit was the si2168 firmware.
I'm still for option 3: If there is a bogus chip revision number it's
likely the VG0022A and we can safely set fw to NULL, in which case
everything works.
All already working devices will continue to work as before.
With a low probability there are other devices that will return 0x
but a) they didn't work until now and b) they receive a clear message
that they return bogus numbers and this works just for the VG0022A, in
which case this hardware can be tested.
At last, *my* VG0022A will work without a custom kernel which I'm a
big fan of. :))
Are there any counterarguments except that it is not the cleanest
solution in the universe? ;)
That's a really bad solution. Returning 0xff is what happens when
things go wrong during I2C transfers. Several problems can cause it,
including device misfunction. Every time someone comes with a patch
trying to ignore it, things go sideways for other devices (existing
or future ones).
Ignoring errors is always a bad idea.
add module param say 'gonso_hack_vg0022a'
if true, act on error by setting a flag
if this flag is set don't load firmware
Adding a module param should be the last resort, only when there's
no way for the driver to autodetect.
Remember the guy reported the hw fix? Could be that
only some receiver units are affected. Therefore the
module param.
The hw fix was original 4k7 and 10k added. That looks
like 3k3 total and all 3 chips on the bus work. 10k per
chip. Now Gon reported that said bus works with 2 chips
active on a faulty device with 4k7 resistor, which is 2
times 10k. It looks same hw error to me.
Making af9035 to detect vg0022a is quite simple.
Considering this device's entry:
{ DVB_USB_DEVICE(USB_VID_DEXATEK, 0x0100,
&it930x_props, "Logilink VG0022A", NULL) },
the check, at af9035 would be:
if (le16_to_cpu(d->udev->descriptor.idVendor) == USB_VID_DEXATEK &&
le16_to_cpu(d->udev->descriptor.idProduct) == 0x0100)
/* do something to disable firmware load */
So, no need to add any load time parameter.
It should be noticed that a change just at af9035 won't work, as the
firmware is updated by si2168 driver. So, the caller code needs to
pass a config parameter to si2168 driver.
If it is a failing pull-up resistor on only some individual receiver
units, this seems overkill to me. In my proposal I did not realized
this change in the demod driver was needed.
Thanks,
Mauro
Thank you.
Re: [PATCH] si2157: Add support for Logilink VG0022A.
Em Fri, 4 Oct 2019 13:50:43 +0200
JP escreveu:
> On 10/3/19 10:03 PM, Mauro Carvalho Chehab wrote:
> > Em Thu, 3 Oct 2019 21:51:35 +0200
> > Gonsolo escreveu:
> >
> >>> 1) The firmware file is likely at the Windows driver for this device
> >>> (probably using a different format). It should be possible to get
> >>> it from there.
> >> If you tell me how I'm willing to do this. :)
> > I don't know. I was not the one that extracted the firmware. I guess
> > Antti did it.
> >
> > I suspect that there are some comments about that in the past at the
> > ML. seek at lore.kernel.org.
> >
> >>> 2) Another possibility would be to add a way to tell the si2168 driver
> >>> to not try to load a firmware, using the original one. That would
> >>> require adding a field at si2168_config to allow signalizing to it
> >>> that it should not try to load a firmware file, and add a quirk at
> >>> the af9035 that would set such flag for Logilink VG0022A.
> >> I don't get this. Which firmware, si2168 or si2157?
> > The one that it is causing the problem. If I understood well, the
> > culprit was the si2168 firmware.
> >
> >> I'm still for option 3: If there is a bogus chip revision number it's
> >> likely the VG0022A and we can safely set fw to NULL, in which case
> >> everything works.
> >> All already working devices will continue to work as before.
> >> With a low probability there are other devices that will return 0x
> >> but a) they didn't work until now and b) they receive a clear message
> >> that they return bogus numbers and this works just for the VG0022A, in
> >> which case this hardware can be tested.
> >> At last, *my* VG0022A will work without a custom kernel which I'm a
> >> big fan of. :))
> >>
> >> Are there any counterarguments except that it is not the cleanest
> >> solution in the universe? ;)
> > That's a really bad solution. Returning 0xff is what happens when
> > things go wrong during I2C transfers. Several problems can cause it,
> > including device misfunction. Every time someone comes with a patch
> > trying to ignore it, things go sideways for other devices (existing
> > or future ones).
> >
> > Ignoring errors is always a bad idea.
> add module param say 'gonso_hack_vg0022a'
> if true, act on error by setting a flag
> if this flag is set don't load firmware
Adding a module param should be the last resort, only when there's
no way for the driver to autodetect.
Making af9035 to detect vg0022a is quite simple.
Considering this device's entry:
{ DVB_USB_DEVICE(USB_VID_DEXATEK, 0x0100,
&it930x_props, "Logilink VG0022A", NULL) },
the check, at af9035 would be:
if (le16_to_cpu(d->udev->descriptor.idVendor) == USB_VID_DEXATEK &&
le16_to_cpu(d->udev->descriptor.idProduct) == 0x0100)
/* do something to disable firmware load */
So, no need to add any load time parameter.
It should be noticed that a change just at af9035 won't work, as the
firmware is updated by si2168 driver. So, the caller code needs to
pass a config parameter to si2168 driver.
Thanks,
Mauro
Re: [PATCH] si2157: Add support for Logilink VG0022A.
On 10/3/19 10:03 PM, Mauro Carvalho Chehab wrote: Em Thu, 3 Oct 2019 21:51:35 +0200 Gonsolo escreveu: 1) The firmware file is likely at the Windows driver for this device (probably using a different format). It should be possible to get it from there. If you tell me how I'm willing to do this. :) I don't know. I was not the one that extracted the firmware. I guess Antti did it. I suspect that there are some comments about that in the past at the ML. seek at lore.kernel.org. 2) Another possibility would be to add a way to tell the si2168 driver to not try to load a firmware, using the original one. That would require adding a field at si2168_config to allow signalizing to it that it should not try to load a firmware file, and add a quirk at the af9035 that would set such flag for Logilink VG0022A. I don't get this. Which firmware, si2168 or si2157? The one that it is causing the problem. If I understood well, the culprit was the si2168 firmware. I'm still for option 3: If there is a bogus chip revision number it's likely the VG0022A and we can safely set fw to NULL, in which case everything works. All already working devices will continue to work as before. With a low probability there are other devices that will return 0x but a) they didn't work until now and b) they receive a clear message that they return bogus numbers and this works just for the VG0022A, in which case this hardware can be tested. At last, *my* VG0022A will work without a custom kernel which I'm a big fan of. :)) Are there any counterarguments except that it is not the cleanest solution in the universe? ;) That's a really bad solution. Returning 0xff is what happens when things go wrong during I2C transfers. Several problems can cause it, including device misfunction. Every time someone comes with a patch trying to ignore it, things go sideways for other devices (existing or future ones). Ignoring errors is always a bad idea. add module param say 'gonso_hack_vg0022a' if true, act on error by setting a flag if this flag is set don't load firmware Jan Pieter.
Re: [PATCH] si2157: Add support for Logilink VG0022A.
> I don't know. I was not the one that extracted the firmware. I guess > Antti did it. Ok. > That's a really bad solution. Returning 0xff is what happens when > things go wrong during I2C transfers. Several problems can cause it, > including device misfunction. Every time someone comes with a patch > trying to ignore it, things go sideways for other devices (existing > or future ones). > > Ignoring errors is always a bad idea. I understand. Anyway, I have to give up for now. Maybe I will have some time in the future to come back to this or somebody else can use the information in this thread. :( Thanks for your time, I appreciate that very much. It was worth a try. :) -- g
Re: [PATCH] si2157: Add support for Logilink VG0022A.
Em Thu, 3 Oct 2019 21:51:35 +0200 Gonsolo escreveu: > > 1) The firmware file is likely at the Windows driver for this device > > (probably using a different format). It should be possible to get > > it from there. > > If you tell me how I'm willing to do this. :) I don't know. I was not the one that extracted the firmware. I guess Antti did it. I suspect that there are some comments about that in the past at the ML. seek at lore.kernel.org. > > > 2) Another possibility would be to add a way to tell the si2168 driver > > to not try to load a firmware, using the original one. That would > > require adding a field at si2168_config to allow signalizing to it > > that it should not try to load a firmware file, and add a quirk at > > the af9035 that would set such flag for Logilink VG0022A. > > I don't get this. Which firmware, si2168 or si2157? The one that it is causing the problem. If I understood well, the culprit was the si2168 firmware. > I'm still for option 3: If there is a bogus chip revision number it's > likely the VG0022A and we can safely set fw to NULL, in which case > everything works. > All already working devices will continue to work as before. > With a low probability there are other devices that will return 0x > but a) they didn't work until now and b) they receive a clear message > that they return bogus numbers and this works just for the VG0022A, in > which case this hardware can be tested. > At last, *my* VG0022A will work without a custom kernel which I'm a > big fan of. :)) > > Are there any counterarguments except that it is not the cleanest > solution in the universe? ;) That's a really bad solution. Returning 0xff is what happens when things go wrong during I2C transfers. Several problems can cause it, including device misfunction. Every time someone comes with a patch trying to ignore it, things go sideways for other devices (existing or future ones). Ignoring errors is always a bad idea. Also, it is a very bad idea to load a firmware that it is not compatible with a device. There are even cases of devices that were burned due to that[1]. [1] XCeive has two versions of 3028/2028 chipsets. One with a "normal power" and a "low power" version. If the firmware for the "normal power" (version 2.7) is used at the "low power" chip (with requires version 3.6), it makes the chipset hotter and reduces a lot the lifetime of the tuner. Thanks, Mauro
Re: [PATCH] si2157: Add support for Logilink VG0022A.
> Huh? are you using the upstream Kernel? the above code is at line 215! > Please always use the upstream code when sending patches. Sorry, I was confused by my vi line: "drivers/media/dvb-frontends/si2168.c" 808 lines --26%-- 212,1-8 25%" Twelve hours behind this screen. I think I have to have a walk in the forest right now. :) -- g
Re: [PATCH] si2157: Add support for Logilink VG0022A.
Em Thu, 3 Oct 2019 21:40:28 +0200 Gonsolo escreveu: > From si2168.c:808: >/* Sometimes firmware returns bogus value */ > if (utmp1 == 0x) > utmp1 = 0; Huh? are you using the upstream Kernel? the above code is at line 215! Please always use the upstream code when sending patches. > > Maybe we can include my "bogus" hack to get at least Logilink running. > Maybe with an info message to tell users what is going on. > > g Thanks, Mauro
Re: [PATCH] si2157: Add support for Logilink VG0022A.
> 1) The firmware file is likely at the Windows driver for this device > (probably using a different format). It should be possible to get > it from there. If you tell me how I'm willing to do this. :) > 2) Another possibility would be to add a way to tell the si2168 driver > to not try to load a firmware, using the original one. That would > require adding a field at si2168_config to allow signalizing to it > that it should not try to load a firmware file, and add a quirk at > the af9035 that would set such flag for Logilink VG0022A. I don't get this. Which firmware, si2168 or si2157? I'm still for option 3: If there is a bogus chip revision number it's likely the VG0022A and we can safely set fw to NULL, in which case everything works. All already working devices will continue to work as before. With a low probability there are other devices that will return 0x but a) they didn't work until now and b) they receive a clear message that they return bogus numbers and this works just for the VG0022A, in which case this hardware can be tested. At last, *my* VG0022A will work without a custom kernel which I'm a big fan of. :)) Are there any counterarguments except that it is not the cleanest solution in the universe? ;) -- g
Re: [PATCH] si2157: Add support for Logilink VG0022A.
Em Thu, 3 Oct 2019 16:39:14 -0300 Mauro Carvalho Chehab escreveu: > Em Thu, 3 Oct 2019 21:19:16 +0200 > Gonsolo escreveu: > > > > try other firmware? > > > http://palosaari.fi/linux/v4l-dvb/firmware/Si2168/ > > > > I tried all of them. No difference. > > Maybe the vendor of this device wrote a different firmware. That happens. Two additional comments: 1) The firmware file is likely at the Windows driver for this device (probably using a different format). It should be possible to get it from there. 2) Another possibility would be to add a way to tell the si2168 driver to not try to load a firmware, using the original one. That would require adding a field at si2168_config to allow signalizing to it that it should not try to load a firmware file, and add a quirk at the af9035 that would set such flag for Logilink VG0022A. Option (1) is the best one. Thanks, Mauro
Re: [PATCH] si2157: Add support for Logilink VG0022A.
>From si2168.c:808: /* Sometimes firmware returns bogus value */ if (utmp1 == 0x) utmp1 = 0; Maybe we can include my "bogus" hack to get at least Logilink running. Maybe with an info message to tell users what is going on. g
Re: [PATCH] si2157: Add support for Logilink VG0022A.
Em Thu, 3 Oct 2019 21:19:16 +0200 Gonsolo escreveu: > > try other firmware? > > http://palosaari.fi/linux/v4l-dvb/firmware/Si2168/ > > I tried all of them. No difference. Maybe the vendor of this device wrote a different firmware. That happens. Thanks, Mauro
Re: [PATCH] si2157: Add support for Logilink VG0022A.
> try other firmware? > http://palosaari.fi/linux/v4l-dvb/firmware/Si2168/ I tried all of them. No difference. -- g
Re: [PATCH] si2157: Add support for Logilink VG0022A.
I also tried to add a msleep(1000) after the si2168 firmware upload; no difference. g
Re: [PATCH] si2157: Add support for Logilink VG0022A.
> try other firmware? > http://palosaari.fi/linux/v4l-dvb/firmware/Si2168/ I already downloaded version 4.0.25 from there. -rw-r--r-- 1 rootroot6,8K Apr 5 2018 /lib/firmware/dvb-demod-si2168-b40-01.fw.gonzo -rw-rw-r-- 1 gonsolo gonsolo 16K Okt 3 13:08 /lib/firmware/dvb-demod-si2168-b40-01.fw No difference. -- g
Re: [PATCH] si2157: Add support for Logilink VG0022A.
On 10/3/19 8:32 PM, Gon Solo wrote: You could also try to disable the firmware upload at si2168 and see if the si2157 reads will succeed. I disabled the upload and while vlc wasn't working anymore I got the following messages: [ 168.196656] si2157 2-0063: found a 'Silicon Labs Si2147-A30' [ 168.223009] si2157 2-0063: firmware version: 3.0.5 It really seems that the firmware upload is the culprit. try other firmware? http://palosaari.fi/linux/v4l-dvb/firmware/Si2168/ g
Re: [PATCH] si2157: Add support for Logilink VG0022A.
> You could also try to disable the firmware upload at si2168 and see > if the si2157 reads will succeed. I disabled the upload and while vlc wasn't working anymore I got the following messages: [ 168.196656] si2157 2-0063: found a 'Silicon Labs Si2147-A30' [ 168.223009] si2157 2-0063: firmware version: 3.0.5 It really seems that the firmware upload is the culprit. g
Re: [PATCH] si2157: Add support for Logilink VG0022A.
> Weird... it sounds that, after si2168 has its firmware updated, it > starts interfering at si2157. Perhaps there's a bug at si2168 I2C > gate mux logic. Are you using a new Kernel like 5.2? Everything is based on git://linuxtv.org/media_tree.git which is at 5.4-rc1 right now. > I guess the best is to enable the debug logs in order to see what's > happening on both cases. > > You can enable all debug messages (after loading the modules) with: > > # for i in $(cat /sys/kernel/debug/dynamic_debug/control |grep -E > '(si21|af9035)' |cut -d' ' -f 1); do echo "file $i +p" > >>/sys/kernel/debug/dynamic_debug/control; done I'll give that a try. > You could also try to disable the firmware upload at si2168 and see > if the si2157 reads will succeed. That too. Thanks for the advice. -- g
Re: [PATCH] si2157: Add support for Logilink VG0022A.
Em Thu, 3 Oct 2019 18:23:26 +0200 Gon Solo escreveu: > > No, I mean with the first patch you sent to the ML, with the powerup > > hack. > > Boot time: > > [4.653257] si2168 1-0067: Silicon Labs Si2168-B40 successfully identified > [4.653259] si2168 1-0067: firmware version: B 4.0.2 > [4.653279] usb 2-1: DVB: registering adapter 0 frontend 0 (Silicon Labs > Si2168)... > [4.653284] dvbdev: dvb_create_media_entity: media entity 'Silicon Labs > Si2168' registered. > ... > [4.694785] si2157 2-0063: found a 'Silicon Labs Si2147-A30' > [4.694787] si2157 2-0063: Silicon Labs Si2147/2148/2157/2158 successfully > attached > [4.717814] usb 2-1: dvb_usb_v2: 'Logilink VG0022A' successfully > initialized and connected > [4.717880] usbcore: registered new interface driver dvb_usb_af9035 > > VLC time: > > [ 175.490609] si2168 1-0067: downloading firmware from file > 'dvb-demod-si2168-b40-01.fw' > [ 176.746585] si2168 1-0067: firmware version: B 4.0.25 > [ 176.781570] si2157 2-0063: firmware version: \xff.\xff.255 Weird... it sounds that, after si2168 has its firmware updated, it starts interfering at si2157. Perhaps there's a bug at si2168 I2C gate mux logic. Are you using a new Kernel like 5.2? I guess the best is to enable the debug logs in order to see what's happening on both cases. You can enable all debug messages (after loading the modules) with: # for i in $(cat /sys/kernel/debug/dynamic_debug/control |grep -E '(si21|af9035)' |cut -d' ' -f 1); do echo "file $i +p" >>/sys/kernel/debug/dynamic_debug/control; done You could also try to disable the firmware upload at si2168 and see if the si2157 reads will succeed. Thanks, Mauro
Re: [PATCH] si2157: Add support for Logilink VG0022A.
> No, I mean with the first patch you sent to the ML, with the powerup > hack. Boot time: [4.653257] si2168 1-0067: Silicon Labs Si2168-B40 successfully identified [4.653259] si2168 1-0067: firmware version: B 4.0.2 [4.653279] usb 2-1: DVB: registering adapter 0 frontend 0 (Silicon Labs Si2168)... [4.653284] dvbdev: dvb_create_media_entity: media entity 'Silicon Labs Si2168' registered. ... [4.694785] si2157 2-0063: found a 'Silicon Labs Si2147-A30' [4.694787] si2157 2-0063: Silicon Labs Si2147/2148/2157/2158 successfully attached [4.717814] usb 2-1: dvb_usb_v2: 'Logilink VG0022A' successfully initialized and connected [4.717880] usbcore: registered new interface driver dvb_usb_af9035 VLC time: [ 175.490609] si2168 1-0067: downloading firmware from file 'dvb-demod-si2168-b40-01.fw' [ 176.746585] si2168 1-0067: firmware version: B 4.0.25 [ 176.781570] si2157 2-0063: firmware version: \xff.\xff.255 g
Re: [PATCH] si2157: Add support for Logilink VG0022A.
Em Thu, 3 Oct 2019 18:03:36 +0200
Gon Solo escreveu:
> > With the original patch you proposed, what are the logs?
>
> With the following patch applied to media_tree master:
>
> diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
> index e87040d6eca7..4c1ab0b6876a 100644
> --- a/drivers/media/tuners/si2157.c
> +++ b/drivers/media/tuners/si2157.c
> @@ -129,13 +129,14 @@ static int si2157_init(struct dvb_frontend *fe)
> chip_id = cmd.args[1] << 24 | cmd.args[2] << 16 | cmd.args[3] << 8 |
> cmd.args[4] << 0;
>
> - #define SI2177_A30 ('A' << 24 | 77 << 16 | '3' << 8 | '0' << 0)
> - #define SI2158_A20 ('A' << 24 | 58 << 16 | '2' << 8 | '0' << 0)
> - #define SI2148_A20 ('A' << 24 | 48 << 16 | '2' << 8 | '0' << 0)
> - #define SI2157_A30 ('A' << 24 | 57 << 16 | '3' << 8 | '0' << 0)
> - #define SI2147_A30 ('A' << 24 | 47 << 16 | '3' << 8 | '0' << 0)
> - #define SI2146_A10 ('A' << 24 | 46 << 16 | '1' << 8 | '0' << 0)
> - #define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)
> + #define SI2177_A30 ('A' << 24 | 77 << 16 | '3' << 8 | '0' << 0)
> + #define SI2158_A20 ('A' << 24 | 58 << 16 | '2' << 8 | '0' << 0)
> + #define SI2148_A20 ('A' << 24 | 48 << 16 | '2' << 8 | '0' << 0)
> + #define SI2157_A30 ('A' << 24 | 57 << 16 | '3' << 8 | '0' << 0)
> + #define SI2147_A30 ('A' << 24 | 47 << 16 | '3' << 8 | '0' << 0)
> + #define SI2146_A10 ('A' << 24 | 46 << 16 | '1' << 8 | '0' << 0)
> + #define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)
> + #define SI_BOGUS (255 << 24 | 255 << 16 | 255 << 8 | 255 << 0)
>
> switch (chip_id) {
> case SI2158_A20:
> @@ -148,6 +149,10 @@ static int si2157_init(struct dvb_frontend *fe)
> case SI2177_A30:
> fw_name = SI2157_A30_FIRMWARE;
> break;
> + case SI_BOGUS:
> + dev_info(&client->dev, "Bogus chip version, trying with no
> firmware\n");
> + fw_name = NULL;
> + break;
> case SI2157_A30:
> case SI2147_A30:
> case SI2146_A10:
> @@ -225,6 +230,7 @@ static int si2157_init(struct dvb_frontend *fe)
>
> dev_info(&client->dev, "firmware version: %c.%c.%d\n",
> cmd.args[6], cmd.args[7], cmd.args[8]);
> warm:
> /* init statistics in order signal app which are supported */
> c->strength.len = 1;
> diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c
> b/drivers/media/usb/dvb-usb-v2/af9035.c
> index 3afd18733614..a8d59cf06b1e 100644
> --- a/drivers/media/usb/dvb-usb-v2/af9035.c
> +++ b/drivers/media/usb/dvb-usb-v2/af9035.c
> @@ -1197,6 +1197,11 @@ static int af9035_frontend_attach(struct
> dvb_usb_adapter *adap)
> return ret;
> }
>
> +/* I2C speed register = (10 / (24.4 * 16 * I2C_speed))
> + * 7 equals ~400k, 26 ~100k and 260 ~10k.
> + * */
> +#define I2C_SPEED_REGISTER 7
> +
> static int it930x_frontend_attach(struct dvb_usb_adapter *adap)
> {
> struct state *state = adap_to_priv(adap);
> @@ -1208,13 +1213,13 @@ static int it930x_frontend_attach(struct
> dvb_usb_adapter *adap)
>
> dev_dbg(&intf->dev, "adap->id=%d\n", adap->id);
>
> - /* I2C master bus 2 clock speed 300k */
> - ret = af9035_wr_reg(d, 0x00f6a7, 0x07);
> + /* I2C master bus 2 clock speed */
> + ret = af9035_wr_reg(d, 0x00f6a7, I2C_SPEED_REGISTER);
> if (ret < 0)
> goto err;
>
> - /* I2C master bus 1,3 clock speed 300k */
> - ret = af9035_wr_reg(d, 0x00f103, 0x07);
> + /* I2C master bus 1,3 clock speed */
> + ret = af9035_wr_reg(d, 0x00f103, I2C_SPEED_REGISTER);
> if (ret < 0)
> goto err;
>
> @@ -2119,6 +2124,8 @@ static const struct usb_device_id af9035_id_table[] = {
> /* IT930x devices */
> { DVB_USB_DEVICE(USB_VID_ITETECH, USB_PID_ITETECH_IT9303,
> &it930x_props, "ITE 9303 Generic", NULL) },
> + { DVB_USB_DEVICE(USB_VID_DEXATEK, 0x0100,
> + &it930x_props, "Logilink VG0022A", NULL) },
> { DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_TD310,
> &it930x_props, "AVerMedia TD310 DVB-T2", NULL) },
> { }
>
> the Messages at boot time are
>
> [4.262882] si2168 1-0067: Silicon Labs Si2168-B40 successfully identified
> [4.262884] si2168 1-0067: firmware version: B 4.0.2
> [4.262902] usb 2-1: DVB: registering adapter 0 frontend 0 (Silicon Labs
> Si2168)...
> [4.262908] dvbdev: dvb_create_media_entity: media entity 'Silicon Labs
> Si2168' registered.
> [4.289776] si2157 2-0063: Silicon Labs Si2147/2148/2157/2158 successfully
> attached
>
> and the messages when running vlc (successfully) are
>
> [ 486.537128] si2168 1-0067: downloading firmware from file
> 'dvb-demod-si2168-b40-01.fw'
> [ 487.795436] si2168 1-0067: firmware version: B 4.0.25
> [ 487.807614] si2157 2-0063: Bogus chip version, trying with no firmware
> [ 487.807618] si2157 2-0
Re: [PATCH] si2157: Add support for Logilink VG0022A.
> With the original patch you proposed, what are the logs?
With the following patch applied to media_tree master:
diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index e87040d6eca7..4c1ab0b6876a 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -129,13 +129,14 @@ static int si2157_init(struct dvb_frontend *fe)
chip_id = cmd.args[1] << 24 | cmd.args[2] << 16 | cmd.args[3] << 8 |
cmd.args[4] << 0;
- #define SI2177_A30 ('A' << 24 | 77 << 16 | '3' << 8 | '0' << 0)
- #define SI2158_A20 ('A' << 24 | 58 << 16 | '2' << 8 | '0' << 0)
- #define SI2148_A20 ('A' << 24 | 48 << 16 | '2' << 8 | '0' << 0)
- #define SI2157_A30 ('A' << 24 | 57 << 16 | '3' << 8 | '0' << 0)
- #define SI2147_A30 ('A' << 24 | 47 << 16 | '3' << 8 | '0' << 0)
- #define SI2146_A10 ('A' << 24 | 46 << 16 | '1' << 8 | '0' << 0)
- #define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)
+ #define SI2177_A30 ('A' << 24 | 77 << 16 | '3' << 8 | '0' << 0)
+ #define SI2158_A20 ('A' << 24 | 58 << 16 | '2' << 8 | '0' << 0)
+ #define SI2148_A20 ('A' << 24 | 48 << 16 | '2' << 8 | '0' << 0)
+ #define SI2157_A30 ('A' << 24 | 57 << 16 | '3' << 8 | '0' << 0)
+ #define SI2147_A30 ('A' << 24 | 47 << 16 | '3' << 8 | '0' << 0)
+ #define SI2146_A10 ('A' << 24 | 46 << 16 | '1' << 8 | '0' << 0)
+ #define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)
+ #define SI_BOGUS (255 << 24 | 255 << 16 | 255 << 8 | 255 << 0)
switch (chip_id) {
case SI2158_A20:
@@ -148,6 +149,10 @@ static int si2157_init(struct dvb_frontend *fe)
case SI2177_A30:
fw_name = SI2157_A30_FIRMWARE;
break;
+ case SI_BOGUS:
+ dev_info(&client->dev, "Bogus chip version, trying with no
firmware\n");
+ fw_name = NULL;
+ break;
case SI2157_A30:
case SI2147_A30:
case SI2146_A10:
@@ -225,6 +230,7 @@ static int si2157_init(struct dvb_frontend *fe)
dev_info(&client->dev, "firmware version: %c.%c.%d\n",
cmd.args[6], cmd.args[7], cmd.args[8]);
warm:
/* init statistics in order signal app which are supported */
c->strength.len = 1;
diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c
b/drivers/media/usb/dvb-usb-v2/af9035.c
index 3afd18733614..a8d59cf06b1e 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.c
+++ b/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -1197,6 +1197,11 @@ static int af9035_frontend_attach(struct dvb_usb_adapter
*adap)
return ret;
}
+/* I2C speed register = (10 / (24.4 * 16 * I2C_speed))
+ * 7 equals ~400k, 26 ~100k and 260 ~10k.
+ * */
+#define I2C_SPEED_REGISTER 7
+
static int it930x_frontend_attach(struct dvb_usb_adapter *adap)
{
struct state *state = adap_to_priv(adap);
@@ -1208,13 +1213,13 @@ static int it930x_frontend_attach(struct
dvb_usb_adapter *adap)
dev_dbg(&intf->dev, "adap->id=%d\n", adap->id);
- /* I2C master bus 2 clock speed 300k */
- ret = af9035_wr_reg(d, 0x00f6a7, 0x07);
+ /* I2C master bus 2 clock speed */
+ ret = af9035_wr_reg(d, 0x00f6a7, I2C_SPEED_REGISTER);
if (ret < 0)
goto err;
- /* I2C master bus 1,3 clock speed 300k */
- ret = af9035_wr_reg(d, 0x00f103, 0x07);
+ /* I2C master bus 1,3 clock speed */
+ ret = af9035_wr_reg(d, 0x00f103, I2C_SPEED_REGISTER);
if (ret < 0)
goto err;
@@ -2119,6 +2124,8 @@ static const struct usb_device_id af9035_id_table[] = {
/* IT930x devices */
{ DVB_USB_DEVICE(USB_VID_ITETECH, USB_PID_ITETECH_IT9303,
&it930x_props, "ITE 9303 Generic", NULL) },
+ { DVB_USB_DEVICE(USB_VID_DEXATEK, 0x0100,
+ &it930x_props, "Logilink VG0022A", NULL) },
{ DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_TD310,
&it930x_props, "AVerMedia TD310 DVB-T2", NULL) },
{ }
the Messages at boot time are
[4.262882] si2168 1-0067: Silicon Labs Si2168-B40 successfully identified
[4.262884] si2168 1-0067: firmware version: B 4.0.2
[4.262902] usb 2-1: DVB: registering adapter 0 frontend 0 (Silicon Labs
Si2168)...
[4.262908] dvbdev: dvb_create_media_entity: media entity 'Silicon Labs
Si2168' registered.
[4.289776] si2157 2-0063: Silicon Labs Si2147/2148/2157/2158 successfully
attached
and the messages when running vlc (successfully) are
[ 486.537128] si2168 1-0067: downloading firmware from file
'dvb-demod-si2168-b40-01.fw'
[ 487.795436] si2168 1-0067: firmware version: B 4.0.25
[ 487.807614] si2157 2-0063: Bogus chip version, trying with no firmware
[ 487.807618] si2157 2-0063: found a 'Silicon Labs Si21255-\xff\xff\xff'
[ 487.833876] si2157 2-0063: firmware version: \xff.\xff.255
g
Re: [PATCH] si2157: Add support for Logilink VG0022A.
> With the original patch you proposed, what are the logs? Which one do you mean? That one: + case SI_BOGUS: + dev_info(&client->dev, "Bogus chip version, trying with no firmware\n"); + fw_name = NULL; + break; With this patch VLC is running but the chip revision number and firmware version are still bogus. Which means if you receive 0x you can force no firmware and it runs. -- g
Re: [PATCH] si2157: Add support for Logilink VG0022A.
Em Thu, 3 Oct 2019 17:00:08 +0200 Gonsolo escreveu: > > So, I would add a msleep() somewhere after the firmware update. > > I tried that to no avail: > > release_firmware(fw); > + msleep(1000); > > [ 107.903918] si2157 2-0063: firmware version: \xff.\xff.255 > [ 107.903920] si2157 2-0063: querying chip revision... > [ 107.906970] si2157 2-0063: chip revision: 255.255.255.255 > With the original patch you proposed, what are the logs? Thanks, Mauro
Re: [PATCH] si2157: Add support for Logilink VG0022A.
> So, I would add a msleep() somewhere after the firmware update. I tried that to no avail: release_firmware(fw); + msleep(1000); [ 107.903918] si2157 2-0063: firmware version: \xff.\xff.255 [ 107.903920] si2157 2-0063: querying chip revision... [ 107.906970] si2157 2-0063: chip revision: 255.255.255.255 -- g
Re: [PATCH] si2157: Add support for Logilink VG0022A.
> Try to reduce the bus speed to 10kbps Nope: #define I2C_SPEED_REGISTER 260 // ~10k [ 117.860961] si2168 1-0067: downloading firmware from file 'dvb-demod-si2168-b40-01.fw' [ 118.958355] si2168 1-0067: firmware version: B 4.0.25 [ 118.968882] si2157 2-0063: Bogus chip version, trying with new firmware [ 118.96] si2157 2-0063: found a 'Silicon Labs Si21255-\xff\xff\xff' [ 118.972005] si2157 2-0063: downloading firmware from file 'dvb-tuner-si2157-a30-01.fw' [ 121.154130] si2157 2-0063: rebooting tuner... [ 121.169626] si2157 2-0063: querying firmware version... [ 121.172799] si2157 2-0063: firmware version: \xff.\xff.255 [ 121.172803] si2157 2-0063: querying chip revision... [ 121.175911] si2157 2-0063: chip revision: 255.255.255.255 g -- g
Re: [PATCH] si2157: Add support for Logilink VG0022A.
Em Thu, 3 Oct 2019 15:53:30 +0200 Gonsolo escreveu: > Hi! > > I tried downloading a new firmware via > >case SI_BOGUS: > - dev_info(&client->dev, "Bogus chip version, trying > with no firmware\n"); > - fw_name = NULL; > + dev_info(&client->dev, "Bogus chip version, trying > with new firmware\n"); > + fw_name = SI2157_A30_FIRMWARE; > break; > > which I downloaded from > > + // > https://github.com/CoreELEC/dvb-firmware/blob/master/firmware/dvb-tuner-si2157-a30-01.fw > > resulting in > > [ 209.312086] si2168 1-0067: downloading firmware from file > 'dvb-demod-si2168-b40-01.fw' > [ 211.535097] si2168 1-0067: firmware version: B 4.0.25 > [ 211.554938] si2157 2-0063: Bogus chip version, trying with new firmware > [ 211.554944] si2157 2-0063: found a 'Silicon Labs Si21255-\xff\xff\xff' > [ 211.557978] si2157 2-0063: downloading firmware from file > 'dvb-tuner-si2157-a30-01.fw' > [ 215.739092] si2157 2-0063: rebooting tuner... > [ 215.755271] si2157 2-0063: querying firmware version... > [ 215.760756] si2157 2-0063: firmware version: \xff.\xff.255 > > . So even with a new firmware the queried numbers are bogus. Try to reduce the bus speed to 10kbps > > g Thanks, Mauro
Re: [PATCH] si2157: Add support for Logilink VG0022A.
Hi! I tried downloading a new firmware via case SI_BOGUS: - dev_info(&client->dev, "Bogus chip version, trying with no firmware\n"); - fw_name = NULL; + dev_info(&client->dev, "Bogus chip version, trying with new firmware\n"); + fw_name = SI2157_A30_FIRMWARE; break; which I downloaded from + // https://github.com/CoreELEC/dvb-firmware/blob/master/firmware/dvb-tuner-si2157-a30-01.fw resulting in [ 209.312086] si2168 1-0067: downloading firmware from file 'dvb-demod-si2168-b40-01.fw' [ 211.535097] si2168 1-0067: firmware version: B 4.0.25 [ 211.554938] si2157 2-0063: Bogus chip version, trying with new firmware [ 211.554944] si2157 2-0063: found a 'Silicon Labs Si21255-\xff\xff\xff' [ 211.557978] si2157 2-0063: downloading firmware from file 'dvb-tuner-si2157-a30-01.fw' [ 215.739092] si2157 2-0063: rebooting tuner... [ 215.755271] si2157 2-0063: querying firmware version... [ 215.760756] si2157 2-0063: firmware version: \xff.\xff.255 . So even with a new firmware the queried numbers are bogus. g
Re: [PATCH] si2157: Add support for Logilink VG0022A.
> Maybe something like this would make it work? Nope. :( [ 47.371022] si2168 1-0067: downloading firmware from file 'dvb-demod-si2168-b40-01.fw' [ 48.632824] si2168 1-0067: firmware version: B 4.0.25 [ 48.671268] si2157 2-0063: unknown chip version Si21255-\xff\xff\xff g
Re: [PATCH] si2157: Add support for Logilink VG0022A.
Em Thu, 3 Oct 2019 09:49:04 -0300
Mauro Carvalho Chehab escreveu:
> Em Thu, 3 Oct 2019 13:41:23 +0200
> Gonsolo escreveu:
>
> > Hi!
> >
> > > It means that there's a firmware stored at the device's eeprom
> > > (version 4.0.2). When the driver starts, it downloads a newer firmware
> > > from the file dvb-demod-si2168-b40-01.fw.
> >
> > Thanks for the explanation.
> >
> > > Btw, could you please try the enclosed hack and post the results?
> >
> > Will do in a second.
> >
> > FWIW, this hack worked:
> >
> > diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
> > index e87040d6eca7..28a3a4f1640e 100644
> > --- a/drivers/media/tuners/si2157.c
> > +++ b/drivers/media/tuners/si2157.c
> > @@ -136,6 +136,7 @@ static int si2157_init(struct dvb_frontend *fe)
> > #define SI2147_A30 ('A' << 24 | 47 << 16 | '3' << 8 | '0' << 0)
> > #define SI2146_A10 ('A' << 24 | 46 << 16 | '1' << 8 | '0' << 0)
> > #define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)
> > + #define GONZO (255 << 24 | 255 << 16 | 255 << 8 | 255 << 0)
> >
> > switch (chip_id) {
> > case SI2158_A20:
> > @@ -148,6 +149,10 @@ static int si2157_init(struct dvb_frontend *fe)
> > case SI2177_A30:
> > fw_name = SI2157_A30_FIRMWARE;
> > break;
> > + case GONZO:
> > + dev_info(&client->dev, "trying null\n");
> > + fw_name = NULL;
> > + break;
> > case SI2157_A30:
> > case SI2147_A30:
> > case SI2146_A10:
>
> What does it print with this hack?
>
> Also, could you get the SI version after the reset code at
> skip_fw_download, just after retrieving si2157 firmware version?
Maybe something like this would make it work?
diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index e87040d6eca7..86d945fd50b9 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -129,6 +129,28 @@ static int si2157_init(struct dvb_frontend *fe)
chip_id = cmd.args[1] << 24 | cmd.args[2] << 16 | cmd.args[3] << 8 |
cmd.args[4] << 0;
+ if (chip_id == 0x) {
+ /* reboot the tuner */
+ memcpy(cmd.args, "\x01\x01", 2);
+ cmd.wlen = 2;
+ cmd.rlen = 1;
+ ret = si2157_cmd_execute(client, &cmd);
+ if (ret)
+ goto err;
+
+ /* query chip revision */
+ memcpy(cmd.args, "\x02", 1);
+ cmd.wlen = 1;
+ cmd.rlen = 13;
+ ret = si2157_cmd_execute(client, &cmd);
+ if (ret)
+ goto err;
+
+ chip_id = cmd.args[1] << 24 | cmd.args[2] << 16 | cmd.args[3]
<< 8 |
+ cmd.args[4] << 0;
+
+ }
+
#define SI2177_A30 ('A' << 24 | 77 << 16 | '3' << 8 | '0' << 0)
#define SI2158_A20 ('A' << 24 | 58 << 16 | '2' << 8 | '0' << 0)
#define SI2148_A20 ('A' << 24 | 48 << 16 | '2' << 8 | '0' << 0)
Thanks,
Mauro
Re: [PATCH] si2157: Add support for Logilink VG0022A.
Em Thu, 3 Oct 2019 13:41:23 +0200
Gonsolo escreveu:
> Hi!
>
> > It means that there's a firmware stored at the device's eeprom
> > (version 4.0.2). When the driver starts, it downloads a newer firmware
> > from the file dvb-demod-si2168-b40-01.fw.
>
> Thanks for the explanation.
>
> > Btw, could you please try the enclosed hack and post the results?
>
> Will do in a second.
>
> FWIW, this hack worked:
>
> diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
> index e87040d6eca7..28a3a4f1640e 100644
> --- a/drivers/media/tuners/si2157.c
> +++ b/drivers/media/tuners/si2157.c
> @@ -136,6 +136,7 @@ static int si2157_init(struct dvb_frontend *fe)
> #define SI2147_A30 ('A' << 24 | 47 << 16 | '3' << 8 | '0' << 0)
> #define SI2146_A10 ('A' << 24 | 46 << 16 | '1' << 8 | '0' << 0)
> #define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)
> + #define GONZO (255 << 24 | 255 << 16 | 255 << 8 | 255 << 0)
>
> switch (chip_id) {
> case SI2158_A20:
> @@ -148,6 +149,10 @@ static int si2157_init(struct dvb_frontend *fe)
> case SI2177_A30:
> fw_name = SI2157_A30_FIRMWARE;
> break;
> + case GONZO:
> + dev_info(&client->dev, "trying null\n");
> + fw_name = NULL;
> + break;
> case SI2157_A30:
> case SI2147_A30:
> case SI2146_A10:
What does it print with this hack?
Also, could you get the SI version after the reset code at
skip_fw_download, just after retrieving si2157 firmware version?
Thanks,
Mauro
Re: [PATCH] si2157: Add support for Logilink VG0022A.
> well, you could try to increase the timeout - although 100 ms seems a lot > of time to me. I tried 5s, still no change. Would it be possible to include my patch, possibly with a warning like "bogus chip version, trying with no firmware"? g
Re: [PATCH] si2157: Add support for Logilink VG0022A.
Em Thu, 3 Oct 2019 14:01:43 +0200 Gon Solo escreveu: > Hi! > > > > Btw, could you please try the enclosed hack and post the results? > > [ 210.178948] si2168 1-0067: downloading firmware from file > 'dvb-demod-si2168-b40-01.fw' > [ 212.404011] si2168 1-0067: firmware version: B 4.0.25 > [ 212.656467] si2157 2-0063: Needed to wait 100 ms to get chip version > [ 212.656470] si2157 2-0063: Unable to retrieve chip version well, you could try to increase the timeout - although 100 ms seems a lot of time to me. Thanks, Mauro
Re: [PATCH] si2157: Add support for Logilink VG0022A.
Hi! > Btw, could you please try the enclosed hack and post the results? [ 210.178948] si2168 1-0067: downloading firmware from file 'dvb-demod-si2168-b40-01.fw' [ 212.404011] si2168 1-0067: firmware version: B 4.0.25 [ 212.656467] si2157 2-0063: Needed to wait 100 ms to get chip version [ 212.656470] si2157 2-0063: Unable to retrieve chip version :( g
Re: [PATCH] si2157: Add support for Logilink VG0022A.
Hi!
> It means that there's a firmware stored at the device's eeprom
> (version 4.0.2). When the driver starts, it downloads a newer firmware
> from the file dvb-demod-si2168-b40-01.fw.
Thanks for the explanation.
> Btw, could you please try the enclosed hack and post the results?
Will do in a second.
FWIW, this hack worked:
diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index e87040d6eca7..28a3a4f1640e 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -136,6 +136,7 @@ static int si2157_init(struct dvb_frontend *fe)
#define SI2147_A30 ('A' << 24 | 47 << 16 | '3' << 8 | '0' << 0)
#define SI2146_A10 ('A' << 24 | 46 << 16 | '1' << 8 | '0' << 0)
#define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)
+ #define GONZO (255 << 24 | 255 << 16 | 255 << 8 | 255 << 0)
switch (chip_id) {
case SI2158_A20:
@@ -148,6 +149,10 @@ static int si2157_init(struct dvb_frontend *fe)
case SI2177_A30:
fw_name = SI2157_A30_FIRMWARE;
break;
+ case GONZO:
+ dev_info(&client->dev, "trying null\n");
+ fw_name = NULL;
+ break;
case SI2157_A30:
case SI2147_A30:
case SI2146_A10:
Re: [PATCH] si2157: Add support for Logilink VG0022A.
Em Thu, 3 Oct 2019 12:57:50 +0200
Gonsolo escreveu:
> Hi!
>
> Boot time:
>
> > [5.380991] si2168 1-0067: firmware version: B 4.0.2
>
> When starting VLC:
>
> > [ 457.677363] si2168 1-0067: downloading firmware from file
> > 'dvb-demod-si2168-b40-01.fw'
> > [ 458.631034] si2168 1-0067: firmware version: B 4.0.11
> > [ 458.650309] si2157 2-0063: unknown chip version Si21255-\xff\xff\xff
>
> There are two different firmware versions, 4.0.2 and 4.0.11. Is that expected?
It means that there's a firmware stored at the device's eeprom
(version 4.0.2). When the driver starts, it downloads a newer firmware
from the file dvb-demod-si2168-b40-01.fw.
Btw, could you please try the enclosed hack and post the results?
Thanks,
Mauro
diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index e87040d6eca7..3ccfd602934b 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -76,6 +76,7 @@ static int si2157_init(struct dvb_frontend *fe)
const struct firmware *fw;
const char *fw_name;
unsigned int uitmp, chip_id;
+ int i;
dev_dbg(&client->dev, "\n");
@@ -118,16 +119,32 @@ static int si2157_init(struct dvb_frontend *fe)
goto err;
}
- /* query chip revision */
- memcpy(cmd.args, "\x02", 1);
- cmd.wlen = 1;
- cmd.rlen = 13;
- ret = si2157_cmd_execute(client, &cmd);
- if (ret)
- goto err;
+ for (i = 0; i < 10; i++) {
+ /* query chip revision */
+ memcpy(cmd.args, "\x02", 1);
+ cmd.wlen = 1;
+ cmd.rlen = 13;
+ ret = si2157_cmd_execute(client, &cmd);
+ if (ret)
+ goto err;
+
+ chip_id = cmd.args[1] << 24 | cmd.args[2] << 16 | cmd.args[3]
<< 8 |
+ cmd.args[4] << 0;
- chip_id = cmd.args[1] << 24 | cmd.args[2] << 16 | cmd.args[3] << 8 |
- cmd.args[4] << 0;
+ if (chip_id != 0x)
+ break;
+
+ msleep(10);
+ }
+
+ if (i)
+ dev_info(&client->dev, "Needed to wait %i ms to get chip
version", i * 10);
+
+ if (chip_id == 0x) {
+ dev_err(&client->dev, "Unable to retrieve chip version\n");
+ ret = -EINVAL;
+ goto err;
+ }
#define SI2177_A30 ('A' << 24 | 77 << 16 | '3' << 8 | '0' << 0)
#define SI2158_A20 ('A' << 24 | 58 << 16 | '2' << 8 | '0' << 0)
Re: [PATCH] si2157: Add support for Logilink VG0022A.
Em Thu, 3 Oct 2019 12:13:33 +0200
Gonsolo escreveu:
> Hi!
>
> > Could you please test the enclosing patch and see if, with that, you
> > can remove the hacks you added for the tuner probe to work?
>
> I tested again on a vanilla media_tree with Mauro's patch attached.
> Doesn't work. Dmesg output:
>
> [0.788387] kernel: usb 1-1: new high-speed USB device number 2
> using ehci-pci
> [0.792384] kernel: usb 2-1: new high-speed USB device number 2
> using xhci_hcd
> [0.944937] kernel: usb 2-1: New USB device found, idVendor=1d19,
> idProduct=0100, bcdDevice= 1.00
> [0.944939] kernel: usb 2-1: New USB device strings: Mfr=1,
> Product=2, SerialNumber=3
> [0.944940] kernel: usb 2-1: Product: TS Aggregator
> [0.944941] kernel: usb 2-1: Manufacturer: ITE Tech., Inc.
> [0.944942] kernel: usb 2-1: SerialNumber: AF010202071
>
> I then also used the following (additional) patch:
>
> @@ -2119,6 +2122,8 @@ static const struct usb_device_id af9035_id_table[] = {
> /* IT930x devices */
> { DVB_USB_DEVICE(USB_VID_ITETECH, USB_PID_ITETECH_IT9303,
> &it930x_props, "ITE 9303 Generic", NULL) },
> + { DVB_USB_DEVICE(USB_VID_DEXATEK, 0x0100,
> + &it930x_props, "Logilink VG0022A", NULL) },
> { DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_TD310,
> &it930x_props, "AVerMedia TD310 DVB-T2", NULL) },
> { }
>
> Which gives the following output:
>
> [5.380989] si2168 1-0067: Silicon Labs Si2168-B40 successfully identified
> [5.380991] si2168 1-0067: firmware version: B 4.0.2
> [5.381013] usb 2-1: DVB: registering adapter 0 frontend 0 (Silicon
> Labs Si2168)...
> [5.381018] dvbdev: dvb_create_media_entity: media entity 'Silicon
> Labs Si2168' registered.
> [5.390058] checking generic (e000 41) vs hw (e000 1000)
> [5.390062] fb0: switching to inteldrmfb from EFI VGA
> [5.390268] Console: switching to colour dummy device 80x25
> [5.390281] i915 :00:02.0: vgaarb: deactivate vga console
> [5.393438] si2157 2-0063: Silicon Labs Si2147/2148/2157/2158
> successfully attached
Ok. It sounds that the issues you're facing are indeed related to timing
conditions.
>
> But when I try to use VLC I get the following:
>
> [ 457.677363] si2168 1-0067: downloading firmware from file
> 'dvb-demod-si2168-b40-01.fw'
> [ 458.631034] si2168 1-0067: firmware version: B 4.0.11
> [ 458.650309] si2157 2-0063: unknown chip version Si21255-\xff\xff\xff
>
> Now I'm trying other timings.
Returning 0xff, 0xff, 0xff, ... usually means that the tuner chip didn't
respond in time.
This could indicate:
1) The device requires more time to go to sane state after firmware
load and/or device reset/power up;
2) Tuner may be using I2C clock stretching, but the bridge doesn't
support it.
3) The clock used at the I2C bus is still too high;
4) The tuner is hidden by an I2C gate.
I think that using the standard I2C bus clock of 100kbps should be
enough.
I2C clock stretching seems to be unlikely for a command that it is
just getting the device's version.
What seems more likely is that this device may need some time after
firmware load to start working.
So, I would add a msleep() somewhere after the firmware update.
Thanks,
Mauro
Re: [PATCH] si2157: Add support for Logilink VG0022A.
Hi! Boot time: > [5.380991] si2168 1-0067: firmware version: B 4.0.2 When starting VLC: > [ 457.677363] si2168 1-0067: downloading firmware from file > 'dvb-demod-si2168-b40-01.fw' > [ 458.631034] si2168 1-0067: firmware version: B 4.0.11 > [ 458.650309] si2157 2-0063: unknown chip version Si21255-\xff\xff\xff There are two different firmware versions, 4.0.2 and 4.0.11. Is that expected? -- g
Re: [PATCH] si2157: Add support for Logilink VG0022A.
Hi!
> Could you please test the enclosing patch and see if, with that, you
> can remove the hacks you added for the tuner probe to work?
I tested again on a vanilla media_tree with Mauro's patch attached.
Doesn't work. Dmesg output:
[0.788387] kernel: usb 1-1: new high-speed USB device number 2
using ehci-pci
[0.792384] kernel: usb 2-1: new high-speed USB device number 2
using xhci_hcd
[0.944937] kernel: usb 2-1: New USB device found, idVendor=1d19,
idProduct=0100, bcdDevice= 1.00
[0.944939] kernel: usb 2-1: New USB device strings: Mfr=1,
Product=2, SerialNumber=3
[0.944940] kernel: usb 2-1: Product: TS Aggregator
[0.944941] kernel: usb 2-1: Manufacturer: ITE Tech., Inc.
[0.944942] kernel: usb 2-1: SerialNumber: AF010202071
I then also used the following (additional) patch:
@@ -2119,6 +2122,8 @@ static const struct usb_device_id af9035_id_table[] = {
/* IT930x devices */
{ DVB_USB_DEVICE(USB_VID_ITETECH, USB_PID_ITETECH_IT9303,
&it930x_props, "ITE 9303 Generic", NULL) },
+ { DVB_USB_DEVICE(USB_VID_DEXATEK, 0x0100,
+ &it930x_props, "Logilink VG0022A", NULL) },
{ DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_TD310,
&it930x_props, "AVerMedia TD310 DVB-T2", NULL) },
{ }
Which gives the following output:
[5.380989] si2168 1-0067: Silicon Labs Si2168-B40 successfully identified
[5.380991] si2168 1-0067: firmware version: B 4.0.2
[5.381013] usb 2-1: DVB: registering adapter 0 frontend 0 (Silicon
Labs Si2168)...
[5.381018] dvbdev: dvb_create_media_entity: media entity 'Silicon
Labs Si2168' registered.
[5.390058] checking generic (e000 41) vs hw (e000 1000)
[5.390062] fb0: switching to inteldrmfb from EFI VGA
[5.390268] Console: switching to colour dummy device 80x25
[5.390281] i915 :00:02.0: vgaarb: deactivate vga console
[5.393438] si2157 2-0063: Silicon Labs Si2147/2148/2157/2158
successfully attached
But when I try to use VLC I get the following:
[ 457.677363] si2168 1-0067: downloading firmware from file
'dvb-demod-si2168-b40-01.fw'
[ 458.631034] si2168 1-0067: firmware version: B 4.0.11
[ 458.650309] si2157 2-0063: unknown chip version Si21255-\xff\xff\xff
Now I'm trying other timings.
Thanks,
g
Re: [PATCH] si2157: Add support for Logilink VG0022A.
Em Wed, 2 Oct 2019 19:23:02 +0200
JP escreveu:
> Hi all.
>
> On 10/2/19 5:21 PM, Gonsolo wrote:
> > Hi!
> >
> >> Antti has some great suggestions in that thread:
> >> https://lkml.org/lkml/2017/5/24/245
> >>
> >> Also note https://lkml.org/lkml/2017/5/26/357 if you have access to a
> >> logic analyser.
> > I read that thread. Unfortunately I'm not a hardware engineer nor do I
> > have access to a logic analyser, just the device and a remote hope not
> > to keep my custom 4.13 kernel forever or to have to buy a new DVB T2
> > device. :(
> > In the above thread it is mentioned that even the Windows driver
> > receives the 's so maybe it is a hardware bug?
> Looks like it is:
> http://lkml.iu.edu/hypermail/linux/kernel/1710.3/03205.html
Hmm... changing the pull-up register will very likely change the
timings.
There's a logic at the driver that changes the I2C bus speed to
300k (with is non-standard):
/* I2C master bus 2 clock speed 300k */
ret = af9035_wr_reg(d, 0x00f6a7, 0x07);
if (ret < 0)
goto err;
/* I2C master bus 1,3 clock speed 300k */
ret = af9035_wr_reg(d, 0x00f103, 0x07);
if (ret < 0)
goto err;
Perhaps if we reduce the bus speed to 100k, the device will work
without the hacks.
I don't have af9035 datasheets. Perhaps Antti could shed some
light about how to change the speed to 100k, but on a quick search
at the Internet, I found this:
https://lore.kernel.org/linux-media/1312539895.2763.33.camel@Jason-Linux/
With has a comment that explains how the I2C speed is calculated on those
ITE devices:
#definep_reg_one_cycle_counter_tuner0xF103
/* Define I2C master speed, the default value 0x07 means 366KHz
(10 / (24.4 * 16 * User_I2C_SPEED)). */
#define User_I2C_SPEED 0x07
error =
it9135_write_reg(data, 0, PROCESSOR_LINK,
p_reg_one_cycle_counter_tuner, User_I2C_SPEED);
So, in summary, the value for the I2C speed register is given by:
I2C_speed_register = (10 / (24.4 * 16 * I2C_speed))
So, for 100 kbps, the I2C speed register should be set with a value
close to ~25,6.
Doing the reverse math, we have:
I2C_speed_register = 25 -> I2C_speed = 102,46 kbps
I2C_speed_register = 26 -> I2C_speed = 98,52 kbps
So, if we do:
/* I2C master bus 2 clock speed ~100k */
ret = af9035_wr_reg(d, 0x00f6a7, 26);
if (ret < 0)
goto err;
/* I2C master bus 1,3 clock speed ~100k */
ret = af9035_wr_reg(d, 0x00f103, 26);
if (ret < 0)
goto err;
The bus speed will reduce to 98,52 kbps, with is a typical nominal
value for I2C tuners and other devices. With that, the device should
probably work fine without needing to replace the pull up resistor.
Ok, tuner firmware load would be ~3,7 times slower, but this is
something that we do just once need a firmware).
All other I2C messages are short enough to not cause any real difference.
Could you please test the enclosing patch and see if, with that, you
can remove the hacks you added for the tuner probe to work?
Regards,
Mauro
[PATCH] media: af9035: slow down I2C bus speed on it930x devices
We have a few reports about tuner probing instability with
some it930x devices.
As it is better safe than sorry, let's slow down the I2C,
using the formula found at an old patch:
https://lore.kernel.org/linux-media/1312539895.2763.33.camel@Jason-Linux/
Signed-off-by: Mauro Carvalho Chehab
diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c
b/drivers/media/usb/dvb-usb-v2/af9035.c
index 3afd18733614..df2c75b84be8 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.c
+++ b/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -1197,6 +1197,9 @@ static int af9035_frontend_attach(struct dvb_usb_adapter
*adap)
return ret;
}
+/* I2C speed register = (10 / (24.4 * 16 * I2C_speed)) */
+#define I2C_SPEED_REGISTER 26
+
static int it930x_frontend_attach(struct dvb_usb_adapter *adap)
{
struct state *state = adap_to_priv(adap);
@@ -1208,13 +1211,13 @@ static int it930x_frontend_attach(struct
dvb_usb_adapter *adap)
dev_dbg(&intf->dev, "adap->id=%d\n", adap->id);
- /* I2C master bus 2 clock speed 300k */
- ret = af9035_wr_reg(d, 0x00f6a7, 0x07);
+ /* I2C master bus 2 clock speed ~100k */
+ ret = af9035_wr_reg(d, 0x00f6a7, I2C_SPEED_REGISTER);
if (ret < 0)
goto err;
- /* I2C master bus 1,3 clock speed 300k */
- ret = af9035_wr_reg(d, 0x00f103, 0x07);
+ /* I2C master bus 1,3 clock speed ~100k */
+ ret = af9035_wr_reg(d, 0x00f103, I2C_SPEED_REGISTER);
if (ret < 0)
goto err;
Re: [PATCH] si2157: Add support for Logilink VG0022A.
Hi all. On 10/2/19 5:21 PM, Gonsolo wrote: Hi! Antti has some great suggestions in that thread: https://lkml.org/lkml/2017/5/24/245 Also note https://lkml.org/lkml/2017/5/26/357 if you have access to a logic analyser. I read that thread. Unfortunately I'm not a hardware engineer nor do I have access to a logic analyser, just the device and a remote hope not to keep my custom 4.13 kernel forever or to have to buy a new DVB T2 device. :( In the above thread it is mentioned that even the Windows driver receives the 's so maybe it is a hardware bug? Looks like it is: http://lkml.iu.edu/hypermail/linux/kernel/1710.3/03205.html I'd love to see this driver upstream but I have no idea how to proceed. Any suggestions?
Re: [PATCH] si2157: Add support for Logilink VG0022A.
Hi! > Antti has some great suggestions in that thread: > https://lkml.org/lkml/2017/5/24/245 > > Also note https://lkml.org/lkml/2017/5/26/357 if you have access to a > logic analyser. I read that thread. Unfortunately I'm not a hardware engineer nor do I have access to a logic analyser, just the device and a remote hope not to keep my custom 4.13 kernel forever or to have to buy a new DVB T2 device. :( In the above thread it is mentioned that even the Windows driver receives the 's so maybe it is a hardware bug? I'd love to see this driver upstream but I have no idea how to proceed. Any suggestions? -- g
Re: [PATCH] si2157: Add support for Logilink VG0022A.
On Wed, Oct 02, 2019 at 04:44:24PM +0200, Gonsolo wrote: > Hi! > > > You need a message and a Signed-off-by: here. > > Ok, I'll try to get that right the next time. > > > > + ret = si2157_power_up(dev, client); > > > + if (ret) > > > + goto err; > > > + /* query chip revision */ > > > + /* hack: do it here because after the si2168 gets 0101, commands > > > will > > > + * still be executed here but no result > > > > I don't understand. What problem are you seeing here? Why can't you do a > > query chip revision first? > > This was explained here: https://lkml.org/lkml/2017/3/15/778. To quote: Antti has some great suggestions in that thread: https://lkml.org/lkml/2017/5/24/245 Also note https://lkml.org/lkml/2017/5/26/357 if you have access to a logic analyser. Sean
Re: [PATCH] si2157: Add support for Logilink VG0022A.
Em Wed, 2 Oct 2019 16:13:59 +0200
Gon Solo escreveu:
All patches should have a description at the beginning of the e-mail
body, even the trivial ones.
You also forgot to add your Signed-off-by.
> ---
> drivers/media/tuners/si2157.c | 68 +--
> drivers/media/tuners/si2157_priv.h| 1 +
> drivers/media/usb/dvb-usb-v2/af9035.c | 47 ++
> 3 files changed, 90 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
> index e87040d6eca7..8f9df2485d51 100644
> --- a/drivers/media/tuners/si2157.c
> +++ b/drivers/media/tuners/si2157.c
> @@ -66,6 +66,24 @@ static int si2157_cmd_execute(struct i2c_client *client,
> struct si2157_cmd *cmd)
> return ret;
> }
>
> +static int si2157_power_up(struct si2157_dev *dev, struct i2c_client *client)
> +{
> + struct si2157_cmd cmd;
> +
> + if (dev->chiptype == SI2157_CHIPTYPE_SI2146) {
> + memcpy(cmd.args, "\xc0\x05\x01\x00\x00\x0b\x00\x00\x01", 9);
> + cmd.wlen = 9;
> + } else if (dev->chiptype == SI2157_CHIPTYPE_SI2141) {
> + memcpy(cmd.args, "\xc0\x00\x0d\x0e\x00\x01\x01\x01\x01\x03",
> 10);
> + cmd.wlen = 10;
> + } else {
> + memcpy(cmd.args,
> "\xc0\x00\x0c\x00\x00\x01\x01\x01\x01\x01\x01\x02\x00\x00\x01", 15);
> + cmd.wlen = 15;
> + }
> + cmd.rlen = 1;
> + return si2157_cmd_execute(client, &cmd);
> +}
> +
> static int si2157_init(struct dvb_frontend *fe)
> {
> struct i2c_client *client = fe->tuner_priv;
> @@ -75,7 +93,7 @@ static int si2157_init(struct dvb_frontend *fe)
> struct si2157_cmd cmd;
> const struct firmware *fw;
> const char *fw_name;
> - unsigned int uitmp, chip_id;
> + unsigned int uitmp;
>
> dev_dbg(&client->dev, "\n");
>
> @@ -93,19 +111,7 @@ static int si2157_init(struct dvb_frontend *fe)
> if (uitmp == dev->if_frequency / 1000)
> goto warm;
>
> - /* power up */
> - if (dev->chiptype == SI2157_CHIPTYPE_SI2146) {
> - memcpy(cmd.args, "\xc0\x05\x01\x00\x00\x0b\x00\x00\x01", 9);
> - cmd.wlen = 9;
> - } else if (dev->chiptype == SI2157_CHIPTYPE_SI2141) {
> - memcpy(cmd.args, "\xc0\x00\x0d\x0e\x00\x01\x01\x01\x01\x03",
> 10);
> - cmd.wlen = 10;
> - } else {
> - memcpy(cmd.args,
> "\xc0\x00\x0c\x00\x00\x01\x01\x01\x01\x01\x01\x02\x00\x00\x01", 15);
> - cmd.wlen = 15;
> - }
> - cmd.rlen = 1;
> - ret = si2157_cmd_execute(client, &cmd);
> + ret = si2157_power_up(dev, client);
> if (ret)
> goto err;
Ok, you're moving the power-op code to a function. That's OK, but
the rule is one patch per functional change.
So, the first patch in this series should be the one moving the
power up code to a separate function, e. g. the e-mail would be
something like:
Subject: [PATCH 1/3] media: si2157: move power up code to a function
On some devices, we need to power up the device on other places,
so move the code to a separate function.
Signed-off-by: your name
...
>
> @@ -118,17 +124,6 @@ static int si2157_init(struct dvb_frontend *fe)
> goto err;
> }
>
> - /* query chip revision */
> - memcpy(cmd.args, "\x02", 1);
> - cmd.wlen = 1;
> - cmd.rlen = 13;
> - ret = si2157_cmd_execute(client, &cmd);
> - if (ret)
> - goto err;
> -
> - chip_id = cmd.args[1] << 24 | cmd.args[2] << 16 | cmd.args[3] << 8 |
> - cmd.args[4] << 0;
> -
> #define SI2177_A30 ('A' << 24 | 77 << 16 | '3' << 8 | '0' << 0)
> #define SI2158_A20 ('A' << 24 | 58 << 16 | '2' << 8 | '0' << 0)
> #define SI2148_A20 ('A' << 24 | 48 << 16 | '2' << 8 | '0' << 0)
> @@ -137,7 +132,7 @@ static int si2157_init(struct dvb_frontend *fe)
> #define SI2146_A10 ('A' << 24 | 46 << 16 | '1' << 8 | '0' << 0)
> #define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)
>
> - switch (chip_id) {
> + switch (dev->chip_id) {
> case SI2158_A20:
> case SI2148_A20:
> fw_name = SI2158_A20_FIRMWARE;
> @@ -456,6 +451,27 @@ static int si2157_probe(struct i2c_client *client,
> memcpy(&fe->ops.tuner_ops, &si2157_ops, sizeof(struct dvb_tuner_ops));
> fe->tuner_priv = client;
>
> + ret = si2157_power_up(dev, client);
> + if (ret)
> + goto err;
> + /* query chip revision */
> + /* hack: do it here because after the si2168 gets 0101, commands will
> + * still be executed here but no result
> + */
> + memcpy(cmd.args, "\x02", 1);
> + cmd.wlen = 1;
> + cmd.rlen = 13;
> + ret = si2157_cmd_execute(client, &cmd);
> + if (ret)
> + goto err_kfree;
> + dev->chip_id = cmd.args[1] << 24 |
> + cmd.args[2] << 16 |
> + cmd.args[3] << 8 |
> +
Re: [PATCH] si2157: Add support for Logilink VG0022A.
Hi! > You need a message and a Signed-off-by: here. Ok, I'll try to get that right the next time. > > + ret = si2157_power_up(dev, client); > > + if (ret) > > + goto err; > > + /* query chip revision */ > > + /* hack: do it here because after the si2168 gets 0101, commands will > > + * still be executed here but no result > > I don't understand. What problem are you seeing here? Why can't you do a > query chip revision first? This was explained here: https://lkml.org/lkml/2017/3/15/778. To quote: If the si2157 is behind a e.g. si2168, the si2157 will at least in some situations not be readable after the si268 got the command 0101. It still accepts commands but the answer is just ff. So read the chip id before that so the information is not lost. The following line in kernel output is a symptome of that problem: si2157 7-0063: unknown chip version Si21255-\x\x\x > This needs resolving of course. I hope so. :) g
Re: [PATCH] si2157: Add support for Logilink VG0022A.
On Wed, Oct 02, 2019 at 04:13:59PM +0200, Gon Solo wrote:
You need a message and a Signed-off-by: here.
> ---
> drivers/media/tuners/si2157.c | 68 +--
> drivers/media/tuners/si2157_priv.h| 1 +
> drivers/media/usb/dvb-usb-v2/af9035.c | 47 ++
> 3 files changed, 90 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
> index e87040d6eca7..8f9df2485d51 100644
> --- a/drivers/media/tuners/si2157.c
> +++ b/drivers/media/tuners/si2157.c
> @@ -66,6 +66,24 @@ static int si2157_cmd_execute(struct i2c_client *client,
> struct si2157_cmd *cmd)
> return ret;
> }
>
> +static int si2157_power_up(struct si2157_dev *dev, struct i2c_client *client)
> +{
> + struct si2157_cmd cmd;
> +
> + if (dev->chiptype == SI2157_CHIPTYPE_SI2146) {
> + memcpy(cmd.args, "\xc0\x05\x01\x00\x00\x0b\x00\x00\x01", 9);
> + cmd.wlen = 9;
> + } else if (dev->chiptype == SI2157_CHIPTYPE_SI2141) {
> + memcpy(cmd.args, "\xc0\x00\x0d\x0e\x00\x01\x01\x01\x01\x03",
> 10);
> + cmd.wlen = 10;
> + } else {
> + memcpy(cmd.args,
> "\xc0\x00\x0c\x00\x00\x01\x01\x01\x01\x01\x01\x02\x00\x00\x01", 15);
> + cmd.wlen = 15;
> + }
> + cmd.rlen = 1;
> + return si2157_cmd_execute(client, &cmd);
> +}
> +
> static int si2157_init(struct dvb_frontend *fe)
> {
> struct i2c_client *client = fe->tuner_priv;
> @@ -75,7 +93,7 @@ static int si2157_init(struct dvb_frontend *fe)
> struct si2157_cmd cmd;
> const struct firmware *fw;
> const char *fw_name;
> - unsigned int uitmp, chip_id;
> + unsigned int uitmp;
>
> dev_dbg(&client->dev, "\n");
>
> @@ -93,19 +111,7 @@ static int si2157_init(struct dvb_frontend *fe)
> if (uitmp == dev->if_frequency / 1000)
> goto warm;
>
> - /* power up */
> - if (dev->chiptype == SI2157_CHIPTYPE_SI2146) {
> - memcpy(cmd.args, "\xc0\x05\x01\x00\x00\x0b\x00\x00\x01", 9);
> - cmd.wlen = 9;
> - } else if (dev->chiptype == SI2157_CHIPTYPE_SI2141) {
> - memcpy(cmd.args, "\xc0\x00\x0d\x0e\x00\x01\x01\x01\x01\x03",
> 10);
> - cmd.wlen = 10;
> - } else {
> - memcpy(cmd.args,
> "\xc0\x00\x0c\x00\x00\x01\x01\x01\x01\x01\x01\x02\x00\x00\x01", 15);
> - cmd.wlen = 15;
> - }
> - cmd.rlen = 1;
> - ret = si2157_cmd_execute(client, &cmd);
> + ret = si2157_power_up(dev, client);
> if (ret)
> goto err;
>
> @@ -118,17 +124,6 @@ static int si2157_init(struct dvb_frontend *fe)
> goto err;
> }
>
> - /* query chip revision */
> - memcpy(cmd.args, "\x02", 1);
> - cmd.wlen = 1;
> - cmd.rlen = 13;
> - ret = si2157_cmd_execute(client, &cmd);
> - if (ret)
> - goto err;
> -
> - chip_id = cmd.args[1] << 24 | cmd.args[2] << 16 | cmd.args[3] << 8 |
> - cmd.args[4] << 0;
> -
> #define SI2177_A30 ('A' << 24 | 77 << 16 | '3' << 8 | '0' << 0)
> #define SI2158_A20 ('A' << 24 | 58 << 16 | '2' << 8 | '0' << 0)
> #define SI2148_A20 ('A' << 24 | 48 << 16 | '2' << 8 | '0' << 0)
> @@ -137,7 +132,7 @@ static int si2157_init(struct dvb_frontend *fe)
> #define SI2146_A10 ('A' << 24 | 46 << 16 | '1' << 8 | '0' << 0)
> #define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)
>
> - switch (chip_id) {
> + switch (dev->chip_id) {
> case SI2158_A20:
> case SI2148_A20:
> fw_name = SI2158_A20_FIRMWARE;
> @@ -456,6 +451,27 @@ static int si2157_probe(struct i2c_client *client,
> memcpy(&fe->ops.tuner_ops, &si2157_ops, sizeof(struct dvb_tuner_ops));
> fe->tuner_priv = client;
>
> + ret = si2157_power_up(dev, client);
> + if (ret)
> + goto err;
> + /* query chip revision */
> + /* hack: do it here because after the si2168 gets 0101, commands will
> + * still be executed here but no result
I don't understand. What problem are you seeing here? Why can't you do a
query chip revision first?
This needs resolving of course.
> + */
> + memcpy(cmd.args, "\x02", 1);
> + cmd.wlen = 1;
> + cmd.rlen = 13;
> + ret = si2157_cmd_execute(client, &cmd);
> + if (ret)
> + goto err_kfree;
> + dev->chip_id = cmd.args[1] << 24 |
> + cmd.args[2] << 16 |
> + cmd.args[3] << 8 |
> + cmd.args[4] << 0;
> + dev_info(&client->dev, "found a 'Silicon Labs Si21%d-%c%c%c'\n",
> + cmd.args[2], cmd.args[1], cmd.args[3], cmd.args[4]);
> +
> +
> #ifdef CONFIG_MEDIA_CONTROLLER
> if (cfg->mdev) {
> dev->mdev = cfg->mdev;
> diff --git a/drivers/media/tuners/si2157_priv.h
> b/drivers/media/tuners/si2157_priv.h
> index 2bda903358da..9ab7c88b01b4 100644
> --- a/drivers/media/tuners/si2157_priv.h
> +++ b/drivers/m
[PATCH] si2157: Add support for Logilink VG0022A.
---
drivers/media/tuners/si2157.c | 68 +--
drivers/media/tuners/si2157_priv.h| 1 +
drivers/media/usb/dvb-usb-v2/af9035.c | 47 ++
3 files changed, 90 insertions(+), 26 deletions(-)
diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index e87040d6eca7..8f9df2485d51 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -66,6 +66,24 @@ static int si2157_cmd_execute(struct i2c_client *client,
struct si2157_cmd *cmd)
return ret;
}
+static int si2157_power_up(struct si2157_dev *dev, struct i2c_client *client)
+{
+ struct si2157_cmd cmd;
+
+ if (dev->chiptype == SI2157_CHIPTYPE_SI2146) {
+ memcpy(cmd.args, "\xc0\x05\x01\x00\x00\x0b\x00\x00\x01", 9);
+ cmd.wlen = 9;
+ } else if (dev->chiptype == SI2157_CHIPTYPE_SI2141) {
+ memcpy(cmd.args, "\xc0\x00\x0d\x0e\x00\x01\x01\x01\x01\x03",
10);
+ cmd.wlen = 10;
+ } else {
+ memcpy(cmd.args,
"\xc0\x00\x0c\x00\x00\x01\x01\x01\x01\x01\x01\x02\x00\x00\x01", 15);
+ cmd.wlen = 15;
+ }
+ cmd.rlen = 1;
+ return si2157_cmd_execute(client, &cmd);
+}
+
static int si2157_init(struct dvb_frontend *fe)
{
struct i2c_client *client = fe->tuner_priv;
@@ -75,7 +93,7 @@ static int si2157_init(struct dvb_frontend *fe)
struct si2157_cmd cmd;
const struct firmware *fw;
const char *fw_name;
- unsigned int uitmp, chip_id;
+ unsigned int uitmp;
dev_dbg(&client->dev, "\n");
@@ -93,19 +111,7 @@ static int si2157_init(struct dvb_frontend *fe)
if (uitmp == dev->if_frequency / 1000)
goto warm;
- /* power up */
- if (dev->chiptype == SI2157_CHIPTYPE_SI2146) {
- memcpy(cmd.args, "\xc0\x05\x01\x00\x00\x0b\x00\x00\x01", 9);
- cmd.wlen = 9;
- } else if (dev->chiptype == SI2157_CHIPTYPE_SI2141) {
- memcpy(cmd.args, "\xc0\x00\x0d\x0e\x00\x01\x01\x01\x01\x03",
10);
- cmd.wlen = 10;
- } else {
- memcpy(cmd.args,
"\xc0\x00\x0c\x00\x00\x01\x01\x01\x01\x01\x01\x02\x00\x00\x01", 15);
- cmd.wlen = 15;
- }
- cmd.rlen = 1;
- ret = si2157_cmd_execute(client, &cmd);
+ ret = si2157_power_up(dev, client);
if (ret)
goto err;
@@ -118,17 +124,6 @@ static int si2157_init(struct dvb_frontend *fe)
goto err;
}
- /* query chip revision */
- memcpy(cmd.args, "\x02", 1);
- cmd.wlen = 1;
- cmd.rlen = 13;
- ret = si2157_cmd_execute(client, &cmd);
- if (ret)
- goto err;
-
- chip_id = cmd.args[1] << 24 | cmd.args[2] << 16 | cmd.args[3] << 8 |
- cmd.args[4] << 0;
-
#define SI2177_A30 ('A' << 24 | 77 << 16 | '3' << 8 | '0' << 0)
#define SI2158_A20 ('A' << 24 | 58 << 16 | '2' << 8 | '0' << 0)
#define SI2148_A20 ('A' << 24 | 48 << 16 | '2' << 8 | '0' << 0)
@@ -137,7 +132,7 @@ static int si2157_init(struct dvb_frontend *fe)
#define SI2146_A10 ('A' << 24 | 46 << 16 | '1' << 8 | '0' << 0)
#define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)
- switch (chip_id) {
+ switch (dev->chip_id) {
case SI2158_A20:
case SI2148_A20:
fw_name = SI2158_A20_FIRMWARE;
@@ -456,6 +451,27 @@ static int si2157_probe(struct i2c_client *client,
memcpy(&fe->ops.tuner_ops, &si2157_ops, sizeof(struct dvb_tuner_ops));
fe->tuner_priv = client;
+ ret = si2157_power_up(dev, client);
+ if (ret)
+ goto err;
+ /* query chip revision */
+ /* hack: do it here because after the si2168 gets 0101, commands will
+* still be executed here but no result
+*/
+ memcpy(cmd.args, "\x02", 1);
+ cmd.wlen = 1;
+ cmd.rlen = 13;
+ ret = si2157_cmd_execute(client, &cmd);
+ if (ret)
+ goto err_kfree;
+ dev->chip_id = cmd.args[1] << 24 |
+ cmd.args[2] << 16 |
+ cmd.args[3] << 8 |
+ cmd.args[4] << 0;
+ dev_info(&client->dev, "found a 'Silicon Labs Si21%d-%c%c%c'\n",
+ cmd.args[2], cmd.args[1], cmd.args[3], cmd.args[4]);
+
+
#ifdef CONFIG_MEDIA_CONTROLLER
if (cfg->mdev) {
dev->mdev = cfg->mdev;
diff --git a/drivers/media/tuners/si2157_priv.h
b/drivers/media/tuners/si2157_priv.h
index 2bda903358da..9ab7c88b01b4 100644
--- a/drivers/media/tuners/si2157_priv.h
+++ b/drivers/media/tuners/si2157_priv.h
@@ -28,6 +28,7 @@ struct si2157_dev {
u8 chiptype;
u8 if_port;
u32 if_frequency;
+ u32 chip_id;
struct delayed_work stat_work;
#if defined(CONFIG_MEDIA_CONTROLLER)
diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c
b/drivers/media/usb/dvb-usb-v2/af9035.c
index 3afd18
[PATCH] si2157: Add support for Logilink VG0022A.
Dear Mauro! Thanks for your reply, I tried to address most of your concerns, so please bear with me (especially this is my first email with git send-email). :) > First of all, don't attach a patch. Instead, just send it with a decent > emailer (with won't mangle whitespaces) or use git send-email... Done. > You shouldn't just blindly comment out some code, as this will very likely > break support for all other devices supported by the driver... Done. I extracted power_up into its own function, chip querying is now done in probe. I don't have enough knowledge about hardware to do the right thing on my own. If there is anybody willing to guide me I will spend some time on it. The original patch where the problem is discussed is https://lkml.kernel.org/lkml/1489616530-4025-1-git-send-email-andr...@kemnade.info > ... yet, looking on what you've done, it seems that you're actually > adding support for a different tuner at the si2157 driver. > If this is the case, this should be on a separate patch, and in a way > that it will become clear that it won't break support for any existing > device. I'm not entirely sure how to split this up. Can you give some advice? > Why did you do such change? dev_dbg can already print the function, and > much more. See: Thanks for the link. I removed these lines. > The above seems specific for your device. You need to check if the device > is USB_VID_DEXATEK, running the code only on such case. Done. Though I'm not sure whether I did it the right way. Thanks for all the advice, I hope this will be included eventually. g
