Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35626 )
Change subject: superio: add support for IT8380 ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35626/3/src/superio/ite/it8380/supe... File src/superio/ite/it8380/superio.c:
https://review.coreboot.org/c/coreboot/+/35626/3/src/superio/ite/it8380/supe... PS3, Line 25: {
My board doesn't have the keyboard controller exposed. […]
most SIOs do that. not sure though why a few non-ancient ITE SIOs call set_kbc_ps2_mode before pc_keyboard_init (they should all come up in PS/2 mode) and why it has a skip_keyboard devicetree setting when this gets skipped anyway when the kbc device is disabled. my guess would be that a simple call to pc_keyboard_init is sufficient and the rest is either cargo-cult (set_kbc_ps2_mode) or in combination with a devicetree setting (skip_keyboard) specific to the payload used in the chromebooks. Since you probably can't test the KBC on the SIO on the board, I'm ok with just having the switch case statement here and a TODO comment that the KBC init still needs to be implemented, so that someone else wants to use the SIO with PS/2 keyboard connected to it sees that there is some call missing. or just a comment in the empty function.
-
Felix Held (Code Review)