Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31634 )
Change subject: util/inteltool: Improve GPIO specification ......................................................................
Patch Set 10:
(4 comments)
https://review.coreboot.org/#/c/31634/10/util/inteltool/gpio.h File util/inteltool/gpio.h:
https://review.coreboot.org/#/c/31634/10/util/inteltool/gpio.h@34 PS10, Line 34: * Modes: : * -2 = GP-Out : * -1 = GP-In : * 0 = Undefined : * >0 = Native functions
this should be an `enum`.
Ack
https://review.coreboot.org/#/c/31634/10/util/inteltool/gpio.h@44 PS10, Line 44: uint32_t config0; : uint32_t config1;
So these would be used to cache the register content for later processing? […]
These are meant as cache for the register contents. I also thought about storing the default values (ex. for comparison), but according to the datasheets (ex. Kabylake) some registers have values like this 44000x00h. I think x is a placeholder, but I have no idea what that means.
https://review.coreboot.org/#/c/31634/10/util/inteltool/gpio_test.h File util/inteltool/gpio_test.h:
https://review.coreboot.org/#/c/31634/10/util/inteltool/gpio_test.h@24 PS10, Line 24: .name
What do we gain from repeating the field identifiers in each […]
I think the brackets make it more messy. Using the field identifiers I tried to improve the overview over the columns.
We also can use spaces/tabs.
https://review.coreboot.org/#/c/31634/10/util/inteltool/gpio_test.h@26 PS10, Line 26: FUNC(3, "ESPI_IO0") } },
Personally, I prefer more condensed tables where a row fits a single line. […]
I am fine with both.