RE: [PATCHv5 2/3] ARM: imx: Add gpio-keys to plat-mxc

From: Nguyen Dinh-R00091
Date: Fri Oct 22 2010 - 14:58:25 EST


Hi Uwe,

>-----Original Message-----
>From: Uwe Kleine-König [mailto:u.kleine-koenig@xxxxxxxxxxxxxx]
>Sent: Friday, October 22, 2010 1:28 PM
>To: Nguyen Dinh-R00091
>Cc: linux-kernel@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux@xxxxxxxxxxxxxxxx;
>s.hauer@xxxxxxxxxxxxxx; valentin.longchamp@xxxxxxx; daniel@xxxxxxxx; grant.likely@xxxxxxxxxxxx;
>bryan.wu@xxxxxxxxxxxxx; amit.kucheria@xxxxxxxxxxxxx
>Subject: Re: [PATCHv5 2/3] ARM: imx: Add gpio-keys to plat-mxc
>
>On Fri, Oct 22, 2010 at 11:56:14AM -0500, Dinh.Nguyen@xxxxxxxxxxxxx wrote:
>> From: Dinh Nguyen <Dinh.Nguyen@xxxxxxxxxxxxx>
>>
>> Add imx_add_gpio_keys function to add gpio-keys in plat-mxc
>>
>> This patch is based on Sascha Hauer's imx/linux-2.6 for-rmk tree.
>(This information isn't interesting once the patch is in git, so it can
>better go after the tripple-dash below.)
>
>> Signed-off-by: Dinh Nguyen <Dinh.Nguyen@xxxxxxxxxxxxx>
>Patch 1 depends on this one, so you should swap their order.
>
>> ---
>> arch/arm/plat-mxc/devices/Kconfig | 4 +++
>> arch/arm/plat-mxc/devices/Makefile | 1 +
>> arch/arm/plat-mxc/devices/platform-gpio_keys.c | 27 +++++++++++++++++++++++
>> arch/arm/plat-mxc/include/mach/devices-common.h | 4 +++
>> arch/arm/plat-mxc/include/mach/iomux-mx51.h | 2 +-
>> 5 files changed, 37 insertions(+), 1 deletions(-)
>> create mode 100644 arch/arm/plat-mxc/devices/platform-gpio_keys.c
>>
>> diff --git a/arch/arm/plat-mxc/devices/Kconfig b/arch/arm/plat-mxc/devices/Kconfig
>> index 4047994..f4772e7 100644
>> --- a/arch/arm/plat-mxc/devices/Kconfig
>> +++ b/arch/arm/plat-mxc/devices/Kconfig
>> @@ -9,6 +9,10 @@ config IMX_HAVE_PLATFORM_FLEXCAN
>> select HAVE_CAN_FLEXCAN
>> bool
>>
>> +config IMX_HAVE_PLATFORM_GPIO_KEYS
>> + bool
>> + default y if ARCH_MX51
>> +
>> config IMX_HAVE_PLATFORM_IMX_I2C
>> bool
>>
>> diff --git a/arch/arm/plat-mxc/devices/Makefile b/arch/arm/plat-mxc/devices/Makefile
>> index 0a3c1f0..45aefeb 100644
>> --- a/arch/arm/plat-mxc/devices/Makefile
>> +++ b/arch/arm/plat-mxc/devices/Makefile
>> @@ -1,6 +1,7 @@
>> obj-$(CONFIG_IMX_HAVE_PLATFORM_ESDHC) += platform-esdhc.o
>> obj-$(CONFIG_IMX_HAVE_PLATFORM_FEC) += platform-fec.o
>> obj-$(CONFIG_IMX_HAVE_PLATFORM_FLEXCAN) += platform-flexcan.o
>> +obj-$(CONFIG_IMX_HAVE_PLATFORM_GPIO_KEYS) += platform-gpio_keys.o
>> obj-y += platform-imx-dma.o
>> obj-$(CONFIG_IMX_HAVE_PLATFORM_IMX_I2C) += platform-imx-i2c.o
>> obj-$(CONFIG_IMX_HAVE_PLATFORM_IMX_SSI) += platform-imx-ssi.o
>> diff --git a/arch/arm/plat-mxc/devices/platform-gpio_keys.c b/arch/arm/plat-mxc/devices/platform-
>gpio_keys.c
>> new file mode 100644
>> index 0000000..1c53a53
>> --- /dev/null
>> +++ b/arch/arm/plat-mxc/devices/platform-gpio_keys.c
>> @@ -0,0 +1,27 @@
>> +/*
>> + * Copyright (C) 2010 Freescale Semiconductor, Inc. All Rights Reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * 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/hardware.h>
>> +#include <mach/devices-common.h>
>> +
>> +struct platform_device *__init imx_add_gpio_keys(
>> + const struct gpio_keys_platform_data *pdata)
>> +{
>> + return imx_add_platform_device("gpio-keys", -1, NULL,
>> + 0, pdata, sizeof(*pdata));
>> +}
>> diff --git a/arch/arm/plat-mxc/include/mach/devices-common.h b/arch/arm/plat-
>mxc/include/mach/devices-common.h
>> index 86d7575..8c6896f 100644
>> --- a/arch/arm/plat-mxc/include/mach/devices-common.h
>> +++ b/arch/arm/plat-mxc/include/mach/devices-common.h
>> @@ -29,6 +29,10 @@ struct platform_device *__init imx_add_flexcan(int id,
>> resource_size_t irq,
>> const struct flexcan_platform_data *pdata);
>>
>> +#include <linux/gpio_keys.h>
>> +struct platform_device *__init imx_add_gpio_keys(
>> + const struct gpio_keys_platform_data *pdata);
>> +
>> #include <mach/i2c.h>
>> struct imx_imx_i2c_data {
>> int id;
>> diff --git a/arch/arm/plat-mxc/include/mach/iomux-mx51.h b/arch/arm/plat-mxc/include/mach/iomux-
>mx51.h
>> index e46b1c2..7ae27e8 100644
>> --- a/arch/arm/plat-mxc/include/mach/iomux-mx51.h
>> +++ b/arch/arm/plat-mxc/include/mach/iomux-mx51.h
>> @@ -114,7 +114,7 @@ typedef enum iomux_config {
>> #define MX51_PAD_EIM_A24__GPIO_2_18 IOMUX_PAD(0x450, 0x0bc, 1, 0x0, 0, NO_PAD_CTRL)
>> #define MX51_PAD_EIM_A25__GPIO_2_19 IOMUX_PAD(0x454, 0x0c0, 1, 0x0, 0, NO_PAD_CTRL)
>> #define MX51_PAD_EIM_A26__GPIO_2_20 IOMUX_PAD(0x458, 0x0c4, 1, 0x0, 0, NO_PAD_CTRL)
>> -#define MX51_PAD_EIM_A27__GPIO_2_21 IOMUX_PAD(0x45c, 0x0c8, 1, 0x0, 0, NO_PAD_CTRL)
>> +#define MX51_PAD_EIM_A27__GPIO_2_21 IOMUX_PAD(0x45c, 0x0c8, 1, 0x0, 0,
>MX51_GPIO_PAD_CTRL | PAD_CTL_PUS_100K_UP)
>I share the concerns of Arnaud. MX51_GPIO_PAD_CTRL looks OK,
>PAD_CTL_PUS_100K_UP does not.

How do you suggest I define this pin for Babbage board only? Do I add to iomux-mx51.h
#define MX51_BABBAGE_PAD_EIM_A27__GPIO_2_21 IOMUX_PAD(0x45c, 0x0c8, 1, 0x0, 0,MX51_GPIO_PAD_CTRL | PAD_CTL_PUS_100K_UP) ?

Or do I define the pin in the board file with a 100K_UP? Like how eukrea_mbimxsd-baseboard.c did for MBIMXSD_GPIO_3_31?

>
>> #define MX51_PAD_EIM_EB0__EIM_EB0 IOMUX_PAD(0x460, 0x0cc, 0, 0x0, 0, NO_PAD_CTRL)
>> #define MX51_PAD_EIM_EB1__EIM_EB1 IOMUX_PAD(0x464, 0x0d0, 0, 0x0, 0, NO_PAD_CTRL)
>> #define MX51_PAD_EIM_EB2__GPIO_2_22 IOMUX_PAD(0x468, 0x0d4, 1, 0x0, 0, NO_PAD_CTRL)
>
>Best regards
>Uwe
>
>--
>Pengutronix e.K. | Uwe Kleine-König |
>Industrial Linux Solutions | http://www.pengutronix.de/ |

Thanks for your review comments...

Dinh


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/