Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35623 )
Change subject: TEST-ONLY: add support anx7625 driver ......................................................................
Patch Set 3:
(14 comments)
https://review.coreboot.org/c/coreboot/+/35623/3/src/drivers/analogix/anx762... File src/drivers/analogix/anx7625/anx7625.c:
https://review.coreboot.org/c/coreboot/+/35623/3/src/drivers/analogix/anx762... PS3, Line 36: if (saddr != saddr_backup) { if (saddr == saddr_backup) return ret;
saddr_backup = saddr;
switch(saddr) { case ... offset = ... break; ... }
https://review.coreboot.org/c/coreboot/+/35623/3/src/drivers/analogix/anx762... PS3, Line 70: read "%s: read i2c...", __func__, ...
https://review.coreboot.org/c/coreboot/+/35623/3/src/drivers/analogix/anx762... PS3, Line 70: BIOS_INFO BIOS_ERROR
https://review.coreboot.org/c/coreboot/+/35623/3/src/drivers/analogix/anx762... PS3, Line 87: BIOS_INFO BIOS_ERR, "%s" ... __func__
(same to all error below)
https://review.coreboot.org/c/coreboot/+/35623/3/src/drivers/analogix/anx762... PS3, Line 140: 150 comment why this is 150 (or make a named constant instead)
https://review.coreboot.org/c/coreboot/+/35623/3/src/drivers/analogix/anx762... PS3, Line 150: BIOS_INFO BIOS_ERROR?
https://review.coreboot.org/c/coreboot/+/35623/3/src/drivers/analogix/anx762... PS3, Line 155: BIOS_INFO BIOS_DEBUG ?
https://review.coreboot.org/c/coreboot/+/35623/3/src/drivers/analogix/anx762... PS3, Line 182: *a change these params to _a, _b
And create a 'unsigned long a, b, old_a, old_b'; (and use old_a, old_b for tmp_a, tmp_b)
And before returning, do
*_a = a >> 1; *_b = b >> 1;
https://review.coreboot.org/c/coreboot/+/35623/3/src/drivers/analogix/anx762... PS3, Line 219: BIOS_INFO BIOS_ERROR? And please indicate what may happen (panel won't work?)
https://review.coreboot.org/c/coreboot/+/35623/3/src/drivers/analogix/anx762... PS3, Line 648: pedid_blocks_buf shouldn't we also pass size of this buffer?
https://review.coreboot.org/c/coreboot/+/35623/3/src/drivers/analogix/anx762... PS3, Line 746: ANXGPIO Move this to a Kconfig, and do
if (CONFIG(ANXGPIO)) { ... }
https://review.coreboot.org/c/coreboot/+/35623/3/src/drivers/analogix/anx762... PS3, Line 940: cnt < 50; cnt++) { : mdelay(10); why 50 and why delay(10ms)?
https://review.coreboot.org/c/coreboot/+/35623/3/src/drivers/analogix/anx762... PS3, Line 948: remove the extra new lines
https://review.coreboot.org/c/coreboot/+/35623/3/src/mainboard/google/kukui/... File src/mainboard/google/kukui/Kconfig:
https://review.coreboot.org/c/coreboot/+/35623/3/src/mainboard/google/kukui/... PS3, Line 45: DRIVER_ANALOGIX_ANX7625 Please include ANX7625 for Juniper and don't change jacuzzi.