Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31504 )
Change subject: inteltool: Move GPIOs to c files per platform ......................................................................
Patch Set 39:
(6 comments)
https://review.coreboot.org/#/c/31504/39/util/inteltool/gpio.h File util/inteltool/gpio.h:
https://review.coreboot.org/#/c/31504/39/util/inteltool/gpio.h@24 PS39, Line 24: const char *const *pad_names; /* indexed by 'pad * func_count + func' */ Please don't mix comment styles. I guess C style (this here) is preferred, though.
https://review.coreboot.org/#/c/31504/39/util/inteltool/gpio.h@40 PS39, Line 40: /* Description of GPIO 'bank' ex. {ncore, score. ssus} */ Mention that this is used for BayTrail? Also, maybe put it between `new` and `old`?
https://review.coreboot.org/#/c/31504/39/util/inteltool/gpio.c File util/inteltool/gpio.c:
https://review.coreboot.org/#/c/31504/39/util/inteltool/gpio.c@5 PS39, Line 5: * Copyright (C) 2019 by 9elements Agency GmbH For the added #includes?
https://review.coreboot.org/#/c/31504/39/util/inteltool/gpio_baytrail.h File util/inteltool/gpio_baytrail.h:
PS39: Why is this a .h?
https://review.coreboot.org/#/c/31504/39/util/inteltool/platform_i631x.c File util/inteltool/platform_i631x.c:
PS39: Maybe keep it alongside `gpio_ich`? Because of its similarity.
https://review.coreboot.org/#/c/31504/39/util/inteltool/platform_lynxpoint.c File util/inteltool/platform_lynxpoint.c:
PS39: This is for Lynx Point LP (low power) only. The file name should reflect that.