Re: [PATCH] ipaq.c bugfixes

[Greg KH <gregkh@suse.de>]

On Thu, Jun 01, 2006 at 12:42:45AM +0200, Frank Gevaerts wrote:
> This patch fixes several problems in the ipaq.c driver with connecting
> and disconnecting pocketpc devices: 
> * The read urb stayed active if the connect failed, causing nullpointer
>   dereferences later on. 
> * If a write failed, the driver continued as if nothing happened. Now it
>   handles that case the same way as other usb serial devices (fix by 
>   "Luiz Fernando N. Capitulino" <lcapitulino@mandriva.com.br>)
> 
> The connect_retries parameter is added because if a pocketpc device is
> connected while it is rebooting, it can take a long time after the USB
> connect (sometimes several minutes) before it starts accepting the
> control packet that starts the serial connection. Since this is not the
> normal usecase, it is probably better to leave the default number of
> retries as-is.
> 
> Signed-off-by: Frank Gevaerts <frank.gevaerts@fks.be>
> 
> diff -pur linux-2.6.17-rc4/drivers/usb/serial/ipaq.c linux-2.6.17-rc4.test/drivers/usb/serial/ipaq.c
> --- linux-2.6.17-rc4/drivers/usb/serial/ipaq.c	2006-03-20 06:53:29.000000000 +0100
> +++ linux-2.6.17-rc4.test/drivers/usb/serial/ipaq.c	2006-05-30 20:46:23.000000000 +0200
> @@ -71,6 +71,7 @@
>  
>  static __u16 product, vendor;
>  static int debug;
> +static int connect_retries;
>  
>  /* Function prototypes for an ipaq */
>  static int  ipaq_open (struct usb_serial_port *port, struct file *filp);
> @@ -583,7 +584,7 @@ static int ipaq_open(struct usb_serial_p
>  	struct ipaq_private	*priv;
>  	struct ipaq_packet	*pkt;
>  	int			i, result = 0;
> -	int			retries = KP_RETRIES;
> +	int			retries = connect_retries;
>  
>  	dbg("%s - port %d", __FUNCTION__, port->number);
>  
> @@ -681,6 +682,7 @@ enomem:
>  	result = -ENOMEM;
>  	err("%s - Out of memory", __FUNCTION__);
>  error:
> +	usb_kill_urb(port->read_urb);
>  	ipaq_destroy_lists(port);
>  	kfree(priv);
>  	return result;
> @@ -855,6 +857,7 @@ static void ipaq_write_bulk_callback(str
>  	
>  	if (urb->status) {
>  		dbg("%s - nonzero write bulk status received: %d", __FUNCTION__, urb->status);
> +		return;
>  	}
>  
>  	spin_lock_irqsave(&write_list_lock, flags);
> @@ -967,3 +970,6 @@ MODULE_PARM_DESC(vendor, "User specified
>  
>  module_param(product, ushort, 0);
>  MODULE_PARM_DESC(product, "User specified USB idProduct");
> +
> +module_param(connect_retries, int, KP_RETRIES);

I really do not think that you want KP_RETRIES as a mode value in sysfs
:)

This is not how you pre-initialize a module parameter...

thanks,

greg k-h