[U-Boot] [PATCH] tegra20: tamonten: Fix the early gpio init

[Simon Glass]

Hi Stephen,

On 2 October 2014 13:08, Stephen Warren <swarren at wwwdotorg.org> wrote:
> On 10/02/2014 01:05 PM, Simon Glass wrote:
>>
>> Hi Alban,
>>
>> On 2 October 2014 10:12, Alban Bedel <alban.bedel at avionic-design.de>
>> wrote:
>>>
>>>
>>> On Thu, 02 Oct 2014 09:42:18 -0600
>>> Stephen Warren <swarren at wwwdotorg.org> wrote:
>>>
>>>> On 10/02/2014 09:14 AM, Alban Bedel wrote:
>>>>>
>>>>> To set gpio during the early init we now need to use
>>>>> tegra_spl_gpio_direction_output(), copied from seaboard.
>>>>>
>>>>> Change-Id: Id0aadb17a71b78e75e8c3f8de374102b3eab767b
>>>>
>>>>
>>>> That shouldn't be present on upstream patches.
>>>
>>>
>>> Sorry about this.
>>
>>
>> If you use patman it will take these out for you automatically without
>> you having to think about it.
>>
>>>
>>>
>>>>> Signed-off-by: Alban Bedel <alban.bedel at avionic-design.de>
>>>>> ---
>>>>>    board/avionic-design/common/tamonten.c | 4 +++-
>>>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/board/avionic-design/common/tamonten.c
>>>>> b/board/avionic-design/common/tamonten.c
>>>>> index 9c86779..ea2425a 100644
>>>>> --- a/board/avionic-design/common/tamonten.c
>>>>> +++ b/board/avionic-design/common/tamonten.c
>>>>> @@ -23,8 +23,10 @@
>>>>>    #ifdef CONFIG_BOARD_EARLY_INIT_F
>>>>>    void gpio_early_init(void)
>>>>>    {
>>>>> +#ifndef CONFIG_SPL_BUILD
>>>>>      gpio_request(GPIO_PI4, NULL);
>>>>> -   gpio_direction_output(GPIO_PI4, 1);
>>>>> +#endif
>>>>> +   tegra_spl_gpio_direction_output(GPIO_PI4, 1);
>>>>>    }
>>>>
>>>>
>>>> Surely you only want to call tegra_spl_*() from SPL, and not from
>>>> non-SPL code? In other words, don't you need something more like:
>>>>
>>>> #ifdef CONFIG_SPL_BUILD
>>>>        tegra_spl_gpio_direction_output(GPIO_PI4, 1);
>>>> #else
>>>>        gpio_request(GPIO_PI4, NULL);
>>>>        gpio_direction_output(GPIO_PI4, 1);
>>>> #endif
>>>
>>>
>>> Sadly not, at this point the gpio driver isn't available yet, that's
>>> why one need to use tegra_spl_gpio_direction_output(). As mentioned in
>>> the commit log I copied this from seaboard, assuming it would be
>>> correct.
>>>
>>> AFAICT the gpio_request() could be removed too, it doesn't work at this
>>> point anyway.
>>
>>
>> This is a temporary measure until the SPL series is applied to dm/next
>> at least (I expect sometime this month). While driver model is
>> available before relocation, the Tegra GPIO driver is not marked in
>> the device tree with "u-boot,dm-pre-reloc", so is not available this
>> early. Device tree additions give Stephen a headache, and I decided it
>> was better for it not to work for now, and just use the SPL function.
>>
>> There are three stages to consider:
>>
>> 1. SPL
>> 2. U-Boot, before relocation
>> 3. U-Boot, after relocation
>>
>> At present, driver model does not support 1 (see dm/spl-working for
>> patches if you want to see this). It supports 2 but only for drivers
>> marked pre-relocation (except serial, for which a hack forces the
>> console to be bound and used in the device tree case without the
>> u-boot,dm-pre-reloc marker). It supports 3 fully.
>
>
> That's roughly what I expected. Given that, I fail to see why the following
> wouldn't work then:
>
>>>> #ifdef CONFIG_SPL_BUILD
>>>>        tegra_spl_gpio_direction_output(GPIO_PI4, 1);
>>>> #else
>>>>        gpio_request(GPIO_PI4, NULL);
>>>>        gpio_direction_output(GPIO_PI4, 1);
>>>> #endif
>
> (or have just 1 of those two ifdef branches; presumably the GPIO only needs
> to be initialized in one place not both?)

Yes that's true, although be aware that the gpio_request() and
gpio_direction_output() will in fact always fail at present, since
they are called before the GPIO driver is inited pre-reloc. But since
it has happened in SPL it doesn't matter.

I haven't tested this, and it's icky to rely on failure and not
checking the return code, and I slightly prefer Alban's solution at
least for now. This is only going to be a problem for another month or
so.

Regards,
Simon