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 2:
(77 comments)
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... File src/soc/mediatek/mt8192/include/soc/pmif_spi.h:
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 87: SPI_CLK = 0x1, code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 87: SPI_CLK = 0x1, please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 88: SPI_CSN = 0x1 << 1, code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 88: SPI_CSN = 0x1 << 1, please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 89: SPI_MOSI = 0x1 << 2, code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 89: SPI_MOSI = 0x1 << 2, please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 90: SPI_MISO = 0x1 << 3, code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 90: SPI_MISO = 0x1 << 3, please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 91: 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/2/src/soc/mediatek/mt8192/inc... PS2, Line 91: 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/2/src/soc/mediatek/mt8192/inc... PS2, Line 92: SPI_SMT = (SPI_CLK | SPI_CSN | SPI_MOSI | SPI_MISO), code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 92: 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/2/src/soc/mediatek/mt8192/inc... PS2, Line 93: 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/2/src/soc/mediatek/mt8192/inc... PS2, Line 93: 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/2/src/soc/mediatek/mt8192/inc... PS2, Line 97: IO_4_MA = 0x8, code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 97: IO_4_MA = 0x8, please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 101: SPI_CLK_SHIFT = 0, code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 101: SPI_CLK_SHIFT = 0, please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 102: SPI_CSN_SHIFT = 4, code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 102: SPI_CSN_SHIFT = 4, please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 103: SPI_MOSI_SHIFT = 8, code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 103: SPI_MOSI_SHIFT = 8, please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 104: SPI_MISO_SHIFT = 12, code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 104: SPI_MISO_SHIFT = 12, please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 105: 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/2/src/soc/mediatek/mt8192/inc... PS2, Line 105: 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/2/src/soc/mediatek/mt8192/inc... PS2, Line 106: 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/2/src/soc/mediatek/mt8192/inc... PS2, Line 106: 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/2/src/soc/mediatek/mt8192/inc... PS2, Line 122: DUMMY_READ_CYCLES = 0X8, code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 122: DUMMY_READ_CYCLES = 0X8, please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... File src/soc/mediatek/mt8192/include/soc/pmif_spmi.h:
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 89: { open brace '{' following enum go on the same line
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 95: { open brace '{' following enum go on the same line
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 101: { open brace '{' following enum go on the same line
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 108: { open brace '{' following enum go on the same line
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 114: { open brace '{' following enum go on the same line
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 120: { open brace '{' following enum go on the same line
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... File src/soc/mediatek/mt8192/include/soc/spmi.h:
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 21: { open brace '{' following enum go on the same line
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 29: { open brace '{' following enum go on the same line
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 49: { open brace '{' following enum go on the same line
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/inc... PS2, Line 61: { open brace '{' following enum go on the same line
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/pmi... File src/soc/mediatek/mt8192/pmif.c:
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/pmi... PS2, 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/2/src/soc/mediatek/mt8192/pmi... PS2, Line 28: return 0; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/pmi... PS2, 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/2/src/soc/mediatek/mt8192/pmi... PS2, Line 44: return 0; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/pmi... PS2, Line 57: if(ret) space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/pmi... PS2, Line 76: if(ret) space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/pmi... PS2, 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/2/src/soc/mediatek/mt8192/pmi... PS2, 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/2/src/soc/mediatek/mt8192/pmi... PS2, 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/2/src/soc/mediatek/mt8192/pmi... PS2, 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/2/src/soc/mediatek/mt8192/pmi... PS2, 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/2/src/soc/mediatek/mt8192/pmi... File src/soc/mediatek/mt8192/pmif_clk.c:
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/pmi... PS2, Line 28: 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/2/src/soc/mediatek/mt8192/pmi... PS2, Line 31: 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/2/src/soc/mediatek/mt8192/pmi... PS2, Line 35: 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/2/src/soc/mediatek/mt8192/pmi... PS2, Line 39: 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/2/src/soc/mediatek/mt8192/pmi... PS2, Line 43: write32(&mtk_apmixed->ulposc1_con0, read32(&mtk_apmixed->ulposc1_con0) &~(0x7f)); need consistent spacing around '&' (ctx:WxO)
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/pmi... PS2, Line 51: 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/2/src/soc/mediatek/mt8192/pmi... PS2, Line 54: 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/2/src/soc/mediatek/mt8192/pmi... PS2, Line 57: 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/2/src/soc/mediatek/mt8192/pmi... PS2, Line 60: 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/2/src/soc/mediatek/mt8192/pmi... PS2, Line 64: write32(&mtk_apmixed->ulposc1_con1, read32(&mtk_apmixed->ulposc1_con1) &~(0xff)); need consistent spacing around '&' (ctx:WxO)
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/pmi... PS2, Line 71: write32(&mtk_apmixed->ulposc1_con2, read32(&mtk_apmixed->ulposc1_con2) &~(0xff)); need consistent spacing around '&' (ctx:WxO)
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/pmi... PS2, 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/2/src/soc/mediatek/mt8192/pmi... PS2, Line 162: if(ret) space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/pmi... File src/soc/mediatek/mt8192/pmif_spi.c:
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/pmi... PS2, Line 69: SET32_BITFIELDS(&mtk_pmicspi_mst->man_acc, MAN_ACC_SPI_RW, OP_WR, MAN_ACC_SPI_OP, OP_CSL); line over 96 characters
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/pmi... PS2, Line 71: SET32_BITFIELDS(&mtk_pmicspi_mst->man_acc, MAN_ACC_SPI_RW, OP_WR, MAN_ACC_SPI_OP, OP_OUTS); line over 96 characters
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/pmi... PS2, Line 72: SET32_BITFIELDS(&mtk_pmicspi_mst->man_acc, MAN_ACC_SPI_RW, OP_WR, MAN_ACC_SPI_OP, OP_CSH); line over 96 characters
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/pmi... PS2, Line 77: SET32_BITFIELDS(&mtk_pmicspi_mst->man_acc, MAN_ACC_SPI_RW, OP_WR, MAN_ACC_SPI_OP, OP_OUTS); line over 96 characters
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/pmi... PS2, Line 78: SET32_BITFIELDS(&mtk_pmicspi_mst->man_acc, MAN_ACC_SPI_RW, OP_WR, MAN_ACC_SPI_OP, OP_OUTS); line over 96 characters
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/pmi... PS2, Line 79: SET32_BITFIELDS(&mtk_pmicspi_mst->man_acc, MAN_ACC_SPI_RW, OP_WR, MAN_ACC_SPI_OP, OP_OUTS); line over 96 characters
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/pmi... PS2, Line 80: SET32_BITFIELDS(&mtk_pmicspi_mst->man_acc, MAN_ACC_SPI_RW, OP_WR, MAN_ACC_SPI_OP, OP_OUTS); line over 96 characters
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/pmi... PS2, Line 311: 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/2/src/soc/mediatek/mt8192/pmi... File src/soc/mediatek/mt8192/pmif_spmi.c:
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/pmi... PS2, Line 48: (WDT_UNLOCK_KEY << WDT_UNLOCK_SHIFT) | \ Avoid unnecessary line continuations
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/pmi... PS2, Line 120: spmi_device_cnt = sizeof(spmi_dev)/sizeof(spmi_dev[0]); Prefer ARRAY_SIZE(spmi_dev)
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/pmi... PS2, Line 200: hw_bytecnt = PMIF_BYTECNT_MAX -1; need consistent spacing around '-' (ctx:WxV)
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/pmi... PS2, Line 238: if(ret) space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/45398/2/src/soc/mediatek/mt8192/pmi... PS2, Line 243: if(ret) space required before the open parenthesis '('