[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, Eric Lai, Jingyuan Liang, Kyoung Il Kim.

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 4:

(18 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/81331/comment/21b7fd22_d07dd5a5 :
PS4, Line 9: controller(THC)
Please add space before the (.

https://review.coreboot.org/c/coreboot/+/81331/comment/2e07ac77_0a81f556 :
PS4, Line 14: Meteorlake
Meteor Lake

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

https://review.coreboot.org/c/coreboot/+/81331/comment/6079db90_272f9425 :
PS4, Line 11: MAX_TOUCH_PORTS
MAX_TOUCH_PORTS_PER_CONTROLLER?

https://review.coreboot.org/c/coreboot/+/81331/comment/e2fad248_8d42d2e5 :
PS4, Line 23: Meteorlake
Meteor Lake

https://review.coreboot.org/c/coreboot/+/81331/comment/2829eceb_c5b5cfd3 :
PS4, Line 30: /* Touch Host Controller's (THC) 3 protocol modes:
            : 	 * Switch among IPTS, HID-SPI and HID-I2C.
            : 	 * 0x0: THC IPTS (not supported at this time)
            : 	 * 0x1: THC HID SPI
            : 	 * 0x2: THC HID I2C (for future support)
            : 	 */
Blank line at the top.

/*
     * […]
     * /

https://review.coreboot.org/c/coreboot/+/81331/comment/01bfc2aa_f6bddc00 :
PS4, Line 45: 	unsigned int enable_delay_ms;
Is there a better name? `delay_after_enabling_device_ms`?

https://review.coreboot.org/c/coreboot/+/81331/comment/2f5595f1_7ce9040b :
PS4, Line 57: rpt
I’d use report.

https://review.coreboot.org/c/coreboot/+/81331/comment/1e5a691a_d05ef1fb :
PS4, Line 63: /* Touch Host Controller Wake On Touch
            : 	   Based on this setting vGPIO for given THC will be in native mode, and additional
            : 	  _CRS for wake will be exposed in ACPI
            : 	*/
Please use the recommended commenting styles.

https://review.coreboot.org/c/coreboot/+/81331/comment/d2af3da3_44bb5423 :
PS4, Line 70: Spi
SPI

https://review.coreboot.org/c/coreboot/+/81331/comment/5063f253_fe7876c8 :
PS4, Line 71: this
This

https://review.coreboot.org/c/coreboot/+/81331/comment/c4ddea38_85023cbc :
PS4, Line 90: /* Touch Host Controller Hid Over Spi Connection Speed
            : 	   Hid Over Spi Connection Speed - SPI Frequency
            : 	 */
Ditto. (Also below.)

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

https://review.coreboot.org/c/coreboot/+/81331/comment/0daab7da_af039e1e :
PS4, Line 16: // #define ELAN_CONNECTION_SPEED 17000000
Why commented out?

File src/drivers/intel/touch/touch.c:

https://review.coreboot.org/c/coreboot/+/81331/comment/d20b2927_17518b75 :
PS4, Line 30: in rpt hdr addr
Spell it out?

https://review.coreboot.org/c/coreboot/+/81331/comment/6fb5515f_8cdc2f2a :
PS4, Line 65: 		"Use TH_SENSOR_GENERIC for parameters from the devicetree!\n");
Indent more?

https://review.coreboot.org/c/coreboot/+/81331/comment/57643706_98188594 :
PS4, Line 106: 		printk(BIOS_INFO, "use WACOM Write Opcode\n");
Why only the info message for the first case? What should the user do with this information?

https://review.coreboot.org/c/coreboot/+/81331/comment/e3b5ab55_7251018d :
PS4, Line 526: static void
             : touch_generate_acpi_method_on(
Why not one line?

https://review.coreboot.org/c/coreboot/+/81331/comment/c1c41ed5_9de8e41b :
PS4, Line 719: .read_resources   = pci_dev_read_resources,
             : 	.set_resources    = pci_dev_set_resources,
             : 	.enable_resources = pci_dev_enable_resources,
             : 	.init             = pci_dev_init,
             : 	.scan_bus         = &scan_generic_bus, /* Non-default */
             : 	.ops_pci          = &pci_dev_ops_pci,
Use tabs for aligning = like below?

https://review.coreboot.org/c/coreboot/+/81331/comment/9e9501e0_0800e0a9 :
PS4, Line 743:  
Tab?