Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44512 )
Change subject: soc/intel/common: Call pci_dev_request_bus_master() from .final ops ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44512/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44512/2//COMMIT_MSG@15 PS2, Line 15: access
enable BM for BIOS critical device like host bridge, output, input and block device
My understanding is that we currently enable PCI_COMMAND_MASTER for most internal controllers within coreboot - I2C, GSPI, Fast SPI, UART, integrated graphics, etc. These are primarily the controllers that get utilized within coreboot/payload.
However, in a lot of cases, we rely on FSP to do this configuration for us. Example: Early I2C configuration does the PCI_COMMAND_MASTER setting, but late I2C configuration doesn't do this. Also, I don't think FSP is consistent about performing this configuration for all controllers.
The most recent issue that we ran into was SATA configuration being performed by FSP but not really setting PCI_COMMAND_MASTER which resulted in issues in payload.
I have thought of coreboot as performing the initialization of controllers so that the payload can use them without having to redo that initialization. Thus, my comment to Subrata that we should ensure that PCI_COMMAND_MASTER is set for all controllers consistently within coreboot. I understand concerns about doing this for hotpluggable buses(e.g. usb4 controllers). But, other than that I believe it should be okay to do the setting of PCI_COMMAND_MASTER within coreboot.
I might be wrong with my understanding here. So if this understanding is not correct, I think we should at least ensure that the assumptions are properly documented so that the payloads can take appropriate actions to handle this.