* [PATCH 1/3] nozomi: Add tty_port usage
@ 2010-02-08 10:06 Alan Cox
2010-02-08 10:07 ` [PATCH 2/3] nozomi: Fix mutex handling Alan Cox
2010-02-08 10:07 ` [PATCH 3/3] nozomi: Tidy up the PCI table Alan Cox
0 siblings, 2 replies; 6+ messages in thread
From: Alan Cox @ 2010-02-08 10:06 UTC (permalink / raw)
To: greg, linux-kernel, p.hardwick
The Nozomi tty handling is very broken on the open/close side (See
http://bugzilla.kernel.org/show_bug.cgi?id=13024 for one example). In
particular it marks the tty as closed on the first close() not on the last.
Most of the logic is pretty solid except for the open/close path so switch
to the tty_port helpers and let them do all the heavy lifting. This is also
fixes all the POSIX behaviour violations in the open/close paths.
Begin by adding the tty port usage
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
drivers/char/nozomi.c | 115 +++++++++++++++++++++++++++++--------------------
1 files changed, 68 insertions(+), 47 deletions(-)
diff --git a/drivers/char/nozomi.c b/drivers/char/nozomi.c
index 2ad7d37..cd405bc 100644
--- a/drivers/char/nozomi.c
+++ b/drivers/char/nozomi.c
@@ -371,6 +371,8 @@ struct port {
struct mutex tty_sem;
wait_queue_head_t tty_wait;
struct async_icount tty_icount;
+
+ struct nozomi *dc;
};
/* Private data one for each card in the system */
@@ -414,6 +416,8 @@ MODULE_DEVICE_TABLE(pci, nozomi_pci_tbl);
static struct nozomi *ndevs[NOZOMI_MAX_CARDS];
static struct tty_driver *ntty_driver;
+static const struct tty_port_operations noz_tty_port_ops;
+
/*
* find card by tty_index
*/
@@ -1473,9 +1477,11 @@ static int __devinit nozomi_card_init(struct pci_dev *pdev,
for (i = 0; i < MAX_PORT; i++) {
struct device *tty_dev;
-
- mutex_init(&dc->port[i].tty_sem);
- tty_port_init(&dc->port[i].port);
+ struct port *port = &dc->port[i];
+ port->dc = dc;
+ mutex_init(&port->tty_sem);
+ tty_port_init(&port->port);
+ port->port.ops = &noz_tty_port_ops;
tty_dev = tty_register_device(ntty_driver, dc->index_start + i,
&pdev->dev);
@@ -1600,67 +1606,74 @@ static void set_dtr(const struct tty_struct *tty, int dtr)
* ----------------------------------------------------------------------------
*/
-/* Called when the userspace process opens the tty, /dev/noz*. */
-static int ntty_open(struct tty_struct *tty, struct file *file)
+static int ntty_install(struct tty_driver *driver, struct tty_struct *tty)
{
struct port *port = get_port_by_tty(tty);
struct nozomi *dc = get_dc_by_tty(tty);
- unsigned long flags;
-
+ int ret;
if (!port || !dc || dc->state != NOZOMI_STATE_READY)
return -ENODEV;
-
- if (mutex_lock_interruptible(&port->tty_sem))
- return -ERESTARTSYS;
-
- port->port.count++;
- dc->open_ttys++;
-
- /* Enable interrupt downlink for channel */
- if (port->port.count == 1) {
- tty->driver_data = port;
- tty_port_tty_set(&port->port, tty);
- DBG1("open: %d", port->token_dl);
- spin_lock_irqsave(&dc->spin_mutex, flags);
- dc->last_ier = dc->last_ier | port->token_dl;
- writew(dc->last_ier, dc->reg_ier);
- spin_unlock_irqrestore(&dc->spin_mutex, flags);
+ ret = tty_init_termios(tty);
+ if (ret == 0) {
+ tty_driver_kref_get(driver);
+ driver->ttys[tty->index] = tty;
}
- mutex_unlock(&port->tty_sem);
- return 0;
+ return ret;
}
-/* Called when the userspace process close the tty, /dev/noz*. Also
- called immediately if ntty_open fails in which case tty->driver_data
- will be NULL an we exit by the first return */
+static void ntty_cleanup(struct tty_struct *tty)
+{
+ tty->driver_data = NULL;
+}
-static void ntty_close(struct tty_struct *tty, struct file *file)
+static int ntty_activate(struct tty_port *tport, struct tty_struct *tty)
{
- struct nozomi *dc = get_dc_by_tty(tty);
- struct port *nport = tty->driver_data;
- struct tty_port *port = &nport->port;
+ struct port *port = container_of(tport, struct port, port);
+ struct nozomi *dc = port->dc;
unsigned long flags;
- if (!dc || !nport)
- return;
+ DBG1("open: %d", port->token_dl);
+ spin_lock_irqsave(&dc->spin_mutex, flags);
+ dc->last_ier = dc->last_ier | port->token_dl;
+ writew(dc->last_ier, dc->reg_ier);
+ dc->open_ttys++;
+ spin_unlock_irqrestore(&dc->spin_mutex, flags);
+ printk("noz: activated %d: %p\n", tty->index, tport);
+ return 0;
+}
- /* Users cannot interrupt a close */
- mutex_lock(&nport->tty_sem);
+static int ntty_open(struct tty_struct *tty, struct file *filp)
+{
+ struct port *port = get_port_by_tty(tty);
+ return tty_port_open(&port->port, tty, filp);
+}
- WARN_ON(!port->count);
+static void ntty_shutdown(struct tty_port *tport)
+{
+ struct port *port = container_of(tport, struct port, port);
+ struct nozomi *dc = port->dc;
+ unsigned long flags;
+ DBG1("close: %d", port->token_dl);
+ spin_lock_irqsave(&dc->spin_mutex, flags);
+ dc->last_ier &= ~(port->token_dl);
+ writew(dc->last_ier, dc->reg_ier);
dc->open_ttys--;
- port->count--;
+ spin_unlock_irqrestore(&dc->spin_mutex, flags);
+ printk("noz: shutdown %p\n", tport);
+}
- if (port->count == 0) {
- DBG1("close: %d", nport->token_dl);
- tty_port_tty_set(port, NULL);
- spin_lock_irqsave(&dc->spin_mutex, flags);
- dc->last_ier &= ~(nport->token_dl);
- writew(dc->last_ier, dc->reg_ier);
- spin_unlock_irqrestore(&dc->spin_mutex, flags);
- }
- mutex_unlock(&nport->tty_sem);
+static void ntty_close(struct tty_struct *tty, struct file *filp)
+{
+ struct port *port = tty->driver_data;
+ if (port)
+ tty_port_close(&port->port, tty, filp);
+}
+
+static void ntty_hangup(struct tty_struct *tty)
+{
+ struct port *port = tty->driver_data;
+ tty_port_hangup(&port->port);
}
/*
@@ -1906,10 +1919,16 @@ exit_in_buffer:
return rval;
}
+static const struct tty_port_operations noz_tty_port_ops = {
+ .activate = ntty_activate,
+ .shutdown = ntty_shutdown,
+};
+
static const struct tty_operations tty_ops = {
.ioctl = ntty_ioctl,
.open = ntty_open,
.close = ntty_close,
+ .hangup = ntty_hangup,
.write = ntty_write,
.write_room = ntty_write_room,
.unthrottle = ntty_unthrottle,
@@ -1917,6 +1936,8 @@ static const struct tty_operations tty_ops = {
.chars_in_buffer = ntty_chars_in_buffer,
.tiocmget = ntty_tiocmget,
.tiocmset = ntty_tiocmset,
+ .install = ntty_install,
+ .cleanup = ntty_cleanup,
};
/* Module initialization */
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] nozomi: Fix mutex handling
2010-02-08 10:06 [PATCH 1/3] nozomi: Add tty_port usage Alan Cox
@ 2010-02-08 10:07 ` Alan Cox
2010-02-08 10:07 ` [PATCH 3/3] nozomi: Tidy up the PCI table Alan Cox
1 sibling, 0 replies; 6+ messages in thread
From: Alan Cox @ 2010-02-08 10:07 UTC (permalink / raw)
To: greg, linux-kernel, p.hardwick
The original author didn't realise the kernel lock was a drop while sleep
lock so did clever (and wrong) things to work around the non need to avoid
deadlocks. Remove the cleverness and the comment (as we don't hold the BKL
now anyway in those paths)
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
drivers/char/nozomi.c | 34 ++++++++++++----------------------
1 files changed, 12 insertions(+), 22 deletions(-)
diff --git a/drivers/char/nozomi.c b/drivers/char/nozomi.c
index cd405bc..fac9157 100644
--- a/drivers/char/nozomi.c
+++ b/drivers/char/nozomi.c
@@ -1693,15 +1693,7 @@ static int ntty_write(struct tty_struct *tty, const unsigned char *buffer,
if (!dc || !port)
return -ENODEV;
- if (unlikely(!mutex_trylock(&port->tty_sem))) {
- /*
- * must test lock as tty layer wraps calls
- * to this function with BKL
- */
- dev_err(&dc->pdev->dev, "Would have deadlocked - "
- "return EAGAIN\n");
- return -EAGAIN;
- }
+ mutex_lock(&port->tty_sem);
if (unlikely(!port->port.count)) {
DBG1(" ");
@@ -1741,25 +1733,23 @@ exit:
* This method is called by the upper tty layer.
* #according to sources N_TTY.c it expects a value >= 0 and
* does not check for negative values.
+ *
+ * If the port is unplugged report lots of room and let the bits
+ * dribble away so we don't block anything.
*/
static int ntty_write_room(struct tty_struct *tty)
{
struct port *port = tty->driver_data;
- int room = 0;
+ int room = 4096;
const struct nozomi *dc = get_dc_by_tty(tty);
- if (!dc || !port)
- return 0;
- if (!mutex_trylock(&port->tty_sem))
- return 0;
-
- if (!port->port.count)
- goto exit;
-
- room = port->fifo_ul.size - kfifo_len(&port->fifo_ul);
-
-exit:
- mutex_unlock(&port->tty_sem);
+ if (dc) {
+ mutex_lock(&port->tty_sem);
+ if (port->port.count)
+ room = port->fifo_ul.size -
+ kfifo_len(&port->fifo_ul);
+ mutex_unlock(&port->tty_sem);
+ }
return room;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] nozomi: Tidy up the PCI table
2010-02-08 10:06 [PATCH 1/3] nozomi: Add tty_port usage Alan Cox
2010-02-08 10:07 ` [PATCH 2/3] nozomi: Fix mutex handling Alan Cox
@ 2010-02-08 10:07 ` Alan Cox
2010-02-08 10:49 ` Marcel Holtmann
1 sibling, 1 reply; 6+ messages in thread
From: Alan Cox @ 2010-02-08 10:07 UTC (permalink / raw)
To: greg, linux-kernel, p.hardwick
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
drivers/char/nozomi.c | 6 +-----
1 files changed, 1 insertions(+), 5 deletions(-)
diff --git a/drivers/char/nozomi.c b/drivers/char/nozomi.c
index fac9157..86c9e64 100644
--- a/drivers/char/nozomi.c
+++ b/drivers/char/nozomi.c
@@ -136,10 +136,6 @@ static int debug;
#define RECEIVE_BUF_MAX 4
-/* Define all types of vendors and devices to support */
-#define VENDOR1 0x1931 /* Vendor Option */
-#define DEVICE1 0x000c /* HSDPA card */
-
#define R_IIR 0x0000 /* Interrupt Identity Register */
#define R_FCR 0x0000 /* Flow Control Register */
#define R_IER 0x0004 /* Interrupt Enable Register */
@@ -407,7 +403,7 @@ struct buffer {
/* Global variables */
static const struct pci_device_id nozomi_pci_tbl[] __devinitconst = {
- {PCI_DEVICE(VENDOR1, DEVICE1)},
+ {PCI_DEVICE(0x1931, 0x000c)}, /* Nozomi HSPDA */
{},
};
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] nozomi: Tidy up the PCI table
2010-02-08 10:07 ` [PATCH 3/3] nozomi: Tidy up the PCI table Alan Cox
@ 2010-02-08 10:49 ` Marcel Holtmann
2010-02-17 22:59 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Marcel Holtmann @ 2010-02-08 10:49 UTC (permalink / raw)
To: Alan Cox; +Cc: greg, linux-kernel, p.hardwick
Hi Alan,
> Signed-off-by: Alan Cox <alan@linux.intel.com>
> ---
>
> drivers/char/nozomi.c | 6 +-----
> 1 files changed, 1 insertions(+), 5 deletions(-)
>
>
> diff --git a/drivers/char/nozomi.c b/drivers/char/nozomi.c
> index fac9157..86c9e64 100644
> --- a/drivers/char/nozomi.c
> +++ b/drivers/char/nozomi.c
> @@ -136,10 +136,6 @@ static int debug;
> #define RECEIVE_BUF_MAX 4
>
>
> -/* Define all types of vendors and devices to support */
> -#define VENDOR1 0x1931 /* Vendor Option */
> -#define DEVICE1 0x000c /* HSDPA card */
> -
> #define R_IIR 0x0000 /* Interrupt Identity Register */
> #define R_FCR 0x0000 /* Flow Control Register */
> #define R_IER 0x0004 /* Interrupt Enable Register */
> @@ -407,7 +403,7 @@ struct buffer {
>
> /* Global variables */
> static const struct pci_device_id nozomi_pci_tbl[] __devinitconst = {
> - {PCI_DEVICE(VENDOR1, DEVICE1)},
> + {PCI_DEVICE(0x1931, 0x000c)}, /* Nozomi HSPDA */
> {},
that would be HSDPA actually.
Regards
Marcel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] nozomi: Tidy up the PCI table
2010-02-08 10:49 ` Marcel Holtmann
@ 2010-02-17 22:59 ` Greg KH
0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2010-02-17 22:59 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: Alan Cox, linux-kernel, p.hardwick
On Mon, Feb 08, 2010 at 11:49:49AM +0100, Marcel Holtmann wrote:
> Hi Alan,
>
> > Signed-off-by: Alan Cox <alan@linux.intel.com>
> > ---
> >
> > drivers/char/nozomi.c | 6 +-----
> > 1 files changed, 1 insertions(+), 5 deletions(-)
> >
> >
> > diff --git a/drivers/char/nozomi.c b/drivers/char/nozomi.c
> > index fac9157..86c9e64 100644
> > --- a/drivers/char/nozomi.c
> > +++ b/drivers/char/nozomi.c
> > @@ -136,10 +136,6 @@ static int debug;
> > #define RECEIVE_BUF_MAX 4
> >
> >
> > -/* Define all types of vendors and devices to support */
> > -#define VENDOR1 0x1931 /* Vendor Option */
> > -#define DEVICE1 0x000c /* HSDPA card */
> > -
> > #define R_IIR 0x0000 /* Interrupt Identity Register */
> > #define R_FCR 0x0000 /* Flow Control Register */
> > #define R_IER 0x0004 /* Interrupt Enable Register */
> > @@ -407,7 +403,7 @@ struct buffer {
> >
> > /* Global variables */
> > static const struct pci_device_id nozomi_pci_tbl[] __devinitconst = {
> > - {PCI_DEVICE(VENDOR1, DEVICE1)},
> > + {PCI_DEVICE(0x1931, 0x000c)}, /* Nozomi HSPDA */
> > {},
>
> that would be HSDPA actually.
I edited the patch, thanks.
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] nozomi: Add tty_port usage
@ 2010-02-17 12:59 Alan Cox
0 siblings, 0 replies; 6+ messages in thread
From: Alan Cox @ 2010-02-17 12:59 UTC (permalink / raw)
To: fseidel, greg, linux-serial
The Nozomi tty handling is very broken on the open/close side (See
http://bugzilla.kernel.org/show_bug.cgi?id=13024 for one example). In
particular it marks the tty as closed on the first close() not on the last.
Most of the logic is pretty solid except for the open/close path so switch
to the tty_port helpers and let them do all the heavy lifting. This is also
fixes all the POSIX behaviour violations in the open/close paths.
Begin by adding the tty port usage
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
drivers/char/nozomi.c | 115 +++++++++++++++++++++++++++++--------------------
1 files changed, 68 insertions(+), 47 deletions(-)
diff --git a/drivers/char/nozomi.c b/drivers/char/nozomi.c
index 2ad7d37..cd405bc 100644
--- a/drivers/char/nozomi.c
+++ b/drivers/char/nozomi.c
@@ -371,6 +371,8 @@ struct port {
struct mutex tty_sem;
wait_queue_head_t tty_wait;
struct async_icount tty_icount;
+
+ struct nozomi *dc;
};
/* Private data one for each card in the system */
@@ -414,6 +416,8 @@ MODULE_DEVICE_TABLE(pci, nozomi_pci_tbl);
static struct nozomi *ndevs[NOZOMI_MAX_CARDS];
static struct tty_driver *ntty_driver;
+static const struct tty_port_operations noz_tty_port_ops;
+
/*
* find card by tty_index
*/
@@ -1473,9 +1477,11 @@ static int __devinit nozomi_card_init(struct pci_dev *pdev,
for (i = 0; i < MAX_PORT; i++) {
struct device *tty_dev;
-
- mutex_init(&dc->port[i].tty_sem);
- tty_port_init(&dc->port[i].port);
+ struct port *port = &dc->port[i];
+ port->dc = dc;
+ mutex_init(&port->tty_sem);
+ tty_port_init(&port->port);
+ port->port.ops = &noz_tty_port_ops;
tty_dev = tty_register_device(ntty_driver, dc->index_start + i,
&pdev->dev);
@@ -1600,67 +1606,74 @@ static void set_dtr(const struct tty_struct *tty, int dtr)
* ----------------------------------------------------------------------------
*/
-/* Called when the userspace process opens the tty, /dev/noz*. */
-static int ntty_open(struct tty_struct *tty, struct file *file)
+static int ntty_install(struct tty_driver *driver, struct tty_struct *tty)
{
struct port *port = get_port_by_tty(tty);
struct nozomi *dc = get_dc_by_tty(tty);
- unsigned long flags;
-
+ int ret;
if (!port || !dc || dc->state != NOZOMI_STATE_READY)
return -ENODEV;
-
- if (mutex_lock_interruptible(&port->tty_sem))
- return -ERESTARTSYS;
-
- port->port.count++;
- dc->open_ttys++;
-
- /* Enable interrupt downlink for channel */
- if (port->port.count == 1) {
- tty->driver_data = port;
- tty_port_tty_set(&port->port, tty);
- DBG1("open: %d", port->token_dl);
- spin_lock_irqsave(&dc->spin_mutex, flags);
- dc->last_ier = dc->last_ier | port->token_dl;
- writew(dc->last_ier, dc->reg_ier);
- spin_unlock_irqrestore(&dc->spin_mutex, flags);
+ ret = tty_init_termios(tty);
+ if (ret == 0) {
+ tty_driver_kref_get(driver);
+ driver->ttys[tty->index] = tty;
}
- mutex_unlock(&port->tty_sem);
- return 0;
+ return ret;
}
-/* Called when the userspace process close the tty, /dev/noz*. Also
- called immediately if ntty_open fails in which case tty->driver_data
- will be NULL an we exit by the first return */
+static void ntty_cleanup(struct tty_struct *tty)
+{
+ tty->driver_data = NULL;
+}
-static void ntty_close(struct tty_struct *tty, struct file *file)
+static int ntty_activate(struct tty_port *tport, struct tty_struct *tty)
{
- struct nozomi *dc = get_dc_by_tty(tty);
- struct port *nport = tty->driver_data;
- struct tty_port *port = &nport->port;
+ struct port *port = container_of(tport, struct port, port);
+ struct nozomi *dc = port->dc;
unsigned long flags;
- if (!dc || !nport)
- return;
+ DBG1("open: %d", port->token_dl);
+ spin_lock_irqsave(&dc->spin_mutex, flags);
+ dc->last_ier = dc->last_ier | port->token_dl;
+ writew(dc->last_ier, dc->reg_ier);
+ dc->open_ttys++;
+ spin_unlock_irqrestore(&dc->spin_mutex, flags);
+ printk("noz: activated %d: %p\n", tty->index, tport);
+ return 0;
+}
- /* Users cannot interrupt a close */
- mutex_lock(&nport->tty_sem);
+static int ntty_open(struct tty_struct *tty, struct file *filp)
+{
+ struct port *port = get_port_by_tty(tty);
+ return tty_port_open(&port->port, tty, filp);
+}
- WARN_ON(!port->count);
+static void ntty_shutdown(struct tty_port *tport)
+{
+ struct port *port = container_of(tport, struct port, port);
+ struct nozomi *dc = port->dc;
+ unsigned long flags;
+ DBG1("close: %d", port->token_dl);
+ spin_lock_irqsave(&dc->spin_mutex, flags);
+ dc->last_ier &= ~(port->token_dl);
+ writew(dc->last_ier, dc->reg_ier);
dc->open_ttys--;
- port->count--;
+ spin_unlock_irqrestore(&dc->spin_mutex, flags);
+ printk("noz: shutdown %p\n", tport);
+}
- if (port->count == 0) {
- DBG1("close: %d", nport->token_dl);
- tty_port_tty_set(port, NULL);
- spin_lock_irqsave(&dc->spin_mutex, flags);
- dc->last_ier &= ~(nport->token_dl);
- writew(dc->last_ier, dc->reg_ier);
- spin_unlock_irqrestore(&dc->spin_mutex, flags);
- }
- mutex_unlock(&nport->tty_sem);
+static void ntty_close(struct tty_struct *tty, struct file *filp)
+{
+ struct port *port = tty->driver_data;
+ if (port)
+ tty_port_close(&port->port, tty, filp);
+}
+
+static void ntty_hangup(struct tty_struct *tty)
+{
+ struct port *port = tty->driver_data;
+ tty_port_hangup(&port->port);
}
/*
@@ -1906,10 +1919,16 @@ exit_in_buffer:
return rval;
}
+static const struct tty_port_operations noz_tty_port_ops = {
+ .activate = ntty_activate,
+ .shutdown = ntty_shutdown,
+};
+
static const struct tty_operations tty_ops = {
.ioctl = ntty_ioctl,
.open = ntty_open,
.close = ntty_close,
+ .hangup = ntty_hangup,
.write = ntty_write,
.write_room = ntty_write_room,
.unthrottle = ntty_unthrottle,
@@ -1917,6 +1936,8 @@ static const struct tty_operations tty_ops = {
.chars_in_buffer = ntty_chars_in_buffer,
.tiocmget = ntty_tiocmget,
.tiocmset = ntty_tiocmset,
+ .install = ntty_install,
+ .cleanup = ntty_cleanup,
};
/* Module initialization */
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-02-17 23:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-02-08 10:06 [PATCH 1/3] nozomi: Add tty_port usage Alan Cox
2010-02-08 10:07 ` [PATCH 2/3] nozomi: Fix mutex handling Alan Cox
2010-02-08 10:07 ` [PATCH 3/3] nozomi: Tidy up the PCI table Alan Cox
2010-02-08 10:49 ` Marcel Holtmann
2010-02-17 22:59 ` Greg KH
2010-02-17 12:59 [PATCH 1/3] nozomi: Add tty_port usage Alan Cox
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.