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

Kris Bahnsen
2026-04-27T16:20:04Z

On 4/25/26 9:51 PM, Dmitry Torokhov wrote:
> 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?

Well spotted! I'm quite annoyed with myself that I fixed one pointer
use bug to introduce a buffer overflow.

Will send a v2 patch later today.

> [credit to sashiko].
>
> Thanks.
>

--
Kris Bahnsen
Software Engineer
embeddedTS