[L] Change in coreboot[main]: drivers/intel/touch: Add driver for Intel Touch Controller and Devices

[Paul Menzel (Code Review)]

Attention is currently required from: Cliff Huang, Eran Mitrani, Jingyuan Liang.

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81331?usp=email )

Change subject: drivers/intel/touch: Add driver for Intel Touch Controller and Devices
......................................................................

Patch Set 2:

(13 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/81331/comment/184e0e62_7a5ce069 :
PS2, Line 9: Support WACOM, ELAN, and generic touch sensor devices.
Please write an introduction what that new device it, and what driver is needed for it.

https://review.coreboot.org/c/coreboot/+/81331/comment/89bf1ef8_4788c5d9 :
PS2, Line 10: This driver generates the following ACPI objects:
Please add a blank line between paragraphs.

https://review.coreboot.org/c/coreboot/+/81331/comment/0e02b73b_dd8b24bb :
PS2, Line 11: _DSM
            : _CRS
            : Power resource with _STA, _ON, and _OFF
            : _RST
Please format this as a list.

File src/drivers/intel/touch/chip.h:

PS2: 
Mention the datasheet name somewhere in the file?

https://review.coreboot.org/c/coreboot/+/81331/comment/a098ed5e_d908f332 :
PS2, Line 31: Thc
THC?

https://review.coreboot.org/c/coreboot/+/81331/comment/c1e7adf1_5ea83c8c :
PS2, Line 31: :
Please add a space after the colon.

https://review.coreboot.org/c/coreboot/+/81331/comment/92dc8e80_6154a2d0 :
PS2, Line 31: supporded
supported

https://review.coreboot.org/c/coreboot/+/81331/comment/d76bf549_880c6a02 :
PS2, Line 33: supporded
supported

https://review.coreboot.org/c/coreboot/+/81331/comment/03e4030c_14ee889b :
PS2, Line 33: ;
Necessary?

https://review.coreboot.org/c/coreboot/+/81331/comment/7e5475d6_29fd9b22 :
PS2, Line 29: /* Touch Host Controller Mode
            : 	   Switch between Intel THC protocol and Industry standard HID Over SPI protocol.
            : 	   0x0:Thc IPTS (not supporded at this time)
            : 	   0x1:Thc HID SPI
            : 	   0x2 Thc HID I2C; (not supporded at this time)
            : 	*/
I’d use

/*
     * […]
     */

https://review.coreboot.org/c/coreboot/+/81331/comment/750ea4c8_7d71a0b7 :
PS2, Line 41: 	/* GPIO used to enable device. */
Redundant as the variable name says the same?

https://review.coreboot.org/c/coreboot/+/81331/comment/ef8ae9aa_f360d06a :
PS2, Line 46: enable_off
Why not disable?

File src/drivers/intel/touch/elan.h:

https://review.coreboot.org/c/coreboot/+/81331/comment/53a68958_4d92ff22 :
PS2, Line 9: #define ELAN_RST_SEQ_DLY 50
Is that time? Put the unit into the name?