| Message ID | 1314972139-20068-1-git-send-email-fabio.estevam@freescale.com |
|---|---|
| State | Superseded |
| Delegated to: | Stefano Babic |
| Headers | show |
On 09/02/2011 04:02 PM, Fabio Estevam wrote: > Avoid the usage of extern in C file as pointed out by checkpatch. > > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> Hi Fabio, > diff --git a/arch/arm/include/asm/arch-mx25/imx-regs.h b/arch/arm/include/asm/arch-mx25/imx-regs.h > index 9e30f7c..7e34050 100644 > --- a/arch/arm/include/asm/arch-mx25/imx-regs.h > +++ b/arch/arm/include/asm/arch-mx25/imx-regs.h > @@ -37,6 +37,7 @@ > #ifdef CONFIG_FEC_MXC > extern void mx25_fec_init_pins(void); > extern void imx_get_mac_from_fuse(unsigned char *mac); > +extern void mx25_uart1_init_pins(void); As i can see, in the imx-regs.h for the different processors there is (as the name says) only definitions and structures for the internal registers of each processor. No function prototypes. What about to move the prototype ? For MX5/MX35 there is a sys_proto.h (as it is done for other SOCs), in this case, well, it seems too much to add a file for a single line. However, we could move it in clock.h and creating a sys_proto.h if the number of exported functions will increase. Best regards, Stefano Babic
On Mon, Sep 5, 2011 at 7:43 AM, Stefano Babic <sbabic@denx.de> wrote: ... > What about to move the prototype ? For MX5/MX35 there is a sys_proto.h > (as it is done for other SOCs), in this case, well, it seems too much to > add a file for a single line. However, we could move it in clock.h and > creating a sys_proto.h if the number of exported functions will increase. Ok, I can create a sys_proto.h for MX25 and put the following extern´s there: extern void mx25_fec_init_pins(void); extern void imx_get_mac_from_fuse(unsigned char *mac); extern void mx25_uart1_init_pins(void); Regards, Fabio Estevam
On Monday, September 05, 2011 01:37:17 PM Fabio Estevam wrote: > On Mon, Sep 5, 2011 at 7:43 AM, Stefano Babic <sbabic@denx.de> wrote: > ... > > > What about to move the prototype ? For MX5/MX35 there is a sys_proto.h > > (as it is done for other SOCs), in this case, well, it seems too much to > > add a file for a single line. However, we could move it in clock.h and > > creating a sys_proto.h if the number of exported functions will increase. > > Ok, I can create a sys_proto.h for MX25 and put the following extern´s > there: > > extern void mx25_fec_init_pins(void); > extern void imx_get_mac_from_fuse(unsigned char *mac); > extern void mx25_uart1_init_pins(void); Ok, this might be a stupid one, but ... why use externs in header files ? > > Regards, > > Fabio Estevam > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot
On Mon, Sep 5, 2011 at 10:05 AM, Marek Vasut <marek.vasut@gmail.com> wrote: ... >> extern void mx25_fec_init_pins(void); >> extern void imx_get_mac_from_fuse(unsigned char *mac); >> extern void mx25_uart1_init_pins(void); > > Ok, this might be a stupid one, but ... > > why use externs in header files ? This will make checkpatch happy :-) Regards, Fabio Estevam
On Monday, September 05, 2011 03:15:44 PM Fabio Estevam wrote: > On Mon, Sep 5, 2011 at 10:05 AM, Marek Vasut <marek.vasut@gmail.com> wrote: > ... > > >> extern void mx25_fec_init_pins(void); > >> extern void imx_get_mac_from_fuse(unsigned char *mac); > >> extern void mx25_uart1_init_pins(void); > > > > Ok, this might be a stupid one, but ... > > > > why use externs in header files ? > > This will make checkpatch happy :-) > I'm not quite sure I understand ... ? > Regards, > > Fabio Estevam
On Mon, Sep 5, 2011 at 12:05 PM, Marek Vasut <marek.vasut@gmail.com> wrote: > On Monday, September 05, 2011 03:15:44 PM Fabio Estevam wrote: >> On Mon, Sep 5, 2011 at 10:05 AM, Marek Vasut <marek.vasut@gmail.com> wrote: >> ... >> >> >> extern void mx25_fec_init_pins(void); >> >> extern void imx_get_mac_from_fuse(unsigned char *mac); >> >> extern void mx25_uart1_init_pins(void); >> > >> > Ok, this might be a stupid one, but ... >> > >> > why use externs in header files ? >> >> This will make checkpatch happy :-) >> > I'm not quite sure I understand ... ? ./scripts/checkpatch.pl -F u-boot/board/karo/tx25/tx25.c .... WARNING: externs should be avoided in .c files #144: FILE: home/fabio/denx/u-boot/board/karo/tx25/tx25.c:144: + extern void mx25_uart1_init_pins(void);
On Monday, September 05, 2011 05:38:26 PM Fabio Estevam wrote: > On Mon, Sep 5, 2011 at 12:05 PM, Marek Vasut <marek.vasut@gmail.com> wrote: > > On Monday, September 05, 2011 03:15:44 PM Fabio Estevam wrote: > >> On Mon, Sep 5, 2011 at 10:05 AM, Marek Vasut <marek.vasut@gmail.com> > >> wrote: ... > >> > >> >> extern void mx25_fec_init_pins(void); > >> >> extern void imx_get_mac_from_fuse(unsigned char *mac); > >> >> extern void mx25_uart1_init_pins(void); > >> > > >> > Ok, this might be a stupid one, but ... > >> > > >> > why use externs in header files ? > >> > >> This will make checkpatch happy :-) > > > > I'm not quite sure I understand ... ? > > ./scripts/checkpatch.pl -F u-boot/board/karo/tx25/tx25.c > .... > WARNING: externs should be avoided in .c files > #144: FILE: home/fabio/denx/u-boot/board/karo/tx25/tx25.c:144: > + extern void mx25_uart1_init_pins(void); But you're using extern in _header_ (.h) file ... so ... why ?
On Mon, Sep 5, 2011 at 12:40 PM, Marek Vasut <marek.vasut@gmail.com> wrote: ... >> ./scripts/checkpatch.pl -F u-boot/board/karo/tx25/tx25.c >> .... >> WARNING: externs should be avoided in .c files >> #144: FILE: home/fabio/denx/u-boot/board/karo/tx25/tx25.c:144: >> + extern void mx25_uart1_init_pins(void); > > But you're using extern in _header_ (.h) file ... so ... why ? > My patch removes the extern from the C file and put it on a header file with other extern's. The checkpatch warning I showed happens with the original file, and not after my patch is applied.
Dear Fabio Estevam, In message <CAOMZO5A1v5jUkXTF9nKnZh8VNewzkoMB1tMmE9cRJCJDp+y2ug@mail.gmail.com> you wrote: > > > But you're using extern in _header_ (.h) file ... so ... why ? > > > > My patch removes the extern from the C file and put it on a header > file with other extern's. What Marek probably wants to tell yet (in a bit a complicated way) is that you should omit the "extern" in the prototype declaration. Just remove all "extern" from the header file. Best regards, Wolfgang Denk
On Monday, September 05, 2011 06:02:21 PM Wolfgang Denk wrote: > Dear Fabio Estevam, > > In message <CAOMZO5A1v5jUkXTF9nKnZh8VNewzkoMB1tMmE9cRJCJDp+y2ug@mail.gmail.com> you wrote: > > > But you're using extern in _header_ (.h) file ... so ... why ? > > > > My patch removes the extern from the C file and put it on a header > > file with other extern's. > > What Marek probably wants to tell yet (in a bit a complicated way) is > that you should omit the "extern" in the prototype declaration. Just > remove all "extern" from the header file. No, I was asking if there's some special reason for this or it's plain wrong. I see the later's the case. Thanks, cheers! > > Best regards, > > Wolfgang Denk
On 09/05/2011 05:48 PM, Fabio Estevam wrote: > On Mon, Sep 5, 2011 at 12:40 PM, Marek Vasut <marek.vasut@gmail.com> wrote: > ... > Hi Fabio, >>> ./scripts/checkpatch.pl -F u-boot/board/karo/tx25/tx25.c >>> .... >>> WARNING: externs should be avoided in .c files >>> #144: FILE: home/fabio/denx/u-boot/board/karo/tx25/tx25.c:144: >>> + extern void mx25_uart1_init_pins(void); >> >> But you're using extern in _header_ (.h) file ... so ... why ? >> > > My patch removes the extern from the C file and put it on a header > file with other extern's. > > The checkpatch warning I showed happens with the original file, and > not after my patch is applied. > I think checkpatch warnings because the original file has already some extern. In the most of u-boot code we do not mark the prototypes with extern. I made this simple test - I create a new file (from sys_proto.h) and I run checkpatch on it: +#ifndef _SYS_PROTO_H_ +#define _SYS_PROTO_H_ + +u32 get_cpu_rev(void); +#define is_soc_rev(rev) ((get_cpu_rev() & 0xFF) - rev) +void sdelay(unsigned long); +void set_chipselect_size(int const); +#endif
diff --git a/arch/arm/include/asm/arch-mx25/imx-regs.h b/arch/arm/include/asm/arch-mx25/imx-regs.h index 9e30f7c..7e34050 100644 --- a/arch/arm/include/asm/arch-mx25/imx-regs.h +++ b/arch/arm/include/asm/arch-mx25/imx-regs.h @@ -37,6 +37,7 @@ #ifdef CONFIG_FEC_MXC extern void mx25_fec_init_pins(void); extern void imx_get_mac_from_fuse(unsigned char *mac); +extern void mx25_uart1_init_pins(void); #endif /* Clock Control Module (CCM) registers */ diff --git a/board/karo/tx25/tx25.c b/board/karo/tx25/tx25.c index 25b99e8..2787715 100644 --- a/board/karo/tx25/tx25.c +++ b/board/karo/tx25/tx25.c @@ -141,7 +141,6 @@ void tx25_fec_init(void) int board_init() { #ifdef CONFIG_MXC_UART - extern void mx25_uart1_init_pins(void); mx25_uart1_init_pins(); #endif
Avoid the usage of extern in C file as pointed out by checkpatch. Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> --- Changes since v2: - Place the extern in the MX25 imx-regs.h instead of inside the MX27 imx-regs.h Changes since v1: - No changes arch/arm/include/asm/arch-mx25/imx-regs.h | 1 + board/karo/tx25/tx25.c | 1 - 2 files changed, 1 insertions(+), 1 deletions(-)