Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38341 )
Change subject: soc/intel/tigerlake: Update pci dev definition ......................................................................
Patch Set 7:
(8 comments)
https://review.coreboot.org/c/coreboot/+/38341/5/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.c:
https://review.coreboot.org/c/coreboot/+/38341/5/src/soc/intel/tigerlake/chi... PS5, Line 54: case 9: return "HS10";
acpi/xhci.asl lists upto HS12 devices, but there are only devices upto HS10 here.
No update here? I don't expect to see any changes for this clubbed into this same CL. This is just something that I noticed and I think should be addressed somewhere (probably in a separate CL).
https://review.coreboot.org/c/coreboot/+/38341/5/src/soc/intel/tigerlake/chi... PS5, Line 63: case 3: return "SS04";
acpi/xhci.asl lists upto SS06 devices, but there are only devices upto SS04 here.
No update here?
https://review.coreboot.org/c/coreboot/+/38341/5/src/soc/intel/tigerlake/chi... PS5, Line 72: devfn
I see a number of devices that do not have their corresponding device in any .asl file. […]
No update here?
https://review.coreboot.org/c/coreboot/+/38341/5/src/soc/intel/tigerlake/chi... PS5, Line 74: GFX0
Shouldn't there be a corresponding device in . […]
No update here?
https://review.coreboot.org/c/coreboot/+/38341/5/src/soc/intel/tigerlake/chi... PS5, Line 75: ISHB
same here.
No update here?
https://review.coreboot.org/c/coreboot/+/38341/7/src/soc/intel/tigerlake/fin... File src/soc/intel/tigerlake/finalize.c:
https://review.coreboot.org/c/coreboot/+/38341/7/src/soc/intel/tigerlake/fin... PS7, Line 78: pch_thermal_configuration(); Why was this removed? This seems totally unrelated?
https://review.coreboot.org/c/coreboot/+/38341/5/src/soc/intel/tigerlake/inc... File src/soc/intel/tigerlake/include/soc/pci_devs.h:
https://review.coreboot.org/c/coreboot/+/38341/5/src/soc/intel/tigerlake/inc... PS5, Line 57: SIO4
This is based on or order of Device id(0x10, 0x11, etc)
I understand the order of device id, but shouldn't this be SIO0, and 0x11 SIO1 and so on?
https://review.coreboot.org/c/coreboot/+/38341/5/src/soc/intel/tigerlake/inc... PS5, Line 71: #define PCH_DEVFN_THERMAL _PCH_DEVFN(ISH, 1)
It just added dummy number(unused device) for avoid compile failure and it'll not be used in finaliz […]
Where is it being tracked? At minimum, there should be a comment indicating why this is being put in.
Is it really difficult to just remove the SOC_INTEL_COMMON_BLOCK_THERMAL selection from SOC_INTEL_COMMON_PCH_BASE and move it to individual SoCs that really have this block?
BTW, does TGL not have this block altogether?