* Re: [PATCH 1/1] HID: usbhid: add usb_clear_halt determination for next hid_start_in
From: Alan Stern @ 2014-08-22 18:23 UTC (permalink / raw)
To: vichy; +Cc: Jiri Kosina, linux-usb@vger.kernel.org, linux-input
In-Reply-To: <CAOVJa8HLSQ9cqEHR8srT3QikvJ6i0ZYFqmOZ7MvMpN5aULdk9A@mail.gmail.com>
On Sat, 23 Aug 2014, vichy wrote:
> from your patch, I have some questions:
> a. in Alan's version, if both HID_CLEAR_HALT and HID_RESET_PENDING are
> set, hid_reset will both "clear ep halt" and "reset devcie".
> But in original one, even HID_CLEAR_HALT and HID_RESET_PENDING are
> both set, hid_reset only do one of them.
Yes. In my patch, the clear-halt handler will turn on the
HID_RESET_PENDING bit if something goes wrong. In that case we want to
do both things.
> is there any special reason in original hid_reset to use below flow?
> if (test_bit(HID_CLEAR_HALT, &usbhid->iofl)) {
> xxxxx
> } else if (test_bit(HID_RESET_PENDING, &usbhid->iofl)) {
> xxxxxx
> }
No special reason. We probably never thought that both flags would be
set.
> b. in original hid_reset, if "Clear halt ep" or "Rest device" success
> or none of those flags raise up, it will call hid_io_error for later
> resending the urb.
No, the clear-halt part calls hid_start_in when everything works. It
doesn't call hid_io_error, because hid_start_in turns on the
HID_IN_RUNNING flag.
> Shall we need to call "hid_io_error(hid);" in the
> end if "clear halt" or "reset device" success?
In the clear-halt case, we don't have to because we call hid_start_in.
In the reset device case, we don't have to because hid_post_reset calls
hid_start_in if there are no errors.
> c. why we chose to use usb_queue_reset_device instead of usb_reset_device()?
I originally tried using usb_reset_device, and it caused a deadlock:
Unplug the HID device.
I/O error occurs. hid_io_error schedules reset_work.
The reset_work callback routine is hid_reset. It calls
usb_reset_device.
The reset fails because the device is gone. At the end of the
reset, hid_post_reset is called.
hid_post_reset returns 1 because hid_get_class_descriptor
fails.
Because the post_reset routine failed, usb_reset_device calls
usb_unbind_and_rebind_marked_interfaces.
That routine indirectly calls usbhid_disconnect.
usbhid_disconnect calls usbhid_close by way of
hid_destroy_device.
usbhid_close calls hid_cancel_delayed_stuff.
hid_cancel_delayed_stuff calls cancel_work_sync for reset_work.
So the reset_work routine tries to cancel itself synchronously while it
is running. usb_queue_reset_device was carefully written to avoid such
deadlocks.
> d. shall we "useusb_lock_device_for_reset(hid_to_usb_dev(hid),
> usbhid->intf)" or "spin_lock_irq(&usbhid->lock)" before calling
> usb_queue_reset_device?
No. usb_queue_reset_device takes care of the locking.
Alan Stern
^ permalink raw reply
* [PATCH 1/2] HID: logitech-dj: prevent false errors to be shown
From: Benjamin Tissoires @ 2014-08-22 20:16 UTC (permalink / raw)
To: Jiri Kosina; +Cc: linux-input, linux-kernel
Commit "HID: logitech: perform bounds checking on device_id early
enough" unfortunately leaks some errors to dmesg which are not real
ones:
- if the report is not a DJ one, then there is not point in checking
the device_id
- the receiver (index 0) can also receive some notifications which
can be safely ignored given the current implementation
Move out the test regarding the report_id and also discards
printing errors when the receiver got notified.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/hid-logitech-dj.c | 43 +++++++++++++++++++++++++------------------
drivers/hid/hid-logitech-dj.h | 1 +
2 files changed, 26 insertions(+), 18 deletions(-)
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index b7ba829..9bf8637 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -656,7 +656,6 @@ static int logi_dj_raw_event(struct hid_device *hdev,
struct dj_receiver_dev *djrcv_dev = hid_get_drvdata(hdev);
struct dj_report *dj_report = (struct dj_report *) data;
unsigned long flags;
- bool report_processed = false;
dbg_hid("%s, size:%d\n", __func__, size);
@@ -683,34 +682,42 @@ static int logi_dj_raw_event(struct hid_device *hdev,
* device (via hid_input_report() ) and return 1 so hid-core does not do
* anything else with it.
*/
+
+ /* case 1) */
+ if (data[0] != REPORT_ID_DJ_SHORT)
+ return false;
+
if ((dj_report->device_index < DJ_DEVICE_INDEX_MIN) ||
(dj_report->device_index > DJ_DEVICE_INDEX_MAX)) {
- dev_err(&hdev->dev, "%s: invalid device index:%d\n",
+ /*
+ * Device index is wrong, bail out.
+ * This driver can ignore safely the receiver notifications,
+ * so ignore those reports too.
+ */
+ if (dj_report->device_index != DJ_RECEIVER_INDEX)
+ dev_err(&hdev->dev, "%s: invalid device index:%d\n",
__func__, dj_report->device_index);
return false;
}
spin_lock_irqsave(&djrcv_dev->lock, flags);
- if (dj_report->report_id == REPORT_ID_DJ_SHORT) {
- switch (dj_report->report_type) {
- case REPORT_TYPE_NOTIF_DEVICE_PAIRED:
- case REPORT_TYPE_NOTIF_DEVICE_UNPAIRED:
- logi_dj_recv_queue_notification(djrcv_dev, dj_report);
- break;
- case REPORT_TYPE_NOTIF_CONNECTION_STATUS:
- if (dj_report->report_params[CONNECTION_STATUS_PARAM_STATUS] ==
- STATUS_LINKLOSS) {
- logi_dj_recv_forward_null_report(djrcv_dev, dj_report);
- }
- break;
- default:
- logi_dj_recv_forward_report(djrcv_dev, dj_report);
+ switch (dj_report->report_type) {
+ case REPORT_TYPE_NOTIF_DEVICE_PAIRED:
+ case REPORT_TYPE_NOTIF_DEVICE_UNPAIRED:
+ logi_dj_recv_queue_notification(djrcv_dev, dj_report);
+ break;
+ case REPORT_TYPE_NOTIF_CONNECTION_STATUS:
+ if (dj_report->report_params[CONNECTION_STATUS_PARAM_STATUS] ==
+ STATUS_LINKLOSS) {
+ logi_dj_recv_forward_null_report(djrcv_dev, dj_report);
}
- report_processed = true;
+ break;
+ default:
+ logi_dj_recv_forward_report(djrcv_dev, dj_report);
}
spin_unlock_irqrestore(&djrcv_dev->lock, flags);
- return report_processed;
+ return true;
}
static int logi_dj_probe(struct hid_device *hdev,
diff --git a/drivers/hid/hid-logitech-dj.h b/drivers/hid/hid-logitech-dj.h
index 4a40003..daeb0aa 100644
--- a/drivers/hid/hid-logitech-dj.h
+++ b/drivers/hid/hid-logitech-dj.h
@@ -27,6 +27,7 @@
#define DJ_MAX_PAIRED_DEVICES 6
#define DJ_MAX_NUMBER_NOTIFICATIONS 8
+#define DJ_RECEIVER_INDEX 0
#define DJ_DEVICE_INDEX_MIN 1
#define DJ_DEVICE_INDEX_MAX 6
--
2.1.0
^ permalink raw reply related
* [PATCH 2/2] HID: logitech-dj: break out testing of validity of dj_device
From: Benjamin Tissoires @ 2014-08-22 20:16 UTC (permalink / raw)
To: Jiri Kosina; +Cc: linux-input, linux-kernel
In-Reply-To: <1408738566-6805-1-git-send-email-benjamin.tissoires@redhat.com>
We can do once the test of the validity of the dj_device, which removes
some duplicated code in various functions.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/hid-logitech-dj.c | 35 +++++++++++------------------------
1 file changed, 11 insertions(+), 24 deletions(-)
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 9bf8637..71f5692 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -385,18 +385,6 @@ static void logi_dj_recv_forward_null_report(struct dj_receiver_dev *djrcv_dev,
djdev = djrcv_dev->paired_dj_devices[dj_report->device_index];
- if (!djdev) {
- dbg_hid("djrcv_dev->paired_dj_devices[dj_report->device_index]"
- " is NULL, index %d\n", dj_report->device_index);
- kfifo_in(&djrcv_dev->notif_fifo, dj_report, sizeof(struct dj_report));
-
- if (schedule_work(&djrcv_dev->work) == 0) {
- dbg_hid("%s: did not schedule the work item, was already "
- "queued\n", __func__);
- }
- return;
- }
-
memset(reportbuffer, 0, sizeof(reportbuffer));
for (i = 0; i < NUMBER_OF_HID_REPORTS; i++) {
@@ -421,18 +409,6 @@ static void logi_dj_recv_forward_report(struct dj_receiver_dev *djrcv_dev,
dj_device = djrcv_dev->paired_dj_devices[dj_report->device_index];
- if (dj_device == NULL) {
- dbg_hid("djrcv_dev->paired_dj_devices[dj_report->device_index]"
- " is NULL, index %d\n", dj_report->device_index);
- kfifo_in(&djrcv_dev->notif_fifo, dj_report, sizeof(struct dj_report));
-
- if (schedule_work(&djrcv_dev->work) == 0) {
- dbg_hid("%s: did not schedule the work item, was already "
- "queued\n", __func__);
- }
- return;
- }
-
if ((dj_report->report_type > ARRAY_SIZE(hid_reportid_size_map) - 1) ||
(hid_reportid_size_map[dj_report->report_type] == 0)) {
dbg_hid("invalid report type:%x\n", dj_report->report_type);
@@ -701,8 +677,17 @@ static int logi_dj_raw_event(struct hid_device *hdev,
}
spin_lock_irqsave(&djrcv_dev->lock, flags);
+
+ if (!djrcv_dev->paired_dj_devices[dj_report->device_index]) {
+ /* received an event for an unknown device, bail out */
+ logi_dj_recv_queue_notification(djrcv_dev, dj_report);
+ goto out;
+ }
+
switch (dj_report->report_type) {
case REPORT_TYPE_NOTIF_DEVICE_PAIRED:
+ /* pairing notifications are handled above the switch */
+ break;
case REPORT_TYPE_NOTIF_DEVICE_UNPAIRED:
logi_dj_recv_queue_notification(djrcv_dev, dj_report);
break;
@@ -715,6 +700,8 @@ static int logi_dj_raw_event(struct hid_device *hdev,
default:
logi_dj_recv_forward_report(djrcv_dev, dj_report);
}
+
+out:
spin_unlock_irqrestore(&djrcv_dev->lock, flags);
return true;
--
2.1.0
^ permalink raw reply related
* Lenovo T60 and X60: psmouse serio1: alps: Unknown ALPS touchpad: E7=10 00 64, EC=10 00 64
From: Paul Menzel @ 2014-08-23 10:16 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Yunkang Tang
[-- Attachment #1: Type: text/plain, Size: 1928 bytes --]
Dear Linux developers,
with at least Linux 3.12, 3.13 and 3.14 the Linux kernel log contains a
message about an unknown ALPS touchpad.
[…]
[ 8.793232] psmouse serio1: alps: Unknown ALPS touchpad: E7=10 00 64, EC=10 00 64
[ 9.068102] Console: switching to colour frame buffer device 128x48
[ 9.072053] i915 0000:00:02.0: fb0: inteldrmfb frame buffer device
[ 9.072056] i915 0000:00:02.0: registered panic notifier
[ 9.072075] [drm] Initialized i915 1.6.0 20080730 for 0000:00:02.0 on minor 0
[ 9.073703] hda_intel: probe_mask set to 0x1 for device 17aa:2010
[ 9.074158] snd_hda_intel 0000:00:1b.0: irq 47 for MSI/MSI-X
[ 9.104435] input: HDA Digital PCBeep as /devices/pci0000:00/0000:00:1b.0/input/input8
[ 9.107546] input: HDA Intel Front Headphone as /devices/pci0000:00/0000:00:1b.0/sound/card0/input13
[ 9.107678] input: HDA Intel Line Out as /devices/pci0000:00/0000:00:1b.0/sound/card0/input12
[ 9.107793] input: HDA Intel Line as /devices/pci0000:00/0000:00:1b.0/sound/card0/input11
[ 9.107909] input: HDA Intel Front Mic as /devices/pci0000:00/0000:00:1b.0/sound/card0/input10
[ 9.108044] input: HDA Intel Rear Mic as /devices/pci0000:00/0000:00:1b.0/sound/card0/input9
[ 9.168843] psmouse serio1: trackpoint: IBM TrackPoint firmware: 0x0e, buttons: 3/3
[ 9.189695] input: TPPS/2 IBM TrackPoint as /devices/platform/i8042/serio1/input/input6
[…]
The Lenovo T60 actually has a touchpad, a trackpoint and three buttons,
where the Lenovo X60 only has a trackpoint and three buttons.
Searching the Web with this message, there are also hits for other
laptop models.
Looking at `drivers/input/mouse/alps.c` the values checked there for E7
and EC are not related at all. It’s also interesting that E7 and EC
contain the same values.
Is that the trackpoint device detected later and should therefore be
ignored?
Thanks,
Paul
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* [RFC][PATCH] Add support for mouse logitech m560
From: Goffredo Baroncelli @ 2014-08-23 12:40 UTC (permalink / raw)
To: linux-input
Hi all,
I bought the mouse in subject. This is a mouse designed for windows
8. Comparing to a standard one, some buttons (the middle one and the
two ones placed on the side) are bounded to a key combination
instead of a button.
Think this mouse as a pair of mouse and keyboard. When the middle
button is pressed the first time it sends a key (as keyboard)
combination, the same for the other two side button.
Instead the left/right/wheel work correctly.
To complicate further the things, the middle button send a
key combination the odd press, and another one for the even press;
in the latter case it sends also a left click. No event is
generated when the middle button is released.
It is very frustrating to use this mouse in Linux.
Moreover this device is a wireless mouse which uses the unifying
receiver.
I modified the driver hid-logitech-dj.c to support this, and now
it behaves like an ordinary mouse. I added some hooks to the
standard driver, which translate the "keyboard" event into the
appropriate mouse event.
These hooks call the "right" handler on the basis of the name of
the device (the USB id are the one of the receiver).
Below the patch. This is an RFC because I am not sure if this is
the right thing to do. I would prefer implements these handler in
other kernel module (to simplify the developing), bu the logic of
the plug-and-play (call the right driver when the device is
discovered and/or the driver is available), fights when exists
a "default driver" (i.e the one provided today by
hid-logitech-dj.c.
BR
G.Baroncelli
P.S.
Please put me in CC in case of reply because I am not subscribed
to the mailing list. Thank
--
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
^ permalink raw reply
* [PATCH 1/3] Don't copy more than 'count' byte
From: Goffredo Baroncelli @ 2014-08-23 12:40 UTC (permalink / raw)
To: linux-input; +Cc: Goffredo Baroncelli
In-Reply-To: <1408797629-3474-1-git-send-email-kreijack@inwind.it>
Don't copy more than 'count' byte, because *buf may be
shorter than DJREPORT_SHORT_LENGTH,and this means an
access violation. Of course count cannot be greather
than the result buffer.
Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
---
drivers/hid/hid-logitech-dj.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 486dbde..ca0ab51 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -557,7 +557,7 @@ static int logi_dj_ll_raw_request(struct hid_device *hid,
if (!out_buf)
return -ENOMEM;
- if (count < DJREPORT_SHORT_LENGTH - 2)
+ if (count > DJREPORT_SHORT_LENGTH - 2)
count = DJREPORT_SHORT_LENGTH - 2;
out_buf[0] = REPORT_ID_DJ_SHORT;
--
1.9.3
^ permalink raw reply related
* [PATCH 2/3] Add support for specific device methods.
From: Goffredo Baroncelli @ 2014-08-23 12:40 UTC (permalink / raw)
To: linux-input; +Cc: Goffredo Baroncelli
In-Reply-To: <1408797629-3474-1-git-send-email-kreijack@inwind.it>
This patch adds hook into the code to add support for specific
device driver. The right driver is select on the basis of the name.
4 hooks are added:
- init_device() called during the device initialization
- parse() called during the device description generation phase
- parse_raw_event() which handles the events from the device
- destroy() called when the device is destroyed
The construction of the device is dived in two phases:
1) the device is discovered by the receiver. The struct dj_device
is allocated BUT the device is not enabled.
It is asked the device name top the device itself
2) when the device name arrived from the device, the
the right driver is selected, structure dj_device is finalized,
the hid-device id created.
If the device name returned doesn't match, the generic driver
(e.g. the one provided until now) is used.
Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
---
drivers/hid/hid-logitech-dj.c | 422 ++++++++++++++++++++++++++++++++++--------
drivers/hid/hid-logitech-dj.h | 41 ++++
2 files changed, 388 insertions(+), 75 deletions(-)
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index ca0ab51..feddd3d 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -193,7 +193,51 @@ static const u8 hid_reportid_size_map[NUMBER_OF_HID_REPORTS] = {
static struct hid_ll_driver logi_dj_ll_driver;
+static inline struct dj_device *logi_djrcv2djdev(
+ struct dj_receiver_dev *djrcv_dev,
+ int index);
static int logi_dj_recv_query_paired_devices(struct dj_receiver_dev *djrcv_dev);
+static int logi_dj_recv_send_report(struct dj_receiver_dev *djrcv_dev,
+ struct dj_report *dj_report);
+
+static inline int call_init_device(struct dj_device *djdev)
+{
+ if (!djdev || !djdev->methods || !djdev->methods->init_device)
+ return false;
+ return djdev->methods->init_device(djdev);
+}
+
+static inline int call_parse_raw_event(struct dj_device *djdev,
+ struct dj_report *djrep)
+{
+ if (!djdev || !djdev->methods || !djdev->methods->parse_raw_event)
+ return false;
+ return djdev->methods->parse_raw_event(djdev, djrep);
+}
+
+static inline void call_destroy(struct dj_device *djdev)
+{
+ if (!djdev || !djdev->methods || !djdev->methods->destroy)
+ return;
+ djdev->methods->destroy(djdev);
+}
+
+static struct dj_device_method dj_device_method[] = {
+
+ /* last element */
+ { NULL, }
+};
+
+static inline struct dj_device *logi_djrcv2djdev(
+ struct dj_receiver_dev *djrcv_dev,
+ int index)
+{
+ if ((index < DJ_DEVICE_INDEX_MIN) ||
+ (index > DJ_DEVICE_INDEX_MAX))
+ return NULL;
+
+ return djrcv_dev->paired_dj_devices[index];
+}
static void logi_dj_recv_destroy_djhid_device(struct dj_receiver_dev *djrcv_dev,
struct dj_report *dj_report)
@@ -203,7 +247,11 @@ static void logi_dj_recv_destroy_djhid_device(struct dj_receiver_dev *djrcv_dev,
unsigned long flags;
spin_lock_irqsave(&djrcv_dev->lock, flags);
- dj_dev = djrcv_dev->paired_dj_devices[dj_report->device_index];
+ dj_dev = logi_djrcv2djdev(djrcv_dev, dj_report->device_index);
+ if (!dj_dev)
+ return;
+ call_destroy(dj_dev);
+
djrcv_dev->paired_dj_devices[dj_report->device_index] = NULL;
spin_unlock_irqrestore(&djrcv_dev->lock, flags);
@@ -216,93 +264,270 @@ static void logi_dj_recv_destroy_djhid_device(struct dj_receiver_dev *djrcv_dev,
}
}
-static void logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev,
+/*
+ * This function send a request of the device name
+ * see "Logitech hidpp 1.0 excerpt for public release" chapter 4.5.3
+ * for further information.
+ *
+ * The results are read in logi_dj_raw_event()
+ */
+static int logi_send_name_request(struct dj_device *dj_device)
+{
+ struct dj_report *dj_report;
+ int retval;
+
+ dj_report = kzalloc(sizeof(struct dj_report), GFP_KERNEL);
+ if (!dj_report)
+ return -ENOMEM;
+
+ dj_report->report_id = REPORT_ID_RECV_SHORT;
+ dj_report->device_index = REPORT_RECEIVER_INDEX;
+ dj_report->report_type = REPORT_GET_LONG_REGISTER_REQ;
+ dj_report->report_params[0] = REPORT_REG_PAIRING_INFORMATION;
+
+ /*
+ * 0x40 device id 1, 0x41 device id 2....
+ */
+ dj_report->report_params[1] = 0x40 + dj_device->device_index - 1;
+
+ retval = hid_hw_raw_request(dj_device->dj_receiver_dev->hdev,
+ dj_report->report_id,
+ (void *)dj_report, REPORT_ID_RECV_SHORT_LENGTH,
+ HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
+
+ kfree(dj_report);
+ return retval;
+}
+
+static int logi_dj_recv_create_dj_device(struct dj_receiver_dev *djrcv_dev,
struct dj_report *dj_report)
{
/* Called in delayed work context */
struct hid_device *djrcv_hdev = djrcv_dev->hdev;
- struct usb_interface *intf = to_usb_interface(djrcv_hdev->dev.parent);
- struct usb_device *usbdev = interface_to_usbdev(intf);
- struct hid_device *dj_hiddev;
struct dj_device *dj_dev;
- /* Device index goes from 1 to 6, we need 3 bytes to store the
- * semicolon, the index, and a null terminator
- */
- unsigned char tmpstr[3];
-
if (dj_report->report_params[DEVICE_PAIRED_PARAM_SPFUNCTION] &
SPFUNCTION_DEVICE_LIST_EMPTY) {
dbg_hid("%s: device list is empty\n", __func__);
djrcv_dev->querying_devices = false;
- return;
+ return -EINVAL;
}
if ((dj_report->device_index < DJ_DEVICE_INDEX_MIN) ||
(dj_report->device_index > DJ_DEVICE_INDEX_MAX)) {
dev_err(&djrcv_hdev->dev, "%s: invalid device index:%d\n",
__func__, dj_report->device_index);
- return;
+ return -EINVAL;
}
if (djrcv_dev->paired_dj_devices[dj_report->device_index]) {
/* The device is already known. No need to reallocate it. */
dbg_hid("%s: device is already known\n", __func__);
- return;
+ return -EEXIST;
+ }
+
+ dj_dev = kzalloc(sizeof(struct dj_device), GFP_KERNEL);
+
+ if (!dj_dev) {
+ dev_err(&djrcv_hdev->dev, "%s: failed allocating dj_device\n",
+ __func__);
+ return -ENOMEM;
}
+ dj_dev->reports_supported = get_unaligned_le32(
+ dj_report->report_params + DEVICE_PAIRED_RF_REPORT_TYPE);
+ dj_dev->dj_receiver_dev = djrcv_dev;
+ dj_dev->device_index = dj_report->device_index;
+ dj_dev->enabled = false;
+
+ dj_dev->wpid =
+ dj_report->report_params[DEVICE_PAIRED_PARAM_EQUAD_ID_MSB]<<8|
+ dj_report->report_params[DEVICE_PAIRED_PARAM_EQUAD_ID_LSB];
+
+ djrcv_dev->paired_dj_devices[dj_report->device_index] = dj_dev;
+
+ return 0;
+
+}
+
+/*
+ * this function finalize the device
+ */
+static int logi_dj_recv_add_djhid_device(struct dj_device *dj_device)
+{
+ struct hid_device *dj_hiddev;
+ struct dj_receiver_dev *djrcv_dev;
+
+ struct hid_device *djrcv_hdev;
+ struct usb_interface *intf;
+ struct usb_device *usbdev;
+
+ /* Device index goes from 1 to 6, we need 3 bytes to store the
+ * semicolon, the index, and a null terminator
+ */
+ unsigned char tmpstr[3];
+
+ BUG_ON(!dj_device);
+ djrcv_dev = dj_device->dj_receiver_dev;
+ djrcv_hdev = djrcv_dev->hdev;
+
+ /* Called in delayed work context */
+ intf = to_usb_interface(djrcv_hdev->dev.parent);
+ usbdev = interface_to_usbdev(intf);
+
dj_hiddev = hid_allocate_device();
if (IS_ERR(dj_hiddev)) {
dev_err(&djrcv_hdev->dev, "%s: hid_allocate_device failed\n",
__func__);
- return;
+ return -ENOMEM;
}
dj_hiddev->ll_driver = &logi_dj_ll_driver;
-
+ dj_device->hdev = dj_hiddev;
+ dj_hiddev->driver_data = dj_device;
dj_hiddev->dev.parent = &djrcv_hdev->dev;
dj_hiddev->bus = BUS_USB;
dj_hiddev->vendor = le16_to_cpu(usbdev->descriptor.idVendor);
dj_hiddev->product = le16_to_cpu(usbdev->descriptor.idProduct);
snprintf(dj_hiddev->name, sizeof(dj_hiddev->name),
- "Logitech Unifying Device. Wireless PID:%02x%02x",
- dj_report->report_params[DEVICE_PAIRED_PARAM_EQUAD_ID_MSB],
- dj_report->report_params[DEVICE_PAIRED_PARAM_EQUAD_ID_LSB]);
+ "Logitech Unifying Device. Wireless PID:%04x",
+ dj_device->wpid);
usb_make_path(usbdev, dj_hiddev->phys, sizeof(dj_hiddev->phys));
- snprintf(tmpstr, sizeof(tmpstr), ":%d", dj_report->device_index);
+ snprintf(tmpstr, sizeof(tmpstr), ":%d", dj_device->device_index);
strlcat(dj_hiddev->phys, tmpstr, sizeof(dj_hiddev->phys));
- dj_dev = kzalloc(sizeof(struct dj_device), GFP_KERNEL);
-
- if (!dj_dev) {
- dev_err(&djrcv_hdev->dev, "%s: failed allocating dj_device\n",
+ if (hid_add_device(dj_hiddev)) {
+ dev_err(&djrcv_dev->hdev->dev,
+ "%s: failed adding dj_device\n",
__func__);
- goto dj_device_allocate_fail;
+ djrcv_dev->paired_dj_devices[dj_device->device_index] = NULL;
+ kfree(dj_device);
+ hid_destroy_device(dj_hiddev);
+ return -ENODEV;
}
- dj_dev->reports_supported = get_unaligned_le32(
- dj_report->report_params + DEVICE_PAIRED_RF_REPORT_TYPE);
- dj_dev->hdev = dj_hiddev;
- dj_dev->dj_receiver_dev = djrcv_dev;
- dj_dev->device_index = dj_report->device_index;
- dj_hiddev->driver_data = dj_dev;
+ hid_info(djrcv_dev->hdev, "Device %d: '%s' added\n",
+ dj_device->device_index, dj_device->device_name);
- djrcv_dev->paired_dj_devices[dj_report->device_index] = dj_dev;
+ return 0;
+}
- if (hid_add_device(dj_hiddev)) {
- dev_err(&djrcv_hdev->dev, "%s: failed adding dj_device\n",
- __func__);
- goto hid_add_device_fail;
+static void logi_set_device_name(struct dj_device *dj_device,
+ struct dj_report *dj_report)
+{
+
+ struct dj_device_method *m;
+ struct dj_receiver_dev *djrcv_dev = dj_device->dj_receiver_dev;
+
+ strlcpy(dj_device->device_name, dj_report->report_params+3,
+ sizeof(dj_device->device_name));
+
+ hid_info(djrcv_dev->hdev, "Device %d: '%s'\n",
+ dj_device->device_index,
+ dj_device->device_name);
+
+ for (m = dj_device_method ; m->device_names ; m++) {
+ char **name;
+
+ for (name = m->device_names ; name ; name++) {
+ if (!strcmp(*name, dj_device->device_name)) {
+ dj_device->methods = m;
+ hid_info(djrcv_dev->hdev,
+ "Found methods for device '%s'\n",
+ dj_device->device_name);
+ return;
+ }
+ }
}
- return;
+}
+
+/*
+ * finalize the dj_device creation, because now we know the driver name
+ */
+static inline cvoid __dj_device_init_2(struct dj_receiver_dev *djrcv_dev,
+ struct dj_report *dj_report)
+{
+ int dev_index = dj_report->report_params[1]-0x40+1;
+ struct dj_device *dj_device = logi_djrcv2djdev(djrcv_dev, dev_index);
+ int retval;
-hid_add_device_fail:
- djrcv_dev->paired_dj_devices[dj_report->device_index] = NULL;
- kfree(dj_dev);
-dj_device_allocate_fail:
- hid_destroy_device(dj_hiddev);
+ if (!dj_device) {
+ dbg_hid("%s: cannot find %d device\n",
+ __func__, dev_index);
+ /* do rescan */
+ retval = logi_dj_recv_query_paired_devices(djrcv_dev);
+ if (!retval)
+ dev_err(&djrcv_dev->hdev->dev,
+ "%s:logi_dj_recv_query_paired_devices error:%d\n",
+ __func__, retval);
+
+ return;
+ }
+
+ if (dj_device->enabled) {
+ dbg_hid("%s: device %d already initializated\n",
+ __func__, dev_index);
+ return;
+ }
+
+ logi_set_device_name(dj_device, dj_report);
+
+ if (logi_dj_recv_add_djhid_device(dj_device) < 0) {
+ logi_dj_recv_destroy_djhid_device(djrcv_dev, dj_report);
+ dev_err(&djrcv_dev->hdev->dev,
+ "Cannot create the hid device\n");
+ return;
+ }
+ if (call_init_device(dj_device)) {
+ logi_dj_recv_destroy_djhid_device(djrcv_dev, dj_report);
+ dev_err(&djrcv_dev->hdev->dev,
+ "Cannot init the hid device\n");
+ return;
+ }
+
+ barrier();
+ dj_device->enabled = true;
+}
+
+/*
+ * Initialize the dj_device structure, but a) doesn't enable it, and b)
+ * doesn't crete the hid device, because we don't now the driver.
+ */
+static inline void __dj_device_init_1(struct dj_receiver_dev *djrcv_dev,
+ struct dj_report *dj_report)
+{
+ int dev_index;
+ struct dj_device *dj_device;
+ int ret;
+
+ dev_index = dj_report->device_index;
+ dj_device = logi_djrcv2djdev(djrcv_dev, dev_index);
+ if (dj_device) {
+ dev_warn(&djrcv_dev->hdev->dev,
+ "This device (%d) already exist\n", dev_index);
+ return;
+ }
+
+ ret = logi_dj_recv_create_dj_device(djrcv_dev, dj_report);
+ if (ret == -EINVAL) {
+ /* This report is not valid, ignore it */
+ return;
+ }
+ if (ret < 0) {
+ dev_err(&djrcv_dev->hdev->dev,
+ "Cannot allocate the hid device %d.%d\n",
+ dj_report->device_index, dj_report->report_type);
+ return;
+ }
+ /* we have to re-taken it, because before was not created */
+ dj_device = logi_djrcv2djdev(djrcv_dev, dev_index);
+ if (logi_send_name_request(dj_device) < 0) {
+ logi_dj_recv_destroy_djhid_device(djrcv_dev, dj_report);
+ dev_err(&djrcv_dev->hdev->dev,
+ "Cannot query the device name\n");
+ }
}
static void delayedwork_callback(struct work_struct *work)
@@ -314,6 +539,8 @@ static void delayedwork_callback(struct work_struct *work)
unsigned long flags;
int count;
int retval;
+ int dev_index;
+ struct dj_device *dj_device;
dbg_hid("%s\n", __func__);
@@ -339,9 +566,14 @@ static void delayedwork_callback(struct work_struct *work)
spin_unlock_irqrestore(&djrcv_dev->lock, flags);
switch (dj_report.report_type) {
+ case REPORT_GET_LONG_REGISTER_REQ:
+ __dj_device_init_2(djrcv_dev, &dj_report);
+ break;
+
case REPORT_TYPE_NOTIF_DEVICE_PAIRED:
- logi_dj_recv_add_djhid_device(djrcv_dev, &dj_report);
+ __dj_device_init_1(djrcv_dev, &dj_report);
break;
+
case REPORT_TYPE_NOTIF_DEVICE_UNPAIRED:
logi_dj_recv_destroy_djhid_device(djrcv_dev, &dj_report);
break;
@@ -353,20 +585,23 @@ static void delayedwork_callback(struct work_struct *work)
* to this dj_device never arrived to this driver. The reason is that
* hid-core discards all packets coming from a device while probe() is
* executing. */
- if (!djrcv_dev->paired_dj_devices[dj_report.device_index]) {
- /* ok, we don't know the device, just re-ask the
- * receiver for the list of connected devices. */
- retval = logi_dj_recv_query_paired_devices(djrcv_dev);
- if (!retval) {
- /* everything went fine, so just leave */
- break;
- }
- dev_err(&djrcv_dev->hdev->dev,
- "%s:logi_dj_recv_query_paired_devices "
- "error:%d\n", __func__, retval);
- }
- dbg_hid("%s: unexpected report type\n", __func__);
+ dev_index = dj_report.device_index;
+ dj_device = logi_djrcv2djdev(djrcv_dev, dev_index);
+ if (!dj_device) {
+ /* ok, we don't know the device, just re-ask the
+ * receiver for the list of connected devices. */
+ retval = logi_dj_recv_query_paired_devices(djrcv_dev);
+ if (!retval) {
+ /* everything went fine, so just leave */
+ break;
+ }
+ dev_err(&djrcv_dev->hdev->dev,
+ "%s:logi_dj_recv_query_paired_devices error:%d\n",
+ __func__, retval);
+ }
+ dbg_hid("%s: unexpected report type\n", __func__);
}
+
}
static void logi_dj_recv_queue_notification(struct dj_receiver_dev *djrcv_dev,
@@ -390,7 +625,7 @@ static void logi_dj_recv_forward_null_report(struct dj_receiver_dev *djrcv_dev,
u8 reportbuffer[MAX_REPORT_SIZE];
struct dj_device *djdev;
- djdev = djrcv_dev->paired_dj_devices[dj_report->device_index];
+ djdev = logi_djrcv2djdev(djrcv_dev, dj_report->device_index);
if (!djdev) {
dbg_hid("djrcv_dev->paired_dj_devices[dj_report->device_index]"
@@ -404,6 +639,10 @@ static void logi_dj_recv_forward_null_report(struct dj_receiver_dev *djrcv_dev,
return;
}
+ /* device not enabled */
+ if (!djdev->enabled)
+ return;
+
memset(reportbuffer, 0, sizeof(reportbuffer));
for (i = 0; i < NUMBER_OF_HID_REPORTS; i++) {
@@ -440,6 +679,10 @@ static void logi_dj_recv_forward_report(struct dj_receiver_dev *djrcv_dev,
return;
}
+ /* device not enabled */
+ if (!dj_device->enabled)
+ return;
+
if ((dj_report->report_type > ARRAY_SIZE(hid_reportid_size_map) - 1) ||
(hid_reportid_size_map[dj_report->report_type] == 0)) {
dbg_hid("invalid report type:%x\n", dj_report->report_type);
@@ -451,8 +694,8 @@ static void logi_dj_recv_forward_report(struct dj_receiver_dev *djrcv_dev,
hid_reportid_size_map[dj_report->report_type], 1)) {
dbg_hid("hid_input_report error\n");
}
-}
+}
static int logi_dj_recv_send_report(struct dj_receiver_dev *djrcv_dev,
struct dj_report *dj_report)
@@ -584,6 +827,10 @@ static int logi_dj_ll_parse(struct hid_device *hid)
char *rdesc;
int retval;
+ /* try the specific hook */
+ if (djdev->methods && djdev->methods->parse)
+ return djdev->methods->parse(djdev);
+
dbg_hid("%s\n", __func__);
djdev->hdev->version = 0x0111;
@@ -655,7 +902,6 @@ static struct hid_ll_driver logi_dj_ll_driver = {
.raw_request = logi_dj_ll_raw_request,
};
-
static int logi_dj_raw_event(struct hid_device *hdev,
struct hid_report *report, u8 *data,
int size)
@@ -692,25 +938,51 @@ static int logi_dj_raw_event(struct hid_device *hdev,
*/
spin_lock_irqsave(&djrcv_dev->lock, flags);
- if (dj_report->report_id == REPORT_ID_DJ_SHORT) {
- switch (dj_report->report_type) {
- case REPORT_TYPE_NOTIF_DEVICE_PAIRED:
- case REPORT_TYPE_NOTIF_DEVICE_UNPAIRED:
- logi_dj_recv_queue_notification(djrcv_dev, dj_report);
- break;
- case REPORT_TYPE_NOTIF_CONNECTION_STATUS:
- if (dj_report->report_params[CONNECTION_STATUS_PARAM_STATUS] ==
- STATUS_LINKLOSS) {
- logi_dj_recv_forward_null_report(djrcv_dev, dj_report);
- }
- break;
- default:
- logi_dj_recv_forward_report(djrcv_dev, dj_report);
+
+ if (dj_report->report_id == REPORT_ID_RECV_LONG &&
+ dj_report->device_index == REPORT_RECEIVER_INDEX &&
+ dj_report->report_type == REPORT_GET_LONG_REGISTER_REQ &&
+ dj_report->report_params[0] == REPORT_REG_PAIRING_INFORMATION &&
+ dj_report->report_params[1] >= REPORT_REG2_NOTIFY_DEVICE_NAME &&
+ dj_report->report_params[1] < (REPORT_REG2_NOTIFY_DEVICE_NAME+
+ DJ_DEVICE_INDEX_COUNT)) {
+
+ logi_dj_recv_queue_notification(djrcv_dev, dj_report);
+ report_processed = true;
+ } else if (dj_report->report_id == REPORT_ID_DJ_SHORT &&
+ (dj_report->report_type ==
+ REPORT_TYPE_NOTIF_DEVICE_PAIRED ||
+ dj_report->report_type ==
+ REPORT_TYPE_NOTIF_DEVICE_UNPAIRED)) {
+
+ logi_dj_recv_queue_notification(djrcv_dev, dj_report);
+ report_processed = true;
+ } else if (dj_report->report_id == REPORT_ID_DJ_SHORT &&
+ dj_report->report_type ==
+ REPORT_TYPE_NOTIF_CONNECTION_STATUS) {
+
+ if (dj_report->report_params[CONNECTION_STATUS_PARAM_STATUS] ==
+ STATUS_LINKLOSS) {
+
+ logi_dj_recv_forward_null_report(djrcv_dev, dj_report);
}
report_processed = true;
+ } else {
+ bool skip = false;
+ struct dj_device *djdev;
+
+ djdev = logi_djrcv2djdev(djrcv_dev, dj_report->device_index);
+ if (djdev && djdev->enabled) {
+ skip = call_parse_raw_event(djdev, dj_report);
+ report_processed = skip;
+ }
+ if (!skip && dj_report->report_id == REPORT_ID_DJ_SHORT) {
+ logi_dj_recv_forward_report(djrcv_dev, dj_report);
+ report_processed = true;
+ }
}
- spin_unlock_irqrestore(&djrcv_dev->lock, flags);
+ spin_unlock_irqrestore(&djrcv_dev->lock, flags);
return report_processed;
}
@@ -865,12 +1137,12 @@ static void logi_dj_remove(struct hid_device *hdev)
for (i = 0; i < (DJ_MAX_PAIRED_DEVICES + DJ_DEVICE_INDEX_MIN); i++) {
dj_dev = djrcv_dev->paired_dj_devices[i];
if (dj_dev != NULL) {
- hid_destroy_device(dj_dev->hdev);
+ if (dj_dev->hdev)
+ hid_destroy_device(dj_dev->hdev);
kfree(dj_dev);
djrcv_dev->paired_dj_devices[i] = NULL;
}
}
-
kfifo_free(&djrcv_dev->notif_fifo);
kfree(djrcv_dev);
hid_set_drvdata(hdev, NULL);
diff --git a/drivers/hid/hid-logitech-dj.h b/drivers/hid/hid-logitech-dj.h
index 4a40003..d81ed7d 100644
--- a/drivers/hid/hid-logitech-dj.h
+++ b/drivers/hid/hid-logitech-dj.h
@@ -29,12 +29,17 @@
#define DJ_MAX_NUMBER_NOTIFICATIONS 8
#define DJ_DEVICE_INDEX_MIN 1
#define DJ_DEVICE_INDEX_MAX 6
+#define DJ_DEVICE_INDEX_COUNT (DJ_DEVICE_INDEX_MAX - \
+ DJ_DEVICE_INDEX_MIN+1)
#define DJREPORT_SHORT_LENGTH 15
#define DJREPORT_LONG_LENGTH 32
+#define REPORT_ID_RECV_SHORT_LENGTH 7
#define REPORT_ID_DJ_SHORT 0x20
#define REPORT_ID_DJ_LONG 0x21
+#define REPORT_ID_RECV_SHORT 0x10
+#define REPORT_ID_RECV_LONG 0x11
#define REPORT_TYPE_RFREPORT_FIRST 0x01
#define REPORT_TYPE_RFREPORT_LAST 0x1F
@@ -60,6 +65,10 @@
/* Device Un-Paired Notification */
#define REPORT_TYPE_NOTIF_DEVICE_UNPAIRED 0x40
+/* Device "Unifying Device name" Notification */
+#define REPORT_GET_LONG_REGISTER_REQ 0x83
+#define REPORT_REG_PAIRING_INFORMATION 0xb5
+#define REPORT_REG2_NOTIFY_DEVICE_NAME 0x40
/* Connection Status Notification */
#define REPORT_TYPE_NOTIF_CONNECTION_STATUS 0x42
@@ -79,6 +88,9 @@
#define REPORT_TYPE_MEDIA_CENTER 0x08
#define REPORT_TYPE_LEDS 0x0E
+/* receiver device index */
+#define REPORT_RECEIVER_INDEX 0xff
+
/* RF Report types bitfield */
#define STD_KEYBOARD 0x00000002
#define STD_MOUSE 0x00000004
@@ -104,11 +116,40 @@ struct dj_receiver_dev {
bool querying_devices;
};
+struct dj_device;
+
+/**
+ * these methods are invoked instead the default ones
+ * @devices_name array of device name supported by this driver
+ * @init_device init method for the device. If the returned
+ * value is 0, the device was initiated correctly.
+ * Otherwise the device is not created.
+ * @parse_raw_event this method is called when a raw event arrived
+ * it permits to change the event.
+ * If the value returned is not zero the standard
+ * handler is called after.
+ * @parse this method allow to generate and pass a
+ * device descriptor to the higher level.
+ * @destroy called when the device has to be deleted
+ */
+struct dj_device_method {
+ char **device_names;
+ int (*init_device)(struct dj_device *);
+ int (*parse_raw_event)(struct dj_device *, struct dj_report *);
+ int (*parse)(struct dj_device *);
+ void (*destroy)(struct dj_device *);
+};
+
struct dj_device {
struct hid_device *hdev;
struct dj_receiver_dev *dj_receiver_dev;
u32 reports_supported;
u8 device_index;
+ int enabled;
+ char device_name[16];
+ u16 wpid;
+ struct dj_device_method *methods;
+ void *userdata;
};
/**
--
1.9.3
^ permalink raw reply related
* [PATCH 3/3] Add logitech m560 driver
From: Goffredo Baroncelli @ 2014-08-23 12:40 UTC (permalink / raw)
To: linux-input; +Cc: Goffredo Baroncelli
In-Reply-To: <1408797629-3474-1-git-send-email-kreijack@inwind.it>
Add logitech m560 support. In the init phase the driver
send a sequence which avoid some unnecessary key sending
from the mouse.
The mouse appears as a couple of mouse and keyboard. Some buttons
emit a key event instead of the "mouse button" event. However
some event (like the middle button release) aren't generated.
Fortunately, the device generates an additional event to
track it. The event type is 0x0a.
Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
---
drivers/hid/hid-logitech-dj.c | 92 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 92 insertions(+)
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index feddd3d..40c5ea1 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -222,7 +222,99 @@ static inline void call_destroy(struct dj_device *djdev)
djdev->methods->destroy(djdev);
}
+/*
+ * Send the sequence 10xx0a35 00af03
+ * to the mouse id xx. It disables the key sending by the central button.
+ */
+
+static int lg_m560_init_device(struct dj_device *dj_device)
+{
+ struct dj_report *dj_report;
+ int retval;
+ static u8 reset_data[] = {0x35, 0x00, 0xaf, 0x03};
+
+ dj_report = kzalloc(sizeof(struct dj_report), GFP_KERNEL);
+ if (!dj_report)
+ return -ENOMEM;
+
+ dj_device->userdata = dj_report;
+
+ dj_report->report_id = REPORT_ID_RECV_SHORT;
+ dj_report->device_index = dj_device->device_index;
+ dj_report->report_type = 0x0a;
+
+ memcpy(dj_report->report_params, reset_data, sizeof(reset_data));
+
+ retval = hid_hw_raw_request(dj_device->dj_receiver_dev->hdev,
+ dj_report->report_id,
+ (void *)dj_report, REPORT_ID_RECV_SHORT_LENGTH,
+ HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
+
+ memset(dj_report, 0, sizeof(struct dj_report));
+
+ return retval;
+}
+
+static int lg_m560_parse_raw_event(struct dj_device *dj_device,
+ struct dj_report *dj_report)
+{
+ u8 *m506_last_mouse_report = dj_device->userdata;
+
+ BUG_ON(!dj_device);
+
+ /* don't pass keys when the mouse is a m560 */
+ if (dj_report->report_type == REPORT_TYPE_KEYBOARD)
+ return true;
+
+ if (dj_report->report_id == 0x11 && dj_report->report_type == 0x0a) {
+ int btn;
+
+ /* check if the event is a button */
+ btn = dj_report->report_params[2];
+ if (btn != 0x00 && btn != 0xb0 && btn != 0xae && btn != 0xaf)
+ return true;
+
+ if (btn == 0xaf)
+ m506_last_mouse_report[1] |= 4;
+ if (btn == 0xae)
+ m506_last_mouse_report[2] |= 2;
+ if (btn == 0xb0)
+ m506_last_mouse_report[2] |= 4;
+ if (btn == 0x00) {
+ m506_last_mouse_report[1] &= ~4;
+ m506_last_mouse_report[2] &= ~(4|2);
+ }
+
+ if (hid_input_report(dj_device->hdev, HID_INPUT_REPORT,
+ m506_last_mouse_report, 8, 1)) {
+ dbg_hid("hid_input_report error\n");
+ }
+ return true;
+ }
+
+ /* copy the button status */
+ if (dj_report->report_type == REPORT_TYPE_MOUSE) {
+ /* copy only the first 3 bytes: type, btn0..7, btn8..16 */
+ memcpy(dj_device->userdata,
+ &dj_report->report_type, 3);
+ }
+
+ /* continue with the standard handler */
+ return false;
+}
+
+static void lg_m560_destroy(struct dj_device *dev)
+{
+ kfree(dev->userdata);
+}
+
static struct dj_device_method dj_device_method[] = {
+ { /* logitech M560 */
+ .device_names = (char *[]){ "M560", NULL },
+ .init_device = lg_m560_init_device,
+ .parse_raw_event = lg_m560_parse_raw_event,
+ .destroy = lg_m560_destroy
+ },
/* last element */
{ NULL, }
--
1.9.3
^ permalink raw reply related
* Urgent Assistance from Syria
From: Abdul Nasser Sokariah @ 2014-08-23 14:09 UTC (permalink / raw)
--
Good Day From Syria,
My name is Abdul Nasser Sokariah and I am writing you from Syria, I
choose to contact you directly as I need a reliable person to trust
who can help me make claims to my huge deposit with a vault company in
AFRICA, and based on my present situation in Syria, I need you
urgently to take possession of everything and further
modalities/directives will follow.
Contact me only on my private Email:nabdul247@gmail.com for
clarifications,I await your response.
Yours truly, Abdul Nasser Sokariah
Contact Email:nabdul247@gmail.com
^ permalink raw reply
* Re: [PATCH 1/2] SOUND: kill gameport bits
From: Andreas Mohr @ 2014-08-24 5:07 UTC (permalink / raw)
To: Takashi Iwai
Cc: Dmitry Torokhov, Andreas Mohr, linux-input, linux-kernel,
Vojtech Pavlik, Jiri Kosina
In-Reply-To: <s5h4mx6rshs.wl-tiwai@suse.de>
On Thu, Aug 21, 2014 at 01:29:03PM +0200, Takashi Iwai wrote:
> I did a quick hack and it seems working on my box.
> The patch is below.
Thanks!!
Further comments below.
I will be testing this ASAP.
> +static bool use_ktime = true;
> +module_param(use_ktime, bool, 0400);
Towards final commit, should probably add param docs on what may be switched here and why.
> +
> /*
> * gameport_mutex protects entire gameport subsystem and is taken
> * every time gameport port or driver registrered or unregistered.
> @@ -76,6 +80,36 @@ static unsigned int get_time_pit(void)
>
> static int gameport_measure_speed(struct gameport *gameport)
> {
> + unsigned int i, t, tx;
> + u64 t1, t2;
> + unsigned long flags;
> +
> + if (gameport_open(gameport, NULL, GAMEPORT_MODE_RAW))
> + return 0;
> +
> + tx = ~0;
> +
> + for (i = 0; i < 50; i++) {
> + local_irq_save(flags);
> + t1 = ktime_get_ns();
> + for (t = 0; t < 50; t++)
> + gameport_read(gameport);
> + t2 = ktime_get_ns();
> + local_irq_restore(flags);
> + udelay(i * 10);
> + if (t2 - t1 < tx)
> + tx = t2 - t1;
This impl is not doing the more complex t3, t2, t1 calculation
that the PIT impl is doing (likely for the uncommented purpose
of eliminating timer I/O delay from timing consideration).
Do/don't ktime/TSC impls better need such an I/O timing correction,
or are they so fast relative to gameport I/O delays
that it does not matter? (probably the case for TSC at least).
Oh, and any reason that such a speed calculation remains painfully duplicated
in both source files? That's possibly done for layering reasons,
but I'd have to analyze it further.
> +static inline u64 get_time(void)
> +{
> + if (use_ktime) {
> + return ktime_get_ns();
> + } else {
> + unsigned int x;
> + GET_TIME(x);
> + return x;
> + }
> +}
It might be useful to have a first commit to introduce these helpers,
and a second commit to then add ktime support (to keep review code size
down).
Thanks,
Andreas Mohr
--
GNU/Linux. It's not the software that's free, it's you.
^ permalink raw reply
* Re: [PATCH 1/1] HID: usbhid: add usb_clear_halt determination for next hid_start_in
From: vichy @ 2014-08-24 14:52 UTC (permalink / raw)
To: Alan Stern; +Cc: Jiri Kosina, linux-usb@vger.kernel.org, linux-input
In-Reply-To: <Pine.LNX.4.44L0.1408221358030.967-100000@iolanthe.rowland.org>
hi Alan:
>
> I originally tried using usb_reset_device, and it caused a deadlock:
>
> Unplug the HID device.
>
> I/O error occurs. hid_io_error schedules reset_work.
>
> The reset_work callback routine is hid_reset. It calls
> usb_reset_device.
>
> The reset fails because the device is gone. At the end of the
> reset, hid_post_reset is called.
>
> hid_post_reset returns 1 because hid_get_class_descriptor
> fails.
>
> Because the post_reset routine failed, usb_reset_device calls
> usb_unbind_and_rebind_marked_interfaces.
"usb_unbind_and_rebind_marked_interfaces" is called if config
parameter is not null,
it seems no matter post_reset routine fail or not.
>
> That routine indirectly calls usbhid_disconnect.
Why that routine, usb_reset_device, will call usbhid_disconnect?
No matter port reset success or fail, usbhid_disconnect should not be
called except that did happen disconnection.
>
> usbhid_disconnect calls usbhid_close by way of
> hid_destroy_device.
below is the content of hid_destroy and hid_remove_device, but I
cannot find where usbhid_close be called by way of hid_destroy_device.
static void hid_remove_device(struct hid_device *hdev)
{
if (hdev->status & HID_STAT_ADDED) {
device_del(&hdev->dev);
hid_debug_unregister(hdev);
hdev->status &= ~HID_STAT_ADDED;
}
kfree(hdev->dev_rdesc);
hdev->dev_rdesc = NULL;
hdev->dev_rsize = 0;
}
void hid_destroy_device(struct hid_device *hdev)
{
hid_remove_device(hdev);
put_device(&hdev->dev);
}
>
> usbhid_close calls hid_cancel_delayed_stuff.
>
> hid_cancel_delayed_stuff calls cancel_work_sync for reset_work.
>
> So the reset_work routine tries to cancel itself synchronously while it
> is running. usb_queue_reset_device was carefully written to avoid such
> deadlocks.
>
thanks for your help,
^ permalink raw reply
* Re: [PATCH 1/1] HID: usbhid: add usb_clear_halt determination for next hid_start_in
From: Alan Stern @ 2014-08-24 15:34 UTC (permalink / raw)
To: vichy; +Cc: Jiri Kosina, linux-usb@vger.kernel.org, linux-input
In-Reply-To: <CAOVJa8FgtSDM-F=Nu1kOU_Ax_6a0_U3jKU6_PKPDoVD9zW+JWg@mail.gmail.com>
On Sun, 24 Aug 2014, vichy wrote:
> hi Alan:
> >
> > I originally tried using usb_reset_device, and it caused a deadlock:
> >
> > Unplug the HID device.
> >
> > I/O error occurs. hid_io_error schedules reset_work.
> >
> > The reset_work callback routine is hid_reset. It calls
> > usb_reset_device.
> >
> > The reset fails because the device is gone. At the end of the
> > reset, hid_post_reset is called.
> >
> > hid_post_reset returns 1 because hid_get_class_descriptor
> > fails.
> >
> > Because the post_reset routine failed, usb_reset_device calls
> > usb_unbind_and_rebind_marked_interfaces.
>
> "usb_unbind_and_rebind_marked_interfaces" is called if config
> parameter is not null,
> it seems no matter post_reset routine fail or not.
Yes, that's right. I should have said: "Because the post_reset routine
failed, usb_unbind_and_rebind_marked_interfaces indirectly calls
usbhid_disconnect.
> > That routine indirectly calls usbhid_disconnect.
>
> Why that routine, usb_reset_device, will call usbhid_disconnect?
> No matter port reset success or fail, usbhid_disconnect should not be
> called except that did happen disconnection.
usbhid_disconnect gets called when the driver is unbound, regardless of
whether the device was unplugged. The kernel unbinds a driver when a
device is reset but the driver isn't able to handle the reset.
> > usbhid_disconnect calls usbhid_close by way of
> > hid_destroy_device.
>
> below is the content of hid_destroy and hid_remove_device, but I
> cannot find where usbhid_close be called by way of hid_destroy_device.
> static void hid_remove_device(struct hid_device *hdev)
> {
> if (hdev->status & HID_STAT_ADDED) {
> device_del(&hdev->dev);
> hid_debug_unregister(hdev);
> hdev->status &= ~HID_STAT_ADDED;
> }
> kfree(hdev->dev_rdesc);
> hdev->dev_rdesc = NULL;
> hdev->dev_rsize = 0;
> }
>
> void hid_destroy_device(struct hid_device *hdev)
> {
> hid_remove_device(hdev);
> put_device(&hdev->dev);
> }
I don't remember the entire call chain. It was pretty long.
hid_destroy_device calls hid_remove_device, which calls device_del,
which calls lots of other things. If you really want to see all the
details, put a dump_stack() call in usbhid_close and examine what it
prints in the kernel log when you unplug an HID device.
Alan Stern
^ permalink raw reply
* Re: [PATCH v4 5/14] input: cyapa: add read firmware image and raw data interfaces in debugfs system
From: Dmitry Torokhov @ 2014-08-22 23:51 UTC (permalink / raw)
To: Dudley Du
Cc: Rafael J. Wysocki, Benson Leung, Patrik Fimml, linux-input,
linux-kernel, Dudley Du
In-Reply-To: <53c772c1.e683460a.3dd2.4a4c@mx.google.com>
Hi Dudley,
On Thu, Jul 17, 2014 at 02:52:36PM +0800, Dudley Du wrote:
> Add read_fw and raw_data debugfs interfaces for easier issues location
> and collection when report by user.
> TEST=test on Chromebooks.
>
> Signed-off-by: Dudley Du <dudl@cypress.com>
> ---
> drivers/input/mouse/cyapa.c | 219 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 219 insertions(+)
>
> diff --git a/drivers/input/mouse/cyapa.c b/drivers/input/mouse/cyapa.c
> index 53c9d59..ae24b02 100644
> --- a/drivers/input/mouse/cyapa.c
> +++ b/drivers/input/mouse/cyapa.c
> @@ -39,6 +39,8 @@
>
> const char unique_str[] = "CYTRA";
>
> +/* Global root node of the cyapa debugfs directory. */
> +static struct dentry *cyapa_debugfs_root;
>
>
> ssize_t cyapa_i2c_reg_read_block(struct cyapa *cyapa, u8 reg, size_t len,
> @@ -461,6 +463,205 @@ done:
> }
>
> /*
> + **************************************************************
> + * debugfs interface
> + **************************************************************
> +*/
> +static int cyapa_debugfs_open(struct inode *inode, struct file *file)
> +{
> + struct cyapa *cyapa = inode->i_private;
> + int ret;
> +
> + if (!cyapa)
> + return -ENODEV;
> +
> + ret = mutex_lock_interruptible(&cyapa->debugfs_mutex);
> + if (ret)
> + return ret;
> +
> + if (!kobject_get(&cyapa->client->dev.kobj)) {
Why not get_device() here and elsewhere?
> + ret = -ENODEV;
> + goto out;
> + }
> +
> + file->private_data = cyapa;
> +
> + if (cyapa->fw_image) {
> + ret = 0;
> + goto out;
> + }
> +
> + mutex_lock(&cyapa->state_sync_lock);
> + /*
> + * If firmware hasn't been read yet, read it all in one pass.
> + * Subsequent opens will reuse the data in this same buffer.
> + */
> + if (cyapa->ops->read_fw)
> + ret = cyapa->ops->read_fw(cyapa);
> + else
> + ret = -EPERM;
> + mutex_unlock(&cyapa->state_sync_lock);
> +
> + /* Redetect trackpad device states. */
> + cyapa_detect_async(cyapa, 0);
> +
> +out:
> + mutex_unlock(&cyapa->debugfs_mutex);
> + return ret;
> +}
> +
> +static int cyapa_debugfs_release(struct inode *inode, struct file *file)
> +{
> + struct cyapa *cyapa = file->private_data;
> + int ret;
> +
> + if (!cyapa)
> + return 0;
> +
> + ret = mutex_lock_interruptible(&cyapa->debugfs_mutex);
> + if (ret)
> + return ret;
> + file->private_data = NULL;
> + kobject_put(&cyapa->client->dev.kobj);
> + mutex_unlock(&cyapa->debugfs_mutex);
> +
> + return 0;
> +}
> +
> +/* Return some bytes from the buffered firmware image, starting from *ppos */
> +static ssize_t cyapa_debugfs_read_fw(struct file *file, char __user *buffer,
> + size_t count, loff_t *ppos)
> +{
> + struct cyapa *cyapa = file->private_data;
> +
> + if (!cyapa->fw_image)
> + return -EINVAL;
> +
> + if (*ppos >= cyapa->fw_image_size)
> + return 0;
> +
> + if (count + *ppos > cyapa->fw_image_size)
> + count = cyapa->fw_image_size - *ppos;
> +
> + if (copy_to_user(buffer, &cyapa->fw_image[*ppos], count))
> + return -EFAULT;
> +
> + *ppos += count;
> + return count;
> +}
> +
> +static const struct file_operations cyapa_read_fw_fops = {
> + .open = cyapa_debugfs_open,
> + .release = cyapa_debugfs_release,
> + .read = cyapa_debugfs_read_fw
> +};
> +
> +static int cyapa_debugfs_raw_data_open(struct inode *inode, struct file *file)
> +{
> + struct cyapa *cyapa = inode->i_private;
> + int ret;
> +
> + if (!cyapa)
> + return -ENODEV;
> +
> + /* Start to be supported after Gen5 trackpad devices. */
> + if (cyapa->gen < CYAPA_GEN5)
> + return -ENOTSUPP;
> +
> + ret = mutex_lock_interruptible(&cyapa->debugfs_mutex);
> + if (ret)
> + return ret;
> +
> + if (!kobject_get(&cyapa->client->dev.kobj)) {
> + ret = -ENODEV;
> + goto out;
> + }
> +
> + file->private_data = cyapa;
> +
> + mutex_lock(&cyapa->state_sync_lock);
> + if (cyapa->ops->read_raw_data)
> + ret = cyapa->ops->read_raw_data(cyapa);
> + else
> + ret = -EPERM;
> + mutex_unlock(&cyapa->state_sync_lock);
> +out:
> + mutex_unlock(&cyapa->debugfs_mutex);
> + return ret;
> +}
> +
> +static int cyapa_debugfs_raw_data_release(struct inode *inode,
> + struct file *file)
> +{
> + struct cyapa *cyapa = file->private_data;
> + int ret;
> +
> + if (!cyapa)
> + return 0;
> +
> + ret = mutex_lock_interruptible(&cyapa->debugfs_mutex);
> + if (ret)
> + return ret;
> + file->private_data = NULL;
> + kobject_put(&cyapa->client->dev.kobj);
> + mutex_unlock(&cyapa->debugfs_mutex);
> +
> + return 0;
> +}
> +
> +/* Always return the sensors' latest raw data from trackpad device. */
> +static ssize_t cyapa_debugfs_read_raw_data(struct file *file,
> + char __user *buffer,
> + size_t count, loff_t *ppos)
> +{
> + struct cyapa *cyapa = file->private_data;
> +
> + if (!cyapa->tp_raw_data)
> + return -EINVAL;
> +
> + if (*ppos >= cyapa->tp_raw_data_size)
> + return 0;
> +
> + if (count + *ppos > cyapa->tp_raw_data_size)
> + count = cyapa->tp_raw_data_size - *ppos;
> +
> + if (copy_to_user(buffer, &cyapa->tp_raw_data[*ppos], count))
> + return -EFAULT;
> +
> + *ppos += count;
> + return count;
> +}
> +
> +static const struct file_operations cyapa_read_raw_data_fops = {
> + .open = cyapa_debugfs_raw_data_open,
> + .release = cyapa_debugfs_raw_data_release,
> + .read = cyapa_debugfs_read_raw_data
> +};
> +
> +static int cyapa_debugfs_init(struct cyapa *cyapa)
> +{
> + struct device *dev = &cyapa->client->dev;
> +
> + if (!cyapa_debugfs_root)
> + return -ENODEV;
> +
> + cyapa->dentry_dev = debugfs_create_dir(kobject_name(&dev->kobj),
> + cyapa_debugfs_root);
> +
> + if (!cyapa->dentry_dev)
> + return -ENODEV;
> +
> + mutex_init(&cyapa->debugfs_mutex);
> +
> + debugfs_create_file(CYAPA_DEBUGFS_READ_FW, S_IRUSR, cyapa->dentry_dev,
> + cyapa, &cyapa_read_fw_fops);
> +
> + debugfs_create_file(CYAPA_DEBUGFS_RAW_DATA, S_IRUSR, cyapa->dentry_dev,
> + cyapa, &cyapa_read_raw_data_fops);
> + return 0;
> +}
> +
> +/*
> * Sysfs Interface.
> */
>
> @@ -869,6 +1070,13 @@ static int cyapa_probe(struct i2c_client *client,
> if (sysfs_create_group(&client->dev.kobj, &cyapa_sysfs_group))
> dev_warn(dev, "error creating sysfs entries.\n");
>
> + /* Create a global debugfs root for all cyapa devices */
> + cyapa_debugfs_root = debugfs_create_dir("cyapa", NULL);
> + if (cyapa_debugfs_root == ERR_PTR(-ENODEV))
> + cyapa_debugfs_root = NULL;
> + if (cyapa_debugfs_init(cyapa))
> + dev_warn(dev, "error creating debugfs entries.\n");
> +
> #ifdef CONFIG_PM_SLEEP
> if (device_can_wakeup(dev) &&
> sysfs_merge_group(&client->dev.kobj, &cyapa_power_wakeup_group))
> @@ -902,8 +1110,19 @@ static int cyapa_remove(struct i2c_client *client)
> #ifdef CONFIG_PM_RUNTIME
> sysfs_unmerge_group(&client->dev.kobj, &cyapa_power_runtime_group);
> #endif
> +
> + kfree(cyapa->fw_image);
> + cyapa->fw_image = NULL;
> + cyapa->fw_image_size = 0;
> + kfree(cyapa->tp_raw_data);
> + cyapa->tp_raw_data = NULL;
> + cyapa->tp_raw_data_size = 0;
This should be happening only after you remove debugfs entries.
> free_irq(cyapa->irq, cyapa);
>
> + debugfs_remove_recursive(cyapa->dentry_dev);
> + debugfs_remove_recursive(cyapa_debugfs_root);
Hm, it seems that your debugfs entries are created per-devce, but you remove
all of them at one when very first device us unbound. You probably want to
create and destroy root of your debugfs hierarchy on module level.
> + mutex_destroy(&cyapa->debugfs_mutex);
> +
> input_unregister_device(cyapa->input);
> if (cyapa->ops->set_power_mode)
> cyapa->ops->set_power_mode(cyapa, PWR_MODE_OFF, 0);
> --
> 1.7.9.5
>
>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v4 6/14] input: cyapa: add gen3 trackpad device basic functions support
From: Dmitry Torokhov @ 2014-08-22 23:55 UTC (permalink / raw)
To: Dudley Du
Cc: Rafael J. Wysocki, Benson Leung, Patrik Fimml, linux-input,
linux-kernel, Dudley Du
In-Reply-To: <53c7730a.a10d450a.407e.4c90@mx.google.com>
On Thu, Jul 17, 2014 at 02:53:48PM +0800, Dudley Du wrote:
> Based on the cyapa core, add the gen3 trackpad device's basic functions
> supported, so gen3 trackpad device can work with kernel input system.
> The basic function is absolutely same as previous cyapa driver only
> support gen3 trackpad device.
> TEST=test on Chromebooks.
>
> Signed-off-by: Dudley Du <dudl@cypress.com>
> ---
> drivers/input/mouse/Makefile | 2 +-
> drivers/input/mouse/cyapa.c | 96 ++++-
> drivers/input/mouse/cyapa.h | 1 +
> drivers/input/mouse/cyapa_gen3.c | 784 ++++++++++++++++++++++++++++++++++++++
> 4 files changed, 881 insertions(+), 2 deletions(-)
> create mode 100644 drivers/input/mouse/cyapa_gen3.c
>
> diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile
> index 8608eb7..63b42e0 100644
> --- a/drivers/input/mouse/Makefile
> +++ b/drivers/input/mouse/Makefile
> @@ -35,4 +35,4 @@ psmouse-$(CONFIG_MOUSE_PS2_TRACKPOINT) += trackpoint.o
> psmouse-$(CONFIG_MOUSE_PS2_TOUCHKIT) += touchkit_ps2.o
> psmouse-$(CONFIG_MOUSE_PS2_CYPRESS) += cypress_ps2.o
>
> -cyapatp-y := cyapa.o
> +cyapatp-y := cyapa.o cyapa_gen3.o
> diff --git a/drivers/input/mouse/cyapa.c b/drivers/input/mouse/cyapa.c
> index ae24b02..5c62503 100644
> --- a/drivers/input/mouse/cyapa.c
> +++ b/drivers/input/mouse/cyapa.c
> @@ -113,6 +113,15 @@ ssize_t cyapa_i2c_write(struct cyapa *cyapa, u8 reg,
>
> void cyapa_default_irq_handler(struct cyapa *cyapa)
> {
> + bool cont;
> +
> + /* Interrupt triggerred by command response in detecting. */
> + cont = true;
> + if (cyapa_gen3_ops.irq_cmd_handler)
> + cont = cyapa_gen3_ops.irq_cmd_handler(cyapa);
Why not simply
cont = cyapa->ops->irq_cmd_handler(cyapa)?
> + if (!cont)
> + return;
> +
> /*
> * Do redetecting when device states is still unknown and
> * interrupt envent is received from device.
> @@ -252,6 +261,9 @@ static int cyapa_check_is_operational(struct cyapa *cyapa)
> return ret;
>
> switch (cyapa->gen) {
> + case CYAPA_GEN3:
> + cyapa->ops = &cyapa_gen3_ops;
> + break;
> default:
> cyapa->ops = &cyapa_default_ops;
> cyapa->gen = CYAPA_GEN_UNKNOWN;
> @@ -314,9 +326,85 @@ out:
> */
> static int cyapa_get_state(struct cyapa *cyapa)
> {
> + int ret;
> + u8 status[BL_STATUS_SIZE];
> + u8 cmd[32];
> + /* The i2c address of gen4 and gen5 trackpad device must be even. */
> + bool even_addr = ((cyapa->client->addr & 0x0001) == 0);
> + bool smbus = false;
> + int retries = 2;
> +
> cyapa->state = CYAPA_STATE_NO_DEVICE;
>
> - return -ENODEV;
> + /*
> + * Get trackpad status by reading 3 registers starting from 0.
> + * If the device is in the bootloader, this will be BL_HEAD.
> + * If the device is in operation mode, this will be the DATA regs.
> + *
> + */
> + ret = cyapa_i2c_reg_read_block(cyapa, BL_HEAD_OFFSET, BL_STATUS_SIZE,
> + status);
> +
> + /*
> + * On smbus systems in OP mode, the i2c_reg_read will fail with
> + * -ETIMEDOUT. In this case, try again using the smbus equivalent
> + * command. This should return a BL_HEAD indicating CYAPA_STATE_OP.
> + */
> + if (cyapa->smbus && (ret == -ETIMEDOUT || ret == -ENXIO)) {
> + if (!even_addr)
> + ret = cyapa_read_block(cyapa,
> + CYAPA_CMD_BL_STATUS, status);
> + smbus = true;
> + }
> + if (ret != BL_STATUS_SIZE)
> + goto error;
> +
> + /*
> + * Detect trackpad protocol based on characristic registers and bits.
> + */
> + do {
> + cyapa->status[REG_OP_STATUS] = status[REG_OP_STATUS];
> + cyapa->status[REG_BL_STATUS] = status[REG_BL_STATUS];
> + cyapa->status[REG_BL_ERROR] = status[REG_BL_ERROR];
> +
> + if (cyapa->gen == CYAPA_GEN_UNKNOWN ||
> + cyapa->gen == CYAPA_GEN3) {
> + ret = cyapa_gen3_ops.state_parse(cyapa,
> + status, BL_STATUS_SIZE);
> + if (ret == 0)
> + goto out_detected;
> + }
> +
> + /*
> + * Cannot detect communication protocol based on current
> + * charateristic registers and bits.
> + * So write error command to do further detection.
> + * this method only valid on I2C bus.
> + * for smbus interface, it won't have overwrite issue.
> + */
> + if (!smbus) {
> + cmd[0] = 0x00;
> + cmd[1] = 0x00;
> + ret = cyapa_i2c_write(cyapa, 0, 2, cmd);
> + if (ret)
> + goto error;
> +
> + msleep(50);
> +
> + ret = cyapa_i2c_read(cyapa, BL_HEAD_OFFSET,
> + BL_STATUS_SIZE, status);
> + if (ret < 0)
> + goto error;
> + }
> + } while (--retries > 0 && !smbus);
> +
> + goto error;
> +
> +out_detected:
> + return 0;
> +
> +error:
> + return (ret < 0) ? ret : -EAGAIN;
> }
>
> /*
> @@ -994,11 +1082,17 @@ static void cyapa_detect_and_start(void *data, async_cookie_t cookie)
>
> static int cyapa_tp_modules_init(struct cyapa *cyapa)
> {
> + if (cyapa_gen3_ops.initialize)
> + return cyapa_gen3_ops.initialize(cyapa);
Same here.
> +
> return 0;
> }
>
> static int cyapa_tp_modules_uninit(struct cyapa *cyapa)
> {
> + if (cyapa_gen3_ops.uninitialize)
> + return cyapa_gen3_ops.uninitialize(cyapa);
And here.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH RESEND v4 2/2] elantech: Call psmouse_reset when elantech probe fails
From: Dmitry Torokhov @ 2014-08-23 0:11 UTC (permalink / raw)
To: Ulrik De Bie; +Cc: linux-input, Hans de Goede, David Herrmann
In-Reply-To: <1408558692-11736-3-git-send-email-ulrik.debie-os@e2big.org>
On Wed, Aug 20, 2014 at 08:18:12PM +0200, Ulrik De Bie wrote:
> elantech_init() calls elantech_set_absolute_mode which sets the driver in
> an absolute mode. When after this the elantech_init fails, it is best
> to turn the ps/2 mouse emulation mode back on by calling psmouse_reset()
> so that it can work as a regular mouse.
>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Ulrik De Bie <ulrik.debie-os@e2big.org>
Applied, thank you.
> ---
> drivers/input/mouse/elantech.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index 5dd620a..771291c 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -1620,6 +1620,7 @@ int elantech_init(struct psmouse *psmouse)
> sysfs_remove_group(&psmouse->ps2dev.serio->dev.kobj,
> &elantech_attr_group);
> init_fail:
> + psmouse_reset(psmouse);
> kfree(etd);
> return error;
> }
> --
> 2.1.0.rc1
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH RESEND v4 1/2] elantech: Add support for trackpoint found on some v3 models
From: Dmitry Torokhov @ 2014-08-24 22:14 UTC (permalink / raw)
To: Ulrik De Bie; +Cc: linux-input, Hans de Goede, David Herrmann
In-Reply-To: <1408558692-11736-2-git-send-email-ulrik.debie-os@e2big.org>
Hi Ulrik,
On Wed, Aug 20, 2014 at 08:18:11PM +0200, Ulrik De Bie wrote:
> +
> + t = (((u32)packet[0] & 0xF8) << 24) | ((u32)packet[1] << 16)
> + | (u32)packet[2] << 8 | (u32)packet[3];
Majority of devices with Elantech will be little-endian devices, so if we use
get_unaligned_le32() we'll save a few ops. Does the version of patch below
still work for you?
Thanks!
--
Dmitry
Input: elantech - add support for trackpoint found on some v3 models
From: Ulrik De Bie <ulrik.debie-os@e2big.org>
Some elantech v3 touchpad equipped laptops also have a trackpoint, before
this commit, these give sync errors. With this patch, the trackpoint is
provided as another input device: 'Elantech PS/2 TrackPoint'
The patch will also output messages that do not follow the expected pattern.
In the mean time I've seen 2 unknown packets occasionally:
0x04 , 0x00 , 0x00 , 0x00 , 0x00 , 0x00
0x00 , 0x00 , 0x00 , 0x02 , 0x00 , 0x00
I don't know what those are for, but they can be safely ignored.
Currently all packets that are not known to v3 touchpad and where
packet[3] (the fourth byte) lowest nibble is 6 are now recognized as
PACKET_TRACKPOINT and processed by the new elantech_report_trackpoint.
This has been verified to work on a laptop Lenovo L530 where the
touchpad/trackpoint combined identify themselves as:
psmouse serio1: elantech: assuming hardware version 3 (with firmware version 0x350f02)
psmouse serio1: elantech: Synaptics capabilities query result 0xb9, 0x15, 0x0c.
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Ulrik De Bie <ulrik.debie-os@e2big.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/mouse/elantech.c | 136 +++++++++++++++++++++++++++++++++++++---
drivers/input/mouse/elantech.h | 3 +
2 files changed, 130 insertions(+), 9 deletions(-)
diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index ee2a04d..b4d645a 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -18,6 +18,7 @@
#include <linux/input/mt.h>
#include <linux/serio.h>
#include <linux/libps2.h>
+#include <asm/unaligned.h>
#include "psmouse.h"
#include "elantech.h"
@@ -403,6 +404,68 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
input_sync(dev);
}
+static void elantech_report_trackpoint(struct psmouse *psmouse,
+ int packet_type)
+{
+ /*
+ * byte 0: 0 0 ~sx ~sy 0 M R L
+ * byte 1: sx 0 0 0 0 0 0 0
+ * byte 2: sy 0 0 0 0 0 0 0
+ * byte 3: 0 0 sy sx 0 1 1 0
+ * byte 4: x7 x6 x5 x4 x3 x2 x1 x0
+ * byte 5: y7 y6 y5 y4 y3 y2 y1 y0
+ *
+ * x and y are written in two's complement spread
+ * over 9 bits with sx/sy the relative top bit and
+ * x7..x0 and y7..y0 the lower bits.
+ * The sign of y is opposite to what the input driver
+ * expects for a relative movement
+ */
+
+ struct elantech_data *etd = psmouse->private;
+ struct input_dev *tp_dev = etd->tp_dev;
+ unsigned char *packet = psmouse->packet;
+ int x, y;
+ u32 t;
+
+ if (dev_WARN_ONCE(&psmouse->ps2dev.serio->dev,
+ !tp_dev,
+ psmouse_fmt("Unexpected trackpoint message\n"))) {
+ if (etd->debug == 1)
+ elantech_packet_dump(psmouse);
+ return;
+ }
+
+ t = get_unaligned_le32(&packet[0]);
+
+ switch (t & ~7U) {
+ case 0x06000030U:
+ case 0x16008020U:
+ case 0x26800010U:
+ case 0x36808000U:
+ x = packet[4] - (int)(packet[1] << 1);
+ y = (int)(packet[2] << 1) - packet[5];
+
+ input_report_key(tp_dev, BTN_LEFT, packet[0] & 0x01);
+ input_report_key(tp_dev, BTN_RIGHT, packet[0] & 0x02);
+ input_report_key(tp_dev, BTN_MIDDLE, packet[0] & 0x04);
+
+ input_report_rel(tp_dev, REL_X, x);
+ input_report_rel(tp_dev, REL_Y, y);
+
+ input_sync(tp_dev);
+
+ break;
+
+ default:
+ /* Dump unexpected packet sequences if debug=1 (default) */
+ if (etd->debug == 1)
+ elantech_packet_dump(psmouse);
+
+ break;
+ }
+}
+
/*
* Interpret complete data packets and report absolute mode input events for
* hardware version 3. (12 byte packets for two fingers)
@@ -715,6 +778,8 @@ static int elantech_packet_check_v3(struct psmouse *psmouse)
if ((packet[0] & 0x0c) == 0x0c && (packet[3] & 0xce) == 0x0c)
return PACKET_V3_TAIL;
+ if ((packet[3] & 0x0f) == 0x06)
+ return PACKET_TRACKPOINT;
}
return PACKET_UNKNOWN;
@@ -791,14 +856,23 @@ static psmouse_ret_t elantech_process_byte(struct psmouse *psmouse)
case 3:
packet_type = elantech_packet_check_v3(psmouse);
- /* ignore debounce */
- if (packet_type == PACKET_DEBOUNCE)
- return PSMOUSE_FULL_PACKET;
-
- if (packet_type == PACKET_UNKNOWN)
+ switch (packet_type) {
+ case PACKET_UNKNOWN:
return PSMOUSE_BAD_DATA;
- elantech_report_absolute_v3(psmouse, packet_type);
+ case PACKET_DEBOUNCE:
+ /* ignore debounce */
+ break;
+
+ case PACKET_TRACKPOINT:
+ elantech_report_trackpoint(psmouse, packet_type);
+ break;
+
+ default:
+ elantech_report_absolute_v3(psmouse, packet_type);
+ break;
+ }
+
break;
case 4:
@@ -1018,8 +1092,10 @@ static int elantech_get_resolution_v4(struct psmouse *psmouse,
* Asus UX31 0x361f00 20, 15, 0e clickpad
* Asus UX32VD 0x361f02 00, 15, 0e clickpad
* Avatar AVIU-145A2 0x361f00 ? clickpad
+ * Fujitsu H730 0x570f00 c0, 14, 0c 3 hw buttons (**)
* Gigabyte U2442 0x450f01 58, 17, 0c 2 hw buttons
* Lenovo L430 0x350f02 b9, 15, 0c 2 hw buttons (*)
+ * Lenovo L530 0x350f02 b9, 15, 0c 2 hw buttons (*)
* Samsung NF210 0x150b00 78, 14, 0a 2 hw buttons
* Samsung NP770Z5E 0x575f01 10, 15, 0f clickpad
* Samsung NP700Z5B 0x361f06 21, 15, 0f clickpad
@@ -1029,6 +1105,8 @@ static int elantech_get_resolution_v4(struct psmouse *psmouse,
* Samsung RF710 0x450f00 ? 2 hw buttons
* System76 Pangolin 0x250f01 ? 2 hw buttons
* (*) + 3 trackpoint buttons
+ * (**) + 0 trackpoint buttons
+ * Note: Lenovo L430 and Lenovo L430 have the same fw_version/caps
*/
static void elantech_set_buttonpad_prop(struct psmouse *psmouse)
{
@@ -1324,6 +1402,10 @@ int elantech_detect(struct psmouse *psmouse, bool set_properties)
*/
static void elantech_disconnect(struct psmouse *psmouse)
{
+ struct elantech_data *etd = psmouse->private;
+
+ if (etd->tp_dev)
+ input_unregister_device(etd->tp_dev);
sysfs_remove_group(&psmouse->ps2dev.serio->dev.kobj,
&elantech_attr_group);
kfree(psmouse->private);
@@ -1438,8 +1520,10 @@ static int elantech_set_properties(struct elantech_data *etd)
int elantech_init(struct psmouse *psmouse)
{
struct elantech_data *etd;
- int i, error;
+ int i;
+ int error = -EINVAL;
unsigned char param[3];
+ struct input_dev *tp_dev;
psmouse->private = etd = kzalloc(sizeof(struct elantech_data), GFP_KERNEL);
if (!etd)
@@ -1498,14 +1582,48 @@ int elantech_init(struct psmouse *psmouse)
goto init_fail;
}
+ /* The MSB indicates the presence of the trackpoint */
+ if ((etd->capabilities[0] & 0x80) == 0x80) {
+ tp_dev = input_allocate_device();
+
+ if (!tp_dev) {
+ error = -ENOMEM;
+ goto init_fail_tp_alloc;
+ }
+
+ etd->tp_dev = tp_dev;
+ snprintf(etd->tp_phys, sizeof(etd->tp_phys), "%s/input1",
+ psmouse->ps2dev.serio->phys);
+ tp_dev->phys = etd->tp_phys;
+ tp_dev->name = "Elantech PS/2 TrackPoint";
+ tp_dev->id.bustype = BUS_I8042;
+ tp_dev->id.vendor = 0x0002;
+ tp_dev->id.product = PSMOUSE_ELANTECH;
+ tp_dev->id.version = 0x0000;
+ tp_dev->dev.parent = &psmouse->ps2dev.serio->dev;
+ tp_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REL);
+ tp_dev->relbit[BIT_WORD(REL_X)] =
+ BIT_MASK(REL_X) | BIT_MASK(REL_Y);
+ tp_dev->keybit[BIT_WORD(BTN_LEFT)] =
+ BIT_MASK(BTN_LEFT) | BIT_MASK(BTN_MIDDLE) |
+ BIT_MASK(BTN_RIGHT);
+ error = input_register_device(etd->tp_dev);
+ if (error < 0)
+ goto init_fail_tp_reg;
+ }
+
psmouse->protocol_handler = elantech_process_byte;
psmouse->disconnect = elantech_disconnect;
psmouse->reconnect = elantech_reconnect;
psmouse->pktsize = etd->hw_version > 1 ? 6 : 4;
return 0;
-
+ init_fail_tp_reg:
+ input_free_device(tp_dev);
+ init_fail_tp_alloc:
+ sysfs_remove_group(&psmouse->ps2dev.serio->dev.kobj,
+ &elantech_attr_group);
init_fail:
kfree(etd);
- return -1;
+ return error;
}
diff --git a/drivers/input/mouse/elantech.h b/drivers/input/mouse/elantech.h
index 9e0e2a1..6f3afec 100644
--- a/drivers/input/mouse/elantech.h
+++ b/drivers/input/mouse/elantech.h
@@ -94,6 +94,7 @@
#define PACKET_V4_HEAD 0x05
#define PACKET_V4_MOTION 0x06
#define PACKET_V4_STATUS 0x07
+#define PACKET_TRACKPOINT 0x08
/*
* track up to 5 fingers for v4 hardware
@@ -114,6 +115,8 @@ struct finger_pos {
};
struct elantech_data {
+ struct input_dev *tp_dev; /* Relative device for trackpoint */
+ char tp_phys[32];
unsigned char reg_07;
unsigned char reg_10;
unsigned char reg_11;
^ permalink raw reply related
* RE: [PATCH v4 5/14] input: cyapa: add read firmware image and raw data interfaces in debugfs system
From: Dudley Du @ 2014-08-25 2:26 UTC (permalink / raw)
To: 'Dmitry Torokhov', Dudley Du
Cc: Rafael J. Wysocki, Benson Leung, Patrik Fimml, linux-input,
linux-kernel
In-Reply-To: <20140822235138.GA24242@core.coreip.homeip.net>
Hi Dmitry,
> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> Sent: Saturday, August 23, 2014 7:52 AM
> To: Dudley Du
> Cc: Rafael J. Wysocki; Benson Leung; Patrik Fimml; linux-input@vger.kernel.org;
> linux-kernel@vger.kernel.org; Dudley Du
> Subject: Re: [PATCH v4 5/14] input: cyapa: add read firmware image and raw
> data interfaces in debugfs system
>
> Hi Dudley,
>
> On Thu, Jul 17, 2014 at 02:52:36PM +0800, Dudley Du wrote:
> > Add read_fw and raw_data debugfs interfaces for easier issues location
> > and collection when report by user.
> > TEST=test on Chromebooks.
> >
> > Signed-off-by: Dudley Du <dudl@cypress.com>
> > ---
> > drivers/input/mouse/cyapa.c | 219
> +++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 219 insertions(+)
> >
> > diff --git a/drivers/input/mouse/cyapa.c b/drivers/input/mouse/cyapa.c
> > index 53c9d59..ae24b02 100644
> > --- a/drivers/input/mouse/cyapa.c
> > +++ b/drivers/input/mouse/cyapa.c
> > @@ -39,6 +39,8 @@
> >
> > const char unique_str[] = "CYTRA";
> >
> > +/* Global root node of the cyapa debugfs directory. */
> > +static struct dentry *cyapa_debugfs_root;
> >
> >
> > ssize_t cyapa_i2c_reg_read_block(struct cyapa *cyapa, u8 reg, size_t len,
> > @@ -461,6 +463,205 @@ done:
> > }
> >
> > /*
> > + **************************************************************
> > + * debugfs interface
> > + **************************************************************
> > +*/
> > +static int cyapa_debugfs_open(struct inode *inode, struct file *file)
> > +{
> > + struct cyapa *cyapa = inode->i_private;
> > + int ret;
> > +
> > + if (!cyapa)
> > + return -ENODEV;
> > +
> > + ret = mutex_lock_interruptible(&cyapa->debugfs_mutex);
> > + if (ret)
> > + return ret;
> > +
> > + if (!kobject_get(&cyapa->client->dev.kobj)) {
>
> Why not get_device() here and elsewhere?
Thanks. I will use get_device()/put_device() instead.
>
> > + ret = -ENODEV;
> > + goto out;
> > + }
> > +
> > + file->private_data = cyapa;
> > +
> > + if (cyapa->fw_image) {
> > + ret = 0;
> > + goto out;
> > + }
> > +
> > + mutex_lock(&cyapa->state_sync_lock);
> > + /*
> > + * If firmware hasn't been read yet, read it all in one pass.
> > + * Subsequent opens will reuse the data in this same buffer.
> > + */
> > + if (cyapa->ops->read_fw)
> > + ret = cyapa->ops->read_fw(cyapa);
> > + else
> > + ret = -EPERM;
> > + mutex_unlock(&cyapa->state_sync_lock);
> > +
> > + /* Redetect trackpad device states. */
> > + cyapa_detect_async(cyapa, 0);
> > +
> > +out:
> > + mutex_unlock(&cyapa->debugfs_mutex);
> > + return ret;
> > +}
> > +
> > +static int cyapa_debugfs_release(struct inode *inode, struct file *file)
> > +{
> > + struct cyapa *cyapa = file->private_data;
> > + int ret;
> > +
> > + if (!cyapa)
> > + return 0;
> > +
> > + ret = mutex_lock_interruptible(&cyapa->debugfs_mutex);
> > + if (ret)
> > + return ret;
> > + file->private_data = NULL;
> > + kobject_put(&cyapa->client->dev.kobj);
> > + mutex_unlock(&cyapa->debugfs_mutex);
> > +
> > + return 0;
> > +}
> > +
> > +/* Return some bytes from the buffered firmware image, starting from *ppos
> */
> > +static ssize_t cyapa_debugfs_read_fw(struct file *file, char __user *buffer,
> > + size_t count, loff_t *ppos)
> > +{
> > + struct cyapa *cyapa = file->private_data;
> > +
> > + if (!cyapa->fw_image)
> > + return -EINVAL;
> > +
> > + if (*ppos >= cyapa->fw_image_size)
> > + return 0;
> > +
> > + if (count + *ppos > cyapa->fw_image_size)
> > + count = cyapa->fw_image_size - *ppos;
> > +
> > + if (copy_to_user(buffer, &cyapa->fw_image[*ppos], count))
> > + return -EFAULT;
> > +
> > + *ppos += count;
> > + return count;
> > +}
> > +
> > +static const struct file_operations cyapa_read_fw_fops = {
> > + .open = cyapa_debugfs_open,
> > + .release = cyapa_debugfs_release,
> > + .read = cyapa_debugfs_read_fw
> > +};
> > +
> > +static int cyapa_debugfs_raw_data_open(struct inode *inode, struct file
> *file)
> > +{
> > + struct cyapa *cyapa = inode->i_private;
> > + int ret;
> > +
> > + if (!cyapa)
> > + return -ENODEV;
> > +
> > + /* Start to be supported after Gen5 trackpad devices. */
> > + if (cyapa->gen < CYAPA_GEN5)
> > + return -ENOTSUPP;
> > +
> > + ret = mutex_lock_interruptible(&cyapa->debugfs_mutex);
> > + if (ret)
> > + return ret;
> > +
> > + if (!kobject_get(&cyapa->client->dev.kobj)) {
> > + ret = -ENODEV;
> > + goto out;
> > + }
> > +
> > + file->private_data = cyapa;
> > +
> > + mutex_lock(&cyapa->state_sync_lock);
> > + if (cyapa->ops->read_raw_data)
> > + ret = cyapa->ops->read_raw_data(cyapa);
> > + else
> > + ret = -EPERM;
> > + mutex_unlock(&cyapa->state_sync_lock);
> > +out:
> > + mutex_unlock(&cyapa->debugfs_mutex);
> > + return ret;
> > +}
> > +
> > +static int cyapa_debugfs_raw_data_release(struct inode *inode,
> > + struct file *file)
> > +{
> > + struct cyapa *cyapa = file->private_data;
> > + int ret;
> > +
> > + if (!cyapa)
> > + return 0;
> > +
> > + ret = mutex_lock_interruptible(&cyapa->debugfs_mutex);
> > + if (ret)
> > + return ret;
> > + file->private_data = NULL;
> > + kobject_put(&cyapa->client->dev.kobj);
> > + mutex_unlock(&cyapa->debugfs_mutex);
> > +
> > + return 0;
> > +}
> > +
> > +/* Always return the sensors' latest raw data from trackpad device. */
> > +static ssize_t cyapa_debugfs_read_raw_data(struct file *file,
> > + char __user *buffer,
> > + size_t count, loff_t *ppos)
> > +{
> > + struct cyapa *cyapa = file->private_data;
> > +
> > + if (!cyapa->tp_raw_data)
> > + return -EINVAL;
> > +
> > + if (*ppos >= cyapa->tp_raw_data_size)
> > + return 0;
> > +
> > + if (count + *ppos > cyapa->tp_raw_data_size)
> > + count = cyapa->tp_raw_data_size - *ppos;
> > +
> > + if (copy_to_user(buffer, &cyapa->tp_raw_data[*ppos], count))
> > + return -EFAULT;
> > +
> > + *ppos += count;
> > + return count;
> > +}
> > +
> > +static const struct file_operations cyapa_read_raw_data_fops = {
> > + .open = cyapa_debugfs_raw_data_open,
> > + .release = cyapa_debugfs_raw_data_release,
> > + .read = cyapa_debugfs_read_raw_data
> > +};
> > +
> > +static int cyapa_debugfs_init(struct cyapa *cyapa)
> > +{
> > + struct device *dev = &cyapa->client->dev;
> > +
> > + if (!cyapa_debugfs_root)
> > + return -ENODEV;
> > +
> > + cyapa->dentry_dev = debugfs_create_dir(kobject_name(&dev->kobj),
> > + cyapa_debugfs_root);
> > +
> > + if (!cyapa->dentry_dev)
> > + return -ENODEV;
> > +
> > + mutex_init(&cyapa->debugfs_mutex);
> > +
> > + debugfs_create_file(CYAPA_DEBUGFS_READ_FW, S_IRUSR, cyapa->dentry_dev,
> > + cyapa, &cyapa_read_fw_fops);
> > +
> > + debugfs_create_file(CYAPA_DEBUGFS_RAW_DATA, S_IRUSR, cyapa->dentry_dev,
> > + cyapa, &cyapa_read_raw_data_fops);
> > + return 0;
> > +}
> > +
> > +/*
> > * Sysfs Interface.
> > */
> >
> > @@ -869,6 +1070,13 @@ static int cyapa_probe(struct i2c_client *client,
> > if (sysfs_create_group(&client->dev.kobj, &cyapa_sysfs_group))
> > dev_warn(dev, "error creating sysfs entries.\n");
> >
> > + /* Create a global debugfs root for all cyapa devices */
> > + cyapa_debugfs_root = debugfs_create_dir("cyapa", NULL);
> > + if (cyapa_debugfs_root == ERR_PTR(-ENODEV))
> > + cyapa_debugfs_root = NULL;
> > + if (cyapa_debugfs_init(cyapa))
> > + dev_warn(dev, "error creating debugfs entries.\n");
> > +
> > #ifdef CONFIG_PM_SLEEP
> > if (device_can_wakeup(dev) &&
> > sysfs_merge_group(&client->dev.kobj, &cyapa_power_wakeup_group))
> > @@ -902,8 +1110,19 @@ static int cyapa_remove(struct i2c_client *client)
> > #ifdef CONFIG_PM_RUNTIME
> > sysfs_unmerge_group(&client->dev.kobj, &cyapa_power_runtime_group);
> > #endif
> > +
> > + kfree(cyapa->fw_image);
> > + cyapa->fw_image = NULL;
> > + cyapa->fw_image_size = 0;
> > + kfree(cyapa->tp_raw_data);
> > + cyapa->tp_raw_data = NULL;
> > + cyapa->tp_raw_data_size = 0;
>
> This should be happening only after you remove debugfs entries.
Thanks, will change to the correct order.
>
>
> > free_irq(cyapa->irq, cyapa);
> >
> > + debugfs_remove_recursive(cyapa->dentry_dev);
> > + debugfs_remove_recursive(cyapa_debugfs_root);
>
> Hm, it seems that your debugfs entries are created per-devce, but you remove
> all of them at one when very first device us unbound. You probably want to
> create and destroy root of your debugfs hierarchy on module level.
Yes. You are correct.
I would change it to use module_init()/module_exit() to create and remove cyapa_debugfs_root.
>
> > + mutex_destroy(&cyapa->debugfs_mutex);
> > +
> > input_unregister_device(cyapa->input);
> > if (cyapa->ops->set_power_mode)
> > cyapa->ops->set_power_mode(cyapa, PWR_MODE_OFF, 0);
> > --
> > 1.7.9.5
> >
> >
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* RE: [PATCH v4 6/14] input: cyapa: add gen3 trackpad device basic functions support
From: Dudley Du @ 2014-08-25 2:27 UTC (permalink / raw)
To: 'Dmitry Torokhov', Dudley Du
Cc: Rafael J. Wysocki, Benson Leung, Patrik Fimml, linux-input,
linux-kernel
In-Reply-To: <20140822235528.GB24242@core.coreip.homeip.net>
Hi Dmitry,
> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> Sent: Saturday, August 23, 2014 7:55 AM
> To: Dudley Du
> Cc: Rafael J. Wysocki; Benson Leung; Patrik Fimml; linux-input@vger.kernel.org;
> linux-kernel@vger.kernel.org; Dudley Du
> Subject: Re: [PATCH v4 6/14] input: cyapa: add gen3 trackpad device basic
> functions support
>
> On Thu, Jul 17, 2014 at 02:53:48PM +0800, Dudley Du wrote:
> > Based on the cyapa core, add the gen3 trackpad device's basic functions
> > supported, so gen3 trackpad device can work with kernel input system.
> > The basic function is absolutely same as previous cyapa driver only
> > support gen3 trackpad device.
> > TEST=test on Chromebooks.
> >
> > Signed-off-by: Dudley Du <dudl@cypress.com>
> > ---
> > drivers/input/mouse/Makefile | 2 +-
> > drivers/input/mouse/cyapa.c | 96 ++++-
> > drivers/input/mouse/cyapa.h | 1 +
> > drivers/input/mouse/cyapa_gen3.c | 784
> ++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 881 insertions(+), 2 deletions(-)
> > create mode 100644 drivers/input/mouse/cyapa_gen3.c
> >
> > diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile
> > index 8608eb7..63b42e0 100644
> > --- a/drivers/input/mouse/Makefile
> > +++ b/drivers/input/mouse/Makefile
> > @@ -35,4 +35,4 @@ psmouse-$(CONFIG_MOUSE_PS2_TRACKPOINT) += trackpoint.o
> > psmouse-$(CONFIG_MOUSE_PS2_TOUCHKIT) += touchkit_ps2.o
> > psmouse-$(CONFIG_MOUSE_PS2_CYPRESS) += cypress_ps2.o
> >
> > -cyapatp-y := cyapa.o
> > +cyapatp-y := cyapa.o cyapa_gen3.o
> > diff --git a/drivers/input/mouse/cyapa.c b/drivers/input/mouse/cyapa.c
> > index ae24b02..5c62503 100644
> > --- a/drivers/input/mouse/cyapa.c
> > +++ b/drivers/input/mouse/cyapa.c
> > @@ -113,6 +113,15 @@ ssize_t cyapa_i2c_write(struct cyapa *cyapa, u8 reg,
> >
> > void cyapa_default_irq_handler(struct cyapa *cyapa)
> > {
> > + bool cont;
> > +
> > + /* Interrupt triggerred by command response in detecting. */
> > + cont = true;
> > + if (cyapa_gen3_ops.irq_cmd_handler)
> > + cont = cyapa_gen3_ops.irq_cmd_handler(cyapa);
>
> Why not simply
>
> cont = cyapa->ops->irq_cmd_handler(cyapa)?
When this the default irq handler is called, at this time, means the device
haven't been initialized yet, the cyapa->ops hasn't been setup correctly, and
the cyapa->ops->irq_cmd_handler is NULL.
So currently, maybe the driver is still on detecting, which device module should
be setup to cyapa->ops is unknown yet. But like gen5 device, it will execute commands
that based on interrupt signals, so the irq_cmd_handler() must be executed
for each module to complete the possible command response when an interrupt comes.
So, here, for each device module, their irq_cmd_handler() should be called directly.
>
>
> > + if (!cont)
> > + return;
> > +
> > /*
> > * Do redetecting when device states is still unknown and
> > * interrupt envent is received from device.
> > @@ -252,6 +261,9 @@ static int cyapa_check_is_operational(struct cyapa
> *cyapa)
> > return ret;
> >
> > switch (cyapa->gen) {
> > + case CYAPA_GEN3:
> > + cyapa->ops = &cyapa_gen3_ops;
> > + break;
> > default:
> > cyapa->ops = &cyapa_default_ops;
> > cyapa->gen = CYAPA_GEN_UNKNOWN;
> > @@ -314,9 +326,85 @@ out:
> > */
> > static int cyapa_get_state(struct cyapa *cyapa)
> > {
> > + int ret;
> > + u8 status[BL_STATUS_SIZE];
> > + u8 cmd[32];
> > + /* The i2c address of gen4 and gen5 trackpad device must be even. */
> > + bool even_addr = ((cyapa->client->addr & 0x0001) == 0);
> > + bool smbus = false;
> > + int retries = 2;
> > +
> > cyapa->state = CYAPA_STATE_NO_DEVICE;
> >
> > - return -ENODEV;
> > + /*
> > + * Get trackpad status by reading 3 registers starting from 0.
> > + * If the device is in the bootloader, this will be BL_HEAD.
> > + * If the device is in operation mode, this will be the DATA regs.
> > + *
> > + */
> > + ret = cyapa_i2c_reg_read_block(cyapa, BL_HEAD_OFFSET, BL_STATUS_SIZE,
> > + status);
> > +
> > + /*
> > + * On smbus systems in OP mode, the i2c_reg_read will fail with
> > + * -ETIMEDOUT. In this case, try again using the smbus equivalent
> > + * command. This should return a BL_HEAD indicating CYAPA_STATE_OP.
> > + */
> > + if (cyapa->smbus && (ret == -ETIMEDOUT || ret == -ENXIO)) {
> > + if (!even_addr)
> > + ret = cyapa_read_block(cyapa,
> > + CYAPA_CMD_BL_STATUS, status);
> > + smbus = true;
> > + }
> > + if (ret != BL_STATUS_SIZE)
> > + goto error;
> > +
> > + /*
> > + * Detect trackpad protocol based on characristic registers and bits.
> > + */
> > + do {
> > + cyapa->status[REG_OP_STATUS] = status[REG_OP_STATUS];
> > + cyapa->status[REG_BL_STATUS] = status[REG_BL_STATUS];
> > + cyapa->status[REG_BL_ERROR] = status[REG_BL_ERROR];
> > +
> > + if (cyapa->gen == CYAPA_GEN_UNKNOWN ||
> > + cyapa->gen == CYAPA_GEN3) {
> > + ret = cyapa_gen3_ops.state_parse(cyapa,
> > + status, BL_STATUS_SIZE);
> > + if (ret == 0)
> > + goto out_detected;
> > + }
> > +
> > + /*
> > + * Cannot detect communication protocol based on current
> > + * charateristic registers and bits.
> > + * So write error command to do further detection.
> > + * this method only valid on I2C bus.
> > + * for smbus interface, it won't have overwrite issue.
> > + */
> > + if (!smbus) {
> > + cmd[0] = 0x00;
> > + cmd[1] = 0x00;
> > + ret = cyapa_i2c_write(cyapa, 0, 2, cmd);
> > + if (ret)
> > + goto error;
> > +
> > + msleep(50);
> > +
> > + ret = cyapa_i2c_read(cyapa, BL_HEAD_OFFSET,
> > + BL_STATUS_SIZE, status);
> > + if (ret < 0)
> > + goto error;
> > + }
> > + } while (--retries > 0 && !smbus);
> > +
> > + goto error;
> > +
> > +out_detected:
> > + return 0;
> > +
> > +error:
> > + return (ret < 0) ? ret : -EAGAIN;
> > }
> >
> > /*
> > @@ -994,11 +1082,17 @@ static void cyapa_detect_and_start(void *data,
> async_cookie_t cookie)
> >
> > static int cyapa_tp_modules_init(struct cyapa *cyapa)
> > {
> > + if (cyapa_gen3_ops.initialize)
> > + return cyapa_gen3_ops.initialize(cyapa);
>
> Same here.
Same reason as above.
>
> > +
> > return 0;
> > }
> >
> > static int cyapa_tp_modules_uninit(struct cyapa *cyapa)
> > {
> > + if (cyapa_gen3_ops.uninitialize)
> > + return cyapa_gen3_ops.uninitialize(cyapa);
>
> And here.
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* Re: [PATCH 1/2] SOUND: kill gameport bits
From: Takashi Iwai @ 2014-08-25 7:13 UTC (permalink / raw)
To: Andreas Mohr
Cc: Dmitry Torokhov, linux-input, linux-kernel, Vojtech Pavlik,
Jiri Kosina
In-Reply-To: <20140824050716.GA523@rhlx01.hs-esslingen.de>
At Sun, 24 Aug 2014 07:07:16 +0200,
Andreas Mohr wrote:
>
> On Thu, Aug 21, 2014 at 01:29:03PM +0200, Takashi Iwai wrote:
> > I did a quick hack and it seems working on my box.
> > The patch is below.
>
> Thanks!!
>
> Further comments below.
>
> I will be testing this ASAP.
> > +static bool use_ktime = true;
> > +module_param(use_ktime, bool, 0400);
>
> Towards final commit, should probably add param docs on what may be switched here and why.
>
> > +
> > /*
> > * gameport_mutex protects entire gameport subsystem and is taken
> > * every time gameport port or driver registrered or unregistered.
> > @@ -76,6 +80,36 @@ static unsigned int get_time_pit(void)
> >
> > static int gameport_measure_speed(struct gameport *gameport)
> > {
> > + unsigned int i, t, tx;
> > + u64 t1, t2;
> > + unsigned long flags;
> > +
> > + if (gameport_open(gameport, NULL, GAMEPORT_MODE_RAW))
> > + return 0;
> > +
> > + tx = ~0;
> > +
> > + for (i = 0; i < 50; i++) {
> > + local_irq_save(flags);
> > + t1 = ktime_get_ns();
> > + for (t = 0; t < 50; t++)
> > + gameport_read(gameport);
> > + t2 = ktime_get_ns();
> > + local_irq_restore(flags);
> > + udelay(i * 10);
> > + if (t2 - t1 < tx)
> > + tx = t2 - t1;
>
> This impl is not doing the more complex t3, t2, t1 calculation
> that the PIT impl is doing (likely for the uncommented purpose
> of eliminating timer I/O delay from timing consideration).
> Do/don't ktime/TSC impls better need such an I/O timing correction,
> or are they so fast relative to gameport I/O delays
> that it does not matter? (probably the case for TSC at least).
It's based on x86-64 implementation that doesn't take t3 into
account. I don't think it doesn't matter so much on the recent
systems, but certainly it can't hurt to measure it, too.
> Oh, and any reason that such a speed calculation remains painfully duplicated
> in both source files? That's possibly done for layering reasons,
> but I'd have to analyze it further.
Yeah, a layer should be one reason. Another reason is that TSC read
has to be a macro, thus you'd need anyway reimplementation, either
static inline or such.
In my patch, I didn't want to change too much in a shot. It just adds
the replacement using ktime, that's all. If you'd like to work on
this further, feel free to do it.
> > +static inline u64 get_time(void)
> > +{
> > + if (use_ktime) {
> > + return ktime_get_ns();
> > + } else {
> > + unsigned int x;
> > + GET_TIME(x);
> > + return x;
> > + }
> > +}
>
> It might be useful to have a first commit to introduce these helpers,
> and a second commit to then add ktime support (to keep review code size
> down).
The very purpose of this helper is for ktime. For TSC, the helper
*is* GET_TIME(). So, splitting commit without introducing ktime
doesn't make much sense.
Nevertheless: did anyone test the patch at all...?
thanks,
Takashi
^ permalink raw reply
* Re: [PATCH 1/2] HID: logitech-dj: prevent false errors to be shown
From: Jiri Kosina @ 2014-08-25 7:50 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: linux-input, linux-kernel, Markus Trippelsdorf, Ben Hawkes
In-Reply-To: <1408738566-6805-1-git-send-email-benjamin.tissoires@redhat.com>
On Fri, 22 Aug 2014, Benjamin Tissoires wrote:
> Commit "HID: logitech: perform bounds checking on device_id early
> enough" unfortunately leaks some errors to dmesg which are not real
> ones:
> - if the report is not a DJ one, then there is not point in checking
> the device_id
> - the receiver (index 0) can also receive some notifications which
> can be safely ignored given the current implementation
>
> Move out the test regarding the report_id and also discards
> printing errors when the receiver got notified.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
I have now queued this one for 3.17. Adding Markus to CC -- Markus, this
should make the spurious error messages you have reported go away.
Thanks.
> ---
> drivers/hid/hid-logitech-dj.c | 43 +++++++++++++++++++++++++------------------
> drivers/hid/hid-logitech-dj.h | 1 +
> 2 files changed, 26 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
> index b7ba829..9bf8637 100644
> --- a/drivers/hid/hid-logitech-dj.c
> +++ b/drivers/hid/hid-logitech-dj.c
> @@ -656,7 +656,6 @@ static int logi_dj_raw_event(struct hid_device *hdev,
> struct dj_receiver_dev *djrcv_dev = hid_get_drvdata(hdev);
> struct dj_report *dj_report = (struct dj_report *) data;
> unsigned long flags;
> - bool report_processed = false;
>
> dbg_hid("%s, size:%d\n", __func__, size);
>
> @@ -683,34 +682,42 @@ static int logi_dj_raw_event(struct hid_device *hdev,
> * device (via hid_input_report() ) and return 1 so hid-core does not do
> * anything else with it.
> */
> +
> + /* case 1) */
> + if (data[0] != REPORT_ID_DJ_SHORT)
> + return false;
> +
> if ((dj_report->device_index < DJ_DEVICE_INDEX_MIN) ||
> (dj_report->device_index > DJ_DEVICE_INDEX_MAX)) {
> - dev_err(&hdev->dev, "%s: invalid device index:%d\n",
> + /*
> + * Device index is wrong, bail out.
> + * This driver can ignore safely the receiver notifications,
> + * so ignore those reports too.
> + */
> + if (dj_report->device_index != DJ_RECEIVER_INDEX)
> + dev_err(&hdev->dev, "%s: invalid device index:%d\n",
> __func__, dj_report->device_index);
> return false;
> }
>
> spin_lock_irqsave(&djrcv_dev->lock, flags);
> - if (dj_report->report_id == REPORT_ID_DJ_SHORT) {
> - switch (dj_report->report_type) {
> - case REPORT_TYPE_NOTIF_DEVICE_PAIRED:
> - case REPORT_TYPE_NOTIF_DEVICE_UNPAIRED:
> - logi_dj_recv_queue_notification(djrcv_dev, dj_report);
> - break;
> - case REPORT_TYPE_NOTIF_CONNECTION_STATUS:
> - if (dj_report->report_params[CONNECTION_STATUS_PARAM_STATUS] ==
> - STATUS_LINKLOSS) {
> - logi_dj_recv_forward_null_report(djrcv_dev, dj_report);
> - }
> - break;
> - default:
> - logi_dj_recv_forward_report(djrcv_dev, dj_report);
> + switch (dj_report->report_type) {
> + case REPORT_TYPE_NOTIF_DEVICE_PAIRED:
> + case REPORT_TYPE_NOTIF_DEVICE_UNPAIRED:
> + logi_dj_recv_queue_notification(djrcv_dev, dj_report);
> + break;
> + case REPORT_TYPE_NOTIF_CONNECTION_STATUS:
> + if (dj_report->report_params[CONNECTION_STATUS_PARAM_STATUS] ==
> + STATUS_LINKLOSS) {
> + logi_dj_recv_forward_null_report(djrcv_dev, dj_report);
> }
> - report_processed = true;
> + break;
> + default:
> + logi_dj_recv_forward_report(djrcv_dev, dj_report);
> }
> spin_unlock_irqrestore(&djrcv_dev->lock, flags);
>
> - return report_processed;
> + return true;
> }
>
> static int logi_dj_probe(struct hid_device *hdev,
> diff --git a/drivers/hid/hid-logitech-dj.h b/drivers/hid/hid-logitech-dj.h
> index 4a40003..daeb0aa 100644
> --- a/drivers/hid/hid-logitech-dj.h
> +++ b/drivers/hid/hid-logitech-dj.h
> @@ -27,6 +27,7 @@
>
> #define DJ_MAX_PAIRED_DEVICES 6
> #define DJ_MAX_NUMBER_NOTIFICATIONS 8
> +#define DJ_RECEIVER_INDEX 0
> #define DJ_DEVICE_INDEX_MIN 1
> #define DJ_DEVICE_INDEX_MAX 6
>
> --
> 2.1.0
>
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 2/2] HID: logitech-dj: break out testing of validity of dj_device
From: Jiri Kosina @ 2014-08-25 7:51 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: linux-input, linux-kernel
In-Reply-To: <1408738566-6805-2-git-send-email-benjamin.tissoires@redhat.com>
On Fri, 22 Aug 2014, Benjamin Tissoires wrote:
> We can do once the test of the validity of the dj_device, which removes
> some duplicated code in various functions.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
I will queue this cleanup for next merge window. Thanks.
> ---
> drivers/hid/hid-logitech-dj.c | 35 +++++++++++------------------------
> 1 file changed, 11 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
> index 9bf8637..71f5692 100644
> --- a/drivers/hid/hid-logitech-dj.c
> +++ b/drivers/hid/hid-logitech-dj.c
> @@ -385,18 +385,6 @@ static void logi_dj_recv_forward_null_report(struct dj_receiver_dev *djrcv_dev,
>
> djdev = djrcv_dev->paired_dj_devices[dj_report->device_index];
>
> - if (!djdev) {
> - dbg_hid("djrcv_dev->paired_dj_devices[dj_report->device_index]"
> - " is NULL, index %d\n", dj_report->device_index);
> - kfifo_in(&djrcv_dev->notif_fifo, dj_report, sizeof(struct dj_report));
> -
> - if (schedule_work(&djrcv_dev->work) == 0) {
> - dbg_hid("%s: did not schedule the work item, was already "
> - "queued\n", __func__);
> - }
> - return;
> - }
> -
> memset(reportbuffer, 0, sizeof(reportbuffer));
>
> for (i = 0; i < NUMBER_OF_HID_REPORTS; i++) {
> @@ -421,18 +409,6 @@ static void logi_dj_recv_forward_report(struct dj_receiver_dev *djrcv_dev,
>
> dj_device = djrcv_dev->paired_dj_devices[dj_report->device_index];
>
> - if (dj_device == NULL) {
> - dbg_hid("djrcv_dev->paired_dj_devices[dj_report->device_index]"
> - " is NULL, index %d\n", dj_report->device_index);
> - kfifo_in(&djrcv_dev->notif_fifo, dj_report, sizeof(struct dj_report));
> -
> - if (schedule_work(&djrcv_dev->work) == 0) {
> - dbg_hid("%s: did not schedule the work item, was already "
> - "queued\n", __func__);
> - }
> - return;
> - }
> -
> if ((dj_report->report_type > ARRAY_SIZE(hid_reportid_size_map) - 1) ||
> (hid_reportid_size_map[dj_report->report_type] == 0)) {
> dbg_hid("invalid report type:%x\n", dj_report->report_type);
> @@ -701,8 +677,17 @@ static int logi_dj_raw_event(struct hid_device *hdev,
> }
>
> spin_lock_irqsave(&djrcv_dev->lock, flags);
> +
> + if (!djrcv_dev->paired_dj_devices[dj_report->device_index]) {
> + /* received an event for an unknown device, bail out */
> + logi_dj_recv_queue_notification(djrcv_dev, dj_report);
> + goto out;
> + }
> +
> switch (dj_report->report_type) {
> case REPORT_TYPE_NOTIF_DEVICE_PAIRED:
> + /* pairing notifications are handled above the switch */
> + break;
> case REPORT_TYPE_NOTIF_DEVICE_UNPAIRED:
> logi_dj_recv_queue_notification(djrcv_dev, dj_report);
> break;
> @@ -715,6 +700,8 @@ static int logi_dj_raw_event(struct hid_device *hdev,
> default:
> logi_dj_recv_forward_report(djrcv_dev, dj_report);
> }
> +
> +out:
> spin_unlock_irqrestore(&djrcv_dev->lock, flags);
>
> return true;
> --
> 2.1.0
>
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 1/2] HID: logitech-dj: prevent false errors to be shown
From: Markus Trippelsdorf @ 2014-08-25 7:55 UTC (permalink / raw)
To: Jiri Kosina; +Cc: Benjamin Tissoires, linux-input, linux-kernel, Ben Hawkes
In-Reply-To: <alpine.LNX.2.00.1408250249150.23162@pobox.suse.cz>
On 2014.08.25 at 02:50 -0500, Jiri Kosina wrote:
> On Fri, 22 Aug 2014, Benjamin Tissoires wrote:
>
> > Commit "HID: logitech: perform bounds checking on device_id early
> > enough" unfortunately leaks some errors to dmesg which are not real
> > ones:
> > - if the report is not a DJ one, then there is not point in checking
> > the device_id
> > - the receiver (index 0) can also receive some notifications which
> > can be safely ignored given the current implementation
> >
> > Move out the test regarding the report_id and also discards
> > printing errors when the receiver got notified.
> >
> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>
> I have now queued this one for 3.17. Adding Markus to CC -- Markus, this
> should make the spurious error messages you have reported go away.
Indeed it does. Feel free to add:
Reported-and-tested-by: Markus Trippelsdorf <markus@trippelsdorf.de>
--
Markus
^ permalink raw reply
* Re: [PATCH 1/2] HID: logitech-dj: prevent false errors to be shown
From: Jiri Kosina @ 2014-08-25 8:00 UTC (permalink / raw)
To: Markus Trippelsdorf
Cc: Benjamin Tissoires, linux-input, linux-kernel, Ben Hawkes
In-Reply-To: <20140825075552.GA298@x4>
On Mon, 25 Aug 2014, Markus Trippelsdorf wrote:
> > > Commit "HID: logitech: perform bounds checking on device_id early
> > > enough" unfortunately leaks some errors to dmesg which are not real
> > > ones:
> > > - if the report is not a DJ one, then there is not point in checking
> > > the device_id
> > > - the receiver (index 0) can also receive some notifications which
> > > can be safely ignored given the current implementation
> > >
> > > Move out the test regarding the report_id and also discards
> > > printing errors when the receiver got notified.
> > >
> > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> >
> > I have now queued this one for 3.17. Adding Markus to CC -- Markus, this
> > should make the spurious error messages you have reported go away.
>
> Indeed it does. Feel free to add:
>
> Reported-and-tested-by: Markus Trippelsdorf <markus@trippelsdorf.de>
Excellent, thanks. Will be pushing it to Linus this week.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 00/12] HID: Convert UHID to new HID transport-layer
From: Jiri Kosina @ 2014-08-25 8:34 UTC (permalink / raw)
To: David Herrmann; +Cc: linux-input, Benjamin Tissoires
In-Reply-To: <1406646866-999-1-git-send-email-dh.herrmann@gmail.com>
On Tue, 29 Jul 2014, David Herrmann wrote:
> Hi
>
> When Benjamin cleanup up the transport layer, he left UHID mostly unconverted
> due to ABI issues. I promised to take a look, so here're the patches.
Hi David,
now queued for 3.18 in for-3.18/uhid branch.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 3/3] Add logitech m560 driver
From: Nestor Lopez Casado @ 2014-08-25 8:38 UTC (permalink / raw)
To: Goffredo Baroncelli; +Cc: open list:HID CORE LAYER, Goffredo Baroncelli
In-Reply-To: <1408797629-3474-4-git-send-email-kreijack@inwind.it>
Hi,
If you want to have a special driver for a specific Unifying device
you should put in place the right structure to have a separate driver
for it.
hid-logitech-dj is a "bus" driver for the Unifying *receiver*, not a
driver for any specific Unifying device.
Benjamin Tisssoires (github bentiss) has uploaded a few patches to his
github account to finish the infrastructure for specific Unifying
devices drivers. Look for the more complete branch 'for-whot' See how
he created a hid-logitech-wtp driver for the Different wireless
touchpads.
This would be the right approach to what you are trying to do.
Cheers,
-nestor
On Sat, Aug 23, 2014 at 2:40 PM, Goffredo Baroncelli <kreijack@gmail.com> wrote:
> Add logitech m560 support. In the init phase the driver
> send a sequence which avoid some unnecessary key sending
> from the mouse.
>
> The mouse appears as a couple of mouse and keyboard. Some buttons
> emit a key event instead of the "mouse button" event. However
> some event (like the middle button release) aren't generated.
> Fortunately, the device generates an additional event to
> track it. The event type is 0x0a.
>
> Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
> ---
> drivers/hid/hid-logitech-dj.c | 92 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 92 insertions(+)
>
> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
> index feddd3d..40c5ea1 100644
> --- a/drivers/hid/hid-logitech-dj.c
> +++ b/drivers/hid/hid-logitech-dj.c
> @@ -222,7 +222,99 @@ static inline void call_destroy(struct dj_device *djdev)
> djdev->methods->destroy(djdev);
> }
>
> +/*
> + * Send the sequence 10xx0a35 00af03
> + * to the mouse id xx. It disables the key sending by the central button.
> + */
> +
> +static int lg_m560_init_device(struct dj_device *dj_device)
> +{
> + struct dj_report *dj_report;
> + int retval;
> + static u8 reset_data[] = {0x35, 0x00, 0xaf, 0x03};
> +
> + dj_report = kzalloc(sizeof(struct dj_report), GFP_KERNEL);
> + if (!dj_report)
> + return -ENOMEM;
> +
> + dj_device->userdata = dj_report;
> +
> + dj_report->report_id = REPORT_ID_RECV_SHORT;
> + dj_report->device_index = dj_device->device_index;
> + dj_report->report_type = 0x0a;
> +
> + memcpy(dj_report->report_params, reset_data, sizeof(reset_data));
> +
> + retval = hid_hw_raw_request(dj_device->dj_receiver_dev->hdev,
> + dj_report->report_id,
> + (void *)dj_report, REPORT_ID_RECV_SHORT_LENGTH,
> + HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
> +
> + memset(dj_report, 0, sizeof(struct dj_report));
> +
> + return retval;
> +}
> +
> +static int lg_m560_parse_raw_event(struct dj_device *dj_device,
> + struct dj_report *dj_report)
> +{
> + u8 *m506_last_mouse_report = dj_device->userdata;
> +
> + BUG_ON(!dj_device);
> +
> + /* don't pass keys when the mouse is a m560 */
> + if (dj_report->report_type == REPORT_TYPE_KEYBOARD)
> + return true;
> +
> + if (dj_report->report_id == 0x11 && dj_report->report_type == 0x0a) {
> + int btn;
> +
> + /* check if the event is a button */
> + btn = dj_report->report_params[2];
> + if (btn != 0x00 && btn != 0xb0 && btn != 0xae && btn != 0xaf)
> + return true;
> +
> + if (btn == 0xaf)
> + m506_last_mouse_report[1] |= 4;
> + if (btn == 0xae)
> + m506_last_mouse_report[2] |= 2;
> + if (btn == 0xb0)
> + m506_last_mouse_report[2] |= 4;
> + if (btn == 0x00) {
> + m506_last_mouse_report[1] &= ~4;
> + m506_last_mouse_report[2] &= ~(4|2);
> + }
> +
> + if (hid_input_report(dj_device->hdev, HID_INPUT_REPORT,
> + m506_last_mouse_report, 8, 1)) {
> + dbg_hid("hid_input_report error\n");
> + }
> + return true;
> + }
> +
> + /* copy the button status */
> + if (dj_report->report_type == REPORT_TYPE_MOUSE) {
> + /* copy only the first 3 bytes: type, btn0..7, btn8..16 */
> + memcpy(dj_device->userdata,
> + &dj_report->report_type, 3);
> + }
> +
> + /* continue with the standard handler */
> + return false;
> +}
> +
> +static void lg_m560_destroy(struct dj_device *dev)
> +{
> + kfree(dev->userdata);
> +}
> +
> static struct dj_device_method dj_device_method[] = {
> + { /* logitech M560 */
> + .device_names = (char *[]){ "M560", NULL },
> + .init_device = lg_m560_init_device,
> + .parse_raw_event = lg_m560_parse_raw_event,
> + .destroy = lg_m560_destroy
> + },
>
> /* last element */
> { NULL, }
> --
> 1.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox