public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] dvb-core: Handle failures to create devices
@ 2007-05-01 17:12 Simon Arlott
  2007-05-01 17:14 ` Simon Arlott
  2007-05-01 21:21 ` [linux-dvb] " Trent Piepho
  0 siblings, 2 replies; 6+ messages in thread
From: Simon Arlott @ 2007-05-01 17:12 UTC (permalink / raw)
  To: Linux Kernel Mailing List, linux-dvb

dvb-core is not started early enough when device drivers that use dvb are 
compiled in so device_register_device fails (silently) since dvb_class is 
NULL, this runs dvb_init using subsys_initclass instead of module_init.

dvb_register_device will now check the return value of class_device_create.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
 drivers/media/dvb/dvb-core/dvbdev.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/media/dvb/dvb-core/dvbdev.c b/drivers/media/dvb/dvb-core/dvbdev.c
index 7683983..b859a60 100644
--- a/drivers/media/dvb/dvb-core/dvbdev.c
+++ b/drivers/media/dvb/dvb-core/dvbdev.c
@@ -201,6 +201,7 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
 	struct dvb_device *dvbdev;
 	struct file_operations *dvbdevfops;
 
+	struct class_device *clsdev;
 	int id;
 
 	mutex_lock(&dvbdev_register_lock);
@@ -242,9 +243,15 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
 
 	mutex_unlock(&dvbdev_register_lock);
 
-	class_device_create(dvb_class, NULL, MKDEV(DVB_MAJOR, nums2minor(adap->num, type, id)),
+	clsdev = class_device_create(dvb_class, NULL, MKDEV(DVB_MAJOR, nums2minor(adap->num, type, id)),
 			    adap->device, "dvb%d.%s%d", adap->num, dnames[type], id);
 
+	if (IS_ERR(clsdev)) {
+		printk("%s: failed to create device dvb%d.%s%d (%ld)\n", __FUNCTION__,
+			adap->num, dnames[type], id, PTR_ERR(clsdev));
+		return PTR_ERR(clsdev);
+	}
+
 	dprintk("DVB: register adapter%d/%s%d @ minor: %i (0x%02x)\n",
 		adap->num, dnames[type], id, nums2minor(adap->num, type, id),
 		nums2minor(adap->num, type, id));
@@ -431,7 +438,7 @@ static void __exit exit_dvbdev(void)
 	unregister_chrdev_region(MKDEV(DVB_MAJOR, 0), MAX_DVB_MINORS);
 }
 
-module_init(init_dvbdev);
+subsys_initcall(init_dvbdev);
 module_exit(exit_dvbdev);
 
 MODULE_DESCRIPTION("DVB Core Driver");
-- 
1.5.0.1

-- 
Simon Arlott

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] dvb-core: Handle failures to create devices
  2007-05-01 17:12 [PATCH] dvb-core: Handle failures to create devices Simon Arlott
@ 2007-05-01 17:14 ` Simon Arlott
  2007-05-01 21:21 ` [linux-dvb] " Trent Piepho
  1 sibling, 0 replies; 6+ messages in thread
From: Simon Arlott @ 2007-05-01 17:14 UTC (permalink / raw)
  To: Linux Kernel Mailing List, linux-dvb

On 01/05/07 18:12, Simon Arlott wrote:
> dvb_class is NULL, this runs dvb_init using subsys_initclass instead of 
> module_init.

That should of course be subsystem_initCALL not "initclass".

-- 
Simon Arlott

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [linux-dvb] [PATCH] dvb-core: Handle failures to create devices
  2007-05-01 17:12 [PATCH] dvb-core: Handle failures to create devices Simon Arlott
  2007-05-01 17:14 ` Simon Arlott
@ 2007-05-01 21:21 ` Trent Piepho
  2007-05-01 21:33   ` Simon Arlott
  1 sibling, 1 reply; 6+ messages in thread
From: Trent Piepho @ 2007-05-01 21:21 UTC (permalink / raw)
  To: Simon Arlott; +Cc: Linux Kernel Mailing List, Linux DVB

On Tue, 1 May 2007, Simon Arlott wrote:
> dvb-core is not started early enough when device drivers that use dvb are
> compiled in so device_register_device fails (silently) since dvb_class is
> NULL, this runs dvb_init using subsys_initclass instead of module_init.
>
> dvb_register_device will now check the return value of class_device_create.

Good catch.

> +		printk("%s: failed to create device dvb%d.%s%d (%ld)\n", __FUNCTION__,
> +			adap->num, dnames[type], id, PTR_ERR(clsdev));
> +		return PTR_ERR(clsdev);

printk(KERN_ERR "...."


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [linux-dvb] [PATCH] dvb-core: Handle failures to create devices
  2007-05-01 21:21 ` [linux-dvb] " Trent Piepho
@ 2007-05-01 21:33   ` Simon Arlott
  2007-05-06 22:50     ` Trent Piepho
  0 siblings, 1 reply; 6+ messages in thread
From: Simon Arlott @ 2007-05-01 21:33 UTC (permalink / raw)
  To: Linux Kernel Mailing List, Linux DVB; +Cc: Trent Piepho

dvb-core is not started early enough when device drivers that use dvb are 
compiled in so device_register_device fails (silently) since dvb_class is 
NULL, this runs dvb_init using subsys_initcall instead of module_init.

dvb_register_device will now check the return value of class_device_create.
All the printks had missing level prefixes so I've fixed these too.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
On 01/05/07 22:21, Trent Piepho wrote:
> On Tue, 1 May 2007, Simon Arlott wrote:
>> dvb-core is not started early enough when device drivers that use dvb are
>> compiled in so device_register_device fails (silently) since dvb_class is
>> NULL, this runs dvb_init using subsys_initclass instead of module_init.
>>
>> dvb_register_device will now check the return value of class_device_create.
> 
> Good catch.

Thanks, it's only been 11 months since I first pointed it out.

>> +		printk("%s: failed to create device dvb%d.%s%d (%ld)\n", __FUNCTION__,
>> +			adap->num, dnames[type], id, PTR_ERR(clsdev));
>> +		return PTR_ERR(clsdev);
> 
> printk(KERN_ERR "...."

I copied another printk from that file without thinking.

 drivers/media/dvb/dvb-core/dvbdev.c |   21 ++++++++++++++-------
 1 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/media/dvb/dvb-core/dvbdev.c b/drivers/media/dvb/dvb-core/dvbdev.c
index 7683983..297da62 100644
--- a/drivers/media/dvb/dvb-core/dvbdev.c
+++ b/drivers/media/dvb/dvb-core/dvbdev.c
@@ -201,6 +201,7 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
 	struct dvb_device *dvbdev;
 	struct file_operations *dvbdevfops;
 
+	struct class_device *clsdev;
 	int id;
 
 	mutex_lock(&dvbdev_register_lock);
@@ -208,7 +209,7 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
 	if ((id = dvbdev_get_free_id (adap, type)) < 0){
 		mutex_unlock(&dvbdev_register_lock);
 		*pdvbdev = NULL;
-		printk ("%s: could get find free device id...\n", __FUNCTION__);
+		printk(KERN_ERR "%s: could get find free device id...\n", __FUNCTION__);
 		return -ENFILE;
 	}
 
@@ -242,10 +243,16 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
 
 	mutex_unlock(&dvbdev_register_lock);
 
-	class_device_create(dvb_class, NULL, MKDEV(DVB_MAJOR, nums2minor(adap->num, type, id)),
+	clsdev = class_device_create(dvb_class, NULL, MKDEV(DVB_MAJOR, nums2minor(adap->num, type, id)),
 			    adap->device, "dvb%d.%s%d", adap->num, dnames[type], id);
 
-	dprintk("DVB: register adapter%d/%s%d @ minor: %i (0x%02x)\n",
+	if (IS_ERR(clsdev)) {
+		printk(KERN_ERR "%s: failed to create device dvb%d.%s%d (%ld)\n", __FUNCTION__,
+			adap->num, dnames[type], id, PTR_ERR(clsdev));
+		return PTR_ERR(clsdev);
+	}
+
+	dprintk(KERN_DEBUG "DVB: register adapter%d/%s%d @ minor: %i (0x%02x)\n",
 		adap->num, dnames[type], id, nums2minor(adap->num, type, id),
 		nums2minor(adap->num, type, id));
 
@@ -304,7 +311,7 @@ int dvb_register_adapter(struct dvb_adapter *adap, const char *name, struct modu
 	memset (adap, 0, sizeof(struct dvb_adapter));
 	INIT_LIST_HEAD (&adap->device_list);
 
-	printk ("DVB: registering new adapter (%s).\n", name);
+	printk(KERN_INFO "DVB: registering new adapter (%s).\n", name);
 
 	adap->num = num;
 	adap->name = name;
@@ -400,13 +407,13 @@ static int __init init_dvbdev(void)
 	dev_t dev = MKDEV(DVB_MAJOR, 0);
 
 	if ((retval = register_chrdev_region(dev, MAX_DVB_MINORS, "DVB")) != 0) {
-		printk("dvb-core: unable to get major %d\n", DVB_MAJOR);
+		printk(KERN_ERR "dvb-core: unable to get major %d\n", DVB_MAJOR);
 		return retval;
 	}
 
 	cdev_init(&dvb_device_cdev, &dvb_device_fops);
 	if ((retval = cdev_add(&dvb_device_cdev, dev, MAX_DVB_MINORS)) != 0) {
-		printk("dvb-core: unable to get major %d\n", DVB_MAJOR);
+		printk(KERN_ERR "dvb-core: unable to get major %d\n", DVB_MAJOR);
 		goto error;
 	}
 
@@ -431,7 +438,7 @@ static void __exit exit_dvbdev(void)
 	unregister_chrdev_region(MKDEV(DVB_MAJOR, 0), MAX_DVB_MINORS);
 }
 
-module_init(init_dvbdev);
+subsys_initcall(init_dvbdev);
 module_exit(exit_dvbdev);
 
 MODULE_DESCRIPTION("DVB Core Driver");
-- 
1.5.0.1

-- 
Simon Arlott
09F911029D74E35BD84156C5635688C0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [linux-dvb] [PATCH] dvb-core: Handle failures to create devices
  2007-05-01 21:33   ` Simon Arlott
@ 2007-05-06 22:50     ` Trent Piepho
  2007-05-06 22:58       ` Simon Arlott
  0 siblings, 1 reply; 6+ messages in thread
From: Trent Piepho @ 2007-05-06 22:50 UTC (permalink / raw)
  To: Simon Arlott; +Cc: Linux Kernel Mailing List, Linux DVB

I've tested this and can confirm it works.  dvb_class will be set too late
without the change to subsys_initcall.

On Tue, 1 May 2007, Simon Arlott wrote:
> dvb-core is not started early enough when device drivers that use dvb are
> compiled in so device_register_device fails (silently) since dvb_class is

                ^dvb_register_device

> NULL, this runs dvb_init using subsys_initcall instead of module_init.
>
> dvb_register_device will now check the return value of class_device_create.
> All the printks had missing level prefixes so I've fixed these too.

Probably better to make this a separate patch, since it's not related.

> -		printk ("%s: could get find free device id...\n", __FUNCTION__);
> +		printk(KERN_ERR "%s: could get find free device id...\n", __FUNCTION__);

"couldn't find free device id"

If it's ok with you, I'll import your patch as two seperate patches with
the spelling errors fixed?


> +	dprintk(KERN_DEBUG "DVB: register adapter%d/%s%d @ minor: %i (0x%02x)\n",
>  		adap->num, dnames[type], id, nums2minor(adap->num, type, id),
>  		nums2minor(adap->num, type, id));

The dvb-core dvbdev_debug parameter does nothing but turn on this one
single dprintk.  I'm tempted to just delete it.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [linux-dvb] [PATCH] dvb-core: Handle failures to create devices
  2007-05-06 22:50     ` Trent Piepho
@ 2007-05-06 22:58       ` Simon Arlott
  0 siblings, 0 replies; 6+ messages in thread
From: Simon Arlott @ 2007-05-06 22:58 UTC (permalink / raw)
  To: Trent Piepho; +Cc: Linux Kernel Mailing List, Linux DVB

On 06/05/07 23:50, Trent Piepho wrote:
> I've tested this and can confirm it works.  dvb_class will be set too late
> without the change to subsys_initcall.
> 
> On Tue, 1 May 2007, Simon Arlott wrote:
>> dvb-core is not started early enough when device drivers that use dvb are
>> compiled in so device_register_device fails (silently) since dvb_class is
> 
>                 ^dvb_register_device
> 
>> NULL, this runs dvb_init using subsys_initcall instead of module_init.
>>
>> dvb_register_device will now check the return value of class_device_create.
>> All the printks had missing level prefixes so I've fixed these too.
> 
> Probably better to make this a separate patch, since it's not related.
> 
>> -		printk ("%s: could get find free device id...\n", __FUNCTION__);
>> +		printk(KERN_ERR "%s: could get find free device id...\n", __FUNCTION__);
> 
> "couldn't find free device id"
> 
> If it's ok with you, I'll import your patch as two seperate patches with
> the spelling errors fixed?

That's fine with me.
 
>> +	dprintk(KERN_DEBUG "DVB: register adapter%d/%s%d @ minor: %i (0x%02x)\n",
>>  		adap->num, dnames[type], id, nums2minor(adap->num, type, id),
>>  		nums2minor(adap->num, type, id));
> 
> The dvb-core dvbdev_debug parameter does nothing but turn on this one
> single dprintk.  I'm tempted to just delete it.

Just always output that information, or remove it? (I have it enabled, 
although I'm not sure if there's any point).

-- 
Simon Arlott

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2007-05-06 22:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-05-01 17:12 [PATCH] dvb-core: Handle failures to create devices Simon Arlott
2007-05-01 17:14 ` Simon Arlott
2007-05-01 21:21 ` [linux-dvb] " Trent Piepho
2007-05-01 21:33   ` Simon Arlott
2007-05-06 22:50     ` Trent Piepho
2007-05-06 22:58       ` Simon Arlott

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox