Re: [PATCH] si2157: Add support for Logilink VG0022A.

[JP]

On 10/4/19 2:08 PM, Mauro Carvalho Chehab wrote:
> Em Fri, 4 Oct 2019 13:50:43 +0200
> JP <jp@xxxxxxx> escreveu:
>
> > On 10/3/19 10:03 PM, Mauro Carvalho Chehab wrote:
> > > Em Thu, 3 Oct 2019 21:51:35 +0200
> > > Gonsolo <gonsolo@xxxxxxxxx> 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 0xffff
> > > > 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.