small design problem with hso driver

Hi,
I'm getting a fairly consistently reproducable problem with the 3g modem hso driver. The most reproducable bug I have.

It happens when I plug out the usb modem & ctrl-c
out of an ioctl call with the modem connected in paralell over ppp with wvdial

Basically whats happening is the hso_serial_close is being called more times than the hso_serial_open & I don't know why.

I can put in a kludge to cure the symptom which would prevent the crash but it would be nice to know why this might be
happening.

best regards,
D.J. Barrow
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Care to put a trace or printk() to determine who is doing each open and close?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Hi,
I've found something suspicous but am not sure if
it's my problem. It looks like it when I'm
doing a gdb debug session but I can't believe
code so wrong can get into the kernel
& it didn't cause problems before.

Have a look at /drivers/char/tty_io.c

tty_open

around line 2234 this calls
tty->ops->open which calls hso_serial_open
& if this returns an error i.e. -ENODEV release_dev is called which calls in tty_open which calls tty->ops_close around line 2003 in tty_io.c.

Surely if the driver returns an error code on hso_serial_open hso_serial_close shouldn't be called
or have I a complete misunderstanding
of char driver semantics.

best regards,
D.J. Barrow
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Generally yes that would be sane. The tty layer has always called the close method of drivers on a failed open and the drivers are all written assuming that.

It would be nice to change but there are far bigger fish to fry.

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Hi Alan,
No they are not.
Have a look at.
/drivers/char/nozomi.c
admittedly it's written by some of the same guys.
/* Called when the userspace process opens the tty, /dev/noz*. */ static int ntty_open(struct tty_struct *tty, struct file *file)

{
	struct port *port = get_port_by_tty(tty);
	struct nozomi *dc = get_dc_by_tty(tty);
	unsigned long flags;

	if (!port || !dc || dc->state != NOZOMI_STATE_READY)
		return -ENODEV;

	if (mutex_lock_interruptible(&port->tty_sem))
		return -ERESTARTSYS;

	port->tty_open_count++;
	dc->open_ttys++;

	/* Enable interrupt downlink for channel */
	if (port->tty_open_count == 1) {
		tty->low_latency = 1;
		tty->driver_data = port;
		port->tty = 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);
	}

	mutex_unlock(&port->tty_sem);

	return 0;
}

/* Called when the userspace process close the tty, /dev/noz*. */ static void ntty_close(struct tty_struct *tty, struct file *file)

{
	struct nozomi *dc = get_dc_by_tty(tty);
	struct port *port = tty->driver_data;
	unsigned long flags;

	if (!dc || !port)
		return;

	if (mutex_lock_interruptible(&port->tty_sem))
		return;

	if (!port->tty_open_count)
		goto exit;

	dc->open_ttys--;
	port->tty_open_count--;

	if (port->tty_open_count == 0) {
		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);
		spin_unlock_irqrestore(&dc->spin_mutex, flags);
	}

exit:

	mutex_unlock(&port->tty_sem);
}

best regards,
D.J. Barrow
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Nozomi still has lots and lots of bugs left. Compared to the original its now a work of art and much good stuff has been done but it is not by any means 'done'.

File a bug with the nozomi author
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html