Attention is currently required from: Shelley Chen, Ravi kumar, Paul Menzel, mturney mturney. Taniya Das has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55078 )
Change subject: sc7280: gpio: eGPIO support on sc7280 ......................................................................
Patch Set 10:
(6 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/55078/comment/24446ab1_75af6702 PS8, Line 7: sc7280: gpio: eGPIO support on sc7280
Please use statement (by using a verb in imperative mood) [1], and avoid redundancy. […]
Done
https://review.coreboot.org/c/coreboot/+/55078/comment/5b2c70cf_8ea41bca PS8, Line 15: This change provides an API to configure the eGPIO's.
Please describe the API.
Done
https://review.coreboot.org/c/coreboot/+/55078/comment/ac7c8d8f_42319b61 PS8, Line 18: TEST=Validated on qualcomm sc7280 development board
How?
Done
File src/soc/qualcomm/sc7280/include/soc/gpio.h:
https://review.coreboot.org/c/coreboot/+/55078/comment/3dc2a2bf_04970214 PS8, Line 198: EGPIO_CFG_BMSK = 0x1,
Why hex, when below decimal is used?
Done
File src/soc/qualcomm/sc7280/sc7280_egpio.c:
https://review.coreboot.org/c/coreboot/+/55078/comment/6ec1cc38_c781ffe5 PS8, Line 7: int egpio_configure_pins(int start, int end)
I’d use size_t or unsigned int.
Done
https://review.coreboot.org/c/coreboot/+/55078/comment/2c77b5a7_c5a4a38b PS8, Line 19: EGPIO_CFG_BMSK)) {
Fits on the line above.
Done
-
Taniya Das (Code Review)