[U-Boot] [PATCH 2/8] ARM: UniPhier: add dummy gpio.h to enable CONFIG_OF_CONTROL

[Simon Glass]

+Stephen for GPIO discussion

Hi Masahiro,

On 27 November 2014 at 07:52, Masahiro YAMADA <yamada.m at jp.panasonic.com> wrote:
> Hi Simon,
>
>
> 2014-11-27 5:18 GMT+09:00 Simon Glass <sjg at chromium.org>:
>> Hi Masahiro,
>>
>> On 26 November 2014 at 02:33, Masahiro Yamada <yamada.m at 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 at 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