build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32692 )
Change subject: drivers/i2c: Add LP5562 LED driver ......................................................................
Patch Set 10:
(32 comments)
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... File src/drivers/i2c/lp5562/led_lp5562.h:
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 33: int led_lp5562_display_pattern(unsigned i2c_bus, enum display_pattern pattern); Prefer 'unsigned int' to bare use of 'unsigned'
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... File src/drivers/i2c/lp5562/led_lp5562.c:
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 110: * The controller has 96 bytes of SRAM for code/data, availabe as three 32 byte 'availabe' may be misspelled - perhaps 'available'?
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 124: unsigned i2c_bus; Prefer 'unsigned int' to bare use of 'unsigned'
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 129: static void led_lp5562_init(unsigned i2c_bus); Prefer 'unsigned int' to bare use of 'unsigned'
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 154: /* Accessing reset regsiter is expected to fail. */ 'regsiter' may be misspelled - perhaps 'register'?
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 177: const uint8_t *data, unsigned count) Prefer 'unsigned int' to bare use of 'unsigned'
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 254: const uint8_t *program, unsigned count) Prefer 'unsigned int' to bare use of 'unsigned'
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 288: enable_reg &= ~LP5562_ENABLE_ALL_MASK; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 288: enable_reg &= ~LP5562_ENABLE_ALL_MASK; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 289: enable_reg |= LP5562_ENABLE_CHIP_EN; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 289: enable_reg |= LP5562_ENABLE_CHIP_EN; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 290: enable_reg |= LP5562_ENABLE_ALL_HOLD; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 290: enable_reg |= LP5562_ENABLE_ALL_HOLD; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 293: /* All engines in LOAD mode. */ code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 294: ledc_write_opmode(ledc, LP5562_OPMODE_ALL_LOAD); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 294: ledc_write_opmode(ledc, LP5562_OPMODE_ALL_LOAD); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 296: /* Inject program code for each engine and start address */ code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 297: load_addr = LP5562_PROG_MEM_ENG1; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 297: load_addr = LP5562_PROG_MEM_ENG1; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 298: for (i = 0; i < LED_LP5562_NUM_OF_ENGINES; i++) { code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 298: for (i = 0; i < LED_LP5562_NUM_OF_ENGINES; i++) { please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 306: } code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 306: } please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 308: /* All engines on run. */ code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 309: ledc_write_opmode(ledc, LP5562_OPMODE_ALL_RUN); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 309: ledc_write_opmode(ledc, LP5562_OPMODE_ALL_RUN); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 311: enable_reg &= ~LP5562_ENABLE_ALL_MASK; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 311: enable_reg &= ~LP5562_ENABLE_ALL_MASK; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 312: enable_reg |= LP5562_ENABLE_ALL_RUN; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 312: enable_reg |= LP5562_ENABLE_ALL_RUN; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 359: int led_lp5562_display_pattern(unsigned i2c_bus, enum display_pattern pattern) Prefer 'unsigned int' to bare use of 'unsigned'
https://review.coreboot.org/c/coreboot/+/32692/10/src/drivers/i2c/lp5562/led... PS10, Line 403: static void led_lp5562_init(unsigned i2c_bus) Prefer 'unsigned int' to bare use of 'unsigned'