Hello Marco Chen,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/34189
to review the following change.
Change subject: gpio: Add a function to get current output value of a GPIO. ......................................................................
gpio: Add a function to get current output value of a GPIO.
Add a new function - gpio_get_output() which can be implemented by SoC/board code to provide the current output value of a GPIO.
If not implemented then the default handler will always return 0.
BUG=b:137033609 BRANCH=octopus TEST=build octopus variants.
Change-Id: Iba67c8426f3049dc1ed0cdbb6561d9cb5ddb81d9 Signed-off-by: Marco Chen marcochen@google.com --- M src/include/gpio.h M src/lib/gpio.c 2 files changed, 7 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/34189/1
diff --git a/src/include/gpio.h b/src/include/gpio.h index 0a37ee7..a605f17 100644 --- a/src/include/gpio.h +++ b/src/include/gpio.h @@ -24,6 +24,7 @@
/* The following functions must be implemented by SoC/board code. */ int gpio_get(gpio_t gpio); +int gpio_get_output(gpio_t gpio); void gpio_set(gpio_t gpio, int value); void gpio_input_pulldown(gpio_t gpio); void gpio_input_pullup(gpio_t gpio); diff --git a/src/lib/gpio.c b/src/lib/gpio.c index 8ea3b5e..9e22b63 100644 --- a/src/lib/gpio.c +++ b/src/lib/gpio.c @@ -190,3 +190,9 @@ { return 0; } + +/* Default handler returns 0 because type of gpio_t is unknown */ +__weak int gpio_get_output(gpio_t gpio) +{ + return 0; +}
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34189 )
Change subject: gpio: Add a function to get current output value of a GPIO. ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34189/1/src/include/gpio.h File src/include/gpio.h:
https://review.coreboot.org/c/coreboot/+/34189/1/src/include/gpio.h@27 PS1, Line 27: int gpio_get_output(gpio_t gpio); This is a pretty odd use case, I think it might be better to just have this function at the platform level. I can see how it makes sense for x86 SMM, but in other architectures without runtime components it doesn't really come up that you wouldn't know what value a certain pin is set to.
If you do want to add this to the generic API, please move it away from those other definitions (because the comment above says these are required), and add a comment clarifying that this one is optional.
Stefan Reinauer has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/34189?usp=email )
Change subject: gpio: Add a function to get current output value of a GPIO. ......................................................................
Abandoned