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: [email protected] ICQ: 279852819
linux user number: 307850 GPG ID: 5F88735E
GPG FP: 0F01 BAB8 3148 94DB B95D 1FCA 8B63 CA06 5F88 735E
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
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: [email protected] ICQ: 279852819
linux user number: 307850 GPG ID: 5F88735E
GPG FP: 0F01 BAB8 3148 94DB B95D 1FCA 8B63 CA06 5F88 735E
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: [email protected] ICQ: 279852819
linux user number: 307850 GPG ID: 5F88735E
GPG FP: 0F01 BAB8 3148 94DB B95D 1FCA 8B63 CA06 5F88 735E
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...
On Sat, Apr 10, 2004 at 09:28:59PM +0100, [email protected] 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: [email protected] ICQ: 279852819
linux user number: 307850 GPG ID: 5F88735E
GPG FP: 0F01 BAB8 3148 94DB B95D 1FCA 8B63 CA06 5F88 735E
On Sun, Apr 11, 2004 at 10:25:53AM +0200, Marcel Sebek wrote:
> On Sat, Apr 10, 2004 at 09:28:59PM +0100, [email protected] 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
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: [email protected] ICQ: 279852819
linux user number: 307850 GPG ID: 5F88735E
GPG FP: 0F01 BAB8 3148 94DB B95D 1FCA 8B63 CA06 5F88 735E