DVBSky S960CI hard broken in 4.18

DVBSky S960CI hard broken in 4.18

[Oliver Freyermuth]

Dear DVB experts,

commit:
7d95fb7 - media: dvbsky: use just one mutex for serializing device R/W ops
hard breaks DVBSky cards. 
Also the previous locking commits have caused several runtime issues. 

Since the bug tracker is not regularly checked, I'd love to make everybody aware of the corresponding issue
with much more detail: 
https://bugzilla.kernel.org/show_bug.cgi?id=199323
which has gotten large attention from several users. 

>From my side, I can confirm that reverting 7d95fb7 makes the card work again for me. on 4.18. 
With 7d95fb7 applied, the card tunes fine, but does not deliver any data. 

Please include me in any replies, I am not subscribed to the list. 

Cheers and all the best,
	Oliver

[Regression] DVBSky S960CI hard broken in 4.18

[Oliver Freyermuth]

Hi all,

sorry to bump this, but I wonder whether I could help by submitting a [PATCH] reverting the buggy change from 7d95fb7 ? 
As discussed in https://bugzilla.kernel.org/show_bug.cgi?id=199323 , the widespread card is unusable since 7d95fb7 . 

Please include me in any replies, I am not subscribed to the list. 

Cheers,
	Oliver

Am 24.09.18 um 22:26 schrieb Oliver Freyermuth:
> Dear DVB experts,
> 
> commit:
> 7d95fb7 - media: dvbsky: use just one mutex for serializing device R/W ops
> hard breaks DVBSky cards. 
> Also the previous locking commits have caused several runtime issues. 
> 
> Since the bug tracker is not regularly checked, I'd love to make everybody aware of the corresponding issue
> with much more detail: 
> https://bugzilla.kernel.org/show_bug.cgi?id=199323
> which has gotten large attention from several users. 
> 
> From my side, I can confirm that reverting 7d95fb7 makes the card work again for me. on 4.18. 
> With 7d95fb7 applied, the card tunes fine, but does not deliver any data. 
> 
> Please include me in any replies, I am not subscribed to the list. 
> 
> Cheers and all the best,
> 	Oliver
> 

[PATCH] Revert "media: dvbsky: use just one mutex for serializing device R/W ops"

[Mauro Carvalho Chehab]

As pointed at:
	https://bugzilla.kernel.org/show_bug.cgi?id=199323

This patch causes a bad effect on RPi. I suspect that the root
cause is at the USB RPi driver, with uses high priority
interrupts instead of normal ones. Anyway, as this patch
is mostly a cleanup, better to revert it.

This reverts commit 7d95fb746c4eece67308f1642a666ea1ebdbd2cc.

Cc: stable@vger.kernel.org # For Kernel 4.18
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
 drivers/media/usb/dvb-usb-v2/dvbsky.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/dvb-usb-v2/dvbsky.c b/drivers/media/usb/dvb-usb-v2/dvbsky.c
index 1aa88d94e57f..e28bd8836751 100644
--- a/drivers/media/usb/dvb-usb-v2/dvbsky.c
+++ b/drivers/media/usb/dvb-usb-v2/dvbsky.c
@@ -31,6 +31,7 @@ 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;
@@ -67,17 +68,18 @@ static int dvbsky_usb_generic_rw(struct dvb_usb_device *d,
 
 static int dvbsky_stream_ctrl(struct dvb_usb_device *d, u8 onoff)
 {
+	struct dvbsky_state *state = d_to_priv(d);
 	int ret;
-	static u8 obuf_pre[3] = { 0x37, 0, 0 };
-	static u8 obuf_post[3] = { 0x36, 3, 0 };
+	u8 obuf_pre[3] = { 0x37, 0, 0 };
+	u8 obuf_post[3] = { 0x36, 3, 0 };
 
-	mutex_lock(&d->usb_mutex);
-	ret = dvb_usbv2_generic_rw_locked(d, obuf_pre, 3, NULL, 0);
+	mutex_lock(&state->stream_mutex);
+	ret = dvbsky_usb_generic_rw(d, obuf_pre, 3, NULL, 0);
 	if (!ret && onoff) {
 		msleep(20);
-		ret = dvb_usbv2_generic_rw_locked(d, obuf_post, 3, NULL, 0);
+		ret = dvbsky_usb_generic_rw(d, obuf_post, 3, NULL, 0);
 	}
-	mutex_unlock(&d->usb_mutex);
+	mutex_unlock(&state->stream_mutex);
 	return ret;
 }
 
@@ -606,6 +608,8 @@ 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.17.1

[PATCH] Revert "media: dvbsky: use just one mutex for serializing device R/W ops"

[Mauro Carvalho Chehab]

As pointed at:
	https://bugzilla.kernel.org/show_bug.cgi?id=199323

This patch causes a bad effect on RPi. I suspect that the root
cause is at the USB RPi driver, with uses high priority
interrupts instead of normal ones. Anyway, as this patch
is mostly a cleanup, better to revert it.

This reverts commit 7d95fb746c4eece67308f1642a666ea1ebdbd2cc.

Cc: stable@vger.kernel.org # For Kernel 4.18
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
 drivers/media/usb/dvb-usb-v2/dvbsky.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/dvb-usb-v2/dvbsky.c b/drivers/media/usb/dvb-usb-v2/dvbsky.c
index 1aa88d94e57f..e28bd8836751 100644
--- a/drivers/media/usb/dvb-usb-v2/dvbsky.c
+++ b/drivers/media/usb/dvb-usb-v2/dvbsky.c
@@ -31,6 +31,7 @@ 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;
@@ -67,17 +68,18 @@ static int dvbsky_usb_generic_rw(struct dvb_usb_device *d,
 
 static int dvbsky_stream_ctrl(struct dvb_usb_device *d, u8 onoff)
 {
+	struct dvbsky_state *state = d_to_priv(d);
 	int ret;
-	static u8 obuf_pre[3] = { 0x37, 0, 0 };
-	static u8 obuf_post[3] = { 0x36, 3, 0 };
+	u8 obuf_pre[3] = { 0x37, 0, 0 };
+	u8 obuf_post[3] = { 0x36, 3, 0 };
 
-	mutex_lock(&d->usb_mutex);
-	ret = dvb_usbv2_generic_rw_locked(d, obuf_pre, 3, NULL, 0);
+	mutex_lock(&state->stream_mutex);
+	ret = dvbsky_usb_generic_rw(d, obuf_pre, 3, NULL, 0);
 	if (!ret && onoff) {
 		msleep(20);
-		ret = dvb_usbv2_generic_rw_locked(d, obuf_post, 3, NULL, 0);
+		ret = dvbsky_usb_generic_rw(d, obuf_post, 3, NULL, 0);
 	}
-	mutex_unlock(&d->usb_mutex);
+	mutex_unlock(&state->stream_mutex);
 	return ret;
 }
 
@@ -606,6 +608,8 @@ 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.17.1

[PATCH RESEND] Revert "media: dvbsky: use just one mutex for serializing device R/W ops"

[Mauro Carvalho Chehab]

As pointed at:
	https://bugzilla.kernel.org/show_bug.cgi?id=199323

This patch causes a bad effect on RPi. I suspect that the root
cause is at the USB RPi driver, with uses high priority
interrupts instead of normal ones. Anyway, as this patch
is mostly a cleanup, better to revert it.

This reverts commit 7d95fb746c4eece67308f1642a666ea1ebdbd2cc.

Cc: stable@vger.kernel.org # For Kernel 4.18
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---

Re-sending it with the right message ID

 drivers/media/usb/dvb-usb-v2/dvbsky.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/dvb-usb-v2/dvbsky.c b/drivers/media/usb/dvb-usb-v2/dvbsky.c
index 1aa88d94e57f..e28bd8836751 100644
--- a/drivers/media/usb/dvb-usb-v2/dvbsky.c
+++ b/drivers/media/usb/dvb-usb-v2/dvbsky.c
@@ -31,6 +31,7 @@ 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;
@@ -67,17 +68,18 @@ static int dvbsky_usb_generic_rw(struct dvb_usb_device *d,
 
 static int dvbsky_stream_ctrl(struct dvb_usb_device *d, u8 onoff)
 {
+	struct dvbsky_state *state = d_to_priv(d);
 	int ret;
-	static u8 obuf_pre[3] = { 0x37, 0, 0 };
-	static u8 obuf_post[3] = { 0x36, 3, 0 };
+	u8 obuf_pre[3] = { 0x37, 0, 0 };
+	u8 obuf_post[3] = { 0x36, 3, 0 };
 
-	mutex_lock(&d->usb_mutex);
-	ret = dvb_usbv2_generic_rw_locked(d, obuf_pre, 3, NULL, 0);
+	mutex_lock(&state->stream_mutex);
+	ret = dvbsky_usb_generic_rw(d, obuf_pre, 3, NULL, 0);
 	if (!ret && onoff) {
 		msleep(20);
-		ret = dvb_usbv2_generic_rw_locked(d, obuf_post, 3, NULL, 0);
+		ret = dvbsky_usb_generic_rw(d, obuf_post, 3, NULL, 0);
 	}
-	mutex_unlock(&d->usb_mutex);
+	mutex_unlock(&state->stream_mutex);
 	return ret;
 }
 
@@ -606,6 +608,8 @@ 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.17.1

[PATCH RESEND] Revert "media: dvbsky: use just one mutex for serializing device R/W ops"

[Mauro Carvalho Chehab]

As pointed at:
	https://bugzilla.kernel.org/show_bug.cgi?id=199323

This patch causes a bad effect on RPi. I suspect that the root
cause is at the USB RPi driver, with uses high priority
interrupts instead of normal ones. Anyway, as this patch
is mostly a cleanup, better to revert it.

This reverts commit 7d95fb746c4eece67308f1642a666ea1ebdbd2cc.

Cc: stable@vger.kernel.org # For Kernel 4.18
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---

Re-sending it with the right message ID

 drivers/media/usb/dvb-usb-v2/dvbsky.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/dvb-usb-v2/dvbsky.c b/drivers/media/usb/dvb-usb-v2/dvbsky.c
index 1aa88d94e57f..e28bd8836751 100644
--- a/drivers/media/usb/dvb-usb-v2/dvbsky.c
+++ b/drivers/media/usb/dvb-usb-v2/dvbsky.c
@@ -31,6 +31,7 @@ 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;
@@ -67,17 +68,18 @@ static int dvbsky_usb_generic_rw(struct dvb_usb_device *d,
 
 static int dvbsky_stream_ctrl(struct dvb_usb_device *d, u8 onoff)
 {
+	struct dvbsky_state *state = d_to_priv(d);
 	int ret;
-	static u8 obuf_pre[3] = { 0x37, 0, 0 };
-	static u8 obuf_post[3] = { 0x36, 3, 0 };
+	u8 obuf_pre[3] = { 0x37, 0, 0 };
+	u8 obuf_post[3] = { 0x36, 3, 0 };
 
-	mutex_lock(&d->usb_mutex);
-	ret = dvb_usbv2_generic_rw_locked(d, obuf_pre, 3, NULL, 0);
+	mutex_lock(&state->stream_mutex);
+	ret = dvbsky_usb_generic_rw(d, obuf_pre, 3, NULL, 0);
 	if (!ret && onoff) {
 		msleep(20);
-		ret = dvb_usbv2_generic_rw_locked(d, obuf_post, 3, NULL, 0);
+		ret = dvbsky_usb_generic_rw(d, obuf_post, 3, NULL, 0);
 	}
-	mutex_unlock(&d->usb_mutex);
+	mutex_unlock(&state->stream_mutex);
 	return ret;
 }
 
@@ -606,6 +608,8 @@ 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.17.1

Re: [PATCH RESEND] Revert "media: dvbsky: use just one mutex for serializing device R/W ops"

[Oliver Freyermuth]

Dear Mauro,

thanks! Just to clarify, the issue I described in https://bugzilla.kernel.org/show_bug.cgi?id=199323
was on an Intel x86_64 system, with an onboard USB Controller handled by the standard xhci driver,
so this does not affect RPi alone. 

Cheers and thanks,
	Oliver

Am 05.10.18 um 16:26 schrieb Mauro Carvalho Chehab:
> As pointed at:
> 	https://bugzilla.kernel.org/show_bug.cgi?id=199323
> 
> This patch causes a bad effect on RPi. I suspect that the root
> cause is at the USB RPi driver, with uses high priority
> interrupts instead of normal ones. Anyway, as this patch
> is mostly a cleanup, better to revert it.
> 
> This reverts commit 7d95fb746c4eece67308f1642a666ea1ebdbd2cc.
> 
> Cc: stable@vger.kernel.org # For Kernel 4.18
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> ---
> 
> Re-sending it with the right message ID
> 
>  drivers/media/usb/dvb-usb-v2/dvbsky.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/usb/dvb-usb-v2/dvbsky.c b/drivers/media/usb/dvb-usb-v2/dvbsky.c
> index 1aa88d94e57f..e28bd8836751 100644
> --- a/drivers/media/usb/dvb-usb-v2/dvbsky.c
> +++ b/drivers/media/usb/dvb-usb-v2/dvbsky.c
> @@ -31,6 +31,7 @@ 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;
> @@ -67,17 +68,18 @@ static int dvbsky_usb_generic_rw(struct dvb_usb_device *d,
>  
>  static int dvbsky_stream_ctrl(struct dvb_usb_device *d, u8 onoff)
>  {
> +	struct dvbsky_state *state = d_to_priv(d);
>  	int ret;
> -	static u8 obuf_pre[3] = { 0x37, 0, 0 };
> -	static u8 obuf_post[3] = { 0x36, 3, 0 };
> +	u8 obuf_pre[3] = { 0x37, 0, 0 };
> +	u8 obuf_post[3] = { 0x36, 3, 0 };
>  
> -	mutex_lock(&d->usb_mutex);
> -	ret = dvb_usbv2_generic_rw_locked(d, obuf_pre, 3, NULL, 0);
> +	mutex_lock(&state->stream_mutex);
> +	ret = dvbsky_usb_generic_rw(d, obuf_pre, 3, NULL, 0);
>  	if (!ret && onoff) {
>  		msleep(20);
> -		ret = dvb_usbv2_generic_rw_locked(d, obuf_post, 3, NULL, 0);
> +		ret = dvbsky_usb_generic_rw(d, obuf_post, 3, NULL, 0);
>  	}
> -	mutex_unlock(&d->usb_mutex);
> +	mutex_unlock(&state->stream_mutex);
>  	return ret;
>  }
>  
> @@ -606,6 +608,8 @@ static int dvbsky_init(struct dvb_usb_device *d)
>  	if (ret)
>  		return ret;
>  	*/
> +	mutex_init(&state->stream_mutex);
> +
>  	state->last_lock = 0;
>  
>  	return 0;
> 

Re: [PATCH RESEND] Revert "media: dvbsky: use just one mutex for serializing device R/W ops"

[Mauro Carvalho Chehab]

Em Fri, 5 Oct 2018 16:34:28 +0200
Oliver Freyermuth <o.freyermuth@googlemail.com> escreveu:

> Dear Mauro,
> 
> thanks! Just to clarify, the issue I described in https://bugzilla.kernel.org/show_bug.cgi?id=199323
> was on an Intel x86_64 system, with an onboard USB Controller handled by the standard xhci driver,
> so this does not affect RPi alone. 

That's weird... I tested such patch here before applying (and it was
tested by someone else, as far as I know), and it worked fine.

Perhaps the x86 bug is related to some recent changes at the USB
subsystem. Dunno.

Anyway, patch revert applied upstream.

Regards,
Mauro

Re: [PATCH RESEND] Revert "media: dvbsky: use just one mutex for serializing device R/W ops"

[Lars Buerding]

On 05.10.2018 17:30, Mauro Carvalho Chehab wrote:

> Em Fri, 5 Oct 2018 16:34:28 +0200
> Oliver Freyermuth <o.freyermuth@googlemail.com> escreveu:
> 
>> Dear Mauro,
>>
>> thanks! Just to clarify, the issue I described in https://bugzilla.kernel.org/show_bug.cgi?id=199323
>> was on an Intel x86_64 system, with an onboard USB Controller handled by the standard xhci driver,
>> so this does not affect RPi alone. 
> 
> That's weird... I tested such patch here before applying (and it was
> tested by someone else, as far as I know), and it worked fine.
> 
> Perhaps the x86 bug is related to some recent changes at the USB
> subsystem. Dunno.
> 
> Anyway, patch revert applied upstream.

Thanks Mauro,

just to second this is good:
* my TechnoTrend CT2-4400 / 0b48:3014 stopped to work with v4.18 / x86_64
  (Tuner is still working / lock on all channels, but the stick didnt return a transport stream, easy to validate using usbtop)
* applying this revert patch to v4.19-rc7 fixed it


Best,
Lars