Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36493 )
Change subject: [WIP] Documentation: PCI resource allocations ......................................................................
Patch Set 1:
(4 comments)
Patch Set 1:
(2 comments)
This seems like it's capturing potential future behavior and not documenting current, correct?
Yep. This is more of future behaviour, should we implement MINIMAL_PCI_RESOURCE.
https://review.coreboot.org/c/coreboot/+/36493/1/Documentation/device/pci_re... File Documentation/device/pci_resource.md:
https://review.coreboot.org/c/coreboot/+/36493/1/Documentation/device/pci_re... PS1, Line 13: 1. Within PCI drivers, *.init* is the only callback where resource assignment may be required.
And `. […]
Right.
https://review.coreboot.org/c/coreboot/+/36493/1/Documentation/device/pci_re... PS1, Line 16: *.init*
This is also implying not having a __pci_driver that matches on didvid. […]
I think we would continue to allow __pci_driver with empty .init and .final. Maybe I did not completely understand the question.
https://review.coreboot.org/c/coreboot/+/36493/1/Documentation/device/pci_re... PS1, Line 16: implicitly opt-out
Why the change of behavior? This is implying that command register will not be enabled, correct?
That's the approach Ron wants to drive, to opt-out as much as possible. I would not necessarily agree with him here. Not having BARs programmed implies indeed command register must keep decodes disabled.
To opt-out would always be conditional on CONFIG(MINIMAL_PCI_RESOURCES). I am trying to point out here that little devices would need to be explicitly opted-out, if we make decisions on .init/.final existence.
https://review.coreboot.org/c/coreboot/+/36493/1/Documentation/device/pci_re... PS1, Line 25:
Maybe […]
Subrata placed some comment about SoC PCI tree being static. It's almost as if he is saying that Intel wants this MINIMAL_PCI_RESOURCE thing because they want to assign the required/temporary resources inside the blob instead.
If someone is saying payload/OS can re-enumerate and reallocate everything, I think we should explicitly make it so that everything is unassigned on entry to payload.