* [PATCH v5 0/3] add the GPMI-NFC support for imx23/imx28
@ 2011-06-30 3:53 Huang Shijie
2011-06-30 3:53 ` [PATCH v5 1/3] ARM: mxs: add " Huang Shijie
` (2 more replies)
0 siblings, 3 replies; 26+ messages in thread
From: Huang Shijie @ 2011-06-30 3:53 UTC (permalink / raw)
To: linux-arm-kernel
The patch is based on branch "imx-for-3.1" of tree :
git://git.pengutronix.de/git/imx/linux-2.6.git
v4 --> v5:
[0] split out the mach code and the machine code.
v3 --> v4:
merge the GPMI-NFC device code to the new branch.
Huang Shijie (3):
ARM: mxs: add GPMI-NFC support for imx23/imx28
ARM: mxs/mx23evk: add GPMI-NFC device
ARM: mxs/mx28evk: add GPMI-NFC device
arch/arm/mach-mxs/Kconfig | 2 +
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 ++++++++++++++++++
arch/arm/mach-mxs/mach-mx23evk.c | 37 +++++++++
arch/arm/mach-mxs/mach-mx28evk.c | 37 +++++++++
12 files changed, 261 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
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH v5 1/3] ARM: mxs: add GPMI-NFC support for imx23/imx28 2011-06-30 3:53 [PATCH v5 0/3] add the GPMI-NFC support for imx23/imx28 Huang Shijie @ 2011-06-30 3:53 ` Huang Shijie 2011-06-30 13:55 ` Arnd Bergmann 2011-06-30 3:53 ` [PATCH v5 2/3] ARM: mxs/mx23evk: add GPMI-NFC device Huang Shijie 2011-06-30 3:53 ` [PATCH v5 3/3] ARM: mxs/mx28evk: " Huang Shijie 2 siblings, 1 reply; 26+ messages in thread From: Huang Shijie @ 2011-06-30 3:53 UTC (permalink / raw) To: linux-arm-kernel 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 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 -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v5 1/3] ARM: mxs: add GPMI-NFC support for imx23/imx28 2011-06-30 3:53 ` [PATCH v5 1/3] ARM: mxs: add " Huang Shijie @ 2011-06-30 13:55 ` Arnd Bergmann 2011-06-30 14:58 ` Lothar Waßmann 2011-07-08 7:31 ` Uwe Kleine-König 0 siblings, 2 replies; 26+ messages in thread From: Arnd Bergmann @ 2011-06-30 13:55 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 1/3] ARM: mxs: add GPMI-NFC support for imx23/imx28 2011-06-30 13:55 ` Arnd Bergmann @ 2011-06-30 14:58 ` Lothar Waßmann 2011-06-30 22:22 ` Arnd Bergmann 2011-07-08 7:31 ` Uwe Kleine-König 1 sibling, 1 reply; 26+ messages in thread From: Lothar Waßmann @ 2011-06-30 14:58 UTC (permalink / raw) To: linux-arm-kernel 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 -- ___________________________________________________________ Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10 Gesch?ftsf?hrer: Matthias Kaussen Handelsregistereintrag: Amtsgericht Aachen, HRB 4996 www.karo-electronics.de | info at karo-electronics.de ___________________________________________________________ ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 1/3] ARM: mxs: add GPMI-NFC support for imx23/imx28 2011-06-30 14:58 ` Lothar Waßmann @ 2011-06-30 22:22 ` Arnd Bergmann 2011-07-01 5:59 ` Lothar Waßmann 2011-07-01 6:03 ` Wolfram Sang 0 siblings, 2 replies; 26+ messages in thread From: Arnd Bergmann @ 2011-06-30 22:22 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 1/3] ARM: mxs: add GPMI-NFC support for imx23/imx28 2011-06-30 22:22 ` Arnd Bergmann @ 2011-07-01 5:59 ` Lothar Waßmann 2011-07-01 6:03 ` Wolfram Sang 1 sibling, 0 replies; 26+ messages in thread From: Lothar Waßmann @ 2011-07-01 5:59 UTC (permalink / raw) To: linux-arm-kernel 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 -- ___________________________________________________________ Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10 Gesch?ftsf?hrer: Matthias Kaussen Handelsregistereintrag: Amtsgericht Aachen, HRB 4996 www.karo-electronics.de | info at karo-electronics.de ___________________________________________________________ ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 1/3] ARM: mxs: add GPMI-NFC support for imx23/imx28 2011-06-30 22:22 ` Arnd Bergmann 2011-07-01 5:59 ` Lothar Waßmann @ 2011-07-01 6:03 ` Wolfram Sang 2011-07-01 7:53 ` Huang Shijie 1 sibling, 1 reply; 26+ messages in thread From: Wolfram Sang @ 2011-07-01 6:03 UTC (permalink / raw) To: linux-arm-kernel 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 -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 198 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110701/e03d6241/attachment.sig> ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 1/3] ARM: mxs: add GPMI-NFC support for imx23/imx28 2011-07-01 6:03 ` Wolfram Sang @ 2011-07-01 7:53 ` Huang Shijie 2011-07-01 8:01 ` Wolfram Sang 2011-07-01 9:25 ` Arnd Bergmann 0 siblings, 2 replies; 26+ messages in thread From: Huang Shijie @ 2011-07-01 7:53 UTC (permalink / raw) To: linux-arm-kernel 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 > ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 1/3] ARM: mxs: add GPMI-NFC support for imx23/imx28 2011-07-01 7:53 ` Huang Shijie @ 2011-07-01 8:01 ` Wolfram Sang 2011-07-01 8:39 ` Huang Shijie 2011-07-01 9:25 ` Arnd Bergmann 1 sibling, 1 reply; 26+ messages in thread From: Wolfram Sang @ 2011-07-01 8:01 UTC (permalink / raw) To: linux-arm-kernel 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? -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 198 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110701/cd5f6c2d/attachment.sig> ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 1/3] ARM: mxs: add GPMI-NFC support for imx23/imx28 2011-07-01 8:01 ` Wolfram Sang @ 2011-07-01 8:39 ` Huang Shijie 2011-07-01 8:45 ` Huang Shijie 0 siblings, 1 reply; 26+ messages in thread From: Huang Shijie @ 2011-07-01 8:39 UTC (permalink / raw) To: linux-arm-kernel 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. I will take a business trip in the following several days. I will submit new version when i come back to office. Best regards Huang Shijie ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 1/3] ARM: mxs: add GPMI-NFC support for imx23/imx28 2011-07-01 8:39 ` Huang Shijie @ 2011-07-01 8:45 ` Huang Shijie 0 siblings, 0 replies; 26+ messages in thread From: Huang Shijie @ 2011-07-01 8:45 UTC (permalink / raw) To: linux-arm-kernel ? 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. > ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 1/3] ARM: mxs: add GPMI-NFC support for imx23/imx28 2011-07-01 7:53 ` Huang Shijie 2011-07-01 8:01 ` Wolfram Sang @ 2011-07-01 9:25 ` Arnd Bergmann 1 sibling, 0 replies; 26+ messages in thread From: Arnd Bergmann @ 2011-07-01 9:25 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 1/3] ARM: mxs: add GPMI-NFC support for imx23/imx28 2011-06-30 13:55 ` Arnd Bergmann 2011-06-30 14:58 ` Lothar Waßmann @ 2011-07-08 7:31 ` Uwe Kleine-König 2011-07-08 7:40 ` Huang Shijie 2011-07-08 9:02 ` Arnd Bergmann 1 sibling, 2 replies; 26+ messages in thread From: Uwe Kleine-König @ 2011-07-08 7:31 UTC (permalink / raw) To: linux-arm-kernel 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 -- Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 1/3] ARM: mxs: add GPMI-NFC support for imx23/imx28 2011-07-08 7:31 ` Uwe Kleine-König @ 2011-07-08 7:40 ` Huang Shijie 2011-07-08 9:09 ` Uwe Kleine-König 2011-07-08 9:02 ` Arnd Bergmann 1 sibling, 1 reply; 26+ messages in thread From: Huang Shijie @ 2011-07-08 7:40 UTC (permalink / raw) To: linux-arm-kernel ? 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 > ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 1/3] ARM: mxs: add GPMI-NFC support for imx23/imx28 2011-07-08 7:40 ` Huang Shijie @ 2011-07-08 9:09 ` Uwe Kleine-König 2011-07-08 9:27 ` Huang Shijie 0 siblings, 1 reply; 26+ messages in thread From: Uwe Kleine-König @ 2011-07-08 9:09 UTC (permalink / raw) To: linux-arm-kernel 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 -- Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 1/3] ARM: mxs: add GPMI-NFC support for imx23/imx28 2011-07-08 9:09 ` Uwe Kleine-König @ 2011-07-08 9:27 ` Huang Shijie 2011-07-08 10:16 ` Uwe Kleine-König 0 siblings, 1 reply; 26+ messages in thread From: Huang Shijie @ 2011-07-08 9:27 UTC (permalink / raw) To: linux-arm-kernel ? 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 > ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 1/3] ARM: mxs: add GPMI-NFC support for imx23/imx28 2011-07-08 9:27 ` Huang Shijie @ 2011-07-08 10:16 ` Uwe Kleine-König 2011-07-08 10:24 ` Lothar Waßmann 0 siblings, 1 reply; 26+ messages in thread From: Uwe Kleine-König @ 2011-07-08 10:16 UTC (permalink / raw) To: linux-arm-kernel 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 -- Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 1/3] ARM: mxs: add GPMI-NFC support for imx23/imx28 2011-07-08 10:16 ` Uwe Kleine-König @ 2011-07-08 10:24 ` Lothar Waßmann 2011-07-11 8:00 ` Uwe Kleine-König 0 siblings, 1 reply; 26+ messages in thread From: Lothar Waßmann @ 2011-07-08 10:24 UTC (permalink / raw) To: linux-arm-kernel 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 -- ___________________________________________________________ Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10 Gesch?ftsf?hrer: Matthias Kaussen Handelsregistereintrag: Amtsgericht Aachen, HRB 4996 www.karo-electronics.de | info at karo-electronics.de ___________________________________________________________ ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 1/3] ARM: mxs: add GPMI-NFC support for imx23/imx28 2011-07-08 10:24 ` Lothar Waßmann @ 2011-07-11 8:00 ` Uwe Kleine-König 2011-07-11 8:30 ` Huang Shijie 2011-07-11 8:37 ` Lothar Waßmann 0 siblings, 2 replies; 26+ messages in thread From: Uwe Kleine-König @ 2011-07-11 8:00 UTC (permalink / raw) To: linux-arm-kernel 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 -- Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 1/3] ARM: mxs: add GPMI-NFC support for imx23/imx28 2011-07-11 8:00 ` Uwe Kleine-König @ 2011-07-11 8:30 ` Huang Shijie 2011-07-11 8:37 ` Lothar Waßmann 1 sibling, 0 replies; 26+ messages in thread From: Huang Shijie @ 2011-07-11 8:30 UTC (permalink / raw) To: linux-arm-kernel 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 > ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 1/3] ARM: mxs: add GPMI-NFC support for imx23/imx28 2011-07-11 8:00 ` Uwe Kleine-König 2011-07-11 8:30 ` Huang Shijie @ 2011-07-11 8:37 ` Lothar Waßmann 1 sibling, 0 replies; 26+ messages in thread From: Lothar Waßmann @ 2011-07-11 8:37 UTC (permalink / raw) To: linux-arm-kernel 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 -- ___________________________________________________________ Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10 Gesch?ftsf?hrer: Matthias Kaussen Handelsregistereintrag: Amtsgericht Aachen, HRB 4996 www.karo-electronics.de | info at karo-electronics.de ___________________________________________________________ ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 1/3] ARM: mxs: add GPMI-NFC support for imx23/imx28 2011-07-08 7:31 ` Uwe Kleine-König 2011-07-08 7:40 ` Huang Shijie @ 2011-07-08 9:02 ` Arnd Bergmann 1 sibling, 0 replies; 26+ messages in thread From: Arnd Bergmann @ 2011-07-08 9:02 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 2/3] ARM: mxs/mx23evk: add GPMI-NFC device 2011-06-30 3:53 [PATCH v5 0/3] add the GPMI-NFC support for imx23/imx28 Huang Shijie 2011-06-30 3:53 ` [PATCH v5 1/3] ARM: mxs: add " Huang Shijie @ 2011-06-30 3:53 ` Huang Shijie 2011-06-30 7:55 ` Uwe Kleine-König 2011-06-30 3:53 ` [PATCH v5 3/3] ARM: mxs/mx28evk: " Huang Shijie 2 siblings, 1 reply; 26+ messages in thread From: Huang Shijie @ 2011-06-30 3:53 UTC (permalink / raw) To: linux-arm-kernel add the GPMI-NFC device for mx23evk borad. Signed-off-by: Huang Shijie <b32955@freescale.com> --- arch/arm/mach-mxs/Kconfig | 1 + arch/arm/mach-mxs/mach-mx23evk.c | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-mxs/Kconfig b/arch/arm/mach-mxs/Kconfig index 1d3985f..f55bcfe 100644 --- a/arch/arm/mach-mxs/Kconfig +++ b/arch/arm/mach-mxs/Kconfig @@ -32,6 +32,7 @@ config MACH_MX23EVK select SOC_IMX23 select MXS_HAVE_AMBA_DUART select MXS_HAVE_PLATFORM_AUART + select MXS_HAVE_PLATFORM_GPMI_NFC select MXS_HAVE_PLATFORM_MXS_MMC select MXS_HAVE_PLATFORM_MXSFB help diff --git a/arch/arm/mach-mxs/mach-mx23evk.c b/arch/arm/mach-mxs/mach-mx23evk.c index 3c2de33..772b373 100644 --- a/arch/arm/mach-mxs/mach-mx23evk.c +++ b/arch/arm/mach-mxs/mach-mx23evk.c @@ -107,6 +107,42 @@ static const iomux_cfg_t mx23evk_pads[] __initconst = { (MXS_PAD_4MA | MXS_PAD_3V3 | MXS_PAD_NOPULL), }; +/* gpmi-nfc */ +#define MXS_PAD_GPMI_NFC (MXS_PAD_12MA | MXS_PAD_3V3 | MXS_PAD_NOPULL) +static iomux_cfg_t mx23evk_gpmi_nfc_pads[] = { + MX23_PAD_GPMI_D00__GPMI_D00 | MXS_PAD_CTRL, + MX23_PAD_GPMI_D01__GPMI_D01 | MXS_PAD_CTRL, + MX23_PAD_GPMI_D02__GPMI_D02 | MXS_PAD_CTRL, + MX23_PAD_GPMI_D03__GPMI_D03 | MXS_PAD_CTRL, + MX23_PAD_GPMI_D04__GPMI_D04 | MXS_PAD_CTRL, + MX23_PAD_GPMI_D05__GPMI_D05 | MXS_PAD_CTRL, + MX23_PAD_GPMI_D06__GPMI_D06 | MXS_PAD_CTRL, + MX23_PAD_GPMI_D07__GPMI_D07 | MXS_PAD_CTRL, + MX23_PAD_GPMI_CLE__GPMI_CLE | MXS_PAD_CTRL, + MX23_PAD_GPMI_ALE__GPMI_ALE | MXS_PAD_CTRL, + MX23_PAD_GPMI_WPN__GPMI_WPN | MXS_PAD_GPMI_NFC, + MX23_PAD_GPMI_WRN__GPMI_WRN | MXS_PAD_GPMI_NFC, + MX23_PAD_GPMI_RDN__GPMI_RDN | MXS_PAD_GPMI_NFC, + MX23_PAD_GPMI_RDY0__GPMI_RDY0 | MXS_PAD_CTRL, + MX23_PAD_GPMI_RDY1__GPMI_RDY1 | MXS_PAD_CTRL, + MX23_PAD_GPMI_CE0N__GPMI_CE0N | MXS_PAD_CTRL, + MX23_PAD_GPMI_CE1N__GPMI_CE1N | MXS_PAD_CTRL, +}; + +static int mx23evk_gpmi_nfc_platform_init(void) +{ + return mxs_iomux_setup_multiple_pads(mx23evk_gpmi_nfc_pads, + ARRAY_SIZE(mx23evk_gpmi_nfc_pads)); +} + +static const +struct gpmi_nfc_platform_data mx23evk_gpmi_nfc_data __initconst = { + .platform_init = mx23evk_gpmi_nfc_platform_init, + .min_prop_delay_in_ns = 5, + .max_prop_delay_in_ns = 9, + .max_chip_count = 1, +}; + /* mxsfb (lcdif) */ static struct fb_videomode mx23evk_video_modes[] = { { @@ -166,6 +202,7 @@ static void __init mx23evk_init(void) else gpio_set_value(MX23EVK_BL_ENABLE, 1); + mx23_add_gpmi_nfc(&mx23evk_gpmi_nfc_data); mx23_add_mxsfb(&mx23evk_mxsfb_pdata); } -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v5 2/3] ARM: mxs/mx23evk: add GPMI-NFC device 2011-06-30 3:53 ` [PATCH v5 2/3] ARM: mxs/mx23evk: add GPMI-NFC device Huang Shijie @ 2011-06-30 7:55 ` Uwe Kleine-König 2011-06-30 8:37 ` Huang Shijie 0 siblings, 1 reply; 26+ messages in thread From: Uwe Kleine-König @ 2011-06-30 7:55 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jun 30, 2011 at 11:53:47AM +0800, Huang Shijie wrote: > add the GPMI-NFC device for mx23evk borad. s/borad/board/ > Signed-off-by: Huang Shijie <b32955@freescale.com> > --- > arch/arm/mach-mxs/Kconfig | 1 + > arch/arm/mach-mxs/mach-mx23evk.c | 37 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 38 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/mach-mxs/Kconfig b/arch/arm/mach-mxs/Kconfig > index 1d3985f..f55bcfe 100644 > --- a/arch/arm/mach-mxs/Kconfig > +++ b/arch/arm/mach-mxs/Kconfig > @@ -32,6 +32,7 @@ config MACH_MX23EVK > select SOC_IMX23 > select MXS_HAVE_AMBA_DUART > select MXS_HAVE_PLATFORM_AUART > + select MXS_HAVE_PLATFORM_GPMI_NFC > select MXS_HAVE_PLATFORM_MXS_MMC > select MXS_HAVE_PLATFORM_MXSFB > help > diff --git a/arch/arm/mach-mxs/mach-mx23evk.c b/arch/arm/mach-mxs/mach-mx23evk.c > index 3c2de33..772b373 100644 > --- a/arch/arm/mach-mxs/mach-mx23evk.c > +++ b/arch/arm/mach-mxs/mach-mx23evk.c > @@ -107,6 +107,42 @@ static const iomux_cfg_t mx23evk_pads[] __initconst = { > (MXS_PAD_4MA | MXS_PAD_3V3 | MXS_PAD_NOPULL), > }; > > +/* gpmi-nfc */ > +#define MXS_PAD_GPMI_NFC (MXS_PAD_12MA | MXS_PAD_3V3 | MXS_PAD_NOPULL) Maybe put that into a more central place, as mach-mx28evk uses it, too? OTOH the name sounds more generic than its use. Do you really need 12mA for the Write protect pin? If not, you could rename it to MXS_PAD_GPMI_NFC_STROBE. > +static iomux_cfg_t mx23evk_gpmi_nfc_pads[] = { > + MX23_PAD_GPMI_D00__GPMI_D00 | MXS_PAD_CTRL, > + MX23_PAD_GPMI_D01__GPMI_D01 | MXS_PAD_CTRL, > + MX23_PAD_GPMI_D02__GPMI_D02 | MXS_PAD_CTRL, > + MX23_PAD_GPMI_D03__GPMI_D03 | MXS_PAD_CTRL, > + MX23_PAD_GPMI_D04__GPMI_D04 | MXS_PAD_CTRL, > + MX23_PAD_GPMI_D05__GPMI_D05 | MXS_PAD_CTRL, > + MX23_PAD_GPMI_D06__GPMI_D06 | MXS_PAD_CTRL, > + MX23_PAD_GPMI_D07__GPMI_D07 | MXS_PAD_CTRL, > + MX23_PAD_GPMI_CLE__GPMI_CLE | MXS_PAD_CTRL, > + MX23_PAD_GPMI_ALE__GPMI_ALE | MXS_PAD_CTRL, > + MX23_PAD_GPMI_WPN__GPMI_WPN | MXS_PAD_GPMI_NFC, > + MX23_PAD_GPMI_WRN__GPMI_WRN | MXS_PAD_GPMI_NFC, > + MX23_PAD_GPMI_RDN__GPMI_RDN | MXS_PAD_GPMI_NFC, > + MX23_PAD_GPMI_RDY0__GPMI_RDY0 | MXS_PAD_CTRL, > + MX23_PAD_GPMI_RDY1__GPMI_RDY1 | MXS_PAD_CTRL, > + MX23_PAD_GPMI_CE0N__GPMI_CE0N | MXS_PAD_CTRL, > + MX23_PAD_GPMI_CE1N__GPMI_CE1N | MXS_PAD_CTRL, > +}; Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 2/3] ARM: mxs/mx23evk: add GPMI-NFC device 2011-06-30 7:55 ` Uwe Kleine-König @ 2011-06-30 8:37 ` Huang Shijie 0 siblings, 0 replies; 26+ messages in thread From: Huang Shijie @ 2011-06-30 8:37 UTC (permalink / raw) To: linux-arm-kernel Hi: > On Thu, Jun 30, 2011 at 11:53:47AM +0800, Huang Shijie wrote: >> add the GPMI-NFC device for mx23evk borad. > s/borad/board/ > >> Signed-off-by: Huang Shijie<b32955@freescale.com> >> --- >> arch/arm/mach-mxs/Kconfig | 1 + >> arch/arm/mach-mxs/mach-mx23evk.c | 37 +++++++++++++++++++++++++++++++++++++ >> 2 files changed, 38 insertions(+), 0 deletions(-) >> >> diff --git a/arch/arm/mach-mxs/Kconfig b/arch/arm/mach-mxs/Kconfig >> index 1d3985f..f55bcfe 100644 >> --- a/arch/arm/mach-mxs/Kconfig >> +++ b/arch/arm/mach-mxs/Kconfig >> @@ -32,6 +32,7 @@ config MACH_MX23EVK >> select SOC_IMX23 >> select MXS_HAVE_AMBA_DUART >> select MXS_HAVE_PLATFORM_AUART >> + select MXS_HAVE_PLATFORM_GPMI_NFC >> select MXS_HAVE_PLATFORM_MXS_MMC >> select MXS_HAVE_PLATFORM_MXSFB >> help >> diff --git a/arch/arm/mach-mxs/mach-mx23evk.c b/arch/arm/mach-mxs/mach-mx23evk.c >> index 3c2de33..772b373 100644 >> --- a/arch/arm/mach-mxs/mach-mx23evk.c >> +++ b/arch/arm/mach-mxs/mach-mx23evk.c >> @@ -107,6 +107,42 @@ static const iomux_cfg_t mx23evk_pads[] __initconst = { >> (MXS_PAD_4MA | MXS_PAD_3V3 | MXS_PAD_NOPULL), >> }; >> >> +/* gpmi-nfc */ >> +#define MXS_PAD_GPMI_NFC (MXS_PAD_12MA | MXS_PAD_3V3 | MXS_PAD_NOPULL) > Maybe put that into a more central place, as mach-mx28evk uses it, too? yes. The mach-mx28evk needs it too. If I put it into the iomux.h, what's the name for it? MXS_PAD_CTRL_12MA , or something else ? > OTOH the name sounds more generic than its use. Do you really need 12mA Yes, I need the 12mA for the Write protect pin, and other two pins. Best Regards Huang Shijie > for the Write protect pin? If not, you could rename it to > MXS_PAD_GPMI_NFC_STROBE. > >> +static iomux_cfg_t mx23evk_gpmi_nfc_pads[] = { >> + MX23_PAD_GPMI_D00__GPMI_D00 | MXS_PAD_CTRL, >> + MX23_PAD_GPMI_D01__GPMI_D01 | MXS_PAD_CTRL, >> + MX23_PAD_GPMI_D02__GPMI_D02 | MXS_PAD_CTRL, >> + MX23_PAD_GPMI_D03__GPMI_D03 | MXS_PAD_CTRL, >> + MX23_PAD_GPMI_D04__GPMI_D04 | MXS_PAD_CTRL, >> + MX23_PAD_GPMI_D05__GPMI_D05 | MXS_PAD_CTRL, >> + MX23_PAD_GPMI_D06__GPMI_D06 | MXS_PAD_CTRL, >> + MX23_PAD_GPMI_D07__GPMI_D07 | MXS_PAD_CTRL, >> + MX23_PAD_GPMI_CLE__GPMI_CLE | MXS_PAD_CTRL, >> + MX23_PAD_GPMI_ALE__GPMI_ALE | MXS_PAD_CTRL, >> + MX23_PAD_GPMI_WPN__GPMI_WPN | MXS_PAD_GPMI_NFC, >> + MX23_PAD_GPMI_WRN__GPMI_WRN | MXS_PAD_GPMI_NFC, >> + MX23_PAD_GPMI_RDN__GPMI_RDN | MXS_PAD_GPMI_NFC, >> + MX23_PAD_GPMI_RDY0__GPMI_RDY0 | MXS_PAD_CTRL, >> + MX23_PAD_GPMI_RDY1__GPMI_RDY1 | MXS_PAD_CTRL, >> + MX23_PAD_GPMI_CE0N__GPMI_CE0N | MXS_PAD_CTRL, >> + MX23_PAD_GPMI_CE1N__GPMI_CE1N | MXS_PAD_CTRL, >> +}; > Best regards > Uwe > ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 3/3] ARM: mxs/mx28evk: add GPMI-NFC device 2011-06-30 3:53 [PATCH v5 0/3] add the GPMI-NFC support for imx23/imx28 Huang Shijie 2011-06-30 3:53 ` [PATCH v5 1/3] ARM: mxs: add " Huang Shijie 2011-06-30 3:53 ` [PATCH v5 2/3] ARM: mxs/mx23evk: add GPMI-NFC device Huang Shijie @ 2011-06-30 3:53 ` Huang Shijie 2 siblings, 0 replies; 26+ messages in thread From: Huang Shijie @ 2011-06-30 3:53 UTC (permalink / raw) To: linux-arm-kernel add GPMI-NFC device for mx28evk board. Signed-off-by: Huang Shijie <b32955@freescale.com> --- arch/arm/mach-mxs/Kconfig | 1 + arch/arm/mach-mxs/mach-mx28evk.c | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-mxs/Kconfig b/arch/arm/mach-mxs/Kconfig index f55bcfe..a5b753a 100644 --- a/arch/arm/mach-mxs/Kconfig +++ b/arch/arm/mach-mxs/Kconfig @@ -47,6 +47,7 @@ config MACH_MX28EVK select MXS_HAVE_PLATFORM_AUART select MXS_HAVE_PLATFORM_FEC select MXS_HAVE_PLATFORM_FLEXCAN + select MXS_HAVE_PLATFORM_GPMI_NFC select MXS_HAVE_PLATFORM_MXS_MMC select MXS_HAVE_PLATFORM_MXSFB select MXS_OCOTP diff --git a/arch/arm/mach-mxs/mach-mx28evk.c b/arch/arm/mach-mxs/mach-mx28evk.c index eaaf6ff..74d0868 100644 --- a/arch/arm/mach-mxs/mach-mx28evk.c +++ b/arch/arm/mach-mxs/mach-mx28evk.c @@ -314,6 +314,42 @@ static const struct flexcan_platform_data } }; +/* gpmi-nfc */ +#define MXS_PAD_GPMI_NFC (MXS_PAD_12MA | MXS_PAD_3V3 | MXS_PAD_NOPULL) +static iomux_cfg_t mx28evk_gpmi_nfc_pads[] = { + MX28_PAD_GPMI_D00__GPMI_D0 | MXS_PAD_CTRL, + MX28_PAD_GPMI_D01__GPMI_D1 | MXS_PAD_CTRL, + MX28_PAD_GPMI_D02__GPMI_D2 | MXS_PAD_CTRL, + MX28_PAD_GPMI_D03__GPMI_D3 | MXS_PAD_CTRL, + MX28_PAD_GPMI_D04__GPMI_D4 | MXS_PAD_CTRL, + MX28_PAD_GPMI_D05__GPMI_D5 | MXS_PAD_CTRL, + MX28_PAD_GPMI_D06__GPMI_D6 | MXS_PAD_CTRL, + MX28_PAD_GPMI_D07__GPMI_D7 | MXS_PAD_CTRL, + MX28_PAD_GPMI_ALE__GPMI_ALE | MXS_PAD_CTRL, + MX28_PAD_GPMI_CLE__GPMI_CLE | MXS_PAD_CTRL, + MX28_PAD_GPMI_CE0N__GPMI_CE0N | MXS_PAD_CTRL, + MX28_PAD_GPMI_CE1N__GPMI_CE1N | MXS_PAD_CTRL, + MX28_PAD_GPMI_RDY0__GPMI_READY0 | MXS_PAD_CTRL, + MX28_PAD_GPMI_RDY1__GPMI_READY1 | MXS_PAD_CTRL, + MX28_PAD_GPMI_RDN__GPMI_RDN | MXS_PAD_GPMI_NFC, + MX28_PAD_GPMI_WRN__GPMI_WRN | MXS_PAD_GPMI_NFC, + MX28_PAD_GPMI_RESETN__GPMI_RESETN | MXS_PAD_GPMI_NFC, +}; + +static int mx28evk_gpmi_nfc_platform_init(void) +{ + return mxs_iomux_setup_multiple_pads(mx28evk_gpmi_nfc_pads, + ARRAY_SIZE(mx28evk_gpmi_nfc_pads)); +} + +static const struct gpmi_nfc_platform_data +mx28evk_gpmi_nfc_data __initconst = { + .platform_init = mx28evk_gpmi_nfc_platform_init, + .min_prop_delay_in_ns = 5, + .max_prop_delay_in_ns = 9, + .max_chip_count = 1, +}; + /* mxsfb (lcdif) */ static struct fb_videomode mx28evk_video_modes[] = { { @@ -390,6 +426,7 @@ static void __init mx28evk_init(void) else gpio_set_value(MX28EVK_BL_ENABLE, 1); + mx28_add_gpmi_nfc(&mx28evk_gpmi_nfc_data); mx28_add_mxsfb(&mx28evk_mxsfb_pdata); /* power on mmc slot by writing 0 to the gpio */ -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 26+ messages in thread
end of thread, other threads:[~2011-07-11 8:37 UTC | newest] Thread overview: 26+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-06-30 3:53 [PATCH v5 0/3] add the GPMI-NFC support for imx23/imx28 Huang Shijie 2011-06-30 3:53 ` [PATCH v5 1/3] ARM: mxs: add " Huang Shijie 2011-06-30 13:55 ` Arnd Bergmann 2011-06-30 14:58 ` Lothar Waßmann 2011-06-30 22:22 ` Arnd Bergmann 2011-07-01 5:59 ` Lothar Waßmann 2011-07-01 6:03 ` Wolfram Sang 2011-07-01 7:53 ` Huang Shijie 2011-07-01 8:01 ` Wolfram Sang 2011-07-01 8:39 ` Huang Shijie 2011-07-01 8:45 ` Huang Shijie 2011-07-01 9:25 ` Arnd Bergmann 2011-07-08 7:31 ` Uwe Kleine-König 2011-07-08 7:40 ` Huang Shijie 2011-07-08 9:09 ` Uwe Kleine-König 2011-07-08 9:27 ` Huang Shijie 2011-07-08 10:16 ` Uwe Kleine-König 2011-07-08 10:24 ` Lothar Waßmann 2011-07-11 8:00 ` Uwe Kleine-König 2011-07-11 8:30 ` Huang Shijie 2011-07-11 8:37 ` Lothar Waßmann 2011-07-08 9:02 ` Arnd Bergmann 2011-06-30 3:53 ` [PATCH v5 2/3] ARM: mxs/mx23evk: add GPMI-NFC device Huang Shijie 2011-06-30 7:55 ` Uwe Kleine-König 2011-06-30 8:37 ` Huang Shijie 2011-06-30 3:53 ` [PATCH v5 3/3] ARM: mxs/mx28evk: " Huang Shijie
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).