build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45398 )
Change subject: soc/mediatek/mt8192: add pmif driver ......................................................................
Patch Set 1:
(76 comments)
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... File src/soc/mediatek/mt8192/include/soc/pmif_spi.h:
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 80: #define PMIF_SPI_AP_SECURE_SWINF_CHAN_NO BIT(PMIF_SPI_SWINF_0_CHAN_NO + PMIF_SPI_AP_SECURE_SWINF_NO) line over 96 characters
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 89: SPI_CLK = 0x1, code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 89: SPI_CLK = 0x1, please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 90: SPI_CSN = 0x1 << 1, code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 90: SPI_CSN = 0x1 << 1, please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 91: SPI_MOSI = 0x1 << 2, code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 91: SPI_MOSI = 0x1 << 2, please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 92: SPI_MISO = 0x1 << 3, code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 92: SPI_MISO = 0x1 << 3, please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 93: SPI_FILTER = (SPI_CLK | SPI_CSN | SPI_MOSI | SPI_MISO) << 4, code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 93: SPI_FILTER = (SPI_CLK | SPI_CSN | SPI_MOSI | SPI_MISO) << 4, please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 94: SPI_SMT = (SPI_CLK | SPI_CSN | SPI_MOSI | SPI_MISO), code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 94: SPI_SMT = (SPI_CLK | SPI_CSN | SPI_MOSI | SPI_MISO), please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 95: SPI_PULL_DISABLE = (SPI_CLK | SPI_CSN | SPI_MOSI | SPI_MISO) << 4, code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 95: SPI_PULL_DISABLE = (SPI_CLK | SPI_CSN | SPI_MOSI | SPI_MISO) << 4, please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 99: IO_4_MA = 0x8, code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 99: IO_4_MA = 0x8, please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 103: SPI_CLK_SHIFT = 0, code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 103: SPI_CLK_SHIFT = 0, please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 104: SPI_CSN_SHIFT = 4, code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 104: SPI_CSN_SHIFT = 4, please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 105: SPI_MOSI_SHIFT = 8, code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 105: SPI_MOSI_SHIFT = 8, please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 106: SPI_MISO_SHIFT = 12, code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 106: SPI_MISO_SHIFT = 12, please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 107: SPI_DRIVING = (IO_4_MA << SPI_CLK_SHIFT | IO_4_MA << SPI_CSN_SHIFT | code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 107: SPI_DRIVING = (IO_4_MA << SPI_CLK_SHIFT | IO_4_MA << SPI_CSN_SHIFT | please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 108: IO_4_MA << SPI_MOSI_SHIFT | IO_4_MA << SPI_MISO_SHIFT), code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 108: IO_4_MA << SPI_MOSI_SHIFT | IO_4_MA << SPI_MISO_SHIFT), please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 124: DUMMY_READ_CYCLES = 0X8, code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 124: DUMMY_READ_CYCLES = 0X8, please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... File src/soc/mediatek/mt8192/include/soc/pmif_spmi.h:
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 89: { open brace '{' following enum go on the same line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 95: { open brace '{' following enum go on the same line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 101: { open brace '{' following enum go on the same line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 108: { open brace '{' following enum go on the same line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 114: { open brace '{' following enum go on the same line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 120: { open brace '{' following enum go on the same line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... File src/soc/mediatek/mt8192/include/soc/spmi.h:
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 21: { open brace '{' following enum go on the same line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 29: { open brace '{' following enum go on the same line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 49: { open brace '{' following enum go on the same line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/inc... PS1, Line 61: { open brace '{' following enum go on the same line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... File src/soc/mediatek/mt8192/pmif.c:
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 15: static unsigned int pmif_check_idle(struct pmif *arb, unsigned int timeout_us) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 28: return 0; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 31: static inline unsigned int pmif_check_vldclr(struct pmif *arb, unsigned int timeout_us) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 44: return 0; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 57: if(ret) space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 76: if(ret) space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 87: static int pmif_spmi_read_cmd(struct pmif *arb,unsigned int slvid, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 91: return pmif_send_cmd(arb, 0, PMIF_CMD_EXT_REG_LONG, slvid, addr, rdata, 0, (PMIF_BYTECNT_MAX - 1)); line over 96 characters
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 97: return pmif_send_cmd(arb, 1, PMIF_CMD_EXT_REG_LONG, slvid, addr, NULL, wdata, (PMIF_BYTECNT_MAX - 1)); line over 96 characters
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 104: return pmif_send_cmd(arb, 0, PMIF_CMD_REG_0, slvid, addr, rdata, 0, (PMIF_BYTECNT_MAX - 1)); line over 96 characters
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 110: return pmif_send_cmd(arb, 1, PMIF_CMD_REG_0, slvid, addr, NULL, wdata, (PMIF_BYTECNT_MAX - 1)); line over 96 characters
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... File src/soc/mediatek/mt8192/pmif_clk.c:
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 43: while (read32(&mtk_topckgen->clk26cali_0) & 0x10) that open brace { should be on the previous line
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 47: if(i > 100) space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 52: output = ((temp * 26000) ) / 1024; /* Khz */ space prohibited before that close parenthesis ')'
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 66: write32(&mtk_apmixed->ulposc1_con0, read32(&mtk_apmixed->ulposc1_con0) &~(0x1 << 24)); need consistent spacing around '&' (ctx:WxO)
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 69: write32(&mtk_apmixed->ulposc1_con0, read32(&mtk_apmixed->ulposc1_con0) &~(0x3f << 18)); need consistent spacing around '&' (ctx:WxO)
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 73: write32(&mtk_apmixed->ulposc1_con0, read32(&mtk_apmixed->ulposc1_con0) &~(0xf << 14)); need consistent spacing around '&' (ctx:WxO)
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 77: write32(&mtk_apmixed->ulposc1_con0, read32(&mtk_apmixed->ulposc1_con0) &~(0x7f << 7)); need consistent spacing around '&' (ctx:WxO)
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 81: write32(&mtk_apmixed->ulposc1_con0, read32(&mtk_apmixed->ulposc1_con0) &~(0x7f)); need consistent spacing around '&' (ctx:WxO)
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 89: write32(&mtk_apmixed->ulposc1_con1, read32(&mtk_apmixed->ulposc1_con1) &~(0x1 << 26)); need consistent spacing around '&' (ctx:WxO)
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 92: write32(&mtk_apmixed->ulposc1_con1, read32(&mtk_apmixed->ulposc1_con1) &~(0x3 << 24)); need consistent spacing around '&' (ctx:WxO)
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 95: write32(&mtk_apmixed->ulposc1_con1, read32(&mtk_apmixed->ulposc1_con1) &~(0xff << 16)); need consistent spacing around '&' (ctx:WxO)
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 98: write32(&mtk_apmixed->ulposc1_con1, read32(&mtk_apmixed->ulposc1_con1) &~(0xff << 8)); need consistent spacing around '&' (ctx:WxO)
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 102: write32(&mtk_apmixed->ulposc1_con1, read32(&mtk_apmixed->ulposc1_con1) &~(0xff)); need consistent spacing around '&' (ctx:WxO)
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 109: write32(&mtk_apmixed->ulposc1_con2, read32(&mtk_apmixed->ulposc1_con2) &~(0xff)); need consistent spacing around '&' (ctx:WxO)
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 119: write32(&mtk_apmixed->ulposc1_con0, read32(&mtk_apmixed->ulposc1_con0) &~(0x7f)); need consistent spacing around '&' (ctx:WxO)
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 203: if(ret) space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... File src/soc/mediatek/mt8192/pmif_spi.c:
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 39: SET32_BITFIELDS(&arb->mtk_pmif->spi_mode_ctrl, SPI_MODE_CTRL_SRCLK_EN, 1, SPI_MODE_CTRL_SRVOL_EN, 1); line over 96 characters
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 307: write32(&arb->mtk_pmif->other_inf_en, read32(&arb->mtk_pmif->other_inf_en) | (0x3 << 4)); line over 96 characters
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... File src/soc/mediatek/mt8192/pmif_spmi.c:
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 51: (WDT_UNLOCK_KEY << WDT_UNLOCK_SHIFT) | \ Avoid unnecessary line continuations
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 100: for (i = 0;i < 3; i++) { space required after that ';' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 128: spmi_device_cnt = sizeof(spmi_dev)/sizeof(spmi_dev[0]); Prefer ARRAY_SIZE(spmi_dev)
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 209: hw_bytecnt = PMIF_BYTECNT_MAX -1; need consistent spacing around '-' (ctx:WxV)
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 247: if(ret) space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/45398/1/src/soc/mediatek/mt8192/pmi... PS1, Line 252: if(ret) space required before the open parenthesis '('