2007-10-28 15:52:18

by Adrian Bunk

[permalink] [raw]
Subject: [RFC: 2.6 patch] remove the USB_STORAGE_ONETOUCH driver

Nearly two years ago, USB_STORAGE_ONETOUCH got a dependency on !PM.
Considering that this also implies that the driver is not available
e.g. when ACPI is enabled in the kernel that's not far from a dependency
on BROKEN.

This patch therefore removes this driver.

Signed-off-by: Adrian Bunk <[email protected]>

---

drivers/usb/storage/Kconfig | 12 -
drivers/usb/storage/Makefile | 1
drivers/usb/storage/onetouch.c | 237 -----------------------------
drivers/usb/storage/onetouch.h | 9 -
drivers/usb/storage/unusual_devs.h | 17 --
drivers/usb/storage/usb.c | 3
6 files changed, 279 deletions(-)

876e6730e6fb60856da318ad652a912757359f44
diff --git a/drivers/usb/storage/Kconfig b/drivers/usb/storage/Kconfig
index 7e53333..9b7df5f 100644
--- a/drivers/usb/storage/Kconfig
+++ b/drivers/usb/storage/Kconfig
@@ -121,18 +121,6 @@ config USB_STORAGE_ALAUDA
These devices are based on the Alauda chip and support both
XD and SmartMedia cards.

-config USB_STORAGE_ONETOUCH
- bool "Support OneTouch Button on Maxtor Hard Drives (EXPERIMENTAL)"
- depends on USB_STORAGE && INPUT_EVDEV && EXPERIMENTAL && !PM
- help
- Say Y here to include additional code to support the Maxtor OneTouch
- USB hard drive's onetouch button.
-
- This code registers the button on the front of Maxtor OneTouch USB
- hard drive's as an input device. An action can be associated with
- this input in any keybinding software. (e.g. gnome's keyboard short-
- cuts)
-
config USB_STORAGE_KARMA
bool "Support for Rio Karma music player"
depends on USB_STORAGE
diff --git a/drivers/usb/storage/Makefile b/drivers/usb/storage/Makefile
index 023969b..a93ae7a 100644
--- a/drivers/usb/storage/Makefile
+++ b/drivers/usb/storage/Makefile
@@ -19,7 +19,6 @@ usb-storage-obj-$(CONFIG_USB_STORAGE_ISD200) += isd200.o
usb-storage-obj-$(CONFIG_USB_STORAGE_DATAFAB) += datafab.o
usb-storage-obj-$(CONFIG_USB_STORAGE_JUMPSHOT) += jumpshot.o
usb-storage-obj-$(CONFIG_USB_STORAGE_ALAUDA) += alauda.o
-usb-storage-obj-$(CONFIG_USB_STORAGE_ONETOUCH) += onetouch.o
usb-storage-obj-$(CONFIG_USB_STORAGE_KARMA) += karma.o

usb-storage-objs := scsiglue.o protocol.o transport.o usb.o \
diff --git a/drivers/usb/storage/onetouch.c b/drivers/usb/storage/onetouch.c
deleted file mode 100644
index dfd42fe..0000000
--- a/drivers/usb/storage/onetouch.c
+++ /dev/null
@@ -1,237 +0,0 @@
-/*
- * Support for the Maxtor OneTouch USB hard drive's button
- *
- * Current development and maintenance by:
- * Copyright (c) 2005 Nick Sillik <[email protected]>
- *
- * Initial work by:
- * Copyright (c) 2003 Erik Thyren <[email protected]>
- *
- * Based on usbmouse.c (Vojtech Pavlik) and xpad.c (Marko Friedemann)
- *
- */
-
-/*
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
- *
- */
-
-#include <linux/kernel.h>
-#include <linux/input.h>
-#include <linux/init.h>
-#include <linux/slab.h>
-#include <linux/module.h>
-#include <linux/usb/input.h>
-#include "usb.h"
-#include "onetouch.h"
-#include "debug.h"
-
-void onetouch_release_input(void *onetouch_);
-
-struct usb_onetouch {
- char name[128];
- char phys[64];
- struct input_dev *dev; /* input device interface */
- struct usb_device *udev; /* usb device */
-
- struct urb *irq; /* urb for interrupt in report */
- unsigned char *data; /* input data */
- dma_addr_t data_dma;
- unsigned int is_open:1;
-};
-
-static void usb_onetouch_irq(struct urb *urb)
-{
- struct usb_onetouch *onetouch = urb->context;
- signed char *data = onetouch->data;
- struct input_dev *dev = onetouch->dev;
- int status = urb->status;
- int retval;
-
- switch (status) {
- case 0: /* success */
- break;
- case -ECONNRESET: /* unlink */
- case -ENOENT:
- case -ESHUTDOWN:
- return;
- /* -EPIPE: should clear the halt */
- default: /* error */
- goto resubmit;
- }
-
- input_report_key(dev, ONETOUCH_BUTTON, data[0] & 0x02);
- input_sync(dev);
-
-resubmit:
- retval = usb_submit_urb (urb, GFP_ATOMIC);
- if (retval)
- err ("can't resubmit intr, %s-%s/input0, retval %d",
- onetouch->udev->bus->bus_name,
- onetouch->udev->devpath, retval);
-}
-
-static int usb_onetouch_open(struct input_dev *dev)
-{
- struct usb_onetouch *onetouch = input_get_drvdata(dev);
-
- onetouch->is_open = 1;
- onetouch->irq->dev = onetouch->udev;
- if (usb_submit_urb(onetouch->irq, GFP_KERNEL)) {
- err("usb_submit_urb failed");
- return -EIO;
- }
-
- return 0;
-}
-
-static void usb_onetouch_close(struct input_dev *dev)
-{
- struct usb_onetouch *onetouch = input_get_drvdata(dev);
-
- usb_kill_urb(onetouch->irq);
- onetouch->is_open = 0;
-}
-
-#ifdef CONFIG_PM
-static void usb_onetouch_pm_hook(struct us_data *us, int action)
-{
- struct usb_onetouch *onetouch = (struct usb_onetouch *) us->extra;
-
- if (onetouch->is_open) {
- switch (action) {
- case US_SUSPEND:
- usb_kill_urb(onetouch->irq);
- break;
- case US_RESUME:
- if (usb_submit_urb(onetouch->irq, GFP_KERNEL) != 0)
- err("usb_submit_urb failed");
- break;
- default:
- break;
- }
- }
-}
-#endif /* CONFIG_PM */
-
-int onetouch_connect_input(struct us_data *ss)
-{
- struct usb_device *udev = ss->pusb_dev;
- struct usb_host_interface *interface;
- struct usb_endpoint_descriptor *endpoint;
- struct usb_onetouch *onetouch;
- struct input_dev *input_dev;
- int pipe, maxp;
- int error = -ENOMEM;
-
- interface = ss->pusb_intf->cur_altsetting;
-
- if (interface->desc.bNumEndpoints != 3)
- return -ENODEV;
-
- endpoint = &interface->endpoint[2].desc;
- if (!usb_endpoint_is_int_in(endpoint))
- return -ENODEV;
-
- pipe = usb_rcvintpipe(udev, endpoint->bEndpointAddress);
- maxp = usb_maxpacket(udev, pipe, usb_pipeout(pipe));
-
- onetouch = kzalloc(sizeof(struct usb_onetouch), GFP_KERNEL);
- input_dev = input_allocate_device();
- if (!onetouch || !input_dev)
- goto fail1;
-
- onetouch->data = usb_buffer_alloc(udev, ONETOUCH_PKT_LEN,
- GFP_ATOMIC, &onetouch->data_dma);
- if (!onetouch->data)
- goto fail1;
-
- onetouch->irq = usb_alloc_urb(0, GFP_KERNEL);
- if (!onetouch->irq)
- goto fail2;
-
- onetouch->udev = udev;
- onetouch->dev = input_dev;
-
- if (udev->manufacturer)
- strlcpy(onetouch->name, udev->manufacturer,
- sizeof(onetouch->name));
- if (udev->product) {
- if (udev->manufacturer)
- strlcat(onetouch->name, " ", sizeof(onetouch->name));
- strlcat(onetouch->name, udev->product, sizeof(onetouch->name));
- }
-
- if (!strlen(onetouch->name))
- snprintf(onetouch->name, sizeof(onetouch->name),
- "Maxtor Onetouch %04x:%04x",
- le16_to_cpu(udev->descriptor.idVendor),
- le16_to_cpu(udev->descriptor.idProduct));
-
- usb_make_path(udev, onetouch->phys, sizeof(onetouch->phys));
- strlcat(onetouch->phys, "/input0", sizeof(onetouch->phys));
-
- input_dev->name = onetouch->name;
- input_dev->phys = onetouch->phys;
- usb_to_input_id(udev, &input_dev->id);
- input_dev->dev.parent = &udev->dev;
-
- set_bit(EV_KEY, input_dev->evbit);
- set_bit(ONETOUCH_BUTTON, input_dev->keybit);
- clear_bit(0, input_dev->keybit);
-
- input_set_drvdata(input_dev, onetouch);
-
- input_dev->open = usb_onetouch_open;
- input_dev->close = usb_onetouch_close;
-
- usb_fill_int_urb(onetouch->irq, udev, pipe, onetouch->data,
- (maxp > 8 ? 8 : maxp),
- usb_onetouch_irq, onetouch, endpoint->bInterval);
- onetouch->irq->transfer_dma = onetouch->data_dma;
- onetouch->irq->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
-
- ss->extra_destructor = onetouch_release_input;
- ss->extra = onetouch;
-#ifdef CONFIG_PM
- ss->suspend_resume_hook = usb_onetouch_pm_hook;
-#endif
-
- error = input_register_device(onetouch->dev);
- if (error)
- goto fail3;
-
- return 0;
-
- fail3: usb_free_urb(onetouch->irq);
- fail2: usb_buffer_free(udev, ONETOUCH_PKT_LEN,
- onetouch->data, onetouch->data_dma);
- fail1: kfree(onetouch);
- input_free_device(input_dev);
- return error;
-}
-
-void onetouch_release_input(void *onetouch_)
-{
- struct usb_onetouch *onetouch = (struct usb_onetouch *) onetouch_;
-
- if (onetouch) {
- usb_kill_urb(onetouch->irq);
- input_unregister_device(onetouch->dev);
- usb_free_urb(onetouch->irq);
- usb_buffer_free(onetouch->udev, ONETOUCH_PKT_LEN,
- onetouch->data, onetouch->data_dma);
- }
-}
diff --git a/drivers/usb/storage/onetouch.h b/drivers/usb/storage/onetouch.h
deleted file mode 100644
index 41c7aa8..0000000
--- a/drivers/usb/storage/onetouch.h
+++ /dev/null
@@ -1,9 +0,0 @@
-#ifndef _ONETOUCH_H_
-#define _ONETOUCH_H_
-
-#define ONETOUCH_PKT_LEN 0x02
-#define ONETOUCH_BUTTON KEY_PROG1
-
-int onetouch_connect_input(struct us_data *ss);
-
-#endif
diff --git a/drivers/usb/storage/unusual_devs.h b/drivers/usb/storage/unusual_devs.h
index 22ab238..2d18eae 100644
--- a/drivers/usb/storage/unusual_devs.h
+++ b/drivers/usb/storage/unusual_devs.h
@@ -1282,23 +1282,6 @@ UNUSUAL_DEV( 0x0c0b, 0xa109, 0x0000, 0xffff,
US_FL_SINGLE_LUN ),
#endif

-/* Submitted by: Nick Sillik <[email protected]>
- * Needed for OneTouch extension to usb-storage
- *
- */
-#ifdef CONFIG_USB_STORAGE_ONETOUCH
- UNUSUAL_DEV( 0x0d49, 0x7000, 0x0000, 0x9999,
- "Maxtor",
- "OneTouch External Harddrive",
- US_SC_DEVICE, US_PR_DEVICE, onetouch_connect_input,
- 0),
- UNUSUAL_DEV( 0x0d49, 0x7010, 0x0000, 0x9999,
- "Maxtor",
- "OneTouch External Harddrive",
- US_SC_DEVICE, US_PR_DEVICE, onetouch_connect_input,
- 0),
-#endif
-
/*
* Pete Zaitcev <[email protected]>, bz#164688.
* The device blatantly ignores LUN and returns 1 in GetMaxLUN.
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index ac6114e..2691fa4 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -92,9 +92,6 @@
#ifdef CONFIG_USB_STORAGE_JUMPSHOT
#include "jumpshot.h"
#endif
-#ifdef CONFIG_USB_STORAGE_ONETOUCH
-#include "onetouch.h"
-#endif
#ifdef CONFIG_USB_STORAGE_ALAUDA
#include "alauda.h"
#endif


2007-10-28 17:48:21

by Torsten Kaiser

[permalink] [raw]
Subject: Re: [RFC: 2.6 patch] remove the USB_STORAGE_ONETOUCH driver

On 10/28/07, Adrian Bunk <[email protected]> wrote:
> Nearly two years ago, USB_STORAGE_ONETOUCH got a dependency on !PM.
> Considering that this also implies that the driver is not available
> e.g. when ACPI is enabled in the kernel that's not far from a dependency
> on BROKEN.
>
> This patch therefore removes this driver.

Hmm...

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=c9d00fc148b210aa8cf388d6e1eac187a0e855a6
-> Adds !PM because suspend was broken (17.11.2005)

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=7931e1c6f8007d5fef8a0bb2dc71bd97315eeae9
-> Adds suspend support, but fails to remove the !PM (5.12.2005)

So it looks like that !PM is false.

I have the following device:
Bus 001 Device 005: ID 1058:0400 Western Digital Technologies, Inc.
Bus 001 Device 004: ID 1058:0600 Western Digital Technologies, Inc.
Bus 001 Device 003: ID 1058:0500 Western Digital Technologies, Inc.
aka Western Digital Media Center
This firewire+usb hdd enclosure also has three buttons.
But it looks like that uses another driver:
hiddev0hidraw1: USB HID v1.10 Device [Western Digital External HDD] on
usb-0000:00:02.1-5.2

Otherwise I would be willing to try to test this, if someone would
tell me how to check that the second commit did fix the suspend
problem.

Torsten

2007-10-28 20:40:44

by Jiri Kosina

[permalink] [raw]
Subject: Re: [RFC: 2.6 patch] remove the USB_STORAGE_ONETOUCH driver

On Sun, 28 Oct 2007, Torsten Kaiser wrote:

> But it looks like that uses another driver:
> hiddev0hidraw1: USB HID v1.10 Device [Western Digital External HDD] on
> usb-0000:00:02.1-5.2
> Otherwise I would be willing to try to test this, if someone would tell
> me how to check that the second commit did fix the suspend problem.

If you need to unbind usbhid driver from the device and bind another one,
you can use the 'unbind' and 'bind' files in sysfs.

As soon as the driver you are willing to test is bound to the device, you
can go ahead with testing any functionality you wish (probably
suspend/resume cycle is needed here?).

If the device is claimed by usbhid driver (because its descriptor probably
states that it's HID-compliant device) and should be claimed by another
driver, it's necessary to add it to usbhid blacklist -- just let me know.

--
Jiri Kosina

2007-10-29 02:38:22

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [RFC: 2.6 patch] remove the USB_STORAGE_ONETOUCH driver

On Sunday 28 October 2007 16:39, Jiri Kosina wrote:
> On Sun, 28 Oct 2007, Torsten Kaiser wrote:
>
> > But it looks like that uses another driver:
> > hiddev0hidraw1: USB HID v1.10 Device [Western Digital External HDD] on
> > usb-0000:00:02.1-5.2
> > Otherwise I would be willing to try to test this, if someone would tell
> > me how to check that the second commit did fix the suspend problem.
>
> If you need to unbind usbhid driver from the device and bind another one,
> you can use the 'unbind' and 'bind' files in sysfs.
>
> As soon as the driver you are willing to test is bound to the device, you
> can go ahead with testing any functionality you wish (probably
> suspend/resume cycle is needed here?).
>
> If the device is claimed by usbhid driver (because its descriptor probably
> states that it's HID-compliant device) and should be claimed by another
> driver, it's necessary to add it to usbhid blacklist -- just let me know.
>

Maybe onetouch driver is not needed after all. I wonder what key/button does
HID deriver reports when it binds to it... Torsten, could you please post
your /proc/bus/input/devices?

--
Dmitry

2007-10-29 06:01:36

by Torsten Kaiser

[permalink] [raw]
Subject: Re: [RFC: 2.6 patch] remove the USB_STORAGE_ONETOUCH driver

On 10/29/07, Dmitry Torokhov <[email protected]> wrote:
> On Sunday 28 October 2007 16:39, Jiri Kosina wrote:
> > On Sun, 28 Oct 2007, Torsten Kaiser wrote:
> >
> > > But it looks like that uses another driver:
> > > hiddev0hidraw1: USB HID v1.10 Device [Western Digital External HDD] on
> > > usb-0000:00:02.1-5.2
> > > Otherwise I would be willing to try to test this, if someone would tell
> > > me how to check that the second commit did fix the suspend problem.
[snip]
> Maybe onetouch driver is not needed after all. I wonder what key/button does
> HID deriver reports when it binds to it... Torsten, could you please post
> your /proc/bus/input/devices?

I only read the mail from Adrian, because I was 100% sure that my
device is such a Maxtor Onetouch, because it's a external HDD and has
buttons. (I even compiled this driver into my kernel when it was
added...)
But after finding these two git commits and starting to write my
initial mail I looked for the exact name of my device and found that
this is not a Maxtor at all but a Western Digital.
So I'm rather unsure if this driver will work for me...

Anyway here is my /proc/bus/input/devices:
I: Bus=0019 Vendor=0000 Product=0002 Version=0000
N: Name="Power Button (FF)"
P: Phys=LNXPWRBN/button/input0
S: Sysfs=/devices/virtual/input/input0
U: Uniq=
H: Handlers=kbd event0
B: EV=3
B: KEY=10000000000000 0

I: Bus=0019 Vendor=0000 Product=0001 Version=0000
N: Name="Power Button (CM)"
P: Phys=PNP0C0C/button/input0
S: Sysfs=/devices/virtual/input/input1
U: Uniq=
H: Handlers=kbd event1
B: EV=3
B: KEY=10000000000000 0

I: Bus=0011 Vendor=0001 Product=0001 Version=ab41
N: Name="AT Translated Set 2 keyboard"
P: Phys=isa0060/serio0/input0
S: Sysfs=/devices/platform/i8042/serio0/input/input2
U: Uniq=
H: Handlers=kbd event2
B: EV=120013
B: KEY=402000000 3803078f800d001 feffffdfffefffff fffffffffffffffe
B: MSC=10
B: LED=7

I: Bus=0010 Vendor=001f Product=0001 Version=0100
N: Name="PC Speaker"
P: Phys=isa0061/input0
S: Sysfs=/devices/platform/pcspkr/input/input3
U: Uniq=
H: Handlers=kbd event3
B: EV=40001
B: SND=6

I: Bus=0003 Vendor=062a Product=0000 Version=0110
N: Name="HID 062a:0000"
P: Phys=usb-0000:00:02.0-9/input0
S: Sysfs=/devices/pci0000:00/0000:00:02.0/usb2/2-9/2-9:1.0/input/input4
U: Uniq=
H: Handlers=mouse0 event4
B: EV=17
B: KEY=70000 0 0 0 0
B: REL=103
B: MSC=10

The button part of the device is visible in sysfs under bus/usb, but
not under class/input:
(There is also a cardreader, a hub and the hdd visible under
/sys/bus/usb/devices/1-5 )

/sys/bus/usb/devices/1-5.2:1.1
|-- driver -> ../../../../../../../bus/usb/drivers/usbhid
|-- ep_81 -> ../../../../../../../devices/pci0000:00/0000:00:02.1/usb1/1-5/1-5.2/1-5.2:1.1/usb_endpoint/usbdev1.5_ep81
|-- subsystem -> ../../../../../../../bus/usb
|-- usb
| `-- hiddev0
| |-- device ->
../../../../../../../../../devices/pci0000:00/0000:00:02.1/usb1/1-5/1-5.2/1-5.2:1.1
| `-- subsystem -> ../../../../../../../../../class/usb
`-- usb_endpoint
`-- usbdev1.5_ep81
|-- device ->
../../../../../../../../../devices/pci0000:00/0000:00:02.1/usb1/1-5/1-5.2/1-5.2:1.1
`-- subsystem -> ../../../../../../../../../class/usb_endpoint

Torsten

2007-10-29 14:22:17

by Alan Stern

[permalink] [raw]
Subject: Re: [linux-usb-devel] [RFC: 2.6 patch] remove the USB_STORAGE_ONETOUCH driver

On Sun, 28 Oct 2007, Dmitry Torokhov wrote:

> On Sunday 28 October 2007 16:39, Jiri Kosina wrote:
> > On Sun, 28 Oct 2007, Torsten Kaiser wrote:
> >
> > > But it looks like that uses another driver:
> > > hiddev0hidraw1: USB HID v1.10 Device [Western Digital External HDD] on
> > > usb-0000:00:02.1-5.2
> > > Otherwise I would be willing to try to test this, if someone would tell
> > > me how to check that the second commit did fix the suspend problem.
> >
> > If you need to unbind usbhid driver from the device and bind another one,
> > you can use the 'unbind' and 'bind' files in sysfs.
> >
> > As soon as the driver you are willing to test is bound to the device, you
> > can go ahead with testing any functionality you wish (probably
> > suspend/resume cycle is needed here?).
> >
> > If the device is claimed by usbhid driver (because its descriptor probably
> > states that it's HID-compliant device) and should be claimed by another
> > driver, it's necessary to add it to usbhid blacklist -- just let me know.
> >
>
> Maybe onetouch driver is not needed after all. I wonder what key/button does
> HID deriver reports when it binds to it... Torsten, could you please post
> your /proc/bus/input/devices?

You guys all sound confused about this.

The original reason writing for the onetouch driver was because the
button on this disk drive isn't an HID device! That is, the drive
doesn't include any of the usual HID descriptors for the button and the
button doesn't send HID-style reports.

Torsten's assessment was correct. Originally there was no PM support
for the onetouch driver. Now there is (although it isn't very good),
so it would be legitimate to remove the !PM condition.

As far as I know, the only person who ever used the onetouch driver was
its author.

Alan Stern

2007-10-29 14:29:28

by Jiri Kosina

[permalink] [raw]
Subject: Re: [linux-usb-devel] [RFC: 2.6 patch] remove the USB_STORAGE_ONETOUCH driver

On Mon, 29 Oct 2007, Alan Stern wrote:

> You guys all sound confused about this. The original reason writing for
> the onetouch driver was because the button on this disk drive isn't an
> HID device! That is, the drive doesn't include any of the usual HID
> descriptors for the button and the button doesn't send HID-style
> reports.

The only reason I was talking about it in terms of being a HID device was
because Thorsten stated that usbhid driver gets bound to it.

Now that we know that Torsten was having different HW, my point is of
course invalid :)

Thanks,

--
Jiri Kosina