Hi, On 01/29/2015 07:48 PM, Benjamin Tissoires wrote: > Hi Daniel, > > in one hand I would like to see that upstream ASAP. But on the other > hand, I have some concerns here and there regarding this series. > > On Tue, Jan 27, 2015 at 3:34 AM, Daniel Martin > <daniel.martin@xxxxxxxxxxx> wrote: >> From: Daniel Martin <consume.noise@xxxxxxxxx> >> >> Query the min dimensions even if the check >> SYN_EXT_CAP_REQUESTS(priv->capabilities) >= 7 >> fails, but we know that the firmware version is safe. Atm. this is >> version 8.1 only. >> >> With that we don't need quirks for post-2013 models anymore as they >> expose correct min and max dimensions. >> >> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=91541 >> Signed-off-by: Daniel Martin <consume.noise@xxxxxxxxx> >> --- >> drivers/input/mouse/synaptics.c | 27 ++++++++++++++++++++++++++- >> 1 file changed, 26 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c >> index 7a78d75..37d4dff 100644 >> --- a/drivers/input/mouse/synaptics.c >> +++ b/drivers/input/mouse/synaptics.c >> @@ -333,6 +333,30 @@ static int synaptics_identify(struct psmouse *psmouse) >> return -1; >> } >> >> +struct fw_version { >> + unsigned char major; >> + unsigned char minor; >> +}; >> + >> +static const struct fw_version safe_fw_list[] = { >> + {8, 1}, /* safe for SYN_QUE_EXT_MIN_COORDS */ >> + { } >> +}; >> + >> +static bool synaptics_is_safe_fw(struct psmouse *psmouse) >> +{ >> + struct synaptics_data *priv = psmouse->private; >> + int i; >> + >> + for (i = 0; safe_fw_list[i].major; i++) { >> + if (safe_fw_list[i].major == SYN_ID_MAJOR(priv->identity) && >> + safe_fw_list[i].minor == SYN_ID_MINOR(priv->identity)) >> + return true; >> + } >> + >> + return false; >> +} > > This seems a little bit too much for just one firmware ID. > Plus the name safe_fw_list gives hope that this list can be used for > something else later, but then it may conflict with it current > purpose. > > How about we just rely on the current list of PNPIDs we already have > for this generation? > This might not be a good idea either, so an other solution would be to > directly check for firmware 8.1 in the test below. > > I'd like to hear other opinions on this however before we commit to a decision. I think that checking the firmware version, rather then relying on pnp-ids, is the right thing to do, this e.g. also gives us more accurate min/max values on the x230 / t430 series. As for using the list, rather then just the one id, I'm fine either way, the list is more future proof, which is why I acked this patch as is. But we could go with just a check for the single version now and extend this later if needed. Regards, Hans > > > Cheers, > Benjamin > > PS: I think Hans already mentioned, but if you want your patches to > land in Dmitry's tree, it's better to CC him directly when submitting > a patch. > >> + >> /* >> * Read touchpad resolution and maximum reported coordinates >> * Resolution is left zero if touchpad does not support the query >> @@ -368,7 +392,8 @@ static int synaptics_resolution(struct psmouse *psmouse) >> } >> } >> >> - if (SYN_EXT_CAP_REQUESTS(priv->capabilities) >= 7 && >> + if ((SYN_EXT_CAP_REQUESTS(priv->capabilities) >= 7 || >> + synaptics_is_safe_fw(psmouse)) && >> SYN_CAP_MIN_DIMENSIONS(priv->ext_cap_0c)) { >> if (synaptics_send_cmd(psmouse, SYN_QUE_EXT_MIN_COORDS, resp)) { >> psmouse_warn(psmouse, >> -- >> 2.2.2 >> >> -- >> 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 -- 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
