Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34505 )
Change subject: mb/google/kukui: Add panels for Kodama ......................................................................
Patch Set 29:
(10 comments)
https://review.coreboot.org/c/coreboot/+/34505/11//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34505/11//COMMIT_MSG@7 PS11, Line 7: a
You can remove the article. It’s not needed in the summary.
Ack
https://review.coreboot.org/c/coreboot/+/34505/11//COMMIT_MSG@8 PS11, Line 8:
Please mention the panel name in the commit message description, and the datasheet name and revision […]
Ack
https://review.coreboot.org/c/coreboot/+/34505/11//COMMIT_MSG@12 PS11, Line 12: Signed-off-by: Peichao Wang peichao.wang@bitland.corp-partner.google.com
Please move the Signed-off-by line below the Change-Id. (The git hooks should do it that way. […]
Ack
https://review.coreboot.org/c/coreboot/+/34505/2/src/mainboard/google/kukui/... File src/mainboard/google/kukui/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/34505/2/src/mainboard/google/kukui/... PS2, Line 25: ramstage-y += display.c : ramstage-$(CONFIG_BOARD_GOOGLE_KUKUI) += panel_kukui.c : ramstage-$(CONFIG_BOARD_GOOGLE_KRANE) += panel_krane.c
these should not exist in your commit.
Ack
https://review.coreboot.org/c/coreboot/+/34505/2/src/mainboard/google/kukui/... PS2, Line 28: panel_kodama
can we just reuse panel_krane. […]
Ack
https://review.coreboot.org/c/coreboot/+/34505/2/src/mainboard/google/kukui/... File src/mainboard/google/kukui/display.h:
https://review.coreboot.org/c/coreboot/+/34505/2/src/mainboard/google/kukui/... PS2, Line 2: This
this file should be removed
Ack
https://review.coreboot.org/c/coreboot/+/34505/9/src/mainboard/google/kukui/... File src/mainboard/google/kukui/panel_kodama.c:
https://review.coreboot.org/c/coreboot/+/34505/9/src/mainboard/google/kukui/... PS9, Line 16: #include <console/console.h> : #include <console/console.h> : #include <delay.h> : #include <device/device.h> : #include <edid.h> : #include <gpio.h> : #include <soc/auxadc.h> : #include <soc/dsi.h> : #include <soc/gpio.h> : #include <boardid.h> : : #include "gpio.h"
You don't need all these include. Just […]
Ack
https://review.coreboot.org/c/coreboot/+/34505/9/src/mainboard/google/kukui/... PS9, Line 343: // [0] = &AUO_??? is not ready
remove this if it's not ready.
Ack
https://review.coreboot.org/c/coreboot/+/34505/11/src/mainboard/google/kukui... File src/mainboard/google/kukui/panel_kodama.c:
https://review.coreboot.org/c/coreboot/+/34505/11/src/mainboard/google/kukui... PS11, Line 28: BOE_TV101WUM_N53
New panel source is BOE_TV101WUM_N53
Ack
https://review.coreboot.org/c/coreboot/+/34505/11/src/mainboard/google/kukui... PS11, Line 342: // [0] = &AUO_??? is not ready
Should this be left in here?
Ack