diff mbox

[6/6] EM28xx - don't sleep on disconnect

Message ID 4E4F9C77.6010008@yahoo.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Chris Rankin Aug. 20, 2011, 11:37 a.m. UTC 
  The DVB framework will try to power-down an adapter that no-one is using any 
more, but this assumes that the adapter is still connected to the machine. 
That's not always true for a USB adapter, so disable the sleep operations when 
the adapter has been physically unplugged.

This prevents I2C write failures with error -19 from appearing occasionally in 
the dmesg log.

Signed-off-by: Chris Rankin <rankincj@yahoo.com>

Comments

Mauro Carvalho Chehab Aug. 20, 2011, 12:17 p.m. UTC | #1 
Em 20-08-2011 04:37, Chris Rankin escreveu:
> +
> +		if (dev->state & DEV_DISCONNECTED) {
> +			/* We cannot tell the device to sleep
> +			 * once it has been unplugged. */
> +			prevent_sleep(&dvb->fe[0]->ops);
> +			prevent_sleep(&dvb->fe[1]->ops);

This will cause an OOPS if dvb->fe[n] == NULL.

> +		}
> +

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chris Rankin Aug. 20, 2011, 1:46 p.m. UTC | #2 
--- On Sat, 20/8/11, Mauro Carvalho Chehab <mchehab@redhat.com> wrot
> 
> This will cause an OOPS if dvb->fe[n] == NULL.
> 

OK, that's trivially fixable. I'll send you an updated patch. Is it safe to assume that dvb->fe[0] at least will always be non-NULL?

Cheers,
Chris

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mauro Carvalho Chehab Aug. 20, 2011, 2:20 p.m. UTC | #3 
Em 20-08-2011 06:46, Chris Rankin escreveu:
> --- On Sat, 20/8/11, Mauro Carvalho Chehab <mchehab@redhat.com> wrot
>>
>> This will cause an OOPS if dvb->fe[n] == NULL.
>>
> 
> OK, that's trivially fixable. I'll send you an updated patch. Is it safe to assume that dvb->fe[0] at least will always be non-NULL?

No, it isn't. The dvb initialization may fail or the device can be analog only,
but somebody might manually load em28xx-dvb (or two devices were plugged).

Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

--- linux-3.0/drivers/media/common/tuners/tda18271-fe.c.orig	2011-08-18 16:55:53.000000000 +0100
+++ linux-3.0/drivers/media/common/tuners/tda18271-fe.c	2011-08-18 23:12:55.000000000 +0100
@@ -1230,7 +1230,7 @@ 
 	return 0;
 }
 
-static struct dvb_tuner_ops tda18271_tuner_ops = {
+static const struct dvb_tuner_ops tda18271_tuner_ops = {
 	.info = {
 		.name = "NXP TDA18271HD",
 		.frequency_min  =  45000000,
--- linux-3.0/drivers/media/dvb/frontends/cxd2820r_core.c.orig	2011-08-18 16:56:02.000000000 +0100
+++ linux-3.0/drivers/media/dvb/frontends/cxd2820r_core.c	2011-08-18 23:14:06.000000000 +0100
@@ -778,7 +778,7 @@ 
 }
 EXPORT_SYMBOL(cxd2820r_get_tuner_i2c_adapter);
 
-static struct dvb_frontend_ops cxd2820r_ops[2];
+static const struct dvb_frontend_ops cxd2820r_ops[2];
 
 struct dvb_frontend *cxd2820r_attach(const struct cxd2820r_config *cfg,
 	struct i2c_adapter *i2c, struct dvb_frontend *fe)
@@ -844,7 +844,7 @@ 
 }
 EXPORT_SYMBOL(cxd2820r_attach);
 
-static struct dvb_frontend_ops cxd2820r_ops[2] = {
+static const struct dvb_frontend_ops cxd2820r_ops[2] = {
 	{
 		/* DVB-T/T2 */
 		.info = {
--- linux-3.0/drivers/media/video/em28xx/em28xx-dvb.c.orig	2011-08-17 08:52:30.000000000 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-dvb.c	2011-08-18 23:17:42.000000000 +0100
@@ -720,6 +720,12 @@ 
 	goto ret;
 }
 
+static inline void prevent_sleep(struct dvb_frontend_ops *ops) {
+	ops->set_voltage = NULL;
+	ops->sleep = NULL;
+	ops->tuner_ops.sleep = NULL;
+}
+
 static int dvb_fini(struct em28xx *dev)
 {
 	if (!dev->board.has_dvb) {
@@ -728,8 +734,17 @@ 
 	}
 
 	if (dev->dvb) {
-		unregister_dvb(dev->dvb);
-		kfree(dev->dvb);
+		struct em28xx_dvb *dvb = dev->dvb;
+
+		if (dev->state & DEV_DISCONNECTED) {
+			/* We cannot tell the device to sleep
+			 * once it has been unplugged. */
+			prevent_sleep(&dvb->fe[0]->ops);
+			prevent_sleep(&dvb->fe[1]->ops);
+		}
+
+		unregister_dvb(dvb);
+		kfree(dvb);
 		dev->dvb = NULL;
 	}