2009-01-22 10:06:44

by Pekka Enberg

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

From: Pekka Enberg <[email protected]>

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 <[email protected]>
Signed-off-by: Pekka Enberg <[email protected]>
---
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


2009-01-22 11:42:48

by Pavel Machek

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

> From: Pekka Enberg <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Pekka Enberg <[email protected]>

Acked-by: Pavel Machek <[email protected]>

> ---
> 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

2009-01-22 19:25:26

by Sandro Bonazzola

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

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

Pavel Machek ha scritto:
>> From: Pekka Enberg <[email protected]>
>>
>> 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 <[email protected]>
>> Signed-off-by: Pekka Enberg <[email protected]>
>
> Acked-by: Pavel Machek <[email protected]>

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-----

2009-01-22 21:11:18

by Pavel Machek

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

On Thu 2009-01-22 20:25:06, Sandro Bonazzola wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Pavel Machek ha scritto:
> >> From: Pekka Enberg <[email protected]>
> >>
> >> 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 <[email protected]>
> >> Signed-off-by: Pekka Enberg <[email protected]>
> >
> > Acked-by: Pavel Machek <[email protected]>
>
> 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

2009-01-22 21:19:20

by Harvey Harrison

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

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 <[email protected]>
> > >>
> > >> 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 <[email protected]>
> > >> Signed-off-by: Pekka Enberg <[email protected]>
> > >
> > > Acked-by: Pavel Machek <[email protected]>
> >
> > 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

2009-01-22 22:32:26

by Pekka Enberg

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

On Thu, Jan 22, 2009 at 11:19 PM, Harvey Harrison
<[email protected]> 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 <[email protected]>
>> > >>
>> > >> 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 <[email protected]>
>> > >> Signed-off-by: Pekka Enberg <[email protected]>
>> > >
>> > > Acked-by: Pavel Machek <[email protected]>
>> >
>> > 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

2009-01-22 22:34:53

by Sandro Bonazzola

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

-----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-----

2009-01-22 22:36:17

by Pekka Enberg

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

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-----

2009-01-22 22:37:52

by Pekka Enberg

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

On Thu, Jan 22, 2009 at 9:25 PM, Sandro Bonazzola
<[email protected]> 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.

2009-01-26 17:57:17

by Greg KH

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

On Thu, Jan 22, 2009 at 12:06:33PM +0200, Pekka J Enberg wrote:
> From: Pekka Enberg <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Pekka Enberg <[email protected]>

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

thanks,

greg k-h

2009-01-26 18:08:18

by Pekka Enberg

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

Greg KH wrote:
> On Thu, Jan 22, 2009 at 12:06:33PM +0200, Pekka J Enberg wrote:
>> From: Pekka Enberg <[email protected]>
>>
>> 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 <[email protected]>
>> Signed-off-by: Pekka Enberg <[email protected]>
>
> I'm confused, should I apply this patch or not?

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

Pekka

2009-01-26 18:11:18

by Pekka Enberg

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

On Mon, Jan 26, 2009 at 8:03 PM, Pekka Enberg <[email protected]> wrote:
> Greg KH wrote:
>>
>> On Thu, Jan 22, 2009 at 12:06:33PM +0200, Pekka J Enberg wrote:
>>>
>>> From: Pekka Enberg <[email protected]>
>>>
>>> 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 <[email protected]>
>>> Signed-off-by: Pekka Enberg <[email protected]>
>>
>> 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.

2009-01-26 19:31:49

by Sandro Bonazzola

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

-----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-----

2009-01-26 19:41:21

by Pekka Enberg

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

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
<[email protected]> 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

2009-01-26 19:43:58

by Pekka Enberg

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

On Mon, Jan 26, 2009 at 9:40 PM, Pekka Enberg <[email protected]> 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
> <[email protected]> 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

2009-01-26 20:14:15

by Sandro Bonazzola

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

-----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-----

2009-01-26 20:26:43

by Pekka Enberg

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

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

2009-01-26 21:42:51

by Pavel Machek

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

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

2009-01-29 19:27:55

by Greg KH

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

On Mon, Jan 26, 2009 at 08:10:42PM +0200, Pekka Enberg wrote:
> On Mon, Jan 26, 2009 at 8:03 PM, Pekka Enberg <[email protected]> wrote:
> > Greg KH wrote:
> >>
> >> On Thu, Jan 22, 2009 at 12:06:33PM +0200, Pekka J Enberg wrote:
> >>>
> >>> From: Pekka Enberg <[email protected]>
> >>>
> >>> 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 <[email protected]>
> >>> Signed-off-by: Pekka Enberg <[email protected]>
> >>
> >> 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

2009-01-30 09:33:06

by Pekka Enberg

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

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 <[email protected]>
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 <[email protected]>
Reported-and-tested-by: Sandro Bonazzola <[email protected]>
Signed-off-by: Pekka Enberg <[email protected]>
---
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