Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31757 )
Change subject: [WIP] device/pci_ops: Store pci_devfn_t in devicetree ......................................................................
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
This is very much incomplete.
But idea is, that as PCI bus gets enumerated, each secondary side PCI device path.pci.devfn is updated with the bus number. Changing path.pci structure may be painful, so much of that is open-coded.
$ git grep 'pci[.]devfn'
Oh man. That's quiet a bit of slop in there to clean up. I could give it a pass if you want. We likely need it so that there's only a few touch points to adjust when we need to make changes. Let me know what you guys think.
Luckily, I don't see this so urgent. I need to check the asm again, but it was something like 12 instructions per accessed PCI device (not per register) this would save. And actual MMIO is a completely different magnitude in execution speed.
I just noticed we are not consistent about pci_devfn_t and we assign values with both PCI_DEVFN() and PCI_DEV(b,d,f), latter is left-shifted 12 bits. If we add bus there, calling it .devfn sounds odd too.
Completely agree about the naming. I was thinking of just removing the open coding at first so we can manipulate things easier. Ya, the 2 macro usages should hopefully be fixed up in the process as well.
As for urgency it may be worth at least starting a clean up. If I feel ambitious I may give it a go.
Indeed. I'll ping you here I complete pushing my related work. I think there is huge potential for merge conflicts here. And we may need to touch entire soc/intel in one commit. We'll see how it goes.
-
Kyösti Mälkki (Code Review)