[coreboot-gerrit] [M] Change in coreboot[main]: soc/mediatek/common/dp: Add read/write APIs for DP Phy register

[Yu-Ping Wu (Code Review)]

Attention is currently required from: Hung-Te Lin, Yidi Lin.

Yu-Ping Wu has posted comments on this change by Yidi Lin. ( https://review.coreboot.org/c/coreboot/+/85914?usp=email )

Change subject: soc/mediatek/common/dp: Add read/write APIs for DP Phy register
......................................................................

Patch Set 1:

(3 comments)

File src/soc/mediatek/common/dp/dptx_hal_common.c:

https://review.coreboot.org/c/coreboot/+/85914/comment/bcf9b505_1d3afbd6?usp... :
PS1, Line 73: 	if (!mtk_dp->phy_regs) {
Same here. Use an assertion.

https://review.coreboot.org/c/coreboot/+/85914/comment/6a0a44c0_c9fafc8e?usp... :
PS1, Line 78: if (offset % 4 != 0 || offset > REG_OFFSET_LIMIT) {
            : 		printk(BIOS_ERR, "[%s] invalid offset %#x for reg %p\n",
            : 		       __func__, offset, mtk_dp->regs);
            : 		return 0;
            : 	}
Although existing code is also written this way, I think this should be assertions (instead of run-time errors). Since this is the newly added code for mt8196, can we start using assertions? We may fix `mtk_dp_mask` and other functions in a separate patch.

https://review.coreboot.org/c/coreboot/+/85914/comment/7354f221_6d20eb2b?usp... :
PS1, Line 120: /*
             : 	 * TODO: modify to clrsetbits32(addr, mask, val);
             : 	 * There is asserion error when testing assert((val & mask) == val).
             : 	 */
Since this is newly added code, could you find where the bug is? We may add `assert(val & mask == val)` here.