[PATCH] Input: ads7846 - consolidate coordinate filtering logic

[PATCH] Input: ads7846 - consolidate coordinate filtering logic

[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.

Assisted-by: Gemini:gemini-3.1-pro
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

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(-)

diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index d3b529333ca2..093f4b56cc18 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -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;
 	}
 
-- 
2.54.0.545.g6539524ca2-goog


-- 
Dmitry

Re: [PATCH] Input: ads7846 - consolidate coordinate filtering logic

[Kris Bahnsen]

Dmitry,

On 5/4/26 9:54 PM, Dmitry Torokhov wrote:
> 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 <dmitry.torokhov@gmail.com>
> ---
> 
> 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.

>  drivers/input/touchscreen/ads7846.c | 74 +++++++++++++++--------------
>  1 file changed, 38 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
> index d3b529333ca2..093f4b56cc18 100644
> --- a/drivers/input/touchscreen/ads7846.c
> +++ b/drivers/input/touchscreen/ads7846.c
> @@ -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;
>  	}
>  

-- 
Kris Bahnsen
Software Engineer
embeddedTS

Re: [PATCH] Input: ads7846 - consolidate coordinate filtering logic

[Dmitry Torokhov]

Hi Kris,

On Thu, May 07, 2026 at 10:48:57AM -0700, Kris Bahnsen wrote:
> On 5/4/26 9:54 PM, Dmitry Torokhov wrote:
> > 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 <dmitry.torokhov@gmail.com>
> > ---
> > 
> > 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.

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.

-- 
Dmitry

Re: [PATCH] Input: ads7846 - consolidate coordinate filtering logic

[Kris Bahnsen]

Dmitry,

On 5/7/26 12:35 PM, Dmitry Torokhov wrote:
> Hi Kris,
> 
> On Thu, May 07, 2026 at 10:48:57AM -0700, Kris Bahnsen wrote:
>> On 5/4/26 9:54 PM, Dmitry Torokhov wrote:
>>> 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 <dmitry.torokhov@gmail.com>
>>> ---
>>>
>>> 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.
> 
> 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")


Please confirm that the referenced commits are pushed and that I
am using the correct remote URLs. I am unable to find the referenced
commits still:

$ git remote -v
input   git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git (fetch)
input   git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git (push)
next    git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git (fetch)
next    git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git (push)
[...]
$ git fetch next
$ git fetch input 
$ git show c68bc840f06c
fatal: ambiguous argument 'c68bc840f06c': unknown revision or path not in the working tree.
$ git show 011bdf9f3a9d
fatal: ambiguous argument '011bdf9f3a9d': unknown revision or path not in the working tree.

Apologies in advance if I am missing something obvious.

> I hope they also build ;)
> 
> c68bc840f06c should appear in linux-next soon too.
> 
> Thanks.
> 

-- 
Kris Bahnsen
Software Engineer
embeddedTS