| Message ID | 1306767139-24763-3-git-send-email-shawn.guo@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi, On Mon, May 30, 2011 at 10:52:17PM +0800, Shawn Guo wrote: > It adds platform device for drivers/gpio/gpio-mxc, and migrates > mx50/mx51/mx53 gpio driver to gpio-mxc. > [...] > diff --git a/arch/arm/plat-mxc/devices/platform-gpio-mxc.c b/arch/arm/plat-mxc/devices/platform-gpio-mxc.c > new file mode 100644 > index 0000000..3b10da0 > --- /dev/null > +++ b/arch/arm/plat-mxc/devices/platform-gpio-mxc.c > @@ -0,0 +1,69 @@ > +/* > + * Copyright 2011 Freescale Semiconductor, Inc. All Rights Reserved. > + * Copyright 2011 Linaro Limited > + * > + * 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 <linux/compiler.h> > +#include <linux/err.h> > +#include <linux/init.h> > + > +#include <mach/hardware.h> > +#include <mach/devices-common.h> > + > +static struct platform_device *__init mxc_add_gpio(int id, > + resource_size_t iobase, resource_size_t iosize, int irq, int irq_high) > +{ > + struct resource res[] = { > + { > + .start = iobase, > + .end = iobase + iosize - 1, > + .flags = IORESOURCE_MEM, > + }, { > + .start = irq, > + .end = irq, > + .flags = IORESOURCE_IRQ, > + }, { > + .start = irq_high, > + .end = irq_high, > + .flags = IORESOURCE_IRQ, > + }, > + }; > + > + return platform_device_register_resndata(&mxc_aips_bus, > + "gpio-mxc", id, res, ARRAY_SIZE(res), NULL, 0); Why bother returning the value, it's never checked below? > +static int __init mxc_add_mxc_gpio(void) Minor nits: Redundant mxcs? Also, 'gpios' would be more accurate naming. > +{ > + if (cpu_is_mx50()) { > + mxc_add_gpio(0, MX50_GPIO1_BASE_ADDR, SZ_16K, MX50_INT_GPIO1_LOW, MX50_INT_GPIO1_HIGH); > + mxc_add_gpio(1, MX50_GPIO2_BASE_ADDR, SZ_16K, MX50_INT_GPIO2_LOW, MX50_INT_GPIO2_HIGH); > + mxc_add_gpio(2, MX50_GPIO3_BASE_ADDR, SZ_16K, MX50_INT_GPIO3_LOW, MX50_INT_GPIO3_HIGH); > + mxc_add_gpio(3, MX50_GPIO4_BASE_ADDR, SZ_16K, MX50_INT_GPIO4_LOW, MX50_INT_GPIO4_HIGH); > + mxc_add_gpio(4, MX50_GPIO5_BASE_ADDR, SZ_16K, MX50_INT_GPIO5_LOW, MX50_INT_GPIO5_HIGH); > + mxc_add_gpio(5, MX50_GPIO6_BASE_ADDR, SZ_16K, MX50_INT_GPIO6_LOW, MX50_INT_GPIO6_HIGH); > + } > + > + if (cpu_is_mx51()) { > + mxc_add_gpio(0, MX51_GPIO1_BASE_ADDR, SZ_16K, MX51_MXC_INT_GPIO1_LOW, MX51_MXC_INT_GPIO1_HIGH); > + mxc_add_gpio(1, MX51_GPIO2_BASE_ADDR, SZ_16K, MX51_MXC_INT_GPIO2_LOW, MX51_MXC_INT_GPIO2_HIGH); > + mxc_add_gpio(2, MX51_GPIO3_BASE_ADDR, SZ_16K, MX51_MXC_INT_GPIO3_LOW, MX51_MXC_INT_GPIO3_HIGH); > + mxc_add_gpio(3, MX51_GPIO4_BASE_ADDR, SZ_16K, MX51_MXC_INT_GPIO4_LOW, MX51_MXC_INT_GPIO4_HIGH); > + } > + > + if (cpu_is_mx53()) { > + mxc_add_gpio(0, MX53_GPIO1_BASE_ADDR, SZ_16K, MX53_INT_GPIO1_LOW, MX53_INT_GPIO1_HIGH); > + mxc_add_gpio(1, MX53_GPIO2_BASE_ADDR, SZ_16K, MX53_INT_GPIO2_LOW, MX53_INT_GPIO2_HIGH); > + mxc_add_gpio(2, MX53_GPIO3_BASE_ADDR, SZ_16K, MX53_INT_GPIO3_LOW, MX53_INT_GPIO3_HIGH); > + mxc_add_gpio(3, MX53_GPIO4_BASE_ADDR, SZ_16K, MX53_INT_GPIO4_LOW, MX53_INT_GPIO4_HIGH); > + mxc_add_gpio(4, MX53_GPIO5_BASE_ADDR, SZ_16K, MX53_INT_GPIO5_LOW, MX53_INT_GPIO5_HIGH); > + mxc_add_gpio(5, MX53_GPIO6_BASE_ADDR, SZ_16K, MX53_INT_GPIO6_LOW, MX53_INT_GPIO6_HIGH); > + mxc_add_gpio(6, MX53_GPIO7_BASE_ADDR, SZ_16K, MX53_INT_GPIO7_LOW, MX53_INT_GPIO7_HIGH); > + } The above tables are pretty crazy, and they only get worse later in the series when the other SoCs are added. Is it really worth it to have a common initcall entry for the various SoCs here? It'd seem cleaner to me to just call the registration function for the family you're running from in per-family-init code such as where it was removed above (irq init, which makes some sense since the gpios provide interrupt sources as well). It's different on OMAP since there it is data driven with a shared registration function, but here it's really just a hardcoded table (as code even, not as data) per SoC. -Olof
On Mon, May 30, 2011 at 05:16:34PM -0700, Olof Johansson wrote: > Hi, > > On Mon, May 30, 2011 at 10:52:17PM +0800, Shawn Guo wrote: > > It adds platform device for drivers/gpio/gpio-mxc, and migrates > > mx50/mx51/mx53 gpio driver to gpio-mxc. > > > > [...] > > > diff --git a/arch/arm/plat-mxc/devices/platform-gpio-mxc.c b/arch/arm/plat-mxc/devices/platform-gpio-mxc.c > > new file mode 100644 > > index 0000000..3b10da0 > > --- /dev/null > > +++ b/arch/arm/plat-mxc/devices/platform-gpio-mxc.c > > @@ -0,0 +1,69 @@ > > +/* > > + * Copyright 2011 Freescale Semiconductor, Inc. All Rights Reserved. > > + * Copyright 2011 Linaro Limited > > + * > > + * 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 <linux/compiler.h> > > +#include <linux/err.h> > > +#include <linux/init.h> > > + > > +#include <mach/hardware.h> > > +#include <mach/devices-common.h> > > + > > +static struct platform_device *__init mxc_add_gpio(int id, > > + resource_size_t iobase, resource_size_t iosize, int irq, int irq_high) > > +{ > > + struct resource res[] = { > > + { > > + .start = iobase, > > + .end = iobase + iosize - 1, > > + .flags = IORESOURCE_MEM, > > + }, { > > + .start = irq, > > + .end = irq, > > + .flags = IORESOURCE_IRQ, > > + }, { > > + .start = irq_high, > > + .end = irq_high, > > + .flags = IORESOURCE_IRQ, > > + }, > > + }; > > + > > + return platform_device_register_resndata(&mxc_aips_bus, > > + "gpio-mxc", id, res, ARRAY_SIZE(res), NULL, 0); > > Why bother returning the value, it's never checked below? > Please help me understand. You are saying the return value should be checked below? > > +static int __init mxc_add_mxc_gpio(void) > > Minor nits: Redundant mxcs? Also, 'gpios' would be more accurate naming. > The first one is the namespace of plat-mxc function, and the second one is to reflect gpio driver name 'mxc gpio'. > > +{ > > + if (cpu_is_mx50()) { > > + mxc_add_gpio(0, MX50_GPIO1_BASE_ADDR, SZ_16K, MX50_INT_GPIO1_LOW, MX50_INT_GPIO1_HIGH); > > + mxc_add_gpio(1, MX50_GPIO2_BASE_ADDR, SZ_16K, MX50_INT_GPIO2_LOW, MX50_INT_GPIO2_HIGH); > > + mxc_add_gpio(2, MX50_GPIO3_BASE_ADDR, SZ_16K, MX50_INT_GPIO3_LOW, MX50_INT_GPIO3_HIGH); > > + mxc_add_gpio(3, MX50_GPIO4_BASE_ADDR, SZ_16K, MX50_INT_GPIO4_LOW, MX50_INT_GPIO4_HIGH); > > + mxc_add_gpio(4, MX50_GPIO5_BASE_ADDR, SZ_16K, MX50_INT_GPIO5_LOW, MX50_INT_GPIO5_HIGH); > > + mxc_add_gpio(5, MX50_GPIO6_BASE_ADDR, SZ_16K, MX50_INT_GPIO6_LOW, MX50_INT_GPIO6_HIGH); > > + } > > + > > + if (cpu_is_mx51()) { > > + mxc_add_gpio(0, MX51_GPIO1_BASE_ADDR, SZ_16K, MX51_MXC_INT_GPIO1_LOW, MX51_MXC_INT_GPIO1_HIGH); > > + mxc_add_gpio(1, MX51_GPIO2_BASE_ADDR, SZ_16K, MX51_MXC_INT_GPIO2_LOW, MX51_MXC_INT_GPIO2_HIGH); > > + mxc_add_gpio(2, MX51_GPIO3_BASE_ADDR, SZ_16K, MX51_MXC_INT_GPIO3_LOW, MX51_MXC_INT_GPIO3_HIGH); > > + mxc_add_gpio(3, MX51_GPIO4_BASE_ADDR, SZ_16K, MX51_MXC_INT_GPIO4_LOW, MX51_MXC_INT_GPIO4_HIGH); > > + } > > + > > + if (cpu_is_mx53()) { > > + mxc_add_gpio(0, MX53_GPIO1_BASE_ADDR, SZ_16K, MX53_INT_GPIO1_LOW, MX53_INT_GPIO1_HIGH); > > + mxc_add_gpio(1, MX53_GPIO2_BASE_ADDR, SZ_16K, MX53_INT_GPIO2_LOW, MX53_INT_GPIO2_HIGH); > > + mxc_add_gpio(2, MX53_GPIO3_BASE_ADDR, SZ_16K, MX53_INT_GPIO3_LOW, MX53_INT_GPIO3_HIGH); > > + mxc_add_gpio(3, MX53_GPIO4_BASE_ADDR, SZ_16K, MX53_INT_GPIO4_LOW, MX53_INT_GPIO4_HIGH); > > + mxc_add_gpio(4, MX53_GPIO5_BASE_ADDR, SZ_16K, MX53_INT_GPIO5_LOW, MX53_INT_GPIO5_HIGH); > > + mxc_add_gpio(5, MX53_GPIO6_BASE_ADDR, SZ_16K, MX53_INT_GPIO6_LOW, MX53_INT_GPIO6_HIGH); > > + mxc_add_gpio(6, MX53_GPIO7_BASE_ADDR, SZ_16K, MX53_INT_GPIO7_LOW, MX53_INT_GPIO7_HIGH); > > + } > > The above tables are pretty crazy, and they only get worse later in the series > when the other SoCs are added. Is it really worth it to have a common initcall > entry for the various SoCs here? > This common initcall seems a good place to concentrate the gpio device registration. It's easy to look at the common/different things among these SoCs. The only problem you reminded me is the scanning of the long cpu_is_mx list. It can be optimized a little bit by sorting the list from the latest (most used) SoC to the oldest (least used) one, and breaking out the scanning immediately when hitting one. > It'd seem cleaner to me to just call the registration function for the > family you're running from in per-family-init code such as where it was > removed above (irq init, which makes some sense since the gpios provide > interrupt sources as well). > I just gave it a test, and it's not working at all. (too early to register device in irq init?)
On Tuesday 31 May 2011, Shawn Guo wrote: > > The above tables are pretty crazy, and they only get worse later in the series > > when the other SoCs are added. Is it really worth it to have a common initcall > > entry for the various SoCs here? > > > This common initcall seems a good place to concentrate the gpio device > registration. It's easy to look at the common/different things among > these SoCs. The only problem you reminded me is the scanning of the > long cpu_is_mx list. It can be optimized a little bit by sorting the > list from the latest (most used) SoC to the oldest (least used) one, > and breaking out the scanning immediately when hitting one. No, better make them separate functions. > > It'd seem cleaner to me to just call the registration function for the > > family you're running from in per-family-init code such as where it was > > removed above (irq init, which makes some sense since the gpios provide > > interrupt sources as well). > > > I just gave it a test, and it's not working at all. (too early > to register device in irq init?) Just open-code the mxc_add_mxc_gpio() by moving the individual calls to mxc_add_gpio() into the respective callers. Having a global mxc_add_mxc_gpio() function that does something different for each caller seems entirely pointless to me. Best think of it from the point of view how it should look like when everything is converted to the device tree. In that case, you would have a function scanning the device tree that calls mxc_add_gpio for each entry. Arnd
On Tue, May 31, 2011 at 10:26:11AM +0200, Arnd Bergmann wrote: > On Tuesday 31 May 2011, Shawn Guo wrote: > > > The above tables are pretty crazy, and they only get worse later in the series > > > when the other SoCs are added. Is it really worth it to have a common initcall > > > entry for the various SoCs here? > > > > > This common initcall seems a good place to concentrate the gpio device > > registration. It's easy to look at the common/different things among > > these SoCs. The only problem you reminded me is the scanning of the > > long cpu_is_mx list. It can be optimized a little bit by sorting the > > list from the latest (most used) SoC to the oldest (least used) one, > > and breaking out the scanning immediately when hitting one. > > No, better make them separate functions. > > > > It'd seem cleaner to me to just call the registration function for the > > > family you're running from in per-family-init code such as where it was > > > removed above (irq init, which makes some sense since the gpios provide > > > interrupt sources as well). > > > > > I just gave it a test, and it's not working at all. (too early > > to register device in irq init?) > > Just open-code the mxc_add_mxc_gpio() by moving the individual calls to > mxc_add_gpio() into the respective callers. Having a global > mxc_add_mxc_gpio() function that does something different for each > caller seems entirely pointless to me. > Right now, mxc_add_mxc_gpio() is a postcore_initcall. Moving individual mxc_add_gpio() call into irq_init function does not work. And I need to find a proper caller for each SoC to call mxc_add_gpio to register gpio devices.
On Tuesday 31 May 2011, Shawn Guo wrote: > > Just open-code the mxc_add_mxc_gpio() by moving the individual calls to > > mxc_add_gpio() into the respective callers. Having a global > > mxc_add_mxc_gpio() function that does something different for each > > caller seems entirely pointless to me. > > > Right now, mxc_add_mxc_gpio() is a postcore_initcall. Moving > individual mxc_add_gpio() call into irq_init function does not work. > And I need to find a proper caller for each SoC to call mxc_add_gpio > to register gpio devices. Why not init_machine? That is an arch_initcall(), so it's probably close enough. Arnd
On Tue, May 31, 2011 at 01:28:17PM +0200, Arnd Bergmann wrote: > On Tuesday 31 May 2011, Shawn Guo wrote: > > > Just open-code the mxc_add_mxc_gpio() by moving the individual calls to > > > mxc_add_gpio() into the respective callers. Having a global > > > mxc_add_mxc_gpio() function that does something different for each > > > caller seems entirely pointless to me. > > > > > Right now, mxc_add_mxc_gpio() is a postcore_initcall. Moving > > individual mxc_add_gpio() call into irq_init function does not work. > > And I need to find a proper caller for each SoC to call mxc_add_gpio > > to register gpio devices. > > Why not init_machine? That is an arch_initcall(), so it's probably close > enough. > The init_machine is mostly a board specific function than SoC specific one. That is to say we will call mxc_add_gpio() in every single board init function even for the same SoC.
On Tuesday 31 May 2011, Shawn Guo wrote: > > On Tue, May 31, 2011 at 01:28:17PM +0200, Arnd Bergmann wrote: > > On Tuesday 31 May 2011, Shawn Guo wrote: > > > > Just open-code the mxc_add_mxc_gpio() by moving the individual calls to > > > > mxc_add_gpio() into the respective callers. Having a global > > > > mxc_add_mxc_gpio() function that does something different for each > > > > caller seems entirely pointless to me. > > > > > > > Right now, mxc_add_mxc_gpio() is a postcore_initcall. Moving > > > individual mxc_add_gpio() call into irq_init function does not work. > > > And I need to find a proper caller for each SoC to call mxc_add_gpio > > > to register gpio devices. > > > > Why not init_machine? That is an arch_initcall(), so it's probably close > > enough. > > > The init_machine is mostly a board specific function than SoC specific > one. That is to say we will call mxc_add_gpio() in every single board > init function even for the same SoC. But the machine is the only place that knows what SOC is being used. Your patch right now detects it by looking at the CPU type that is also being set by the board file using the init_early call, which is a bit silly. I would leave e.g. the imx51_register_gpios() in place, but only change the definition and the caller to void __init imx51_register_gpios(void) { mxc_add_gpio(0, MX51_GPIO1_BASE_ADDR, SZ_16K, MX51_MXC_INT_GPIO1_LOW, MX51_MXC_INT_GPIO1_HIGH); mxc_add_gpio(1, MX51_GPIO2_BASE_ADDR, SZ_16K, MX51_MXC_INT_GPIO2_LOW, MX51_MXC_INT_GPIO2_HIGH); mxc_add_gpio(2, MX51_GPIO3_BASE_ADDR, SZ_16K, MX51_MXC_INT_GPIO3_LOW, MX51_MXC_INT_GPIO3_HIGH); mxc_add_gpio(3, MX51_GPIO4_BASE_ADDR, SZ_16K, MX51_MXC_INT_GPIO4_LOW, MX51_MXC_INT_GPIO4_HIGH); } Then you can call that function from each i.mx51 based board, or from a new imx51_soc_init() function that groups multiple such functions. Arnd
On Tue, May 31, 2011 at 05:27:55PM +0200, Arnd Bergmann wrote: > On Tuesday 31 May 2011, Shawn Guo wrote: > > > > On Tue, May 31, 2011 at 01:28:17PM +0200, Arnd Bergmann wrote: > > > On Tuesday 31 May 2011, Shawn Guo wrote: > > > > > Just open-code the mxc_add_mxc_gpio() by moving the individual calls to > > > > > mxc_add_gpio() into the respective callers. Having a global > > > > > mxc_add_mxc_gpio() function that does something different for each > > > > > caller seems entirely pointless to me. > > > > > > > > > Right now, mxc_add_mxc_gpio() is a postcore_initcall. Moving > > > > individual mxc_add_gpio() call into irq_init function does not work. > > > > And I need to find a proper caller for each SoC to call mxc_add_gpio > > > > to register gpio devices. > > > > > > Why not init_machine? That is an arch_initcall(), so it's probably close > > > enough. > > > > > The init_machine is mostly a board specific function than SoC specific > > one. That is to say we will call mxc_add_gpio() in every single board > > init function even for the same SoC. > > But the machine is the only place that knows what SOC is being used. > Your patch right now detects it by looking at the CPU type that is > also being set by the board file using the init_early call, which > is a bit silly. > I would say that the CPU type is being set by SoC file (e.g. mach-mx5/mm.c) than board file. All the things in mach-mx5/mm.c are common to any mx51 based boards. > I would leave e.g. the imx51_register_gpios() in place, but only > change the definition and the caller to > > void __init imx51_register_gpios(void) > { > mxc_add_gpio(0, MX51_GPIO1_BASE_ADDR, SZ_16K, MX51_MXC_INT_GPIO1_LOW, MX51_MXC_INT_GPIO1_HIGH); > mxc_add_gpio(1, MX51_GPIO2_BASE_ADDR, SZ_16K, MX51_MXC_INT_GPIO2_LOW, MX51_MXC_INT_GPIO2_HIGH); > mxc_add_gpio(2, MX51_GPIO3_BASE_ADDR, SZ_16K, MX51_MXC_INT_GPIO3_LOW, MX51_MXC_INT_GPIO3_HIGH); > mxc_add_gpio(3, MX51_GPIO4_BASE_ADDR, SZ_16K, MX51_MXC_INT_GPIO4_LOW, MX51_MXC_INT_GPIO4_HIGH); > } > > Then you can call that function from each i.mx51 based board, or > from a new imx51_soc_init() function that groups multiple such > functions. > It will anyway touch every single board file (a lot) to have this SoC specific device registration plugged in. If you still think it's worthy to do so, I can turn around.
[was offline last night, apologies for the latency] On Tue, May 31, 2011 at 9:08 AM, Shawn Guo <shawn.guo@freescale.com> wrote: > On Tue, May 31, 2011 at 05:27:55PM +0200, Arnd Bergmann wrote: >> On Tuesday 31 May 2011, Shawn Guo wrote: >> > >> > On Tue, May 31, 2011 at 01:28:17PM +0200, Arnd Bergmann wrote: >> > > On Tuesday 31 May 2011, Shawn Guo wrote: >> > > > > Just open-code the mxc_add_mxc_gpio() by moving the individual calls to >> > > > > mxc_add_gpio() into the respective callers. Having a global >> > > > > mxc_add_mxc_gpio() function that does something different for each >> > > > > caller seems entirely pointless to me. >> > > > > >> > > > Right now, mxc_add_mxc_gpio() is a postcore_initcall. Moving >> > > > individual mxc_add_gpio() call into irq_init function does not work. >> > > > And I need to find a proper caller for each SoC to call mxc_add_gpio >> > > > to register gpio devices. >> > > >> > > Why not init_machine? That is an arch_initcall(), so it's probably close >> > > enough. >> > > >> > The init_machine is mostly a board specific function than SoC specific >> > one. That is to say we will call mxc_add_gpio() in every single board >> > init function even for the same SoC. >> >> But the machine is the only place that knows what SOC is being used. >> Your patch right now detects it by looking at the CPU type that is >> also being set by the board file using the init_early call, which >> is a bit silly. >> > I would say that the CPU type is being set by SoC file (e.g. > mach-mx5/mm.c) than board file. All the things in mach-mx5/mm.c are > common to any mx51 based boards. The thing is that all initialization today starts at the board file. Instead of setting a variable that determines code paths in asyncronous initcalls later, you might as well call the appropriate initcall functions from common functions called by board files. So, you'd just add a call to a common per-soc init in each of the per-board machine_init functions. Over time it'll be a good place to add code that would otherwise be duplicated per board of the same soc family. >> I would leave e.g. the imx51_register_gpios() in place, but only >> change the definition and the caller to >> >> void __init imx51_register_gpios(void) >> { >> mxc_add_gpio(0, MX51_GPIO1_BASE_ADDR, SZ_16K, MX51_MXC_INT_GPIO1_LOW, MX51_MXC_INT_GPIO1_HIGH); >> mxc_add_gpio(1, MX51_GPIO2_BASE_ADDR, SZ_16K, MX51_MXC_INT_GPIO2_LOW, MX51_MXC_INT_GPIO2_HIGH); >> mxc_add_gpio(2, MX51_GPIO3_BASE_ADDR, SZ_16K, MX51_MXC_INT_GPIO3_LOW, MX51_MXC_INT_GPIO3_HIGH); >> mxc_add_gpio(3, MX51_GPIO4_BASE_ADDR, SZ_16K, MX51_MXC_INT_GPIO4_LOW, MX51_MXC_INT_GPIO4_HIGH); >> } >> >> Then you can call that function from each i.mx51 based board, or >> from a new imx51_soc_init() function that groups multiple such >> functions. >> > It will anyway touch every single board file (a lot) to have this SoC > specific device registration plugged in. If you still think it's > worthy to do so, I can turn around. I personally prefer the imx51_soc_init() Arnd proposed above, since there's a chance you will want to add more things to it over time. It is worth adding it now, it's just a one-line change per board and it gives you a good place to hook in other per-family setup that might be needed later. -Olof
diff --git a/arch/arm/mach-mx5/devices.c b/arch/arm/mach-mx5/devices.c index 153ada5..371ca8c 100644 --- a/arch/arm/mach-mx5/devices.c +++ b/arch/arm/mach-mx5/devices.c @@ -12,7 +12,6 @@ #include <linux/platform_device.h> #include <linux/dma-mapping.h> -#include <linux/gpio.h> #include <mach/hardware.h> #include <mach/imx-uart.h> #include <mach/irqs.h> @@ -119,66 +118,3 @@ struct platform_device mxc_usbh2_device = { .coherent_dma_mask = DMA_BIT_MASK(32), }, }; - -static struct mxc_gpio_port mxc_gpio_ports[] = { - { - .chip.label = "gpio-0", - .base = MX51_IO_ADDRESS(MX51_GPIO1_BASE_ADDR), - .irq = MX51_MXC_INT_GPIO1_LOW, - .irq_high = MX51_MXC_INT_GPIO1_HIGH, - .virtual_irq_start = MXC_GPIO_IRQ_START - }, - { - .chip.label = "gpio-1", - .base = MX51_IO_ADDRESS(MX51_GPIO2_BASE_ADDR), - .irq = MX51_MXC_INT_GPIO2_LOW, - .irq_high = MX51_MXC_INT_GPIO2_HIGH, - .virtual_irq_start = MXC_GPIO_IRQ_START + 32 * 1 - }, - { - .chip.label = "gpio-2", - .base = MX51_IO_ADDRESS(MX51_GPIO3_BASE_ADDR), - .irq = MX51_MXC_INT_GPIO3_LOW, - .irq_high = MX51_MXC_INT_GPIO3_HIGH, - .virtual_irq_start = MXC_GPIO_IRQ_START + 32 * 2 - }, - { - .chip.label = "gpio-3", - .base = MX51_IO_ADDRESS(MX51_GPIO4_BASE_ADDR), - .irq = MX51_MXC_INT_GPIO4_LOW, - .irq_high = MX51_MXC_INT_GPIO4_HIGH, - .virtual_irq_start = MXC_GPIO_IRQ_START + 32 * 3 - }, - { - .chip.label = "gpio-4", - .base = MX53_IO_ADDRESS(MX53_GPIO5_BASE_ADDR), - .irq = MX53_INT_GPIO5_LOW, - .irq_high = MX53_INT_GPIO5_HIGH, - .virtual_irq_start = MXC_GPIO_IRQ_START + 32 * 4 - }, - { - .chip.label = "gpio-5", - .base = MX53_IO_ADDRESS(MX53_GPIO6_BASE_ADDR), - .irq = MX53_INT_GPIO6_LOW, - .irq_high = MX53_INT_GPIO6_HIGH, - .virtual_irq_start = MXC_GPIO_IRQ_START + 32 * 5 - }, - { - .chip.label = "gpio-6", - .base = MX53_IO_ADDRESS(MX53_GPIO7_BASE_ADDR), - .irq = MX53_INT_GPIO7_LOW, - .irq_high = MX53_INT_GPIO7_HIGH, - .virtual_irq_start = MXC_GPIO_IRQ_START + 32 * 6 - }, -}; - -int __init imx51_register_gpios(void) -{ - return mxc_gpio_init(mxc_gpio_ports, 4); -} - -int __init imx53_register_gpios(void) -{ - return mxc_gpio_init(mxc_gpio_ports, ARRAY_SIZE(mxc_gpio_ports)); -} - diff --git a/arch/arm/mach-mx5/mm-mx50.c b/arch/arm/mach-mx5/mm-mx50.c index b9c363b..e1377ca 100644 --- a/arch/arm/mach-mx5/mm-mx50.c +++ b/arch/arm/mach-mx5/mm-mx50.c @@ -26,7 +26,6 @@ #include <mach/hardware.h> #include <mach/common.h> #include <mach/iomux-v3.h> -#include <mach/gpio.h> #include <mach/irqs.h> /* @@ -56,17 +55,7 @@ void __init imx50_init_early(void) mxc_arch_reset_init(MX50_IO_ADDRESS(MX50_WDOG_BASE_ADDR)); } -static struct mxc_gpio_port imx50_gpio_ports[] = { - DEFINE_IMX_GPIO_PORT_IRQ_HIGH(MX50, 0, 1, MX50_INT_GPIO1_LOW, MX50_INT_GPIO1_HIGH), - DEFINE_IMX_GPIO_PORT_IRQ_HIGH(MX50, 1, 2, MX50_INT_GPIO2_LOW, MX50_INT_GPIO2_HIGH), - DEFINE_IMX_GPIO_PORT_IRQ_HIGH(MX50, 2, 3, MX50_INT_GPIO3_LOW, MX50_INT_GPIO3_HIGH), - DEFINE_IMX_GPIO_PORT_IRQ_HIGH(MX50, 3, 4, MX50_INT_GPIO3_LOW, MX50_INT_GPIO3_HIGH), - DEFINE_IMX_GPIO_PORT_IRQ_HIGH(MX50, 4, 5, MX50_INT_GPIO3_LOW, MX50_INT_GPIO3_HIGH), - DEFINE_IMX_GPIO_PORT_IRQ_HIGH(MX50, 5, 6, MX50_INT_GPIO3_LOW, MX50_INT_GPIO3_HIGH), -}; - void __init mx50_init_irq(void) { tzic_init_irq(MX50_IO_ADDRESS(MX50_TZIC_BASE_ADDR)); - mxc_gpio_init(imx50_gpio_ports, ARRAY_SIZE(imx50_gpio_ports)); } diff --git a/arch/arm/mach-mx5/mm.c b/arch/arm/mach-mx5/mm.c index ff55730..ccc6ceb 100644 --- a/arch/arm/mach-mx5/mm.c +++ b/arch/arm/mach-mx5/mm.c @@ -69,8 +69,6 @@ void __init imx53_init_early(void) mxc_arch_reset_init(MX53_IO_ADDRESS(MX53_WDOG1_BASE_ADDR)); } -int imx51_register_gpios(void); - void __init mx51_init_irq(void) { unsigned long tzic_addr; @@ -86,11 +84,8 @@ void __init mx51_init_irq(void) panic("unable to map TZIC interrupt controller\n"); tzic_init_irq(tzic_virt); - imx51_register_gpios(); } -int imx53_register_gpios(void); - void __init mx53_init_irq(void) { unsigned long tzic_addr; @@ -103,5 +98,4 @@ void __init mx53_init_irq(void) panic("unable to map TZIC interrupt controller\n"); tzic_init_irq(tzic_virt); - imx53_register_gpios(); } diff --git a/arch/arm/plat-mxc/devices.c b/arch/arm/plat-mxc/devices.c index eee1b60..fb166b2 100644 --- a/arch/arm/plat-mxc/devices.c +++ b/arch/arm/plat-mxc/devices.c @@ -89,3 +89,14 @@ err: return pdev; } + +struct device mxc_aips_bus = { + .init_name = "mxc_aips", + .parent = &platform_bus, +}; + +static int __init mxc_device_init(void) +{ + return device_register(&mxc_aips_bus); +} +core_initcall(mxc_device_init); diff --git a/arch/arm/plat-mxc/devices/Makefile b/arch/arm/plat-mxc/devices/Makefile index ad2922a..b41bf97 100644 --- a/arch/arm/plat-mxc/devices/Makefile +++ b/arch/arm/plat-mxc/devices/Makefile @@ -2,6 +2,7 @@ obj-$(CONFIG_IMX_HAVE_PLATFORM_FEC) += platform-fec.o obj-$(CONFIG_IMX_HAVE_PLATFORM_FLEXCAN) += platform-flexcan.o obj-$(CONFIG_IMX_HAVE_PLATFORM_FSL_USB2_UDC) += platform-fsl-usb2-udc.o obj-$(CONFIG_IMX_HAVE_PLATFORM_GPIO_KEYS) += platform-gpio_keys.o +obj-y += platform-gpio-mxc.o obj-$(CONFIG_IMX_HAVE_PLATFORM_IMX21_HCD) += platform-imx21-hcd.o obj-$(CONFIG_IMX_HAVE_PLATFORM_IMX2_WDT) += platform-imx2-wdt.o obj-$(CONFIG_IMX_HAVE_PLATFORM_IMXDI_RTC) += platform-imxdi_rtc.o diff --git a/arch/arm/plat-mxc/devices/platform-gpio-mxc.c b/arch/arm/plat-mxc/devices/platform-gpio-mxc.c new file mode 100644 index 0000000..3b10da0 --- /dev/null +++ b/arch/arm/plat-mxc/devices/platform-gpio-mxc.c @@ -0,0 +1,69 @@ +/* + * Copyright 2011 Freescale Semiconductor, Inc. All Rights Reserved. + * Copyright 2011 Linaro Limited + * + * 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 <linux/compiler.h> +#include <linux/err.h> +#include <linux/init.h> + +#include <mach/hardware.h> +#include <mach/devices-common.h> + +static struct platform_device *__init mxc_add_gpio(int id, + resource_size_t iobase, resource_size_t iosize, int irq, int irq_high) +{ + struct resource res[] = { + { + .start = iobase, + .end = iobase + iosize - 1, + .flags = IORESOURCE_MEM, + }, { + .start = irq, + .end = irq, + .flags = IORESOURCE_IRQ, + }, { + .start = irq_high, + .end = irq_high, + .flags = IORESOURCE_IRQ, + }, + }; + + return platform_device_register_resndata(&mxc_aips_bus, + "gpio-mxc", id, res, ARRAY_SIZE(res), NULL, 0); +} + +static int __init mxc_add_mxc_gpio(void) +{ + if (cpu_is_mx50()) { + mxc_add_gpio(0, MX50_GPIO1_BASE_ADDR, SZ_16K, MX50_INT_GPIO1_LOW, MX50_INT_GPIO1_HIGH); + mxc_add_gpio(1, MX50_GPIO2_BASE_ADDR, SZ_16K, MX50_INT_GPIO2_LOW, MX50_INT_GPIO2_HIGH); + mxc_add_gpio(2, MX50_GPIO3_BASE_ADDR, SZ_16K, MX50_INT_GPIO3_LOW, MX50_INT_GPIO3_HIGH); + mxc_add_gpio(3, MX50_GPIO4_BASE_ADDR, SZ_16K, MX50_INT_GPIO4_LOW, MX50_INT_GPIO4_HIGH); + mxc_add_gpio(4, MX50_GPIO5_BASE_ADDR, SZ_16K, MX50_INT_GPIO5_LOW, MX50_INT_GPIO5_HIGH); + mxc_add_gpio(5, MX50_GPIO6_BASE_ADDR, SZ_16K, MX50_INT_GPIO6_LOW, MX50_INT_GPIO6_HIGH); + } + + if (cpu_is_mx51()) { + mxc_add_gpio(0, MX51_GPIO1_BASE_ADDR, SZ_16K, MX51_MXC_INT_GPIO1_LOW, MX51_MXC_INT_GPIO1_HIGH); + mxc_add_gpio(1, MX51_GPIO2_BASE_ADDR, SZ_16K, MX51_MXC_INT_GPIO2_LOW, MX51_MXC_INT_GPIO2_HIGH); + mxc_add_gpio(2, MX51_GPIO3_BASE_ADDR, SZ_16K, MX51_MXC_INT_GPIO3_LOW, MX51_MXC_INT_GPIO3_HIGH); + mxc_add_gpio(3, MX51_GPIO4_BASE_ADDR, SZ_16K, MX51_MXC_INT_GPIO4_LOW, MX51_MXC_INT_GPIO4_HIGH); + } + + if (cpu_is_mx53()) { + mxc_add_gpio(0, MX53_GPIO1_BASE_ADDR, SZ_16K, MX53_INT_GPIO1_LOW, MX53_INT_GPIO1_HIGH); + mxc_add_gpio(1, MX53_GPIO2_BASE_ADDR, SZ_16K, MX53_INT_GPIO2_LOW, MX53_INT_GPIO2_HIGH); + mxc_add_gpio(2, MX53_GPIO3_BASE_ADDR, SZ_16K, MX53_INT_GPIO3_LOW, MX53_INT_GPIO3_HIGH); + mxc_add_gpio(3, MX53_GPIO4_BASE_ADDR, SZ_16K, MX53_INT_GPIO4_LOW, MX53_INT_GPIO4_HIGH); + mxc_add_gpio(4, MX53_GPIO5_BASE_ADDR, SZ_16K, MX53_INT_GPIO5_LOW, MX53_INT_GPIO5_HIGH); + mxc_add_gpio(5, MX53_GPIO6_BASE_ADDR, SZ_16K, MX53_INT_GPIO6_LOW, MX53_INT_GPIO6_HIGH); + mxc_add_gpio(6, MX53_GPIO7_BASE_ADDR, SZ_16K, MX53_INT_GPIO7_LOW, MX53_INT_GPIO7_HIGH); + } + + return 0; +} +postcore_initcall(mxc_add_mxc_gpio); diff --git a/arch/arm/plat-mxc/include/mach/devices-common.h b/arch/arm/plat-mxc/include/mach/devices-common.h index fa84773..03f6266 100644 --- a/arch/arm/plat-mxc/include/mach/devices-common.h +++ b/arch/arm/plat-mxc/include/mach/devices-common.h @@ -10,6 +10,8 @@ #include <linux/platform_device.h> #include <linux/init.h> +extern struct device mxc_aips_bus; + struct platform_device *imx_add_platform_device_dmamask( const char *name, int id, const struct resource *res, unsigned int num_resources,
It adds platform device for drivers/gpio/gpio-mxc, and migrates mx50/mx51/mx53 gpio driver to gpio-mxc. Signed-off-by: Shawn Guo <shawn.guo@linaro.org> --- arch/arm/mach-mx5/devices.c | 64 --------------------- arch/arm/mach-mx5/mm-mx50.c | 11 ---- arch/arm/mach-mx5/mm.c | 6 -- arch/arm/plat-mxc/devices.c | 11 ++++ arch/arm/plat-mxc/devices/Makefile | 1 + arch/arm/plat-mxc/devices/platform-gpio-mxc.c | 69 +++++++++++++++++++++++ arch/arm/plat-mxc/include/mach/devices-common.h | 2 + 7 files changed, 83 insertions(+), 81 deletions(-) create mode 100644 arch/arm/plat-mxc/devices/platform-gpio-mxc.c