All of lore.kernel.org
 help / color / mirror / Atom feed
From: Artem Borisov <dedsa2002@gmail.com>
To: unlisted-recipients:; (no To-header on input)
Cc: jikos@kernel.org, masaki.ota@jp.alps.com,
	Artem Borisov <dedsa2002@gmail.com>,
	Benjamin Tissoires <benjamin.tissoires@redhat.com>,
	Henrik Rydberg <rydberg@bitmath.org>,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH 2/2] HID: alps: Refactor axis resolution logic
Date: Fri, 10 Apr 2020 03:00:08 +0400	[thread overview]
Message-ID: <20200409230009.22551-1-dedsa2002@gmail.com> (raw)
In-Reply-To: <nycvar.YFH.7.76.2004100019450.19713@cbobk.fhfr.pm>

AUI1657 doesn't follow the same logic for resolution calculation, since
the resulting values are incorrect. Instead, it reports the actual
resolution values in place of the pitch ones.
While we're at it, also refactor the whole resolution logic to make it more
generic and sensible for multiple device support.

There are two main logical problems with the current code:
1. active_len_mm values are only used for resolution calculation on U1,
yet are exposed globally as part of alps_dev structure.
2. The resolution calculation process happens in alps_input_configured,
while everything else is calculated in u1_init function.

These problems become more apparent when we try to support a device
that doesn't follow the same resolution calculation logic as U1.
Since alps_input_configured is a device-agnostic function, we should
avoid doing any measurements there and handle them in device-specific
init functions like u1/T4_init instead.

To eliminate these problems we add global x_res and y_res values
and populate them on a device-specific basis in the according init
functions.

Signed-off-by: Artem Borisov <dedsa2002@gmail.com>
---
 drivers/hid/hid-alps.c | 41 +++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/drivers/hid/hid-alps.c b/drivers/hid/hid-alps.c
index c2a2bd528890..494c08cca645 100644
--- a/drivers/hid/hid-alps.c
+++ b/drivers/hid/hid-alps.c
@@ -83,8 +83,8 @@ enum dev_num {
  * @max_fingers: total number of fingers
  * @has_sp: boolean of sp existense
  * @sp_btn_info: button information
- * @x_active_len_mm: active area length of X (mm)
- * @y_active_len_mm: active area length of Y (mm)
+ * @x_res: resolution of X
+ * @y_res: resolution of Y
  * @x_max: maximum x coordinate value
  * @y_max: maximum y coordinate value
  * @x_min: minimum x coordinate value
@@ -100,9 +100,10 @@ struct alps_dev {
 	enum dev_num dev_type;
 	u8  max_fingers;
 	u8  has_sp;
+	u8  no_pitch;
 	u8	sp_btn_info;
-	u32	x_active_len_mm;
-	u32	y_active_len_mm;
+	u32	x_res;
+	u32	y_res;
 	u32	x_max;
 	u32	y_max;
 	u32	x_min;
@@ -550,10 +551,6 @@ static int u1_init(struct hid_device *hdev, struct alps_dev *pri_data)
 		dev_err(&hdev->dev, "failed U1_RESO_DWN_ABS (%d)\n", ret);
 		goto exit;
 	}
-	pri_data->x_active_len_mm =
-		(pitch_x * (sen_line_num_x - 1)) / 10;
-	pri_data->y_active_len_mm =
-		(pitch_y * (sen_line_num_y - 1)) / 10;
 
 	pri_data->x_max =
 		(resolution << 2) * (sen_line_num_x - 1);
@@ -562,6 +559,18 @@ static int u1_init(struct hid_device *hdev, struct alps_dev *pri_data)
 		(resolution << 2) * (sen_line_num_y - 1);
 	pri_data->y_min = 1;
 
+	if (pri_data->no_pitch) {
+		pri_data->x_res = pitch_x;
+		pri_data->y_res = pitch_y;
+	} else {
+		pri_data->x_res =
+			(pri_data->x_max - 1) /
+			((pitch_x * (sen_line_num_x - 1)) / 10);
+		pri_data->y_res =
+			(pri_data->y_max - 1) /
+			((pitch_y * (sen_line_num_y - 1)) / 10);
+	}
+
 	ret = u1_read_write_register(hdev, ADDRESS_U1_PAD_BTN,
 			&tmp, 0, true);
 	if (ret < 0) {
@@ -622,7 +631,7 @@ static int T4_init(struct hid_device *hdev, struct alps_dev *pri_data)
 	pri_data->x_min = T4_COUNT_PER_ELECTRODE;
 	pri_data->y_max = sen_line_num_y * T4_COUNT_PER_ELECTRODE;
 	pri_data->y_min = T4_COUNT_PER_ELECTRODE;
-	pri_data->x_active_len_mm = pri_data->y_active_len_mm = 0;
+	pri_data->x_res = pri_data->y_res = 0;
 	pri_data->btn_cnt = 1;
 
 	ret = t4_read_write_register(hdev, PRM_SYS_CONFIG_1, &tmp, 0, true);
@@ -675,7 +684,7 @@ static int alps_input_configured(struct hid_device *hdev, struct hid_input *hi)
 	struct alps_dev *data = hid_get_drvdata(hdev);
 	struct input_dev *input = hi->input, *input2;
 	int ret;
-	int res_x, res_y, i;
+	int i;
 
 	data->input = input;
 
@@ -706,12 +715,9 @@ static int alps_input_configured(struct hid_device *hdev, struct hid_input *hi)
 	input_set_abs_params(input, ABS_MT_POSITION_Y,
 						data->y_min, data->y_max, 0, 0);
 
-	if (data->x_active_len_mm && data->y_active_len_mm) {
-		res_x = (data->x_max - 1) / data->x_active_len_mm;
-		res_y = (data->y_max - 1) / data->y_active_len_mm;
-
-		input_abs_set_res(input, ABS_MT_POSITION_X, res_x);
-		input_abs_set_res(input, ABS_MT_POSITION_Y, res_y);
+	if (data->x_res && data->y_res) {
+		input_abs_set_res(input, ABS_MT_POSITION_X, data->x_res);
+		input_abs_set_res(input, ABS_MT_POSITION_Y, data->y_res);
 	}
 
 	input_set_abs_params(input, ABS_MT_PRESSURE, 0, 64, 0, 0);
@@ -802,8 +808,11 @@ static int alps_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		break;
 	case HID_DEVICE_ID_ALPS_U1_DUAL:
 	case HID_DEVICE_ID_ALPS_U1:
+		data->dev_type = U1;
+		break;
 	case HID_DEVICE_ID_ALPS_1657:
 		data->dev_type = U1;
+		data->no_pitch = 1;
 		break;
 	default:
 		data->dev_type = UNKNOWN;
-- 
2.26.0


  reply	other threads:[~2020-04-09 23:01 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-05 23:55 [PATCH 1/2] HID: alps: Add AUI1657 device ID Artem Borisov
2020-04-05 23:55 ` [PATCH 2/2] HID: alps: Refactor axis resolution logic Artem Borisov
2020-04-09 22:21   ` Jiri Kosina
2020-04-09 23:00     ` Artem Borisov [this message]
2020-04-10  0:28       ` Masaki Ota
2020-04-10  0:47         ` Jiri Kosina
2020-04-10  2:03           ` Xiaojian Cao
2020-04-10  1:03         ` Xiaojian Cao
2020-04-10  1:50           ` Masaki Ota
2020-04-10  2:28             ` Xiaojian Cao
     [not found]               ` <CAMr=fxLXT2fMdhmnfj3ZH2Ptc50nvMVU0nAvW-3fw-wAKb9rYQ@mail.gmail.com>
     [not found]                 ` <OSAPR01MB305753C5B0DD35DE7001436DC8DB0@OSAPR01MB3057.jpnprd01.prod.outlook.com>
2020-04-15  9:16                   ` Artem Borisov
2020-04-15  9:54                     ` Xiaojian Cao
2020-04-15 10:14                       ` Artem Borisov
2020-04-16  8:52                         ` Xiaojian Cao
2020-04-16 10:57                           ` Artem Borisov
2020-04-15 12:54                       ` Jiri Kosina
2020-04-16  8:10                         ` Xiaojian Cao
2020-04-14  9:23 ` [PATCH 1/2] HID: alps: Add AUI1657 device ID Jiri Kosina

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=20200409230009.22551-1-dedsa2002@gmail.com \
    --to=dedsa2002@gmail.com \
    --cc=benjamin.tissoires@redhat.com \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masaki.ota@jp.alps.com \
    --cc=rydberg@bitmath.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.