linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: Jiri Kosina <jikos@kernel.org>,
	Mario.Limonciello@dell.com,
	Peter Hutterer <peter.hutterer@who-t.net>,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/9] Input: mt - export MT_TOOL in input_mt_init_slot()
Date: Tue, 29 May 2018 11:21:07 -0700	[thread overview]
Message-ID: <20180529182107.GC167092@dtor-ws> (raw)
In-Reply-To: <20180529095800.13504-2-benjamin.tissoires@redhat.com>

Hi Benjamin,

On Tue, May 29, 2018 at 11:57:52AM +0200, Benjamin Tissoires wrote:
> Looks like we require users to set a tool but input_mt_init_slots() never
> set the tool bit for us. Meaning that this is a useless information the
> driver tries to forward.

I am not sure if I agree with this patch: up until now drivers could
decide if they export presence of the tool type to userspace and
userspace could distinguish between "pure" touchscreens and touchscreens
that support additional tools, such as stylus. Now we unconditionally
claim that all multi-touch devices support multiple tools.

The fact that we were dropping MT_TOOL_TYPE for devices that do not
support anything but finger I'd consider a "feature".

> 
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
>  drivers/input/input-mt.c                 | 1 +
>  drivers/input/rmi4/rmi_2d_sensor.c       | 2 --
>  drivers/input/touchscreen/atmel_mxt_ts.c | 2 --
>  drivers/input/touchscreen/hideep.c       | 2 --
>  drivers/input/touchscreen/wacom_w8001.c  | 2 --
>  5 files changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
> index a1bbec9cda8d..3900686c5d0f 100644
> --- a/drivers/input/input-mt.c
> +++ b/drivers/input/input-mt.c
> @@ -57,6 +57,7 @@ int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots,
>  	mt->flags = flags;
>  	input_set_abs_params(dev, ABS_MT_SLOT, 0, num_slots - 1, 0, 0);
>  	input_set_abs_params(dev, ABS_MT_TRACKING_ID, 0, TRKID_MAX, 0, 0);
> +	input_set_abs_params(dev, ABS_MT_TOOL_TYPE, 0, MT_TOOL_MAX, 0, 0);
>  
>  	if (flags & (INPUT_MT_POINTER | INPUT_MT_DIRECT)) {
>  		__set_bit(EV_KEY, dev->evbit);
> diff --git a/drivers/input/rmi4/rmi_2d_sensor.c b/drivers/input/rmi4/rmi_2d_sensor.c
> index 8bb866c7b985..23e666bd2539 100644
> --- a/drivers/input/rmi4/rmi_2d_sensor.c
> +++ b/drivers/input/rmi4/rmi_2d_sensor.c
> @@ -186,8 +186,6 @@ static void rmi_2d_sensor_set_input_params(struct rmi_2d_sensor *sensor)
>  		input_set_abs_params(input, ABS_MT_TOUCH_MAJOR, 0, 0x0f, 0, 0);
>  		input_set_abs_params(input, ABS_MT_TOUCH_MINOR, 0, 0x0f, 0, 0);
>  		input_set_abs_params(input, ABS_MT_ORIENTATION, 0, 1, 0, 0);
> -		input_set_abs_params(input, ABS_MT_TOOL_TYPE,
> -				     0, MT_TOOL_MAX, 0, 0);
>  
>  		if (sensor->sensor_type == rmi_sensor_touchpad)
>  			input_flags = INPUT_MT_POINTER;
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 54fe190fd4bc..0dcf48100dc1 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -2021,8 +2021,6 @@ static int mxt_initialize_input_device(struct mxt_data *data)
>  	}
>  
>  	if (data->multitouch == MXT_TOUCH_MULTITOUCHSCREEN_T100) {
> -		input_set_abs_params(input_dev, ABS_MT_TOOL_TYPE,
> -				     0, MT_TOOL_MAX, 0, 0);
>  		input_set_abs_params(input_dev, ABS_MT_DISTANCE,
>  				     MXT_DISTANCE_ACTIVE_TOUCH,
>  				     MXT_DISTANCE_HOVERING,
> diff --git a/drivers/input/touchscreen/hideep.c b/drivers/input/touchscreen/hideep.c
> index f1cd4dd9a4a3..980539d8d551 100644
> --- a/drivers/input/touchscreen/hideep.c
> +++ b/drivers/input/touchscreen/hideep.c
> @@ -799,8 +799,6 @@ static int hideep_init_input(struct hideep_ts *ts)
>  	input_set_capability(ts->input_dev, EV_ABS, ABS_MT_POSITION_Y);
>  	input_set_abs_params(ts->input_dev, ABS_MT_PRESSURE, 0, 65535, 0, 0);
>  	input_set_abs_params(ts->input_dev, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
> -	input_set_abs_params(ts->input_dev, ABS_MT_TOOL_TYPE,
> -			     0, MT_TOOL_MAX, 0, 0);
>  	touchscreen_parse_properties(ts->input_dev, true, &ts->prop);
>  
>  	if (ts->prop.max_x == 0 || ts->prop.max_y == 0) {
> diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
> index 3715d1eace92..946dca30560b 100644
> --- a/drivers/input/touchscreen/wacom_w8001.c
> +++ b/drivers/input/touchscreen/wacom_w8001.c
> @@ -529,8 +529,6 @@ static int w8001_setup_touch(struct w8001 *w8001, char *basename,
>  					0, touch.x, 0, 0);
>  		input_set_abs_params(dev, ABS_MT_POSITION_Y,
>  					0, touch.y, 0, 0);
> -		input_set_abs_params(dev, ABS_MT_TOOL_TYPE,
> -					0, MT_TOOL_MAX, 0, 0);
>  		input_abs_set_res(dev, ABS_MT_POSITION_X, touch.panel_res);
>  		input_abs_set_res(dev, ABS_MT_POSITION_Y, touch.panel_res);
>  
> -- 
> 2.14.3
> 

Thanks.

-- 
Dmitry

  reply	other threads:[~2018-05-29 18:21 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-29  9:57 [PATCH 0/9] Hid multitouch rewrite, support os system multi-axis devices Benjamin Tissoires
2018-05-29  9:57 ` [PATCH 1/9] Input: mt - export MT_TOOL in input_mt_init_slot() Benjamin Tissoires
2018-05-29 18:21   ` Dmitry Torokhov [this message]
2018-05-30  8:53     ` Benjamin Tissoires
2018-05-29  9:57 ` [PATCH 2/9] HID: multitouch: make sure the static list of class is not changed Benjamin Tissoires
2018-05-29  9:57 ` [PATCH 3/9] HID: multitouch: Store per collection multitouch data Benjamin Tissoires
2018-05-29  9:57 ` [PATCH 4/9] HID: multitouch: store a per application quirks value Benjamin Tissoires
2018-05-29  9:57 ` [PATCH 5/9] HID: multitouch: ditch mt_report_id Benjamin Tissoires
2018-05-29  9:57 ` [PATCH 6/9] HID: multitouch: remove one copy of values Benjamin Tissoires
2018-05-29  9:57 ` [PATCH 7/9] HID: input: enable Totem on the Dell Canvas 27 Benjamin Tissoires
2018-05-29  9:57 ` [PATCH 8/9] HID: core: do not upper bound the collection stack Benjamin Tissoires
2018-05-29  9:58 ` [PATCH 9/9] HID: microsoft: support the Surface Dial Benjamin Tissoires
2018-05-29 10:38 ` [PATCH 0/9] Hid multitouch rewrite, support os system multi-axis devices Florian Echtler
2018-05-29 15:06   ` Benjamin Tissoires

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180529182107.GC167092@dtor-ws \
    --to=dmitry.torokhov@gmail.com \
    --cc=Mario.Limonciello@dell.com \
    --cc=benjamin.tissoires@redhat.com \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peter.hutterer@who-t.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).