All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] fdt: Include arch specific gpio.h
@ 2012-07-10  9:56 Michal Simek
  2012-07-10 21:13 ` Simon Glass
  0 siblings, 1 reply; 16+ messages in thread
From: Michal Simek @ 2012-07-10  9:56 UTC (permalink / raw)
  To: u-boot

Also include arch specific gpio.h which can contain
gpio function implementation.

Signed-off-by: Michal Simek <monstr@monstr.eu>
CC: Simon Glass <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>
 #include <asm-generic/gpio.h>
 
 DECLARE_GLOBAL_DATA_PTR;
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] fdt: Include arch specific gpio.h
  2012-07-10  9:56 [U-Boot] [PATCH] fdt: Include arch specific gpio.h Michal Simek
@ 2012-07-10 21:13 ` Simon Glass
  2012-07-11  8:19   ` Michal Simek
  0 siblings, 1 reply; 16+ messages in thread
From: Simon Glass @ 2012-07-10 21:13 UTC (permalink / raw)
  To: u-boot

Hi Michal,

On Tue, Jul 10, 2012 at 11:56 AM, Michal Simek <monstr@monstr.eu> wrote:

> Also include arch specific gpio.h which can contain
> gpio function implementation.
>
> Signed-off-by: Michal Simek <monstr@monstr.eu>
> CC: Simon Glass <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?


>  #include <asm-generic/gpio.h>
>
>  DECLARE_GLOBAL_DATA_PTR;
> --
> 1.7.0.4
>
> Regards,
Simon

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] fdt: Include arch specific gpio.h
  2012-07-10 21:13 ` Simon Glass
@ 2012-07-11  8:19   ` Michal Simek
  2012-07-11  9:59     ` Simon Glass
  0 siblings, 1 reply; 16+ messages in thread
From: Michal Simek @ 2012-07-11  8:19 UTC (permalink / raw)
  To: u-boot

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

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] fdt: Include arch specific gpio.h
  2012-07-11  8:19   ` Michal Simek
@ 2012-07-11  9:59     ` Simon Glass
  2012-07-11 10:43       ` Michal Simek
  0 siblings, 1 reply; 16+ messages in thread
From: Simon Glass @ 2012-07-11  9:59 UTC (permalink / raw)
  To: u-boot

Hi Michal,

On Wed, Jul 11, 2012 at 10:19 AM, Michal Simek <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 at 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 at monstr.eu>>
>>     CC: Simon Glass <sjg at 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

Regards,
Simon


> 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
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] fdt: Include arch specific gpio.h
  2012-07-11  9:59     ` Simon Glass
@ 2012-07-11 10:43       ` Michal Simek
  2012-07-11 10:59         ` Simon Glass
  0 siblings, 1 reply; 16+ messages in thread
From: Michal Simek @ 2012-07-11 10:43 UTC (permalink / raw)
  To: u-boot

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

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] fdt: Include arch specific gpio.h
  2012-07-11 10:43       ` Michal Simek
@ 2012-07-11 10:59         ` Simon Glass
  2012-07-11 11:21           ` Michal Simek
  0 siblings, 1 reply; 16+ messages in thread
From: Simon Glass @ 2012-07-11 10:59 UTC (permalink / raw)
  To: u-boot

Hi,

On Wed, Jul 11, 2012 at 12:43 PM, Michal Simek <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 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@monstr.eu<mailto:
>> monstr at monstr.eu> <mailto:monstr at 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 at monstr.eu> <mailto:monstr at monstr.eu <mailto:monstr@monstr.eu>>>
>>              CC: Simon Glass <sjg at chromium.org <mailto:sjg@chromium.org>
>> <mailto:sjg at 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?

Regards,
Simon


>
>
> 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
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] fdt: Include arch specific gpio.h
  2012-07-11 10:59         ` Simon Glass
@ 2012-07-11 11:21           ` Michal Simek
  2012-07-11 12:00             ` Simon Glass
  2012-07-12 16:37             ` Joe Hershberger
  0 siblings, 2 replies; 16+ messages in thread
From: Michal Simek @ 2012-07-11 11:21 UTC (permalink / raw)
  To: u-boot

On 07/11/2012 12:59 PM, Simon Glass wrote:
> Hi,
>
> On Wed, Jul 11, 2012 at 12:43 PM, Michal Simek <monstr at 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 at monstr.eu <mailto:monstr@monstr.eu> <mailto:monstr at 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 at monstr.eu <mailto:monstr@monstr.eu> <mailto:monstr at monstr.eu <mailto:monstr@monstr.eu>> <mailto:monstr at monstr.eu <mailto:monstr@monstr.eu> <mailto:monstr at 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 at monstr.eu <mailto:monstr@monstr.eu> <mailto:monstr at monstr.eu <mailto:monstr@monstr.eu>> <mailto:monstr at monstr.eu <mailto:monstr@monstr.eu> <mailto:monstr at monstr.eu <mailto:monstr@monstr.eu>>>>
>                       CC: Simon Glass <sjg at chromium.org <mailto:sjg@chromium.org> <mailto:sjg at chromium.org <mailto:sjg@chromium.org>> <mailto:sjg at chromium.org <mailto:sjg@chromium.org> <mailto:sjg at 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).

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 */
-#include <asm-generic/gpio.h>
+#include <asm/gpio.h>

  DECLARE_GLOBAL_DATA_PTR;


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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] fdt: Include arch specific gpio.h
  2012-07-11 11:21           ` Michal Simek
@ 2012-07-11 12:00             ` Simon Glass
  2012-07-11 12:21               ` Michal Simek
  2012-07-12 16:37             ` Joe Hershberger
  1 sibling, 1 reply; 16+ messages in thread
From: Simon Glass @ 2012-07-11 12:00 UTC (permalink / raw)
  To: u-boot

Hi Michal,

On Wed, Jul 11, 2012 at 1:21 PM, Michal Simek <monstr@monstr.eu> wrote:

> On 07/11/2012 12:59 PM, Simon Glass wrote:
>
>> Hi,
>>
>>
>> On Wed, Jul 11, 2012 at 12:43 PM, Michal Simek <monstr@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@monstr.eu<mailto:
>> monstr at monstr.eu> <mailto:monstr at 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 at monstr.eu <mailto:monstr@monstr.eu> <mailto:monstr@monstr.eu<mailto:
>> monstr at monstr.eu>> <mailto:monstr at monstr.eu <mailto:monstr@monstr.eu>
>> <mailto:monstr at 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 at monstr.eu> <mailto:monstr at monstr.eu <mailto:monstr@monstr.eu>>
>> <mailto:monstr at monstr.eu <mailto:monstr@monstr.eu> <mailto:
>> monstr at monstr.eu <mailto:monstr@monstr.eu>>>>
>>                       CC: Simon Glass <sjg@chromium.org <mailto:
>> sjg at chromium.org> <mailto:sjg at chromium.org <mailto:sjg@chromium.org>>
>> <mailto:sjg at chromium.org <mailto:sjg@chromium.org> <mailto:
>> sjg at 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.


> 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?


> -#include <asm-generic/gpio.h>
> +#include <asm/gpio.h>
>
>  DECLARE_GLOBAL_DATA_PTR;
>
>
>
> Thanks,
> Michal
>
> Regards,
Simon


>
>
>
>
>
>
> --
> 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
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] fdt: Include arch specific gpio.h
  2012-07-11 12:00             ` Simon Glass
@ 2012-07-11 12:21               ` Michal Simek
  2012-07-11 21:02                 ` Joe Hershberger
  0 siblings, 1 reply; 16+ messages in thread
From: Michal Simek @ 2012-07-11 12:21 UTC (permalink / raw)
  To: u-boot

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@monstr.eu>> wrote:
>
>     On 07/11/2012 12:59 PM, Simon Glass wrote:
>
>         Hi,
>
>
>         On Wed, Jul 11, 2012 at 12:43 PM, Michal Simek <monstr at monstr.eu <mailto:monstr@monstr.eu> <mailto:monstr at 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 at monstr.eu <mailto:monstr@monstr.eu> <mailto:monstr at monstr.eu <mailto:monstr@monstr.eu>> <mailto:monstr at monstr.eu <mailto:monstr@monstr.eu> <mailto:monstr at 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 at monstr.eu <mailto:monstr@monstr.eu> <mailto:monstr at monstr.eu <mailto:monstr@monstr.eu>> <mailto:monstr at monstr.eu <mailto:monstr@monstr.eu> <mailto:monstr at monstr.eu <mailto:monstr@monstr.eu>>> <mailto:monstr at monstr.eu <mailto:monstr@monstr.eu> <mailto:monstr at monstr.eu <mailto:monstr@monstr.eu>> <mailto:monstr at monstr.eu <mailto:monstr@monstr.eu> <mailto:monstr at 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 at monstr.eu <mailto:monstr@monstr.eu> <mailto:monstr at monstr.eu <mailto:monstr@monstr.eu>> <mailto:monstr at monstr.eu <mailto:monstr@monstr.eu> <mailto:monstr at monstr.eu <mailto:monstr@monstr.eu>>> <mailto:monstr at monstr.eu <mailto:monstr@monstr.eu> <mailto:monstr at monstr.eu <mailto:monstr@monstr.eu>> <mailto:monstr at monstr.eu <mailto:monstr@monstr.eu> <mailto:monstr at monstr.eu <mailto:monstr@monstr.eu>>>>>
>                                CC: Simon Glass <sjg at chromium.org <mailto:sjg@chromium.org> <mailto:sjg at chromium.org <mailto:sjg@chromium.org>> <mailto:sjg at chromium.org <mailto:sjg@chromium.org> <mailto:sjg at chromium.org <mailto:sjg@chromium.org>>> <mailto:sjg at chromium.org <mailto:sjg@chromium.org> <mailto:sjg at chromium.org <mailto:sjg@chromium.org>> <mailto:sjg at chromium.org <mailto:sjg@chromium.org> <mailto:sjg at 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.
>
>
>     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,
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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] fdt: Include arch specific gpio.h
  2012-07-11 12:21               ` Michal Simek
@ 2012-07-11 21:02                 ` Joe Hershberger
  2012-07-12  5:05                   ` Simon Glass
  2012-07-12  5:33                   ` Michal Simek
  0 siblings, 2 replies; 16+ messages in thread
From: Joe Hershberger @ 2012-07-11 21:02 UTC (permalink / raw)
  To: u-boot

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 at 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 at 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 at 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.

>>
>>     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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] fdt: Include arch specific gpio.h
  2012-07-11 21:02                 ` Joe Hershberger
@ 2012-07-12  5:05                   ` Simon Glass
  2012-07-12  5:33                   ` Michal Simek
  1 sibling, 0 replies; 16+ messages in thread
From: Simon Glass @ 2012-07-12  5:05 UTC (permalink / raw)
  To: u-boot

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 at 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 at 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 at 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] fdt: Include arch specific gpio.h
  2012-07-11 21:02                 ` Joe Hershberger
  2012-07-12  5:05                   ` Simon Glass
@ 2012-07-12  5:33                   ` Michal Simek
  2012-07-12  5:38                     ` Simon Glass
  1 sibling, 1 reply; 16+ messages in thread
From: Michal Simek @ 2012-07-12  5:33 UTC (permalink / raw)
  To: u-boot

On 07/11/2012 11:02 PM, Joe Hershberger 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.

Will look at it. Thanks for pointing to it. I probably used gmail
for my reply. I believe that Thunderbird will be fine.

>>>
>>>                                 ---
>>>                                   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.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] fdt: Include arch specific gpio.h
  2012-07-12  5:33                   ` Michal Simek
@ 2012-07-12  5:38                     ` Simon Glass
  0 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2012-07-12  5:38 UTC (permalink / raw)
  To: u-boot

Hi Michal,

On Thu, Jul 12, 2012 at 7:33 AM, Michal Simek <monstr@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@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.
>
>
> 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] fdt: Include arch specific gpio.h
  2012-07-11 11:21           ` Michal Simek
  2012-07-11 12:00             ` Simon Glass
@ 2012-07-12 16:37             ` Joe Hershberger
  2012-07-13  5:43               ` Michal Simek
  1 sibling, 1 reply; 16+ messages in thread
From: Joe Hershberger @ 2012-07-12 16:37 UTC (permalink / raw)
  To: u-boot

Hi Michal,

On Wed, Jul 11, 2012 at 6:21 AM, Michal Simek <monstr@monstr.eu> wrote:
> On 07/11/2012 12:59 PM, Simon Glass wrote:
>>
>> Hi,
>>
>>
>> On Wed, Jul 11, 2012 at 12:43 PM, Michal Simek <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>>> 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@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>>>>
>>                       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>>>>
>>
>>
>>
>>                       ---
>>                         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).

When you send an updated patch you need to add V2 or whatever to the
subject and you need to include a change log here.

I think you meant to reject this, not the other one here:
http://patchwork.ozlabs.org/patch/170457

> 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 */
> -#include <asm-generic/gpio.h>
> +#include <asm/gpio.h>
>
>  DECLARE_GLOBAL_DATA_PTR;

Thanks,
-Joe

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] fdt: Include arch specific gpio.h
  2012-07-12 16:37             ` Joe Hershberger
@ 2012-07-13  5:43               ` Michal Simek
  0 siblings, 0 replies; 16+ messages in thread
From: Michal Simek @ 2012-07-13  5:43 UTC (permalink / raw)
  To: u-boot

Hi Joe,

<snip>
>>>
>>>
>>>      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).
>
> When you send an updated patch you need to add V2 or whatever to the
> subject and you need to include a change log here.
>
> I think you meant to reject this, not the other one here:
> http://patchwork.ozlabs.org/patch/170457

I have sent this patch
http://lists.denx.de/pipermail/u-boot/2012-July/128257.html

which has broken subject that's why I have sent this message (it took me 2-3 mins)
http://lists.denx.de/pipermail/u-boot/2012-July/128259.html


And also send correct patch with proper subject and changelog
http://lists.denx.de/pipermail/u-boot/2012-July/128258.html
http://patchwork.ozlabs.org/patch/170458/


Can you please clarify what's wrong?

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] fdt: Include arch specific gpio.h
@ 2012-06-29  7:28 Michal Simek
  0 siblings, 0 replies; 16+ messages in thread
From: Michal Simek @ 2012-06-29  7:28 UTC (permalink / raw)
  To: u-boot

Also include arch specific gpio.h which can contain
gpio function implementation.

Signed-off-by: Michal Simek <monstr@monstr.eu>
CC: Simon Glass <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>
 #include <asm-generic/gpio.h>
 
 DECLARE_GLOBAL_DATA_PTR;
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2012-07-13  5:43 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-10  9:56 [U-Boot] [PATCH] fdt: Include arch specific gpio.h Michal Simek
2012-07-10 21:13 ` Simon Glass
2012-07-11  8:19   ` Michal Simek
2012-07-11  9:59     ` Simon Glass
2012-07-11 10:43       ` Michal Simek
2012-07-11 10:59         ` Simon Glass
2012-07-11 11:21           ` Michal Simek
2012-07-11 12:00             ` Simon Glass
2012-07-11 12:21               ` Michal Simek
2012-07-11 21:02                 ` Joe Hershberger
2012-07-12  5:05                   ` Simon Glass
2012-07-12  5:33                   ` Michal Simek
2012-07-12  5:38                     ` Simon Glass
2012-07-12 16:37             ` Joe Hershberger
2012-07-13  5:43               ` Michal Simek
  -- strict thread matches above, loose matches on Subject: below --
2012-06-29  7:28 Michal Simek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.