[3/7] pinctrl: imx1 core driver

Commit Message

Markus Pargmann Aug. 2, 2013, 10:38 a.m. UTC

Signed-off-by: Markus Pargmann <[e-mail]>
---
 drivers/pinctrl/Kconfig             |   5 +
 drivers/pinctrl/Makefile            |   1 +
 drivers/pinctrl/pinctrl-imx1-core.c | 667 ++++++++++++++++++++++++++++++++++++
 drivers/pinctrl/pinctrl-imx1.h      |  23 ++
 4 files changed, 696 insertions(+)
 create mode 100644 drivers/pinctrl/pinctrl-imx1-core.c
 create mode 100644 drivers/pinctrl/pinctrl-imx1.h

Comments

Alexander Shiyan Aug. 2, 2013, 10:51 a.m. UTC | #1

> Signed-off-by: Markus Pargmann <[e-mail]>
> ---
[...]
> +++ b/drivers/pinctrl/pinctrl-imx1-core.c
> @@ -0,0 +1,667 @@
> +/*
> + * Core driver for the imx pin controller in imx1/21/27
[...]
> +static void imx1_write_2bit(struct imx1_pinctrl *ipctl, unsigned int pin_id,
> +		u32 value, u32 reg_offset)
> +{
> +	void __iomem *reg = imx1_mem(ipctl, pin_id) + reg_offset;
> +	int shift = (pin_id % 16) * 2;
> +	int mask = ~(0x3 << shift);

0x3.

> +	u32 old_value;
> +
> +	dev_dbg(ipctl->dev, "write: register 0x%p shift %d value 0x%x\n",
> +			reg, shift, value);
> +
> +	if (pin_id % 32 >= 16)
> +		reg += 0x04;
> +
> +	value = (value & 0x11) << shift;

0x11. Is this correct?

[...]

---

Markus Pargmann Aug. 2, 2013, 10:54 a.m. UTC | #2

On Fri, Aug 02, 2013 at 02:51:24PM +0400, Alexander Shiyan wrote:
> > Signed-off-by: Markus Pargmann <[e-mail]>
> > ---
> [...]
> > +++ b/drivers/pinctrl/pinctrl-imx1-core.c
> > @@ -0,0 +1,667 @@
> > +/*
> > + * Core driver for the imx pin controller in imx1/21/27
> [...]
> > +static void imx1_write_2bit(struct imx1_pinctrl *ipctl, unsigned int pin_id,
> > +		u32 value, u32 reg_offset)
> > +{
> > +	void __iomem *reg = imx1_mem(ipctl, pin_id) + reg_offset;
> > +	int shift = (pin_id % 16) * 2;
> > +	int mask = ~(0x3 << shift);
> 
> 0x3.
> 
> > +	u32 old_value;
> > +
> > +	dev_dbg(ipctl->dev, "write: register 0x%p shift %d value 0x%x\n",
> > +			reg, shift, value);
> > +
> > +	if (pin_id % 32 >= 16)
> > +		reg += 0x04;
> > +
> > +	value = (value & 0x11) << shift;
> 
> 0x11. Is this correct?

oh no, should be 0x3.

Thanks,

Markus

Sascha Hauer Aug. 5, 2013, 9:29 a.m. UTC | #3

On Fri, Aug 02, 2013 at 12:38:23PM +0200, Markus Pargmann wrote:
> Signed-off-by: Markus Pargmann <[e-mail]>
> ---
>  drivers/pinctrl/Kconfig             |   5 +
>  drivers/pinctrl/Makefile            |   1 +
>  drivers/pinctrl/pinctrl-imx1-core.c | 667 ++++++++++++++++++++++++++++++++++++
>  drivers/pinctrl/pinctrl-imx1.h      |  23 ++
>  4 files changed, 696 insertions(+)
>  create mode 100644 drivers/pinctrl/pinctrl-imx1-core.c
>  create mode 100644 drivers/pinctrl/pinctrl-imx1.h
> 
> +static int imx1_pinctrl_parse_groups(struct device_node *np,
> +				    struct imx_pin_group *grp,
> +				    struct imx_pinctrl_soc_info *info,
> +				    u32 index)
> +{
> +	int size;
> +	const __be32 *list;
> +	int i;
> +	u32 config;
> +
> +	dev_dbg(info->dev, "group(%d): %s\n", index, np->name);
> +
> +	/* Initialise group */
> +	grp->name = np->name;
> +
> +	/*
> +	 * the binding format is fsl,pins = <PIN_FUNC_ID CONFIG ...>,
> +	 * do sanity check and calculate pins number
> +	 */
> +	list = of_get_property(np, "fsl,pins", &size);
> +	/* we do not check return since it's safe node passed down */
> +	if (!size || size % 12) {
> +		dev_notice(info->dev, "Not a valid fsl,pins property (%s)\n",
> +				np->name);
> +		return -EINVAL;
> +	}
> +
> +	grp->npins = size / 12;
> +	grp->pins = devm_kzalloc(info->dev, grp->npins * sizeof(unsigned int),
> +				GFP_KERNEL);
> +	grp->mux_mode = devm_kzalloc(info->dev,
> +				grp->npins * sizeof(unsigned int), GFP_KERNEL);
> +	grp->configs = devm_kzalloc(info->dev,
> +				grp->npins * sizeof(unsigned long), GFP_KERNEL);

I know this is copied from the existing imx pinctrl driver, but normally
we check for allocation failures.

Also I find it very irritating that you allocate three arrays of
integers instead of a single array of a struct type. I know the pinctrl
framework forces you to pass an array of pin numbers (which is quite
horrible for drivers to handle, who came up with the idea that a pin is
a number instead of some struct pointer which can be attached data to?)


> +	for (i = 0; i < grp->npins; i++) {
> +		grp->pins[i] = be32_to_cpu(*list++);
> +		grp->mux_mode[i] = be32_to_cpu(*list++);
> +
> +		config = be32_to_cpu(*list++);
> +		grp->configs[i] = config;
> +	}
> +
> +	return 0;
> +}
> +

[...]

> +
> +	for_each_child_of_node(np, child) {
> +		func->groups[i] = child->name;
> +		grp = &info->groups[grp_index++];
> +		ret = imx1_pinctrl_parse_groups(child, grp, info, i++);
> +		if (ret)
> +			return ret;

This is one thing which nags me in the imx pinctrl driver aswell. Once
you made a single mistake in a single pinctrl group in the devicetree
you will never see the error message because the whole pinctrl driver
bails out leaving the serial driver with the console unusable. Wouldn't
it be possible to continue here instead of bailing out?

Sascha

Linus Walleij Aug. 7, 2013, 7:25 p.m. UTC | #4

On Fri, Aug 2, 2013 at 12:38 PM, Markus Pargmann <[e-mail]> wrote:

> Signed-off-by: Markus Pargmann <[e-mail]>

I think you need a bit of patch description here. Zero lines of
commit message is not acceptable for an entire new driver.
Elaborate a bit on the imx27 hardware and so on.

> +/*
> + * Calculates the register offset from a pin_id
> + */
> +static void __iomem *imx1_mem(struct imx1_pinctrl *ipctl, unsigned int pin_id)
> +{
> +       unsigned int port = pin_id / 32;
> +       return ipctl->base + port * 0x100;

Use this:
#define IMX1_PORT_STRIDE 0x100

> +/*
> + * Write to a register with 2 bits per pin. The function will automatically
> + * use the next register if the pin is managed in the second register.
> + */
> +static void imx1_write_2bit(struct imx1_pinctrl *ipctl, unsigned int pin_id,
> +               u32 value, u32 reg_offset)
> +{
> +       void __iomem *reg = imx1_mem(ipctl, pin_id) + reg_offset;
> +       int shift = (pin_id % 16) * 2;
> +       int mask = ~(0x3 << shift);

I think I understand this, but insert a comment on what this is
anyway.

> +       u32 old_value;
> +
> +       dev_dbg(ipctl->dev, "write: register 0x%p shift %d value 0x%x\n",
> +                       reg, shift, value);
> +
> +       if (pin_id % 32 >= 16)
> +               reg += 0x04;

Comment on what is happening here.

> +
> +       value = (value & 0x11) << shift;

What is this? 0x11? Shall this be #defined or
what does it mean? The "value" argument really needs
some documentation I think.

You're modifying the argument "value" which is confusing.
Try to avoid this.

> +       old_value = readl(reg) & mask;
> +       writel(value | old_value, reg);

This is a bit over-complicating things. Write out
the sequence and let the compiler do the optimization.

val = readl(reg);
val &= mask;
val |= value;
writel(val, reg);


> +static void imx1_write_bit(struct imx1_pinctrl *ipctl, unsigned int pin_id,
> +               u32 value, u32 reg_offset)
> +{
> +       void __iomem *reg = imx1_mem(ipctl, pin_id) + reg_offset;
> +       int shift = pin_id % 32;
> +       int mask = ~(0x1 << shift);
> +       u32 old_value;
> +
> +       dev_dbg(ipctl->dev, "write: register 0x%p shift %d value 0x%x\n",
> +                       reg, shift, value);
> +
> +       value = (value & 0x1) << shift;
> +       old_value = readl(reg) & mask;
> +       writel(value | old_value, reg);

Same comments here.

> +static int imx1_read_bit(struct imx1_pinctrl *ipctl, unsigned int pin_id,
> +               u32 reg_offset)
> +{
> +       void __iomem *reg = imx1_mem(ipctl, pin_id) + reg_offset;
> +       int shift = pin_id % 32;
> +
> +       return (readl(reg) >> shift) & 0x1;

Hard to read. Can't you just do this?

#include <linux/bitops.h>

u32 offset = pin_id % 32;

return !!(readl(reg) & BIT(offset));

> +static void imx1_pin_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
> +                  unsigned offset)
> +{
> +       seq_printf(s, "%s", dev_name(pctldev->dev));
> +}

Don't you want to print some other more interesting things about
each pin?

This template is really just an example. The debugfs file is
intended to be helpful.

> +static int imx1_pmx_enable(struct pinctrl_dev *pctldev, unsigned selector,
> +                          unsigned group)
> +{
> +       struct imx1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> +       const struct imx_pinctrl_soc_info *info = ipctl->info;
> +       const unsigned *pins, *mux;
> +       unsigned int npins;
> +       int i;
> +
> +       /*
> +        * Configure the mux mode for each pin in the group for a specific
> +        * function.
> +        */
> +       pins = info->groups[group].pins;
> +       npins = info->groups[group].npins;
> +       mux = info->groups[group].mux_mode;
> +
> +       WARN_ON(!pins || !npins || !mux);
> +
> +       dev_dbg(ipctl->dev, "enable function %s group %s\n",
> +               info->functions[selector].name, info->groups[group].name);
> +
> +       for (i = 0; i < npins; i++) {
> +               unsigned int pin_id = pins[i];
> +               unsigned int afunction = 0x001 & mux[i];
> +               unsigned int gpio_in_use = (0x002 & mux[i]) >> 1;
> +               unsigned int direction = (0x004 & mux[i]) >> 2;
> +               unsigned int gpio_oconf = (0x030 & mux[i]) >> 4;
> +               unsigned int gpio_iconfa = (0x300 & mux[i]) >> 8;
> +               unsigned int gpio_iconfb = (0xc00 & mux[i]) >> 10;

If you use <linux/bitops.h> this can be made more understandable,
BIT(0), BIT(1), BIT(2) etc.

The shift offsets should be #defined.

> +static void imx1_pinconf_dbg_show(struct pinctrl_dev *pctldev,
> +                                  struct seq_file *s, unsigned pin_id)
> +{
> +       struct imx1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> +       const struct imx_pinctrl_soc_info *info = ipctl->info;
> +       const struct imx_pin_reg *pin_reg = &info->pin_regs[pin_id];
> +       unsigned long config;
> +
> +       if (!pin_reg || !pin_reg->conf_reg) {
> +               seq_puts(s, "N/A");
> +               return;
> +       }
> +
> +       config = readl(ipctl->base + pin_reg->conf_reg);
> +       seq_printf(s, "0x%lx", config);
> +}

That's pretty helpful, nice!

> +static int imx1_pinctrl_parse_gpio(struct platform_device *pdev,
> +               struct imx1_pinctrl *pctl, struct device_node *np, int i,
> +               u32 base)
> +{
> +       int ret;
> +       u32 memoffset;
> +
> +       ret = of_property_read_u32(np, "reg", &memoffset);
> +       if (ret)
> +               return ret;
> +
> +       memoffset -= base;
> +       pctl->gpio_pdata[i].base = pctl->base + memoffset;
> +
> +       pctl->gpio_dev[i] = of_device_alloc(np, NULL, &pdev->dev);
> +       pctl->gpio_dev[i]->dev.platform_data = &pctl->gpio_pdata[i];
> +       pctl->gpio_dev[i]->dev.bus = &platform_bus_type;
> +
> +       ret = of_device_add(pctl->gpio_dev[i]);
> +       if (ret) {
> +               dev_err(&pdev->dev,
> +                               "Failed to add child gpio device\n");
> +               return ret;
> +       }
> +       return 0;
> +}

As mentioned for the other patch I think this is the wrong approach.
Try to make the GPIO driver wholly independent.

Yours,
Linus Walleij

Markus Pargmann Aug. 9, 2013, 6:16 p.m. UTC | #5

On Mon, Aug 05, 2013 at 11:29:52AM +0200, Sascha Hauer wrote:
> On Fri, Aug 02, 2013 at 12:38:23PM +0200, Markus Pargmann wrote:
> > Signed-off-by: Markus Pargmann <[e-mail]>
> > ---
> >  drivers/pinctrl/Kconfig             |   5 +
> >  drivers/pinctrl/Makefile            |   1 +
> >  drivers/pinctrl/pinctrl-imx1-core.c | 667 ++++++++++++++++++++++++++++++++++++
> >  drivers/pinctrl/pinctrl-imx1.h      |  23 ++
> >  4 files changed, 696 insertions(+)
> >  create mode 100644 drivers/pinctrl/pinctrl-imx1-core.c
> >  create mode 100644 drivers/pinctrl/pinctrl-imx1.h
> > 
> > +static int imx1_pinctrl_parse_groups(struct device_node *np,
> > +				    struct imx_pin_group *grp,
> > +				    struct imx_pinctrl_soc_info *info,
> > +				    u32 index)
> > +{
> > +	int size;
> > +	const __be32 *list;
> > +	int i;
> > +	u32 config;
> > +
> > +	dev_dbg(info->dev, "group(%d): %s\n", index, np->name);
> > +
> > +	/* Initialise group */
> > +	grp->name = np->name;
> > +
> > +	/*
> > +	 * the binding format is fsl,pins = <PIN_FUNC_ID CONFIG ...>,
> > +	 * do sanity check and calculate pins number
> > +	 */
> > +	list = of_get_property(np, "fsl,pins", &size);
> > +	/* we do not check return since it's safe node passed down */
> > +	if (!size || size % 12) {
> > +		dev_notice(info->dev, "Not a valid fsl,pins property (%s)\n",
> > +				np->name);
> > +		return -EINVAL;
> > +	}
> > +
> > +	grp->npins = size / 12;
> > +	grp->pins = devm_kzalloc(info->dev, grp->npins * sizeof(unsigned int),
> > +				GFP_KERNEL);
> > +	grp->mux_mode = devm_kzalloc(info->dev,
> > +				grp->npins * sizeof(unsigned int), GFP_KERNEL);
> > +	grp->configs = devm_kzalloc(info->dev,
> > +				grp->npins * sizeof(unsigned long), GFP_KERNEL);
> 
> I know this is copied from the existing imx pinctrl driver, but normally
> we check for allocation failures.

Yes, I added allocation checks.

> 
> Also I find it very irritating that you allocate three arrays of
> integers instead of a single array of a struct type. I know the pinctrl
> framework forces you to pass an array of pin numbers (which is quite
> horrible for drivers to handle, who came up with the idea that a pin is
> a number instead of some struct pointer which can be attached data to?)

Fixed, using new imx1_pin structs now.

> 
> > +	for (i = 0; i < grp->npins; i++) {
> > +		grp->pins[i] = be32_to_cpu(*list++);
> > +		grp->mux_mode[i] = be32_to_cpu(*list++);
> > +
> > +		config = be32_to_cpu(*list++);
> > +		grp->configs[i] = config;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> 
> [...]
> 
> > +
> > +	for_each_child_of_node(np, child) {
> > +		func->groups[i] = child->name;
> > +		grp = &info->groups[grp_index++];
> > +		ret = imx1_pinctrl_parse_groups(child, grp, info, i++);
> > +		if (ret)
> > +			return ret;
> 
> This is one thing which nags me in the imx pinctrl driver aswell. Once
> you made a single mistake in a single pinctrl group in the devicetree
> you will never see the error message because the whole pinctrl driver
> bails out leaving the serial driver with the console unusable. Wouldn't
> it be possible to continue here instead of bailing out?

Yes, it is possible. I changed it to continue if ret != -ENOMEM.

Thanks,

Markus

Markus Pargmann Aug. 9, 2013, 7:33 p.m. UTC | #6

On Wed, Aug 07, 2013 at 09:25:35PM +0200, Linus Walleij wrote:
> On Fri, Aug 2, 2013 at 12:38 PM, Markus Pargmann <[e-mail]> wrote:
> 
> > Signed-off-by: Markus Pargmann <[e-mail]>
> 
> I think you need a bit of patch description here. Zero lines of
> commit message is not acceptable for an entire new driver.
> Elaborate a bit on the imx27 hardware and so on.
> 
> > +/*
> > + * Calculates the register offset from a pin_id
> > + */
> > +static void __iomem *imx1_mem(struct imx1_pinctrl *ipctl, unsigned int pin_id)
> > +{
> > +       unsigned int port = pin_id / 32;
> > +       return ipctl->base + port * 0x100;
> 
> Use this:
> #define IMX1_PORT_STRIDE 0x100
> 
> > +/*
> > + * Write to a register with 2 bits per pin. The function will automatically
> > + * use the next register if the pin is managed in the second register.
> > + */
> > +static void imx1_write_2bit(struct imx1_pinctrl *ipctl, unsigned int pin_id,
> > +               u32 value, u32 reg_offset)
> > +{
> > +       void __iomem *reg = imx1_mem(ipctl, pin_id) + reg_offset;
> > +       int shift = (pin_id % 16) * 2;
> > +       int mask = ~(0x3 << shift);
> 
> I think I understand this, but insert a comment on what this is
> anyway.
> 
> > +       u32 old_value;
> > +
> > +       dev_dbg(ipctl->dev, "write: register 0x%p shift %d value 0x%x\n",
> > +                       reg, shift, value);
> > +
> > +       if (pin_id % 32 >= 16)
> > +               reg += 0x04;
> 
> Comment on what is happening here.
> 
> > +
> > +       value = (value & 0x11) << shift;
> 
> What is this? 0x11? Shall this be #defined or
> what does it mean? The "value" argument really needs
> some documentation I think.
> 
> You're modifying the argument "value" which is confusing.
> Try to avoid this.
> 
> > +       old_value = readl(reg) & mask;
> > +       writel(value | old_value, reg);
> 
> This is a bit over-complicating things. Write out
> the sequence and let the compiler do the optimization.
> 
> val = readl(reg);
> val &= mask;
> val |= value;
> writel(val, reg);
> 
> 
> > +static void imx1_write_bit(struct imx1_pinctrl *ipctl, unsigned int pin_id,
> > +               u32 value, u32 reg_offset)
> > +{
> > +       void __iomem *reg = imx1_mem(ipctl, pin_id) + reg_offset;
> > +       int shift = pin_id % 32;
> > +       int mask = ~(0x1 << shift);
> > +       u32 old_value;
> > +
> > +       dev_dbg(ipctl->dev, "write: register 0x%p shift %d value 0x%x\n",
> > +                       reg, shift, value);
> > +
> > +       value = (value & 0x1) << shift;
> > +       old_value = readl(reg) & mask;
> > +       writel(value | old_value, reg);
> 
> Same comments here.
> 
> > +static int imx1_read_bit(struct imx1_pinctrl *ipctl, unsigned int pin_id,
> > +               u32 reg_offset)
> > +{
> > +       void __iomem *reg = imx1_mem(ipctl, pin_id) + reg_offset;
> > +       int shift = pin_id % 32;
> > +
> > +       return (readl(reg) >> shift) & 0x1;
> 
> Hard to read. Can't you just do this?
> 
> #include <linux/bitops.h>
> 
> u32 offset = pin_id % 32;
> 
> return !!(readl(reg) & BIT(offset));
> 
> > +static void imx1_pin_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
> > +                  unsigned offset)
> > +{
> > +       seq_printf(s, "%s", dev_name(pctldev->dev));
> > +}
> 
> Don't you want to print some other more interesting things about
> each pin?
> 
> This template is really just an example. The debugfs file is
> intended to be helpful.
> 
> > +static int imx1_pmx_enable(struct pinctrl_dev *pctldev, unsigned selector,
> > +                          unsigned group)
> > +{
> > +       struct imx1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> > +       const struct imx_pinctrl_soc_info *info = ipctl->info;
> > +       const unsigned *pins, *mux;
> > +       unsigned int npins;
> > +       int i;
> > +
> > +       /*
> > +        * Configure the mux mode for each pin in the group for a specific
> > +        * function.
> > +        */
> > +       pins = info->groups[group].pins;
> > +       npins = info->groups[group].npins;
> > +       mux = info->groups[group].mux_mode;
> > +
> > +       WARN_ON(!pins || !npins || !mux);
> > +
> > +       dev_dbg(ipctl->dev, "enable function %s group %s\n",
> > +               info->functions[selector].name, info->groups[group].name);
> > +
> > +       for (i = 0; i < npins; i++) {
> > +               unsigned int pin_id = pins[i];
> > +               unsigned int afunction = 0x001 & mux[i];
> > +               unsigned int gpio_in_use = (0x002 & mux[i]) >> 1;
> > +               unsigned int direction = (0x004 & mux[i]) >> 2;
> > +               unsigned int gpio_oconf = (0x030 & mux[i]) >> 4;
> > +               unsigned int gpio_iconfa = (0x300 & mux[i]) >> 8;
> > +               unsigned int gpio_iconfb = (0xc00 & mux[i]) >> 10;
> 
> If you use <linux/bitops.h> this can be made more understandable,
> BIT(0), BIT(1), BIT(2) etc.
> 
> The shift offsets should be #defined.
> 
> > +static void imx1_pinconf_dbg_show(struct pinctrl_dev *pctldev,
> > +                                  struct seq_file *s, unsigned pin_id)
> > +{
> > +       struct imx1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> > +       const struct imx_pinctrl_soc_info *info = ipctl->info;
> > +       const struct imx_pin_reg *pin_reg = &info->pin_regs[pin_id];
> > +       unsigned long config;
> > +
> > +       if (!pin_reg || !pin_reg->conf_reg) {
> > +               seq_puts(s, "N/A");
> > +               return;
> > +       }
> > +
> > +       config = readl(ipctl->base + pin_reg->conf_reg);
> > +       seq_printf(s, "0x%lx", config);
> > +}
> 
> That's pretty helpful, nice!
> 
> > +static int imx1_pinctrl_parse_gpio(struct platform_device *pdev,
> > +               struct imx1_pinctrl *pctl, struct device_node *np, int i,
> > +               u32 base)
> > +{
> > +       int ret;
> > +       u32 memoffset;
> > +
> > +       ret = of_property_read_u32(np, "reg", &memoffset);
> > +       if (ret)
> > +               return ret;
> > +
> > +       memoffset -= base;
> > +       pctl->gpio_pdata[i].base = pctl->base + memoffset;
> > +
> > +       pctl->gpio_dev[i] = of_device_alloc(np, NULL, &pdev->dev);
> > +       pctl->gpio_dev[i]->dev.platform_data = &pctl->gpio_pdata[i];
> > +       pctl->gpio_dev[i]->dev.bus = &platform_bus_type;
> > +
> > +       ret = of_device_add(pctl->gpio_dev[i]);
> > +       if (ret) {
> > +               dev_err(&pdev->dev,
> > +                               "Failed to add child gpio device\n");
> > +               return ret;
> > +       }
> > +       return 0;
> > +}
> 
> As mentioned for the other patch I think this is the wrong approach.
> Try to make the GPIO driver wholly independent.

Thanks for all your comments, I tried to fix everything you mentioned.

Regards,

Markus

Patch

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index bc830af..266a206 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -80,6 +80,11 @@  config PINCTRL_IMX
 	select PINMUX
 	select PINCONF
 
+config PINCTRL_IMX1_CORE
+	bool
+	select PINMUX
+	select PINCONF
+
 config PINCTRL_IMX35
 	bool "IMX35 pinctrl driver"
 	depends on OF
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index d64563bf..43bb05e 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -18,6 +18,7 @@  obj-$(CONFIG_PINCTRL_AT91)	+= pinctrl-at91.o
 obj-$(CONFIG_PINCTRL_BCM2835)	+= pinctrl-bcm2835.o
 obj-$(CONFIG_PINCTRL_BAYTRAIL)	+= pinctrl-baytrail.o
 obj-$(CONFIG_PINCTRL_IMX)	+= pinctrl-imx.o
+obj-$(CONFIG_PINCTRL_IMX1_CORE)	+= pinctrl-imx1-core.o
 obj-$(CONFIG_PINCTRL_IMX35)	+= pinctrl-imx35.o
 obj-$(CONFIG_PINCTRL_IMX51)	+= pinctrl-imx51.o
 obj-$(CONFIG_PINCTRL_IMX53)	+= pinctrl-imx53.o
diff --git a/drivers/pinctrl/pinctrl-imx1-core.c b/drivers/pinctrl/pinctrl-imx1-core.c
new file mode 100644
index 0000000..318b976
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-imx1-core.c
@@ -0,0 +1,667 @@ 
+/*
+ * Core driver for the imx pin controller in imx1/21/27
+ *
+ * Copyright (C) 2013 Pengutronix
+ * Author: Markus Pargmann <[e-mail]>
+ *
+ * Based on pinctrl-imx.c:
+ *	Author: Dong Aisheng <[e-mail]>
+ *	Copyright (C) 2012 Freescale Semiconductor, Inc.
+ *	Copyright (C) 2012 Linaro Ltd.
+ *
+ * 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/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/pinctrl/machine.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/platform_data/gpio-mxc.h>
+#include <linux/slab.h>
+
+#include "core.h"
+#include "pinctrl-imx1.h"
+
+struct imx1_pinctrl {
+	struct device *dev;
+	struct pinctrl_dev *pctl;
+	void __iomem *base;
+	const struct imx_pinctrl_soc_info *info;
+
+	int num_gpio_childs;
+	struct platform_device **gpio_dev;
+	struct mxc_gpio_platform_data *gpio_pdata;
+};
+
+
+/*
+ * MX1 register offsets
+ */
+
+#define MX1_DDIR 0x00
+#define MX1_OCR 0x04
+#define MX1_ICONFA 0x0c
+#define MX1_ICONFB 0x10
+#define MX1_GIUS 0x20
+#define MX1_GPR 0x38
+#define MX1_PUEN 0x40
+
+/*
+ * Calculates the register offset from a pin_id
+ */
+static void __iomem *imx1_mem(struct imx1_pinctrl *ipctl, unsigned int pin_id)
+{
+	unsigned int port = pin_id / 32;
+	return ipctl->base + port * 0x100;
+}
+
+/*
+ * Write to a register with 2 bits per pin. The function will automatically
+ * use the next register if the pin is managed in the second register.
+ */
+static void imx1_write_2bit(struct imx1_pinctrl *ipctl, unsigned int pin_id,
+		u32 value, u32 reg_offset)
+{
+	void __iomem *reg = imx1_mem(ipctl, pin_id) + reg_offset;
+	int shift = (pin_id % 16) * 2;
+	int mask = ~(0x3 << shift);
+	u32 old_value;
+
+	dev_dbg(ipctl->dev, "write: register 0x%p shift %d value 0x%x\n",
+			reg, shift, value);
+
+	if (pin_id % 32 >= 16)
+		reg += 0x04;
+
+	value = (value & 0x11) << shift;
+	old_value = readl(reg) & mask;
+	writel(value | old_value, reg);
+}
+
+static void imx1_write_bit(struct imx1_pinctrl *ipctl, unsigned int pin_id,
+		u32 value, u32 reg_offset)
+{
+	void __iomem *reg = imx1_mem(ipctl, pin_id) + reg_offset;
+	int shift = pin_id % 32;
+	int mask = ~(0x1 << shift);
+	u32 old_value;
+
+	dev_dbg(ipctl->dev, "write: register 0x%p shift %d value 0x%x\n",
+			reg, shift, value);
+
+	value = (value & 0x1) << shift;
+	old_value = readl(reg) & mask;
+	writel(value | old_value, reg);
+}
+
+static int imx1_read_bit(struct imx1_pinctrl *ipctl, unsigned int pin_id,
+		u32 reg_offset)
+{
+	void __iomem *reg = imx1_mem(ipctl, pin_id) + reg_offset;
+	int shift = pin_id % 32;
+
+	return (readl(reg) >> shift) & 0x1;
+}
+
+static const inline struct imx_pin_group *imx1_pinctrl_find_group_by_name(
+				const struct imx_pinctrl_soc_info *info,
+				const char *name)
+{
+	const struct imx_pin_group *grp = NULL;
+	int i;
+
+	for (i = 0; i < info->ngroups; i++) {
+		if (!strcmp(info->groups[i].name, name)) {
+			grp = &info->groups[i];
+			break;
+		}
+	}
+
+	return grp;
+}
+
+static int imx1_get_groups_count(struct pinctrl_dev *pctldev)
+{
+	struct imx1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+	const struct imx_pinctrl_soc_info *info = ipctl->info;
+
+	return info->ngroups;
+}
+
+static const char *imx1_get_group_name(struct pinctrl_dev *pctldev,
+				       unsigned selector)
+{
+	struct imx1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+	const struct imx_pinctrl_soc_info *info = ipctl->info;
+
+	return info->groups[selector].name;
+}
+
+static int imx1_get_group_pins(struct pinctrl_dev *pctldev, unsigned selector,
+			       const unsigned **pins,
+			       unsigned *npins)
+{
+	struct imx1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+	const struct imx_pinctrl_soc_info *info = ipctl->info;
+
+	if (selector >= info->ngroups)
+		return -EINVAL;
+
+	*pins = info->groups[selector].pins;
+	*npins = info->groups[selector].npins;
+
+	return 0;
+}
+
+static void imx1_pin_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
+		   unsigned offset)
+{
+	seq_printf(s, "%s", dev_name(pctldev->dev));
+}
+
+static int imx1_dt_node_to_map(struct pinctrl_dev *pctldev,
+			struct device_node *np,
+			struct pinctrl_map **map, unsigned *num_maps)
+{
+	struct imx1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+	const struct imx_pinctrl_soc_info *info = ipctl->info;
+	const struct imx_pin_group *grp;
+	struct pinctrl_map *new_map;
+	struct device_node *parent;
+	int map_num = 1;
+	int i, j;
+
+	/*
+	 * first find the group of this node and check if we need create
+	 * config maps for pins
+	 */
+	grp = imx1_pinctrl_find_group_by_name(info, np->name);
+	if (!grp) {
+		dev_err(info->dev, "unable to find group for node %s\n",
+			np->name);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < grp->npins; i++)
+		map_num++;
+
+	new_map = kmalloc(sizeof(struct pinctrl_map) * map_num, GFP_KERNEL);
+	if (!new_map)
+		return -ENOMEM;
+
+	*map = new_map;
+	*num_maps = map_num;
+
+	/* create mux map */
+	parent = of_get_parent(np);
+	if (!parent) {
+		kfree(new_map);
+		return -EINVAL;
+	}
+	new_map[0].type = PIN_MAP_TYPE_MUX_GROUP;
+	new_map[0].data.mux.function = parent->name;
+	new_map[0].data.mux.group = np->name;
+	of_node_put(parent);
+
+	/* create config map */
+	new_map++;
+	for (i = j = 0; i < grp->npins; i++) {
+		new_map[j].type = PIN_MAP_TYPE_CONFIGS_PIN;
+		new_map[j].data.configs.group_or_pin =
+				pin_get_name(pctldev, grp->pins[i]);
+		new_map[j].data.configs.configs = &grp->configs[i];
+		new_map[j].data.configs.num_configs = 1;
+		j++;
+	}
+
+	dev_dbg(pctldev->dev, "maps: function %s group %s num %d\n",
+		(*map)->data.mux.function, (*map)->data.mux.group, map_num);
+
+	return 0;
+}
+
+static void imx1_dt_free_map(struct pinctrl_dev *pctldev,
+				struct pinctrl_map *map, unsigned num_maps)
+{
+	kfree(map);
+}
+
+static const struct pinctrl_ops imx1_pctrl_ops = {
+	.get_groups_count = imx1_get_groups_count,
+	.get_group_name = imx1_get_group_name,
+	.get_group_pins = imx1_get_group_pins,
+	.pin_dbg_show = imx1_pin_dbg_show,
+	.dt_node_to_map = imx1_dt_node_to_map,
+	.dt_free_map = imx1_dt_free_map,
+
+};
+
+static int imx1_pmx_enable(struct pinctrl_dev *pctldev, unsigned selector,
+			   unsigned group)
+{
+	struct imx1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+	const struct imx_pinctrl_soc_info *info = ipctl->info;
+	const unsigned *pins, *mux;
+	unsigned int npins;
+	int i;
+
+	/*
+	 * Configure the mux mode for each pin in the group for a specific
+	 * function.
+	 */
+	pins = info->groups[group].pins;
+	npins = info->groups[group].npins;
+	mux = info->groups[group].mux_mode;
+
+	WARN_ON(!pins || !npins || !mux);
+
+	dev_dbg(ipctl->dev, "enable function %s group %s\n",
+		info->functions[selector].name, info->groups[group].name);
+
+	for (i = 0; i < npins; i++) {
+		unsigned int pin_id = pins[i];
+		unsigned int afunction = 0x001 & mux[i];
+		unsigned int gpio_in_use = (0x002 & mux[i]) >> 1;
+		unsigned int direction = (0x004 & mux[i]) >> 2;
+		unsigned int gpio_oconf = (0x030 & mux[i]) >> 4;
+		unsigned int gpio_iconfa = (0x300 & mux[i]) >> 8;
+		unsigned int gpio_iconfb = (0xc00 & mux[i]) >> 10;
+
+		dev_dbg(pctldev->dev, "%s, pin 0x%x, function %d, gpio %d, direction %d, oconf %d, iconfa %d, iconfb %d\n",
+				__func__, pin_id, afunction, gpio_in_use,
+				direction, gpio_oconf, gpio_iconfa,
+				gpio_iconfb);
+
+		imx1_write_bit(ipctl, pin_id, gpio_in_use, MX1_GIUS);
+		imx1_write_bit(ipctl, pin_id, direction, MX1_DDIR);
+
+		if (gpio_in_use) {
+			imx1_write_2bit(ipctl, pin_id, gpio_oconf, MX1_OCR);
+			imx1_write_2bit(ipctl, pin_id, gpio_iconfa,
+					MX1_ICONFA);
+			imx1_write_2bit(ipctl, pin_id, gpio_iconfb,
+					MX1_ICONFB);
+		} else {
+			imx1_write_bit(ipctl, pin_id, afunction, MX1_GPR);
+		}
+	}
+
+	return 0;
+}
+
+static int imx1_pmx_get_funcs_count(struct pinctrl_dev *pctldev)
+{
+	struct imx1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+	const struct imx_pinctrl_soc_info *info = ipctl->info;
+
+	return info->nfunctions;
+}
+
+static const char *imx1_pmx_get_func_name(struct pinctrl_dev *pctldev,
+					  unsigned selector)
+{
+	struct imx1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+	const struct imx_pinctrl_soc_info *info = ipctl->info;
+
+	return info->functions[selector].name;
+}
+
+static int imx1_pmx_get_groups(struct pinctrl_dev *pctldev, unsigned selector,
+			       const char * const **groups,
+			       unsigned * const num_groups)
+{
+	struct imx1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+	const struct imx_pinctrl_soc_info *info = ipctl->info;
+
+	*groups = info->functions[selector].groups;
+	*num_groups = info->functions[selector].num_groups;
+
+	return 0;
+}
+
+static const struct pinmux_ops imx1_pmx_ops = {
+	.get_functions_count = imx1_pmx_get_funcs_count,
+	.get_function_name = imx1_pmx_get_func_name,
+	.get_function_groups = imx1_pmx_get_groups,
+	.enable = imx1_pmx_enable,
+};
+
+static int imx1_pinconf_get(struct pinctrl_dev *pctldev,
+			     unsigned pin_id, unsigned long *config)
+{
+	struct imx1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+
+	*config = imx1_read_bit(ipctl, pin_id, MX1_PUEN);
+
+	return 0;
+}
+
+static int imx1_pinconf_set(struct pinctrl_dev *pctldev,
+			     unsigned pin_id, unsigned long config)
+{
+	struct imx1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+	const struct imx_pinctrl_soc_info *info = ipctl->info;
+
+	imx1_write_bit(ipctl, pin_id, config & 0x01, MX1_PUEN);
+
+	dev_dbg(ipctl->dev, "pinconf set pullup pin %s\n",
+		info->pins[pin_id].name);
+
+	return 0;
+}
+
+static void imx1_pinconf_dbg_show(struct pinctrl_dev *pctldev,
+				   struct seq_file *s, unsigned pin_id)
+{
+	struct imx1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+	const struct imx_pinctrl_soc_info *info = ipctl->info;
+	const struct imx_pin_reg *pin_reg = &info->pin_regs[pin_id];
+	unsigned long config;
+
+	if (!pin_reg || !pin_reg->conf_reg) {
+		seq_puts(s, "N/A");
+		return;
+	}
+
+	config = readl(ipctl->base + pin_reg->conf_reg);
+	seq_printf(s, "0x%lx", config);
+}
+
+static void imx1_pinconf_group_dbg_show(struct pinctrl_dev *pctldev,
+					 struct seq_file *s, unsigned group)
+{
+	struct imx1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+	const struct imx_pinctrl_soc_info *info = ipctl->info;
+	struct imx_pin_group *grp;
+	unsigned long config;
+	const char *name;
+	int i, ret;
+
+	if (group > info->ngroups)
+		return;
+
+	seq_puts(s, "\n");
+	grp = &info->groups[group];
+	for (i = 0; i < grp->npins; i++) {
+		name = pin_get_name(pctldev, grp->pins[i]);
+		ret = imx1_pinconf_get(pctldev, grp->pins[i], &config);
+		if (ret)
+			return;
+		seq_printf(s, "%s: 0x%lx", name, config);
+	}
+}
+
+static const struct pinconf_ops imx1_pinconf_ops = {
+	.pin_config_get = imx1_pinconf_get,
+	.pin_config_set = imx1_pinconf_set,
+	.pin_config_dbg_show = imx1_pinconf_dbg_show,
+	.pin_config_group_dbg_show = imx1_pinconf_group_dbg_show,
+};
+
+static struct pinctrl_desc imx1_pinctrl_desc = {
+	.pctlops = &imx1_pctrl_ops,
+	.pmxops = &imx1_pmx_ops,
+	.confops = &imx1_pinconf_ops,
+	.owner = THIS_MODULE,
+};
+
+static int imx1_pinctrl_parse_groups(struct device_node *np,
+				    struct imx_pin_group *grp,
+				    struct imx_pinctrl_soc_info *info,
+				    u32 index)
+{
+	int size;
+	const __be32 *list;
+	int i;
+	u32 config;
+
+	dev_dbg(info->dev, "group(%d): %s\n", index, np->name);
+
+	/* Initialise group */
+	grp->name = np->name;
+
+	/*
+	 * the binding format is fsl,pins = <PIN_FUNC_ID CONFIG ...>,
+	 * do sanity check and calculate pins number
+	 */
+	list = of_get_property(np, "fsl,pins", &size);
+	/* we do not check return since it's safe node passed down */
+	if (!size || size % 12) {
+		dev_notice(info->dev, "Not a valid fsl,pins property (%s)\n",
+				np->name);
+		return -EINVAL;
+	}
+
+	grp->npins = size / 12;
+	grp->pins = devm_kzalloc(info->dev, grp->npins * sizeof(unsigned int),
+				GFP_KERNEL);
+	grp->mux_mode = devm_kzalloc(info->dev,
+				grp->npins * sizeof(unsigned int), GFP_KERNEL);
+	grp->configs = devm_kzalloc(info->dev,
+				grp->npins * sizeof(unsigned long), GFP_KERNEL);
+	for (i = 0; i < grp->npins; i++) {
+		grp->pins[i] = be32_to_cpu(*list++);
+		grp->mux_mode[i] = be32_to_cpu(*list++);
+
+		config = be32_to_cpu(*list++);
+		grp->configs[i] = config;
+	}
+
+	return 0;
+}
+
+static int imx1_pinctrl_parse_functions(struct device_node *np,
+				       struct imx_pinctrl_soc_info *info,
+				       u32 index)
+{
+	struct device_node *child;
+	struct imx_pmx_func *func;
+	struct imx_pin_group *grp;
+	int ret;
+	static u32 grp_index;
+	u32 i = 0;
+
+	dev_dbg(info->dev, "parse function(%d): %s\n", index, np->name);
+
+	func = &info->functions[index];
+
+	/* Initialise function */
+	func->name = np->name;
+	func->num_groups = of_get_child_count(np);
+	if (func->num_groups <= 0)
+		return -EINVAL;
+
+	func->groups = devm_kzalloc(info->dev,
+			func->num_groups * sizeof(char *), GFP_KERNEL);
+
+	for_each_child_of_node(np, child) {
+		func->groups[i] = child->name;
+		grp = &info->groups[grp_index++];
+		ret = imx1_pinctrl_parse_groups(child, grp, info, i++);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int imx1_pinctrl_parse_gpio(struct platform_device *pdev,
+		struct imx1_pinctrl *pctl, struct device_node *np, int i,
+		u32 base)
+{
+	int ret;
+	u32 memoffset;
+
+	ret = of_property_read_u32(np, "reg", &memoffset);
+	if (ret)
+		return ret;
+
+	memoffset -= base;
+	pctl->gpio_pdata[i].base = pctl->base + memoffset;
+
+	pctl->gpio_dev[i] = of_device_alloc(np, NULL, &pdev->dev);
+	pctl->gpio_dev[i]->dev.platform_data = &pctl->gpio_pdata[i];
+	pctl->gpio_dev[i]->dev.bus = &platform_bus_type;
+
+	ret = of_device_add(pctl->gpio_dev[i]);
+	if (ret) {
+		dev_err(&pdev->dev,
+				"Failed to add child gpio device\n");
+		return ret;
+	}
+	return 0;
+}
+
+static int imx1_pinctrl_parse_dt(struct platform_device *pdev,
+		struct imx1_pinctrl *pctl, struct imx_pinctrl_soc_info *info)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *child;
+	int ret;
+	u32 nfuncs = 0;
+	u32 ngpios = 0;
+	u32 ngroups = 0;
+	u32 ifunc = 0;
+	u32 igpio = 0;
+	u32 base_addr;
+
+	if (!np)
+		return -ENODEV;
+
+	ret = of_property_read_u32(np, "reg", &base_addr);
+	if (ret)
+		return ret;
+
+	for_each_child_of_node(np, child) {
+		if (!of_device_is_compatible(child, "fsl,imx21-gpio") &&
+				!of_device_is_compatible(child, "fsl,imx1-gpio")) {
+			++nfuncs;
+			ngroups += of_get_child_count(child);
+		} else if (of_device_is_available(child)) {
+			++ngpios;
+		}
+	}
+
+	if (!nfuncs && !ngpios) {
+		dev_err(&pdev->dev, "No pin functions or gpio subdevices defined\n");
+		return -EINVAL;
+	}
+
+	info->nfunctions = nfuncs;
+	info->functions = devm_kzalloc(&pdev->dev,
+			nfuncs * sizeof(struct imx_pmx_func), GFP_KERNEL);
+
+	info->ngroups = ngroups;
+	info->groups = devm_kzalloc(&pdev->dev,
+			ngroups * sizeof(struct imx_pin_group), GFP_KERNEL);
+
+	pctl->num_gpio_childs = ngpios;
+	pctl->gpio_dev = devm_kzalloc(&pdev->dev,
+			sizeof(*pctl->gpio_dev) * ngpios, GFP_KERNEL);
+	pctl->gpio_pdata = devm_kzalloc(&pdev->dev,
+			sizeof(*pctl->gpio_pdata) * ngpios, GFP_KERNEL);
+
+	if (!info->functions || !info->groups || !pctl->gpio_dev ||
+			!pctl->gpio_pdata)
+		return -ENOMEM;
+
+	for_each_child_of_node(np, child) {
+		if (!of_device_is_compatible(child, "fsl,imx21-gpio") &&
+				!of_device_is_compatible(child, "fsl,imx1-gpio")) {
+			ret = imx1_pinctrl_parse_functions(child, info,
+					ifunc++);
+			if (ret)
+				goto child_err;
+		} else if (of_device_is_available(child)) {
+			ret = imx1_pinctrl_parse_gpio(pdev, pctl, child,
+					igpio++, base_addr);
+			if (ret)
+				goto child_err;
+		}
+	}
+
+	return 0;
+child_err:
+	while (igpio--)
+		put_device(&pctl->gpio_dev[igpio]->dev);
+	return ret;
+}
+
+int imx1_pinctrl_core_probe(struct platform_device *pdev,
+		      struct imx_pinctrl_soc_info *info)
+{
+	struct imx1_pinctrl *ipctl;
+	struct resource *res;
+	struct pinctrl_desc *pctl_desc;
+	int ret;
+
+	if (!info || !info->pins || !info->npins) {
+		dev_err(&pdev->dev, "wrong pinctrl info\n");
+		return -EINVAL;
+	}
+	info->dev = &pdev->dev;
+
+	/* Create state holders etc for this driver */
+	ipctl = devm_kzalloc(&pdev->dev, sizeof(*ipctl), GFP_KERNEL);
+	if (!ipctl)
+		return -ENOMEM;
+
+	info->pin_regs = devm_kzalloc(&pdev->dev, sizeof(*info->pin_regs) *
+				      info->npins, GFP_KERNEL);
+	if (!info->pin_regs)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENOENT;
+
+	ipctl->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(ipctl->base))
+		return PTR_ERR(ipctl->base);
+
+	pctl_desc = &imx1_pinctrl_desc;
+	pctl_desc->name = dev_name(&pdev->dev);
+	pctl_desc->pins = info->pins;
+	pctl_desc->npins = info->npins;
+
+	ret = imx1_pinctrl_parse_dt(pdev, ipctl, info);
+	if (ret) {
+		dev_err(&pdev->dev, "fail to probe dt properties\n");
+		return ret;
+	}
+
+	ipctl->info = info;
+	ipctl->dev = info->dev;
+	platform_set_drvdata(pdev, ipctl);
+	ipctl->pctl = pinctrl_register(pctl_desc, &pdev->dev, ipctl);
+	if (!ipctl->pctl) {
+		dev_err(&pdev->dev, "could not register IMX pinctrl driver\n");
+		return -EINVAL;
+	}
+
+	dev_info(&pdev->dev, "initialized IMX pinctrl driver\n");
+
+	return 0;
+}
+
+int imx1_pinctrl_core_remove(struct platform_device *pdev)
+{
+	struct imx1_pinctrl *ipctl = platform_get_drvdata(pdev);
+	int i;
+
+	for (i = 0; i != ipctl->num_gpio_childs; ++i)
+		put_device(&ipctl->gpio_dev[i]->dev);
+
+	pinctrl_unregister(ipctl->pctl);
+
+	return 0;
+}
diff --git a/drivers/pinctrl/pinctrl-imx1.h b/drivers/pinctrl/pinctrl-imx1.h
new file mode 100644
index 0000000..63161f1
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-imx1.h
@@ -0,0 +1,23 @@ 
+/*
+ * IMX pinmux core definitions
+ *
+ * Copyright (C) 2012 Freescale Semiconductor, Inc.
+ * Copyright (C) 2012 Linaro Ltd.
+ *
+ * Author: Dong Aisheng <[e-mail]>
+ *
+ * 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_IMX1_H
+#define __DRIVERS_PINCTRL_IMX1_H
+
+#include "pinctrl-imx.h"
+
+int imx1_pinctrl_core_probe(struct platform_device *pdev,
+			struct imx_pinctrl_soc_info *info);
+int imx1_pinctrl_core_remove(struct platform_device *pdev);
+#endif /* __DRIVERS_PINCTRL_IMX1_H */