+Stephen for GPIO discussion
Hi Masahiro,
On 27 November 2014 at 07:52, Masahiro YAMADA yamada.m@jp.panasonic.com wrote:
Hi Simon,
2014-11-27 5:18 GMT+09:00 Simon Glass sjg@chromium.org:
Hi Masahiro,
On 26 November 2014 at 02:33, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
If CONFIG_OF_CONTROL is enabled, lib/fdtdec.c is compiled. It includes <asm/gpio.h> and then <asm/gpio.h> includes <asm/arch/gpio.h>. Consequently, all the SoCs that enable CONFIG_OF_CONTROL must have <asm/arch/gpio.h> even if they do not support GPIO.
In the first place, GPIO has nothing to do with OF_CONTROL. It is wrong that lib/fdtdec.c includes GPIO functions; it should be split into two files, FDT-common things and GPIO things. It is, however, a pretty big work to fix that correctly.
This is a compromised commit to add a dummy <asm/arch/gpio.h> to support OF_CONTROL for UniPhier platform. This dummy header will be removed after FDT-GPIO stuff is fixed correctly.
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com
I am working on the task to split lib/fdtdec.c and move GPIO functions to drivers/gpio/.
That code is only temporary and we can probably remove it soon. It should move to the GPIO uclass.
Do you mean, is it better to not touch lib/fdtdec.c ?
The GPIO functions in there are not great. They are not implemented in a general way and don't respect the GPIO binding properly - e.g. a certain value of #gpio-cells is assumed. We need to to be able to decode a GPIO in a general way, something like this function in Linux:
/** * of_get_named_gpio() - Get a GPIO number to use with GPIO API * @np: device node to get GPIO from * @propname: Name of property containing gpio specifier(s) * @index: index of the GPIO * * Returns GPIO number to use with Linux generic GPIO API, or one of the errno * value on the error condition. */ static inline int of_get_named_gpio(struct device_node *np, const char *propname, int index)
(In our case it would be passed the device tree blob and an offset instead of np since we don't have unflattened' device tree support yet)
This function would replace fdtdec_decode_gpio(). If you have time to work on that, it would be great to resolve this and remove all that code from fdtdec. I get Tegra and Exynos working. Now that we have a GPIO uclass there is nothing standing in the way.
I agree that lib/libfdt/ is not enough for our use, but I am afraid our fdt support code is getting ugly.
Other than GPIO stuff, my concern is we have our libfdt extensions in some places: common/fdt_support.c and lib/fdtdec.c
The functions in the former is prefixed with fdt_ but the ones in the latter is prefixed with fdtdec_.
I do not see consistency.
fdt_support - used with CONFIG_OF_LIBFDT, provides support functions to allow booting an OS with device tree. Mostly these functions modify the device tree.
fdtdec - used with CONFIG_OF_CONTROL, provides support for device tree control of U-Boot. May be greatly simplified if we have unflattened device tree support (although we will still want basic functions for memory-constrained use so I'm not sure). Mostly these are decode functions (read-only).
Regards, Simon