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

[Kris Bahnsen <kris@embeddedts.com>]

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