Attention is currently required from: Shelley Chen, Ravi kumar, Taniya Das, mturney mturney. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55078 )
Change subject: sc7280: gpio: eGPIO support on sc7280 ......................................................................
Patch Set 8:
(6 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/55078/comment/5d7155b5_69cfaca4 PS8, Line 7: sc7280: gpio: eGPIO support on sc7280 Please use statement (by using a verb in imperative mood) [1], and avoid redundancy.
qualcomm/sc7280: gpio: Support eGPIO scheme
[1]: https://chris.beams.io/posts/git-commit/
https://review.coreboot.org/c/coreboot/+/55078/comment/183b691d_92af77bc PS8, Line 15: This change provides an API to configure the eGPIO's. Please describe the API.
https://review.coreboot.org/c/coreboot/+/55078/comment/0490a439_86cbcbcd PS8, Line 18: TEST=Validated on qualcomm sc7280 development board How?
File src/soc/qualcomm/sc7280/include/soc/gpio.h:
https://review.coreboot.org/c/coreboot/+/55078/comment/b2fee2c8_cea30f01 PS8, Line 198: EGPIO_CFG_BMSK = 0x1, Why hex, when below decimal is used?
File src/soc/qualcomm/sc7280/sc7280_egpio.c:
https://review.coreboot.org/c/coreboot/+/55078/comment/42888e4e_cb1d9b0c PS8, Line 7: int egpio_configure_pins(int start, int end) I’d use size_t or unsigned int.
https://review.coreboot.org/c/coreboot/+/55078/comment/64df7663_b80596d6 PS8, Line 19: EGPIO_CFG_BMSK)) { Fits on the line above.
-
Paul Menzel (Code Review)