HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41255 )
Change subject: src/include/gpio.h: Drop 'include <soc/gpio.h>' ......................................................................
src/include/gpio.h: Drop 'include <soc/gpio.h>'
include <soc/gpio.h> in 'src/include/gpio.h' does not make sense.
Change-Id: I38deed0981032ff3aacc71e2b1afb8fac7dbf62a Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/include/gpio.h 1 file changed, 0 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/41255/1
diff --git a/src/include/gpio.h b/src/include/gpio.h index 57360f9..f4e1161 100644 --- a/src/include/gpio.h +++ b/src/include/gpio.h @@ -3,7 +3,6 @@ #ifndef __SRC_INCLUDE_GPIO_H__ #define __SRC_INCLUDE_GPIO_H__
-#include <soc/gpio.h> #include <types.h>
/* <soc/gpio.h> must typedef a gpio_t that fits in 32 bits. */
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41255 )
Change subject: src/include/gpio.h: Drop 'include <soc/gpio.h>' ......................................................................
Patch Set 1: Code-Review-2
(1 comment)
Uhh...
https://review.coreboot.org/c/coreboot/+/41255/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41255/1//COMMIT_MSG@9 PS1, Line 9: include <soc/gpio.h> in 'src/include/gpio.h' does not make sense. Uhh... yeah it does, this is intentional. There is a comment literally two lines below the #include you're removing to explain why (and if that's not enough, the commit message where this was added would have gone into more detail).
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41255 )
Change subject: src/include/gpio.h: Drop 'include <soc/gpio.h>' ......................................................................
Patch Set 1: Code-Review-1
(1 comment)
https://review.coreboot.org/c/coreboot/+/41255/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41255/1//COMMIT_MSG@9 PS1, Line 9: include <soc/gpio.h> in 'src/include/gpio.h' does not make sense.
Uhh... yeah it does, this is intentional. […]
looks like this file is only used by soc. what I don't understand is why should we include 'soc' in common part of coreboot tree.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41255 )
Change subject: src/include/gpio.h: Drop 'include <soc/gpio.h>' ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41255/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41255/1//COMMIT_MSG@9 PS1, Line 9: include <soc/gpio.h> in 'src/include/gpio.h' does not make sense.
looks like this file is only used by soc. […]
This is a common API that relies on SoC-specific interna in the backend. gpio_input(), gpio_output(), etc. are common coreboot functions that can be used by all platforms which implement the GPIO API, which allows us to build common code on top of it. But the hardware details of how to actually set or read a GPIO are of course specific to each SoC and have to be implemented by SoC code. It is in that sense no different than e.g. <device/i2c.h> which declares platform_i2c_transfer() that is then implemented by different files in the respective SoC directories.
The difference here is that we don't just need SoC-specific code but also SoC-specific definitions, for gpio_t. The API just specifies that each gpio_t must be 4 bytes long, but not exactly how they look. For many SoCs GPIOs are identified by more than one number (e.g. bank number and pin number), so it is advantageous to define gpio_t as a struct of bit fields (e.g. see rockchip/common/include/soc/gpio.h). <soc/gpio.h> is a well-known SoC header name that any SoC which wants to be compatible with this API must provide and which must contain a typedef for gpio_t, as is explained in the comment in <gpio.h>.
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41255 )
Change subject: src/include/gpio.h: Drop 'include <soc/gpio.h>' ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41255/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41255/1//COMMIT_MSG@9 PS1, Line 9: include <soc/gpio.h> in 'src/include/gpio.h' does not make sense.
This is a common API that relies on SoC-specific interna in the backend. […]
Thx
HAOUAS Elyes has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/41255 )
Change subject: src/include/gpio.h: Drop 'include <soc/gpio.h>' ......................................................................
Abandoned
-
HAOUAS Elyes (Code Review) -
Julius Werner (Code Review)