Goodix: Obviously wrong reset logic

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi all,

since linux v4.5 the goodix touch driver support the reset by commit
ec6e1b4082d9 ("Input: goodix - reset device at init"). I was a bit
confused while I read the goodix_reset() function:
8<----------------------------------------------------------------------
static int goodix_reset(struct goodix_ts_data *ts)                                                                                      
{              
	int error;

	/* begin select I2C slave addr */
	error = gpiod_direction_output(ts->gpiod_rst, 0);
	if (error)
		return error;

	msleep(20);				/* T2: > 10ms */                                                                        

	/* HIGH: 0x28/0x29, LOW: 0xBA/0xBB */                                                                                           
	error = goodix_irq_direction_output(ts, ts->client->addr == 0x14);
	(error)
		return error;

	usleep_range(100, 2000);		/* T3: > 100us */

	error = gpiod_direction_output(ts->gpiod_rst, 1);
	if (error)
		return error;
	
	...
}
8<----------------------------------------------------------------------
because the gpiod_direction_output() takes the logical level and and not
the physical level. Also it is not intuitive to read. Releasing the reset
line first and set it after?

As pointed out by the commit message, the reset logic is based on the
datasheet GT911 [1] and GT9271[2]. Those datasheets says that the reset
pin is active low. Setting this in my DT-based board will break
everything. I checked the DT's already using a goodix device and all of
them are specifying the pin as active-high. IMHO this is wrong.

Now the question is: Does the ACPI tables specify it as active high too
and was this the reason to miss-use the gpiod_ API? If this is the case
fixing the driver would break everything and in that case we should at
least mention it within the driver and within the DT-Bindings.

Regards,
  Marco



[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux