[00/04] HSO driver bugfixes and updates

[00/04] HSO driver bugfixes and updates

[Greg KH]

Jeff,

Here are 4 patches against 2.6.27-rc2 for the drivers/net/usb/hso.c
driver.

The first 2 fix problems that people have been having with the driver
The last 2 are just good things to have based on code reviews from Arjan
and Andrew.

Please apply them to your tree(s) and push them upward.

thanks,

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

[01/04] hso: fix oops in read/write callbacks

[Greg KH]

From: Olivier Blin <blino@mandriva.com>

The tty may be closed already when the read/write callbacks are called.
This patch checks that the ttys still exist before waking them up.

Signed-off-by: Olivier Blin <blino@mandriva.com>
Acked-by: Alan Cox <alan@redhat.com>
Cc: Jari Tenhunen <jari.tenhunen@iki.fi>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 drivers/net/usb/hso.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -1467,7 +1467,8 @@ static void hso_std_serial_write_bulk_ca
 		return;
 	}
 	hso_put_activity(serial->parent);
-	tty_wakeup(serial->tty);
+	if (serial->tty)
+		tty_wakeup(serial->tty);
 	hso_kick_transmit(serial);
 
 	D1(" ");
@@ -1538,7 +1539,8 @@ static void ctrl_callback(struct urb *ur
 			clear_bit(HSO_SERIAL_FLAG_RX_SENT, &serial->flags);
 	} else {
 		hso_put_activity(serial->parent);
-		tty_wakeup(serial->tty);
+		if (serial->tty)
+			tty_wakeup(serial->tty);
 		/* response to a write command */
 		hso_kick_transmit(serial);
 	}

[02/04] hso: fix refcounting on the ttyHSx devices

[Greg KH]

From: Olivier Blin <blino-4qZELD6FgxhWk0Htik3J/w@public.gmane.org>


The references on ttyHSx devices were not decremented correctly when
the tty was closed. The helper freeing the serial devices was never
called because of that, and the module left some dangling sysfs
devices after being unloaded.

Signed-off-by: Olivier Blin <blino-4qZELD6FgxhWk0Htik3J/w@public.gmane.org>
Cc: Jari Tenhunen <jari.tenhunen-X3B1VOXEql0@public.gmane.org>
Signed-off-by: Greg Kroah-Hartman <gregkh-l3A5Bk7waGM@public.gmane.org>

---
 drivers/net/usb/hso.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -1103,8 +1103,8 @@ static void hso_serial_close(struct tty_
 	/* reset the rts and dtr */
 	/* do the actual close */
 	serial->open_count--;
+	kref_put(&serial->parent->ref, hso_serial_ref_free);
 	if (serial->open_count <= 0) {
-		kref_put(&serial->parent->ref, hso_serial_ref_free);
 		serial->open_count = 0;
 		if (serial->tty) {
 			serial->tty->driver_data = NULL;
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[03/04] USB: HSO: make tty_operations const

[Greg KH]

From: Greg Kroah-Hartman <gregkh@suse.de>

As recommended by Arjan.

Cc: Arjan van de Ven <arjan@infradead.org>
Cc: Andrew Bird <ajb@spheresystems.co.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 drivers/net/usb/hso.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2725,7 +2725,7 @@ static int hso_mux_submit_intr_urb(struc
 }
 
 /* operations setup of the serial interface */
-static struct tty_operations hso_serial_ops = {
+static const struct tty_operations hso_serial_ops = {
 	.open = hso_serial_open,
 	.close = hso_serial_close,
 	.write = hso_serial_write,

[04/04] USB: HSO: minor fixes due to code review

[Greg KH]

From: Greg Kroah-Hartman <gregkh@suse.de>

Fix up problems in hso.c driver as pointed out by Andrew.

Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 drivers/net/usb/hso.c |   43 ++++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 19 deletions(-)

--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -102,8 +102,12 @@
 
 #define MAX_RX_URBS			2
 
-#define get_serial_by_tty(x)	\
-	(x ? (struct hso_serial *)x->driver_data : NULL)
+static inline struct hso_serial *get_serial_by_tty(struct tty_struct *tty)
+{
+	if (tty)
+		return tty->driver_data;
+	return NULL;
+}
 
 /*****************************************************************************/
 /* Debugging functions                                                       */
@@ -294,24 +298,25 @@ static int hso_get_activity(struct hso_d
 
 /* #define DEBUG */
 
-#define dev2net(x) (x->port_data.dev_net)
-#define dev2ser(x) (x->port_data.dev_serial)
+static inline struct hso_net *dev2net(struct hso_device *hso_dev)
+{
+	return hso_dev->port_data.dev_net;
+}
+
+static inline struct hso_serial *dev2ser(struct hso_device *hso_dev)
+{
+	return hso_dev->port_data.dev_serial;
+}
 
 /* Debugging functions */
 #ifdef DEBUG
 static void dbg_dump(int line_count, const char *func_name, unsigned char *buf,
 		     unsigned int len)
 {
-	u8 i = 0;
-
-	printk(KERN_DEBUG "[%d:%s]: len %d", line_count, func_name, len);
+	static char name[255];
 
-	for (i = 0; i < len; i++) {
-		if (!(i % 16))
-			printk("\n    0x%03x:  ", i);
-		printk("%02x ", (unsigned char)buf[i]);
-	}
-	printk("\n");
+	sprintf(name, "hso[%d:%s]", line_count, func_name);
+	print_hex_dump_bytes(name, DUMP_PREFIX_NONE, buf, len);
 }
 
 #define DUMP(buf_, len_)	\
@@ -528,13 +533,12 @@ static struct hso_serial *get_serial_by_
 
 static struct hso_serial *get_serial_by_index(unsigned index)
 {
-	struct hso_serial *serial;
+	struct hso_serial *serial = NULL;
 	unsigned long flags;
 
-	if (!serial_table[index])
-		return NULL;
 	spin_lock_irqsave(&serial_table_lock, flags);
-	serial = dev2ser(serial_table[index]);
+	if (serial_table[index])
+		serial = dev2ser(serial_table[index]);
 	spin_unlock_irqrestore(&serial_table_lock, flags);
 
 	return serial;
@@ -561,6 +565,7 @@ static int get_free_serial_index(void)
 static void set_serial_by_index(unsigned index, struct hso_serial *serial)
 {
 	unsigned long flags;
+
 	spin_lock_irqsave(&serial_table_lock, flags);
 	if (serial)
 		serial_table[index] = serial->parent;
@@ -569,7 +574,7 @@ static void set_serial_by_index(unsigned
 	spin_unlock_irqrestore(&serial_table_lock, flags);
 }
 
-/* log a meaningfull explanation of an USB status */
+/* log a meaningful explanation of an USB status */
 static void log_usb_status(int status, const char *function)
 {
 	char *explanation;
@@ -2654,7 +2659,7 @@ static void hso_free_interface(struct us
 			hso_stop_net_device(network_table[i]);
 			cancel_work_sync(&network_table[i]->async_put_intf);
 			cancel_work_sync(&network_table[i]->async_get_intf);
-			if(rfk)
+			if (rfk)
 				rfkill_unregister(rfk);
 			hso_free_net_device(network_table[i]);
 		}

Re: [00/04] HSO driver bugfixes and updates

[David Brownell]

On Friday 08 August 2008, Greg KH wrote:
> Jeff,
> 
> Here are 4 patches against 2.6.27-rc2 for the drivers/net/usb/hso.c
> driver.

And here's a fifth (resent) ... just fixing brokenKconfig.

==== CUT HERE
From: David Brownell <dbrownell@users.sourceforge.net>

Move the Kconfig for the new "Option" driver so it's not in the
middle of the usbnet-based drivers, so the dependency displays
in the Kconfig user interfaces don't get trashed.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
 drivers/net/usb/Kconfig |   21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

--- a/drivers/net/usb/Kconfig	2008-07-26 17:11:01.000000000 -0700
+++ b/drivers/net/usb/Kconfig	2008-07-26 17:40:33.000000000 -0700
@@ -154,17 +154,6 @@ config USB_NET_AX8817X
 	  This driver creates an interface named "ethX", where X depends on
 	  what other networking devices you have in use.
 
-config USB_HSO
-	tristate "Option USB High Speed Mobile Devices"
-	depends on USB && RFKILL
-	default n
-	help
-	  Choose this option if you have an Option HSDPA/HSUPA card.
-	  These cards support downlink speeds of 7.2Mbps or greater.
-
-	  To compile this driver as a module, choose M here: the
-	  module will be called hso.
-
 config USB_NET_CDCETHER
 	tristate "CDC Ethernet support (smart devices such as cable modems)"
 	depends on USB_USBNET
@@ -337,5 +326,15 @@ config USB_NET_ZAURUS
 	  really need this non-conformant variant of CDC Ethernet (or in
 	  some cases CDC MDLM) protocol, not "g_ether".
 
+config USB_HSO
+	tristate "Option USB High Speed Mobile Devices"
+	depends on USB && RFKILL
+	default n
+	help
+	  Choose this option if you have an Option HSDPA/HSUPA card.
+	  These cards support downlink speeds of 7.2Mbps or greater.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called hso.
 
 endmenu

Re: [02/04] hso: fix refcounting on the ttyHSx devices

[Paul Rolland]

Hello,

This single patch fixes the problem of the left-over /dev/ttyHSx when
unplugging the Icon device from the USB port. Everything else I'm using is
still working correctly as far as I can see, so you can add a :

Tested-By: Paul Rolland <rol@as2917.net>

Thanks for that,
Regards,
Paul

On Fri, 8 Aug 2008 12:01:41 -0700
Greg KH <greg@kroah.com> wrote:

> From: Olivier Blin <blino@mandriva.com>
> 
> 
> The references on ttyHSx devices were not decremented correctly when
> the tty was closed. The helper freeing the serial devices was never
> called because of that, and the module left some dangling sysfs
> devices after being unloaded.
> 
> Signed-off-by: Olivier Blin <blino@mandriva.com>
> Cc: Jari Tenhunen <jari.tenhunen@iki.fi>
> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
> 
> ---
>  drivers/net/usb/hso.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/drivers/net/usb/hso.c
> +++ b/drivers/net/usb/hso.c
> @@ -1103,8 +1103,8 @@ static void hso_serial_close(struct tty_
>  	/* reset the rts and dtr */
>  	/* do the actual close */
>  	serial->open_count--;
> +	kref_put(&serial->parent->ref, hso_serial_ref_free);
>  	if (serial->open_count <= 0) {
> -		kref_put(&serial->parent->ref, hso_serial_ref_free);
>  		serial->open_count = 0;
>  		if (serial->tty) {
>  			serial->tty->driver_data = NULL;
> 


-- 
Paul Rolland                                E-Mail : rol(at)witbe.net
CTO - Witbe.net SA                          Tel. +33 (0)1 47 67 77 77
Les Collines de l'Arche                     Fax. +33 (0)1 47 67 77 99
F-92057 Paris La Defense                    RIPE : PR12-RIPE

Please no HTML, I'm not a browser - Pas d'HTML, je ne suis pas un navigateur 
"Some people dream of success... while others wake up and work hard at it" 

"I worry about my child and the Internet all the time, even though she's too 
young to have logged on yet. Here's what I worry about. I worry that 10 or 15 
years from now, she will come to me and say 'Daddy, where were you when they 
took freedom of the press away from the Internet?'"
--Mike Godwin, Electronic Frontier Foundation 

Re: [00/04] HSO driver bugfixes and updates

[Jeff Garzik]

David Brownell wrote:
> On Friday 08 August 2008, Greg KH wrote:
>> Jeff,
>>
>> Here are 4 patches against 2.6.27-rc2 for the drivers/net/usb/hso.c
>> driver.
> 
> And here's a fifth (resent) ... just fixing brokenKconfig.
> 
> ==== CUT HERE
> From: David Brownell <dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> 
> Move the Kconfig for the new "Option" driver so it's not in the
> middle of the usbnet-based drivers, so the dependency displays
> in the Kconfig user interfaces don't get trashed.
> 
> Signed-off-by: David Brownell <dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> ---
>  drivers/net/usb/Kconfig |   21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
> 
> --- a/drivers/net/usb/Kconfig	2008-07-26 17:11:01.000000000 -0700
> +++ b/drivers/net/usb/Kconfig	2008-07-26 17:40:33.000000000 -0700
> @@ -154,17 +154,6 @@ config USB_NET_AX8817X
>  	  This driver creates an interface named "ethX", where X depends on
>  	  what other networking devices you have in use.
>  
> -config USB_HSO
> -	tristate "Option USB High Speed Mobile Devices"
> -	depends on USB && RFKILL
> -	default n
> -	help
> -	  Choose this option if you have an Option HSDPA/HSUPA card.
> -	  These cards support downlink speeds of 7.2Mbps or greater.
> -
> -	  To compile this driver as a module, choose M here: the
> -	  module will be called hso.
> -
>  config USB_NET_CDCETHER
>  	tristate "CDC Ethernet support (smart devices such as cable modems)"
>  	depends on USB_USBNET
> @@ -337,5 +326,15 @@ config USB_NET_ZAURUS
>  	  really need this non-conformant variant of CDC Ethernet (or in
>  	  some cases CDC MDLM) protocol, not "g_ether".
>  
> +config USB_HSO
> +	tristate "Option USB High Speed Mobile Devices"
> +	depends on USB && RFKILL
> +	default n
> +	help
> +	  Choose this option if you have an Option HSDPA/HSUPA card.
> +	  These cards support downlink speeds of 7.2Mbps or greater.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called hso.
>  
>  endmenu

applied


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [04/04] USB: HSO: minor fixes due to code review

[Jeff Garzik]

Greg KH wrote:
> From: Greg Kroah-Hartman <gregkh@suse.de>
> 
> Fix up problems in hso.c driver as pointed out by Andrew.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
> 
> ---
>  drivers/net/usb/hso.c |   43 ++++++++++++++++++++++++-------------------
>  1 file changed, 24 insertions(+), 19 deletions(-)
> 
> --- a/drivers/net/usb/hso.c
> +++ b/drivers/net/usb/hso.c
> @@ -102,8 +102,12 @@
>  
>  #define MAX_RX_URBS			2
>  
> -#define get_serial_by_tty(x)	\
> -	(x ? (struct hso_serial *)x->driver_data : NULL)
> +static inline struct hso_serial *get_serial_by_tty(struct tty_struct *tty)
> +{
> +	if (tty)
> +		return tty->driver_data;
> +	return NULL;
> +}
>  
>  /*****************************************************************************/
>  /* Debugging functions                                                       */
> @@ -294,24 +298,25 @@ static int hso_get_activity(struct hso_d
>  
>  /* #define DEBUG */
>  
> -#define dev2net(x) (x->port_data.dev_net)
> -#define dev2ser(x) (x->port_data.dev_serial)
> +static inline struct hso_net *dev2net(struct hso_device *hso_dev)
> +{
> +	return hso_dev->port_data.dev_net;
> +}
> +
> +static inline struct hso_serial *dev2ser(struct hso_device *hso_dev)
> +{
> +	return hso_dev->port_data.dev_serial;
> +}
>  
>  /* Debugging functions */
>  #ifdef DEBUG
>  static void dbg_dump(int line_count, const char *func_name, unsigned char *buf,
>  		     unsigned int len)
>  {
> -	u8 i = 0;
> -
> -	printk(KERN_DEBUG "[%d:%s]: len %d", line_count, func_name, len);
> +	static char name[255];
>  
> -	for (i = 0; i < len; i++) {
> -		if (!(i % 16))
> -			printk("\n    0x%03x:  ", i);
> -		printk("%02x ", (unsigned char)buf[i]);
> -	}
> -	printk("\n");
> +	sprintf(name, "hso[%d:%s]", line_count, func_name);
> +	print_hex_dump_bytes(name, DUMP_PREFIX_NONE, buf, len);
>  }
>  
>  #define DUMP(buf_, len_)	\
> @@ -528,13 +533,12 @@ static struct hso_serial *get_serial_by_
>  
>  static struct hso_serial *get_serial_by_index(unsigned index)
>  {
> -	struct hso_serial *serial;
> +	struct hso_serial *serial = NULL;
>  	unsigned long flags;
>  
> -	if (!serial_table[index])
> -		return NULL;
>  	spin_lock_irqsave(&serial_table_lock, flags);
> -	serial = dev2ser(serial_table[index]);
> +	if (serial_table[index])
> +		serial = dev2ser(serial_table[index]);
>  	spin_unlock_irqrestore(&serial_table_lock, flags);
>  
>  	return serial;
> @@ -561,6 +565,7 @@ static int get_free_serial_index(void)
>  static void set_serial_by_index(unsigned index, struct hso_serial *serial)
>  {
>  	unsigned long flags;
> +
>  	spin_lock_irqsave(&serial_table_lock, flags);
>  	if (serial)
>  		serial_table[index] = serial->parent;
> @@ -569,7 +574,7 @@ static void set_serial_by_index(unsigned
>  	spin_unlock_irqrestore(&serial_table_lock, flags);
>  }
>  
> -/* log a meaningfull explanation of an USB status */
> +/* log a meaningful explanation of an USB status */
>  static void log_usb_status(int status, const char *function)
>  {
>  	char *explanation;
> @@ -2654,7 +2659,7 @@ static void hso_free_interface(struct us
>  			hso_stop_net_device(network_table[i]);
>  			cancel_work_sync(&network_table[i]->async_put_intf);
>  			cancel_work_sync(&network_table[i]->async_get_intf);
> -			if(rfk)
> +			if (rfk)
>  				rfkill_unregister(rfk);

applied 1-4