Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48251 )
Change subject: soc/intel/common/block/lpc: Include correct acpi.h ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48251/3/src/soc/intel/common/block/... File src/soc/intel/common/block/lpc/lpc.c:
https://review.coreboot.org/c/coreboot/+/48251/3/src/soc/intel/common/block/... PS3, Line 14: <soc/acpi.h>
#2 doesn't seem correct either and could make the common block stuff very confusing if the header is […]
Sure. I don't want to block you on this. If there is something that needs to be used by more than this driver, then it should have a common place -- under arch, lib or drivers. I would say that it is inherently wrong that lpc is using southbridge_write_acpi_tables. There should be a specific device for that rather than tagging along with LPC. I know the scope grows much more than what this is intending.
Given that southbridge_write_acpi_tables() is used only by LPC, it seems fine to me that it is declared in lpc.h. When/if we ever get to cleaning it properly, we can move the declaration and the call to the appropriate device.
I don't really have any other better ideas which do not involve more work. If you plan to keep it this way, I think it would be good to add a comment here explaining why this is being done so that it provides some context on how it can be cleaned up as well.