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

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

[Alban Bedel]

To set gpio during the early init we now need to use
tegra_spl_gpio_direction_output(), copied from seaboard.

Signed-off-by: Alban Bedel <alban.bedel@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);
 }
 #endif
 
-- 
2.3.0

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

[Stephen Warren]

On 02/24/2015 09:58 AM, Alban Bedel wrote:
> To set gpio during the early init we now need to use
> tegra_spl_gpio_direction_output(), copied from seaboard.

Can you explain "now" a bit more. Did some previous commit change the 
behaviour of the GPIO API? If so, if you could reference the commit hash 
and subject that'd be useful.

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

[Alban Bedel]

On Tue, 24 Feb 2015 10:02:04 -0700
Stephen Warren <swarren@wwwdotorg.org> wrote:

> On 02/24/2015 09:58 AM, Alban Bedel wrote:
> > To set gpio during the early init we now need to use
> > tegra_spl_gpio_direction_output(), copied from seaboard.
> 
> Can you explain "now" a bit more. Did some previous commit change the 
> behaviour of the GPIO API? If so, if you could reference the commit hash 
> and subject that'd be useful.

This a repost, see [1] for the details. In short this is needed because
the GPIO driver isn't available at this point. In October I have been
told this will be fixed soon. Now the GPIO driver is still not fixed
and all tamonten boards are still broken.

Without this fix all board peripherals are unusable on all Tamonten
board, it would be really nice to have this integrated.

Alban

[1] http://lists.denx.de/pipermail/u-boot/2014-October/190464.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150224/531257b9/attachment.sig>

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

[Stephen Warren]

On 02/24/2015 10:18 AM, Alban Bedel wrote:
> On Tue, 24 Feb 2015 10:02:04 -0700
> Stephen Warren <swarren@wwwdotorg.org> wrote:
>
>> On 02/24/2015 09:58 AM, Alban Bedel wrote:
>>> To set gpio during the early init we now need to use
>>> tegra_spl_gpio_direction_output(), copied from seaboard.
>>
>> Can you explain "now" a bit more. Did some previous commit change the
>> behaviour of the GPIO API? If so, if you could reference the commit hash
>> and subject that'd be useful.
>
> This a repost, see [1] for the details. In short this is needed because
> the GPIO driver isn't available at this point. In October I have been
> told this will be fixed soon. Now the GPIO driver is still not fixed
> and all tamonten boards are still broken.
>
> Without this fix all board peripherals are unusable on all Tamonten
> board, it would be really nice to have this integrated.
>
> Alban
>
> [1] http://lists.denx.de/pipermail/u-boot/2014-October/190464.html

OK. My point was that the commit description should describe the source 
of the problem so that anyone looking through git history understands 
the issues without having to search email lists too.

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

[Alban Bedel]

On Tue, 24 Feb 2015 10:22:52 -0700
Stephen Warren <swarren@wwwdotorg.org> wrote:

> On 02/24/2015 10:18 AM, Alban Bedel wrote:
> > On Tue, 24 Feb 2015 10:02:04 -0700
> > Stephen Warren <swarren@wwwdotorg.org> wrote:
> >
> >> On 02/24/2015 09:58 AM, Alban Bedel wrote:
> >>> To set gpio during the early init we now need to use
> >>> tegra_spl_gpio_direction_output(), copied from seaboard.
> >>
> >> Can you explain "now" a bit more. Did some previous commit change the
> >> behaviour of the GPIO API? If so, if you could reference the commit hash
> >> and subject that'd be useful.
> >
> > This a repost, see [1] for the details. In short this is needed because
> > the GPIO driver isn't available at this point. In October I have been
> > told this will be fixed soon. Now the GPIO driver is still not fixed
> > and all tamonten boards are still broken.
> >
> > Without this fix all board peripherals are unusable on all Tamonten
> > board, it would be really nice to have this integrated.
> >
> > Alban
> >
> > [1] http://lists.denx.de/pipermail/u-boot/2014-October/190464.html
> 
> OK. My point was that the commit description should describe the source 
> of the problem so that anyone looking through git history understands 
> the issues without having to search email lists too.

Right, I'll post a V2 tomorrow with a better log.

Alban
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150224/d0b7af21/attachment.sig>

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

[Alban Bedel]

To set gpio during the early init we now need to use
tegra_spl_gpio_direction_output(), copied from seaboard.

Change-Id: Id0aadb17a71b78e75e8c3f8de374102b3eab767b
Signed-off-by: Alban Bedel <alban.bedel@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);
 }
 #endif
 
-- 
2.1.1

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

[Stephen Warren]

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.

> Signed-off-by: Alban Bedel <alban.bedel@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

... although perhaps the SPL and non-SPL code should simply be separated 
into separate files, so that there's no need for ifdefs, and it's 
obvious if SPL and non-SPL code are duplicating the same work?

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

[Alban Bedel]

On Thu, 02 Oct 2014 09:42:18 -0600
Stephen Warren <swarren@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.

> > Signed-off-by: Alban Bedel <alban.bedel@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.

> ... although perhaps the SPL and non-SPL code should simply be separated 
> into separate files, so that there's no need for ifdefs, and it's 
> obvious if SPL and non-SPL code are duplicating the same work?

Actually none of the code from tamonten.c is needed for the SPL, a
build with both function under #ifndef CONFIG_SPL_BUILD works just fine.
Any pointer on how I can get tamonten.c out of the SPL build?

Alban
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141002/5e94bc1a/attachment.pgp>

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

[Stephen Warren]

On 10/02/2014 10:12 AM, Alban Bedel wrote:
> On Thu, 02 Oct 2014 09:42:18 -0600
> Stephen Warren <swarren@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.
>
>>> Signed-off-by: Alban Bedel <alban.bedel@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.

Hmm. CC Simon to comment on which GPIO drivers are available in 
SPL/non-SPL, and why the above ifdef doesn't work.

>> ... although perhaps the SPL and non-SPL code should simply be separated
>> into separate files, so that there's no need for ifdefs, and it's
>> obvious if SPL and non-SPL code are duplicating the same work?
>
> Actually none of the code from tamonten.c is needed for the SPL, a
> build with both function under #ifndef CONFIG_SPL_BUILD works just fine.

Indeed, if manipulating the GPIO in SPL isn't even necessary, then just 
wrapping the whole code in #ifndef CONFIG_SPL_BUILD makes sense to me.

> Any pointer on how I can get tamonten.c out of the SPL build?

I'm not sure now that the Makefile structure has changed to Kbuild. It 
might be as simple as:

obj-$(CONFIG_SPL_BUILD) += foo.o

(but using whatever the opposite of CONFIG_SPL_BUILD is.

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

[Simon Glass]

Hi Stephen,

On 2 October 2014 12:39, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 10/02/2014 10:12 AM, Alban Bedel wrote:
>>
>> On Thu, 02 Oct 2014 09:42:18 -0600
>> Stephen Warren <swarren@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.
>>
>>>> Signed-off-by: Alban Bedel <alban.bedel@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.
>
>
> Hmm. CC Simon to comment on which GPIO drivers are available in SPL/non-SPL,
> and why the above ifdef doesn't work.

See my previous reply.

>
>>> ... although perhaps the SPL and non-SPL code should simply be separated
>>> into separate files, so that there's no need for ifdefs, and it's
>>> obvious if SPL and non-SPL code are duplicating the same work?
>>
>>
>> Actually none of the code from tamonten.c is needed for the SPL, a
>> build with both function under #ifndef CONFIG_SPL_BUILD works just fine.

I think this will drop the message. But I should have mentioned in my
other reply that we don't really need to call gpio_early_init_uart()
from board_early_init_f() if we are already calling it from
spl_board_init(). Separate issue to this patch of course.

>
>
> Indeed, if manipulating the GPIO in SPL isn't even necessary, then just
> wrapping the whole code in #ifndef CONFIG_SPL_BUILD makes sense to me.
>
>> Any pointer on how I can get tamonten.c out of the SPL build?
>
>
> I'm not sure now that the Makefile structure has changed to Kbuild. It might
> be as simple as:
>
> obj-$(CONFIG_SPL_BUILD) += foo.o
>
> (but using whatever the opposite of CONFIG_SPL_BUILD is.

Regards,
Simon

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

[Simon Glass]

Hi Alban,

On 2 October 2014 10:12, Alban Bedel <alban.bedel@avionic-design.de> wrote:
>
> On Thu, 02 Oct 2014 09:42:18 -0600
> Stephen Warren <swarren@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@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.

There is a bit of an update on current status here:

http://www.denx.de/wiki/U-Boot/DriverModel

My approach with driver model is incremental, since that's the only
way to make any progress.

>
> > ... although perhaps the SPL and non-SPL code should simply be separated
> > into separate files, so that there's no need for ifdefs, and it's
> > obvious if SPL and non-SPL code are duplicating the same work?
>
> Actually none of the code from tamonten.c is needed for the SPL, a
> build with both function under #ifndef CONFIG_SPL_BUILD works just fine.
> Any pointer on how I can get tamonten.c out of the SPL build?

This should work:

ifndef CONFIG_SPL_BUILD
obj-y += tamonten.o
endif

But it seems odd to me. At least on seaboard I didn't get the SPL
message unless this code was executed.

Regards,
Simon

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

[Stephen Warren]

On 10/02/2014 01:05 PM, Simon Glass wrote:
> Hi Alban,
>
> On 2 October 2014 10:12, Alban Bedel <alban.bedel@avionic-design.de> wrote:
>>
>> On Thu, 02 Oct 2014 09:42:18 -0600
>> Stephen Warren <swarren@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@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?)

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

[Simon Glass]

Hi Stephen,

On 2 October 2014 13:08, Stephen Warren <swarren@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@avionic-design.de>
>> wrote:
>>>
>>>
>>> On Thu, 02 Oct 2014 09:42:18 -0600
>>> Stephen Warren <swarren@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@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