Re: [RFC PATCH 7/7] ARM: davinci: add support for the am1808 based enbw_cmc board

Hello Grant,

Grant Likely wrote:

On Mon, Jan 23, 2012 at 09:56:07AM +0100, Heiko Schocher wrote:

quoted

- AM1808 based board
- 64 MiB DDR ram
- 2 MiB Nor flash
- 128 MiB NAND flash
- use internal RTC
- I2C support
- hwmon lm75 support
- UBI/UBIFS support
- MMC support
- USB OTG support

Signed-off-by: Heiko Schocher <redacted>
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
Cc: davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/@public.gmane.org
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: David Woodhouse <redacted>
Cc: Ben Dooks <redacted>
Cc: Wolfram Sang <redacted>
Cc: Sekhar Nori <redacted>
Cc: Kevin Hilman <redacted>
Cc: Wolfgang Denk <redacted>

---
- post this board support with USB support, even though
  USB is only working with the 10 ms "workaround", posted here:
  http://comments.gmane.org/gmane.linux.usb.general/54505
  I see this issue also on the AM1808 TMDXEXP1808L evalboard.
- MMC and USB are not using OF support yet, ideas how to port
  this are welcome. I need for USB and MMC boards board
  specific callbacks, how to solve this with OF support?

Can you give me a hint, how to add callbacks to OF support?

quoted

 arch/arm/boot/dts/enbw_cmc.dts                  |  286 +++++++++++++++++
 arch/arm/configs/enbw_cmc_defconfig             |  125 ++++++++
 arch/arm/mach-davinci/Kconfig                   |    8 +
 arch/arm/mach-davinci/Makefile                  |    1 +
 arch/arm/mach-davinci/board-enbw-cmc.c          |  384 +++++++++++++++++++++++
 arch/arm/mach-davinci/include/mach/uncompress.h |    1 +
 6 files changed, 805 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/boot/dts/enbw_cmc.dts
 create mode 100644 arch/arm/configs/enbw_cmc_defconfig
 create mode 100644 arch/arm/mach-davinci/board-enbw-cmc.c

diff --git a/arch/arm/boot/dts/enbw_cmc.dts b/arch/arm/boot/dts/enbw_cmc.dts
new file mode 100644
index 0000000..e5995ce
--- /dev/null
+++ b/arch/arm/boot/dts/enbw_cmc.dts

@@ -0,0 +1,286 @@
+/*
+ * Device Tree for the EnBW CMC plattform
+ *
+ * Copyright 2011 DENX Software Engineering GmbH
+ * Heiko Schocher <hs-ynQEQJNshbs@public.gmane.org>
+ *
+ * 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.
+ */
+/dts-v1/;
+/include/ "skeleton.dtsi"
+
+/ {
+	model = "EnBW CMC";
+	compatible = "enbw,cmc";
+
+	aliases {
+		ethernet0 = &eth0;
+	};
+
+	arm {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0 0xfffee000 0x00020000>;
+		intc: interrupt-controller@1 {
+			compatible = "ti,davinci-intc";
+			interrupt-controller;
+			#interrupt-cells = <1>;
+			ti,intc-size = <101>;
+			reg = <0x0 0x2000>;
+		};
+	};
+	soc@1c00000 {
+		compatible = "ti,da8xx";

As previously mentioned, using wildcard ('x') characters in compatible
properties is bad practice.  Always be specific and use the compatible
list to make sure the correct driver gets bound (assuming the device
is 100% register level compatible with the older one).

fixed.

quoted

+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0x0 0x01c00000 0x400000>;
+
+		serial0: serial@1c42000 {
+			cell-index = <2>;

As previously mentioned, don't use cell index.  If you *really* need
to assign a number to a device, then use a property in the /aliases
node and make sure pdev->id does *not* get modified.

removed, as not needed.

quoted

+			compatible = "ns16550a";

Should include a string for the specific device.  Something like:

	compatible = "ti,da800", "ns16550a";

added.

quoted

diff --git a/arch/arm/mach-davinci/board-enbw-cmc.c b/arch/arm/mach-davinci/board-enbw-cmc.c
new file mode 100644
index 0000000..4473c4a
--- /dev/null
+++ b/arch/arm/mach-davinci/board-enbw-cmc.c

@@ -0,0 +1,384 @@

[...]

quoted

+#if defined(CONFIG_OF)
+static __init void enbw_cmc_usb_init(void)

All the focus is on DT now.  How about making CONFIG_OF required for
this board?  Also, this is just another board file.  The focus should

fixed.

be on creating a generic board support file for DT platforms.

move the arch/arm/mach-davinci/board-enbw-cmc.c to
arch/arm/mach-davinci/board-da850_simple.c

and do it like

arch/powerpc/platforms/52xx/mpc5200_simple.c ?

and extract common nodes from arch/arm/boot/dts/enbw_cmc.dts
to arch/arm/boot/dts/da850.dtsi?

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany