Attention is currently required from: Alper Nebi Yasak, Arthur Heymans, Jianjun Wang, Nico Huber, Ron Minnich, Shelley Chen, Yu-Ping Wu.
Julius Werner has posted comments on this change by Alper Nebi Yasak. ( https://review.coreboot.org/c/coreboot/+/80372?usp=email )
Change subject: arch/io.h: Add port I/O functions to other architectures ......................................................................
Patch Set 5:
(2 comments)
File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/80372/comment/dcf87bc0_2fa5f5a2?usp... : PS5, Line 532: CPU communicate with peripheral devices over PCI I/O space.
Sorry, I have to admit I know basically nothing about PCI […]
Hmm... well, to be honest the more I read about this the more I feel that this shouldn't be handled at a global level like this. Yes, it looks like Linux does it this way but that doesn't necessarily mean it's the best way. coreboot already has a `resource_t` system that's able to model both I/O port and MMIO resources with the same data type, and it seems like that's the best way to cleanly implement drivers that can work with either here.
I think your problem may be(?) that you're trying to use the VGA driver and VGA is "special" in that it has "well-known" I/O ports (because it's a legacy PS/2 device, older than PCI), as opposed to PCI-enumerated I/O ports whose drivers already use the resource system anyway. Maybe for that it would be easiest to create a `CONFIG_VGA_MMIO_ADDRESS` Kconfig within that driver that specifically for that driver contains the correct base address for MMIO translation (and implement that in the fundamental accessor functions in `vga_io.c`). Other device drivers can either implement their own MMIO base address Kconfigs or use the resource system to take the right base address from `devicetree.cb`.
https://review.coreboot.org/c/coreboot/+/80372/comment/c1eb62e2_7aa61c9a?usp... : PS5, Line 536: default 0xffff Not sure what the point of this is? Just masking out the higher bits seems just as incorrect as accessing an invalid address. I'd say either leave this out completely or turn accesses beyond it into an error.