From ca2c87bee6548a35f8df52c4cdbbe809082e2226 Mon Sep 17 00:00:00 2001 From: James Hutchinson Date: Wed, 12 Dec 2018 10:20:18 +0000 Subject: [PATCH] media: dvbsky: use a single mutex and state buffers for all R/W ops See: https://bugzilla.kernel.org/show_bug.cgi?id=199323 This builds on the previous attempt to serialize all R/W ops, which caused bad effects for several users: media: dvbsky: use just one mutex for serializing device R/W ops Mutex locking and timeout issues have been reported by several users, on various kernel versions. With the issue seemingly more prevalent on kernel 4.10 and above following changes to the mutex/locking code. Debug tracing shows the device malfunctioning shortly after receiving an extra {10} message in-between the {37 00 00}, {36 03 00} message pair. dvb_usb_v2:dvb_usb_v2_generic_io: usb 1-1: dvb_usb_v2_generic_io: >>> 37 00 00 dvb_usb_v2:dvb_usb_v2_generic_io: usb 1-1: dvb_usb_v2_generic_io: >>> 10 dvb_usb_v2:dvb_usb_v2_generic_io: usb 1-1: dvb_usb_v2_generic_io: <<< ff ff dvb_usb_v2:dvb_usb_v2_generic_io: usb 1-1: dvb_usb_v2_generic_io: >>> 36 03 00 ... m88ds3103:m88ds3103_diseqc_send_master_cmd: m88ds3103 4-0068: diseqc tx timeout m88ds3103:m88ds3103_diseqc_send_master_cmd: m88ds3103 4-0068: failed=-110 Resolve this by using the single usb_mutex as attempted previously, this time using the obuf state buffer. Also, simplify things a little by using dvb_usbv2_generic_write_locked rather than dvb_usbv2_generic_rw_locked. Finally, the dvbsky_i2c_xfer algo was altered to grab a non-interruptible lock on the i2c_mutex. --- drivers/media/usb/dvb-usb-v2/dvbsky.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/drivers/media/usb/dvb-usb-v2/dvbsky.c b/drivers/media/usb/dvb-usb-v2/dvbsky.c index e28bd8836751..b825621e085d 100644 --- a/drivers/media/usb/dvb-usb-v2/dvbsky.c +++ b/drivers/media/usb/dvb-usb-v2/dvbsky.c @@ -31,7 +31,6 @@ MODULE_PARM_DESC(disable_rc, "Disable inbuilt IR receiver."); DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr); struct dvbsky_state { - struct mutex stream_mutex; u8 ibuf[DVBSKY_BUF_LEN]; u8 obuf[DVBSKY_BUF_LEN]; u8 last_lock; @@ -70,16 +69,18 @@ static int dvbsky_stream_ctrl(struct dvb_usb_device *d, u8 onoff) { struct dvbsky_state *state = d_to_priv(d); int ret; - u8 obuf_pre[3] = { 0x37, 0, 0 }; - u8 obuf_post[3] = { 0x36, 3, 0 }; + static u8 obuf_pre[3] = { 0x37, 0, 0 }; + static u8 obuf_post[3] = { 0x36, 3, 0 }; - mutex_lock(&state->stream_mutex); - ret = dvbsky_usb_generic_rw(d, obuf_pre, 3, NULL, 0); + mutex_lock(&d->usb_mutex); + memcpy(state->obuf, obuf_pre, 3); + ret = dvb_usbv2_generic_write_locked(d, state->obuf, 3); if (!ret && onoff) { msleep(20); - ret = dvbsky_usb_generic_rw(d, obuf_post, 3, NULL, 0); + memcpy(state->obuf, obuf_post, 3); + ret = dvb_usbv2_generic_write_locked(d, state->obuf, 3); } - mutex_unlock(&state->stream_mutex); + mutex_unlock(&d->usb_mutex); return ret; } @@ -113,8 +114,7 @@ static int dvbsky_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msg[], int ret = 0; u8 ibuf[64], obuf[64]; - if (mutex_lock_interruptible(&d->i2c_mutex) < 0) - return -EAGAIN; + mutex_lock(&d->i2c_mutex); if (num > 2) { dev_err(&d->udev->dev, @@ -608,8 +608,6 @@ static int dvbsky_init(struct dvb_usb_device *d) if (ret) return ret; */ - mutex_init(&state->stream_mutex); - state->last_lock = 0; return 0; -- 2.11.0