[U-Boot] [PATCH] fdt: Include arch specific gpio.h

[Simon Glass]

Hi Joe,

On Wed, Jul 11, 2012 at 11:02 PM, Joe Hershberger
<joe.hershberger at gmail.com> wrote:
>
> Hi Michal,
>
> On Wed, Jul 11, 2012 at 7:21 AM, Michal Simek <monstr at monstr.eu> wrote:
> > On 07/11/2012 02:00 PM, Simon Glass wrote:
> >>
> >> Hi Michal,
> >>
> >>
> >> On Wed, Jul 11, 2012 at 1:21 PM, Michal Simek <monstr at monstr.eu
> >> <mailto:monstr at monstr.eu>> wrote:
> >>
> >>     On 07/11/2012 12:59 PM, Simon Glass wrote:
>
> Please use text only email.  your email is causing this ugly,
> hard-to-follow formatting.
>
> >>
> >>         Hi,
> >>
> >>
> >>         On Wed, Jul 11, 2012 at 12:43 PM, Michal Simek
> >> <monstr at monstr.eu
> >> <mailto:monstr at monstr.eu> <mailto:monstr at monstr.eu
> >> <mailto:monstr at monstr.eu>>> wrote:
> >>
> >>              On 07/11/2012 11:59 AM, Simon Glass wrote:
> >>
> >>                  Hi Michal,
> >>
> >>
> >>                  On Wed, Jul 11, 2012 at 10:19 AM, Michal Simek
> >> <monstr at monstr.eu <mailto:monstr at monstr.eu> <mailto:monstr at monstr.eu
> >> <mailto:monstr at monstr.eu>> <mailto:monstr at monstr.eu
> >> <mailto:monstr at monstr.eu> <mailto:monstr at monstr.eu
> >> <mailto:monstr at monstr.eu>>>> wrote:
> >>
> >>                       On 07/10/2012 11:13 PM, Simon Glass wrote:
> >>
> >>                           Hi Michal,
> >>
> >>
> >>                           On Tue, Jul 10, 2012 at 11:56 AM, Michal
> >> Simek
> >> <monstr at monstr.eu <mailto:monstr at monstr.eu> <mailto:monstr at monstr.eu
> >> <mailto:monstr at monstr.eu>> <mailto:monstr at monstr.eu
> >> <mailto:monstr at monstr.eu> <mailto:monstr at monstr.eu
> >> <mailto:monstr at monstr.eu>>> <mailto:monstr at monstr.eu
> >> <mailto:monstr at monstr.eu> <mailto:monstr at monstr.eu
> >> <mailto:monstr at monstr.eu>> <mailto:monstr at monstr.eu
> >> <mailto:monstr at monstr.eu> <mailto:monstr at monstr.eu
> >> <mailto:monstr at monstr.eu>>>>> wrote:
> >>
> >>                                Also include arch specific gpio.h which
> >> can
> >> contain
> >>                                gpio function implementation.
> >>
> >>                                Signed-off-by: Michal Simek
> >> <monstr at monstr.eu <mailto:monstr at monstr.eu> <mailto:monstr at monstr.eu
> >> <mailto:monstr at monstr.eu>> <mailto:monstr at monstr.eu
> >> <mailto:monstr at monstr.eu> <mailto:monstr at monstr.eu
> >> <mailto:monstr at monstr.eu>>> <mailto:monstr at monstr.eu
> >> <mailto:monstr at monstr.eu> <mailto:monstr at monstr.eu
> >> <mailto:monstr at monstr.eu>> <mailto:monstr at monstr.eu
> >> <mailto:monstr at monstr.eu> <mailto:monstr at monstr.eu
> >> <mailto:monstr at monstr.eu>>>>>
> >>                                CC: Simon Glass <sjg at chromium.org
> >> <mailto:sjg at chromium.org> <mailto:sjg at chromium.org
> >> <mailto:sjg at chromium.org>> <mailto:sjg at chromium.org
> >> <mailto:sjg at chromium.org> <mailto:sjg at chromium.org
> >> <mailto:sjg at chromium.org>>> <mailto:sjg at chromium.org
> >> <mailto:sjg at chromium.org> <mailto:sjg at chromium.org
> >> <mailto:sjg at chromium.org>> <mailto:sjg at chromium.org
> >> <mailto:sjg at chromium.org> <mailto:sjg at chromium.org
> >> <mailto:sjg at chromium.org>>>>>
> >>
> >>
> >>
> >>
> >>                                ---
> >>                                  lib/fdtdec.c |    1 +
> >>                                  1 files changed, 1 insertions(+), 0
> >> deletions(-)
> >>
> >>                                diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> >>                                index cc09e06..b12eb77 100644
> >>                                --- a/lib/fdtdec.c
> >>                                +++ b/lib/fdtdec.c
> >>                                @@ -25,6 +25,7 @@
> >>                                  #include <fdtdec.h>
> >>
> >>                                  /* we need the generic GPIO interface
> >> here */
> >>                                +#include <asm/gpio.h>
> >>
> >>
> >>                           Can you not instead support the generic
> >> functions? What are you missing from from there that fdtdec.c needs?
> >>
> >>
> >>                       OK. The problem is with fdtdec_setup_gpio
> >> function
> >> where you called gpio_request which
> >>                       is gpio specific function which you haven't
> >> included.
> >>
> >>                       The most archs have this defined in
> >> arch/<cpuname>/include/asm/______gpio.h.
> >>
> >>
> >>
> >>
> >>
> >>                  This is there in my tree. Please see commit:
> >>
> >>                  5f533aeb gpio: Modify common gpio.h to more closely
> >> match
> >> Linux
> >>
> >>
> >>              This is generic gpio cleanup but I don't think this is the
> >> same thing.
> >>              I think you should get some warnings for fdtdec
> >> compilation
> >> around missing gpio_request
> >>              declaration or you include gpio.h in any other header file
> >> which is in fdtdec.c/h.
> >>
> >>
> >>         Sorry I don't really understand that. For me fdtdec.c has
> >> #include
> >> <asm-generic/gpio.h> and that is enough to get gpio_request(). Can you
> >> please advise what warning you see and for what board?
> >>
> >>
> >>     Microblaze:
> >>     lib/libgeneric.o: In function `fdtdec_setup_gpio':
> >>     /mnt/projects/u-boot/lib/__fdtdec.c:477: undefined reference to
> >> `gpio_request'
> >>
> >>
> >>     The same error will be for nios2, openrisc and blackfin because
> >> they
> >> define
> >>     gpio_request as static inline function in
> >> arch/<arch>/include/asm/gpio.h
> >>
> >>     Because I think there should be asm/gpio.h included which can
> >> include
> >> asm-generic/gpio.h
> >>     (this is arm case).
> >>
> >>
> >> OK I see, that makes sense.
> >>
>
> It seems to me that those platforms simply haven't been updated to use
> the generic gpio.h and should be changed.

Yes, I agree they should implement gpio_request() and gpio_free()
properly, but should we ask for that to be done for all archs, or just
accept this patch?

>
> >>
> >>     diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> >>     index cc09e06..50a5061 100644
> >>     --- a/lib/fdtdec.c
> >>     +++ b/lib/fdtdec.c
> >>     @@ -25,7 +25,7 @@
> >>
> >>       #include <fdtdec.h>
> >>
> >>       /* we need the generic GPIO interface here */
> >>
> >>
> >> Can we remove this comment then?
> >
> >
> > Yes, we can! :-)
> >
> > I will create proper patch.
>
> Thanks,
> -Joe


Regards,
Simon