[PATCH 1/2] tty_io: add an API to get device corresponding to tty_struct

[PATCH 1/2] tty_io: add an API to get device corresponding to tty_struct

[Dmitry Eremin-Solenikov]

Some device drivers (mostly tty line disciplines) would like to have way
to get a struct device instancve corresponding to passed tty_struct. Add
respective API call.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 drivers/char/tty_io.c |   20 ++++++++++++++++++++
 include/linux/tty.h   |    1 +
 2 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index d71f0fc..43fdeef 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -3017,6 +3017,26 @@ dev_t tty_devnum(struct tty_struct *tty)
 }
 EXPORT_SYMBOL(tty_devnum);
 
+static int dev_match_devt(struct device *dev, void *data)
+{
+	dev_t *devt = data;
+	return dev->devt == *devt;
+}
+
+/**
+ * 	tty_get_device - get a device corresponding to tty
+ * 	@tty: the struct that describes the tty device
+ *
+ * 	Returns a pointer to the struct device for this tty device
+ * 	(or NULL in case of error).
+ */
+struct device *tty_get_device(struct tty_struct *tty)
+{
+	dev_t devt = tty_devnum(tty);
+	return class_find_device(tty_class, NULL, &devt, dev_match_devt);
+}
+EXPORT_SYMBOL(tty_get_device);
+
 void proc_clear_tty(struct task_struct *p)
 {
 	unsigned long flags;
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 931078b..5038433 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -451,6 +451,7 @@ extern int tty_mode_ioctl(struct tty_struct *tty, struct file *file,
 			unsigned int cmd, unsigned long arg);
 extern int tty_perform_flush(struct tty_struct *tty, unsigned long arg);
 extern dev_t tty_devnum(struct tty_struct *tty);
+struct device *tty_get_device(struct tty_struct *tty);
 extern void proc_clear_tty(struct task_struct *p);
 extern struct tty_struct *get_current_tty(void);
 extern void tty_default_fops(struct file_operations *fops);
-- 
1.7.1

[PATCH 2/2] serport: place serport serio device correctly in the device tree

[Dmitry Eremin-Solenikov]

Make serport serio device to be a child of corresponding tty device
instead of just hanging at /sys/devices/serioX.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 drivers/input/serio/serport.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/input/serio/serport.c b/drivers/input/serio/serport.c
index 6d34511..ee0ff0c 100644
--- a/drivers/input/serio/serport.c
+++ b/drivers/input/serio/serport.c
@@ -165,6 +165,7 @@ static ssize_t serport_ldisc_read(struct tty_struct * tty, struct file * file, u
 	serio->open = serport_serio_open;
 	serio->close = serport_serio_close;
 	serio->port_data = serport;
+	serio->dev.parent = tty_get_device(tty);
 
 	serio_register_port(serport->serio);
 	printk(KERN_INFO "serio: Serial port %s\n", tty_name(tty, name));
-- 
1.7.1

Re: [PATCH 2/2] serport: place serport serio device correctly in the device tree

[Greg KH]

On Fri, Aug 06, 2010 at 08:29:29PM +0400, Dmitry Eremin-Solenikov wrote:
> Make serport serio device to be a child of corresponding tty device
> instead of just hanging at /sys/devices/serioX.
> 
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> ---
>  drivers/input/serio/serport.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/input/serio/serport.c b/drivers/input/serio/serport.c
> index 6d34511..ee0ff0c 100644
> --- a/drivers/input/serio/serport.c
> +++ b/drivers/input/serio/serport.c
> @@ -165,6 +165,7 @@ static ssize_t serport_ldisc_read(struct tty_struct * tty, struct file * file, u
>  	serio->open = serport_serio_open;
>  	serio->close = serport_serio_close;
>  	serio->port_data = serport;
> +	serio->dev.parent = tty_get_device(tty);

You just grabbed a reference to this device, do you ever need to free it
up?

thanks,

greg k-h

[PATCH 2/2] serport: place serport serio device correctly in the device tree

[Dmitry Eremin-Solenikov]

Make serport serio device to be a child of corresponding tty device
instead of just hanging at /sys/devices/serioX.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 drivers/input/serio/serport.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/input/serio/serport.c b/drivers/input/serio/serport.c
index 6d34511..d3d832a 100644
--- a/drivers/input/serio/serport.c
+++ b/drivers/input/serio/serport.c
@@ -148,6 +148,7 @@ static ssize_t serport_ldisc_read(struct tty_struct * tty, struct file * file, u
 {
 	struct serport *serport = (struct serport*) tty->disc_data;
 	struct serio *serio;
+	struct device *parent;
 	char name[64];
 
 	if (test_and_set_bit(SERPORT_BUSY, &serport->flags))
@@ -157,6 +158,8 @@ static ssize_t serport_ldisc_read(struct tty_struct * tty, struct file * file, u
 	if (!serio)
 		return -ENOMEM;
 
+	parent = tty_get_device(tty);
+
 	strlcpy(serio->name, "Serial port", sizeof(serio->name));
 	snprintf(serio->phys, sizeof(serio->phys), "%s/serio0", tty_name(tty, name));
 	serio->id = serport->id;
@@ -165,6 +168,7 @@ static ssize_t serport_ldisc_read(struct tty_struct * tty, struct file * file, u
 	serio->open = serport_serio_open;
 	serio->close = serport_serio_close;
 	serio->port_data = serport;
+	serio->dev.parent = parent;
 
 	serio_register_port(serport->serio);
 	printk(KERN_INFO "serio: Serial port %s\n", tty_name(tty, name));
@@ -173,6 +177,8 @@ static ssize_t serport_ldisc_read(struct tty_struct * tty, struct file * file, u
 	serio_unregister_port(serport->serio);
 	serport->serio = NULL;
 
+	put_device(parent);
+
 	clear_bit(SERPORT_DEAD, &serport->flags);
 	clear_bit(SERPORT_BUSY, &serport->flags);
 
-- 
1.7.1

Re: [PATCH 1/2] tty_io: add an API to get device corresponding to tty_struct

[Greg KH]

On Fri, Aug 06, 2010 at 08:29:28PM +0400, Dmitry Eremin-Solenikov wrote:
> Some device drivers (mostly tty line disciplines) would like to have way
> to get a struct device instancve corresponding to passed tty_struct. Add
> respective API call.
> 
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> ---
>  drivers/char/tty_io.c |   20 ++++++++++++++++++++
>  include/linux/tty.h   |    1 +
>  2 files changed, 21 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
> index d71f0fc..43fdeef 100644
> --- a/drivers/char/tty_io.c
> +++ b/drivers/char/tty_io.c
> @@ -3017,6 +3017,26 @@ dev_t tty_devnum(struct tty_struct *tty)
>  }
>  EXPORT_SYMBOL(tty_devnum);
>  
> +static int dev_match_devt(struct device *dev, void *data)
> +{
> +	dev_t *devt = data;
> +	return dev->devt == *devt;
> +}
> +
> +/**
> + * 	tty_get_device - get a device corresponding to tty
> + * 	@tty: the struct that describes the tty device
> + *
> + * 	Returns a pointer to the struct device for this tty device
> + * 	(or NULL in case of error).

You should add some wording here that says that the device now has an
increased reference count and that you better drop it when you are done
with it, otherwise it will stay around for a while.

Other than that, I like the idea.

Care to redo it?

thanks,

greg k-h

[PATCH 1/2] tty_io: add an API to get device corresponding to tty_struct

[Dmitry Eremin-Solenikov]

Some device drivers (mostly tty line disciplines) would like to have way
to get a struct device instancve corresponding to passed tty_struct. Add
respective API call.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 drivers/char/tty_io.c |   21 +++++++++++++++++++++
 include/linux/tty.h   |    1 +
 2 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index d71f0fc..dcdc8fc 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -3017,6 +3017,27 @@ dev_t tty_devnum(struct tty_struct *tty)
 }
 EXPORT_SYMBOL(tty_devnum);
 
+static int dev_match_devt(struct device *dev, void *data)
+{
+	dev_t *devt = data;
+	return dev->devt == *devt;
+}
+
+/**
+ * 	tty_get_device - get a device corresponding to tty
+ * 	@tty: the struct that describes the tty device
+ *
+ * 	Returns a pointer to the struct device for this tty device
+ * 	(or NULL in case of error). Returned device has it's refcount
+ * 	incremented by 1, so you should call put_device() after usage.
+ */
+struct device *tty_get_device(struct tty_struct *tty)
+{
+	dev_t devt = tty_devnum(tty);
+	return class_find_device(tty_class, NULL, &devt, dev_match_devt);
+}
+EXPORT_SYMBOL(tty_get_device);
+
 void proc_clear_tty(struct task_struct *p)
 {
 	unsigned long flags;
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 931078b..5038433 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -451,6 +451,7 @@ extern int tty_mode_ioctl(struct tty_struct *tty, struct file *file,
 			unsigned int cmd, unsigned long arg);
 extern int tty_perform_flush(struct tty_struct *tty, unsigned long arg);
 extern dev_t tty_devnum(struct tty_struct *tty);
+struct device *tty_get_device(struct tty_struct *tty);
 extern void proc_clear_tty(struct task_struct *p);
 extern struct tty_struct *get_current_tty(void);
 extern void tty_default_fops(struct file_operations *fops);
-- 
1.7.1

Re: [PATCH 1/2] tty_io: add an API to get device corresponding to tty_struct

[Alan Cox]

On Sat,  7 Aug 2010 13:55:02 +0400
Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> wrote:

> Some device drivers (mostly tty line disciplines) would like to have way
> to get a struct device instancve corresponding to passed tty_struct. Add
> respective API call.

Most of the kernel simply keeps a parent device pointer in the struct. If
we have a lookup method then we'll simply end up with a struct device in
each tty ldisc struct instead, and it will be harder to clean up later.

I think I'd much rather see this set when the tty is created and always
available. That would also let the whole tty layer use dev_err() and
friends usefully ?

Alan

Re: [PATCH 1/2] tty_io: add an API to get device corresponding to tty_struct

[Dmitry Eremin-Solenikov]

On Sat, Aug 07, 2010 at 12:33:33PM +0100, Alan Cox wrote:
> On Sat,  7 Aug 2010 13:55:02 +0400
> Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> wrote:
> 
> > Some device drivers (mostly tty line disciplines) would like to have way
> > to get a struct device instancve corresponding to passed tty_struct. Add
> > respective API call.
> 
> Most of the kernel simply keeps a parent device pointer in the struct. If
> we have a lookup method then we'll simply end up with a struct device in
> each tty ldisc struct instead, and it will be harder to clean up later.

The proposed usage (at least what we did in the second patch and what
I did in IEEE 802.15.4 tty driver) is to use this device pointer to
set up parent node for all devices created on top of given tty port.

> I think I'd much rather see this set when the tty is created and always
> available. That would also let the whole tty layer use dev_err() and
> friends usefully ?

I was afraid of messing with tty layer in such way, as it's a bit too
complex for me.

Do you mean adding struct device pointer to struct tty_struct (easy),
or refactoring code so, that struct tty_struct contains embedded
struct device and uses it for refcounting & stuff?

-- 
With best wishes
Dmitry

Re: [PATCH 1/2] tty_io: add an API to get device corresponding to tty_struct

[Alan Cox]

> I was afraid of messing with tty layer in such way, as it's a bit too
> complex for me.
> 
> Do you mean adding struct device pointer to struct tty_struct (easy),
> or refactoring code so, that struct tty_struct contains embedded
> struct device and uses it for refcounting & stuff?

The former.

Alan

[PATCH 1/2] tty: add tty_struct->dev pointer to corresponding device instance

[Dmitry Eremin-Solenikov]

Some device drivers (mostly tty line disciplines) would like to have way
know a struct device instance corresponding to passed tty_struct. Add
a struct device pointer to struct tty_struct and populate it during
initialize_tty_struct().

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
---
 drivers/char/tty_io.c |   17 +++++++++++++++++
 include/linux/tty.h   |    1 +
 2 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index d71f0fc..2469682 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -179,6 +179,8 @@ struct tty_struct *alloc_tty_struct(void)
 
 void free_tty_struct(struct tty_struct *tty)
 {
+	if (tty->dev)
+		put_device(tty->dev);
 	kfree(tty->write_buf);
 	tty_buffer_free_all(tty);
 	kfree(tty);
@@ -2737,6 +2739,20 @@ void do_SAK(struct tty_struct *tty)
 
 EXPORT_SYMBOL(do_SAK);
 
+static int dev_match_devt(struct device *dev, void *data)
+{
+	dev_t *devt = data;
+	return dev->devt == *devt;
+}
+
+/* Must put_device() after it's unused! */
+static struct device *tty_get_device(struct tty_struct *tty)
+{
+	dev_t devt = tty_devnum(tty);
+	return class_find_device(tty_class, NULL, &devt, dev_match_devt);
+}
+
+
 /**
  *	initialize_tty_struct
  *	@tty: tty to initialize
@@ -2777,6 +2793,7 @@ void initialize_tty_struct(struct tty_struct *tty,
 	tty->ops = driver->ops;
 	tty->index = idx;
 	tty_line_name(driver, idx, tty->name);
+	tty->dev = tty_get_device(tty);
 }
 
 /**
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 931078b..d64db5a 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -254,6 +254,7 @@ struct tty_operations;
 struct tty_struct {
 	int	magic;
 	struct kref kref;
+	struct device *dev;
 	struct tty_driver *driver;
 	const struct tty_operations *ops;
 	int index;
-- 
1.7.1

[PATCH 2/2] serport: place serport serio device correctly in the device tree

[Dmitry Eremin-Solenikov]

Make serport serio device to be a child of corresponding tty device
instead of just hanging at /sys/devices/serioX.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/serio/serport.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/input/serio/serport.c b/drivers/input/serio/serport.c
index 6d34511..6e362de 100644
--- a/drivers/input/serio/serport.c
+++ b/drivers/input/serio/serport.c
@@ -165,6 +165,7 @@ static ssize_t serport_ldisc_read(struct tty_struct * tty, struct file * file, u
 	serio->open = serport_serio_open;
 	serio->close = serport_serio_close;
 	serio->port_data = serport;
+	serio->dev.parent = tty->dev;
 
 	serio_register_port(serport->serio);
 	printk(KERN_INFO "serio: Serial port %s\n", tty_name(tty, name));
-- 
1.7.1

Re: [PATCH 2/2] serport: place serport serio device correctly in the device tree

[Dmitry Torokhov]

On Mon, Aug 09, 2010 at 06:22:50PM +0400, Dmitry Eremin-Solenikov wrote:
> Make serport serio device to be a child of corresponding tty device
> instead of just hanging at /sys/devices/serioX.
> 
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Looks good to me.

Acked-by: Dmitry Torokhov <dtor@mail.ru>

> ---
>  drivers/input/serio/serport.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/input/serio/serport.c b/drivers/input/serio/serport.c
> index 6d34511..6e362de 100644
> --- a/drivers/input/serio/serport.c
> +++ b/drivers/input/serio/serport.c
> @@ -165,6 +165,7 @@ static ssize_t serport_ldisc_read(struct tty_struct * tty, struct file * file, u
>  	serio->open = serport_serio_open;
>  	serio->close = serport_serio_close;
>  	serio->port_data = serport;
> +	serio->dev.parent = tty->dev;
>  
>  	serio_register_port(serport->serio);
>  	printk(KERN_INFO "serio: Serial port %s\n", tty_name(tty, name));
> -- 
> 1.7.1
> 

-- 
Dmitry