Re: [PATCH] Input: ads7846 - don't use scratch for tx_buf when clearing register

[Dmitry Torokhov]

Hi Kris,

On Fri, Apr 24, 2026 at 07:25:34PM +0000, Kris Bahnsen wrote:
> The workaround for XPT2046 clears the command register, giving the
> touchscreen controller a NOP. The change incorrectly re-uses the
> req->scratch variable which is used as rx_buf for xfer[5], so by
> the time xfer[6] occurs, the contents of req->scratch may not be
> 0. It was found that the touchscreen controller can end up in
> a completely unresponsive state due to it being given a command
> the driver does not expect.
> 
> Instead, rely on the spi_transfer behavior of tx_buf being NULL to
> transmit all 0 bits, moving the 3 bytes to a single message.
> 
> This change was tested on real TSC2046 and ADS7843 controllers,
> but not the XPT2046 the workaround was originally created for.
> Confirming that the original modification to clear the command
> register does not impact either real controller.
> 
> Fixes: 781a07da9bb94 ("Input: ads7846 - add dummy command register clearing cycle")
> Cc: stable@xxxxxxxxxxxxxxx
> Co-developed-by: Mark Featherston <mark@xxxxxxxxxxxxxx>
> Signed-off-by: Mark Featherston <mark@xxxxxxxxxxxxxx>
> Signed-off-by: Kris Bahnsen <kris@xxxxxxxxxxxxxx>
> ---
>  drivers/input/touchscreen/ads7846.c | 13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
> index 4b39f7212d35c..599793d27129e 100644
> --- a/drivers/input/touchscreen/ads7846.c
> +++ b/drivers/input/touchscreen/ads7846.c
> @@ -327,7 +327,7 @@ struct ser_req {
>  	u8			ref_off;
>  	u16			scratch;
>  	struct spi_message	msg;
> -	struct spi_transfer	xfer[8];
> +	struct spi_transfer	xfer[7];
>  	/*
>  	 * DMA (thus cache coherency maintenance) requires the
>  	 * transfer buffers to live in their own cache lines.
> @@ -403,16 +403,11 @@ static int ads7846_read12_ser(struct device *dev, unsigned command)
>  	spi_message_add_tail(&req->xfer[5], &req->msg);
>  
>  	/* clear the command register */
> -	req->scratch = 0;
> -	req->xfer[6].tx_buf = &req->scratch;
> -	req->xfer[6].len = 1;
> +	req->xfer[6].rx_buf = &req->scratch;
> +	req->xfer[6].len = 3;

Doesn't this overflow "scratch" which is only 2 bytes? I guess there is
a hole in ser_req between "scratch" and "msg" but I do not think we
should rely on this.

Can we also set rx_buf to NULL to discard incoming data?

[credit to sashiko].

Thanks.

-- 
Dmitry