Attention is currently required from: Tarun Tuli, Subrata Banik, Kapil Porwal, Sridhar Siricilla.
Pratikkumar V Prajapati has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/72722 )
Change subject: soc/intel/common: Add Intel Trace Hub driver ......................................................................
Patch Set 4:
(1 comment)
This change is ready for review.
Patchset:
PS3:
FSP allocates memory for tracehub to save traces (depending on UPD params). […]
Sorry for the long pause in replying.
Seems like in this driver code, you are unconditionally checking the HOB, what if the HOB is not present ? does that mean we don't need to provide the fixed BAR as TH might not have configured/initialized?
I am not sure what condition you are expecting! If the HOB is not present, we simply do not mark any region reserved. I am calling pci_dev_read_resources from tracehub_read_resources. pci_dev_read_resources would set the BAR, isnt it? Did i miss anything?
But from that, you first need to make sure that ITH is bus/device/function enabled by boot firmware aka coreboot.
I dont think its needed as this driver will get called only when coreboot enumerated PCIe IDs are matched with pci_device_ids mentioned in my driver. This match will not happen if TH is disabled from device tree.
First and for most, you need to make sure that TH is not disabled by boot firmware, then assign the UPDs (SOCTracehobmode, AetTraceHubmode and/or PchTraceHubMode) are updated. Without those IMO, the Hob will not be produced and the added driver code is just not in use.
Since this is stand-alone PCIe driver, and I have mentioned TH PCI IDs in pci_device_ids for this driver, no need to check if the boot firmware has TH disabled. This diver will not get called if the TH is disabled.