Re: Kingsun KS-959 USB IrDA dongle - rele ase candidate for submission (try 1)

Hi,

> +struct ks959_speedparams {
> +	__le32 baudrate;		/* baud rate, little endian */
> +	unsigned int data_bits : 2;	/* data bits - 5 (5..8) */
> +	unsigned int : 1;
> +	unsigned int stop_bits : 1;
> +	unsigned int parity_enable : 1;
> +	unsigned int parity_type : 1;
> +	unsigned int : 1;
> +	unsigned int reset : 1;
> +	__u8 reserved[3];
> +} __attribute__ ((packed));

The attribute is not needed.

> +/* Send a control request to change speed of the dongle */
> +static int ks959_change_speed(struct ks959_cb * kingsun, unsigned speed)
> +{
> +	static unsigned int supported_speeds[] = {2400, 9600, 19200, 38400,
> +		57600, 115200, 576000, 1152000, 4000000, 0};
> +	int err;
> +	unsigned int i;
> +
> +	if (kingsun->speed_setuprequest == NULL || kingsun->speed_urb == NULL)
> +		return -ENOMEM;
> +
> +	/* Check that requested speed is among the supported ones */
> +	for (i = 0; supported_speeds[i] != 0 && supported_speeds[i] != speed; i++);
> +	if (supported_speeds[i] == 0) return -EOPNOTSUPP;
> +
> +	memset(&(kingsun->speedparams), 0, sizeof(struct ks959_speedparams));
> +	kingsun->speedparams.baudrate = cpu_to_le32(speed);
> +	kingsun->speedparams.data_bits = 3;	/* 8 data bits */
> +
> +	kingsun->speed_setuprequest->bRequestType = USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE;
> +	kingsun->speed_setuprequest->bRequest = KINGSUN_REQ_SEND;
> +	kingsun->speed_setuprequest->wValue = cpu_to_le16(0x0200);
> +	kingsun->speed_setuprequest->wIndex = cpu_to_le16(0x0001);
> +	kingsun->speed_setuprequest->wLength = cpu_to_le16(8);
> +	usb_fill_control_urb(kingsun->speed_urb, kingsun->usbdev,
> +			usb_sndctrlpipe(kingsun->usbdev, 0),
> +			(unsigned char *)kingsun->speed_setuprequest,
> +			&(kingsun->speedparams), sizeof(struct ks959_speedparams),
> +			ks959_speed_irq, kingsun);
> +	kingsun->speed_urb->status = 0;
> +	err = usb_submit_urb(kingsun->speed_urb, GFP_ATOMIC);
> +
> +	return err;
> +}

Are you sure the URB doesn't need to be waited for?


> +close_irlap:
> +	irlap_close(kingsun->irlap);
> +free_mem:
> +	if (kingsun->speed_urb) {
> +		usb_free_urb(kingsun->speed_urb);
> +		kingsun->speed_urb = NULL;
> +	}
> +	if (kingsun->tx_urb) {
> +		usb_free_urb(kingsun->tx_urb);
> +		kingsun->tx_urb = NULL;
> +	}
> +	if (kingsun->rx_urb) {
> +		usb_free_urb(kingsun->rx_urb);
> +		kingsun->rx_urb = NULL;
> +	}
> +	if (kingsun->rx_unwrap_buff.skb) {
> +		kfree_skb(kingsun->rx_unwrap_buff.skb);
> +		kingsun->rx_unwrap_buff.skb = NULL;
> +		kingsun->rx_unwrap_buff.head = NULL;
> +	}
> +	return err;
> +}

usb_free_urb() can take NULL.

> +static int ks959_net_close(struct net_device *netdev)
> +{
> +	struct ks959_cb *kingsun = netdev_priv(netdev);
> +
> +	/* Stop transmit processing */
> +	netif_stop_queue(netdev);
> +
> +	/* Mop up receive && transmit urb's */
> +	usb_kill_urb(kingsun->speed_urb);
> +	usb_kill_urb(kingsun->tx_urb);
> +	usb_kill_urb(kingsun->rx_urb);

Race condition. You must kill the tx_urb first, as it can submit the speed_urb

	Regards
		Oliver

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel