build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33431 )
Change subject: TEMP: NOT FOR REVIEW: google/mistral: Add LED calibration ......................................................................
Patch Set 1:
(34 comments)
https://review.coreboot.org/#/c/33431/1/src/drivers/i2c/lp5562/led_lp5562_ca... File src/drivers/i2c/lp5562/led_lp5562_calibration.h:
https://review.coreboot.org/#/c/33431/1/src/drivers/i2c/lp5562/led_lp5562_ca... PS1, Line 36: uint8_t code_offset; trailing whitespace
https://review.coreboot.org/#/c/33431/1/src/drivers/i2c/lp5562/led_lp5562_ca... PS1, Line 38: uint8_t ramp_num; trailing whitespace
https://review.coreboot.org/#/c/33431/1/src/drivers/i2c/lp5562/led_lp5562_ca... PS1, Line 56: const struct lp5562_calibration_code_map* code_map; "foo* bar" should be "foo *bar"
https://review.coreboot.org/#/c/33431/1/src/drivers/i2c/lp5562/led_lp5562_ca... PS1, Line 67: int led_lp5562_calibrate(const struct lp5562_calibration_data* cal_data, "foo* bar" should be "foo *bar"
https://review.coreboot.org/#/c/33431/1/src/drivers/i2c/lp5562/led_lp5562_ca... PS1, Line 68: struct lp5562_calibrated_color* calibrated_colors); "foo* bar" should be "foo *bar"
https://review.coreboot.org/#/c/33431/1/src/drivers/i2c/lp5562/led_lp5562_ca... File src/drivers/i2c/lp5562/led_lp5562_calibration.c:
https://review.coreboot.org/#/c/33431/1/src/drivers/i2c/lp5562/led_lp5562_ca... PS1, Line 24: if (step_time > 63) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/#/c/33431/1/src/drivers/i2c/lp5562/led_lp5562_ca... PS1, Line 33: const struct lp5562_calibration_code_map* code_map, "foo* bar" should be "foo *bar"
https://review.coreboot.org/#/c/33431/1/src/drivers/i2c/lp5562/led_lp5562_ca... PS1, Line 47: const struct lp5562_calibration_code_map* code_map, "foo* bar" should be "foo *bar"
https://review.coreboot.org/#/c/33431/1/src/drivers/i2c/lp5562/led_lp5562_ca... PS1, Line 125: int led_lp5562_calibrate(const struct lp5562_calibration_data* cal_data, "foo* bar" should be "foo *bar"
https://review.coreboot.org/#/c/33431/1/src/drivers/i2c/lp5562/led_lp5562_ca... PS1, Line 126: struct lp5562_calibrated_color* calibrated_colors) "foo* bar" should be "foo *bar"
https://review.coreboot.org/#/c/33431/1/src/drivers/i2c/lp5562/led_lp5562_ca... PS1, Line 136: engine_program[bgr_index].program_text; Avoid multiple line dereference - prefer 'cal_data->pattern->engine_program[bgr_index].program_text'
https://review.coreboot.org/#/c/33431/1/src/drivers/i2c/lp5562/led_lp5562_ca... PS1, Line 140: current_values[rgb_index]; Avoid multiple line dereference - prefer 'calibrated_colors[cal_data->color_index].current_values[rgb_index]'
https://review.coreboot.org/#/c/33431/1/src/drivers/i2c/lp5562/led_lp5562_ca... PS1, Line 148: pwm_values[rgb_index]; Avoid multiple line dereference - prefer 'calibrated_colors[cal_data->color_index].pwm_values[rgb_index]'
https://review.coreboot.org/#/c/33431/1/src/drivers/i2c/lp5562/led_lp5562_pr... File src/drivers/i2c/lp5562/led_lp5562_programs.c:
https://review.coreboot.org/#/c/33431/1/src/drivers/i2c/lp5562/led_lp5562_pr... PS1, Line 75: }, code indent should use tabs where possible
https://review.coreboot.org/#/c/33431/1/src/drivers/i2c/lp5562/led_lp5562_pr... PS1, Line 75: }, please, no space before tabs
https://review.coreboot.org/#/c/33431/1/src/drivers/i2c/lp5562/led_lp5562_pr... PS1, Line 75: }, please, no spaces at the start of a line
https://review.coreboot.org/#/c/33431/1/src/mainboard/google/mistral/led_cal... File src/mainboard/google/mistral/led_calibration.c:
https://review.coreboot.org/#/c/33431/1/src/mainboard/google/mistral/led_cal... PS1, Line 60: if (*str == *p) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/#/c/33431/1/src/mainboard/google/mistral/led_cal... PS1, Line 65: if (*p == 0) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/#/c/33431/1/src/mainboard/google/mistral/led_cal... PS1, Line 81: if (*str == *p++) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/#/c/33431/1/src/mainboard/google/mistral/led_cal... PS1, Line 116: if (d & 0x80) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/#/c/33431/1/src/mainboard/google/mistral/led_cal... PS1, Line 135: if (fmap_locate_area_as_rdev("RO_VPD", &rdev)) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/#/c/33431/1/src/mainboard/google/mistral/led_cal... PS1, Line 147: if (read_len < 0) break; trailing statements should be on next line
https://review.coreboot.org/#/c/33431/1/src/mainboard/google/mistral/led_cal... PS1, Line 149: if ((type != VPD_TYPE_STRING) && (type != VPD_TYPE_INFO)) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/#/c/33431/1/src/mainboard/google/mistral/led_cal... PS1, Line 154: if (len < 0) break; trailing statements should be on next line
https://review.coreboot.org/#/c/33431/1/src/mainboard/google/mistral/led_cal... PS1, Line 160: if (read_len < 0) break; trailing statements should be on next line
https://review.coreboot.org/#/c/33431/1/src/mainboard/google/mistral/led_cal... PS1, Line 163: (strcmp((char*)buff, key_name) == 0)) { "(foo*)" should be "(foo *)"
https://review.coreboot.org/#/c/33431/1/src/mainboard/google/mistral/led_cal... PS1, Line 171: if (read_len < 0) break; trailing statements should be on next line
https://review.coreboot.org/#/c/33431/1/src/mainboard/google/mistral/led_cal... PS1, Line 174: if (len < 0) break; trailing statements should be on next line
https://review.coreboot.org/#/c/33431/1/src/mainboard/google/mistral/led_cal... PS1, Line 186: read_len = rdev_readat(&rdev, caldata_buff, offset, len); line over 80 characters
https://review.coreboot.org/#/c/33431/1/src/mainboard/google/mistral/led_cal... PS1, Line 187: if (read_len < 0) break; trailing statements should be on next line
https://review.coreboot.org/#/c/33431/1/src/mainboard/google/mistral/led_cal... PS1, Line 221: if (len < 1) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/#/c/33431/1/src/mainboard/google/mistral/led_cal... PS1, Line 225: if (strncmp(color_names[color], p, len) == 0) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/#/c/33431/1/src/mainboard/google/mistral/led_cal... PS1, Line 231: if (len < 0) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/#/c/33431/1/src/mainboard/google/mistral/led_cal... PS1, Line 262: trailing whitespace