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

[Kris Bahnsen <kris@embeddedts.com>]

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