Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45567 )
Change subject: drivers/tpm: Move PPI stub ......................................................................
Patch Set 1:
(8 comments)
https://review.coreboot.org/c/coreboot/+/45567/1/src/drivers/tpm/ppi_stub.c File src/drivers/tpm/ppi_stub.c:
https://review.coreboot.org/c/coreboot/+/45567/1/src/drivers/tpm/ppi_stub.c@... PS1, Line 16: 2 sizeof(buf) ?
https://review.coreboot.org/c/coreboot/+/45567/1/src/drivers/tpm/ppi_stub.c@... PS1, Line 47: 2 symbolic constant or a comment would be nice
https://review.coreboot.org/c/coreboot/+/45567/1/src/drivers/tpm/ppi_stub.c@... PS1, Line 62: aka AKA
https://review.coreboot.org/c/coreboot/+/45567/1/src/drivers/tpm/ppi_stub.c@... PS1, Line 59: static void tpm_ppi_func6_cb(void *arg) : { : /* : * Set preferred user language: deprecated and must return 3 aka : * "not implemented". : */ : acpigen_write_return_byte(3); : } : static void tpm_ppi_func7_cb(void *arg) : { : /* Submit operations: deny. */ : acpigen_write_return_byte(3); : } : static void tpm_ppi_func8_cb(void *arg) : { : /* All actions are forbidden. */ : acpigen_write_return_byte(1); please add symbolic constants for these return values
https://review.coreboot.org/c/coreboot/+/45567/1/src/drivers/tpm/ppi_stub.c@... PS1, Line 121: &tpm_ppi_callbacks[0] can just be `tpm_ppi_callbacks`
https://review.coreboot.org/c/coreboot/+/45567/1/src/drivers/tpm/ppi_stub.c@... PS1, Line 122: (void *) &arg) It doesn't look like any of the callbacks use this arg, could we just pass NULL instead?
https://review.coreboot.org/c/coreboot/+/45567/1/src/drivers/tpm/ppi_stub.c@... PS1, Line 124: &tpm_mci_callbacks[0] can just be `tpm_mci_callbacks`
https://review.coreboot.org/c/coreboot/+/45567/1/src/drivers/tpm/ppi_stub.c@... PS1, Line 125: (void *) &arg) It doesn't look like any of the callbacks use this arg, could we just pass NULL instead?