[PATCH] ARM:MXS: Initial support for the Crystalfontz CFA-10036

[PATCH] ARM:MXS: Initial support for the Crystalfontz CFA-10036

[Maxime Ripard]

Hi everyone,

This is an attempt for a initial support for the Crystalfontz CFA
10036 module.

The module basically has a iMX28 SoC, plus a micro-SD slot, 3 pins to
access the DUART, a USB OTG port, a OLED screen using the Solomon
SSD1307 controller over I2C and a 200-pins SODIMM port to plug an
expansion board.

Support for this board and the additional devices will come
eventually.

For now, only the DUART and the MMC controller are used.

On the MMC part though, Something odd is going on.  On the first
patches I made against 3.4, everything was working fine. With 3.5-rc2
though, the MMC never replies to the first CMD52 command sent to
it. The pin muxing hasn't changed and has been checked, the card is
well detected through the card detect pin, the power enable pin is
correctly set to a low level, and so the SD card is correctly
powered. The MMC works fine with the exact same kernel on the
imx28-evk though.

What seems odd to me, is that when booting the board with the 3.4
kernel, at boot, the mxs-mmc driver reports a SSP clock of 288MHz,
while on 3.5, it reports a 480MHz SSP clock. By looking at the clk
driver for the iMX28, there is a comment saying that the 480MHz clock
seems to high for SD controlers, and that it should be 288MHz
instead. Given that the clock part has changed quite a lot from 3.4 to
3.5, I think it might be a good lead, but hardcoding the SSP clock to
288MHz in the mxs-mmc driver instead of using the clk_get_rate
function does not resolve the problem.

Also, this problem is seen with both DT-probing and traditionnal
probing.

Do someone have an idea on what could possibly happen here ?

Thanks,
Maxime

[PATCH 1/2] Early support for the Crystalfontz CFA10036 module

[Maxime Ripard]

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Cc: Brian Lily <brian@crystalfontz.com>
---
 arch/arm/mach-mxs/Kconfig         |   15 +++++
 arch/arm/mach-mxs/Makefile        |    1 +
 arch/arm/mach-mxs/mach-cfa10036.c |  129 +++++++++++++++++++++++++++++++++++++
 3 files changed, 145 insertions(+)
 create mode 100644 arch/arm/mach-mxs/mach-cfa10036.c

diff --git a/arch/arm/mach-mxs/Kconfig b/arch/arm/mach-mxs/Kconfig
index 91cf062..54ace90 100644
--- a/arch/arm/mach-mxs/Kconfig
+++ b/arch/arm/mach-mxs/Kconfig
@@ -70,6 +70,21 @@ config MACH_MX28EVK
 	  Include support for MX28EVK platform. This includes specific
 	  configurations for the board and its peripherals.
 
+config MACH_CFA10036
+	bool "Support Crystalfontz CFA-10036 Module"
+	select SOC_IMX28
+	select NEW_LEDS
+	select LEDS_CLASS
+	select LEDS_GPIO
+	select LEDS_GPIO_REGISTER
+	select MXS_HAVE_AMBA_DUART
+	select MXS_HAVE_PLATFORM_MXS_MMC
+	select MXS_HAVE_PLATFORM_MXS_I2C
+	select MXS_HAVE_PLATFORM_RTC_STMP3XXX
+	help
+	  Include support for CFA10036 module. This includes specific
+	  configurations for the board and its peripherals.
+
 config MODULE_TX28
 	bool
 	select SOC_IMX28
diff --git a/arch/arm/mach-mxs/Makefile b/arch/arm/mach-mxs/Makefile
index e41590c..36633b9 100644
--- a/arch/arm/mach-mxs/Makefile
+++ b/arch/arm/mach-mxs/Makefile
@@ -12,5 +12,6 @@ obj-$(CONFIG_MACH_M28EVK)    += mach-m28evk.o
 obj-$(CONFIG_MACH_APX4DEVKIT) += mach-apx4devkit.o
 obj-$(CONFIG_MODULE_TX28) += module-tx28.o
 obj-$(CONFIG_MACH_TX28)    += mach-tx28.o
+obj-$(CONFIG_MACH_CFA10036) += mach-cfa10036.o
 
 obj-y += devices/
diff --git a/arch/arm/mach-mxs/mach-cfa10036.c b/arch/arm/mach-mxs/mach-cfa10036.c
new file mode 100644
index 0000000..2cea113
--- /dev/null
+++ b/arch/arm/mach-mxs/mach-cfa10036.c
@@ -0,0 +1,129 @@
+/*
+ * Copyright 2012, Free Electrons. All Rights Reserved.
+ *
+ * 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.
+ */
+
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/gpio.h>
+#include <linux/leds.h>
+#include <linux/clk.h>
+#include <linux/i2c.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/fixed.h>
+
+#include <asm/mach-types.h>
+#include <asm/mach/arch.h>
+#include <asm/mach/time.h>
+
+#include <mach/common.h>
+#include <mach/iomux-mx28.h>
+#include <mach/digctl.h>
+
+#include "devices-mx28.h"
+
+#define CFA10036_GPIO_PWR_LED		MXS_GPIO_NR(3, 4)
+#define CFA10036_MMC0_SLOT_POWER	MXS_GPIO_NR(3, 28)
+
+static const iomux_cfg_t cfa10036_pads[] __initconst = {
+	/* duart */
+	MXS_IOMUX_PAD_NAKED(3, 2, PAD_MUXSEL_2) | MXS_PAD_CTRL,
+	MXS_IOMUX_PAD_NAKED(3, 3, PAD_MUXSEL_2) | MXS_PAD_CTRL,
+
+	/* mmc0 */
+	MX28_PAD_SSP0_DATA0__SSP0_D0 |
+		(MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+	MX28_PAD_SSP0_DATA1__SSP0_D1 |
+		(MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+	MX28_PAD_SSP0_DATA2__SSP0_D2 |
+		(MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+	MX28_PAD_SSP0_DATA3__SSP0_D3 |
+		(MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+	MX28_PAD_SSP0_CMD__SSP0_CMD |
+		(MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+	MX28_PAD_SSP0_DETECT__SSP0_CARD_DETECT |
+		(MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_NOPULL),
+	MX28_PAD_SSP0_SCK__SSP0_SCK |
+		(MXS_PAD_12MA | MXS_PAD_3V3 | MXS_PAD_NOPULL),
+	/* slot power enable */
+	MX28_PAD_PWM3__GPIO_3_28 |
+		(MXS_PAD_4MA | MXS_PAD_3V3 | MXS_PAD_NOPULL),
+
+	/* Power Led */
+	MXS_IOMUX_PAD_NAKED(3, 4, PAD_MUXSEL_GPIO) | MXS_PAD_CTRL,
+};
+
+/* led */
+static const struct gpio_led cfa10036_leds[] __initconst = {
+	{
+		.name = "power-led",
+		.default_state = LEDS_GPIO_DEFSTATE_ON,
+		.gpio = CFA10036_GPIO_PWR_LED,
+		.active_low = true,
+	},
+};
+
+static const struct gpio_led_platform_data cfa10036_led_data __initconst = {
+	.leds = cfa10036_leds,
+	.num_leds = ARRAY_SIZE(cfa10036_leds),
+};
+
+/* mmc */
+static struct mxs_mmc_platform_data cfa10036_mmc_pdata[] __initdata = {
+	{
+		/* mmc0 */
+		.wp_gpio = -EINVAL,
+		.flags = SLOTF_4_BIT_CAPABLE,
+	},
+};
+
+static const struct gpio cfa10036_gpios[] __initconst = {
+	{ CFA10036_MMC0_SLOT_POWER, GPIOF_OUT_INIT_LOW, "mmc0-slot-power" },
+};
+
+static void __init cfa10036_init(void)
+{
+	int ret;
+
+	mx28_soc_init();
+
+	mxs_iomux_setup_multiple_pads(cfa10036_pads, ARRAY_SIZE(cfa10036_pads));
+
+	mx28_add_duart();
+
+	ret = gpio_request_array(cfa10036_gpios, ARRAY_SIZE(cfa10036_gpios));
+	if (ret)
+		pr_err("One or more GPIOs failed to be requested: %d\n", ret);
+
+	mx28_add_mxs_mmc(0, &cfa10036_mmc_pdata[0]);
+
+	mx28_add_rtc_stmp3xxx();
+
+	gpio_led_register_device(0, &cfa10036_led_data);
+}
+
+static void __init cfa10036_timer_init(void)
+{
+	mx28_clocks_init();
+}
+
+static struct sys_timer cfa10036_timer = {
+	.init	= cfa10036_timer_init,
+};
+
+MACHINE_START(CFA10036, "Crystalfontz CFA-10036")
+	.map_io		= mx28_map_io,
+	.init_irq	= mx28_init_irq,
+	.timer		= &cfa10036_timer,
+	.init_machine	= cfa10036_init,
+	.restart	= mxs_restart,
+MACHINE_END
-- 
1.7.9.5

[PATCH 2/2] ARM: MXS: Add Crystalfontz CFA-10036 DTS

[Maxime Ripard]

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Cc: Brian Lily <brian@crystalfontz.com>
---
 arch/arm/boot/dts/cfa10036.dts |   66 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)
 create mode 100644 arch/arm/boot/dts/cfa10036.dts

diff --git a/arch/arm/boot/dts/cfa10036.dts b/arch/arm/boot/dts/cfa10036.dts
new file mode 100644
index 0000000..ceae960
--- /dev/null
+++ b/arch/arm/boot/dts/cfa10036.dts
@@ -0,0 +1,66 @@
+/*
+ * Copyright 2012 Free Electrons
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+/dts-v1/;
+/include/ "imx28.dtsi"
+
+/ {
+	model = "Crystalfontz CFA-10036 Board";
+	compatible = "crystalfontz,cfa10036", "fsl,imx28-evk", "fsl,imx28";
+
+	memory {
+		reg = <0x40000000 0x08000000>;
+	};
+
+	apb at 80000000 {
+		apbh at 80000000 {
+			ssp0: ssp at 80010000 {
+				compatible = "fsl,imx28-mmc";
+				pinctrl-names = "default";
+				pinctrl-0 = <&mmc0_4bit_pins_a
+					&mmc0_cd_cfg &mmc0_sck_cfg>;
+				bus-width = <4>;
+				status = "okay";
+			};
+			pinctrl at 80018000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "fsl,imx28-pinctrl", "simple-bus";
+				reg = <0x80018000 2000>;
+
+				mmc0_4bit_pins_a: mmc0-4bit at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <0x2000 0x2010 0x2020
+						0x2030 0x2080 0x2090 0x20a0>;
+					fsl,drive-strength = <1>;
+					fsl,voltage = <1>;
+					fsl,pull-up = <1>;
+				};
+
+				duart_pins_a: duart at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <0x3022 0x3032>;
+					fsl,drive-strength = <0>;
+					fsl,voltage = <1>;
+					fsl,pull-up = <0>;
+				};
+			};
+		};
+
+		apbx at 80040000 {
+			duart: serial at 80074000 {
+				pinctrl-names = "default";
+				pinctrl-0 = <&duart_pins_a>;
+				status = "okay";
+			};
+		};
+	};
+};
-- 
1.7.9.5

[PATCH 1/2] Early support for the Crystalfontz CFA10036 module

[Marek Vasut]

Dear Maxime Ripard,

> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> 
> Cc: Brian Lily <brian@crystalfontz.com>
> ---
>  arch/arm/mach-mxs/Kconfig         |   15 +++++
>  arch/arm/mach-mxs/Makefile        |    1 +
>  arch/arm/mach-mxs/mach-cfa10036.c |  129
> +++++++++++++++++++++++++++++++++++++ 3 files changed, 145 insertions(+)
>  create mode 100644 arch/arm/mach-mxs/mach-cfa10036.c

Well before someone will tear your ass all the way to the spatulae for this one, 
I'll try explaining :)

Basically, everyone is trying as hard as possibly (sometimes excessively in my 
opinion, so don't run away if you're asked for V2, V3 ... of the patch) to 
convert mxs to device tree. Therefore we want no additions of the kind of mach-
cfa10036.c .

Your module actually does look very simple, therefore writing the DT file will 
be quite easy:

1) Check arch/arm/boot/dts/imx28.dtsi and copy arch/arm/boot/dts/imx28-evk.dts 
to arch/arm/boot/dts/imx28-cfa10036.dts .
2) Edit arch/arm/boot/dts/imx28-cfa10036.dts to fit your needs (basically, 
configure GPIOs as you see fit, enable your peripherals etc.). Note you should 
avoid as much possible duplication from the imx28.dtsi file as possible .
3) Compile it:
make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- imx28-cfa10036.dtb uImage
4) Pick it up:
arch/arm/boot/uImage
arch/arm/boot/imx28-cfa10036.dtb
5) Boot it
bootm <uImage addr> <initrd addr or "-" if none> <dtb addr>

Don't hesitate to ask in any case!

> 
> diff --git a/arch/arm/mach-mxs/Kconfig b/arch/arm/mach-mxs/Kconfig
> index 91cf062..54ace90 100644
> --- a/arch/arm/mach-mxs/Kconfig
> +++ b/arch/arm/mach-mxs/Kconfig
> @@ -70,6 +70,21 @@ config MACH_MX28EVK
>  	  Include support for MX28EVK platform. This includes specific
>  	  configurations for the board and its peripherals.
> 
> +config MACH_CFA10036
> +	bool "Support Crystalfontz CFA-10036 Module"
> +	select SOC_IMX28
> +	select NEW_LEDS
> +	select LEDS_CLASS
> +	select LEDS_GPIO
> +	select LEDS_GPIO_REGISTER
> +	select MXS_HAVE_AMBA_DUART
> +	select MXS_HAVE_PLATFORM_MXS_MMC
> +	select MXS_HAVE_PLATFORM_MXS_I2C
> +	select MXS_HAVE_PLATFORM_RTC_STMP3XXX
> +	help
> +	  Include support for CFA10036 module. This includes specific
> +	  configurations for the board and its peripherals.
> +
>  config MODULE_TX28
>  	bool
>  	select SOC_IMX28
> diff --git a/arch/arm/mach-mxs/Makefile b/arch/arm/mach-mxs/Makefile
> index e41590c..36633b9 100644
> --- a/arch/arm/mach-mxs/Makefile
> +++ b/arch/arm/mach-mxs/Makefile
> @@ -12,5 +12,6 @@ obj-$(CONFIG_MACH_M28EVK)    += mach-m28evk.o
>  obj-$(CONFIG_MACH_APX4DEVKIT) += mach-apx4devkit.o
>  obj-$(CONFIG_MODULE_TX28) += module-tx28.o
>  obj-$(CONFIG_MACH_TX28)    += mach-tx28.o
> +obj-$(CONFIG_MACH_CFA10036) += mach-cfa10036.o
> 
>  obj-y += devices/
> diff --git a/arch/arm/mach-mxs/mach-cfa10036.c
> b/arch/arm/mach-mxs/mach-cfa10036.c new file mode 100644
> index 0000000..2cea113
> --- /dev/null
> +++ b/arch/arm/mach-mxs/mach-cfa10036.c
> @@ -0,0 +1,129 @@
> +/*
> + * Copyright 2012, Free Electrons. All Rights Reserved.
> + *
> + * 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.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/platform_device.h>
> +#include <linux/gpio.h>
> +#include <linux/leds.h>
> +#include <linux/clk.h>
> +#include <linux/i2c.h>
> +#include <linux/regulator/machine.h>
> +#include <linux/regulator/fixed.h>
> +
> +#include <asm/mach-types.h>
> +#include <asm/mach/arch.h>
> +#include <asm/mach/time.h>
> +
> +#include <mach/common.h>
> +#include <mach/iomux-mx28.h>
> +#include <mach/digctl.h>
> +
> +#include "devices-mx28.h"
> +
> +#define CFA10036_GPIO_PWR_LED		MXS_GPIO_NR(3, 4)
> +#define CFA10036_MMC0_SLOT_POWER	MXS_GPIO_NR(3, 28)
> +
> +static const iomux_cfg_t cfa10036_pads[] __initconst = {
> +	/* duart */
> +	MXS_IOMUX_PAD_NAKED(3, 2, PAD_MUXSEL_2) | MXS_PAD_CTRL,
> +	MXS_IOMUX_PAD_NAKED(3, 3, PAD_MUXSEL_2) | MXS_PAD_CTRL,
> +
> +	/* mmc0 */
> +	MX28_PAD_SSP0_DATA0__SSP0_D0 |
> +		(MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
> +	MX28_PAD_SSP0_DATA1__SSP0_D1 |
> +		(MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
> +	MX28_PAD_SSP0_DATA2__SSP0_D2 |
> +		(MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
> +	MX28_PAD_SSP0_DATA3__SSP0_D3 |
> +		(MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
> +	MX28_PAD_SSP0_CMD__SSP0_CMD |
> +		(MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
> +	MX28_PAD_SSP0_DETECT__SSP0_CARD_DETECT |
> +		(MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_NOPULL),
> +	MX28_PAD_SSP0_SCK__SSP0_SCK |
> +		(MXS_PAD_12MA | MXS_PAD_3V3 | MXS_PAD_NOPULL),
> +	/* slot power enable */
> +	MX28_PAD_PWM3__GPIO_3_28 |
> +		(MXS_PAD_4MA | MXS_PAD_3V3 | MXS_PAD_NOPULL),
> +
> +	/* Power Led */
> +	MXS_IOMUX_PAD_NAKED(3, 4, PAD_MUXSEL_GPIO) | MXS_PAD_CTRL,
> +};
> +
> +/* led */
> +static const struct gpio_led cfa10036_leds[] __initconst = {
> +	{
> +		.name = "power-led",
> +		.default_state = LEDS_GPIO_DEFSTATE_ON,
> +		.gpio = CFA10036_GPIO_PWR_LED,
> +		.active_low = true,
> +	},
> +};
> +
> +static const struct gpio_led_platform_data cfa10036_led_data __initconst =
> { +	.leds = cfa10036_leds,
> +	.num_leds = ARRAY_SIZE(cfa10036_leds),
> +};
> +
> +/* mmc */
> +static struct mxs_mmc_platform_data cfa10036_mmc_pdata[] __initdata = {
> +	{
> +		/* mmc0 */
> +		.wp_gpio = -EINVAL,
> +		.flags = SLOTF_4_BIT_CAPABLE,
> +	},
> +};
> +
> +static const struct gpio cfa10036_gpios[] __initconst = {
> +	{ CFA10036_MMC0_SLOT_POWER, GPIOF_OUT_INIT_LOW, "mmc0-slot-power" },
> +};
> +
> +static void __init cfa10036_init(void)
> +{
> +	int ret;
> +
> +	mx28_soc_init();
> +
> +	mxs_iomux_setup_multiple_pads(cfa10036_pads, ARRAY_SIZE(cfa10036_pads));
> +
> +	mx28_add_duart();
> +
> +	ret = gpio_request_array(cfa10036_gpios, ARRAY_SIZE(cfa10036_gpios));
> +	if (ret)
> +		pr_err("One or more GPIOs failed to be requested: %d\n", ret);
> +
> +	mx28_add_mxs_mmc(0, &cfa10036_mmc_pdata[0]);
> +
> +	mx28_add_rtc_stmp3xxx();
> +
> +	gpio_led_register_device(0, &cfa10036_led_data);
> +}
> +
> +static void __init cfa10036_timer_init(void)
> +{
> +	mx28_clocks_init();
> +}
> +
> +static struct sys_timer cfa10036_timer = {
> +	.init	= cfa10036_timer_init,
> +};
> +
> +MACHINE_START(CFA10036, "Crystalfontz CFA-10036")
> +	.map_io		= mx28_map_io,
> +	.init_irq	= mx28_init_irq,
> +	.timer		= &cfa10036_timer,
> +	.init_machine	= cfa10036_init,
> +	.restart	= mxs_restart,
> +MACHINE_END

[PATCH 2/2] ARM: MXS: Add Crystalfontz CFA-10036 DTS

[Marek Vasut]

Dear Maxime Ripard,

> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> 
> Cc: Brian Lily <brian@crystalfontz.com>

But then, wait ... why did you add the mach-cfa10036.c entry?

> ---
>  arch/arm/boot/dts/cfa10036.dts |   66
> ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+)
>  create mode 100644 arch/arm/boot/dts/cfa10036.dts
> 
> diff --git a/arch/arm/boot/dts/cfa10036.dts
> b/arch/arm/boot/dts/cfa10036.dts new file mode 100644
> index 0000000..ceae960
> --- /dev/null
> +++ b/arch/arm/boot/dts/cfa10036.dts
> @@ -0,0 +1,66 @@
> +/*
> + * Copyright 2012 Free Electrons
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +/dts-v1/;
> +/include/ "imx28.dtsi"
> +
> +/ {
> +	model = "Crystalfontz CFA-10036 Board";
> +	compatible = "crystalfontz,cfa10036", "fsl,imx28-evk", "fsl,imx28";
> +
> +	memory {
> +		reg = <0x40000000 0x08000000>;
> +	};
> +
> +	apb at 80000000 {
> +		apbh at 80000000 {
> +			ssp0: ssp at 80010000 {
> +				compatible = "fsl,imx28-mmc";
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&mmc0_4bit_pins_a
> +					&mmc0_cd_cfg &mmc0_sck_cfg>;
> +				bus-width = <4>;
> +				status = "okay";
> +			};
> +			pinctrl at 80018000 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				compatible = "fsl,imx28-pinctrl", "simple-bus";
> +				reg = <0x80018000 2000>;
> +
> +				mmc0_4bit_pins_a: mmc0-4bit at 0 {
> +					reg = <0>;
> +					fsl,pinmux-ids = <0x2000 0x2010 0x2020
> +						0x2030 0x2080 0x2090 0x20a0>;
> +					fsl,drive-strength = <1>;
> +					fsl,voltage = <1>;
> +					fsl,pull-up = <1>;

Do you need this copy here?

> +				};
> +
> +				duart_pins_a: duart at 0 {
> +					reg = <0>;
> +					fsl,pinmux-ids = <0x3022 0x3032>;
> +					fsl,drive-strength = <0>;
> +					fsl,voltage = <1>;
> +					fsl,pull-up = <0>;

And this?

> +				};
> +			};
> +		};
> +
> +		apbx at 80040000 {
> +			duart: serial at 80074000 {
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&duart_pins_a>;
> +				status = "okay";
> +			};
> +		};
> +	};
> +};

Best regards,
Marek Vasut

[PATCH 2/2] ARM: MXS: Add Crystalfontz CFA-10036 DTS

[Maxime Ripard]

Hi Marek,

Le 21/06/2012 11:49, Marek Vasut a ?crit :
> Dear Maxime Ripard,
> 
>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>>
>> Cc: Brian Lily <brian@crystalfontz.com>
> 
> But then, wait ... why did you add the mach-cfa10036.c entry?

As I stated in my introduction mail, I first did the job on 3.4 before
forward porting it to 3.5. And since the MMC part worked on 3.4 and no
longer on 3.5, I wanted to keep an easily trackable file between these
two versions, which is not the case with DT.

So partly historical, partly debugging reasons :)

Anyway, I totally understand your point, I'll remove the board file for
the next postings.

>> ---
>>  arch/arm/boot/dts/cfa10036.dts |   66
>> ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+)
>>  create mode 100644 arch/arm/boot/dts/cfa10036.dts
>>
>> diff --git a/arch/arm/boot/dts/cfa10036.dts
>> b/arch/arm/boot/dts/cfa10036.dts new file mode 100644
>> index 0000000..ceae960
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/cfa10036.dts
>> @@ -0,0 +1,66 @@
>> +/*
>> + * Copyright 2012 Free Electrons
>> + *
>> + * The code contained herein is licensed under the GNU General Public
>> + * License. You may obtain a copy of the GNU General Public License
>> + * Version 2 or later at the following locations:
>> + *
>> + * http://www.opensource.org/licenses/gpl-license.html
>> + * http://www.gnu.org/copyleft/gpl.html
>> + */
>> +
>> +/dts-v1/;
>> +/include/ "imx28.dtsi"
>> +
>> +/ {
>> +	model = "Crystalfontz CFA-10036 Board";
>> +	compatible = "crystalfontz,cfa10036", "fsl,imx28-evk", "fsl,imx28";
>> +
>> +	memory {
>> +		reg = <0x40000000 0x08000000>;
>> +	};
>> +
>> +	apb at 80000000 {
>> +		apbh at 80000000 {
>> +			ssp0: ssp at 80010000 {
>> +				compatible = "fsl,imx28-mmc";
>> +				pinctrl-names = "default";
>> +				pinctrl-0 = <&mmc0_4bit_pins_a
>> +					&mmc0_cd_cfg &mmc0_sck_cfg>;
>> +				bus-width = <4>;
>> +				status = "okay";
>> +			};
>> +			pinctrl at 80018000 {
>> +				#address-cells = <1>;
>> +				#size-cells = <0>;
>> +				compatible = "fsl,imx28-pinctrl", "simple-bus";
>> +				reg = <0x80018000 2000>;
>> +
>> +				mmc0_4bit_pins_a: mmc0-4bit at 0 {
>> +					reg = <0>;
>> +					fsl,pinmux-ids = <0x2000 0x2010 0x2020
>> +						0x2030 0x2080 0x2090 0x20a0>;
>> +					fsl,drive-strength = <1>;
>> +					fsl,voltage = <1>;
>> +					fsl,pull-up = <1>;
> 
> Do you need this copy here?

This is not a direct copy, as there is only 4 pins for the MMC bus on
this board, and one of the pins used for the additional bus pins on the
imx28evk is used to power the OLED screen on this one, so I don't want
all of the imx28 pins allocated for the mmc0 to be muxed as such.

Anyway, I'm not very familiar with pinctrl yet, so I might be missing
something.

>> +				};
>> +
>> +				duart_pins_a: duart at 0 {
>> +					reg = <0>;
>> +					fsl,pinmux-ids = <0x3022 0x3032>;
>> +					fsl,drive-strength = <0>;
>> +					fsl,voltage = <1>;
>> +					fsl,pull-up = <0>;
> 
> And this?

The pins to be muxed for the DUART differs from the imx28 here as well.
On the iMX28EVK, the pins used for the DUART are 3-16 and 3-17, while we
use the 3-2 and 3-3 on this board.

> 
>> +				};
>> +			};
>> +		};
>> +
>> +		apbx at 80040000 {
>> +			duart: serial at 80074000 {
>> +				pinctrl-names = "default";
>> +				pinctrl-0 = <&duart_pins_a>;
>> +				status = "okay";
>> +			};
>> +		};
>> +	};
>> +};

Thanks!

Maxime


-- 
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 2/2] ARM: MXS: Add Crystalfontz CFA-10036 DTS

[Marek Vasut]

Dear Maxime Ripard,

> Hi Marek,
> 
> Le 21/06/2012 11:49, Marek Vasut a ?crit :
> > Dear Maxime Ripard,
> > 
> >> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >> 
> >> Cc: Brian Lily <brian@crystalfontz.com>
> > 
> > But then, wait ... why did you add the mach-cfa10036.c entry?
> 
> As I stated in my introduction mail, I first did the job on 3.4 before
> forward porting it to 3.5. And since the MMC part worked on 3.4 and no
> longer on 3.5, I wanted to keep an easily trackable file between these
> two versions, which is not the case with DT.
> 
> So partly historical, partly debugging reasons :)
> 
> Anyway, I totally understand your point, I'll remove the board file for
> the next postings.
> 
> >> ---
> >> 
> >>  arch/arm/boot/dts/cfa10036.dts |   66
> >> 
> >> ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66
> >> insertions(+)
> >> 
> >>  create mode 100644 arch/arm/boot/dts/cfa10036.dts
> >> 
> >> diff --git a/arch/arm/boot/dts/cfa10036.dts
> >> b/arch/arm/boot/dts/cfa10036.dts new file mode 100644
> >> index 0000000..ceae960
> >> --- /dev/null
> >> +++ b/arch/arm/boot/dts/cfa10036.dts
> >> @@ -0,0 +1,66 @@
> >> +/*
> >> + * Copyright 2012 Free Electrons
> >> + *
> >> + * The code contained herein is licensed under the GNU General Public
> >> + * License. You may obtain a copy of the GNU General Public License
> >> + * Version 2 or later at the following locations:
> >> + *
> >> + * http://www.opensource.org/licenses/gpl-license.html
> >> + * http://www.gnu.org/copyleft/gpl.html
> >> + */
> >> +
> >> +/dts-v1/;
> >> +/include/ "imx28.dtsi"
> >> +
> >> +/ {
> >> +	model = "Crystalfontz CFA-10036 Board";
> >> +	compatible = "crystalfontz,cfa10036", "fsl,imx28-evk", "fsl,imx28";
> >> +
> >> +	memory {
> >> +		reg = <0x40000000 0x08000000>;
> >> +	};
> >> +
> >> +	apb at 80000000 {
> >> +		apbh at 80000000 {
> >> +			ssp0: ssp at 80010000 {
> >> +				compatible = "fsl,imx28-mmc";
> >> +				pinctrl-names = "default";
> >> +				pinctrl-0 = <&mmc0_4bit_pins_a
> >> +					&mmc0_cd_cfg &mmc0_sck_cfg>;
> >> +				bus-width = <4>;
> >> +				status = "okay";
> >> +			};
> >> +			pinctrl at 80018000 {
> >> +				#address-cells = <1>;
> >> +				#size-cells = <0>;
> >> +				compatible = "fsl,imx28-pinctrl", "simple-bus";
> >> +				reg = <0x80018000 2000>;
> >> +
> >> +				mmc0_4bit_pins_a: mmc0-4bit at 0 {
> >> +					reg = <0>;
> >> +					fsl,pinmux-ids = <0x2000 0x2010 0x2020
> >> +						0x2030 0x2080 0x2090 0x20a0>;
> >> +					fsl,drive-strength = <1>;
> >> +					fsl,voltage = <1>;
> >> +					fsl,pull-up = <1>;
> > 
> > Do you need this copy here?
> 
> This is not a direct copy, as there is only 4 pins for the MMC bus on
> this board, and one of the pins used for the additional bus pins on the
> imx28evk is used to power the OLED screen on this one, so I don't want
> all of the imx28 pins allocated for the mmc0 to be muxed as such.

Aha!

> Anyway, I'm not very familiar with pinctrl yet, so I might be missing
> something.

It's completely crazy and it's still going to be adjusted anyway.

> >> +				};
> >> +
> >> +				duart_pins_a: duart at 0 {
> >> +					reg = <0>;
> >> +					fsl,pinmux-ids = <0x3022 0x3032>;
> >> +					fsl,drive-strength = <0>;
> >> +					fsl,voltage = <1>;
> >> +					fsl,pull-up = <0>;
> > 
> > And this?
> 
> The pins to be muxed for the DUART differs from the imx28 here as well.
> On the iMX28EVK, the pins used for the DUART are 3-16 and 3-17, while we
> use the 3-2 and 3-3 on this board.

Understood

> >> +				};
> >> +			};
> >> +		};
> >> +
> >> +		apbx at 80040000 {
> >> +			duart: serial at 80074000 {
> >> +				pinctrl-names = "default";
> >> +				pinctrl-0 = <&duart_pins_a>;
> >> +				status = "okay";
> >> +			};
> >> +		};
> >> +	};
> >> +};
> 
> Thanks!
> 
> Maxime

Best regards,
Marek Vasut

[PATCH] ARM:MXS: Initial support for the Crystalfontz CFA-10036

[Shawn Guo]

On Thu, Jun 21, 2012 at 10:46:52AM +0200, Maxime Ripard wrote:
> On the MMC part though, Something odd is going on.  On the first
> patches I made against 3.4, everything was working fine. With 3.5-rc2
> though, the MMC never replies to the first CMD52 command sent to
> it. The pin muxing hasn't changed and has been checked, the card is
> well detected through the card detect pin, the power enable pin is
> correctly set to a low level, and so the SD card is correctly
> powered. The MMC works fine with the exact same kernel on the
> imx28-evk though.
> 
> What seems odd to me, is that when booting the board with the 3.4
> kernel, at boot, the mxs-mmc driver reports a SSP clock of 288MHz,
> while on 3.5, it reports a 480MHz SSP clock. By looking at the clk
> driver for the iMX28, there is a comment saying that the 480MHz clock
> seems to high for SD controlers, and that it should be 288MHz
> instead. Given that the clock part has changed quite a lot from 3.4 to
> 3.5, I think it might be a good lead, but hardcoding the SSP clock to
> 288MHz in the mxs-mmc driver instead of using the clk_get_rate
> function does not resolve the problem.
> 
> Also, this problem is seen with both DT-probing and traditionnal
> probing.
> 
> Do someone have an idea on what could possibly happen here ?
> 
Can you try the change below to see if it fixes the problem?

Regards,
Shawn

--8<---

diff --git a/drivers/clk/mxs/clk-imx28.c b/drivers/clk/mxs/clk-imx28.c
index 003b0f3..1a41d4e 100644
--- a/drivers/clk/mxs/clk-imx28.c
+++ b/drivers/clk/mxs/clk-imx28.c
@@ -245,8 +245,8 @@ int __init mx28_clocks_init(void)
        clks[pll2] = mxs_clk_pll("pll2", "ref_xtal", PLL2CTRL0, 23, 50000000);
        clks[ref_cpu] = mxs_clk_ref("ref_cpu", "pll0", FRAC0, 0);
        clks[ref_emi] = mxs_clk_ref("ref_emi", "pll0", FRAC0, 1);
-       clks[ref_io0] = mxs_clk_ref("ref_io0", "pll0", FRAC0, 2);
-       clks[ref_io1] = mxs_clk_ref("ref_io1", "pll0", FRAC0, 3);
+       clks[ref_io1] = mxs_clk_ref("ref_io1", "pll0", FRAC0, 2);
+       clks[ref_io0] = mxs_clk_ref("ref_io0", "pll0", FRAC0, 3);
        clks[ref_pix] = mxs_clk_ref("ref_pix", "pll0", FRAC1, 0);
        clks[ref_hsadc] = mxs_clk_ref("ref_hsadc", "pll0", FRAC1, 1);
        clks[ref_gpmi] = mxs_clk_ref("ref_gpmi", "pll0", FRAC1, 2);

[PATCH 2/2] ARM: MXS: Add Crystalfontz CFA-10036 DTS

[Shawn Guo]

On Thu, Jun 21, 2012 at 10:46:54AM +0200, Maxime Ripard wrote:
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> 
> Cc: Brian Lily <brian@crystalfontz.com>
> ---
>  arch/arm/boot/dts/cfa10036.dts |   66 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
>  create mode 100644 arch/arm/boot/dts/cfa10036.dts
> 
> diff --git a/arch/arm/boot/dts/cfa10036.dts b/arch/arm/boot/dts/cfa10036.dts
> new file mode 100644
> index 0000000..ceae960
> --- /dev/null
> +++ b/arch/arm/boot/dts/cfa10036.dts
> @@ -0,0 +1,66 @@
> +/*
> + * Copyright 2012 Free Electrons
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +/dts-v1/;
> +/include/ "imx28.dtsi"
> +
> +/ {
> +	model = "Crystalfontz CFA-10036 Board";
> +	compatible = "crystalfontz,cfa10036", "fsl,imx28-evk", "fsl,imx28";

"fsl,imx28-evk" may not need to be here.

> +
> +	memory {
> +		reg = <0x40000000 0x08000000>;
> +	};
> +
> +	apb at 80000000 {
> +		apbh at 80000000 {
> +			ssp0: ssp at 80010000 {
> +				compatible = "fsl,imx28-mmc";
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&mmc0_4bit_pins_a
> +					&mmc0_cd_cfg &mmc0_sck_cfg>;
> +				bus-width = <4>;
> +				status = "okay";
> +			};

Nit: a new line is needed here.

> +			pinctrl at 80018000 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				compatible = "fsl,imx28-pinctrl", "simple-bus";
> +				reg = <0x80018000 2000>;
> +
> +				mmc0_4bit_pins_a: mmc0-4bit at 0 {
> +					reg = <0>;
> +					fsl,pinmux-ids = <0x2000 0x2010 0x2020
> +						0x2030 0x2080 0x2090 0x20a0>;
> +					fsl,drive-strength = <1>;
> +					fsl,voltage = <1>;
> +					fsl,pull-up = <1>;
> +				};

Still, this needs to be in imx28.dtsi, so that if there is another
board designed with 4bit mmc slot on the same pins can just reuse it.

> +
> +				duart_pins_a: duart at 0 {
> +					reg = <0>;
> +					fsl,pinmux-ids = <0x3022 0x3032>;
> +					fsl,drive-strength = <0>;
> +					fsl,voltage = <1>;
> +					fsl,pull-up = <0>;
> +				};

Same here.  If other boards pick the same pinmux on duart as your
boards, this should be shared.

Bear it in mind, what pins can be muxed on what functions is determined
by SoC design not the board.

> +			};
> +		};
> +
> +		apbx at 80040000 {
> +			duart: serial at 80074000 {
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&duart_pins_a>;
> +				status = "okay";
> +			};
> +		};
> +	};
> +};
> -- 
> 1.7.9.5
> 

-- 
Regards,
Shawn

[PATCH] ARM:MXS: Initial support for the Crystalfontz CFA-10036

[Maxime Ripard]

Hi Shawn,

Le 21/06/2012 15:11, Shawn Guo a ?crit :
> On Thu, Jun 21, 2012 at 10:46:52AM +0200, Maxime Ripard wrote:
>> On the MMC part though, Something odd is going on.  On the first
>> patches I made against 3.4, everything was working fine. With 3.5-rc2
>> though, the MMC never replies to the first CMD52 command sent to
>> it. The pin muxing hasn't changed and has been checked, the card is
>> well detected through the card detect pin, the power enable pin is
>> correctly set to a low level, and so the SD card is correctly
>> powered. The MMC works fine with the exact same kernel on the
>> imx28-evk though.
>>
>> What seems odd to me, is that when booting the board with the 3.4
>> kernel, at boot, the mxs-mmc driver reports a SSP clock of 288MHz,
>> while on 3.5, it reports a 480MHz SSP clock. By looking at the clk
>> driver for the iMX28, there is a comment saying that the 480MHz clock
>> seems to high for SD controlers, and that it should be 288MHz
>> instead. Given that the clock part has changed quite a lot from 3.4 to
>> 3.5, I think it might be a good lead, but hardcoding the SSP clock to
>> 288MHz in the mxs-mmc driver instead of using the clk_get_rate
>> function does not resolve the problem.
>>
>> Also, this problem is seen with both DT-probing and traditionnal
>> probing.
>>
>> Do someone have an idea on what could possibly happen here ?
>>
> Can you try the change below to see if it fixes the problem?

It is indeed working now, thanks a lot :)
I've also tested it on the imx28evk, and it works as well.

You can add by Tested-by I guess :)

We have found that indeed the two registers were inverted compared to
the datasheet, but why was it still working on the iMX28-evk ?

Thanks again,
Maxime

-- 
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH] ARM:MXS: Initial support for the Crystalfontz CFA-10036

[Shawn Guo]

On Thu, Jun 21, 2012 at 04:22:41PM +0200, Maxime Ripard wrote:
> You can add by Tested-by I guess :)
> 
Thanks for reporting and testing.

> We have found that indeed the two registers were inverted compared to
> the datasheet, but why was it still working on the iMX28-evk ?
> 
I guess imx28-evk will also have issue with some stressful testing.

-- 
Regards,
Shawn