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

[Simon Glass]

Hi Michal,

On Thu, Jul 12, 2012 at 7:33 AM, Michal Simek <monstr at monstr.eu> wrote:
> On 07/11/2012 11:02 PM, Joe Hershberger 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.
>
>
> Will look at it. Thanks for pointing to it. I probably used gmail
> for my reply. I believe that Thunderbird will be fine.

It might have been me, sorry.

>
>
>>>>
>>>>                                 ---
>>>>                                   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.
>>
>
> That can be truth but on the other hand I think that including
> asm-generic is not the best.
>
> Also in connection to this issue that code in fdtdec should be used
> when any CONFIG_GPIO config is enabled.

Well that highlights that we probably need a CONFIG to tell use when
the gpio layer is available. We can't be checking all the different
arch-specific options to find this out. Since, including
asm-generic/gpio.h is a reasonable solution in the fdtdec.c case for
most archs with -ffunction-sections (except ARM unless we apply
Charles Mannings' patch), since if no one actually calls those
functions we won't get a link error.

>
> Thanks
>
> Michal
>
>
> --
> Michal Simek, Ing. (M.Eng)
> w: www.monstr.eu p: +42-0-721842854
> Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
> Microblaze U-BOOT custodian