[L] Change in coreboot[master]: drivers/i2c: Add PI7C9X2G608GP PCIe switch driver (pi608gp)
Attention is currently required from: Paul Menzel, Mario Scheithauer, Lean Sheng Tan, Werner Zeh.
Jan Samek has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74433 )
Change subject: drivers/i2c: Add PI7C9X2G608GP PCIe switch driver (pi608gp) ......................................................................
Patch Set 12:
(6 comments)
File src/drivers/i2c/pi608gp/pi608gp.h:
https://review.coreboot.org/c/coreboot/+/74433/comment/c0497a40_9575ce10 PS12, Line 10: #define DEEMPH_LVL_MV(_LVL, _LVL_10) {.lvl = _LVL, .lvl_10 = _LVL_10}
Add spaces after { and before }?
Done
File src/drivers/i2c/pi608gp/pi608gp.c:
https://review.coreboot.org/c/coreboot/+/74433/comment/a6ad36b5_e2f5c282 PS12, Line 17: /* Only some of the available registers are implemented. : For a full list, see the PI7C9X2G608GP datasheet. */
As it’s not in a function body you could use: […]
Done
https://review.coreboot.org/c/coreboot/+/74433/comment/f5de48c0_1d8adccf PS12, Line 26: level
Add the unit to the name?
Done
https://review.coreboot.org/c/coreboot/+/74433/comment/e0ac379c_1237578c PS12, Line 48: { 0, 0}
Use spaces consistently?
I would like to keep the numbers aligned, as it reads better and can be easier compared with the table in the datasheet. This is quite a special case as the numbers can be read as decimals.
https://review.coreboot.org/c/coreboot/+/74433/comment/a931928c_9d0ef3b7 PS12, Line 54: uint8_t
Please use int [1]? […]
Done
https://review.coreboot.org/c/coreboot/+/74433/comment/a5ad2d69_0fb1ba7a PS12, Line 71: /* Compose the SMBus message for register read init operation (from MSB to LSB): : Byte 1: 7:3 = Rsvd., 2:0 = Command, : Byte 2: 7:4 = Rsvd., 3:0 = Port Select[4:1], : Byte 3: 7 = Port Select[0], 6 = Rsvd., 5:2 = Byte Enable, 1:0 = Reg. Addr. [11:10], : Byte 4: 7:0 = Reg. Addr.[9:2] (Reg. Addr. [1:0] fixed to 0). */
Maybe use non-concise form?
Can you please elaborate, e.g. what could be added?
-
Jan Samek (Code Review)