Hi, On 02/05/2015 10:22 PM, Benjamin Tissoires wrote: > On Sat, Jan 31, 2015 at 3:18 AM, Daniel Martin <consume.noise@xxxxxxxxx> wrote: >> 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 <hdegoede@xxxxxxxxxx> wrote: >>>> Hi, >>>> >>>> On 01/29/2015 08:50 PM, Benjamin Tissoires wrote: >>>>> On Thu, Jan 29, 2015 at 2:02 PM, Benjamin Tissoires >>>>> <benjamin.tissoires@xxxxxxxxx> wrote: >>>>>> Hi Daniel, >>>>>> >>>>>> On Tue, Jan 27, 2015 at 3:34 AM, Daniel Martin >>>>>> <daniel.martin@xxxxxxxxxxx> wrote: >>>>>>> From: Daniel Martin <consume.noise@xxxxxxxxx> >>>>>>> >>>>>>> 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 <consume.noise@xxxxxxxxx> >>>>>>> --- >>>>>>> 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. > > IIRC, Andrew said in a private mail that extracting this kind of > information is quite tricky. > > I would say that we should add a per-pnp_id board limit with the data we know. > > I think you could add this in the struct min_max_quirk, and if the > max_board_id is > 0, we check against it. > This way, we could have a finer grain when dealing with the hardware refreshes. ACK, I agree that this is the best way forward with this. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
