[PATCH] w35und: fix usb_control_msg() error handling in wb35_probe()

[PATCH] w35und: fix usb_control_msg() error handling in wb35_probe()

[Pekka J Enberg]

From: Pekka Enberg <penberg@cs.helsinki.fi>

If successful, the usb_control_msg() function returns the number of
bytes transferred. Fix up wb35_probe() to only bail out if the function
returns a negative number.

Reported-by: Sandro Bonazzola <sandro.bonazzola@gmail.com>
Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---
 drivers/staging/winbond/wbusb.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/winbond/wbusb.c b/drivers/staging/winbond/wbusb.c
index fd5c8eb..7ced028 100644
--- a/drivers/staging/winbond/wbusb.c
+++ b/drivers/staging/winbond/wbusb.c
@@ -302,17 +302,19 @@ static int wb35_probe(struct usb_interface *intf,
 	struct usb_device *udev = interface_to_usbdev(intf);
 	struct wbsoft_priv *priv;
 	struct ieee80211_hw *dev;
-	int err;
+	int err, nr;
 
 	usb_get_dev(udev);
 
 	/* Check if the device has already been opened */
-	err = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
-			      0x01,
-			      USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
-			      0x0, 0x400, &ltmp, 4, HZ * 100);
-	if (err)
+	nr = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
+			     0x01,
+			     USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
+			     0x0, 0x400, &ltmp, 4, HZ * 100);
+	if (nr < 0) {
+		err = nr;
 		goto error;
+	}
 
 	/* Is already initialized? */
 	ltmp = cpu_to_le32(ltmp);
-- 
1.5.6.4

Re: [PATCH] w35und: fix usb_control_msg() error handling in wb35_probe()

[Pavel Machek]

> From: Pekka Enberg <penberg@cs.helsinki.fi>
> 
> If successful, the usb_control_msg() function returns the number of
> bytes transferred. Fix up wb35_probe() to only bail out if the function
> returns a negative number.
> 
> Reported-by: Sandro Bonazzola <sandro.bonazzola@gmail.com>
> Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>

Acked-by: Pavel Machek <pavel@suse.cz>

> ---
>  drivers/staging/winbond/wbusb.c |   14 ++++++++------
>  1 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/winbond/wbusb.c b/drivers/staging/winbond/wbusb.c
> index fd5c8eb..7ced028 100644
> --- a/drivers/staging/winbond/wbusb.c
> +++ b/drivers/staging/winbond/wbusb.c
> @@ -302,17 +302,19 @@ static int wb35_probe(struct usb_interface *intf,
>  	struct usb_device *udev = interface_to_usbdev(intf);
>  	struct wbsoft_priv *priv;
>  	struct ieee80211_hw *dev;
> -	int err;
> +	int err, nr;
>  
>  	usb_get_dev(udev);
>  
>  	/* Check if the device has already been opened */
> -	err = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
> -			      0x01,
> -			      USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
> -			      0x0, 0x400, &ltmp, 4, HZ * 100);
> -	if (err)
> +	nr = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
> +			     0x01,
> +			     USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
> +			     0x0, 0x400, &ltmp, 4, HZ * 100);
> +	if (nr < 0) {
> +		err = nr;
>  		goto error;
> +	}
>  
>  	/* Is already initialized? */
>  	ltmp = cpu_to_le32(ltmp);

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] w35und: fix usb_control_msg() error handling in wb35_probe()

[Sandro Bonazzola]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Pavel Machek ha scritto:
>> From: Pekka Enberg <penberg@cs.helsinki.fi>
>>
>> If successful, the usb_control_msg() function returns the number of
>> bytes transferred. Fix up wb35_probe() to only bail out if the function
>> returns a negative number.
>>
>> Reported-by: Sandro Bonazzola <sandro.bonazzola@gmail.com>
>> Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
> 
> Acked-by: Pavel Machek <pavel@suse.cz>

Ok, tested. Here is the result:

# uname -a
Linux arilinn 2.6.29-rc2-00013-gf3b8436-dirty #1 Thu Jan 22 19:39:23 CET 2009
x86_64 AMD Athlon(tm) 64 Processor 3500+ AuthenticAMD GNU/Linux

Inserting the USB device:
# dmesg
usb 1-3: new high speed USB device using ehci_hcd and address 4
usb 1-3: New USB device found, idVendor=18e8, idProduct=6201
usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3
usb 1-3: Product: Usb2Wlan
usb 1-3: Manufacturer: WINBOND
usb 1-3: SerialNumber: 101d350112
usb 1-3: configuration #1 chosen from 1 choice
w35und: module is from the staging directory, the quality is unknown, you have
been warned.
wmaster0 (usb): not using net_device_ops yet
phy0: Selected rate control algorithm 'minstrel'
usbcore: registered new interface driver w35und

#  iwlist wlan0 scan
wlan0     Interface doesn't support scanning.

#  iwlist wmaster0 scan
wmaster0  Interface doesn't support scanning.

# ifconfig wlan0 10.0.4.1
SIOCSIFADDR: No such device
wlan0: ERROR while getting interface flags: No such device

# ifconfig wmaster0 10.0.4.1
SIOCSIFFLAGS: Operation not supported

# lsusb
Bus 002 Device 003: ID 045e:00f5 Microsoft Corp. LifeCam VX-3000
Bus 002 Device 002: ID 046d:c505 Logitech, Inc. Cordless Mouse+Keyboard Receiver
Bus 002 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 001 Device 004: ID 18e8:6201 Qcom
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub

#  lsusb -v -s 001:004

Bus 001 Device 004: ID 18e8:6201 Qcom
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass            0 (Defined at Interface level)
  bDeviceSubClass         0
  bDeviceProtocol         0
  bMaxPacketSize0        64
  idVendor           0x18e8 Qcom
  idProduct          0x6201
  bcdDevice           87.65
  iManufacturer           1 WINBOND
  iProduct                2 Usb2Wlan
  iSerial                 3 101d350112
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength           49
    bNumInterfaces          1
    bConfigurationValue     1
    iConfiguration          0
    bmAttributes         0x80
      (Bus Powered)
    MaxPower              500mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           4
      bInterfaceClass         0 (Defined at Interface level)
      bInterfaceSubClass      0
      bInterfaceProtocol      0
      iInterface              0
      ** UNRECOGNIZED:  03 08 3f
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               1
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x82  EP 2 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               1
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x83  EP 3 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval               0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x04  EP 4 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval               1
Device Qualifier (for other device speed):
  bLength                10
  bDescriptorType         6
  bcdUSB               2.00
  bDeviceClass            0 (Defined at Interface level)
  bDeviceSubClass         0
  bDeviceProtocol         0
  bMaxPacketSize0        64
  bNumConfigurations      1
Device Status:     0x0000
  (Bus Powered)


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkl4yBEACgkQV871CybFezntuQCguBg2n5aFYv0MQ1+P5bRcNw4Q
LosAnRwszYu076vzA2PglXC6Pub0ofvs
=zuIX
-----END PGP SIGNATURE-----

Re: [PATCH] w35und: fix usb_control_msg() error handling in wb35_probe()

[Pavel Machek]

On Thu 2009-01-22 20:25:06, Sandro Bonazzola wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Pavel Machek ha scritto:
> >> From: Pekka Enberg <penberg@cs.helsinki.fi>
> >>
> >> If successful, the usb_control_msg() function returns the number of
> >> bytes transferred. Fix up wb35_probe() to only bail out if the function
> >> returns a negative number.
> >>
> >> Reported-by: Sandro Bonazzola <sandro.bonazzola@gmail.com>
> >> Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
> > 
> > Acked-by: Pavel Machek <pavel@suse.cz>
> 
> Ok, tested. Here is the result:
> 
> # uname -a
> Linux arilinn 2.6.29-rc2-00013-gf3b8436-dirty #1 Thu Jan 22 19:39:23 CET 2009
> x86_64 AMD Athlon(tm) 64 Processor 3500+ AuthenticAMD GNU/Linux
> 
> Inserting the USB device:
> # dmesg
> usb 1-3: new high speed USB device using ehci_hcd and address 4
> usb 1-3: New USB device found, idVendor=18e8, idProduct=6201
> usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> usb 1-3: Product: Usb2Wlan
> usb 1-3: Manufacturer: WINBOND
> usb 1-3: SerialNumber: 101d350112
> usb 1-3: configuration #1 chosen from 1 choice
> w35und: module is from the staging directory, the quality is unknown, you have
> been warned.
> wmaster0 (usb): not using net_device_ops yet
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This looks like a key clue...

Maybe you could try _current_ w35und on 2.6.28 or something like that?
I have feeling that networking core changed in incompatible way here.

								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] w35und: fix usb_control_msg() error handling in wb35_probe()

[Harvey Harrison]

On Thu, 2009-01-22 at 22:10 +0100, Pavel Machek wrote:
> On Thu 2009-01-22 20:25:06, Sandro Bonazzola wrote:
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA1
> > 
> > Pavel Machek ha scritto:
> > >> From: Pekka Enberg <penberg@cs.helsinki.fi>
> > >>
> > >> If successful, the usb_control_msg() function returns the number of
> > >> bytes transferred. Fix up wb35_probe() to only bail out if the function
> > >> returns a negative number.
> > >>
> > >> Reported-by: Sandro Bonazzola <sandro.bonazzola@gmail.com>
> > >> Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
> > > 
> > > Acked-by: Pavel Machek <pavel@suse.cz>
> > 
> > Ok, tested. Here is the result:
> > 
> > # uname -a
> > Linux arilinn 2.6.29-rc2-00013-gf3b8436-dirty #1 Thu Jan 22 19:39:23 CET 2009
> > x86_64 AMD Athlon(tm) 64 Processor 3500+ AuthenticAMD GNU/Linux
> > 
> > Inserting the USB device:
> > # dmesg
> > usb 1-3: new high speed USB device using ehci_hcd and address 4
> > usb 1-3: New USB device found, idVendor=18e8, idProduct=6201
> > usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> > usb 1-3: Product: Usb2Wlan
> > usb 1-3: Manufacturer: WINBOND
> > usb 1-3: SerialNumber: 101d350112
> > usb 1-3: configuration #1 chosen from 1 choice
> > w35und: module is from the staging directory, the quality is unknown, you have
> > been warned.
> > wmaster0 (usb): not using net_device_ops yet
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> This looks like a key clue...
> 
> Maybe you could try _current_ w35und on 2.6.28 or something like that?
> I have feeling that networking core changed in incompatible way here.
> 

I thought the net-device-ops conversion was opt in?  It shouldn't break
anything if the driver hasn't been converted yet.

Harvey

Re: [PATCH] w35und: fix usb_control_msg() error handling in wb35_probe()

[Pekka Enberg]

Sandro Bonazzola wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Pavel Machek ha scritto:
> 
>> Maybe you could try _current_ w35und on 2.6.28 or something like that?
>> I have feeling that networking core changed in incompatible way here.
> 
> Ok, test done with git current version of w35und over 2.6.28-gentoo-r1 (2.6.28.1
> and some patches)
> 
> # uname -a
> Linux arilinn 2.6.28-gentoo-r1 #1 Wed Jan 21 19:37:07 CET 2009 x86_64 AMD
> Athlon(tm) 64 Processor 3500+ AuthenticAMD GNU/Linux
> 
> # dmesg
> usb 1-3: new high speed USB device using ehci_hcd and address 4
> usb 1-3: configuration #1 chosen from 1 choice
> usb 1-3: New USB device found, idVendor=18e8, idProduct=6201
> usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> usb 1-3: Product: Usb2Wlan
> usb 1-3: Manufacturer: WINBOND
> usb 1-3: SerialNumber: 101d350112
> w35und: module is from the staging directory, the quality is unknown, you have
> been warned.
> w35und: probe of 1-3:1.0 failed with error 4

Yeah, my patch is not in -git yet so you still need to apply it manually 
to get over this part.

> usbcore: registered new interface driver w35und
> 
> I'll be away untill monday.
> 
> - --
> Sandro
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2.0.9 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
> 
> iEYEARECAAYFAkl49HwACgkQV871CybFezmbDACgmYRXmSVpXePYuNndXDf0x1WO
> UvUAn3e456zicd7M05KCLQlXyoUlRtGg
> =JW6n
> -----END PGP SIGNATURE-----

Re: [PATCH] w35und: fix usb_control_msg() error handling in wb35_probe()

[Pekka Enberg]

On Thu, Jan 22, 2009 at 11:19 PM, Harvey Harrison
<harvey.harrison@gmail.com> wrote:
> On Thu, 2009-01-22 at 22:10 +0100, Pavel Machek wrote:
>> On Thu 2009-01-22 20:25:06, Sandro Bonazzola wrote:
>> > -----BEGIN PGP SIGNED MESSAGE-----
>> > Hash: SHA1
>> >
>> > Pavel Machek ha scritto:
>> > >> From: Pekka Enberg <penberg@cs.helsinki.fi>
>> > >>
>> > >> If successful, the usb_control_msg() function returns the number of
>> > >> bytes transferred. Fix up wb35_probe() to only bail out if the function
>> > >> returns a negative number.
>> > >>
>> > >> Reported-by: Sandro Bonazzola <sandro.bonazzola@gmail.com>
>> > >> Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
>> > >
>> > > Acked-by: Pavel Machek <pavel@suse.cz>
>> >
>> > Ok, tested. Here is the result:
>> >
>> > # uname -a
>> > Linux arilinn 2.6.29-rc2-00013-gf3b8436-dirty #1 Thu Jan 22 19:39:23 CET 2009
>> > x86_64 AMD Athlon(tm) 64 Processor 3500+ AuthenticAMD GNU/Linux
>> >
>> > Inserting the USB device:
>> > # dmesg
>> > usb 1-3: new high speed USB device using ehci_hcd and address 4
>> > usb 1-3: New USB device found, idVendor=18e8, idProduct=6201
>> > usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3
>> > usb 1-3: Product: Usb2Wlan
>> > usb 1-3: Manufacturer: WINBOND
>> > usb 1-3: SerialNumber: 101d350112
>> > usb 1-3: configuration #1 chosen from 1 choice
>> > w35und: module is from the staging directory, the quality is unknown, you have
>> > been warned.
>> > wmaster0 (usb): not using net_device_ops yet
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> This looks like a key clue...
>>
>> Maybe you could try _current_ w35und on 2.6.28 or something like that?
>> I have feeling that networking core changed in incompatible way here.
>>
>
> I thought the net-device-ops conversion was opt in?  It shouldn't break
> anything if the driver hasn't been converted yet.

Looking at commit d314774cf2cd5dfeb39a00d37deee65d4c627927 ("netdev:
network device operations infrastructure") that seems to be the case.

                                  Pekka

Re: [PATCH] w35und: fix usb_control_msg() error handling in wb35_probe()

[Sandro Bonazzola]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Pavel Machek ha scritto:

> Maybe you could try _current_ w35und on 2.6.28 or something like that?
> I have feeling that networking core changed in incompatible way here.

Ok, test done with git current version of w35und over 2.6.28-gentoo-r1 (2.6.28.1
and some patches)

# uname -a
Linux arilinn 2.6.28-gentoo-r1 #1 Wed Jan 21 19:37:07 CET 2009 x86_64 AMD
Athlon(tm) 64 Processor 3500+ AuthenticAMD GNU/Linux

# dmesg
usb 1-3: new high speed USB device using ehci_hcd and address 4
usb 1-3: configuration #1 chosen from 1 choice
usb 1-3: New USB device found, idVendor=18e8, idProduct=6201
usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3
usb 1-3: Product: Usb2Wlan
usb 1-3: Manufacturer: WINBOND
usb 1-3: SerialNumber: 101d350112
w35und: module is from the staging directory, the quality is unknown, you have
been warned.
w35und: probe of 1-3:1.0 failed with error 4
usbcore: registered new interface driver w35und

I'll be away untill monday.

- --
Sandro
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkl49HwACgkQV871CybFezmbDACgmYRXmSVpXePYuNndXDf0x1WO
UvUAn3e456zicd7M05KCLQlXyoUlRtGg
=JW6n
-----END PGP SIGNATURE-----

Re: [PATCH] w35und: fix usb_control_msg() error handling in wb35_probe()

[Pekka Enberg]

On Thu, Jan 22, 2009 at 9:25 PM, Sandro Bonazzola
<sandro.bonazzola@gmail.com> wrote:
> #  iwlist wlan0 scan
> wlan0     Interface doesn't support scanning.
>
> #  iwlist wmaster0 scan
> wmaster0  Interface doesn't support scanning.
>
> # ifconfig wlan0 10.0.4.1
> SIOCSIFADDR: No such device
> wlan0: ERROR while getting interface flags: No such device
>
> # ifconfig wmaster0 10.0.4.1
> SIOCSIFFLAGS: Operation not supported

Can you run that under strace? Looks to me like there indeed is a
wmaster0 device but there's something fishy going on how we initialize
it.

Re: [PATCH] w35und: fix usb_control_msg() error handling in wb35_probe()

[Greg KH]

On Thu, Jan 22, 2009 at 12:06:33PM +0200, Pekka J Enberg wrote:
> From: Pekka Enberg <penberg@cs.helsinki.fi>
> 
> If successful, the usb_control_msg() function returns the number of
> bytes transferred. Fix up wb35_probe() to only bail out if the function
> returns a negative number.
> 
> Reported-by: Sandro Bonazzola <sandro.bonazzola@gmail.com>
> Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>

I'm confused, should I apply this patch or not?

thanks,

greg k-h

Re: [PATCH] w35und: fix usb_control_msg() error handling in wb35_probe()

[Pekka Enberg]

Greg KH wrote:
> On Thu, Jan 22, 2009 at 12:06:33PM +0200, Pekka J Enberg wrote:
>> From: Pekka Enberg <penberg@cs.helsinki.fi>
>>
>> If successful, the usb_control_msg() function returns the number of
>> bytes transferred. Fix up wb35_probe() to only bail out if the function
>> returns a negative number.
>>
>> Reported-by: Sandro Bonazzola <sandro.bonazzola@gmail.com>
>> Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
> 
> I'm confused, should I apply this patch or not?

Yes, you should. What's confusing you here?

		Pekka

Re: [PATCH] w35und: fix usb_control_msg() error handling in wb35_probe()

[Pekka Enberg]

On Mon, Jan 26, 2009 at 8:03 PM, Pekka Enberg <penberg@cs.helsinki.fi> wrote:
> Greg KH wrote:
>>
>> On Thu, Jan 22, 2009 at 12:06:33PM +0200, Pekka J Enberg wrote:
>>>
>>> From: Pekka Enberg <penberg@cs.helsinki.fi>
>>>
>>> If successful, the usb_control_msg() function returns the number of
>>> bytes transferred. Fix up wb35_probe() to only bail out if the function
>>> returns a negative number.
>>>
>>> Reported-by: Sandro Bonazzola <sandro.bonazzola@gmail.com>
>>> Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
>>
>> I'm confused, should I apply this patch or not?
>
> Yes, you should. What's confusing you here?

Oh, looking at the thread, I see how you got confused. The patch
doesn't fix all of Sandro's problems but is an obvious improvement so
it should be merged.

Re: [PATCH] w35und: fix usb_control_msg() error handling in wb35_probe()

[Sandro Bonazzola]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Pekka Enberg ha scritto:
> Ok, test done with git current version of w35und over 2.6.28-gentoo-r1
> (2.6.28.1
> and some patches)

Adding Pekka's patch

> # uname -a
> Linux arilinn 2.6.28-gentoo-r1 #1 Wed Jan 21 19:37:07 CET 2009 x86_64 AMD
> Athlon(tm) 64 Processor 3500+ AuthenticAMD GNU/Linux


# dmesg
skge eth0: Link is down.
skge eth0: Link is up at 100 Mbps, full duplex, flow control none
usb 1-3: new high speed USB device using ehci_hcd and address 4
usb 1-3: configuration #1 chosen from 1 choice
usb 1-3: New USB device found, idVendor=18e8, idProduct=6201
usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3
usb 1-3: Product: Usb2Wlan
usb 1-3: Manufacturer: WINBOND
usb 1-3: SerialNumber: 101d350112
w35und: module is from the staging directory, the quality is unknown, you have
been warned.
phy0: Selected rate control algorithm 'minstrel'
usbcore: registered new interface driver w35und


# iwlist wlan0 scan
wlan0     Interface doesn't support scanning : Network is down

# ifconfig wlan0 10.0.4.1

# ifconfig wlan0
wlan0     Link encap:Ethernet  HWaddr 00:0d:f0:2e:42:e7
          inet addr:10.0.4.1  Bcast:10.255.255.255  Mask:255.0.0.0
          UP BROADCAST MULTICAST  MTU:1500  Metric:1
          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
          TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:1000
          RX bytes:0 (0.0 B)  TX bytes:0 (0.0 B)

# iwlist scan
lo        Interface doesn't support scanning.

eth0      Interface doesn't support scanning.

eth1      Interface doesn't support scanning.

ppp0      Interface doesn't support scanning.

wmaster0  Interface doesn't support scanning.

wlan0     Scan completed :
          Cell 01 - Address: 02:A6:21:59:01:00
                    ESSID:"home"
                    Mode:Ad-Hoc
                    Channel:1
                    Frequency:2.412 GHz (Channel 1)
                    Quality:0  Signal level:0  Noise level:0
                    Encryption key:off
                    IE: Unknown: 0004686F6D65
                    IE: Unknown: 010482848B96
                    IE: Unknown: 030101
                    IE: Unknown: 06020000
                    IE: Unknown: 00B247DA2800218B0000000040420F000000000080841E
                    Bit Rates:1 Mb/s; 2 Mb/s; 5.5 Mb/s; 11 Mb/s
                    Extra:tsf=0000000004a83302
                    Extra: Last beacon: 161ms ago


# ping 10.0.4.1
PING 10.0.4.1 (10.0.4.1) 56(84) bytes of data.
64 bytes from 10.0.4.1: icmp_seq=1 ttl=64 time=0.064 ms
64 bytes from 10.0.4.1: icmp_seq=2 ttl=64 time=0.060 ms


#dmesg:
usb 1-3: new high speed USB device using ehci_hcd and address 4
usb 1-3: configuration #1 chosen from 1 choice
usb 1-3: New USB device found, idVendor=18e8, idProduct=6201
usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3
usb 1-3: Product: Usb2Wlan
usb 1-3: Manufacturer: WINBOND
usb 1-3: SerialNumber: 101d350112
w35und: module is from the staging directory, the quality is unknown, you have
been warned.
phy0: Selected rate control algorithm 'minstrel'
usbcore: registered new interface driver w35und
wbsoft_add interface called
wbsoft_config called
Going to channel: 1/1
Should call ether_crc here
Should call ether_crc here
Should call ether_crc here
ADDRCONF(NETDEV_UP): wlan0: link is not ready
Should call ether_crc here
Should call ether_crc here
Should call ether_crc here
Should call ether_crc here
Should call ether_crc here
Should call ether_crc here
Should call ether_crc here
Should call ether_crc here
wbsoft_config called
Going to channel: 1/1
wbsoft_config called
Going to channel: 1/1
Should call ether_crc here
Should call ether_crc here
Should call ether_crc here
skge eth0: Link is down.
Should call ether_crc here
Should call ether_crc here
Should call ether_crc here
wbsoft_config called
Going to channel: 1/1
wbsoft_config called
Going to channel: 1/1
Should call ether_crc here
Should call ether_crc here
Should call ether_crc here
Should call ether_crc here
Should call ether_crc here
Should call ether_crc here
wbsoft_config called
Going to channel: 1/1
wbsoft_config called
Going to channel: 1/1
Should call ether_crc here
Should call ether_crc here
Should call ether_crc here
Should call ether_crc here
Should call ether_crc here
Should call ether_crc here
wbsoft_config called
Going to channel: 1/1
wbsoft_config called
Going to channel: 1/1
Should call ether_crc here
Should call ether_crc here
Should call ether_crc here
Should call ether_crc here
Should call ether_crc here
Should call ether_crc here
wbsoft_config called
Going to channel: 1/1
wbsoft_config called
Going to channel: 1/1
Should call ether_crc here
Should call ether_crc here
Should call ether_crc here



Ok, with kernel 2.6.8.1, and your patch it seems to work with channel 1, no
encryption, opensystem authentication.

- --
Sandro
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkl+D5QACgkQV871CybFezluagCcCzepY3jKrRxXKyg/YdvMfwQ0
ixQAn0eTkk5/cyblnhvsobBfjPsATAhm
=O5Wx
-----END PGP SIGNATURE-----

Re: [PATCH] w35und: fix usb_control_msg() error handling in wb35_probe()

[Pekka Enberg]

Hi Sandro,

Pekka Enberg ha scritto:
>> Ok, test done with git current version of w35und over 2.6.28-gentoo-r1
>> (2.6.28.1 and some patches)
>
> Adding Pekka's patch

On Mon, Jan 26, 2009 at 9:31 PM, Sandro Bonazzola
<sandro.bonazzola@gmail.com> wrote:
>> # uname -a
>> Linux arilinn 2.6.28-gentoo-r1 #1 Wed Jan 21 19:37:07 CET 2009 x86_64 AMD
>> Athlon(tm) 64 Processor 3500+ AuthenticAMD GNU/Linux

[snip]

> Ok, with kernel 2.6.8.1, and your patch it seems to work with channel 1, no
> encryption, opensystem authentication.

So the driver doesn't work in 2.6.29-rc1 but does in 2.6.28.1? I
wonder what changed in the networking/wireless stack that broke the
winbond driver.... I'm cc'ing netdev even though this is a driver in
staging.

                                     Pekka

Re: [PATCH] w35und: fix usb_control_msg() error handling in wb35_probe()

[Pekka Enberg]

On Mon, Jan 26, 2009 at 9:40 PM, Pekka Enberg <penberg@cs.helsinki.fi> wrote:
> Hi Sandro,
>
> Pekka Enberg ha scritto:
>>> Ok, test done with git current version of w35und over 2.6.28-gentoo-r1
>>> (2.6.28.1 and some patches)
>>
>> Adding Pekka's patch
>
> On Mon, Jan 26, 2009 at 9:31 PM, Sandro Bonazzola
> <sandro.bonazzola@gmail.com> wrote:
>>> # uname -a
>>> Linux arilinn 2.6.28-gentoo-r1 #1 Wed Jan 21 19:37:07 CET 2009 x86_64 AMD
>>> Athlon(tm) 64 Processor 3500+ AuthenticAMD GNU/Linux
>
> [snip]
>
>> Ok, with kernel 2.6.8.1, and your patch it seems to work with channel 1, no
>> encryption, opensystem authentication.
>
> So the driver doesn't work in 2.6.29-rc1 but does in 2.6.28.1? I
> wonder what changed in the networking/wireless stack that broke the
> winbond driver.... I'm cc'ing netdev even though this is a driver in
> staging.

Actually, it's probably me who broke the driver. I think Greg merged
most of my patches after 2.6.28 came out. Sandro, if you have some
time, please consider doing git bisect to find out which changeset
causes the problem.

                            Pekka

Re: [PATCH] w35und: fix usb_control_msg() error handling in wb35_probe()

[Sandro Bonazzola]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Pekka Enberg ha scritto:

>>> Ok, with kernel 2.6.8.1, and your patch it seems to work with channel 1, no
>>> encryption, opensystem authentication.
>> So the driver doesn't work in 2.6.29-rc1 but does in 2.6.28.1? I
>> wonder what changed in the networking/wireless stack that broke the
>> winbond driver.... I'm cc'ing netdev even though this is a driver in
>> staging.

Well, I don't know if this can be somehow related to a bug in the driver, but on
my system, after I've unplugged the USB trasnsitter, ksoftirqd/0 begin to use
89% of my CPU time (top). I suspect that there are some issue that should be
fixed somewhere.


> Actually, it's probably me who broke the driver. I think Greg merged
> most of my patches after 2.6.28 came out. Sandro, if you have some
> time, please consider doing git bisect to find out which changeset
> causes the problem.

Well, actually my 2.6.29 copy seems to be broken:
arch/x86/kernel/amd_iommu.o: In function `debugfs_create_size_t':
amd_iommu.c:(.text+0x0): multiple definition of `debugfs_create_size_t'
arch/x86/kernel/kdebugfs.o:kdebugfs.c:(.text+0x0): first defined here
make[1]: *** [arch/x86/kernel/built-in.o] Error 1
make: *** [arch/x86/kernel] Error 2

# git pull
Already up-to-date.

# git status
# On branch master
# Changed but not updated:
#   (use "git add <file>..." to update what will be committed)
#   (use "git checkout -- <file>..." to discard changes in working directory)
#
#       modified:   drivers/staging/winbond/wbusb.c
#
no changes added to commit (use "git add" and/or "git commit -a")


I don't know too much how to use git to do what you're asking. I've some years
of experience with svn and cvs but git is quite a new tool for me.
I'll wait a git kernel that compile while I'll try to find out how to do what
you're asking.
I have not so much free time to spend in a checkout / build / test if the test
phase need a kernel reinstall / system reboot. If it's only the driver module I
think I can try to test it for each changeset in a couple of weeks /starting
from the day I know how to bisect :-) ).

- --
Sandro
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkl+GWIACgkQV871CybFezlRXACfQTBrMm2hL9b+o5oU1/6/xZ+f
gaYAnjyyawOFPBPIe/scq5pYupNrRLa3
=d1SV
-----END PGP SIGNATURE-----

Re: [PATCH] w35und: fix usb_control_msg() error handling in wb35_probe()

[Pekka Enberg]

Sandro Bonazzola wrote:
> Well, I don't know if this can be somehow related to a bug in the driver, but on
> my system, after I've unplugged the USB trasnsitter, ksoftirqd/0 begin to use
> 89% of my CPU time (top). I suspect that there are some issue that should be
> fixed somewhere.

Heh, yes, there's plenty of things to fix. :-)

>> Actually, it's probably me who broke the driver. I think Greg merged
>> most of my patches after 2.6.28 came out. Sandro, if you have some
>> time, please consider doing git bisect to find out which changeset
>> causes the problem.
> 
> Well, actually my 2.6.29 copy seems to be broken:
> arch/x86/kernel/amd_iommu.o: In function `debugfs_create_size_t':
> amd_iommu.c:(.text+0x0): multiple definition of `debugfs_create_size_t'
> arch/x86/kernel/kdebugfs.o:kdebugfs.c:(.text+0x0): first defined here
> make[1]: *** [arch/x86/kernel/built-in.o] Error 1
> make: *** [arch/x86/kernel] Error 2
> 
> # git pull
> Already up-to-date.
> 
> # git status
> # On branch master
> # Changed but not updated:
> #   (use "git add <file>..." to update what will be committed)
> #   (use "git checkout -- <file>..." to discard changes in working directory)
> #
> #       modified:   drivers/staging/winbond/wbusb.c
> #
> no changes added to commit (use "git add" and/or "git commit -a")
> 
> 
> I don't know too much how to use git to do what you're asking. I've some years
> of experience with svn and cvs but git is quite a new tool for me.
> I'll wait a git kernel that compile while I'll try to find out how to do what
> you're asking.

Here's a tutorial on git bisect.

http://kernel.org/pub/software/scm/git/docs/v1.3.3/howto/isolate-bugs-with-bisect.txt

> I have not so much free time to spend in a checkout / build / test if the test
> phase need a kernel reinstall / system reboot. If it's only the driver module I
> think I can try to test it for each changeset in a couple of weeks /starting
> from the day I know how to bisect :-) ).

Assuming 2.6.29-rc1 is broken, you initialize the bisection with:

   git bisect start -- drivers/staging/winbond/
   git bisect good v2.6.28
   git bisect bad v2.6.29-rc1

Then you continue doing this:

   <apply my patch, compile, test>

and

   git bisect good

or

   git bisect bad

depending on whether the kernel worked or not. Repeat until git bisect 
tells you which changeset broke the kernel. The process shouldn't take 
more than 5 compiles/reboots.

		Pekka

Re: [PATCH] w35und: fix usb_control_msg() error handling in wb35_probe()

[Pavel Machek]

Hi!

> > Ok, test done with git current version of w35und over 2.6.28-gentoo-r1
> > (2.6.28.1
> > and some patches)
> 
> Adding Pekka's patch

> # ping 10.0.4.1
> PING 10.0.4.1 (10.0.4.1) 56(84) bytes of data.
> 64 bytes from 10.0.4.1: icmp_seq=1 ttl=64 time=0.064 ms
> 64 bytes from 10.0.4.1: icmp_seq=2 ttl=64 time=0.060 ms
......

> Ok, with kernel 2.6.8.1, and your patch it seems to work with channel 1, no
> encryption, opensystem authentication.


Congratulation, you have made it :-). Now, question is why it does not
work on 2.6.29-rc1. And with your testing we know something in core
kernel changed and broke it.

Thanks!
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] w35und: fix usb_control_msg() error handling in wb35_probe()

[Greg KH]

On Mon, Jan 26, 2009 at 08:10:42PM +0200, Pekka Enberg wrote:
> On Mon, Jan 26, 2009 at 8:03 PM, Pekka Enberg <penberg@cs.helsinki.fi> wrote:
> > Greg KH wrote:
> >>
> >> On Thu, Jan 22, 2009 at 12:06:33PM +0200, Pekka J Enberg wrote:
> >>>
> >>> From: Pekka Enberg <penberg@cs.helsinki.fi>
> >>>
> >>> If successful, the usb_control_msg() function returns the number of
> >>> bytes transferred. Fix up wb35_probe() to only bail out if the function
> >>> returns a negative number.
> >>>
> >>> Reported-by: Sandro Bonazzola <sandro.bonazzola@gmail.com>
> >>> Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
> >>
> >> I'm confused, should I apply this patch or not?
> >
> > Yes, you should. What's confusing you here?
> 
> Oh, looking at the thread, I see how you got confused. The patch
> doesn't fix all of Sandro's problems but is an obvious improvement so
> it should be merged.

Hm, it doesn't apply to the latest -next tree, care to respin it?

thanks,

greg k-h

Re: [PATCH] w35und: fix usb_control_msg() error handling in wb35_probe()

[Pekka Enberg]

On Thu, 2009-01-29 at 11:24 -0800, Greg KH wrote:
> Hm, it doesn't apply to the latest -next tree, care to respin it?

Here you go:

>From 0d8febfea5c6f2a608584328771eba72df7c5806 Mon Sep 17 00:00:00 2001
From: Pekka Enberg <penberg@cs.helsinki.fi>
Date: Fri, 30 Jan 2009 11:29:27 +0200
Subject: [PATCH] w35und: fix usb_control_msg() error handling in wb35_probe()

If successful, the usb_control_msg() function returns the number of
bytes transferred. Fix up wb35_probe() to only bail out if the function returns
a negative number. Also, fix up ieee80211_alloc_hw() error code to ENOMEM;
otherwise GCC complains that err might be undefined (and is right about that).

Acked-by: Pavel Machek <pavel@suse.cz>
Reported-and-tested-by: Sandro Bonazzola <sandro.bonazzola@gmail.com>
Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---
 drivers/staging/winbond/wbusb.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/winbond/wbusb.c b/drivers/staging/winbond/wbusb.c
index 3f7d4e5..69b4ed0 100644
--- a/drivers/staging/winbond/wbusb.c
+++ b/drivers/staging/winbond/wbusb.c
@@ -319,16 +319,18 @@ static int wb35_probe(struct usb_interface *intf, const struct usb_device_id *id
 	struct usb_device *udev = interface_to_usbdev(intf);
 	struct wbsoft_priv *priv;
 	struct ieee80211_hw *dev;
-	int err;
+	int nr, err;
 
 	usb_get_dev(udev);
 
 	// 20060630.2 Check the device if it already be opened
-	err = usb_control_msg(udev, usb_rcvctrlpipe( udev, 0 ),
-			      0x01, USB_TYPE_VENDOR|USB_RECIP_DEVICE|USB_DIR_IN,
-			      0x0, 0x400, &ltmp, 4, HZ*100 );
-	if (err)
+	nr = usb_control_msg(udev, usb_rcvctrlpipe( udev, 0 ),
+			     0x01, USB_TYPE_VENDOR|USB_RECIP_DEVICE|USB_DIR_IN,
+			     0x0, 0x400, &ltmp, 4, HZ*100 );
+	if (nr < 0) {
+		err = nr;
 		goto error;
+	}
 
 	ltmp = cpu_to_le32(ltmp);
 	if (ltmp) {  // Is already initialized?
@@ -337,8 +339,10 @@ static int wb35_probe(struct usb_interface *intf, const struct usb_device_id *id
 	}
 
 	dev = ieee80211_alloc_hw(sizeof(*priv), &wbsoft_ops);
-	if (!dev)
+	if (!dev) {
+		err = -ENOMEM;
 		goto error;
+	}
 
 	priv = dev->priv;
 
-- 
1.5.4.3