Wonkyu Kim 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:
(9 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";
No update here? I don't expect to see any changes for this clubbed into this same CL. […]
This will be updated https://review.coreboot.org/c/coreboot/+/37781
https://review.coreboot.org/c/coreboot/+/38341/5/src/soc/intel/tigerlake/chi... PS5, Line 63: case 3: return "SS04";
No update here?
This will be updated https://review.coreboot.org/c/coreboot/+/37781
https://review.coreboot.org/c/coreboot/+/38341/5/src/soc/intel/tigerlake/chi... PS5, Line 72: devfn
No update here?
We have minimum ASL file in Coreboot, not all these return have coresponding asl and we'll add need base. We'll check and clean up if TGL doesn't use it in ASL file. Update will be done in https://review.coreboot.org/c/coreboot/+/37783
https://review.coreboot.org/c/coreboot/+/38341/5/src/soc/intel/tigerlake/chi... PS5, Line 74: GFX0
No update here?
Not used in TGL ASL, delete in https://review.coreboot.org/c/coreboot/+/37783
https://review.coreboot.org/c/coreboot/+/38341/5/src/soc/intel/tigerlake/chi... PS5, Line 75: ISHB
No update here?
Not used in TGL ASL, delete in https://review.coreboot.org/c/coreboot/+/37783
https://review.coreboot.org/c/coreboot/+/38341/5/src/soc/intel/tigerlake/chi... PS5, Line 116: IGBE
Ack
Will update in https://review.coreboot.org/c/coreboot/+/37783
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?
This function is INTEL_COMMON_BLOCK_THERMAL code.
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
I understand the order of device id, but shouldn't this be SIO0, and 0x11 SIO1 and so on?
Will update naming
https://review.coreboot.org/c/coreboot/+/38341/5/src/soc/intel/tigerlake/inc... PS5, Line 71: #define PCH_DEVFN_THERMAL _PCH_DEVFN(ISH, 1)
Where is it being tracked? At minimum, there should be a comment indicating why this is being put in […]
We're discussing internally, we'll have patch for fixing this issue. TGL doesn't have issue with dummy definition as TGL does not use INTEL_COMMON_BLOCK_THERMAL code.