base board consolidation

Hello,

for a customer who uses a SoC-module already supported in mainline but a custom base board I'm currently coding the umpteenth baseboard abstraction. So I tried to come up with a generic definition.

I converted only one machine (mx31moboard) for now, possible other users I found are:

	mx31lilly
	mx31lite
	colibri-pxa270

but I wouldn't be surprised if there are more.

Patch 2 keeps the existing kernel parameter for mx31moboard. That can be removed later as is done in patch 3. The latter shouldn't be applied now and is only included to improve the diffstat :-)

At some time this will probably be obsoleted by device tree stuff, the upside of this approach is that it already works today without the need to adapt the bootloader.

Best regards
Uwe

The following changes since commit ca56a95eedcc95f8fea7b49c87565cd961d74fe2:

Merge branch 'drm-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/airlied/drm-2.6 (2011-06-29 18:18:48 -0700)

are available in the git repository at:

git.ptx:/git/ukl/linux-2.6.git baseboardconsolidation

Uwe Kleine-K?nig (3):
ARM: common abstraction for specifying a baseboard
ARM: imx/mx31moboard: convert to new baseboard handling
ARM: imx/mx31moboard: remove obsolete baseboard handling

 Documentation/kernel-parameters.txt                |    2 +
 arch/arm/common/Kconfig                            |    3 +
 arch/arm/common/Makefile                           |    1 +
 arch/arm/common/baseboard.c                        |   58 ++++++++++++++++++++
 arch/arm/include/asm/baseboard.h                   |   31 +++++++++++
 arch/arm/mach-imx/Kconfig                          |    1 +
 arch/arm/mach-imx/mach-mx31moboard.c               |   38 +++++++-------
 arch/arm/mach-imx/mx31moboard-devboard.c           |    5 +-
 arch/arm/mach-imx/mx31moboard-marxbot.c            |    5 +-
 arch/arm/mach-imx/mx31moboard-smartbot.c           |   18 ++----
 arch/arm/plat-mxc/include/mach/board-mx31moboard.h |   16 ++----
 11 files changed, 133 insertions(+), 45 deletions(-)
 create mode 100644 arch/arm/common/baseboard.c
 create mode 100644 arch/arm/include/asm/baseboard.h

Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

This patch unifies handling of baseboards. It allows to use the same parameter name on all boards.

---
 Documentation/kernel-parameters.txt |    2 +
 arch/arm/common/Kconfig             |    3 ++
 arch/arm/common/Makefile            |    1 +
 arch/arm/common/baseboard.c         |   58 +++++++++++++++++++++++++++++++++++
 arch/arm/include/asm/baseboard.h    |   31 ++++++++++++++++++
 5 files changed, 95 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/common/baseboard.c
 create mode 100644 arch/arm/include/asm/baseboard.h

index fd248a31..b349a43 100644
@@ -373,6 +373,8 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 
 	autotest	[IA64]
 
+	baseboard=	[ARM] Select a baseboard
+
 	baycom_epp=	[HW,AX25]
 			Format: <io>,<mode>
 

index 4b71766..c91f7c6 100644
@@ -39,3 +39,6 @@ config SHARP_PARAM
 
 config SHARP_SCOOP
 	bool
+
+config BASEBOARD
+	bool

index 6ea9b6f..4020be3 100644
@@ -17,3 +17,4 @@ obj-$(CONFIG_ARCH_IXP2000)	+= uengine.o
 obj-$(CONFIG_ARCH_IXP23XX)	+= uengine.o
 obj-$(CONFIG_PCI_HOST_ITE8152)  += it8152.o
 obj-$(CONFIG_ARM_TIMER_SP804)	+= timer-sp.o
+obj-$(CONFIG_BASEBOARD)		+= baseboard.o

new file mode 100644
index 0000000..9e49648
@@ -0,0 +1,31 @@
+/*
+ * Copyright (C) 2011 Pengutronix
+ * Uwe Kleine-Koenig <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License version 2 as published by the
+ * Free Software Foundation.
+ */
+#ifndef __ASM_BASEBOARD_H
+#define __ASM_BASEBOARD_H
+
+#include <linux/kernel.h>
+
+/**
+ * struct baseboard_entry
+ *
+ * @name: identifying name used by baseboard_set and baseboard kernel parameter.
+ * @initfunc: function called by baseboard_setup when the corresponding
+ * 	baseboard was selected.
+ * @initdata: callback data for @initfunc.
+ */
+struct baseboard_entry {
+	const char *name;
+	int (*initfunc)(const struct baseboard_entry *);
+	void *initdata;
+};
+
+int baseboard_select(const char *name);
+int baseboard_setup(const struct baseboard_entry *map, size_t mapsize);
+
+#endif /* ifndef __ASM_BASEBOARD_H */

1.7.5.3

The old mx31moboard way to pass the board is still supported but will go away later saving aa few more lines.

---
 arch/arm/mach-imx/Kconfig                          |    1 +
 arch/arm/mach-imx/mach-mx31moboard.c               |   48 +++++++++++++-------
 arch/arm/mach-imx/mx31moboard-devboard.c           |    5 +-
 arch/arm/mach-imx/mx31moboard-marxbot.c            |    5 +-
 arch/arm/mach-imx/mx31moboard-smartbot.c           |   18 +++-----
 arch/arm/plat-mxc/include/mach/board-mx31moboard.h |   16 +++----
 6 files changed, 51 insertions(+), 42 deletions(-)

index 59c97a3..98bb0ca 100644
@@ -463,6 +463,7 @@ config MACH_MX31_3DS_MXC_NAND_USE_BBT
 
 config MACH_MX31MOBOARD
 	bool "Support mx31moboard platforms (EPFL Mobots group)"
+	select BASEBOARD
 	select SOC_IMX31
 	select IMX_HAVE_PLATFORM_FSL_USB2_UDC
 	select IMX_HAVE_PLATFORM_IMX_I2C

index 0aa2536..0fdb79d 100644
@@ -26,6 +26,7 @@
 #include <mach/iomux-mx3.h>
 #include <mach/hardware.h>
 #include <mach/ulpi.h>
+#include <mach/board-mx31moboard.h>
 
 #include "devices-imx31.h"
 

@@ -223,7 +224,7 @@ static const struct fsl_usb2_platform_data usb_pdata __initconst = {
 /*
  * system init for baseboard usage. Will be called by mx31moboard init.
  */
-void __init mx31moboard_devboard_init(void)
+int __init mx31moboard_devboard_init(const struct baseboard_entry *unused)
 {
 	printk(KERN_INFO "Initializing mx31devboard peripherals\n");
 

@@ -238,5 +239,5 @@ void __init mx31moboard_devboard_init(void)
 
 	imx31_add_fsl_usb2_udc(&usb_pdata);
 
-	devboard_usbh1_init();
+	return devboard_usbh1_init();
 }

index bb639cb..ed943a1 100644
@@ -28,6 +28,7 @@
 #include <mach/hardware.h>
 #include <mach/iomux-mx3.h>
 #include <mach/ulpi.h>
+#include <mach/board-mx31moboard.h>
 
 #include <media/soc_camera.h>
 

@@ -336,7 +337,7 @@ static const struct fsl_usb2_platform_data usb_pdata __initconst = {
 /*
  * system init for baseboard usage. Will be called by mx31moboard init.
  */
-void __init mx31moboard_marxbot_init(void)
+int __init mx31moboard_marxbot_init(const struct baseboard_entry *unused)
 {
 	printk(KERN_INFO "Initializing mx31marxbot peripherals\n");
 

@@ -362,5 +363,5 @@ void __init mx31moboard_marxbot_init(void)
 
 	imx31_add_fsl_usb2_udc(&usb_pdata);
 
-	marxbot_usbh1_init();
+	return marxbot_usbh1_init();
 }

index fabb801..9afc630 100644
@@ -181,7 +181,7 @@ static void smartbot_resets_init(void)
 /*
  * system init for baseboard usage. Will be called by mx31moboard init.
  */
-void __init mx31moboard_smartbot_init(int board)
+int __init mx31moboard_smartbot_init(const struct baseboard_entry *entry)
 {
 	printk(KERN_INFO "Initializing mx31smartbot peripherals\n");
 

@@ -190,20 +190,14 @@ void __init mx31moboard_smartbot_init(int board)
 
 	imx31_add_imx_uart1(&uart_pdata);
 
-	switch (board) {
-	case MX31SMARTBOT:
-		imx31_add_fsl_usb2_udc(&usb_pdata);
-		break;
-	case MX31EYEBOT:
+	if ((unsigned long)entry->initdata & MX31MOBOARD_SMARTBOT_USB_OTG)
 		smartbot_otg_host_init();
-		break;
-	default:
-		printk(KERN_WARNING "Unknown board %d, USB OTG not initialized",
-			board);
-	}
+	else
+		imx31_add_fsl_usb2_udc(&usb_pdata);
 
 	smartbot_resets_init();
 
 	smartbot_cam_init();
-	platform_add_devices(smartbot_cameras, ARRAY_SIZE(smartbot_cameras));
+	return platform_add_devices(smartbot_cameras,
+			ARRAY_SIZE(smartbot_cameras));
 }

index de14543..8d33d47 100644
@@ -19,24 +19,20 @@
 #ifndef __ASM_ARCH_MXC_BOARD_MX31MOBOARD_H__
 #define __ASM_ARCH_MXC_BOARD_MX31MOBOARD_H__
 
+#include <asm/baseboard.h>
+
 #ifndef __ASSEMBLY__
 
-enum mx31moboard_boards {
-	MX31NOBOARD	= 0,
-	MX31DEVBOARD	= 1,
-	MX31MARXBOT	= 2,
-	MX31SMARTBOT	= 3,
-	MX31EYEBOT	= 4,
-};
+#define MX31MOBOARD_SMARTBOT_USB_OTG	1UL
 
 /*
  * This CPU module needs a baseboard to work. After basic initializing
  * its own devices, it calls the baseboard's init function.
  */
 
-extern void mx31moboard_devboard_init(void);
-extern void mx31moboard_marxbot_init(void);
-extern void mx31moboard_smartbot_init(int board);
+int mx31moboard_devboard_init(const struct baseboard_entry *);
+int mx31moboard_marxbot_init(const struct baseboard_entry *);
+int mx31moboard_smartbot_init(const struct baseboard_entry *);
 
 #endif
 

1.7.5.3

---
 arch/arm/mach-imx/mach-mx31moboard.c |   16 ----------------
 1 files changed, 0 insertions(+), 16 deletions(-)

index 802595b..6681de4 100644
@@ -517,16 +517,6 @@ static const struct baseboard_entry
 	},
 };
 
-static const char *baseboardnames[] = {
-	"",
-	"devboard",
-	"marxbot",
-	"smartbot",
-	"eyebot",
-};
-static int mx31moboard_baseboard;
-core_param(mx31moboard_baseboard, mx31moboard_baseboard, int, 0444);
-
 /*
  * Board specific initialization.
  */

@@ -559,12 +549,6 @@ static void __init mx31moboard_init(void)
 
 	moboard_usbh2_init();
 
-	if (mx31moboard_baseboard > 0 &&
-			mx31moboard_baseboard < ARRAY_SIZE(baseboardnames)) {
-		pr_warn("mx31moboard_baseboard=<id> is deprecated, use baseboard=<fullname>\n");
-		baseboard_select(baseboardnames[mx31moboard_baseboard]);
-	}
-
 	baseboard_setup(mx31moboard_baseboard_map,
 			ARRAY_SIZE(mx31moboard_baseboard_map));
 }

1.7.5.3

Hi

(dropping Valentin in CC, as he is not working at epfl anymore)

I would like to keep the mx31moboard parameter for compatibility reasons as some board are outside of epfl and are upgraded remotly without access to the bootloader. But I can keep the compatibility stuff in my tree if needed.

Yeah, I don't see much the point defining now a new common abstraction that will be obsoleted by the device tree.

Thanks,

Philippe

Philippe R?tornaz
EPFL - STI - IMT - LSRO1
bureau MEB330, Station 9
CH-1015 Lausanne, Switzerland

tel: +41 21 693 78 42
fax: +41 21 693 78 07
http://lsro.epfl.ch

The other issue is that the reverse is also true - platforms can be a common baseboard, and its the CPU tile which changes. That's standard for the ARM development boards.

While the CPU tiles for platforms before Versatile Express had very little variance other than the actual CPU, Versatile Express can have dramatically different physical address maps depending on the CPU tile fitted. The only real common thing is the offsets for peripherals within each chip select region.

As also said, all this should be solved by DT, so I don't see there's much point in creating this abstraction just to have it obsoleted shortly thereafter. Much better to put that effort into ensuring that DT can handle these setups...

Yes, I would also much more like to see these boards being supported by DT, and eventually, we can probably provide static tree definitions for these boards and get rid of the whole code entirely.

Daniel

While some of that can be done, I think the idea of getting rid of all the code is a pipedream - things like detecting the LCD panel is something which has to be in code.

Having separate DT blobs because you happen to have connected a different LCD display is not sane when you can detect the LCD type at run time - from either the support perspective or the maintainence perspective.

There's also a similar issue where the ethernet controller may be of two different types, which can only be detected at runtime - and again having different DT blobs would be a nightmare.

With that alone, you're talking about 10 different DT blobs (5 different CLCD panel configs + 2 ethernet configs) for just one Realview board.

There's also the matter of the platform specific registers setting the clock speed for the SP804 timers.

So no, I don't think that we'll ever get rid of all the code for ARMs development platforms without losing functionality/utility, but we should be able to get rid of quite an amount like I've already done by consolidating across the entire set with plat-versatile.

Which leads me to a question (likely quite off-topic here). Working on a pxa- device tree support, there're a lot of procedures called during init doing exactly this -- how do you handle this via device tree ? Is there any way to call them ? Or does the kernel need to patch device tree on run-time ? Or how?

Thanks

That is _precisely_ my concern which caused me to be unconvinced that DT actually solves the problem which Linus is concerned about.

What I see is that DT solves part of the problem, but we're still going to end up having board files to deal with procedural stuff which can't be encoded in DT.

That in turn leads to the problem of how to bind the board specific code in board files into DT declared devices (that information will likely be Linux specific and may not be stable between kernel versions, so would probably require kernel version specific DTs).

I think others (eg Nico) share that view.

If that is how things will turn out, then we've got a far bigger problem to deal with than that which we started with - and at that point it may be far easier maintainability wise to fork the kernel from the pre-DT stage and continue as we have been.

The only reason that I've said yes to DT is through pressure from multiple directions (including Linus), and I'm willing to see whether it _can_ work. However, I remain entirely unconvinced, even at this stage.

If it was up to me, then I'd still have DT out of the kernel tree, pending some real platforms with the issue you raised having been converted to it so we can see how these problems are addressed.