Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45149 )
Change subject: device: Rework bus master option ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45149/3/src/device/Kconfig File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/45149/3/src/device/Kconfig@537 PS3, Line 537: config PCI_ALLOW_BUS_MASTER_ANY_DEVICE
could be skipped, assuming this is clear to the user in context of the options in the follow-ups
I agree. A help text is expected for any user-visible config. Also for the follow up. e.g.
Allow coreboot to enable PCI bus mastering for any device. The actual selection of devices depends on the various PCI drivers in coreboot.
https://review.coreboot.org/c/coreboot/+/45149/3/src/device/Kconfig@538 PS3, Line 538: Any devices
ah, reading the follow-up It got it ;) maybe a choice would be better here to select "any device", " […]
The currently added configs are necessary anyway, to check them in the code. We can add a choice on top and then make these existing configs invisible and let their defaults depend on the choice e.g.
config PCI_SET_BUS_MASTER_PCI_BRIDGES def_bool PCI_BUS_MASTER_CHOICE_BRIDGES || PCI_BUS_MASTER_CHOICE_ANY
But that can be done later, I suppose, once the semantics are clear.
-
Nico Huber (Code Review)