Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32789 )
Change subject: soc/intel/icelake: Make use of gpio_pm_configure() ......................................................................
Patch Set 6:
(5 comments)
https://review.coreboot.org/#/c/32789/6/src/soc/intel/icelake/chip.h File src/soc/intel/icelake/chip.h:
https://review.coreboot.org/#/c/32789/6/src/soc/intel/icelake/chip.h@278 PS6, Line 278: TOTAL_GPIO_COMM
Can we add an enum or macro in some header file for each community? Then it can be used in gpio. […]
Done
https://review.coreboot.org/#/c/32789/6/src/soc/intel/icelake/chip.c File src/soc/intel/icelake/chip.c:
https://review.coreboot.org/#/c/32789/6/src/soc/intel/icelake/chip.c@113 PS6, Line 113: PCH_DEV_SLOT_LPC
Why LPC?
need any device to access device tree variable ? I will use first device
https://review.coreboot.org/#/c/32789/6/src/soc/intel/icelake/chip.c@120 PS6, Line 120: config->gpio_pm[i].config
Can we treat 0 as special value or maybe have another flag that indicates mainboard has set these va […]
add a print ?
https://review.coreboot.org/#/c/32789/6/src/soc/intel/icelake/chip.c@121 PS6, Line 121: mask[i] = ~value[i];
I don't think this is correct. […]
Done
https://review.coreboot.org/#/c/32789/6/src/soc/intel/icelake/chip.c@142 PS6, Line 142: TODO(subrata): Need to implement 4us IRQ width recommendation.
Where do you plan to implement this?
Done