Skip to contentSkip to conversation

MX25: tx25: Avoid the usage of extern in C file

PATCH Superseded Fabio Estevam 5 participants 13 messages Sep 2, 2011 – Sep 5, 2011

Subsystems (3)

v3 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

PATCH v3 1/2 MX25: tx25: Avoid the usage of extern in C file

Avoid the usage of extern in C file as pointed out by checkpatch.

---
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(-)
arch/arm/include/asm/arch-mx25/imx-regs.h +1 -0
index 9e30f7c..7e34050 100644
@@ -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 */
board/karo/tx25/tx25.c +0 -1
index 25b99e8..2787715 100644
@@ -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
Signature · 1 line
1.7.1
#1 lore raw

PATCH v3 2/2 MX25: Add initial support for MX25PDK

Add the initial support for MX25PDK booting from SD card via internal boot.

---
Changes since v2:
- No changes
Changes since v1:
- Keep mx25pdk entry to boards.cfg sorted
- Removed unused defines in mx25pdk.h
- Do not use the whole RAM space for memory test

 MAINTAINERS                             |    1 +
 board/freescale/mx25pdk/Makefile        |   49 ++++++++++++++
 board/freescale/mx25pdk/imximage.cfg    |   73 +++++++++++++++++++++
 board/freescale/mx25pdk/lowlevel_init.S |   21 ++++++
 board/freescale/mx25pdk/mx25pdk.c       |   62 ++++++++++++++++++
 boards.cfg                              |    1 +
 include/configs/mx25pdk.h               |  106 +++++++++++++++++++++++++++++++
 7 files changed, 313 insertions(+), 0 deletions(-)
 create mode 100644 board/freescale/mx25pdk/Makefile
 create mode 100644 board/freescale/mx25pdk/imximage.cfg
 create mode 100644 board/freescale/mx25pdk/lowlevel_init.S
 create mode 100644 board/freescale/mx25pdk/mx25pdk.c
 create mode 100644 include/configs/mx25pdk.h
MAINTAINERS +1 -0
index f895e9a..7f07b60 100644
@@ -645,6 +645,7 @@ Kristoffer Ericson <[email protected]>
 
 Fabio Estevam <[email protected]>
 
+	mx25pdk		i.MX25
 	mx31pdk		i.MX31
 	mx53ard		i.MX53
 	mx53smd		i.MX53
board/freescale/mx25pdk/Makefile new file +49 -0

49 lines not on this page · raw message

board/freescale/mx25pdk/imximage.cfg new file +73 -0

73 lines not on this page · raw message

board/freescale/mx25pdk/lowlevel_init.S new file +21 -0
new file mode 100644
index 0000000..6e6810f
@@ -0,0 +1,21 @@
+/*
+ * Copyright (c) 2011  Freescale Semiconductor
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+.globl lowlevel_init
+lowlevel_init:
+
+	mov	pc, lr
board/freescale/mx25pdk/mx25pdk.c new file +62 -0

62 lines not on this page · raw message

boards.cfg +1 -0
index c31114d..17452e4 100644
@@ -162,6 +162,7 @@ rd6281a                      arm         arm926ejs   -                   Marvell
 sheevaplug                   arm         arm926ejs   -                   Marvell        kirkwood
 dockstar                     arm         arm926ejs   -                   Seagate        kirkwood
 jadecpu                      arm         arm926ejs   jadecpu             syteco         mb86r0x
+mx25pdk                      arm         arm926ejs   mx25pdk             freescale      mx25		mx25pdk:IMX_CONFIG=board/freescale/mx25pdk/imximage.cfg
 zmx25                        arm         arm926ejs   zmx25               syteco         mx25
 imx27lite                    arm         arm926ejs   imx27lite           logicpd        mx27
 magnesium                    arm         arm926ejs   imx27lite           logicpd        mx27
include/configs/mx25pdk.h new file +106 -0

106 lines not on this page · raw message

Signature · 1 line
1.7.1
#2 lore raw

PATCH v3 1/2 MX25: tx25: Avoid the usage of extern in C file

In reply to Fabio Estevam
Quoted 3 lines · Fabio Estevam

Avoid the usage of extern in C file as pointed out by checkpatch.

Hi Fabio,

arch/arm/include/asm/arch-mx25/imx-regs.h line 40

Quoted 9 lines
arch/arm/include/asm/arch-mx25/imx-regs.h +1 -0
index 9e30f7c..7e34050 100644
@@ -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

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
reviewer
#3 lore raw

PATCH v3 1/2 MX25: tx25: Avoid the usage of extern in C file

On Mon, Sep 5, 2011 at 7:43 AM, Stefano Babic <[email protected]> wrote:
...

Quoted 4 lines

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

#4 lore raw

PATCH v3 1/2 MX25: tx25: Avoid the usage of extern in C file

Quoted 8 lines · Fabio Estevam

On Mon, Sep 5, 2011 at 7:43 AM, Stefano Babic <[email protected]> wrote:
...

Quoted 4 lines

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.

Quoted 6 lines

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 ?

Quoted 8 lines

Regards,

Fabio Estevam
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

reviewer
#5 lore raw

PATCH v3 1/2 MX25: tx25: Avoid the usage of extern in C file

On Mon, Sep 5, 2011 at 10:05 AM, Marek Vasut <[email protected]> wrote:
...

Quoted 7 lines
Quoted 3 lines

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

#6 lore raw

PATCH v3 1/2 MX25: tx25: Avoid the usage of extern in C file

Quoted 7 lines · Fabio Estevam

On Mon, Sep 5, 2011 at 10:05 AM, Marek Vasut <[email protected]> wrote:
...

Quoted 4 lines
Quoted 3 lines

extern void mx25_fec_init_pins(void);
extern void imx_get_mac_from_fuse(unsigned char *mac); extern void mx25_uart1_init_pins(void);

Quoted 6 lines
Quoted 3 lines

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

Quoted 3 lines

Regards,

Fabio Estevam

reviewer
#7 lore raw

PATCH v3 1/2 MX25: tx25: Avoid the usage of extern in C file

Quoted 8 lines · Marek Vasut
Quoted 7 lines · Fabio Estevam

On Mon, Sep 5, 2011 at 10:05 AM, Marek Vasut <[email protected]> wrote:
...

Quoted 4 lines
Quoted 3 lines

extern void mx25_fec_init_pins(void);
extern void imx_get_mac_from_fuse(unsigned char *mac); extern void mx25_uart1_init_pins(void);

Quoted 7 lines
Quoted 6 lines
Quoted 3 lines

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

#8 lore raw

PATCH v3 1/2 MX25: tx25: Avoid the usage of extern in C file

Quoted 21 lines · Fabio Estevam
Quoted 8 lines · Marek Vasut
Quoted 7 lines · Fabio Estevam

On Mon, Sep 5, 2011 at 10:05 AM, Marek Vasut <[email protected]>
wrote: ...

Quoted 4 lines
Quoted 3 lines

extern void mx25_fec_init_pins(void);
extern void imx_get_mac_from_fuse(unsigned char *mac); extern void mx25_uart1_init_pins(void);

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines

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:

Review

Quoted 1 line
(quoted diff) +1 -0
+	extern void mx25_uart1_init_pins(void);

But you're using extern in _header_ (.h) file ... so ... why ?

reviewer
#9 lore raw

PATCH v3 1/2 MX25: tx25: Avoid the usage of extern in C file

On Mon, Sep 5, 2011 at 12:40 PM, Marek Vasut <[email protected]> wrote:
...

Quoted 8 lines
Quoted 5 lines

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

#10 lore raw

PATCH v3 1/2 MX25: tx25: Avoid the usage of extern in C file

Dear Fabio Estevam,

Quoted 6 lines · In message <CAOMZO5A1v5jUkXTF9nKnZh8VNewzkoMB1tMmE9cRJCJDp+y2ug@mail.gmail.com>…
Quoted 3 lines

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

Signature · 6 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
One essential to success is that you desire be an all-obsessing  one,
your thoughts and aims be co-ordinated, and your energy be concentra-
ted and applied without letup.
#11 lore raw

PATCH v3 1/2 MX25: tx25: Avoid the usage of extern in C file

Quoted 3 lines · Wolfgang Denk

Dear Fabio Estevam,

In message

Quoted 8 lines · <CAOMZO5A1v5jUkXTF9nKnZh8VNewzkoMB1tMmE9cRJCJDp+y2ug@mail.gmail.com> you wrote:
Quoted 4 lines
Quoted 1 line

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!

Quoted 4 lines

Best regards,

Wolfgang Denk

#12 lore raw

PATCH v3 1/2 MX25: tx25: Avoid the usage of extern in C file

In reply to Fabio Estevam
Quoted 3 lines · Fabio Estevam

On Mon, Sep 5, 2011 at 12:40 PM, Marek Vasut <[email protected]> wrote:
...

Hi Fabio,

Quoted 6 lines
Quoted 6 lines
Quoted 5 lines

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

Quoted 9 lines
Quoted 2 lines

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

Signature · 17 lines
checpatch reports neither errors nor warnings:

checkpatch.pl --no-tree 0001-tmp.patch
total: 0 errors, 0 warnings, 31 lines checked

I think you can try moving the prototypes in sys_proto.h, and run
checkpatch on it without extern. I expect to see no warnings at all.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================