Attention is currently required from: Hung-Te Lin, Paul Menzel, Rex-BC Chen. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62471 )
Change subject: soc/mediatek/mt8186: Add GPIO driving functions ......................................................................
Patch Set 8:
(2 comments)
File src/soc/mediatek/common/include/soc/gpio_common.h:
https://review.coreboot.org/c/coreboot/+/62471/comment/a93d250e_8bc8a440 PS4, Line 31: };
thanks.
This enum provides no benefit. I'd rather define the specific enum values in soc/gpio.h and use non-enum types (some unsigned integer type) for the function parameters. I see that the mt8186 implementation already checks if the value is in range anyway.
File src/soc/mediatek/mt8186/gpio.c:
https://review.coreboot.org/c/coreboot/+/62471/comment/6cf5f514_89af8b98 PS8, Line 203: { -1, -1, -1, } I imagine this indicates the bitfield isn't implemented for this GPIO. Instead of using negative values, I'd reuse the `width` field: a bitfield with a width of 0 isn't implemented. This approach allows omitting entries for unimplemented bitfields, as unspecified array/struct fields in an initializer default to 0.
-
Angel Pons (Code Review)