Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43133 )
Change subject: baytrail RFC: native replacement for refcode.elf ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/43133/3/src/soc/intel/baytrail/modp... File src/soc/intel/baytrail/modphy_table.c:
PS3: This table should make use of the existing IOSF infrastructure in coreboot. For instance, the first column seems to be a port number, the second the register offset. The last two columns are opcodes for read and write (the latter is always just `read | 1`). Didn't check in detail, but I guess the other two columns are mask + value for a read-modify-write.
If there are no existing functions for all the ports used here, I'd suggest to deflate the table a little. For instance, port and (read)opcode could be combined into one 16-bit word (write opcode doesn't need a column because we know it's read | 1), register offsets seem to be 16-bit wide too. Overall, that could cut the table size in half.
https://review.coreboot.org/c/coreboot/+/43133/3/src/soc/intel/baytrail/refc... File src/soc/intel/baytrail/refcode_native.c:
https://review.coreboot.org/c/coreboot/+/43133/3/src/soc/intel/baytrail/refc... PS3, Line 8: static u8 mmio_read_8(u32 addr) : { : return *(u8 *) addr; : } : : static u32 mmio_read_32(u32 addr) : { : return *(u32 *) addr; : } : : static void mmio_write_32(u32 addr, u32 val) : { : *(u32 *) addr = val; : } : : static void mmio_or_32(u32 addr, u32 val) : { : u32 tmp; : : tmp = mmio_read_32(addr); : mmio_write_32(addr, tmp | val); : } : : static void mmio_and_32(u32 addr, u32 val) : { : u32 tmp; : : tmp = mmio_read_32(addr); : mmio_write_32(addr, tmp & val); : } : : static void mmio_andor_32(u32 addr, u32 val1, u32 val2) : { : u32 tmp; : : tmp = mmio_read_32(addr); : mmio_write_32(addr, (tmp & val1) | val2); : } : Please use `#include <device/mmio.h>` instead. It also draws in <arch/mmio.h> i.e. `src/arch/x86/include/arch/mmio.h`.