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

Hi Joe,

On Wed, Jul 11, 2012 at 11:02 PM, Joe Hershberger joe.hershberger@gmail.com wrote:

Hi Michal,

On Wed, Jul 11, 2012 at 7:21 AM, Michal Simek monstr@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@monstr.eu mailto:monstr@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@monstr.eu mailto:monstr@monstr.eu <mailto:monstr@monstr.eu mailto:monstr@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@monstr.eu mailto:monstr@monstr.eu <mailto:monstr@monstr.eu mailto:monstr@monstr.eu> <mailto:monstr@monstr.eu mailto:monstr@monstr.eu <mailto:monstr@monstr.eu mailto:monstr@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@monstr.eu mailto:monstr@monstr.eu <mailto:monstr@monstr.eu mailto:monstr@monstr.eu> <mailto:monstr@monstr.eu mailto:monstr@monstr.eu <mailto:monstr@monstr.eu mailto:monstr@monstr.eu>> <mailto:monstr@monstr.eu mailto:monstr@monstr.eu <mailto:monstr@monstr.eu mailto:monstr@monstr.eu> <mailto:monstr@monstr.eu mailto:monstr@monstr.eu <mailto:monstr@monstr.eu mailto:monstr@monstr.eu>>>> wrote:

                           Also include arch specific gpio.h which

can contain gpio function implementation.

                           Signed-off-by: Michal Simek

<monstr@monstr.eu mailto:monstr@monstr.eu <mailto:monstr@monstr.eu mailto:monstr@monstr.eu> <mailto:monstr@monstr.eu mailto:monstr@monstr.eu <mailto:monstr@monstr.eu mailto:monstr@monstr.eu>> <mailto:monstr@monstr.eu mailto:monstr@monstr.eu <mailto:monstr@monstr.eu mailto:monstr@monstr.eu> <mailto:monstr@monstr.eu mailto:monstr@monstr.eu <mailto:monstr@monstr.eu mailto:monstr@monstr.eu>>>> CC: Simon Glass <sjg@chromium.org mailto:sjg@chromium.org <mailto:sjg@chromium.org mailto:sjg@chromium.org> <mailto:sjg@chromium.org mailto:sjg@chromium.org <mailto:sjg@chromium.org mailto:sjg@chromium.org>> <mailto:sjg@chromium.org mailto:sjg@chromium.org <mailto:sjg@chromium.org mailto:sjg@chromium.org> <mailto:sjg@chromium.org mailto:sjg@chromium.org <mailto:sjg@chromium.org mailto:sjg@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