Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39888 )
Change subject: ec/google/chromeec: Emit SSDT for PS2 keyboard if needed ......................................................................
Patch Set 1:
(4 comments)
c const
https://review.coreboot.org/c/coreboot/+/39888/1/src/ec/google/chromeec/ec_p... File src/ec/google/chromeec/ec_ps2_keyboard.c:
https://review.coreboot.org/c/coreboot/+/39888/1/src/ec/google/chromeec/ec_p... PS1, Line 21: static static const?
https://review.coreboot.org/c/coreboot/+/39888/1/src/ec/google/chromeec/ec_p... PS1, Line 224: : #define KEYMAP(scancode, keycode) (((uint32_t)(scancode) << 16) | (keycode & 0xFFFF)) nit: Preferably all #defines go at the top of the file, unless they are in a #define/#undef combo.
https://review.coreboot.org/c/coreboot/+/39888/1/src/ec/google/chromeec/ec_p... PS1, Line 261: there is an extra blank line here
https://review.coreboot.org/c/coreboot/+/39888/1/src/ec/google/chromeec/ec_p... PS1, Line 283: _SB.PCI0.PS2K Is this the required path? Shouldn't it be under the parent (EC) ? If it doesn't belong under the EC, then why it in the EC driver? I don't actually see any google_chromeec_() calls in here.