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

Dmitry Torokhov
2026-04-26T04:51:05Z

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@vger.kernel.org
> Co-developed-by: Mark Featherston <mark@embeddedTS.com>
> Signed-off-by: Mark Featherston <mark@embeddedTS.com>
> Signed-off-by: Kris Bahnsen <kris@embeddedTS.com>
> ---
> 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