Hi Dmitry,
Thanks for the review. Comments in-line.
On Wed, Mar 27, 2013 at 10:32 PM, Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> wrote:
> Hi Shawn,
>
> On Tue, Mar 26, 2013 at 12:36:41PM -0700, Shawn Nematbakhsh wrote:
>> The trackpoint driver sets various parameter default values, all of
>> which happen to be power-on defaults (Source: IBM TrackPoint Engineering
>> Specification, Version 4.0. Also confirmed by empirical data).
>>
>> By sending the power-on reset command to reset all parameters to
>> power-on state, we can skip the lengthy process of programming all
>> parameters. In testing, ~2.5 secs of time writing parameters was reduced
>> to .35 seconds waiting for power-on reset to complete.
>>
>> Signed-off-by: Shawn Nematbakhsh <shawnn@xxxxxxxxxxxx>
>> ---
>> drivers/input/mouse/trackpoint.c | 223 +++++++++++++++++++++++++--------------
>> drivers/input/mouse/trackpoint.h | 7 +-
>> 2 files changed, 149 insertions(+), 81 deletions(-)
>>
>> diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
>> index f310249..17e9b36 100644
>> --- a/drivers/input/mouse/trackpoint.c
>> +++ b/drivers/input/mouse/trackpoint.c
>> @@ -20,6 +20,26 @@
>> #include "trackpoint.h"
>>
>> /*
>> + * Power-on Reset: Resets all trackpoint parameters, including RAM values,
>> + * to defaults.
>> + * Returns zero on success, non-zero on failure.
>> + */
>> +static int trackpoint_power_on_reset(struct ps2dev *ps2dev)
>> +{
>> + unsigned char results[2];
>> +
>> + if (ps2_command(ps2dev, NULL, MAKE_PS2_CMD(0, 0, TP_COMMAND)) ||
>> + ps2_command(ps2dev, results, MAKE_PS2_CMD(0, 2, TP_POR))) {
>> + return -1;
>> + }
>> +
>> + /* POR response should be 0xAA00 or 0xFC00 */
>> + if ((results[0] != 0xAA && results[0] != 0xFC) || results[1] != 0x00)
>> + return -1;
>
> I am a bit concerned about this. 0xfc00 indicates POR failure. In this
> case shouldn't we try to reset the device, issue another POR and bail if
> it fails again?
>
Yes, you are correct. I just now posted v2 to fix this. Now, on
0xfc00, we attempt one retry. On subsequent 0xfc00, we fail POR and
attempt to set parameters manually.
>>
>> static struct attribute *trackpoint_attrs[] = {
>> &psmouse_attr_sensitivity.dattr.attr,
>> @@ -179,6 +241,9 @@ static struct attribute *trackpoint_attrs[] = {
>> &psmouse_attr_press_to_select.dattr.attr,
>> &psmouse_attr_skipback.dattr.attr,
>> &psmouse_attr_ext_dev.dattr.attr,
>> + &psmouse_attr_twohand.dattr.attr,
>> + &psmouse_attr_source_tag.dattr.attr,
>> + &psmouse_attr_mb.dattr.attr,
>
> What is the benefit of adding these 3 new attributes?
>
Previously, these attributes were handled in a special way -- the
corresponding TP values were set to default, but the attributes were
not exported. Now, they are set to default and exported. It makes for
cleaner code.
I see no good use for these attributes other than driver hacking. I
can remove them if you think it's best.
> Thanks.
>
> --
> Dmitry
Thanks,
Shawn
--
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
