* Disabling the tsc2005 driver at runtime
From: Phil Carmody @ 2013-10-11 16:38 UTC (permalink / raw)
To: dmitry.torokhov
Cc: freemangordon, linux-input, wfp5p, broonie, linux-kernel, pc+n900
Greetings, Dmitry,
I'm looking for some advice regarding locally restoring a feature into
the tsc2005 driver that was removed back in:
http://www.spinics.net/lists/linux-input/msg14575.html
commit 5cb81d1: Input: tsc2005 - remove 'disable' sysfs attribute
Given the prior patch in the patchset:
commit 0b950d3: Input: tsc2005 - add open/close
"Introduce open and close methods for the input device
to keep the device powered down when it is not in use."
it seems you were expecting the device node to only have one reader, and
that reader would be the entity who decided when it was time to disable
and power down the device.
However, in the Nokia N900, the device node has at least 3 readers, 2 of
which I believe are closed source components, and therefore cannot be
modified to follow kernel changes. And even then, forcing 3 userspace
programs to now have to actively participate in the disabling of the
device seems excessively wasteful, compared with the ultimate simplicity
of the clients not even having to know about such things, which is how
it was before the change.
Back then you mentioned a generic interface that should replace this
device-specific one, but I can see no documentation for such an
interface in the kernel Documentation/ABI. Can you elaborate?
At the moment, as we can't break userspace, it seems that if we want to
run a more modern kernel on the N900 or another tsc2005-based device
running Fremantle, we should just locally revert that patch, and take
the maintenance hit ourselves for the implications of that. (Here 'we'
= the volunteer community still supporting the device because of its
longevity.)
Cheers,
Phil
--
People generally seem to want software to be free as in speech and/or
free as in beer. Unfortunately rather too much of it is free as in jazz.
-- Janet McKnight, on a private newsgroup
^ permalink raw reply
* Re: [PATCH 1/2] Input: twl4030_keypad - add device tree support
From: Tony Lindgren @ 2013-10-11 23:38 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Sebastian Reichel, linux-input, 'Benoît Cousson',
Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Rob Landley, Russell King, Dmitry Torokhov,
Grant Likely, devicetree, linux-doc, linux-kernel,
linux-arm-kernel, linux-omap
In-Reply-To: <1381353447-32708-1-git-send-email-sre@debian.org>
* Sebastian Reichel <sre@debian.org> [131009 14:25]:
> Add device tree support for twl4030 keypad driver and update the
> Documentation with twl4030 keypad device tree binding information.
>
> This patch also adds a twl4030 keypad node to the twl4030.dtsi file,
> so that board files can just add the keymap.
>
> Tested on Nokia N900.
Nice :) Just few cosmetic comments below.
>
> +#ifdef CONFIG_OF
> +static int twl4030_keypad_parse_dt(struct device *dev,
> + struct twl4030_keypad *keypad_data)
> +{
I guess the way to go nowadays is to use #IS_ENABLED(CONFIG_OF) here
and later on in this patch.
> @@ -331,20 +358,12 @@ static int twl4030_kp_program(struct twl4030_keypad *kp)
> static int twl4030_kp_probe(struct platform_device *pdev)
> {
> struct twl4030_keypad_data *pdata = pdev->dev.platform_data;
> - const struct matrix_keymap_data *keymap_data;
> + const struct matrix_keymap_data *keymap_data = NULL;
> struct twl4030_keypad *kp;
> struct input_dev *input;
> u8 reg;
> int error;
>
> - if (!pdata || !pdata->rows || !pdata->cols || !pdata->keymap_data ||
> - pdata->rows > TWL4030_MAX_ROWS || pdata->cols > TWL4030_MAX_COLS) {
> - dev_err(&pdev->dev, "Invalid platform_data\n");
> - return -EINVAL;
> - }
> -
> - keymap_data = pdata->keymap_data;
> -
> kp = kzalloc(sizeof(*kp), GFP_KERNEL);
> input = input_allocate_device();
> if (!kp || !input) {
I assume you have tested the above so it does not break things
for legacy booting?
Other than that:
Acked-by: Tony Lindgren <tony@atomide.com>
^ permalink raw reply
* Re: [PATCH 2/2] ARM: dts: N900: TWL4030 Keypad Matrix definition
From: Tony Lindgren @ 2013-10-11 23:39 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Sebastian Reichel, linux-input, 'Benoît Cousson',
Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Rob Landley, Russell King, Dmitry Torokhov,
Grant Likely, devicetree, linux-doc, linux-kernel,
linux-arm-kernel, linux-omap
In-Reply-To: <1381353447-32708-2-git-send-email-sre@debian.org>
* Sebastian Reichel <sre@debian.org> [131009 14:26]:
> Add Keyboard Matrix information to N900's DTS file.
> This patch maps the keys exactly as the original
> board code.
This should be queued by Benoit along with other .dts
changes, not via the input tree:
Acked-by: Tony Lindgren <tony@atomide.com>
^ permalink raw reply
* [PATCH] Input: nspire-keypad - add missing clk_disable_unprepare() on error path
From: Wei Yongjun @ 2013-10-12 6:32 UTC (permalink / raw)
To: dmitry.torokhov, grant.likely, rob.herring, dt.tangr, yongjun_wei,
Julia.Lawall, viresh.kumar
Cc: linux-input
From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Add the missing clk_disable_unprepare() before return
from nspire_keypad_open() in the error handling case.
Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
drivers/input/keyboard/nspire-keypad.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/input/keyboard/nspire-keypad.c b/drivers/input/keyboard/nspire-keypad.c
index b3e3eda..85e8d80 100644
--- a/drivers/input/keyboard/nspire-keypad.c
+++ b/drivers/input/keyboard/nspire-keypad.c
@@ -143,8 +143,10 @@ static int nspire_keypad_open(struct input_dev *input)
return error;
error = nspire_keypad_chip_init(keypad);
- if (error)
+ if (error) {
+ clk_disable_unprepare(keypad->clk);
return error;
+ }
return 0;
}
^ permalink raw reply related
* Re: [PATCH v2 1/2] Input: rotary-encoder: Add 'stepped' irq handler
From: Ezequiel García @ 2013-10-12 16:58 UTC (permalink / raw)
To: Daniel Mack; +Cc: linux-kernel@vger.kernel.org, linux-input, Dmitry Torokhov
In-Reply-To: <524EC022.2050203@gmail.com>
Dmitry,
On 4 October 2013 10:18, Daniel Mack <zonque@gmail.com> wrote:
> On 04.10.2013 14:53, Ezequiel Garcia wrote:
>> Some rotary-encoder devices (such as those with detents) are capable
>> of producing a stable event on each step. This commit adds support
>> for this case, by implementing a new interruption handler.
>>
>> The handler needs only detect the direction of the turn and generate
>> an event according to this detection.
>
> I think you can squash patch 2/2 into this one. It doesn't make much
> sense to have a one-liner patch to just update the documenation separately.
>
> Other than that, the code looks fine to me.
>
> Acked-by: Daniel Mack <zonque@gmail.com>
>
Could you merge these two patches?
It's already acked by Daniel, and there wasn't any other feedback
so I think we're ready to get them in and support more devices :-)
--
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar
--
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
* joydev: Keyboards incorrectly identified as joystick
From: Jethro Beekman @ 2013-10-12 22:05 UTC (permalink / raw)
To: linux-input
Or, really: "Keyboards incorrectly say they're a joystick". Some keyboard like
to say they have joystick capabilities while they don't. This results in a
joystick device showing up (e.g. in games) that is not calibrated and just
generally messes things up.
This seems to happen with multiple keyboards, mostly from Microsoft: e.g. the
Microsoft Digital Media Pro Keyboard and Microsoft Wired Keyboard 600.
Earlier reports on this mailing list:
http://thread.gmane.org/gmane.linux.kernel.input/25913
http://thread.gmane.org/gmane.linux.kernel.input/25926
Downstream bug reports:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/987877
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/390959
evtest output for certain keyboards:
https://launchpadlibrarian.net/112444244/evtest2
http://pastebin.com/tX4nnhAg
It would be easy to add an exception to joydev_match() like there currently are
for touchpads, tablets, etc. The question is: what should the test be? Or should
this be handled in userspace (udev)?
Jethro Beekman
^ permalink raw reply
* Problem with Mighty Mouse
From: Bastien Nocera @ 2013-10-12 23:48 UTC (permalink / raw)
To: linux-input
Heya,
I've tested a Bluetooth Apple Mighty Mouse (AA1197) and though I can
move the cursor without a problem, none of the buttons work. evtest
doesn't see any button events.
Any ideas what I should look at, or which tool I could use to capture
the raw data?
Cheers
^ permalink raw reply
* [PATCH 1/1] HID: Fix unit exponent parsing again
From: Nikolai Kondrashov @ 2013-10-13 12:09 UTC (permalink / raw)
To: Jiri Kosina; +Cc: linux-input, Benjamin Tissoires, Nikolai Kondrashov
Revert some changes done in 774638386826621c984ab6994439f474709cac5e.
Revert all changes done in hidinput_calc_abs_res as it mistakingly used
"Unit" item exponent nibbles to affect resolution value. This wasn't
breaking resolution calculation of relevant axes of any existing
devices, though, as they have only one dimension to their units and thus
1 in the corresponding nible.
Revert to reading "Unit Exponent" item value as a signed integer in
hid_parser_global to fix reading specification-complying values. This
fixes resolution calculation of devices complying to the HID standard,
including Huion, KYE, Waltop and UC-Logic graphics tablets which have
their report descriptors fixed by the drivers.
Explanations follow.
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.
However, there is a common misunderstanding about the "Unit Exponent"
value encoding, where it is assumed to be stored the same as nibbles in
"Unit" item. This is most likely due to the specification being a bit
vague and overloading the term "unit exponent". This also was and still
is proliferated by the official "HID Descriptor Tool", which makes this
mistake and stores "Unit Exponent" as such. This format is also
mentioned in books such as "USB Complete" and in Microsoft's hardware
design guides.
As a result many devices currently on the market use this encoding and
so the driver should support them.
---
drivers/hid/hid-core.c | 11 ++++++-----
drivers/hid/hid-input.c | 13 ++++---------
2 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index b8470b1..013cad0 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -319,7 +319,7 @@ static s32 item_sdata(struct hid_item *item)
static int hid_parser_global(struct hid_parser *parser, struct hid_item *item)
{
- __u32 raw_value;
+ __s32 raw_value;
switch (item->tag) {
case HID_GLOBAL_ITEM_TAG_PUSH:
@@ -370,10 +370,11 @@ 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);
+ /* Many devices provide unit exponent as a two's complement
+ * nibble due to the common misunderstanding of HID
+ * specification 1.11, 6.2.2.7 Global Items. Attempt to handle
+ * both this and the standard encoding. */
+ raw_value = item_sdata(item);
if (!(raw_value & 0xfffffff0))
parser->global.unit_exponent = hid_snto32(raw_value, 4);
else
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;
--
1.8.4.rc3
^ permalink raw reply related
* Re: [PATCH v2 1/2] Add an additional argument for decode routine, change secondary device name
From: Kevin Cernekee @ 2013-10-13 16:23 UTC (permalink / raw)
To: Yunkang Tang; +Cc: Dmitry Torokhov, yunkang.tang, linux-input, david turvene
In-Reply-To: <1379045010-3347-2-git-send-email-yunkang.tang@cn.alps.com>
> [PATCH v2 2/2] Improve the performance of alps v5-protocol's touchpad
>
> From: Yunkang Tang <yunkang.tang@xxxxxxxxxxx>
>
> - Calculate the device's dimension in alps_identify().
> - Change the logic of packet decoding.
> - Add the new data process logic.
>
> Signed-off-by: Yunkang Tang <yunkang.tang@xxxxxxxxxxx>
Hmm, somehow the [2/2] patch made it into the linux-input list
archives, but linux-input did not forward it to my inbox so I missed
it the first time around.
But thanks for sending these changes. There has been a great deal of
interest in enhancing ALPS touchpad support, especially for the
Dolphin models.
Some review comments follow:
> ---
> drivers/input/mouse/alps.c | 278 ++++++++++++++++++++++++++++++++++++++++++---
> drivers/input/mouse/alps.h | 4 +
> 2 files changed, 264 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 7e8a4fb..86c956a 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -257,6 +257,75 @@ static void alps_process_packet_v1_v2(struct psmouse *psmouse)
> }
>
> /*
> + * Process bitmap data for V5 protocols. Return value is null.
> + *
> + * The bitmaps don't have enough data to track fingers, so this function
> + * only generates points representing a bounding box of at most two contacts.
> + * These two points are returned in x1, y1, x2, and y2.
> + */
> +static void alps_process_bitmap_dolphin(struct alps_data *priv,
> + struct alps_fields *fields,
> + int *x1, int *y1, int *x2, int *y2)
> +{
> + struct alps_palm_bitmap {
> + int start_bit;
> + int end_bit;
> + };
> +
> + int i;
> + int box_middle_x, box_middle_y;
> + unsigned int x_map, y_map;
> + struct alps_palm_bitmap x_bitmap = {0}, y_bitmap = {0};
> +
> + x_map = fields->x_map;
> + y_map = fields->y_map;
> +
> + if (!x_map || !y_map)
> + return;
> +
> + *x1 = *y1 = *x2 = *y2 = 0;
> +
> + if (fields->fingers > 1) {
> + for (i = 0; i < 32; i++) {
Extra space after "i = 0;"
> + if (x_map & (1 << i)) {
> + x_bitmap.end_bit = priv->x_bits - i;
> + break;
> + }
> + }
> +
> + for (i = 31; i >= 0; i--) {
> + if (x_map & (1 << i)) {
> + x_bitmap.start_bit = priv->x_bits - i;
> + break;
> + }
> + }
> +
> + for (i = 0; i < 32; i++) {
Same here
> + if (y_map & (1 << i)) {
> + y_bitmap.start_bit = i;
> + break;
> + }
> + }
So, assuming x_bits == y_bits == 8, and both x_map and y_map are in
the range 0x00..0xff:
y_bitmap.* lies in the range 0..7
x_bitmap.* lies in the range 1..8
Is that correct, or is x_bitmap off by one?
> + for (i = 31; i >= 0; i--) {
> + if (y_map & (1 << i)) {
> + y_bitmap.end_bit = i;
> + break;
> + }
> + }
I suspect that this whole function could be simplified a bit.
Would it be possible to use ffs() and fls() instead of loops?
Just a personal preference, but if you calculated X first and then Y,
it may require fewer temporary variables and the alps_palm_bitmap
struct could be removed.
> +
> + box_middle_x = (priv->x_max * (x_bitmap.start_bit + x_bitmap.end_bit)) /
> + (2 * (priv->x_bits - 1));
> + box_middle_y = (priv->y_max * (y_bitmap.start_bit + y_bitmap.end_bit)) /
> + (2 * (priv->y_bits - 1));
These lines look a bit long - suggest running through checkpatch.pl
I assume start_bit and end_bit can never be negative, because the code
that calculates e.g. x_map will populate at most bits 0..(x_bits-1)?
> + *x1 = fields->x;
> + *y1 = fields->y;
> + *x2 = 2 * box_middle_x - *x1;
> + *y2 = 2 * box_middle_y - *y1;
> + }
> +}
> +
> +/*
> * Process bitmap data from v3 and v4 protocols. Returns the number of
> * fingers detected. A return value of 0 means at least one of the
> * bitmaps was empty.
> @@ -495,25 +564,55 @@ static void alps_decode_rushmore(struct alps_fields *f, unsigned char *p,
> static void alps_decode_dolphin(struct alps_fields *f, unsigned char *p,
> struct psmouse *psmouse)
> {
> + unsigned int palm_high = 0, palm_low = 0, palm_mask = 0;
> + int i, remain_xbits;
> + struct alps_data *priv = psmouse->private;
> +
> f->first_mp = !!(p[0] & 0x02);
> f->is_mp = !!(p[0] & 0x20);
>
> - f->fingers = ((p[0] & 0x6) >> 1 |
> + if (!f->is_mp) {
> + f->x = ((p[1] & 0x7f) | ((p[4] & 0x0f) << 7));
> + f->y = ((p[2] & 0x7f) | ((p[4] & 0xf0) << 3));
> + f->z = (p[0] & 4) ? 0 : p[5] & 0x7f;
> + alps_decode_buttons_v3(f, p);
> + } else {
> + f->fingers = ((p[0] & 0x6) >> 1 |
> (p[0] & 0x10) >> 2);
> - f->x_map = ((p[2] & 0x60) >> 5) |
> - ((p[4] & 0x7f) << 2) |
> - ((p[5] & 0x7f) << 9) |
> - ((p[3] & 0x07) << 16) |
> - ((p[3] & 0x70) << 15) |
> - ((p[0] & 0x01) << 22);
> - f->y_map = (p[1] & 0x7f) |
> - ((p[2] & 0x1f) << 7);
> -
> - f->x = ((p[1] & 0x7f) | ((p[4] & 0x0f) << 7));
> - f->y = ((p[2] & 0x7f) | ((p[4] & 0xf0) << 3));
> - f->z = (p[0] & 4) ? 0 : p[5] & 0x7f;
>
> - alps_decode_buttons_v3(f, p);
> + palm_low = (p[1] & 0x7f) |
> + ((p[2] & 0x7f) << 7) |
> + ((p[4] & 0x7f) << 14) |
> + ((p[5] & 0x7f) << 21) |
> + ((p[3] & 0x07) << 28) | ((p[3] & 0x10) << 27);
> + palm_high = ((p[3] & 0x60) >> 5) | ((p[0] & 0x01) << 2);
> +
> + for (i = 0; i < priv->y_bits; i++)
> + palm_mask |= 1 << i;
> +
> + /* Y-profile is stored in P(0) to p(n), n = y_bits; */
> + f->y_map = palm_low & palm_mask;
Suggest something like:
f->y_map = palm_low & (BIT(priv->y_bits) - 1);
> +
> + /* X-profile is stored in p(n) to p(n+x_bits). */
> + palm_mask = 0;
> + for (i = priv->y_bits; i < 32 && i < priv->y_bits + priv->x_bits; i++)
> + palm_mask |= 1 << i;
> +
> + f->x_map = ((palm_low & (palm_mask)) >> priv->y_bits);
Likewise, maybe use:
f->x_map = (palm_low >> priv->y_bits) & (BIT(priv->x_bits) - 1);
(but double-check my math first)
> +
> + /*
> + * In some cases, palm_low's 32bit is not enough to save
> + * all X&Y-profiles, we need to use palm_high.
> + */
> + remain_xbits = priv->x_bits + priv->y_bits - 32;
> + if (remain_xbits > 0) {
> + palm_mask = 0;
> + for (i = 0; i < remain_xbits; i++)
> + palm_mask |= 1 << i;
> +
> + f->x_map |= ((palm_high & palm_mask) << (32 - priv->y_bits));
Would it be cleaner to just use a u64 for the palm data, instead of
splitting into low/high? (If so you'll probably want to compile-test
a 32-bit kernel.)
> + }
> + }
> }
>
> static void alps_process_touchpad_packet_v3(struct psmouse *psmouse)
> @@ -745,6 +844,112 @@ static void alps_process_packet_v4(struct psmouse *psmouse)
> input_sync(dev);
> }
>
> +
> +static void alps_process_touchpad_packet_v5(struct psmouse *psmouse)
> +{
> + struct alps_data *priv = psmouse->private;
> + unsigned char *packet = psmouse->packet;
> + struct input_dev *dev = psmouse->dev;
> + int x1 = 0, y1 = 0, x2 = 0, y2 = 0;
> + int fingers = 0;
> + struct alps_fields f;
> +
> + priv->decode_fields(&f, packet, psmouse);
> +
> + /*
> + * There's no single feature of touchpad position and bitmap packets
> + * that can be used to distinguish between them. We rely on the fact
> + * that a bitmap packet should always follow a position packet with
> + * bit 6 of packet[4] set.
> + */
> + if (priv->multi_packet) {
> + /*
> + * Sometimes a position packet will indicate a multi-packet
> + * sequence, but then what follows is another position
> + * packet. Check for this, and when it happens process the
> + * position packet as usual.
> + */
> + if (f.is_mp) {
> + fingers = f.fingers;
> + priv->decode_fields(&f, priv->multi_data, psmouse);
> + alps_process_bitmap_dolphin(priv, &f, &x1, &y1,
> + &x2, &y2);
Aside from this clause, alps_process_touchpad_packet_v5() is virtually
identical to alps_process_touchpad_packet_v3(). It would be good to
find a way to reuse more of the code.
> + } else {
> + priv->multi_packet = 0;
> + }
> + }
> +
> + /*
> + * Bit 6 of byte 0 is not usually set in position packets. The only
> + * times it seems to be set is in situations where the data is
> + * suspect anyway, e.g. a palm resting flat on the touchpad. Given
> + * this combined with the fact that this bit is useful for filtering
> + * out misidentified bitmap packets, we reject anything with this
> + * bit set.
> + */
> + if (f.is_mp)
> + return;
> +
> + if (!priv->multi_packet && f.first_mp) {
> + priv->multi_packet = 1;
> + memcpy(priv->multi_data, packet, sizeof(priv->multi_data));
> + return;
> + }
> +
> + priv->multi_packet = 0;
> +
> + /*
> + * Sometimes the hardware sends a single packet with z = 0
> + * in the middle of a stream. Real releases generate packets
> + * with x, y, and z all zero, so these seem to be flukes.
> + * Ignore them.
> + */
> + if (f.x && f.y && !f.z)
> + return;
> +
> + /*
> + * If we don't have MT data or the bitmaps were empty, we have
> + * to rely on ST data.
> + */
> + if (!fingers) {
> + x1 = f.x;
> + y1 = f.y;
> + fingers = f.z > 0 ? 1 : 0;
> + }
> +
> + if (f.z >= 64)
> + input_report_key(dev, BTN_TOUCH, 1);
> + else
> + input_report_key(dev, BTN_TOUCH, 0);
> +
> + alps_report_semi_mt_data(dev, fingers, x1, y1, x2, y2);
> +
> + input_mt_report_finger_count(dev, fingers);
> +
> + input_report_key(dev, BTN_LEFT, f.left);
> + input_report_key(dev, BTN_RIGHT, f.right);
> + input_report_key(dev, BTN_MIDDLE, f.middle);
> +
> + if (f.z > 0) {
> + input_report_abs(dev, ABS_X, f.x);
> + input_report_abs(dev, ABS_Y, f.y);
> + }
> + input_report_abs(dev, ABS_PRESSURE, f.z);
> +
> + input_sync(dev);
> +}
> +
> +static void alps_process_packet_v5(struct psmouse *psmouse)
> +{
> + unsigned char *packet = psmouse->packet;
> +
> + /* Ignore stick point data */
> + if (packet[0] == 0xD8)
> + return;
> +
> + alps_process_touchpad_packet_v5(psmouse);
> +}
> +
> static void alps_report_bare_ps2_packet(struct psmouse *psmouse,
> unsigned char packet[],
> bool report_buttons)
> @@ -1522,6 +1727,41 @@ error:
> return -1;
> }
>
> +static int alps_dolphin_get_device_area(struct psmouse *psmouse,
> + struct alps_data *priv)
> +{
> + struct ps2dev *ps2dev = &psmouse->ps2dev;
> + unsigned char param[4] = {0};
> + int num_x_electrode, num_y_electrode;
> +
> + if (alps_enter_command_mode(psmouse))
> + return -1;
> +
> + if (ps2_command(ps2dev, NULL, 0x00EC) ||
> + ps2_command(ps2dev, NULL, 0x00F0) ||
> + ps2_command(ps2dev, NULL, 0x00F0) ||
> + ps2_command(ps2dev, NULL, 0x00F3) ||
> + ps2_command(ps2dev, NULL, 0x000A) ||
> + ps2_command(ps2dev, NULL, 0x00F3) ||
> + ps2_command(ps2dev, NULL, 0x000A))
> + return -1;
> +
> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETINFO))
> + return -1;
> +
> + num_x_electrode = DOLPHIN_PROFILE_XOFFSET + (param[2] & 0x0F);
> + num_y_electrode = DOLPHIN_PROFILE_YOFFSET + ((param[2] >> 4) & 0x0F);
This is strictly optional, but adding a comment indicating typical
values for num_x_electrode and num_y_electrode could aid in
understanding the math.
> + priv->x_bits = num_x_electrode;
> + priv->y_bits = num_y_electrode;
> + priv->x_max = (num_x_electrode - 1) * DOLPHIN_COUNT_PER_ELECTRODE;
> + priv->y_max = (num_y_electrode - 1) * DOLPHIN_COUNT_PER_ELECTRODE;
> +
> + if (alps_exit_command_mode(psmouse))
> + return -1;
> +
> + return 0;
> +}
> +
> static int alps_hw_init_dolphin_v1(struct psmouse *psmouse)
> {
> struct ps2dev *ps2dev = &psmouse->ps2dev;
> @@ -1574,7 +1814,7 @@ static void alps_set_defaults(struct alps_data *priv)
> break;
> case ALPS_PROTO_V5:
> priv->hw_init = alps_hw_init_dolphin_v1;
> - priv->process_packet = alps_process_packet_v3;
> + priv->process_packet = alps_process_packet_v5;
> priv->decode_fields = alps_decode_dolphin;
> priv->set_abs_params = alps_set_abs_params_mt;
> priv->nibble_commands = alps_v3_nibble_commands;
> @@ -1648,11 +1888,13 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
> if (alps_match_table(psmouse, priv, e7, ec) == 0) {
> return 0;
> } else if (e7[0] == 0x73 && e7[1] == 0x03 && e7[2] == 0x50 &&
> - ec[0] == 0x73 && ec[1] == 0x01) {
> + ec[0] == 0x73 && (ec[1] == 0x01 || ec[1] == 0x02)) {
> priv->proto_version = ALPS_PROTO_V5;
> alps_set_defaults(priv);
> -
> - return 0;
> + if (alps_dolphin_get_device_area(psmouse, priv))
> + return -EIO;
> + else
> + return 0;
> } else if (ec[0] == 0x88 && ec[1] == 0x08) {
> priv->proto_version = ALPS_PROTO_V3;
> alps_set_defaults(priv);
> diff --git a/drivers/input/mouse/alps.h b/drivers/input/mouse/alps.h
> index cdc10f3..43d89fc7 100644
> --- a/drivers/input/mouse/alps.h
> +++ b/drivers/input/mouse/alps.h
> @@ -18,6 +18,10 @@
> #define ALPS_PROTO_V4 4
> #define ALPS_PROTO_V5 5
>
> +#define DOLPHIN_COUNT_PER_ELECTRODE 64
> +#define DOLPHIN_PROFILE_XOFFSET 8 /* x-electrode offset */
> +#define DOLPHIN_PROFILE_YOFFSET 1 /* y-electrode offset */
> +
> /**
> * struct alps_model_info - touchpad ID table
> * @signature: E7 response string to match.
^ permalink raw reply
* [PATCH] Input: wacom - Export battery scope
From: Bastien Nocera @ 2013-10-13 19:49 UTC (permalink / raw)
To: linux-input; +Cc: dmitry.torokhov, peter.hutterer, chris, killertofu
This will stop UPower from detecting the tablet as a power supply,
and using its battery status to hibernate or switch off the machine.
https://bugs.freedesktop.org/show_bug.cgi?id=70321
Signed-off-by: Bastien Nocera <hadess@hadess.net>
---
drivers/input/tablet/wacom_sys.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/input/tablet/wacom_sys.c b/drivers/input/tablet/wacom_sys.c
index 79b69ea..e53416a 100644
--- a/drivers/input/tablet/wacom_sys.c
+++ b/drivers/input/tablet/wacom_sys.c
@@ -1031,6 +1031,7 @@ static void wacom_destroy_leds(struct wacom *wacom)
}
static enum power_supply_property wacom_battery_props[] = {
+ POWER_SUPPLY_PROP_SCOPE,
POWER_SUPPLY_PROP_CAPACITY
};
@@ -1042,6 +1043,9 @@ static int wacom_battery_get_property(struct power_supply *psy,
int ret = 0;
switch (psp) {
+ case POWER_SUPPLY_PROP_SCOPE:
+ val->intval = POWER_SUPPLY_SCOPE_DEVICE;
+ break;
case POWER_SUPPLY_PROP_CAPACITY:
val->intval =
wacom->wacom_wac.battery_capacity * 100 / 31;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v2 2/2] Input: wacom - add support for three new Intuos devices
From: Peter Hutterer @ 2013-10-14 0:27 UTC (permalink / raw)
To: Ping Cheng; +Cc: linux-input, dmitry.torokhov, chris, Ping Cheng
In-Reply-To: <1381439866-1161-1-git-send-email-pingc@wacom.com>
On Thu, Oct 10, 2013 at 02:17:46PM -0700, Ping Cheng wrote:
> This series of models added a hardware switch to turn touch
> data on/off. To report the state of the switch, SW_TOUCH
> is added in include/uapi/linux/input.h.
>
> The driver is also updated to process wireless devices that do
> not support touch interface.
>
> Tested-by: Jason Gerecke <killertofu@gmail.com>
> Signed-off-by: Ping Cheng <pingc@wacom.com>
> ---
> v2: Change SW_TOUCH_ENABLED to SW_TOUCH and clear BTN_TOUCH bit
> for button only interfaces as suggested by Peter Hutterer.
> ---
> drivers/input/tablet/wacom_sys.c | 16 +++++++-
> drivers/input/tablet/wacom_wac.c | 87 ++++++++++++++++++++++++++++++++--------
> drivers/input/tablet/wacom_wac.h | 7 ++++
> include/uapi/linux/input.h | 1 +
> 4 files changed, 93 insertions(+), 18 deletions(-)
>
[...]
> diff --git a/drivers/input/tablet/wacom_wac.h b/drivers/input/tablet/wacom_wac.h
> index fd23a37..ba9e335 100644
> --- a/drivers/input/tablet/wacom_wac.h
> +++ b/drivers/input/tablet/wacom_wac.h
> @@ -54,6 +54,8 @@
> #define WACOM_REPORT_TPCST 16
> #define WACOM_REPORT_TPC1FGE 18
> #define WACOM_REPORT_24HDT 1
> +#define WACOM_REPORT_WL_MODE 128
> +#define WACOM_REPORT_USB_MODE 192
>
> /* device quirks */
> #define WACOM_QUIRK_MULTI_INPUT 0x0001
> @@ -81,6 +83,7 @@ enum {
> INTUOSPS,
> INTUOSPM,
> INTUOSPL,
> + INTUOS_HT,
nitpick: why the underscore? all other intuos enums are one word.
Cheers,
Peter
> WACOM_21UX2,
> WACOM_22HD,
> DTK,
^ permalink raw reply
* Re: [PATCH] input: qt1070: add power management ops
From: Bo Shen @ 2013-10-14 5:45 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: Nicolas Ferre, Wenyou.Yang, linux-arm-kernel, linux-input
In-Reply-To: <520B2921.3000906@atmel.com>
Hi Dmitry,
On 8/14/2013 14:52, Nicolas Ferre wrote:
> On 13/08/2013 09:43, Bo Shen :
>> Add power management ops for qt1070, it maybe a wakeup source
>>
>> Signed-off-by: Bo Shen <voice.shen@atmel.com>
>
> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Would this patch be applied with Nicolas' ACK?
Best Regards,
Bo Shen
>> ---
>> drivers/input/keyboard/qt1070.c | 25 +++++++++++++++++++++++++
>> 1 file changed, 25 insertions(+)
>>
>> diff --git a/drivers/input/keyboard/qt1070.c
>> b/drivers/input/keyboard/qt1070.c
>> index 42b773b..deefe5a 100644
>> --- a/drivers/input/keyboard/qt1070.c
>> +++ b/drivers/input/keyboard/qt1070.c
>> @@ -243,6 +243,30 @@ static int qt1070_remove(struct i2c_client *client)
>> return 0;
>> }
>>
>> +#ifdef CONFIG_PM_SLEEP
>> +static int qt1070_suspend(struct device *dev)
>> +{
>> + struct qt1070_data *data = dev_get_drvdata(dev);
>> +
>> + if (device_may_wakeup(dev))
>> + enable_irq_wake(data->irq);
>> +
>> + return 0;
>> +}
>> +
>> +static int qt1070_resume(struct device *dev)
>> +{
>> + struct qt1070_data *data = dev_get_drvdata(dev);
>> +
>> + if (device_may_wakeup(dev))
>> + disable_irq_wake(data->irq);
>> +
>> + return 0;
>> +}
>> +#endif
>> +
>> +static SIMPLE_DEV_PM_OPS(qt1070_pm_ops, qt1070_suspend, qt1070_resume);
>> +
>> static const struct i2c_device_id qt1070_id[] = {
>> { "qt1070", 0 },
>> { },
>> @@ -253,6 +277,7 @@ static struct i2c_driver qt1070_driver = {
>> .driver = {
>> .name = "qt1070",
>> .owner = THIS_MODULE,
>> + .pm = &qt1070_pm_ops,
>> },
>> .id_table = qt1070_id,
>> .probe = qt1070_probe,
>>
>
>
^ permalink raw reply
* Re: [PATCH v2 1/2] Add an additional argument for decode routine, change secondary device name
From: Will Tommy @ 2013-10-14 5:50 UTC (permalink / raw)
To: Kevin Cernekee; +Cc: Dmitry Torokhov, yunkang.tang, linux-input, david turvene
In-Reply-To: <CAJiQ=7D9ZBKJcGgYT+3se92t2zn6nzJ8645CzWdB2xh1S60+nQ@mail.gmail.com>
Hi Kevin,
Thanks for your carefully review. It's amazing !
> So, assuming x_bits == y_bits == 8, and both x_map and y_map are in
> the range 0x00..0xff:
>
> y_bitmap.* lies in the range 0..7
> x_bitmap.* lies in the range 1..8
>
> Is that correct, or is x_bitmap off by one?
Hmm, it seems a bug of algorithm. Both x_bitmay&y_bitmap's range should be 0..7.
I'll fix it.
> I suspect that this whole function could be simplified a bit.
>
> Would it be possible to use ffs() and fls() instead of loops?
>
> Just a personal preference, but if you calculated X first and then Y,
> it may require fewer temporary variables and the alps_palm_bitmap
> struct could be removed.
Thanks for your proposal. I'll re-write this function in next submission.
> > + box_middle_x = (priv->x_max * (x_bitmap.start_bit + x_bitmap.end_bit)) /
> > + (2 * (priv->x_bits - 1));
> > + box_middle_y = (priv->y_max * (y_bitmap.start_bit + y_bitmap.end_bit)) /
> > + (2 * (priv->y_bits - 1));
> These lines look a bit long - suggest running through checkpatch.pl
>
Yes, they're longer than 80. However, in order not to let code hard to
be understood,
I decided not to split them. Anyway, since this function would be
re-written, there
would be no problem.
> I assume start_bit and end_bit can never be negative, because the code
> that calculates e.g. x_map will populate at most bits 0..(x_bits-1)?
Do you mean it's not good to define start_bit & end_bit as "int" since they have
no chance to be negative ? Actually, "unsigned char" is much better.
> Suggest something like:
>
> f->y_map = palm_low & (BIT(priv->y_bits) - 1);
> Likewise, maybe use:
>
> f->x_map = (palm_low >> priv->y_bits) & (BIT(priv->x_bits) - 1);
>
> (but double-check my math first)
> Would it be cleaner to just use a u64 for the palm data, instead of
> splitting into low/high? (If so you'll probably want to compile-test
> a 32-bit kernel.)
> Aside from this clause, alps_process_touchpad_packet_v5() is virtually
> identical to alps_process_touchpad_packet_v3(). It would be good to
> find a way to reuse more of the code.
Okay, I'll take your advice and check the behavior on both 32-bit and 64-bit.
> This is strictly optional, but adding a comment indicating typical
> values for num_x_electrode and num_y_electrode could aid in
> understanding the math.
That's a good idea! I'll add them.
Tommy
^ permalink raw reply
* Re: [PATCH v2 1/2] Add an additional argument for decode routine, change secondary device name
From: Kevin Cernekee @ 2013-10-14 6:02 UTC (permalink / raw)
To: Will Tommy; +Cc: Dmitry Torokhov, yunkang.tang, linux-input, david turvene
In-Reply-To: <CA+F6ckOfK25hhCxcJ5JRbzt+o1BKYvOynB1DQkPY+RSZrp8KFw@mail.gmail.com>
On Sun, Oct 13, 2013 at 10:50 PM, Will Tommy <tommywill2011@gmail.com> wrote:
>> I assume start_bit and end_bit can never be negative, because the code
>> that calculates e.g. x_map will populate at most bits 0..(x_bits-1)?
>
> Do you mean it's not good to define start_bit & end_bit as "int" since they have
> no chance to be negative ? Actually, "unsigned char" is much better.
No type change needed - I was just checking to make sure it wasn't a
risk, since getting a negative number in there would really throw off
the math...
^ permalink raw reply
* Re: [PATCH v2 1/2] Add an additional argument for decode routine, change secondary device name
From: Tommy Will @ 2013-10-14 6:10 UTC (permalink / raw)
To: Kevin Cernekee; +Cc: Dmitry Torokhov, yunkang.tang, linux-input, david turvene
In-Reply-To: <CAJiQ=7ChMZDEbUE_761NLwByWSCQTBM8ek=NQvVNQwjBN=SqBA@mail.gmail.com>
2013/10/14 Kevin Cernekee <cernekee@gmail.com>:
> On Sun, Oct 13, 2013 at 10:50 PM, Will Tommy <tommywill2011@gmail.com> wrote:
>
> No type change needed - I was just checking to make sure it wasn't a
> risk, since getting a negative number in there would really throw off
> the math...
Got that. Thanks : )
^ permalink raw reply
* WPC8769L (WEC1020) support in winbond-cir?
From: Tom Gundersen @ 2013-10-14 13:16 UTC (permalink / raw)
To: Juan J. Garcia de Soria, David Härdeman
Cc: Sean Young, linux-input@vger.kernel.org
Hi David and Juan,
I'm going through the various out-of-tree LIRC drivers to see if we
can stop shipping them in Arch Linux [0]. So far it appears we can
drop all except for lirc_wpc8769l [1] (PnP id WEC1020).
I noticed the comment in windownd-cir [2]:
* Currently supports the Winbond WPCD376i chip (PNP id WEC1022), but
* could probably support others (Winbond WEC102X, NatSemi, etc)
* with minor modifications.
What are your thoughts on adding support for WEC1020 upstream? Is
anyone interested in doing this work (I sadly don't have the correct
device, so can't really do it myself)?
Cheers,
Tom
[0]: <https://mailman.archlinux.org/pipermail/arch-dev-public/2013-October/025541.html>
[1]: <http://sourceforge.net/p/lirc/git/ci/master/tree/drivers/lirc_wpc8769l/>
[2]: <https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/media/rc/winbond-cir.c#n5>
^ permalink raw reply
* Re: [PATCHv2 3/3] HID:Kconfig: Correct MOMO description in wrong place (handled by LG4FF).
From: Jiri Kosina @ 2013-10-14 14:03 UTC (permalink / raw)
To: Elias Vanderstuyft, Simon Wood
Cc: linux-input, linux-kernel, Michal Malý
In-Reply-To: <CADbOyBQodqzSb04OCO6oduJSZvveZXxQjSdp+CdyKKk5Bs=O7w@mail.gmail.com>
On Thu, 10 Oct 2013, Elias Vanderstuyft wrote:
> I don't see any report-descriptor being fixed up, this results in having
> only two axes: steering and combined accel-brake axis.
> So, same procedure as the Formula Vibration wheel, I'll send you the report
> descriptor and some hidraw data when using the accel and brake pedals, in
> my next mail.
So should I wait for v3, where the rdesc fixup for splitting the axes will
be done, or are you planning to submit it to me as a followup patch?
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: WPC8769L (WEC1020) support in winbond-cir?
From: Mauro Carvalho Chehab @ 2013-10-14 14:07 UTC (permalink / raw)
To: Tom Gundersen
Cc: Juan J. Garcia de Soria, David Härdeman, Sean Young,
linux-input@vger.kernel.org, LMML
In-Reply-To: <CAG-2HqX-TO7h8zJ6F01r2LfRVjQtb0pK_1wKGsYVKzB0zC7TQA@mail.gmail.com>
Hi Tom,
Em Mon, 14 Oct 2013 15:16:20 +0200
Tom Gundersen <teg@jklm.no> escreveu:
> Hi David and Juan,
>
> I'm going through the various out-of-tree LIRC drivers to see if we
> can stop shipping them in Arch Linux [0]. So far it appears we can
> drop all except for lirc_wpc8769l [1] (PnP id WEC1020).
Please copy the Linux Media ML to all Remote Controller drivers. There's
where we're discussing those drivers. No need to c/c linux-input.
Yeah, both lirc_atiusb and lirc_i2c were now obsoleted by upstream
non-staging drivers. I suggest to just drop it from Arch Linux.
Not sure about lirc_wpc8769l. David/Sean/Juan can have a better view.
Btw, there are also a number of lirc staging drivers under
drivers/staging/media/lirc/. We'd love some help trying to convert them
to use the rc core, moving them out of staging.
>
> I noticed the comment in windownd-cir [2]:
>
> * Currently supports the Winbond WPCD376i chip (PNP id WEC1022), but
> * could probably support others (Winbond WEC102X, NatSemi, etc)
> * with minor modifications.
>
> What are your thoughts on adding support for WEC1020 upstream? Is
> anyone interested in doing this work (I sadly don't have the correct
> device, so can't really do it myself)?
>
> Cheers,
>
> Tom
>
> [0]: <https://mailman.archlinux.org/pipermail/arch-dev-public/2013-October/025541.html>
> [1]: <http://sourceforge.net/p/lirc/git/ci/master/tree/drivers/lirc_wpc8769l/>
> [2]: <https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/media/rc/winbond-cir.c#n5>
> --
> 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
Regards,
Mauro
^ permalink raw reply
* Re: WPC8769L (WEC1020) support in winbond-cir?
From: Tom Gundersen @ 2013-10-14 14:42 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Juan J. Garcia de Soria, David Härdeman, Sean Young,
linux-input@vger.kernel.org, LMML
In-Reply-To: <20131014110748.366ea45e@samsung.com>
On Mon, Oct 14, 2013 at 4:07 PM, Mauro Carvalho Chehab
<m.chehab@samsung.com> wrote:
> Hi Tom,
>
> Em Mon, 14 Oct 2013 15:16:20 +0200
> Tom Gundersen <teg@jklm.no> escreveu:
>
>> Hi David and Juan,
>>
>> I'm going through the various out-of-tree LIRC drivers to see if we
>> can stop shipping them in Arch Linux [0]. So far it appears we can
>> drop all except for lirc_wpc8769l [1] (PnP id WEC1020).
>
> Please copy the Linux Media ML to all Remote Controller drivers. There's
> where we're discussing those drivers. No need to c/c linux-input.
>
> Yeah, both lirc_atiusb and lirc_i2c were now obsoleted by upstream
> non-staging drivers. I suggest to just drop it from Arch Linux.
Thanks for the info, we'll drop those drivers.
Cheers,
Tom
^ permalink raw reply
* Re: [PATCHv2 3/3] HID:Kconfig: Correct MOMO description in wrong place (handled by LG4FF).
From: simon @ 2013-10-14 14:58 UTC (permalink / raw)
To: Jiri Kosina
Cc: Elias Vanderstuyft, Simon Wood, linux-input, linux-kernel,
"Michal Malý"
In-Reply-To: <alpine.LNX.2.00.1310141602540.12405@pobox.suse.cz>
> On Thu, 10 Oct 2013, Elias Vanderstuyft wrote:
>
>> I don't see any report-descriptor being fixed up, this results in having
>> only two axes: steering and combined accel-brake axis.
>> So, same procedure as the Formula Vibration wheel, I'll send you the
>> report
>> descriptor and some hidraw data when using the accel and brake pedals,
>> in
>> my next mail.
>
> So should I wait for v3, where the rdesc fixup for splitting the axes will
> be done, or are you planning to submit it to me as a followup patch?
The HID descriptor for the MOMO Black was re-written in part2 of the patch
series:
https://patchwork.kernel.org/patch/3016111/
Simon
^ permalink raw reply
* Re: [PATCHv2 3/3] HID:Kconfig: Correct MOMO description in wrong place (handled by LG4FF).
From: Jiri Kosina @ 2013-10-14 18:48 UTC (permalink / raw)
To: Simon Wood
Cc: Elias Vanderstuyft, linux-input, linux-kernel,
"Michal Malý"
In-Reply-To: <d241ee8c4a607eebd06cfdf5b04ab200.squirrel@mungewell.org>
On Mon, 14 Oct 2013, simon@mungewell.org wrote:
> >> only two axes: steering and combined accel-brake axis.
> >> So, same procedure as the Formula Vibration wheel, I'll send you the
> >> report
> >> descriptor and some hidraw data when using the accel and brake pedals,
> >> in
> >> my next mail.
> >
> > So should I wait for v3, where the rdesc fixup for splitting the axes will
> > be done, or are you planning to submit it to me as a followup patch?
>
> The HID descriptor for the MOMO Black was re-written in part2 of the patch
> series:
> https://patchwork.kernel.org/patch/3016111/
Bah, you are right, I was still by mistake looking at the original series.
I am now applying all the 3 patches from v2.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* [PATCH] input: libps2: add ID of IBM terminal keyboards
From: Michał Pecio @ 2013-10-14 18:49 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
In-Reply-To: <20120907204510.74418ae7@students.mimuw.edu.pl>
From: Michal Pecio <michal.pecio@gmail.com>
Keyboards of several IBM terminals from 1980's and 1990's are
compatible with the PS/2 protocol.
Add their ID (0xBF) to the whitelist of supported keyboards.
Only untranslated ID needs to be added because atkbd doesn't
support translated scancode set 3 anyway.
Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
---
drivers/input/serio/libps2.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
index 07a8363..f946544 100644
--- a/drivers/input/serio/libps2.c
+++ b/drivers/input/serio/libps2.c
@@ -114,6 +114,7 @@ int ps2_is_keyboard_id(char id_byte)
0x5d, /* Trust keyboard */
0x60, /* NMB SGI keyboard, translated */
0x47, /* NMB SGI keyboard */
+ 0xbf, /* IBM terminal keyboards */
};
return memchr(keyboard_ids, id_byte, sizeof(keyboard_ids)) != NULL;
^ permalink raw reply related
* RE: [PATCH] HID: i2c-hid: add platform data for quirks
From: Bibek Basu @ 2013-10-15 5:21 UTC (permalink / raw)
To: Benjamin Tissoires, Jiri Kosina
Cc: linux-tegra@vger.kernel.org, Kiran Adduri, linux-input
In-Reply-To: <CAN+gG=Fi2cmJktiUCTT0mBQNnkAFcL8jTctw7EngQh50odQO-g@mail.gmail.com>
> -----Original Message-----
> From: Benjamin Tissoires [mailto:benjamin.tissoires@gmail.com]
> Sent: Wednesday, October 09, 2013 4:30 PM
> To: Jiri Kosina
> Cc: Bibek Basu; linux-tegra@vger.kernel.org; Kiran Adduri; linux-input
> Subject: Re: [PATCH] HID: i2c-hid: add platform data for quirks
>
> On Wed, Oct 9, 2013 at 12:26 PM, Jiri Kosina <jkosina@suse.cz> wrote:
> > 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;
> >> };
> >
>
> Hi Jiri,
>
> > Umm, and where does the quirks field of platform data actually get
> > set? I don't seem to see any followup patchset doing that?
>
> The platform data is filled by specific march-* board definition data.
> In this case some I would say a tegra board. So it's fine not having a patchset
> setting the quirk (as it is per board, and maybe per version/manufacturer).
>
> Other than that I would love seeing the same for the device tree binding if
> we add this to the struct i2c_hid_platform_data.
>
> BTW, Jiri, the aim of the patch was to be able to add the quirk
> HID_QUIRK_NO_INIT_INPUT_REPORTS to some of the i2c-hid devices.
> However, the Windows driver uses this by default, so I already told to Bibek
> that we could just drop the related calls in i2c_hid_init_reports().
>
Hi,
Sorry, I was OOO.
OK, I am repushing a new patch to just drop the i2c_hid_init_reports calls
regards
Bibek
> Cheers,
> Benjamin
^ permalink raw reply
* [PATCH] HID: i2c-hid: Stop querying for init reports
From: Bibek Basu @ 2013-10-15 8:28 UTC (permalink / raw)
To: benjamin.tissoires, jkosina; +Cc: linux-input, linux-kernel, Bibek Basu
According to specifications, HID over I2C devices
are not bound to respond to query for INPUT
REPORTS. Thus dropping the call during init
as many devices does not respond causing error
messages during boot.
Signed-off-by: Bibek Basu <bbasu@nvidia.com>
---
drivers/hid/i2c-hid/i2c-hid.c | 59 -------------------------------------------
1 file changed, 59 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index fd7ce37..58a4f12 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -409,62 +409,6 @@ static int i2c_hid_get_report_length(struct hid_report *report)
report->device->report_enum[report->type].numbered + 2;
}
-static void i2c_hid_init_report(struct hid_report *report, u8 *buffer,
- size_t bufsize)
-{
- struct hid_device *hid = report->device;
- struct i2c_client *client = hid->driver_data;
- struct i2c_hid *ihid = i2c_get_clientdata(client);
- unsigned int size, ret_size;
-
- size = i2c_hid_get_report_length(report);
- if (i2c_hid_get_report(client,
- report->type == HID_FEATURE_REPORT ? 0x03 : 0x01,
- report->id, buffer, size))
- return;
-
- i2c_hid_dbg(ihid, "report (len=%d): %*ph\n", size, size, ihid->inbuf);
-
- ret_size = buffer[0] | (buffer[1] << 8);
-
- if (ret_size != size) {
- dev_err(&client->dev, "error in %s size:%d / ret_size:%d\n",
- __func__, size, ret_size);
- return;
- }
-
- /* hid->driver_lock is held as we are in probe function,
- * we just need to setup the input fields, so using
- * hid_report_raw_event is safe. */
- hid_report_raw_event(hid, report->type, buffer + 2, size - 2, 1);
-}
-
-/*
- * Initialize all reports
- */
-static void i2c_hid_init_reports(struct hid_device *hid)
-{
- struct hid_report *report;
- struct i2c_client *client = hid->driver_data;
- struct i2c_hid *ihid = i2c_get_clientdata(client);
- u8 *inbuf = kzalloc(ihid->bufsize, GFP_KERNEL);
-
- if (!inbuf) {
- dev_err(&client->dev, "can not retrieve initial reports\n");
- return;
- }
-
- list_for_each_entry(report,
- &hid->report_enum[HID_INPUT_REPORT].report_list, list)
- i2c_hid_init_report(report, inbuf, ihid->bufsize);
-
- list_for_each_entry(report,
- &hid->report_enum[HID_FEATURE_REPORT].report_list, list)
- i2c_hid_init_report(report, inbuf, ihid->bufsize);
-
- kfree(inbuf);
-}
-
/*
* Traverse the supplied list of reports and find the longest
*/
@@ -683,9 +627,6 @@ static int i2c_hid_start(struct hid_device *hid)
return ret;
}
- if (!(hid->quirks & HID_QUIRK_NO_INIT_REPORTS))
- i2c_hid_init_reports(hid);
-
return 0;
}
--
1.8.1.5
^ permalink raw reply related
* [PATCH] Input: usbtouchscreen - separate report and transmit buffer size handling
From: Christian Engelmayer @ 2013-10-15 14:50 UTC (permalink / raw)
To: Dmitry Torokhov, Forest Bond, Daniel Ritz
Cc: linux-input, linux-usb, christian.engelmayer
This patch supports the separate handling of the USB transfer buffer length
and the length of the buffer used for multi packet support. The USB transfer
size can now be explicitly configured via the device_info record. Otherwise
it defaults to the configured report packet size as before. For devices
supporting multiple report or diagnostic packets, the USB transfer size is
now reduced to the USB endpoints wMaxPacketSize if not explicitly set.
This fixes an issue where event reporting can be delayed for an arbitrary
time for multi packet devices. For instance the report size for eGalax devices
is defined to the 16 byte maximum diagnostic packet size as opposed to the 5
byte report packet size. In case the driver requests 16 byte from the USB
interrupt endpoint, the USB host controller driver needs to split up the
request into 2 accesses according to the endpoints wMaxPacketSize of 8 byte.
When the first transfer is answered by the eGalax device with not less than
the full 8 byte requested, the host controller has got no way of knowing
whether the touch controller has got additional data queued and will issue
the second transfer. If per example a liftoff event finishes at such a
wMaxPacketSize boundary, the data will not be available to the usbtouch driver
until a further event is triggered and transfered to the host. From user
perspective the BTN_TOUCH release event in this case is stuck until the next
touch down event.
Signed-off-by: Christian Engelmayer <christian.engelmayer@frequentis.com>
---
drivers/input/touchscreen/usbtouchscreen.c | 16 ++++++++++++----
1 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/input/touchscreen/usbtouchscreen.c b/drivers/input/touchscreen/usbtouchscreen.c
index 721fdb3..aa1f6a7 100644
--- a/drivers/input/touchscreen/usbtouchscreen.c
+++ b/drivers/input/touchscreen/usbtouchscreen.c
@@ -76,6 +76,7 @@ struct usbtouch_device_info {
int min_yc, max_yc;
int min_press, max_press;
int rept_size;
+ int xmit_size;
/*
* Always service the USB devices irq not just when the input device is
@@ -1523,7 +1524,7 @@ static int usbtouch_reset_resume(struct usb_interface *intf)
static void usbtouch_free_buffers(struct usb_device *udev,
struct usbtouch_usb *usbtouch)
{
- usb_free_coherent(udev, usbtouch->type->rept_size,
+ usb_free_coherent(udev, usbtouch->type->xmit_size,
usbtouch->data, usbtouch->data_dma);
kfree(usbtouch->buffer);
}
@@ -1567,8 +1568,15 @@ static int usbtouch_probe(struct usb_interface *intf,
usbtouch->type = type;
if (!type->process_pkt)
type->process_pkt = usbtouch_process_pkt;
+ if (!type->xmit_size) {
+ if ((type->get_pkt_len) &&
+ (type->rept_size > le16_to_cpu(endpoint->wMaxPacketSize)))
+ type->xmit_size = le16_to_cpu(endpoint->wMaxPacketSize);
+ else
+ type->xmit_size = type->rept_size;
+ }
- usbtouch->data = usb_alloc_coherent(udev, type->rept_size,
+ usbtouch->data = usb_alloc_coherent(udev, type->xmit_size,
GFP_KERNEL, &usbtouch->data_dma);
if (!usbtouch->data)
goto out_free;
@@ -1628,12 +1636,12 @@ static int usbtouch_probe(struct usb_interface *intf,
if (usb_endpoint_type(endpoint) == USB_ENDPOINT_XFER_INT)
usb_fill_int_urb(usbtouch->irq, udev,
usb_rcvintpipe(udev, endpoint->bEndpointAddress),
- usbtouch->data, type->rept_size,
+ usbtouch->data, type->xmit_size,
usbtouch_irq, usbtouch, endpoint->bInterval);
else
usb_fill_bulk_urb(usbtouch->irq, udev,
usb_rcvbulkpipe(udev, endpoint->bEndpointAddress),
- usbtouch->data, type->rept_size,
+ usbtouch->data, type->xmit_size,
usbtouch_irq, usbtouch);
usbtouch->irq->dev = udev;
--
1.7.2.5
^ permalink raw reply related
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