ARM: mxs: Add initial support for Bluegiga APX4 Development Kit

Added initial support for Bluegiga APX4 module and Development Kit. Patches are based on Linux v3.1-rc9.

Signed-off-by Lauri Hintsala <[email protected]>

Signed-off-by Veli-Pekka Peltola <[email protected]>

---
 arch/arm/configs/mxs_defconfig              |    1 +
 arch/arm/mach-mxs/Kconfig                   |   15 ++
 arch/arm/mach-mxs/Makefile                  |    1 +
 arch/arm/mach-mxs/include/mach/mxs.h        |    1 +
 arch/arm/mach-mxs/include/mach/uncompress.h |    1 +
 arch/arm/mach-mxs/mach-apx4devkit.c         |  201 +++++++++++++++++++++++++++
 6 files changed, 220 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-mxs/mach-apx4devkit.c

arch/arm/configs/mxs_defconfig +1 -0

index db2cb7d..6a6268d 100644
@@ -26,6 +26,7 @@ CONFIG_MACH_MX23EVK=y
 CONFIG_MACH_MX28EVK=y
 CONFIG_MACH_STMP378X_DEVB=y
 CONFIG_MACH_TX28=y
+CONFIG_MACH_APX4DEVKIT=y
 # CONFIG_ARM_THUMB is not set
 CONFIG_NO_HZ=y
 CONFIG_HIGH_RES_TIMERS=y

arch/arm/mach-mxs/Kconfig +0 -0

index 4cd0231..b73ec11 100644
@@ -64,8 +64,23 @@ config MODULE_TX28
 	select MXS_HAVE_PLATFORM_MXS_MMC

arch/arm/mach-mxs/Kconfig +4 -0

 config MACH_TX28
 	bool "Ka-Ro TX28 module"
 	select MODULE_TX28
 
+config MACH_APX4DEVKIT
+	bool "Support Bluegiga APX4 Development Kit"
+	select MODULE_APX4
+
 endif

arch/arm/mach-mxs/Makefile +1 -0

index 6c38262..1838466 100644
@@ -10,6 +10,7 @@ obj-$(CONFIG_SOC_IMX28) += clock-mx28.o mm-mx28.o
 obj-$(CONFIG_MACH_STMP378X_DEVB) += mach-stmp378x_devb.o
 obj-$(CONFIG_MACH_MX23EVK) += mach-mx23evk.o
 obj-$(CONFIG_MACH_MX28EVK) += mach-mx28evk.o
+obj-$(CONFIG_MACH_APX4DEVKIT) += mach-apx4devkit.o
 obj-$(CONFIG_MODULE_TX28) += module-tx28.o
 obj-$(CONFIG_MACH_TX28)    += mach-tx28.o
 

arch/arm/mach-mxs/include/mach/mxs.h +0 -0

index 35a89dd..209698c 100644

arch/arm/mach-mxs/include/mach/mxs.h +0 -0

 		0)
 

arch/arm/mach-mxs/include/mach/uncompress.h +1 -0

index 7f8bf65..d13ebc1 100644
@@ -63,6 +63,7 @@ static inline void __arch_decomp_setup(unsigned long arch_id)
 		mxs_duart_base = MX23_DUART_BASE_ADDR;
 		break;
 	case MACH_TYPE_MX28EVK:
+	case MACH_TYPE_APX4DEVKIT:
 	case MACH_TYPE_TX28:
 		mxs_duart_base = MX28_DUART_BASE_ADDR;
 		break;

arch/arm/mach-mxs/mach-apx4devkit.c new file +135 -0

new file mode 100644
index 0000000..02b7545
@@ -0,0 +1,201 @@
+/*
+ * Copyright (C) 2011
+ * Lauri Hintsala, Bluegiga, <[email protected]>
+ *
+ * based on: mach-mx28evk.c
+ * Copyright 2010 Freescale Semiconductor, Inc. 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/irq.h>
+#include <linux/clk.h>
+#include <linux/i2c.h>
+#include <linux/i2c/at24.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 "devices-mx28.h"
+
+#define APX4DEVKIT_GPIO_USERLED	MXS_GPIO_NR(3, 28)
+
+static const iomux_cfg_t apx4devkit_pads[] __initconst = {
+	/* duart */
+	MX28_PAD_PWM0__DUART_RX | MXS_PAD_CTRL,
+	MX28_PAD_PWM1__DUART_TX | MXS_PAD_CTRL,
+
+	/* auart0 */
+	MX28_PAD_AUART0_RX__AUART0_RX | MXS_PAD_CTRL,
+	MX28_PAD_AUART0_TX__AUART0_TX | MXS_PAD_CTRL,
+	MX28_PAD_AUART0_CTS__AUART0_CTS | MXS_PAD_CTRL,
+	MX28_PAD_AUART0_RTS__AUART0_RTS | MXS_PAD_CTRL,
+
+	/* auart1 */
+	MX28_PAD_AUART1_RX__AUART1_RX | MXS_PAD_CTRL,
+	MX28_PAD_AUART1_TX__AUART1_TX | MXS_PAD_CTRL,
+
+	/* auart2 */
+	MX28_PAD_SSP2_SCK__AUART2_RX | MXS_PAD_CTRL,
+	MX28_PAD_SSP2_MOSI__AUART2_TX | MXS_PAD_CTRL,
+
+	/* auart3 */
+	MX28_PAD_SSP2_MISO__AUART3_RX | MXS_PAD_CTRL,
+	MX28_PAD_SSP2_SS0__AUART3_TX | MXS_PAD_CTRL,
+
+#define MXS_PAD_FEC	(MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP)
+	/* fec0 */
+	MX28_PAD_ENET0_MDC__ENET0_MDC | MXS_PAD_FEC,
+	MX28_PAD_ENET0_MDIO__ENET0_MDIO | MXS_PAD_FEC,
+	MX28_PAD_ENET0_RX_EN__ENET0_RX_EN | MXS_PAD_FEC,
+	MX28_PAD_ENET0_RXD0__ENET0_RXD0 | MXS_PAD_FEC,
+	MX28_PAD_ENET0_RXD1__ENET0_RXD1 | MXS_PAD_FEC,
+	MX28_PAD_ENET0_TX_EN__ENET0_TX_EN | MXS_PAD_FEC,
+	MX28_PAD_ENET0_TXD0__ENET0_TXD0 | MXS_PAD_FEC,
+	MX28_PAD_ENET0_TXD1__ENET0_TXD1 | MXS_PAD_FEC,
+	MX28_PAD_ENET_CLK__CLKCTRL_ENET | MXS_PAD_FEC,
+
+	/* I2C */
+	MX28_PAD_I2C0_SCL__I2C0_SCL,
+	MX28_PAD_I2C0_SDA__I2C0_SDA,
+
+	/* 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_DATA4__SSP0_D4 |
+		(MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+	MX28_PAD_SSP0_DATA5__SSP0_D5 |
+		(MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+	MX28_PAD_SSP0_DATA6__SSP0_D6 |
+		(MXS_PAD_8MA | MXS_PAD_3V3 | MXS_PAD_PULLUP),
+	MX28_PAD_SSP0_DATA7__SSP0_D7 |
+		(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),
+
+	/* led */
+	MX28_PAD_PWM3__GPIO_3_28 | MXS_PAD_CTRL,
+};
+
+/* led */
+static const struct gpio_led apx4devkit_leds[] __initconst = {
+	{
+		.name = "user-led",
+		.default_trigger = "heartbeat",
+		.gpio = APX4DEVKIT_GPIO_USERLED,
+	},
+};
+
+static const struct gpio_led_platform_data apx4devkit_led_data __initconst = {
+	.leds = apx4devkit_leds,
+	.num_leds = ARRAY_SIZE(apx4devkit_leds),
+};
+
+static struct fec_platform_data mx28_fec_pdata __initdata = {
+	.phy = PHY_INTERFACE_MODE_RMII,
+};
+
+static int __init apx4devkit_fec_get_mac(char *macstr)
+{
+	int i, h, l;
+
+	macstr++;
+
+	for (i = 0; i < 6; i++) {
+		if (i != 5 && *(macstr + 2) != ':')
+			goto error;
+
+		h = hex_to_bin(*macstr++);
+		if (h == -1)

arch/arm/mach-mxs/mach-apx4devkit.c new file +39 -0

+
+static struct mxs_mmc_platform_data apx4devkit_mmc_pdata __initdata = {
+	.wp_gpio = -EINVAL,
+	.flags = SLOTF_8_BIT_CAPABLE,
+};
+
+static struct i2c_board_info apx4devkit_i2c_boardinfo[] __initdata = {
+	{ I2C_BOARD_INFO("sgtl5000", 0x0a) }, /* ASoC */
+	{ I2C_BOARD_INFO("pcf8563", 0x51) }, /* RTC */
+};
+
+static void __init apx4devkit_init(void)
+{
+	mxs_iomux_setup_multiple_pads(apx4devkit_pads,
+			ARRAY_SIZE(apx4devkit_pads));
+
+	mx28_add_duart();
+	mx28_add_auart0();
+	mx28_add_auart1();
+	mx28_add_auart2();
+	mx28_add_auart3();
+
+	mx28_add_fec(0, &mx28_fec_pdata);
+
+	mx28_add_mxs_mmc(0, &apx4devkit_mmc_pdata);
+
+	gpio_led_register_device(0, &apx4devkit_led_data);
+
+	/* I2C */
+	mx28_add_mxs_i2c(0);
+	i2c_register_board_info(0, apx4devkit_i2c_boardinfo,
+			ARRAY_SIZE(apx4devkit_i2c_boardinfo));
+}
+
+static void __init apx4devkit_timer_init(void)
+{
+	mx28_clocks_init();
+}
+

Hello,

Quoted 2 lines · Lauri Hintsala

Added initial support for Bluegiga APX4 module and Development Kit. Patches are based on Linux v3.1-rc9.

Quoted 12 lines

arch/arm/mach-mxs/mach-apx4devkit.c new file +12 -0

+static int __init apx4devkit_fec_get_mac(char *macstr)
+{
+	int i, h, l;
+
+	macstr++;
+
+	for (i = 0; i < 6; i++) {
+		if (i != 5 && *(macstr + 2) != ':')
+			goto error;
+
+		h = hex_to_bin(*macstr++);
+		if (h == -1)

arch/arm/mach-mxs/mach-apx4devkit.c line 145

Quoted 10 lines

arch/arm/mach-mxs/mach-apx4devkit.c new file +10 -0

+			goto error;
+
+		l = hex_to_bin(*macstr++);
+		if (l == -1)
+			goto error;
+
+		macstr++;
+		mx28_fec_pdata.mac[i] = (h << 4) + l;
+	}
+	return 0;

I wonder if there isn't a more generic way to parse a mac address.

Other machines put the mac into the otp.

arch/arm/mach-mxs/mach-apx4devkit.c line 152

Quoted 6 lines

arch/arm/mach-mxs/mach-apx4devkit.c new file +6 -0

+error:
+	pr_err("%s: invalid mac address\n", __func__);
+	return -EINVAL;
+}
+
+__setup("ethaddr", apx4devkit_fec_get_mac);

the name is IMHO too generic for a board specific parameter. Think about a kernel that runs on all mxs based machines. ethaddr=... only affects your machine type. And it's not possible to add the same parameter to a different board.

Hi Uwe, Lauri,

Quoted 5 lines · Uwe Kleine-K?nig

Hello,

Quoted 2 lines · Lauri Hintsala

Added initial support for Bluegiga APX4 module and Development Kit. Patches are based on Linux v3.1-rc9.

[snip]

Quoted 26 lines

Quoted 12 lines

arch/arm/mach-mxs/mach-apx4devkit.c new file +12 -0

+static int __init apx4devkit_fec_get_mac(char *macstr)
+{
+	int i, h, l;
+
+	macstr++;
+
+	for (i = 0; i<  6; i++) {
+		if (i != 5&&  *(macstr + 2) != ':')
+			goto error;
+
+		h = hex_to_bin(*macstr++);
+		if (h == -1)

arch/arm/mach-mxs/mach-apx4devkit.c line 145

Quoted 10 lines

arch/arm/mach-mxs/mach-apx4devkit.c new file +10 -0

+			goto error;
+
+		l = hex_to_bin(*macstr++);
+		if (l == -1)
+			goto error;
+
+		macstr++;
+		mx28_fec_pdata.mac[i] = (h<<  4) + l;
+	}
+	return 0;

I wonder if there isn't a more generic way to parse a mac address.

Other machines put the mac into the otp.

Quoted 10 lines

arch/arm/mach-mxs/mach-apx4devkit.c line 152

Quoted 6 lines

arch/arm/mach-mxs/mach-apx4devkit.c new file +6 -0

+error:
+	pr_err("%s: invalid mac address\n", __func__);
+	return -EINVAL;
+}
+
+__setup("ethaddr", apx4devkit_fec_get_mac);

the name is IMHO too generic for a board specific parameter. Think about a kernel that runs on all mxs based machines. ethaddr=... only affects your machine type. And it's not possible to add the same parameter to a different board.

The fec driver already supports the macaddr module parameter. See the comment at the beginning of the fec_get_mac() routine in drivers/net/fec.c.

Quoted 47 lines · Baruch Siach

Hi Uwe, Lauri,

Quoted 5 lines · Uwe Kleine-K?nig

Hello,

Quoted 2 lines · Lauri Hintsala

Added initial support for Bluegiga APX4 module and Development Kit. Patches are based on Linux v3.1-rc9.

[snip]

Quoted 36 lines

Quoted 12 lines

arch/arm/mach-mxs/mach-apx4devkit.c new file +12 -0

+static int __init apx4devkit_fec_get_mac(char *macstr)
+{
+	int i, h, l;
+
+	macstr++;
+
+	for (i = 0; i<  6; i++) {
+		if (i != 5&&  *(macstr + 2) != ':')
+			goto error;
+
+		h = hex_to_bin(*macstr++);
+		if (h == -1)

arch/arm/mach-mxs/mach-apx4devkit.c line 145

Quoted 10 lines

arch/arm/mach-mxs/mach-apx4devkit.c new file +10 -0

+			goto error;
+
+		l = hex_to_bin(*macstr++);
+		if (l == -1)
+			goto error;
+
+		macstr++;
+		mx28_fec_pdata.mac[i] = (h<<  4) + l;
+	}
+	return 0;

I wonder if there isn't a more generic way to parse a mac address.

Other machines put the mac into the otp.

arch/arm/mach-mxs/mach-apx4devkit.c line 152

Quoted 6 lines

arch/arm/mach-mxs/mach-apx4devkit.c new file +6 -0

+error:
+	pr_err("%s: invalid mac address\n", __func__);
+	return -EINVAL;
+}
+
+__setup("ethaddr", apx4devkit_fec_get_mac);

the name is IMHO too generic for a board specific parameter. Think about a kernel that runs on all mxs based machines. ethaddr=... only affects your machine type. And it's not possible to add the same parameter to a different board.

Quoted 2 lines

The fec driver already supports the macaddr module parameter. See the comment at the beginning of the fec_get_mac() routine in drivers/net/fec.c.

We are passing mac address from uboot. Uboot's format is "xx:xx:xx:xx:xx:xx" and fec is getting mac address as array "0x00,0x04,0x9f,0x01,0x30,0xe0". So using fec's routine is not very useful in our case.

Hi Lauri,

Quoted 51 lines · Lauri Hintsala

Quoted 49 lines · Baruch Siach

Hi Uwe, Lauri,

Quoted 5 lines · Uwe Kleine-K?nig

Hello,

Quoted 2 lines · Lauri Hintsala

Added initial support for Bluegiga APX4 module and Development Kit. Patches are based on Linux v3.1-rc9.

[snip]

Quoted 26 lines

Quoted 12 lines

arch/arm/mach-mxs/mach-apx4devkit.c new file +12 -0

+static int __init apx4devkit_fec_get_mac(char *macstr)
+{
+	int i, h, l;
+
+	macstr++;
+
+	for (i = 0; i<   6; i++) {
+		if (i != 5&&   *(macstr + 2) != ':')
+			goto error;
+
+		h = hex_to_bin(*macstr++);
+		if (h == -1)

arch/arm/mach-mxs/mach-apx4devkit.c line 145

Quoted 10 lines

arch/arm/mach-mxs/mach-apx4devkit.c new file +10 -0

+			goto error;
+
+		l = hex_to_bin(*macstr++);
+		if (l == -1)
+			goto error;
+
+		macstr++;
+		mx28_fec_pdata.mac[i] = (h<<   4) + l;
+	}
+	return 0;

I wonder if there isn't a more generic way to parse a mac address.

Other machines put the mac into the otp.

Quoted 10 lines

arch/arm/mach-mxs/mach-apx4devkit.c line 152

Quoted 6 lines

arch/arm/mach-mxs/mach-apx4devkit.c new file +6 -0

+error:
+	pr_err("%s: invalid mac address\n", __func__);
+	return -EINVAL;
+}
+
+__setup("ethaddr", apx4devkit_fec_get_mac);

the name is IMHO too generic for a board specific parameter. Think about a kernel that runs on all mxs based machines. ethaddr=... only affects your machine type. And it's not possible to add the same parameter to a different board.

The fec driver already supports the macaddr module parameter. See the comment at the beginning of the fec_get_mac() routine in drivers/net/fec.c.

Quoted 4 lines

We are passing mac address from uboot. Uboot's format is "xx:xx:xx:xx:xx:xx" and fec is getting mac address as array "0x00,0x04,0x9f,0x01,0x30,0xe0". So using fec's routine is not very useful in our case.

u-boot can pass any string you like to the kernel. Just do

setenv bootargs "$bootargs fec.macaddr=0x00,0x04,0x9f,0x01,0x30,0xe0"

at the u-boot prompt, or in a script.

Quoted 1 line · Baruch Siach

u-boot can pass any string you like to the kernel. Just do

Yes I know I can pass any string I like.

Quoted 3 lines

setenv bootargs "$bootargs fec.macaddr=0x00,0x04,0x9f,0x01,0x30,0xe0"

at the u-boot prompt, or in a script.

ethaddr variable of uboot is taking mac addr in colon separated form (e.g. "00:04.9f:01:30:e0") and it is not very useful to have mac address in two different form in uboot envs.

Hello,

Quoted 2 lines · Lauri Hintsala

Added initial support for Bluegiga APX4 module and Development Kit. Patches are based on Linux v3.1-rc9.

Specifying the base isn't helpful in the commit log. You can better write things like these after the tripple dash below.

arch/arm/mach-mxs/Kconfig config MODULE_TX28 · line 74

Quoted 8 lines

arch/arm/mach-mxs/Kconfig +8 -0

+config MODULE_APX4
+	bool
+	select SOC_IMX28
+	select LEDS_GPIO_REGISTER
+	select MXS_HAVE_AMBA_DUART
+	select MXS_HAVE_PLATFORM_AUART
+	select MXS_HAVE_PLATFORM_FEC
+    select MXS_HAVE_PLATFORM_MXS_I2C

broken indention

arch/arm/mach-mxs/Kconfig config MODULE_TX28 · line 77

Quoted 3 lines

arch/arm/mach-mxs/Kconfig +3 -0

+	select MXS_HAVE_PLATFORM_MXS_MMC
+	select MXS_OCOTP
+

MODULE_APX4 is unused, do you plan to use it later?

Quoted 21 lines

config MACH_TX28
bool "Ka-Ro TX28 module"
select MODULE_TX28

arch/arm/mach-mxs/Kconfig +4 -0

+config MACH_APX4DEVKIT
+	bool "Support Bluegiga APX4 Development Kit"
+	select MODULE_APX4
+
  endif

arch/arm/mach-mxs/Makefile +1 -0

index 6c38262..1838466 100644
@@ -10,6 +10,7 @@ obj-$(CONFIG_SOC_IMX28) += clock-mx28.o mm-mx28.o
  obj-$(CONFIG_MACH_STMP378X_DEVB) += mach-stmp378x_devb.o
  obj-$(CONFIG_MACH_MX23EVK) += mach-mx23evk.o
  obj-$(CONFIG_MACH_MX28EVK) += mach-mx28evk.o
+obj-$(CONFIG_MACH_APX4DEVKIT) += mach-apx4devkit.o
  obj-$(CONFIG_MODULE_TX28) += module-tx28.o
  obj-$(CONFIG_MACH_TX28)    += mach-tx28.o
 

arch/arm/mach-mxs/include/mach/mxs.h line 37

Quoted 10 lines

arch/arm/mach-mxs/include/mach/mxs.h +1 -0

index 35a89dd..209698c 100644
@@ -33,6 +33,7 @@
  		0)
  #define cpu_is_mx28()		(					\
  		machine_is_mx28evk() ||					\
+		machine_is_apx4devkit() ||				\
  		machine_is_tx28() ||					\

please keep this list sorted alphabetically.

Quoted 19 lines

arch/arm/mach-mxs/include/mach/mxs.h +19 -0

+static struct fec_platform_data mx28_fec_pdata __initdata = {
+	.phy = PHY_INTERFACE_MODE_RMII,
+};
+
+static int __init apx4devkit_fec_get_mac(char *macstr)
+{
+	int i, h, l;
+
+	macstr++;
+
+	for (i = 0; i<  6; i++) {
+		if (i != 5&&  *(macstr + 2) != ':')
+			goto error;
+
+		h = hex_to_bin(*macstr++);
+		if (h == -1)
+			goto error;
+
+		l = hex_to_bin(*macstr++);

arch/arm/mach-mxs/include/mach/mxs.h

Quoted 7 lines

arch/arm/mach-mxs/include/mach/mxs.h +7 -0

+		if (l == -1)
+			goto error;
+
+		macstr++;
+		mx28_fec_pdata.mac[i] = (h<<  4) + l;
+	}
+	return 0;

I wonder if there isn't a more generic way to parse a mac address.

Other machines put the mac into the otp.

arch/arm/mach-mxs/include/mach/mxs.h

Quoted 6 lines

arch/arm/mach-mxs/include/mach/mxs.h +6 -0

+error:
+	pr_err("%s: invalid mac address\n", __func__);
+	return -EINVAL;
+}
+
+__setup("ethaddr", apx4devkit_fec_get_mac);

the name is IMHO too generic for a board specific parameter. Think about a kernel that runs on all mxs based machines. ethaddr=... only affects your machine type. And it's not possible to add the same parameter to a different board.

Quoted 8 lines

arch/arm/mach-mxs/include/mach/mxs.h +8 -0

+static void __init apx4devkit_timer_init(void)
+{
+	mx28_clocks_init();
+}
+
+static struct sys_timer apx4devkit_timer = {
+	.init	= apx4devkit_timer_init,
+};

arch/arm/mach-mxs/include/mach/mxs.h

Quoted 7 lines

arch/arm/mach-mxs/include/mach/mxs.h +7 -0

+
+MACHINE_START(APX4DEVKIT, "Bluegiga APX4 Development Kit")
+	.map_io		= mx28_map_io,
+	.init_irq	= mx28_init_irq,
+	.init_machine	= apx4devkit_init,
+	.timer		=&apx4devkit_timer,
+MACHINE_END

please read http://article.gmane.org/gmane.linux.ports.arm.omap/50721

Hi Uwe,

Quoted 7 lines · Uwe Kleine-K?nig

Hello,

Quoted 2 lines · Lauri Hintsala

Added initial support for Bluegiga APX4 module and Development Kit. Patches are based on Linux v3.1-rc9.

Specifying the base isn't helpful in the commit log. You can better write things like these after the tripple dash below.

I'll drop the version information from commit log.

Quoted 9 lines

arch/arm/mach-mxs/Kconfig config MODULE_TX28 · line 74

Quoted 8 lines

arch/arm/mach-mxs/Kconfig +8 -0

+config MODULE_APX4
+	bool
+	select SOC_IMX28
+	select LEDS_GPIO_REGISTER
+	select MXS_HAVE_AMBA_DUART
+	select MXS_HAVE_PLATFORM_AUART
+	select MXS_HAVE_PLATFORM_FEC
+    select MXS_HAVE_PLATFORM_MXS_I2C

broken indention

Thanks for catching it.

Quoted 4 lines

arch/arm/mach-mxs/Kconfig config MODULE_TX28 · line 77

Quoted 3 lines

arch/arm/mach-mxs/Kconfig +3 -0

+	select MXS_HAVE_PLATFORM_MXS_MMC
+	select MXS_OCOTP
+

MODULE_APX4 is unused, do you plan to use it later?

It is already used. MODULE_APX4 is selected by MACH_APX4DEVKIT.

Quoted 24 lines

Quoted 24 lines

config MACH_TX28

(quoted diff) +19 -0

 	bool "Ka-Ro TX28 module"
 	select MODULE_TX28
 
+config MACH_APX4DEVKIT
+	bool "Support Bluegiga APX4 Development Kit"
+	select MODULE_APX4
+
 endif
+static int __init apx4devkit_fec_get_mac(char *macstr)
+{
+	int i, h, l;
+
+	macstr++;
+
+	for (i = 0; i<  6; i++) {
+		if (i != 5&&  *(macstr + 2) != ':')
+			goto error;
+
+		h = hex_to_bin(*macstr++);
+		if (h == -1)
+			goto error;
+
+		l = hex_to_bin(*macstr++);

Quoted 8 lines

arch/arm/mach-mxs/mach-apx4devkit.c line 145

Quoted 7 lines

arch/arm/mach-mxs/mach-apx4devkit.c new file +7 -0

+		if (l == -1)
+			goto error;
+
+		macstr++;
+		mx28_fec_pdata.mac[i] = (h<<  4) + l;
+	}
+	return 0;

I wonder if there isn't a more generic way to parse a mac address.

Maybe it is better to use sscanf for parsing mac address, is it. I didn't found any generic function for this purpose. Is there any?

Other boards seems to use similar methods for parsing mac address:
arch/arm/mach-orion5x/dns323-setup.c: dns323_read_mac_addr arch/arm/mach-orion5x/tsx09-common.c: qnap_tsx09_check_mac_addr arch/mips/rb532/devices.c: parse_mac_addr

I dont't mean it is right/best way to do parsing but it seems to be generic problem.

Quoted 1 line

Other machines put the mac into the otp.

We do not want to use otps for mac.

Quoted 10 lines

arch/arm/mach-mxs/mach-apx4devkit.c line 152

Quoted 6 lines

arch/arm/mach-mxs/mach-apx4devkit.c new file +6 -0

+error:
+	pr_err("%s: invalid mac address\n", __func__);
+	return -EINVAL;
+}
+
+__setup("ethaddr", apx4devkit_fec_get_mac);

the name is IMHO too generic for a board specific parameter. Think about a kernel that runs on all mxs based machines. ethaddr=... only affects your machine type. And it's not possible to add the same parameter to a different board.

I could rename the parameter name.

Quoted 8 lines

Quoted 8 lines

arch/arm/mach-mxs/mach-apx4devkit.c new file +8 -0

+static void __init apx4devkit_timer_init(void)
+{
+	mx28_clocks_init();
+}
+
+static struct sys_timer apx4devkit_timer = {
+	.init	= apx4devkit_timer_init,
+};

Quoted 8 lines

arch/arm/mach-mxs/mach-apx4devkit.c line 201

Quoted 7 lines

arch/arm/mach-mxs/mach-apx4devkit.c new file +7 -0

+
+MACHINE_START(APX4DEVKIT, "Bluegiga APX4 Development Kit")
+	.map_io		= mx28_map_io,
+	.init_irq	= mx28_init_irq,
+	.init_machine	= apx4devkit_init,
+	.timer		=&apx4devkit_timer,
+MACHINE_END

please read http://article.gmane.org/gmane.linux.ports.arm.omap/50721

Do you mean init_machine and timer are in wrong order?

Quoted 32 lines · Lauri Hintsala

Hi Uwe,

Quoted 7 lines · Uwe Kleine-K?nig

Hello,

Quoted 2 lines · Lauri Hintsala

Added initial support for Bluegiga APX4 module and Development Kit. Patches are based on Linux v3.1-rc9.

Specifying the base isn't helpful in the commit log. You can better write things like these after the tripple dash below.

I'll drop the version information from commit log.

Quoted 9 lines

arch/arm/mach-mxs/Kconfig config MODULE_TX28 · line 74

Quoted 8 lines

arch/arm/mach-mxs/Kconfig +8 -0

+config MODULE_APX4
+	bool
+	select SOC_IMX28
+	select LEDS_GPIO_REGISTER
+	select MXS_HAVE_AMBA_DUART
+	select MXS_HAVE_PLATFORM_AUART
+	select MXS_HAVE_PLATFORM_FEC
+    select MXS_HAVE_PLATFORM_MXS_I2C

broken indention

Thanks for catching it.

Quoted 4 lines

arch/arm/mach-mxs/Kconfig config MODULE_TX28 · line 77

Quoted 3 lines

arch/arm/mach-mxs/Kconfig +3 -0

+	select MXS_HAVE_PLATFORM_MXS_MMC
+	select MXS_OCOTP
+

MODULE_APX4 is unused, do you plan to use it later?

Quoted 1 line

It is already used. MODULE_APX4 is selected by MACH_APX4DEVKIT.

Yeah, so you have two symbols that are =y for your board. One of them doesn't do anything apart from being there and having a value. Just removing config MODULE_APX4, putting all the selects into MACH_APX4DEVKIT and remove the select MODULE_APX4 from it doesn't break anything.

MODULE_TX28 at least has a separate file that is compiled for it and that might be useful for maschines that don't necessarily use mach-tx28.c .

Quoted 33 lines

Quoted 24 lines

Quoted 24 lines

config MACH_TX28
bool "Ka-Ro TX28 module"
select MODULE_TX28

arch/arm/mach-mxs/mach-apx4devkit.c new file +19 -0

+config MACH_APX4DEVKIT
+	bool "Support Bluegiga APX4 Development Kit"
+	select MODULE_APX4
+
  endif
+static int __init apx4devkit_fec_get_mac(char *macstr)
+{
+	int i, h, l;
+
+	macstr++;
+
+	for (i = 0; i<   6; i++) {
+		if (i != 5&&   *(macstr + 2) != ':')
+			goto error;
+
+		h = hex_to_bin(*macstr++);
+		if (h == -1)
+			goto error;
+
+		l = hex_to_bin(*macstr++);

Quoted 8 lines

arch/arm/mach-mxs/mach-apx4devkit.c line 145

Quoted 7 lines

arch/arm/mach-mxs/mach-apx4devkit.c new file +7 -0

+		if (l == -1)
+			goto error;
+
+		macstr++;
+		mx28_fec_pdata.mac[i] = (h<<   4) + l;
+	}
+	return 0;

I wonder if there isn't a more generic way to parse a mac address.

Quoted 2 lines

Maybe it is better to use sscanf for parsing mac address, is it. I didn't found any generic function for this purpose. Is there any?

sscanf("%pM", ...) would be straight forward, but I'd be surprised if that worked. I'd ask on netdev at vger.kernel.org.

Quoted 7 lines

Other boards seems to use similar methods for parsing mac address:
arch/arm/mach-orion5x/dns323-setup.c: dns323_read_mac_addr arch/arm/mach-orion5x/tsx09-common.c: qnap_tsx09_check_mac_addr arch/mips/rb532/devices.c: parse_mac_addr

I dont't mean it is right/best way to do parsing but it seems to be generic problem.

Yeah, so a generic solution would be great!

Quoted 16 lines

Quoted 1 line

Other machines put the mac into the otp.

We do not want to use otps for mac.

Quoted 10 lines

arch/arm/mach-mxs/mach-apx4devkit.c line 152

Quoted 6 lines

arch/arm/mach-mxs/mach-apx4devkit.c new file +6 -0

+error:
+	pr_err("%s: invalid mac address\n", __func__);
+	return -EINVAL;
+}
+
+__setup("ethaddr", apx4devkit_fec_get_mac);

the name is IMHO too generic for a board specific parameter. Think about a kernel that runs on all mxs based machines. ethaddr=... only affects your machine type. And it's not possible to add the same parameter to a different board.

Quoted 1 line

I could rename the parameter name.

fec.macaddr should work already today.

Quoted 17 lines

Quoted 16 lines

Quoted 5 lines

arch/arm/mach-mxs/mach-apx4devkit.c new file +5 -0

+static void __init apx4devkit_timer_init(void)
+{
+	mx28_clocks_init();
+}
+

arch/arm/mach-mxs/mach-apx4devkit.c line 201

Quoted 10 lines

arch/arm/mach-mxs/mach-apx4devkit.c new file +10 -0

+static struct sys_timer apx4devkit_timer = {
+	.init	= apx4devkit_timer_init,
+};
+
+MACHINE_START(APX4DEVKIT, "Bluegiga APX4 Development Kit")
+	.map_io		= mx28_map_io,
+	.init_irq	= mx28_init_irq,
+	.init_machine	= apx4devkit_init,
+	.timer		=&apx4devkit_timer,
+MACHINE_END

please read http://article.gmane.org/gmane.linux.ports.arm.omap/50721

Quoted 1 line

Do you mean init_machine and timer are in wrong order?

yes.

Quoted 2 lines

Quoted 1 line

Do you mean init_machine and timer are in wrong order?

yes.

Ok.

I noticed mx28evk and mx28evk have also those functions in wrong order.