[PATCH 0/3] mxc: mx51: Add support for efikamx nettop

[PATCH 0/3] mxc: mx51: Add support for efikamx nettop

[Amit Kucheria]

The following patches add basic support for the Genesi EfikaMX nettop[1].
Serial port and OTG is enabled for now. Ethernet hangs off of OTG.

The new machine name has been agreed with Russell[2].

Regards,
Amit

[1] http://www.genesi-usa.com/
[2] http://www.spinics.net/lists/arm-kernel/msg98960.html


Amit Kucheria (3):
  mx51: add support for genesi efikamx nettop
  mx51: efikamx: add otg support
  mx51: Move OTG initialisation for all boards to a single file

 arch/arm/mach-mx5/Kconfig              |    7 ++
 arch/arm/mach-mx5/Makefile             |    3 +-
 arch/arm/mach-mx5/board-cpuimx51.c     |   25 +--------
 arch/arm/mach-mx5/board-mx51_babbage.c |   25 +--------
 arch/arm/mach-mx5/board-mx51_efikamx.c |  100 ++++++++++++++++++++++++++++++++
 arch/arm/mach-mx5/usb.c                |   40 +++++++++++++
 6 files changed, 151 insertions(+), 49 deletions(-)
 create mode 100644 arch/arm/mach-mx5/board-mx51_efikamx.c
 create mode 100644 arch/arm/mach-mx5/usb.c

[PATCH 1/3] mx51: add support for genesi efikamx nettop

[Amit Kucheria]

Get serial port working for now

Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
---
 arch/arm/mach-mx5/Kconfig              |    7 +++
 arch/arm/mach-mx5/Makefile             |    1 +
 arch/arm/mach-mx5/board-mx51_efikamx.c |   91 ++++++++++++++++++++++++++++++++
 3 files changed, 99 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-mx5/board-mx51_efikamx.c

diff --git a/arch/arm/mach-mx5/Kconfig b/arch/arm/mach-mx5/Kconfig
index 3a4c3b3..fad31cc 100644
--- a/arch/arm/mach-mx5/Kconfig
+++ b/arch/arm/mach-mx5/Kconfig
@@ -49,4 +49,11 @@ config MACH_EUKREA_MBIMX51_BASEBOARD
 
 endchoice
 
+config MACH_MX51_EFIKAMX
+	bool "Support MX51 Genesi Efika MX nettop"
+	select IMX_HAVE_PLATFORM_IMX_UART
+	help
+	  Include support for Genesi Efika MX nettop. This includes specific
+	  configurations for the board and its peripherals.
+
 endif
diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile
index 86c66e7..d1aac9c 100644
--- a/arch/arm/mach-mx5/Makefile
+++ b/arch/arm/mach-mx5/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_MACH_MX51_BABBAGE) += board-mx51_babbage.o
 obj-$(CONFIG_MACH_MX51_3DS) += board-mx51_3ds.o
 obj-$(CONFIG_MACH_EUKREA_CPUIMX51) += board-cpuimx51.o
 obj-$(CONFIG_MACH_EUKREA_MBIMX51_BASEBOARD) += eukrea_mbimx51-baseboard.o
+obj-$(CONFIG_MACH_MX51_EFIKAMX) += board-mx51_efikamx.o
diff --git a/arch/arm/mach-mx5/board-mx51_efikamx.c b/arch/arm/mach-mx5/board-mx51_efikamx.c
new file mode 100644
index 0000000..4c921fc
--- /dev/null
+++ b/arch/arm/mach-mx5/board-mx51_efikamx.c
@@ -0,0 +1,91 @@
+/*
+ * Copyright (C) 2010 Linaro Limited
+ *
+ * based on code from the following
+ * Copyright 2009-2010 Freescale Semiconductor, Inc. All Rights Reserved.
+ * Copyright 2009-2010 Pegatron Corporation. All Rights Reserved.
+ * Copyright 2009-2010 Genesi USA, Inc. All Rights Reserved.
+ *
+ * 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
+ */
+
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/i2c.h>
+#include <linux/gpio.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/fsl_devices.h>
+
+#include <mach/common.h>
+#include <mach/hardware.h>
+#include <mach/iomux-mx51.h>
+#include <mach/i2c.h>
+#include <mach/mxc_ehci.h>
+
+#include <asm/irq.h>
+#include <asm/setup.h>
+#include <asm/mach-types.h>
+#include <asm/mach/arch.h>
+#include <asm/mach/time.h>
+
+#include "devices-imx51.h"
+#include "devices.h"
+
+static struct pad_desc mx51efikamx_pads[] = {
+	/* UART1 */
+	MX51_PAD_UART1_RXD__UART1_RXD,
+	MX51_PAD_UART1_TXD__UART1_TXD,
+	MX51_PAD_UART1_RTS__UART1_RTS,
+	MX51_PAD_UART1_CTS__UART1_CTS,
+};
+
+/* Serial ports */
+#if defined(CONFIG_SERIAL_IMX) || defined(CONFIG_SERIAL_IMX_MODULE)
+static const struct imxuart_platform_data uart_pdata = {
+	.flags = IMXUART_HAVE_RTSCTS,
+};
+
+static inline void mxc_init_imx_uart(void)
+{
+	imx51_add_imx_uart(0, &uart_pdata);
+	imx51_add_imx_uart(1, &uart_pdata);
+	imx51_add_imx_uart(2, &uart_pdata);
+}
+#else /* !SERIAL_IMX */
+static inline void mxc_init_imx_uart(void)
+{
+}
+#endif /* SERIAL_IMX */
+
+static void __init mxc_board_init(void)
+{
+	mxc_iomux_v3_setup_multiple_pads(mx51efikamx_pads,
+					ARRAY_SIZE(mx51efikamx_pads));
+	mxc_init_imx_uart();
+}
+
+static void __init mx51_efikamx_timer_init(void)
+{
+	mx51_clocks_init(32768, 24000000, 22579200, 24576000);
+}
+
+static struct sys_timer mxc_timer = {
+	.init	= mx51_efikamx_timer_init,
+};
+
+MACHINE_START(MX51_EFIKAMX, "Genesi EfikaMX nettop")
+	/* Maintainer: Amit Kucheria <amit.kucheria@linaro.org> */
+	.phys_io = MX51_AIPS1_BASE_ADDR,
+	.io_pg_offst = ((MX51_AIPS1_BASE_ADDR_VIRT) >> 18) & 0xfffc,
+	.boot_params = MX51_PHYS_OFFSET + 0x100,
+	.map_io = mx51_map_io,
+	.init_irq = mx51_init_irq,
+	.init_machine =  mxc_board_init,
+	.timer = &mxc_timer,
+MACHINE_END
-- 
1.7.0.4

[PATCH 1/3] mx51: add support for genesi efikamx nettop

[Uwe Kleine-König]

On Thu, Oct 07, 2010 at 03:58:12AM +0300, Amit Kucheria wrote:
> Get serial port working for now
> 
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> ---
>  arch/arm/mach-mx5/Kconfig              |    7 +++
>  arch/arm/mach-mx5/Makefile             |    1 +
>  arch/arm/mach-mx5/board-mx51_efikamx.c |   91 ++++++++++++++++++++++++++++++++
>  3 files changed, 99 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/mach-mx5/board-mx51_efikamx.c
> 
> diff --git a/arch/arm/mach-mx5/Kconfig b/arch/arm/mach-mx5/Kconfig
> index 3a4c3b3..fad31cc 100644
> --- a/arch/arm/mach-mx5/Kconfig
> +++ b/arch/arm/mach-mx5/Kconfig
> @@ -49,4 +49,11 @@ config MACH_EUKREA_MBIMX51_BASEBOARD
>  
>  endchoice
>  
> +config MACH_MX51_EFIKAMX
> +	bool "Support MX51 Genesi Efika MX nettop"
> +	select IMX_HAVE_PLATFORM_IMX_UART
> +	help
> +	  Include support for Genesi Efika MX nettop. This includes specific
> +	  configurations for the board and its peripherals.
> +
>  endif
> diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile
> index 86c66e7..d1aac9c 100644
> --- a/arch/arm/mach-mx5/Makefile
> +++ b/arch/arm/mach-mx5/Makefile
> @@ -9,3 +9,4 @@ obj-$(CONFIG_MACH_MX51_BABBAGE) += board-mx51_babbage.o
>  obj-$(CONFIG_MACH_MX51_3DS) += board-mx51_3ds.o
>  obj-$(CONFIG_MACH_EUKREA_CPUIMX51) += board-cpuimx51.o
>  obj-$(CONFIG_MACH_EUKREA_MBIMX51_BASEBOARD) += eukrea_mbimx51-baseboard.o
> +obj-$(CONFIG_MACH_MX51_EFIKAMX) += board-mx51_efikamx.o
> diff --git a/arch/arm/mach-mx5/board-mx51_efikamx.c b/arch/arm/mach-mx5/board-mx51_efikamx.c
> new file mode 100644
> index 0000000..4c921fc
> --- /dev/null
> +++ b/arch/arm/mach-mx5/board-mx51_efikamx.c
> @@ -0,0 +1,91 @@
> +/*
> + * Copyright (C) 2010 Linaro Limited
> + *
> + * based on code from the following
> + * Copyright 2009-2010 Freescale Semiconductor, Inc. All Rights Reserved.
> + * Copyright 2009-2010 Pegatron Corporation. All Rights Reserved.
> + * Copyright 2009-2010 Genesi USA, Inc. All Rights Reserved.
> + *
> + * 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:
IMHO it's better to put the "Version 2 or later" stuff in the first
sentence.  Otherwise it's not entirely clear that GPLv1 should not be
allowed.  That is what is meant here, isn't it?

> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/i2c.h>
> +#include <linux/gpio.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/fsl_devices.h>
> +
> +#include <mach/common.h>
> +#include <mach/hardware.h>
> +#include <mach/iomux-mx51.h>
> +#include <mach/i2c.h>
> +#include <mach/mxc_ehci.h>
> +
> +#include <asm/irq.h>
> +#include <asm/setup.h>
> +#include <asm/mach-types.h>
> +#include <asm/mach/arch.h>
> +#include <asm/mach/time.h>
> +
> +#include "devices-imx51.h"
> +#include "devices.h"
> +
> +static struct pad_desc mx51efikamx_pads[] = {
> +	/* UART1 */
> +	MX51_PAD_UART1_RXD__UART1_RXD,
> +	MX51_PAD_UART1_TXD__UART1_TXD,
> +	MX51_PAD_UART1_RTS__UART1_RTS,
> +	MX51_PAD_UART1_CTS__UART1_CTS,
> +};
> +
> +/* Serial ports */
> +#if defined(CONFIG_SERIAL_IMX) || defined(CONFIG_SERIAL_IMX_MODULE)
> +static const struct imxuart_platform_data uart_pdata = {
__initconst please

> +	.flags = IMXUART_HAVE_RTSCTS,
> +};
> +
> +static inline void mxc_init_imx_uart(void)
> +{
> +	imx51_add_imx_uart(0, &uart_pdata);
> +	imx51_add_imx_uart(1, &uart_pdata);
> +	imx51_add_imx_uart(2, &uart_pdata);
> +}
> +#else /* !SERIAL_IMX */
> +static inline void mxc_init_imx_uart(void)
> +{
> +}
> +#endif /* SERIAL_IMX */
> +
> +static void __init mxc_board_init(void)
There are currently 13 functions with this name.  While it is no problem
for the toolchain as all of them are static, it's bad for people
searching for it's definition and having to figure out which is the
right one.  I admit it's not hard, but still I'd prefer if you'd name it
(say) mx51_efikamx_board_init.

> +{
> +	mxc_iomux_v3_setup_multiple_pads(mx51efikamx_pads,
> +					ARRAY_SIZE(mx51efikamx_pads));
> +	mxc_init_imx_uart();
> +}
> +
> +static void __init mx51_efikamx_timer_init(void)
> +{
> +	mx51_clocks_init(32768, 24000000, 22579200, 24576000);
> +}
> +
> +static struct sys_timer mxc_timer = {
> +	.init	= mx51_efikamx_timer_init,
> +};
> +
> +MACHINE_START(MX51_EFIKAMX, "Genesi EfikaMX nettop")
> +	/* Maintainer: Amit Kucheria <amit.kucheria@linaro.org> */
> +	.phys_io = MX51_AIPS1_BASE_ADDR,
> +	.io_pg_offst = ((MX51_AIPS1_BASE_ADDR_VIRT) >> 18) & 0xfffc,
> +	.boot_params = MX51_PHYS_OFFSET + 0x100,
Do you need this?  AFAIK it is only needed if the machine comes with an
old (or broken) bootloader that fails to pass the address to the atag
list in r2.

> +	.map_io = mx51_map_io,
> +	.init_irq = mx51_init_irq,
> +	.init_machine =  mxc_board_init,
s/  / /

> +	.timer = &mxc_timer,
> +MACHINE_END

Thanks
Uwe

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

[PATCH 1/3] mx51: add support for genesi efikamx nettop

[Amit Kucheria]

On 10 Oct 07, Uwe Kleine-K?nig wrote:
> On Thu, Oct 07, 2010 at 03:58:12AM +0300, Amit Kucheria wrote:
> > Get serial port working for now
> > 
> > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> > ---
> >  arch/arm/mach-mx5/Kconfig              |    7 +++
> >  arch/arm/mach-mx5/Makefile             |    1 +
> >  arch/arm/mach-mx5/board-mx51_efikamx.c |   91 ++++++++++++++++++++++++++++++++
> >  3 files changed, 99 insertions(+), 0 deletions(-)
> >  create mode 100644 arch/arm/mach-mx5/board-mx51_efikamx.c
> > 
> > diff --git a/arch/arm/mach-mx5/Kconfig b/arch/arm/mach-mx5/Kconfig
> > index 3a4c3b3..fad31cc 100644
> > --- a/arch/arm/mach-mx5/Kconfig
> > +++ b/arch/arm/mach-mx5/Kconfig
> > @@ -49,4 +49,11 @@ config MACH_EUKREA_MBIMX51_BASEBOARD
> >  
> >  endchoice
> >  
> > +config MACH_MX51_EFIKAMX
> > +	bool "Support MX51 Genesi Efika MX nettop"
> > +	select IMX_HAVE_PLATFORM_IMX_UART
> > +	help
> > +	  Include support for Genesi Efika MX nettop. This includes specific
> > +	  configurations for the board and its peripherals.
> > +
> >  endif
> > diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile
> > index 86c66e7..d1aac9c 100644
> > --- a/arch/arm/mach-mx5/Makefile
> > +++ b/arch/arm/mach-mx5/Makefile
> > @@ -9,3 +9,4 @@ obj-$(CONFIG_MACH_MX51_BABBAGE) += board-mx51_babbage.o
> >  obj-$(CONFIG_MACH_MX51_3DS) += board-mx51_3ds.o
> >  obj-$(CONFIG_MACH_EUKREA_CPUIMX51) += board-cpuimx51.o
> >  obj-$(CONFIG_MACH_EUKREA_MBIMX51_BASEBOARD) += eukrea_mbimx51-baseboard.o
> > +obj-$(CONFIG_MACH_MX51_EFIKAMX) += board-mx51_efikamx.o
> > diff --git a/arch/arm/mach-mx5/board-mx51_efikamx.c b/arch/arm/mach-mx5/board-mx51_efikamx.c
> > new file mode 100644
> > index 0000000..4c921fc
> > --- /dev/null
> > +++ b/arch/arm/mach-mx5/board-mx51_efikamx.c
> > @@ -0,0 +1,91 @@
> > +/*
> > + * Copyright (C) 2010 Linaro Limited
> > + *
> > + * based on code from the following
> > + * Copyright 2009-2010 Freescale Semiconductor, Inc. All Rights Reserved.
> > + * Copyright 2009-2010 Pegatron Corporation. All Rights Reserved.
> > + * Copyright 2009-2010 Genesi USA, Inc. All Rights Reserved.
> > + *
> > + * 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:
> IMHO it's better to put the "Version 2 or later" stuff in the first
> sentence.  Otherwise it's not entirely clear that GPLv1 should not be
> allowed.  That is what is meant here, isn't it?

OK

> > + *
> > + * http://www.opensource.org/licenses/gpl-license.html
> > + * http://www.gnu.org/copyleft/gpl.html
> > + */
> > +
> > +#include <linux/init.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/i2c.h>
> > +#include <linux/gpio.h>
> > +#include <linux/delay.h>
> > +#include <linux/io.h>
> > +#include <linux/fsl_devices.h>
> > +
> > +#include <mach/common.h>
> > +#include <mach/hardware.h>
> > +#include <mach/iomux-mx51.h>
> > +#include <mach/i2c.h>
> > +#include <mach/mxc_ehci.h>
> > +
> > +#include <asm/irq.h>
> > +#include <asm/setup.h>
> > +#include <asm/mach-types.h>
> > +#include <asm/mach/arch.h>
> > +#include <asm/mach/time.h>
> > +
> > +#include "devices-imx51.h"
> > +#include "devices.h"
> > +
> > +static struct pad_desc mx51efikamx_pads[] = {
> > +	/* UART1 */
> > +	MX51_PAD_UART1_RXD__UART1_RXD,
> > +	MX51_PAD_UART1_TXD__UART1_TXD,
> > +	MX51_PAD_UART1_RTS__UART1_RTS,
> > +	MX51_PAD_UART1_CTS__UART1_CTS,
> > +};
> > +
> > +/* Serial ports */
> > +#if defined(CONFIG_SERIAL_IMX) || defined(CONFIG_SERIAL_IMX_MODULE)
> > +static const struct imxuart_platform_data uart_pdata = {
> __initconst please

OK.

> > +	.flags = IMXUART_HAVE_RTSCTS,
> > +};
> > +
> > +static inline void mxc_init_imx_uart(void)
> > +{
> > +	imx51_add_imx_uart(0, &uart_pdata);
> > +	imx51_add_imx_uart(1, &uart_pdata);
> > +	imx51_add_imx_uart(2, &uart_pdata);
> > +}
> > +#else /* !SERIAL_IMX */
> > +static inline void mxc_init_imx_uart(void)
> > +{
> > +}
> > +#endif /* SERIAL_IMX */
> > +
> > +static void __init mxc_board_init(void)
> There are currently 13 functions with this name.  While it is no problem
> for the toolchain as all of them are static, it's bad for people
> searching for it's definition and having to figure out which is the
> right one.  I admit it's not hard, but still I'd prefer if you'd name it
> (say) mx51_efikamx_board_init.

OK.

> > +{
> > +	mxc_iomux_v3_setup_multiple_pads(mx51efikamx_pads,
> > +					ARRAY_SIZE(mx51efikamx_pads));
> > +	mxc_init_imx_uart();
> > +}
> > +
> > +static void __init mx51_efikamx_timer_init(void)
> > +{
> > +	mx51_clocks_init(32768, 24000000, 22579200, 24576000);
> > +}
> > +
> > +static struct sys_timer mxc_timer = {
> > +	.init	= mx51_efikamx_timer_init,
> > +};
> > +
> > +MACHINE_START(MX51_EFIKAMX, "Genesi EfikaMX nettop")
> > +	/* Maintainer: Amit Kucheria <amit.kucheria@linaro.org> */
> > +	.phys_io = MX51_AIPS1_BASE_ADDR,
> > +	.io_pg_offst = ((MX51_AIPS1_BASE_ADDR_VIRT) >> 18) & 0xfffc,
> > +	.boot_params = MX51_PHYS_OFFSET + 0x100,
> Do you need this?  AFAIK it is only needed if the machine comes with an
> old (or broken) bootloader that fails to pass the address to the atag
> list in r2.

TBH, it is boilerplate code that I cut 'n' pasted. Removed now.

> > +	.map_io = mx51_map_io,
> > +	.init_irq = mx51_init_irq,
> > +	.init_machine =  mxc_board_init,
> s/  / /
> 
> > +	.timer = &mxc_timer,
> > +MACHINE_END
> 
> Thanks
> Uwe
> 
> -- 
> Pengutronix e.K.                           | Uwe Kleine-K?nig            |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH 2/3] mx51: efikamx: add otg support

[Amit Kucheria]

Ethernet hangs off OTG

Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
---
 arch/arm/mach-mx5/board-mx51_efikamx.c |   30 ++++++++++++++++++++++++++++++
 1 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mx5/board-mx51_efikamx.c b/arch/arm/mach-mx5/board-mx51_efikamx.c
index 4c921fc..b00502a 100644
--- a/arch/arm/mach-mx5/board-mx51_efikamx.c
+++ b/arch/arm/mach-mx5/board-mx51_efikamx.c
@@ -37,6 +37,8 @@
 #include "devices-imx51.h"
 #include "devices.h"
 
+#define	MX51_USB_PLL_DIV_24_MHZ	0x01
+
 static struct pad_desc mx51efikamx_pads[] = {
 	/* UART1 */
 	MX51_PAD_UART1_RXD__UART1_RXD,
@@ -63,10 +65,38 @@ static inline void mxc_init_imx_uart(void)
 }
 #endif /* SERIAL_IMX */
 
+/* This function is board specific as the bit mask for the plldiv will also
+ * be different for other Freescale SoCs, thus a common bitmask is not
+ * possible and cannot get place in /plat-mxc/ehci.c.
+ */
+static int initialize_otg_port(struct platform_device *pdev)
+{
+	u32 v;
+	void __iomem *usb_base;
+	void __iomem *usbother_base;
+	usb_base = ioremap(MX51_OTG_BASE_ADDR, SZ_4K);
+	usbother_base = (void __iomem *)(usb_base + MX5_USBOTHER_REGS_OFFSET);
+
+	/* Set the PHY clock to 19.2MHz */
+	v = __raw_readl(usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
+	v &= ~MX5_USB_UTMI_PHYCTRL1_PLLDIV_MASK;
+	v |= MX51_USB_PLL_DIV_24_MHZ;
+	__raw_writel(v, usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
+	iounmap(usb_base);
+	return 0;
+}
+
+static struct mxc_usbh_platform_data dr_utmi_config = {
+	.init   = initialize_otg_port,
+	.portsc = MXC_EHCI_UTMI_16BIT,
+	.flags  = MXC_EHCI_INTERNAL_PHY,
+};
+
 static void __init mxc_board_init(void)
 {
 	mxc_iomux_v3_setup_multiple_pads(mx51efikamx_pads,
 					ARRAY_SIZE(mx51efikamx_pads));
+	mxc_register_device(&mxc_usbdr_host_device, &dr_utmi_config);
 	mxc_init_imx_uart();
 }
 
-- 
1.7.0.4

[PATCH 2/3] mx51: efikamx: add otg support

[Uwe Kleine-König]

Hi Amit,

On Thu, Oct 07, 2010 at 03:58:25AM +0300, Amit Kucheria wrote:
> Ethernet hangs off OTG
> 
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> ---
>  arch/arm/mach-mx5/board-mx51_efikamx.c |   30 ++++++++++++++++++++++++++++++
>  1 files changed, 30 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-mx5/board-mx51_efikamx.c b/arch/arm/mach-mx5/board-mx51_efikamx.c
> index 4c921fc..b00502a 100644
> --- a/arch/arm/mach-mx5/board-mx51_efikamx.c
> +++ b/arch/arm/mach-mx5/board-mx51_efikamx.c
> @@ -37,6 +37,8 @@
>  #include "devices-imx51.h"
>  #include "devices.h"
>  
> +#define	MX51_USB_PLL_DIV_24_MHZ	0x01
> +
>  static struct pad_desc mx51efikamx_pads[] = {
>  	/* UART1 */
>  	MX51_PAD_UART1_RXD__UART1_RXD,
> @@ -63,10 +65,38 @@ static inline void mxc_init_imx_uart(void)
>  }
>  #endif /* SERIAL_IMX */
>  
> +/* This function is board specific as the bit mask for the plldiv will also
> + * be different for other Freescale SoCs, thus a common bitmask is not
> + * possible and cannot get place in /plat-mxc/ehci.c.
But maybe a function that removes the boilerplate.  Such that is just
becomes (say):

	imxXX_initialize_otg_port(MX51_USB_PLL_DIV_24_MHZ, maybe, some, more, parameters);

> + */
> +static int initialize_otg_port(struct platform_device *pdev)
> +{
> +	u32 v;
> +	void __iomem *usb_base;
> +	void __iomem *usbother_base;
> +	usb_base = ioremap(MX51_OTG_BASE_ADDR, SZ_4K);
ioremap can fail.

> +	usbother_base = (void __iomem *)(usb_base + MX5_USBOTHER_REGS_OFFSET);
do you need this cast?

> +
> +	/* Set the PHY clock to 19.2MHz */
> +	v = __raw_readl(usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
> +	v &= ~MX5_USB_UTMI_PHYCTRL1_PLLDIV_MASK;
> +	v |= MX51_USB_PLL_DIV_24_MHZ;
> +	__raw_writel(v, usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
> +	iounmap(usb_base);
> +	return 0;
> +}
> +
> +static struct mxc_usbh_platform_data dr_utmi_config = {
> +	.init   = initialize_otg_port,
> +	.portsc = MXC_EHCI_UTMI_16BIT,
> +	.flags  = MXC_EHCI_INTERNAL_PHY,
> +};
> +
>  static void __init mxc_board_init(void)
>  {
>  	mxc_iomux_v3_setup_multiple_pads(mx51efikamx_pads,
>  					ARRAY_SIZE(mx51efikamx_pads));
> +	mxc_register_device(&mxc_usbdr_host_device, &dr_utmi_config);
>  	mxc_init_imx_uart();
>  }
>  
Best regards
Uwe

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

[PATCH 2/3] mx51: efikamx: add otg support

[Sergei Shtylyov]

Hello.

On 07-10-2010 4:58, Amit Kucheria wrote:

> Ethernet hangs off OTG

    Hm, what?

> Signed-off-by: Amit Kucheria<amit.kucheria@linaro.org>
[...]

> diff --git a/arch/arm/mach-mx5/board-mx51_efikamx.c b/arch/arm/mach-mx5/board-mx51_efikamx.c
> index 4c921fc..b00502a 100644
> --- a/arch/arm/mach-mx5/board-mx51_efikamx.c
> +++ b/arch/arm/mach-mx5/board-mx51_efikamx.c
> @@ -37,6 +37,8 @@
>   #include "devices-imx51.h"
>   #include "devices.h"
>
> +#define	MX51_USB_PLL_DIV_24_MHZ	0x01
> +
>   static struct pad_desc mx51efikamx_pads[] = {
>   	/* UART1 */
>   	MX51_PAD_UART1_RXD__UART1_RXD,
> @@ -63,10 +65,38 @@ static inline void mxc_init_imx_uart(void)
>   }
>   #endif /* SERIAL_IMX */
>
> +/* This function is board specific as the bit mask for the plldiv will also
> + * be different for other Freescale SoCs, thus a common bitmask is not
> + * possible and cannot get place in /plat-mxc/ehci.c.
> + */
> +static int initialize_otg_port(struct platform_device *pdev)
> +{
> +	u32 v;
> +	void __iomem *usb_base;
> +	void __iomem *usbother_base;

    Insert an empty line here please.

> +	usb_base = ioremap(MX51_OTG_BASE_ADDR, SZ_4K);
> +	usbother_base = (void __iomem *)(usb_base + MX5_USBOTHER_REGS_OFFSET);

WBR, Sergei

[PATCH 3/3] mx51: Move OTG initialisation for all boards to a single file

[Amit Kucheria]

The OTG initialisation is the same for all MX51 boards currently known. Move
to a common file.

Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
---
 arch/arm/mach-mx5/Makefile             |    2 +-
 arch/arm/mach-mx5/board-cpuimx51.c     |   25 +-------------------
 arch/arm/mach-mx5/board-mx51_babbage.c |   25 +-------------------
 arch/arm/mach-mx5/board-mx51_efikamx.c |   23 +-----------------
 arch/arm/mach-mx5/usb.c                |   40 ++++++++++++++++++++++++++++++++
 5 files changed, 44 insertions(+), 71 deletions(-)
 create mode 100644 arch/arm/mach-mx5/usb.c

diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile
index d1aac9c..1daba15 100644
--- a/arch/arm/mach-mx5/Makefile
+++ b/arch/arm/mach-mx5/Makefile
@@ -3,7 +3,7 @@
 #
 
 # Object file lists.
-obj-y   := cpu.o mm.o clock-mx51.o devices.o
+obj-y   := cpu.o mm.o clock-mx51.o devices.o usb.o
 
 obj-$(CONFIG_MACH_MX51_BABBAGE) += board-mx51_babbage.o
 obj-$(CONFIG_MACH_MX51_3DS) += board-mx51_3ds.o
diff --git a/arch/arm/mach-mx5/board-cpuimx51.c b/arch/arm/mach-mx5/board-cpuimx51.c
index a6c09c7..c54d26e 100644
--- a/arch/arm/mach-mx5/board-cpuimx51.c
+++ b/arch/arm/mach-mx5/board-cpuimx51.c
@@ -56,9 +56,7 @@
 #define MX51_USB_CTRL_1_OFFSET		0x10
 #define MX51_USB_CTRL_UH1_EXT_CLK_EN	(1 << 25)
 
-#define	MX51_USB_PLLDIV_12_MHZ		0x00
-#define	MX51_USB_PLL_DIV_19_2_MHZ	0x01
-#define	MX51_USB_PLL_DIV_24_MHZ		0x02
+extern int initialize_otg_port(struct platform_device *pdev);
 
 #if defined(CONFIG_SERIAL_8250) || defined(CONFIG_SERIAL_8250_MODULE)
 static struct plat_serial8250_port serial_platform_data[] = {
@@ -162,27 +160,6 @@ static struct i2c_board_info eukrea_cpuimx51_i2c_devices[] = {
 	},
 };
 
-/* This function is board specific as the bit mask for the plldiv will also
-be different for other Freescale SoCs, thus a common bitmask is not
-possible and cannot get place in /plat-mxc/ehci.c.*/
-static int initialize_otg_port(struct platform_device *pdev)
-{
-	u32 v;
-	void __iomem *usb_base;
-	void __iomem *usbother_base;
-
-	usb_base = ioremap(MX51_OTG_BASE_ADDR, SZ_4K);
-	usbother_base = usb_base + MX5_USBOTHER_REGS_OFFSET;
-
-	/* Set the PHY clock to 19.2MHz */
-	v = __raw_readl(usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
-	v &= ~MX5_USB_UTMI_PHYCTRL1_PLLDIV_MASK;
-	v |= MX51_USB_PLL_DIV_19_2_MHZ;
-	__raw_writel(v, usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
-	iounmap(usb_base);
-	return 0;
-}
-
 static int initialize_usbh1_port(struct platform_device *pdev)
 {
 	u32 v;
diff --git a/arch/arm/mach-mx5/board-mx51_babbage.c b/arch/arm/mach-mx5/board-mx51_babbage.c
index 7c0b661..cdbbec8 100644
--- a/arch/arm/mach-mx5/board-mx51_babbage.c
+++ b/arch/arm/mach-mx5/board-mx51_babbage.c
@@ -42,9 +42,7 @@
 #define MX51_USB_CTRL_1_OFFSET			0x10
 #define MX51_USB_CTRL_UH1_EXT_CLK_EN		(1 << 25)
 
-#define	MX51_USB_PLLDIV_12_MHZ		0x00
-#define	MX51_USB_PLL_DIV_19_2_MHZ	0x01
-#define	MX51_USB_PLL_DIV_24_MHZ	0x02
+extern int initialize_otg_port(struct platform_device *pdev);
 
 static struct platform_device *devices[] __initdata = {
 	&mxc_fec_device,
@@ -210,27 +208,6 @@ static inline void babbage_fec_reset(void)
 	gpio_set_value(BABBAGE_FEC_PHY_RESET, 1);
 }
 
-/* This function is board specific as the bit mask for the plldiv will also
-be different for other Freescale SoCs, thus a common bitmask is not
-possible and cannot get place in /plat-mxc/ehci.c.*/
-static int initialize_otg_port(struct platform_device *pdev)
-{
-	u32 v;
-	void __iomem *usb_base;
-	void __iomem *usbother_base;
-
-	usb_base = ioremap(MX51_OTG_BASE_ADDR, SZ_4K);
-	usbother_base = usb_base + MX5_USBOTHER_REGS_OFFSET;
-
-	/* Set the PHY clock to 19.2MHz */
-	v = __raw_readl(usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
-	v &= ~MX5_USB_UTMI_PHYCTRL1_PLLDIV_MASK;
-	v |= MX51_USB_PLL_DIV_19_2_MHZ;
-	__raw_writel(v, usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
-	iounmap(usb_base);
-	return 0;
-}
-
 static int initialize_usbh1_port(struct platform_device *pdev)
 {
 	u32 v;
diff --git a/arch/arm/mach-mx5/board-mx51_efikamx.c b/arch/arm/mach-mx5/board-mx51_efikamx.c
index b00502a..93734ae 100644
--- a/arch/arm/mach-mx5/board-mx51_efikamx.c
+++ b/arch/arm/mach-mx5/board-mx51_efikamx.c
@@ -37,7 +37,7 @@
 #include "devices-imx51.h"
 #include "devices.h"
 
-#define	MX51_USB_PLL_DIV_24_MHZ	0x01
+extern int initialize_otg_port(struct platform_device *pdev);
 
 static struct pad_desc mx51efikamx_pads[] = {
 	/* UART1 */
@@ -65,27 +65,6 @@ static inline void mxc_init_imx_uart(void)
 }
 #endif /* SERIAL_IMX */
 
-/* This function is board specific as the bit mask for the plldiv will also
- * be different for other Freescale SoCs, thus a common bitmask is not
- * possible and cannot get place in /plat-mxc/ehci.c.
- */
-static int initialize_otg_port(struct platform_device *pdev)
-{
-	u32 v;
-	void __iomem *usb_base;
-	void __iomem *usbother_base;
-	usb_base = ioremap(MX51_OTG_BASE_ADDR, SZ_4K);
-	usbother_base = (void __iomem *)(usb_base + MX5_USBOTHER_REGS_OFFSET);
-
-	/* Set the PHY clock to 19.2MHz */
-	v = __raw_readl(usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
-	v &= ~MX5_USB_UTMI_PHYCTRL1_PLLDIV_MASK;
-	v |= MX51_USB_PLL_DIV_24_MHZ;
-	__raw_writel(v, usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
-	iounmap(usb_base);
-	return 0;
-}
-
 static struct mxc_usbh_platform_data dr_utmi_config = {
 	.init   = initialize_otg_port,
 	.portsc = MXC_EHCI_UTMI_16BIT,
diff --git a/arch/arm/mach-mx5/usb.c b/arch/arm/mach-mx5/usb.c
new file mode 100644
index 0000000..277f957
--- /dev/null
+++ b/arch/arm/mach-mx5/usb.c
@@ -0,0 +1,40 @@
+/*
+ * Copyright (C) 2010 Linaro Limited
+ *
+ * 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@the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/platform_device.h>
+#include <linux/io.h>
+
+#include <mach/hardware.h>
+#include <mach/mxc_ehci.h>
+
+#define	MX51_USB_PLL_DIV_24_MHZ	0x01
+
+/* This function is SoC specific as the bit mask for the plldiv will also
+ * be different for other Freescale SoCs, thus a common bitmask is not
+ * possible and cannot get place in /plat-mxc/ehci.c.
+ */
+int initialize_otg_port(struct platform_device *pdev)
+{
+	u32 v;
+	void __iomem *usb_base;
+	void __iomem *usbother_base;
+
+	usb_base = ioremap(MX51_OTG_BASE_ADDR, SZ_4K);
+	usbother_base = (void __iomem *)(usb_base + MX5_USBOTHER_REGS_OFFSET);
+
+	/* Set the PHY clock to 24 MHz */
+	v = __raw_readl(usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
+	v &= ~MX5_USB_UTMI_PHYCTRL1_PLLDIV_MASK;
+	v |= MX51_USB_PLL_DIV_24_MHZ;
+	__raw_writel(v, usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
+	iounmap(usb_base);
+	return 0;
+}
-- 
1.7.0.4

[PATCH 3/3] mx51: Move OTG initialisation for all boards to a single file

[Uwe Kleine-König]

On Thu, Oct 07, 2010 at 03:58:48AM +0300, Amit Kucheria wrote:
> The OTG initialisation is the same for all MX51 boards currently known. Move
> to a common file.
> 
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> ---
>  arch/arm/mach-mx5/Makefile             |    2 +-
>  arch/arm/mach-mx5/board-cpuimx51.c     |   25 +-------------------
>  arch/arm/mach-mx5/board-mx51_babbage.c |   25 +-------------------
>  arch/arm/mach-mx5/board-mx51_efikamx.c |   23 +-----------------
>  arch/arm/mach-mx5/usb.c                |   40 ++++++++++++++++++++++++++++++++
>  5 files changed, 44 insertions(+), 71 deletions(-)
>  create mode 100644 arch/arm/mach-mx5/usb.c
> 
> diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile
> index d1aac9c..1daba15 100644
> --- a/arch/arm/mach-mx5/Makefile
> +++ b/arch/arm/mach-mx5/Makefile
> @@ -3,7 +3,7 @@
>  #
>  
>  # Object file lists.
> -obj-y   := cpu.o mm.o clock-mx51.o devices.o
> +obj-y   := cpu.o mm.o clock-mx51.o devices.o usb.o
>  
>  obj-$(CONFIG_MACH_MX51_BABBAGE) += board-mx51_babbage.o
>  obj-$(CONFIG_MACH_MX51_3DS) += board-mx51_3ds.o
> diff --git a/arch/arm/mach-mx5/board-cpuimx51.c b/arch/arm/mach-mx5/board-cpuimx51.c
> index a6c09c7..c54d26e 100644
> --- a/arch/arm/mach-mx5/board-cpuimx51.c
> +++ b/arch/arm/mach-mx5/board-cpuimx51.c
> @@ -56,9 +56,7 @@
>  #define MX51_USB_CTRL_1_OFFSET		0x10
>  #define MX51_USB_CTRL_UH1_EXT_CLK_EN	(1 << 25)
>  
> -#define	MX51_USB_PLLDIV_12_MHZ		0x00
> -#define	MX51_USB_PLL_DIV_19_2_MHZ	0x01
> -#define	MX51_USB_PLL_DIV_24_MHZ		0x02
> +extern int initialize_otg_port(struct platform_device *pdev);
I suggest to put this in a header file.

>  
>  #if defined(CONFIG_SERIAL_8250) || defined(CONFIG_SERIAL_8250_MODULE)
>  static struct plat_serial8250_port serial_platform_data[] = {
> @@ -162,27 +160,6 @@ static struct i2c_board_info eukrea_cpuimx51_i2c_devices[] = {
>  	},
>  };
>  
> -/* This function is board specific as the bit mask for the plldiv will also
> -be different for other Freescale SoCs, thus a common bitmask is not
> -possible and cannot get place in /plat-mxc/ehci.c.*/
> -static int initialize_otg_port(struct platform_device *pdev)
> -{
> -	u32 v;
> -	void __iomem *usb_base;
> -	void __iomem *usbother_base;
> -
> -	usb_base = ioremap(MX51_OTG_BASE_ADDR, SZ_4K);
> -	usbother_base = usb_base + MX5_USBOTHER_REGS_OFFSET;
> -
> -	/* Set the PHY clock to 19.2MHz */
> -	v = __raw_readl(usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
> -	v &= ~MX5_USB_UTMI_PHYCTRL1_PLLDIV_MASK;
> -	v |= MX51_USB_PLL_DIV_19_2_MHZ;
> -	__raw_writel(v, usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
> -	iounmap(usb_base);
> -	return 0;
> -}
> -
>  static int initialize_usbh1_port(struct platform_device *pdev)
>  {
>  	u32 v;
> diff --git a/arch/arm/mach-mx5/board-mx51_babbage.c b/arch/arm/mach-mx5/board-mx51_babbage.c
> index 7c0b661..cdbbec8 100644
> --- a/arch/arm/mach-mx5/board-mx51_babbage.c
> +++ b/arch/arm/mach-mx5/board-mx51_babbage.c
> @@ -42,9 +42,7 @@
>  #define MX51_USB_CTRL_1_OFFSET			0x10
>  #define MX51_USB_CTRL_UH1_EXT_CLK_EN		(1 << 25)
>  
> -#define	MX51_USB_PLLDIV_12_MHZ		0x00
> -#define	MX51_USB_PLL_DIV_19_2_MHZ	0x01
> -#define	MX51_USB_PLL_DIV_24_MHZ	0x02
> +extern int initialize_otg_port(struct platform_device *pdev);
>  
>  static struct platform_device *devices[] __initdata = {
>  	&mxc_fec_device,
> @@ -210,27 +208,6 @@ static inline void babbage_fec_reset(void)
>  	gpio_set_value(BABBAGE_FEC_PHY_RESET, 1);
>  }
>  
> -/* This function is board specific as the bit mask for the plldiv will also
> -be different for other Freescale SoCs, thus a common bitmask is not
> -possible and cannot get place in /plat-mxc/ehci.c.*/
> -static int initialize_otg_port(struct platform_device *pdev)
> -{
> -	u32 v;
> -	void __iomem *usb_base;
> -	void __iomem *usbother_base;
> -
> -	usb_base = ioremap(MX51_OTG_BASE_ADDR, SZ_4K);
> -	usbother_base = usb_base + MX5_USBOTHER_REGS_OFFSET;
> -
> -	/* Set the PHY clock to 19.2MHz */
> -	v = __raw_readl(usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
> -	v &= ~MX5_USB_UTMI_PHYCTRL1_PLLDIV_MASK;
> -	v |= MX51_USB_PLL_DIV_19_2_MHZ;
> -	__raw_writel(v, usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
> -	iounmap(usb_base);
> -	return 0;
> -}
> -
>  static int initialize_usbh1_port(struct platform_device *pdev)
>  {
>  	u32 v;
> diff --git a/arch/arm/mach-mx5/board-mx51_efikamx.c b/arch/arm/mach-mx5/board-mx51_efikamx.c
> index b00502a..93734ae 100644
> --- a/arch/arm/mach-mx5/board-mx51_efikamx.c
> +++ b/arch/arm/mach-mx5/board-mx51_efikamx.c
> @@ -37,7 +37,7 @@
>  #include "devices-imx51.h"
>  #include "devices.h"
>  
> -#define	MX51_USB_PLL_DIV_24_MHZ	0x01
> +extern int initialize_otg_port(struct platform_device *pdev);
>  
>  static struct pad_desc mx51efikamx_pads[] = {
>  	/* UART1 */
> @@ -65,27 +65,6 @@ static inline void mxc_init_imx_uart(void)
>  }
>  #endif /* SERIAL_IMX */
>  
> -/* This function is board specific as the bit mask for the plldiv will also
> - * be different for other Freescale SoCs, thus a common bitmask is not
> - * possible and cannot get place in /plat-mxc/ehci.c.
> - */
> -static int initialize_otg_port(struct platform_device *pdev)
> -{
> -	u32 v;
> -	void __iomem *usb_base;
> -	void __iomem *usbother_base;
> -	usb_base = ioremap(MX51_OTG_BASE_ADDR, SZ_4K);
> -	usbother_base = (void __iomem *)(usb_base + MX5_USBOTHER_REGS_OFFSET);
> -
> -	/* Set the PHY clock to 19.2MHz */
> -	v = __raw_readl(usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
> -	v &= ~MX5_USB_UTMI_PHYCTRL1_PLLDIV_MASK;
> -	v |= MX51_USB_PLL_DIV_24_MHZ;
> -	__raw_writel(v, usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
> -	iounmap(usb_base);
> -	return 0;
> -}
> -
>  static struct mxc_usbh_platform_data dr_utmi_config = {
>  	.init   = initialize_otg_port,
>  	.portsc = MXC_EHCI_UTMI_16BIT,
> diff --git a/arch/arm/mach-mx5/usb.c b/arch/arm/mach-mx5/usb.c
> new file mode 100644
> index 0000000..277f957
> --- /dev/null
> +++ b/arch/arm/mach-mx5/usb.c
> @@ -0,0 +1,40 @@
> +/*
> + * Copyright (C) 2010 Linaro Limited
> + *
> + * 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
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +
> +#include <mach/hardware.h>
> +#include <mach/mxc_ehci.h>
> +
> +#define	MX51_USB_PLL_DIV_24_MHZ	0x01
> +
> +/* This function is SoC specific as the bit mask for the plldiv will also
> + * be different for other Freescale SoCs, thus a common bitmask is not
> + * possible and cannot get place in /plat-mxc/ehci.c.
> + */
> +int initialize_otg_port(struct platform_device *pdev)
> +{
> +	u32 v;
> +	void __iomem *usb_base;
> +	void __iomem *usbother_base;
> +
> +	usb_base = ioremap(MX51_OTG_BASE_ADDR, SZ_4K);
> +	usbother_base = (void __iomem *)(usb_base + MX5_USBOTHER_REGS_OFFSET);
> +
> +	/* Set the PHY clock to 24 MHz */
> +	v = __raw_readl(usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
> +	v &= ~MX5_USB_UTMI_PHYCTRL1_PLLDIV_MASK;
I wonder why a field in a register called MXC_USB_PHY_CTR_FUNC2 is named
MX5_USB_UTMI_PHYCTRL1_PLLDIV.  Usually the register name is a prefix for
the field name?!

> +	v |= MX51_USB_PLL_DIV_24_MHZ;
hmm, babbage used

	> -     v &= ~MX5_USB_UTMI_PHYCTRL1_PLLDIV_MASK;
	> -     v |= MX51_USB_PLL_DIV_19_2_MHZ;

?

Maybe you want to reorder your patches such that your patch 2 comes
after this one, as currently this patch removes ~80% of the code patch 2
introduces.

Otherwise the same comments apply here that I made on patch 2.

Best regards
Uwe

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

[PATCH 3/3] mx51: Move OTG initialisation for all boards to a single file

[Amit Kucheria]

On 10 Oct 07, Uwe Kleine-K?nig wrote:
> On Thu, Oct 07, 2010 at 03:58:48AM +0300, Amit Kucheria wrote:
> > The OTG initialisation is the same for all MX51 boards currently known. Move
> > to a common file.
> > 
> > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> > ---
> >  arch/arm/mach-mx5/Makefile             |    2 +-
> >  arch/arm/mach-mx5/board-cpuimx51.c     |   25 +-------------------
> >  arch/arm/mach-mx5/board-mx51_babbage.c |   25 +-------------------
> >  arch/arm/mach-mx5/board-mx51_efikamx.c |   23 +-----------------
> >  arch/arm/mach-mx5/usb.c                |   40 ++++++++++++++++++++++++++++++++
> >  5 files changed, 44 insertions(+), 71 deletions(-)
> >  create mode 100644 arch/arm/mach-mx5/usb.c
> > 
> > diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile
> > index d1aac9c..1daba15 100644
> > --- a/arch/arm/mach-mx5/Makefile
> > +++ b/arch/arm/mach-mx5/Makefile
> > @@ -3,7 +3,7 @@
> >  #
> >  
> >  # Object file lists.
> > -obj-y   := cpu.o mm.o clock-mx51.o devices.o
> > +obj-y   := cpu.o mm.o clock-mx51.o devices.o usb.o
> >  
> >  obj-$(CONFIG_MACH_MX51_BABBAGE) += board-mx51_babbage.o
> >  obj-$(CONFIG_MACH_MX51_3DS) += board-mx51_3ds.o
> > diff --git a/arch/arm/mach-mx5/board-cpuimx51.c b/arch/arm/mach-mx5/board-cpuimx51.c
> > index a6c09c7..c54d26e 100644
> > --- a/arch/arm/mach-mx5/board-cpuimx51.c
> > +++ b/arch/arm/mach-mx5/board-cpuimx51.c
> > @@ -56,9 +56,7 @@
> >  #define MX51_USB_CTRL_1_OFFSET		0x10
> >  #define MX51_USB_CTRL_UH1_EXT_CLK_EN	(1 << 25)
> >  
> > -#define	MX51_USB_PLLDIV_12_MHZ		0x00
> > -#define	MX51_USB_PLL_DIV_19_2_MHZ	0x01
> > -#define	MX51_USB_PLL_DIV_24_MHZ		0x02
> > +extern int initialize_otg_port(struct platform_device *pdev);
> I suggest to put this in a header file.

OK.

> >  
> >  #if defined(CONFIG_SERIAL_8250) || defined(CONFIG_SERIAL_8250_MODULE)
> >  static struct plat_serial8250_port serial_platform_data[] = {
> > @@ -162,27 +160,6 @@ static struct i2c_board_info eukrea_cpuimx51_i2c_devices[] = {
> >  	},
> >  };
> >  
> > -/* This function is board specific as the bit mask for the plldiv will also
> > -be different for other Freescale SoCs, thus a common bitmask is not
> > -possible and cannot get place in /plat-mxc/ehci.c.*/
> > -static int initialize_otg_port(struct platform_device *pdev)
> > -{
> > -	u32 v;
> > -	void __iomem *usb_base;
> > -	void __iomem *usbother_base;
> > -
> > -	usb_base = ioremap(MX51_OTG_BASE_ADDR, SZ_4K);
> > -	usbother_base = usb_base + MX5_USBOTHER_REGS_OFFSET;
> > -
> > -	/* Set the PHY clock to 19.2MHz */
> > -	v = __raw_readl(usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
> > -	v &= ~MX5_USB_UTMI_PHYCTRL1_PLLDIV_MASK;
> > -	v |= MX51_USB_PLL_DIV_19_2_MHZ;
> > -	__raw_writel(v, usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
> > -	iounmap(usb_base);
> > -	return 0;
> > -}
> > -
> >  static int initialize_usbh1_port(struct platform_device *pdev)
> >  {
> >  	u32 v;
> > diff --git a/arch/arm/mach-mx5/board-mx51_babbage.c b/arch/arm/mach-mx5/board-mx51_babbage.c
> > index 7c0b661..cdbbec8 100644
> > --- a/arch/arm/mach-mx5/board-mx51_babbage.c
> > +++ b/arch/arm/mach-mx5/board-mx51_babbage.c
> > @@ -42,9 +42,7 @@
> >  #define MX51_USB_CTRL_1_OFFSET			0x10
> >  #define MX51_USB_CTRL_UH1_EXT_CLK_EN		(1 << 25)
> >  
> > -#define	MX51_USB_PLLDIV_12_MHZ		0x00
> > -#define	MX51_USB_PLL_DIV_19_2_MHZ	0x01
> > -#define	MX51_USB_PLL_DIV_24_MHZ	0x02
> > +extern int initialize_otg_port(struct platform_device *pdev);
> >  
> >  static struct platform_device *devices[] __initdata = {
> >  	&mxc_fec_device,
> > @@ -210,27 +208,6 @@ static inline void babbage_fec_reset(void)
> >  	gpio_set_value(BABBAGE_FEC_PHY_RESET, 1);
> >  }
> >  
> > -/* This function is board specific as the bit mask for the plldiv will also
> > -be different for other Freescale SoCs, thus a common bitmask is not
> > -possible and cannot get place in /plat-mxc/ehci.c.*/
> > -static int initialize_otg_port(struct platform_device *pdev)
> > -{
> > -	u32 v;
> > -	void __iomem *usb_base;
> > -	void __iomem *usbother_base;
> > -
> > -	usb_base = ioremap(MX51_OTG_BASE_ADDR, SZ_4K);
> > -	usbother_base = usb_base + MX5_USBOTHER_REGS_OFFSET;
> > -
> > -	/* Set the PHY clock to 19.2MHz */
> > -	v = __raw_readl(usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
> > -	v &= ~MX5_USB_UTMI_PHYCTRL1_PLLDIV_MASK;
> > -	v |= MX51_USB_PLL_DIV_19_2_MHZ;
> > -	__raw_writel(v, usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
> > -	iounmap(usb_base);
> > -	return 0;
> > -}
> > -
> >  static int initialize_usbh1_port(struct platform_device *pdev)
> >  {
> >  	u32 v;
> > diff --git a/arch/arm/mach-mx5/board-mx51_efikamx.c b/arch/arm/mach-mx5/board-mx51_efikamx.c
> > index b00502a..93734ae 100644
> > --- a/arch/arm/mach-mx5/board-mx51_efikamx.c
> > +++ b/arch/arm/mach-mx5/board-mx51_efikamx.c
> > @@ -37,7 +37,7 @@
> >  #include "devices-imx51.h"
> >  #include "devices.h"
> >  
> > -#define	MX51_USB_PLL_DIV_24_MHZ	0x01
> > +extern int initialize_otg_port(struct platform_device *pdev);
> >  
> >  static struct pad_desc mx51efikamx_pads[] = {
> >  	/* UART1 */
> > @@ -65,27 +65,6 @@ static inline void mxc_init_imx_uart(void)
> >  }
> >  #endif /* SERIAL_IMX */
> >  
> > -/* This function is board specific as the bit mask for the plldiv will also
> > - * be different for other Freescale SoCs, thus a common bitmask is not
> > - * possible and cannot get place in /plat-mxc/ehci.c.
> > - */
> > -static int initialize_otg_port(struct platform_device *pdev)
> > -{
> > -	u32 v;
> > -	void __iomem *usb_base;
> > -	void __iomem *usbother_base;
> > -	usb_base = ioremap(MX51_OTG_BASE_ADDR, SZ_4K);
> > -	usbother_base = (void __iomem *)(usb_base + MX5_USBOTHER_REGS_OFFSET);
> > -
> > -	/* Set the PHY clock to 19.2MHz */
> > -	v = __raw_readl(usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
> > -	v &= ~MX5_USB_UTMI_PHYCTRL1_PLLDIV_MASK;
> > -	v |= MX51_USB_PLL_DIV_24_MHZ;
> > -	__raw_writel(v, usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
> > -	iounmap(usb_base);
> > -	return 0;
> > -}
> > -
> >  static struct mxc_usbh_platform_data dr_utmi_config = {
> >  	.init   = initialize_otg_port,
> >  	.portsc = MXC_EHCI_UTMI_16BIT,
> > diff --git a/arch/arm/mach-mx5/usb.c b/arch/arm/mach-mx5/usb.c
> > new file mode 100644
> > index 0000000..277f957
> > --- /dev/null
> > +++ b/arch/arm/mach-mx5/usb.c
> > @@ -0,0 +1,40 @@
> > +/*
> > + * Copyright (C) 2010 Linaro Limited
> > + *
> > + * 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
> > + */
> > +
> > +#include <linux/platform_device.h>
> > +#include <linux/io.h>
> > +
> > +#include <mach/hardware.h>
> > +#include <mach/mxc_ehci.h>
> > +
> > +#define	MX51_USB_PLL_DIV_24_MHZ	0x01
> > +
> > +/* This function is SoC specific as the bit mask for the plldiv will also
> > + * be different for other Freescale SoCs, thus a common bitmask is not
> > + * possible and cannot get place in /plat-mxc/ehci.c.
> > + */
> > +int initialize_otg_port(struct platform_device *pdev)
> > +{
> > +	u32 v;
> > +	void __iomem *usb_base;
> > +	void __iomem *usbother_base;
> > +
> > +	usb_base = ioremap(MX51_OTG_BASE_ADDR, SZ_4K);
> > +	usbother_base = (void __iomem *)(usb_base + MX5_USBOTHER_REGS_OFFSET);
> > +
> > +	/* Set the PHY clock to 24 MHz */
> > +	v = __raw_readl(usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
> > +	v &= ~MX5_USB_UTMI_PHYCTRL1_PLLDIV_MASK;
> I wonder why a field in a register called MXC_USB_PHY_CTR_FUNC2 is named
> MX5_USB_UTMI_PHYCTRL1_PLLDIV.  Usually the register name is a prefix for
> the field name?!

I have a patch renaming the various bit-fields and registers to conform to
the reference manual. I'll post it soon.

> > +	v |= MX51_USB_PLL_DIV_24_MHZ;
> hmm, babbage used

> 	> -     v &= ~MX5_USB_UTMI_PHYCTRL1_PLLDIV_MASK;
> 	> -     v |= MX51_USB_PLL_DIV_19_2_MHZ;
> 
> ?

Good catch :)

But if you look at the reference manual, table 60-6, 19.2 MHz would use a
value of 0x0, but the actual code uses a value of 0x1 that corresponds to
24MHz. 19.2MHz (0x0) doesn't even work for me.

> Maybe you want to reorder your patches such that your patch 2 comes
> after this one, as currently this patch removes ~80% of the code patch 2
> introduces.

I did it to make bisection easier. I didn't want to add a new board feature
and do consolidation in a single patch.

> Otherwise the same comments apply here that I made on patch 2.

Thanks for the review.

Will send out updates.

[PATCH 3/3] mx51: Move OTG initialisation for all boards to a single file

[Uwe Kleine-König]

Hi Amit,

On Thu, Oct 07, 2010 at 03:04:59PM +0300, Amit Kucheria wrote:
> On 10 Oct 07, Uwe Kleine-K?nig wrote:
> > On Thu, Oct 07, 2010 at 03:58:48AM +0300, Amit Kucheria wrote:
> > > +	/* Set the PHY clock to 24 MHz */
> > > +	v = __raw_readl(usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
> > > +	v &= ~MX5_USB_UTMI_PHYCTRL1_PLLDIV_MASK;
> > I wonder why a field in a register called MXC_USB_PHY_CTR_FUNC2 is named
> > MX5_USB_UTMI_PHYCTRL1_PLLDIV.  Usually the register name is a prefix for
> > the field name?!
> 
> I have a patch renaming the various bit-fields and registers to conform to
> the reference manual. I'll post it soon.
great

> > > +	v |= MX51_USB_PLL_DIV_24_MHZ;
> > hmm, babbage used
> 
> > 	> -     v &= ~MX5_USB_UTMI_PHYCTRL1_PLLDIV_MASK;
> > 	> -     v |= MX51_USB_PLL_DIV_19_2_MHZ;
> > 
> > ?
> 
> Good catch :)
> 
> But if you look at the reference manual, table 60-6, 19.2 MHz would use a
> value of 0x0, but the actual code uses a value of 0x1 that corresponds to
> 24MHz. 19.2MHz (0x0) doesn't even work for me.
Then maybe note it in the commit log?!

> > Maybe you want to reorder your patches such that your patch 2 comes
> > after this one, as currently this patch removes ~80% of the code patch 2
> > introduces.
> 
> I did it to make bisection easier. I didn't want to add a new board feature
> and do consolidation in a single patch.
Yep, that is ok.  Currently you have:

	1/3: add board support for iforgotthename
	2/3: add usb support for iforgotthename
	3/3: consolidate usb support

I suggested to do 1, 3, 2 instead.  Thinking again 3, 1+2 might even be
better.

Best regards
Uwe

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

[PATCH 3/3] mx51: Move OTG initialisation for all boards to a single file

[Sascha Hauer]

Added Daniel to Cc as he might say something here aswell.

On Thu, Oct 07, 2010 at 03:58:48AM +0300, Amit Kucheria wrote:
> The OTG initialisation is the same for all MX51 boards currently known. Move
> to a common file.
> 
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> ---
>  arch/arm/mach-mx5/Makefile             |    2 +-
>  arch/arm/mach-mx5/board-cpuimx51.c     |   25 +-------------------
>  arch/arm/mach-mx5/board-mx51_babbage.c |   25 +-------------------
>  arch/arm/mach-mx5/board-mx51_efikamx.c |   23 +-----------------
>  arch/arm/mach-mx5/usb.c                |   40 ++++++++++++++++++++++++++++++++
>  5 files changed, 44 insertions(+), 71 deletions(-)
>  create mode 100644 arch/arm/mach-mx5/usb.c
> 
> diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile
> index d1aac9c..1daba15 100644
> --- a/arch/arm/mach-mx5/Makefile
> +++ b/arch/arm/mach-mx5/Makefile
> @@ -3,7 +3,7 @@
>  #
>

[snip]

>  {
>  	u32 v;
> diff --git a/arch/arm/mach-mx5/board-mx51_efikamx.c b/arch/arm/mach-mx5/board-mx51_efikamx.c
> index b00502a..93734ae 100644
> --- a/arch/arm/mach-mx5/board-mx51_efikamx.c
> +++ b/arch/arm/mach-mx5/board-mx51_efikamx.c
> @@ -37,7 +37,7 @@
>  #include "devices-imx51.h"
>  #include "devices.h"
>  
> -#define	MX51_USB_PLL_DIV_24_MHZ	0x01
> +extern int initialize_otg_port(struct platform_device *pdev);
>  
>  static struct pad_desc mx51efikamx_pads[] = {
>  	/* UART1 */
> @@ -65,27 +65,6 @@ static inline void mxc_init_imx_uart(void)
>  }
>  #endif /* SERIAL_IMX */
>  
> -/* This function is board specific as the bit mask for the plldiv will also
> - * be different for other Freescale SoCs, thus a common bitmask is not
> - * possible and cannot get place in /plat-mxc/ehci.c.
> - */
> -static int initialize_otg_port(struct platform_device *pdev)
> -{
> -	u32 v;
> -	void __iomem *usb_base;
> -	void __iomem *usbother_base;
> -	usb_base = ioremap(MX51_OTG_BASE_ADDR, SZ_4K);
> -	usbother_base = (void __iomem *)(usb_base + MX5_USBOTHER_REGS_OFFSET);
> -
> -	/* Set the PHY clock to 19.2MHz */
> -	v = __raw_readl(usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
> -	v &= ~MX5_USB_UTMI_PHYCTRL1_PLLDIV_MASK;
> -	v |= MX51_USB_PLL_DIV_24_MHZ;
> -	__raw_writel(v, usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
> -	iounmap(usb_base);
> -	return 0;
> -}
> -
>  static struct mxc_usbh_platform_data dr_utmi_config = {
>  	.init   = initialize_otg_port,
>  	.portsc = MXC_EHCI_UTMI_16BIT,
> diff --git a/arch/arm/mach-mx5/usb.c b/arch/arm/mach-mx5/usb.c
> new file mode 100644
> index 0000000..277f957
> --- /dev/null
> +++ b/arch/arm/mach-mx5/usb.c
> @@ -0,0 +1,40 @@
> +/*
> + * Copyright (C) 2010 Linaro Limited
> + *
> + * 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
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +
> +#include <mach/hardware.h>
> +#include <mach/mxc_ehci.h>
> +
> +#define	MX51_USB_PLL_DIV_24_MHZ	0x01
> +
> +/* This function is SoC specific as the bit mask for the plldiv will also
> + * be different for other Freescale SoCs, thus a common bitmask is not
> + * possible and cannot get place in /plat-mxc/ehci.c.
> + */
> +int initialize_otg_port(struct platform_device *pdev)
> +{

First of all please do not introduce global functions without the
correct prefix, mx51_ in this case.

I have recently looked at the way the USB phy settings are handled on
i.MX and it's coming to its limits. Currently the phy settings are coded
into generic flags, passed to the usb driver which then calls
mxc_initialize_usb_hw() which dispatches the different SoCs and translates
the generic flags back into SoC specific ones.  While this was doable
until now it fails badly on i.MX51 as we see here.
The phy settings are totally static to a given board, so we could simply
let the board call a mx51_init_usb_phy() which gets a pointer to a
struct with all relevant register settings. And we could pass this
function SoC specific register settings without having to encode them
into generic flags and decode them again later.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 3/3] mx51: Move OTG initialisation for all boards to a single file

[Fabio Estevam]

Hi Amit,

>  +?? void __iomem *usbother_base;
>  +??? usb_base =  ioremap(MX51_OTG_BASE_ADDR, SZ_4K);

Shouldn't you check whether ioremap failed?

Regards,

Fabio Estevam



      

[PATCH 3/3] mx51: Move OTG initialisation for all boards to a single file

[Amit Kucheria]

On Thu, Oct 7, 2010 at 10:48 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> Added Daniel to Cc as he might say something here aswell.
>
> On Thu, Oct 07, 2010 at 03:58:48AM +0300, Amit Kucheria wrote:
>> The OTG initialisation is the same for all MX51 boards currently known. Move
>> to a common file.
>>
>> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
>> ---
>> ?arch/arm/mach-mx5/Makefile ? ? ? ? ? ? | ? ?2 +-
>> ?arch/arm/mach-mx5/board-cpuimx51.c ? ? | ? 25 +-------------------
>> ?arch/arm/mach-mx5/board-mx51_babbage.c | ? 25 +-------------------
>> ?arch/arm/mach-mx5/board-mx51_efikamx.c | ? 23 +-----------------
>> ?arch/arm/mach-mx5/usb.c ? ? ? ? ? ? ? ?| ? 40 ++++++++++++++++++++++++++++++++
>> ?5 files changed, 44 insertions(+), 71 deletions(-)
>> ?create mode 100644 arch/arm/mach-mx5/usb.c
>>
>> diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile
>> index d1aac9c..1daba15 100644
>> --- a/arch/arm/mach-mx5/Makefile
>> +++ b/arch/arm/mach-mx5/Makefile
>> @@ -3,7 +3,7 @@
>> ?#
>>
>
> [snip]
>
>> ?{
>> ? ? ? u32 v;
>> diff --git a/arch/arm/mach-mx5/board-mx51_efikamx.c b/arch/arm/mach-mx5/board-mx51_efikamx.c
>> index b00502a..93734ae 100644
>> --- a/arch/arm/mach-mx5/board-mx51_efikamx.c
>> +++ b/arch/arm/mach-mx5/board-mx51_efikamx.c
>> @@ -37,7 +37,7 @@
>> ?#include "devices-imx51.h"
>> ?#include "devices.h"
>>
>> -#define ? ? ?MX51_USB_PLL_DIV_24_MHZ 0x01
>> +extern int initialize_otg_port(struct platform_device *pdev);
>>
>> ?static struct pad_desc mx51efikamx_pads[] = {
>> ? ? ? /* UART1 */
>> @@ -65,27 +65,6 @@ static inline void mxc_init_imx_uart(void)
>> ?}
>> ?#endif /* SERIAL_IMX */
>>
>> -/* This function is board specific as the bit mask for the plldiv will also
>> - * be different for other Freescale SoCs, thus a common bitmask is not
>> - * possible and cannot get place in /plat-mxc/ehci.c.
>> - */
>> -static int initialize_otg_port(struct platform_device *pdev)
>> -{
>> - ? ? u32 v;
>> - ? ? void __iomem *usb_base;
>> - ? ? void __iomem *usbother_base;
>> - ? ? usb_base = ioremap(MX51_OTG_BASE_ADDR, SZ_4K);
>> - ? ? usbother_base = (void __iomem *)(usb_base + MX5_USBOTHER_REGS_OFFSET);
>> -
>> - ? ? /* Set the PHY clock to 19.2MHz */
>> - ? ? v = __raw_readl(usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
>> - ? ? v &= ~MX5_USB_UTMI_PHYCTRL1_PLLDIV_MASK;
>> - ? ? v |= MX51_USB_PLL_DIV_24_MHZ;
>> - ? ? __raw_writel(v, usbother_base + MXC_USB_PHY_CTR_FUNC2_OFFSET);
>> - ? ? iounmap(usb_base);
>> - ? ? return 0;
>> -}
>> -
>> ?static struct mxc_usbh_platform_data dr_utmi_config = {
>> ? ? ? .init ? = initialize_otg_port,
>> ? ? ? .portsc = MXC_EHCI_UTMI_16BIT,
>> diff --git a/arch/arm/mach-mx5/usb.c b/arch/arm/mach-mx5/usb.c
>> new file mode 100644
>> index 0000000..277f957
>> --- /dev/null
>> +++ b/arch/arm/mach-mx5/usb.c
>> @@ -0,0 +1,40 @@
>> +/*
>> + * Copyright (C) 2010 Linaro Limited
>> + *
>> + * 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
>> + */
>> +
>> +#include <linux/platform_device.h>
>> +#include <linux/io.h>
>> +
>> +#include <mach/hardware.h>
>> +#include <mach/mxc_ehci.h>
>> +
>> +#define ? ? ?MX51_USB_PLL_DIV_24_MHZ 0x01
>> +
>> +/* This function is SoC specific as the bit mask for the plldiv will also
>> + * be different for other Freescale SoCs, thus a common bitmask is not
>> + * possible and cannot get place in /plat-mxc/ehci.c.
>> + */
>> +int initialize_otg_port(struct platform_device *pdev)
>> +{
>
> First of all please do not introduce global functions without the
> correct prefix, mx51_ in this case.
>
> I have recently looked at the way the USB phy settings are handled on
> i.MX and it's coming to its limits. Currently the phy settings are coded
> into generic flags, passed to the usb driver which then calls
> mxc_initialize_usb_hw() which dispatches the different SoCs and translates
> the generic flags back into SoC specific ones. ?While this was doable
> until now it fails badly on i.MX51 as we see here.
> The phy settings are totally static to a given board, so we could simply
> let the board call a mx51_init_usb_phy() which gets a pointer to a
> struct with all relevant register settings. And we could pass this
> function SoC specific register settings without having to encode them
> into generic flags and decode them again later.

OK, working on a patch for mx51 now. It it looks ok, I can convert the rest.

/Amit

[PATCH 3/3] mx51: Move OTG initialisation for all boards to a single file

[Amit Kucheria]

On Thu, Oct 7, 2010 at 10:48 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> Added Daniel to Cc as he might say something here aswell.

<snip>

> I have recently looked at the way the USB phy settings are handled on
> i.MX and it's coming to its limits. Currently the phy settings are coded
> into generic flags, passed to the usb driver which then calls
> mxc_initialize_usb_hw() which dispatches the different SoCs and translates
> the generic flags back into SoC specific ones. ?While this was doable
> until now it fails badly on i.MX51 as we see here.
> The phy settings are totally static to a given board, so we could simply
> let the board call a mx51_init_usb_phy() which gets a pointer to a
> struct with all relevant register settings. And we could pass this
> function SoC specific register settings without having to encode them
> into generic flags and decode them again later.
>
> Sascha

Sascha,

Do you think something like this will be better? It looks even more
scattered to me, the usb configuration information is scattered across
platform_data and phy_data.

Or did you want to get rid of platform_data->init too from the driver?

Regards,
Amit
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-mxc-Stop-passing-usb-phy-configuration-to-the-ehci-d.patch
Type: text/x-patch
Size: 9144 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20101011/0d8b79a1/attachment.bin>

[PATCH 3/3] mx51: Move OTG initialisation for all boards to a single file

[Sascha Hauer]

Hi Amit,

On Mon, Oct 11, 2010 at 01:34:30PM +0300, Amit Kucheria wrote:
> On Thu, Oct 7, 2010 at 10:48 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > Added Daniel to Cc as he might say something here aswell.
> 
> <snip>
> 
> > I have recently looked at the way the USB phy settings are handled on
> > i.MX and it's coming to its limits. Currently the phy settings are coded
> > into generic flags, passed to the usb driver which then calls
> > mxc_initialize_usb_hw() which dispatches the different SoCs and translates
> > the generic flags back into SoC specific ones. ?While this was doable
> > until now it fails badly on i.MX51 as we see here.
> > The phy settings are totally static to a given board, so we could simply
> > let the board call a mx51_init_usb_phy() which gets a pointer to a
> > struct with all relevant register settings. And we could pass this
> > function SoC specific register settings without having to encode them
> > into generic flags and decode them again later.
> >
> > Sascha
> 
> Sascha,
> 
> Do you think something like this will be better? It looks even more
> scattered to me, the usb configuration information is scattered across
> platform_data and phy_data.

My problem is not that it's scattered around platform_data and phy_data,
but that it's not possible to add proper i.MX51 support without
introducing a initialize_otg_port which wont' be flexible enough for
future boards.

> 
> Or did you want to get rid of platform_data->init too from the driver?

No, I think we can't do this without breaking at least some boards. I
found out that on some boards we have to configure the iomux pins and
the rest of the USB hardware in a short time frame, otherwise we get
ULPI errors.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 3/3] mx51: Move OTG initialisation for all boards to a single file

[Amit Kucheria]

On Mon, Oct 11, 2010 at 4:10 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
>>
>> Do you think something like this will be better? It looks even more
>> scattered to me, the usb configuration information is scattered across
>> platform_data and phy_data.
>
> My problem is not that it's scattered around platform_data and phy_data,
> but that it's not possible to add proper i.MX51 support without
> introducing a initialize_otg_port which wont' be flexible enough for
> future boards.
>
>>
>> Or did you want to get rid of platform_data->init too from the driver?
>
> No, I think we can't do this without breaking at least some boards. I
> found out that on some boards we have to configure the iomux pins and
> the rest of the USB hardware in a short time frame, otherwise we get
> ULPI errors.
>
> Sascha

So are the original patches(v2) to consolidate initialize_otg_port()
and introduce efikamx board support acceptable?

(I want to ensure I don't miss the next merge window, because Debian
wants to use these machines as their build machines)