Input: ads7846 - consolidate coordinate filtering logic

The ads7846 driver has two separate filtering functions, ads7846_filter() and ads7846_filter_one(), for the full-duplex and half-duplex SPI paths, respectively.

They can be consolidated by extracting the core filtering logic for a single command into a helper function, ads7846_filter_cmd(), which iterates from l->skip to l->count. The half-duplex setup function is updated to set l->skip = l->count - 1 so that the helper only processes the last sample, preserving the original half-duplex behavior.

Assisted-by Gemini:gemini-3.1-pro

Signed-off-by Dmitry Torokhov <[email protected]>

---

Not tested so will appreciate if someone could give it a spin.

 drivers/input/touchscreen/ads7846.c | 74 +++++++++++++++--------------
 1 file changed, 38 insertions(+), 36 deletions(-)

drivers/input/touchscreen/ads7846.c +38 -36

index d3b529333ca2..093f4b56cc18 100644
@@ -759,52 +759,54 @@ static bool ads7846_cmd_need_settle(enum ads7846_cmds cmd_idx)
 	return false;
 }
 
-static int ads7846_filter(struct ads7846 *ts)
+static int ads7846_filter_cmd(struct ads7846 *ts, unsigned int cmd_idx)
 {
 	struct ads7846_packet *packet = ts->packet;
-	int action;
-	int val;
-	unsigned int cmd_idx, b;
+	struct ads7846_buf_layout *l = &packet->l[cmd_idx];
+	unsigned int b;
 
-	packet->ignore = false;
-	for (cmd_idx = packet->last_cmd_idx; cmd_idx < packet->cmds - 1; cmd_idx++) {
-		struct ads7846_buf_layout *l = &packet->l[cmd_idx];
+	for (b = l->skip; b < l->count; b++) {
+		int val = ads7846_get_value(&packet->rx[l->offset + b]);
 
-		packet->last_cmd_idx = cmd_idx;
+		switch (ts->filter(ts->filter_data, cmd_idx, &val)) {
+		case ADS7846_FILTER_REPEAT:
+			if (b == l->count - 1)
+				return -EAGAIN;
+			break;
 
-		for (b = l->skip; b < l->count; b++) {
-			val = ads7846_get_value(&packet->rx[l->offset + b]);
-
-			action = ts->filter(ts->filter_data, cmd_idx, &val);
-			if (action == ADS7846_FILTER_REPEAT) {
-				if (b == l->count - 1)
-					return -EAGAIN;
-			} else if (action == ADS7846_FILTER_OK) {
-				ads7846_set_cmd_val(ts, cmd_idx, val);
-				break;
-			} else {
-				packet->ignore = true;
-				return 0;
-			}
+		case ADS7846_FILTER_OK:
+			ads7846_set_cmd_val(ts, cmd_idx, val);
+			return 0;
+
+		case ADS7846_FILTER_IGNORE:
+		default:
+			return -EIO;
 		}
 	}
 
-	return 0;
+	return -EIO;
 }
 
-static int ads7846_filter_one(struct ads7846 *ts, unsigned int cmd_idx)
+static int ads7846_filter(struct ads7846 *ts)
 {
 	struct ads7846_packet *packet = ts->packet;
-	struct ads7846_buf_layout *l = &packet->l[cmd_idx];
-	int action, val;
-
-	val = ads7846_get_value(&packet->rx[l->offset + l->count - 1]);
-	action = ts->filter(ts->filter_data, cmd_idx, &val);
-	if (action == ADS7846_FILTER_REPEAT)
-		return -EAGAIN;
-	else if (action != ADS7846_FILTER_OK)
-		return -EIO;
-	ads7846_set_cmd_val(ts, cmd_idx, val);
+	unsigned int cmd_idx;
+	int error;
+
+	packet->ignore = false;
+	for (cmd_idx = packet->last_cmd_idx; cmd_idx < packet->cmds - 1; cmd_idx++) {
+		packet->last_cmd_idx = cmd_idx;
+
+		error = ads7846_filter_cmd(ts, cmd_idx);
+		if (error) {
+			if (error == -EAGAIN)
+				return -EAGAIN;
+
+			packet->ignore = true;
+			return 0;
+		}
+	}
+
 	return 0;
 }
 

@@ -857,7 +859,7 @@ static void ads7846_halfd_read_state(struct ads7846 *ts)
 		if (msg_idx == ts->msg_count - 1)
 			break;
 
-		error = ads7846_filter_one(ts, msg_idx);
+		error = ads7846_filter_cmd(ts, msg_idx);
 		if (error == -EAGAIN) {
 			continue;
 		} else if (error) {

@@ -1119,7 +1121,7 @@ static int ads7846_halfd_spi_msg(struct ads7846 *ts,
 		l->offset = offset;
 		offset += max_count;
 		l->count = max_count;
-		l->skip = 0;
+		l->skip = max_count - 1;
 		size += sizeof(*packet->rx) * max_count;
 	}
 

Quoted 10 lines · Dmitry Torokhov

The ads7846 driver has two separate filtering functions, ads7846_filter() and ads7846_filter_one(), for the full-duplex and half-duplex SPI paths, respectively.

They can be consolidated by extracting the core filtering logic for a single command into a helper function, ads7846_filter_cmd(), which iterates from l->skip to l->count. The half-duplex setup function is updated to set l->skip = l->count - 1 so that the helper only processes the last sample, preserving the original half-duplex behavior.

Quoted 5 lines

Assisted-by Gemini:gemini-3.1-pro

Signed-off-by Dmitry Torokhov <[email protected]>

---

Not tested so will appreciate if someone could give it a spin.

Forgive my ignorance, but I am unsure of the base commit this applies to and I'm unable to apply it to current HEAD of linux, linux-next, or your input tree.

I also am not easily able to test this if it doesn't cleanly apply to the latest LTS, but will do what I can once I know what other patches I would need to pull in to our tree.

Hi Kris,

Quoted 17 lines · Kris Bahnsen

Quoted 10 lines · Dmitry Torokhov

The ads7846 driver has two separate filtering functions, ads7846_filter() and ads7846_filter_one(), for the full-duplex and half-duplex SPI paths, respectively.

They can be consolidated by extracting the core filtering logic for a single command into a helper function, ads7846_filter_cmd(), which iterates from l->skip to l->count. The half-duplex setup function is updated to set l->skip = l->count - 1 so that the helper only processes the last sample, preserving the original half-duplex behavior.

Quoted 5 lines

Assisted-by Gemini:gemini-3.1-pro

Signed-off-by Dmitry Torokhov <[email protected]>

---

Not tested so will appreciate if someone could give it a spin.

Quoted 7 lines

Forgive my ignorance, but I am unsure of the base commit this applies to and I'm unable to apply it to current HEAD of linux, linux-next, or your input tree.

I also am not easily able to test this if it doesn't cleanly apply to the latest LTS, but will do what I can once I know what other patches I would need to pull in to our tree.

I think if you pull 'next' branch of my tree, then the following patches seem to apply cleanly on 7.0:

c68bc840f06c ("Input: ads7846 - restore half-duplex support") 011bdf9f3a9d ("Input: ads7846 - consolidate coordinate filtering logic")

I hope they also build ;)

c68bc840f06c should appear in linux-next soon too.

Thanks.

I think if you pull 'next' branch of my tree, then the following patches seem to apply cleanly on 7.0:

c68bc840f06c ("Input: ads7846 - restore half-duplex support") 011bdf9f3a9d ("Input: ads7846 - consolidate coordinate filtering logic")

I hope they also build ;)

c68bc840f06c should appear in linux-next soon too.

Hi,

Quoted 6 lines · Dmitry Torokhov

I think if you pull 'next' branch of my tree, then the following patches seem to apply cleanly on 7.0:

c68bc840f06c ("Input: ads7846 - restore half-duplex support") 011bdf9f3a9d ("Input: ads7846 - consolidate coordinate filtering logic")

Quoted 3 lines

I hope they also build ;)

c68bc840f06c should appear in linux-next soon too.

Did you forgot to push this, or was it dropped? Because I fail to see it any any tree...

Quoted 9 lines · Aaro Koskinen

Hi,

Quoted 6 lines · Dmitry Torokhov

I think if you pull 'next' branch of my tree, then the following patches seem to apply cleanly on 7.0:

c68bc840f06c ("Input: ads7846 - restore half-duplex support") 011bdf9f3a9d ("Input: ads7846 - consolidate coordinate filtering logic")

Quoted 6 lines

Quoted 3 lines

I hope they also build ;)

c68bc840f06c should appear in linux-next soon too.

Did you forgot to push this, or was it dropped? Because I fail to see it any any tree...

Well, that explains why Kris could not find them... Pushed out.