Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36642 )
Change subject: drivers/i2c/ptn3460: Provide chip driver for PTN3460 ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36642/1/src/drivers/i2c/ptn3460/ptn... File src/drivers/i2c/ptn3460/ptn3460.c:
https://review.coreboot.org/c/coreboot/+/36642/1/src/drivers/i2c/ptn3460/ptn... PS1, Line 69: }
How long does this loop need?
We are writing 128 bytes byte-per-byte here. So per EDID byte there will be a full I2C transfer which consists of 1 slave address byte, 1 register address byte and one value for this register (in total 3). So in total there will be 128 * 3 bytes transferred over I2C, which should take ~4ms@100 kHz I2C speed. You can add some overhead here due to function calls and the like, but that should be small.
https://review.coreboot.org/c/coreboot/+/36642/1/src/drivers/i2c/ptn3460/ptn... PS1, Line 110: *ptr++ = (uint8_t)status; /* fill config structure via ptr */
Is pointer arithmetic really needed?
We have this structure (struct ptn_3460_config cfg) which needs to be read byte-per-byte over I2C. Any better approaches than using pointers in mind?