Change in coreboot[master]: drivers/i2c/cs42l42: Add driver for generating device in SSDT
Attention is currently required from: Furquan Shaikh, Martin Roth, Paul Menzel, Zhuohao Lee, EricR Lai, Karthik Ramasubramanian. Vitaly Rodionov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52395 )
Change subject: drivers/i2c/cs42l42: Add driver for generating device in SSDT ......................................................................
Patch Set 7:
(5 comments)
File src/drivers/i2c/cs42l42/chip.h:
https://review.coreboot.org/c/coreboot/+/52395/comment/dd000e25_d5088cbf PS7, Line 27: hs
Just for my understanding, I think "ts" means tip sense and "hs" means headset?
Yes, correct ts - tip sense. hs- headset.
https://review.coreboot.org/c/coreboot/+/52395/comment/b8bdca9a_62374b52 PS7, Line 59: Default = Non-inverted
For this and all other entries, do we really need to mention "Default = " case? There is nothing tha […]
I think it is OK to mention default value, for someone who is looking at this code first time it will will give an idea of what is set/recommended by default.
File src/drivers/i2c/cs42l42/chip.h:
https://review.coreboot.org/c/coreboot/+/52395/comment/3a0d8e76_9b3524e5 PS6, Line 6: RISE_DEB_0_MS = 0,
please, no spaces at the start of a line
Please fix.
File src/drivers/i2c/cs42l42/cs42l42.c:
https://review.coreboot.org/c/coreboot/+/52395/comment/d00efd94_7a95d290 PS7, Line 72: btn_det_init_dbnce
Since this allows only 0-200, I think you need to sanitize the values in case they are incorrect? […]
Done
https://review.coreboot.org/c/coreboot/+/52395/comment/bab67422_4e49ecd7 PS7, Line 73: btn_det_event_dbnce
Same here for 0-20.
Done
-
Vitaly Rodionov (Code Review)