build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: Add support for System76 EC ......................................................................
Patch Set 3:
(35 comments)
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... File src/ec/system76/ec/system76_ec.c:
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 7: static inline uint8_t system76_ec_read(uint8_t addr) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 8: return inb(SYSTEM76_EC_BASE + (uint16_t)addr); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 11: static inline void system76_ec_write(uint8_t addr, uint8_t data) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 12: outb(data, SYSTEM76_EC_BASE + (uint16_t)addr); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 15: void system76_ec_init(void) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 17: for (int i = 0; i < 256; i++) { please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 17: for (int i = 0; i < 256; i++) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 18: system76_ec_write((uint8_t)i, 0); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 18: system76_ec_write((uint8_t)i, 0); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 19: } please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 22: void system76_ec_flush(void) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 24: system76_ec_write(0, 4); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 27: int timeout; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 28: for (timeout = 10000; timeout > 0; timeout--) { please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 29: if (system76_ec_read(0) == 0) break; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 29: if (system76_ec_read(0) == 0) break; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 29: if (system76_ec_read(0) == 0) break; trailing statements should be on next line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 30: udelay(1); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 30: udelay(1); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 31: } please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 34: system76_ec_write(3, 0); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 37: void system76_ec_print(uint8_t byte) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 39: uint8_t len = system76_ec_read(3); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 41: system76_ec_write(len + 4, byte); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 43: system76_ec_write(3, len + 1); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 46: if (byte == '\n' || len >= 128) { please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 46: if (byte == '\n' || len >= 128) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 47: system76_ec_flush(); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 47: system76_ec_flush(); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 48: } please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/include/console/system7... File src/include/console/system76_ec.h:
https://review.coreboot.org/c/coreboot/+/43612/3/src/include/console/system7... PS3, Line 16: static inline void __system76_ec_init(void) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/43612/3/src/include/console/system7... PS3, Line 17: system76_ec_init(); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/include/console/system7... PS3, Line 19: static inline void __system76_ec_tx_flush(void) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/43612/3/src/include/console/system7... PS3, Line 20: system76_ec_flush(); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/include/console/system7... PS3, Line 22: static inline void __system76_ec_tx_byte(unsigned char byte) { open brace '{' following function definitions go on the next line