Skip to contentSkip to conversation

ARM: Support for Embedian MXM-8x10 eval kit

PATCH New Edwin Peer 4 participants 7 messages Jan 18, 2010 – Jan 19, 2010

Subsystems (3)
Arm (32-bit) Arm sub-architectures PXA2xx/PXA3xx support
lore
Share & tools
lore

Edwin Peer

author
#0 lore raw

PATCH 1/1 ARM: Support for Embedian MXM-8x10 eval kit

Dear Russell, Eric,

I believe we have finally managed to address all your review points - third time's a charm. :)

Here's the diffstat against 2.6.33-rc4:

Kconfig                |    5
Makefile               |    1
capc7117.c             |  157 ++++++++++++++++
include/mach/mxm8x10.h |   21 ++
mxm8x10.c              |  480 +++++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 664 insertions(+)

Please consider for inclusion again.

Thank you.

Regards,
Edwin Peer

Add support for the Embedian MXM-8x10 evaluation kit.
Product URL: http://www.embedian.com/index.php?main_page=product_info&cPath=68&products_id=229

Signed-off-by Edwin Peer <[email protected]>

---
Files linux-2.6.33-rc4/arch/arm/boot/compressed/piggy.gzip and linux-embedian-2.6.33-rc4/arch/arm/boot/compressed/piggy.gzip differ

arch/arm/mach-pxa/Kconfig +5 -0
diff -Nurp linux-2.6.33-rc4/arch/arm/mach-pxa/Kconfig linux-embedian-2.6.33-rc4/arch/arm/mach-pxa/Kconfig
@@ -115,6 +115,11 @@ config MACH_CM_X300
 	select CPU_PXA310
 	select HAVE_PWM
 
+config MACH_CAPC7117
+	bool "Embedian CAPC-7117 evaluation kit based on the MXM-8x10 CoM"
+	select CPU_PXA320
+	select PXA3xx
+
 config ARCH_GUMSTIX
 	bool "Gumstix XScale 255 boards"
 	select PXA25x
arch/arm/mach-pxa/Makefile +1 -0
diff -Nurp linux-2.6.33-rc4/arch/arm/mach-pxa/Makefile linux-embedian-2.6.33-rc4/arch/arm/mach-pxa/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_MACH_ARMCORE)	+= cm-x2xx-pc
 endif
 obj-$(CONFIG_MACH_EM_X270)	+= em-x270.o
 obj-$(CONFIG_MACH_CM_X300)      += cm-x300.o
+obj-$(CONFIG_MACH_CAPC7117)     += capc7117.o mxm8x10.o
 obj-$(CONFIG_ARCH_GUMSTIX)	+= gumstix.o
 obj-$(CONFIG_GUMSTIX_AM200EPD)	+= am200epd.o
 obj-$(CONFIG_GUMSTIX_AM300EPD)	+= am300epd.o
arch/arm/mach-pxa/capc7117.c +157 -0

157 lines not on this page · raw message

arch/arm/mach-pxa/include/mach/mxm8x10.h +21 -0
diff -Nurp linux-2.6.33-rc4/arch/arm/mach-pxa/include/mach/mxm8x10.h linux-embedian-2.6.33-rc4/arch/arm/mach-pxa/include/mach/mxm8x10.h
@@ -0,0 +1,21 @@
+#ifndef __MACH_MXM_8X10_H
+#define __MACH_MXM_8X10_H
+
+#define MXM_8X10_ETH_PHYS	0x13000000
+
+#define NAND_MMIO_START         0x43100000
+#define NAND_MMIO_END           0x43100080
+
+#define EXT_GPIO(x)     (128 + (x))
+
+extern void mxm_8x10_mmc_init(void);
+extern void mxm_8x10_usb_host_init(void);
+extern void mxm_8x10_ac97_init(void);
+extern void mxm_8x10_nand_init(void);
+extern void mxm_8x10_ethernet_init(void);
+extern void mxm_8x10_uart_init(void);
+extern void mxm_8x10_i2c_init(void);
+
+extern void mxm_8x10_barebones_init(void);
+
+#endif /* __MACH_MXM_8X10_H */
arch/arm/mach-pxa/mxm8x10.c +480 -0

480 lines not on this page · raw message

#1 lore raw

PATCH 1/1 ARM: Support for Embedian MXM-8x10 eval kit

Quoted 3 lines · Edwin Peer

I believe we have finally managed to address all your review points - third time's a charm. :)

Here's the diffstat against 2.6.33-rc4:

One more thing that caught my eye.

[...]

arch/arm/mach-pxa/mxm8x10.c line 341

Quoted 8 lines
arch/arm/mach-pxa/mxm8x10.c +8 -0
+void __init mxm_8x10_mmc_init(void)
+{
+	pxa_set_mci_info(&mxm_8x10_mci_platform_data);
+}
+#else
+inline void mxm_8x10_mmc_init(void)
+{
+}

You should make that inline function static.

[...]

arch/arm/mach-pxa/mxm8x10.c line 409

Quoted 8 lines
arch/arm/mach-pxa/mxm8x10.c +8 -0
+void __init mxm_8x10_nand_init(void)
+{
+	pxa3xx_set_nand_info(&mxm_8x10_nand_info);
+}
+#else
+inline mxm_8x10_nand_init(void)
+{
+}

Same here, and it should return void. Without any return type, it will default to int, which is not what you want. And as you're on it, you can also put the whole thing in one line (but that's really nit-picking).

Daniel

#2 lore raw

PATCH 1/1 ARM: Support for Embedian MXM-8x10 eval kit

Hi Daniel,

Thanks for the comments.

Quoted 7 lines · Daniel Mack

arch/arm/mach-pxa/mxm8x10.c line 341

Quoted 4 lines
arch/arm/mach-pxa/mxm8x10.c +3 -0
+inline void mxm_8x10_mmc_init(void)
+{
+}

You should make that inline function static.

I originally wrote it like this because I wanted the option for the machine implementing the MXM-8x10 module to initialise the MMC or not. I'm not sure static is appropriate (as it is exported in the header). Could we perhaps drop the inline and just make it a void __init instead?

Quoted 4 lines

arch/arm/mach-pxa/mxm8x10.c line 409

Quoted 3 lines
arch/arm/mach-pxa/mxm8x10.c +3 -0
+inline mxm_8x10_nand_init(void)
+{
+}

and it should return void.

Agreed. This one must have just slipped by... Too much coffee, not enough sleep :)

Thanks,
Hennie
-------------- next part --------------
A non-text attachment was scrubbed...
Name: hvdmerwe.vcf
Type: text/x-vcard
Size: 361 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100119/4a6797fb/attachment-0001.vcf>

#3 lore raw

PATCH 1/1 ARM: Support for Embedian MXM-8x10 eval kit

Quoted 14 lines · Hennie van der Merwe

Hi Daniel,

Thanks for the comments.

Quoted 8 lines · Daniel Mack

arch/arm/mach-pxa/mxm8x10.c line 341

Quoted 5 lines
arch/arm/mach-pxa/mxm8x10.c +3 -0
+inline void mxm_8x10_mmc_init(void)
+{
+}

You should make that inline function static.

Quoted 5 lines

I originally wrote it like this because I wanted the option for the machine implementing the MXM-8x10 module to initialise the MMC or not. I'm not sure static is appropriate (as it is exported in the header). Could we perhaps drop the inline and just ?make it a void __init instead?

You could make this declaration in .h file, e.g.

#ifdef ...
extern void mxm_8x10_mmc_init(void);
#else
static inline void mxm_8x10_mmc_init(void) {}
#endif

which will be optimized away by the compiler. Having a null global function isn't very interesting.

Quoted 12 lines
Quoted 5 lines

arch/arm/mach-pxa/mxm8x10.c line 409

Quoted 3 lines
arch/arm/mach-pxa/mxm8x10.c +3 -0
+inline mxm_8x10_nand_init(void)
+{
+}

?and it should return void.

Agreed. This one must have just slipped by... Too much coffee, not enough sleep :)

Thanks,
Hennie

#4 lore raw

PATCH 1/1 ARM: Support for Embedian MXM-8x10 eval kit

Quoted 8 lines · Eric Miao

You could make this declaration in .h file, e.g.

#ifdef ...
extern void mxm_8x10_mmc_init(void);
#else
static inline void mxm_8x10_mmc_init(void) {}
#endif
which will be optimized away by the compiler.

Perfect.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: hvdmerwe.vcf
Type: text/x-vcard
Size: 373 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100119/681f2737/attachment.vcf>

author
#5 lore raw

PATCH 1/1 ARM: Support for Embedian MXM-8x10 eval kit

In reply to Hennie van der Merwe
Quoted 4 lines · On Tuesday 19 January 2010 07:15:56 Hennie van der Merwe wrote:

I originally wrote it like this because I wanted the option for the machine implementing the MXM-8x10 module to initialise the MMC or not. I'm not sure static is appropriate (as it is exported in the header). Could we perhaps drop the inline and just make it a void __init instead?

Indeed, those can't be made static. What I have done is make all the functions we call in barebones_init() static and remove their declarations from the header, since those are never optional.

Quoted 2 lines

Agreed. This one must have just slipped by... Too much coffee, not enough sleep :)

Fixed. I've also corrected what I hope are the last formatting issues and fixed one more #include that I somehow missed before.

I'm giving the changes a quick test on the hardware before submitting what I hope will be the final patch.

Regards,
Edwin Peer

#6 lore raw

PATCH 1/1 ARM: Support for Embedian MXM-8x10 eval kit

Hi Edwin,

Quoted 4 lines · Edwin Peer

Indeed, those can't be made static. What I have done is make all the functions we call in barebones_init() static and remove their declarations from the header, since those are never optional.

I don't agree. I think Eric's solution is very elegant without sacrificing flexibility.

Hennie
-------------- next part --------------
A non-text attachment was scrubbed...
Name: hvdmerwe.vcf
Type: text/x-vcard
Size: 373 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100119/f868fc0e/attachment.vcf>