diff mbox

[v5,1/3] ARM: mxs: add GPMI-NFC support for imx23/imx28

Message ID 1309406028-2924-2-git-send-email-b32955@freescale.com
State New
Headers show

Commit Message

Huang Shijie June 30, 2011, 3:53 a.m. UTC
add GPMI-NFC support for imx23 and imx28.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 arch/arm/mach-mxs/clock-mx23.c                  |    1 +
 arch/arm/mach-mxs/clock-mx28.c                  |    1 +
 arch/arm/mach-mxs/devices-mx23.h                |    4 +
 arch/arm/mach-mxs/devices-mx28.h                |    4 +
 arch/arm/mach-mxs/devices/Kconfig               |    3 +
 arch/arm/mach-mxs/devices/Makefile              |    1 +
 arch/arm/mach-mxs/devices/platform-gpmi-nfc.c   |   90 +++++++++++++++++++++++
 arch/arm/mach-mxs/include/mach/devices-common.h |   10 +++
 arch/arm/mach-mxs/include/mach/gpmi-nfc.h       |   71 ++++++++++++++++++
 9 files changed, 185 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-mxs/devices/platform-gpmi-nfc.c
 create mode 100644 arch/arm/mach-mxs/include/mach/gpmi-nfc.h

Comments

Arnd Bergmann June 30, 2011, 1:55 p.m. UTC | #1
On Thursday 30 June 2011, Huang Shijie wrote:
> add GPMI-NFC support for imx23 and imx28.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>

This needs a better changelog, please at least spell out what GPMI-NFC means.
I'm probably not the only one who gets confused into reading this as
'near field communication' instead of 'nand flash controller'.

Could you rename this all into gpmi-nand or gpmi-flash instead, to avoid
confusion when you add support for near field communication?

> +#define RES_MEM(soc, _id, _s, _n)				\
> +	{							\
> +		.start = soc ##_## _id ## _BASE_ADDR,		\
> +		.end   = soc ##_## _id ## _BASE_ADDR + (_s) - 1,\
> +		.name  = (_n),					\
> +		.flags = IORESOURCE_MEM,			\
> +	}
> +
> +#define RES_IRQ(soc, _id, _n)			\
> +	{					\
> +		.start = soc ##_INT_## _id,	\
> +		.end   = soc ##_INT_## _id,	\
> +		.name  = (_n),			\
> +		.flags = IORESOURCE_IRQ,	\
> +	}
> +
> +#define RES_DMA(soc, _i_s, _i_e, _n)		\
> +	{					\
> +		.start = soc ##_## _i_s,	\
> +		.end   = soc ##_## _i_e,	\
> +		.name  = (_n),			\
> +		.flags = IORESOURCE_DMA,	\
> +	}

I know that you didn't start this pattern, but I find these macros
extremely annoying. It obscures the use of the macros with the
string concatenation and the macro names are way too generic
for something platform specific. If people think it's a good idea
to have these, please submit a patch to add macros (without the
string concatenation) into include/linux/ioport.h.

Until then, better spell out the resources.

> +/**
> + * struct gpmi_nfc_platform_data - GPMI NFC driver platform data.
> + *
> + * This structure communicates platform-specific information to the GPMI NFC
> + * driver that can't be expressed as resources.
> + *
> + * @platform_init:           A pointer to a function the driver will call to
> + *                           initialize the platform (e.g., set up the pin mux).
> + * @platform_exit:           A pointer to a function the driver will call to
> + *                           exit the platform (e.g., free pins).
> + * @min_prop_delay_in_ns:    Minimum propagation delay of GPMI signals to and
> + *                           from the NAND Flash device, in nanoseconds.
> + * @max_prop_delay_in_ns:    Maximum propagation delay of GPMI signals to and
> + *                           from the NAND Flash device, in nanoseconds.
> + * @max_chip_count:          The maximum number of chips for which the driver
> + *                           should configure the hardware. This value most
> + *                           likely reflects the number of pins that are
> + *                           connected to a NAND Flash device. If this is
> + *                           greater than the SoC hardware can support, the
> + *                           driver will print a message and fail to initialize.
> + * @partitions:              An optional pointer to an array of partition
> + *                           descriptions.
> + * @partition_count:         The number of elements in the partitions array.
> + */
> +struct gpmi_nfc_platform_data {
> +	/* SoC hardware information. */
> +	int		(*platform_init)(void);
> +	void		(*platform_exit)(void);
> +
> +	/* NAND Flash information. */
> +	unsigned int	min_prop_delay_in_ns;
> +	unsigned int	max_prop_delay_in_ns;
> +	unsigned int	max_chip_count;
> +
> +	/* Medium information. */
> +	struct mtd_partition *partitions;
> +	unsigned	partition_count;
> +};

When adding new infrastructure, always keep in mind how you want it to look
after the device tree conversion. The partitions and min/max_* are easily covered
with that, but the init/exit function pointers are somewhat problematic.

Fortunately, you don't really require these functions for this driver. The _exit
function is completely unused, so just get rid of it.

The init function is used only to set up iomux, so the logical replacement is
a pointer to the iomux data, and calling mxs_iomux_setup_multiple_pads
directly from the driver.

	Arnd
Lothar Waßmann June 30, 2011, 2:58 p.m. UTC | #2
Hi,

Arnd Bergmann writes:
> On Thursday 30 June 2011, Huang Shijie wrote:
> > add GPMI-NFC support for imx23 and imx28.
> > 
> > Signed-off-by: Huang Shijie <b32955@freescale.com>
> 
[...]
> > +struct gpmi_nfc_platform_data {
> > +	/* SoC hardware information. */
> > +	int		(*platform_init)(void);
> > +	void		(*platform_exit)(void);
> > +
> > +	/* NAND Flash information. */
> > +	unsigned int	min_prop_delay_in_ns;
> > +	unsigned int	max_prop_delay_in_ns;
> > +	unsigned int	max_chip_count;
> > +
> > +	/* Medium information. */
> > +	struct mtd_partition *partitions;
> > +	unsigned	partition_count;
> > +};
> 
> When adding new infrastructure, always keep in mind how you want it to look
> after the device tree conversion. The partitions and min/max_* are easily covered
> with that, but the init/exit function pointers are somewhat problematic.
> 
> Fortunately, you don't really require these functions for this driver. The _exit
> function is completely unused, so just get rid of it.
> 
> The init function is used only to set up iomux, so the logical replacement is
> a pointer to the iomux data, and calling mxs_iomux_setup_multiple_pads
> directly from the driver.
> 
NO! I strongly object that. mxs_iomux_setup_multiple_pads() is a
platform specific function and has no business inside a device driver
that should be platform agnostic.
Consider the same controller being part of a different SoC that
requires you to call mxs_iomux_v2_setup_multiple_pads() instead. You
would then have to check for the SoC type inside the driver to find
the right function to call which is quite obscene.

I would rather live with the iomux statically configured in the
platform init code than having direct calls into platform specific
code from device drivers.


Lothar Waßmann
Arnd Bergmann June 30, 2011, 10:22 p.m. UTC | #3
On Thursday 30 June 2011 16:58:38 Lothar Waßmann wrote:
> > 
> > When adding new infrastructure, always keep in mind how you want it to look
> > after the device tree conversion. The partitions and min/max_* are easily covered
> > with that, but the init/exit function pointers are somewhat problematic.
> > 
> > Fortunately, you don't really require these functions for this driver. The _exit
> > function is completely unused, so just get rid of it.
> > 
> > The init function is used only to set up iomux, so the logical replacement is
> > a pointer to the iomux data, and calling mxs_iomux_setup_multiple_pads
> > directly from the driver.
> > 
> NO! I strongly object that. mxs_iomux_setup_multiple_pads() is a
> platform specific function and has no business inside a device driver
> that should be platform agnostic.
> Consider the same controller being part of a different SoC that
> requires you to call mxs_iomux_v2_setup_multiple_pads() instead. You
> would then have to check for the SoC type inside the driver to find
> the right function to call which is quite obscene.

Won't this problem just go away as soon as we get the mxs converted to the
generic pinmux subsystem that Linus Walleij is doing?

Obviously, you would not have to check the soc type really, you would have
to instead check for different versions of the gpmi, which you most likely
have to anyway because reusing the same hardware block in a new chip usually
comes with other changes as well. 

Note how the driver already does this with code like

+	if (GPMI_IS_MX23(this) || GPMI_IS_MX28(this))
+		nfc = &gpmi_nfc_hal_mxs;

If you really want to call out obsceneties, how about the fact that this
driver comes with an 805 line patch to add a HAL for a single chip!

Such abstractions should not be introduced as long as there is only
a single instance of the hardware.

> I would rather live with the iomux statically configured in the
> platform init code than having direct calls into platform specific
> code from device drivers.

That would certainly work until we have the pinmux subsystem to
take care of it.

	Arnd
Lothar Waßmann July 1, 2011, 5:59 a.m. UTC | #4
Hi,

Arnd Bergmann writes:
> On Thursday 30 June 2011 16:58:38 Lothar Waßmann wrote:
> > > 
> > > When adding new infrastructure, always keep in mind how you want it to look
> > > after the device tree conversion. The partitions and min/max_* are easily covered
> > > with that, but the init/exit function pointers are somewhat problematic.
> > > 
> > > Fortunately, you don't really require these functions for this driver. The _exit
> > > function is completely unused, so just get rid of it.
> > > 
> > > The init function is used only to set up iomux, so the logical replacement is
> > > a pointer to the iomux data, and calling mxs_iomux_setup_multiple_pads
> > > directly from the driver.
> > > 
> > NO! I strongly object that. mxs_iomux_setup_multiple_pads() is a
> > platform specific function and has no business inside a device driver
> > that should be platform agnostic.
> > Consider the same controller being part of a different SoC that
> > requires you to call mxs_iomux_v2_setup_multiple_pads() instead. You
> > would then have to check for the SoC type inside the driver to find
> > the right function to call which is quite obscene.
> 
> Won't this problem just go away as soon as we get the mxs converted to the
> generic pinmux subsystem that Linus Walleij is doing?
> 
> Obviously, you would not have to check the soc type really, you would have
> to instead check for different versions of the gpmi, which you most likely
> have to anyway because reusing the same hardware block in a new chip usually
> comes with other changes as well. 
> 
The pin muxing is SoC specific, not GPMI specific. So the SoC version
should be checked, not something else that may be loosely linked to
it.

> Note how the driver already does this with code like
> 
> +	if (GPMI_IS_MX23(this) || GPMI_IS_MX28(this))
> +		nfc = &gpmi_nfc_hal_mxs;
> 
> If you really want to call out obsceneties, how about the fact that this
> driver comes with an 805 line patch to add a HAL for a single chip!
> 
> Such abstractions should not be introduced as long as there is only
> a single instance of the hardware.
> 
I had already commented on this in the first round of patches from
Huang in March this year.


Lothar Waßmann
Wolfram Sang July 1, 2011, 6:03 a.m. UTC | #5
Hi Arnd,

> If you really want to call out obsceneties, how about the fact that this
> driver comes with an 805 line patch to add a HAL for a single chip!
> 
> Such abstractions should not be introduced as long as there is only
> a single instance of the hardware.

If I understood correctly, most if not all upcoming i.MX will have the GPMI
(mx50, mx6). Huang, do you already have a draft for the mx50-hal?

Regards,

   Wolfram
Huang Shijie July 1, 2011, 7:53 a.m. UTC | #6
Hi:
> Hi Arnd,
>
>> If you really want to call out obsceneties, how about the fact that this
>> driver comes with an 805 line patch to add a HAL for a single chip!
>>
>> Such abstractions should not be introduced as long as there is only
>> a single instance of the hardware.
> If I understood correctly, most if not all upcoming i.MX will have the GPMI
> (mx50, mx6). Huang, do you already have a draft for the mx50-hal?
>
I have finished the code for mx50's GPMI.
And I am coding for the MX6's GPMI recently.

I need a separate mx50-hal (or mx60-hal) to make the code tidy.
The MX50 and mx60 support ONFI NAND and TOGGLE nand(which mx23/mx28 do 
not support),
they need a long code to initialize the TIMING register. What's more, 
the READ/WRITE functions
are different from the mx23/mx28.

Frankly speaking, I can merge the mxs-hal.c file to the gpmi-nfc.c, but 
don't you think it too
messy?


Best Regards
Huang Shijie





> Regards,
>
>     Wolfram
>
Wolfram Sang July 1, 2011, 8:01 a.m. UTC | #7
On Fri, Jul 01, 2011 at 03:53:13PM +0800, Huang Shijie wrote:
> Hi:
> >Hi Arnd,
> >
> >>If you really want to call out obsceneties, how about the fact that this
> >>driver comes with an 805 line patch to add a HAL for a single chip!
> >>
> >>Such abstractions should not be introduced as long as there is only
> >>a single instance of the hardware.
> >If I understood correctly, most if not all upcoming i.MX will have the GPMI
> >(mx50, mx6). Huang, do you already have a draft for the mx50-hal?
> >
> I have finished the code for mx50's GPMI.
> And I am coding for the MX6's GPMI recently.
> 
> I need a separate mx50-hal (or mx60-hal) to make the code tidy.
> The MX50 and mx60 support ONFI NAND and TOGGLE nand(which mx23/mx28
> do not support),
> they need a long code to initialize the TIMING register. What's
> more, the READ/WRITE functions
> are different from the mx23/mx28.
> 
> Frankly speaking, I can merge the mxs-hal.c file to the gpmi-nfc.c,
> but don't you think it too
> messy?

Is it possible to post the mx50 code (as RFC with a note saying that it
is not ready yet and is not intended to be merged) so we can see better?
Huang Shijie July 1, 2011, 8:45 a.m. UTC | #8
于 2011年07月01日 16:39, Huang Shijie 写道:
> Hi:
>> On Fri, Jul 01, 2011 at 03:53:13PM +0800, Huang Shijie wrote:
>>> Hi:
>>>> Hi Arnd,
>>>>
>>>>> If you really want to call out obsceneties, how about the fact 
>>>>> that this
>>>>> driver comes with an 805 line patch to add a HAL for a single chip!
>>>>>
>>>>> Such abstractions should not be introduced as long as there is only
>>>>> a single instance of the hardware.
>>>> If I understood correctly, most if not all upcoming i.MX will have 
>>>> the GPMI
>>>> (mx50, mx6). Huang, do you already have a draft for the mx50-hal?
>>>>
>>> I have finished the code for mx50's GPMI.
>>> And I am coding for the MX6's GPMI recently.
>>>
>>> I need a separate mx50-hal (or mx60-hal) to make the code tidy.
>>> The MX50 and mx60 support ONFI NAND and TOGGLE nand(which mx23/mx28
>>> do not support),
>>> they need a long code to initialize the TIMING register. What's
>>> more, the READ/WRITE functions
>>> are different from the mx23/mx28.
>>>
>>> Frankly speaking, I can merge the mxs-hal.c file to the gpmi-nfc.c,
>>> but don't you think it too
>>> messy?
>> Is it possible to post the mx50 code (as RFC with a note saying that it
>> is not ready yet and is not intended to be merged) so we can see better?
>>
> The attachment is for mx50 GPMI driver.
> You can see the long timing initialization.
The normal mx50 board does not supports DDR mode, so i do not enable it 
in the patch.


But another special MX50 board(ddr2 board) supports DDR mode, the code 
is for that board.

Best Regards
Huang Shijie
>
> I will take a business trip in the following several days.
> I will submit new version when i come back to office.
>
Arnd Bergmann July 1, 2011, 9:25 a.m. UTC | #9
On Friday 01 July 2011 09:53:13 Huang Shijie wrote:
> >> If you really want to call out obsceneties, how about the fact that this
> >> driver comes with an 805 line patch to add a HAL for a single chip!
> >>
> >> Such abstractions should not be introduced as long as there is only
> >> a single instance of the hardware.
> > If I understood correctly, most if not all upcoming i.MX will have the GPMI
> > (mx50, mx6). Huang, do you already have a draft for the mx50-hal?
> >
> I have finished the code for mx50's GPMI.
> And I am coding for the MX6's GPMI recently.

Ok.

> I need a separate mx50-hal (or mx60-hal) to make the code tidy.
> The MX50 and mx60 support ONFI NAND and TOGGLE nand(which mx23/mx28 do 
> not support),
> they need a long code to initialize the TIMING register. What's more, 
> the READ/WRITE functions
> are different from the mx23/mx28.
> 
> Frankly speaking, I can merge the mxs-hal.c file to the gpmi-nfc.c, but 
> don't you think it too
> messy?

If you already have the code for the other SoCs, it's fine to have an
abstraction. From reading your patches that were specifically targetted
at mxs, that was not clear.

After a very brief look at the driver, my impression is that your layering
is upside-down though:

You have a "main" driver that registers to all devices and then pulls
in hardware specific bits. What you should have instead is a "library"
driver for the common code that provides all the common functionality
as exported functions, and individual drivers for each SoC type that
each register a platform_driver for one device id and use the functions
from the common module.

I don't know how hard it would be to retrofit that model, and I'm not
asking you to do it, unless other people feel strongly about it.
However, I think it would make your life easier when maintaining the
driver in the future. There are multiple advantages to that:

* You would not need a HAL. For your information, that term causes strong
  negative associations with many Linux developers. To us, the kernel
  drivers themselves are the hardware abstraction, you don't need another
  one.

* It's more easy to extend to multiple levels of libraries. E.g. you could
  have one module for stuff that is common between i.MX5 and i.MX6 but
  not i.MX3, and make the later drivers use both libraries.

* It gets rid of the need for #ifdef in the common driver module.

* It becomes very easy to override one function just for a specific
  instance of the hardware, while everything else uses a common version
  from the library.

	Arnd
Uwe Kleine-König July 8, 2011, 7:31 a.m. UTC | #10
Hello,

On Thu, Jun 30, 2011 at 03:55:19PM +0200, Arnd Bergmann wrote:
> On Thursday 30 June 2011, Huang Shijie wrote:
> > add GPMI-NFC support for imx23 and imx28.
> > 
> > Signed-off-by: Huang Shijie <b32955@freescale.com>
> 
> > +/**
> > + * struct gpmi_nfc_platform_data - GPMI NFC driver platform data.
> > + *
> > + * This structure communicates platform-specific information to the GPMI NFC
> > + * driver that can't be expressed as resources.
> > + *
> > + * @platform_init:           A pointer to a function the driver will call to
> > + *                           initialize the platform (e.g., set up the pin mux).
> > + * @platform_exit:           A pointer to a function the driver will call to
> > + *                           exit the platform (e.g., free pins).
> > + * @min_prop_delay_in_ns:    Minimum propagation delay of GPMI signals to and
> > + *                           from the NAND Flash device, in nanoseconds.
> > + * @max_prop_delay_in_ns:    Maximum propagation delay of GPMI signals to and
> > + *                           from the NAND Flash device, in nanoseconds.
> > + * @max_chip_count:          The maximum number of chips for which the driver
> > + *                           should configure the hardware. This value most
> > + *                           likely reflects the number of pins that are
> > + *                           connected to a NAND Flash device. If this is
> > + *                           greater than the SoC hardware can support, the
> > + *                           driver will print a message and fail to initialize.
> > + * @partitions:              An optional pointer to an array of partition
> > + *                           descriptions.
> > + * @partition_count:         The number of elements in the partitions array.
> > + */
> > +struct gpmi_nfc_platform_data {
> > +	/* SoC hardware information. */
> > +	int		(*platform_init)(void);
> > +	void		(*platform_exit)(void);
> > +
> > +	/* NAND Flash information. */
> > +	unsigned int	min_prop_delay_in_ns;
> > +	unsigned int	max_prop_delay_in_ns;
> > +	unsigned int	max_chip_count;
> > +
> > +	/* Medium information. */
> > +	struct mtd_partition *partitions;
> > +	unsigned	partition_count;
> > +};
> 
> When adding new infrastructure, always keep in mind how you want it to look
> after the device tree conversion. The partitions and min/max_* are easily covered
> with that, but the init/exit function pointers are somewhat problematic.
> 
> Fortunately, you don't really require these functions for this driver. The _exit
> function is completely unused, so just get rid of it.
> 
> The init function is used only to set up iomux, so the logical replacement is
> a pointer to the iomux data, and calling mxs_iomux_setup_multiple_pads
> directly from the driver.
Why not put the iomux stuff into the per-machine table and get rid of
the init callback, too?

Best regards
Uwe
Huang Shijie July 8, 2011, 7:40 a.m. UTC | #11
于 2011年07月08日 15:31, Uwe Kleine-König 写道:
> Hello,
>
> On Thu, Jun 30, 2011 at 03:55:19PM +0200, Arnd Bergmann wrote:
>> On Thursday 30 June 2011, Huang Shijie wrote:
>>> add GPMI-NFC support for imx23 and imx28.
>>>
>>> Signed-off-by: Huang Shijie<b32955@freescale.com>
>>> +/**
>>> + * struct gpmi_nfc_platform_data - GPMI NFC driver platform data.
>>> + *
>>> + * This structure communicates platform-specific information to the GPMI NFC
>>> + * driver that can't be expressed as resources.
>>> + *
>>> + * @platform_init:           A pointer to a function the driver will call to
>>> + *                           initialize the platform (e.g., set up the pin mux).
>>> + * @platform_exit:           A pointer to a function the driver will call to
>>> + *                           exit the platform (e.g., free pins).
>>> + * @min_prop_delay_in_ns:    Minimum propagation delay of GPMI signals to and
>>> + *                           from the NAND Flash device, in nanoseconds.
>>> + * @max_prop_delay_in_ns:    Maximum propagation delay of GPMI signals to and
>>> + *                           from the NAND Flash device, in nanoseconds.
>>> + * @max_chip_count:          The maximum number of chips for which the driver
>>> + *                           should configure the hardware. This value most
>>> + *                           likely reflects the number of pins that are
>>> + *                           connected to a NAND Flash device. If this is
>>> + *                           greater than the SoC hardware can support, the
>>> + *                           driver will print a message and fail to initialize.
>>> + * @partitions:              An optional pointer to an array of partition
>>> + *                           descriptions.
>>> + * @partition_count:         The number of elements in the partitions array.
>>> + */
>>> +struct gpmi_nfc_platform_data {
>>> +	/* SoC hardware information. */
>>> +	int		(*platform_init)(void);
>>> +	void		(*platform_exit)(void);
>>> +
>>> +	/* NAND Flash information. */
>>> +	unsigned int	min_prop_delay_in_ns;
>>> +	unsigned int	max_prop_delay_in_ns;
>>> +	unsigned int	max_chip_count;
>>> +
>>> +	/* Medium information. */
>>> +	struct mtd_partition *partitions;
>>> +	unsigned	partition_count;
>>> +};
>> When adding new infrastructure, always keep in mind how you want it to look
>> after the device tree conversion. The partitions and min/max_* are easily covered
>> with that, but the init/exit function pointers are somewhat problematic.
>>
>> Fortunately, you don't really require these functions for this driver. The _exit
>> function is completely unused, so just get rid of it.
>>
>> The init function is used only to set up iomux, so the logical replacement is
>> a pointer to the iomux data, and calling mxs_iomux_setup_multiple_pads
>> directly from the driver.
> Why not put the iomux stuff into the per-machine table and get rid of
> the init callback, too?

The mmc (ssp) has pin conflict with gpmi on both mx23evk and mx28evk.
So, it's better to initialize the pin when the driver(GPMI or MMC) is 
enabled.
I think the init callback is good solution.

Best Regards
Huang Shijie

> Best regards
> Uwe
>
Arnd Bergmann July 8, 2011, 9:02 a.m. UTC | #12
On Friday 08 July 2011 09:31:19 Uwe Kleine-König wrote:
> > The init function is used only to set up iomux, so the logical replacement is
> > a pointer to the iomux data, and calling mxs_iomux_setup_multiple_pads
> > directly from the driver.
> Why not put the iomux stuff into the per-machine table and get rid of
> the init callback, too?

Sounds good to me.

	Arnd
Uwe Kleine-König July 8, 2011, 9:09 a.m. UTC | #13
Hello,

On Fri, Jul 08, 2011 at 03:40:46PM +0800, Huang Shijie wrote:
> 于 2011年07月08日 15:31, Uwe Kleine-König 写道:
> >Hello,
> >
> >On Thu, Jun 30, 2011 at 03:55:19PM +0200, Arnd Bergmann wrote:
> >>On Thursday 30 June 2011, Huang Shijie wrote:
> >>>add GPMI-NFC support for imx23 and imx28.
> >>>
> >>>Signed-off-by: Huang Shijie<b32955@freescale.com>
> >>>+/**
> >>>+ * struct gpmi_nfc_platform_data - GPMI NFC driver platform data.
> >>>+ *
> >>>+ * This structure communicates platform-specific information to the GPMI NFC
> >>>+ * driver that can't be expressed as resources.
> >>>+ *
> >>>+ * @platform_init:           A pointer to a function the driver will call to
> >>>+ *                           initialize the platform (e.g., set up the pin mux).
> >>>+ * @platform_exit:           A pointer to a function the driver will call to
> >>>+ *                           exit the platform (e.g., free pins).
> >>>+ * @min_prop_delay_in_ns:    Minimum propagation delay of GPMI signals to and
> >>>+ *                           from the NAND Flash device, in nanoseconds.
> >>>+ * @max_prop_delay_in_ns:    Maximum propagation delay of GPMI signals to and
> >>>+ *                           from the NAND Flash device, in nanoseconds.
> >>>+ * @max_chip_count:          The maximum number of chips for which the driver
> >>>+ *                           should configure the hardware. This value most
> >>>+ *                           likely reflects the number of pins that are
> >>>+ *                           connected to a NAND Flash device. If this is
> >>>+ *                           greater than the SoC hardware can support, the
> >>>+ *                           driver will print a message and fail to initialize.
> >>>+ * @partitions:              An optional pointer to an array of partition
> >>>+ *                           descriptions.
> >>>+ * @partition_count:         The number of elements in the partitions array.
> >>>+ */
> >>>+struct gpmi_nfc_platform_data {
> >>>+	/* SoC hardware information. */
> >>>+	int		(*platform_init)(void);
> >>>+	void		(*platform_exit)(void);
> >>>+
> >>>+	/* NAND Flash information. */
> >>>+	unsigned int	min_prop_delay_in_ns;
> >>>+	unsigned int	max_prop_delay_in_ns;
> >>>+	unsigned int	max_chip_count;
> >>>+
> >>>+	/* Medium information. */
> >>>+	struct mtd_partition *partitions;
> >>>+	unsigned	partition_count;
> >>>+};
> >>When adding new infrastructure, always keep in mind how you want it to look
> >>after the device tree conversion. The partitions and min/max_* are easily covered
> >>with that, but the init/exit function pointers are somewhat problematic.
> >>
> >>Fortunately, you don't really require these functions for this driver. The _exit
> >>function is completely unused, so just get rid of it.
> >>
> >>The init function is used only to set up iomux, so the logical replacement is
> >>a pointer to the iomux data, and calling mxs_iomux_setup_multiple_pads
> >>directly from the driver.
> >Why not put the iomux stuff into the per-machine table and get rid of
> >the init callback, too?
> 
> The mmc (ssp) has pin conflict with gpmi on both mx23evk and mx28evk.
> So, it's better to initialize the pin when the driver(GPMI or MMC)
> is enabled.
What do you do to prevent userspace from trying to use both devices?
I guess you need to configure the hardware somehow to switch between the
two using a jumper? Isn't it possible to detect the hardware setting and
setup the muxer accordingly?

IMHO an per-device init-callback is the wrong approach to solve a pin
conflict.

Best regards
Uwe
Huang Shijie July 8, 2011, 9:27 a.m. UTC | #14
于 2011年07月08日 17:09, Uwe Kleine-König 写道:
> Hello,
>
> On Fri, Jul 08, 2011 at 03:40:46PM +0800, Huang Shijie wrote:
>> 于 2011年07月08日 15:31, Uwe Kleine-König 写道:
>>> Hello,
>>>
>>> On Thu, Jun 30, 2011 at 03:55:19PM +0200, Arnd Bergmann wrote:
>>>> On Thursday 30 June 2011, Huang Shijie wrote:
>>>>> add GPMI-NFC support for imx23 and imx28.
>>>>>
>>>>> Signed-off-by: Huang Shijie<b32955@freescale.com>
>>>>> +/**
>>>>> + * struct gpmi_nfc_platform_data - GPMI NFC driver platform data.
>>>>> + *
>>>>> + * This structure communicates platform-specific information to the GPMI NFC
>>>>> + * driver that can't be expressed as resources.
>>>>> + *
>>>>> + * @platform_init:           A pointer to a function the driver will call to
>>>>> + *                           initialize the platform (e.g., set up the pin mux).
>>>>> + * @platform_exit:           A pointer to a function the driver will call to
>>>>> + *                           exit the platform (e.g., free pins).
>>>>> + * @min_prop_delay_in_ns:    Minimum propagation delay of GPMI signals to and
>>>>> + *                           from the NAND Flash device, in nanoseconds.
>>>>> + * @max_prop_delay_in_ns:    Maximum propagation delay of GPMI signals to and
>>>>> + *                           from the NAND Flash device, in nanoseconds.
>>>>> + * @max_chip_count:          The maximum number of chips for which the driver
>>>>> + *                           should configure the hardware. This value most
>>>>> + *                           likely reflects the number of pins that are
>>>>> + *                           connected to a NAND Flash device. If this is
>>>>> + *                           greater than the SoC hardware can support, the
>>>>> + *                           driver will print a message and fail to initialize.
>>>>> + * @partitions:              An optional pointer to an array of partition
>>>>> + *                           descriptions.
>>>>> + * @partition_count:         The number of elements in the partitions array.
>>>>> + */
>>>>> +struct gpmi_nfc_platform_data {
>>>>> +	/* SoC hardware information. */
>>>>> +	int		(*platform_init)(void);
>>>>> +	void		(*platform_exit)(void);
>>>>> +
>>>>> +	/* NAND Flash information. */
>>>>> +	unsigned int	min_prop_delay_in_ns;
>>>>> +	unsigned int	max_prop_delay_in_ns;
>>>>> +	unsigned int	max_chip_count;
>>>>> +
>>>>> +	/* Medium information. */
>>>>> +	struct mtd_partition *partitions;
>>>>> +	unsigned	partition_count;
>>>>> +};
>>>> When adding new infrastructure, always keep in mind how you want it to look
>>>> after the device tree conversion. The partitions and min/max_* are easily covered
>>>> with that, but the init/exit function pointers are somewhat problematic.
>>>>
>>>> Fortunately, you don't really require these functions for this driver. The _exit
>>>> function is completely unused, so just get rid of it.
>>>>
>>>> The init function is used only to set up iomux, so the logical replacement is
>>>> a pointer to the iomux data, and calling mxs_iomux_setup_multiple_pads
>>>> directly from the driver.
>>> Why not put the iomux stuff into the per-machine table and get rid of
>>> the init callback, too?
>> The mmc (ssp) has pin conflict with gpmi on both mx23evk and mx28evk.
>> So, it's better to initialize the pin when the driver(GPMI or MMC)
>> is enabled.
> What do you do to prevent userspace from trying to use both devices?
The board can not support the two devices at the same time.
So the user can only use one device with the board.

> I guess you need to configure the hardware somehow to switch between the
> two using a jumper? Isn't it possible to detect the hardware setting and
> setup the muxer accordingly?
>
> IMHO an per-device init-callback is the wrong approach to solve a pin
> conflict.
Do you have any good solution about this?


Best Regards
Huang Shijie

> Best regards
> Uwe
>
Uwe Kleine-König July 8, 2011, 10:16 a.m. UTC | #15
Hello Huang,

On Fri, Jul 08, 2011 at 05:27:11PM +0800, Huang Shijie wrote:
> >>>>The init function is used only to set up iomux, so the logical replacement is
> >>>>a pointer to the iomux data, and calling mxs_iomux_setup_multiple_pads
> >>>>directly from the driver.
> >>>Why not put the iomux stuff into the per-machine table and get rid of
> >>>the init callback, too?
> >>The mmc (ssp) has pin conflict with gpmi on both mx23evk and mx28evk.
> >>So, it's better to initialize the pin when the driver(GPMI or MMC)
> >>is enabled.
> >What do you do to prevent userspace from trying to use both devices?
> The board can not support the two devices at the same time.
> So the user can only use one device with the board.
> 
> >I guess you need to configure the hardware somehow to switch between the
> >two using a jumper? Isn't it possible to detect the hardware setting and
> >setup the muxer accordingly?
> >
> >IMHO an per-device init-callback is the wrong approach to solve a pin
> >conflict.
> Do you have any good solution about this?
Put the pinmux corresponding to the one device that currently works in
the pinmux list!?

Best regards
Uwe
Lothar Waßmann July 8, 2011, 10:24 a.m. UTC | #16
Uwe Kleine-König writes:
> Hello Huang,
> 
> On Fri, Jul 08, 2011 at 05:27:11PM +0800, Huang Shijie wrote:
> > >>>>The init function is used only to set up iomux, so the logical replacement is
> > >>>>a pointer to the iomux data, and calling mxs_iomux_setup_multiple_pads
> > >>>>directly from the driver.
> > >>>Why not put the iomux stuff into the per-machine table and get rid of
> > >>>the init callback, too?
> > >>The mmc (ssp) has pin conflict with gpmi on both mx23evk and mx28evk.
> > >>So, it's better to initialize the pin when the driver(GPMI or MMC)
> > >>is enabled.
> > >What do you do to prevent userspace from trying to use both devices?
> > The board can not support the two devices at the same time.
> > So the user can only use one device with the board.
> > 
> > >I guess you need to configure the hardware somehow to switch between the
> > >two using a jumper? Isn't it possible to detect the hardware setting and
> > >setup the muxer accordingly?
> > >
> > >IMHO an per-device init-callback is the wrong approach to solve a pin
> > >conflict.
> > Do you have any good solution about this?
> Put the pinmux corresponding to the one device that currently works in
> the pinmux list!?
> 
#define 'that currently works'

For a dedicated system that may not be a problem. But for development
kits and modular systems that allow peripheral modules to be plugged
in there is no 'one device that currently works'.


Lothar Waßmann
Uwe Kleine-König July 11, 2011, 8 a.m. UTC | #17
On Fri, Jul 08, 2011 at 12:24:22PM +0200, Lothar Waßmann wrote:
> Uwe Kleine-König writes:
> > Hello Huang,
> > 
> > On Fri, Jul 08, 2011 at 05:27:11PM +0800, Huang Shijie wrote:
> > > >>>>The init function is used only to set up iomux, so the logical replacement is
> > > >>>>a pointer to the iomux data, and calling mxs_iomux_setup_multiple_pads
> > > >>>>directly from the driver.
> > > >>>Why not put the iomux stuff into the per-machine table and get rid of
> > > >>>the init callback, too?
> > > >>The mmc (ssp) has pin conflict with gpmi on both mx23evk and mx28evk.
> > > >>So, it's better to initialize the pin when the driver(GPMI or MMC)
> > > >>is enabled.
> > > >What do you do to prevent userspace from trying to use both devices?
> > > The board can not support the two devices at the same time.
> > > So the user can only use one device with the board.
> > > 
> > > >I guess you need to configure the hardware somehow to switch between the
> > > >two using a jumper? Isn't it possible to detect the hardware setting and
> > > >setup the muxer accordingly?
> > > >
> > > >IMHO an per-device init-callback is the wrong approach to solve a pin
> > > >conflict.
> > > Do you have any good solution about this?
> > Put the pinmux corresponding to the one device that currently works in
> > the pinmux list!?
> > 
> #define 'that currently works'
> 
> For a dedicated system that may not be a problem. But for development
> kits and modular systems that allow peripheral modules to be plugged
> in there is no 'one device that currently works'.
Yeah, I know that problem. Back when I worked for a company selling
development boards I solved it with clks. Not pretty but more convenient
than kernel parameters or #ifdefs.
The upside of doing it with clks is that if $customer tries to use both
conflicting devices you get an error message instead of breaking device1
when device2 is opened/probed.
IMHO this last scenario must not happen, so I strongly object to setup
the pinmuxing in an .init callback.

Best regards
Uwe
Huang Shijie July 11, 2011, 8:30 a.m. UTC | #18
Hi Uwe:
> On Fri, Jul 08, 2011 at 12:24:22PM +0200, Lothar Waßmann wrote:
>> Uwe Kleine-König writes:
>>> Hello Huang,
>>>
>>> On Fri, Jul 08, 2011 at 05:27:11PM +0800, Huang Shijie wrote:
>>>>>>>> The init function is used only to set up iomux, so the logical replacement is
>>>>>>>> a pointer to the iomux data, and calling mxs_iomux_setup_multiple_pads
>>>>>>>> directly from the driver.
>>>>>>> Why not put the iomux stuff into the per-machine table and get rid of
>>>>>>> the init callback, too?
>>>>>> The mmc (ssp) has pin conflict with gpmi on both mx23evk and mx28evk.
>>>>>> So, it's better to initialize the pin when the driver(GPMI or MMC)
>>>>>> is enabled.
>>>>> What do you do to prevent userspace from trying to use both devices?
>>>> The board can not support the two devices at the same time.
>>>> So the user can only use one device with the board.
>>>>
>>>>> I guess you need to configure the hardware somehow to switch between the
>>>>> two using a jumper? Isn't it possible to detect the hardware setting and
>>>>> setup the muxer accordingly?
>>>>>
>>>>> IMHO an per-device init-callback is the wrong approach to solve a pin
>>>>> conflict.
>>>> Do you have any good solution about this?
>>> Put the pinmux corresponding to the one device that currently works in
>>> the pinmux list!?
>>>
>> #define 'that currently works'
>>
>> For a dedicated system that may not be a problem. But for development
>> kits and modular systems that allow peripheral modules to be plugged
>> in there is no 'one device that currently works'.
> Yeah, I know that problem. Back when I worked for a company selling
> development boards I solved it with clks. Not pretty but more convenient
Could you give me some more details about how did you solve it with clks?

I am confused about it.

thanks

Best Regards
Huang Shijie
> than kernel parameters or #ifdefs.
> The upside of doing it with clks is that if $customer tries to use both
> conflicting devices you get an error message instead of breaking device1
> when device2 is opened/probed.
> IMHO this last scenario must not happen, so I strongly object to setup
> the pinmuxing in an .init callback.
>
> Best regards
> Uwe
>
Lothar Waßmann July 11, 2011, 8:37 a.m. UTC | #19
Uwe Kleine-König writes:
> On Fri, Jul 08, 2011 at 12:24:22PM +0200, Lothar Waßmann wrote:
> > Uwe Kleine-König writes:
> > > Hello Huang,
> > > 
> > > On Fri, Jul 08, 2011 at 05:27:11PM +0800, Huang Shijie wrote:
> > > > >>>>The init function is used only to set up iomux, so the logical replacement is
> > > > >>>>a pointer to the iomux data, and calling mxs_iomux_setup_multiple_pads
> > > > >>>>directly from the driver.
> > > > >>>Why not put the iomux stuff into the per-machine table and get rid of
> > > > >>>the init callback, too?
> > > > >>The mmc (ssp) has pin conflict with gpmi on both mx23evk and mx28evk.
> > > > >>So, it's better to initialize the pin when the driver(GPMI or MMC)
> > > > >>is enabled.
> > > > >What do you do to prevent userspace from trying to use both devices?
> > > > The board can not support the two devices at the same time.
> > > > So the user can only use one device with the board.
> > > > 
> > > > >I guess you need to configure the hardware somehow to switch between the
> > > > >two using a jumper? Isn't it possible to detect the hardware setting and
> > > > >setup the muxer accordingly?
> > > > >
> > > > >IMHO an per-device init-callback is the wrong approach to solve a pin
> > > > >conflict.
> > > > Do you have any good solution about this?
> > > Put the pinmux corresponding to the one device that currently works in
> > > the pinmux list!?
> > > 
> > #define 'that currently works'
> > 
> > For a dedicated system that may not be a problem. But for development
> > kits and modular systems that allow peripheral modules to be plugged
> > in there is no 'one device that currently works'.
> Yeah, I know that problem. Back when I worked for a company selling
> development boards I solved it with clks. Not pretty but more convenient
> than kernel parameters or #ifdefs.
> The upside of doing it with clks is that if $customer tries to use both
> conflicting devices you get an error message instead of breaking device1
> when device2 is opened/probed.
>
This could be prevented by a reservation mechanism for the iomux like
for GPIOs.


Lothar Waßmann
diff mbox

Patch

diff --git a/arch/arm/mach-mxs/clock-mx23.c b/arch/arm/mach-mxs/clock-mx23.c
index 0163b6d..e190c53 100644
--- a/arch/arm/mach-mxs/clock-mx23.c
+++ b/arch/arm/mach-mxs/clock-mx23.c
@@ -456,6 +456,7 @@  static struct clk_lookup lookups[] = {
 	_REGISTER_CLOCK("mxs-pwm.3", NULL, pwm_clk)
 	_REGISTER_CLOCK("mxs-pwm.4", NULL, pwm_clk)
 	_REGISTER_CLOCK("imx23-fb", NULL, lcdif_clk)
+	_REGISTER_CLOCK("imx23-gpmi-nfc", NULL, gpmi_clk)
 };
 
 static int clk_misc_init(void)
diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c
index 5dcc59d..0397f65 100644
--- a/arch/arm/mach-mxs/clock-mx28.c
+++ b/arch/arm/mach-mxs/clock-mx28.c
@@ -614,6 +614,7 @@  static struct clk_lookup lookups[] = {
 	_REGISTER_CLOCK("duart", NULL, uart_clk)
 	_REGISTER_CLOCK("imx28-fec.0", NULL, fec_clk)
 	_REGISTER_CLOCK("imx28-fec.1", NULL, fec_clk)
+	_REGISTER_CLOCK("imx28-gpmi-nfc", NULL, gpmi_clk)
 	_REGISTER_CLOCK("mxs-auart.0", NULL, uart_clk)
 	_REGISTER_CLOCK("mxs-auart.1", NULL, uart_clk)
 	_REGISTER_CLOCK("mxs-auart.2", NULL, uart_clk)
diff --git a/arch/arm/mach-mxs/devices-mx23.h b/arch/arm/mach-mxs/devices-mx23.h
index c6f345f..a9c495d 100644
--- a/arch/arm/mach-mxs/devices-mx23.h
+++ b/arch/arm/mach-mxs/devices-mx23.h
@@ -21,6 +21,10 @@  extern const struct mxs_auart_data mx23_auart_data[] __initconst;
 #define mx23_add_auart0()		mx23_add_auart(0)
 #define mx23_add_auart1()		mx23_add_auart(1)
 
+extern const struct mxs_gpmi_nfc_data mx23_gpmi_nfc_data __initconst;
+#define mx23_add_gpmi_nfc(pdata)	\
+	mxs_add_gpmi_nfc(pdata, &mx23_gpmi_nfc_data)
+
 extern const struct mxs_mxs_mmc_data mx23_mxs_mmc_data[] __initconst;
 #define mx23_add_mxs_mmc(id, pdata) \
 	mxs_add_mxs_mmc(&mx23_mxs_mmc_data[id], pdata)
diff --git a/arch/arm/mach-mxs/devices-mx28.h b/arch/arm/mach-mxs/devices-mx28.h
index 79b9452..bbc8f0c 100644
--- a/arch/arm/mach-mxs/devices-mx28.h
+++ b/arch/arm/mach-mxs/devices-mx28.h
@@ -34,6 +34,10 @@  extern const struct mxs_flexcan_data mx28_flexcan_data[] __initconst;
 #define mx28_add_flexcan0(pdata)	mx28_add_flexcan(0, pdata)
 #define mx28_add_flexcan1(pdata)	mx28_add_flexcan(1, pdata)
 
+extern const struct mxs_gpmi_nfc_data mx28_gpmi_nfc_data __initconst;
+#define mx28_add_gpmi_nfc(pdata)	\
+	mxs_add_gpmi_nfc(pdata, &mx28_gpmi_nfc_data)
+
 extern const struct mxs_mxs_i2c_data mx28_mxs_i2c_data[] __initconst;
 #define mx28_add_mxs_i2c(id)		mxs_add_mxs_i2c(&mx28_mxs_i2c_data[id])
 
diff --git a/arch/arm/mach-mxs/devices/Kconfig b/arch/arm/mach-mxs/devices/Kconfig
index acf9eea..b42a14b 100644
--- a/arch/arm/mach-mxs/devices/Kconfig
+++ b/arch/arm/mach-mxs/devices/Kconfig
@@ -12,6 +12,9 @@  config MXS_HAVE_PLATFORM_FLEXCAN
 	select HAVE_CAN_FLEXCAN if CAN
 	bool
 
+config MXS_HAVE_PLATFORM_GPMI_NFC
+	bool
+
 config MXS_HAVE_PLATFORM_MXS_I2C
 	bool
 
diff --git a/arch/arm/mach-mxs/devices/Makefile b/arch/arm/mach-mxs/devices/Makefile
index 351915c..972abdc 100644
--- a/arch/arm/mach-mxs/devices/Makefile
+++ b/arch/arm/mach-mxs/devices/Makefile
@@ -3,6 +3,7 @@  obj-$(CONFIG_MXS_HAVE_PLATFORM_AUART) += platform-auart.o
 obj-y += platform-dma.o
 obj-$(CONFIG_MXS_HAVE_PLATFORM_FEC) += platform-fec.o
 obj-$(CONFIG_MXS_HAVE_PLATFORM_FLEXCAN) += platform-flexcan.o
+obj-$(CONFIG_MXS_HAVE_PLATFORM_GPMI_NFC) += platform-gpmi-nfc.o
 obj-$(CONFIG_MXS_HAVE_PLATFORM_MXS_I2C) += platform-mxs-i2c.o
 obj-$(CONFIG_MXS_HAVE_PLATFORM_MXS_MMC) += platform-mxs-mmc.o
 obj-$(CONFIG_MXS_HAVE_PLATFORM_MXS_PWM) += platform-mxs-pwm.o
diff --git a/arch/arm/mach-mxs/devices/platform-gpmi-nfc.c b/arch/arm/mach-mxs/devices/platform-gpmi-nfc.c
new file mode 100644
index 0000000..ae88672
--- /dev/null
+++ b/arch/arm/mach-mxs/devices/platform-gpmi-nfc.c
@@ -0,0 +1,90 @@ 
+/*
+ * Copyright (C) 2011 Freescale Semiconductor, Inc. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+#include <asm/sizes.h>
+#include <mach/mx23.h>
+#include <mach/mx28.h>
+#include <mach/devices-common.h>
+
+#define RES_MEM(soc, _id, _s, _n)				\
+	{							\
+		.start = soc ##_## _id ## _BASE_ADDR,		\
+		.end   = soc ##_## _id ## _BASE_ADDR + (_s) - 1,\
+		.name  = (_n),					\
+		.flags = IORESOURCE_MEM,			\
+	}
+
+#define RES_IRQ(soc, _id, _n)			\
+	{					\
+		.start = soc ##_INT_## _id,	\
+		.end   = soc ##_INT_## _id,	\
+		.name  = (_n),			\
+		.flags = IORESOURCE_IRQ,	\
+	}
+
+#define RES_DMA(soc, _i_s, _i_e, _n)		\
+	{					\
+		.start = soc ##_## _i_s,	\
+		.end   = soc ##_## _i_e,	\
+		.name  = (_n),			\
+		.flags = IORESOURCE_DMA,	\
+	}
+
+#ifdef CONFIG_SOC_IMX23
+const struct mxs_gpmi_nfc_data mx23_gpmi_nfc_data __initconst = {
+	.devid = "imx23-gpmi-nfc",
+	.res = {
+		/* GPMI */
+		RES_MEM(MX23, GPMI, SZ_8K, GPMI_NFC_GPMI_REGS_ADDR_RES_NAME),
+		RES_IRQ(MX23, GPMI_ATTENTION, GPMI_NFC_GPMI_INTERRUPT_RES_NAME),
+		/* BCH */
+		RES_MEM(MX23, BCH, SZ_8K, GPMI_NFC_BCH_REGS_ADDR_RES_NAME),
+		RES_IRQ(MX23, BCH, GPMI_NFC_BCH_INTERRUPT_RES_NAME),
+		/* DMA */
+		RES_DMA(MX23, DMA_GPMI0, DMA_GPMI3,
+					GPMI_NFC_DMA_CHANNELS_RES_NAME),
+		RES_IRQ(MX23, GPMI_DMA, GPMI_NFC_DMA_INTERRUPT_RES_NAME),
+	},
+};
+#endif
+
+#ifdef CONFIG_SOC_IMX28
+const struct mxs_gpmi_nfc_data mx28_gpmi_nfc_data __initconst = {
+	.devid = "imx28-gpmi-nfc",
+	.res = {
+		/* GPMI */
+		RES_MEM(MX28, GPMI, SZ_8K, GPMI_NFC_GPMI_REGS_ADDR_RES_NAME),
+		RES_IRQ(MX28, GPMI, GPMI_NFC_GPMI_INTERRUPT_RES_NAME),
+		/* BCH */
+		RES_MEM(MX28, BCH, SZ_8K, GPMI_NFC_BCH_REGS_ADDR_RES_NAME),
+		RES_IRQ(MX28, BCH, GPMI_NFC_BCH_INTERRUPT_RES_NAME),
+		/* DMA */
+		RES_DMA(MX28, DMA_GPMI0, DMA_GPMI7,
+					GPMI_NFC_DMA_CHANNELS_RES_NAME),
+		RES_IRQ(MX28, GPMI_DMA, GPMI_NFC_DMA_INTERRUPT_RES_NAME),
+	},
+};
+#endif
+
+struct platform_device *__init
+mxs_add_gpmi_nfc(const struct gpmi_nfc_platform_data *pdata,
+		const struct mxs_gpmi_nfc_data *data)
+{
+	return mxs_add_platform_device_dmamask(data->devid, -1,
+				data->res, RES_SIZE,
+				pdata, sizeof(*pdata), DMA_BIT_MASK(32));
+}
diff --git a/arch/arm/mach-mxs/include/mach/devices-common.h b/arch/arm/mach-mxs/include/mach/devices-common.h
index 812d7a8..e032120 100644
--- a/arch/arm/mach-mxs/include/mach/devices-common.h
+++ b/arch/arm/mach-mxs/include/mach/devices-common.h
@@ -66,6 +66,16 @@  struct platform_device *__init mxs_add_flexcan(
 		const struct mxs_flexcan_data *data,
 		const struct flexcan_platform_data *pdata);
 
+/* gpmi-nfc */
+#include <mach/gpmi-nfc.h>
+struct mxs_gpmi_nfc_data {
+	const char *devid;
+	const struct resource res[RES_SIZE];
+};
+struct platform_device *__init
+mxs_add_gpmi_nfc(const struct gpmi_nfc_platform_data *pdata,
+		const struct mxs_gpmi_nfc_data *data);
+
 /* i2c */
 struct mxs_mxs_i2c_data {
 	int id;
diff --git a/arch/arm/mach-mxs/include/mach/gpmi-nfc.h b/arch/arm/mach-mxs/include/mach/gpmi-nfc.h
new file mode 100644
index 0000000..eda8192
--- /dev/null
+++ b/arch/arm/mach-mxs/include/mach/gpmi-nfc.h
@@ -0,0 +1,71 @@ 
+/*
+ * Copyright (C) 2011 Freescale Semiconductor, Inc. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifndef __MACH_MXS_GPMI_NFC_H__
+#define __MACH_MXS_GPMI_NFC_H__
+
+/* The size of the resource is fixed. */
+#define RES_SIZE	6
+
+/* Resource names for the GPMI NFC driver. */
+#define GPMI_NFC_GPMI_REGS_ADDR_RES_NAME  "GPMI NFC GPMI Registers"
+#define GPMI_NFC_GPMI_INTERRUPT_RES_NAME  "GPMI NFC GPMI Interrupt"
+#define GPMI_NFC_BCH_REGS_ADDR_RES_NAME   "GPMI NFC BCH Registers"
+#define GPMI_NFC_BCH_INTERRUPT_RES_NAME   "GPMI NFC BCH Interrupt"
+#define GPMI_NFC_DMA_CHANNELS_RES_NAME    "GPMI NFC DMA Channels"
+#define GPMI_NFC_DMA_INTERRUPT_RES_NAME   "GPMI NFC DMA Interrupt"
+
+/**
+ * struct gpmi_nfc_platform_data - GPMI NFC driver platform data.
+ *
+ * This structure communicates platform-specific information to the GPMI NFC
+ * driver that can't be expressed as resources.
+ *
+ * @platform_init:           A pointer to a function the driver will call to
+ *                           initialize the platform (e.g., set up the pin mux).
+ * @platform_exit:           A pointer to a function the driver will call to
+ *                           exit the platform (e.g., free pins).
+ * @min_prop_delay_in_ns:    Minimum propagation delay of GPMI signals to and
+ *                           from the NAND Flash device, in nanoseconds.
+ * @max_prop_delay_in_ns:    Maximum propagation delay of GPMI signals to and
+ *                           from the NAND Flash device, in nanoseconds.
+ * @max_chip_count:          The maximum number of chips for which the driver
+ *                           should configure the hardware. This value most
+ *                           likely reflects the number of pins that are
+ *                           connected to a NAND Flash device. If this is
+ *                           greater than the SoC hardware can support, the
+ *                           driver will print a message and fail to initialize.
+ * @partitions:              An optional pointer to an array of partition
+ *                           descriptions.
+ * @partition_count:         The number of elements in the partitions array.
+ */
+struct gpmi_nfc_platform_data {
+	/* SoC hardware information. */
+	int		(*platform_init)(void);
+	void		(*platform_exit)(void);
+
+	/* NAND Flash information. */
+	unsigned int	min_prop_delay_in_ns;
+	unsigned int	max_prop_delay_in_ns;
+	unsigned int	max_chip_count;
+
+	/* Medium information. */
+	struct mtd_partition *partitions;
+	unsigned	partition_count;
+};
+#endif