[PATCH v2] ARM: mxs: Change duart device to use amba-pl011
From: Uwe Kleine-König <hidden>
Date: 2010-12-29 09:12:47
Hello Shawn, just a few thought that came to my mind when reading your patch ... On Tue, Dec 28, 2010 at 11:23:14PM +0800, Shawn Guo wrote:
quoted hunk ↗ jump to hunk
The mxs duart is actually an amba-pl011 device. This commit changes the duart device code to dynamically allocate amba-pl011 device, so that drivers/serial/amba-pl011.c can be used on mxs. Signed-off-by: Shawn Guo <redacted> --- Changes for v2: - Add #include <asm/irq.h> into amba-duart.c. When cleaning headers includsion in other files, compiler start complaining NO_IRQ not defined, so add the inclusion to fix it. - Change MXS_AMBA_DEVICE to MXS_AMBA_DUART_DEVICE arch/arm/mach-mxs/Kconfig | 6 ++- arch/arm/mach-mxs/clock-mx23.c | 2 +- arch/arm/mach-mxs/clock-mx28.c | 2 +- arch/arm/mach-mxs/devices-mx23.h | 4 +- arch/arm/mach-mxs/devices-mx28.h | 4 +- arch/arm/mach-mxs/devices/Kconfig | 2 +- arch/arm/mach-mxs/devices/Makefile | 2 +- arch/arm/mach-mxs/devices/amba-duart.c | 40 +++++++++++++++++++ arch/arm/mach-mxs/devices/platform-duart.c | 48 ----------------------- arch/arm/mach-mxs/include/mach/devices-common.h | 9 +--- 10 files changed, 54 insertions(+), 65 deletions(-) create mode 100644 arch/arm/mach-mxs/devices/amba-duart.c delete mode 100644 arch/arm/mach-mxs/devices/platform-duart.cdiff --git a/arch/arm/mach-mxs/Kconfig b/arch/arm/mach-mxs/Kconfig index c4ac7b4..0d072e9 100644 --- a/arch/arm/mach-mxs/Kconfig +++ b/arch/arm/mach-mxs/Kconfig@@ -5,17 +5,19 @@ source "arch/arm/mach-mxs/devices/Kconfig" config SOC_IMX23 bool select CPU_ARM926T + select ARM_AMBA config SOC_IMX28 bool select CPU_ARM926T + select ARM_AMBA
Maybe let MXS_HAVE_AMBA_DUART let select ARM_AMBA?
quoted hunk ↗ jump to hunk
comment "MXS platforms:" config MACH_MX23EVK bool "Support MX23EVK Platform" select SOC_IMX23 - select MXS_HAVE_PLATFORM_DUART + select MXS_HAVE_AMBA_DUART default y help Include support for MX23EVK platform. This includes specific@@ -24,7 +26,7 @@ config MACH_MX23EVK config MACH_MX28EVK bool "Support MX28EVK Platform" select SOC_IMX28 - select MXS_HAVE_PLATFORM_DUART + select MXS_HAVE_AMBA_DUART select MXS_HAVE_PLATFORM_FEC default y helpdiff --git a/arch/arm/mach-mxs/clock-mx23.c b/arch/arm/mach-mxs/clock-mx23.c index 8f5a19a..0f65d15 100644 --- a/arch/arm/mach-mxs/clock-mx23.c +++ b/arch/arm/mach-mxs/clock-mx23.c@@ -437,7 +437,7 @@ _DEFINE_CLOCK(clk32k_clk, XTAL, TIMROT_CLK32K_GATE, &ref_xtal_clk); }, static struct clk_lookup lookups[] = { - _REGISTER_CLOCK("mxs-duart.0", NULL, uart_clk) + _REGISTER_CLOCK("uart", NULL, uart_clk)
hmm, I still don't like that. What do you think about having an alias for uart_clk that is named "duart" or similar?
quoted hunk ↗ jump to hunk
_REGISTER_CLOCK("rtc", NULL, rtc_clk) _REGISTER_CLOCK(NULL, "hclk", hbus_clk) _REGISTER_CLOCK(NULL, "xclk", xbus_clk)diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c index 74e2103..dd6d158 100644 --- a/arch/arm/mach-mxs/clock-mx28.c +++ b/arch/arm/mach-mxs/clock-mx28.c@@ -602,7 +602,7 @@ _DEFINE_CLOCK(fec_clk, ENET, DISABLE, &hbus_clk); }, static struct clk_lookup lookups[] = { - _REGISTER_CLOCK("mxs-duart.0", NULL, uart_clk) + _REGISTER_CLOCK("uart", NULL, uart_clk) _REGISTER_CLOCK("fec.0", NULL, fec_clk) _REGISTER_CLOCK("rtc", NULL, rtc_clk) _REGISTER_CLOCK("pll2", NULL, pll2_clk)diff --git a/arch/arm/mach-mxs/devices-mx23.h b/arch/arm/mach-mxs/devices-mx23.h index d0f49fc..36397ff 100644 --- a/arch/arm/mach-mxs/devices-mx23.h +++ b/arch/arm/mach-mxs/devices-mx23.h@@ -11,6 +11,6 @@ #include <mach/mx23.h> #include <mach/devices-common.h> -extern const struct mxs_duart_data mx23_duart_data __initconst; +extern struct amba_device mx23_duart_device; #define mx23_add_duart() \ - mxs_add_duart(&mx23_duart_data) + mxs_add_duart(&mx23_duart_device)diff --git a/arch/arm/mach-mxs/devices-mx28.h b/arch/arm/mach-mxs/devices-mx28.h index 00b736c..e84a7fe 100644 --- a/arch/arm/mach-mxs/devices-mx28.h +++ b/arch/arm/mach-mxs/devices-mx28.h@@ -11,9 +11,9 @@ #include <mach/mx28.h> #include <mach/devices-common.h> -extern const struct mxs_duart_data mx28_duart_data __initconst; +extern struct amba_device mx28_duart_device; #define mx28_add_duart() \ - mxs_add_duart(&mx28_duart_data) + mxs_add_duart(&mx28_duart_device) extern const struct mxs_fec_data mx28_fec_data[] __initconst; #define mx28_add_fec(id, pdata) \diff --git a/arch/arm/mach-mxs/devices/Kconfig b/arch/arm/mach-mxs/devices/Kconfig index a35a2dc..3dff25c 100644 --- a/arch/arm/mach-mxs/devices/Kconfig +++ b/arch/arm/mach-mxs/devices/Kconfig@@ -1,4 +1,4 @@ -config MXS_HAVE_PLATFORM_DUART +config MXS_HAVE_AMBA_DUART bool config MXS_HAVE_PLATFORM_FECdiff --git a/arch/arm/mach-mxs/devices/Makefile b/arch/arm/mach-mxs/devices/Makefile index 4b5266a..d0a09f6 100644 --- a/arch/arm/mach-mxs/devices/Makefile +++ b/arch/arm/mach-mxs/devices/Makefile@@ -1,2 +1,2 @@ -obj-$(CONFIG_MXS_HAVE_PLATFORM_DUART) += platform-duart.o +obj-$(CONFIG_MXS_HAVE_AMBA_DUART) += amba-duart.o obj-$(CONFIG_MXS_HAVE_PLATFORM_FEC) += platform-fec.odiff --git a/arch/arm/mach-mxs/devices/amba-duart.c b/arch/arm/mach-mxs/devices/amba-duart.c new file mode 100644 index 0000000..aa77d28 --- /dev/null +++ b/arch/arm/mach-mxs/devices/amba-duart.c@@ -0,0 +1,40 @@ +/* + * Copyright (C) 2009-2010 Pengutronix + * Uwe Kleine-Koenig <u.kleine-koenig@pengutronix.de> + * + * 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 version 2 as published by the + * Free Software Foundation. + */ +#include <asm/irq.h> +#include <mach/mx23.h> +#include <mach/mx28.h> +#include <mach/devices-common.h> + +#define MXS_AMBA_DUART_DEVICE(name, soc) \ +struct amba_device name##_device = { \ + .dev = { \ + .init_name = "uart", \ + }, \ + .res = { \ + .start = soc ## _DUART_BASE_ADDR, \ + .end = (soc ## _DUART_BASE_ADDR) + SZ_8K - 1, \ + .flags = IORESOURCE_MEM, \ + }, \ + .irq = {soc ## _INT_DUART, NO_IRQ}, \ +} + +#ifdef CONFIG_SOC_IMX23 +MXS_AMBA_DUART_DEVICE(mx23_duart, MX23); +#endif + +#ifdef CONFIG_SOC_IMX28 +MXS_AMBA_DUART_DEVICE(mx28_duart, MX28); +#endif + +int __init mxs_add_duart(struct amba_device *dev) +{ + return amba_device_register(dev, &iomem_resource); +}
This doesn't have the benefits of the construct for platform devices. (i.e. mx23_duart_device isn't marked with __initdata (or __initconst) and so occupies memory even when the kernel runs on an i.MX28.) At least making it __initconst and then do struct amba_device *adev = kmalloc(sizeof(*adev)); *adev = *dev return amba_device_register(adev, ...); (plus some error checking) would be nice. Maybe put that into a wrapper similar to mxs_add_platform_device? I didn't check the size of a struct ambadevice, so I cannot tell if it's worth to create an extra struct to just carry the relevant data. Best regards Uwe
quoted hunk ↗ jump to hunk
diff --git a/arch/arm/mach-mxs/devices/platform-duart.c b/arch/arm/mach-mxs/devices/platform-duart.c deleted file mode 100644 index 2fe0df5..0000000 --- a/arch/arm/mach-mxs/devices/platform-duart.c +++ /dev/null@@ -1,48 +0,0 @@ -/* - * Copyright (C) 2009-2010 Pengutronix - * Uwe Kleine-Koenig <u.kleine-koenig@pengutronix.de> - * - * 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 version 2 as published by the - * Free Software Foundation. - */ -#include <mach/mx23.h> -#include <mach/mx28.h> -#include <mach/devices-common.h> - -#define mxs_duart_data_entry(soc) \ - { \ - .iobase = soc ## _DUART_BASE_ADDR, \ - .irq = soc ## _INT_DUART, \ - } - -#ifdef CONFIG_SOC_IMX23 -const struct mxs_duart_data mx23_duart_data __initconst = - mxs_duart_data_entry(MX23); -#endif - -#ifdef CONFIG_SOC_IMX28 -const struct mxs_duart_data mx28_duart_data __initconst = - mxs_duart_data_entry(MX28); -#endif - -struct platform_device *__init mxs_add_duart( - const struct mxs_duart_data *data) -{ - struct resource res[] = { - { - .start = data->iobase, - .end = data->iobase + SZ_8K - 1, - .flags = IORESOURCE_MEM, - }, { - .start = data->irq, - .end = data->irq, - .flags = IORESOURCE_IRQ, - }, - }; - - return mxs_add_platform_device("mxs-duart", 0, res, ARRAY_SIZE(res), - NULL, 0); -}diff --git a/arch/arm/mach-mxs/include/mach/devices-common.h b/arch/arm/mach-mxs/include/mach/devices-common.h index 3da48d4..fca0551 100644 --- a/arch/arm/mach-mxs/include/mach/devices-common.h +++ b/arch/arm/mach-mxs/include/mach/devices-common.h@@ -9,6 +9,7 @@ #include <linux/kernel.h> #include <linux/platform_device.h> #include <linux/init.h> +#include <linux/amba/bus.h> struct platform_device *mxs_add_platform_device_dmamask( const char *name, int id,@@ -25,13 +26,7 @@ static inline struct platform_device *mxs_add_platform_device( } /* duart */ -struct mxs_duart_data { - resource_size_t iobase; - resource_size_t iosize; - resource_size_t irq; -}; -struct platform_device *__init mxs_add_duart( - const struct mxs_duart_data *data); +int __init mxs_add_duart(struct amba_device *dev); /* fec */ #include <linux/fec.h>
-- Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |