diff mbox

[v3,10/13] of: add a generic pinmux helper

Message ID 1314315824-9687-11-git-send-email-swarren@nvidia.com (mailing list archive)
State New, archived
Headers show

Commit Message

Stephen Warren Aug. 25, 2011, 11:43 p.m. UTC
From: Jamie Iles <jamie@jamieiles.com>

This patch adds a helper function of_pinmux_parse() that can be used to
extract common pinmux configuration to avoid each platform implementing
a parsing loop.  Platforms supply the node containing the pinmux
definitions and a platform specific callback for configuring a pingroup.

Signed-off-by: Jamie Iles <jamie@jamieiles.com>
[swarren: Added support for pins property, added parse() callback, use
 dev_err instead of pr_err, related minor changes]
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 drivers/of/Kconfig        |    5 ++
 drivers/of/Makefile       |    1 +
 drivers/of/of_pinmux.c    |  109 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of_pinmux.h |   74 ++++++++++++++++++++++++++++++
 4 files changed, 189 insertions(+), 0 deletions(-)
 create mode 100644 drivers/of/of_pinmux.c
 create mode 100644 include/linux/of_pinmux.h

Comments

Jamie Iles Aug. 26, 2011, 9:16 a.m. UTC | #1
Hi Stephen,

On Thu, Aug 25, 2011 at 05:43:41PM -0600, Stephen Warren wrote:
> From: Jamie Iles <jamie@jamieiles.com>
> 
> This patch adds a helper function of_pinmux_parse() that can be used to
> extract common pinmux configuration to avoid each platform implementing
> a parsing loop.  Platforms supply the node containing the pinmux
> definitions and a platform specific callback for configuring a pingroup.
> 
> Signed-off-by: Jamie Iles <jamie@jamieiles.com>
> [swarren: Added support for pins property, added parse() callback, use
>  dev_err instead of pr_err, related minor changes]
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
>  drivers/of/Kconfig        |    5 ++
>  drivers/of/Makefile       |    1 +
>  drivers/of/of_pinmux.c    |  109 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/of_pinmux.h |   74 ++++++++++++++++++++++++++++++
>  4 files changed, 189 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/of/of_pinmux.c
>  create mode 100644 include/linux/of_pinmux.h
> 
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index cac63c9..71a19a3 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -81,4 +81,9 @@ config OF_PCI_IRQ
>  	help
>  	  OpenFirmware PCI IRQ routing helpers
>  
> +config OF_PINMUX
> +	def_bool y
> +	help
> +	  OpenFirmware pin multiplexing helpers
> +
>  endmenu # OF
> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
> index dccb117..0666ff3 100644
> --- a/drivers/of/Makefile
> +++ b/drivers/of/Makefile
> @@ -11,3 +11,4 @@ obj-$(CONFIG_OF_SPI)	+= of_spi.o
>  obj-$(CONFIG_OF_MDIO)	+= of_mdio.o
>  obj-$(CONFIG_OF_PCI)	+= of_pci.o
>  obj-$(CONFIG_OF_PCI_IRQ)  += of_pci_irq.o
> +obj-$(CONFIG_OF_PINMUX) += of_pinmux.o
> diff --git a/drivers/of/of_pinmux.c b/drivers/of/of_pinmux.c
> new file mode 100644
> index 0000000..8050b8b
> --- /dev/null
> +++ b/drivers/of/of_pinmux.c
> @@ -0,0 +1,109 @@
> +/*
> + * Copyright (c) 2011 Picochip Ltd., Jamie Iles
> + * Copyright (c) 2011 NVIDIA, Inc.
> + *
> + * Generic pinmux bindings for device tree.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +#include <linux/device.h>
> +#include <linux/errno.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/of.h>
> +#include <linux/of_iter_prop.h>
> +#include <linux/of_pinmux.h>
> +
> +/**
> + * of_pinmux_parse - configure a set of pinmux groups for a pinmux controller
> + *
> + * @controller: the controller to configure the pinmux entries for.  This
> + *	defines the controller device node and the callback for configuring
> + *	the pingroups.
> + *
> + * This helper loops over all of the child nodes of a pinmux controller and
> + * collects the configuration for each pinmux group.  A pinmux group is
> + * defined as one or more pins that are configured to a common function.  This
> + * handles common properties that many platforms may implement, but for
> + * platform specific properties these may be handled in the configure
> + * callback.
> + */
> +int of_pinmux_parse(const struct of_pinmux_ctrl *ctrl,
> +		    struct of_pinmux_cfg *cfg)
> +{
> +	struct device_node *np;
> +
> +	if (!ctrl || !ctrl->dev || !ctrl->node || !ctrl->configure)
> +		return -EINVAL;
> +
> +	for_each_child_of_node(ctrl->node, np) {
> +		int ret;
> +		bool hadpins = 0;
> +		struct of_iter_string_prop iter;
> +
> +		cfg->node = np;
> +
> +		ret = of_property_read_string(np, "function",
> +					      &cfg->function);
> +		if (ret < 0) {
> +			dev_err(ctrl->dev, "no function for node %s\n",
> +				np->name);
> +			continue;
> +		}
> +
> +		cfg->flags &= 0;

cfg->flags = 0?

> +
> +		if (of_find_property(np, "pull-up", NULL))
> +			cfg->flags |= OF_PINMUX_PULL_UP;
> +		if (of_find_property(np, "pull-down", NULL))
> +			cfg->flags |= OF_PINMUX_PULL_DOWN;
> +
> +		if ((cfg->flags & OF_PINMUX_PULL_MASK) ==
> +		    OF_PINMUX_PULL_MASK) {
> +			dev_warn(ctrl->dev, "node %s has both "
> +				 "pull-up and pull-down properties - "
> +				 "defaulting to no pull\n",
> +				 np->name);

For format strings I believe it's preferred to keep it on one long line 
so it can at least be grepped for.

> +			cfg->flags &= ~OF_PINMUX_PULL_MASK;
> +		}
> +
> +		if (of_find_property(np, "tristate", NULL))
> +			cfg->flags |= OF_PINMUX_TRISTATE;
> +
> +		if (ctrl->parse && ctrl->parse(ctrl, cfg)) {
> +			dev_warn(ctrl->dev,
> +				    "failed to parse node %s\n",
> +				    np->name);
> +			continue;
> +		}
> +
> +		for_each_string_property_value(iter, np, "pins") {
> +			hadpins = 1;
> +
> +			cfg->pin = iter.value;
> +
> +			dev_dbg(ctrl->dev,
> +				"configure pin %s func=%s flags=0x%lx\n",
> +				cfg->pin, cfg->function, cfg->flags);
> +			if (ctrl->configure(ctrl, cfg))
> +				dev_warn(ctrl->dev,
> +					 "failed to configure pin %s\n",
> +					 cfg->pin);
> +		}
> +
> +		if (!hadpins)
> +			dev_warn(ctrl->dev, "no pins for node %s\n",
> +				 np->name);
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(of_pinmux_parse);
> diff --git a/include/linux/of_pinmux.h b/include/linux/of_pinmux.h
> new file mode 100644
> index 0000000..7ccddcf
> --- /dev/null
> +++ b/include/linux/of_pinmux.h
> @@ -0,0 +1,74 @@
> +/*
> + * Copyright (c) 2011 Picochip Ltd., Jamie Iles
> + * Copyright (c) 2011 NVIDIA, Inc.
> + *
> + * Generic pinmux bindings for device tree.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +#ifndef __OF_PINMUX_H__
> +#define __OF_PINMUX_H__
> +
> +struct device_node;
> +
> +#define OF_PINMUX_PULL_UP	(1 << 0)
> +#define OF_PINMUX_PULL_DOWN	(1 << 1)
> +#define OF_PINMUX_TRISTATE	(1 << 2)
> +
> +#define OF_PINMUX_PULL_MASK	(OF_PINMUX_PULL_UP | OF_PINMUX_PULL_DOWN)
> +
> +/**
> + * struct of_pinmux_cfg - configuration state for a single pinmux entry.
> + *
> + * @function: the name of the function that the pinmux entry should be
> + *	configured to.
> + * @pin: the device_node of the pinmux entry that should be configured.
> + *	Platform specific properties that aren't in the generic binding may be
> + *	obtained from this device node.
> + * @flags: flags for common pinmux options such as pull and tristate.

The kerneldoc is out of date here.

> + */
> +struct of_pinmux_cfg {
> +	struct device_node	*node;
> +	const char		*pin;
> +	const char		*function;
> +	unsigned long		flags;
> +};
> +
> +/**
> + * struct of_pinmux_ctrl - platform specific pinmux control state.
> + *
> + * @pinmux: the pinmux device node.  All child nodes are required to be the
> + *	pinmux entry definitions.  Depending on the platform, this may either be
> + *	a single pin or a group of pins where they can be set to a common
> + *	function.
> + * @configure: platform specific callback to configure the pinmux entry.

and here.

> + */
> +struct of_pinmux_ctrl {
> +	struct device	   *dev;
> +	struct device_node *node;
> +	int		   (*parse)(const struct of_pinmux_ctrl *ctrl,
> +				    struct of_pinmux_cfg *cfg);
> +	int		   (*configure)(const struct of_pinmux_ctrl *ctrl,
> +					const struct of_pinmux_cfg *cfg);
> +};
> +
> +#ifdef CONFIG_OF
> +extern int of_pinmux_parse(const struct of_pinmux_ctrl *ctrl,
> +			   struct of_pinmux_cfg *cfg);
> +#else
> +static inline int of_pinmux_parse(const struct of_pinmux_ctrl *ctrl,
> +				  struct of_pinmux_cfg *cfg)
> +{
> +	return -ENOSYS;
> +}
> +#endif /* CONFIG_OF */
> +
> +#endif /* __OF_PINMUX_H__ */
> -- 
> 1.7.0.4
>
Linus Walleij Aug. 29, 2011, 11:09 a.m. UTC | #2
On Fri, Aug 26, 2011 at 1:43 AM, Stephen Warren <swarren@nvidia.com> wrote:

> diff --git a/drivers/of/of_pinmux.c b/drivers/of/of_pinmux.c

> +int of_pinmux_parse(const struct of_pinmux_ctrl *ctrl,
> +                   struct of_pinmux_cfg *cfg)

OK...

> +{
> +       struct device_node *np;
> +
> +       if (!ctrl || !ctrl->dev || !ctrl->node || !ctrl->configure)
> +               return -EINVAL;
> +
> +       for_each_child_of_node(ctrl->node, np) {
> +               int ret;
> +               bool hadpins = 0;
> +               struct of_iter_string_prop iter;
> +
> +               cfg->node = np;
> +
> +               ret = of_property_read_string(np, "function",
> +                                             &cfg->function);
> +               if (ret < 0) {
> +                       dev_err(ctrl->dev, "no function for node %s\n",
> +                               np->name);
> +                       continue;
> +               }

I buy this part.

> +
> +               cfg->flags &= 0;
> +
> +               if (of_find_property(np, "pull-up", NULL))
> +                       cfg->flags |= OF_PINMUX_PULL_UP;
> +               if (of_find_property(np, "pull-down", NULL))
> +                       cfg->flags |= OF_PINMUX_PULL_DOWN;
> +
> +               if ((cfg->flags & OF_PINMUX_PULL_MASK) ==
> +                   OF_PINMUX_PULL_MASK) {
> +                       dev_warn(ctrl->dev, "node %s has both "
> +                                "pull-up and pull-down properties - "
> +                                "defaulting to no pull\n",
> +                                np->name);
> +                       cfg->flags &= ~OF_PINMUX_PULL_MASK;
> +               }
> +
> +               if (of_find_property(np, "tristate", NULL))
> +                       cfg->flags |= OF_PINMUX_TRISTATE;

But what does this stuff has to do with pinmux?

I call this "pin biasing" and it has very little to do with muxing.

If a broader, generic term is to be used, I'd prefer "pin control"
which sort of nails the thing.

> +               for_each_string_property_value(iter, np, "pins") {
> +                       hadpins = 1;
> +
> +                       cfg->pin = iter.value;
> +
> +                       dev_dbg(ctrl->dev,
> +                               "configure pin %s func=%s flags=0x%lx\n",
> +                               cfg->pin, cfg->function, cfg->flags);
> +                       if (ctrl->configure(ctrl, cfg))
> +                               dev_warn(ctrl->dev,
> +                                        "failed to configure pin %s\n",
> +                                        cfg->pin);
> +               }
> +
> +               if (!hadpins)
> +                       dev_warn(ctrl->dev, "no pins for node %s\n",
> +                                np->name);
> +       }
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(of_pinmux_parse);

Renamed of_pinctrl_parse I'm happier with it.

> +/**
> + * struct of_pinmux_cfg - configuration state for a single pinmux entry.
> + *
> + * @function: the name of the function that the pinmux entry should be
> + *     configured to.
> + * @pin: the device_node of the pinmux entry that should be configured.
> + *     Platform specific properties that aren't in the generic binding may be
> + *     obtained from this device node.
> + * @flags: flags for common pinmux options such as pull and tristate.

I don't think these things has anything to do with pinmux at all.

But with the struct renamed of_pinctrl_cfg I'm again happier.

> + */
> +struct of_pinmux_cfg {
> +       struct device_node      *node;
> +       const char              *pin;
> +       const char              *function;
> +       unsigned long           flags;
> +};

The current pinctrl patch set would probably want an unsigned
"position" attribute too. (If we should build on that.)

> +/**
> + * struct of_pinmux_ctrl - platform specific pinmux control state.
> + *
> + * @pinmux: the pinmux device node.  All child nodes are required to be the
> + *     pinmux entry definitions.  Depending on the platform, this may either be
> + *     a single pin or a group of pins where they can be set to a common
> + *     function.
> + * @configure: platform specific callback to configure the pinmux entry.
> + */
> +struct of_pinmux_ctrl {
> +       struct device      *dev;
> +       struct device_node *node;
> +       int                (*parse)(const struct of_pinmux_ctrl *ctrl,
> +                                   struct of_pinmux_cfg *cfg);
> +       int                (*configure)(const struct of_pinmux_ctrl *ctrl,
> +                                       const struct of_pinmux_cfg *cfg);
> +};

s/pinmux/pinctrl/g

Yours,
Linus Walleij
Stephen Warren Aug. 29, 2011, 9:46 p.m. UTC | #3
Linus Walleij wrote at Monday, August 29, 2011 5:09 AM:
> On Fri, Aug 26, 2011 at 1:43 AM, Stephen Warren <swarren@nvidia.com> wrote:
> 
> > diff --git a/drivers/of/of_pinmux.c b/drivers/of/of_pinmux.c
> 
> > +int of_pinmux_parse(const struct of_pinmux_ctrl *ctrl,
> > +                   struct of_pinmux_cfg *cfg)
> 
> OK...
> 
> > +{
> > +       struct device_node *np;
> > +
> > +       if (!ctrl || !ctrl->dev || !ctrl->node || !ctrl->configure)
> > +               return -EINVAL;
> > +
> > +       for_each_child_of_node(ctrl->node, np) {
> > +               int ret;
> > +               bool hadpins = 0;
> > +               struct of_iter_string_prop iter;
> > +
> > +               cfg->node = np;
> > +
> > +               ret = of_property_read_string(np, "function",
> > +                                             &cfg->function);
> > +               if (ret < 0) {
> > +                       dev_err(ctrl->dev, "no function for node %s\n",
> > +                               np->name);
> > +                       continue;
> > +               }
> 
> I buy this part.
> 
> > +
> > +               cfg->flags &= 0;
> > +
> > +               if (of_find_property(np, "pull-up", NULL))
> > +                       cfg->flags |= OF_PINMUX_PULL_UP;
> > +               if (of_find_property(np, "pull-down", NULL))
> > +                       cfg->flags |= OF_PINMUX_PULL_DOWN;
> > +
> > +               if ((cfg->flags & OF_PINMUX_PULL_MASK) ==
> > +                   OF_PINMUX_PULL_MASK) {
> > +                       dev_warn(ctrl->dev, "node %s has both "
> > +                                "pull-up and pull-down properties - "
> > +                                "defaulting to no pull\n",
> > +                                np->name);
> > +                       cfg->flags &= ~OF_PINMUX_PULL_MASK;
> > +               }
> > +
> > +               if (of_find_property(np, "tristate", NULL))
> > +                       cfg->flags |= OF_PINMUX_TRISTATE;
> 
> But what does this stuff has to do with pinmux?
> 
> I call this "pin biasing" and it has very little to do with muxing.
> 
> If a broader, generic term is to be used, I'd prefer "pin control"
> which sort of nails the thing.
> 
> > +               for_each_string_property_value(iter, np, "pins") {
> > +                       hadpins = 1;
> > +
> > +                       cfg->pin = iter.value;
> > +
> > +                       dev_dbg(ctrl->dev,
> > +                               "configure pin %s func=%s flags=0x%lx\n",
> > +                               cfg->pin, cfg->function, cfg->flags);
> > +                       if (ctrl->configure(ctrl, cfg))
> > +                               dev_warn(ctrl->dev,
> > +                                        "failed to configure pin %s\n",
> > +                                        cfg->pin);
> > +               }
> > +
> > +               if (!hadpins)
> > +                       dev_warn(ctrl->dev, "no pins for node %s\n",
> > +                                np->name);
> > +       }
> > +
> > +       return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(of_pinmux_parse);
> 
> Renamed of_pinctrl_parse I'm happier with it.

Well, it's not just that; the struct contains the function field to
select for each pingroup, so it's not just pinctrl either.

> > +/**
> > + * struct of_pinmux_cfg - configuration state for a single pinmux entry.
> > + *
> > + * @function: the name of the function that the pinmux entry should be
> > + *     configured to.
> > + * @pin: the device_node of the pinmux entry that should be configured.
> > + *     Platform specific properties that aren't in the generic binding may be
> > + *     obtained from this device node.
> > + * @flags: flags for common pinmux options such as pull and tristate.
> 
> I don't think these things has anything to do with pinmux at all.
> 
> But with the struct renamed of_pinctrl_cfg I'm again happier.
> 
> > + */
> > +struct of_pinmux_cfg {
> > +       struct device_node      *node;
> > +       const char              *pin;
> > +       const char              *function;
> > +       unsigned long           flags;
> > +};
> 
> The current pinctrl patch set would probably want an unsigned
> "position" attribute too. (If we should build on that.)

This does beg the question I asked previously:

The intent of this patch was to provide a way for devicetree to set the
complete initial state of the pinmux, in a SoC-specific fashion, without
interaction with the pinmux API.

However, I'm guessing you see the pinmux driver as not touching HW at
boot at all, and never programming anything except in response to a
driver calling pinmux_get()/pinmux_enable()?

In the former case, this code and related bindings wouldn't be influenced
by the pinmux API at all (and indeed arguably never should be, since DT
is supposed to be oriented purely at HW, and not influenced by driver
design). This approach might make the pinmux API mostly relevant only
for drivers that actively change between configurations at run-time, and
not for drivers that don't need dynamic muxing.

However, in the latter case, this code should be part of the pinmux core,
and my patches to Tegra's pinmux driver dropped. It'd be difficult to
define a pinmux binding that wasn't influenced by the pinmux API's needs
in this case.

So, that's probably something we have to resolve first.
Linus Walleij Sept. 1, 2011, 11:30 a.m. UTC | #4
On Mon, Aug 29, 2011 at 11:46 PM, Stephen Warren <swarren@nvidia.com> wrote:

>> > +EXPORT_SYMBOL_GPL(of_pinmux_parse);
>>
>> Renamed of_pinctrl_parse I'm happier with it.
>
> Well, it's not just that; the struct contains the function field to
> select for each pingroup, so it's not just pinctrl either.

OK as of my thinking in the v6 pin control patch set groups
are an abstract concept that is part of pin control and pin
definitions (does not need to be used for muxing, could be
anything you want to do with pins) so that sorts it out I guess?

So in line with that terminology I think this should be
named pinctrl.

>> The current pinctrl patch set would probably want an unsigned
>> "position" attribute too. (If we should build on that.)
>
> This does beg the question I asked previously:
>
> The intent of this patch was to provide a way for devicetree to set the
> complete initial state of the pinmux, in a SoC-specific fashion, without
> interaction with the pinmux API.
>
> However, I'm guessing you see the pinmux driver as not touching HW at
> boot at all, and never programming anything except in response to a
> driver calling pinmux_get()/pinmux_enable()?

Since the pin control driver is (supposedly) the only driver that knows
about the register range dealing with muxing, all kind of fiddling
with these registers that are *necessary* on boot (e.g. to get the
thing up in a known state) need to be in the pincontrol driver.

So it may touch hardware on boot, i.e. write default values,
even though one may argue that the boot loader should have
taken care about such things, it's now always the case in
practice.

> In the former case, this code and related bindings wouldn't be influenced
> by the pinmux API at all (and indeed arguably never should be, since DT
> is supposed to be oriented purely at HW, and not influenced by driver
> design). This approach might make the pinmux API mostly relevant only
> for drivers that actively change between configurations at run-time, and
> not for drivers that don't need dynamic muxing.

Platforms that do not need dynamic muxing should be using it
if they need to configure default states of the pins at boot I think.

> However, in the latter case, this code should be part of the pinmux core,
> and my patches to Tegra's pinmux driver dropped. It'd be difficult to
> define a pinmux binding that wasn't influenced by the pinmux API's needs
> in this case.

Well that depends on what you want to do.

The patch subject seems to suggest that all SoCs should use this,
not just the Tegra driver. If it had the subject "of: add a tegra
pinmux helper" and functions named of_tegra_pinmux_parse() and
so on I wouldn't have any objections whatsoever.

If however you want to create a DT binding that should be used by
anyone (as indicated by the subject) it must use abstract concepts of
some kind and then I have opinions on how to name things and
some idea that I'd like them to correspond to what the pin control
subsystem will call things.

For example not call things dealing with biasing as part of
something called "pinmux", since in the pin controller
subsystem I'm envisioning I'm striving to separation of
concerns to the point where biasing and muxing are
completely orthogonal concepts, though they might be
using the same abstract concept of pin groups...

I don't know if I'm getting across properly, I'm trying
my best.

Thanks,
Linus Walleij
diff mbox

Patch

diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index cac63c9..71a19a3 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -81,4 +81,9 @@  config OF_PCI_IRQ
 	help
 	  OpenFirmware PCI IRQ routing helpers
 
+config OF_PINMUX
+	def_bool y
+	help
+	  OpenFirmware pin multiplexing helpers
+
 endmenu # OF
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index dccb117..0666ff3 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -11,3 +11,4 @@  obj-$(CONFIG_OF_SPI)	+= of_spi.o
 obj-$(CONFIG_OF_MDIO)	+= of_mdio.o
 obj-$(CONFIG_OF_PCI)	+= of_pci.o
 obj-$(CONFIG_OF_PCI_IRQ)  += of_pci_irq.o
+obj-$(CONFIG_OF_PINMUX) += of_pinmux.o
diff --git a/drivers/of/of_pinmux.c b/drivers/of/of_pinmux.c
new file mode 100644
index 0000000..8050b8b
--- /dev/null
+++ b/drivers/of/of_pinmux.c
@@ -0,0 +1,109 @@ 
+/*
+ * Copyright (c) 2011 Picochip Ltd., Jamie Iles
+ * Copyright (c) 2011 NVIDIA, Inc.
+ *
+ * Generic pinmux bindings for device tree.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/of_iter_prop.h>
+#include <linux/of_pinmux.h>
+
+/**
+ * of_pinmux_parse - configure a set of pinmux groups for a pinmux controller
+ *
+ * @controller: the controller to configure the pinmux entries for.  This
+ *	defines the controller device node and the callback for configuring
+ *	the pingroups.
+ *
+ * This helper loops over all of the child nodes of a pinmux controller and
+ * collects the configuration for each pinmux group.  A pinmux group is
+ * defined as one or more pins that are configured to a common function.  This
+ * handles common properties that many platforms may implement, but for
+ * platform specific properties these may be handled in the configure
+ * callback.
+ */
+int of_pinmux_parse(const struct of_pinmux_ctrl *ctrl,
+		    struct of_pinmux_cfg *cfg)
+{
+	struct device_node *np;
+
+	if (!ctrl || !ctrl->dev || !ctrl->node || !ctrl->configure)
+		return -EINVAL;
+
+	for_each_child_of_node(ctrl->node, np) {
+		int ret;
+		bool hadpins = 0;
+		struct of_iter_string_prop iter;
+
+		cfg->node = np;
+
+		ret = of_property_read_string(np, "function",
+					      &cfg->function);
+		if (ret < 0) {
+			dev_err(ctrl->dev, "no function for node %s\n",
+				np->name);
+			continue;
+		}
+
+		cfg->flags &= 0;
+
+		if (of_find_property(np, "pull-up", NULL))
+			cfg->flags |= OF_PINMUX_PULL_UP;
+		if (of_find_property(np, "pull-down", NULL))
+			cfg->flags |= OF_PINMUX_PULL_DOWN;
+
+		if ((cfg->flags & OF_PINMUX_PULL_MASK) ==
+		    OF_PINMUX_PULL_MASK) {
+			dev_warn(ctrl->dev, "node %s has both "
+				 "pull-up and pull-down properties - "
+				 "defaulting to no pull\n",
+				 np->name);
+			cfg->flags &= ~OF_PINMUX_PULL_MASK;
+		}
+
+		if (of_find_property(np, "tristate", NULL))
+			cfg->flags |= OF_PINMUX_TRISTATE;
+
+		if (ctrl->parse && ctrl->parse(ctrl, cfg)) {
+			dev_warn(ctrl->dev,
+				    "failed to parse node %s\n",
+				    np->name);
+			continue;
+		}
+
+		for_each_string_property_value(iter, np, "pins") {
+			hadpins = 1;
+
+			cfg->pin = iter.value;
+
+			dev_dbg(ctrl->dev,
+				"configure pin %s func=%s flags=0x%lx\n",
+				cfg->pin, cfg->function, cfg->flags);
+			if (ctrl->configure(ctrl, cfg))
+				dev_warn(ctrl->dev,
+					 "failed to configure pin %s\n",
+					 cfg->pin);
+		}
+
+		if (!hadpins)
+			dev_warn(ctrl->dev, "no pins for node %s\n",
+				 np->name);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_pinmux_parse);
diff --git a/include/linux/of_pinmux.h b/include/linux/of_pinmux.h
new file mode 100644
index 0000000..7ccddcf
--- /dev/null
+++ b/include/linux/of_pinmux.h
@@ -0,0 +1,74 @@ 
+/*
+ * Copyright (c) 2011 Picochip Ltd., Jamie Iles
+ * Copyright (c) 2011 NVIDIA, Inc.
+ *
+ * Generic pinmux bindings for device tree.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#ifndef __OF_PINMUX_H__
+#define __OF_PINMUX_H__
+
+struct device_node;
+
+#define OF_PINMUX_PULL_UP	(1 << 0)
+#define OF_PINMUX_PULL_DOWN	(1 << 1)
+#define OF_PINMUX_TRISTATE	(1 << 2)
+
+#define OF_PINMUX_PULL_MASK	(OF_PINMUX_PULL_UP | OF_PINMUX_PULL_DOWN)
+
+/**
+ * struct of_pinmux_cfg - configuration state for a single pinmux entry.
+ *
+ * @function: the name of the function that the pinmux entry should be
+ *	configured to.
+ * @pin: the device_node of the pinmux entry that should be configured.
+ *	Platform specific properties that aren't in the generic binding may be
+ *	obtained from this device node.
+ * @flags: flags for common pinmux options such as pull and tristate.
+ */
+struct of_pinmux_cfg {
+	struct device_node	*node;
+	const char		*pin;
+	const char		*function;
+	unsigned long		flags;
+};
+
+/**
+ * struct of_pinmux_ctrl - platform specific pinmux control state.
+ *
+ * @pinmux: the pinmux device node.  All child nodes are required to be the
+ *	pinmux entry definitions.  Depending on the platform, this may either be
+ *	a single pin or a group of pins where they can be set to a common
+ *	function.
+ * @configure: platform specific callback to configure the pinmux entry.
+ */
+struct of_pinmux_ctrl {
+	struct device	   *dev;
+	struct device_node *node;
+	int		   (*parse)(const struct of_pinmux_ctrl *ctrl,
+				    struct of_pinmux_cfg *cfg);
+	int		   (*configure)(const struct of_pinmux_ctrl *ctrl,
+					const struct of_pinmux_cfg *cfg);
+};
+
+#ifdef CONFIG_OF
+extern int of_pinmux_parse(const struct of_pinmux_ctrl *ctrl,
+			   struct of_pinmux_cfg *cfg);
+#else
+static inline int of_pinmux_parse(const struct of_pinmux_ctrl *ctrl,
+				  struct of_pinmux_cfg *cfg)
+{
+	return -ENOSYS;
+}
+#endif /* CONFIG_OF */
+
+#endif /* __OF_PINMUX_H__ */