Re: [RFC] Patch to option HSO driver to the kernel

[Oliver Neukum <oliver@neukum.org>]

Am Mittwoch, 16. April 2008 15:55:02 schrieb Paulius Zaleckas:
> Oliver Neukum wrote:
> > Am Mittwoch, 16. April 2008 14:12:06 schrieb Paulius Zaleckas:
> >> I always get crash when closing serial terminal application. I will
> >> post dump in couple minutes.
> > 
> > Does this fix it?
> > 
> 
> Yes, it does! Thank you.

Hi,

can you test this one as well. The method used to determine whether
a device is asleep is racy. This introduces a private test.

	Regards
		Oliver

---

--- linux-2.6.25hso/drivers/net/usb/hso.c.alt2	2008-04-17 14:15:32.000000000 +0200
+++ linux-2.6.25hso/drivers/net/usb/hso.c	2008-04-17 16:11:50.000000000 +0200
@@ -242,6 +242,7 @@ struct hso_device {
 	u8 is_active;
 	u8 suspend_disabled;
 	u8 usb_gone;
+	u8 sleeping;
 	struct work_struct async_get_intf;
 	struct work_struct async_put_intf;
 
@@ -252,6 +253,7 @@ struct hso_device {
 	struct device *dev;
 	struct kref ref;
 	struct mutex mutex;
+	spinlock_t lock;
 
 	/* TODO: Not sure about the ones below */
 	struct proc_dir_entry *ourproc;
@@ -2184,6 +2186,7 @@ static struct hso_device *hso_create_dev
 	hso_dev->interface = intf;
 	kref_init(&hso_dev->ref);
 	mutex_init(&hso_dev->mutex);
+	spin_lock_init(&hso_dev->lock);
 
 	INIT_WORK(&hso_dev->async_get_intf, async_get_intf);
 	INIT_WORK(&hso_dev->async_put_intf, async_put_intf);
@@ -2757,12 +2760,16 @@ static void async_put_intf(struct work_s
 
 static int hso_get_activity(struct hso_device *hso_dev)
 {
-	if (hso_dev->usb->state == USB_STATE_SUSPENDED) {
+	unsigned long flags;
+
+	spin_lock_irqsave(&hso_dev->lock, flags);
+	if (hso_dev->sleeping) {
 		if (!hso_dev->is_active) {
 			hso_dev->is_active = 1;
 			schedule_work(&hso_dev->async_get_intf);
 		}
 	}
+	spin_unlock_irqrestore(&hso_dev->lock, flags);
 
 	if (hso_dev->usb->state != USB_STATE_CONFIGURED)
 		return -EAGAIN;
@@ -2774,22 +2781,32 @@ static int hso_get_activity(struct hso_d
 
 static int hso_put_activity(struct hso_device *hso_dev)
 {
-	if (hso_dev->usb->state != USB_STATE_SUSPENDED) {
+	unsigned long flags;
+
+	spin_lock_irqsave(&hso_dev->lock, flags);
+	if (hso_dev->sleeping) {
 		if (hso_dev->is_active) {
 			hso_dev->is_active = 0;
 			schedule_work(&hso_dev->async_put_intf);
+			spin_unlock_irqrestore(&hso_dev->lock, flags);
 			return -EAGAIN;
 		}
 	}
 	hso_dev->is_active = 0;
+	spin_unlock_irqrestore(&hso_dev->lock, flags);
 	return 0;
 }
 
 /* called by kernel when we need to suspend device */
 static int hso_suspend(struct usb_interface *iface, pm_message_t message)
 {
+	struct hso_device *hso_dev = usb_get_intfdata(iface);
 	int i, result = 0;
 
+	spin_lock_irq(&hso_dev->lock);
+	hso_dev->sleeping = 1;
+	spin_unlock_irq(&hso_dev->lock);
+
 	/* Stop all serial ports */
 	for (i = 0; i < HSO_SERIAL_TTY_MINORS; i++) {
 		if (serial_table[i] && (serial_table[i]->interface == iface)) {
@@ -2816,9 +2833,14 @@ out:
 /* called by kernel when we need to resume device */
 static int hso_resume(struct usb_interface *iface)
 {
+	struct hso_device *hso_dev = usb_get_intfdata(iface);
 	int i, result = 0;
 	struct hso_net *hso_net;
 
+	spin_lock_irq(&hso_dev->lock);
+	hso_dev->sleeping = 0;
+	spin_unlock_irq(&hso_dev->lock);
+
 	/* Start all serial ports */
 	for (i = 0; i < HSO_SERIAL_TTY_MINORS; i++) {
 		if (serial_table[i] && (serial_table[i]->interface == iface)) {