Re: [PATCH] [media] af9035: fix for MXL5007T devices with I2C read issues

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/20/2016 12:13 PM, Alex Rad wrote:
On Wed, Apr 20, 2016 at 1:02 AM, Antti Palosaari <crope@xxxxxx> wrote:
Hello
I am not happy with that new module parameter as I cannot see real need for
it. So get rid of it.

My reasoning for this is:
1) We know of just two devices which may have the issue, but there are
probably more.  The module parameter allows a user to apply the
workaround to other devices we did not consider or test.  Should we
perhaps apply for all mxl5007t devices?

So what. It is easier for me add just new IDs to driver when problematic device is found than adding new module parameter which allows user to work-around issues and I will likely newer even hear about those issues.

2) Not all devices that match VID and PID have the issue, so it allows
the user to disable the workaround.

Due to that better to add three checks
1) it is Avermedia VID
2) it is known problematic Avermedia device PID
3) it has tuner MxL5007t

It is not surprise it is just only Avermedia which has this kind of problems. Not first time at all. I have added such Avermedia hacks for af9015 driver too and if you look af9035 there is tuner id hack - which was added also due to Avermedia.

Worst case there could be is some mxl5007t device having reference design ID. But even on that case returning fake values is pretty much OK.


regards
Antti


Better to compare both VID and PID when enabling that work-around. Driver
supports currently quite many different USB IDs and there is still small
risk duplicate PID will exists at some point enabling work-around for wrong
device.


OK.  Will wait for comments on above before a v2.

Thanks,
Alessandro

regards
Antti




On 04/15/2016 06:37 PM, Alessandro Radicati wrote:

The MXL5007T tuner will lock-up on some devices after an I2C read
transaction.  This patch adds a kernel module parameter "no_read" to work
around this issue by inhibiting such operations and emulating a 0x00
response.  The workaround is applied automatically to USB product IDs
known
to exhibit this flaw, unless the kernel module parameter is specified.

Signed-off-by: Alessandro Radicati <alessandro@xxxxxxxxxxxx>
---
   drivers/media/usb/dvb-usb-v2/af9035.c | 27 +++++++++++++++++++++++++++
   drivers/media/usb/dvb-usb-v2/af9035.h |  1 +
   2 files changed, 28 insertions(+)

diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c
b/drivers/media/usb/dvb-usb-v2/af9035.c
index 2638e32..8225403 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.c
+++ b/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -24,6 +24,10 @@
   /* Max transfer size done by I2C transfer functions */
   #define MAX_XFER_SIZE  64

+static int dvb_usb_af9035_no_read = -1;
+module_param_named(no_read, dvb_usb_af9035_no_read, int, 0644);
+MODULE_PARM_DESC(no_read, "Emulate I2C reads for devices that do not
support them.");
+
   DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);

   static u16 af9035_checksum(const u8 *buf, size_t len)
@@ -348,6 +352,9 @@ static int af9035_i2c_master_xfer(struct i2c_adapter
*adap,

                         ret = af9035_rd_regs(d, reg, &msg[1].buf[0],
                                         msg[1].len);
+               } else if (state->no_read) {
+                       memset(msg[1].buf, 0, msg[1].len);
+                       ret = 0;
                 } else {
                         /* I2C write + read */
                         u8 buf[MAX_XFER_SIZE];
@@ -421,6 +428,9 @@ static int af9035_i2c_master_xfer(struct i2c_adapter
*adap,
                 if (msg[0].len > 40) {
                         /* TODO: correct limits > 40 */
                         ret = -EOPNOTSUPP;
+               } else if (state->no_read) {
+                       memset(msg[0].buf, 0, msg[0].len);
+                       ret = 0;
                 } else {
                         /* I2C read */
                         u8 buf[5];
@@ -962,6 +972,23 @@ skip_eeprom:
                         state->af9033_config[i].clock =
clock_lut_af9035[tmp];
         }

+       /* Some MXL5007T devices cannot properly handle tuner I2C read
ops. */
+       if (dvb_usb_af9035_no_read != -1) { /* Override with module param
*/
+               state->no_read = dvb_usb_af9035_no_read == 0 ? false :
true;
+       } else {
+               switch (le16_to_cpu(d->udev->descriptor.idProduct)) {
+               case USB_PID_AVERMEDIA_A867:
+               case USB_PID_AVERMEDIA_TWINSTAR:
+                       dev_info(&d->udev->dev,
+                               "%s: Device may have issues with I2C read
operations. Enabling fix.\n",
+                               KBUILD_MODNAME);
+                       state->no_read = true;
+                       break;
+               default:
+                       state->no_read = false;
+               }
+       }
+
         return 0;

   err:
diff --git a/drivers/media/usb/dvb-usb-v2/af9035.h
b/drivers/media/usb/dvb-usb-v2/af9035.h
index df22001..a76dafa 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.h
+++ b/drivers/media/usb/dvb-usb-v2/af9035.h
@@ -62,6 +62,7 @@ struct state {
         u8 chip_version;
         u16 chip_type;
         u8 dual_mode:1;
+       u8 no_read:1;
         u16 eeprom_addr;
         u8 af9033_i2c_addr[2];
         struct af9033_config af9033_config[2];


--
http://palosaari.fi/

--
http://palosaari.fi/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux