[RFC PATCH v2 0/4] pinctrl: imx: add pinnmux support

[RFC PATCH v2 0/4] pinctrl: imx: add pinnmux support

[Dong Aisheng]

The new patch series shows the basic idea of parsing data from device tree.
The implementation bases on the assumption that board knows the function uses
which pin group and it only passes one group per one function to ease the parsing
effort. The core driver will construct the structure dynamically by parsing
the data from dts file in probe.

For DT support, there's still one issue that for pinmux_map,
since it's board specific, i wonder if it can be parsed by pinctrl
subsystem via dt rather than by each driver.
but pinctrl still does not supported that, i still only called
it in mach-imx6q.c which seems not a correct place.

The patch still has a few other work to do like add pinconf, gpio support and
one know issue is that when add pinconf support, the trick that using pin->id
to calculate the reg offset may not work since pin config registers are not
layout regularly and we may find a better way to do that.

So it's mainly for discussion on the visiable issues.

Changes since v1:
 * add a cover letter
 * remove the pin functon and group defines out of driver
 * parsing data from device tree for imx6q support
 * a few minor fixes suggested by Shawn

Dong Aisheng (4):
  dt: add of_get_child_number helper function
  pinctrl: imx: add pinmux imx driver
  ARM: imx6q: using pinmux subsystem
  mmc: sdhci-esdhc-imx: using pinmux subsystem

 arch/arm/boot/dts/imx6q-sabreauto.dts |   22 ++
 arch/arm/boot/dts/imx6q.dtsi          |    1 +
 arch/arm/mach-imx/Kconfig             |    1 +
 arch/arm/mach-imx/mach-imx6q.c        |    8 +-
 drivers/mmc/host/sdhci-esdhc-imx.c    |   20 ++
 drivers/pinctrl/Kconfig               |   20 ++
 drivers/pinctrl/Makefile              |    3 +
 drivers/pinctrl/pinmux-imx-core.c     |  435 ++++++++++++++++++++++++++++++++
 drivers/pinctrl/pinmux-imx-core.h     |   86 +++++++
 drivers/pinctrl/pinmux-imx53.c        |  443 +++++++++++++++++++++++++++++++++
 drivers/pinctrl/pinmux-imx6q.c        |  433 ++++++++++++++++++++++++++++++++
 include/linux/of.h                    |   17 ++
 12 files changed, 1488 insertions(+), 1 deletions(-)
 create mode 100644 drivers/pinctrl/pinmux-imx-core.c
 create mode 100644 drivers/pinctrl/pinmux-imx-core.h
 create mode 100644 drivers/pinctrl/pinmux-imx53.c
 create mode 100644 drivers/pinctrl/pinmux-imx6q.c

[RFC PATCH v2 1/4] dt: add of_get_child_number helper function

[Dong Aisheng]

From: Dong Aisheng <dong.aisheng@linaro.org>

Currently most code to get child number in kernel are almost same,
add a helper to implement this function for dt to use.

Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Rob Herring <rob.herring@calxeda.com>
---
 include/linux/of.h |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/include/linux/of.h b/include/linux/of.h
index 4948552..09d53dc 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -189,6 +189,18 @@ extern struct device_node *of_get_next_child(const struct device_node *node,
 	for (child = of_get_next_child(parent, NULL); child != NULL; \
 	     child = of_get_next_child(parent, child))
 
+static inline int of_get_child_number(struct device_node *np)
+
+{
+	struct device_node *child = NULL;
+	int num = 0;
+
+	while((child = of_get_next_child(np, child)))
+		num++;
+
+	return num;
+}
+
 extern struct device_node *of_find_node_with_property(
 	struct device_node *from, const char *prop_name);
 #define for_each_node_with_property(dn, prop_name) \
@@ -262,6 +274,11 @@ static inline bool of_have_populated_dt(void)
 #define for_each_child_of_node(parent, child) \
 	while (0)
 
+static inline int of_get_child_number(struct device_node *np)
+{
+	return -ENOSYS;
+}
+
 static inline int of_device_is_compatible(const struct device_node *device,
 					  const char *name)
 {
-- 
1.7.0.4

[RFC PATCH v2 1/4] dt: add of_get_child_number helper function

[Rob Herring]

Dong,

On 12/14/2011 10:03 AM, Dong Aisheng wrote:
> From: Dong Aisheng <dong.aisheng@linaro.org>
> 
> Currently most code to get child number in kernel are almost same,
> add a helper to implement this function for dt to use.
>
> Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Rob Herring <rob.herring@calxeda.com>
> ---
>  include/linux/of.h |   17 +++++++++++++++++
>  1 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 4948552..09d53dc 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -189,6 +189,18 @@ extern struct device_node *of_get_next_child(const struct device_node *node,
>  	for (child = of_get_next_child(parent, NULL); child != NULL; \
>  	     child = of_get_next_child(parent, child))
>  
> +static inline int of_get_child_number(struct device_node *np)

I would call this of_get_child_count instead.

Rob

[RFC PATCH v2 1/4] dt: add of_get_child_number helper function

[Grant Likely]

On Wed, Dec 14, 2011 at 11:04 AM, Rob Herring <robherring2@gmail.com> wrote:
> Dong,
>
> On 12/14/2011 10:03 AM, Dong Aisheng wrote:
>> From: Dong Aisheng <dong.aisheng@linaro.org>
>>
>> Currently most code to get child number in kernel are almost same,
>> add a helper to implement this function for dt to use.
>>
>> Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
>> Cc: Grant Likely <grant.likely@secretlab.ca>
>> Cc: Rob Herring <rob.herring@calxeda.com>
>> ---
>> ?include/linux/of.h | ? 17 +++++++++++++++++
>> ?1 files changed, 17 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/linux/of.h b/include/linux/of.h
>> index 4948552..09d53dc 100644
>> --- a/include/linux/of.h
>> +++ b/include/linux/of.h
>> @@ -189,6 +189,18 @@ extern struct device_node *of_get_next_child(const struct device_node *node,
>> ? ? ? for (child = of_get_next_child(parent, NULL); child != NULL; \
>> ? ? ? ? ? ?child = of_get_next_child(parent, child))
>>
>> +static inline int of_get_child_number(struct device_node *np)
>
> I would call this of_get_child_count instead.

I agree.

>
> Rob



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

[RFC PATCH v2 1/4] dt: add of_get_child_number helper function

[Dong Aisheng-B29396]

> -----Original Message-----
> From: glikely at secretlab.ca [mailto:glikely at secretlab.ca] On Behalf Of
> Grant Likely
> Sent: Thursday, December 15, 2011 5:41 AM
> To: Rob Herring
> Cc: Dong Aisheng-B29396; linux-kernel at vger.kernel.org; linux-arm-
> kernel at lists.infradead.org; linus.walleij at stericsson.com;
> s.hauer at pengutronix.de; Guo Shawn-R65073; kernel at pengutronix.de
> Subject: Re: [RFC PATCH v2 1/4] dt: add of_get_child_number helper
> function
> Importance: High
> 
> On Wed, Dec 14, 2011 at 11:04 AM, Rob Herring <robherring2@gmail.com>
> wrote:
> > Dong,
> >
> > On 12/14/2011 10:03 AM, Dong Aisheng wrote:
> >> From: Dong Aisheng <dong.aisheng@linaro.org>
> >>
> >> Currently most code to get child number in kernel are almost same,
> >> add a helper to implement this function for dt to use.
> >>
> >> Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
> >> Cc: Grant Likely <grant.likely@secretlab.ca>
> >> Cc: Rob Herring <rob.herring@calxeda.com>
> >> ---
> >> ?include/linux/of.h | ? 17 +++++++++++++++++
> >> ?1 files changed, 17 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/include/linux/of.h b/include/linux/of.h index
> >> 4948552..09d53dc 100644
> >> --- a/include/linux/of.h
> >> +++ b/include/linux/of.h
> >> @@ -189,6 +189,18 @@ extern struct device_node
> >> *of_get_next_child(const struct device_node *node,
> >> ? ? ? for (child = of_get_next_child(parent, NULL); child != NULL; \
> >> ? ? ? ? ? ?child = of_get_next_child(parent, child))
> >>
> >> +static inline int of_get_child_number(struct device_node *np)
> >
> > I would call this of_get_child_count instead.
> 
> I agree.
> 
Yes, will change to that name.
Thanks for the suggestion.

Regards
Dong Aisheng

[RFC PATCH v2 2/4] pinctrl: imx: add pinmux imx driver

[Dong Aisheng]

The driver contains the initial support for imx53 and
imx6q.

Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Shawn Guo <shanw.guo@freescale.com>
---
 drivers/pinctrl/Kconfig           |   20 ++
 drivers/pinctrl/Makefile          |    3 +
 drivers/pinctrl/pinmux-imx-core.c |  435 ++++++++++++++++++++++++++++++++++++
 drivers/pinctrl/pinmux-imx-core.h |   86 +++++++
 drivers/pinctrl/pinmux-imx53.c    |  443 +++++++++++++++++++++++++++++++++++++
 drivers/pinctrl/pinmux-imx6q.c    |  433 ++++++++++++++++++++++++++++++++++++
 6 files changed, 1420 insertions(+), 0 deletions(-)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index e17e2f8..268c212 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -20,6 +20,26 @@ config DEBUG_PINCTRL
 	help
 	  Say Y here to add some extra checks and diagnostics to PINCTRL calls.
 
+config PINMUX_IMX
+	bool "Freescale IMX core pinmux driver"
+	depends on ARCH_MXC
+
+config PINMUX_IMX53
+	bool "IMX53 pinmux driver"
+	depends on ARCH_MX5
+	select PINMUX
+	select PINMUX_IMX
+	help
+	  Say Y here to enable the imx6q pinmux driver
+
+config PINMUX_IMX6Q
+	bool "IMX6Q pinmux driver"
+	depends on SOC_IMX6Q
+	select PINMUX
+	select PINMUX_IMX
+	help
+	  Say Y here to enable the imx6q pinmux driver
+
 config PINMUX_SIRF
 	bool "CSR SiRFprimaII pinmux driver"
 	depends on ARCH_PRIMA2
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index bdc548a..a9bca9f 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -4,5 +4,8 @@ ccflags-$(CONFIG_DEBUG_PINMUX)	+= -DDEBUG
 
 obj-$(CONFIG_PINCTRL)		+= core.o
 obj-$(CONFIG_PINMUX)		+= pinmux.o
+obj-$(CONFIG_PINMUX_IMX)	+= pinmux-imx-core.o
+obj-$(CONFIG_PINMUX_IMX53)	+= pinmux-imx53.o
+obj-$(CONFIG_PINMUX_IMX6Q)	+= pinmux-imx6q.o
 obj-$(CONFIG_PINMUX_SIRF)	+= pinmux-sirf.o
 obj-$(CONFIG_PINMUX_U300)	+= pinmux-u300.o
diff --git a/drivers/pinctrl/pinmux-imx-core.c b/drivers/pinctrl/pinmux-imx-core.c
new file mode 100644
index 0000000..4647d3b
--- /dev/null
+++ b/drivers/pinctrl/pinmux-imx-core.c
@@ -0,0 +1,435 @@
+/*
+ * Core driver for the imx pin controller
+ *
+ * Copyright (C) 2011 Freescale Semiconductor, Inc.
+ * Copyright (C) 2011 Linaro Ltd.
+ *
+ * Author: Dong Aisheng <dong.aisheng@linaro.org>
+ *
+ * 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.
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/io.h>
+#include <linux/err.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/slab.h>
+
+#include "pinmux-imx-core.h"
+
+#define DRIVER_NAME "pinmux-imx"
+
+/**
+ * @dev: a pointer back to containing device
+ * @virtbase: the offset to the controller in virtual memory
+ */
+struct imx_pmx {
+	struct device *dev;
+	struct pinctrl_dev *pctl;
+	void __iomem *virtbase;
+	struct imx_pinctrl_info *info;
+};
+
+#define IMX_PINCTRL_REG_SIZE 4
+#define IMX_PINCTRL_MAX_FUNC 7
+
+static int imx_list_groups(struct pinctrl_dev *pctldev, unsigned selector)
+{
+	struct imx_pmx *ipmx = pinctrl_dev_get_drvdata(pctldev);
+	struct imx_pinctrl_info *info = ipmx->info;
+
+	if (selector >= info->ngroups)
+		return -EINVAL;
+
+	return 0;
+}
+
+static const char *imx_get_group_name(struct pinctrl_dev *pctldev,
+				       unsigned selector)
+{
+	struct imx_pmx *ipmx = pinctrl_dev_get_drvdata(pctldev);
+	struct imx_pinctrl_info *info = ipmx->info;
+
+	if (selector >= info->ngroups)
+		return NULL;
+
+	return info->groups[selector].name;
+}
+
+static int imx_get_group_pins(struct pinctrl_dev *pctldev, unsigned selector,
+			       const unsigned **pins,
+			       unsigned *num_pins)
+{
+	struct imx_pmx *ipmx = pinctrl_dev_get_drvdata(pctldev);
+	struct imx_pinctrl_info *info = ipmx->info;
+
+	if (selector >= info->ngroups)
+		return -EINVAL;
+
+	*pins = info->groups[selector].pins;
+	*num_pins = info->groups[selector].num_pins;
+
+	return 0;
+}
+
+static void imx_pin_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
+		   unsigned offset)
+{
+	seq_printf(s, " " DRIVER_NAME);
+}
+
+static struct pinctrl_ops imx_pctrl_ops = {
+	.list_groups = imx_list_groups,
+	.get_group_name = imx_get_group_name,
+	.get_group_pins = imx_get_group_pins,
+	.pin_dbg_show = imx_pin_dbg_show,
+};
+
+static int imx_pmx_enable(struct pinctrl_dev *pctldev, unsigned selector,
+			   unsigned group)
+{
+	struct imx_pmx *ipmx = pinctrl_dev_get_drvdata(pctldev);
+	struct imx_pinctrl_info *info = ipmx->info;
+	const unsigned *pins, *mux;
+	unsigned int num_pins, num_mux;
+	u32 regval, offset;
+	int i;
+
+	/*
+	 * Configure the mux mode for each pin in the group for a specific
+	 * function.
+	 */
+	pins = info->groups[group].pins;
+	num_pins = info->groups[group].num_pins;
+	mux = info->groups[group].mux_mode;
+	num_mux = info->groups[group].num_mux;
+
+	dev_dbg(ipmx->dev, "function %s group %s\n",
+		info->functions[selector].name, info->groups[group].name);
+
+	if (num_pins != num_mux) {
+		dev_err(ipmx->dev, "num_mux is not equal to num_pins\n");
+		return -EINVAL;
+	}
+
+	for (i = 0; i < num_pins; i++) {
+		if (mux[i] > IMX_PINCTRL_MAX_FUNC)
+			dev_err(ipmx->dev, "exceeds the maximum mux mode(0x7)\n");
+		offset = info->mux_offset + pins[i] * IMX_PINCTRL_REG_SIZE;
+		regval = readl(ipmx->virtbase + offset);
+		regval &= ~IMX_PINCTRL_MAX_FUNC;
+		writel(mux[i] | regval, ipmx->virtbase + offset);
+		dev_dbg(ipmx->dev, "write: offset 0x%x val 0x%x\n",
+			offset, regval | mux[i]);
+	}
+
+	return 0;
+}
+
+static void imx_pmx_disable(struct pinctrl_dev *pctldev, unsigned func_selector,
+			    unsigned group_selector)
+{
+	/* nothing to do here */
+}
+
+static int imx_pmx_list_funcs(struct pinctrl_dev *pctldev, unsigned selector)
+{
+	struct imx_pmx *ipmx = pinctrl_dev_get_drvdata(pctldev);
+	struct imx_pinctrl_info *info = ipmx->info;
+
+	if (selector >= info->nfunctions)
+		return -EINVAL;
+
+	return 0;
+}
+
+static const char *imx_pmx_get_func_name(struct pinctrl_dev *pctldev,
+					  unsigned selector)
+{
+	struct imx_pmx *ipmx = pinctrl_dev_get_drvdata(pctldev);
+	struct imx_pinctrl_info *info = ipmx->info;
+
+	return info->functions[selector].name;
+}
+
+static int imx_pmx_get_groups(struct pinctrl_dev *pctldev, unsigned selector,
+			       const char * const **groups,
+			       unsigned * const num_groups)
+{
+	struct imx_pmx *ipmx = pinctrl_dev_get_drvdata(pctldev);
+	struct imx_pinctrl_info *info = ipmx->info;
+
+	*groups = info->functions[selector].groups;
+	*num_groups = info->functions[selector].num_groups;
+
+	return 0;
+}
+
+static struct pinmux_ops imx_pmx_ops = {
+	.list_functions = imx_pmx_list_funcs,
+	.get_function_name = imx_pmx_get_func_name,
+	.get_function_groups = imx_pmx_get_groups,
+	.enable = imx_pmx_enable,
+	.disable = imx_pmx_disable,
+};
+
+static struct pinctrl_desc imx_pmx_desc = {
+	.name = DRIVER_NAME,
+	.pctlops = &imx_pctrl_ops,
+	.pmxops = &imx_pmx_ops,
+	.owner = THIS_MODULE,
+};
+
+#ifdef CONFIG_OF
+static int __devinit imx_pmx_parse_functions(struct device_node *np,
+			struct imx_pinctrl_info *info, u32 num)
+{
+	struct imx_pmx_func *function;
+	struct imx_pin_group *group;
+	int ret, len;
+
+	dev_dbg(info->dev, "parse function %d\n", num);
+
+	group = &info->groups[num];
+	function = &info->functions[num];
+
+	/* Initialise group */
+	ret = of_property_read_string(np, "grp_name", &group->name);
+	if (ret) {
+		dev_err(info->dev, "failed to get grp_name\n");
+		return ret;
+	}
+
+	ret = of_property_read_u32(np, "num_pins", &group->num_pins);
+	if (ret) {
+		dev_err(info->dev, "failed to get num_pins\n");
+		return ret;
+	}
+
+	ret = of_property_read_u32(np, "num_mux", &group->num_mux);
+	if (ret) {
+		dev_err(info->dev, "failed to get num_mux\n");
+		return ret;
+	}
+
+	if (group->num_pins != group->num_mux)
+		return -EINVAL;
+
+	group->pins = devm_kzalloc(info->dev, group->num_pins * sizeof(unsigned int),
+				GFP_KERNEL);
+	group->mux_mode = devm_kzalloc(info->dev, group->num_mux * sizeof(unsigned int),
+				GFP_KERNEL);
+	if (!group->pins || !group->mux_mode)
+		return -ENOMEM;
+
+	/* sanity check */
+	if (of_get_property(np, "grp_pins", &len) &&
+		len != group->num_pins * sizeof(unsigned int)) {
+		dev_err(info->dev, "wrong pins number?\n");
+		return -EINVAL;
+	}
+
+	if (of_get_property(np, "grp_mux", &len) &&
+		len != group->num_mux * sizeof(unsigned int)) {
+		dev_err(info->dev, "wrong pin mux number?\n");
+		return -EINVAL;
+	}
+
+	ret = of_property_read_u32_array(np, "grp_pins",
+			group->pins, group->num_pins);
+	if (ret) {
+		dev_err(info->dev, "failed to get grp_pins\n");
+		return ret;
+	}
+
+	ret = of_property_read_u32_array(np, "grp_mux",
+			group->mux_mode, group->num_mux);
+	if (ret) {
+		dev_err(info->dev, "failed to get grp_mux\n");
+		return ret;
+	}
+
+	/* Initialise function */
+	ret = of_property_read_string(np, "func_name", &function->name);
+	if (ret) {
+		dev_err(info->dev, "failed to get func_name\n");
+		return ret;
+	}
+
+	function->groups = devm_kzalloc(info->dev, sizeof(char **), GFP_KERNEL);
+	function->num_groups = 1;
+	function->groups[0] = group->name;
+
+	dev_dbg(info->dev, "func_name %s grp_name %s num_groups %d\n",
+				function->name, function->groups[0],
+				function->num_groups);
+
+	return 0;
+}
+
+static int __devinit imx_pmx_probe_dt(struct platform_device *pdev,
+				struct imx_pinctrl_info *info)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *child = NULL;
+	int ret, i;
+	u32 nfuncs = 0;
+
+	if (!np)
+		return -ENODEV;
+
+	nfuncs = of_get_child_number(np);
+	if (nfuncs <= 0) {
+		dev_err(&pdev->dev, "no functions defined\n");
+		return -EINVAL;
+	}
+
+	info->nfunctions = nfuncs;
+	info->functions = devm_kzalloc(&pdev->dev, nfuncs * sizeof(struct imx_pmx_func),
+					GFP_KERNEL);
+	if (!info->functions)
+		return -ENOMEM;
+
+	/* DT file only passes one group per one function */
+	info->ngroups = nfuncs;
+	info->groups = devm_kzalloc(&pdev->dev, nfuncs * sizeof(struct imx_pin_group),
+					GFP_KERNEL);
+	if (!info->groups)
+		return -ENOMEM;
+
+	child = NULL;
+	i = 0;
+	for_each_child_of_node(np, child) {
+		ret = imx_pmx_parse_functions(child, info, i++);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to parse function\n");
+			return ret;
+		}
+	}
+
+	return 0;
+}
+#else
+static int __devinit imx_pmx_probe_dt(struct platform_device *pdev,
+				struct imx_pinctrl_info *info)
+{
+	return -ENODEV;
+}
+#endif
+
+static inline void imx_pmx_desc_init(struct pinctrl_desc *pmx_desc,
+				const struct imx_pinctrl_info *info)
+{
+	pmx_desc->pins = info->pins;
+	pmx_desc->npins = info->npins;
+	pmx_desc->maxpin = info->maxpin;
+}
+
+static const struct of_device_id imx_pmx_dt_ids[] = {
+#ifdef CONFIG_PINMUX_IMX6Q
+	{ .compatible = "fsl,imx6q-iomuxc", .data = (void *) &imx6q_pinctrl_info, },
+#endif
+#ifdef CONFIG_PINMUX_IMX53
+	{ .compatible = "fsl,imx53-iomuxc", .data = (void *) &imx53_pinctrl_info, },
+#endif
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, imx_pmx_dt_ids);
+
+static int __init imx_pmx_probe(struct platform_device *pdev)
+{
+	const struct of_device_id *of_id =
+			of_match_device(imx_pmx_dt_ids, &pdev->dev);
+	struct device *dev = &pdev->dev;
+	struct imx_pmx *ipmx;
+	struct resource *res;
+	struct imx_pinctrl_info *info;
+	resource_size_t res_size;
+	int ret;
+
+	info = of_id->data;
+	if (!info || !info->pins || !(info->maxpin > info->npins)) {
+		dev_err(&pdev->dev, "wrong pinctrl info\n");
+		return -EINVAL;
+	}
+	info->dev = &pdev->dev;
+
+	/* Create state holders etc for this driver */
+	ipmx = devm_kzalloc(&pdev->dev, sizeof(*ipmx), GFP_KERNEL);
+	if (!ipmx)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENOENT;
+
+	res_size = resource_size(res);
+	if (!devm_request_mem_region(dev, res->start, res_size, res->name))
+		return -EBUSY;
+
+	ipmx->virtbase = devm_ioremap_nocache(dev, res->start, res_size);
+	if (!ipmx->virtbase)
+		return -EBUSY;
+
+	imx_pmx_desc_init(&imx_pmx_desc, info);
+	ret = imx_pmx_probe_dt(pdev, info);
+	if (ret) {
+		dev_err(&pdev->dev, "fail to probe dt properties\n");
+		return ret;
+	}
+
+	ipmx->pctl = pinctrl_register(&imx_pmx_desc, &pdev->dev, ipmx);
+	if (!ipmx->pctl) {
+		dev_err(&pdev->dev, "could not register IMX pinmux driver\n");
+		return -EINVAL;
+	}
+
+	ipmx->info = info;
+	ipmx->dev = info->dev;
+	platform_set_drvdata(pdev, ipmx);
+
+	dev_info(&pdev->dev, "initialized IMX pinmux driver\n");
+
+	return 0;
+}
+
+static int __exit imx_pmx_remove(struct platform_device *pdev)
+{
+	struct imx_pmx *ipmx = platform_get_drvdata(pdev);
+
+	pinctrl_unregister(ipmx->pctl);
+	platform_set_drvdata(pdev, NULL);
+
+	return 0;
+}
+
+static struct platform_driver imx_pmx_driver = {
+	.driver = {
+		.name = DRIVER_NAME,
+		.owner = THIS_MODULE,
+		.of_match_table = imx_pmx_dt_ids,
+	},
+	.remove = __exit_p(imx_pmx_remove),
+};
+
+static int __init imx_pmx_init(void)
+{
+	return platform_driver_probe(&imx_pmx_driver, imx_pmx_probe);
+}
+arch_initcall(imx_pmx_init);
+
+static void __exit imx_pmx_exit(void)
+{
+	platform_driver_unregister(&imx_pmx_driver);
+}
+module_exit(imx_pmx_exit);
+
+MODULE_AUTHOR("Dong Aisheng <dong.aisheng@linaro.org>");
+MODULE_DESCRIPTION("IMX Pin Control Driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/pinctrl/pinmux-imx-core.h b/drivers/pinctrl/pinmux-imx-core.h
new file mode 100644
index 0000000..df69cd0
--- /dev/null
+++ b/drivers/pinctrl/pinmux-imx-core.h
@@ -0,0 +1,86 @@
+/*
+ * IMX pinmux core definitions
+ *
+ * Copyright (C) 2011 Freescale Semiconductor, Inc.
+ * Copyright (C) 2011 Linaro Ltd.
+ *
+ * Author: Dong Aisheng <dong.aisheng@linaro.org>
+ *
+ * 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.
+ */
+
+#ifndef __DRIVERS_PINCTRL_PINMUX_IMX_H
+#define __DRIVERS_PINCTRL_PINMUX_IMX_H
+
+/* Supported Pinctrl type */
+enum imx_pinctrl_type {
+	IMX53_PINCTRL,
+	IMX6Q_PINCTRL,
+};
+
+/**
+ * struct imx_pin_group - describes an IMX pin group
+ * @name: the name of this specific pin group
+ * @pins: an array of discrete physical pins used in this group, taken
+ *	from the driver-local pin enumeration space
+ * @num_pins: the number of pins in this group array, i.e. the number of
+ *	elements in .pins so we can iterate over that array
+ * @mux_mode: the mux mode for each pins in this group. The size of this
+ *	array is the same as pins.
+ */
+struct imx_pin_group {
+	const char *name;
+	unsigned int *pins;
+	unsigned num_pins;
+	unsigned int *mux_mode;
+	unsigned num_mux;
+};
+
+/**
+ * struct imx_pmx_func - describes IMX pinmux functions
+ * @name: the name of this specific function
+ * @groups: corresponding pin groups
+ */
+struct imx_pmx_func {
+	const char *name;
+	const char **groups;
+	unsigned num_groups;
+};
+
+struct imx_pinctrl_info {
+	struct device *dev;
+	u32 type;
+	const struct pinctrl_pin_desc *pins;
+	unsigned int npins;
+	unsigned int maxpin;
+	struct imx_pin_group *groups;
+	unsigned int ngroups;
+	struct imx_pmx_func *functions;
+	unsigned int nfunctions;
+	u32 mux_offset;
+};
+
+#define IMX_PINCTRL_PIN(pin) PINCTRL_PIN(pin, #pin)
+
+#define IMX_PIN_GROUP(n, p, m)  \
+	{			\
+		.name = n,	\
+		.pins = p,	\
+		.num_pins = ARRAY_SIZE(p),	\
+		.mux_mode = m,	\
+		.num_mux = ARRAY_SIZE(m),	\
+	}
+
+#define IMX_PMX_FUNC(n, g)  \
+	{			\
+		.name = n,	\
+		.groups = g,	\
+		.num_groups = ARRAY_SIZE(g),	\
+	}
+
+extern struct imx_pinctrl_info imx53_pinctrl_info;
+extern struct imx_pinctrl_info imx6q_pinctrl_info;
+#endif /* __DRIVERS_PINCTRL_PINMUX_IMX_H */
diff --git a/drivers/pinctrl/pinmux-imx53.c b/drivers/pinctrl/pinmux-imx53.c
new file mode 100644
index 0000000..0b2faa7
--- /dev/null
+++ b/drivers/pinctrl/pinmux-imx53.c
@@ -0,0 +1,443 @@
+/*
+ * imx53 pinmux driver based on imx pinmux core
+ *
+ * Copyright (C) 2011 Freescale Semiconductor, Inc.
+ * Copyright (C) 2011 Linaro, Inc.
+ *
+ * Author: Dong Aisheng <dong.aisheng@linaro.org>
+ *
+ * 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.
+ */
+
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/err.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+
+#include "pinmux-imx-core.h"
+
+#define IMX53_IOMUXC_MUX_OFFSET 0x20
+#define IMX53_IOMUXC_MAXPIN	(23*23)
+
+enum imx_imx53_pinctrl_pads {
+	MX53_GPIO_19 = 0,
+	MX53_KEY_COL0 = 1,
+	MX53_KEY_ROW0 = 2,
+	MX53_KEY_COL1 = 3,
+	MX53_KEY_ROW1 = 4,
+	MX53_KEY_COL2 = 5,
+	MX53_KEY_ROW2 = 6,
+	MX53_KEY_COL3 = 7,
+	MX53_KEY_ROW3 = 8,
+	MX53_KEY_COL4 = 9,
+	MX53_KEY_ROW4 = 10,
+	MX53_DI0_DISP_CLK = 11,
+	MX53_DI0_PIN15 = 12,
+	MX53_DI0_PIN2 = 13,
+	MX53_DI0_PIN3 = 14,
+	MX53_DI0_PIN4 = 15,
+	MX53_DISP0_DAT0 = 16,
+	MX53_DISP0_DAT1 = 17,
+	MX53_DISP0_DAT2 = 18,
+	MX53_DISP0_DAT3 = 19,
+	MX53_DISP0_DAT4 = 20,
+	MX53_DISP0_DAT5 = 21,
+	MX53_DISP0_DAT6 = 22,
+	MX53_DISP0_DAT7 = 23,
+	MX53_DISP0_DAT8 = 24,
+	MX53_DISP0_DAT9 = 25,
+	MX53_DISP0_DAT10 = 26,
+	MX53_DISP0_DAT11 = 27,
+	MX53_DISP0_DAT12 = 28,
+	MX53_DISP0_DAT13 = 29,
+	MX53_DISP0_DAT14 = 30,
+	MX53_DISP0_DAT15 = 31,
+	MX53_DISP0_DAT16 = 32,
+	MX53_DISP0_DAT17 = 33,
+	MX53_DISP0_DAT18 = 34,
+	MX53_DISP0_DAT19 = 35,
+	MX53_DISP0_DAT20 = 36,
+	MX53_DISP0_DAT21 = 37,
+	MX53_DISP0_DAT22 = 38,
+	MX53_DISP0_DAT23 = 39,
+	MX53_CSI0_PIXCLK = 40,
+	MX53_CSI0_MCLK = 41,
+	MX53_CSI0_DATA_EN = 42,
+	MX53_CSI0_VSYNC = 43,
+	MX53_CSI0_DAT4 = 44,
+	MX53_CSI0_DAT5 = 45,
+	MX53_CSI0_DAT6 = 46,
+	MX53_CSI0_DAT7 = 47,
+	MX53_CSI0_DAT8 = 48,
+	MX53_CSI0_DAT9 = 49,
+	MX53_CSI0_DAT10 = 50,
+	MX53_CSI0_DAT11 = 51,
+	MX53_CSI0_DAT12 = 52,
+	MX53_CSI0_DAT13 = 53,
+	MX53_CSI0_DAT14 = 54,
+	MX53_CSI0_DAT15 = 55,
+	MX53_CSI0_DAT16 = 56,
+	MX53_CSI0_DAT17 = 57,
+	MX53_CSI0_DAT18 = 58,
+	MX53_CSI0_DAT19 = 59,
+	MX53_EIM_A25 = 60,
+	MX53_EIM_EB2 = 61,
+	MX53_EIM_D16 = 62,
+	MX53_EIM_D17 = 63,
+	MX53_EIM_D18 = 64,
+	MX53_EIM_D19 = 65,
+	MX53_EIM_D20 = 66,
+	MX53_EIM_D21 = 67,
+	MX53_EIM_D22 = 68,
+	MX53_EIM_D23 = 69,
+	MX53_EIM_EB3 = 70,
+	MX53_EIM_D24 = 71,
+	MX53_EIM_D25 = 72,
+	MX53_EIM_D26 = 73,
+	MX53_EIM_D27 = 74,
+	MX53_EIM_D28 = 75,
+	MX53_EIM_D29 = 76,
+	MX53_EIM_D30 = 77,
+	MX53_EIM_D31 = 78,
+	MX53_EIM_A24 = 79,
+	MX53_EIM_A23 = 80,
+	MX53_EIM_A22 = 81,
+	MX53_EIM_A21 = 82,
+	MX53_EIM_A20 = 83,
+	MX53_EIM_A19 = 84,
+	MX53_EIM_A18 = 85,
+	MX53_EIM_A17 = 86,
+	MX53_EIM_A16 = 87,
+	MX53_EIM_CS0 = 88,
+	MX53_EIM_CS1 = 89,
+	MX53_EIM_OE = 90,
+	MX53_EIM_RW = 91,
+	MX53_EIM_LBA = 92,
+	MX53_EIM_EB0 = 93,
+	MX53_EIM_EB1 = 94,
+	MX53_EIM_DA0 = 95,
+	MX53_EIM_DA1 = 96,
+	MX53_EIM_DA2 = 97,
+	MX53_EIM_DA3 = 98,
+	MX53_EIM_DA4 = 99,
+	MX53_EIM_DA5 = 100,
+	MX53_EIM_DA6 = 101,
+	MX53_EIM_DA7 = 102,
+	MX53_EIM_DA8 = 103,
+	MX53_EIM_DA9 = 104,
+	MX53_EIM_DA10 = 105,
+	MX53_EIM_DA11 = 106,
+	MX53_EIM_DA12 = 107,
+	MX53_EIM_DA13 = 108,
+	MX53_EIM_DA14 = 109,
+	MX53_EIM_DA15 = 110,
+	MX53_NANDF_WE_B = 111,
+	MX53_NANDF_RE_B = 112,
+	MX53_EIM_WAIT = 113,
+	MX53_EIM_BCLK = 114,
+	MX53_LVDS1_TX3_P = 115,
+	MX53_LVDS1_TX2_P = 116,
+	MX53_LVDS1_CLK_P = 117,
+	MX53_LVDS1_TX1_P = 118,
+	MX53_LVDS1_TX0_P = 119,
+	MX53_LVDS0_TX3_P = 120,
+	MX53_LVDS0_CLK_P = 121,
+	MX53_LVDS0_TX2_P = 122,
+	MX53_LVDS0_TX1_P = 123,
+	MX53_LVDS0_TX0_P = 124,
+	MX53_GPIO_10 = 125,
+	MX53_GPIO_11 = 126,
+	MX53_GPIO_12 = 127,
+	MX53_GPIO_13 = 128,
+	MX53_GPIO_14 = 129,
+	MX53_NANDF_CLE = 130,
+	MX53_NANDF_ALE = 131,
+	MX53_NANDF_WP_B = 132,
+	MX53_NANDF_RB0 = 133,
+	MX53_NANDF_CS0 = 134,
+	MX53_NANDF_CS1 = 135,
+	MX53_NANDF_CS2 = 136,
+	MX53_NANDF_CS3 = 137,
+	MX53_FEC_MDIO = 138,
+	MX53_FEC_REF_CLK = 139,
+	MX53_FEC_RX_ER = 140,
+	MX53_FEC_CRS_DV = 141,
+	MX53_FEC_RXD1 = 142,
+	MX53_FEC_RXD0 = 143,
+	MX53_FEC_TX_EN = 144,
+	MX53_FEC_TXD1 = 145,
+	MX53_FEC_TXD0 = 146,
+	MX53_FEC_MDC = 147,
+	MX53_PATA_DIOW = 148,
+	MX53_PATA_DMACK = 149,
+	MX53_PATA_DMARQ = 150,
+	MX53_PATA_BUFFER_EN = 151,
+	MX53_PATA_INTRQ = 152,
+	MX53_PATA_DIOR = 153,
+	MX53_PATA_RESET_B = 154,
+	MX53_PATA_IORDY = 155,
+	MX53_PATA_DA_0 = 156,
+	MX53_PATA_DA_1 = 157,
+	MX53_PATA_DA_2 = 158,
+	MX53_PATA_CS_0 = 159,
+	MX53_PATA_CS_1 = 160,
+	MX53_PATA_DATA0 = 161,
+	MX53_PATA_DATA1 = 162,
+	MX53_PATA_DATA2 = 163,
+	MX53_PATA_DATA3 = 164,
+	MX53_PATA_DATA4 = 165,
+	MX53_PATA_DATA5 = 166,
+	MX53_PATA_DATA6 = 167,
+	MX53_PATA_DATA7 = 168,
+	MX53_PATA_DATA8 = 169,
+	MX53_PATA_DATA9 = 170,
+	MX53_PATA_DATA10 = 171,
+	MX53_PATA_DATA11 = 172,
+	MX53_PATA_DATA12 = 173,
+	MX53_PATA_DATA13 = 174,
+	MX53_PATA_DATA14 = 175,
+	MX53_PATA_DATA15 = 176,
+	MX53_SD1_DATA0 = 177,
+	MX53_SD1_DATA1 = 178,
+	MX53_SD1_CMD = 179,
+	MX53_SD1_DATA2 = 180,
+	MX53_SD1_CLK = 181,
+	MX53_SD1_DATA3 = 182,
+	MX53_SD2_CLK = 183,
+	MX53_SD2_CMD = 184,
+	MX53_SD2_DATA3 = 185,
+	MX53_SD2_DATA2 = 186,
+	MX53_SD2_DATA1 = 187,
+	MX53_SD2_DATA0 = 188,
+	MX53_GPIO_0 = 189,
+	MX53_GPIO_1 = 190,
+	MX53_GPIO_9 = 191,
+	MX53_GPIO_3 = 192,
+	MX53_GPIO_6 = 193,
+	MX53_GPIO_2 = 194,
+	MX53_GPIO_4 = 195,
+	MX53_GPIO_5 = 196,
+	MX53_GPIO_7 = 197,
+	MX53_GPIO_8 = 198,
+	MX53_GPIO_16 = 199,
+	MX53_GPIO_17 = 200,
+	MX53_GPIO_18 = 201,
+};
+
+/* Pad names for the pinmux subsystem */
+static const struct pinctrl_pin_desc imx53_pinctrl_pads[] = {
+	IMX_PINCTRL_PIN(MX53_GPIO_19),
+	IMX_PINCTRL_PIN(MX53_KEY_COL0),
+	IMX_PINCTRL_PIN(MX53_KEY_ROW0),
+	IMX_PINCTRL_PIN(MX53_KEY_COL1),
+	IMX_PINCTRL_PIN(MX53_KEY_ROW1),
+	IMX_PINCTRL_PIN(MX53_KEY_COL2),
+	IMX_PINCTRL_PIN(MX53_KEY_ROW2),
+	IMX_PINCTRL_PIN(MX53_KEY_COL3),
+	IMX_PINCTRL_PIN(MX53_KEY_ROW3),
+	IMX_PINCTRL_PIN(MX53_KEY_COL4),
+	IMX_PINCTRL_PIN(MX53_KEY_ROW4),
+	IMX_PINCTRL_PIN(MX53_DI0_DISP_CLK),
+	IMX_PINCTRL_PIN(MX53_DI0_PIN15),
+	IMX_PINCTRL_PIN(MX53_DI0_PIN2),
+	IMX_PINCTRL_PIN(MX53_DI0_PIN3),
+	IMX_PINCTRL_PIN(MX53_DI0_PIN4),
+	IMX_PINCTRL_PIN(MX53_DISP0_DAT0),
+	IMX_PINCTRL_PIN(MX53_DISP0_DAT1),
+	IMX_PINCTRL_PIN(MX53_DISP0_DAT2),
+	IMX_PINCTRL_PIN(MX53_DISP0_DAT3),
+	IMX_PINCTRL_PIN(MX53_DISP0_DAT4),
+	IMX_PINCTRL_PIN(MX53_DISP0_DAT5),
+	IMX_PINCTRL_PIN(MX53_DISP0_DAT6),
+	IMX_PINCTRL_PIN(MX53_DISP0_DAT7),
+	IMX_PINCTRL_PIN(MX53_DISP0_DAT8),
+	IMX_PINCTRL_PIN(MX53_DISP0_DAT9),
+	IMX_PINCTRL_PIN(MX53_DISP0_DAT10),
+	IMX_PINCTRL_PIN(MX53_DISP0_DAT11),
+	IMX_PINCTRL_PIN(MX53_DISP0_DAT12),
+	IMX_PINCTRL_PIN(MX53_DISP0_DAT13),
+	IMX_PINCTRL_PIN(MX53_DISP0_DAT14),
+	IMX_PINCTRL_PIN(MX53_DISP0_DAT15),
+	IMX_PINCTRL_PIN(MX53_DISP0_DAT16),
+	IMX_PINCTRL_PIN(MX53_DISP0_DAT17),
+	IMX_PINCTRL_PIN(MX53_DISP0_DAT18),
+	IMX_PINCTRL_PIN(MX53_DISP0_DAT19),
+	IMX_PINCTRL_PIN(MX53_DISP0_DAT20),
+	IMX_PINCTRL_PIN(MX53_DISP0_DAT21),
+	IMX_PINCTRL_PIN(MX53_DISP0_DAT22),
+	IMX_PINCTRL_PIN(MX53_DISP0_DAT23),
+	IMX_PINCTRL_PIN(MX53_CSI0_PIXCLK),
+	IMX_PINCTRL_PIN(MX53_CSI0_MCLK),
+	IMX_PINCTRL_PIN(MX53_CSI0_DATA_EN),
+	IMX_PINCTRL_PIN(MX53_CSI0_VSYNC),
+	IMX_PINCTRL_PIN(MX53_CSI0_DAT4),
+	IMX_PINCTRL_PIN(MX53_CSI0_DAT5),
+	IMX_PINCTRL_PIN(MX53_CSI0_DAT6),
+	IMX_PINCTRL_PIN(MX53_CSI0_DAT7),
+	IMX_PINCTRL_PIN(MX53_CSI0_DAT8),
+	IMX_PINCTRL_PIN(MX53_CSI0_DAT9),
+	IMX_PINCTRL_PIN(MX53_CSI0_DAT10),
+	IMX_PINCTRL_PIN(MX53_CSI0_DAT11),
+	IMX_PINCTRL_PIN(MX53_CSI0_DAT12),
+	IMX_PINCTRL_PIN(MX53_CSI0_DAT13),
+	IMX_PINCTRL_PIN(MX53_CSI0_DAT14),
+	IMX_PINCTRL_PIN(MX53_CSI0_DAT15),
+	IMX_PINCTRL_PIN(MX53_CSI0_DAT16),
+	IMX_PINCTRL_PIN(MX53_CSI0_DAT17),
+	IMX_PINCTRL_PIN(MX53_CSI0_DAT18),
+	IMX_PINCTRL_PIN(MX53_CSI0_DAT19),
+	IMX_PINCTRL_PIN(MX53_EIM_A25),
+	IMX_PINCTRL_PIN(MX53_EIM_EB2),
+	IMX_PINCTRL_PIN(MX53_EIM_D16),
+	IMX_PINCTRL_PIN(MX53_EIM_D17),
+	IMX_PINCTRL_PIN(MX53_EIM_D18),
+	IMX_PINCTRL_PIN(MX53_EIM_D19),
+	IMX_PINCTRL_PIN(MX53_EIM_D20),
+	IMX_PINCTRL_PIN(MX53_EIM_D21),
+	IMX_PINCTRL_PIN(MX53_EIM_D22),
+	IMX_PINCTRL_PIN(MX53_EIM_D23),
+	IMX_PINCTRL_PIN(MX53_EIM_EB3),
+	IMX_PINCTRL_PIN(MX53_EIM_D24),
+	IMX_PINCTRL_PIN(MX53_EIM_D25),
+	IMX_PINCTRL_PIN(MX53_EIM_D26),
+	IMX_PINCTRL_PIN(MX53_EIM_D27),
+	IMX_PINCTRL_PIN(MX53_EIM_D28),
+	IMX_PINCTRL_PIN(MX53_EIM_D29),
+	IMX_PINCTRL_PIN(MX53_EIM_D30),
+	IMX_PINCTRL_PIN(MX53_EIM_D31),
+	IMX_PINCTRL_PIN(MX53_EIM_A24),
+	IMX_PINCTRL_PIN(MX53_EIM_A23),
+	IMX_PINCTRL_PIN(MX53_EIM_A22),
+	IMX_PINCTRL_PIN(MX53_EIM_A21),
+	IMX_PINCTRL_PIN(MX53_EIM_A20),
+	IMX_PINCTRL_PIN(MX53_EIM_A19),
+	IMX_PINCTRL_PIN(MX53_EIM_A18),
+	IMX_PINCTRL_PIN(MX53_EIM_A17),
+	IMX_PINCTRL_PIN(MX53_EIM_A16),
+	IMX_PINCTRL_PIN(MX53_EIM_CS0),
+	IMX_PINCTRL_PIN(MX53_EIM_CS1),
+	IMX_PINCTRL_PIN(MX53_EIM_OE),
+	IMX_PINCTRL_PIN(MX53_EIM_RW),
+	IMX_PINCTRL_PIN(MX53_EIM_LBA),
+	IMX_PINCTRL_PIN(MX53_EIM_EB0),
+	IMX_PINCTRL_PIN(MX53_EIM_EB1),
+	IMX_PINCTRL_PIN(MX53_EIM_DA0),
+	IMX_PINCTRL_PIN(MX53_EIM_DA1),
+	IMX_PINCTRL_PIN(MX53_EIM_DA2),
+	IMX_PINCTRL_PIN(MX53_EIM_DA3),
+	IMX_PINCTRL_PIN(MX53_EIM_DA4),
+	IMX_PINCTRL_PIN(MX53_EIM_DA5),
+	IMX_PINCTRL_PIN(MX53_EIM_DA6),
+	IMX_PINCTRL_PIN(MX53_EIM_DA7),
+	IMX_PINCTRL_PIN(MX53_EIM_DA8),
+	IMX_PINCTRL_PIN(MX53_EIM_DA9),
+	IMX_PINCTRL_PIN(MX53_EIM_DA10),
+	IMX_PINCTRL_PIN(MX53_EIM_DA11),
+	IMX_PINCTRL_PIN(MX53_EIM_DA12),
+	IMX_PINCTRL_PIN(MX53_EIM_DA13),
+	IMX_PINCTRL_PIN(MX53_EIM_DA14),
+	IMX_PINCTRL_PIN(MX53_EIM_DA15),
+	IMX_PINCTRL_PIN(MX53_NANDF_WE_B),
+	IMX_PINCTRL_PIN(MX53_NANDF_RE_B),
+	IMX_PINCTRL_PIN(MX53_EIM_WAIT),
+	IMX_PINCTRL_PIN(MX53_EIM_BCLK),
+	IMX_PINCTRL_PIN(MX53_LVDS1_TX3_P),
+	IMX_PINCTRL_PIN(MX53_LVDS1_TX2_P),
+	IMX_PINCTRL_PIN(MX53_LVDS1_CLK_P),
+	IMX_PINCTRL_PIN(MX53_LVDS1_TX1_P),
+	IMX_PINCTRL_PIN(MX53_LVDS1_TX0_P),
+	IMX_PINCTRL_PIN(MX53_LVDS0_TX3_P),
+	IMX_PINCTRL_PIN(MX53_LVDS0_CLK_P),
+	IMX_PINCTRL_PIN(MX53_LVDS0_TX2_P),
+	IMX_PINCTRL_PIN(MX53_LVDS0_TX1_P),
+	IMX_PINCTRL_PIN(MX53_LVDS0_TX0_P),
+	IMX_PINCTRL_PIN(MX53_GPIO_10),
+	IMX_PINCTRL_PIN(MX53_GPIO_11),
+	IMX_PINCTRL_PIN(MX53_GPIO_12),
+	IMX_PINCTRL_PIN(MX53_GPIO_13),
+	IMX_PINCTRL_PIN(MX53_GPIO_14),
+	IMX_PINCTRL_PIN(MX53_NANDF_CLE),
+	IMX_PINCTRL_PIN(MX53_NANDF_ALE),
+	IMX_PINCTRL_PIN(MX53_NANDF_WP_B),
+	IMX_PINCTRL_PIN(MX53_NANDF_RB0),
+	IMX_PINCTRL_PIN(MX53_NANDF_CS0),
+	IMX_PINCTRL_PIN(MX53_NANDF_CS1),
+	IMX_PINCTRL_PIN(MX53_NANDF_CS2),
+	IMX_PINCTRL_PIN(MX53_NANDF_CS3),
+	IMX_PINCTRL_PIN(MX53_FEC_MDIO),
+	IMX_PINCTRL_PIN(MX53_FEC_REF_CLK),
+	IMX_PINCTRL_PIN(MX53_FEC_RX_ER),
+	IMX_PINCTRL_PIN(MX53_FEC_CRS_DV),
+	IMX_PINCTRL_PIN(MX53_FEC_RXD1),
+	IMX_PINCTRL_PIN(MX53_FEC_RXD0),
+	IMX_PINCTRL_PIN(MX53_FEC_TX_EN),
+	IMX_PINCTRL_PIN(MX53_FEC_TXD1),
+	IMX_PINCTRL_PIN(MX53_FEC_TXD0),
+	IMX_PINCTRL_PIN(MX53_FEC_MDC),
+	IMX_PINCTRL_PIN(MX53_PATA_DIOW),
+	IMX_PINCTRL_PIN(MX53_PATA_DMACK),
+	IMX_PINCTRL_PIN(MX53_PATA_DMARQ),
+	IMX_PINCTRL_PIN(MX53_PATA_BUFFER_EN),
+	IMX_PINCTRL_PIN(MX53_PATA_INTRQ),
+	IMX_PINCTRL_PIN(MX53_PATA_DIOR),
+	IMX_PINCTRL_PIN(MX53_PATA_RESET_B),
+	IMX_PINCTRL_PIN(MX53_PATA_IORDY),
+	IMX_PINCTRL_PIN(MX53_PATA_DA_0),
+	IMX_PINCTRL_PIN(MX53_PATA_DA_1),
+	IMX_PINCTRL_PIN(MX53_PATA_DA_2),
+	IMX_PINCTRL_PIN(MX53_PATA_CS_0),
+	IMX_PINCTRL_PIN(MX53_PATA_CS_1),
+	IMX_PINCTRL_PIN(MX53_PATA_DATA0),
+	IMX_PINCTRL_PIN(MX53_PATA_DATA1),
+	IMX_PINCTRL_PIN(MX53_PATA_DATA2),
+	IMX_PINCTRL_PIN(MX53_PATA_DATA3),
+	IMX_PINCTRL_PIN(MX53_PATA_DATA4),
+	IMX_PINCTRL_PIN(MX53_PATA_DATA5),
+	IMX_PINCTRL_PIN(MX53_PATA_DATA6),
+	IMX_PINCTRL_PIN(MX53_PATA_DATA7),
+	IMX_PINCTRL_PIN(MX53_PATA_DATA8),
+	IMX_PINCTRL_PIN(MX53_PATA_DATA9),
+	IMX_PINCTRL_PIN(MX53_PATA_DATA10),
+	IMX_PINCTRL_PIN(MX53_PATA_DATA11),
+	IMX_PINCTRL_PIN(MX53_PATA_DATA12),
+	IMX_PINCTRL_PIN(MX53_PATA_DATA13),
+	IMX_PINCTRL_PIN(MX53_PATA_DATA14),
+	IMX_PINCTRL_PIN(MX53_PATA_DATA15),
+	IMX_PINCTRL_PIN(MX53_SD1_DATA0),
+	IMX_PINCTRL_PIN(MX53_SD1_DATA1),
+	IMX_PINCTRL_PIN(MX53_SD1_CMD),
+	IMX_PINCTRL_PIN(MX53_SD1_DATA2),
+	IMX_PINCTRL_PIN(MX53_SD1_CLK),
+	IMX_PINCTRL_PIN(MX53_SD1_DATA3),
+	IMX_PINCTRL_PIN(MX53_SD2_CLK),
+	IMX_PINCTRL_PIN(MX53_SD2_CMD),
+	IMX_PINCTRL_PIN(MX53_SD2_DATA3),
+	IMX_PINCTRL_PIN(MX53_SD2_DATA2),
+	IMX_PINCTRL_PIN(MX53_SD2_DATA1),
+	IMX_PINCTRL_PIN(MX53_SD2_DATA0),
+	IMX_PINCTRL_PIN(MX53_GPIO_0),
+	IMX_PINCTRL_PIN(MX53_GPIO_1),
+	IMX_PINCTRL_PIN(MX53_GPIO_9),
+	IMX_PINCTRL_PIN(MX53_GPIO_3),
+	IMX_PINCTRL_PIN(MX53_GPIO_6),
+	IMX_PINCTRL_PIN(MX53_GPIO_2),
+	IMX_PINCTRL_PIN(MX53_GPIO_4),
+	IMX_PINCTRL_PIN(MX53_GPIO_5),
+	IMX_PINCTRL_PIN(MX53_GPIO_7),
+	IMX_PINCTRL_PIN(MX53_GPIO_8),
+	IMX_PINCTRL_PIN(MX53_GPIO_16),
+	IMX_PINCTRL_PIN(MX53_GPIO_17),
+	IMX_PINCTRL_PIN(MX53_GPIO_18),
+};
+
+struct imx_pinctrl_info imx53_pinctrl_info = {
+	.type = IMX53_PINCTRL,
+	.pins = imx53_pinctrl_pads,
+	.npins = ARRAY_SIZE(imx53_pinctrl_pads),
+	.maxpin = IMX53_IOMUXC_MAXPIN,
+	.mux_offset = IMX53_IOMUXC_MUX_OFFSET,
+};
diff --git a/drivers/pinctrl/pinmux-imx6q.c b/drivers/pinctrl/pinmux-imx6q.c
new file mode 100644
index 0000000..4749a4f
--- /dev/null
+++ b/drivers/pinctrl/pinmux-imx6q.c
@@ -0,0 +1,433 @@
+/*
+ * imx6q pinmux driver based on imx pinmux core
+ *
+ * Copyright (C) 2011 Freescale Semiconductor, Inc.
+ * Copyright (C) 2011 Linaro, Inc.
+ *
+ * Author: Dong Aisheng <dong.aisheng@linaro.org>
+ *
+ * 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.
+ */
+
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/err.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+
+#include "pinmux-imx-core.h"
+
+#define IMX6Q_IOMUXC_MUX_OFSSET	0x4c
+#define IMX6Q_IOMUXC_MAXPIN	(25*25)
+
+enum imx6q_pads {
+	MX6Q_SD2_DAT1 = 0,
+	MX6Q_SD2_DAT2 = 1,
+	MX6Q_SD2_DAT0 = 2,
+	MX6Q_RGMII_TXC = 3,
+	MX6Q_RGMII_TD0 = 4,
+	MX6Q_RGMII_TD1 = 5,
+	MX6Q_RGMII_TD2 = 6,
+	MX6Q_RGMII_TD3 = 7,
+	MX6Q_RGMII_RX_CTL = 8,
+	MX6Q_RGMII_RD0 = 9,
+	MX6Q_RGMII_TX_CTL = 10,
+	MX6Q_RGMII_RD1 = 11,
+	MX6Q_RGMII_RD2 = 12,
+	MX6Q_RGMII_RD3 = 13,
+	MX6Q_RGMII_RXC = 14,
+	MX6Q_EIM_A25 = 15,
+	MX6Q_EIM_EB2 = 16,
+	MX6Q_EIM_D16 = 17,
+	MX6Q_EIM_D17 = 18,
+	MX6Q_EIM_D18 = 19,
+	MX6Q_EIM_D19 = 20,
+	MX6Q_EIM_D20 = 21,
+	MX6Q_EIM_D21 = 22,
+	MX6Q_EIM_D22 = 23,
+	MX6Q_EIM_D23 = 24,
+	MX6Q_EIM_EB3 = 25,
+	MX6Q_EIM_D24 = 26,
+	MX6Q_EIM_D25 = 27,
+	MX6Q_EIM_D26 = 28,
+	MX6Q_EIM_D27 = 29,
+	MX6Q_EIM_D28 = 30,
+	MX6Q_EIM_D29 = 31,
+	MX6Q_EIM_D30 = 32,
+	MX6Q_EIM_D31 = 33,
+	MX6Q_EIM_A24 = 34,
+	MX6Q_EIM_A23 = 35,
+	MX6Q_EIM_A22 = 36,
+	MX6Q_EIM_A21 = 37,
+	MX6Q_EIM_A20 = 38,
+	MX6Q_EIM_A19 = 39,
+	MX6Q_EIM_A18 = 40,
+	MX6Q_EIM_A17 = 41,
+	MX6Q_EIM_A16 = 42,
+	MX6Q_EIM_CS0 = 43,
+	MX6Q_EIM_CS1 = 44,
+	MX6Q_EIM_OE = 45,
+	MX6Q_EIM_RW = 46,
+	MX6Q_EIM_LBA = 47,
+	MX6Q_EIM_EB0 = 48,
+	MX6Q_EIM_EB1 = 49,
+	MX6Q_EIM_DA0 = 50,
+	MX6Q_EIM_DA1 = 51,
+	MX6Q_EIM_DA2 = 52,
+	MX6Q_EIM_DA3 = 53,
+	MX6Q_EIM_DA4 = 54,
+	MX6Q_EIM_DA5 = 55,
+	MX6Q_EIM_DA6 = 56,
+	MX6Q_EIM_DA7 = 57,
+	MX6Q_EIM_DA8 = 58,
+	MX6Q_EIM_DA9 = 59,
+	MX6Q_EIM_DA10 = 60,
+	MX6Q_EIM_DA11 = 61,
+	MX6Q_EIM_DA12 = 62,
+	MX6Q_EIM_DA13 = 63,
+	MX6Q_EIM_DA14 = 64,
+	MX6Q_EIM_DA15 = 65,
+	MX6Q_EIM_WAIT = 66,
+	MX6Q_EIM_BCLK = 67,
+	MX6Q_DI0_DISP_CLK = 68,
+	MX6Q_DI0_PIN15 = 69,
+	MX6Q_DI0_PIN2 = 70,
+	MX6Q_DI0_PIN3 = 71,
+	MX6Q_DI0_PIN4 = 72,
+	MX6Q_DISP0_DAT0 = 73,
+	MX6Q_DISP0_DAT1 = 74,
+	MX6Q_DISP0_DAT2 = 75,
+	MX6Q_DISP0_DAT3 = 76,
+	MX6Q_DISP0_DAT4 = 77,
+	MX6Q_DISP0_DAT5 = 78,
+	MX6Q_DISP0_DAT6 = 79,
+	MX6Q_DISP0_DAT7 = 80,
+	MX6Q_DISP0_DAT8 = 81,
+	MX6Q_DISP0_DAT9 = 82,
+	MX6Q_DISP0_DAT10 = 83,
+	MX6Q_DISP0_DAT11 = 84,
+	MX6Q_DISP0_DAT12 = 85,
+	MX6Q_DISP0_DAT13 = 86,
+	MX6Q_DISP0_DAT14 = 87,
+	MX6Q_DISP0_DAT15 = 88,
+	MX6Q_DISP0_DAT16 = 89,
+	MX6Q_DISP0_DAT17 = 90,
+	MX6Q_DISP0_DAT18 = 91,
+	MX6Q_DISP0_DAT19 = 92,
+	MX6Q_DISP0_DAT20 = 93,
+	MX6Q_DISP0_DAT21 = 94,
+	MX6Q_DISP0_DAT22 = 95,
+	MX6Q_DISP0_DAT23 = 96,
+	MX6Q_ENET_MDIO = 97,
+	MX6Q_ENET_REF_CLK = 98,
+	MX6Q_ENET_RX_ER = 99,
+	MX6Q_ENET_CRS_DV = 100,
+	MX6Q_ENET_RXD1 = 101,
+	MX6Q_ENET_RXD0 = 102,
+	MX6Q_ENET_TX_EN = 103,
+	MX6Q_ENET_TXD1 = 104,
+	MX6Q_ENET_TXD0 = 105,
+	MX6Q_ENET_MDC = 106,
+	MX6Q_KEY_COL0 = 107,
+	MX6Q_KEY_ROW0 = 108,
+	MX6Q_KEY_COL1 = 109,
+	MX6Q_KEY_ROW1 = 110,
+	MX6Q_KEY_COL2 = 111,
+	MX6Q_KEY_ROW2 = 112,
+	MX6Q_KEY_COL3 = 113,
+	MX6Q_KEY_ROW3 = 114,
+	MX6Q_KEY_COL4 = 115,
+	MX6Q_KEY_ROW4 = 116,
+	MX6Q_GPIO_0 = 117,
+	MX6Q_GPIO_1 = 118,
+	MX6Q_GPIO_9 = 119,
+	MX6Q_GPIO_3 = 120,
+	MX6Q_GPIO_6 = 121,
+	MX6Q_GPIO_2 = 122,
+	MX6Q_GPIO_4 = 123,
+	MX6Q_GPIO_5 = 124,
+	MX6Q_GPIO_7 = 125,
+	MX6Q_GPIO_8 = 126,
+	MX6Q_GPIO_16 = 127,
+	MX6Q_GPIO_17 = 128,
+	MX6Q_GPIO_18 = 129,
+	MX6Q_GPIO_19 = 130,
+	MX6Q_CSI0_PIXCLK = 131,
+	MX6Q_CSI0_MCLK = 132,
+	MX6Q_CSI0_DATA_EN = 133,
+	MX6Q_CSI0_VSYNC = 134,
+	MX6Q_CSI0_DAT4 = 135,
+	MX6Q_CSI0_DAT5 = 136,
+	MX6Q_CSI0_DAT6 = 137,
+	MX6Q_CSI0_DAT7 = 138,
+	MX6Q_CSI0_DAT8 = 139,
+	MX6Q_CSI0_DAT9 = 140,
+	MX6Q_CSI0_DAT10 = 141,
+	MX6Q_CSI0_DAT11 = 142,
+	MX6Q_CSI0_DAT12 = 143,
+	MX6Q_CSI0_DAT13 = 144,
+	MX6Q_CSI0_DAT14 = 145,
+	MX6Q_CSI0_DAT15 = 146,
+	MX6Q_CSI0_DAT16 = 147,
+	MX6Q_CSI0_DAT17 = 148,
+	MX6Q_CSI0_DAT18 = 149,
+	MX6Q_CSI0_DAT19 = 150,
+	MX6Q_SD3_DAT7 = 151,
+	MX6Q_SD3_DAT6 = 152,
+	MX6Q_SD3_DAT5 = 153,
+	MX6Q_SD3_DAT4 = 154,
+	MX6Q_SD3_CMD = 155,
+	MX6Q_SD3_CLK = 156,
+	MX6Q_SD3_DAT0 = 157,
+	MX6Q_SD3_DAT1 = 158,
+	MX6Q_SD3_DAT2 = 159,
+	MX6Q_SD3_DAT3 = 160,
+	MX6Q_SD3_RST = 161,
+	MX6Q_NANDF_CLE = 162,
+	MX6Q_NANDF_ALE = 163,
+	MX6Q_NANDF_WP_B = 164,
+	MX6Q_NANDF_RB0 = 165,
+	MX6Q_NANDF_CS0 = 166,
+	MX6Q_NANDF_CS1 = 167,
+	MX6Q_NANDF_CS2 = 168,
+	MX6Q_NANDF_CS3 = 169,
+	MX6Q_SD4_CMD = 170,
+	MX6Q_SD4_CLK = 171,
+	MX6Q_NANDF_D0 = 172,
+	MX6Q_NANDF_D1 = 173,
+	MX6Q_NANDF_D2 = 174,
+	MX6Q_NANDF_D3 = 175,
+	MX6Q_NANDF_D4 = 176,
+	MX6Q_NANDF_D5 = 177,
+	MX6Q_NANDF_D6 = 178,
+	MX6Q_NANDF_D7 = 179,
+	MX6Q_SD4_DAT0 = 180,
+	MX6Q_SD4_DAT1 = 181,
+	MX6Q_SD4_DAT2 = 182,
+	MX6Q_SD4_DAT3 = 183,
+	MX6Q_SD4_DAT4 = 184,
+	MX6Q_SD4_DAT5 = 185,
+	MX6Q_SD4_DAT6 = 186,
+	MX6Q_SD4_DAT7 = 187,
+	MX6Q_SD1_DAT1 = 188,
+	MX6Q_SD1_DAT0 = 189,
+	MX6Q_SD1_DAT3 = 190,
+	MX6Q_SD1_CMD = 191,
+	MX6Q_SD1_DAT2 = 192,
+	MX6Q_SD1_CLK = 193,
+	MX6Q_SD2_CLK = 194,
+	MX6Q_SD2_CMD = 195,
+	MX6Q_SD2_DAT3 = 196
+};
+
+/* Pad names for the pinmux subsystem */
+static const struct pinctrl_pin_desc imx6q_pinctrl_pads[] = {
+	IMX_PINCTRL_PIN(MX6Q_SD2_DAT1),
+	IMX_PINCTRL_PIN(MX6Q_SD2_DAT2),
+	IMX_PINCTRL_PIN(MX6Q_SD2_DAT0),
+	IMX_PINCTRL_PIN(MX6Q_RGMII_TXC),
+	IMX_PINCTRL_PIN(MX6Q_RGMII_TD0),
+	IMX_PINCTRL_PIN(MX6Q_RGMII_TD1),
+	IMX_PINCTRL_PIN(MX6Q_RGMII_TD2),
+	IMX_PINCTRL_PIN(MX6Q_RGMII_TD3),
+	IMX_PINCTRL_PIN(MX6Q_RGMII_RX_CTL),
+	IMX_PINCTRL_PIN(MX6Q_RGMII_RD0),
+	IMX_PINCTRL_PIN(MX6Q_RGMII_TX_CTL),
+	IMX_PINCTRL_PIN(MX6Q_RGMII_RD1),
+	IMX_PINCTRL_PIN(MX6Q_RGMII_RD2),
+	IMX_PINCTRL_PIN(MX6Q_RGMII_RD3),
+	IMX_PINCTRL_PIN(MX6Q_RGMII_RXC),
+	IMX_PINCTRL_PIN(MX6Q_EIM_A25),
+	IMX_PINCTRL_PIN(MX6Q_EIM_EB2),
+	IMX_PINCTRL_PIN(MX6Q_EIM_D16),
+	IMX_PINCTRL_PIN(MX6Q_EIM_D17),
+	IMX_PINCTRL_PIN(MX6Q_EIM_D18),
+	IMX_PINCTRL_PIN(MX6Q_EIM_D19),
+	IMX_PINCTRL_PIN(MX6Q_EIM_D20),
+	IMX_PINCTRL_PIN(MX6Q_EIM_D21),
+	IMX_PINCTRL_PIN(MX6Q_EIM_D22),
+	IMX_PINCTRL_PIN(MX6Q_EIM_D23),
+	IMX_PINCTRL_PIN(MX6Q_EIM_EB3),
+	IMX_PINCTRL_PIN(MX6Q_EIM_D24),
+	IMX_PINCTRL_PIN(MX6Q_EIM_D25),
+	IMX_PINCTRL_PIN(MX6Q_EIM_D26),
+	IMX_PINCTRL_PIN(MX6Q_EIM_D27),
+	IMX_PINCTRL_PIN(MX6Q_EIM_D28),
+	IMX_PINCTRL_PIN(MX6Q_EIM_D29),
+	IMX_PINCTRL_PIN(MX6Q_EIM_D30),
+	IMX_PINCTRL_PIN(MX6Q_EIM_D31),
+	IMX_PINCTRL_PIN(MX6Q_EIM_A24),
+	IMX_PINCTRL_PIN(MX6Q_EIM_A23),
+	IMX_PINCTRL_PIN(MX6Q_EIM_A22),
+	IMX_PINCTRL_PIN(MX6Q_EIM_A21),
+	IMX_PINCTRL_PIN(MX6Q_EIM_A20),
+	IMX_PINCTRL_PIN(MX6Q_EIM_A19),
+	IMX_PINCTRL_PIN(MX6Q_EIM_A18),
+	IMX_PINCTRL_PIN(MX6Q_EIM_A17),
+	IMX_PINCTRL_PIN(MX6Q_EIM_A16),
+	IMX_PINCTRL_PIN(MX6Q_EIM_CS0),
+	IMX_PINCTRL_PIN(MX6Q_EIM_CS1),
+	IMX_PINCTRL_PIN(MX6Q_EIM_OE),
+	IMX_PINCTRL_PIN(MX6Q_EIM_RW),
+	IMX_PINCTRL_PIN(MX6Q_EIM_LBA),
+	IMX_PINCTRL_PIN(MX6Q_EIM_EB0),
+	IMX_PINCTRL_PIN(MX6Q_EIM_EB1),
+	IMX_PINCTRL_PIN(MX6Q_EIM_DA0),
+	IMX_PINCTRL_PIN(MX6Q_EIM_DA1),
+	IMX_PINCTRL_PIN(MX6Q_EIM_DA2),
+	IMX_PINCTRL_PIN(MX6Q_EIM_DA3),
+	IMX_PINCTRL_PIN(MX6Q_EIM_DA4),
+	IMX_PINCTRL_PIN(MX6Q_EIM_DA5),
+	IMX_PINCTRL_PIN(MX6Q_EIM_DA6),
+	IMX_PINCTRL_PIN(MX6Q_EIM_DA7),
+	IMX_PINCTRL_PIN(MX6Q_EIM_DA8),
+	IMX_PINCTRL_PIN(MX6Q_EIM_DA9),
+	IMX_PINCTRL_PIN(MX6Q_EIM_DA10),
+	IMX_PINCTRL_PIN(MX6Q_EIM_DA11),
+	IMX_PINCTRL_PIN(MX6Q_EIM_DA12),
+	IMX_PINCTRL_PIN(MX6Q_EIM_DA13),
+	IMX_PINCTRL_PIN(MX6Q_EIM_DA14),
+	IMX_PINCTRL_PIN(MX6Q_EIM_DA15),
+	IMX_PINCTRL_PIN(MX6Q_EIM_WAIT),
+	IMX_PINCTRL_PIN(MX6Q_EIM_BCLK),
+	IMX_PINCTRL_PIN(MX6Q_DI0_DISP_CLK),
+	IMX_PINCTRL_PIN(MX6Q_DI0_PIN15),
+	IMX_PINCTRL_PIN(MX6Q_DI0_PIN2),
+	IMX_PINCTRL_PIN(MX6Q_DI0_PIN3),
+	IMX_PINCTRL_PIN(MX6Q_DI0_PIN4),
+	IMX_PINCTRL_PIN(MX6Q_DISP0_DAT0),
+	IMX_PINCTRL_PIN(MX6Q_DISP0_DAT1),
+	IMX_PINCTRL_PIN(MX6Q_DISP0_DAT2),
+	IMX_PINCTRL_PIN(MX6Q_DISP0_DAT3),
+	IMX_PINCTRL_PIN(MX6Q_DISP0_DAT4),
+	IMX_PINCTRL_PIN(MX6Q_DISP0_DAT5),
+	IMX_PINCTRL_PIN(MX6Q_DISP0_DAT6),
+	IMX_PINCTRL_PIN(MX6Q_DISP0_DAT7),
+	IMX_PINCTRL_PIN(MX6Q_DISP0_DAT8),
+	IMX_PINCTRL_PIN(MX6Q_DISP0_DAT9),
+	IMX_PINCTRL_PIN(MX6Q_DISP0_DAT10),
+	IMX_PINCTRL_PIN(MX6Q_DISP0_DAT11),
+	IMX_PINCTRL_PIN(MX6Q_DISP0_DAT12),
+	IMX_PINCTRL_PIN(MX6Q_DISP0_DAT13),
+	IMX_PINCTRL_PIN(MX6Q_DISP0_DAT14),
+	IMX_PINCTRL_PIN(MX6Q_DISP0_DAT15),
+	IMX_PINCTRL_PIN(MX6Q_DISP0_DAT16),
+	IMX_PINCTRL_PIN(MX6Q_DISP0_DAT17),
+	IMX_PINCTRL_PIN(MX6Q_DISP0_DAT18),
+	IMX_PINCTRL_PIN(MX6Q_DISP0_DAT19),
+	IMX_PINCTRL_PIN(MX6Q_DISP0_DAT20),
+	IMX_PINCTRL_PIN(MX6Q_DISP0_DAT21),
+	IMX_PINCTRL_PIN(MX6Q_DISP0_DAT22),
+	IMX_PINCTRL_PIN(MX6Q_DISP0_DAT23),
+	IMX_PINCTRL_PIN(MX6Q_ENET_MDIO),
+	IMX_PINCTRL_PIN(MX6Q_ENET_REF_CLK),
+	IMX_PINCTRL_PIN(MX6Q_ENET_RX_ER),
+	IMX_PINCTRL_PIN(MX6Q_ENET_CRS_DV),
+	IMX_PINCTRL_PIN(MX6Q_ENET_RXD1),
+	IMX_PINCTRL_PIN(MX6Q_ENET_RXD0),
+	IMX_PINCTRL_PIN(MX6Q_ENET_TX_EN),
+	IMX_PINCTRL_PIN(MX6Q_ENET_TXD1),
+	IMX_PINCTRL_PIN(MX6Q_ENET_TXD0),
+	IMX_PINCTRL_PIN(MX6Q_ENET_MDC),
+	IMX_PINCTRL_PIN(MX6Q_KEY_COL0),
+	IMX_PINCTRL_PIN(MX6Q_KEY_ROW0),
+	IMX_PINCTRL_PIN(MX6Q_KEY_COL1),
+	IMX_PINCTRL_PIN(MX6Q_KEY_ROW1),
+	IMX_PINCTRL_PIN(MX6Q_KEY_COL2),
+	IMX_PINCTRL_PIN(MX6Q_KEY_ROW2),
+	IMX_PINCTRL_PIN(MX6Q_KEY_COL3),
+	IMX_PINCTRL_PIN(MX6Q_KEY_ROW3),
+	IMX_PINCTRL_PIN(MX6Q_KEY_COL4),
+	IMX_PINCTRL_PIN(MX6Q_KEY_ROW4),
+	IMX_PINCTRL_PIN(MX6Q_GPIO_0),
+	IMX_PINCTRL_PIN(MX6Q_GPIO_1),
+	IMX_PINCTRL_PIN(MX6Q_GPIO_9),
+	IMX_PINCTRL_PIN(MX6Q_GPIO_3),
+	IMX_PINCTRL_PIN(MX6Q_GPIO_6),
+	IMX_PINCTRL_PIN(MX6Q_GPIO_2),
+	IMX_PINCTRL_PIN(MX6Q_GPIO_4),
+	IMX_PINCTRL_PIN(MX6Q_GPIO_5),
+	IMX_PINCTRL_PIN(MX6Q_GPIO_7),
+	IMX_PINCTRL_PIN(MX6Q_GPIO_8),
+	IMX_PINCTRL_PIN(MX6Q_GPIO_16),
+	IMX_PINCTRL_PIN(MX6Q_GPIO_17),
+	IMX_PINCTRL_PIN(MX6Q_GPIO_18),
+	IMX_PINCTRL_PIN(MX6Q_GPIO_19),
+	IMX_PINCTRL_PIN(MX6Q_CSI0_PIXCLK),
+	IMX_PINCTRL_PIN(MX6Q_CSI0_MCLK),
+	IMX_PINCTRL_PIN(MX6Q_CSI0_DATA_EN),
+	IMX_PINCTRL_PIN(MX6Q_CSI0_VSYNC),
+	IMX_PINCTRL_PIN(MX6Q_CSI0_DAT4),
+	IMX_PINCTRL_PIN(MX6Q_CSI0_DAT5),
+	IMX_PINCTRL_PIN(MX6Q_CSI0_DAT6),
+	IMX_PINCTRL_PIN(MX6Q_CSI0_DAT7),
+	IMX_PINCTRL_PIN(MX6Q_CSI0_DAT8),
+	IMX_PINCTRL_PIN(MX6Q_CSI0_DAT9),
+	IMX_PINCTRL_PIN(MX6Q_CSI0_DAT10),
+	IMX_PINCTRL_PIN(MX6Q_CSI0_DAT11),
+	IMX_PINCTRL_PIN(MX6Q_CSI0_DAT12),
+	IMX_PINCTRL_PIN(MX6Q_CSI0_DAT13),
+	IMX_PINCTRL_PIN(MX6Q_CSI0_DAT14),
+	IMX_PINCTRL_PIN(MX6Q_CSI0_DAT15),
+	IMX_PINCTRL_PIN(MX6Q_CSI0_DAT16),
+	IMX_PINCTRL_PIN(MX6Q_CSI0_DAT17),
+	IMX_PINCTRL_PIN(MX6Q_CSI0_DAT18),
+	IMX_PINCTRL_PIN(MX6Q_CSI0_DAT19),
+	IMX_PINCTRL_PIN(MX6Q_SD3_DAT7),
+	IMX_PINCTRL_PIN(MX6Q_SD3_DAT6),
+	IMX_PINCTRL_PIN(MX6Q_SD3_DAT5),
+	IMX_PINCTRL_PIN(MX6Q_SD3_DAT4),
+	IMX_PINCTRL_PIN(MX6Q_SD3_CMD),
+	IMX_PINCTRL_PIN(MX6Q_SD3_CLK),
+	IMX_PINCTRL_PIN(MX6Q_SD3_DAT0),
+	IMX_PINCTRL_PIN(MX6Q_SD3_DAT1),
+	IMX_PINCTRL_PIN(MX6Q_SD3_DAT2),
+	IMX_PINCTRL_PIN(MX6Q_SD3_DAT3),
+	IMX_PINCTRL_PIN(MX6Q_SD3_RST),
+	IMX_PINCTRL_PIN(MX6Q_NANDF_CLE),
+	IMX_PINCTRL_PIN(MX6Q_NANDF_ALE),
+	IMX_PINCTRL_PIN(MX6Q_NANDF_WP_B),
+	IMX_PINCTRL_PIN(MX6Q_NANDF_RB0),
+	IMX_PINCTRL_PIN(MX6Q_NANDF_CS0),
+	IMX_PINCTRL_PIN(MX6Q_NANDF_CS1),
+	IMX_PINCTRL_PIN(MX6Q_NANDF_CS2),
+	IMX_PINCTRL_PIN(MX6Q_NANDF_CS3),
+	IMX_PINCTRL_PIN(MX6Q_SD4_CMD),
+	IMX_PINCTRL_PIN(MX6Q_SD4_CLK),
+	IMX_PINCTRL_PIN(MX6Q_NANDF_D0),
+	IMX_PINCTRL_PIN(MX6Q_NANDF_D1),
+	IMX_PINCTRL_PIN(MX6Q_NANDF_D2),
+	IMX_PINCTRL_PIN(MX6Q_NANDF_D3),
+	IMX_PINCTRL_PIN(MX6Q_NANDF_D4),
+	IMX_PINCTRL_PIN(MX6Q_NANDF_D5),
+	IMX_PINCTRL_PIN(MX6Q_NANDF_D6),
+	IMX_PINCTRL_PIN(MX6Q_NANDF_D7),
+	IMX_PINCTRL_PIN(MX6Q_SD4_DAT0),
+	IMX_PINCTRL_PIN(MX6Q_SD4_DAT1),
+	IMX_PINCTRL_PIN(MX6Q_SD4_DAT2),
+	IMX_PINCTRL_PIN(MX6Q_SD4_DAT3),
+	IMX_PINCTRL_PIN(MX6Q_SD4_DAT4),
+	IMX_PINCTRL_PIN(MX6Q_SD4_DAT5),
+	IMX_PINCTRL_PIN(MX6Q_SD4_DAT6),
+	IMX_PINCTRL_PIN(MX6Q_SD4_DAT7),
+	IMX_PINCTRL_PIN(MX6Q_SD1_DAT1),
+	IMX_PINCTRL_PIN(MX6Q_SD1_DAT0),
+	IMX_PINCTRL_PIN(MX6Q_SD1_DAT3),
+	IMX_PINCTRL_PIN(MX6Q_SD1_CMD),
+	IMX_PINCTRL_PIN(MX6Q_SD1_DAT2),
+	IMX_PINCTRL_PIN(MX6Q_SD1_CLK),
+	IMX_PINCTRL_PIN(MX6Q_SD2_CLK),
+	IMX_PINCTRL_PIN(MX6Q_SD2_CMD),
+	IMX_PINCTRL_PIN(MX6Q_SD2_DAT3),
+};
+
+struct imx_pinctrl_info imx6q_pinctrl_info = {
+	.type = IMX6Q_PINCTRL,
+	.pins = imx6q_pinctrl_pads,
+	.npins = ARRAY_SIZE(imx6q_pinctrl_pads),
+	.maxpin = IMX6Q_IOMUXC_MAXPIN,
+	.mux_offset = IMX6Q_IOMUXC_MUX_OFSSET,
+};
-- 
1.7.0.4

[RFC PATCH v2 2/4] pinctrl: imx: add pinmux imx driver

[Fabio Estevam]

On Wed, Dec 14, 2011 at 2:03 PM, Dong Aisheng <b29396@freescale.com> wrote:

> +config PINMUX_IMX53
> + ? ? ? bool "IMX53 pinmux driver"
> + ? ? ? depends on ARCH_MX5

Shouldn't this be SOC_IMX53 instead?

Regards,

Fabio Estevam

[RFC PATCH v2 2/4] pinctrl: imx: add pinmux imx driver

[Dong Aisheng-B29396]

> -----Original Message-----
> From: Fabio Estevam [mailto:festevam at gmail.com]
> Sent: Thursday, December 15, 2011 12:01 AM
> To: Dong Aisheng-B29396
> Cc: linux-kernel at vger.kernel.org; linus.walleij at stericsson.com;
> s.hauer at pengutronix.de; rob.herring at calxeda.com;
> grant.likely at secretlab.ca; linux-arm-kernel at lists.infradead.org;
> kernel at pengutronix.de; Guo Shawn-R65073
> Subject: Re: [RFC PATCH v2 2/4] pinctrl: imx: add pinmux imx driver
> Importance: High
> 
> On Wed, Dec 14, 2011 at 2:03 PM, Dong Aisheng <b29396@freescale.com>
> wrote:
> 
> > +config PINMUX_IMX53
> > + ? ? ? bool "IMX53 pinmux driver"
> > + ? ? ? depends on ARCH_MX5
> 
> Shouldn't this be SOC_IMX53 instead?
> 
Yes, it should be SOC_IMX53.
Thanks.

Regards
Dong Aisheng

[RFC PATCH v2 2/4] pinctrl: imx: add pinmux imx driver

[Linus Walleij]

On Wed, Dec 14, 2011 at 5:03 PM, Dong Aisheng <b29396@freescale.com> wrote:

> The driver contains the initial support for imx53 and
> imx6q.

Cool! Comments below...

> --- /dev/null
> +++ b/drivers/pinctrl/pinmux-imx-core.c

We should start naming these pinctrl-* rather than pinmux-*
so just rename it.

Until here it looks like business as usual..

> +#ifdef CONFIG_OF
> +static int __devinit imx_pmx_parse_functions(struct device_node *np,
> + ? ? ? ? ? ? ? ? ? ? ? struct imx_pinctrl_info *info, u32 num)
> +{
> + ? ? ? struct imx_pmx_func *function;
> + ? ? ? struct imx_pin_group *group;
> + ? ? ? int ret, len;
> +
> + ? ? ? dev_dbg(info->dev, "parse function %d\n", num);
> +
> + ? ? ? group = &info->groups[num];
> + ? ? ? function = &info->functions[num];
> +
> + ? ? ? /* Initialise group */
> + ? ? ? ret = of_property_read_string(np, "grp_name", &group->name);
> + ? ? ? if (ret) {
> + ? ? ? ? ? ? ? dev_err(info->dev, "failed to get grp_name\n");
> + ? ? ? ? ? ? ? return ret;
> + ? ? ? }
> +
> + ? ? ? ret = of_property_read_u32(np, "num_pins", &group->num_pins);
> + ? ? ? if (ret) {
> + ? ? ? ? ? ? ? dev_err(info->dev, "failed to get num_pins\n");
> + ? ? ? ? ? ? ? return ret;
> + ? ? ? }
> +
> + ? ? ? ret = of_property_read_u32(np, "num_mux", &group->num_mux);
> + ? ? ? if (ret) {
> + ? ? ? ? ? ? ? dev_err(info->dev, "failed to get num_mux\n");
> + ? ? ? ? ? ? ? return ret;
> + ? ? ? }
> +
> + ? ? ? if (group->num_pins != group->num_mux)
> + ? ? ? ? ? ? ? return -EINVAL;
> +
> + ? ? ? group->pins = devm_kzalloc(info->dev, group->num_pins * sizeof(unsigned int),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? GFP_KERNEL);
> + ? ? ? group->mux_mode = devm_kzalloc(info->dev, group->num_mux * sizeof(unsigned int),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? GFP_KERNEL);
> + ? ? ? if (!group->pins || !group->mux_mode)
> + ? ? ? ? ? ? ? return -ENOMEM;
> +
> + ? ? ? /* sanity check */
> + ? ? ? if (of_get_property(np, "grp_pins", &len) &&
> + ? ? ? ? ? ? ? len != group->num_pins * sizeof(unsigned int)) {
> + ? ? ? ? ? ? ? dev_err(info->dev, "wrong pins number?\n");
> + ? ? ? ? ? ? ? return -EINVAL;
> + ? ? ? }
> +
> + ? ? ? if (of_get_property(np, "grp_mux", &len) &&
> + ? ? ? ? ? ? ? len != group->num_mux * sizeof(unsigned int)) {
> + ? ? ? ? ? ? ? dev_err(info->dev, "wrong pin mux number?\n");
> + ? ? ? ? ? ? ? return -EINVAL;
> + ? ? ? }
> +
> + ? ? ? ret = of_property_read_u32_array(np, "grp_pins",
> + ? ? ? ? ? ? ? ? ? ? ? group->pins, group->num_pins);
> + ? ? ? if (ret) {
> + ? ? ? ? ? ? ? dev_err(info->dev, "failed to get grp_pins\n");
> + ? ? ? ? ? ? ? return ret;
> + ? ? ? }
> +
> + ? ? ? ret = of_property_read_u32_array(np, "grp_mux",
> + ? ? ? ? ? ? ? ? ? ? ? group->mux_mode, group->num_mux);
> + ? ? ? if (ret) {
> + ? ? ? ? ? ? ? dev_err(info->dev, "failed to get grp_mux\n");
> + ? ? ? ? ? ? ? return ret;
> + ? ? ? }
> +
> + ? ? ? /* Initialise function */
> + ? ? ? ret = of_property_read_string(np, "func_name", &function->name);
> + ? ? ? if (ret) {
> + ? ? ? ? ? ? ? dev_err(info->dev, "failed to get func_name\n");
> + ? ? ? ? ? ? ? return ret;
> + ? ? ? }
> +
> + ? ? ? function->groups = devm_kzalloc(info->dev, sizeof(char **), GFP_KERNEL);
> + ? ? ? function->num_groups = 1;
> + ? ? ? function->groups[0] = group->name;
> +
> + ? ? ? dev_dbg(info->dev, "func_name %s grp_name %s num_groups %d\n",
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? function->name, function->groups[0],
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? function->num_groups);
> +
> + ? ? ? return 0;
> +}
> +
> +static int __devinit imx_pmx_probe_dt(struct platform_device *pdev,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct imx_pinctrl_info *info)
> +{
> + ? ? ? struct device_node *np = pdev->dev.of_node;
> + ? ? ? struct device_node *child = NULL;
> + ? ? ? int ret, i;
> + ? ? ? u32 nfuncs = 0;
> +
> + ? ? ? if (!np)
> + ? ? ? ? ? ? ? return -ENODEV;
> +
> + ? ? ? nfuncs = of_get_child_number(np);
> + ? ? ? if (nfuncs <= 0) {
> + ? ? ? ? ? ? ? dev_err(&pdev->dev, "no functions defined\n");
> + ? ? ? ? ? ? ? return -EINVAL;
> + ? ? ? }
> +
> + ? ? ? info->nfunctions = nfuncs;
> + ? ? ? info->functions = devm_kzalloc(&pdev->dev, nfuncs * sizeof(struct imx_pmx_func),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? GFP_KERNEL);
> + ? ? ? if (!info->functions)
> + ? ? ? ? ? ? ? return -ENOMEM;
> +
> + ? ? ? /* DT file only passes one group per one function */
> + ? ? ? info->ngroups = nfuncs;
> + ? ? ? info->groups = devm_kzalloc(&pdev->dev, nfuncs * sizeof(struct imx_pin_group),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? GFP_KERNEL);
> + ? ? ? if (!info->groups)
> + ? ? ? ? ? ? ? return -ENOMEM;
> +
> + ? ? ? child = NULL;
> + ? ? ? i = 0;
> + ? ? ? for_each_child_of_node(np, child) {
> + ? ? ? ? ? ? ? ret = imx_pmx_parse_functions(child, info, i++);
> + ? ? ? ? ? ? ? if (ret) {
> + ? ? ? ? ? ? ? ? ? ? ? dev_err(&pdev->dev, "failed to parse function\n");
> + ? ? ? ? ? ? ? ? ? ? ? return ret;
> + ? ? ? ? ? ? ? }
> + ? ? ? }
> +
> + ? ? ? return 0;
> +}
> +#else
> +static int __devinit imx_pmx_probe_dt(struct platform_device *pdev,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct imx_pinctrl_info *info)
> +{
> + ? ? ? return -ENODEV;
> +}
> +#endif

Looks clever to me! And I think we have agreed that this probing
must be inside each driver.

Can we have a look at the corresponding device tree?
(...)

> +++ b/drivers/pinctrl/pinmux-imx-core.h

Name this pinctrl-* too for consistency.

(...)
> +#define IMX_PINCTRL_PIN(pin) PINCTRL_PIN(pin, #pin)

Now I start to think about the range generators I discussed with
Haojian... but there doesn't seem to be very many ranges in this
driver so I guess it would only confuse things.

> +#define IMX_PIN_GROUP(n, p, m) ?\
> + ? ? ? { ? ? ? ? ? ? ? ? ? ? ? \
> + ? ? ? ? ? ? ? .name = n, ? ? ?\
> + ? ? ? ? ? ? ? .pins = p, ? ? ?\
> + ? ? ? ? ? ? ? .num_pins = ARRAY_SIZE(p), ? ? ?\
> + ? ? ? ? ? ? ? .mux_mode = m, ?\
> + ? ? ? ? ? ? ? .num_mux = ARRAY_SIZE(m), ? ? ? \
> + ? ? ? }
> +
> +#define IMX_PMX_FUNC(n, g) ?\
> + ? ? ? { ? ? ? ? ? ? ? ? ? ? ? \
> + ? ? ? ? ? ? ? .name = n, ? ? ?\
> + ? ? ? ? ? ? ? .groups = g, ? ?\
> + ? ? ? ? ? ? ? .num_groups = ARRAY_SIZE(g), ? ?\
> + ? ? ? }

This looks pretty smart!

> +++ b/drivers/pinctrl/pinmux-imx53.c

Name this pinctrl-*

> diff --git a/drivers/pinctrl/pinmux-imx6q.c b/drivers/pinctrl/pinmux-imx6q.c

This too.

This patch is overall very good, we're quickly approaching merge quality.

Yours,
Linus Walleij

[RFC PATCH v2 2/4] pinctrl: imx: add pinmux imx driver

[Grant Likely]

On Wed, Dec 14, 2011 at 11:00 AM, Linus Walleij
<linus.walleij@linaro.org> wrote:
> On Wed, Dec 14, 2011 at 5:03 PM, Dong Aisheng <b29396@freescale.com> wrote:
>
>> The driver contains the initial support for imx53 and
>> imx6q.
>
> Cool! Comments below...
>
>> --- /dev/null
>> +++ b/drivers/pinctrl/pinmux-imx-core.c
>
> We should start naming these pinctrl-* rather than pinmux-*
> so just rename it.
>
> Until here it looks like business as usual..
>
>> +#ifdef CONFIG_OF
>> +static int __devinit imx_pmx_parse_functions(struct device_node *np,
>> + ? ? ? ? ? ? ? ? ? ? ? struct imx_pinctrl_info *info, u32 num)
>> +{
>> + ? ? ? struct imx_pmx_func *function;
>> + ? ? ? struct imx_pin_group *group;
>> + ? ? ? int ret, len;
>> +
>> + ? ? ? dev_dbg(info->dev, "parse function %d\n", num);
>> +
>> + ? ? ? group = &info->groups[num];
>> + ? ? ? function = &info->functions[num];
>> +
>> + ? ? ? /* Initialise group */
>> + ? ? ? ret = of_property_read_string(np, "grp_name", &group->name);
>> + ? ? ? if (ret) {
>> + ? ? ? ? ? ? ? dev_err(info->dev, "failed to get grp_name\n");
>> + ? ? ? ? ? ? ? return ret;
>> + ? ? ? }
>> +
>> + ? ? ? ret = of_property_read_u32(np, "num_pins", &group->num_pins);
>> + ? ? ? if (ret) {
>> + ? ? ? ? ? ? ? dev_err(info->dev, "failed to get num_pins\n");
>> + ? ? ? ? ? ? ? return ret;
>> + ? ? ? }
>> +
>> + ? ? ? ret = of_property_read_u32(np, "num_mux", &group->num_mux);
>> + ? ? ? if (ret) {
>> + ? ? ? ? ? ? ? dev_err(info->dev, "failed to get num_mux\n");
>> + ? ? ? ? ? ? ? return ret;
>> + ? ? ? }
>> +
>> + ? ? ? if (group->num_pins != group->num_mux)
>> + ? ? ? ? ? ? ? return -EINVAL;
>> +
>> + ? ? ? group->pins = devm_kzalloc(info->dev, group->num_pins * sizeof(unsigned int),
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? GFP_KERNEL);
>> + ? ? ? group->mux_mode = devm_kzalloc(info->dev, group->num_mux * sizeof(unsigned int),
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? GFP_KERNEL);
>> + ? ? ? if (!group->pins || !group->mux_mode)
>> + ? ? ? ? ? ? ? return -ENOMEM;
>> +
>> + ? ? ? /* sanity check */
>> + ? ? ? if (of_get_property(np, "grp_pins", &len) &&
>> + ? ? ? ? ? ? ? len != group->num_pins * sizeof(unsigned int)) {
>> + ? ? ? ? ? ? ? dev_err(info->dev, "wrong pins number?\n");
>> + ? ? ? ? ? ? ? return -EINVAL;
>> + ? ? ? }
>> +
>> + ? ? ? if (of_get_property(np, "grp_mux", &len) &&
>> + ? ? ? ? ? ? ? len != group->num_mux * sizeof(unsigned int)) {
>> + ? ? ? ? ? ? ? dev_err(info->dev, "wrong pin mux number?\n");
>> + ? ? ? ? ? ? ? return -EINVAL;
>> + ? ? ? }
>> +
>> + ? ? ? ret = of_property_read_u32_array(np, "grp_pins",
>> + ? ? ? ? ? ? ? ? ? ? ? group->pins, group->num_pins);
>> + ? ? ? if (ret) {
>> + ? ? ? ? ? ? ? dev_err(info->dev, "failed to get grp_pins\n");
>> + ? ? ? ? ? ? ? return ret;
>> + ? ? ? }
>> +
>> + ? ? ? ret = of_property_read_u32_array(np, "grp_mux",
>> + ? ? ? ? ? ? ? ? ? ? ? group->mux_mode, group->num_mux);
>> + ? ? ? if (ret) {
>> + ? ? ? ? ? ? ? dev_err(info->dev, "failed to get grp_mux\n");
>> + ? ? ? ? ? ? ? return ret;
>> + ? ? ? }
>> +
>> + ? ? ? /* Initialise function */
>> + ? ? ? ret = of_property_read_string(np, "func_name", &function->name);
>> + ? ? ? if (ret) {
>> + ? ? ? ? ? ? ? dev_err(info->dev, "failed to get func_name\n");
>> + ? ? ? ? ? ? ? return ret;
>> + ? ? ? }
>> +
>> + ? ? ? function->groups = devm_kzalloc(info->dev, sizeof(char **), GFP_KERNEL);
>> + ? ? ? function->num_groups = 1;
>> + ? ? ? function->groups[0] = group->name;
>> +
>> + ? ? ? dev_dbg(info->dev, "func_name %s grp_name %s num_groups %d\n",
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? function->name, function->groups[0],
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? function->num_groups);
>> +
>> + ? ? ? return 0;
>> +}
>> +
>> +static int __devinit imx_pmx_probe_dt(struct platform_device *pdev,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct imx_pinctrl_info *info)
>> +{
>> + ? ? ? struct device_node *np = pdev->dev.of_node;
>> + ? ? ? struct device_node *child = NULL;
>> + ? ? ? int ret, i;
>> + ? ? ? u32 nfuncs = 0;
>> +
>> + ? ? ? if (!np)
>> + ? ? ? ? ? ? ? return -ENODEV;
>> +
>> + ? ? ? nfuncs = of_get_child_number(np);
>> + ? ? ? if (nfuncs <= 0) {
>> + ? ? ? ? ? ? ? dev_err(&pdev->dev, "no functions defined\n");
>> + ? ? ? ? ? ? ? return -EINVAL;
>> + ? ? ? }
>> +
>> + ? ? ? info->nfunctions = nfuncs;
>> + ? ? ? info->functions = devm_kzalloc(&pdev->dev, nfuncs * sizeof(struct imx_pmx_func),
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? GFP_KERNEL);
>> + ? ? ? if (!info->functions)
>> + ? ? ? ? ? ? ? return -ENOMEM;
>> +
>> + ? ? ? /* DT file only passes one group per one function */
>> + ? ? ? info->ngroups = nfuncs;
>> + ? ? ? info->groups = devm_kzalloc(&pdev->dev, nfuncs * sizeof(struct imx_pin_group),
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? GFP_KERNEL);
>> + ? ? ? if (!info->groups)
>> + ? ? ? ? ? ? ? return -ENOMEM;
>> +
>> + ? ? ? child = NULL;
>> + ? ? ? i = 0;
>> + ? ? ? for_each_child_of_node(np, child) {
>> + ? ? ? ? ? ? ? ret = imx_pmx_parse_functions(child, info, i++);
>> + ? ? ? ? ? ? ? if (ret) {
>> + ? ? ? ? ? ? ? ? ? ? ? dev_err(&pdev->dev, "failed to parse function\n");
>> + ? ? ? ? ? ? ? ? ? ? ? return ret;
>> + ? ? ? ? ? ? ? }
>> + ? ? ? }
>> +
>> + ? ? ? return 0;
>> +}
>> +#else
>> +static int __devinit imx_pmx_probe_dt(struct platform_device *pdev,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct imx_pinctrl_info *info)
>> +{
>> + ? ? ? return -ENODEV;
>> +}
>> +#endif
>
> Looks clever to me! And I think we have agreed that this probing
> must be inside each driver.
>
> Can we have a look at the corresponding device tree?

... and binding documentation.  While on that topic, the convention is
for property names to use dash '-' characters, not underscores '_'.
The properties I see here are using the later.

g.

[RFC PATCH v2 2/4] pinctrl: imx: add pinmux imx driver

[Dong Aisheng-B29396]

Hi Grant,

> -----Original Message-----
> From: glikely at secretlab.ca [mailto:glikely at secretlab.ca] On Behalf Of
> Grant Likely
> Sent: Thursday, December 15, 2011 5:47 AM
> To: Linus Walleij
> Cc: Dong Aisheng-B29396; linux-kernel at vger.kernel.org; linux-arm-
> kernel at lists.infradead.org; linus.walleij at stericsson.com;
> s.hauer at pengutronix.de; Guo Shawn-R65073; kernel at pengutronix.de;
> rob.herring at calxeda.com
> Subject: Re: [RFC PATCH v2 2/4] pinctrl: imx: add pinmux imx driver
> Importance: High
> 
> On Wed, Dec 14, 2011 at 11:00 AM, Linus Walleij <linus.walleij@linaro.org>
> wrote:
> > On Wed, Dec 14, 2011 at 5:03 PM, Dong Aisheng <b29396@freescale.com>
> wrote:
> >
> >> The driver contains the initial support for imx53 and imx6q.
> >
> > Cool! Comments below...
> >
> >> --- /dev/null
> >> +++ b/drivers/pinctrl/pinmux-imx-core.c
> >
> > We should start naming these pinctrl-* rather than pinmux-* so just
> > rename it.
> >
> > Until here it looks like business as usual..
> >
> >> +#ifdef CONFIG_OF
> >> +static int __devinit imx_pmx_parse_functions(struct device_node *np,
> >> + ? ? ? ? ? ? ? ? ? ? ? struct imx_pinctrl_info *info, u32 num) {
> >> + ? ? ? struct imx_pmx_func *function;
> >> + ? ? ? struct imx_pin_group *group;
> >> + ? ? ? int ret, len;
> >> +
> >> + ? ? ? dev_dbg(info->dev, "parse function %d\n", num);
> >> +
> >> + ? ? ? group = &info->groups[num];
> >> + ? ? ? function = &info->functions[num];
> >> +
> >> + ? ? ? /* Initialise group */
> >> + ? ? ? ret = of_property_read_string(np, "grp_name", &group->name);
> >> + ? ? ? if (ret) {
> >> + ? ? ? ? ? ? ? dev_err(info->dev, "failed to get grp_name\n");
> >> + ? ? ? ? ? ? ? return ret;
> >> + ? ? ? }
> >> +
> >> + ? ? ? ret = of_property_read_u32(np, "num_pins", &group->num_pins);
> >> + ? ? ? if (ret) {
> >> + ? ? ? ? ? ? ? dev_err(info->dev, "failed to get num_pins\n");
> >> + ? ? ? ? ? ? ? return ret;
> >> + ? ? ? }
> >> +
> >> + ? ? ? ret = of_property_read_u32(np, "num_mux", &group->num_mux);
> >> + ? ? ? if (ret) {
> >> + ? ? ? ? ? ? ? dev_err(info->dev, "failed to get num_mux\n");
> >> + ? ? ? ? ? ? ? return ret;
> >> + ? ? ? }
> >> +
> >> + ? ? ? if (group->num_pins != group->num_mux)
> >> + ? ? ? ? ? ? ? return -EINVAL;
> >> +
> >> + ? ? ? group->pins = devm_kzalloc(info->dev, group->num_pins *
> >> + sizeof(unsigned int),
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? GFP_KERNEL);
> >> + ? ? ? group->mux_mode = devm_kzalloc(info->dev, group->num_mux *
> >> + sizeof(unsigned int),
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? GFP_KERNEL);
> >> + ? ? ? if (!group->pins || !group->mux_mode)
> >> + ? ? ? ? ? ? ? return -ENOMEM;
> >> +
> >> + ? ? ? /* sanity check */
> >> + ? ? ? if (of_get_property(np, "grp_pins", &len) &&
> >> + ? ? ? ? ? ? ? len != group->num_pins * sizeof(unsigned int)) {
> >> + ? ? ? ? ? ? ? dev_err(info->dev, "wrong pins number?\n");
> >> + ? ? ? ? ? ? ? return -EINVAL;
> >> + ? ? ? }
> >> +
> >> + ? ? ? if (of_get_property(np, "grp_mux", &len) &&
> >> + ? ? ? ? ? ? ? len != group->num_mux * sizeof(unsigned int)) {
> >> + ? ? ? ? ? ? ? dev_err(info->dev, "wrong pin mux number?\n");
> >> + ? ? ? ? ? ? ? return -EINVAL;
> >> + ? ? ? }
> >> +
> >> + ? ? ? ret = of_property_read_u32_array(np, "grp_pins",
> >> + ? ? ? ? ? ? ? ? ? ? ? group->pins, group->num_pins);
> >> + ? ? ? if (ret) {
> >> + ? ? ? ? ? ? ? dev_err(info->dev, "failed to get grp_pins\n");
> >> + ? ? ? ? ? ? ? return ret;
> >> + ? ? ? }
> >> +
> >> + ? ? ? ret = of_property_read_u32_array(np, "grp_mux",
> >> + ? ? ? ? ? ? ? ? ? ? ? group->mux_mode, group->num_mux);
> >> + ? ? ? if (ret) {
> >> + ? ? ? ? ? ? ? dev_err(info->dev, "failed to get grp_mux\n");
> >> + ? ? ? ? ? ? ? return ret;
> >> + ? ? ? }
> >> +
> >> + ? ? ? /* Initialise function */
> >> + ? ? ? ret = of_property_read_string(np, "func_name",
> >> + &function->name);
> >> + ? ? ? if (ret) {
> >> + ? ? ? ? ? ? ? dev_err(info->dev, "failed to get func_name\n");
> >> + ? ? ? ? ? ? ? return ret;
> >> + ? ? ? }
> >> +
> >> + ? ? ? function->groups = devm_kzalloc(info->dev, sizeof(char **),
> >> + GFP_KERNEL);
> >> + ? ? ? function->num_groups = 1;
> >> + ? ? ? function->groups[0] = group->name;
> >> +
> >> + ? ? ? dev_dbg(info->dev, "func_name %s grp_name %s num_groups
> >> + %d\n",
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? function->name, function->groups[0],
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? function->num_groups);
> >> +
> >> + ? ? ? return 0;
> >> +}
> >> +
> >> +static int __devinit imx_pmx_probe_dt(struct platform_device *pdev,
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct imx_pinctrl_info *info) {
> >> + ? ? ? struct device_node *np = pdev->dev.of_node;
> >> + ? ? ? struct device_node *child = NULL;
> >> + ? ? ? int ret, i;
> >> + ? ? ? u32 nfuncs = 0;
> >> +
> >> + ? ? ? if (!np)
> >> + ? ? ? ? ? ? ? return -ENODEV;
> >> +
> >> + ? ? ? nfuncs = of_get_child_number(np);
> >> + ? ? ? if (nfuncs <= 0) {
> >> + ? ? ? ? ? ? ? dev_err(&pdev->dev, "no functions defined\n");
> >> + ? ? ? ? ? ? ? return -EINVAL;
> >> + ? ? ? }
> >> +
> >> + ? ? ? info->nfunctions = nfuncs;
> >> + ? ? ? info->functions = devm_kzalloc(&pdev->dev, nfuncs *
> >> + sizeof(struct imx_pmx_func),
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? GFP_KERNEL);
> >> + ? ? ? if (!info->functions)
> >> + ? ? ? ? ? ? ? return -ENOMEM;
> >> +
> >> + ? ? ? /* DT file only passes one group per one function */
> >> + ? ? ? info->ngroups = nfuncs;
> >> + ? ? ? info->groups = devm_kzalloc(&pdev->dev, nfuncs *
> >> + sizeof(struct imx_pin_group),
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? GFP_KERNEL);
> >> + ? ? ? if (!info->groups)
> >> + ? ? ? ? ? ? ? return -ENOMEM;
> >> +
> >> + ? ? ? child = NULL;
> >> + ? ? ? i = 0;
> >> + ? ? ? for_each_child_of_node(np, child) {
> >> + ? ? ? ? ? ? ? ret = imx_pmx_parse_functions(child, info, i++);
> >> + ? ? ? ? ? ? ? if (ret) {
> >> + ? ? ? ? ? ? ? ? ? ? ? dev_err(&pdev->dev, "failed to parse
> >> + function\n");
> >> + ? ? ? ? ? ? ? ? ? ? ? return ret;
> >> + ? ? ? ? ? ? ? }
> >> + ? ? ? }
> >> +
> >> + ? ? ? return 0;
> >> +}
> >> +#else
> >> +static int __devinit imx_pmx_probe_dt(struct platform_device *pdev,
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct imx_pinctrl_info *info) {
> >> + ? ? ? return -ENODEV;
> >> +}
> >> +#endif
> >
> > Looks clever to me! And I think we have agreed that this probing must
> > be inside each driver.
> >
> > Can we have a look at the corresponding device tree?
> 
> ... and binding documentation.  While on that topic, the convention is
> for property names to use dash '-' characters, not underscores '_'.
> The properties I see here are using the later.
> 
Yes, will follow the convention and add binding doc.
Thanks for your suggestion.

Regards
Dong Aisheng

[RFC PATCH v2 2/4] pinctrl: imx: add pinmux imx driver

[Dong Aisheng-B29396]

> -----Original Message-----
> From: linux-kernel-owner at vger.kernel.org [mailto:linux-kernel-
> owner at vger.kernel.org] On Behalf Of Linus Walleij
> Sent: Thursday, December 15, 2011 2:01 AM
> To: Dong Aisheng-B29396
> Cc: linux-kernel at vger.kernel.org; linux-arm-kernel at lists.infradead.org;
> linus.walleij at stericsson.com; s.hauer at pengutronix.de; Guo Shawn-R65073;
> kernel at pengutronix.de; grant.likely at secretlab.ca; rob.herring at calxeda.com
> Subject: Re: [RFC PATCH v2 2/4] pinctrl: imx: add pinmux imx driver
> Importance: High
> 
> On Wed, Dec 14, 2011 at 5:03 PM, Dong Aisheng <b29396@freescale.com>
> wrote:
> 
> > The driver contains the initial support for imx53 and imx6q.
> 
> Cool! Comments below...
> 
>
> > --- /dev/null
> > +++ b/drivers/pinctrl/pinmux-imx-core.c
> 
> We should start naming these pinctrl-* rather than pinmux-* so just
> rename it.
> 
> Until here it looks like business as usual..
> 
Sure, will rename in next version.

> > +#ifdef CONFIG_OF
> > +static int __devinit imx_pmx_parse_functions(struct device_node *np,
> > + ? ? ? ? ? ? ? ? ? ? ? struct imx_pinctrl_info *info, u32 num) {
> > + ? ? ? struct imx_pmx_func *function;
> > + ? ? ? struct imx_pin_group *group;
> > + ? ? ? int ret, len;
> > +
> > + ? ? ? dev_dbg(info->dev, "parse function %d\n", num);
> > +
> > + ? ? ? group = &info->groups[num];
> > + ? ? ? function = &info->functions[num];
> > +
> > + ? ? ? /* Initialise group */
> > + ? ? ? ret = of_property_read_string(np, "grp_name", &group->name);
> > + ? ? ? if (ret) {
> > + ? ? ? ? ? ? ? dev_err(info->dev, "failed to get grp_name\n");
> > + ? ? ? ? ? ? ? return ret;
> > + ? ? ? }
> > +
> > + ? ? ? ret = of_property_read_u32(np, "num_pins", &group->num_pins);
> > + ? ? ? if (ret) {
> > + ? ? ? ? ? ? ? dev_err(info->dev, "failed to get num_pins\n");
> > + ? ? ? ? ? ? ? return ret;
> > + ? ? ? }
> > +
> > + ? ? ? ret = of_property_read_u32(np, "num_mux", &group->num_mux);
> > + ? ? ? if (ret) {
> > + ? ? ? ? ? ? ? dev_err(info->dev, "failed to get num_mux\n");
> > + ? ? ? ? ? ? ? return ret;
> > + ? ? ? }
> > +
> > + ? ? ? if (group->num_pins != group->num_mux)
> > + ? ? ? ? ? ? ? return -EINVAL;
> > +
> > + ? ? ? group->pins = devm_kzalloc(info->dev, group->num_pins *
> > + sizeof(unsigned int),
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? GFP_KERNEL);
> > + ? ? ? group->mux_mode = devm_kzalloc(info->dev, group->num_mux *
> > + sizeof(unsigned int),
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? GFP_KERNEL);
> > + ? ? ? if (!group->pins || !group->mux_mode)
> > + ? ? ? ? ? ? ? return -ENOMEM;
> > +
> > + ? ? ? /* sanity check */
> > + ? ? ? if (of_get_property(np, "grp_pins", &len) &&
> > + ? ? ? ? ? ? ? len != group->num_pins * sizeof(unsigned int)) {
> > + ? ? ? ? ? ? ? dev_err(info->dev, "wrong pins number?\n");
> > + ? ? ? ? ? ? ? return -EINVAL;
> > + ? ? ? }
> > +
> > + ? ? ? if (of_get_property(np, "grp_mux", &len) &&
> > + ? ? ? ? ? ? ? len != group->num_mux * sizeof(unsigned int)) {
> > + ? ? ? ? ? ? ? dev_err(info->dev, "wrong pin mux number?\n");
> > + ? ? ? ? ? ? ? return -EINVAL;
> > + ? ? ? }
> > +
> > + ? ? ? ret = of_property_read_u32_array(np, "grp_pins",
> > + ? ? ? ? ? ? ? ? ? ? ? group->pins, group->num_pins);
> > + ? ? ? if (ret) {
> > + ? ? ? ? ? ? ? dev_err(info->dev, "failed to get grp_pins\n");
> > + ? ? ? ? ? ? ? return ret;
> > + ? ? ? }
> > +
> > + ? ? ? ret = of_property_read_u32_array(np, "grp_mux",
> > + ? ? ? ? ? ? ? ? ? ? ? group->mux_mode, group->num_mux);
> > + ? ? ? if (ret) {
> > + ? ? ? ? ? ? ? dev_err(info->dev, "failed to get grp_mux\n");
> > + ? ? ? ? ? ? ? return ret;
> > + ? ? ? }
> > +
> > + ? ? ? /* Initialise function */
> > + ? ? ? ret = of_property_read_string(np, "func_name",
> > + &function->name);
> > + ? ? ? if (ret) {
> > + ? ? ? ? ? ? ? dev_err(info->dev, "failed to get func_name\n");
> > + ? ? ? ? ? ? ? return ret;
> > + ? ? ? }
> > +
> > + ? ? ? function->groups = devm_kzalloc(info->dev, sizeof(char **),
> > + GFP_KERNEL);
> > + ? ? ? function->num_groups = 1;
> > + ? ? ? function->groups[0] = group->name;
> > +
> > + ? ? ? dev_dbg(info->dev, "func_name %s grp_name %s num_groups %d\n",
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? function->name, function->groups[0],
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? function->num_groups);
> > +
> > + ? ? ? return 0;
> > +}
> > +
> > +static int __devinit imx_pmx_probe_dt(struct platform_device *pdev,
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct imx_pinctrl_info *info) {
> > + ? ? ? struct device_node *np = pdev->dev.of_node;
> > + ? ? ? struct device_node *child = NULL;
> > + ? ? ? int ret, i;
> > + ? ? ? u32 nfuncs = 0;
> > +
> > + ? ? ? if (!np)
> > + ? ? ? ? ? ? ? return -ENODEV;
> > +
> > + ? ? ? nfuncs = of_get_child_number(np);
> > + ? ? ? if (nfuncs <= 0) {
> > + ? ? ? ? ? ? ? dev_err(&pdev->dev, "no functions defined\n");
> > + ? ? ? ? ? ? ? return -EINVAL;
> > + ? ? ? }
> > +
> > + ? ? ? info->nfunctions = nfuncs;
> > + ? ? ? info->functions = devm_kzalloc(&pdev->dev, nfuncs *
> > + sizeof(struct imx_pmx_func),
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? GFP_KERNEL);
> > + ? ? ? if (!info->functions)
> > + ? ? ? ? ? ? ? return -ENOMEM;
> > +
> > + ? ? ? /* DT file only passes one group per one function */
> > + ? ? ? info->ngroups = nfuncs;
> > + ? ? ? info->groups = devm_kzalloc(&pdev->dev, nfuncs * sizeof(struct
> > + imx_pin_group),
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? GFP_KERNEL);
> > + ? ? ? if (!info->groups)
> > + ? ? ? ? ? ? ? return -ENOMEM;
> > +
> > + ? ? ? child = NULL;
> > + ? ? ? i = 0;
> > + ? ? ? for_each_child_of_node(np, child) {
> > + ? ? ? ? ? ? ? ret = imx_pmx_parse_functions(child, info, i++);
> > + ? ? ? ? ? ? ? if (ret) {
> > + ? ? ? ? ? ? ? ? ? ? ? dev_err(&pdev->dev, "failed to parse
> > + function\n");
> > + ? ? ? ? ? ? ? ? ? ? ? return ret;
> > + ? ? ? ? ? ? ? }
> > + ? ? ? }
> > +
> > + ? ? ? return 0;
> > +}
> > +#else
> > +static int __devinit imx_pmx_probe_dt(struct platform_device *pdev,
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct imx_pinctrl_info *info) {
> > + ? ? ? return -ENODEV;
> > +}
> > +#endif
> 
> Looks clever to me! And I think we have agreed that this probing must be
> inside each driver.
> 
Yes.

> Can we have a look at the corresponding device tree?
> (...)
> 
> > +++ b/drivers/pinctrl/pinmux-imx-core.h
> 
> Name this pinctrl-* too for consistency.
> 
> (...)
> > +#define IMX_PINCTRL_PIN(pin) PINCTRL_PIN(pin, #pin)
> 
> Now I start to think about the range generators I discussed with
> Haojian... but there doesn't seem to be very many ranges in this driver
> so I guess it would only confuse things.
> 
Yes, I was once also thinking about it when I first looked at the pinctrl,
one concern is that we have to use the id for pin group defines,
So not sure if it's suitable to generate pin id dynamically.

> > +#define IMX_PIN_GROUP(n, p, m) ?\
> > + ? ? ? { ? ? ? ? ? ? ? ? ? ? ? \
> > + ? ? ? ? ? ? ? .name = n, ? ? ?\
> > + ? ? ? ? ? ? ? .pins = p, ? ? ?\
> > + ? ? ? ? ? ? ? .num_pins = ARRAY_SIZE(p), ? ? ?\
> > + ? ? ? ? ? ? ? .mux_mode = m, ?\
> > + ? ? ? ? ? ? ? .num_mux = ARRAY_SIZE(m), ? ? ? \
> > + ? ? ? }
> > +
> > +#define IMX_PMX_FUNC(n, g) ?\
> > + ? ? ? { ? ? ? ? ? ? ? ? ? ? ? \
> > + ? ? ? ? ? ? ? .name = n, ? ? ?\
> > + ? ? ? ? ? ? ? .groups = g, ? ?\
> > + ? ? ? ? ? ? ? .num_groups = ARRAY_SIZE(g), ? ?\
> > + ? ? ? }
> 
> This looks pretty smart!
> 
> > +++ b/drivers/pinctrl/pinmux-imx53.c
> 
> Name this pinctrl-*
> 
> > diff --git a/drivers/pinctrl/pinmux-imx6q.c
> > b/drivers/pinctrl/pinmux-imx6q.c
> 
> This too.
> 
> This patch is overall very good, we're quickly approaching merge quality.
>
Will fix them all.
Thanks for the review.

Regards
Dong Aisheng 

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

[RFC PATCH v2 2/4] pinctrl: imx: add pinmux imx driver

[Shawn Guo]

On Thu, Dec 15, 2011 at 12:03:40AM +0800, Dong Aisheng wrote:
> The driver contains the initial support for imx53 and
> imx6q.
> 
> Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Shawn Guo <shanw.guo@freescale.com>
> ---
>  drivers/pinctrl/Kconfig           |   20 ++
>  drivers/pinctrl/Makefile          |    3 +
>  drivers/pinctrl/pinmux-imx-core.c |  435 ++++++++++++++++++++++++++++++++++++
>  drivers/pinctrl/pinmux-imx-core.h |   86 +++++++
>  drivers/pinctrl/pinmux-imx53.c    |  443 +++++++++++++++++++++++++++++++++++++
>  drivers/pinctrl/pinmux-imx6q.c    |  433 ++++++++++++++++++++++++++++++++++++
>  6 files changed, 1420 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index e17e2f8..268c212 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -20,6 +20,26 @@ config DEBUG_PINCTRL
>  	help
>  	  Say Y here to add some extra checks and diagnostics to PINCTRL calls.
>  
> +config PINMUX_IMX
> +	bool "Freescale IMX core pinmux driver"
> +	depends on ARCH_MXC
> +
> +config PINMUX_IMX53
> +	bool "IMX53 pinmux driver"
> +	depends on ARCH_MX5
> +	select PINMUX
> +	select PINMUX_IMX
> +	help
> +	  Say Y here to enable the imx6q pinmux driver

s/imx6q/imx53

> +
> +config PINMUX_IMX6Q
> +	bool "IMX6Q pinmux driver"
> +	depends on SOC_IMX6Q
> +	select PINMUX
> +	select PINMUX_IMX
> +	help
> +	  Say Y here to enable the imx6q pinmux driver
> +

[...]

> +#ifdef CONFIG_OF
> +static int __devinit imx_pmx_parse_functions(struct device_node *np,
> +			struct imx_pinctrl_info *info, u32 num)
> +{
> +	struct imx_pmx_func *function;
> +	struct imx_pin_group *group;
> +	int ret, len;
> +
> +	dev_dbg(info->dev, "parse function %d\n", num);
> +
> +	group = &info->groups[num];
> +	function = &info->functions[num];
> +
> +	/* Initialise group */
> +	ret = of_property_read_string(np, "grp_name", &group->name);
> +	if (ret) {
> +		dev_err(info->dev, "failed to get grp_name\n");
> +		return ret;
> +	}
> +
> +	ret = of_property_read_u32(np, "num_pins", &group->num_pins);
> +	if (ret) {
> +		dev_err(info->dev, "failed to get num_pins\n");
> +		return ret;
> +	}
> +
> +	ret = of_property_read_u32(np, "num_mux", &group->num_mux);
> +	if (ret) {
> +		dev_err(info->dev, "failed to get num_mux\n");
> +		return ret;
> +	}
> +
> +	if (group->num_pins != group->num_mux)
> +		return -EINVAL;
> +
> +	group->pins = devm_kzalloc(info->dev, group->num_pins * sizeof(unsigned int),
> +				GFP_KERNEL);
> +	group->mux_mode = devm_kzalloc(info->dev, group->num_mux * sizeof(unsigned int),
> +				GFP_KERNEL);
> +	if (!group->pins || !group->mux_mode)
> +		return -ENOMEM;
> +
> +	/* sanity check */
> +	if (of_get_property(np, "grp_pins", &len) &&
> +		len != group->num_pins * sizeof(unsigned int)) {

Since we can figure out the 'num_pins' here, why do we bother to
encode it in dts?

> +		dev_err(info->dev, "wrong pins number?\n");
> +		return -EINVAL;
> +	}
> +
> +	if (of_get_property(np, "grp_mux", &len) &&
> +		len != group->num_mux * sizeof(unsigned int)) {

ditto

> +		dev_err(info->dev, "wrong pin mux number?\n");
> +		return -EINVAL;
> +	}

-- 
Regards,
Shawn

[RFC PATCH v2 2/4] pinctrl: imx: add pinmux imx driver

[Dong Aisheng-B29396]

> -----Original Message-----
> From: Guo Shawn-R65073
> Sent: Thursday, December 15, 2011 4:18 PM
> To: Dong Aisheng-B29396
> Cc: linux-kernel at vger.kernel.org; linux-arm-kernel at lists.infradead.org;
> linus.walleij at stericsson.com; s.hauer at pengutronix.de;
> kernel at pengutronix.de; grant.likely at secretlab.ca; rob.herring at calxeda.com
> Subject: Re: [RFC PATCH v2 2/4] pinctrl: imx: add pinmux imx driver
> Importance: High
> 
> On Thu, Dec 15, 2011 at 12:03:40AM +0800, Dong Aisheng wrote:
> > The driver contains the initial support for imx53 and imx6q.
> >
> > Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > Cc: Shawn Guo <shanw.guo@freescale.com>
> > ---
> >  drivers/pinctrl/Kconfig           |   20 ++
> >  drivers/pinctrl/Makefile          |    3 +
> >  drivers/pinctrl/pinmux-imx-core.c |  435
> ++++++++++++++++++++++++++++++++++++
> >  drivers/pinctrl/pinmux-imx-core.h |   86 +++++++
> >  drivers/pinctrl/pinmux-imx53.c    |  443
> +++++++++++++++++++++++++++++++++++++
> >  drivers/pinctrl/pinmux-imx6q.c    |  433
> ++++++++++++++++++++++++++++++++++++
> >  6 files changed, 1420 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig index
> > e17e2f8..268c212 100644
> > --- a/drivers/pinctrl/Kconfig
> > +++ b/drivers/pinctrl/Kconfig
> > @@ -20,6 +20,26 @@ config DEBUG_PINCTRL
> >  	help
> >  	  Say Y here to add some extra checks and diagnostics to PINCTRL
> calls.
> >
> > +config PINMUX_IMX
> > +	bool "Freescale IMX core pinmux driver"
> > +	depends on ARCH_MXC
> > +
> > +config PINMUX_IMX53
> > +	bool "IMX53 pinmux driver"
> > +	depends on ARCH_MX5
> > +	select PINMUX
> > +	select PINMUX_IMX
> > +	help
> > +	  Say Y here to enable the imx6q pinmux driver
> 
> s/imx6q/imx53
> 
Acked.

> > +
> > +config PINMUX_IMX6Q
> > +	bool "IMX6Q pinmux driver"
> > +	depends on SOC_IMX6Q
> > +	select PINMUX
> > +	select PINMUX_IMX
> > +	help
> > +	  Say Y here to enable the imx6q pinmux driver
> > +
> 
> [...]
> 
> > +#ifdef CONFIG_OF
> > +static int __devinit imx_pmx_parse_functions(struct device_node *np,
> > +			struct imx_pinctrl_info *info, u32 num) {
> > +	struct imx_pmx_func *function;
> > +	struct imx_pin_group *group;
> > +	int ret, len;
> > +
> > +	dev_dbg(info->dev, "parse function %d\n", num);
> > +
> > +	group = &info->groups[num];
> > +	function = &info->functions[num];
> > +
> > +	/* Initialise group */
> > +	ret = of_property_read_string(np, "grp_name", &group->name);
> > +	if (ret) {
> > +		dev_err(info->dev, "failed to get grp_name\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = of_property_read_u32(np, "num_pins", &group->num_pins);
> > +	if (ret) {
> > +		dev_err(info->dev, "failed to get num_pins\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = of_property_read_u32(np, "num_mux", &group->num_mux);
> > +	if (ret) {
> > +		dev_err(info->dev, "failed to get num_mux\n");
> > +		return ret;
> > +	}
> > +
> > +	if (group->num_pins != group->num_mux)
> > +		return -EINVAL;
> > +
> > +	group->pins = devm_kzalloc(info->dev, group->num_pins *
> sizeof(unsigned int),
> > +				GFP_KERNEL);
> > +	group->mux_mode = devm_kzalloc(info->dev, group->num_mux *
> sizeof(unsigned int),
> > +				GFP_KERNEL);
> > +	if (!group->pins || !group->mux_mode)
> > +		return -ENOMEM;
> > +
> > +	/* sanity check */
> > +	if (of_get_property(np, "grp_pins", &len) &&
> > +		len != group->num_pins * sizeof(unsigned int)) {
> 
> Since we can figure out the 'num_pins' here, why do we bother to encode
> it in dts?
> 
Since the pin group is easily to define wrongly by mistake when pins are many,
I would add num_pins in dts to double check in driver to improve the code
Stability.

> > +		dev_err(info->dev, "wrong pins number?\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (of_get_property(np, "grp_mux", &len) &&
> > +		len != group->num_mux * sizeof(unsigned int)) {
> 
> ditto
> 
> > +		dev_err(info->dev, "wrong pin mux number?\n");
> > +		return -EINVAL;
> > +	}
> 
> --
> Regards,
> Shawn

[RFC PATCH v2 3/4] ARM: imx6q: using pinmux subsystem

[Dong Aisheng]

From: Dong Aisheng <dong.aisheng@linaro.org>

Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Shawn Guo <shanw.guo@freescale.com>
---
 arch/arm/boot/dts/imx6q-sabreauto.dts |   22 ++++++++++++++++++++++
 arch/arm/boot/dts/imx6q.dtsi          |    1 +
 arch/arm/mach-imx/Kconfig             |    1 +
 arch/arm/mach-imx/mach-imx6q.c        |    8 +++++++-
 4 files changed, 31 insertions(+), 1 deletions(-)

diff --git a/arch/arm/boot/dts/imx6q-sabreauto.dts b/arch/arm/boot/dts/imx6q-sabreauto.dts
index 072974e..1bd1ef9 100644
--- a/arch/arm/boot/dts/imx6q-sabreauto.dts
+++ b/arch/arm/boot/dts/imx6q-sabreauto.dts
@@ -26,6 +26,28 @@
 	};
 
 	soc {
+		aips-bus at 02000000 { /* AIPS1 */
+			iomuxc at 020e0000 {
+				pinmux_uart4 {
+					func_name = "uart4";
+					grp_name = "uart4grp";
+					grp_pins = <107 108>;
+					num_pins = <2>;
+					grp_mux = <4 4>;
+					num_mux = <2>;
+				};
+
+				pinmux_sd4 {
+					func_name = "sd4";
+					grp_name = "sd4grp";
+					grp_pins = <170 171 180 181 182 183 184 185 186 187>;
+					num_pins = <10>;
+					grp_mux = <0 0 1 1 1 1 1 1 1 1>;
+					num_mux = <10>;
+				};
+			};
+		};
+
 		aips-bus at 02100000 { /* AIPS2 */
 			enet at 02188000 {
 				phy-mode = "rgmii";
diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index 7dda599..42499bb 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -386,6 +386,7 @@
 			};
 
 			iomuxc at 020e0000 {
+				compatible = "fsl,imx6q-iomuxc";
 				reg = <0x020e0000 0x4000>;
 			};
 
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index c44aa97..1e0befa 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -602,6 +602,7 @@ config SOC_IMX6Q
 	select HAVE_IMX_GPC
 	select HAVE_IMX_MMDC
 	select HAVE_IMX_SRC
+	select PINCTRL
 	select USE_OF
 
 	help
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index 9cd860a..e12ae63 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -16,6 +16,7 @@
 #include <linux/of.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
+#include <linux/pinctrl/machine.h>
 #include <asm/hardware/cache-l2x0.h>
 #include <asm/hardware/gic.h>
 #include <asm/mach/arch.h>
@@ -23,10 +24,15 @@
 #include <mach/common.h>
 #include <mach/hardware.h>
 
+static struct pinmux_map imx6q_pinmux_map[] = {
+	PINMUX_MAP_PRIMARY("usdhc4", "sd4", "219c000.usdhc"),
+};
+
 static void __init imx6q_init_machine(void)
 {
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
-
+	pinmux_register_mappings(imx6q_pinmux_map,
+				 ARRAY_SIZE(imx6q_pinmux_map));
 	imx6q_pm_init();
 }
 
-- 
1.7.0.4

[RFC PATCH v2 3/4] ARM: imx6q: using pinmux subsystem

[Linus Walleij]

On Wed, Dec 14, 2011 at 5:03 PM, Dong Aisheng <b29396@freescale.com> wrote:

> From: Dong Aisheng <dong.aisheng@linaro.org>
>
> Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Shawn Guo <shanw.guo@freescale.com>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

> +static struct pinmux_map imx6q_pinmux_map[] = {
> + ? ? ? PINMUX_MAP_PRIMARY("usdhc4", "sd4", "219c000.usdhc"),
> +};
> +
> ?static void __init imx6q_init_machine(void)
> ?{
> ? ? ? ?of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> -
> + ? ? ? pinmux_register_mappings(imx6q_pinmux_map,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?ARRAY_SIZE(imx6q_pinmux_map));
> ? ? ? ?imx6q_pm_init();
> ?}

So ideally this bit of code should also go into the device tree
and we should add a DT hook inside drivers/pinctrl/pinmux.c

Please make a patch for this if you have a clear idea on how
it should work!

Yours,
Linus Walleij

[RFC PATCH v2 3/4] ARM: imx6q: using pinmux subsystem

[Dong Aisheng-B29396]

> -----Original Message-----
> From: Linus Walleij [mailto:linus.walleij at linaro.org]
> Sent: Thursday, December 15, 2011 2:03 AM
> To: Dong Aisheng-B29396
> Cc: linux-kernel at vger.kernel.org; linux-arm-kernel at lists.infradead.org;
> linus.walleij at stericsson.com; s.hauer at pengutronix.de; Guo Shawn-R65073;
> kernel at pengutronix.de; grant.likely at secretlab.ca; rob.herring at calxeda.com
> Subject: Re: [RFC PATCH v2 3/4] ARM: imx6q: using pinmux subsystem
> Importance: High
> 
> On Wed, Dec 14, 2011 at 5:03 PM, Dong Aisheng <b29396@freescale.com>
> wrote:
> 
> > From: Dong Aisheng <dong.aisheng@linaro.org>
> >
> > Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > Cc: Shawn Guo <shanw.guo@freescale.com>
> 
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
> > +static struct pinmux_map imx6q_pinmux_map[] = {
> > + ? ? ? PINMUX_MAP_PRIMARY("usdhc4", "sd4", "219c000.usdhc"), };
> > +
> > ?static void __init imx6q_init_machine(void)
> > ?{
> > ? ? ? ?of_platform_populate(NULL, of_default_bus_match_table, NULL,
> > NULL);
> > -
> > + ? ? ? pinmux_register_mappings(imx6q_pinmux_map,
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?ARRAY_SIZE(imx6q_pinmux_map));
> > ? ? ? ?imx6q_pm_init();
> > ?}
> 
> So ideally this bit of code should also go into the device tree and we
> should add a DT hook inside drivers/pinctrl/pinmux.c
> 
> Please make a patch for this if you have a clear idea on how it should
> work!
> 
Yes, I will try to see that and will discuss with you once got an idea.

Regards
Dong Aisheng

[RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux subsystem

[Dong Aisheng]

From: Dong Aisheng <dong.aisheng@linaro.org>

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Chris Ball <cjb@laptop.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Wolfram Sang <w.sang@pengutronix.de>
---
 drivers/mmc/host/sdhci-esdhc-imx.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 4b976f0..4504136 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -24,6 +24,7 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/of_gpio.h>
+#include <linux/pinctrl/pinmux.h>
 #include <mach/esdhc.h>
 #include "sdhci-pltfm.h"
 #include "sdhci-esdhc.h"
@@ -68,6 +69,7 @@ struct pltfm_imx_data {
 	int flags;
 	u32 scratchpad;
 	enum imx_esdhc_type devtype;
+	struct pinmux *pmx;
 	struct esdhc_platform_data boarddata;
 };
 
@@ -439,6 +441,7 @@ static int __devinit sdhci_esdhc_imx_probe(struct platform_device *pdev)
 	struct clk *clk;
 	int err;
 	struct pltfm_imx_data *imx_data;
+	struct pinmux *pmx;
 
 	host = sdhci_pltfm_init(pdev, &sdhci_esdhc_imx_pdata);
 	if (IS_ERR(host))
@@ -466,6 +469,16 @@ static int __devinit sdhci_esdhc_imx_probe(struct platform_device *pdev)
 	clk_enable(clk);
 	pltfm_host->clk = clk;
 
+	pmx = pinmux_get(&pdev->dev, NULL);
+	if (IS_ERR(pmx)) {
+		err = PTR_ERR(pmx);
+		goto err_pmx_get;
+	}
+	err = pinmux_enable(pmx);
+	if (err)
+		goto err_pmx_enable;
+	imx_data->pmx = pmx;
+
 	if (!is_imx25_esdhc(imx_data))
 		host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
 
@@ -558,6 +571,10 @@ no_card_detect_irq:
 		gpio_free(boarddata->wp_gpio);
 no_card_detect_pin:
 no_board_data:
+	pinmux_disable(imx_data->pmx);
+err_pmx_enable:
+	pinmux_put(imx_data->pmx);
+err_pmx_get:
 	clk_disable(pltfm_host->clk);
 	clk_put(pltfm_host->clk);
 err_clk_get:
@@ -585,6 +602,9 @@ static int __devexit sdhci_esdhc_imx_remove(struct platform_device *pdev)
 		gpio_free(boarddata->cd_gpio);
 	}
 
+	pinmux_disable(imx_data->pmx);
+	pinmux_put(imx_data->pmx);
+
 	clk_disable(pltfm_host->clk);
 	clk_put(pltfm_host->clk);
 	kfree(imx_data);
-- 
1.7.0.4

[RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux subsystem

[Linus Walleij]

On Wed, Dec 14, 2011 at 5:03 PM, Dong Aisheng <b29396@freescale.com> wrote:

> From: Dong Aisheng <dong.aisheng@linaro.org>
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Chris Ball <cjb@laptop.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Wolfram Sang <w.sang@pengutronix.de>

>From a pinctrl point of view everything is fine so
Acked-by: Linus Walleij <linus.walleij@linaro.org>

...but I know Sascha may still have issues with this patch.

Thanks,
Linus Walleij

[RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux subsystem

[Sascha Hauer]

On Thu, Dec 15, 2011 at 12:03:42AM +0800, Dong Aisheng wrote:
> From: Dong Aisheng <dong.aisheng@linaro.org>
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Chris Ball <cjb@laptop.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Wolfram Sang <w.sang@pengutronix.de>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c |   20 ++++++++++++++++++++
>  1 files changed, 20 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 4b976f0..4504136 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -24,6 +24,7 @@
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/of_gpio.h>
> +#include <linux/pinctrl/pinmux.h>
>  #include <mach/esdhc.h>
>  #include "sdhci-pltfm.h"
>  #include "sdhci-esdhc.h"
> @@ -68,6 +69,7 @@ struct pltfm_imx_data {
>  	int flags;
>  	u32 scratchpad;
>  	enum imx_esdhc_type devtype;
> +	struct pinmux *pmx;
>  	struct esdhc_platform_data boarddata;
>  };
>  
> @@ -439,6 +441,7 @@ static int __devinit sdhci_esdhc_imx_probe(struct platform_device *pdev)
>  	struct clk *clk;
>  	int err;
>  	struct pltfm_imx_data *imx_data;
> +	struct pinmux *pmx;
>  
>  	host = sdhci_pltfm_init(pdev, &sdhci_esdhc_imx_pdata);
>  	if (IS_ERR(host))
> @@ -466,6 +469,16 @@ static int __devinit sdhci_esdhc_imx_probe(struct platform_device *pdev)
>  	clk_enable(clk);
>  	pltfm_host->clk = clk;
>  
> +	pmx = pinmux_get(&pdev->dev, NULL);
> +	if (IS_ERR(pmx)) {
> +		err = PTR_ERR(pmx);
> +		goto err_pmx_get;
> +	}
> +	err = pinmux_enable(pmx);
> +	if (err)
> +		goto err_pmx_enable;
> +	imx_data->pmx = pmx;
> +

Won't this break every i.MX except i.MX6?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux subsystem

[Linus Walleij]

On Wed, Dec 14, 2011 at 10:11 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Thu, Dec 15, 2011 at 12:03:42AM +0800, Dong Aisheng wrote:

>> + ? ? pmx = pinmux_get(&pdev->dev, NULL);
>> + ? ? if (IS_ERR(pmx)) {
>> + ? ? ? ? ? ? err = PTR_ERR(pmx);
>> + ? ? ? ? ? ? goto err_pmx_get;
>> + ? ? }
>> + ? ? err = pinmux_enable(pmx);
>> + ? ? if (err)
>> + ? ? ? ? ? ? goto err_pmx_enable;
>> + ? ? imx_data->pmx = pmx;
>> +
>
> Won't this break every i.MX except i.MX6?

Not this patch on its own, first the machines have to
select PINCTRL
select PINMUX_FOO

*Then* it will break :-D

So if you want to do this for i.MX you need something like
selectable dummy pinmuxes, i.e. pinmux_get() to return something
that just say "OK" to everything like the dummy regulators.

Shall I try to create something like that?

Yours,
Linus Walleij

[RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux subsystem

[Shawn Guo]

On Wed, Dec 14, 2011 at 11:15:53PM +0100, Linus Walleij wrote:
> On Wed, Dec 14, 2011 at 10:11 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > On Thu, Dec 15, 2011 at 12:03:42AM +0800, Dong Aisheng wrote:
> 
> >> + ? ? pmx = pinmux_get(&pdev->dev, NULL);
> >> + ? ? if (IS_ERR(pmx)) {
> >> + ? ? ? ? ? ? err = PTR_ERR(pmx);
> >> + ? ? ? ? ? ? goto err_pmx_get;
> >> + ? ? }
> >> + ? ? err = pinmux_enable(pmx);
> >> + ? ? if (err)
> >> + ? ? ? ? ? ? goto err_pmx_enable;
> >> + ? ? imx_data->pmx = pmx;
> >> +
> >
> > Won't this break every i.MX except i.MX6?
> 
It will not if we do not compile other imx together with imx6, because
there are a set of empty functions defined in
include/linux/pinctrl/pinmux.h for those builds without pinctrl enabled.
But yes, it's actually broken since we are on the way to compile imx3,
imx5 and imx6 as single image.

> Not this patch on its own, first the machines have to
> select PINCTRL
> select PINMUX_FOO
> 
> *Then* it will break :-D
> 
> So if you want to do this for i.MX you need something like
> selectable dummy pinmuxes, i.e. pinmux_get() to return something
> that just say "OK" to everything like the dummy regulators.
> 
> Shall I try to create something like that?
> 
Isn't the empty functions defined in include/linux/pinctrl/pinmux.h
for this purpose?  It does not solve the problem with single image.
You might probably mean that we create a dummy_pinctrl_desc and register
it to pinctrl core with pinctrl_register() if we detect that the kernel
is running on a soc that has no pinctrl support?

This is not a problem to pinctrl migration only.  We have the same
problem with common clk migration.  Unless we migrate imx3, imx5 and
imx6 to common clk at the same time, single image build just does not
cope with clk_* api.

-- 
Regards,
Shawn

[RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux subsystem

[Linus Walleij]

On Thu, Dec 15, 2011 at 8:05 AM, Shawn Guo <shawn.guo@freescale.com> wrote:
>[Me]
>> So if you want to do this for i.MX you need something like
>> selectable dummy pinmuxes, i.e. pinmux_get() to return something
>> that just say "OK" to everything like the dummy regulators.
>>
>> Shall I try to create something like that?
>>
> Isn't the empty functions defined in include/linux/pinctrl/pinmux.h
> for this purpose?

No, these are for compiling it *out*, dummy pinmuxes would
be if you compile it *in*, but don't find an apropriate pinmux,
you still get something that does nothing and still works.

Dummy regulators work exactly this way.

> It does not solve the problem with single image.

I think it does.

> You might probably mean that we create a dummy_pinctrl_desc and register
> it to pinctrl core with pinctrl_register() if we detect that the kernel
> is running on a soc that has no pinctrl support?

No. You have a #ifdef CONFIG_PINMUX_DUMMY in pinmux_get()
that makes sure you return a working no-op pinmux handle even
though there is no real pinmux behind it.

> This is not a problem to pinctrl migration only. ?We have the same
> problem with common clk migration. ?Unless we migrate imx3, imx5 and
> imx6 to common clk at the same time, single image build just does not
> cope with clk_* api.

And  you could have the same problem with regulator migration...
Luckily dummy regulators saves you in that case.

Thanks,
Linus Walleij

[RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux subsystem

[Dong Aisheng-B29396]

> -----Original Message-----
> From: Linus Walleij [mailto:linus.walleij at linaro.org]
> Sent: Thursday, December 15, 2011 4:27 PM
> To: Guo Shawn-R65073
> Cc: Sascha Hauer; Dong Aisheng-B29396; linus.walleij at stericsson.com;
> linux-kernel at vger.kernel.org; rob.herring at calxeda.com;
> grant.likely at secretlab.ca; linux-arm-kernel at lists.infradead.org;
> kernel at pengutronix.de
> Subject: Re: [RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux
> subsystem
> Importance: High
> 
> On Thu, Dec 15, 2011 at 8:05 AM, Shawn Guo <shawn.guo@freescale.com>
> wrote:
> >[Me]
> >> So if you want to do this for i.MX you need something like selectable
> >> dummy pinmuxes, i.e. pinmux_get() to return something that just say
> >> "OK" to everything like the dummy regulators.
> >>
> >> Shall I try to create something like that?
> >>
> > Isn't the empty functions defined in include/linux/pinctrl/pinmux.h
> > for this purpose?
> 
> No, these are for compiling it *out*, dummy pinmuxes would be if you
> compile it *in*, but don't find an apropriate pinmux, you still get
> something that does nothing and still works.
> 
> Dummy regulators work exactly this way.
> 

I did not read the dummy regulator code too much.
But does it mean that the dummy regulator or dummy pinmux will also hide the
Real errors since it will always get a available one?
How do we distinguish between the two case(real error and fake error)?

> > It does not solve the problem with single image.
> 
> I think it does.
> 
> > You might probably mean that we create a dummy_pinctrl_desc and
> > register it to pinctrl core with pinctrl_register() if we detect that
> > the kernel is running on a soc that has no pinctrl support?
> 
> No. You have a #ifdef CONFIG_PINMUX_DUMMY in pinmux_get() that makes sure
> you return a working no-op pinmux handle even though there is no real
> pinmux behind it.
> 
> > This is not a problem to pinctrl migration only. ?We have the same
> > problem with common clk migration. ?Unless we migrate imx3, imx5 and
> > imx6 to common clk at the same time, single image build just does not
> > cope with clk_* api.
> 
> And  you could have the same problem with regulator migration...
> Luckily dummy regulators saves you in that case.
> 
> Thanks,
> Linus Walleij

[RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux subsystem

[Sascha Hauer]

On Thu, Dec 15, 2011 at 08:59:28AM +0000, Dong Aisheng-B29396 wrote:
> > -----Original Message-----
> > From: Linus Walleij [mailto:linus.walleij at linaro.org]
> > Sent: Thursday, December 15, 2011 4:27 PM
> > To: Guo Shawn-R65073
> > Cc: Sascha Hauer; Dong Aisheng-B29396; linus.walleij at stericsson.com;
> > linux-kernel at vger.kernel.org; rob.herring at calxeda.com;
> > grant.likely at secretlab.ca; linux-arm-kernel at lists.infradead.org;
> > kernel at pengutronix.de
> > Subject: Re: [RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux
> > subsystem
> > Importance: High
> > 
> > On Thu, Dec 15, 2011 at 8:05 AM, Shawn Guo <shawn.guo@freescale.com>
> > wrote:
> > >[Me]
> > >> So if you want to do this for i.MX you need something like selectable
> > >> dummy pinmuxes, i.e. pinmux_get() to return something that just say
> > >> "OK" to everything like the dummy regulators.
> > >>
> > >> Shall I try to create something like that?
> > >>
> > > Isn't the empty functions defined in include/linux/pinctrl/pinmux.h
> > > for this purpose?
> > 
> > No, these are for compiling it *out*, dummy pinmuxes would be if you
> > compile it *in*, but don't find an apropriate pinmux, you still get
> > something that does nothing and still works.
> > 
> > Dummy regulators work exactly this way.
> > 
> 
> I did not read the dummy regulator code too much.
> But does it mean that the dummy regulator or dummy pinmux will also hide the
> Real errors since it will always get a available one?
> How do we distinguish between the two case(real error and fake error)?

We don't :(

That's the problem with the dummy regulator.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux subsystem

[Shawn Guo]

On Thu, Dec 15, 2011 at 10:33:19AM +0100, Sascha Hauer wrote:
> On Thu, Dec 15, 2011 at 08:59:28AM +0000, Dong Aisheng-B29396 wrote:
> > > -----Original Message-----
> > > From: Linus Walleij [mailto:linus.walleij at linaro.org]
> > > Sent: Thursday, December 15, 2011 4:27 PM
> > > To: Guo Shawn-R65073
> > > Cc: Sascha Hauer; Dong Aisheng-B29396; linus.walleij at stericsson.com;
> > > linux-kernel at vger.kernel.org; rob.herring at calxeda.com;
> > > grant.likely at secretlab.ca; linux-arm-kernel at lists.infradead.org;
> > > kernel at pengutronix.de
> > > Subject: Re: [RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux
> > > subsystem
> > > Importance: High
> > > 
> > > On Thu, Dec 15, 2011 at 8:05 AM, Shawn Guo <shawn.guo@freescale.com>
> > > wrote:
> > > >[Me]
> > > >> So if you want to do this for i.MX you need something like selectable
> > > >> dummy pinmuxes, i.e. pinmux_get() to return something that just say
> > > >> "OK" to everything like the dummy regulators.
> > > >>
> > > >> Shall I try to create something like that?
> > > >>
> > > > Isn't the empty functions defined in include/linux/pinctrl/pinmux.h
> > > > for this purpose?
> > > 
> > > No, these are for compiling it *out*, dummy pinmuxes would be if you
> > > compile it *in*, but don't find an apropriate pinmux, you still get
> > > something that does nothing and still works.
> > > 
> > > Dummy regulators work exactly this way.
> > > 
> > 
> > I did not read the dummy regulator code too much.
> > But does it mean that the dummy regulator or dummy pinmux will also hide the
> > Real errors since it will always get a available one?

What do you mean 'real error'?  When driver calls pimnux api on a
platform with real pinmux support, the error is error.  When driver
calls pinmux api on a platform support with dummy pinmux, it's totally
error free, as the pinmux core will ensure all the pinmux_* calls
always return success.

> > How do we distinguish between the two case(real error and fake error)?
> 
What do we need to distinguish between two cases?  The real success for
real pinmux from the fake success for the dummy pinmux?  It does not
really matter.

> We don't :(
> 
> That's the problem with the dummy regulator.
> 
What is the problem exactly?  I do not quite understand.

-- 
Regards,
Shawn

[RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux subsystem

[Dong Aisheng-B29396]

> -----Original Message-----
> From: Guo Shawn-R65073
> Sent: Thursday, December 15, 2011 7:22 PM
> To: Sascha Hauer
> Cc: Dong Aisheng-B29396; Linus Walleij; Guo Shawn-R65073;
> linus.walleij at stericsson.com; linux-kernel at vger.kernel.org;
> rob.herring at calxeda.com; grant.likely at secretlab.ca; linux-arm-
> kernel at lists.infradead.org; kernel at pengutronix.de
> Subject: Re: [RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux
> subsystem
> Importance: High
> 
> On Thu, Dec 15, 2011 at 10:33:19AM +0100, Sascha Hauer wrote:
> > On Thu, Dec 15, 2011 at 08:59:28AM +0000, Dong Aisheng-B29396 wrote:
> > > > -----Original Message-----
> > > > From: Linus Walleij [mailto:linus.walleij at linaro.org]
> > > > Sent: Thursday, December 15, 2011 4:27 PM
> > > > To: Guo Shawn-R65073
> > > > Cc: Sascha Hauer; Dong Aisheng-B29396;
> > > > linus.walleij at stericsson.com; linux-kernel at vger.kernel.org;
> > > > rob.herring at calxeda.com; grant.likely at secretlab.ca;
> > > > linux-arm-kernel at lists.infradead.org;
> > > > kernel at pengutronix.de
> > > > Subject: Re: [RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux
> > > > subsystem
> > > > Importance: High
> > > >
> > > > On Thu, Dec 15, 2011 at 8:05 AM, Shawn Guo
> > > > <shawn.guo@freescale.com>
> > > > wrote:
> > > > >[Me]
> > > > >> So if you want to do this for i.MX you need something like
> > > > >> selectable dummy pinmuxes, i.e. pinmux_get() to return
> > > > >> something that just say "OK" to everything like the dummy
> regulators.
> > > > >>
> > > > >> Shall I try to create something like that?
> > > > >>
> > > > > Isn't the empty functions defined in
> > > > > include/linux/pinctrl/pinmux.h for this purpose?
> > > >
> > > > No, these are for compiling it *out*, dummy pinmuxes would be if
> > > > you compile it *in*, but don't find an apropriate pinmux, you
> > > > still get something that does nothing and still works.
> > > >
> > > > Dummy regulators work exactly this way.
> > > >
> > >
> > > I did not read the dummy regulator code too much.
> > > But does it mean that the dummy regulator or dummy pinmux will also
> > > hide the Real errors since it will always get a available one?
> 
> What do you mean 'real error'?  When driver calls pimnux api on a
> platform with real pinmux support, the error is error.  When driver calls
> pinmux api on a platform support with dummy pinmux, it's totally error
> free, as the pinmux core will ensure all the pinmux_* calls always return
> success.
> 
My understanding is that pinmux_get will return an error if no proper pinmux
Found without dummy pinmux. That's a real error.
But with dummy pinmux, if no proper pinmux found, the pinctrl core may check
If dummy pinmux is supported, if supported, it will fakely success with returning
a dummy pinmux. Then real error is hiden.
This is due to for supporting one single image, the dummy pinmux may also be enabled
For platforms like mx6q with real pinmux.

I just did a quick look at the regulator code and got this understanding,
please let me know if I understood wrong.

> > > How do we distinguish between the two case(real error and fake error)?
> >
> What do we need to distinguish between two cases?  The real success for
> real pinmux from the fake success for the dummy pinmux?  It does not
> really matter.
> 
> > We don't :(
> >
> > That's the problem with the dummy regulator.
> >
> What is the problem exactly?  I do not quite understand.
> 
> --
> Regards,
> Shawn

[RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux subsystem

[Shawn Guo]

On Thu, Dec 15, 2011 at 07:28:18PM +0800, Dong Aisheng-B29396 wrote:
> > -----Original Message-----
> > From: Guo Shawn-R65073
> > Sent: Thursday, December 15, 2011 7:22 PM
> > To: Sascha Hauer
> > Cc: Dong Aisheng-B29396; Linus Walleij; Guo Shawn-R65073;
> > linus.walleij at stericsson.com; linux-kernel at vger.kernel.org;
> > rob.herring at calxeda.com; grant.likely at secretlab.ca; linux-arm-
> > kernel at lists.infradead.org; kernel at pengutronix.de
> > Subject: Re: [RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux
> > subsystem
> > Importance: High
> > 
> > On Thu, Dec 15, 2011 at 10:33:19AM +0100, Sascha Hauer wrote:
> > > On Thu, Dec 15, 2011 at 08:59:28AM +0000, Dong Aisheng-B29396 wrote:
> > > > > -----Original Message-----
> > > > > From: Linus Walleij [mailto:linus.walleij at linaro.org]
> > > > > Sent: Thursday, December 15, 2011 4:27 PM
> > > > > To: Guo Shawn-R65073
> > > > > Cc: Sascha Hauer; Dong Aisheng-B29396;
> > > > > linus.walleij at stericsson.com; linux-kernel at vger.kernel.org;
> > > > > rob.herring at calxeda.com; grant.likely at secretlab.ca;
> > > > > linux-arm-kernel at lists.infradead.org;
> > > > > kernel at pengutronix.de
> > > > > Subject: Re: [RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux
> > > > > subsystem
> > > > > Importance: High
> > > > >
> > > > > On Thu, Dec 15, 2011 at 8:05 AM, Shawn Guo
> > > > > <shawn.guo@freescale.com>
> > > > > wrote:
> > > > > >[Me]
> > > > > >> So if you want to do this for i.MX you need something like
> > > > > >> selectable dummy pinmuxes, i.e. pinmux_get() to return
> > > > > >> something that just say "OK" to everything like the dummy
> > regulators.
> > > > > >>
> > > > > >> Shall I try to create something like that?
> > > > > >>
> > > > > > Isn't the empty functions defined in
> > > > > > include/linux/pinctrl/pinmux.h for this purpose?
> > > > >
> > > > > No, these are for compiling it *out*, dummy pinmuxes would be if
> > > > > you compile it *in*, but don't find an apropriate pinmux, you
> > > > > still get something that does nothing and still works.
> > > > >
> > > > > Dummy regulators work exactly this way.
> > > > >
> > > >
> > > > I did not read the dummy regulator code too much.
> > > > But does it mean that the dummy regulator or dummy pinmux will also
> > > > hide the Real errors since it will always get a available one?
> > 
> > What do you mean 'real error'?  When driver calls pimnux api on a
> > platform with real pinmux support, the error is error.  When driver calls
> > pinmux api on a platform support with dummy pinmux, it's totally error
> > free, as the pinmux core will ensure all the pinmux_* calls always return
> > success.
> > 
> My understanding is that pinmux_get will return an error if no proper pinmux
> Found without dummy pinmux. That's a real error.
> But with dummy pinmux, if no proper pinmux found, the pinctrl core may check
> If dummy pinmux is supported, if supported, it will fakely success with returning
> a dummy pinmux. Then real error is hiden.
> This is due to for supporting one single image, the dummy pinmux may also be enabled
> For platforms like mx6q with real pinmux.
> 
Got your point.  So your concern is about the fake success when dummy
pinmux comes to play rather than 'fake error'.  I'm not concerned about
that case much, since we will know the failure with a simple debug/info
message in the pinmux core, saying 'I do not find any available pinmux,
and I'm falling into the dummy pinmux'.  When you see this message
imx6q, you know something goes wrong.

-- 
Regards,
Shawn

[RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux subsystem

[Sascha Hauer]

On Thu, Dec 15, 2011 at 07:53:05PM +0800, Shawn Guo wrote:
> On Thu, Dec 15, 2011 at 07:28:18PM +0800, Dong Aisheng-B29396 wrote:
> > > -----Original Message-----
> > > From: Guo Shawn-R65073
> > > Sent: Thursday, December 15, 2011 7:22 PM
> > > To: Sascha Hauer
> > > Cc: Dong Aisheng-B29396; Linus Walleij; Guo Shawn-R65073;
> > > linus.walleij at stericsson.com; linux-kernel at vger.kernel.org;
> > > rob.herring at calxeda.com; grant.likely at secretlab.ca; linux-arm-
> > > kernel at lists.infradead.org; kernel at pengutronix.de
> > > Subject: Re: [RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux
> > > subsystem
> > > Importance: High
> > > 
> > > On Thu, Dec 15, 2011 at 10:33:19AM +0100, Sascha Hauer wrote:
> > > > On Thu, Dec 15, 2011 at 08:59:28AM +0000, Dong Aisheng-B29396 wrote:
> > > > > > -----Original Message-----
> > > > > > From: Linus Walleij [mailto:linus.walleij at linaro.org]
> > > > > > Sent: Thursday, December 15, 2011 4:27 PM
> > > > > > To: Guo Shawn-R65073
> > > > > > Cc: Sascha Hauer; Dong Aisheng-B29396;
> > > > > > linus.walleij at stericsson.com; linux-kernel at vger.kernel.org;
> > > > > > rob.herring at calxeda.com; grant.likely at secretlab.ca;
> > > > > > linux-arm-kernel at lists.infradead.org;
> > > > > > kernel at pengutronix.de
> > > > > > Subject: Re: [RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux
> > > > > > subsystem
> > > > > > Importance: High
> > > > > >
> > > > > > On Thu, Dec 15, 2011 at 8:05 AM, Shawn Guo
> > > > > > <shawn.guo@freescale.com>
> > > > > > wrote:
> > > > > > >[Me]
> > > > > > >> So if you want to do this for i.MX you need something like
> > > > > > >> selectable dummy pinmuxes, i.e. pinmux_get() to return
> > > > > > >> something that just say "OK" to everything like the dummy
> > > regulators.
> > > > > > >>
> > > > > > >> Shall I try to create something like that?
> > > > > > >>
> > > > > > > Isn't the empty functions defined in
> > > > > > > include/linux/pinctrl/pinmux.h for this purpose?
> > > > > >
> > > > > > No, these are for compiling it *out*, dummy pinmuxes would be if
> > > > > > you compile it *in*, but don't find an apropriate pinmux, you
> > > > > > still get something that does nothing and still works.
> > > > > >
> > > > > > Dummy regulators work exactly this way.
> > > > > >
> > > > >
> > > > > I did not read the dummy regulator code too much.
> > > > > But does it mean that the dummy regulator or dummy pinmux will also
> > > > > hide the Real errors since it will always get a available one?
> > > 
> > > What do you mean 'real error'?  When driver calls pimnux api on a
> > > platform with real pinmux support, the error is error.  When driver calls
> > > pinmux api on a platform support with dummy pinmux, it's totally error
> > > free, as the pinmux core will ensure all the pinmux_* calls always return
> > > success.
> > > 
> > My understanding is that pinmux_get will return an error if no proper pinmux
> > Found without dummy pinmux. That's a real error.
> > But with dummy pinmux, if no proper pinmux found, the pinctrl core may check
> > If dummy pinmux is supported, if supported, it will fakely success with returning
> > a dummy pinmux. Then real error is hiden.
> > This is due to for supporting one single image, the dummy pinmux may also be enabled
> > For platforms like mx6q with real pinmux.
> > 
> Got your point.  So your concern is about the fake success when dummy
> pinmux comes to play rather than 'fake error'.  I'm not concerned about
> that case much, since we will know the failure with a simple debug/info
> message in the pinmux core, saying 'I do not find any available pinmux,
> and I'm falling into the dummy pinmux'.  When you see this message
> imx6q, you know something goes wrong.

So i.MX3/5 people must know that it's safe to ignore this message
whereas i.MX6 people must know there's something wrong Adding messages
saying "there might or might not be something wrong" is not good.
People will frequently ask on the mailing list about these messages.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux subsystem

[Shawn Guo]

On Thu, Dec 15, 2011 at 12:54:28PM +0100, Sascha Hauer wrote:
> On Thu, Dec 15, 2011 at 07:53:05PM +0800, Shawn Guo wrote:
> > On Thu, Dec 15, 2011 at 07:28:18PM +0800, Dong Aisheng-B29396 wrote:
> > > > -----Original Message-----
> > > > From: Guo Shawn-R65073
> > > > Sent: Thursday, December 15, 2011 7:22 PM
> > > > To: Sascha Hauer
> > > > Cc: Dong Aisheng-B29396; Linus Walleij; Guo Shawn-R65073;
> > > > linus.walleij at stericsson.com; linux-kernel at vger.kernel.org;
> > > > rob.herring at calxeda.com; grant.likely at secretlab.ca; linux-arm-
> > > > kernel at lists.infradead.org; kernel at pengutronix.de
> > > > Subject: Re: [RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux
> > > > subsystem
> > > > Importance: High
> > > > 
> > > > On Thu, Dec 15, 2011 at 10:33:19AM +0100, Sascha Hauer wrote:
> > > > > On Thu, Dec 15, 2011 at 08:59:28AM +0000, Dong Aisheng-B29396 wrote:
> > > > > > > -----Original Message-----
> > > > > > > From: Linus Walleij [mailto:linus.walleij at linaro.org]
> > > > > > > Sent: Thursday, December 15, 2011 4:27 PM
> > > > > > > To: Guo Shawn-R65073
> > > > > > > Cc: Sascha Hauer; Dong Aisheng-B29396;
> > > > > > > linus.walleij at stericsson.com; linux-kernel at vger.kernel.org;
> > > > > > > rob.herring at calxeda.com; grant.likely at secretlab.ca;
> > > > > > > linux-arm-kernel at lists.infradead.org;
> > > > > > > kernel at pengutronix.de
> > > > > > > Subject: Re: [RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux
> > > > > > > subsystem
> > > > > > > Importance: High
> > > > > > >
> > > > > > > On Thu, Dec 15, 2011 at 8:05 AM, Shawn Guo
> > > > > > > <shawn.guo@freescale.com>
> > > > > > > wrote:
> > > > > > > >[Me]
> > > > > > > >> So if you want to do this for i.MX you need something like
> > > > > > > >> selectable dummy pinmuxes, i.e. pinmux_get() to return
> > > > > > > >> something that just say "OK" to everything like the dummy
> > > > regulators.
> > > > > > > >>
> > > > > > > >> Shall I try to create something like that?
> > > > > > > >>
> > > > > > > > Isn't the empty functions defined in
> > > > > > > > include/linux/pinctrl/pinmux.h for this purpose?
> > > > > > >
> > > > > > > No, these are for compiling it *out*, dummy pinmuxes would be if
> > > > > > > you compile it *in*, but don't find an apropriate pinmux, you
> > > > > > > still get something that does nothing and still works.
> > > > > > >
> > > > > > > Dummy regulators work exactly this way.
> > > > > > >
> > > > > >
> > > > > > I did not read the dummy regulator code too much.
> > > > > > But does it mean that the dummy regulator or dummy pinmux will also
> > > > > > hide the Real errors since it will always get a available one?
> > > > 
> > > > What do you mean 'real error'?  When driver calls pimnux api on a
> > > > platform with real pinmux support, the error is error.  When driver calls
> > > > pinmux api on a platform support with dummy pinmux, it's totally error
> > > > free, as the pinmux core will ensure all the pinmux_* calls always return
> > > > success.
> > > > 
> > > My understanding is that pinmux_get will return an error if no proper pinmux
> > > Found without dummy pinmux. That's a real error.
> > > But with dummy pinmux, if no proper pinmux found, the pinctrl core may check
> > > If dummy pinmux is supported, if supported, it will fakely success with returning
> > > a dummy pinmux. Then real error is hiden.
> > > This is due to for supporting one single image, the dummy pinmux may also be enabled
> > > For platforms like mx6q with real pinmux.
> > > 
> > Got your point.  So your concern is about the fake success when dummy
> > pinmux comes to play rather than 'fake error'.  I'm not concerned about
> > that case much, since we will know the failure with a simple debug/info
> > message in the pinmux core, saying 'I do not find any available pinmux,
> > and I'm falling into the dummy pinmux'.  When you see this message
> > imx6q, you know something goes wrong.
> 
> So i.MX3/5 people must know that it's safe to ignore this message
> whereas i.MX6 people must know there's something wrong Adding messages
> saying "there might or might not be something wrong" is not good.
> People will frequently ask on the mailing list about these messages.
> 
Usually, people checking the serial output and asking question on list
are developers not users.  It's not too hard for developers to figure
out such message.  Or we can making it a debug message, and when some
driver is not working, turn on debug option, you know where it goes
wrong.

Anyway, it's not a problem to me.  I have seen similar messages on my
kernel serial output.  I fail to see why pinmux can not do something
that other subsystems are doing.

-- 
Regards,
Shawn

[RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux subsystem

[Sascha Hauer]

On Thu, Dec 15, 2011 at 08:17:54PM +0800, Shawn Guo wrote:
> On Thu, Dec 15, 2011 at 12:54:28PM +0100, Sascha Hauer wrote:
> > On Thu, Dec 15, 2011 at 07:53:05PM +0800, Shawn Guo wrote:
> > > On Thu, Dec 15, 2011 at 07:28:18PM +0800, Dong Aisheng-B29396 wrote:
> > > > > -----Original Message-----
> > > > > 
> > > > My understanding is that pinmux_get will return an error if no proper pinmux
> > > > Found without dummy pinmux. That's a real error.
> > > > But with dummy pinmux, if no proper pinmux found, the pinctrl core may check
> > > > If dummy pinmux is supported, if supported, it will fakely success with returning
> > > > a dummy pinmux. Then real error is hiden.
> > > > This is due to for supporting one single image, the dummy pinmux may also be enabled
> > > > For platforms like mx6q with real pinmux.
> > > > 
> > > Got your point.  So your concern is about the fake success when dummy
> > > pinmux comes to play rather than 'fake error'.  I'm not concerned about
> > > that case much, since we will know the failure with a simple debug/info
> > > message in the pinmux core, saying 'I do not find any available pinmux,
> > > and I'm falling into the dummy pinmux'.  When you see this message
> > > imx6q, you know something goes wrong.
> > 
> > So i.MX3/5 people must know that it's safe to ignore this message
> > whereas i.MX6 people must know there's something wrong Adding messages
> > saying "there might or might not be something wrong" is not good.
> > People will frequently ask on the mailing list about these messages.
> > 
> Usually, people checking the serial output and asking question on list
> are developers not users.  It's not too hard for developers to figure
> out such message.  Or we can making it a debug message, and when some
> driver is not working, turn on debug option, you know where it goes
> wrong.

So we as developers have to figure out ourselves what can go wrong and
the users, well who cares about users?

> 
> Anyway, it's not a problem to me.  I have seen similar messages on my
> kernel serial output.  I fail to see why pinmux can not do something
> that other subsystems are doing.

Yes, and everytime I see such a message I dig through the sourcecode
trying to find out if that's important or not. That's easy for me on
the systems I work with, but on my PC I am only a user, and honestly
on my PC I *want* to be only a user.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux subsystem

[Shawn Guo]

On Thu, Dec 15, 2011 at 02:23:36PM +0100, Sascha Hauer wrote:
> On Thu, Dec 15, 2011 at 08:17:54PM +0800, Shawn Guo wrote:
> > On Thu, Dec 15, 2011 at 12:54:28PM +0100, Sascha Hauer wrote:
> > > On Thu, Dec 15, 2011 at 07:53:05PM +0800, Shawn Guo wrote:
> > > > On Thu, Dec 15, 2011 at 07:28:18PM +0800, Dong Aisheng-B29396 wrote:
> > > > > > -----Original Message-----
> > > > > > 
> > > > > My understanding is that pinmux_get will return an error if no proper pinmux
> > > > > Found without dummy pinmux. That's a real error.
> > > > > But with dummy pinmux, if no proper pinmux found, the pinctrl core may check
> > > > > If dummy pinmux is supported, if supported, it will fakely success with returning
> > > > > a dummy pinmux. Then real error is hiden.
> > > > > This is due to for supporting one single image, the dummy pinmux may also be enabled
> > > > > For platforms like mx6q with real pinmux.
> > > > > 
> > > > Got your point.  So your concern is about the fake success when dummy
> > > > pinmux comes to play rather than 'fake error'.  I'm not concerned about
> > > > that case much, since we will know the failure with a simple debug/info
> > > > message in the pinmux core, saying 'I do not find any available pinmux,
> > > > and I'm falling into the dummy pinmux'.  When you see this message
> > > > imx6q, you know something goes wrong.
> > > 
> > > So i.MX3/5 people must know that it's safe to ignore this message
> > > whereas i.MX6 people must know there's something wrong Adding messages
> > > saying "there might or might not be something wrong" is not good.
> > > People will frequently ask on the mailing list about these messages.
> > > 
> > Usually, people checking the serial output and asking question on list
> > are developers not users.  It's not too hard for developers to figure
> > out such message.  Or we can making it a debug message, and when some
> > driver is not working, turn on debug option, you know where it goes
> > wrong.
> 
> So we as developers have to figure out ourselves what can go wrong and
> the users, well who cares about users?
> 
It's really the type of problem that can be discovered and fixed in
the development phase, isn't it?

BTW, I did not mean to leave imx3 and imx5 there forever, but mean,
with the dummy pinmux support, we do not have to strictly migrate imx3,
imx5 and imx6 all at the same time.

-- 
Regards,
Shawn

[RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux subsystem

[Sascha Hauer]

On Thu, Dec 15, 2011 at 10:00:25PM +0800, Shawn Guo wrote:
> On Thu, Dec 15, 2011 at 02:23:36PM +0100, Sascha Hauer wrote:
> > On Thu, Dec 15, 2011 at 08:17:54PM +0800, Shawn Guo wrote:
> > > On Thu, Dec 15, 2011 at 12:54:28PM +0100, Sascha Hauer wrote:
> > > > On Thu, Dec 15, 2011 at 07:53:05PM +0800, Shawn Guo wrote:
> > > > > On Thu, Dec 15, 2011 at 07:28:18PM +0800, Dong Aisheng-B29396 wrote:
> > > > > > > -----Original Message-----
> > > > > > > 
> > > > > > My understanding is that pinmux_get will return an error if no proper pinmux
> > > > > > Found without dummy pinmux. That's a real error.
> > > > > > But with dummy pinmux, if no proper pinmux found, the pinctrl core may check
> > > > > > If dummy pinmux is supported, if supported, it will fakely success with returning
> > > > > > a dummy pinmux. Then real error is hiden.
> > > > > > This is due to for supporting one single image, the dummy pinmux may also be enabled
> > > > > > For platforms like mx6q with real pinmux.
> > > > > > 
> > > > > Got your point.  So your concern is about the fake success when dummy
> > > > > pinmux comes to play rather than 'fake error'.  I'm not concerned about
> > > > > that case much, since we will know the failure with a simple debug/info
> > > > > message in the pinmux core, saying 'I do not find any available pinmux,
> > > > > and I'm falling into the dummy pinmux'.  When you see this message
> > > > > imx6q, you know something goes wrong.
> > > > 
> > > > So i.MX3/5 people must know that it's safe to ignore this message
> > > > whereas i.MX6 people must know there's something wrong Adding messages
> > > > saying "there might or might not be something wrong" is not good.
> > > > People will frequently ask on the mailing list about these messages.
> > > > 
> > > Usually, people checking the serial output and asking question on list
> > > are developers not users.  It's not too hard for developers to figure
> > > out such message.  Or we can making it a debug message, and when some
> > > driver is not working, turn on debug option, you know where it goes
> > > wrong.
> > 
> > So we as developers have to figure out ourselves what can go wrong and
> > the users, well who cares about users?
> > 
> It's really the type of problem that can be discovered and fixed in
> the development phase, isn't it?
> 
> BTW, I did not mean to leave imx3 and imx5 there forever, but mean,
> with the dummy pinmux support, we do not have to strictly migrate imx3,
> imx5 and imx6 all at the same time.

I'm afraid your plans for i.MX3/5 clash with Freescale's i.MX7
release...

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux subsystem

[Shawn Guo]

On Thu, Dec 15, 2011 at 09:26:40AM +0100, Linus Walleij wrote:
> On Thu, Dec 15, 2011 at 8:05 AM, Shawn Guo <shawn.guo@freescale.com> wrote:
> >[Me]
> >> So if you want to do this for i.MX you need something like
> >> selectable dummy pinmuxes, i.e. pinmux_get() to return something
> >> that just say "OK" to everything like the dummy regulators.
> >>
> >> Shall I try to create something like that?
> >>
> > Isn't the empty functions defined in include/linux/pinctrl/pinmux.h
> > for this purpose?
> 
> No, these are for compiling it *out*, dummy pinmuxes would
> be if you compile it *in*, but don't find an apropriate pinmux,
> you still get something that does nothing and still works.
> 
> Dummy regulators work exactly this way.
> 
> > It does not solve the problem with single image.
> 
> I think it does.
> 
> > You might probably mean that we create a dummy_pinctrl_desc and register
> > it to pinctrl core with pinctrl_register() if we detect that the kernel
> > is running on a soc that has no pinctrl support?
> 
> No. You have a #ifdef CONFIG_PINMUX_DUMMY in pinmux_get()
> that makes sure you return a working no-op pinmux handle even
> though there is no real pinmux behind it.
> 
Ah, ok.  So the problem will be sorted out at pinctrl core level, and
pinctrl driver will not be bothered at all.  Yes, please, we need this.

> > This is not a problem to pinctrl migration only. ?We have the same
> > problem with common clk migration. ?Unless we migrate imx3, imx5 and
> > imx6 to common clk at the same time, single image build just does not
> > cope with clk_* api.
> 
> And  you could have the same problem with regulator migration...
> Luckily dummy regulators saves you in that case.
> 
Not sure what will save me in common clk case though.

-- 
Regards,
Shawn

[RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux subsystem

[Dong Aisheng-B29396]

> -----Original Message-----
> From: Linus Walleij [mailto:linus.walleij at linaro.org]
> Sent: Thursday, December 15, 2011 6:16 AM
> To: Sascha Hauer
> Cc: Dong Aisheng-B29396; linus.walleij at stericsson.com; linux-
> kernel at vger.kernel.org; rob.herring at calxeda.com;
> grant.likely at secretlab.ca; linux-arm-kernel at lists.infradead.org;
> kernel at pengutronix.de; Guo Shawn-R65073
> Subject: Re: [RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux
> subsystem
> Importance: High
> 
> On Wed, Dec 14, 2011 at 10:11 PM, Sascha Hauer <s.hauer@pengutronix.de>
> wrote:
> > On Thu, Dec 15, 2011 at 12:03:42AM +0800, Dong Aisheng wrote:
> 
> >> + ? ? pmx = pinmux_get(&pdev->dev, NULL);
> >> + ? ? if (IS_ERR(pmx)) {
> >> + ? ? ? ? ? ? err = PTR_ERR(pmx);
> >> + ? ? ? ? ? ? goto err_pmx_get;
> >> + ? ? }
> >> + ? ? err = pinmux_enable(pmx);
> >> + ? ? if (err)
> >> + ? ? ? ? ? ? goto err_pmx_enable;
> >> + ? ? imx_data->pmx = pmx;
> >> +
> >
> > Won't this break every i.MX except i.MX6?
>

If not select PINCTRL, pinmux_enable returns ok by default and pinmux_get
Returns NULL which also does not block.
So it will not break other i.MX currently since only mx6q are using PINCTRL.
 
> Not this patch on its own, first the machines have to select PINCTRL
> select PINMUX_FOO
> 
> *Then* it will break :-D
> 

After we move to one single image for mx5 and mx6, it could be an issue
Since PINCTRL may be selected by default.
So mx5 pinmux driver is in my TODO list.

> So if you want to do this for i.MX you need something like selectable
> dummy pinmuxes, i.e. pinmux_get() to return something that just say "OK"
> to everything like the dummy regulators.
> 
> Shall I try to create something like that?
> 
For those platforms do not select PINCTRL, current code does not block.
For platforms do select PINCTRL but does not have pinmux driver ready,
for example, single image for both mx5&mx6,
IMHO it may be better to fix in driver to avoid introduce too much
complexity in pinctrl core.

Regards
Dong Aisheng

[RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux subsystem

[Linus Walleij]

On Thu, Dec 15, 2011 at 8:23 AM, Dong Aisheng-B29396
<B29396@freescale.com> wrote:
> [Me]
>> So if you want to do this for i.MX you need something like selectable
>> dummy pinmuxes, i.e. pinmux_get() to return something that just say "OK"
>> to everything like the dummy regulators.
>>
>> Shall I try to create something like that?
>
> For those platforms do not select PINCTRL, current code does not block.

Yeah I know...

> For platforms do select PINCTRL but does not have pinmux driver ready,
> for example, single image for both mx5&mx6,
> IMHO it may be better to fix in driver to avoid introduce too much
> complexity in pinctrl core.

If you mean that you fix the i.MX driver for all the combines i.MX variants
then I agree.

If you mean to do stuff like allow the code to continue even if the pinmux
isn't found - no. That is not the way we proceed with clocks and
regulators as Mark has taught me recently, so for consistency we
need to error out if no pinmux is found.

Thanks,
Linus Walleij

[RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux subsystem

[Dong Aisheng-B29396]

> -----Original Message-----
> From: Linus Walleij [mailto:linus.walleij at linaro.org]
> Sent: Thursday, December 15, 2011 4:29 PM
> To: Dong Aisheng-B29396
> Cc: Sascha Hauer; linus.walleij at stericsson.com; linux-
> kernel at vger.kernel.org; rob.herring at calxeda.com;
> grant.likely at secretlab.ca; linux-arm-kernel at lists.infradead.org;
> kernel at pengutronix.de; Guo Shawn-R65073
> Subject: Re: [RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux
> subsystem
> Importance: High
> 
> On Thu, Dec 15, 2011 at 8:23 AM, Dong Aisheng-B29396
> <B29396@freescale.com> wrote:
> > [Me]
> >> So if you want to do this for i.MX you need something like selectable
> >> dummy pinmuxes, i.e. pinmux_get() to return something that just say
> "OK"
> >> to everything like the dummy regulators.
> >>
> >> Shall I try to create something like that?
> >
> > For those platforms do not select PINCTRL, current code does not block.
> 
> Yeah I know...
> 
> > For platforms do select PINCTRL but does not have pinmux driver ready,
> > for example, single image for both mx5&mx6, IMHO it may be better to
> > fix in driver to avoid introduce too much complexity in pinctrl core.
> 
> If you mean that you fix the i.MX driver for all the combines i.MX
> variants then I agree.
> 
Yes, I mean this one.

> If you mean to do stuff like allow the code to continue even if the
> pinmux isn't found - no. That is not the way we proceed with clocks and
> regulators as Mark has taught me recently, so for consistency we need to
> error out if no pinmux is found.
> 

Regards
Dong Aisheng

[RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux subsystem

[Sascha Hauer]

On Thu, Dec 15, 2011 at 07:23:42AM +0000, Dong Aisheng-B29396 wrote:
> > -----Original Message-----
> > From: Linus Walleij [mailto:linus.walleij at linaro.org]
> > Sent: Thursday, December 15, 2011 6:16 AM
> > To: Sascha Hauer
> > Cc: Dong Aisheng-B29396; linus.walleij at stericsson.com; linux-
> > kernel at vger.kernel.org; rob.herring at calxeda.com;
> > grant.likely at secretlab.ca; linux-arm-kernel at lists.infradead.org;
> > kernel at pengutronix.de; Guo Shawn-R65073
> > Subject: Re: [RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux
> > subsystem
> > Importance: High
> > 
> > On Wed, Dec 14, 2011 at 10:11 PM, Sascha Hauer <s.hauer@pengutronix.de>
> > wrote:
> > > On Thu, Dec 15, 2011 at 12:03:42AM +0800, Dong Aisheng wrote:
> > 
> > >> + ? ? pmx = pinmux_get(&pdev->dev, NULL);
> > >> + ? ? if (IS_ERR(pmx)) {
> > >> + ? ? ? ? ? ? err = PTR_ERR(pmx);
> > >> + ? ? ? ? ? ? goto err_pmx_get;
> > >> + ? ? }
> > >> + ? ? err = pinmux_enable(pmx);
> > >> + ? ? if (err)
> > >> + ? ? ? ? ? ? goto err_pmx_enable;
> > >> + ? ? imx_data->pmx = pmx;
> > >> +
> > >
> > > Won't this break every i.MX except i.MX6?
> >
> 
> If not select PINCTRL, pinmux_enable returns ok by default and pinmux_get
> Returns NULL which also does not block.
> So it will not break other i.MX currently since only mx6q are using PINCTRL.
>  
> > Not this patch on its own, first the machines have to select PINCTRL
> > select PINMUX_FOO
> > 
> > *Then* it will break :-D
> > 
> 
> After we move to one single image for mx5 and mx6, it could be an issue
> Since PINCTRL may be selected by default.
> So mx5 pinmux driver is in my TODO list.

Add i.MX3 which is compiled together with i.MX6 :)

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux subsystem

[Dong Aisheng-B29396]

> -----Original Message-----
> From: Sascha Hauer [mailto:s.hauer at pengutronix.de]
> Sent: Thursday, December 15, 2011 5:32 PM
> To: Dong Aisheng-B29396
> Cc: Linus Walleij; linus.walleij at stericsson.com; linux-
> kernel at vger.kernel.org; rob.herring at calxeda.com;
> grant.likely at secretlab.ca; linux-arm-kernel at lists.infradead.org;
> kernel at pengutronix.de; Guo Shawn-R65073
> Subject: Re: [RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux
> subsystem
> Importance: High
> 
> On Thu, Dec 15, 2011 at 07:23:42AM +0000, Dong Aisheng-B29396 wrote:
> > > -----Original Message-----
> > > From: Linus Walleij [mailto:linus.walleij at linaro.org]
> > > Sent: Thursday, December 15, 2011 6:16 AM
> > > To: Sascha Hauer
> > > Cc: Dong Aisheng-B29396; linus.walleij at stericsson.com; linux-
> > > kernel at vger.kernel.org; rob.herring at calxeda.com;
> > > grant.likely at secretlab.ca; linux-arm-kernel at lists.infradead.org;
> > > kernel at pengutronix.de; Guo Shawn-R65073
> > > Subject: Re: [RFC PATCH v2 4/4] mmc: sdhci-esdhc-imx: using pinmux
> > > subsystem
> > > Importance: High
> > >
> > > On Wed, Dec 14, 2011 at 10:11 PM, Sascha Hauer
> > > <s.hauer@pengutronix.de>
> > > wrote:
> > > > On Thu, Dec 15, 2011 at 12:03:42AM +0800, Dong Aisheng wrote:
> > >
> > > >> + ? ? pmx = pinmux_get(&pdev->dev, NULL);
> > > >> + ? ? if (IS_ERR(pmx)) {
> > > >> + ? ? ? ? ? ? err = PTR_ERR(pmx);
> > > >> + ? ? ? ? ? ? goto err_pmx_get;
> > > >> + ? ? }
> > > >> + ? ? err = pinmux_enable(pmx);
> > > >> + ? ? if (err)
> > > >> + ? ? ? ? ? ? goto err_pmx_enable;
> > > >> + ? ? imx_data->pmx = pmx;
> > > >> +
> > > >
> > > > Won't this break every i.MX except i.MX6?
> > >
> >
> > If not select PINCTRL, pinmux_enable returns ok by default and
> > pinmux_get Returns NULL which also does not block.
> > So it will not break other i.MX currently since only mx6q are using
> PINCTRL.
> >
> > > Not this patch on its own, first the machines have to select PINCTRL
> > > select PINMUX_FOO
> > >
> > > *Then* it will break :-D
> > >
> >
> > After we move to one single image for mx5 and mx6, it could be an
> > issue Since PINCTRL may be selected by default.
> > So mx5 pinmux driver is in my TODO list.
> 
> Add i.MX3 which is compiled together with i.MX6 :)
> 
Sure, would like to do it.
Thanks

Regards
Dong Aisheng

[RFC PATCH v2 0/4] pinctrl: imx: add pinnmux support

[Linus Walleij]

On Wed, Dec 14, 2011 at 5:03 PM, Dong Aisheng <b29396@freescale.com> wrote:

> For DT support, there's still one issue that for pinmux_map,
> since it's board specific, i wonder if it can be parsed by pinctrl
> subsystem via dt rather than by each driver.

That is definately how it should be done. Seems like a good
idea to me.

> but pinctrl still does not supported that, i still only called
> it in mach-imx6q.c which seems not a correct place.

Don't worry that pinctrl does not support it - patch pinctrl core!

Just make a separate patch which patches drivers/pinctrl/pinmux.c
to ass the device tree parsing you need from the core, and we'll
review it.

I am by far too incompetent in device tree mechanisms to do
that myself, I have never worked much on device tree.

Thanks,
Linus Walleij