Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31080 )
Change subject: vendorcode/google/chromeos: Add option for using ACPI GPIO pin ......................................................................
Patch Set 1:
(2 comments)
Patch Set 1:
I do understand the requirement to report the mapped pin number. What I don't understand is, when would we ever need to have the raw number? i.e. why can't we make the mapped numbers the default?
That was my original plan, use this if CONFIG_GENERIC_GPIO_LIB was defined but that ended up breaking a bunch of random boards that don't have a full GPIO implementation.
https://review.coreboot.org/#/c/31080/1/src/vendorcode/google/chromeos/acpi.... File src/vendorcode/google/chromeos/acpi.c:
https://review.coreboot.org/#/c/31080/1/src/vendorcode/google/chromeos/acpi.... PS1, Line 18: #if IS_ENABLED(CONFIG_CHROMEOS_ACPI_GPIO_GENERATE_PIN)
Would it hurt to always include the header file?
gpio.h is poor in this regard, if the SOC doesn't have an implementation it will fail to compile.
https://review.coreboot.org/#/c/31080/1/src/vendorcode/google/chromeos/acpi.... PS1, Line 34: #if IS_ENABLED(CONFIG_CHROMEOS_ACPI_GPIO_GENERATE_PIN)
Any reason to use pre-processor code instead of C?
Same reason as above, if the header is now guarded the code has to be as well.