Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH] Input: evdev: Fall back to vmalloc for client event buffer
From: Daniel Stone @ 2013-10-07 13:06 UTC (permalink / raw)
  To: linux-input; +Cc: Henrik Rydberg

evdev always tries to allocate the event buffer for clients using
kzalloc rather than vmalloc, presumably to avoid mapping overhead where
possible.  However, drivers like bcm5974, which claims support for
reporting 16 fingers simultaneously, can have an extraordinarily large
buffer.  The resultant contiguous order-4 allocation attempt fails due
to fragmentation, and the device is thus unusable until reboot.

Try kzalloc if we can to avoid the mapping overhead, but if that fails,
fall back to vzalloc.

Signed-off-by: Daniel Stone <daniels@collabora.com>
---
 drivers/input/evdev.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index f0f8928..edfca4a 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -18,6 +18,7 @@
 #include <linux/poll.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
+#include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/input/mt.h>
@@ -289,7 +290,11 @@ static int evdev_release(struct inode *inode, struct file *file)
 	mutex_unlock(&evdev->mutex);
 
 	evdev_detach_client(evdev, client);
-	kfree(client);
+
+	if (is_vmalloc_addr(client))
+		vfree(client);
+	else
+		kfree(client);
 
 	evdev_close_device(evdev);
 
@@ -311,10 +316,12 @@ static int evdev_open(struct inode *inode, struct file *file)
 	unsigned int bufsize = evdev_compute_buffer_size(evdev->handle.dev);
 	struct evdev_client *client;
 	int error;
+	int size =
+		sizeof(struct evdev_client) + bufsize * sizeof(struct input_event);
 
-	client = kzalloc(sizeof(struct evdev_client) +
-				bufsize * sizeof(struct input_event),
-			 GFP_KERNEL);
+	client = kzalloc(size, GFP_KERNEL | __GFP_NOWARN);
+	if (!client)
+		client = vzalloc(size);
 	if (!client)
 		return -ENOMEM;
 
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCH] HID: multitouch: Fix GeneralTouch products and add more PIDs
From: Jiri Kosina @ 2013-10-07 15:07 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Benjamin Tissoires, Henrik Rydberg, linux-input, linux-kernel,
	Luosong
In-Reply-To: <alpine.LNX.2.00.1310021654240.29701@pobox.suse.cz>

On Wed, 2 Oct 2013, Jiri Kosina wrote:

> On Wed, 2 Oct 2013, Benjamin Tissoires wrote:
> 
> > From: Luosong <android@generaltouch.com>
> > 
> > GeneralTouch products should use the quirk SLOT_IS_CONTACTID
> > instead of SLOT_IS_CONTACTNUMBER.
> > 
> > Adding PIDs 0101,e100,0102,0106,010a from the new products.
> > 
> > Tested on new and older products by GeneralTouch engineers.
> > 
> > Signed-off-by: Luosong <android@generaltouch.com>
> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > ---
> > 
> > Hi Luosong,
> > 
> > I took the liberty to rewrite the commit message for it to be
> > more compliant with the kernel rules:
> > - there should be a one line title commit message first,
> > - and the rest should detail the patch, tell how it was tested, etc...
> > 
> > If you could just give our ack on this one, Jiri would be able to take it
> > through his tree.
> 
> Thanks Benjamin.
> 
> Luosong, the patch as sent by Benjamin is properly formatted and contains 
> proper changelog. Please make sure that you fix your patch submission 
> workflow for your next submissions.
> 
> Luosong, please let me know that you are find with applying the patch as 
> Benjamin sent it (with attribution to you and your signoff), and I'll 
> proceed.

Luosong, hello ... ?

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH] HID: wiimote: fix FF deadlock
From: Jiri Kosina @ 2013-10-07 15:09 UTC (permalink / raw)
  To: David Herrmann; +Cc: linux-input, stable
In-Reply-To: <1380714448-878-1-git-send-email-dh.herrmann@gmail.com>

On Wed, 2 Oct 2013, David Herrmann wrote:

> The input core has an internal spinlock that is acquired during event
> injection via input_event() and friends but also held during FF callbacks.
> That means, there is no way to share a lock between event-injection and FF
> handling. Unfortunately, this is what is required for wiimote state
> tracking and what we do with state.lock and input->lock.
> 
> This deadlock can be triggered when using continuous data reporting and FF
> on a wiimote device at the same time. I takes me at least 30m of
> stress-testing to trigger it but users reported considerably shorter
> times (http://bpaste.net/show/132504/) when using some gaming-console
> emulators.
> 
> The real problem is that we have two copies of internal state, one in the
> wiimote objects and the other in the input device. As the input-lock is
> not supposed to be accessed from outside of input-core, we have no other
> chance than offloading FF handling into a worker. This actually works
> pretty nice and also allows to implictly merge fast rumble changes into a
> single request.
> 
> Due to the 3-layered workers (rumble+queue+l2cap) this might reduce FF
> responsiveness. Initial tests were fine so lets fix the race first and if
> it turns out to be too slow we can always handle FF out-of-band and skip
> the queue-worker.
> 
> Cc: <stable@vger.kernel.org> # 3.11+
> Reported-by: Thomas Schneider
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>

Applied, thanks David.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: New USB core API to change interval and max packet size
From: Hans de Goede @ 2013-10-07 15:25 UTC (permalink / raw)
  To: Sarah Sharp, Alan Stern
  Cc: Xenia Ragiadakou, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20131002183905.GG15395@xanatos>

Hi,

On 10/02/2013 08:39 PM, Sarah Sharp wrote:
> On Wed, Oct 02, 2013 at 10:22:52AM -0400, Alan Stern wrote:

<snip>

>> We should consider this before rushing into a new API.
>
> Yes, I agree. :)  That's why I'd like to see some cases in the media
> drivers code where it could benefit from changing the interval or
> maxpacket size, so that we can see what use cases we have.  Mauro, can
> you point is to places in drivers that would need this?

Allow me to jump in here. As you may remember I pointed out the media
side of this in the ubs-mini summit during the 2nd plumbers conference
in Portland.

I don't know about any media drivers which want to change interval, but
it make sense that there are those who will want to do that too, esp in
the hid area.

I can give 2 examples who want to change the maxpacketsize in media
land. Both for quite old usb-1 webcam chipsets. The manufacturers of
these chip-sets decided that having X alt-settings was too limiting, so
they have only 2. Alt 0 with no isoc endpoints, and alt 1 with 1 isoc
endpoint with a maxpacketsize of 1023. This means that if these device
somehow end up sharing usb-1 bandwidth with anything else (through
say 1 single tt usb-2 hub) they eat up all the periodic bandwidth, or
try to and fail.

Technically these devices actually have an almost unlimited number of
alt-settings, as they have a register through which the maxpacketsize
they will actually use can be configured.

Since we don't always need the full 1000 (interval 1) * 1023 bytes /
second bandwidth their alt-setting 1 gives us, currently their drivers
employ the following hack:

alt = &gspca_dev->dev->actconfig->intf_cache[0]->altsetting[1];
alt->endpoint[0].desc.wMaxPacketSize = cpu_to_le16(packet_size);

Before calling usb_set_interface(), so that they can start streaming
successfully even though the full usb-1 bandwidth which their alt1
descriptor claims it needs is not available, at least on ohci and ehci
this hack has that result, not sure if it works for xhci too (I can test
if people are interested).

For people who want to actually see the code for this admittedly ugly
hack, see:

drivers/media/usb/gspca/xirlink_cit.c

and:

drivers/media/usb/gspca/stv06xx/stv06xx.c

Note that their are several other chipsets which have a maxpacketsize
register too. But their descriptors do properly contain multiple
alt-settings with different maxpacketsizes (for the benefit of the OS
I assume), so all we do their is simply write the current wMaxPacketSize
to the register see ie: drivers/media/usb/gspca/ov519.c at line 3500 ,
technically we could use the discussed functionality there too, to do
finer grained bandwidth management but I don't really see a need for this.

>
>> In particular, do we want to go around changing single endpoints, one
>> at a time?  Or do we want to change all the endpoints in an interface
>> at once?
>>
>> Given that a change to one endpoint may require the entire schedule to
>> be recomputed, it seems to make more sense to do all of them at once.
>> For example, drivers could call a routine to set the desired endpoint
>> parameters, and then the new parameters could take effect when the
>> driver calls usb_set_interface().
>
> I think it makes sense to change several endpoints, and then ask the
> host to recompute the schedule.  Perhaps the driver could issue several
> calls to usb_change_ep_bandwidth and then ask the USB core to update the
> host's schedule?
>
> I'm not sure that usb_set_interface() is the right function for the new
> parameters to take effect.  What if the driver is trying to modify the
> current alt setting?  Would they need to call usb_set_interface() again?

What the 2 media drivers in question currently do is:

1) Determine an optimal wMaxPacketSize for the resolution + framerate
userspace has selected (so one where we can actually reach the framerate
with no framedrop or image degradation due to over aggressive compression).
Also determine a minimum wMaxPacketSize, below which streaming simply
won't work reliabe, even with frame drop, etc.
2) Put the wMaxPacketSize in the intf_cache for the alt1 alt-setting
3) Try a usb_set_interface(dev, iface, 1)
4) If 3). fails, reduce wMaxPacketSize in the intf_cache for the alt1 alt-setting
by 100 bytes, until we hit the minimum, then goto 3, or bail if the last
try was with the minimum already.

They are basically doing the same as other webcam drivers, except that
other drivers determine an optimal and minimum alt-setting with which
to work and then try set_interface with alt-settings with decreasing
wMaxPacketSize, where as these drivers keep retrying with the same
alt-setting, while changing the wMaxPacketSize of that alt-setting.

So from a media (specifically webcam driver) pov, the suggested approach
of first calling usb_change_ep_bandwidth 1 or more times, and then calling
usb_set_interface makes a lot of sense.

Note that in the hid case however where we are likely talking about
cases where the descriptors say something stupid like "poll this mouse
8000 times / second", or "poll this joystick 1 / second", normally the
driver will not call usb_set_interface() at all, because these
devices normally have only 1 alt-setting (only alt0).

Still since this is a special case for quirk handling, I think it is
reasonable that drivers have to call usb_set_interface() after calling
usb_change_ep_bandwidth(), for the changes to go into effect.

>> In any case, the question about what to do when the interface setting
>> gets switched never really arises.  Each usb_host_endpoint structure is
>> referenced from only one altsetting.  If the driver wants the new
>> parameters applied to an endpoint in several altsettings, it will have
>> to change each altsetting separately.
>
> Ok, so it sounds like we want to keep the changes the endpoints across
> alt setting changes.

At a minimum we want to keep them until the first usb_set_interface call,
which comes automatically with the transaction like interface Alan suggested
(so the usb_change_ep_bandwidth() calls just store a value some where and are
otherwise nops).

This makes me realize that the original suggestion to just have a
usb_change_ep_bandwidth() call which would change just that one ep and
immediately apply the changes, is not usable for the webcam case, since
in some cases the usb_set_interface call can only succeed with the
modified wMaxPacketSize and/or interval, so we would never be able to
move cur_altsetting to alt-setting 1, and thus would never be able to
change the wMaxPacketSize in there.

TBH I don't see much reason to keep the changes beyond the first set_interface
call, nor do I see much reason not too (except on driver unbind). Implementation
wise it might be easiest to just keep the override values until the first
call. This could be as simple as an array with 30 wMaxPacketSize override
values, and 30 interval overrides to be applied at the first set_interface
call.

 > But we still want to reset the values after the driver unbinds, correct?

Yes we definitely want to reset the values after an unbind.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re:Re: [PATCH] HID: multitouch: Fix GeneralTouch products and add morePIDs
From: Jiri Kosina @ 2013-10-07 15:44 UTC (permalink / raw)
  To: 罗松
  Cc: Benjamin Tissoires, Benjamin Tissoires, Henrik Rydberg,
	linux-input, linux-kernel
In-Reply-To: <233159121.325488.1381159663010.JavaMail.root@bj-e1-wmthree2>

On Mon, 7 Oct 2013, 罗松 wrote:

>     I am sorry to answer you so late.
> 
> 
>     yes, I have tested by our products in linux system /android system .

Thanks for confirmation. Now queued in hid.git.

-- 
Jiri Kosina
SUSE Labs
--
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

* [PATCH] HID: logitech - lg2ff: Add IDs for Formula Vibration Feedback Wheel
From: Anssi Hannula @ 2013-10-07 16:48 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: linux-usb, linux-input, Elias Vanderstuyft, Simon Wood, stable

From: Elias Vanderstuyft <Elias.vds@gmail.com>

Add USB IDs for Logitech Formula Vibration Feedback Wheel (046d:ca04).

The lg2ff force feedback subdriver is used for vibration and
HID_GD_MULTIAXIS is set to avoid deadzone like other Logitech wheels.

Kconfig description etc are also updated accordingly.

Signed-off-by: Elias Vanderstuyft <Elias.vds@gmail.com>
[anssi.hannula@iki.fi: added description and CCs]
Signed-off-by: Anssi Hannula <anssi.hannula@iki.fi>
Cc: Simon Wood <simon@mungewell.org>
Cc: <stable@vger.kernel.org>
---

Simon, does this look OK to you, or do you think it should be an lg4ff
device? Though I guess lg2ff is better than nothing even in that case.

Jiri, lets see if we get a comment from Simon before applying.

I added CC to stable since it is just ID additions (plus description
stuff), but feel free to drop that if you think it is not appropriate.


 drivers/hid/Kconfig     | 8 +++++---
 drivers/hid/hid-core.c  | 1 +
 drivers/hid/hid-ids.h   | 1 +
 drivers/hid/hid-lg.c    | 3 +++
 drivers/hid/hid-lg2ff.c | 2 +-
 5 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 71b70e3..42c2a8b 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -368,12 +368,14 @@ config LOGITECH_FF
 	  force feedback.
 
 config LOGIRUMBLEPAD2_FF
-	bool "Logitech RumblePad/Rumblepad 2 force feedback support"
+	bool "Logitech force feedback support (variant 2)"
 	depends on HID_LOGITECH
 	select INPUT_FF_MEMLESS
 	help
-	  Say Y here if you want to enable force feedback support for Logitech
-	  RumblePad and Rumblepad 2 devices.
+	  Say Y here if you want to enable force feedback support for:
+	  - Logitech RumblePad
+	  - Logitech Rumblepad 2
+	  - Logitech Formula Vibration Feedback Wheel
 
 config LOGIG940_FF
 	bool "Logitech Flight System G940 force feedback support"
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index b8470b1..037915f 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1752,6 +1752,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_FLIGHT_SYSTEM_G940) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_MOMO_WHEEL) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_MOMO_WHEEL2) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_VIBRATION_WHEEL) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_DFP_WHEEL) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_DFGT_WHEEL) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G25_WHEEL) },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index e60e8d5..0ff958b 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -570,6 +570,7 @@
 #define USB_DEVICE_ID_DINOVO_EDGE	0xc714
 #define USB_DEVICE_ID_DINOVO_MINI	0xc71f
 #define USB_DEVICE_ID_LOGITECH_MOMO_WHEEL2	0xca03
+#define USB_DEVICE_ID_LOGITECH_VIBRATION_WHEEL	0xca04
 
 #define USB_VENDOR_ID_LUMIO		0x202e
 #define USB_DEVICE_ID_CRYSTALTOUCH	0x0006
diff --git a/drivers/hid/hid-lg.c b/drivers/hid/hid-lg.c
index 6f12ecd..c2c7dab 100644
--- a/drivers/hid/hid-lg.c
+++ b/drivers/hid/hid-lg.c
@@ -492,6 +492,7 @@ static int lg_input_mapped(struct hid_device *hdev, struct hid_input *hi,
 		case USB_DEVICE_ID_LOGITECH_G27_WHEEL:
 		case USB_DEVICE_ID_LOGITECH_WII_WHEEL:
 		case USB_DEVICE_ID_LOGITECH_MOMO_WHEEL2:
+		case USB_DEVICE_ID_LOGITECH_VIBRATION_WHEEL:
 			field->application = HID_GD_MULTIAXIS;
 			break;
 		default:
@@ -639,6 +640,8 @@ static const struct hid_device_id lg_devices[] = {
 		.driver_data = LG_NOGET | LG_FF4 },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_MOMO_WHEEL2),
 		.driver_data = LG_FF4 },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_VIBRATION_WHEEL),
+		.driver_data = LG_FF2 },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G25_WHEEL),
 		.driver_data = LG_FF4 },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_DFGT_WHEEL),
diff --git a/drivers/hid/hid-lg2ff.c b/drivers/hid/hid-lg2ff.c
index 1a42eaa..0e3fb1a 100644
--- a/drivers/hid/hid-lg2ff.c
+++ b/drivers/hid/hid-lg2ff.c
@@ -95,7 +95,7 @@ int lg2ff_init(struct hid_device *hid)
 
 	hid_hw_request(hid, report, HID_REQ_SET_REPORT);
 
-	hid_info(hid, "Force feedback for Logitech RumblePad/Rumblepad 2 by Anssi Hannula <anssi.hannula@gmail.com>\n");
+	hid_info(hid, "Force feedback for Logitech variant 2 rumble devices by Anssi Hannula <anssi.hannula@gmail.com>\n");
 
 	return 0;
 }
-- 
1.8.1.5

^ permalink raw reply related

* Re: [PATCH] HID: logitech - lg2ff: Add IDs for Formula Vibration Feedback Wheel
From: simon-wM4F9T/ekXmXDw4h08c5KA @ 2013-10-07 17:48 UTC (permalink / raw)
  To: Anssi Hannula
  Cc: Jiri Kosina, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Elias Vanderstuyft,
	Simon Wood, stable-u79uwXL29TY76Z2rM5mHXA,
	"Michal Malý"
In-Reply-To: <1381164492-27145-1-git-send-email-anssi.hannula-X3B1VOXEql0@public.gmane.org>

> From: Elias Vanderstuyft <Elias.vds-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> Add USB IDs for Logitech Formula Vibration Feedback Wheel (046d:ca04).
>
> The lg2ff force feedback subdriver is used for vibration and
> HID_GD_MULTIAXIS is set to avoid deadzone like other Logitech wheels.
>
> Kconfig description etc are also updated accordingly.
>
> Signed-off-by: Elias Vanderstuyft <Elias.vds-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> [anssi.hannula-X3B1VOXEql0@public.gmane.org: added description and CCs]
> Signed-off-by: Anssi Hannula <anssi.hannula-X3B1VOXEql0@public.gmane.org>
> Cc: Simon Wood <simon-wM4F9T/ekXmXDw4h08c5KA@public.gmane.org>
> Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> ---
>
> Simon, does this look OK to you, or do you think it should be an lg4ff
> device? Though I guess lg2ff is better than nothing even in that case.
>
> Jiri, lets see if we get a comment from Simon before applying.
>

I've cc'ed Michal as he contributed a lot to lg4ff and is looking at
improving ff-memless.

I am unfamiliar with this wheel (180' rotation, bungee cord for autocenter
and rumble motors - or so google tells me...), it's not listed on
Wikipedia (hint, hint):
http://en.wikipedia.org/wiki/Racing_wheel

At present lg4ff does not support RUMBLE effects (only CONSTANT and
AUTOCENTER), so lg2ff seems a sensible place. The only minor concern I
would have is any confusion from the descriptions in Kconfig.

I didn't build, but think that the patch looks OK to commit.

For the other wheels we have re-written the HID descriptors to split
brake/accel into seperate axis. Do you know whether this is possible for
this wheel too?

Thanks for your patch,
Simon.

Signed-off-by: Simon Wood <simon-wM4F9T/ekXmXDw4h08c5KA@public.gmane.org>


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] HID: logitech - lg2ff: Add IDs for Formula Vibration Feedback Wheel
From: Anssi Hannula @ 2013-10-07 17:56 UTC (permalink / raw)
  To: simon
  Cc: Jiri Kosina, linux-usb, linux-input, Elias Vanderstuyft, stable,
	Michal Malý
In-Reply-To: <cec1e0ca61cf17ceb6e52faa4f2baad9.squirrel@mungewell.org>

07.10.2013 20:48, simon@mungewell.org kirjoitti:
>> From: Elias Vanderstuyft <Elias.vds@gmail.com>
>>
>> Add USB IDs for Logitech Formula Vibration Feedback Wheel (046d:ca04).
>>
>> The lg2ff force feedback subdriver is used for vibration and
>> HID_GD_MULTIAXIS is set to avoid deadzone like other Logitech wheels.
>>
>> Kconfig description etc are also updated accordingly.
>>
>> Signed-off-by: Elias Vanderstuyft <Elias.vds@gmail.com>
>> [anssi.hannula@iki.fi: added description and CCs]
>> Signed-off-by: Anssi Hannula <anssi.hannula@iki.fi>
>> Cc: Simon Wood <simon@mungewell.org>
>> Cc: <stable@vger.kernel.org>
>> ---
>>
>> Simon, does this look OK to you, or do you think it should be an lg4ff
>> device? Though I guess lg2ff is better than nothing even in that case.
>>
>> Jiri, lets see if we get a comment from Simon before applying.
>>
> 
> I've cc'ed Michal as he contributed a lot to lg4ff and is looking at
> improving ff-memless.
> 
> I am unfamiliar with this wheel (180' rotation, bungee cord for autocenter
> and rumble motors - or so google tells me...), it's not listed on
> Wikipedia (hint, hint):
> http://en.wikipedia.org/wiki/Racing_wheel
> 
> At present lg4ff does not support RUMBLE effects (only CONSTANT and
> AUTOCENTER), so lg2ff seems a sensible place. The only minor concern I
> would have is any confusion from the descriptions in Kconfig.
> 
> I didn't build, but think that the patch looks OK to commit.
> 
> For the other wheels we have re-written the HID descriptors to split
> brake/accel into seperate axis. Do you know whether this is possible for
> this wheel too?

Elias noted earlier that it is configurable in Windows at least, so that
might be the case. We didn't look at the HID descriptors, though, but
feel free to work with him to see if it is possible :)

> Thanks for your patch,
> Simon.
> 
> Signed-off-by: Simon Wood <simon@mungewell.org>

-- 
Anssi Hannula

^ permalink raw reply

* Re: [PATCH] HID: logitech - lg2ff: Add IDs for Formula Vibration Feedback Wheel
From: simon @ 2013-10-07 20:03 UTC (permalink / raw)
  To: Elias Vanderstuyft
  Cc: Anssi Hannula, simon, Jiri Kosina, linux-usb, linux-input, stable,
	"Michal Malý"
In-Reply-To: <CADbOyBSry1eFsCycM-BaDEhaA7_+JUBpYZEqA4s5s-g2vq5whg@mail.gmail.com>

> I don't really know what you need for the HID descriptors, but I thought
> maybe I should post usbmon output when plugging in the wheel:

I'm not sure I can do anything with that file. It would be great if you
could dump/attach the report descriptor file. This can be found in the
/sys folder structure under the USB ID for your device.

ie. for my Sony Dual Shock controller
--
simon@simon-virtual-machine:/sys/bus/usb/devices/1-1:1.0/0003:054C:0268.0001$
hexdump -C report_descriptor
00000000  05 01 09 04 a1 01 a1 02  85 01 75 08 95 01 15 00 
|..........u.....|
00000010  26 ff 00 81 03 75 01 95  13 15 00 25 01 35 00 45 
|&....u.....%.5.E|
00000020  01 05 09 19 01 29 13 81  02 75 01 95 0d 06 00 ff 
|.....)...u......|
00000030  81 03 15 00 26 ff 00 05  01 09 01 a1 00 75 08 95 
|....&........u..|
00000040  04 35 00 46 ff 00 09 30  09 31 09 32 09 35 81 02 
|.5.F...0.1.2.5..|
00000050  c0 05 01 95 13 09 01 81  02 95 0c 81 01 75 10 95 
|.............u..|
00000060  04 26 ff 03 46 ff 03 09  01 81 02 c0 a1 02 85 02 
|.&..F...........|
00000070  75 08 95 30 09 01 b1 02  c0 a1 02 85 ee 75 08 95 
|u..0.........u..|
00000080  30 09 01 b1 02 c0 a1 02  85 ef 75 08 95 30 09 01 
|0.........u..0..|
00000090  b1 02 c0 c0                                       |....|
00000094
--

To confirm which bits of the HID stream actually change when the pedals
are pushed you could run the command as root.
--
$ hexdump -v -e '17/1 "%02x " "\n"' < /dev/hidraw0
--

You'll have to change the '17' to match the actual number of bytes your
wheel sends, when you get it right the columns of values will change
'nicely' as you steer/depress the pedals/etc.

I'm expecting to see one column (or two adjacent if the bits don't align)
change as you press the brake/etc. If you can send a small capture showing
values changing for each of brake and accel that should be enough
information.

When we know where the data is, we can tell Linux's HID system which bits
to interpret as Y and Z axis.

Cheers,
Simon

^ permalink raw reply

* [PATCH 01/02] input: serio: use DEVICE_ATTR_RO()
From: Greg Kroah-Hartman @ 2013-10-08  1:08 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input; +Cc: Greg Kroah-Hartman

From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Convert the serio sysfs fiels to use the DEVICE_ATTR_RO() macros to make
it easier to audit the correct sysfs file permission usage.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: <linux-input@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/input/serio/serio.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

--- a/drivers/input/serio/serio.c
+++ b/drivers/input/serio/serio.c
@@ -373,34 +373,34 @@ static ssize_t serio_show_modalias(struc
 			serio->id.type, serio->id.proto, serio->id.id, serio->id.extra);
 }
 
-static ssize_t serio_show_id_type(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t type_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct serio *serio = to_serio_port(dev);
 	return sprintf(buf, "%02x\n", serio->id.type);
 }
 
-static ssize_t serio_show_id_proto(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t proto_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct serio *serio = to_serio_port(dev);
 	return sprintf(buf, "%02x\n", serio->id.proto);
 }
 
-static ssize_t serio_show_id_id(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t id_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct serio *serio = to_serio_port(dev);
 	return sprintf(buf, "%02x\n", serio->id.id);
 }
 
-static ssize_t serio_show_id_extra(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t extra_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct serio *serio = to_serio_port(dev);
 	return sprintf(buf, "%02x\n", serio->id.extra);
 }
 
-static DEVICE_ATTR(type, S_IRUGO, serio_show_id_type, NULL);
-static DEVICE_ATTR(proto, S_IRUGO, serio_show_id_proto, NULL);
-static DEVICE_ATTR(id, S_IRUGO, serio_show_id_id, NULL);
-static DEVICE_ATTR(extra, S_IRUGO, serio_show_id_extra, NULL);
+static DEVICE_ATTR_RO(type);
+static DEVICE_ATTR_RO(proto);
+static DEVICE_ATTR_RO(id);
+static DEVICE_ATTR_RO(extra);
 
 static struct attribute *serio_device_id_attrs[] = {
 	&dev_attr_type.attr,

^ permalink raw reply

* [PATCH 02/02] input: serio: remove bus usage of dev_attrs
From: Greg Kroah-Hartman @ 2013-10-08  1:09 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input; +Cc: Greg Kroah-Hartman
In-Reply-To: <20131008010823.GA27012@kroah.com>

From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

The dev_attrs field of struct bus_type is going away soon, so move the
remaining sysfs files that are being described with this field to use
dev_groups instead.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: <linux-input@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/input/serio/serio.c |   62 +++++++++++++++++++++-----------------------
 1 file changed, 30 insertions(+), 32 deletions(-)

--- a/drivers/input/serio/serio.c
+++ b/drivers/input/serio/serio.c
@@ -365,7 +365,7 @@ static ssize_t serio_show_description(st
 	return sprintf(buf, "%s\n", serio->name);
 }
 
-static ssize_t serio_show_modalias(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t modalias_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct serio *serio = to_serio_port(dev);
 
@@ -397,30 +397,7 @@ static ssize_t extra_show(struct device
 	return sprintf(buf, "%02x\n", serio->id.extra);
 }
 
-static DEVICE_ATTR_RO(type);
-static DEVICE_ATTR_RO(proto);
-static DEVICE_ATTR_RO(id);
-static DEVICE_ATTR_RO(extra);
-
-static struct attribute *serio_device_id_attrs[] = {
-	&dev_attr_type.attr,
-	&dev_attr_proto.attr,
-	&dev_attr_id.attr,
-	&dev_attr_extra.attr,
-	NULL
-};
-
-static struct attribute_group serio_id_attr_group = {
-	.name	= "id",
-	.attrs	= serio_device_id_attrs,
-};
-
-static const struct attribute_group *serio_device_attr_groups[] = {
-	&serio_id_attr_group,
-	NULL
-};
-
-static ssize_t serio_rebind_driver(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
+static ssize_t drvctl_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
 {
 	struct serio *serio = to_serio_port(dev);
 	struct device_driver *drv;
@@ -474,14 +451,36 @@ static ssize_t serio_set_bind_mode(struc
 	return retval;
 }
 
-static struct device_attribute serio_device_attrs[] = {
-	__ATTR(description, S_IRUGO, serio_show_description, NULL),
-	__ATTR(modalias, S_IRUGO, serio_show_modalias, NULL),
-	__ATTR(drvctl, S_IWUSR, NULL, serio_rebind_driver),
-	__ATTR(bind_mode, S_IWUSR | S_IRUGO, serio_show_bind_mode, serio_set_bind_mode),
-	__ATTR_NULL
+static DEVICE_ATTR_RO(type);
+static DEVICE_ATTR_RO(proto);
+static DEVICE_ATTR_RO(id);
+static DEVICE_ATTR_RO(extra);
+static DEVICE_ATTR_RO(modalias);
+static DEVICE_ATTR_WO(drvctl);
+static DEVICE_ATTR(description, S_IRUGO, serio_show_description, NULL);
+static DEVICE_ATTR(bind_mode, S_IWUSR | S_IRUGO, serio_show_bind_mode, serio_set_bind_mode);
+
+static struct attribute *serio_device_id_attrs[] = {
+	&dev_attr_type.attr,
+	&dev_attr_proto.attr,
+	&dev_attr_id.attr,
+	&dev_attr_extra.attr,
+	&dev_attr_modalias.attr,
+	&dev_attr_description.attr,
+	&dev_attr_drvctl.attr,
+	&dev_attr_bind_mode.attr,
+	NULL
+};
+
+static struct attribute_group serio_id_attr_group = {
+	.name	= "id",
+	.attrs	= serio_device_id_attrs,
 };
 
+static const struct attribute_group *serio_device_attr_groups[] = {
+	&serio_id_attr_group,
+	NULL
+};
 
 static void serio_release_port(struct device *dev)
 {
@@ -996,7 +995,6 @@ EXPORT_SYMBOL(serio_interrupt);
 
 static struct bus_type serio_bus = {
 	.name		= "serio",
-	.dev_attrs	= serio_device_attrs,
 	.drv_groups	= serio_driver_groups,
 	.match		= serio_bus_match,
 	.uevent		= serio_uevent,

^ permalink raw reply

* [PATCH 3/8] input: gameport: convert bus code to use dev_groups
From: Greg Kroah-Hartman @ 2013-10-08  1:27 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman, Dmitry Torokhov, linux-input
In-Reply-To: <1381195662-28009-1-git-send-email-gregkh@linuxfoundation.org>

The dev_attrs field of struct bus_type is going away soon, dev_groups
should be used instead.  This converts the gameport bus code to use the
correct field.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: <linux-input@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---

Dmitry, I can take this through my driver-core tree if you don't want to
take it through yours, just let me know what works best for you.

 drivers/input/gameport/gameport.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/input/gameport/gameport.c b/drivers/input/gameport/gameport.c
index 922a7fe..24c41ba 100644
--- a/drivers/input/gameport/gameport.c
+++ b/drivers/input/gameport/gameport.c
@@ -422,14 +422,15 @@ static struct gameport *gameport_get_pending_child(struct gameport *parent)
  * Gameport port operations
  */
 
-static ssize_t gameport_show_description(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t gameport_description_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct gameport *gameport = to_gameport_port(dev);
 
 	return sprintf(buf, "%s\n", gameport->name);
 }
+static DEVICE_ATTR(description, S_IRUGO, gameport_description_show, NULL);
 
-static ssize_t gameport_rebind_driver(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
+static ssize_t drvctl_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
 {
 	struct gameport *gameport = to_gameport_port(dev);
 	struct device_driver *drv;
@@ -457,12 +458,14 @@ static ssize_t gameport_rebind_driver(struct device *dev, struct device_attribut
 
 	return error ? error : count;
 }
+static DEVICE_ATTR_WO(drvctl);
 
-static struct device_attribute gameport_device_attrs[] = {
-	__ATTR(description, S_IRUGO, gameport_show_description, NULL),
-	__ATTR(drvctl, S_IWUSR, NULL, gameport_rebind_driver),
-	__ATTR_NULL
+static struct attribute *gameport_device_attrs[] = {
+	&dev_attr_description.attr,
+	&dev_attr_drvctl.attr,
+	NULL,
 };
+ATTRIBUTE_GROUPS(gameport_device);
 
 static void gameport_release_port(struct device *dev)
 {
@@ -750,7 +753,7 @@ static int gameport_bus_match(struct device *dev, struct device_driver *drv)
 
 static struct bus_type gameport_bus = {
 	.name		= "gameport",
-	.dev_attrs	= gameport_device_attrs,
+	.dev_groups	= gameport_device_groups,
 	.drv_groups	= gameport_driver_groups,
 	.match		= gameport_bus_match,
 	.probe		= gameport_driver_probe,
-- 
1.8.4.6.g82e253f.dirty


^ permalink raw reply related

* Re: [PATCH] HID: logitech - lg2ff: Add IDs for Formula Vibration Feedback Wheel
From: Michal Malý @ 2013-10-08 10:00 UTC (permalink / raw)
  To: simon-wM4F9T/ekXmXDw4h08c5KA
  Cc: Elias Vanderstuyft, Anssi Hannula, Jiri Kosina,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA, stable-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <fa1af5a9ba814114ddf5ee8fe65b8b4b.squirrel-uf5OtEnyChf6gvvhDbwh+ti2O/JbrIOy@public.gmane.org>

Hey guys,

I assume that we're talking about this wheel (http://www.logitech.com/en-roeu/support/298)?

Does the wheel have an actual FFB servo or does it have just a rumble motor that shakes the wheel? If it's the latter, supporting it in lg2ff is a way to go. Description in KConfig should probably be updated to reflect this.

As for the HID descriptors, Simon's suggestion will work just fine. Alternatively you can use Wireshark which can use usbmon to capture USB traffic. (Wireshark must run as root and usbmon module must be loaded before Wireshark starts). We can hack the descriptor in hid-logitech just like we did it for DFP. If we come across more wheels that need a hacked descriptor, perhaps we should consider handling the raw data ourselves (check out the Wii remote driver for details)...

Michal.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH] INPUT/MISC: Fix input/misc PWM related undefined reference errors.
From: Majunath Goudar @ 2013-10-08 10:37 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: nataraja.km, Majunath Goudar, Dmitry Torokhov, linux-input,
	linux-kernel

This patch adds a PWM dependency to configure the INPUT_MAX8997_HAPTIC
and INPUT_PWM_BEEPER. Without this patch, build system can lead to build
failure. This was observed during randconfig testing, in which
INPUT_MAX8997_HAPTIC or INPUT_PWM_BEEPER was enabled w/o PWM being
enabled. leading to the following error:

init/built-in.o
drivers/built-in.o: In function `max8997_haptic_disable':
:(.text+0x1073a0): undefined reference to `pwm_disable'
drivers/built-in.o: In function `max8997_haptic_remove':
:(.text+0x1073fc): undefined reference to `pwm_free'
drivers/built-in.o: In function `max8997_haptic_play_effect_work':
:(.text+0x1074f0): undefined reference to `pwm_config'
:(.text+0x1075b0): undefined reference to `pwm_enable'
drivers/built-in.o: In function `max8997_haptic_probe':
:(.text+0x1076f8): undefined reference to `pwm_request'
:(.text+0x1077e8): undefined reference to `pwm_free'
drivers/built-in.o: In function `pwm_beeper_resume':
:(.text+0x1081a4): undefined reference to `pwm_config'
make: *** [vmlinux] Error 1

Signed-off-by: Manjunath Goudar <csmanjuvijay@gmail.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/input/misc/Kconfig |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index aa51baa..265240b 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -156,7 +156,7 @@ config INPUT_MAX8925_ONKEY
 
 config INPUT_MAX8997_HAPTIC
 	tristate "MAXIM MAX8997 haptic controller support"
-	depends on HAVE_PWM && MFD_MAX8997
+	depends on HAVE_PWM && MFD_MAX8997 && PWM
 	select INPUT_FF_MEMLESS
 	help
 	  This option enables device driver support for the haptic controller
@@ -461,7 +461,7 @@ config INPUT_PCF8574
 
 config INPUT_PWM_BEEPER
 	tristate "PWM beeper support"
-	depends on HAVE_PWM || PWM
+	depends on HAVE_PWM && PWM
 	help
 	  Say Y here to get support for PWM based beeper devices.
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH] SOUND/SPI: Fix "clk_round_rate" undefined reference error.
From: Majunath Goudar @ 2013-10-08 10:40 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: nataraja.km, Majunath Goudar, Dmitry Torokhov, linux-kernel,
	linux-input

This patch adds a COMMON_CLK dependency to configure the
SND_AT73C213 for Atmel AT73C213 DAC driver. Without this patch,
build system can lead to build failure. This was observed during
randconfig testing, in which SND_AT73C213 was enabled w/o COMMON_CLK
being enabled. leading to the following error:

sound/built-in.o: In function `snd_at73c213_probe':
:(.text+0x328c4): undefined reference to `clk_round_rate'
make: *** [vmlinux] Error 1

Signed-off-by: Manjunath Goudar <csmanjuvijay@gmail.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 sound/spi/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/spi/Kconfig b/sound/spi/Kconfig
index e6485be..798d1ee 100644
--- a/sound/spi/Kconfig
+++ b/sound/spi/Kconfig
@@ -11,7 +11,7 @@ if SND_SPI
 
 config SND_AT73C213
 	tristate "Atmel AT73C213 DAC driver"
-	depends on ATMEL_SSC
+	depends on ATMEL_SSC && COMMON_CLK
 	select SND_PCM
 	help
 	  Say Y here if you want to use the Atmel AT73C213 external DAC. This
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH] input: touchscreen: add driver for CJTouch touchscreen
From: linuxsea521 @ 2013-10-08 14:21 UTC (permalink / raw)
  To: linux-input; +Cc: linuxsea

From: linuxsea <linuxsea@163.com>

Signed-off-by: linuxsea <linuxsea@163.com>
---
 drivers/input/touchscreen/Kconfig          |    5 ++
 drivers/input/touchscreen/usbtouchscreen.c |   99 ++++++++++++++++++++++++++++
 2 files changed, 104 insertions(+)

diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index e09ec67..2640195 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -812,6 +812,11 @@ config TOUCHSCREEN_USB_NEXIO
 	bool "NEXIO/iNexio device support" if EXPERT
 	depends on TOUCHSCREEN_USB_COMPOSITE
 
+config TOUCHSCREEN_USB_CJTOUCH
+	default y
+	bool "CJTouch Touchscreen device support" if EXPERT
+	depends on TOUCHSCREEN_USB_COMPOSITE
+
 config TOUCHSCREEN_USB_EASYTOUCH
 	default y
 	bool "EasyTouch USB Touch controller device support" if EMBEDDED
diff --git a/drivers/input/touchscreen/usbtouchscreen.c b/drivers/input/touchscreen/usbtouchscreen.c
index 721fdb3..cd3aa8c 100644
--- a/drivers/input/touchscreen/usbtouchscreen.c
+++ b/drivers/input/touchscreen/usbtouchscreen.c
@@ -18,6 +18,7 @@
  *  - NEXIO/iNexio
  *  - Elo TouchSystems 2700 IntelliTouch
  *  - EasyTouch USB Dual/Multi touch controller from Data Modul
+ *  - CJTouch
  *
  * Copyright (C) 2004-2007 by Daniel Ritz <daniel.ritz@gmx.ch>
  * Copyright (C) by Todd E. Johnson (mtouchusb.c)
@@ -142,6 +143,8 @@ enum {
 	DEVTYPE_NEXIO,
 	DEVTYPE_ELO,
 	DEVTYPE_ETOUCH,
+	DEVTYPE_CJTOUCH,
+	DEVTYPE_CJMTOUCH,
 };
 
 #define USB_DEVICE_HID_CLASS(vend, prod) \
@@ -251,6 +254,11 @@ static const struct usb_device_id usbtouch_devices[] = {
 	{USB_DEVICE(0x7374, 0x0001), .driver_info = DEVTYPE_ETOUCH},
 #endif
 
+#ifdef CONFIG_TOUCHSCREEN_USB_CJTOUCH
+	{USB_DEVICE(0x24b8, 0x0001), .driver_info = DEVTYPE_CJTOUCH},
+	{USB_DEVICE(0x24b8, 0x0004), .driver_info = DEVTYPE_CJTOUCH},
+	{USB_DEVICE(0x24b8, 0x000f), .driver_info = DEVTYPE_CJMTOUCH},
+#endif
 	{}
 };
 
@@ -1065,6 +1073,78 @@ static int elo_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
 
 
 /*****************************************************************************
+ * CJTouch part
+ */
+
+#ifdef CONFIG_TOUCHSCREEN_USB_CJTOUCH
+static int x = 1;
+static int y = 1;
+static int old2State = 1;
+
+
+static int cjtouch_touch_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
+{
+	dev->x = ((pkt[2] & 0x00FF) << 8) | (pkt[1] & 0x00FF);
+	dev->y = ((pkt[4] & 0x00FF) << 8) | (pkt[3] & 0x00FF);
+	dev->press = pkt[0];
+	dev->touch = pkt[0];
+
+	return 1;
+}
+
+static void cjmtouch_touch_process_pkt(struct usbtouch_usb *usbtouch,
+				   unsigned char *pkt, int len)
+{
+	int touch1 = pkt[1] & 0x01;
+	int touch2 = pkt[7] & 0x01;
+
+	if (touch1)
+		input_report_abs(usbtouch->input, ABS_MT_TOUCH_MAJOR, 1);
+	else
+		input_report_abs(usbtouch->input, ABS_MT_TOUCH_MAJOR, 0);
+
+	{
+		x = ((pkt[4] << 8) | pkt[3]);
+		y = ((pkt[6] << 8) | pkt[5]);
+
+		input_report_abs(usbtouch->input, ABS_MT_WIDTH_MAJOR, 1);
+		input_report_abs(usbtouch->input, ABS_MT_POSITION_X, x);
+		input_report_abs(usbtouch->input, ABS_MT_POSITION_Y, y);
+
+		input_mt_sync(usbtouch->input);
+	}
+
+	if (touch2) {
+		old2State = 1;
+		input_report_abs(usbtouch->input, ABS_MT_TOUCH_MAJOR, 1);
+
+		input_report_abs(usbtouch->input, ABS_MT_WIDTH_MAJOR, 1);
+		input_report_abs(usbtouch->input, ABS_MT_POSITION_X, x);
+		input_report_abs(usbtouch->input, ABS_MT_POSITION_Y, y);
+		
+		input_mt_sync(usbtouch->input);
+	} else {
+		if (old2State == 1) {
+			old2State = 0;
+			input_report_abs(usbtouch->input, ABS_MT_TOUCH_MAJOR, 0);
+
+			x = (x * 800) / 0xfff;
+			y = (y * 600) / 0xfff;
+
+			input_report_abs(usbtouch->input, ABS_MT_WIDTH_MAJOR, 1);
+			input_report_abs(usbtouch->input, ABS_MT_POSITION_X, x);
+			input_report_abs(usbtouch->input, ABS_MT_POSITION_Y, y);
+
+			input_mt_sync(usbtouch->input);
+		}
+	}
+
+	input_sync(usbtouch->input);
+
+}
+#endif
+
+/*****************************************************************************
  * the different device descriptors
  */
 #ifdef MULTI_PACKET
@@ -1284,6 +1364,25 @@ static struct usbtouch_device_info usbtouch_dev_info[] = {
 		.read_data	= etouch_read_data,
 	},
 #endif
+#ifdef CONFIG_TOUCHSCREEN_USB_CJTOUCH
+	[DEVTYPE_CJTOUCH] = {
+		.min_xc	= 0x0,
+		.max_xc = 0x0fff,
+		.min_yc = 0x0,
+		.max_yc = 0x0fff,
+		.rept_size = 7,
+		.read_data = cjtouch_touch_read_data,
+	},
+
+	[DEVTYPE_CJMTOUCH] = {
+		.min_xc	= 0x0,
+		.max_xc = 0x0fff,
+		.min_yc = 0x0,
+		.max_yc = 0x0fff,
+		.rept_size = 14,
+		.process_pkt = cjmtouch_touch_process_pkt,
+	},
+#endif
 };
 
 
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH] HID: remove self-assignment from hid_input_report
From: Felix Rueegg @ 2013-10-08 17:33 UTC (permalink / raw)
  To: jkosina; +Cc: linux-input, linux-kernel, Felix Rueegg

The ternary expression will always result in a self-assignment, which is pointless.

Signed-off-by: Felix Rueegg <felix.rueegg@gmail.com>
---
 drivers/hid/hid-core.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 5a8c011..ae0c963 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1417,10 +1417,8 @@ int hid_input_report(struct hid_device *hid, int type, u8 *data, int size, int i
 
 	if (hdrv && hdrv->raw_event && hid_match_report(hid, report)) {
 		ret = hdrv->raw_event(hid, report, data, size);
-		if (ret < 0) {
-			ret = ret < 0 ? ret : 0;
+		if (ret < 0)
 			goto unlock;
-		}
 	}
 
 	ret = hid_report_raw_event(hid, type, data, size, interrupt);
-- 
1.8.4


^ permalink raw reply related

* Re: [PATCH] HID: logitech - lg2ff: Add IDs for Formula Vibration Feedback Wheel
From: simon @ 2013-10-08 18:24 UTC (permalink / raw)
  To: "Michal Malý"
  Cc: simon, Elias Vanderstuyft, Anssi Hannula, Jiri Kosina, linux-usb,
	linux-input, stable
In-Reply-To: <4607275.ItDsRbK9eM@geidi-prime>

> We can hack the descriptor in hid-logitech just
> like we did it for DFP. If we come across more wheels that need a hacked
> descriptor, perhaps we should consider handling the raw data ourselves
> (check out the Wii remote driver for details)...

We currently 'hack HID desc.' for the DF, DFP and MOMO-Red. The DFGT, G25
and G27 are pushed into native mode and automatically split the brake and
accel axis.

I'll get the 'hack HID desc.' done for Elias' wheel and the MOMO-black I
picked last week. Should be done this week sometime.

I think we've got every wheel variant covered.... unless the release more
with different IDs (recently saw a new DFGT version).

In the longer term we may wish to add an option/control to split (or not)
the brake/accel as the user requests. I don't actually think that anyone
wants combined axis, but there's no telling with some people ;-)

Simon


^ permalink raw reply

* [PATCH 1/1] Revert "HID: fix unit exponent parsing"
From: Nikolai Kondrashov @ 2013-10-08 18:59 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: linux-input, Benjamin Tissoires, Nikolai Kondrashov

From: Nikolai Kondrashov <Nikolai.Kondrashov@redhat.com>

Revert "HID: fix unit exponent parsing" as it is based on incorrect
understanding of the HID specification and breaks resolution
calculation at least on graphics tablets.

There are two "unit exponents" in HID specification and it is important
not to mix them. One is the global "Unit Exponent" item and another is
nibble values in the global "Unit" item. See 6.2.2.7 Global Items.

The "Unit Exponent" value is just a signed integer and is used to scale
the integer resolution unit values, so fractions can be expressed.

The nibbles of "Unit" value are used to select the unit system (nibble
0), and presence of a particular basic unit type in the unit formula and
its *exponent* (or power, nibbles 1-6). And yes, the latter is in two
complement and zero means absence of the unit type.

Taking the representation example of (integer) joules from the
specification:

    [mass(grams)][length(centimeters)^2][time(seconds)^-2] * 10^-7

the "Unit Exponent" would be -7 (or 0xF9, if stored as a byte) and the
"Unit" value would be 0xE121, signifying:

    Nibble  Part        Value   Meaning
    -----   ----        -----   -------
    0       System      1       SI Linear
    1       Length      2       Centimeters^2
    2       Mass        1       Grams
    3       Time        -2      Seconds^-2

To give the resolution in e.g. hundredth of joules the "Unit Exponent"
item value should have been -9.

See also the examples of "Unit" values for some common units in the same
chapter.

This reverts commit 774638386826621c984ab6994439f474709cac5e.
---

Jiri, I've had the HID specification thoroughly read WRT report descriptors
when implementing hidrd [1] and would be happy to help reviewing any patches
touching that area. Please CC me if there's any need.

[1] https://sf.net/apps/mediawiki/digimend?title=Hidrd

 drivers/hid/hid-core.c  | 16 +---------------
 drivers/hid/hid-input.c | 13 ++++---------
 include/linux/hid.h     |  1 -
 3 files changed, 5 insertions(+), 25 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index b8470b1..94350b7 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -319,7 +319,6 @@ static s32 item_sdata(struct hid_item *item)
 
 static int hid_parser_global(struct hid_parser *parser, struct hid_item *item)
 {
-	__u32 raw_value;
 	switch (item->tag) {
 	case HID_GLOBAL_ITEM_TAG_PUSH:
 
@@ -370,14 +369,7 @@ static int hid_parser_global(struct hid_parser *parser, struct hid_item *item)
 		return 0;
 
 	case HID_GLOBAL_ITEM_TAG_UNIT_EXPONENT:
-		/* Units exponent negative numbers are given through a
-		 * two's complement.
-		 * See "6.2.2.7 Global Items" for more information. */
-		raw_value = item_udata(item);
-		if (!(raw_value & 0xfffffff0))
-			parser->global.unit_exponent = hid_snto32(raw_value, 4);
-		else
-			parser->global.unit_exponent = raw_value;
+		parser->global.unit_exponent = item_sdata(item);
 		return 0;
 
 	case HID_GLOBAL_ITEM_TAG_UNIT:
@@ -983,12 +975,6 @@ static s32 snto32(__u32 value, unsigned n)
 	return value & (1 << (n - 1)) ? value | (-1 << n) : value;
 }
 
-s32 hid_snto32(__u32 value, unsigned n)
-{
-	return snto32(value, n);
-}
-EXPORT_SYMBOL_GPL(hid_snto32);
-
 /*
  * Convert a signed 32-bit integer to a signed n-bit integer.
  */
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 8741d95..d97f232 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -192,6 +192,7 @@ static int hidinput_setkeycode(struct input_dev *dev,
 	return -EINVAL;
 }
 
+
 /**
  * hidinput_calc_abs_res - calculate an absolute axis resolution
  * @field: the HID report field to calculate resolution for
@@ -234,23 +235,17 @@ __s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code)
 	case ABS_MT_TOOL_Y:
 	case ABS_MT_TOUCH_MAJOR:
 	case ABS_MT_TOUCH_MINOR:
-		if (field->unit & 0xffffff00)		/* Not a length */
-			return 0;
-		unit_exponent += hid_snto32(field->unit >> 4, 4) - 1;
-		switch (field->unit & 0xf) {
-		case 0x1:				/* If centimeters */
+		if (field->unit == 0x11) {		/* If centimeters */
 			/* Convert to millimeters */
 			unit_exponent += 1;
-			break;
-		case 0x3:				/* If inches */
+		} else if (field->unit == 0x13) {	/* If inches */
 			/* Convert to millimeters */
 			prev = physical_extents;
 			physical_extents *= 254;
 			if (physical_extents < prev)
 				return 0;
 			unit_exponent -= 1;
-			break;
-		default:
+		} else {
 			return 0;
 		}
 		break;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 31b9d29..96f3432 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -766,7 +766,6 @@ int hid_connect(struct hid_device *hid, unsigned int connect_mask);
 void hid_disconnect(struct hid_device *hid);
 const struct hid_device_id *hid_match_id(struct hid_device *hdev,
 					 const struct hid_device_id *id);
-s32 hid_snto32(__u32 value, unsigned n);
 
 /**
  * hid_device_io_start - enable HID input during probe, remove
-- 
1.8.4.rc3


^ permalink raw reply related

* [PATCH 1/1 FROM FIXED] Revert "HID: fix unit exponent parsing"
From: Nikolai Kondrashov @ 2013-10-08 19:05 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: linux-input, Benjamin Tissoires, Nikolai Kondrashov
In-Reply-To: <1381258791-24966-1-git-send-email-spbnick@gmail.com>

Revert "HID: fix unit exponent parsing" as it is based on incorrect
understanding of the HID specification and breaks resolution
calculation at least on graphics tablets.

There are two "unit exponents" in HID specification and it is important
not to mix them. One is the global "Unit Exponent" item and another is
nibble values in the global "Unit" item. See 6.2.2.7 Global Items.

The "Unit Exponent" value is just a signed integer and is used to scale
the integer resolution unit values, so fractions can be expressed.

The nibbles of "Unit" value are used to select the unit system (nibble
0), and presence of a particular basic unit type in the unit formula and
its *exponent* (or power, nibbles 1-6). And yes, the latter is in two
complement and zero means absence of the unit type.

Taking the representation example of (integer) joules from the
specification:

    [mass(grams)][length(centimeters)^2][time(seconds)^-2] * 10^-7

the "Unit Exponent" would be -7 (or 0xF9, if stored as a byte) and the
"Unit" value would be 0xE121, signifying:

    Nibble  Part        Value   Meaning
    -----   ----        -----   -------
    0       System      1       SI Linear
    1       Length      2       Centimeters^2
    2       Mass        1       Grams
    3       Time        -2      Seconds^-2

To give the resolution in e.g. hundredth of joules the "Unit Exponent"
item value should have been -9.

See also the examples of "Unit" values for some common units in the same
chapter.

This reverts commit 774638386826621c984ab6994439f474709cac5e.
---
This patch has correct e-mail in "From".

 drivers/hid/hid-core.c  | 16 +---------------
 drivers/hid/hid-input.c | 13 ++++---------
 include/linux/hid.h     |  1 -
 3 files changed, 5 insertions(+), 25 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index b8470b1..94350b7 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -319,7 +319,6 @@ static s32 item_sdata(struct hid_item *item)
 
 static int hid_parser_global(struct hid_parser *parser, struct hid_item *item)
 {
-	__u32 raw_value;
 	switch (item->tag) {
 	case HID_GLOBAL_ITEM_TAG_PUSH:
 
@@ -370,14 +369,7 @@ static int hid_parser_global(struct hid_parser *parser, struct hid_item *item)
 		return 0;
 
 	case HID_GLOBAL_ITEM_TAG_UNIT_EXPONENT:
-		/* Units exponent negative numbers are given through a
-		 * two's complement.
-		 * See "6.2.2.7 Global Items" for more information. */
-		raw_value = item_udata(item);
-		if (!(raw_value & 0xfffffff0))
-			parser->global.unit_exponent = hid_snto32(raw_value, 4);
-		else
-			parser->global.unit_exponent = raw_value;
+		parser->global.unit_exponent = item_sdata(item);
 		return 0;
 
 	case HID_GLOBAL_ITEM_TAG_UNIT:
@@ -983,12 +975,6 @@ static s32 snto32(__u32 value, unsigned n)
 	return value & (1 << (n - 1)) ? value | (-1 << n) : value;
 }
 
-s32 hid_snto32(__u32 value, unsigned n)
-{
-	return snto32(value, n);
-}
-EXPORT_SYMBOL_GPL(hid_snto32);
-
 /*
  * Convert a signed 32-bit integer to a signed n-bit integer.
  */
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 8741d95..d97f232 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -192,6 +192,7 @@ static int hidinput_setkeycode(struct input_dev *dev,
 	return -EINVAL;
 }
 
+
 /**
  * hidinput_calc_abs_res - calculate an absolute axis resolution
  * @field: the HID report field to calculate resolution for
@@ -234,23 +235,17 @@ __s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code)
 	case ABS_MT_TOOL_Y:
 	case ABS_MT_TOUCH_MAJOR:
 	case ABS_MT_TOUCH_MINOR:
-		if (field->unit & 0xffffff00)		/* Not a length */
-			return 0;
-		unit_exponent += hid_snto32(field->unit >> 4, 4) - 1;
-		switch (field->unit & 0xf) {
-		case 0x1:				/* If centimeters */
+		if (field->unit == 0x11) {		/* If centimeters */
 			/* Convert to millimeters */
 			unit_exponent += 1;
-			break;
-		case 0x3:				/* If inches */
+		} else if (field->unit == 0x13) {	/* If inches */
 			/* Convert to millimeters */
 			prev = physical_extents;
 			physical_extents *= 254;
 			if (physical_extents < prev)
 				return 0;
 			unit_exponent -= 1;
-			break;
-		default:
+		} else {
 			return 0;
 		}
 		break;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 31b9d29..96f3432 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -766,7 +766,6 @@ int hid_connect(struct hid_device *hid, unsigned int connect_mask);
 void hid_disconnect(struct hid_device *hid);
 const struct hid_device_id *hid_match_id(struct hid_device *hdev,
 					 const struct hid_device_id *id);
-s32 hid_snto32(__u32 value, unsigned n);
 
 /**
  * hid_device_io_start - enable HID input during probe, remove
-- 
1.8.4.rc3


^ permalink raw reply related

* Re: [PATCH 1/1 FROM FIXED] Revert "HID: fix unit exponent parsing"
From: Nikolai Kondrashov @ 2013-10-09  7:37 UTC (permalink / raw)
  To: Nikolai Kondrashov; +Cc: Jiri Kosina, linux-input, Benjamin Tissoires
In-Reply-To: <1381259117-25256-1-git-send-email-spbnick@gmail.com>

On 10/08/2013 10:05 PM, Nikolai Kondrashov wrote:
> Revert "HID: fix unit exponent parsing" as it is based on incorrect
> understanding of the HID specification and breaks resolution
> calculation at least on graphics tablets.
>
> There are two "unit exponents" in HID specification and it is important
> not to mix them. One is the global "Unit Exponent" item and another is
> nibble values in the global "Unit" item. See 6.2.2.7 Global Items.
>
> The "Unit Exponent" value is just a signed integer and is used to scale
> the integer resolution unit values, so fractions can be expressed.

I see now that the official "HID Descriptor Tool" writes the "Unit Exponent"
as a two's complement nibble and Microsoft cites binary representations as
such (although they seem to be made with that tool).

This likely means most report descriptors in the wild store it as such. Which
probably means that it is more practical to expect them to be so, but I don't
have any statistically-significant data.

However, I still think that this interpretation of the standard is incorrect
(the standard is vague in this part). First, the specification says about
"Unit Exponent":

     Value of the unit exponent in base 10. See the
     table later in this section for more information.

I.e. nothing about nibbles. It refers to a table, but doesn't say which one.
However, the first table appearing later is a mouse example mentioning an
exponent.

The specification also mentions the one-byte prefix for the item to be
"0101 01 nn", where "nn" is the size value, which would be pointless to have
anything different from 1 byte, if it was limited to a nibble.

And lastly and most importantly there is no point in limiting the "Unit
Exponent" to a nibble where you have a full 32-bit signed integer available,
thus complicating encoding/decoding and limiting the possible reported unit
scale.

 From what data I have on various non-Wacom graphics tablets, most of the older
ones provide incorrect "unit" item value, so "unit exponent" doesn't apply.
However, some recent ones, such as made by Huion, do have correct units,
and nibble-stored "unit exponent". And I was wondering where that strange data
came from. Sigh...

I'll try to reach usb.org for comments on this.

Meanwhile, can I suggest a hybrid approach? As positive unit exponent beyond 7
is unlikely to appear for axes which have their resolution calculated
currently, can we assume anything higher than 7 to be nibble-stored negative
exponents and anything else normal signed integers?

Sincerely,
Nick

^ permalink raw reply

* Unit Exponent value storage
From: Nikolai Kondrashov @ 2013-10-09  7:53 UTC (permalink / raw)
  To: hidcomments; +Cc: linux-input

Hello,

Could you please clarify how the "Unit Exponent" global item value should be
encoded in report descriptors, according to Device Class Definition for HID
1.11?

In my understanding it is encoded similarly to the rest of the signed integer
items, i.e. two's complement 1, 2, or 4 byte integer.

However, the "HID Descriptor Tool" available on the HID page [1] encodes it as
a two's complement nibble, limited to [-8, 7] range accordingly.

Which encoding is meant by the specification?

I'm CC'ing this message to the "linux-input" maillist where I'm trying to
raise discussion about how "Unit Exponent" value should be interpreted for the
purposes of resolution calculation for HID devices in the Linux kernel.
Could you please keep this CC in your answer, so others get posted?

Thank you very much.

Sincerely,
Nikolai Kondrashov

^ permalink raw reply

* Re: [PATCH 1/1 FROM FIXED] Revert "HID: fix unit exponent parsing"
From: Benjamin Tissoires @ 2013-10-09  9:02 UTC (permalink / raw)
  To: Nikolai Kondrashov; +Cc: Jiri Kosina, linux-input
In-Reply-To: <525507AC.5050807@gmail.com>

Hi Nikolai,

first, I wouldn't revert the patch as this, because I think the patch
also fixes the nibbles parsing in unit:
- the part regarding the unit exponent seems blurry
- the part regarding the unit exponent seems correct to me.

On Wed, Oct 9, 2013 at 9:37 AM, Nikolai Kondrashov <spbnick@gmail.com> wrote:
> On 10/08/2013 10:05 PM, Nikolai Kondrashov wrote:
>>
>> Revert "HID: fix unit exponent parsing" as it is based on incorrect
>> understanding of the HID specification and breaks resolution
>> calculation at least on graphics tablets.

I have re-read the HID spec, and indeed, it is not clear at all.
However, as you mentioned later in this mail, I based this assumption
by looking at the report descriptors I have in hand for hid-multitouch
(which is still quite a lot), and it looked correct this way.

>>
>> There are two "unit exponents" in HID specification and it is important
>> not to mix them. One is the global "Unit Exponent" item and another is
>> nibble values in the global "Unit" item. See 6.2.2.7 Global Items.
>>
>> The "Unit Exponent" value is just a signed integer and is used to scale
>> the integer resolution unit values, so fractions can be expressed.
>
>
> I see now that the official "HID Descriptor Tool" writes the "Unit Exponent"
> as a two's complement nibble and Microsoft cites binary representations as
> such (although they seem to be made with that tool).
>
> This likely means most report descriptors in the wild store it as such.
> Which
> probably means that it is more practical to expect them to be so, but I
> don't
> have any statistically-significant data.
>
> However, I still think that this interpretation of the standard is incorrect
> (the standard is vague in this part). First, the specification says about
> "Unit Exponent":
>
>     Value of the unit exponent in base 10. See the
>     table later in this section for more information.
>
> I.e. nothing about nibbles. It refers to a table, but doesn't say which one.
> However, the first table appearing later is a mouse example mentioning an
> exponent.

>From what I read, the unit exponent has nothing to do with nibbles.
Only "Unit" has them.
However, my experience says that the unit exponent is often used as a
two's complement :(

>
> The specification also mentions the one-byte prefix for the item to be
> "0101 01 nn", where "nn" is the size value, which would be pointless to have
> anything different from 1 byte, if it was limited to a nibble.

It could be, as it allows the firmware maker to avoid to add padding
bits... Which seems a convenient way to do it.

>
> And lastly and most importantly there is no point in limiting the "Unit
> Exponent" to a nibble where you have a full 32-bit signed integer available,
> thus complicating encoding/decoding and limiting the possible reported unit
> scale.

sure.

>
> From what data I have on various non-Wacom graphics tablets, most of the
> older
> ones provide incorrect "unit" item value, so "unit exponent" doesn't apply.

Yes. The "correct" specification of unit and unit exponent has only be
a requirements since Windows 8 [1]. (Note, there used to be a pdf [2],
which I found more convenient to read, but still...).

So definitely, Microsoft considers that the unit exponent is a 4 bits
two's complement, otherwise, the firmware will not get a Windows 8
certification.

> However, some recent ones, such as made by Huion, do have correct units,
> and nibble-stored "unit exponent". And I was wondering where that strange
> data
> came from. Sigh...

See above... Microsoft certification.

>
> I'll try to reach usb.org for comments on this.
>
> Meanwhile, can I suggest a hybrid approach? As positive unit exponent beyond
> 7
> is unlikely to appear for axes which have their resolution calculated
> currently, can we assume anything higher than 7 to be nibble-stored negative
> exponents and anything else normal signed integers?

I would say that the current approach (without the revert) is exactly this:
- if the data is stored on only 1 byte ( if (!(raw_value &
0xfffffff0))), do the two's complement -> any value less than 7 will
be the same, above are considered as negative.
- if not, then use the raw value.

Could you please share some of the report descriptors which you found
problematic, so that I can have a better understanding of the problem?
If you want to have a look at some multitouch descriptors (and few
other devices), I started to build a database of hid devices[3].

Cheers,
Benjamin

[1] http://msdn.microsoft.com/en-us/library/windows/hardware/dn383621.aspx
[2] http://feishare.com/attachments/article/299/windows-pointer-device-protocol.pdf
[3] https://github.com/bentiss/hid-devices

PS: I'll be mostly offline until the 1st of November when I'll finish
my transfer to the RH Westford Office.
PPS: my nick on the RH internal IRC is bentiss :)

^ permalink raw reply

* Re: [PATCH] HID: logitech - lg2ff: Add IDs for Formula Vibration Feedback Wheel
From: Jiri Kosina @ 2013-10-09 10:09 UTC (permalink / raw)
  To: Simon Wood
  Cc: Anssi Hannula, linux-usb, linux-input, Elias Vanderstuyft, stable,
	"Michal Malý"
In-Reply-To: <cec1e0ca61cf17ceb6e52faa4f2baad9.squirrel@mungewell.org>

On Mon, 7 Oct 2013, simon@mungewell.org wrote:

> > The lg2ff force feedback subdriver is used for vibration and
> > HID_GD_MULTIAXIS is set to avoid deadzone like other Logitech wheels.
> >
> > Kconfig description etc are also updated accordingly.
> >
> > Signed-off-by: Elias Vanderstuyft <Elias.vds@gmail.com>
> > [anssi.hannula@iki.fi: added description and CCs]
> > Signed-off-by: Anssi Hannula <anssi.hannula@iki.fi>
> > Cc: Simon Wood <simon@mungewell.org>
> > Cc: <stable@vger.kernel.org>
> > ---
> >
> > Simon, does this look OK to you, or do you think it should be an lg4ff
> > device? Though I guess lg2ff is better than nothing even in that case.
> 
> Signed-off-by: Simon Wood <simon@mungewell.org>

Thanks, I have now applied the patch. In case you are going to work 
further on splitting break/accel into different axes, please send it as a 
followup patch.

Thanks.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH] HID: i2c-hid: add platform data for quirks
From: Jiri Kosina @ 2013-10-09 10:26 UTC (permalink / raw)
  To: Bibek Basu; +Cc: benjamin.tissoires, linux-tegra, kadduri, linux-input
In-Reply-To: <1380790121-1355-1-git-send-email-bbasu@nvidia.com>

On Thu, 3 Oct 2013, Bibek Basu wrote:

> Some HID device does not responds to INPUT/FEATURE
> report query during initialization. As a result HID driver
> throws error even when its not mandatory according to the
> HID specification. Add platform data to provide
> quirk information to the driver so that init query is not
> done on such devices.
> 
> Signed-off-by: Bibek Basu <bbasu@nvidia.com>
> ---
>  drivers/hid/i2c-hid/i2c-hid.c | 1 +
>  include/linux/i2c/i2c-hid.h   | 3 ++-
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index fd7ce37..a7aec2c 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -1017,6 +1017,7 @@ static int i2c_hid_probe(struct i2c_client *client,
>  	hid->version = le16_to_cpu(ihid->hdesc.bcdVersion);
>  	hid->vendor = le16_to_cpu(ihid->hdesc.wVendorID);
>  	hid->product = le16_to_cpu(ihid->hdesc.wProductID);
> +	hid->quirks = platform_data->quirks;
>  
>  	snprintf(hid->name, sizeof(hid->name), "%s %04hX:%04hX",
>  		 client->name, hid->vendor, hid->product);
> diff --git a/include/linux/i2c/i2c-hid.h b/include/linux/i2c/i2c-hid.h
> index 7aa901d..12e682d 100644
> --- a/include/linux/i2c/i2c-hid.h
> +++ b/include/linux/i2c/i2c-hid.h
> @@ -17,7 +17,7 @@
>  /**
>   * struct i2chid_platform_data - used by hid over i2c implementation.
>   * @hid_descriptor_address: i2c register where the HID descriptor is stored.
> - *
> + * @quirks: quirks, if any for i2c-hid devices
>   * Note that it is the responsibility of the platform driver (or the acpi 5.0
>   * driver, or the flattened device tree) to setup the irq related to the gpio in
>   * the struct i2c_board_info.
> @@ -31,6 +31,7 @@
>   */
>  struct i2c_hid_platform_data {
>  	u16 hid_descriptor_address;
> +	u32 quirks;
>  };

Umm, and where does the quirks field of platform data actually get set? I 
don't seem to see any followup patchset doing that?

Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox