* Re: [PATCH v3 0/2] input/serio: Add a firmware_id sysfs attribute
From: Dmitry Torokhov @ 2014-04-20 5:31 UTC (permalink / raw)
To: Hans de Goede; +Cc: Benjamin Tissoires, Peter Hutterer, linux-input
In-Reply-To: <1397471419-31224-1-git-send-email-hdegoede@redhat.com>
On Mon, Apr 14, 2014 at 12:30:17PM +0200, Hans de Goede wrote:
> Hi All,
>
> Here is v3 of my serio firmware_id sysfs attribute patch-set
>
> Changes in v2:
> -Add a helper function to avoid copy/pasting the code for building
> the firmware_id string from pnp_ids
>
> Changes in v3:
> -Prefix the firmware_id string for 8042-pnp devices which "PNP: " so that
> it will be easy to differentiate pnp-ids from ie devicetree ids in the future
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3 0/2] uapi/input.h: Add INPUT_PROP_TOPBUTTONPAD device property
From: Dmitry Torokhov @ 2014-04-20 5:30 UTC (permalink / raw)
To: Hans de Goede; +Cc: Benjamin Tissoires, Peter Hutterer, linux-input
In-Reply-To: <1397546663-4447-1-git-send-email-hdegoede@redhat.com>
On Tue, Apr 15, 2014 at 09:24:21AM +0200, Hans de Goede wrote:
> Hi All,
>
> I've received an updated list of pnpids from synaptics so here is an updated
> version of the patch-set.
>
> Changes in v2:
> -Drop adding of pnp_id to struct serio (drop patches 2/4 and 3/4 of v1)
> -Modify the synaptics patch to use firmware_id instead of pnp_id
>
> Changes in v3:
> -Update topbuttonpad pnpid list in synaptics.c
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Issues with hid-multitouch driver
From: Tomas Sokorai @ 2014-04-19 23:02 UTC (permalink / raw)
To: linux-input
Hi guys,
I'm having issues with the hid-multitouch driver in kernel 3.14.
I have the following touchscreen device:
Bus 001 Device 003: ID 03fc:05d8 Elitegroup Computer Systems
The thing is that the driver detects the device correctly and even
works seemingly OK, until you start doing multi-touch taps on the
screen.
At some point, the input to the Xorg server stops.
I investigated this further, and using evtest, I can see that the events
ABS_X and ABS_Y stop being generated, and also that BTN_TOUCH event
gets a "value 1" for the touch before the freeze, but it is never
released: no "value 0" for the touch release event, ever.
After the driver gets braindamaged, it only generates
ABS_MT_POSITION_Y, ABS_MT_POSITION_X and ABS_MT_TRACKING_ID events,
and therefore no further clicks or normal Xorg pointer movement
on-screen.
I even tried backporting the lastest version from the HID git, with
the same results.
Thanks in advance,
--
Tomas J. Sokorai Sch.
^ permalink raw reply
* Re: [PATCH] Input: evdev - add event-mask API
From: Dmitry Torokhov @ 2014-04-19 21:16 UTC (permalink / raw)
To: David Herrmann
Cc: linux-input, Benjamin Tissoires, Peter Hutterer,
Lennart Poettering
In-Reply-To: <1397601105-17771-1-git-send-email-dh.herrmann@gmail.com>
Hi David,
On Wed, Apr 16, 2014 at 12:31:45AM +0200, David Herrmann wrote:
> Hardware manufacturers group keys in the weirdest way possible. This may
> cause a power-key to be grouped together with normal keyboard keys and
> thus be reported on the same kernel interface.
>
> However, user-space is often only interested in specific sets of events.
> For instance, daemons dealing with system-reboot (like systemd-logind)
> listen for KEY_POWER, but are not interested in any main keyboard keys.
> Usually, power keys are reported via separate interfaces, however,
> some i8042 boards report it in the AT matrix. To avoid waking up those
> system daemons on each key-press, we had two ideas:
> - split off KEY_POWER into a separate interface unconditionally
> - allow masking a specific set of events on evdev FDs
>
> Splitting of KEY_POWER is a rather weird way to deal with this and may
> break backwards-compatibility. It is also specific to KEY_POWER and might
> be required for other stuff, too. Moreover, we might end up with a huge
> set of input-devices just to have them properly split.
>
> Hence, this patchset implements the second idea: An event-mask to specify
> which events you're interested in. Two ioctls allow setting this mask for
> each event-type. If not set, all events are reported. The type==0 entry is
> used same as in EVIOCGBIT to set the actual EV_* mask of masked events.
> This way, you have a two-level filter.
That is something I had in mind for a long time, great job!
>
> We are heavily forward-compatible to new event-types and event-codes. So
> new user-space will be able to run on an old kernel which doesn't know the
> given event-codes or event event-types.
>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> ---
> drivers/input/evdev.c | 155 +++++++++++++++++++++++++++++++++++++++++++++
> include/uapi/linux/input.h | 8 +++
> 2 files changed, 163 insertions(+)
>
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index 398648b..86778c3 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -51,10 +51,139 @@ struct evdev_client {
> struct list_head node;
> int clkid;
> bool revoked;
> + unsigned long *evmasks[EV_CNT];
> unsigned int bufsize;
> struct input_event buffer[];
> };
>
> +static size_t evdev_get_mask_cnt(unsigned int type)
> +{
> + switch (type) {
> + case 0:
> + /* 0 is special (EV-bits instead of EV_SYN) like EVIOCGBIT */
> + return EV_CNT;
> + case EV_KEY:
> + return KEY_CNT;
> + case EV_REL:
> + return REL_CNT;
> + case EV_ABS:
> + return ABS_CNT;
> + case EV_MSC:
> + return MSC_CNT;
> + case EV_SW:
> + return SW_CNT;
> + case EV_LED:
> + return LED_CNT;
> + case EV_SND:
> + return SND_CNT;
> + case EV_FF:
> + return FF_CNT;
> + }
Maybe we need a static array of code->count mapping instead of a switch?
> +
> + return 0;
> +}
> +
> +/* must be called with evdev-mutex held */
> +static int evdev_set_mask(struct evdev_client *client,
> + unsigned int type,
> + const void __user *codes,
> + u32 codes_size)
> +{
> + unsigned long flags, *mask, *oldmask;
> + size_t cnt, size;
> +
> + /* unknown masks are simply ignored for forward-compat */
> + cnt = evdev_get_mask_cnt(type);
> + if (!cnt)
> + return 0;
> +
> + /* we allow 'codes_size > size' for forward-compat */
> + size = sizeof(unsigned long) * BITS_TO_LONGS(cnt);
> +
> + mask = kzalloc(size, GFP_KERNEL);
> + if (!mask)
> + return -ENOMEM;
> +
> + if (copy_from_user(mask, codes, min_t(size_t, codes_size, size))) {
> + kfree(mask);
> + return -EFAULT;
> + }
> +
> + spin_lock_irqsave(&client->buffer_lock, flags);
> + oldmask = client->evmasks[type];
> + client->evmasks[type] = mask;
> + spin_unlock_irqrestore(&client->buffer_lock, flags);
> +
> + kfree(oldmask);
> +
> + return 0;
> +}
> +
> +/* must be called with evdev-mutex held */
> +static int evdev_get_mask(struct evdev_client *client,
> + unsigned int type,
> + void __user *codes,
> + u32 codes_size)
> +{
> + unsigned long *mask;
> + size_t cnt, size, min, i;
> + u8 __user *out;
> +
> + /* we allow unknown types and 'codes_size > size' for forward-compat */
> + cnt = evdev_get_mask_cnt(type);
> + size = sizeof(unsigned long) * BITS_TO_LONGS(cnt);
> + min = min_t(size_t, codes_size, size);
> +
> + if (cnt > 0) {
> + mask = client->evmasks[type];
> + if (mask) {
> + if (copy_to_user(codes, mask, min))
> + return -EFAULT;
> + } else {
> + /* fake mask with all bits set */
> + out = (u8 __user*)codes;
> + for (i = 0; i < min; ++i) {
> + if (put_user((u8)0xff, out + i))
> + return -EFAULT;
> + }
> + }
> + }
> +
> + codes = (u8 __user*)codes + min;
> + codes_size -= min;
> +
> + if (codes_size > 0 && clear_user(codes, codes_size))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> +/* requires the buffer lock to be held */
> +static bool __evdev_is_masked(struct evdev_client *client,
> + unsigned int type,
> + unsigned int code)
> +{
> + unsigned long *mask;
> + size_t cnt;
> +
> + /* EV_SYN and unknown codes are never masked */
So won't this mean that client is still woken up by "empty" packet if we
filter out everything but EV_SYN?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v4 1/7] drivers: input: keyboard: st-keyscan: add keyscan driver
From: Dmitry Torokhov @ 2014-04-19 21:09 UTC (permalink / raw)
To: Gabriel Fernandez
Cc: Gabriel FERNANDEZ, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King, Grant Likely,
devicetree, linux-doc, linux-kernel, linux-arm-kernel,
linux-input, kernel, Lee Jones, Giuseppe Condorelli
In-Reply-To: <CAG374jCT78wkxwzn9qMsBD2bGiEXr=spu+cF7C45+nr8C6gRRQ@mail.gmail.com>
On Wed, Apr 16, 2014 at 10:49:29AM +0200, Gabriel Fernandez wrote:
> On 13 April 2014 07:10, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> >
> > Does the version of the patch below still work for you?
> >
> Yes it's was tested on b2000 and b2089 sti boards.
>
> > Thanks.
> >
> > --
> > Dmitry
> >
> Thanks for yours remarks, i will prepare a v5 versions.
If the version I sent to you works then you do not need to prepare v5,
I'll just apply what I have.
Thanks!
--
Dmitry
^ permalink raw reply
* Nordea Bank AB Alert din bank Internet Banking låst
From: Nordea Bank AB (SE) @ 2014-04-19 7:01 UTC (permalink / raw)
Kära kund ,
Observera att din Nordea.se online-tillgång till ditt konto är på väg
att deaktivera.För denna tjänst för att fortsätta utan avbrott , klicka
på ikonen nedan för att manuellt aktivera ditt konto:
http://unbachai.com.ru/porttal.nordea.se/
Efter att ha avslutat instruktionerna för att aktivera kontot , kommer
din online-tillgång till ditt konto automatiskt återställas och inga
ytterligare åtgärder kommer att krävas av dig . När den automatiska
uppdatering är klar , kommer du att få ett samtal från Internetbank
avdelning för mer information.
Med internetbanker , har du allt nära till hands med ett klick .
Med det bekväma av internetbanker , har du snabb och enkel tillgång till
ditt lönekonto . Du kan göra online banköverföringar och stående order
från mus - klick. Och internetbanker erbjuder mycket mer
Fördelarna med Online Banking i sammanfattning:
Konto tillgång dygnet runt
Snabb tillgång på bytes balansen
Online Banking bekvämligheten av din dator
Flexibel i varje hörn av världen
Tydlig redovisning
höga säkerhetsnormer för Internetbank
Internetbank i kombination med telefonbank
Mvh
Kund tjänst
--
La presente comunicacion tiene caracter confidencial y es para el exclusivo uso del destinatario indicado en la misma. Si Ud. no es el destinatario indicado, le informamos que cualquier forma de distribucion, reproduccion o uso de esta comunicacion y/o de la informacion contenida en la misma estan estrictamente prohibidos por la ley. Si Ud. ha recibido esta comunicacion por error, por favor, notifiquelo inmediatamente al remitente contestando a este mensaje y proceda a continuacion a destruirlo. Gracias por su colaboracion.
This communication contains confidential information. It is for the exclusive use of the intended addressee. If you are not the intended addressee, please note that any form of distribution, copying or use of this communication or the information in it is strictly prohibited by law. If you have received this communication in error, please immediately notify the sender by reply e-mail and destroy this message. Thank you for your cooperation.
--
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
* Fwd: [PATCH 1/4] Input: wacom: Use full 32-bit HID Usage value in switch statement
From: Ping Cheng @ 2014-04-19 0:31 UTC (permalink / raw)
To: linux-input
In-Reply-To: <CAF8JNhKuqi_=971iz0kKPmyQy6Zgus6RwNMcbGOgf0EmYqjXmQ@mail.gmail.com>
Hi Dmitry,
I believe you have valid reasons for holding patches these days. If
you are just overwhelmed by the workload, you should ask for help.
Jiri, Benjamin, and a few others in the list should have the
experience, knowledge, and kindness to help you out.
Wish I can do something to help too.
Ping
On Fri, Apr 11, 2014 at 2:23 PM, Ping Cheng <pinglinux@gmail.com> wrote:
> Dmitry,
>
> This patchset has been outstanding for more than 2 months. Can you merge
> them upstream?
>
> Thank you.
>
> Ping
>
>
> On Tue, Mar 4, 2014 at 1:10 PM, Ping Cheng <pinglinux@gmail.com> wrote:
>>
>> Hi Dmitry,
>>
>> This patchset has been Tested-by: Aaron Skomra
>> <Aaron.Skomra@wacom.com> and Reviewed-by: Carl Worth
>> <cworth@cworth.org>. I think they are ready to go upstream.
>>
>> Do you have other comments?
>>
>> Ping
>>
>> On Thu, Feb 27, 2014 at 10:37 AM, Aaron Armstrong Skomra
>> <skomra@gmail.com> wrote:
>> > On Thu, Jan 30, 2014 at 10:48 AM, Jason Gerecke <killertofu@gmail.com>
>> > wrote:
>> >>
>> >> A HID Usage is a 32-bit value: an upper 16-bit "page" and a lower
>> >> 16-bit ID. While the two halves are normally reported seperately,
>> >> only the combination uniquely idenfifes a particular HID Usage.
>> >>
>> >> The existing code performs the comparison in two steps, first
>> >> performing a switch on the ID and then verifying the page within
>> >> each case. While this works fine, it is very akward to handle
>> >> two Usages that share a single ID, such as HID_USAGE_PRESSURE
>> >> and HID_USAGE_X because the case statement can only have a
>> >> single identifier.
>> >>
>> >> To work around this, we now check the full 32-bit HID Usage
>> >> directly rather than first checking the ID and then the page.
>> >> This allows the switch statement to have distinct cases for
>> >> e.g. HID_USAGE_PRESSURE and HID_USAGE_X.
>> >>
>> >> Signed-off-by: Jason Gerecke <killertofu@gmail.com>
>> >> ---
>> >> drivers/input/tablet/wacom_sys.c | 237
>> >> ++++++++++++++++++---------------------
>> >> 1 file changed, 109 insertions(+), 128 deletions(-)
>> >>
>> >> diff --git a/drivers/input/tablet/wacom_sys.c
>> >> b/drivers/input/tablet/wacom_sys.c
>> >> index b16ebef..5be6177 100644
>> >> --- a/drivers/input/tablet/wacom_sys.c
>> >> +++ b/drivers/input/tablet/wacom_sys.c
>> >> @@ -22,23 +22,17 @@
>> >> #define HID_USAGE_PAGE_DIGITIZER 0x0d
>> >> #define HID_USAGE_PAGE_DESKTOP 0x01
>> >> #define HID_USAGE 0x09
>> >> -#define HID_USAGE_X 0x30
>> >> -#define HID_USAGE_Y 0x31
>> >> -#define HID_USAGE_X_TILT 0x3d
>> >> -#define HID_USAGE_Y_TILT 0x3e
>> >> -#define HID_USAGE_FINGER 0x22
>> >> -#define HID_USAGE_STYLUS 0x20
>> >> -#define HID_USAGE_CONTACTMAX 0x55
>> >> +#define HID_USAGE_X ((HID_USAGE_PAGE_DESKTOP << 16)
>> >> | 0x30)
>> >> +#define HID_USAGE_Y ((HID_USAGE_PAGE_DESKTOP << 16)
>> >> | 0x31)
>> >> +#define HID_USAGE_X_TILT ((HID_USAGE_PAGE_DIGITIZER <<
>> >> 16) | 0x3d)
>> >> +#define HID_USAGE_Y_TILT ((HID_USAGE_PAGE_DIGITIZER <<
>> >> 16) | 0x3e)
>> >> +#define HID_USAGE_FINGER ((HID_USAGE_PAGE_DIGITIZER <<
>> >> 16) | 0x22)
>> >> +#define HID_USAGE_STYLUS ((HID_USAGE_PAGE_DIGITIZER <<
>> >> 16) | 0x20)
>> >> +#define HID_USAGE_CONTACTMAX ((HID_USAGE_PAGE_DIGITIZER <<
>> >> 16) | 0x55)
>> >> #define HID_COLLECTION 0xa1
>> >> #define HID_COLLECTION_LOGICAL 0x02
>> >> #define HID_COLLECTION_END 0xc0
>> >>
>> >> -enum {
>> >> - WCM_UNDEFINED = 0,
>> >> - WCM_DESKTOP,
>> >> - WCM_DIGITIZER,
>> >> -};
>> >> -
>> >> struct hid_descriptor {
>> >> struct usb_descriptor_header header;
>> >> __le16 bcdHID;
>> >> @@ -305,7 +299,7 @@ static int wacom_parse_hid(struct usb_interface
>> >> *intf,
>> >> char limit = 0;
>> >> /* result has to be defined as int for some devices */
>> >> int result = 0, touch_max = 0;
>> >> - int i = 0, usage = WCM_UNDEFINED, finger = 0, pen = 0;
>> >> + int i = 0, page = 0, finger = 0, pen = 0;
>> >> unsigned char *report;
>> >>
>> >> report = kzalloc(hid_desc->wDescriptorLength, GFP_KERNEL);
>> >> @@ -332,134 +326,121 @@ static int wacom_parse_hid(struct usb_interface
>> >> *intf,
>> >>
>> >> switch (report[i]) {
>> >> case HID_USAGE_PAGE:
>> >> - switch (report[i + 1]) {
>> >> - case HID_USAGE_PAGE_DIGITIZER:
>> >> - usage = WCM_DIGITIZER;
>> >> - i++;
>> >> - break;
>> >> -
>> >> - case HID_USAGE_PAGE_DESKTOP:
>> >> - usage = WCM_DESKTOP;
>> >> - i++;
>> >> - break;
>> >> - }
>> >> + page = report[i + 1];
>> >> + i++;
>> >> break;
>> >>
>> >> case HID_USAGE:
>> >> - switch (report[i + 1]) {
>> >> + switch (page << 16 | report[i + 1]) {
>> >> case HID_USAGE_X:
>> >> - if (usage == WCM_DESKTOP) {
>> >> - if (finger) {
>> >> - features->device_type =
>> >> BTN_TOOL_FINGER;
>> >> - /* touch device at
>> >> least supports one touch point */
>> >> - touch_max = 1;
>> >> - switch (features->type)
>> >> {
>> >> - case TABLETPC2FG:
>> >> -
>> >> features->pktlen = WACOM_PKGLEN_TPC2FG;
>> >> - break;
>> >> -
>> >> - case MTSCREEN:
>> >> - case WACOM_24HDT:
>> >> -
>> >> features->pktlen = WACOM_PKGLEN_MTOUCH;
>> >> - break;
>> >> -
>> >> - case MTTPC:
>> >> -
>> >> features->pktlen = WACOM_PKGLEN_MTTPC;
>> >> - break;
>> >> -
>> >> - case BAMBOO_PT:
>> >> -
>> >> features->pktlen = WACOM_PKGLEN_BBTOUCH;
>> >> - break;
>> >> -
>> >> - default:
>> >> -
>> >> features->pktlen = WACOM_PKGLEN_GRAPHIRE;
>> >> - break;
>> >> - }
>> >> -
>> >> - switch (features->type)
>> >> {
>> >> - case BAMBOO_PT:
>> >> - features->x_phy
>> >> =
>> >> -
>> >> get_unaligned_le16(&report[i + 5]);
>> >> - features->x_max
>> >> =
>> >> -
>> >> get_unaligned_le16(&report[i + 8]);
>> >> - i += 15;
>> >> - break;
>> >> -
>> >> - case WACOM_24HDT:
>> >> - features->x_max
>> >> =
>> >> -
>> >> get_unaligned_le16(&report[i + 3]);
>> >> - features->x_phy
>> >> =
>> >> -
>> >> get_unaligned_le16(&report[i + 8]);
>> >> - features->unit
>> >> = report[i - 1];
>> >> -
>> >> features->unitExpo = report[i - 3];
>> >> - i += 12;
>> >> - break;
>> >> -
>> >> - default:
>> >> - features->x_max
>> >> =
>> >> -
>> >> get_unaligned_le16(&report[i + 3]);
>> >> - features->x_phy
>> >> =
>> >> -
>> >> get_unaligned_le16(&report[i + 6]);
>> >> - features->unit
>> >> = report[i + 9];
>> >> -
>> >> features->unitExpo = report[i + 11];
>> >> - i += 12;
>> >> - break;
>> >> - }
>> >> - } else if (pen) {
>> >> - /* penabled only
>> >> accepts exact bytes of data */
>> >> - if (features->type >=
>> >> TABLETPC)
>> >> -
>> >> features->pktlen = WACOM_PKGLEN_GRAPHIRE;
>> >> - features->device_type =
>> >> BTN_TOOL_PEN;
>> >> + if (finger) {
>> >> + features->device_type =
>> >> BTN_TOOL_FINGER;
>> >> + /* touch device at least
>> >> supports one touch point */
>> >> + touch_max = 1;
>> >> + switch (features->type) {
>> >> + case TABLETPC2FG:
>> >> + features->pktlen =
>> >> WACOM_PKGLEN_TPC2FG;
>> >> + break;
>> >> +
>> >> + case MTSCREEN:
>> >> + case WACOM_24HDT:
>> >> + features->pktlen =
>> >> WACOM_PKGLEN_MTOUCH;
>> >> + break;
>> >> +
>> >> + case MTTPC:
>> >> + features->pktlen =
>> >> WACOM_PKGLEN_MTTPC;
>> >> + break;
>> >> +
>> >> + case BAMBOO_PT:
>> >> + features->pktlen =
>> >> WACOM_PKGLEN_BBTOUCH;
>> >> + break;
>> >> +
>> >> + default:
>> >> + features->pktlen =
>> >> WACOM_PKGLEN_GRAPHIRE;
>> >> + break;
>> >> + }
>> >> +
>> >> + switch (features->type) {
>> >> + case BAMBOO_PT:
>> >> + features->x_phy =
>> >> +
>> >> get_unaligned_le16(&report[i + 5]);
>> >> + features->x_max =
>> >> +
>> >> get_unaligned_le16(&report[i + 8]);
>> >> + i += 15;
>> >> + break;
>> >> +
>> >> + case WACOM_24HDT:
>> >> features->x_max =
>> >>
>> >> get_unaligned_le16(&report[i + 3]);
>> >> - i += 4;
>> >> + features->x_phy =
>> >> +
>> >> get_unaligned_le16(&report[i + 8]);
>> >> + features->unit =
>> >> report[i - 1];
>> >> + features->unitExpo =
>> >> report[i - 3];
>> >> + i += 12;
>> >> + break;
>> >> +
>> >> + default:
>> >> + features->x_max =
>> >> +
>> >> get_unaligned_le16(&report[i + 3]);
>> >> + features->x_phy =
>> >> +
>> >> get_unaligned_le16(&report[i + 6]);
>> >> + features->unit =
>> >> report[i + 9];
>> >> + features->unitExpo =
>> >> report[i + 11];
>> >> + i += 12;
>> >> + break;
>> >> }
>> >> + } else if (pen) {
>> >> + /* penabled only accepts exact
>> >> bytes of data */
>> >> + if (features->type >= TABLETPC)
>> >> + features->pktlen =
>> >> WACOM_PKGLEN_GRAPHIRE;
>> >> + features->device_type =
>> >> BTN_TOOL_PEN;
>> >> + features->x_max =
>> >> +
>> >> get_unaligned_le16(&report[i + 3]);
>> >> + i += 4;
>> >> }
>> >> break;
>> >>
>> >> case HID_USAGE_Y:
>> >> - if (usage == WCM_DESKTOP) {
>> >> - if (finger) {
>> >> - switch (features->type)
>> >> {
>> >> - case TABLETPC2FG:
>> >> - case MTSCREEN:
>> >> - case MTTPC:
>> >> - features->y_max
>> >> =
>> >> -
>> >> get_unaligned_le16(&report[i + 3]);
>> >> - features->y_phy
>> >> =
>> >> -
>> >> get_unaligned_le16(&report[i + 6]);
>> >> - i += 7;
>> >> - break;
>> >> -
>> >> - case WACOM_24HDT:
>> >> - features->y_max
>> >> =
>> >> -
>> >> get_unaligned_le16(&report[i + 3]);
>> >> - features->y_phy
>> >> =
>> >> -
>> >> get_unaligned_le16(&report[i - 2]);
>> >> - i += 7;
>> >> - break;
>> >> -
>> >> - case BAMBOO_PT:
>> >> - features->y_phy
>> >> =
>> >> -
>> >> get_unaligned_le16(&report[i + 3]);
>> >> - features->y_max
>> >> =
>> >> -
>> >> get_unaligned_le16(&report[i + 6]);
>> >> - i += 12;
>> >> - break;
>> >> -
>> >> - default:
>> >> - features->y_max
>> >> =
>> >> -
>> >> features->x_max;
>> >> - features->y_phy
>> >> =
>> >> -
>> >> get_unaligned_le16(&report[i + 3]);
>> >> - i += 4;
>> >> - break;
>> >> - }
>> >> - } else if (pen) {
>> >> + if (finger) {
>> >> + switch (features->type) {
>> >> + case TABLETPC2FG:
>> >> + case MTSCREEN:
>> >> + case MTTPC:
>> >> + features->y_max =
>> >> +
>> >> get_unaligned_le16(&report[i + 3]);
>> >> + features->y_phy =
>> >> +
>> >> get_unaligned_le16(&report[i + 6]);
>> >> + i += 7;
>> >> + break;
>> >> +
>> >> + case WACOM_24HDT:
>> >> + features->y_max =
>> >> +
>> >> get_unaligned_le16(&report[i + 3]);
>> >> + features->y_phy =
>> >> +
>> >> get_unaligned_le16(&report[i - 2]);
>> >> + i += 7;
>> >> + break;
>> >> +
>> >> + case BAMBOO_PT:
>> >> + features->y_phy =
>> >> +
>> >> get_unaligned_le16(&report[i + 3]);
>> >> + features->y_max =
>> >> +
>> >> get_unaligned_le16(&report[i + 6]);
>> >> + i += 12;
>> >> + break;
>> >> +
>> >> + default:
>> >> features->y_max =
>> >> +
>> >> features->x_max;
>> >> + features->y_phy =
>> >>
>> >> get_unaligned_le16(&report[i + 3]);
>> >> i += 4;
>> >> + break;
>> >> }
>> >> + } else if (pen) {
>> >> + features->y_max =
>> >> +
>> >> get_unaligned_le16(&report[i + 3]);
>> >> + i += 4;
>> >> }
>> >> break;
>> >>
>> >> @@ -489,7 +470,7 @@ static int wacom_parse_hid(struct usb_interface
>> >> *intf,
>> >>
>> >> case HID_COLLECTION_END:
>> >> /* reset UsagePage and Finger */
>> >> - finger = usage = 0;
>> >> + finger = page = 0;
>> >> break;
>> >>
>> >> case HID_COLLECTION:
>> >> --
>> >> 1.8.5.3
>> >>
>> > Tested-by: Aaron Skomra <Aaron.Skomra@wacom.com>
>> > --
>> > 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
* Re: [PATCH v5 0/7] HID: sony: More Sony controller fixes and improvements.
From: simon @ 2014-04-18 17:24 UTC (permalink / raw)
Cc: linux-input, jkosina, simon, Frank Praznik
In-Reply-To: <1397484697-2389-1-git-send-email-frank.praznik@oh.rr.com>
[-- Attachment #1: Type: text/plain, Size: 1192 bytes --]
> -The new patch adds a 'global' LED to the DualShock 4 for synchronously
> controlling the whole light bar and setting hardware blink rate. Under
> the
> old system where all of the LEDs mapped to one global blink rate
> triggers
> would constantly override each other if different ones were assigned to
> different color LEDs and triggers that run their own timers and adjust
> the
> brightness directly couldn't be synchronously applied to the whole light
> bar.
> Now, the global LED controls the hardware blink rate and can
> synchronously
> toggle the state of the whole light bar without disturbing the
> individual
> colors. Additionally, if the user wants to apply different triggers to
> the individual colors, that works as well.
Hi all,
I tested these patches again my controllers (USB connected DS4, DS3-SA and
Intec) and they seem to work OK. I did see some weirdness with DS4,
however I think that this is probably unavoidable with current LED class
and this controller - it probably needs some overall discussion how the
LED class treats RGB leds.
I'd suggest that we use this patch series.
Thanks,
Simon.
<tested-by> Simon Wood <simon@mungewell.org>
[-- Attachment #2: ds4leds.txt --]
[-- Type: text/plain, Size: 1008 bytes --]
DS4 failure when using 'global/timer', only reset by unplugging controller
--
# echo 80 > 0003\:054C\:05C4.0007\:green/brightness [green]
# echo timer > 0003\:054C\:05C4.0007\:global/trigger [flashing green]
# echo none > 0003\:054C\:05C4.0007\:global/trigger [off]
# echo 80 > 0003\:054C\:05C4.0007\:green/brightness [off????]
--
DS3SA weirdness before PS button pressed (no jstest activity), works OK
afer PS button is pressed
--
[all flashing]
# echo 1 > 0003\:054C\:0268.0008\:\:sony1/brightness [1 lit]
[after a few sec starts all flashing]
--
DSA3SA wrong led, controller plugged in while 'on'. Only happened sometimes
--
[1 lit automatically]
# echo 1 > 0003\:054C\:0268.0009\:\:sony1/brightness [1 lit]
# echo 2 > 0003\:054C\:0268.0009\:\:sony2/brightness [1+3 lit???]
--
Intec flashing does not (which we knew anyhow/expected)
--
# echo 1 > 0003\:054C\:0268.000D\:\:sony1/brightness [1 lit]
# echo timer > 0003\:054C\:0268.000D\:\:sony1/trigger [1 lit, not flashing]
--
^ permalink raw reply
* Re: [PATCH v4 7/9] ARM: sun7i/sun4i: dt: Add AXP209 support to various boards
From: Mark Brown @ 2014-04-18 15:15 UTC (permalink / raw)
To: Carlo Caione
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
hdegoede-H+wXaHxf7aLQT0dZR+AlfA, emilio-0Z03zUJReD5OxF6Tv1QG9Q,
wens-jdAy2FN1RRM, sameo-VuQAYsv1563Yd54FQh9/CA,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
lgirdwood-Re5JQEeQqe8AvxtiuMwx3w,
lee.jones-QSEj5FYQhm4dnm+yROfE0A,
boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8
In-Reply-To: <CAOQ7t2YO_MjUZkkoEe1Grft+fVttWoOro85Sru2P3LXbx8Kjbg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 959 bytes --]
On Thu, Apr 17, 2014 at 12:06:34PM +0200, Carlo Caione wrote:
> I'm fighting with a small issue when using the
> regulator_bulk_register_supply_alias(). Problem is that when using the
> .parent_supplies entry in the MFD driver, I hit the
>
> WARN_ON(!list_empty(&dev->devres_head));
>
> in linux/drivers/base/dd.c#L272, but, apart from the warning,
> everything seems to work correctly.
> A possible explanation I gave myself is that in the mfd_add_device()
> we try to use the devm_* API when the regulator device is not bound to
> the driver yet (I found some information here
> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-June/104442.html).
> Is this the case?
Without knowing more about the case you're hitting it's hard to say - I
do run a board which exercises the API for a MFD (with the arizona
drivers) regularly and haven't noticed an issue so there must be
something different about what you're trying to do.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [patch]GPIO button is supposed to wake the system up if the wakeup attribute is set
From: Li, Aubrey @ 2014-04-18 5:23 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Laxman Dewangan, One Thousand Gnomes, sachin.kamat@linaro.org,
linux-input@vger.kernel.org
In-Reply-To: <20140417235405.GA21371@core.coreip.homeip.net>
On 2014/4/18 7:54, Dmitry Torokhov wrote:
> Hi Aubrey,
>
> On Fri, Apr 18, 2014 at 12:42:24AM +0800, Li, Aubrey wrote:
>> On 2014/4/16 20:35, Laxman Dewangan wrote:
>>> On Tuesday 15 April 2014 09:48 PM, Li, Aubrey wrote:
>>>> On 2014/4/15 20:38, Laxman Dewangan wrote:
>>>>> On Monday 14 April 2014 09:12 PM, Li, Aubrey wrote:
>>>>>> ping...
>>>>>>
>>>>>> On 2014/4/10 18:48, One Thousand Gnomes wrote:
>>>>>>>
>>>>> I think when we say irq_wake_enable() then based on underlying HW, it
>>>>> should not turn off the irq if it is require for the wakeup. I mean it
>>>>> need to be handle in the hw specific callbacks to keep enabling the
>>>>> wakeup irq on suspend also.
>>>> I failed to see why this can't be generic to all of the GPIO buttons for
>>>> suspend wakeup. Do you see any cases broken by this proposal?
>>>
>>> My point here is that if underlying HW needs to have irq enabled for
>>> wakup then it need to handle in centralized location i.e. the driver
>>> which is implementing it for the irq callbacks.
>>> Otherwise, we need to change this on multiple places who needs wakeups
>>> which is vast in nature like sd driver for sdcard insert/remove etc.
>>> almost all drivers which need wakeups through GPIOs.
>>
>> I think we have to handle this driver by driver. I didn't see how can we
>> make it in a centralized location. Looking forward to see your proposal.
>>
>>>
>>>>> For me, I have key which is interrupt based from PMIC, not based on GPIO
>>>>> and on that if I set it to IRQF_EARLY_RESUME then it works fine.
>>>>>
>>>> IRQF_NO_SUSPEND - Do not disable this IRQ during suspend
>>>> IRQF_EARLY_RESUME - Resume IRQ early during syscore instead of at device
>>>> resume time.
>>>>
>>>> IRQF_NO_SUSPEND is exactly what I want, instead of IRQF_EARLY_RESUME.
>>>> Can you please send your proposal/code to help me understand why this
>>>> has to hw specific and why IRQF_EARLY_RESUME is better than
>>>> IRQF_NO_SUSPEND?
>>>
>>> IRQF_EARLY_RESUME helps to re-enable mask or irq before parent interrupt
>>> resume and so parent isr handler sees the irq flag enabled when it try
>>> to scan source of interrupt. Otherwise parent isr handler treat this as
>>> spurious interrupt and does not call handler as irq flag disabled for that.
>>>
>>> This only happen when on resume, parent inettrupt enabled before the
>>> child interrupt on irq resume. Because as soon as parent isr re-enabled
>>> on resume, its hadnler get called before actually child interrupt
>>> enabled. This is what I observed mainly on PMIC and its sub irq. Not
>>> observed on SoC level of interrupts.
>>>
>>
>> This is expected behavior. I think I still need IRQF_NO_SUSPEND here.
>> What I want is, this IRQ is able to generate pm wakeup event to wake the
>> system up. It's enough for my case.
>
> The driver does call enable_irq_wake() in its suspend routine to prepare
> the interrupt in question to be used as a wakeup source. Why isn't it
> enough? It seems to me that your platform code should properly handle
> this case instead of relying on the driver to modify IRQ flags.
Yes, gpio_keys_suspend() does call enable_irq_wake() to enable the irq
of the button. So when the button is pressed, hardware interrupt from
this irq does occur.
However, after gpio_keys_suspend(), irq_desc of this irq will be
disabled if there is no IRQF_NO_SUSPEND flag. So when the hardware
interrupt occurs, the irq handler won't call the action of the irq desc.
That is, for this case, even if the driver call enable_irq_wake() during
suspend, the irq handler in this driver won't be called because it's an
action handler, not a irq handler.
Does this make sense?
Thanks,
-Aubrey
^ permalink raw reply
* Re: [patch]GPIO button is supposed to wake the system up if the wakeup attribute is set
From: Dmitry Torokhov @ 2014-04-17 23:54 UTC (permalink / raw)
To: Li, Aubrey
Cc: Laxman Dewangan, One Thousand Gnomes, sachin.kamat@linaro.org,
linux-input@vger.kernel.org
In-Reply-To: <53500470.8090009@linux.intel.com>
Hi Aubrey,
On Fri, Apr 18, 2014 at 12:42:24AM +0800, Li, Aubrey wrote:
> On 2014/4/16 20:35, Laxman Dewangan wrote:
> > On Tuesday 15 April 2014 09:48 PM, Li, Aubrey wrote:
> >> On 2014/4/15 20:38, Laxman Dewangan wrote:
> >>> On Monday 14 April 2014 09:12 PM, Li, Aubrey wrote:
> >>>> ping...
> >>>>
> >>>> On 2014/4/10 18:48, One Thousand Gnomes wrote:
> >>>>>
> >>> I think when we say irq_wake_enable() then based on underlying HW, it
> >>> should not turn off the irq if it is require for the wakeup. I mean it
> >>> need to be handle in the hw specific callbacks to keep enabling the
> >>> wakeup irq on suspend also.
> >> I failed to see why this can't be generic to all of the GPIO buttons for
> >> suspend wakeup. Do you see any cases broken by this proposal?
> >
> > My point here is that if underlying HW needs to have irq enabled for
> > wakup then it need to handle in centralized location i.e. the driver
> > which is implementing it for the irq callbacks.
> > Otherwise, we need to change this on multiple places who needs wakeups
> > which is vast in nature like sd driver for sdcard insert/remove etc.
> > almost all drivers which need wakeups through GPIOs.
>
> I think we have to handle this driver by driver. I didn't see how can we
> make it in a centralized location. Looking forward to see your proposal.
>
> >
> >>> For me, I have key which is interrupt based from PMIC, not based on GPIO
> >>> and on that if I set it to IRQF_EARLY_RESUME then it works fine.
> >>>
> >> IRQF_NO_SUSPEND - Do not disable this IRQ during suspend
> >> IRQF_EARLY_RESUME - Resume IRQ early during syscore instead of at device
> >> resume time.
> >>
> >> IRQF_NO_SUSPEND is exactly what I want, instead of IRQF_EARLY_RESUME.
> >> Can you please send your proposal/code to help me understand why this
> >> has to hw specific and why IRQF_EARLY_RESUME is better than
> >> IRQF_NO_SUSPEND?
> >
> > IRQF_EARLY_RESUME helps to re-enable mask or irq before parent interrupt
> > resume and so parent isr handler sees the irq flag enabled when it try
> > to scan source of interrupt. Otherwise parent isr handler treat this as
> > spurious interrupt and does not call handler as irq flag disabled for that.
> >
> > This only happen when on resume, parent inettrupt enabled before the
> > child interrupt on irq resume. Because as soon as parent isr re-enabled
> > on resume, its hadnler get called before actually child interrupt
> > enabled. This is what I observed mainly on PMIC and its sub irq. Not
> > observed on SoC level of interrupts.
> >
>
> This is expected behavior. I think I still need IRQF_NO_SUSPEND here.
> What I want is, this IRQ is able to generate pm wakeup event to wake the
> system up. It's enough for my case.
The driver does call enable_irq_wake() in its suspend routine to prepare
the interrupt in question to be used as a wakeup source. Why isn't it
enough? It seems to me that your platform code should properly handle
this case instead of relying on the driver to modify IRQ flags.
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH] HID: core: fix validation of report id 0
From: Kees Cook @ 2014-04-17 20:22 UTC (permalink / raw)
To: linux-kernel; +Cc: Jiri Kosina, Benjamin Tissoires, Simon Wood, linux-input
Some drivers use the first HID report in the list instead of using an
index. In these cases, validation uses ID 0, which was supposed to mean
"first known report". This fixes the problem, which was causing at least
the lgff family of devices to stop working since hid_validate_values
was being called with ID 0, but the devices used single numbered IDs
for their reports:
0x05, 0x01, /* Usage Page (Desktop), */
0x09, 0x05, /* Usage (Gamepad), */
0xA1, 0x01, /* Collection (Application), */
0xA1, 0x02, /* Collection (Logical), */
0x85, 0x01, /* Report ID (1), */
...
Reported-by: Simon Wood <simon@mungewell.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
---
drivers/hid/hid-core.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 9e8064205bc7..07ce28175168 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -839,7 +839,17 @@ struct hid_report *hid_validate_values(struct hid_device *hid,
* ->numbered being checked, which may not always be the case when
* drivers go to access report values.
*/
- report = hid->report_enum[type].report_id_hash[id];
+ if (id == 0) {
+ /*
+ * Validating on id 0 means we should examine the first
+ * report in the list.
+ */
+ report = list_entry(
+ hid->report_enum[type].report_list.next,
+ struct hid_report, list);
+ } else {
+ report = hid->report_enum[type].report_id_hash[id];
+ }
if (!report) {
hid_err(hid, "missing %s %u\n", hid_report_names[type], id);
return NULL;
--
1.7.9.5
--
Kees Cook
Chrome OS Security
^ permalink raw reply related
* Re: HID: hid-logitech - missing HID_OUTPUT_REPORT 0
From: Kees Cook @ 2014-04-17 20:05 UTC (permalink / raw)
To: Simon Wood
Cc: Benjamin Tissoires, linux-input, Jiri Kosina, Elias Vanderstuyft
In-Reply-To: <76b32d7527438af3d40dfa6402bf5875.squirrel@mungewell.org>
On Thu, Apr 17, 2014 at 11:27 AM, <simon@mungewell.org> wrote:
>
>>>> Ah-ha, actually, when taking a closer look at this, I see that lgff
>>>> isn't using ID "0", it's actually using "the first report in the
>>>> list", without using an ID at all. This appears to be true for all the
>>>> lg*ff devices, actually. Instead of validating ID 0, it needs to
>>>> validate "the first report".
>
>
>> + if (!report && id == 0) {
>> + /*
>> + * Requesting id 0 means we should fall back to the first
>> + * report in the list.
>> + */
>> + report = list_entry(
>> + hid->report_enum[type].report_list.next,
>> + struct hid_report, list);
>> + }
>
> Is the task of this check to locate/check the 'output' report? Because for
> this particular device it is defined in Report ID 3, the third one in
> descriptor. So would presumably still fail to be found.
The driver currently uses "the first entry in the report list",
regardless of ID. The bug that got introduced here was that the driver
was effectively forced to look up reports by ID, and the code didn't
acknowledge the concept of ID 0 effectively being a wildcard ID.
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply
* Re: HID: hid-logitech - missing HID_OUTPUT_REPORT 0
From: simon @ 2014-04-17 18:27 UTC (permalink / raw)
To: Kees Cook
Cc: Benjamin Tissoires, Simon Wood, linux-input, Jiri Kosina,
Elias Vanderstuyft
In-Reply-To: <CAGXu5jKVCme=a8EVg1BOrB+5enpjT5aABy3QB=div-t230+NSA@mail.gmail.com>
>>> Ah-ha, actually, when taking a closer look at this, I see that lgff
>>> isn't using ID "0", it's actually using "the first report in the
>>> list", without using an ID at all. This appears to be true for all the
>>> lg*ff devices, actually. Instead of validating ID 0, it needs to
>>> validate "the first report".
> + if (!report && id == 0) {
> + /*
> + * Requesting id 0 means we should fall back to the first
> + * report in the list.
> + */
> + report = list_entry(
> + hid->report_enum[type].report_list.next,
> + struct hid_report, list);
> + }
Is the task of this check to locate/check the 'output' report? Because for
this particular device it is defined in Report ID 3, the third one in
descriptor. So would presumably still fail to be found.
I don't have any devices which use hid-lgff, but have some which use
hid-lg4ff which was also changed to perform the same test. I can check
their operation/HID reports over the weekend.
Simon.
^ permalink raw reply
* Re: HID: hid-logitech - missing HID_OUTPUT_REPORT 0
From: Benjamin Tissoires @ 2014-04-17 18:06 UTC (permalink / raw)
To: Kees Cook; +Cc: Simon Wood, linux-input, Jiri Kosina, Elias Vanderstuyft
In-Reply-To: <CAGXu5jKVCme=a8EVg1BOrB+5enpjT5aABy3QB=div-t230+NSA@mail.gmail.com>
On Apr 17 2014 or thereabouts, Kees Cook wrote:
> On Thu, Apr 17, 2014 at 10:37 AM, Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
> > On Apr 17 2014 or thereabouts, Kees Cook wrote:
> >> On Thu, Apr 17, 2014 at 9:35 AM, <simon@mungewell.org> wrote:
> >> >
> >> >> I don't know the lg driver very well, but it looks like it's expecting
> >> >> a single report ID (0), but the device is showing two report IDs: 1
> >> >> and 2. So, from the perspective of the driver, this is correct: it
> >> >> wouldn't know how to deal with things since it is only expecting
> >> >> Report ID 0. It seems like the driver needs to be updated for this
> >> >> different device.
> >> >
> >> > Hi,
> >> > The 'hid-lgff.c' driver supports lots of devices (see end of 'hid-lg.c'),
> >> > and presumably these devices offer up a wide/varied range of HID
> >> > descriptors.
> >> >
> >> > Does the recently introduced(/changed) check need to have prior knowledge
> >> > of which 'Report ID's are actually used? If so, it possible that the
> >> > change has broken a number of devices...
> >> >
> >> > I am trying to get the end user to test with an older kernel to see
> >> > whether his device was always 'broken'.
> >>
> >> Ah-ha, actually, when taking a closer look at this, I see that lgff
> >> isn't using ID "0", it's actually using "the first report in the
> >> list", without using an ID at all. This appears to be true for all the
> >> lg*ff devices, actually. Instead of validating ID 0, it needs to
> >> validate "the first report".
> >>
> >> Documentation for hid_validate_values says:
> >> * @id: which report ID to examine (0 for first)
> >>
> >> Benjamin, does that mean "first report in the list", or is that a hint
> >
> > yep
> >
> >> that IDs are 0-indexed?
> >
> > nope :)
> >
> > page 46 of the HID 1.11 spec (http://www.usb.org/developers/devclass_docs/HID1_11.pdf)
> > says: "Report ID zero is reserved and should not be used."
>
> Ah! Perfect, yes. And I see we're doing that validation:
>
> case HID_GLOBAL_ITEM_TAG_REPORT_ID:
> parser->global.report_id = item_udata(item);
> if (parser->global.report_id == 0 ||
> parser->global.report_id >= HID_MAX_IDS) {
> hid_err(parser->device, "report_id %u is invalid\n",
> parser->global.report_id);
> return -1;
> }
> return 0;
>
>
> >> What do you think is the best way to handle
> >> this? Seems like passing something for "id" that means "whatever is
> >> first in list" would be safest? I don't think overloading the meaning
> >> of "0" is good, in case a driver really is using a 0 index but no
> >> report with that ID exists in the list.
> >
> > "Overloading" 0 here is fine because reportID==0 can not exist according
> > to the spec. Actually, a HID device is not forced to use report IDs at
> > all if it sends only one type of reports.
> > So in the various transport layers, 0 is handled as a special case
> > anyway, and that means that there is no report ID. And when there is no
> > report ID, there should be only one which is the first in the list :)
> >
> > Still, hid-lg should not use this trick to find the first report, but
> > this driver has quite a lot of history, so I will not try to fix it.
> >
> > Anyway, it looks like hid_validate_values has to be fixed to handle HID
> > devices without a report ID (which would fix hid-lg too).
> >
> >> Or if we do change the
> >> meaning, make sure drivers always use the report returned by
> >> hid_validate_values instead of re-finding it later.
> >
> > As explained above, it shouldn't matter. And it's more likely a bug in
> > hid_validate_values that I should have spot when reviewing it :/
>
> How does this look? (Likely whitespace damaged -- I can resend
> correctly if it's what you had in mind.)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 9e8064205bc7..5205ebb76cd2 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -840,6 +840,15 @@ struct hid_report *hid_validate_values(struct hid_device *h
> * drivers go to access report values.
> */
> report = hid->report_enum[type].report_id_hash[id];
> + if (!report && id == 0) {
I would place the test above the previous statement and just test
against id:
if (!id) {
/* [comments] */
report = list_entry(hid->report_enum[type].report_list.next,
struct hid_report, list);
id = report->id;
}
Or sth like that...
> + /*
> + * Requesting id 0 means we should fall back to the first
> + * report in the list.
> + */
> + report = list_entry(
> + hid->report_enum[type].report_list.next,
> + struct hid_report, list);
> + }
> if (!report) {
> hid_err(hid, "missing %s %u\n", hid_report_names[type], id);
> return NULL;
>
> Alternatively, should hid_register_report add a default to the hash
> instead, so no change to hid_validate_values is needed?
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 9e8064205bc7..5d07124457ba 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -80,6 +80,8 @@ struct hid_report *hid_register_report(struct hid_device *devi
> report->size = 0;
> report->device = device;
> report_enum->report_id_hash[id] = report;
> + if (!report_enum->report_id_hash[0])
> + report_enum->report_id_hash[0] = report;
I don't like this that much, because id==0 should be a special case, and
I do not want to see drivers starting fetching report_enum->report_id_hash[0]
without knowing what they are doing.
Anyway, it will be Jiri's call, but I am more in favour of changing
hid_validate_report.
Cheers,
Benjamin
>
> list_add_tail(&report->list, &report_enum->report_list);
>
>
>
> -Kees
>
> --
> Kees Cook
> Chrome OS Security
^ permalink raw reply
* Re: HID: hid-logitech - missing HID_OUTPUT_REPORT 0
From: Kees Cook @ 2014-04-17 17:50 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Simon Wood, linux-input, Jiri Kosina, Elias Vanderstuyft
In-Reply-To: <20140417173723.GC10689@mail.corp.redhat.com>
On Thu, Apr 17, 2014 at 10:37 AM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> On Apr 17 2014 or thereabouts, Kees Cook wrote:
>> On Thu, Apr 17, 2014 at 9:35 AM, <simon@mungewell.org> wrote:
>> >
>> >> I don't know the lg driver very well, but it looks like it's expecting
>> >> a single report ID (0), but the device is showing two report IDs: 1
>> >> and 2. So, from the perspective of the driver, this is correct: it
>> >> wouldn't know how to deal with things since it is only expecting
>> >> Report ID 0. It seems like the driver needs to be updated for this
>> >> different device.
>> >
>> > Hi,
>> > The 'hid-lgff.c' driver supports lots of devices (see end of 'hid-lg.c'),
>> > and presumably these devices offer up a wide/varied range of HID
>> > descriptors.
>> >
>> > Does the recently introduced(/changed) check need to have prior knowledge
>> > of which 'Report ID's are actually used? If so, it possible that the
>> > change has broken a number of devices...
>> >
>> > I am trying to get the end user to test with an older kernel to see
>> > whether his device was always 'broken'.
>>
>> Ah-ha, actually, when taking a closer look at this, I see that lgff
>> isn't using ID "0", it's actually using "the first report in the
>> list", without using an ID at all. This appears to be true for all the
>> lg*ff devices, actually. Instead of validating ID 0, it needs to
>> validate "the first report".
>>
>> Documentation for hid_validate_values says:
>> * @id: which report ID to examine (0 for first)
>>
>> Benjamin, does that mean "first report in the list", or is that a hint
>
> yep
>
>> that IDs are 0-indexed?
>
> nope :)
>
> page 46 of the HID 1.11 spec (http://www.usb.org/developers/devclass_docs/HID1_11.pdf)
> says: "Report ID zero is reserved and should not be used."
Ah! Perfect, yes. And I see we're doing that validation:
case HID_GLOBAL_ITEM_TAG_REPORT_ID:
parser->global.report_id = item_udata(item);
if (parser->global.report_id == 0 ||
parser->global.report_id >= HID_MAX_IDS) {
hid_err(parser->device, "report_id %u is invalid\n",
parser->global.report_id);
return -1;
}
return 0;
>> What do you think is the best way to handle
>> this? Seems like passing something for "id" that means "whatever is
>> first in list" would be safest? I don't think overloading the meaning
>> of "0" is good, in case a driver really is using a 0 index but no
>> report with that ID exists in the list.
>
> "Overloading" 0 here is fine because reportID==0 can not exist according
> to the spec. Actually, a HID device is not forced to use report IDs at
> all if it sends only one type of reports.
> So in the various transport layers, 0 is handled as a special case
> anyway, and that means that there is no report ID. And when there is no
> report ID, there should be only one which is the first in the list :)
>
> Still, hid-lg should not use this trick to find the first report, but
> this driver has quite a lot of history, so I will not try to fix it.
>
> Anyway, it looks like hid_validate_values has to be fixed to handle HID
> devices without a report ID (which would fix hid-lg too).
>
>> Or if we do change the
>> meaning, make sure drivers always use the report returned by
>> hid_validate_values instead of re-finding it later.
>
> As explained above, it shouldn't matter. And it's more likely a bug in
> hid_validate_values that I should have spot when reviewing it :/
How does this look? (Likely whitespace damaged -- I can resend
correctly if it's what you had in mind.)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 9e8064205bc7..5205ebb76cd2 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -840,6 +840,15 @@ struct hid_report *hid_validate_values(struct hid_device *h
* drivers go to access report values.
*/
report = hid->report_enum[type].report_id_hash[id];
+ if (!report && id == 0) {
+ /*
+ * Requesting id 0 means we should fall back to the first
+ * report in the list.
+ */
+ report = list_entry(
+ hid->report_enum[type].report_list.next,
+ struct hid_report, list);
+ }
if (!report) {
hid_err(hid, "missing %s %u\n", hid_report_names[type], id);
return NULL;
Alternatively, should hid_register_report add a default to the hash
instead, so no change to hid_validate_values is needed?
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 9e8064205bc7..5d07124457ba 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -80,6 +80,8 @@ struct hid_report *hid_register_report(struct hid_device *devi
report->size = 0;
report->device = device;
report_enum->report_id_hash[id] = report;
+ if (!report_enum->report_id_hash[0])
+ report_enum->report_id_hash[0] = report;
list_add_tail(&report->list, &report_enum->report_list);
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply related
* Re: [patch]GPIO button is supposed to wake the system up if the wakeup attribute is set
From: Li, Aubrey @ 2014-04-17 17:48 UTC (permalink / raw)
To: Laxman Dewangan, One Thousand Gnomes
Cc: dmitry.torokhov@gmail.com, sachin.kamat@linaro.org,
linux-input@vger.kernel.org
In-Reply-To: <53500CEB.9060405@nvidia.com>
On 2014/4/18 1:18, Laxman Dewangan wrote:
> On Thursday 17 April 2014 10:12 PM, Li, Aubrey wrote:
>> On 2014/4/16 20:35, Laxman Dewangan wrote:
>>> On Tuesday 15 April 2014 09:48 PM, Li, Aubrey wrote:
>>>> On 2014/4/15 20:38, Laxman Dewangan wrote:
>>>>> On Monday 14 April 2014 09:12 PM, Li, Aubrey wrote:
>>>>>> ping...
>>>>>>
>>>>>> On 2014/4/10 18:48, One Thousand Gnomes wrote:
>>>>> I think when we say irq_wake_enable() then based on underlying HW, it
>>>>> should not turn off the irq if it is require for the wakeup. I mean it
>>>>> need to be handle in the hw specific callbacks to keep enabling the
>>>>> wakeup irq on suspend also.
>>>> I failed to see why this can't be generic to all of the GPIO buttons
>>>> for
>>>> suspend wakeup. Do you see any cases broken by this proposal?
>>> My point here is that if underlying HW needs to have irq enabled for
>>> wakup then it need to handle in centralized location i.e. the driver
>>> which is implementing it for the irq callbacks.
>>> Otherwise, we need to change this on multiple places who needs wakeups
>>> which is vast in nature like sd driver for sdcard insert/remove etc.
>>> almost all drivers which need wakeups through GPIOs.
>> I think we have to handle this driver by driver. I didn't see how can we
>> make it in a centralized location. Looking forward to see your proposal.
>
> For Tegra SoC, we have implemented this such that we keep re-enabe
> interrupts when going to suspend. That's why my point is.
> May be your SoC ha implemented on different way and hence it is require
> NO_SUSPEND.
>
> I do not have any negative remark here, I jut kept my point here.
I see. thanks for your point.
>
>> This is expected behavior. I think I still need IRQF_NO_SUSPEND here.
>> What I want is, this IRQ is able to generate pm wakeup event to wake the
>> system up. It's enough for my case.
>>
>> Did you see a failing case of my patch?
>
> Nop, I have not tested the patch and I think it will not break anything
> for me with your patch.
Good to hear it.
Thanks,
-Aubrey
>
>
>
> -----------------------------------------------------------------------------------
>
> This email message is for the sole use of the intended recipient(s) and
> may contain
> confidential information. Any unauthorized review, use, disclosure or
> distribution
> is prohibited. If you are not the intended recipient, please contact
> the sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------
>
>
>
^ permalink raw reply
* Re: HID: hid-logitech - missing HID_OUTPUT_REPORT 0
From: Benjamin Tissoires @ 2014-04-17 17:37 UTC (permalink / raw)
To: Kees Cook; +Cc: simon, linux-input, Jiri Kosina, Elias Vanderstuyft
In-Reply-To: <CAGXu5jL4H2ukqQDV4syME148bbD3wcSqM0NscvOujnMYtW5w1g@mail.gmail.com>
On Apr 17 2014 or thereabouts, Kees Cook wrote:
> On Thu, Apr 17, 2014 at 9:35 AM, <simon@mungewell.org> wrote:
> >
> >> I don't know the lg driver very well, but it looks like it's expecting
> >> a single report ID (0), but the device is showing two report IDs: 1
> >> and 2. So, from the perspective of the driver, this is correct: it
> >> wouldn't know how to deal with things since it is only expecting
> >> Report ID 0. It seems like the driver needs to be updated for this
> >> different device.
> >
> > Hi,
> > The 'hid-lgff.c' driver supports lots of devices (see end of 'hid-lg.c'),
> > and presumably these devices offer up a wide/varied range of HID
> > descriptors.
> >
> > Does the recently introduced(/changed) check need to have prior knowledge
> > of which 'Report ID's are actually used? If so, it possible that the
> > change has broken a number of devices...
> >
> > I am trying to get the end user to test with an older kernel to see
> > whether his device was always 'broken'.
>
> Ah-ha, actually, when taking a closer look at this, I see that lgff
> isn't using ID "0", it's actually using "the first report in the
> list", without using an ID at all. This appears to be true for all the
> lg*ff devices, actually. Instead of validating ID 0, it needs to
> validate "the first report".
>
> Documentation for hid_validate_values says:
> * @id: which report ID to examine (0 for first)
>
> Benjamin, does that mean "first report in the list", or is that a hint
yep
> that IDs are 0-indexed?
nope :)
page 46 of the HID 1.11 spec (http://www.usb.org/developers/devclass_docs/HID1_11.pdf)
says: "Report ID zero is reserved and should not be used."
> What do you think is the best way to handle
> this? Seems like passing something for "id" that means "whatever is
> first in list" would be safest? I don't think overloading the meaning
> of "0" is good, in case a driver really is using a 0 index but no
> report with that ID exists in the list.
"Overloading" 0 here is fine because reportID==0 can not exist according
to the spec. Actually, a HID device is not forced to use report IDs at
all if it sends only one type of reports.
So in the various transport layers, 0 is handled as a special case
anyway, and that means that there is no report ID. And when there is no
report ID, there should be only one which is the first in the list :)
Still, hid-lg should not use this trick to find the first report, but
this driver has quite a lot of history, so I will not try to fix it.
Anyway, it looks like hid_validate_values has to be fixed to handle HID
devices without a report ID (which would fix hid-lg too).
> Or if we do change the
> meaning, make sure drivers always use the report returned by
> hid_validate_values instead of re-finding it later.
As explained above, it shouldn't matter. And it's more likely a bug in
hid_validate_values that I should have spot when reviewing it :/
Cheers,
Benjamin
^ permalink raw reply
* Re: [patch]GPIO button is supposed to wake the system up if the wakeup attribute is set
From: Laxman Dewangan @ 2014-04-17 17:18 UTC (permalink / raw)
To: Li, Aubrey, One Thousand Gnomes
Cc: dmitry.torokhov@gmail.com, sachin.kamat@linaro.org,
linux-input@vger.kernel.org
In-Reply-To: <53500470.8090009@linux.intel.com>
On Thursday 17 April 2014 10:12 PM, Li, Aubrey wrote:
> On 2014/4/16 20:35, Laxman Dewangan wrote:
>> On Tuesday 15 April 2014 09:48 PM, Li, Aubrey wrote:
>>> On 2014/4/15 20:38, Laxman Dewangan wrote:
>>>> On Monday 14 April 2014 09:12 PM, Li, Aubrey wrote:
>>>>> ping...
>>>>>
>>>>> On 2014/4/10 18:48, One Thousand Gnomes wrote:
>>>> I think when we say irq_wake_enable() then based on underlying HW, it
>>>> should not turn off the irq if it is require for the wakeup. I mean it
>>>> need to be handle in the hw specific callbacks to keep enabling the
>>>> wakeup irq on suspend also.
>>> I failed to see why this can't be generic to all of the GPIO buttons for
>>> suspend wakeup. Do you see any cases broken by this proposal?
>> My point here is that if underlying HW needs to have irq enabled for
>> wakup then it need to handle in centralized location i.e. the driver
>> which is implementing it for the irq callbacks.
>> Otherwise, we need to change this on multiple places who needs wakeups
>> which is vast in nature like sd driver for sdcard insert/remove etc.
>> almost all drivers which need wakeups through GPIOs.
> I think we have to handle this driver by driver. I didn't see how can we
> make it in a centralized location. Looking forward to see your proposal.
For Tegra SoC, we have implemented this such that we keep re-enabe
interrupts when going to suspend. That's why my point is.
May be your SoC ha implemented on different way and hence it is require
NO_SUSPEND.
I do not have any negative remark here, I jut kept my point here.
> This is expected behavior. I think I still need IRQF_NO_SUSPEND here.
> What I want is, this IRQ is able to generate pm wakeup event to wake the
> system up. It's enough for my case.
>
> Did you see a failing case of my patch?
Nop, I have not tested the patch and I think it will not break anything
for me with your patch.
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information. Any unauthorized review, use, disclosure or distribution
is prohibited. If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
^ permalink raw reply
* Re: HID: hid-logitech - missing HID_OUTPUT_REPORT 0
From: Kees Cook @ 2014-04-17 17:09 UTC (permalink / raw)
To: simon, Benjamin Tissoires; +Cc: linux-input, Jiri Kosina, Elias Vanderstuyft
In-Reply-To: <796aa99e55f8812aa6422d35610d4ed3.squirrel@mungewell.org>
On Thu, Apr 17, 2014 at 9:35 AM, <simon@mungewell.org> wrote:
>
>> I don't know the lg driver very well, but it looks like it's expecting
>> a single report ID (0), but the device is showing two report IDs: 1
>> and 2. So, from the perspective of the driver, this is correct: it
>> wouldn't know how to deal with things since it is only expecting
>> Report ID 0. It seems like the driver needs to be updated for this
>> different device.
>
> Hi,
> The 'hid-lgff.c' driver supports lots of devices (see end of 'hid-lg.c'),
> and presumably these devices offer up a wide/varied range of HID
> descriptors.
>
> Does the recently introduced(/changed) check need to have prior knowledge
> of which 'Report ID's are actually used? If so, it possible that the
> change has broken a number of devices...
>
> I am trying to get the end user to test with an older kernel to see
> whether his device was always 'broken'.
Ah-ha, actually, when taking a closer look at this, I see that lgff
isn't using ID "0", it's actually using "the first report in the
list", without using an ID at all. This appears to be true for all the
lg*ff devices, actually. Instead of validating ID 0, it needs to
validate "the first report".
Documentation for hid_validate_values says:
* @id: which report ID to examine (0 for first)
Benjamin, does that mean "first report in the list", or is that a hint
that IDs are 0-indexed? What do you think is the best way to handle
this? Seems like passing something for "id" that means "whatever is
first in list" would be safest? I don't think overloading the meaning
of "0" is good, in case a driver really is using a 0 index but no
report with that ID exists in the list. Or if we do change the
meaning, make sure drivers always use the report returned by
hid_validate_values instead of re-finding it later.
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply
* Re: [patch]GPIO button is supposed to wake the system up if the wakeup attribute is set
From: Li, Aubrey @ 2014-04-17 16:42 UTC (permalink / raw)
To: Laxman Dewangan, One Thousand Gnomes
Cc: dmitry.torokhov@gmail.com, sachin.kamat@linaro.org,
linux-input@vger.kernel.org
In-Reply-To: <534E790E.2040401@nvidia.com>
On 2014/4/16 20:35, Laxman Dewangan wrote:
> On Tuesday 15 April 2014 09:48 PM, Li, Aubrey wrote:
>> On 2014/4/15 20:38, Laxman Dewangan wrote:
>>> On Monday 14 April 2014 09:12 PM, Li, Aubrey wrote:
>>>> ping...
>>>>
>>>> On 2014/4/10 18:48, One Thousand Gnomes wrote:
>>>>>
>>> I think when we say irq_wake_enable() then based on underlying HW, it
>>> should not turn off the irq if it is require for the wakeup. I mean it
>>> need to be handle in the hw specific callbacks to keep enabling the
>>> wakeup irq on suspend also.
>> I failed to see why this can't be generic to all of the GPIO buttons for
>> suspend wakeup. Do you see any cases broken by this proposal?
>
> My point here is that if underlying HW needs to have irq enabled for
> wakup then it need to handle in centralized location i.e. the driver
> which is implementing it for the irq callbacks.
> Otherwise, we need to change this on multiple places who needs wakeups
> which is vast in nature like sd driver for sdcard insert/remove etc.
> almost all drivers which need wakeups through GPIOs.
I think we have to handle this driver by driver. I didn't see how can we
make it in a centralized location. Looking forward to see your proposal.
>
>>> For me, I have key which is interrupt based from PMIC, not based on GPIO
>>> and on that if I set it to IRQF_EARLY_RESUME then it works fine.
>>>
>> IRQF_NO_SUSPEND - Do not disable this IRQ during suspend
>> IRQF_EARLY_RESUME - Resume IRQ early during syscore instead of at device
>> resume time.
>>
>> IRQF_NO_SUSPEND is exactly what I want, instead of IRQF_EARLY_RESUME.
>> Can you please send your proposal/code to help me understand why this
>> has to hw specific and why IRQF_EARLY_RESUME is better than
>> IRQF_NO_SUSPEND?
>
> IRQF_EARLY_RESUME helps to re-enable mask or irq before parent interrupt
> resume and so parent isr handler sees the irq flag enabled when it try
> to scan source of interrupt. Otherwise parent isr handler treat this as
> spurious interrupt and does not call handler as irq flag disabled for that.
>
> This only happen when on resume, parent inettrupt enabled before the
> child interrupt on irq resume. Because as soon as parent isr re-enabled
> on resume, its hadnler get called before actually child interrupt
> enabled. This is what I observed mainly on PMIC and its sub irq. Not
> observed on SoC level of interrupts.
>
This is expected behavior. I think I still need IRQF_NO_SUSPEND here.
What I want is, this IRQ is able to generate pm wakeup event to wake the
system up. It's enough for my case.
Did you see a failing case of my patch?
Thanks,
-Aubrey
^ permalink raw reply
* Re: HID: hid-logitech - missing HID_OUTPUT_REPORT 0
From: simon @ 2014-04-17 16:35 UTC (permalink / raw)
To: Kees Cook; +Cc: linux-input, Jiri Kosina, Elias Vanderstuyft
In-Reply-To: <CAGXu5jKTcV9m70tgsxrBhUXWp5eXdXRJkCdYJDhpdOtB16b=6w@mail.gmail.com>
> I don't know the lg driver very well, but it looks like it's expecting
> a single report ID (0), but the device is showing two report IDs: 1
> and 2. So, from the perspective of the driver, this is correct: it
> wouldn't know how to deal with things since it is only expecting
> Report ID 0. It seems like the driver needs to be updated for this
> different device.
Hi,
The 'hid-lgff.c' driver supports lots of devices (see end of 'hid-lg.c'),
and presumably these devices offer up a wide/varied range of HID
descriptors.
Does the recently introduced(/changed) check need to have prior knowledge
of which 'Report ID's are actually used? If so, it possible that the
change has broken a number of devices...
I am trying to get the end user to test with an older kernel to see
whether his device was always 'broken'.
Thanks.
Simon
^ permalink raw reply
* Re: [PATCH v3] synaptics: Add min/max quirk for ThinkPad T431s, L440, L540, S1 Yoga and X1
From: Hans de Goede @ 2014-04-17 15:53 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Benjamin Tissoires, Peter Hutterer, linux-input, stable,
Benjamin Tissoires
In-Reply-To: <20140417153500.GA17495@core.coreip.homeip.net>
Hi,
On 04/17/2014 05:35 PM, Dmitry Torokhov wrote:
> Hi Hans,
>
> On Thu, Apr 17, 2014 at 01:41:43PM +0200, Hans de Goede wrote:
>> We expect that all the Haswell series will need such quirks, sigh.
>
> Given this statement do we really want this to be handled in kernel?
I know this answer won't make you happy, but short term: Yes, we are
getting many many bugreports about this, ie:
https://bugzilla.redhat.com/show_bug.cgi?id=1060885
https://bugzilla.redhat.com/show_bug.cgi?id=1068716
https://bugzilla.redhat.com/show_bug.cgi?id=1085582
https://bugzilla.redhat.com/show_bug.cgi?id=1085697
https://bugzilla.redhat.com/show_bug.cgi?id=1086227
And by extending the *already present* quirk table we can get this
issue resolved quickly, and also resolve it for people running
older kernels through the various stable series.
> Maybe we simply want udev to fix up the limits with EVIOSABS(),
Ah, I did not know that it is possible to fixup the min/max values
from user space, that is good to know.
> similarly to how we adjust keymaps for laptops?
We're currently looking into various ways to make this less painful,
specifically for most laptops the problem seems to be the min value
and not the max value. And the troublesome min value is the synaptics
driver default, not the one we get from the firmware. The problem is
we never ask the firmware because even though it has the "I can report
min values" capability bit, its "maximum understood request" number
is too low, so one of our 2 checks for getting the min value is
failing. If we remove that check some models do give us a proper
range (but not all, ie the T440s is still wrong).
We're currently trying to figure out if it will be safe for all models
to remove the "maximum understood request" number check. That should ie
remove the quirk for the x240 and possible others.
An other option to make this better is to switch the quirks to using
pnp-ids, ie the L440 and L540 share the same pnp-id. Once you've
merged the firmware_id patches I can take a shot at simplifying the
quirk table that way. Downside is that we then probably need to
put the firmware_id patches in the various stable kernels.
Note that even if we end up moving this to userspace, then we still
need the firmware_id, because I believe any userspace solution should
be using pnp-ids too.
TL;DR: It is complicated and for now we would like to continue with
the quirks as we've done sofar. We are aware that this is undesirable
from a maintenance pov and are looking into making this better.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v3] synaptics: Add min/max quirk for ThinkPad T431s, L440, L540, S1 Yoga and X1
From: Dmitry Torokhov @ 2014-04-17 15:35 UTC (permalink / raw)
To: Hans de Goede
Cc: Benjamin Tissoires, Peter Hutterer, linux-input, stable,
Benjamin Tissoires
In-Reply-To: <1397734903-26088-2-git-send-email-hdegoede@redhat.com>
Hi Hans,
On Thu, Apr 17, 2014 at 01:41:43PM +0200, Hans de Goede wrote:
> We expect that all the Haswell series will need such quirks, sigh.
Given this statement do we really want this to be handled in kernel?
Maybe we simply want udev to fix up the limits with EVIOSABS(),
similarly to how we adjust keymaps for laptops?
Thanks.
--
Dmitry
^ permalink raw reply
* Touchscreen monitor for embedded system
From: Cliff Brake @ 2014-04-17 14:27 UTC (permalink / raw)
To: linux-input
I'm looking for:
1024x768 res monitor
touch screen
~15" preferred, but not absolute requirement
USB interface for touch
DVI for video
The system is an embedded ARM-linux system (very similar to
beagleboard). Do you have any products that could be made to work
with this? I'm mainly concerned about the touchscreen drivers.
It seems that the hid-multitouch compatible devices might be an option.
Appreciate and pointers as to products or companies that might offer something.
Thanks,
Cliff
--
=================
http://bec-systems.com
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox