[PATCH 2.6.17-rc2 1/2] return class device pointer from tty_register_device()

[PATCH 2.6.17-rc2 1/2] return class device pointer from tty_register_device()

[Tilman Schmidt]

[-- Attachment #1: Type: text/plain, Size: 3112 bytes --]

Please consider the following patch for inclusion.

From: Hansjoerg Lipp <hjlipp@web.de>

Let tty_register_device() return a pointer to the class device it creates.
This allows registrants to add their own sysfs files under the class
device node.

Signed-off-by: Hansjoerg Lipp <hjlipp@web.de>
Signed-off-by: Tilman Schmidt <tilman@imap.cc>

---

 drivers/char/tty_io.c |   11 +++++++----
 include/linux/tty.h   |    4 +++-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff -urpN -X exclude linux-2.6.17-rc1-mm2.orig/drivers/char/tty_io.c linux-2.6.17-rc1-mm2/drivers/char/tty_io.c
--- linux-2.6.17-rc1-mm2.orig/drivers/char/tty_io.c	2006-04-09 18:30:56.000000000 +0200
+++ linux-2.6.17-rc1-mm2/drivers/char/tty_io.c	2006-04-18 23:17:32.000000000 +0200
@@ -2957,12 +2957,14 @@ static struct class *tty_class;
  *	This field is optional, if there is no known struct device for this
  *	tty device it can be set to NULL safely.
  *
+ * Returns a pointer to the class device (or NULL on error).
+ *
  * This call is required to be made to register an individual tty device if
  * the tty driver's flags have the TTY_DRIVER_NO_DEVFS bit set.  If that
  * bit is not set, this function should not be called.
  */
-void tty_register_device(struct tty_driver *driver, unsigned index,
-			 struct device *device)
+struct class_device *tty_register_device(struct tty_driver *driver,
+					 unsigned index, struct device *device)
 {
 	char name[64];
 	dev_t dev = MKDEV(driver->major, driver->minor_start) + index;
@@ -2970,7 +2972,7 @@ void tty_register_device(struct tty_driv
 	if (index >= driver->num) {
 		printk(KERN_ERR "Attempt to register invalid tty line number "
 		       " (%d).\n", index);
-		return;
+		return NULL;
 	}

 	devfs_mk_cdev(dev, S_IFCHR | S_IRUSR | S_IWUSR,
@@ -2980,7 +2982,8 @@ void tty_register_device(struct tty_driv
 		pty_line_name(driver, index, name);
 	else
 		tty_line_name(driver, index, name);
-	class_device_create(tty_class, NULL, dev, device, "%s", name);
+
+	return class_device_create(tty_class, NULL, dev, device, "%s", name);
 }

 /**
diff -urpN -X exclude linux-2.6.17-rc1-mm2.orig/include/linux/tty.h linux-2.6.17-rc1-mm2/include/linux/tty.h
--- linux-2.6.17-rc1-mm2.orig/include/linux/tty.h	2006-04-04 23:29:14.000000000 +0200
+++ linux-2.6.17-rc1-mm2/include/linux/tty.h	2006-04-18 23:22:47.000000000 +0200
@@ -291,7 +291,9 @@ extern int tty_register_ldisc(int disc,
 extern int tty_unregister_ldisc(int disc);
 extern int tty_register_driver(struct tty_driver *driver);
 extern int tty_unregister_driver(struct tty_driver *driver);
-extern void tty_register_device(struct tty_driver *driver, unsigned index, struct device *dev);
+extern struct class_device *tty_register_device(struct tty_driver *driver,
+						unsigned index,
+						struct device *dev);
 extern void tty_unregister_device(struct tty_driver *driver, unsigned index);
 extern int tty_read_raw_data(struct tty_struct *tty, unsigned char *bufp,
 			     int buflen);

-- 
Tilman Schmidt                          E-Mail: tilman@imap.cc
Bonn, Germany
Imagine a world without hypothetical situations.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 253 bytes --]

Re: [PATCH 2.6.17-rc2 1/2] return class device pointer from tty_register_device()

[Andrew Morton]

Tilman Schmidt <tilman@imap.cc> wrote:
>
>  + * Returns a pointer to the class device (or NULL on error).
>  + *
>    * This call is required to be made to register an individual tty device if
>    * the tty driver's flags have the TTY_DRIVER_NO_DEVFS bit set.  If that
>    * bit is not set, this function should not be called.
>    */
>  -void tty_register_device(struct tty_driver *driver, unsigned index,
>  -			 struct device *device)
>  +struct class_device *tty_register_device(struct tty_driver *driver,
>  +					 unsigned index, struct device *device)
>   {

It would be better to make this return ERR_PTR(-Efoo) on error, rather than
NULL.

That way, tty_register_device() ends with

-       class_device_create(tty_class, NULL, dev, device, "%s", name);
+       return class_device_create(tty_class, NULL, dev, device, "%s", name);
 }

which is neat.

Re: [PATCH 2.6.17-rc2 1/2] return class device pointer from tty_register_device()

[Greg KH]

On Fri, Apr 21, 2006 at 06:14:29PM -0700, Andrew Morton wrote:
> Tilman Schmidt <tilman@imap.cc> wrote:
> >
> >  + * Returns a pointer to the class device (or NULL on error).
> >  + *
> >    * This call is required to be made to register an individual tty device if
> >    * the tty driver's flags have the TTY_DRIVER_NO_DEVFS bit set.  If that
> >    * bit is not set, this function should not be called.
> >    */
> >  -void tty_register_device(struct tty_driver *driver, unsigned index,
> >  -			 struct device *device)
> >  +struct class_device *tty_register_device(struct tty_driver *driver,
> >  +					 unsigned index, struct device *device)
> >   {
> 
> It would be better to make this return ERR_PTR(-Efoo) on error, rather than
> NULL.
> 
> That way, tty_register_device() ends with
> 
> -       class_device_create(tty_class, NULL, dev, device, "%s", name);
> +       return class_device_create(tty_class, NULL, dev, device, "%s", name);
>  }
> 
> which is neat.

I agree, that would be nicer.

thanks,

greg k-h

Re: [PATCH 2.6.17-rc2 1/2] return class device pointer from tty_register_device()

[Tilman Schmidt]

[-- Attachment #1: Type: text/plain, Size: 3434 bytes --]

On 22.04.2006 03:14, Andrew Morton wrote:

> Tilman Schmidt <tilman@imap.cc> wrote:
> 
>> + * Returns a pointer to the class device (or NULL on error).
>> + *
> 
> It would be better to make this return ERR_PTR(-Efoo) on error, rather than
> NULL.

Good point. Here's an accordingly updated version.
I'll follow up with a matching version of part 2.

From: Hansjoerg Lipp <hjlipp@web.de>

Let tty_register_device() return a pointer to the class device it creates.
This allows registrants to add their own sysfs files under the class
device node.

Signed-off-by: Hansjoerg Lipp <hjlipp@web.de>
Signed-off-by: Tilman Schmidt <tilman@imap.cc>

---

 drivers/char/tty_io.c |   11 +++++++----
 include/linux/tty.h   |    4 +++-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff -urpN -X exclude linux-2.6.17-rc1-mm2.orig/drivers/char/tty_io.c linux-2.6.17-rc1-mm2/drivers/char/tty_io.c
--- linux-2.6.17-rc1-mm2.orig/drivers/char/tty_io.c	2006-04-09 18:30:56.000000000 +0200
+++ linux-2.6.17-rc1-mm2/drivers/char/tty_io.c	2006-04-22 04:57:59.000000000 +0200
@@ -2957,12 +2957,14 @@ static struct class *tty_class;
  *	This field is optional, if there is no known struct device for this
  *	tty device it can be set to NULL safely.
  *
+ * Returns a pointer to the class device (or ERR_PTR(-Efoo) on error).
+ *
  * This call is required to be made to register an individual tty device if
  * the tty driver's flags have the TTY_DRIVER_NO_DEVFS bit set.  If that
  * bit is not set, this function should not be called.
  */
-void tty_register_device(struct tty_driver *driver, unsigned index,
-			 struct device *device)
+struct class_device *tty_register_device(struct tty_driver *driver,
+					 unsigned index, struct device *device)
 {
 	char name[64];
 	dev_t dev = MKDEV(driver->major, driver->minor_start) + index;
@@ -2970,7 +2972,7 @@ void tty_register_device(struct tty_driv
 	if (index >= driver->num) {
 		printk(KERN_ERR "Attempt to register invalid tty line number "
 		       " (%d).\n", index);
-		return;
+		return ERR_PTR(-EINVAL);
 	}

 	devfs_mk_cdev(dev, S_IFCHR | S_IRUSR | S_IWUSR,
@@ -2980,7 +2982,8 @@ void tty_register_device(struct tty_driv
 		pty_line_name(driver, index, name);
 	else
 		tty_line_name(driver, index, name);
-	class_device_create(tty_class, NULL, dev, device, "%s", name);
+
+	return class_device_create(tty_class, NULL, dev, device, "%s", name);
 }

 /**
diff -urpN -X exclude linux-2.6.17-rc1-mm2.orig/include/linux/tty.h linux-2.6.17-rc1-mm2/include/linux/tty.h
--- linux-2.6.17-rc1-mm2.orig/include/linux/tty.h	2006-04-04 23:29:14.000000000 +0200
+++ linux-2.6.17-rc1-mm2/include/linux/tty.h	2006-04-22 05:12:34.000000000 +0200
@@ -291,7 +291,9 @@ extern int tty_register_ldisc(int disc,
 extern int tty_unregister_ldisc(int disc);
 extern int tty_register_driver(struct tty_driver *driver);
 extern int tty_unregister_driver(struct tty_driver *driver);
-extern void tty_register_device(struct tty_driver *driver, unsigned index, struct device *dev);
+extern struct class_device *tty_register_device(struct tty_driver *driver,
+						unsigned index,
+						struct device *dev);
 extern void tty_unregister_device(struct tty_driver *driver, unsigned index);
 extern int tty_read_raw_data(struct tty_struct *tty, unsigned char *bufp,
 			     int buflen);


-- 
Tilman Schmidt                          E-Mail: tilman@imap.cc
Bonn, Germany
Imagine a world without hypothetical situations.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 253 bytes --]

Re: [PATCH 2.6.17-rc2 2/2] i4l gigaset: move sysfs entry to tty class device

[Tilman Schmidt]

[-- Attachment #1: Type: text/plain, Size: 7340 bytes --]

[Repost - vger.kernel.org rejected the first attempt with "User unknown"]

From: Hansjoerg Lipp <hjlipp@web.de>

Using the class device pointer returned by tty_register_device() with
part 1 of the patch, attach the Gigaset drivers' "cidmode" sysfs entry
to its tty class device, where it can be found more easily by users
who do not know nor care which USB port the device is attached to.

Signed-off-by: Hansjoerg Lipp <hjlipp@web.de>
Signed-off-by: Tilman Schmidt <tilman@imap.cc>

---

This patch depends on the preceding one titled:
"return class device pointer from tty_register_device()"
Please merge if and when that one has been accepted.

 Documentation/isdn/README.gigaset |    7 ++++---
 drivers/isdn/gigaset/common.c     |   13 +++++++------
 drivers/isdn/gigaset/gigaset.h    |    1 +
 drivers/isdn/gigaset/interface.c  |   10 +++++++++-
 drivers/isdn/gigaset/proc.c       |   21 +++++++++++++--------
 5 files changed, 34 insertions(+), 18 deletions(-)

diff -urpN -X exclude linux-2.6.17-rc1-mm2.orig/Documentation/isdn/README.gigaset linux-2.6.17-rc1-mm2/Documentation/isdn/README.gigaset
--- linux-2.6.17-rc1-mm2.orig/Documentation/isdn/README.gigaset	2006-04-09 18:30:56.000000000 +0200
+++ linux-2.6.17-rc1-mm2/Documentation/isdn/README.gigaset	2006-04-22 16:10:47.000000000 +0200
@@ -124,7 +124,8 @@ GigaSet 307x Device Driver

      You can use some configuration tool of your distribution to configure this
      "modem" or configure pppd/wvdial manually. There are some example ppp
-     configuration files and chat scripts in the gigaset-VERSION/ppp directory.
+     configuration files and chat scripts in the gigaset-VERSION/ppp directory
+     in the driver packages from http://sourceforge.net/projects/gigaset307x/.
      Please note that the USB drivers are not able to change the state of the
      control lines (the M105 driver can be configured to use some undocumented
      control requests, if you really need the control lines, though). This means
@@ -164,8 +165,8 @@ GigaSet 307x Device Driver

      If you want both of these at once, you are out of luck.

-     You can also use /sys/module/<name>/parameters/cidmode for changing
-     the CID mode setting (<name> is usb_gigaset or bas_gigaset).
+     You can also use /sys/class/tty/ttyGxy/cidmode for changing the CID mode
+     setting (ttyGxy is ttyGU0 or ttyGB0).


 3.   Troubleshooting
diff -urpN -X exclude linux-2.6.17-rc1-mm2.orig/drivers/isdn/gigaset/common.c linux-2.6.17-rc1-mm2/drivers/isdn/gigaset/common.c
--- linux-2.6.17-rc1-mm2.orig/drivers/isdn/gigaset/common.c	2006-04-09 18:30:56.000000000 +0200
+++ linux-2.6.17-rc1-mm2/drivers/isdn/gigaset/common.c	2006-04-18 22:23:02.000000000 +0200
@@ -460,6 +460,9 @@ void gigaset_freecs(struct cardstate *cs

 	switch (cs->cs_init) {
 	default:
+		/* clear device sysfs */
+		gigaset_free_dev_sysfs(cs);
+
 		gigaset_if_free(cs);

 		gig_dbg(DEBUG_INIT, "clearing hw");
@@ -699,6 +702,7 @@ struct cardstate *gigaset_initcs(struct
 	cs->open_count = 0;
 	cs->dev = NULL;
 	cs->tty = NULL;
+	cs->class = NULL;
 	cs->cidmode = cidmode != 0;

 	//if(onechannel) { //FIXME
@@ -760,6 +764,9 @@ struct cardstate *gigaset_initcs(struct

 	gigaset_if_init(cs);

+	/* set up device sysfs */
+	gigaset_init_dev_sysfs(cs);
+
 	spin_lock_irqsave(&cs->lock, flags);
 	cs->running = 1;
 	spin_unlock_irqrestore(&cs->lock, flags);
@@ -903,9 +910,6 @@ int gigaset_start(struct cardstate *cs)

 	wait_event(cs->waitqueue, !cs->waiting);

-	/* set up device sysfs */
-	gigaset_init_dev_sysfs(cs);
-
 	mutex_unlock(&cs->mutex);
 	return 1;

@@ -970,9 +974,6 @@ void gigaset_stop(struct cardstate *cs)
 		//FIXME
 	}

-	/* clear device sysfs */
-	gigaset_free_dev_sysfs(cs);
-
 	cleanup_cs(cs);

 exit:
diff -urpN -X exclude linux-2.6.17-rc1-mm2.orig/drivers/isdn/gigaset/gigaset.h linux-2.6.17-rc1-mm2/drivers/isdn/gigaset/gigaset.h
--- linux-2.6.17-rc1-mm2.orig/drivers/isdn/gigaset/gigaset.h	2006-04-09 18:30:56.000000000 +0200
+++ linux-2.6.17-rc1-mm2/drivers/isdn/gigaset/gigaset.h	2006-04-18 21:29:00.000000000 +0200
@@ -445,6 +445,7 @@ struct cardstate {
 	struct gigaset_driver *driver;
 	unsigned minor_index;
 	struct device *dev;
+	struct class_device *class;

 	const struct gigaset_ops *ops;

diff -urpN -X exclude linux-2.6.17-rc1-mm2.orig/drivers/isdn/gigaset/interface.c linux-2.6.17-rc1-mm2/drivers/isdn/gigaset/interface.c
--- linux-2.6.17-rc1-mm2.orig/drivers/isdn/gigaset/interface.c	2006-04-09 18:30:56.000000000 +0200
+++ linux-2.6.17-rc1-mm2/drivers/isdn/gigaset/interface.c	2006-04-22 05:11:03.000000000 +0200
@@ -625,7 +625,14 @@ void gigaset_if_init(struct cardstate *c
 		return;

 	tasklet_init(&cs->if_wake_tasklet, &if_wake, (unsigned long) cs);
-	tty_register_device(drv->tty, cs->minor_index, NULL);
+	cs->class = tty_register_device(drv->tty, cs->minor_index, NULL);
+
+	if (!IS_ERR(cs->class))
+		class_set_devdata(cs->class, cs);
+	else {
+		warn("could not register device to the tty subsystem");
+		cs->class = NULL;
+	}
 }

 void gigaset_if_free(struct cardstate *cs)
@@ -638,6 +645,7 @@ void gigaset_if_free(struct cardstate *c

 	tasklet_disable(&cs->if_wake_tasklet);
 	tasklet_kill(&cs->if_wake_tasklet);
+	cs->class = NULL;
 	tty_unregister_device(drv->tty, cs->minor_index);
 }

diff -urpN -X exclude linux-2.6.17-rc1-mm2.orig/drivers/isdn/gigaset/proc.c linux-2.6.17-rc1-mm2/drivers/isdn/gigaset/proc.c
--- linux-2.6.17-rc1-mm2.orig/drivers/isdn/gigaset/proc.c	2006-04-09 18:30:56.000000000 +0200
+++ linux-2.6.17-rc1-mm2/drivers/isdn/gigaset/proc.c	2006-04-22 05:21:56.000000000 +0200
@@ -16,12 +16,11 @@
 #include "gigaset.h"
 #include <linux/ctype.h>

-static ssize_t show_cidmode(struct device *dev, struct device_attribute *attr,
-			    char *buf)
+static ssize_t show_cidmode(struct class_device *class, char *buf)
 {
 	int ret;
 	unsigned long flags;
-	struct cardstate *cs = dev_get_drvdata(dev);
+	struct cardstate *cs = class_get_devdata(class);

 	spin_lock_irqsave(&cs->lock, flags);
 	ret = sprintf(buf, "%u\n", cs->cidmode);
@@ -30,10 +29,10 @@ static ssize_t show_cidmode(struct devic
 	return ret;
 }

-static ssize_t set_cidmode(struct device *dev, struct device_attribute *attr,
+static ssize_t set_cidmode(struct class_device *class,
 			   const char *buf, size_t count)
 {
-	struct cardstate *cs = dev_get_drvdata(dev);
+	struct cardstate *cs = class_get_devdata(class);
 	long int value;
 	char *end;

@@ -65,18 +64,24 @@ static ssize_t set_cidmode(struct device
 	return count;
 }

-static DEVICE_ATTR(cidmode, S_IRUGO|S_IWUSR, show_cidmode, set_cidmode);
+static CLASS_DEVICE_ATTR(cidmode, S_IRUGO|S_IWUSR, show_cidmode, set_cidmode);

 /* free sysfs for device */
 void gigaset_free_dev_sysfs(struct cardstate *cs)
 {
+	if (!cs->class)
+		return;
+
 	gig_dbg(DEBUG_INIT, "removing sysfs entries");
-	device_remove_file(cs->dev, &dev_attr_cidmode);
+	class_device_remove_file(cs->class, &class_device_attr_cidmode);
 }

 /* initialize sysfs for device */
 void gigaset_init_dev_sysfs(struct cardstate *cs)
 {
+	if (!cs->class)
+		return;
+
 	gig_dbg(DEBUG_INIT, "setting up sysfs");
-	device_create_file(cs->dev, &dev_attr_cidmode);
+	class_device_create_file(cs->class, &class_device_attr_cidmode);
 }

-- 
Tilman Schmidt                          E-Mail: tilman@imap.cc
Bonn, Germany
Imagine a world without hypothetical situations.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 253 bytes --]