Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32653 )
Change subject: soc/amd/stoneyridge: Move LPC support to common ......................................................................
Patch Set 4:
(4 comments)
https://review.coreboot.org/#/c/32653/4/src/soc/amd/common/block/lpc/lpc.c File src/soc/amd/common/block/lpc/lpc.c:
https://review.coreboot.org/#/c/32653/4/src/soc/amd/common/block/lpc/lpc.c@3... PS4, Line 38: late_lpc_bridge_enable If a function is supposed to be implemented by the SoC, it would be helpful to add _soc_ in its name and add a comment in the .h file indicating when it is supposed to be used/defined.
https://review.coreboot.org/#/c/32653/4/src/soc/amd/common/block/lpc/lpc_uti... File src/soc/amd/common/block/lpc/lpc_util.c:
https://review.coreboot.org/#/c/32653/4/src/soc/amd/common/block/lpc/lpc_uti... PS4, Line 36: struct const
https://review.coreboot.org/#/c/32653/4/src/soc/amd/common/block/lpc/lpc_uti... PS4, Line 40: TOTAL_WIDEIO_PORTS Is this something that the SoC is supposed to provide? If this is always going to be 3, do we really need to get that from the SoC?
https://review.coreboot.org/#/c/32653/4/src/soc/amd/common/block/lpc/lpc_uti... PS4, Line 188: (uintptr_t) Is this typecast necessary?