[PATCH 2.6] Class support for ppdev.c

[PATCH 2.6] Class support for ppdev.c

[Marcel Sebek]

This patch adds class support to ppdev.c.

The module compiles and loads ok.


diff -urN linux-2.6/drivers/char/ppdev.c linux-2.6-new/drivers/char/ppdev.c
--- linux-2.6/drivers/char/ppdev.c	2003-12-31 18:51:23.000000000 +0100
+++ linux-2.6-new/drivers/char/ppdev.c	2004-04-10 14:28:54.000000000 +0200
@@ -59,6 +59,7 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/sched.h>
+#include <linux/device.h>
 #include <linux/devfs_fs_kernel.h>
 #include <linux/ioctl.h>
 #include <linux/parport.h>
@@ -750,31 +751,54 @@
 	.release	= pp_release,
 };
 
+static struct class_simple *ppdev_class;
+
 static int __init ppdev_init (void)
 {
-	int i;
+	int i, err = 0;
 
 	if (register_chrdev (PP_MAJOR, CHRDEV, &pp_fops)) {
 		printk (KERN_WARNING CHRDEV ": unable to get major %d\n",
 			PP_MAJOR);
 		return -EIO;
 	}
+	ppdev_class = class_simple_create(THIS_MODULE, CHRDEV);
+	if (IS_ERR(ppdev_class)) {
+		err = PTR_ERR(ppdev_class);
+		goto out_chrdev;
+	}
 	devfs_mk_dir("parports");
 	for (i = 0; i < PARPORT_MAX; i++) {
-		devfs_mk_cdev(MKDEV(PP_MAJOR, i),
+		class_simple_device_add(ppdev_class, MKDEV(PP_MAJOR, i), 
+				NULL, "parport%d", i);
+		err = devfs_mk_cdev(MKDEV(PP_MAJOR, i),
 				S_IFCHR | S_IRUGO | S_IWUGO, "parports/%d", i);
+		if (err)
+			goto out_class;
 	}
 
 	printk (KERN_INFO PP_VERSION "\n");
-	return 0;
+	goto out;
+
+out_class:
+	for (i = 0; i < PARPORT_MAX; i++)
+		class_simple_device_remove(MKDEV(PP_MAJOR, i));
+	class_simple_destroy(ppdev_class);
+out_chrdev:
+	unregister_chrdev(PP_MAJOR, CHRDEV);
+out:
+	return err;
 }
 
 static void __exit ppdev_cleanup (void)
 {
 	int i;
 	/* Clean up all parport stuff */
-	for (i = 0; i < PARPORT_MAX; i++)
+	for (i = 0; i < PARPORT_MAX; i++) {
+		class_simple_device_remove(MKDEV(PP_MAJOR, i));
 		devfs_remove("parports/%d", i);
+	}
+	class_simple_destroy(ppdev_class);
 	devfs_remove("parports");
 	unregister_chrdev (PP_MAJOR, CHRDEV);
 }


-- 
Marcel Sebek
jabber: sebek@jabber.cz                     ICQ: 279852819
linux user number: 307850                 GPG ID: 5F88735E
GPG FP: 0F01 BAB8 3148 94DB B95D  1FCA 8B63 CA06 5F88 735E

Re: [PATCH 2.6] Class support for ppdev.c

[Greg KH]

On Sat, Apr 10, 2004 at 03:51:15PM +0200, Marcel Sebek wrote:
> This patch adds class support to ppdev.c.
> 
> The module compiles and loads ok.

Looks good, but we really shoulnd't be duplicating the devfs
functionality here.  We should only show the devices that the system
really has present, instead of always showing all of the devices.  Care
to fix your patch up to implement this instead?

thanks,

greg k-h

Re: [PATCH 2.6] Class support for ppdev.c

[Marcel Sebek]

On Sat, Apr 10, 2004 at 10:01:48AM -0700, Greg KH wrote:
> On Sat, Apr 10, 2004 at 03:51:15PM +0200, Marcel Sebek wrote:
> > This patch adds class support to ppdev.c.
> > 
> > The module compiles and loads ok.
> 
> Looks good, but we really shoulnd't be duplicating the devfs
> functionality here.  We should only show the devices that the system
> really has present, instead of always showing all of the devices.  Care
> to fix your patch up to implement this instead?
> 

Ok. Here is an updated patch.


diff -urN linux-2.6/drivers/char/ppdev.c linux-2.6-new/drivers/char/ppdev.c
--- linux-2.6/drivers/char/ppdev.c	2004-04-10 19:38:00.000000000 +0200
+++ linux-2.6-new/drivers/char/ppdev.c	2004-04-10 19:57:15.000000000 +0200
@@ -59,6 +59,7 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/sched.h>
+#include <linux/device.h>
 #include <linux/devfs_fs_kernel.h>
 #include <linux/ioctl.h>
 #include <linux/parport.h>
@@ -750,31 +751,55 @@
 	.release	= pp_release,
 };
 
+static struct class_simple *ppdev_class;
+
 static int __init ppdev_init (void)
 {
-	int i;
+	int i, err = 0;
 
 	if (register_chrdev (PP_MAJOR, CHRDEV, &pp_fops)) {
 		printk (KERN_WARNING CHRDEV ": unable to get major %d\n",
 			PP_MAJOR);
 		return -EIO;
 	}
+	ppdev_class = class_simple_create(THIS_MODULE, CHRDEV);
+	if (IS_ERR(ppdev_class)) {
+		err = PTR_ERR(ppdev_class);
+		goto out_chrdev;
+	}
 	devfs_mk_dir("parports");
 	for (i = 0; i < PARPORT_MAX; i++) {
-		devfs_mk_cdev(MKDEV(PP_MAJOR, i),
+		if (parport_find_number(i))
+			class_simple_device_add(ppdev_class, MKDEV(PP_MAJOR, i),
+				NULL, "parport%d", i);
+		err = devfs_mk_cdev(MKDEV(PP_MAJOR, i),
 				S_IFCHR | S_IRUGO | S_IWUGO, "parports/%d", i);
+		if (err)
+			goto out_class;
 	}
 
 	printk (KERN_INFO PP_VERSION "\n");
-	return 0;
+	goto out;
+
+out_class:
+	for (i = 0; i < PARPORT_MAX; i++)
+		class_simple_device_remove(MKDEV(PP_MAJOR, i));
+	class_simple_destroy(ppdev_class);
+out_chrdev:
+	unregister_chrdev(PP_MAJOR, CHRDEV);
+out:
+	return err;
 }
 
 static void __exit ppdev_cleanup (void)
 {
 	int i;
 	/* Clean up all parport stuff */
-	for (i = 0; i < PARPORT_MAX; i++)
+	for (i = 0; i < PARPORT_MAX; i++) {
+		class_simple_device_remove(MKDEV(PP_MAJOR, i));
 		devfs_remove("parports/%d", i);
+	}
+	class_simple_destroy(ppdev_class);
 	devfs_remove("parports");
 	unregister_chrdev (PP_MAJOR, CHRDEV);
 }

-- 
Marcel Sebek
jabber: sebek@jabber.cz                     ICQ: 279852819
linux user number: 307850                 GPG ID: 5F88735E
GPG FP: 0F01 BAB8 3148 94DB B95D  1FCA 8B63 CA06 5F88 735E

Re: [PATCH 2.6] Class support for ppdev.c

[Marcel Sebek]

On Sat, Apr 10, 2004 at 08:06:36PM +0200, Marcel Sebek wrote:
> On Sat, Apr 10, 2004 at 10:01:48AM -0700, Greg KH wrote:
> > On Sat, Apr 10, 2004 at 03:51:15PM +0200, Marcel Sebek wrote:
> > > This patch adds class support to ppdev.c.
> > > 
> > > The module compiles and loads ok.
> > 
> > Looks good, but we really shoulnd't be duplicating the devfs
> > functionality here.  We should only show the devices that the system
> > really has present, instead of always showing all of the devices.  Care
> > to fix your patch up to implement this instead?
> > 
> 
> Ok. Here is an updated patch.

And new updated patch. partport_find_number() needs to decrement refcount
by parport_put_port().


diff -urN linux-2.6/drivers/char/ppdev.c linux-2.6-new/drivers/char/ppdev.c
--- linux-2.6/drivers/char/ppdev.c	2004-04-10 21:38:17.000000000 +0200
+++ linux-2.6-new/drivers/char/ppdev.c	2004-04-10 21:40:31.000000000 +0200
@@ -59,6 +59,7 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/sched.h>
+#include <linux/device.h>
 #include <linux/devfs_fs_kernel.h>
 #include <linux/ioctl.h>
 #include <linux/parport.h>
@@ -750,31 +751,58 @@
 	.release	= pp_release,
 };
 
+static struct class_simple *ppdev_class;
+
 static int __init ppdev_init (void)
 {
-	int i;
+	int i, err = 0;
+	struct parport *port;
 
 	if (register_chrdev (PP_MAJOR, CHRDEV, &pp_fops)) {
 		printk (KERN_WARNING CHRDEV ": unable to get major %d\n",
 			PP_MAJOR);
 		return -EIO;
 	}
+	ppdev_class = class_simple_create(THIS_MODULE, CHRDEV);
+	if (IS_ERR(ppdev_class)) {
+		err = PTR_ERR(ppdev_class);
+		goto out_chrdev;
+	}
 	devfs_mk_dir("parports");
 	for (i = 0; i < PARPORT_MAX; i++) {
-		devfs_mk_cdev(MKDEV(PP_MAJOR, i),
+		if ((port = parport_find_number(i))) {
+			class_simple_device_add(ppdev_class, MKDEV(PP_MAJOR, i),
+				NULL, "parport%d", i);
+			parport_put_port(port);
+		}
+		err = devfs_mk_cdev(MKDEV(PP_MAJOR, i),
 				S_IFCHR | S_IRUGO | S_IWUGO, "parports/%d", i);
+		if (err)
+			goto out_class;
 	}
 
 	printk (KERN_INFO PP_VERSION "\n");
-	return 0;
+	goto out;
+
+out_class:
+	for (i = 0; i < PARPORT_MAX; i++)
+		class_simple_device_remove(MKDEV(PP_MAJOR, i));
+	class_simple_destroy(ppdev_class);
+out_chrdev:
+	unregister_chrdev(PP_MAJOR, CHRDEV);
+out:
+	return err;
 }
 
 static void __exit ppdev_cleanup (void)
 {
 	int i;
 	/* Clean up all parport stuff */
-	for (i = 0; i < PARPORT_MAX; i++)
+	for (i = 0; i < PARPORT_MAX; i++) {
+		class_simple_device_remove(MKDEV(PP_MAJOR, i));
 		devfs_remove("parports/%d", i);
+	}
+	class_simple_destroy(ppdev_class);
 	devfs_remove("parports");
 	unregister_chrdev (PP_MAJOR, CHRDEV);
 }

-- 
Marcel Sebek
jabber: sebek@jabber.cz                     ICQ: 279852819
linux user number: 307850                 GPG ID: 5F88735E
GPG FP: 0F01 BAB8 3148 94DB B95D  1FCA 8B63 CA06 5F88 735E

Re: [PATCH 2.6] Class support for ppdev.c

[viro]

On Sat, Apr 10, 2004 at 09:46:01PM +0200, Marcel Sebek wrote:
> And new updated patch. partport_find_number() needs to decrement refcount
> by parport_put_port().

And it's still broken.  New parports can appear at any point - hell, we even
have USB->parport converters.  IOW, if you want to do something useful here -
use ->attach()/->detach() of parport_driver.

>  	for (i = 0; i < PARPORT_MAX; i++) {
> -		devfs_mk_cdev(MKDEV(PP_MAJOR, i),
> +		if ((port = parport_find_number(i))) {
> +			class_simple_device_add(ppdev_class, MKDEV(PP_MAJOR, i),
> +				NULL, "parport%d", i);
> +			parport_put_port(port);
> +		}
> +		err = devfs_mk_cdev(MKDEV(PP_MAJOR, i),
>  				S_IFCHR | S_IRUGO | S_IWUGO, "parports/%d", i);
> +		if (err)
> +			goto out_class;
>  	}
>  
>  	printk (KERN_INFO PP_VERSION "\n");
> -	return 0;
> +	goto out;
> +
> +out_class:
> +	for (i = 0; i < PARPORT_MAX; i++)
> +		class_simple_device_remove(MKDEV(PP_MAJOR, i));


*raised brows*

So you are freeing the stuff you've never allocated?  Cute...

Re: [PATCH 2.6] Class support for ppdev.c

[Marcel Sebek]

On Sat, Apr 10, 2004 at 09:28:59PM +0100, viro@parcelfarce.linux.theplanet.co.uk wrote:
> On Sat, Apr 10, 2004 at 09:46:01PM +0200, Marcel Sebek wrote:
> > And new updated patch. partport_find_number() needs to decrement refcount
> > by parport_put_port().
> 
> And it's still broken.  New parports can appear at any point - hell, we even
> have USB->parport converters.  IOW, if you want to do something useful here -
> use ->attach()/->detach() of parport_driver.
> 
Ok. Here's new one. It uses attach()/detach() callbacks for creating
udev and devfs files.


diff -urN linux-2.6/drivers/char/ppdev.c linux-2.6-new/drivers/char/ppdev.c
--- linux-2.6/drivers/char/ppdev.c	2004-04-11 10:11:32.000000000 +0200
+++ linux-2.6-new/drivers/char/ppdev.c	2004-04-11 10:11:57.000000000 +0200
@@ -59,6 +59,7 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/sched.h>
+#include <linux/device.h>
 #include <linux/devfs_fs_kernel.h>
 #include <linux/ioctl.h>
 #include <linux/parport.h>
@@ -739,6 +740,8 @@
 	return mask;
 }
 
+static struct class_simple *ppdev_class;
+
 static struct file_operations pp_fops = {
 	.owner		= THIS_MODULE,
 	.llseek		= no_llseek,
@@ -750,32 +753,64 @@
 	.release	= pp_release,
 };
 
+static void pp_attach(struct parport *port)
+{
+	class_simple_device_add(ppdev_class, MKDEV(PP_MAJOR, port->number),
+			NULL, "parport%d", port->number);
+	devfs_mk_cdev(MKDEV(PP_MAJOR, port->number), S_IFCHR | S_IRUGO | S_IWUGO,
+			"parports/%d", port->number);
+}
+
+static void pp_detach(struct parport *port)
+{
+	devfs_remove("parports/%d", port->number);
+	class_simple_device_remove(MKDEV(PP_MAJOR, port->number));
+}
+
+static struct parport_driver pp_driver = {
+	.name		= CHRDEV,
+	.attach		= pp_attach,
+	.detach		= pp_detach,
+};
+
 static int __init ppdev_init (void)
 {
-	int i;
+	int err = 0;
 
 	if (register_chrdev (PP_MAJOR, CHRDEV, &pp_fops)) {
 		printk (KERN_WARNING CHRDEV ": unable to get major %d\n",
 			PP_MAJOR);
 		return -EIO;
 	}
+	ppdev_class = class_simple_create(THIS_MODULE, CHRDEV);
+	if (IS_ERR(ppdev_class)) {
+		err = PTR_ERR(ppdev_class);
+		goto out_chrdev;
+	}
 	devfs_mk_dir("parports");
-	for (i = 0; i < PARPORT_MAX; i++) {
-		devfs_mk_cdev(MKDEV(PP_MAJOR, i),
-				S_IFCHR | S_IRUGO | S_IWUGO, "parports/%d", i);
+	if (parport_register_driver(&pp_driver)) {
+		printk (KERN_WARNING CHRDEV ": unable to register with parport\n");
+		goto out_class;
 	}
 
 	printk (KERN_INFO PP_VERSION "\n");
-	return 0;
+	goto out;
+
+out_class:
+	class_simple_destroy(ppdev_class);
+	devfs_remove("parports");
+out_chrdev:
+	unregister_chrdev(PP_MAJOR, CHRDEV);
+out:
+	return err;
 }
 
 static void __exit ppdev_cleanup (void)
 {
-	int i;
 	/* Clean up all parport stuff */
-	for (i = 0; i < PARPORT_MAX; i++)
-		devfs_remove("parports/%d", i);
+	parport_unregister_driver(&pp_driver);
 	devfs_remove("parports");
+	class_simple_destroy(ppdev_class);
 	unregister_chrdev (PP_MAJOR, CHRDEV);
 }
 

-- 
Marcel Sebek
jabber: sebek@jabber.cz                     ICQ: 279852819
linux user number: 307850                 GPG ID: 5F88735E
GPG FP: 0F01 BAB8 3148 94DB B95D  1FCA 8B63 CA06 5F88 735E

Re: [PATCH 2.6] Class support for ppdev.c

[Greg KH]

On Sun, Apr 11, 2004 at 10:25:53AM +0200, Marcel Sebek wrote:
> On Sat, Apr 10, 2004 at 09:28:59PM +0100, viro@parcelfarce.linux.theplanet.co.uk wrote:
> > On Sat, Apr 10, 2004 at 09:46:01PM +0200, Marcel Sebek wrote:
> > > And new updated patch. partport_find_number() needs to decrement refcount
> > > by parport_put_port().
> > 
> > And it's still broken.  New parports can appear at any point - hell, we even
> > have USB->parport converters.  IOW, if you want to do something useful here -
> > use ->attach()/->detach() of parport_driver.
> > 
> Ok. Here's new one. It uses attach()/detach() callbacks for creating
> udev and devfs files.

Looks much better, thanks.  But please don't modify the current devfs
usage, those users will generally not like that.  Can you redo this
patch to only add the sysfs changes?

thanks,

greg k-h

Re: [PATCH 2.6] Class support for ppdev.c

[Marcel Sebek]

On Mon, Apr 12, 2004 at 11:51:21AM -0700, Greg KH wrote:
> On Sun, Apr 11, 2004 at 10:25:53AM +0200, Marcel Sebek wrote:
> Looks much better, thanks.  But please don't modify the current devfs
> usage, those users will generally not like that.  Can you redo this
> patch to only add the sysfs changes?
> 

diff -urN linux-2.6/drivers/char/ppdev.c linux-2.6-new/drivers/char/ppdev.c
--- linux-2.6/drivers/char/ppdev.c	2004-04-13 19:17:12.000000000 +0200
+++ linux-2.6-new/drivers/char/ppdev.c	2004-04-13 19:25:50.000000000 +0200
@@ -59,6 +59,7 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/sched.h>
+#include <linux/device.h>
 #include <linux/devfs_fs_kernel.h>
 #include <linux/ioctl.h>
 #include <linux/parport.h>
@@ -739,6 +740,8 @@
 	return mask;
 }
 
+static struct class_simple *ppdev_class;
+
 static struct file_operations pp_fops = {
 	.owner		= THIS_MODULE,
 	.llseek		= no_llseek,
@@ -750,23 +753,59 @@
 	.release	= pp_release,
 };
 
+static void pp_attach(struct parport *port)
+{
+	class_simple_device_add(ppdev_class, MKDEV(PP_MAJOR, port->number),
+			NULL, "parport%d", port->number);
+}
+
+static void pp_detach(struct parport *port)
+{
+	class_simple_device_remove(MKDEV(PP_MAJOR, port->number));
+}
+
+static struct parport_driver pp_driver = {
+	.name		= CHRDEV,
+	.attach		= pp_attach,
+	.detach		= pp_detach,
+};
+
 static int __init ppdev_init (void)
 {
-	int i;
+	int i, err = 0;
 
 	if (register_chrdev (PP_MAJOR, CHRDEV, &pp_fops)) {
 		printk (KERN_WARNING CHRDEV ": unable to get major %d\n",
 			PP_MAJOR);
 		return -EIO;
 	}
+	ppdev_class = class_simple_create(THIS_MODULE, CHRDEV);
+	if (IS_ERR(ppdev_class)) {
+		err = PTR_ERR(ppdev_class);
+		goto out_chrdev;
+	}
 	devfs_mk_dir("parports");
 	for (i = 0; i < PARPORT_MAX; i++) {
 		devfs_mk_cdev(MKDEV(PP_MAJOR, i),
 				S_IFCHR | S_IRUGO | S_IWUGO, "parports/%d", i);
 	}
+	if (parport_register_driver(&pp_driver)) {
+		printk (KERN_WARNING CHRDEV ": unable to register with parport\n");
+		goto out_class;
+	}
 
 	printk (KERN_INFO PP_VERSION "\n");
-	return 0;
+	goto out;
+
+out_class:
+	for (i = 0; i < PARPORT_MAX; i++)
+		devfs_remove("parports/%d", i);
+	devfs_remove("parports");
+	class_simple_destroy(ppdev_class);
+out_chrdev:
+	unregister_chrdev(PP_MAJOR, CHRDEV);
+out:
+	return err;
 }
 
 static void __exit ppdev_cleanup (void)
@@ -775,7 +814,9 @@
 	/* Clean up all parport stuff */
 	for (i = 0; i < PARPORT_MAX; i++)
 		devfs_remove("parports/%d", i);
+	parport_unregister_driver(&pp_driver);
 	devfs_remove("parports");
+	class_simple_destroy(ppdev_class);
 	unregister_chrdev (PP_MAJOR, CHRDEV);
 }

-- 
Marcel Sebek
jabber: sebek@jabber.cz                     ICQ: 279852819
linux user number: 307850                 GPG ID: 5F88735E
GPG FP: 0F01 BAB8 3148 94DB B95D  1FCA 8B63 CA06 5F88 735E

Re: [PATCH 2.6] Class support for ppdev.c

[Greg KH]

On Tue, Apr 13, 2004 at 07:29:24PM +0200, Marcel Sebek wrote:
> On Mon, Apr 12, 2004 at 11:51:21AM -0700, Greg KH wrote:
> > On Sun, Apr 11, 2004 at 10:25:53AM +0200, Marcel Sebek wrote:
> > Looks much better, thanks.  But please don't modify the current devfs
> > usage, those users will generally not like that.  Can you redo this
> > patch to only add the sysfs changes?
> > 

Looks good, applied, thanks.

greg k-h