From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Martin Subject: Re: [PATCH v2 4/5] Input: synaptics - Skip quirks when post-2013 dimensions Date: Sat, 31 Jan 2015 09:18:48 +0100 Message-ID: <20150131081848.GB27794@unplugged.fritz.box> References: <1421945201-1528-1-git-send-email-daniel.martin@secunet.com> <1422347645-5194-1-git-send-email-daniel.martin@secunet.com> <1422347645-5194-5-git-send-email-daniel.martin@secunet.com> <54CB5602.5050807@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from mail-wg0-f48.google.com ([74.125.82.48]:63965 "EHLO mail-wg0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751055AbbAaISk (ORCPT ); Sat, 31 Jan 2015 03:18:40 -0500 Received: by mail-wg0-f48.google.com with SMTP id x12so30516856wgg.7 for ; Sat, 31 Jan 2015 00:18:39 -0800 (PST) Content-Disposition: inline In-Reply-To: Sender: linux-input-owner@vger.kernel.org List-Id: linux-input@vger.kernel.org To: Benjamin Tissoires Cc: Hans de Goede , linux-input , Dmitry Torokhov , Andrew Duggan , Christopher Heiny , Peter Hutterer On Fri, Jan 30, 2015 at 10:34:22AM -0500, Benjamin Tissoires wrote: > On Fri, Jan 30, 2015 at 4:59 AM, Hans de Goede wrote: > > Hi, > > > > On 01/29/2015 08:50 PM, Benjamin Tissoires wrote: > >> On Thu, Jan 29, 2015 at 2:02 PM, Benjamin Tissoires > >> wrote: > >>> Hi Daniel, > >>> > >>> On Tue, Jan 27, 2015 at 3:34 AM, Daniel Martin > >>> wrote: > >>>> From: Daniel Martin > >>>> > >>>> If we queried min/max dimensions of x [1266..5674], y [1170..4684] we > >>>> have post-2013 model and don't need to apply any quirk. > >>>> > >>>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=91541 > >>>> Signed-off-by: Daniel Martin > >>>> --- > >>>> drivers/input/mouse/synaptics.c | 5 +++++ > >>>> 1 file changed, 5 insertions(+) > >>>> > >>>> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c > >>>> index 37d4dff..f6c43ff 100644 > >>>> --- a/drivers/input/mouse/synaptics.c > >>>> +++ b/drivers/input/mouse/synaptics.c > >>>> @@ -420,6 +420,11 @@ static int synaptics_quirks(struct psmouse *psmouse) > >>>> struct synaptics_data *priv = psmouse->private; > >>>> int i; > >>>> > >>>> + /* Post-2013 models expose correct dimensions. */ > >>>> + if (priv->x_min == 1266 && priv->x_max == 5674 && > >>>> + priv->y_min == 1170 && priv->y_max == 4684) > >>>> + return 0; > >>>> + > >>> > >>> Well, this one, I don't like it either :( > >>> > >>> At least, the test should be within the psmouse_matches_pnp_id() below > >>> to ensure we are deciding with Lenovo devices only. > >>> > >>> The other concern is hardcoding these values in the code directly. > >>> What if Synaptics/Lenovo decides to ship a new released model with > >>> proper min_max ranges but with a different offset? > >>> > >>> Andrew told us that the board ID should be enough to discriminate old > >>> and faulty touchpads from the new and valid touchpads. > >>> > >>> My concern here is that we will have to backport these changes in the > >>> various stable kernel and the various distributions. And if we do not > >>> end up with the right solution right now, that means that we will have > >>> to do the job over and over. > >>> > >>> I am quite tempted to find a solution in the userspace for that fix. > >>> Not sure I'll be able to find the right one right now, but it may > >>> worth trying. > >>> > >> > >> So, the user space solution seems difficult because we do not export > >> either the board_id or the firmware_id. So that would required to > >> update the kernel anyway, a bunch of user space tools and a hwdb... :( > >> > >> How about we just add an extra min/max in struct min_max_quirk, > >> compare the current min/max with the 2 possible values and if there is > >> a match, we do not override the values. > >> This way, we keep the crap of wrong/correct min max in the small list > >> of device we know are problematic, and if the new batch of E540 has a > >> different correct min/max range, then we will be able to adjust it > >> without breaking the other we fixed. > >> > >> Dmitry, Hans, any comments on this? > > > > I'm thinking more along the lines of adding a max_broken_board_id field > > to the quirks, and if the touchpad board_id is larger then the > > max_broken_board_id not use the quirk. > > > > Yep, this was confirmed by Synaptics that the board id will be > incremented only at each new board revision. So it should be safe to > only check for that. Could you ask your contact for an exact board id, since when the ranges have been fixed? From the data I can look at it seems to be <= 2962.