From: Stefan Seyfried <[email protected]>
The following two trivial patches add support for this device to
zd1211rw (ejecting the virtual USB drive) and ar9170usb (that drives
the WLAN chip inside).
Best regards,
Stefan Seyfried
On Fri, 11 Dec 2009 15:55:23 +0100
Johannes Berg <[email protected]> wrote:
> On Fri, 2009-12-11 at 09:42 -0500, John W. Linville wrote:
>
> > It strikes me as strange that you are hitting two drivers to support
> > one device. Are you saying that your ar9170usb has the goofy storage
> > device thing using the same USB IDs that the zd1211rw devices like that
> > used? I suppose that makes some sense given the shared heritage...
>
> Good catch -- even if ar9170 descended from zd1211 it doesn't seem
> appropriate to require two drivers for a single device.
Unfortunately, the device ID of the (unswitched) device is already
listed in zd1211, so the easy way was to add the eject there. The
zd1211rw will be loaded anyway, it will just barf that it cannot find
the correct endpoint:
usb 1-1: zd1211rw: Could not find bulk out endpoint
For the long run, a better solution would be nice, yes.
Thanks,
Stefan
--
Stefan Seyfried
"Any ideas, John?"
"Well, surrounding them's out."
On Fri, Dec 11, 2009 at 01:36:03PM +0100, Stefan Seyfried wrote:
> On Tue, 8 Dec 2009 15:21:33 +0100
> Stefan Seyfried <[email protected]> wrote:
>
> > From: Stefan Seyfried <[email protected]>
> >
> > The following two trivial patches add support for this device to
> > zd1211rw (ejecting the virtual USB drive) and ar9170usb (that drives
> > the WLAN chip inside).
>
> Is anyhting wrong with those?
> Are those too trivial and "USB only" and thus better suited for direct
> submission to Greg?
Trivial or not, they are wireless LAN patches and they go here.
FWIW, you posted them in the middle of the maintainer's merge window
for 2.6.33. So for now only fixes are being merged. I wasn't sure
these qualified, although upon closer review they may.
It strikes me as strange that you are hitting two drivers to support
one device. Are you saying that your ar9170usb has the goofy storage
device thing using the same USB IDs that the zd1211rw devices like that
used? I suppose that makes some sense given the shared heritage...
We did just have a long (and possibly unresolved thread) about
this type of practice (i.e. eject in the driver) not too long ago.
But since zd1211rw is already doing this, maybe this patch is
acceptable...?
Anyway, I have your patch. Please be patient.
John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.
On Tue, 8 Dec 2009 15:21:33 +0100
Stefan Seyfried <[email protected]> wrote:
> From: Stefan Seyfried <[email protected]>
>
> The following two trivial patches add support for this device to
> zd1211rw (ejecting the virtual USB drive) and ar9170usb (that drives
> the WLAN chip inside).
Is anyhting wrong with those?
Are those too trivial and "USB only" and thus better suited for direct
submission to Greg?
Thanks,
Stefan
--
Stefan Seyfried
"Any ideas, John?"
"Well, surrounding them's out."
On Fri, 2009-12-11 at 09:42 -0500, John W. Linville wrote:
> It strikes me as strange that you are hitting two drivers to support
> one device. Are you saying that your ar9170usb has the goofy storage
> device thing using the same USB IDs that the zd1211rw devices like that
> used? I suppose that makes some sense given the shared heritage...
Good catch -- even if ar9170 descended from zd1211 it doesn't seem
appropriate to require two drivers for a single device.
johannes
On Fri, 11 Dec 2009 23:37:09 +0100
Stefan Seyfried <[email protected]> wrote:
> On Fri, 11 Dec 2009 11:21:10 -0800
> Dan Williams <[email protected]> wrote:
> > place to put this code is probably unusual_devs instead of
> > touching /two/ drivers. I'd say extract the code from zd1211rw and put
> > the new code that works for both zd1211rw/ar9170 into unusual_devs. In
> > fact, people might be happier if you posted two patches, (1) move the
> > code to unusual_devs unchanged,
Done. Was easier than I had expected and resulted in a nice reduction
in lines of code, because usb storage code has all infrastructure to
handle the device, which zd1211rw had hand-crafted.
> and (2) fix up the moved code to support
> > the new device's behavior.
This was actually not necessary, because the usb-storage infrastructure
automatically chooses the correct endpoint :-)
See the mail
Subject: [PATCH] move eject code from zd1211rw to usb-storage
Thanks for review and suggestions,
Stefan
--
"You sure you software suspend guys haven't been hanging out with the
IDE maintainers?" -- Rob Landley, during one of the suspend wars
From: Stefan Seyfried <[email protected]>
The zd1211rw always assumed that the storage device is at endpoint 1,
but there are devices (Spairon Homelink 1202) that are at endpoint 0.
Try both, starting with 1 to make sure to not break existing setups.
Signed-off-by: Stefan Seyfried <[email protected]>
---
drivers/net/wireless/zd1211rw/zd_usb.c | 14 +++++++++-----
1 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/zd1211rw/zd_usb.c b/drivers/net/wireless/zd1211rw/zd_usb.c
index ac19ecd..4daf1c9 100644
--- a/drivers/net/wireless/zd1211rw/zd_usb.c
+++ b/drivers/net/wireless/zd1211rw/zd_usb.c
@@ -1078,11 +1078,15 @@ static int eject_installer(struct usb_interface *intf)
int r;
/* Find bulk out endpoint */
- endpoint = &iface_desc->endpoint[1].desc;
- if (usb_endpoint_dir_out(endpoint) &&
- usb_endpoint_xfer_bulk(endpoint)) {
- bulk_out_ep = endpoint->bEndpointAddress;
- } else {
+ for (r = 1; r >= 0; r--) {
+ endpoint = &iface_desc->endpoint[r].desc;
+ if (usb_endpoint_dir_out(endpoint) &&
+ usb_endpoint_xfer_bulk(endpoint)) {
+ bulk_out_ep = endpoint->bEndpointAddress;
+ break;
+ }
+ }
+ if (r == -1) {
dev_err(&udev->dev,
"zd1211rw: Could not find bulk out endpoint\n");
return -ENODEV;
--
1.6.5.3
On Fri, 2009-12-11 at 16:01 +0100, Stefan Seyfried wrote:
> On Fri, 11 Dec 2009 09:42:22 -0500
> "John W. Linville" <[email protected]> wrote:
>
> > On Fri, Dec 11, 2009 at 01:36:03PM +0100, Stefan Seyfried wrote:
> > > Is anyhting wrong with those?
> > > Are those too trivial and "USB only" and thus better suited for direct
> > > submission to Greg?
> >
> > Trivial or not, they are wireless LAN patches and they go here.
>
> That's what i guessed, too. I was just deafened by the silence ;)
>
> > It strikes me as strange that you are hitting two drivers to support
> > one device. Are you saying that your ar9170usb has the goofy storage
> > device thing using the same USB IDs that the zd1211rw devices like that
> > used? I suppose that makes some sense given the shared heritage...
>
> Yes, exactly. And a different configuration (different endpoint),
> that's why I had to touch the zd1211rw driver, even though it is
> otherwise totally unrelated to the device.
>
> The method with the reverse loop counting down from ep1 to ep0 was
> chosen to be as sure as possible to not break the other devices that
> are already working.
>
> > We did just have a long (and possibly unresolved thread) about
> > this type of practice (i.e. eject in the driver) not too long ago.
> > But since zd1211rw is already doing this, maybe this patch is
> > acceptable...?
>
> Maybe a separate driver that is ejecting all those fake storage
> devices, be it 3G modems or wireless LAN might be a good idea, but I am
> probably not the right one to code that ;)
> Or maybe we decide that userspace should handle it for all devices...
If the device actually changes USB IDs after the switch, then the right
place to put this code is probably unusual_devs instead of
touching /two/ drivers. I'd say extract the code from zd1211rw and put
the new code that works for both zd1211rw/ar9170 into unusual_devs. In
fact, people might be happier if you posted two patches, (1) move the
code to unusual_devs unchanged, and (2) fix up the moved code to support
the new device's behavior.
Dan
From: Stefan Seyfried <[email protected]>
Signed-off-by: Stefan Seyfried <[email protected]>
---
drivers/net/wireless/ath/ar9170/usb.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/ath/ar9170/usb.c b/drivers/net/wireless/ath/ar9170/usb.c
index e0799d9..0f36118 100644
--- a/drivers/net/wireless/ath/ar9170/usb.c
+++ b/drivers/net/wireless/ath/ar9170/usb.c
@@ -84,6 +84,8 @@ static struct usb_device_id ar9170_usb_ids[] = {
{ USB_DEVICE(0x0cde, 0x0023) },
/* Z-Com UB82 ABG */
{ USB_DEVICE(0x0cde, 0x0026) },
+ /* Sphairon Homelink 1202 */
+ { USB_DEVICE(0x0cde, 0x0027) },
/* Arcadyan WN7512 */
{ USB_DEVICE(0x083a, 0xf522) },
/* Planex GWUS300 */
--
1.6.5.3
On Fri, 11 Dec 2009 11:21:10 -0800
Dan Williams <[email protected]> wrote:
> On Fri, 2009-12-11 at 16:01 +0100, Stefan Seyfried wrote:
> > Maybe a separate driver that is ejecting all those fake storage
> > devices, be it 3G modems or wireless LAN might be a good idea, but I am
> > probably not the right one to code that ;)
> > Or maybe we decide that userspace should handle it for all devices...
>
> If the device actually changes USB IDs after the switch, then the right
Yes, it does.
> place to put this code is probably unusual_devs instead of
> touching /two/ drivers. I'd say extract the code from zd1211rw and put
> the new code that works for both zd1211rw/ar9170 into unusual_devs. In
> fact, people might be happier if you posted two patches, (1) move the
> code to unusual_devs unchanged, and (2) fix up the moved code to support
> the new device's behavior.
Ok, I'll do this next week (might take some time, since I need to get
familiar with the usb / unusual_devs code). I guess this will mostly go
through Greg then?
John, please take the patch 1, that adds the USB ID to ar9170usb. It is
useful on its own since the device can also be switched with
usb_modeswitch from userspace, and it is really trivial.
Thanks,
Stefan
--
Stefan Seyfried
"Any ideas, John?"
"Well, surrounding them's out."
On Fri, 11 Dec 2009 09:42:22 -0500
"John W. Linville" <[email protected]> wrote:
> On Fri, Dec 11, 2009 at 01:36:03PM +0100, Stefan Seyfried wrote:
> > Is anyhting wrong with those?
> > Are those too trivial and "USB only" and thus better suited for direct
> > submission to Greg?
>
> Trivial or not, they are wireless LAN patches and they go here.
That's what i guessed, too. I was just deafened by the silence ;)
> It strikes me as strange that you are hitting two drivers to support
> one device. Are you saying that your ar9170usb has the goofy storage
> device thing using the same USB IDs that the zd1211rw devices like that
> used? I suppose that makes some sense given the shared heritage...
Yes, exactly. And a different configuration (different endpoint),
that's why I had to touch the zd1211rw driver, even though it is
otherwise totally unrelated to the device.
The method with the reverse loop counting down from ep1 to ep0 was
chosen to be as sure as possible to not break the other devices that
are already working.
> We did just have a long (and possibly unresolved thread) about
> this type of practice (i.e. eject in the driver) not too long ago.
> But since zd1211rw is already doing this, maybe this patch is
> acceptable...?
Maybe a separate driver that is ejecting all those fake storage
devices, be it 3G modems or wireless LAN might be a good idea, but I am
probably not the right one to code that ;)
Or maybe we decide that userspace should handle it for all devices...
> Anyway, I have your patch. Please be patient.
Thanks,
Stefan
--
Stefan Seyfried
"Any ideas, John?"
"Well, surrounding them's out."