Hi Clinton,
On 02/07/2014 01:40 AM, Clinton Sprain wrote:
> Dials back the default fuzz and threshold settings for most devices using this driver, based on user input from forums and bug reports, increasing sensitivity. These two settings can also now both be overridden as module parameters in the event that users have hardware that does not respond well to the new defaults, or there is a desire to change the values for devices that do not have new defaults.
>
> Signed-off-by: Clinton Sprain <clintonsprain@xxxxxxxxx>
> ---
> drivers/input/mouse/appletouch.c | 48 ++++++++++++++++++++++++--------------
> 1 file changed, 31 insertions(+), 17 deletions(-)
Thanks for the patches, they seem like a good improvement. There are still some
comments, though, you will find them inline.
> diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
> index 800ca7d..d48181b 100644
> --- a/drivers/input/mouse/appletouch.c
> +++ b/drivers/input/mouse/appletouch.c
> @@ -48,6 +48,8 @@ struct atp_info {
> int yfact; /* Y multiplication factor */
> int datalen; /* size of USB transfers */
> void (*callback)(struct urb *); /* callback function */
> + int fuzz; /* fuzz touchpad generates */
> + int threshold; /* sensitivity threshold */
> };
>
> static void atp_complete_geyser_1_2(struct urb *urb);
> @@ -61,6 +63,8 @@ static const struct atp_info fountain_info = {
> .yfact = 43,
> .datalen = 81,
> .callback = atp_complete_geyser_1_2,
> + .fuzz = 16,
> + .threshold = 5,
> };
>
> static const struct atp_info geyser1_info = {
> @@ -71,6 +75,8 @@ static const struct atp_info geyser1_info = {
> .yfact = 43,
> .datalen = 81,
> .callback = atp_complete_geyser_1_2,
> + .fuzz = 16,
> + .threshold = 5,
> };
>
> static const struct atp_info geyser2_info = {
> @@ -81,6 +87,8 @@ static const struct atp_info geyser2_info = {
> .yfact = 43,
> .datalen = 64,
> .callback = atp_complete_geyser_1_2,
> + .fuzz = 0,
> + .threshold = 3,
> };
>
> static const struct atp_info geyser3_info = {
> @@ -90,6 +98,8 @@ static const struct atp_info geyser3_info = {
> .yfact = 64,
> .datalen = 64,
> .callback = atp_complete_geyser_3_4,
> + .fuzz = 0,
> + .threshold = 3,
> };
>
> static const struct atp_info geyser4_info = {
> @@ -99,6 +109,8 @@ static const struct atp_info geyser4_info = {
> .yfact = 64,
> .datalen = 64,
> .callback = atp_complete_geyser_3_4,
> + .fuzz = 0,
> + .threshold = 3,
> };
>
> #define ATP_DEVICE(prod, info) \
> @@ -155,18 +167,9 @@ MODULE_DEVICE_TABLE(usb, atp_table);
> #define ATP_XSENSORS 26
> #define ATP_YSENSORS 16
>
> -/* amount of fuzz this touchpad generates */
> -#define ATP_FUZZ 16
> -
> /* maximum pressure this driver will report */
> #define ATP_PRESSURE 300
>
> -/*
> - * Threshold for the touchpad sensors. Any change less than ATP_THRESHOLD is
> - * ignored.
> - */
> -#define ATP_THRESHOLD 5
> -
> /* Geyser initialization constants */
> #define ATP_GEYSER_MODE_READ_REQUEST_ID 1
> #define ATP_GEYSER_MODE_WRITE_REQUEST_ID 9
> @@ -235,14 +238,20 @@ MODULE_AUTHOR("Sven Anders");
> MODULE_DESCRIPTION("Apple PowerBook and MacBook USB touchpad driver");
> MODULE_LICENSE("GPL");
>
> -/*
> - * Make the threshold a module parameter
> - */
> -static int threshold = ATP_THRESHOLD;
> +static int threshold = -1;
> module_param(threshold, int, 0644);
> MODULE_PARM_DESC(threshold, "Discard any change in data from a sensor"
> " (the trackpad has many of these sensors)"
> - " less than this value.");
> + " less than this value. Devices have defaults;"
> + " this parameter overrides them.");
> +static int fuzz = -1;
> +
> +module_param(fuzz, int, 0644);
> +MODULE_PARM_DESC(fuzz, "Fuzz the trackpad generates. The higher"
> + " this is, the less sensitive the trackpad"
> + " will feel, but values too low may generate"
> + " random movement. Devices have defaults;"
> + " this parameter overrides them.");
Given that there already is a userland interface for the fuzz parameter - and it
is indeed in frequent use - this one does not seem warranted. If we are unsure
if the changes to the default are not all good, perhaps they should not be
changed at all?
>
> static int debug;
> module_param(debug, int, 0644);
> @@ -455,7 +464,7 @@ static void atp_detect_size(struct atp *dev)
> input_set_abs_params(dev->input, ABS_X, 0,
> (dev->info->xsensors_17 - 1) *
> dev->info->xfact - 1,
> - ATP_FUZZ, 0);
> + fuzz, 0);
> break;
> }
> }
> @@ -808,6 +817,11 @@ static int atp_probe(struct usb_interface *iface,
> dev->info = info;
> dev->overflow_warned = false;
>
> + if (fuzz < 0)
> + fuzz = dev->info->fuzz;
> + if (threshold < 0)
> + threshold = dev->info->threshold;
> +
> dev->urb = usb_alloc_urb(0, GFP_KERNEL);
> if (!dev->urb)
> goto err_free_devs;
> @@ -843,10 +857,10 @@ static int atp_probe(struct usb_interface *iface,
>
> input_set_abs_params(input_dev, ABS_X, 0,
> (dev->info->xsensors - 1) * dev->info->xfact - 1,
> - ATP_FUZZ, 0);
> + fuzz, 0);
> input_set_abs_params(input_dev, ABS_Y, 0,
> (dev->info->ysensors - 1) * dev->info->yfact - 1,
> - ATP_FUZZ, 0);
> + fuzz, 0);
> input_set_abs_params(input_dev, ABS_PRESSURE, 0, ATP_PRESSURE, 0, 0);
>
> set_bit(EV_KEY, input_dev->evbit);
>
Thanks,
Henrik
--
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
