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

[Mauro Carvalho Chehab <mchehab+samsung@kernel.org>]

Em Fri, 4 Oct 2019 13:50:43 +0200
JP <jp@jpvw.nl> escreveu:

> On 10/3/19 10:03 PM, Mauro Carvalho Chehab wrote:
> > Em Thu, 3 Oct 2019 21:51:35 +0200
> > Gonsolo <gonsolo@gmail.com> 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.

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