Attention is currently required from: Shelley Chen, Ravi kumar, mturney mturney, Julius Werner. Taniya Das has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58580 )
Change subject: qualcomm/sc7280: gpio: Support eGPIO scheme ......................................................................
Patch Set 4:
(4 comments)
File src/soc/qualcomm/common/gpio.c:
https://review.coreboot.org/c/coreboot/+/58580/comment/564af3c3_055f0f2b PS2, Line 21: if ((read32(®s->cfg) >> GPIO_EGPIO_SHFT) & GPIO_EGPIO_BMSK)
Seem like we can just add it to the condition above. Something like: […]
I might be missing something here, but we want to set the 12th bit only in case the 11th bit (eGPIO_PRESENT) is set.
File src/soc/qualcomm/common/include/soc/gpio_common.h:
https://review.coreboot.org/c/coreboot/+/58580/comment/8eaa4135_c28dff90 PS2, Line 24: GPIO_EGPIO_BMSK
GPIO_CFG_EGPIO_BMSK?
Done
https://review.coreboot.org/c/coreboot/+/58580/comment/e50f16ee_c1cd183d PS2, Line 39: GPIO_EGPIO_SHFT
GPIO_CFG_EGPIO_SHFT?
Done
https://review.coreboot.org/c/coreboot/+/58580/comment/d5b93369_caa4d319 PS2, Line 87: enum egpio_cfg {
Is the plan to add more fields in here in the future? Just curious why we have an enum for 1 type.
Yes, we might need to add more fields in future, thus added the enum.
-
Taniya Das (Code Review)