2009-12-15 12:07:12

by Stefan Seyfried

[permalink] [raw]
Subject: [PATCH] move eject code from zd1211rw to usb-storage

From: Stefan Seyfried <[email protected]>

The USB ID claimed by zd1211rw for the fake storage device is
also in use by other, non-zd1211rw devices (Sphairon Homelink
1202). Move the eject of these devices to where it belongs and
where all the needed infrastructure already exists: usb-storage.

Signed-off-by: Stefan Seyfried <[email protected]>
---
drivers/net/wireless/zd1211rw/zd_usb.c | 57 +-------------------------------
drivers/net/wireless/zd1211rw/zd_usb.h | 1 -
drivers/usb/storage/initializers.c | 28 +++++++++++++++
drivers/usb/storage/initializers.h | 3 ++
drivers/usb/storage/unusual_devs.h | 13 ++++---
5 files changed, 39 insertions(+), 63 deletions(-)

diff --git a/drivers/net/wireless/zd1211rw/zd_usb.c b/drivers/net/wireless/zd1211rw/zd_usb.c
index ac19ecd..983ebdb 100644
--- a/drivers/net/wireless/zd1211rw/zd_usb.c
+++ b/drivers/net/wireless/zd1211rw/zd_usb.c
@@ -90,9 +90,6 @@ static struct usb_device_id usb_ids[] = {
{ USB_DEVICE(0x157e, 0x300d), .driver_info = DEVICE_ZD1211B },
{ USB_DEVICE(0x1582, 0x6003), .driver_info = DEVICE_ZD1211B },
{ USB_DEVICE(0x2019, 0x5303), .driver_info = DEVICE_ZD1211B },
- /* "Driverless" devices that need ejecting */
- { USB_DEVICE(0x0ace, 0x2011), .driver_info = DEVICE_INSTALLER },
- { USB_DEVICE(0x0ace, 0x20ff), .driver_info = DEVICE_INSTALLER },
{}
};

@@ -1068,54 +1065,6 @@ static void print_id(struct usb_device *udev)
#define print_id(udev) do { } while (0)
#endif

-static int eject_installer(struct usb_interface *intf)
-{
- struct usb_device *udev = interface_to_usbdev(intf);
- struct usb_host_interface *iface_desc = &intf->altsetting[0];
- struct usb_endpoint_descriptor *endpoint;
- unsigned char *cmd;
- u8 bulk_out_ep;
- 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 {
- dev_err(&udev->dev,
- "zd1211rw: Could not find bulk out endpoint\n");
- return -ENODEV;
- }
-
- cmd = kzalloc(31, GFP_KERNEL);
- if (cmd == NULL)
- return -ENODEV;
-
- /* USB bulk command block */
- cmd[0] = 0x55; /* bulk command signature */
- cmd[1] = 0x53; /* bulk command signature */
- cmd[2] = 0x42; /* bulk command signature */
- cmd[3] = 0x43; /* bulk command signature */
- cmd[14] = 6; /* command length */
-
- cmd[15] = 0x1b; /* SCSI command: START STOP UNIT */
- cmd[19] = 0x2; /* eject disc */
-
- dev_info(&udev->dev, "Ejecting virtual installer media...\n");
- r = usb_bulk_msg(udev, usb_sndbulkpipe(udev, bulk_out_ep),
- cmd, 31, NULL, 2000);
- kfree(cmd);
- if (r)
- return r;
-
- /* At this point, the device disconnects and reconnects with the real
- * ID numbers. */
-
- usb_set_intfdata(intf, NULL);
- return 0;
-}
-
int zd_usb_init_hw(struct zd_usb *usb)
{
int r;
@@ -1157,9 +1106,6 @@ static int probe(struct usb_interface *intf, const struct usb_device_id *id)

print_id(udev);

- if (id->driver_info & DEVICE_INSTALLER)
- return eject_installer(intf);
-
switch (udev->speed) {
case USB_SPEED_LOW:
case USB_SPEED_FULL:
@@ -1219,8 +1165,7 @@ static void disconnect(struct usb_interface *intf)
struct zd_mac *mac;
struct zd_usb *usb;

- /* Either something really bad happened, or we're just dealing with
- * a DEVICE_INSTALLER. */
+ /* something really bad happened */
if (hw == NULL)
return;

diff --git a/drivers/net/wireless/zd1211rw/zd_usb.h b/drivers/net/wireless/zd1211rw/zd_usb.h
index 049f8b9..0bc96f2 100644
--- a/drivers/net/wireless/zd1211rw/zd_usb.h
+++ b/drivers/net/wireless/zd1211rw/zd_usb.h
@@ -35,7 +35,6 @@
enum devicetype {
DEVICE_ZD1211 = 0,
DEVICE_ZD1211B = 1,
- DEVICE_INSTALLER = 2,
};

enum endpoints {
diff --git a/drivers/usb/storage/initializers.c b/drivers/usb/storage/initializers.c
index 105d900..da28924 100644
--- a/drivers/usb/storage/initializers.c
+++ b/drivers/usb/storage/initializers.c
@@ -104,3 +104,31 @@ int usb_stor_huawei_e220_init(struct us_data *us)
US_DEBUGP("Huawei mode set result is %d\n", result);
return 0;
}
+
+/* eject fake USB cdrom on zd1211rw and similar devices */
+int zd1211rw_eject_installer(struct us_data *us)
+{
+ unsigned char *cmd;
+ int r;
+
+ cmd = kzalloc(31, GFP_KERNEL);
+ if (cmd == NULL)
+ return -ENODEV;
+
+ /* USB bulk command block */
+ cmd[0] = 0x55; /* bulk command signature */
+ cmd[1] = 0x53; /* bulk command signature */
+ cmd[2] = 0x42; /* bulk command signature */
+ cmd[3] = 0x43; /* bulk command signature */
+ cmd[14] = 6; /* command length */
+
+ cmd[15] = 0x1b; /* SCSI command: START STOP UNIT */
+ cmd[19] = 0x2; /* eject disc */
+
+ US_DEBUGP("Ejecting zd1211rw virtual installer media...\n");
+ r = usb_stor_bulk_transfer_buf(us, us->send_bulk_pipe,
+ cmd, 31, NULL);
+ kfree(cmd);
+
+ return r;
+}
diff --git a/drivers/usb/storage/initializers.h b/drivers/usb/storage/initializers.h
index 529327f..e7bec81 100644
--- a/drivers/usb/storage/initializers.h
+++ b/drivers/usb/storage/initializers.h
@@ -48,3 +48,6 @@ int usb_stor_ucr61s2b_init(struct us_data *us);

/* This places the HUAWEI E220 devices in multi-port mode */
int usb_stor_huawei_e220_init(struct us_data *us);
+
+/* eject fake USB cdrom on zd1211rw and similar devices */
+int zd1211rw_eject_installer(struct us_data *us);
diff --git a/drivers/usb/storage/unusual_devs.h b/drivers/usb/storage/unusual_devs.h
index d4f034e..4f05e01 100644
--- a/drivers/usb/storage/unusual_devs.h
+++ b/drivers/usb/storage/unusual_devs.h
@@ -1103,19 +1103,20 @@ UNUSUAL_DEV( 0x0a17, 0x0004, 0x1000, 0x1000,
US_SC_DEVICE, US_PR_DEVICE, NULL,
US_FL_FIX_INQUIRY ),

-/* These are virtual windows driver CDs, which the zd1211rw driver
- * automatically converts into WLAN devices. */
+/* These are virtual windows driver CDs, which the zd1211rw driver or
+ * ar9170usb handle after we switched them them out of storage mode
+ */
UNUSUAL_DEV( 0x0ace, 0x2011, 0x0101, 0x0101,
"ZyXEL",
"G-220F USB-WLAN Install",
- US_SC_DEVICE, US_PR_DEVICE, NULL,
- US_FL_IGNORE_DEVICE ),
+ US_SC_DEVICE, US_PR_DEVICE, zd1211rw_eject_installer,
+ 0),

UNUSUAL_DEV( 0x0ace, 0x20ff, 0x0101, 0x0101,
"SiteCom",
"WL-117 USB-WLAN Install",
- US_SC_DEVICE, US_PR_DEVICE, NULL,
- US_FL_IGNORE_DEVICE ),
+ US_SC_DEVICE, US_PR_DEVICE, zd1211rw_eject_installer,
+ 0),

/* Reported by Dan Williams <[email protected]>
* Option N.V. mobile broadband modems
--
1.6.5.3



2009-12-16 10:29:13

by Oliver Neukum

[permalink] [raw]
Subject: Re: [PATCH] move eject code from zd1211rw to usb-storage

Am Dienstag, 15. Dezember 2009 19:03:00 schrieb Matthew Dharm:
> > This would break existing systems and thus introduce a regression.
> > We'd need to go through a feature removal process. For the time being
> > I see no alternative to Seife's patch, as we cannot introduce ejection
> > code to another wireless driver and need to support these devices.
>
> The right answer here is neither to move the eject code nor to introduce
> more of it. New devices should be supported via userspace.

Usually I would agree, but in this case the vendor reused IDs.
The legacy kernel space switcher and user space would race.

Regards
Oliver

2009-12-17 14:36:36

by Oliver Neukum

[permalink] [raw]
Subject: Re: [PATCH] move eject code from zd1211rw to usb-storage

Am Donnerstag, 17. Dezember 2009 14:33:06 schrieb Josua Dietze:
> > Can storage tell the devices apart so that it knows which ones to leave
> > to the kernel solution and which devices to accept so that udev can
> > issue an eject command?
>
> If you are thinking about the two specific devices at hand there is
> no need to tell them apart. Same IDs on plugin, same switching
> procedure, different IDs on return, different drivers take care.

How do you issue an eject command without a /dev/sgX node?

Regards
Oliver

2009-12-15 15:10:53

by Oliver Neukum

[permalink] [raw]
Subject: Re: [PATCH] move eject code from zd1211rw to usb-storage

Am Dienstag, 15. Dezember 2009 13:31:13 schrieb Josua Dietze:
> Stefan Seyfried schrieb:
>
> > The USB ID claimed by zd1211rw for the fake storage device is
> > also in use by other, non-zd1211rw devices (Sphairon Homelink
> > 1202). Move the eject of these devices to where it belongs and
> > where all the needed infrastructure already exists: usb-storage.
>
>
> We can do the eject (as other mode switches) in userspace. Previous
> discussions pointed into that direction.
>
> And there is always the question of being able to access the "fake
> CD-ROM" without hacking the kernel.
>
> I'd like to see these devices removed from unusual_devs.h.

This would break existing systems and thus introduce a regression.
We'd need to go through a feature removal process. For the time being
I see no alternative to Seife's patch, as we cannot introduce ejection
code to another wireless driver and need to support these devices.

Regards
Oliver

2009-12-16 19:51:42

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH] move eject code from zd1211rw to usb-storage

On Wed, 2009-12-16 at 10:03 -0800, Matthew Dharm wrote:
> On Wed, Dec 16, 2009 at 11:29:27AM +0100, Oliver Neukum wrote:
> > Am Dienstag, 15. Dezember 2009 19:03:00 schrieb Matthew Dharm:
> > > > This would break existing systems and thus introduce a regression.
> > > > We'd need to go through a feature removal process. For the time being
> > > > I see no alternative to Seife's patch, as we cannot introduce ejection
> > > > code to another wireless driver and need to support these devices.
> > >
> > > The right answer here is neither to move the eject code nor to introduce
> > > more of it. New devices should be supported via userspace.
> >
> > Usually I would agree, but in this case the vendor reused IDs.
> > The legacy kernel space switcher and user space would race.
>
> So, let me see if I understand this... we have two devices that use the
> same IDs, and get mode-switched the same way, but need different
> post-switch drivers?
>
> If this is the case, then the only reasonable answer to is to push the
> modeswitch code for both into udev and out of the kernel. It will take

you mean usb_modeswitch, not udev actually.

> longer to support the new device that way (since we need to wait until udev
> is updated and then remove kernel support), but that's what a vendor gets
> for re-using IDs.
>
> Matt
>


2009-12-16 19:13:17

by Matthew Dharm

[permalink] [raw]
Subject: Re: [PATCH] move eject code from zd1211rw to usb-storage

On Wed, Dec 16, 2009 at 08:47:17AM -0800, Dan Williams wrote:
> On Wed, 2009-12-16 at 13:14 +0100, Stefan Seyfried wrote:
> > On Wed, 16 Dec 2009 12:22:31 +0100
> > Josua Dietze <[email protected]> wrote:
> >
> > > Stefan Seyfried schrieb:
> > > > Preferably some code that can be built actually?
> > >
> > > If the "eject" from SCSI tools does not work, "usb_modeswitch" can
> > > send customized bulk messages to devices.
> >
> > for my device, eject seems to be enough, so I'll be trying to get
> > this into udev.
> >
> > > http://www.draisberghof.de/usb_modeswitch/
> >
> > Unfortunately it cannot be built with current libusb and once you
> > compile it without warnings turned off (which is the default!), you
> > probably don't want to run it anymore ;)
>
> At this point, however, usb_modeswitch is the correct place to put eject
> code for all devices. I wouldn't put this code into udev; I'd put it
> into usb_modeswitch instead, since usb_modeswitch is (a) the de-facto
> standard, (b) has the most users, and (c) has the most devices. Yes, it
> has problems, but at this point we should fix those problems instead of
> creating 5 different eject tools.

udev just calls either usb_modeswitch or eject at the hotplug event. So,
you always need an update to the udev database, regardless.

The choice between using usb_modeswitch or eject is really one of "does the
standard eject work properly" or not. If not, add the code to
usb_modeswitch. Tho, if the usb_modeswitch maintainers want to carry
around a duplicate set of 'eject' code, that's really up to them.

Matt

--
Matthew Dharm Home: [email protected]
Maintainer, Linux USB Mass Storage Driver

Would you mind not using our Web server? We're trying to have a game of
Quake here.
-- Greg
User Friendly, 5/11/1998


Attachments:
(No filename) (1.81 kB)
(No filename) (189.00 B)
Download all attachments

2009-12-15 17:58:34

by Matthew Dharm

[permalink] [raw]
Subject: Re: [usb-storage] [PATCH] move eject code from zd1211rw to usb-storage

On Tue, Dec 15, 2009 at 03:59:15PM +0100, Josua Dietze wrote:
> The more basic arguments that prevailed at the last time such an
> "eject" command was being considered for inclusion into usb-storage
> were:
>
> 1. Userspace can obviously react a lot quicker than kernel space to
> new or changed devices popping up.
>
> 2. Userspace works with older kernels immediately.
>
> > That's a fundamental difference in opinions, and I fear I'm not the one
> > who is going to decice how this will be handled ;)

I hate to say this, but I really consider this discussion to be closed.
Nobody has introduced a new argument in quite some time. This material,
where possible, belongs in userspace.

The primary reason userspace may *not* be apropriate is if the device needs
a non-standard eject command. Some devices need something very *close* to
an eject, but not quite an eject. Or, they have some goofy timing
restriction which makes the startup time for udev too long, or somesuch.

> Neither am I. But the number of known mode-switching USB devices is
> now at around 50. New ones are arriving by the month or even week.
>
> A decision to handle *all of them* in usb-storage would lead to the
> disadvantages pointed out.

The rate at which these devices are arriving makes this more than a
"disadvantage", it makes it practically impossible for management at the
kernel level.

> A decision to handle just *some of them* can hardly be made
> plausible if there are no immediate technical reasons.

The current decision is that all new devices, whenever possible, are to be
handled in userspace. Old devices currently handled by the kernel will be
removed on an "as we get around to it" schedule.

> Oh, and in most cases (including your suggestion) there *are*
> already two drivers necessary to make these devices work,
> independent of where the switching is happening ...

This is true. However, drivers such as usb-serial support dynamic addition
of IDs, so no kernel change is necessary.

Matt

--
Matthew Dharm Home: [email protected]
Maintainer, Linux USB Mass Storage Driver

You were using cheat codes too. You guys suck.
-- Greg to General Studebaker
User Friendly, 12/16/1997


Attachments:
(No filename) (2.20 kB)
(No filename) (189.00 B)
Download all attachments

2009-12-17 18:22:19

by Matthew Dharm

[permalink] [raw]
Subject: Re: [PATCH] move eject code from zd1211rw to usb-storage

On Thu, Dec 17, 2009 at 02:02:41PM +0100, Oliver Neukum wrote:
> Am Mittwoch, 16. Dezember 2009 20:52:58 schrieb Matthew Dharm:
> > > > If this is the case, then the only reasonable answer to is to push the
> > > > modeswitch code for both into udev and out of the kernel. It will take
> > >
> > > you mean usb_modeswitch, not udev actually.
> >
> > That is correct; I had mis-typed. Tho, the actual implementation is udev
> > calling usb_modeswitch and/or eject.
>
> Can storage tell the devices apart so that it knows which ones to leave
> to the kernel solution and which devices to accept so that udev can
> issue an eject command?

Maybe. Depends on how identical the devices are. We would need to compare
descriptors between the two devices.

Matt

--
Matthew Dharm Home: [email protected]
Maintainer, Linux USB Mass Storage Driver

E: You run this ship with Windows?! YOU IDIOT!
L: Give me a break, it came bundled with the computer!
-- ESR and Lan Solaris
User Friendly, 12/8/1998


Attachments:
(No filename) (1.02 kB)
(No filename) (189.00 B)
Download all attachments

2009-12-16 14:15:29

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH] move eject code from zd1211rw to usb-storage

On Wed, Dec 16, 2009 at 01:14:52PM +0100, Stefan Seyfried wrote:
> On Wed, 16 Dec 2009 12:22:31 +0100
> Josua Dietze <[email protected]> wrote:
>
> > Stefan Seyfried schrieb:
> > > Preferably some code that can be built actually?
> >
> > If the "eject" from SCSI tools does not work, "usb_modeswitch" can
> > send customized bulk messages to devices.
>
> for my device, eject seems to be enough, so I'll be trying to get
> this into udev.

When you do that any chance you could account for zd1211rw too? :-)

--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2009-12-16 11:42:31

by Josua Dietze

[permalink] [raw]
Subject: Re: [PATCH] move eject code from zd1211rw to usb-storage

Oliver Neukum schrieb:

> Am Dienstag, 15. Dezember 2009 19:03:00 schrieb Matthew Dharm:
>>> This would break existing systems and thus introduce a regression.
>>> We'd need to go through a feature removal process. For the time being
>>> I see no alternative to Seife's patch, as we cannot introduce ejection
>>> code to another wireless driver and need to support these devices.
>> The right answer here is neither to move the eject code nor to introduce
>> more of it. New devices should be supported via userspace.
>
> Usually I would agree, but in this case the vendor reused IDs.
> The legacy kernel space switcher and user space would race.

User space can test for device/interface class (which will
supposedly change even if the IDs do not) and base it's actions on
that. It was done before.

Apart from that issue, I can see that there is a problem with a
device that has worked without "external" handling up to now.
Unfortunately, I can't think about a clean solution.

Josua
--
Man is the only creature on earth enabled to take a
warm meal while flying! Loriot

2009-12-15 14:01:43

by Stefan Seyfried

[permalink] [raw]
Subject: Re: [PATCH] move eject code from zd1211rw to usb-storage

Hi,

On Tue, 15 Dec 2009 13:31:13 +0100
Josua Dietze <[email protected]> wrote:

> Stefan Seyfried schrieb:
>
> > The USB ID claimed by zd1211rw for the fake storage device is
> > also in use by other, non-zd1211rw devices (Sphairon Homelink
> > 1202). Move the eject of these devices to where it belongs and
> > where all the needed infrastructure already exists: usb-storage.
>
> We can do the eject (as other mode switches) in userspace. Previous
> discussions pointed into that direction.

Well, the zd1211rw already did that, I wanted to fix that up and was
told to do it in usb-storage instead (which, looking at the diffstat,
seems like a good idea).

> And there is always the question of being able to access the "fake
> CD-ROM" without hacking the kernel.

I was thinking that I would be able to achieve that by echoing an
appropriate quirk into /sys/module/usb-storage/parameters/quirks,
however, I could not get it to *not* eject the device.
But given that there is already an "option_zero_cd" parameter in
usb-storage, it should be easy to massage this into a general "do not
eject virtual installer media" flag so that people are still able to
get the windows driver files off their devices should they need to do
so.

> I'd like to see these devices removed from unusual_devs.h.

I'd like them to work with one driver, not two (one in kernel and one
in userspace).

That's a fundamental difference in opinions, and I fear I'm not the one
who is going to decice how this will be handled ;)

Have fun,

Stefan
--
Stefan Seyfried

"Any ideas, John?"
"Well, surrounding them's out."

2009-12-16 11:22:46

by Josua Dietze

[permalink] [raw]
Subject: Re: [PATCH] move eject code from zd1211rw to usb-storage

Stefan Seyfried schrieb:

> On Tue, 15 Dec 2009 10:03:00 -0800
> Matthew Dharm <[email protected]> wrote:
>> The right answer here is neither to move the eject code nor to introduce
>> more of it. New devices should be supported via userspace.
>
> Can somebody point me to the "userspace" that is meant here?
> Preferably some code that can be built actually?


If the "eject" from SCSI tools does not work, "usb_modeswitch" can
send customized bulk messages to devices.

http://www.draisberghof.de/usb_modeswitch/


Josh
--
Man is the only creature on earth enabled to take a
warm meal while flying! Loriot

2009-12-15 12:52:07

by Josua Dietze

[permalink] [raw]
Subject: Re: [PATCH] move eject code from zd1211rw to usb-storage

Stefan Seyfried schrieb:

> The USB ID claimed by zd1211rw for the fake storage device is
> also in use by other, non-zd1211rw devices (Sphairon Homelink
> 1202). Move the eject of these devices to where it belongs and
> where all the needed infrastructure already exists: usb-storage.


We can do the eject (as other mode switches) in userspace. Previous
discussions pointed into that direction.

And there is always the question of being able to access the "fake
CD-ROM" without hacking the kernel.

I'd like to see these devices removed from unusual_devs.h.


Josua Dietze

2009-12-17 13:33:12

by Josua Dietze

[permalink] [raw]
Subject: Re: [PATCH] move eject code from zd1211rw to usb-storage

Oliver Neukum schrieb:

> Am Mittwoch, 16. Dezember 2009 20:52:58 schrieb Matthew Dharm:
>>>> If this is the case, then the only reasonable answer to is to push the
>>>> modeswitch code for both into udev and out of the kernel. It will take
>>> you mean usb_modeswitch, not udev actually.
>> That is correct; I had mis-typed. Tho, the actual implementation is udev
>> calling usb_modeswitch and/or eject.
>
> Can storage tell the devices apart so that it knows which ones to leave
> to the kernel solution and which devices to accept so that udev can
> issue an eject command?

If you are thinking about the two specific devices at hand there is
no need to tell them apart. Same IDs on plugin, same switching
procedure, different IDs on return, different drivers take care.

If you are thinking generally, there is already a case when:
same IDs on plugin, different switching procedures.
See "option_ms.c"; the solution was to check for SCSI ID strings and
only act if there's a match. I filed this patch to enable userspace
handling for a different device with the same IDs.

This is also the way usb_modeswitch (via wrapping script) is
handling ambiguities right now.
(BTW, the next version has no more compiler warnings. No big deal.)


Cheers,
Josua

2009-12-16 10:49:38

by Stefan Seyfried

[permalink] [raw]
Subject: Re: [PATCH] move eject code from zd1211rw to usb-storage

On Tue, 15 Dec 2009 10:03:00 -0800
Matthew Dharm <[email protected]> wrote:
> The right answer here is neither to move the eject code nor to introduce
> more of it. New devices should be supported via userspace.

Can somebody point me to the "userspace" that is meant here?
Preferably some code that can be built actually?
--
Stefan Seyfried

"Any ideas, John?"
"Well, surrounding them's out."

2009-12-16 12:14:58

by Stefan Seyfried

[permalink] [raw]
Subject: Re: [PATCH] move eject code from zd1211rw to usb-storage

On Wed, 16 Dec 2009 12:22:31 +0100
Josua Dietze <[email protected]> wrote:

> Stefan Seyfried schrieb:
> > Preferably some code that can be built actually?
>
> If the "eject" from SCSI tools does not work, "usb_modeswitch" can
> send customized bulk messages to devices.

for my device, eject seems to be enough, so I'll be trying to get
this into udev.

> http://www.draisberghof.de/usb_modeswitch/

Unfortunately it cannot be built with current libusb and once you
compile it without warnings turned off (which is the default!), you
probably don't want to run it anymore ;)

Having code in kernel has one major advantage: it gets reviewed much
more thoroughly.
--
Stefan Seyfried

"Any ideas, John?"
"Well, surrounding them's out."

2009-12-17 10:55:57

by Daniel Drake

[permalink] [raw]
Subject: Re: [usb-storage] [PATCH] move eject code from zd1211rw to usb-storage

El 15/12/09 12:31, Josua Dietze escribi?:
> Stefan Seyfried schrieb:
>
>> The USB ID claimed by zd1211rw for the fake storage device is
>> also in use by other, non-zd1211rw devices (Sphairon Homelink
>> 1202). Move the eject of these devices to where it belongs and
>> where all the needed infrastructure already exists: usb-storage.
>
>
> We can do the eject (as other mode switches) in userspace. Previous
> discussions pointed into that direction.

When I originally wrote this code this wasn't as easy as you might think
- the virtual CD device seems to be very quirky, and utterly confused
Linux to the point where it didn't even create a device node. Maybe
linux is more flexible these days though.

Daniel

2009-12-16 16:48:21

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH] move eject code from zd1211rw to usb-storage

On Wed, 2009-12-16 at 13:14 +0100, Stefan Seyfried wrote:
> On Wed, 16 Dec 2009 12:22:31 +0100
> Josua Dietze <[email protected]> wrote:
>
> > Stefan Seyfried schrieb:
> > > Preferably some code that can be built actually?
> >
> > If the "eject" from SCSI tools does not work, "usb_modeswitch" can
> > send customized bulk messages to devices.
>
> for my device, eject seems to be enough, so I'll be trying to get
> this into udev.
>
> > http://www.draisberghof.de/usb_modeswitch/
>
> Unfortunately it cannot be built with current libusb and once you
> compile it without warnings turned off (which is the default!), you
> probably don't want to run it anymore ;)

At this point, however, usb_modeswitch is the correct place to put eject
code for all devices. I wouldn't put this code into udev; I'd put it
into usb_modeswitch instead, since usb_modeswitch is (a) the de-facto
standard, (b) has the most users, and (c) has the most devices. Yes, it
has problems, but at this point we should fix those problems instead of
creating 5 different eject tools.

Dan



2009-12-16 19:53:32

by Matthew Dharm

[permalink] [raw]
Subject: Re: [PATCH] move eject code from zd1211rw to usb-storage

On Wed, Dec 16, 2009 at 11:50:48AM -0800, Dan Williams wrote:
> On Wed, 2009-12-16 at 10:03 -0800, Matthew Dharm wrote:
> > On Wed, Dec 16, 2009 at 11:29:27AM +0100, Oliver Neukum wrote:
> > > Am Dienstag, 15. Dezember 2009 19:03:00 schrieb Matthew Dharm:
> > > > > This would break existing systems and thus introduce a regression.
> > > > > We'd need to go through a feature removal process. For the time being
> > > > > I see no alternative to Seife's patch, as we cannot introduce ejection
> > > > > code to another wireless driver and need to support these devices.
> > > >
> > > > The right answer here is neither to move the eject code nor to introduce
> > > > more of it. New devices should be supported via userspace.
> > >
> > > Usually I would agree, but in this case the vendor reused IDs.
> > > The legacy kernel space switcher and user space would race.
> >
> > So, let me see if I understand this... we have two devices that use the
> > same IDs, and get mode-switched the same way, but need different
> > post-switch drivers?
> >
> > If this is the case, then the only reasonable answer to is to push the
> > modeswitch code for both into udev and out of the kernel. It will take
>
> you mean usb_modeswitch, not udev actually.

That is correct; I had mis-typed. Tho, the actual implementation is udev
calling usb_modeswitch and/or eject.

Matt

--
Matthew Dharm Home: [email protected]
Maintainer, Linux USB Mass Storage Driver

What, are you one of those Microsoft-bashing Linux freaks?
-- Customer to Greg
User Friendly, 2/10/1999


Attachments:
(No filename) (1.57 kB)
(No filename) (189.00 B)
Download all attachments

2009-12-17 13:02:28

by Oliver Neukum

[permalink] [raw]
Subject: Re: [PATCH] move eject code from zd1211rw to usb-storage

Am Mittwoch, 16. Dezember 2009 20:52:58 schrieb Matthew Dharm:
> > > If this is the case, then the only reasonable answer to is to push the
> > > modeswitch code for both into udev and out of the kernel. It will take
> >
> > you mean usb_modeswitch, not udev actually.
>
> That is correct; I had mis-typed. Tho, the actual implementation is udev
> calling usb_modeswitch and/or eject.

Can storage tell the devices apart so that it knows which ones to leave
to the kernel solution and which devices to accept so that udev can
issue an eject command?

Regards
Oliver

2009-12-16 18:05:32

by Matthew Dharm

[permalink] [raw]
Subject: Re: [PATCH] move eject code from zd1211rw to usb-storage

On Wed, Dec 16, 2009 at 11:29:27AM +0100, Oliver Neukum wrote:
> Am Dienstag, 15. Dezember 2009 19:03:00 schrieb Matthew Dharm:
> > > This would break existing systems and thus introduce a regression.
> > > We'd need to go through a feature removal process. For the time being
> > > I see no alternative to Seife's patch, as we cannot introduce ejection
> > > code to another wireless driver and need to support these devices.
> >
> > The right answer here is neither to move the eject code nor to introduce
> > more of it. New devices should be supported via userspace.
>
> Usually I would agree, but in this case the vendor reused IDs.
> The legacy kernel space switcher and user space would race.

So, let me see if I understand this... we have two devices that use the
same IDs, and get mode-switched the same way, but need different
post-switch drivers?

If this is the case, then the only reasonable answer to is to push the
modeswitch code for both into udev and out of the kernel. It will take
longer to support the new device that way (since we need to wait until udev
is updated and then remove kernel support), but that's what a vendor gets
for re-using IDs.

Matt

--
Matthew Dharm Home: [email protected]
Maintainer, Linux USB Mass Storage Driver

What, are you one of those Microsoft-bashing Linux freaks?
-- Customer to Greg
User Friendly, 2/10/1999


Attachments:
(No filename) (1.38 kB)
(No filename) (189.00 B)
Download all attachments

2009-12-15 18:03:16

by Matthew Dharm

[permalink] [raw]
Subject: Re: [PATCH] move eject code from zd1211rw to usb-storage

On Tue, Dec 15, 2009 at 04:11:08PM +0100, Oliver Neukum wrote:
> Am Dienstag, 15. Dezember 2009 13:31:13 schrieb Josua Dietze:
> > Stefan Seyfried schrieb:
> >
> > > The USB ID claimed by zd1211rw for the fake storage device is
> > > also in use by other, non-zd1211rw devices (Sphairon Homelink
> > > 1202). Move the eject of these devices to where it belongs and
> > > where all the needed infrastructure already exists: usb-storage.
> >
> >
> > We can do the eject (as other mode switches) in userspace. Previous
> > discussions pointed into that direction.
> >
> > And there is always the question of being able to access the "fake
> > CD-ROM" without hacking the kernel.
> >
> > I'd like to see these devices removed from unusual_devs.h.
>
> This would break existing systems and thus introduce a regression.
> We'd need to go through a feature removal process. For the time being
> I see no alternative to Seife's patch, as we cannot introduce ejection
> code to another wireless driver and need to support these devices.

The right answer here is neither to move the eject code nor to introduce
more of it. New devices should be supported via userspace.

Matt

--
Matthew Dharm Home: [email protected]
Maintainer, Linux USB Mass Storage Driver

A: The most ironic oxymoron wins ...
DP: "Microsoft Works"
A: Uh, okay, you win.
-- A.J. & Dust Puppy
User Friendly, 1/18/1998


Attachments:
(No filename) (1.40 kB)
(No filename) (189.00 B)
Download all attachments

2009-12-15 14:59:24

by Josua Dietze

[permalink] [raw]
Subject: Re: [PATCH] move eject code from zd1211rw to usb-storage

Stefan Seyfried schrieb:
> But given that there is already an "option_zero_cd" parameter in

> usb-storage, it should be easy to massage this into a general "do not
> eject virtual installer media" flag so that people are still able to
> get the windows driver files off their devices should they need to do
> so.
>
>> I'd like to see these devices removed from unusual_devs.h.
>
> I'd like them to work with one driver, not two (one in kernel and one
> in userspace).


The more basic arguments that prevailed at the last time such an
"eject" command was being considered for inclusion into usb-storage
were:

1. Userspace can obviously react a lot quicker than kernel space to
new or changed devices popping up.

2. Userspace works with older kernels immediately.

> That's a fundamental difference in opinions, and I fear I'm not the one
> who is going to decice how this will be handled ;)


Neither am I. But the number of known mode-switching USB devices is
now at around 50. New ones are arriving by the month or even week.

A decision to handle *all of them* in usb-storage would lead to the
disadvantages pointed out.

A decision to handle just *some of them* can hardly be made
plausible if there are no immediate technical reasons.

Oh, and in most cases (including your suggestion) there *are*
already two drivers necessary to make these devices work,
independent of where the switching is happening ...

> Have fun,

You too!

Josh
--
Man is the only creature on earth enabled to take a
warm meal while flying! Loriot

2009-12-16 11:22:59

by Oliver Neukum

[permalink] [raw]
Subject: Re: [PATCH] move eject code from zd1211rw to usb-storage

Am Mittwoch, 16. Dezember 2009 11:49:32 schrieb Stefan Seyfried:
> On Tue, 15 Dec 2009 10:03:00 -0800
> Matthew Dharm <[email protected]> wrote:
> > The right answer here is neither to move the eject code nor to introduce
> > more of it. New devices should be supported via userspace.
>
> Can somebody point me to the "userspace" that is meant here?
> Preferably some code that can be built actually?

Sure:
http://www.draisberghof.de/usb_modeswitch/

HTH
Oliver