Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39394 )
Change subject: WIP: inteltool: Add option to output GPIOs as coreboot macros ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/c/coreboot/+/39394/1/util/inteltool/gpio_groups.... File util/inteltool/gpio_groups.c:
https://review.coreboot.org/c/coreboot/+/39394/1/util/inteltool/gpio_groups.... PS1, Line 68: static const char *decode_table_gpio_reset(const size_t reset) I get mismatching output, comparing Maxim's go approach an this patch. Will have to check what is correct
https://review.coreboot.org/c/coreboot/+/39394/1/util/inteltool/gpio_groups.... PS1, Line 239: printf("/* %-8s %-20s */\t", I would put the comments to EOL, but that may be personal preference.
Example:
/* GPP_F8 I2C4_SDA */ PAD_CFG_NF(GPP_F8, UP_20K, PWROK, NF1), vs. PAD_CFG_NF(GPP_F8, UP_20K, PWROK, NF1), /* I2C4_SDA */
https://review.coreboot.org/c/coreboot/+/39394/1/util/inteltool/inteltool.c File util/inteltool/inteltool.c:
https://review.coreboot.org/c/coreboot/+/39394/1/util/inteltool/inteltool.c@... PS1, Line 513: " -C | --macros: dump GPIOs in coreboot macro format\n" instead of introducing a whole new parameter, what about `-Gm` for macro output?
https://review.coreboot.org/c/coreboot/+/39394/1/util/inteltool/inteltool.c@... PS1, Line 808: printf("\n\n"); should this move one line down?