Attention is currently required from: Hung-Te Lin, Paul Menzel, Yu-Ping Wu. Rex-BC Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62471 )
Change subject: soc/mediatek/mt8186: Add GPIO driving functions ......................................................................
Patch Set 6:
(5 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/62471/comment/6681aa4d_b15904bd PS4, Line 11: Value
The value
Done
https://review.coreboot.org/c/coreboot/+/62471/comment/306694dd_1a05075b PS4, Line 11: from different SoCs
for each SoC
Done
File src/soc/mediatek/common/include/soc/gpio_common.h:
https://review.coreboot.org/c/coreboot/+/62471/comment/02d22c47_a9b6950c PS4, Line 31: };
If it’s SoC specific, why put it in common code?
There are 8 stage for all mediatek SoCs, but they may have different drive strength. IMO, it's better there is a enum in common folder which we don't need to include soc/gpio.h. If we include soc/gpio.h, we need to modify previous SoCs(8173/8183/8192/8195) which are not implement drive strength function.
I just put some comment for this in https://review.coreboot.org/c/coreboot/+/62471/6/src/soc/mediatek/mt8186/gpi...
https://review.coreboot.org/c/coreboot/+/62471/comment/9bec4363_28bfe87e PS4, Line 46: };
Why not use the current values as names?
Done
https://review.coreboot.org/c/coreboot/+/62471/comment/22e5a154_e34d17ff PS4, Line 51: int8_t width;
Why not make them unsigned?
we will use -1 if they are not configurable. https://review.coreboot.org/c/coreboot/+/62471/6/src/soc/mediatek/mt8186/gpi...