Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48954 )
Change subject: soc/mediatek/mt8192: pmic: unlock key protection before initial setting ......................................................................
Patch Set 6:
(5 comments)
https://review.coreboot.org/c/coreboot/+/48954/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48954/6//COMMIT_MSG@9 PS6, Line 9: initial settings could be effective Which ones?
https://review.coreboot.org/c/coreboot/+/48954/6//COMMIT_MSG@9 PS6, Line 9: unlock key protection 1. Does that mean *disable key protection*? 2. What keys are protected?
https://review.coreboot.org/c/coreboot/+/48954/6//COMMIT_MSG@11 PS6, Line 11: Please mention the datasheet name and revision.
https://review.coreboot.org/c/coreboot/+/48954/6/src/soc/mediatek/mt8192/mt6... File src/soc/mediatek/mt8192/mt6359p.c:
https://review.coreboot.org/c/coreboot/+/48954/6/src/soc/mediatek/mt8192/mt6... PS6, Line 321: set I would stay with *setting*.
https://review.coreboot.org/c/coreboot/+/48954/6/src/soc/mediatek/mt8192/mt6... PS6, Line 329: } I’d put the for loop outside, and in the body use the ternary operator:
for (int i = 0; i < ARRAY_SIZE(key_protect_setting); i++) mt6359p_write(key_protect_setting[i].addr, lock ? 0 : key_protect_setting[i].val);