build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34332 )
Change subject: mediatek/mt8183: Support DDR frequency 3600Mbps ......................................................................
Patch Set 8:
(82 comments)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... File src/soc/mediatek/mt8183/dramc_init_setting.c:
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 58: }else if (freq_group == LP4X_DDR3200) { space required after that close brace '}'
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 68: if (freq_group == LP4X_DDR1600) that open brace { should be on the previous line
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 88: (0x0 << 31) | (0x0 << 30) | (0x6 << 20) | (0x9 << 16 )| space prohibited before that close parenthesis ')'
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 97: (0x1 << 31) | (0x1 << 30) | (0x7 << 20 )| (0x7 << 16 )| need consistent spacing around '|' (ctx:VxW)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 97: (0x1 << 31) | (0x1 << 30) | (0x7 << 20 )| (0x7 << 16 )| space prohibited before that close parenthesis ')'
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 546: if (freq_group == LP4X_DDR1600) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 554: (midpi_cap_sel << 9) | (0x1 << 10)| (0x3 << 17)| (lp3_sel << 20)); need consistent spacing around '|' (ctx:VxW)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 554: (midpi_cap_sel << 9) | (0x1 << 10)| (0x3 << 17)| (lp3_sel << 20)); need consistent spacing around '|' (ctx:VxW)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 676: resp = (read32(&ch[chn].nao.spcmdresp) >>4) & 0x1; need consistent spacing around '>>' (ctx:WxV)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 846: clrsetbits_le32(&ch[0].ao.shu[0].wodt, (0x1 << 29) |(0x1 << 31), need consistent spacing around '|' (ctx:WxV)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 847: (0x0 << 29) |(0x1 << 31)); need consistent spacing around '|' (ctx:WxV)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 851: int value = ( (rank == 0) ? 0x1a : 0x1e); space prohibited after that open parenthesis '('
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 901: setbits_le32(&ch[0].ao.shu[0].odtctrl,(0x1 << 0) | (0x1 << 30)| (0x1 << 31)); space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 901: setbits_le32(&ch[0].ao.shu[0].odtctrl,(0x1 << 0) | (0x1 << 30)| (0x1 << 31)); need consistent spacing around '|' (ctx:VxW)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 913: int value = ( (rank == 0) ? 0x19 : 0x1f); space prohibited after that open parenthesis '('
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1309: clrsetbits_le32(&ch[0].ao.perfctl0, (0x1 << 18) | (0x1 << 19), (0x0 << 18) | (0x1 << 19)); line over 96 characters
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1376: setbits_le32(&ch[0].phy.shu[0].b[1].dq[7], (0x1 << 12) |(0x1 << 13)); need consistent spacing around '|' (ctx:WxV)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1385: update_initial_settings(freq_group); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1385: update_initial_settings(freq_group); please, no space before tabs
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1385: update_initial_settings(freq_group); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1731: clrsetbits_le32(&ch[chn].ao.shu[0].conf[2], (0xff << 8), (r_dmfspchg_prdcnt << 8)); line over 96 characters
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... File src/soc/mediatek/mt8183/dramc_pi_calibration_api.c:
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 223: (freq_group == LP4X_DDR1600)|| (freq_group == LP4X_DDR2400) )) spaces required around that '||' (ctx:VxW)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 223: (freq_group == LP4X_DDR1600)|| (freq_group == LP4X_DDR2400) )) space prohibited before that close parenthesis ')'
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 645: }else { space required after that close brace '}'
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 820: u8 dqs, fsp, freqDiv= 4; spaces required around that '=' (ctx:VxW)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1002: dramc_mode_reg_write_by_rank(chn, rank, 0x1, MR01Value[fsp] ); space prohibited before that close parenthesis ')'
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1097: } else if (fine_tune > TX_DQ_COARSE_TUNE_TO_FINE_TUNE_TAP -10) { need consistent spacing around '-' (ctx:WxV)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1300: win_size = delay[bit].best_last -delay[bit].best_first ; need consistent spacing around '-' (ctx:WxV)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1300: win_size = delay[bit].best_last -delay[bit].best_first ; space prohibited before semicolon
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1333: win_size = delay[bit].best_last -delay[bit].best_first ; need consistent spacing around '-' (ctx:WxV)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1333: win_size = delay[bit].best_last -delay[bit].best_first ; space prohibited before semicolon
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1344: if ((min_winsize > *win_min_max) || ((min_winsize == *win_min_max) && (tmp_win_sum > vref_dly->max_win_sum))) { line over 96 characters
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1396: dramc_transfer_dly_tune(chn, tx_perbyte_dly[i].final_dly, adjust_center, &dqdly_tune[i]); line over 96 characters
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1410: (dqdly_tune[i].coarse_tune_small_oen << 5) + dqdly_tune[i].fine_tune; line over 96 characters
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1433: write32(&ch[chn].phy.shu[0].rk[rank].b[byte].dq[0], byte_dly_cell[byte]); line over 96 characters
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1438: (dqdly_tune[0].fine_tune << 8) | (dqdly_tune[1].fine_tune << 0) | line over 96 characters
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1439: (dqmdly_tune[0].fine_tune << 24) | (dqmdly_tune[1].fine_tune << 16)); line over 96 characters
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1441: clrsetbits_le32(&ch[chn].ao.shu[0].rk[rank].dqs2dq_cal1, 0x7ff | (0x7ff << 16), line over 96 characters
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1442: (dqdly_tune[0].fine_tune << 0) | (dqdly_tune[1].fine_tune << 16)); line over 96 characters
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1443: clrsetbits_le32(&ch[chn].ao.shu[0].rk[rank].dqs2dq_cal2, 0x7ff | (0x7ff << 16), line over 96 characters
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1444: (dqdly_tune[0].fine_tune << 0) | (dqdly_tune[1].fine_tune << 16)); line over 96 characters
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1445: clrsetbits_le32(&ch[chn].ao.shu[0].rk[rank].dqs2dq_cal5, 0x7ff | (0x7ff << 16), line over 96 characters
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1446: (dqmdly_tune[0].fine_tune << 0) | (dqmdly_tune[1].fine_tune << 16)); line over 96 characters
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1470: value = (dly[index + 1].best_dqdly << 24) | (dly[index + 1].best_dqdly << 16) | line over 96 characters
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1533: chn, rank, byte, center_dly[byte].min_center, center_dly[byte].max_center); line over 96 characters
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1538: center_dly[byte].final_dly = (center_dly[byte].min_center + center_dly[byte].max_center) >> 1; line over 96 characters
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1547: tune_diff = vref_dly[index].win_center - center_dly[byte].min_center; line over 96 characters
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1548: dq_delay_cell[index] = ((tune_diff * 100000000) / (freq / 2 * 64)) line over 96 characters
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1577: dqsdly_byte[byte] = (dqsdly_byte[byte] > 0) ? 0 : -dqsdly_byte[byte] ; space prohibited before semicolon
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1580: perbit_dly[bit].best_dqdly = dqsdly_byte[byte] + perbit_dly[bit].win_center; line over 96 characters
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1596: if (fsp == FSP_0) { braces {} are not necessary for any arm of this statement
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1730: dramc_dbg("all bits window found, early break! delay=0x%x\n", dly); line over 96 characters
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1738: win_perbit[bit].best_last -win_perbit[bit].best_first); need consistent spacing around '-' (ctx:WxV)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1768: u8 StartEXT2=0, StartEXT3=0, LastEXT2=0, LastEXT3=0; spaces required around that '=' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1768: u8 StartEXT2=0, StartEXT3=0, LastEXT2=0, LastEXT3=0; spaces required around that '=' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1768: u8 StartEXT2=0, StartEXT3=0, LastEXT2=0, LastEXT3=0; spaces required around that '=' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1768: u8 StartEXT2=0, StartEXT3=0, LastEXT2=0, LastEXT3=0; spaces required around that '=' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1781: if(val >= 24) space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1783: else if(val >= 18) space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1787: (0x1 << 28)| (0x1 << 27) | (0x1 << 26), need consistent spacing around '|' (ctx:VxW)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1788: (0x1 << 31) | (0x1 << 30)| (StartEXT2 << 29) | need consistent spacing around '|' (ctx:VxW)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1789: (LastEXT2 << 28)| (StartEXT3 << 27) | (LastEXT3 << 26)); need consistent spacing around '|' (ctx:VxW)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1795: u32 datlat, begin = 0, first = 0 ,sum = 0, best_step; space prohibited before that ',' (ctx:WxV)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1795: u32 datlat, begin = 0, first = 0 ,sum = 0, best_step; space required after that ',' (ctx:WxV)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1842: clrsetbits_le32(&ch[chn].ao.padctrl, 0x3 | 0x1 << 3 , space prohibited before that ',' (ctx:WxE)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1863: if ((freq_group == LP4X_DDR3200) ||(freq_group == LP4X_DDR3600) ) spaces required around that '||' (ctx:WxV)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1863: if ((freq_group == LP4X_DDR3200) ||(freq_group == LP4X_DDR3600) ) space prohibited before that close parenthesis ')'
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1920: clrsetbits_le32(&ch[chn].ao.shu[0].rk[1].dqsctl,0xf, read_dqsinctl << 0); space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1921: clrsetbits_le32(&ch[chn].ao.shu[0].rankctl, (0xf << 28) |(0xf << 20) | (0xf << 24) | (0xf ), line over 96 characters
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1921: clrsetbits_le32(&ch[chn].ao.shu[0].rankctl, (0xf << 28) |(0xf << 20) | (0xf << 24) | (0xf ), need consistent spacing around '|' (ctx:WxV)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1921: clrsetbits_le32(&ch[chn].ao.shu[0].rankctl, (0xf << 28) |(0xf << 20) | (0xf << 24) | (0xf ), space prohibited before that close parenthesis ')'
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/dra... PS8, Line 1922: (read_dqsinctl << 28) | (rankinctl_root << 20) | (rankinctl_root << 24) | (rankinctl_root)); line over 96 characters
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/emi... File src/soc/mediatek/mt8183/emi.c:
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/emi... PS8, Line 344: while (loop < loop_cnt) that open brace { should be on the previous line
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/emi... PS8, Line 347: for (u8 *addr = _dram; addr < (u8*)0x100000000; addr+=0x40000000) { "(foo*)" should be "(foo *)"
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/emi... PS8, Line 347: for (u8 *addr = _dram; addr < (u8*)0x100000000; addr+=0x40000000) { spaces required around that '+=' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/inc... File src/soc/mediatek/mt8183/include/soc/dramc_common_mt8183.h:
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/inc... PS8, Line 20: { open brace '{' following enum go on the same line
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/inc... PS8, Line 21: DRAM_DFS_SHUFFLE_1 = 0, please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/inc... PS8, Line 22: DRAM_DFS_SHUFFLE_2, please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/inc... PS8, Line 23: DRAM_DFS_SHUFFLE_3, please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/inc... PS8, Line 24: DRAM_DFS_SHUFFLE_MAX please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/inc... File src/soc/mediatek/mt8183/include/soc/dramc_pi_api.h:
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/inc... PS8, Line 108: DLL_MASTER = 0, please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/34332/8/src/soc/mediatek/mt8183/inc... PS8, Line 109: DLL_SLAVE, please, no spaces at the start of a line