Jitao Shi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47380 )
Change subject: mb/google/kukui: Add panel api after dsi start ......................................................................
Patch Set 4:
(14 comments)
https://review.coreboot.org/c/coreboot/+/47380/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47380/2//COMMIT_MSG@11 PS2, Line 11:
BUG=b:168728787
Done
https://review.coreboot.org/c/coreboot/+/47380/1/src/mainboard/google/kukui/... File src/mainboard/google/kukui/mainboard.c:
https://review.coreboot.org/c/coreboot/+/47380/1/src/mainboard/google/kukui/... PS1, Line 101: if (panel->post_power_on)
suspect code indent for conditional statements (7, 15)
done
https://review.coreboot.org/c/coreboot/+/47380/1/src/mainboard/google/kukui/... PS1, Line 101: if (panel->post_power_on)
please, no spaces at the start of a line
done
https://review.coreboot.org/c/coreboot/+/47380/1/src/mainboard/google/kukui/... PS1, Line 102: panel->post_power_on();
please, no spaces at the start of a line
done
https://review.coreboot.org/c/coreboot/+/47380/1/src/mainboard/google/kukui/... PS1, Line 102: panel->post_power_on();
code indent should use tabs where possible
done
https://review.coreboot.org/c/coreboot/+/47380/2/src/mainboard/google/kukui/... File src/mainboard/google/kukui/mainboard.c:
https://review.coreboot.org/c/coreboot/+/47380/2/src/mainboard/google/kukui/... PS2, Line 99: static void post_power_on_panel(struct panel_description *panel) : { : if (panel->post_power_on) : panel->post_power_on(); : }
If this is two lines, won't be used anywhere else, and no default (unlink power_on_panel) actions ne […]
Done
https://review.coreboot.org/c/coreboot/+/47380/2/src/mainboard/google/kukui/... File src/mainboard/google/kukui/panel.h:
https://review.coreboot.org/c/coreboot/+/47380/2/src/mainboard/google/kukui/... PS2, Line 24: turn on panel
panel is turned on
Done
https://review.coreboot.org/c/coreboot/+/47380/1/src/mainboard/google/kukui/... File src/mainboard/google/kukui/panel_anx7625.c:
https://review.coreboot.org/c/coreboot/+/47380/1/src/mainboard/google/kukui/... PS1, Line 38: * so we should do nothing here.
code indent should use tabs where possible
done
https://review.coreboot.org/c/coreboot/+/47380/1/src/mainboard/google/kukui/... PS1, Line 39: */
code indent should use tabs where possible
done
https://review.coreboot.org/c/coreboot/+/47380/1/src/mainboard/google/kukui/... PS1, Line 47: printk(BIOS_ERR, "start display start_anx7625\n");
Prefer using '"%s... […]
done
https://review.coreboot.org/c/coreboot/+/47380/1/src/mainboard/google/kukui/... PS1, Line 55: .power_on = dummy_power_on,
please, no spaces at the start of a line
done
https://review.coreboot.org/c/coreboot/+/47380/1/src/mainboard/google/kukui/... PS1, Line 56: .post_power_on = start_anx7625,
please, no spaces at the start of a line
done
https://review.coreboot.org/c/coreboot/+/47380/2/src/mainboard/google/kukui/... File src/mainboard/google/kukui/panel_anx7625.c:
https://review.coreboot.org/c/coreboot/+/47380/2/src/mainboard/google/kukui/... PS2, Line 44: u8 i2c_bus = 4;
If this will be used in multiple functions I'd like to replace it with a enum or const, e.g.. […]
Done
https://review.coreboot.org/c/coreboot/+/47380/2/src/mainboard/google/kukui/... PS2, Line 45: struct edid *edid = &anx7625_data.edid;
I think you don't need to do this if it'll be only used one time (the original code did that because […]
Done