fdt: Include arch specific gpio.h

PATCH New Michal Simek 3 participants 15 messages Jul 10, 2012 – Jul 13, 2012 Library code

lore

Share & tools

lore

Michal Simek

author

2012-07-10 09:56 UTC #0 lore raw

PATCH 1/1 fdt: Include arch specific gpio.h

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

Signed-off-by Michal Simek <[email protected]>

CC Simon Glass <[email protected]>

---
 lib/fdtdec.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

lib/fdtdec.c +1 -0

index cc09e06..b12eb77 100644
@@ -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;

Signature · 1 line

1.7.0.4

Simon Glass

2012-07-10 21:13 UTC #1 lore raw

PATCH 1/1 fdt: Include arch specific gpio.h

Hi Michal,

Quoted 14 lines · Michal Simek

Also include arch specific gpio.h which can contain gpio function implementation.
Signed-off-by Michal Simek <[email protected]>
CC Simon Glass <[email protected]>
---
 lib/fdtdec.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
lib/fdtdec.c +0 -0
index cc09e06..b12eb77 100644

lib/fdtdec.c line 29

Quoted 5 lines

lib/fdtdec.c +1 -0

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

Quoted 7 lines

lib/fdtdec.c +0 -0
 #include <asm-generic/gpio.h>
 
DECLARE_GLOBAL_DATA_PTR;
--
1.7.0.4
Regards,

Simon

Michal Simek

author

2012-07-11 08:19 UTC #2 lore raw

PATCH 1/1 fdt: Include arch specific gpio.h

Quoted 24 lines · Simon Glass

Hi Michal,

On Tue, Jul 10, 2012 at 11:56 AM, Michal Simek <monstr at monstr.eu <mailto:[email protected]>> wrote:

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

    Signed-off-by: Michal Simek <monstr at monstr.eu <mailto:[email protected]>>
    CC: Simon Glass <sjg at chromium.org <mailto:[email protected]>>
    ---
      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>

Quoted 1 line

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

Signature · 4 lines

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

Simon Glass

2012-07-11 09:59 UTC #3 lore raw

PATCH 1/1 fdt: Include arch specific gpio.h

Hi Michal,

Quoted 34 lines · Michal Simek

Quoted 28 lines · Simon Glass
Hi Michal,
On Tue, Jul 10, 2012 at 11:56 AM, Michal Simek <[email protected] <mailto:
monstr at monstr.eu>> wrote:
Also include arch specific gpio.h which can contain gpio function implementation.
Signed-off-by Michal Simek <[email protected] <mailto:
monstr at monstr.eu>>
    CC: Simon Glass <sjg at chromium.org <mailto:[email protected]>>

    ---
      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>
Quoted 3 lines
Can you not instead support the generic functions? What are you missing from from there that fdtdec.c needs?

Quoted 6 lines

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

Quoted 10 lines

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

Michal Simek

author

2012-07-11 10:43 UTC #4 lore raw

PATCH 1/1 fdt: Include arch specific gpio.h

Quoted 38 lines · Simon Glass

Hi Michal,
On Wed, Jul 11, 2012 at 10:19 AM, Michal Simek <monstr at monstr.eu <mailto:[email protected]>> 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:[email protected]> <mailto:monstr at monstr.eu <mailto:[email protected]>>> wrote:
Also include arch specific gpio.h which can contain gpio function implementation.
Signed-off-by Michal Simek <monstr at monstr.eu <mailto:[email protected]> <mailto:monstr at monstr.eu <mailto:[email protected]>>>
CC Simon Glass <sjg at chromium.org <mailto:[email protected]> <mailto:sjg at chromium.org <mailto:[email protected]>>>
---
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.

Quoted 6 lines

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

Signature · 4 lines

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

Simon Glass

2012-07-11 10:59 UTC #5 lore raw

PATCH 1/1 fdt: Include arch specific gpio.h

Hi,

Quoted 56 lines · Michal Simek

Quoted 54 lines · Simon Glass
Hi Michal,
On Wed, Jul 11, 2012 at 10:19 AM, Michal Simek <[email protected] <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 <[email protected]<mailto:
monstr at monstr.eu> <mailto:monstr at monstr.eu <mailto:[email protected]>>> wrote:
Also include arch specific gpio.h which can contain gpio function implementation.
Signed-off-by Michal Simek <[email protected] <mailto:
monstr at monstr.eu> <mailto:monstr at monstr.eu <mailto:[email protected]>>>
CC: Simon Glass <sjg at chromium.org <mailto:[email protected]>
<mailto:sjg at chromium.org <mailto:[email protected]>>>
             ---
               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:

Quoted 8 lines

Quoted 2 lines
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

Quoted 12 lines

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

Michal Simek

author

2012-07-11 11:21 UTC #6 lore raw

PATCH 1/1 fdt: Include arch specific gpio.h

Quoted 55 lines · Simon Glass

Hi,
On Wed, Jul 11, 2012 at 12:43 PM, Michal Simek <monstr at monstr.eu <mailto:[email protected]>> 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:[email protected]> <mailto:monstr at monstr.eu <mailto:[email protected]>>> 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:[email protected]> <mailto:monstr at monstr.eu <mailto:[email protected]>> <mailto:monstr at monstr.eu <mailto:[email protected]> <mailto:monstr at monstr.eu <mailto:[email protected]>>>> wrote:
Also include arch specific gpio.h which can contain gpio function implementation.
Signed-off-by Michal Simek <monstr at monstr.eu <mailto:[email protected]> <mailto:monstr at monstr.eu <mailto:[email protected]>> <mailto:monstr at monstr.eu <mailto:[email protected]> <mailto:monstr at monstr.eu <mailto:[email protected]>>>>
CC Simon Glass <sjg at chromium.org <mailto:[email protected]> <mailto:sjg at chromium.org <mailto:[email protected]>> <mailto:sjg at chromium.org <mailto:[email protected]> <mailto:sjg at chromium.org <mailto:[email protected]>>>>
---
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

Quoted 6 lines

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

lib/fdtdec.c +1 -1

index cc09e06..50a5061 100644
@@ -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

Signature · 4 lines

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

Simon Glass

2012-07-11 12:00 UTC #7 lore raw

PATCH 1/1 fdt: Include arch specific gpio.h

Hi Michal,

Quoted 92 lines · Michal Simek

Quoted 80 lines · Simon Glass
Hi,
On Wed, Jul 11, 2012 at 12:43 PM, Michal Simek <[email protected] <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 <[email protected]<mailto:
monstr at monstr.eu> <mailto:monstr at monstr.eu <mailto:[email protected]>>> 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:[email protected]> <mailto:[email protected]<mailto:
monstr at monstr.eu>> <mailto:monstr at monstr.eu <mailto:[email protected]> <mailto:monstr at monstr.eu <mailto:[email protected]>>>> wrote:
Also include arch specific gpio.h which can contain gpio function implementation.
Signed-off-by Michal Simek <[email protected]<mailto:
monstr at monstr.eu> <mailto:monstr at monstr.eu <mailto:[email protected]>> <mailto:monstr at monstr.eu <mailto:[email protected]> <mailto:
monstr at monstr.eu <mailto:[email protected]>>>>
CC: Simon Glass <[email protected] <mailto:
sjg at chromium.org> <mailto:sjg at chromium.org <mailto:[email protected]>> <mailto:sjg at chromium.org <mailto:[email protected]> <mailto:
sjg at chromium.org <mailto:[email protected]>>>>
                      ---
                        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.
Quoted 4 lines
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'

Quoted 7 lines

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.

lib/fdtdec.c line 29

Quoted 10 lines

lib/fdtdec.c +0 -0

index cc09e06..50a5061 100644
@@ -25,7 +25,7 @@
 
 #include <fdtdec.h>
 
 /* we need the generic GPIO interface here */

Can we remove this comment then?

Quoted 11 lines

lib/fdtdec.c +1 -1
-#include <asm-generic/gpio.h>
+#include <asm/gpio.h>
 
DECLARE_GLOBAL_DATA_PTR;
Thanks,
Michal
Regards,

Simon

Quoted 13 lines

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

Michal Simek

author

2012-07-11 12:21 UTC #8 lore raw

PATCH 1/1 fdt: Include arch specific gpio.h

Quoted 97 lines · Simon Glass

Hi Michal,
On Wed, Jul 11, 2012 at 1:21 PM, Michal Simek <monstr at monstr.eu <mailto:[email protected]>> 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:[email protected]> <mailto:monstr at monstr.eu <mailto:[email protected]>>> 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:[email protected]> <mailto:monstr at monstr.eu <mailto:[email protected]>> <mailto:monstr at monstr.eu <mailto:[email protected]> <mailto:monstr at monstr.eu <mailto:[email protected]>>>> 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:[email protected]> <mailto:monstr at monstr.eu <mailto:[email protected]>> <mailto:monstr at monstr.eu <mailto:[email protected]> <mailto:monstr at monstr.eu <mailto:[email protected]>>> <mailto:monstr at monstr.eu <mailto:[email protected]> <mailto:monstr at monstr.eu <mailto:[email protected]>> <mailto:monstr at monstr.eu <mailto:[email protected]> <mailto:monstr at monstr.eu <mailto:[email protected]>>>>> wrote:
Also include arch specific gpio.h which can contain gpio function implementation.
Signed-off-by Michal Simek <monstr at monstr.eu <mailto:[email protected]> <mailto:monstr at monstr.eu <mailto:[email protected]>> <mailto:monstr at monstr.eu <mailto:[email protected]> <mailto:monstr at monstr.eu <mailto:[email protected]>>> <mailto:monstr at monstr.eu <mailto:[email protected]> <mailto:monstr at monstr.eu <mailto:[email protected]>> <mailto:monstr at monstr.eu <mailto:[email protected]> <mailto:monstr at monstr.eu <mailto:[email protected]>>>>>
CC Simon Glass <sjg at chromium.org <mailto:[email protected]> <mailto:sjg at chromium.org <mailto:[email protected]>> <mailto:sjg at chromium.org <mailto:[email protected]> <mailto:sjg at chromium.org <mailto:[email protected]>>> <mailto:sjg at chromium.org <mailto:[email protected]> <mailto:sjg at chromium.org <mailto:[email protected]>> <mailto:sjg at chromium.org <mailto:[email protected]> <mailto:sjg at chromium.org <mailto:[email protected]>>>>>
---
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 */

Quoted 1 line

Can we remove this comment then?

Yes, we can! :-)

I will create proper patch.

Thanks,
Michal

Signature · 4 lines

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

Joe Hershberger

2012-07-11 21:02 UTC #9 lore raw

PATCH 1/1 fdt: Include arch specific gpio.h

Hi Michal,

Quoted 9 lines · Michal Simek

Quoted 8 lines · Simon Glass
Hi Michal,
On Wed, Jul 11, 2012 at 1:21 PM, Michal Simek <[email protected] <mailto:[email protected]>> 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.

Quoted 123 lines

Quoted 123 lines
        Hi,


        On Wed, Jul 11, 2012 at 12:43 PM, Michal Simek <[email protected]
<mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>>> 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:[email protected]> <mailto:[email protected] <mailto:[email protected]>> <mailto:[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>>>> 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:[email protected]> <mailto:[email protected] <mailto:[email protected]>> <mailto:[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>>> <mailto:[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>> <mailto:[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>>>>> wrote:
Also include arch specific gpio.h which can
contain
gpio function implementation.
Signed-off-by: Michal Simek
<monstr at monstr.eu <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>> <mailto:[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>>> <mailto:[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>> <mailto:[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>>>>>
CC: Simon Glass <[email protected]
<mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>> <mailto:[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>>> <mailto:[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>> <mailto:[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>>>>>
---
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

Quoted 7 lines

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

Quoted 12 lines

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

Quoted 6 lines

Quoted 1 line
Can we remove this comment then?
Yes, we can! :-)
I will create proper patch.

Thanks,
-Joe

Simon Glass

2012-07-12 05:05 UTC #10 lore raw

PATCH 1/1 fdt: Include arch specific gpio.h

Hi Joe,

Quoted 154 lines · Joe Hershberger

Hi Michal,
Quoted 9 lines · Michal Simek
Quoted 8 lines · Simon Glass
Hi Michal,
On Wed, Jul 11, 2012 at 1:21 PM, Michal Simek <[email protected] <mailto:[email protected]>> 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.
Quoted 137 lines
Quoted 137 lines
        Hi,


        On Wed, Jul 11, 2012 at 12:43 PM, Michal Simek
<[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>>> 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:[email protected]> <mailto:[email protected] <mailto:[email protected]>> <mailto:[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>>>> 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:[email protected]> <mailto:[email protected] <mailto:[email protected]>> <mailto:[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>>> <mailto:[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>> <mailto:[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>>>>> wrote:
Also include arch specific gpio.h which
can
contain
gpio function implementation.
Signed-off-by: Michal Simek
<monstr at monstr.eu <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>> <mailto:[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>>> <mailto:[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>> <mailto:[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>>>>>
CC: Simon Glass <[email protected]
<mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>> <mailto:[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>>> <mailto:[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>> <mailto:[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>>>>>
---
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).

Quoted 5 lines

Quoted 2 lines
Quoted 2 lines
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?

Quoted 16 lines

Quoted 14 lines


    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?

Quoted 6 lines

Quoted 3 lines
Yes, we can! :-)
I will create proper patch.
Thanks,
-Joe

Regards,
Simon

Michal Simek

author

2012-07-12 05:33 UTC #11 lore raw

PATCH 1/1 fdt: Include arch specific gpio.h

In reply to Joe Hershberger

Quoted 8 lines · Joe Hershberger

Hi Michal,
Quoted 5 lines · Michal Simek
Quoted 4 lines · Simon Glass
Hi Michal,

Quoted 7 lines

Quoted 4 lines
Quoted 4 lines
On Wed, Jul 11, 2012 at 1:21 PM, Michal Simek<[email protected] <mailto:[email protected]>> 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.

Quoted 69 lines

Quoted 69 lines
Quoted 69 lines
---
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).

Quoted 6 lines

Quoted 2 lines
Quoted 2 lines
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

Signature · 4 lines

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

Simon Glass

2012-07-12 05:38 UTC #12 lore raw

PATCH 1/1 fdt: Include arch specific gpio.h

Hi Michal,

Quoted 18 lines · Michal Simek

Quoted 17 lines · Joe Hershberger
Hi Michal,
Quoted 11 lines · Michal Simek
Quoted 9 lines · Simon Glass
Hi Michal,
On Wed, Jul 11, 2012 at 1:21 PM, Michal Simek<[email protected] <mailto:[email protected]>> wrote:
     On 07/11/2012 12:59 PM, Simon Glass wrote:

Quoted 6 lines

Quoted 2 lines
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.

Quoted 84 lines

Quoted 77 lines
Quoted 77 lines
Quoted 77 lines
---
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).
Quoted 6 lines
Quoted 2 lines
Quoted 2 lines
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.

Quoted 5 lines

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.

Quoted 11 lines

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

Joe Hershberger

2012-07-12 16:37 UTC #13 lore raw

PATCH 1/1 fdt: Include arch specific gpio.h

In reply to Michal Simek

Hi Michal,

Quoted 94 lines · Michal Simek

Quoted 84 lines · Simon Glass
Hi,
On Wed, Jul 11, 2012 at 12:43 PM, Michal Simek <[email protected] <mailto:[email protected]>> wrote:
    On 07/11/2012 11:59 AM, Simon Glass wrote:

        Hi Michal,


        On Wed, Jul 11, 2012 at 10:19 AM, Michal Simek <[email protected]
<mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>>> 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:[email protected]> <mailto:[email protected] <mailto:[email protected]>> <mailto:[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>>>> wrote:
Also include arch specific gpio.h which can contain gpio function implementation.
Signed-off-by Michal Simek <[email protected]
<mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>> <mailto:[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>>>>
CC: Simon Glass <[email protected]
<mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>> <mailto:[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>>>>
                      ---
                        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.
Quoted 3 lines
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'

Quoted 6 lines

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

lib/fdtdec.c line 31

Quoted 13 lines

lib/fdtdec.c +1 -1

index cc09e06..50a5061 100644
@@ -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

Michal Simek

author

2012-07-13 05:43 UTC #14 lore raw

PATCH 1/1 fdt: Include arch specific gpio.h

Hi Joe,

<snip>

Quoted 25 lines

Quoted 18 lines
Quoted 9 lines
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.
Quoted 3 lines
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'
Quoted 6 lines
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).

Quoted 5 lines

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

Signature · 4 lines

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