Máté Kukri has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43133 )
Change subject: soc/intel/baytrail: Add native refcode replacement ......................................................................
Patch Set 13:
(32 comments)
Patch Set 1: Code-Review+1
(34 comments)
Good work!
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/incl... File src/soc/intel/baytrail/include/soc/modphy.h:
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/incl... PS1, Line 6: static u32 baytrail_modphy_table[] = {
See `src/northbridge/intel/sandybridge/raminit_tables. […]
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... File src/soc/intel/baytrail/refcode.c:
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 9: #if 0
Instead of preprocessor, I'd have two different files and choose which one to build depending on whe […]
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 90: static u8 mmio_read_8(u32 addr)
read8
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 95: static u32 mmio_read_32(u32 addr)
read32
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 100: static void mmio_write_32(u32 addr, u32 val)
write32
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 121: val1
I'd rename this: and, mask
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 121: val2
I'd rename this: or, data
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 129: static u8 soc_stepping_table[] =
const?
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 138: 0xE00F8008
That's something that falls into the PCI config space. […]
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 143: static void program_modphy_table(u32 *table, u32 entries)
See "Message Bus Register Access" section on the datasheet. […]
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 148: mmio_write_32(0xE00000D8, table[1] & 0xffffff00);
This is writing MCRX on PCI bus:dev.func 0:0.0 (SoC Transaction Router): […]
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 149: mmio_write_32(0xE00000D0, 0xf0 : | ((table[1] & 0xff) << 8) : | ((table[0] & 0xff) << 16) : | ((table[4] & 0xff) << 24));
Same story here, but with MCR: […]
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 153: tmp = mmio_read_32(0xE00000D4)
Now we read MDR: […]
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 155: table[3]
This can be called `value`
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 155: table[2]
This should be called `mask` (you can put names if you use a struct instead of a 2D array)
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 157: table[1]
Call this `offset`
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 158: table[0]
Call this `port`
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 159: table[5]
Call this `opcode`
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 171: static void delay_usecs(u64 usecs)
udelay
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 181: do_magic_writes
gpio_sc_sdcard_workaround
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 182: {
0xfed0c0000 is "IO_BASE_ADDRESS" and in this case is used to poke some GPIO_SCORE registers. […]
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 200: do_magic_writes2
ssa_safe_config
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 203:
These are message bus operations, all but one are and-or operations. […]
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 204: mmio_write_32(0xE00000D8, 0); : mmio_write_32(0xE00000D0, 0x10030BF0); : tmp = mmio_read_32(0x0E00000D4); : mmio_write_32(0xE00000D8, 0); : mmio_write_32(0xE00000D4, (tmp & 0xC0D0D0D0) | 0x1F2F2F2F); : : mmio_write_32(0xE00000D0, 0x11030BF0);
Port 0x03, Offset 0x0b = BUNIT_BALIMIT0
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 211: mmio_write_32(0xE00000D8, 0); : mmio_write_32(0xE00000D0, 0x100021F0); : tmp = mmio_read_32(0xE00000D4); : mmio_write_32(0xE00000D8, 0); : mmio_write_32(0xE00000D4, tmp | 0x80000100); : : mmio_write_32(0xE00000D0, 0x110021F0);
Port 0x00, Offset 0x21 = AUNIT_AVCCTL
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 218: mmio_write_32(0xE00000D8, 0); : mmio_write_32(0xE00000D0, 0x100060F0); : tmp = mmio_read_32(0xE00000D4); : mmio_write_32(0xE00000D8, 0); : mmio_write_32(0xE00000D4, tmp | 0x7FFFFFFF); : : mmio_write_32(0xE00000D0, 0x110060F0);
Port 0x00, Offset 0x60 = AUNIT_ACFCACV
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 225: mmio_write_32(0xE00000D8, 0); : mmio_write_32(0xE00000D0, 0x100841F0); : tmp = mmio_read_32(0xE00000D4); : mmio_write_32(0xE00000D8, 0); : mmio_write_32(0xE00000D4, tmp | 0x7FFFFFFF); : : mmio_write_32(0xE00000D0, 0x110841F0);
Port 0x08, Offset 0x41 = CUNIT_ACCESS_CTRL_VIOL
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 232: mmio_write_32(0xE00000D8, 0); : mmio_write_32(0xE00000D4, 0x70008); : mmio_write_32(0xE00000D0, 0x110843F0);
These three lines are different: instead of a and-or RMW operation, it's just a direct write. […]
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 235: mmio_write_32(0xE00000D8, 0); : mmio_write_32(0xE00000D0, 0x100203F0); : tmp = mmio_read_32(0xE00000D4); : mmio_write_32(0xE00000D8, 0); : mmio_write_32(0xE00000D4, tmp | 0x110430); : : mmio_write_32(0xE00000D0, 0x110203F0);
Port 0x02, Offset 0x03 = TUNIT_CTL
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 242: mmio_write_32(0xE00000D8, 0); : mmio_write_32(0xE00000D0, 0x110204F0); : tmp = mmio_read_32(0xE00000D4); : mmio_write_32(0xE00000D8, 0); : mmio_write_32(0xE00000D4, tmp | 0x40010); : mmio_write_32(0xE00000D0, 0x110204F0);
Port 0x02, Offset 0x04 = TUNIT_MISC_CTL
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 276: mmio_read_32(0xE00F8044)
Another PCI operation! […]
Done
https://review.coreboot.org/c/coreboot/+/43133/1/src/soc/intel/baytrail/refc... PS1, Line 276: 0xB0
R_PCH_PMC_MTPMC1
Done