diff mbox

[05/14] ARM: at91: add pinctrl support

Message ID 1344603731-32667-5-git-send-email-plagnioj@jcrosoft.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jean-Christophe PLAGNIOL-VILLARD Aug. 10, 2012, 1:02 p.m. UTC
This is also include the gpio controller as the IP share both.
Each soc will have to describe the SoC limitation and pin configuration via
DT.

This will allow to do not need to touch the C code when adding new SoC if the
IP version is supported.

Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 .../bindings/pinctrl/atmel,at91-pinctrl.txt        |   84 ++
 arch/arm/Kconfig                                   |    1 +
 arch/arm/mach-at91/board-dt.c                      |    2 -
 arch/arm/mach-at91/gpio.c                          |  165 +--
 drivers/pinctrl/Kconfig                            |    9 +
 drivers/pinctrl/Makefile                           |    1 +
 drivers/pinctrl/pinctrl-at91.c                     | 1448 ++++++++++++++++++++
 7 files changed, 1548 insertions(+), 162 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
 create mode 100644 drivers/pinctrl/pinctrl-at91.c

Comments

Richard Genoud Aug. 13, 2012, 2:53 p.m. UTC | #1
2012/8/10 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> This is also include the gpio controller as the IP share both.
> Each soc will have to describe the SoC limitation and pin configuration via
> DT.
>
> This will allow to do not need to touch the C code when adding new SoC if the
> IP version is supported.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
>  .../bindings/pinctrl/atmel,at91-pinctrl.txt        |   84 ++
>  arch/arm/Kconfig                                   |    1 +
>  arch/arm/mach-at91/board-dt.c                      |    2 -
>  arch/arm/mach-at91/gpio.c                          |  165 +--
>  drivers/pinctrl/Kconfig                            |    9 +
>  drivers/pinctrl/Makefile                           |    1 +
>  drivers/pinctrl/pinctrl-at91.c                     | 1448 ++++++++++++++++++++
>  7 files changed, 1548 insertions(+), 162 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>  create mode 100644 drivers/pinctrl/pinctrl-at91.c
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
> new file mode 100644
> index 0000000..0296ef4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
> @@ -0,0 +1,84 @@
> +* Atmel AT91 Pinmux Controller
> +
> +The AT91 Pinmux Controler, enables the IC
> +to share one PAD to several functional blocks. The sharing is done by
> +multiplexing the PAD input/output signals. For each PAD there are up to
> +8 muxing options (called periph modes). Since different modules require
> +different PAD settings (like pull up, keeper, etc) the contoller controls
> +also the PAD settings parameters.
> +
> +Please refer to pinctrl-bindings.txt in this directory for details of the
> +common pinctrl bindings used by client devices, including the meaning of the
> +phrase "pin configuration node".
> +
> +Atmel AT91 pin configuration node is a node of a group of pins which can be
> +used for a specific device or function. This node represents both mux and config
> +of the pins in that group. The 'pins' selects the function mode(also named pin
> +mode) this pin can work on and the 'config' configures various pad settings
> +such as pull-up, multi drive, etc.
> +
> +Required properties for iomux controller:
> +- compatible: "atmel,at91rm9200-pinctrl"
> +- atmel,mux-mask: array of mask (periph per bank) to describe if a pin can be
> +  configured in this periph mode. All the periph and bank need to be describe.
> +
> +Required properties for pin configuration node:
> +- atmel,pins: 4 integers array, represents a group of pins mux and config
> +  setting. The format is atmel,pins = <PIN_BANK PIN_BANK_NUM PERIPH CONFIG>.
> +  The PERIPH 0 means gpio.
> +
> +Bits used for CONFIG:
> +PULL_UP(1 << 0): indicate this pin need a pull up.
> +MULTIDRIVE(1 << 1): indicate this pin need to be configured as multidrive.
> +
> +NOTE:
> +Some requirements for using atmel,at91rm9200-pinctrl binding:
> +1. We have pin function node defined under at91 controller node to represent
> +   what pinmux functions this SoC supports.
> +2. The pin configuration node intends to work on a specific function should
> +   to be defined under that specific function node.
> +   The function node's name should represent well about what function
> +   this group of pins in this pin configuration node are working on.
> +3. The driver can use the function node's name and pin configuration node's
> +   name describe the pin function and group hierarchy.
> +   For example, Linux Iat91 pinctrl driver takes the function node's name
It seems to be a typo : Iat91

> +   as the function name and pin configuration node's name as group name to
> +   create the map table.
> +4. Each pin configuration node should have a phandle, devices can set pins
> +   configurations by referring to the phandle of that pin configuration node.
> +5. The gpio controller must be describe in the pinctrl simple-bus.
> +
> +Examples:
> +
> +pinctrl@fffff400 {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +       ranges;
> +       compatible = "atmel,at91rm9200-pinctrl", "simple-bus";
> +       reg = <0xfffff400 0x600>;
> +
> +       atmel,mux-mask = <
> +             /*    A         B     */
> +              0xffffffff 0xffc00c3b  /* pioA */
> +              0xffffffff 0x7fff3ccf  /* pioB */
> +              0xffffffff 0x007fffff  /* pioC */
> +             >;
> +
> +       /* shared pinctrl settings */
> +       dbgu {
> +               pinctrl_dbgu: dbgu-0 {
> +                       atmel,pins =
> +                               <1 14 0x1 0x0   /* PB14 periph A */
> +                                1 15 0x1 0x1>; /* PB15 periph with pullup */
> +               };
> +       };
> +};
> +
> +dbgu: serial@fffff200 {
> +       compatible = "atmel,at91sam9260-usart";
> +       reg = <0xfffff200 0x200>;
> +       interrupts = <1 4 7>;
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&pinctrl_dbgu>;
> +       status = "disabled";
> +};
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index e91c7cd..178a619 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -352,6 +352,7 @@ config ARCH_AT91
>         select CLKDEV_LOOKUP
>         select IRQ_DOMAIN
>         select NEED_MACH_IO_H if PCCARD
> +       select PINCTRL
>         help
>           This enables support for systems based on Atmel
>           AT91RM9200 and AT91SAM9* processors.
There's a problem with this: if CONFIG_PINCRTL is forced on ARCH_AT91
and PINCTRL_AT91 is not selected,
all calls to devm_pinctrl_get_select_default() will fail. => no
serial, no nand etc..
IMHO, CONFIG_PINCTRL should not be forced, otherwise it will break
configs that don't want pinctrl
Or, if pinctrl is the new and only way, PINCTRL_AT91 should also be
forced on ARCH_AT91

> diff --git a/arch/arm/mach-at91/board-dt.c b/arch/arm/mach-at91/board-dt.c
> index e8f45c4..3b6a948 100644
> --- a/arch/arm/mach-at91/board-dt.c
> +++ b/arch/arm/mach-at91/board-dt.c
> @@ -30,8 +30,6 @@
>  static const struct of_device_id irq_of_match[] __initconst = {
>
>         { .compatible = "atmel,at91rm9200-aic", .data = at91_aic_of_init },
> -       { .compatible = "atmel,at91rm9200-gpio", .data = at91_gpio_of_irq_setup },
> -       { .compatible = "atmel,at91sam9x5-gpio", .data = at91_gpio_of_irq_setup },
>         { /*sentinel*/ }
>  };
>
> diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
> index 8b476fd..67d7fab 100644
> --- a/arch/arm/mach-at91/gpio.c
> +++ b/arch/arm/mach-at91/gpio.c
> @@ -23,8 +23,6 @@
>  #include <linux/io.h>
>  #include <linux/irqdomain.h>
>  #include <linux/of_address.h>
> -#include <linux/of_irq.h>
> -#include <linux/of_gpio.h>
>
>  #include <asm/mach/irq.h>
>
> @@ -719,80 +717,6 @@ postcore_initcall(at91_gpio_debugfs_init);
>   */
>  static struct lock_class_key gpio_lock_class;
>
> -#if defined(CONFIG_OF)
> -static int at91_gpio_irq_map(struct irq_domain *h, unsigned int virq,
> -                                                       irq_hw_number_t hw)
> -{
> -       struct at91_gpio_chip   *at91_gpio = h->host_data;
> -
> -       irq_set_lockdep_class(virq, &gpio_lock_class);
> -
> -       /*
> -        * Can use the "simple" and not "edge" handler since it's
> -        * shorter, and the AIC handles interrupts sanely.
> -        */
> -       irq_set_chip_and_handler(virq, &gpio_irqchip,
> -                                handle_simple_irq);
> -       set_irq_flags(virq, IRQF_VALID);
> -       irq_set_chip_data(virq, at91_gpio);
> -
> -       return 0;
> -}
> -
> -static struct irq_domain_ops at91_gpio_ops = {
> -       .map    = at91_gpio_irq_map,
> -       .xlate  = irq_domain_xlate_twocell,
> -};
> -
> -int __init at91_gpio_of_irq_setup(struct device_node *node,
> -                                    struct device_node *parent)
> -{
> -       struct at91_gpio_chip   *prev = NULL;
> -       int                     alias_idx = of_alias_get_id(node, "gpio");
> -       struct at91_gpio_chip   *at91_gpio = &gpio_chip[alias_idx];
> -
> -       /* Setup proper .irq_set_type function */
> -       if (has_pio3())
> -               gpio_irqchip.irq_set_type = alt_gpio_irq_type;
> -       else
> -               gpio_irqchip.irq_set_type = gpio_irq_type;
> -
> -       /* Disable irqs of this PIO controller */
> -       __raw_writel(~0, at91_gpio->regbase + PIO_IDR);
> -
> -       /* Setup irq domain */
> -       at91_gpio->domain = irq_domain_add_linear(node, at91_gpio->chip.ngpio,
> -                                               &at91_gpio_ops, at91_gpio);
> -       if (!at91_gpio->domain)
> -               panic("at91_gpio.%d: couldn't allocate irq domain (DT).\n",
> -                       at91_gpio->pioc_idx);
> -
> -       /* Setup chained handler */
> -       if (at91_gpio->pioc_idx)
> -               prev = &gpio_chip[at91_gpio->pioc_idx - 1];
> -
> -       /* The toplevel handler handles one bank of GPIOs, except
> -        * on some SoC it can handles up to three...
> -        * We only set up the handler for the first of the list.
> -        */
> -       if (prev && prev->next == at91_gpio)
> -               return 0;
> -
> -       at91_gpio->pioc_virq = irq_create_mapping(irq_find_host(parent),
> -                                                       at91_gpio->pioc_hwirq);
> -       irq_set_chip_data(at91_gpio->pioc_virq, at91_gpio);
> -       irq_set_chained_handler(at91_gpio->pioc_virq, gpio_irq_handler);
> -
> -       return 0;
> -}
> -#else
> -int __init at91_gpio_of_irq_setup(struct device_node *node,
> -                                    struct device_node *parent)
> -{
> -       return -EINVAL;
> -}
> -#endif
> -
>  /*
>   * irqdomain initialization: pile up irqdomains on top of AIC range
>   */
> @@ -996,85 +920,6 @@ err:
>         return -EINVAL;
>  }
>
> -#ifdef CONFIG_OF_GPIO
> -static void __init of_at91_gpio_init_one(struct device_node *np)
> -{
> -       int alias_idx;
> -       struct at91_gpio_chip *at91_gpio;
> -       uint32_t ngpio;
> -
> -       if (!np)
> -               return;
> -
> -       alias_idx = of_alias_get_id(np, "gpio");
> -       if (alias_idx >= MAX_GPIO_BANKS) {
> -               pr_err("at91_gpio, failed alias idx(%d) > MAX_GPIO_BANKS(%d), ignoring.\n",
> -                                               alias_idx, MAX_GPIO_BANKS);
> -               return;
> -       }
> -
> -       at91_gpio = &gpio_chip[alias_idx];
> -       at91_gpio->chip.base = alias_idx * MAX_NB_GPIO_PER_BANK;
> -
> -       at91_gpio->regbase = of_iomap(np, 0);
> -       if (!at91_gpio->regbase) {
> -               pr_err("at91_gpio.%d, failed to map registers, ignoring.\n",
> -                                                               alias_idx);
> -               return;
> -       }
> -
> -       /* Get the interrupts property */
> -       if (of_property_read_u32(np, "interrupts", &at91_gpio->pioc_hwirq)) {
> -               pr_err("at91_gpio.%d, failed to get interrupts property, ignoring.\n",
> -                                                               alias_idx);
> -               goto ioremap_err;
> -       }
> -
> -       /* Get capabilities from compatibility property */
> -       if (of_device_is_compatible(np, "atmel,at91sam9x5-gpio"))
> -               at91_gpio_caps |= AT91_GPIO_CAP_PIO3;
> -
> -       if (!of_property_read_u32(np, "gpio-nb", &ngpio)) {
> -               if (ngpio >= MAX_NB_GPIO_PER_BANK)
> -                       pr_err("at91_gpio.%d, gpio-nb >= %d failback to %d\n",
> -                              alias_idx, MAX_NB_GPIO_PER_BANK, MAX_NB_GPIO_PER_BANK);
> -               else
> -                       at91_gpio->chip.ngpio = ngpio;
> -       }
> -
> -       /* Setup clock */
> -       if (at91_gpio_setup_clk(alias_idx))
> -               goto ioremap_err;
> -
> -       at91_gpio->chip.of_node = np;
> -       gpio_banks = max(gpio_banks, alias_idx + 1);
> -       at91_gpio->pioc_idx = alias_idx;
> -       return;
> -
> -ioremap_err:
> -       iounmap(at91_gpio->regbase);
> -}
> -
> -static int __init of_at91_gpio_init(void)
> -{
> -       struct device_node *np = NULL;
> -
> -       /*
> -        * This isn't ideal, but it gets things hooked up until this
> -        * driver is converted into a platform_device
> -        */
> -       for_each_compatible_node(np, NULL, "atmel,at91rm9200-gpio")
> -               of_at91_gpio_init_one(np);
> -
> -       return gpio_banks > 0 ? 0 : -EINVAL;
> -}
> -#else
> -static int __init of_at91_gpio_init(void)
> -{
> -       return -EINVAL;
> -}
> -#endif
> -
>  static void __init at91_gpio_init_one(int idx, u32 regbase, int pioc_hwirq)
>  {
>         struct at91_gpio_chip *at91_gpio = &gpio_chip[idx];
> @@ -1109,11 +954,11 @@ void __init at91_gpio_init(struct at91_gpio_bank *data, int nr_banks)
>
>         BUG_ON(nr_banks > MAX_GPIO_BANKS);
>
> -       if (of_at91_gpio_init() < 0) {
> -               /* No GPIO controller found in device tree */
> -               for (i = 0; i < nr_banks; i++)
> -                       at91_gpio_init_one(i, data[i].regbase, data[i].id);
> -       }
> +       if (of_have_populated_dt())
> +               return;
> +
> +       for (i = 0; i < nr_banks; i++)
> +               at91_gpio_init_one(i, data[i].regbase, data[i].id);
>
>         for (i = 0; i < gpio_banks; i++) {
>                 at91_gpio = &gpio_chip[i];
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index 54e3588..953932a 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -26,6 +26,15 @@ config DEBUG_PINCTRL
>         help
>           Say Y here to add some extra checks and diagnostics to PINCTRL calls.
>
> +config PINCTRL_AT91
> +       bool "AT91 pinctrl driver"
> +       depends on OF
> +       depends on ARCH_AT91
> +       select PINMUX
> +       select PINCONF
> +       help
> +         Say Y here to enable the at91 pinctrl driver
> +
>  config PINCTRL_IMX
>         bool
>         select PINMUX
> diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
> index f40b1f8..d3fda00 100644
> --- a/drivers/pinctrl/Makefile
> +++ b/drivers/pinctrl/Makefile
> @@ -9,6 +9,7 @@ ifeq ($(CONFIG_OF),y)
>  obj-$(CONFIG_PINCTRL)          += devicetree.o
>  endif
>  obj-$(CONFIG_GENERIC_PINCONF)  += pinconf-generic.o
> +obj-$(CONFIG_PINCTRL_AT91)     += pinctrl-at91.o
>  obj-$(CONFIG_PINCTRL_IMX)      += pinctrl-imx.o
>  obj-$(CONFIG_PINCTRL_IMX51)    += pinctrl-imx51.o
>  obj-$(CONFIG_PINCTRL_IMX53)    += pinctrl-imx53.o
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> new file mode 100644
> index 0000000..540943b
> --- /dev/null
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -0,0 +1,1448 @@
> +/*
> + * at91 pinctrl driver based on at91 pinmux core
> + *
> + * Copyright (C) 2011-2012 Jean-Christophe PLAGNIOL-VILLARD
> + * <plagnioj@jcrosoft.com>
> + *
> + * Under GPLv2 only
> + */
> +
> +#define DEBUG
Please remove that, it's already handled in CONFIG_DEBUG_PINCTRL option.

> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/slab.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/io.h>
> +#include <linux/gpio.h>
> +#include <linux/irqdomain.h>
> +#include <linux/pinctrl/machine.h>
> +#include <linux/pinctrl/pinconf.h>
> +#include <linux/pinctrl/pinctrl.h>
> +#include <linux/pinctrl/pinmux.h>
> +/* Since we request GPIOs from ourself */
> +#include <linux/pinctrl/consumer.h>
> +
> +#include <asm/mach/irq.h>
> +
> +#include <mach/hardware.h>
> +#include <mach/at91_pio.h>
> +
> +#include "core.h"
> +
> +#define MAX_NB_GPIO_PER_BANK   32
This can be factorized with the same define in arch/arm/mach-at91/gpio.c

> +
> +struct at91_gpio_chip {
> +       struct gpio_chip        chip;
> +       struct pinctrl_gpio_range range;
> +       struct at91_gpio_chip   *next;          /* Bank sharing same clock */
> +       int                     pioc_hwirq;     /* PIO bank interrupt identifier on AIC */
> +       int                     pioc_virq;      /* PIO bank Linux virtual interrupt */
> +       int                     pioc_idx;       /* PIO bank index */
> +       void __iomem            *regbase;       /* PIO bank virtual address */
> +       struct clk              *clock;         /* associated clock */
> +       struct irq_domain       *domain;        /* associated irq domain */
> +};
> +
> +#define to_at91_gpio_chip(c) container_of(c, struct at91_gpio_chip, chip)
> +
> +static struct at91_gpio_chip *gpio_chip[MAX_GPIO_BANKS];
> +
> +static int gpio_banks;
> +static unsigned long at91_gpio_caps;
> +
> +/* All PIO controllers support PIO3 features */
> +#define AT91_GPIO_CAP_PIO3     (1 <<  0)
> +
> +#define has_pio3()     (at91_gpio_caps & AT91_GPIO_CAP_PIO3)
> +
> +
> +#define PULL_UP                (0 << 1)
> +#define MULTI_DRIVE    (1 << 1)
> +
> +/**
> + * struct at91_pmx_func - describes AT91 pinmux functions
> + * @name: the name of this specific function
> + * @groups: corresponding pin groups
> + * @ngroups: the number of groups
> + */
> +struct at91_pmx_func {
> +       const char *name;
> +       const char **groups;
> +       unsigned ngroups;
> +};
> +
> +struct at91_pmx_pin {
> +       uint32_t bank;
> +       uint32_t pin;
> +       uint32_t mux;
> +       uint32_t pullup;
> +       unsigned long conf;
> +};
> +
> +/**
> + * struct at91_pin_group - describes an At91 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
> + * @npins: the number of pins in this group array, i.e. the number of
> + *     elements in .pins so we can iterate over that array
> + */
> +struct at91_pin_group {
> +       const char *name;
> +       struct at91_pmx_pin *pins_conf;
> +       unsigned int *pins;
> +       unsigned npins;
> +};
> +
> +struct at91_pinctrl {
> +       struct device *dev;
> +       struct pinctrl_dev *pctl;
> +
> +       int nbanks;
> +
> +       int nmux;
> +       uint32_t *mux_mask;
> +
> +       struct at91_pmx_func *functions;
> +       int nfunctions;
> +
> +       struct at91_pin_group *groups;
> +       int ngroups;
> +
> +       void (*set_A_periph)(void __iomem *pio, unsigned mask);
> +       void (*set_B_periph)(void __iomem *pio, unsigned mask);
> +};
> +
> +static const inline struct at91_pin_group *at91_pinctrl_find_group_by_name(
> +                               const struct at91_pinctrl *info,
> +                               const char *name)
> +{
> +       const struct at91_pin_group *grp = NULL;
> +       int i;
> +
> +       for (i = 0; i < info->ngroups; i++) {
> +               if (strcmp(info->groups[i].name, name))
> +                       continue;
> +
> +               grp = &info->groups[i];
> +               dev_dbg(info->dev, "%s: %d 0:%d\n", name, grp->npins, grp->pins[0]);
> +               break;
> +       }
> +
> +       return grp;
> +}
> +
> +static int at91_get_groups_count(struct pinctrl_dev *pctldev)
> +{
> +       struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
> +
> +       return info->ngroups;
> +}
> +
> +static const char *at91_get_group_name(struct pinctrl_dev *pctldev,
> +                                      unsigned selector)
> +{
> +       struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
> +
> +       return info->groups[selector].name;
> +}
> +
> +static int at91_get_group_pins(struct pinctrl_dev *pctldev, unsigned selector,
> +                              const unsigned **pins,
> +                              unsigned *npins)
> +{
> +       struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
> +
> +       if (selector >= info->ngroups)
> +               return -EINVAL;
> +
> +       *pins = info->groups[selector].pins;
> +       *npins = info->groups[selector].npins;
> +
> +       return 0;
> +}
> +
> +static void at91_pin_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
> +                  unsigned offset)
> +{
> +       seq_printf(s, "%s", dev_name(pctldev->dev));
> +}
> +
> +static int at91_dt_node_to_map(struct pinctrl_dev *pctldev,
> +                       struct device_node *np,
> +                       struct pinctrl_map **map, unsigned *num_maps)
> +{
> +       struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
> +       const struct at91_pin_group *grp;
> +       struct pinctrl_map *new_map;
> +       struct device_node *parent;
> +       int map_num = 1;
> +       int i;
> +       struct at91_pmx_pin *pin;
> +
> +       /*
> +        * first find the group of this node and check if we need create
> +        * config maps for pins
> +        */
> +       grp = at91_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;
> +       }
> +
> +       map_num += grp->npins;
> +       new_map = kmalloc(sizeof(*new_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 = 0; i < grp->npins; i++) {
> +               pin = &grp->pins_conf[i];
> +
> +               new_map[i].type = PIN_MAP_TYPE_CONFIGS_PIN;
> +               new_map[i].data.configs.group_or_pin =
> +                               pin_get_name(pctldev, grp->pins[i]);
> +               new_map[i].data.configs.configs = &grp->pins_conf[i].conf;
> +               new_map[i].data.configs.num_configs = 1;
> +       }
> +
> +       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 at91_dt_free_map(struct pinctrl_dev *pctldev,
> +                               struct pinctrl_map *map, unsigned num_maps)
> +{
> +       kfree(map);
> +}
> +
> +static struct pinctrl_ops at91_pctrl_ops = {
> +       .get_groups_count = at91_get_groups_count,
> +       .get_group_name = at91_get_group_name,
> +       .get_group_pins = at91_get_group_pins,
> +       .pin_dbg_show = at91_pin_dbg_show,
> +       .dt_node_to_map = at91_dt_node_to_map,
> +       .dt_free_map = at91_dt_free_map,
> +
> +};
> +
> +static void __iomem * pin_to_controller(struct at91_pinctrl *info,
> +                                unsigned int bank)
> +{
> +       return gpio_chip[bank]->regbase;
> +}
> +
> +static inline int pin_to_bank(unsigned pin)
> +{
> +       return pin /= MAX_NB_GPIO_PER_BANK;
> +}
> +
> +static unsigned pin_to_mask(unsigned int pin)
> +{
> +       return 1 << pin;
> +}
> +
> +static void at91_mux_disable_interrupt(void __iomem *pio, unsigned mask)
> +{
> +       __raw_writel(mask, pio + PIO_IDR);
> +}
> +
> +static unsigned at91_mux_get_pullup(void __iomem *pio, unsigned pin)
> +{
> +       return (__raw_readl(pio + PIO_PUSR) >> pin) & 0x1;
> +}
> +
> +static void at91_mux_set_pullup(void __iomem *pio, unsigned mask, unsigned on)
> +{
> +       __raw_writel(mask, pio + (on ? PIO_PUER : PIO_PUDR));
> +}
> +
> +static unsigned at91_mux_get_multidrive(void __iomem *pio, unsigned pin)
> +{
> +       return (__raw_readl(pio + PIO_MDSR) >> pin) & 0x1;
> +}
> +
> +static void at91_mux_set_multidrive(void __iomem *pio, unsigned mask, unsigned on)
> +{
> +       __raw_writel(mask, pio + (on ? PIO_MDER : PIO_MDDR));
> +}
> +
> +static void at91_mux_pio3_set_A_periph(void __iomem *pio, unsigned mask)
> +{
> +
> +       __raw_writel(__raw_readl(pio + PIO_ABCDSR1) & ~mask,
> +                                               pio + PIO_ABCDSR1);
> +       __raw_writel(__raw_readl(pio + PIO_ABCDSR2) & ~mask,
> +                                               pio + PIO_ABCDSR2);
> +}
> +
> +static void at91_mux_pio3_set_B_periph(void __iomem *pio, unsigned mask)
> +{
> +       __raw_writel(__raw_readl(pio + PIO_ABCDSR1) | mask,
> +                                               pio + PIO_ABCDSR1);
> +       __raw_writel(__raw_readl(pio + PIO_ABCDSR2) & ~mask,
> +                                               pio + PIO_ABCDSR2);
> +}
> +
> +static void at91_mux_set_A_periph(void __iomem *pio, unsigned mask)
> +{
> +       __raw_writel(mask, pio + PIO_ASR);
> +}
> +
> +static void at91_mux_set_B_periph(void __iomem *pio, unsigned mask)
> +{
> +       __raw_writel(mask, pio + PIO_BSR);
> +}
> +
> +static void at91_mux_set_C_periph(void __iomem *pio, unsigned mask)
> +{
> +       __raw_writel(__raw_readl(pio + PIO_ABCDSR1) & ~mask, pio + PIO_ABCDSR1);
> +       __raw_writel(__raw_readl(pio + PIO_ABCDSR2) | mask, pio + PIO_ABCDSR2);
> +}
> +
> +static void at91_mux_set_D_periph(void __iomem *pio, unsigned mask)
> +{
> +       __raw_writel(__raw_readl(pio + PIO_ABCDSR1) | mask, pio + PIO_ABCDSR1);
> +       __raw_writel(__raw_readl(pio + PIO_ABCDSR2) | mask, pio + PIO_ABCDSR2);
> +}
> +
> +static void at91_pin_dbg(const struct device *dev, const struct at91_pmx_pin *pin)
> +{
> +       if (pin->mux) {
> +               dev_dbg(dev, "pio%c%d configured as periph%c with conf = 0x%lu\n",
> +                       pin->bank + 'A', pin->pin, pin->mux - 1 + 'A', pin->conf);
> +       } else {
> +               dev_dbg(dev, "pio%c%d configured as gpio with conf = 0x%lu\n",
> +                       pin->bank + 'A', pin->pin, pin->conf);
> +       }
> +}
> +
> +static int pin_check_config(struct at91_pinctrl *info, const char* name,
> +                           int index, const struct at91_pmx_pin *pin)
> +{
> +       int mux;
> +
> +       /* check if it's a valid config */
> +       if (pin->bank >= info->nbanks) {
> +               dev_err(info->dev, "%s: pin conf %d bank_id %d >= nbanks %d\n",
> +                       name, index, pin->bank, info->nbanks);
> +               return -EINVAL;
> +       }
> +
> +       if (pin->pin >= 32) {
> +               dev_err(info->dev, "%s: pin conf %d pin_bank_id %d >= 32\n",
> +                       name, index, pin->pin);
> +               return -EINVAL;
> +       }
> +
> +       if (!pin->mux)
> +               return 0;
> +
> +       mux = pin->mux - 1;
> +
> +       if (mux >= info->nmux) {
> +               dev_err(info->dev, "%s: pin conf %d mux_id %d >= nmux %d\n",
> +                       name, index, mux, info->nmux);
> +               return -EINVAL;
> +       }
> +
> +       if (!(info->mux_mask[pin->bank * info->nmux + mux] & 1 << pin->pin)) {
> +               dev_err(info->dev, "%s: pin conf %d mux_id %d not supported for pio%c%d\n",
> +                       name, index, mux, pin->bank + 'A', pin->pin);
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +
> +static void at91_mux_gpio_disable(void __iomem *pio, unsigned mask)
> +{
> +       __raw_writel(mask, pio + PIO_PDR);
> +}
> +
> +static void at91_mux_gpio_enable(void __iomem *pio, unsigned mask, unsigned input)
> +{
> +       __raw_writel(mask, pio + PIO_PER);
> +       __raw_writel(mask, pio + (input ? PIO_ODR : PIO_OER));
> +}
> +
> +static int at91_pmx_enable(struct pinctrl_dev *pctldev, unsigned selector,
> +                          unsigned group)
> +{
> +       struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
> +       const struct at91_pmx_pin *pins_conf = info->groups[group].pins_conf;
> +       const struct at91_pmx_pin *pin;
> +       uint32_t npins = info->groups[group].npins;
> +       int i, ret;
> +       unsigned mask;
> +       void __iomem *pio;
> +
> +       dev_dbg(info->dev, "enable function %s group %s\n",
> +               info->functions[selector].name, info->groups[group].name);
> +
> +       /* first check that all the pins of the group are valid with a valid
> +        * paramter */
> +       for (i = 0; i < npins; i++) {
> +               pin = &pins_conf[i];
> +               ret = pin_check_config(info, info->groups[group].name, i, pin);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       for (i = 0; i < npins; i++) {
> +               pin = &pins_conf[i];
> +               at91_pin_dbg(info->dev, pin);
> +               pio = pin_to_controller(info, pin->bank);
> +               mask = pin_to_mask(pin->pin);
> +               at91_mux_disable_interrupt(pio, mask);
> +               switch(pin->mux) {
> +               case 0:
> +                       at91_mux_gpio_enable(pio, mask, 1);
> +                       break;
> +               case 1:
> +                       info->set_A_periph(pio, mask);
> +                       break;
> +               case 2:
> +                       info->set_B_periph(pio, mask);
> +                       break;
> +               case 3:
> +                       at91_mux_set_C_periph(pio, mask);
> +                       break;
> +               case 4:
> +                       at91_mux_set_D_periph(pio, mask);
> +                       break;
> +               }
> +               if (pin->mux)
> +                       at91_mux_gpio_disable(pio, mask);
> +       }
> +
> +       return 0;
> +}
> +
> +static void at91_pmx_disable(struct pinctrl_dev *pctldev, unsigned selector,
> +                          unsigned group)
> +{
> +       struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
> +       const struct at91_pmx_pin *pins_conf = info->groups[group].pins_conf;
> +       const struct at91_pmx_pin *pin;
> +       uint32_t npins = info->groups[group].npins;
> +       int i;
> +       unsigned mask;
> +       void __iomem *pio;
> +
> +       for (i = 0; i < npins; i++) {
> +               pin = &pins_conf[i];
> +               at91_pin_dbg(info->dev, pin);
> +               pio = pin_to_controller(info, pin->bank);
> +               mask = pin_to_mask(pin->pin);
> +               at91_mux_gpio_enable(pio, mask, 1);
> +       }
> +}
> +
> +static int at91_pmx_get_funcs_count(struct pinctrl_dev *pctldev)
> +{
> +       struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
> +
> +       return info->nfunctions;
> +}
> +
> +static const char *at91_pmx_get_func_name(struct pinctrl_dev *pctldev,
> +                                         unsigned selector)
> +{
> +       struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
> +
> +       return info->functions[selector].name;
> +}
> +
> +static int at91_pmx_get_groups(struct pinctrl_dev *pctldev, unsigned selector,
> +                              const char * const **groups,
> +                              unsigned * const num_groups)
> +{
> +       struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
> +
> +       *groups = info->functions[selector].groups;
> +       *num_groups = info->functions[selector].ngroups;
> +
> +       return 0;
> +}
> +
> +int at91_gpio_request_enable(struct pinctrl_dev *pctldev,
> +                           struct pinctrl_gpio_range *range,
> +                           unsigned offset)
> +{
> +       struct at91_pinctrl *npct = pinctrl_dev_get_drvdata(pctldev);
> +       struct at91_gpio_chip *at91_chip;
> +       struct gpio_chip *chip;
> +       unsigned mask;
> +
> +       if (!range) {
> +               dev_err(npct->dev, "invalid range\n");
> +               return -EINVAL;
> +       }
> +       if (!range->gc) {
> +               dev_err(npct->dev, "missing GPIO chip in range\n");
> +               return -EINVAL;
> +       }
> +       chip = range->gc;
> +       at91_chip = container_of(chip, struct at91_gpio_chip, chip);
> +
> +       dev_dbg(npct->dev, "enable pin %u as GPIO\n", offset);
> +
> +       mask = 1 << (offset - chip->base);
> +
> +       dev_dbg(npct->dev, "enable pin %u as PIO%c%d 0x%x\n",
> +               offset, 'A' + range->id, offset - chip->base, mask);
> +
> +       __raw_writel(mask, at91_chip->regbase + PIO_PER);
> +
> +       return 0;
> +}
> +
> +void at91_gpio_disable_free(struct pinctrl_dev *pctldev,
> +                          struct pinctrl_gpio_range *range,
> +                          unsigned offset)
> +{
> +       struct at91_pinctrl *npct = pinctrl_dev_get_drvdata(pctldev);
> +
> +       dev_dbg(npct->dev, "disable pin %u as GPIO\n", offset);
> +       /* Set the pin to some default state, GPIO is usually default */
> +}
> +
> +static struct pinmux_ops at91_pmx_ops = {
> +       .get_functions_count = at91_pmx_get_funcs_count,
> +       .get_function_name = at91_pmx_get_func_name,
> +       .get_function_groups = at91_pmx_get_groups,
> +       .enable = at91_pmx_enable,
> +       .disable = at91_pmx_disable,
> +       .gpio_request_enable = at91_gpio_request_enable,
> +       .gpio_disable_free = at91_gpio_disable_free,
> +};
> +
> +static int at91_pinconf_get(struct pinctrl_dev *pctldev,
> +                            unsigned pin_id, unsigned long *config)
> +{
> +       struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
> +       void __iomem *pio;
> +       unsigned pin;
> +
> +       dev_dbg(info->dev, "%s:%d, pin_id=%d, config=0x%lx", __func__, __LINE__, pin_id, *config);
> +       pio = pin_to_controller(info, pin_to_bank(pin_id));
> +       pin = pin_id % MAX_NB_GPIO_PER_BANK;
> +
> +       if (at91_mux_get_multidrive(pio, pin))
> +               *config |= MULTI_DRIVE;
> +
> +       if (at91_mux_get_pullup(pio, pin))
> +               *config |= PULL_UP;
> +
> +       return 0;
> +}
> +
> +static int at91_pinconf_set(struct pinctrl_dev *pctldev,
> +                            unsigned pin_id, unsigned long config)
> +{
> +       struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
> +       unsigned mask;
> +       void __iomem *pio;
> +
> +       dev_dbg(info->dev, "%s:%d, pin_id=%d, config=0x%lx", __func__, __LINE__, pin_id, config);
> +       pio = pin_to_controller(info, pin_to_bank(pin_id));
> +       mask = pin_to_mask(pin_id % MAX_NB_GPIO_PER_BANK);
> +
> +       at91_mux_set_pullup(pio, mask, config & PULL_UP);
> +       at91_mux_set_multidrive(pio, mask, config & MULTI_DRIVE);
> +       return 0;
> +}
> +
> +static void at91_pinconf_dbg_show(struct pinctrl_dev *pctldev,
> +                                  struct seq_file *s, unsigned pin_id)
> +{
> +
> +}
> +
> +static void at91_pinconf_group_dbg_show(struct pinctrl_dev *pctldev,
> +                                        struct seq_file *s, unsigned group)
> +{
> +}
> +
> +struct pinconf_ops at91_pinconf_ops = {
> +       .pin_config_get = at91_pinconf_get,
> +       .pin_config_set = at91_pinconf_set,
> +       .pin_config_dbg_show = at91_pinconf_dbg_show,
> +       .pin_config_group_dbg_show = at91_pinconf_group_dbg_show,
> +};
> +
> +static struct pinctrl_desc at91_pinctrl_desc = {
> +       .pctlops = &at91_pctrl_ops,
> +       .pmxops = &at91_pmx_ops,
> +       .confops = &at91_pinconf_ops,
> +       .owner = THIS_MODULE,
> +};
> +
> +static const char *gpio_compat = "atmel,at91rm9200-gpio";
> +
> +static void __devinit at91_pinctrl_child_count(struct at91_pinctrl *info,
> +                                             struct device_node *np)
> +{
> +       struct device_node *child;
> +
> +       for_each_child_of_node(np, child) {
> +               if (of_device_is_compatible(child, gpio_compat)) {
> +                       info->nbanks++;
> +               } else {
> +                       info->nfunctions++;
> +                       info->ngroups += of_get_child_count(child);
> +               }
> +       }
> +}
> +
> +static int __devinit at91_pinctrl_mux_mask(struct at91_pinctrl *info,
> +                                         struct device_node *np)
> +{
> +       int ret = 0;
> +       int size;
> +       const const __be32 *list;
> +
> +       list = of_get_property(np, "atmel,mux-mask", &size);
> +       if (!list) {
> +               dev_err(info->dev, "can not read the mux-mask of %d\n", size);
> +               return -EINVAL;
> +       }
> +
> +       size /= sizeof(*list);
> +       if (!size || size % info->nbanks) {
> +               dev_err(info->dev, "wrong mux mask array should be by %d\n", info->nbanks);
> +               return -EINVAL;
> +       }
> +       info->nmux = size / info->nbanks;
> +
> +       info->mux_mask = devm_kzalloc(info->dev, sizeof(u32) * size, GFP_KERNEL);
> +       if (!info->mux_mask) {
> +               dev_err(info->dev, "could not alloc mux_mask\n");
> +               return -ENOMEM;
> +       }
> +
> +       ret = of_property_read_u32_array(np, "atmel,mux-mask",
> +                                         info->mux_mask, size);
> +       if (ret)
> +               dev_err(info->dev, "can not read the mux-mask of %d\n", size);
> +       return ret;
> +}
> +
> +static int __devinit at91_pinctrl_parse_groups(struct device_node *np,
> +                               struct at91_pin_group *grp,
> +                               struct at91_pinctrl *info,
> +                               u32 index)
> +{
> +       struct at91_pmx_pin *pin;
> +       int size;
> +       const const __be32 *list;
> +       int i, j;
> +
> +       dev_dbg(info->dev, "group(%d): %s\n", index, np->name);
> +
> +       /* Initialise group */
> +       grp->name = np->name;
> +
> +       /*
> +        * the binding format is fsl,pins = <bank pin mux CONFIG ...>,
> +        * do sanity check and calculate pins number
> +        */
> +       list = of_get_property(np, "atmel,pins", &size);
> +       /* we do not check return since it's safe node passed down */
> +       size /= sizeof(*list);
> +       if (!size || size % 4) {
> +               dev_err(info->dev, "wrong pins number or pins and configs should be by 4\n");
> +               return -EINVAL;
> +       }
> +
> +       grp->npins = size / 4;
> +       pin = grp->pins_conf = devm_kzalloc(info->dev, grp->npins * sizeof(struct at91_pmx_pin),
> +                               GFP_KERNEL);
> +       grp->pins = devm_kzalloc(info->dev, grp->npins * sizeof(unsigned int),
> +                               GFP_KERNEL);
> +       if (!grp->pins_conf || !grp->pins)
> +               return -ENOMEM;
> +
> +       for (i = 0, j = 0; i < size; i += 4, j++) {
> +               pin->bank = be32_to_cpu(*list++);
> +               pin->pin = be32_to_cpu(*list++);
> +               grp->pins[j] = pin->bank * MAX_NB_GPIO_PER_BANK + pin->pin;
> +               pin->mux = be32_to_cpu(*list++);
> +               pin->conf = be32_to_cpu(*list++);
> +
> +               at91_pin_dbg(info->dev, pin);
> +               pin++;
> +       }
> +
> +       return 0;
> +}
> +
> +static int __devinit at91_pinctrl_parse_functions(struct device_node *np,
> +                       struct at91_pinctrl *info, u32 index)
> +{
> +       struct device_node *child;
> +       struct at91_pmx_func *func;
> +       struct at91_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->ngroups = of_get_child_count(np);
> +       if (func->ngroups <= 0) {
> +               dev_err(info->dev, "no groups defined\n");
> +               return -EINVAL;
> +       }
> +       func->groups = devm_kzalloc(info->dev,
> +                       func->ngroups * sizeof(char *), GFP_KERNEL);
> +       if (!func->groups)
> +               return -ENOMEM;
> +
> +       for_each_child_of_node(np, child) {
> +               func->groups[i] = child->name;
> +               grp = &info->groups[grp_index++];
> +               ret = at91_pinctrl_parse_groups(child, grp, info, i++);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +static int __devinit at91_pinctrl_probe_dt(struct platform_device *pdev,
> +                                          struct at91_pinctrl *info)
> +{
> +       int ret = 0;
> +       int i, j;
> +       uint32_t *tmp;
> +       struct device_node *np = pdev->dev.of_node;
> +       struct device_node *child;
> +
> +       if (!np)
> +               return -ENODEV;
> +
> +       info->dev = &pdev->dev;
> +       at91_pinctrl_child_count(info, np);
> +
> +       if (info->nbanks < 1) {
> +               dev_err(&pdev->dev, "you need to specify atleast one gpio-controller\n");
> +               return -EINVAL;
> +       }
> +
> +       ret = at91_pinctrl_mux_mask(info, np);
> +       if (ret)
> +               return ret;
> +
> +       dev_dbg(&pdev->dev, "nmux = %d\n", info->nmux);
> +
> +       dev_dbg(&pdev->dev, "mux-mask\n");
> +       tmp = info->mux_mask;
> +       for (i = 0; i < info->nbanks; i++) {
> +               for (j = 0; j < info->nmux; j++, tmp++) {
> +                       dev_dbg(&pdev->dev, "%d:%d\t0x%x\n", i, j, tmp[0]);
> +               }
> +       }
> +
> +       dev_dbg(&pdev->dev, "nfunctions = %d\n", info->nfunctions);
> +       dev_dbg(&pdev->dev, "ngroups = %d\n", info->ngroups);
> +       info->functions = devm_kzalloc(&pdev->dev, info->nfunctions * sizeof(struct at91_pmx_func),
> +                                       GFP_KERNEL);
> +       if (!info->functions)
> +               return -ENOMEM;
> +
> +       info->groups = devm_kzalloc(&pdev->dev, info->ngroups * sizeof(struct at91_pin_group),
> +                                       GFP_KERNEL);
> +       if (!info->groups)
> +               return -ENOMEM;
> +
> +       dev_dbg(&pdev->dev, "nbanks = %d\n", info->nbanks);
> +       dev_dbg(&pdev->dev, "nfunctions = %d\n", info->nfunctions);
> +       dev_dbg(&pdev->dev, "ngroups = %d\n", info->ngroups);
> +
> +       i = 0;
> +
> +       for_each_child_of_node(np, child) {
> +               if (of_device_is_compatible(child, gpio_compat))
> +                       continue;
> +               ret = at91_pinctrl_parse_functions(child, info, i++);
> +               if (ret) {
> +                       dev_err(&pdev->dev, "failed to parse function\n");
> +                       return ret;
> +               }
> +       }
> +
> +       return 0;
> +}
> +
> +static int __devinit at91_pinctrl_probe(struct platform_device *pdev)
> +{
> +       struct at91_pinctrl *info;
> +       struct pinctrl_pin_desc *pdesc;
> +       int ret, i, j ,k;
> +
> +       info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
> +       if (!info)
> +               return -ENOMEM;
> +
> +       ret = at91_pinctrl_probe_dt(pdev, info);
> +       if (ret)
> +               return ret;
> +
> +       /*
> +        * We need all the GPIO drivers to probe FIRST, or we will not be able
> +        * to obtain references to the struct gpio_chip * for them, and we
> +        * need this to proceed.
> +        */
> +       for (i = 0; i < info->nbanks; i++) {
> +               if (!gpio_chip[i]) {
> +                       dev_warn(&pdev->dev, "GPIO chip %d not registered yet\n", i);
> +                       devm_kfree(&pdev->dev, info);
> +                       return -EPROBE_DEFER;
> +               }
> +       }
> +
> +       at91_pinctrl_desc.name = dev_name(&pdev->dev);
> +       at91_pinctrl_desc.npins = info->nbanks * MAX_NB_GPIO_PER_BANK;
> +       at91_pinctrl_desc.pins = pdesc =
> +               devm_kzalloc(&pdev->dev, sizeof(*pdesc) * at91_pinctrl_desc.npins, GFP_KERNEL);
> +
> +       if (!at91_pinctrl_desc.pins)
> +               return -ENOMEM;
> +
> +       for (i = 0 , k = 0; i < info->nbanks; i++) {
> +               for (j = 0; j < 32; j++, k++) {
> +                       pdesc->number = k;
> +                       pdesc->name = kasprintf(GFP_KERNEL, "pio%c%d", i + 'A', j);
> +                       pdesc++;
> +               }
> +       }
> +
> +       info->set_A_periph = at91_mux_pio3_set_A_periph;
> +       info->set_B_periph = at91_mux_pio3_set_B_periph;
> +
> +       if (has_pio3()) {
> +               info->set_A_periph = at91_mux_set_A_periph;
> +               info->set_B_periph = at91_mux_set_B_periph;
> +       }
> +
> +       platform_set_drvdata(pdev, info);
> +       info->pctl = pinctrl_register(&at91_pinctrl_desc, &pdev->dev, info);
> +
> +       if (!info->pctl) {
> +               dev_err(&pdev->dev, "could not register AT91 pinctrl driver\n");
> +               ret = -EINVAL;
> +               goto err;
> +       }
> +
> +       /* We will handle a range of GPIO pins */
> +       for (i = 0; i < info->nbanks; i++)
> +               pinctrl_add_gpio_range(info->pctl, &gpio_chip[i]->range);
> +
> +       dev_info(&pdev->dev, "initialized AT91 pinctrl driver\n");
> +
> +       return 0;
> +
> +err:
> +       return ret;
> +}
> +
> +int __devexit at91_pinctrl_remove(struct platform_device *pdev)
> +{
> +       struct at91_pinctrl *info = platform_get_drvdata(pdev);
> +
> +       pinctrl_unregister(info->pctl);
> +
> +       return 0;
> +}
> +
> +static int at91_gpio_request(struct gpio_chip *chip, unsigned offset)
> +{
> +       /*
> +        * Map back to global GPIO space and request muxing, the direction
> +        * parameter does not matter for this controller.
> +        */
> +       int gpio = chip->base + offset;
> +       int bank = chip->base / chip->ngpio;
> +
> +       dev_dbg(chip->dev, "%s:%d pio%c%d(%d)\n", __func__, __LINE__,
> +                'A' + bank, offset, gpio);
> +
> +       return pinctrl_request_gpio(gpio);
> +}
> +
> +static void at91_gpio_free(struct gpio_chip *chip, unsigned offset)
> +{
> +       int gpio = chip->base + offset;
> +
> +       pinctrl_free_gpio(gpio);
> +}
> +
> +static int at91_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
> +{
> +       struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> +       void __iomem *pio = at91_gpio->regbase;
> +       unsigned mask = 1 << offset;
> +
> +       __raw_writel(mask, pio + PIO_ODR);
> +       return 0;
> +}
> +
> +static int at91_gpio_get(struct gpio_chip *chip, unsigned offset)
> +{
> +       struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> +       void __iomem *pio = at91_gpio->regbase;
> +       unsigned mask = 1 << offset;
> +       u32 pdsr;
> +
> +       pdsr = __raw_readl(pio + PIO_PDSR);
> +       return (pdsr & mask) != 0;
> +}
> +
> +static void at91_gpio_set(struct gpio_chip *chip, unsigned offset,
> +                               int val)
> +{
> +       struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> +       void __iomem *pio = at91_gpio->regbase;
> +       unsigned mask = 1 << offset;
> +
> +       __raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
> +}
> +
> +static int at91_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
> +                               int val)
> +{
> +       struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> +       void __iomem *pio = at91_gpio->regbase;
> +       unsigned mask = 1 << offset;
> +
> +       __raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
> +       __raw_writel(mask, pio + PIO_OER);
> +
> +       return 0;
> +}
> +
> +static int at91_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
> +{
> +       struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> +       int virq;
> +
> +       if (offset < chip->ngpio)
> +               virq = irq_create_mapping(at91_gpio->domain, offset);
> +       else
> +               virq = -ENXIO;
> +
> +       dev_dbg(chip->dev, "%s: request IRQ for GPIO %d, return %d\n",
> +                               chip->label, offset + chip->base, virq);
> +       return virq;
> +}
> +
> +static char peripheral_function(void __iomem *pio, unsigned mask)
> +{
> +       char    ret = 'X';
> +       u8      select;
> +
> +       if (!pio)
> +               return ret;
> +
> +       if (has_pio3()) {
> +               select = !!(__raw_readl(pio + PIO_ABCDSR1) & mask);
> +               select |= (!!(__raw_readl(pio + PIO_ABCDSR2) & mask) << 1);
> +               ret = 'A' + select;
> +       } else {
> +               ret = __raw_readl(pio + PIO_ABSR) & mask ?
> +                                               'B' : 'A';
> +       }
> +
> +       return ret;
> +}
> +
> +#ifdef CONFIG_DEBUG_FS
> +static void at91_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
> +{
> +       int i;
> +       struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> +       void __iomem *pio = at91_gpio->regbase;
> +
> +       for (i = 0; i < chip->ngpio; i++) {
> +               unsigned pin = chip->base + i;
> +               unsigned mask = pin_to_mask(pin);
> +               const char *gpio_label;
> +               u32 pdsr;
> +
> +               gpio_label = gpiochip_is_requested(chip, i);
> +               if (!gpio_label)
> +                       continue;
> +
> +               seq_printf(s, "[%s] GPIO%s%d: ",
> +                          gpio_label, chip->label, i);
> +               if (__raw_readl(pio + PIO_PSR) & mask) {
> +                       pdsr = __raw_readl(pio + PIO_PDSR);
> +
> +                       seq_printf(s, "[gpio] %s\n",
> +                                  pdsr & mask ?
> +                                  "set" : "clear");
> +               } else {
> +                       seq_printf(s, "[periph %c]\n",
> +                                  peripheral_function(pio, mask));
> +               }
> +       }
> +}
> +#else
> +#define at91_gpio_dbg_show     NULL
> +#endif
> +
> +/* Several AIC controller irqs are dispatched through this GPIO handler.
> + * To use any AT91_PIN_* as an externally triggered IRQ, first call
> + * at91_set_gpio_input() then maybe enable its glitch filter.
> + * Then just request_irq() with the pin ID; it works like any ARM IRQ
> + * handler.
> + * First implementation always triggers on rising and falling edges
> + * whereas the newer PIO3 can be additionally configured to trigger on
> + * level, edge with any polarity.
> + *
> + * Alternatively, certain pins may be used directly as IRQ0..IRQ6 after
> + * configuring them with at91_set_a_periph() or at91_set_b_periph().
> + * IRQ0..IRQ6 should be configurable, e.g. level vs edge triggering.
> + */
> +
> +static void gpio_irq_mask(struct irq_data *d)
> +{
> +       struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
> +       void __iomem    *pio = at91_gpio->regbase;
> +       unsigned        mask = 1 << d->hwirq;
> +
> +       if (pio)
> +               __raw_writel(mask, pio + PIO_IDR);
> +}
> +
> +static void gpio_irq_unmask(struct irq_data *d)
> +{
> +       struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
> +       void __iomem    *pio = at91_gpio->regbase;
> +       unsigned        mask = 1 << d->hwirq;
> +
> +       if (pio)
> +               __raw_writel(mask, pio + PIO_IER);
> +}
> +
> +static int gpio_irq_type(struct irq_data *d, unsigned type)
> +{
> +       switch (type) {
> +       case IRQ_TYPE_NONE:
> +       case IRQ_TYPE_EDGE_BOTH:
> +               return 0;
> +       default:
> +               return -EINVAL;
> +       }
> +}
> +
> +/* Alternate irq type for PIO3 support */
> +static int alt_gpio_irq_type(struct irq_data *d, unsigned type)
> +{
> +       struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
> +       void __iomem    *pio = at91_gpio->regbase;
> +       unsigned        mask = 1 << d->hwirq;
> +
> +       switch (type) {
> +       case IRQ_TYPE_EDGE_RISING:
> +               __raw_writel(mask, pio + PIO_ESR);
> +               __raw_writel(mask, pio + PIO_REHLSR);
> +               break;
> +       case IRQ_TYPE_EDGE_FALLING:
> +               __raw_writel(mask, pio + PIO_ESR);
> +               __raw_writel(mask, pio + PIO_FELLSR);
> +               break;
> +       case IRQ_TYPE_LEVEL_LOW:
> +               __raw_writel(mask, pio + PIO_LSR);
> +               __raw_writel(mask, pio + PIO_FELLSR);
> +               break;
> +       case IRQ_TYPE_LEVEL_HIGH:
> +               __raw_writel(mask, pio + PIO_LSR);
> +               __raw_writel(mask, pio + PIO_REHLSR);
> +               break;
> +       case IRQ_TYPE_EDGE_BOTH:
> +               /*
> +                * disable additional interrupt modes:
> +                * fall back to default behavior
> +                */
> +               __raw_writel(mask, pio + PIO_AIMDR);
> +               return 0;
> +       case IRQ_TYPE_NONE:
> +       default:
> +               pr_warn("AT91: No type for irq %d\n", gpio_to_irq(d->irq));
> +               return -EINVAL;
> +       }
> +
> +       /* enable additional interrupt modes */
> +       __raw_writel(mask, pio + PIO_AIMER);
> +
> +       return 0;
> +}
> +
> +#ifdef CONFIG_PM
> +static u32 wakeups[MAX_GPIO_BANKS];
> +
> +static int gpio_irq_set_wake(struct irq_data *d, unsigned state)
> +{
> +       struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
> +       unsigned        mask = 1 << d->hwirq;
> +       unsigned        bank = at91_gpio->pioc_idx;
> +
> +       if (unlikely(bank >= MAX_GPIO_BANKS))
> +               return -EINVAL;
> +
> +       if (state)
> +               wakeups[bank] |= mask;
> +       else
> +               wakeups[bank] &= ~mask;
> +
> +       irq_set_irq_wake(at91_gpio->pioc_virq, state);
> +
> +       return 0;
> +}
> +#else
> +#define gpio_irq_set_wake      NULL
> +#endif
> +
> +static struct irq_chip gpio_irqchip = {
> +       .name           = "GPIO",
> +       .irq_disable    = gpio_irq_mask,
> +       .irq_mask       = gpio_irq_mask,
> +       .irq_unmask     = gpio_irq_unmask,
> +       /* .irq_set_type is set dynamically */
> +       .irq_set_wake   = gpio_irq_set_wake,
> +};
> +
> +static void gpio_irq_handler(unsigned irq, struct irq_desc *desc)
> +{
> +       struct irq_chip *chip = irq_desc_get_chip(desc);
> +       struct irq_data *idata = irq_desc_get_irq_data(desc);
> +       struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(idata);
> +       void __iomem    *pio = at91_gpio->regbase;
> +       unsigned long   isr;
> +       int             n;
> +
> +       chained_irq_enter(chip, desc);
> +       for (;;) {
> +               /* Reading ISR acks pending (edge triggered) GPIO interrupts.
> +                * When there none are pending, we're finished unless we need
> +                * to process multiple banks (like ID_PIOCDE on sam9263).
> +                */
> +               isr = __raw_readl(pio + PIO_ISR) & __raw_readl(pio + PIO_IMR);
> +               if (!isr) {
> +                       if (!at91_gpio->next)
> +                               break;
> +                       at91_gpio = at91_gpio->next;
> +                       pio = at91_gpio->regbase;
> +                       continue;
> +               }
> +
> +               n = find_first_bit(&isr, BITS_PER_LONG);
> +               while (n < BITS_PER_LONG) {
> +                       generic_handle_irq(irq_find_mapping(at91_gpio->domain, n));
> +                       n = find_next_bit(&isr, BITS_PER_LONG, n + 1);
> +               }
> +       }
> +       chained_irq_exit(chip, desc);
> +       /* now it may re-trigger */
> +}
> +
> +/*
> + * This lock class tells lockdep that GPIO irqs are in a different
> + * category than their parents, so it won't report false recursion.
> + */
> +static struct lock_class_key gpio_lock_class;
> +
> +static int at91_gpio_irq_map(struct irq_domain *h, unsigned int virq,
> +                                                       irq_hw_number_t hw)
> +{
> +       struct at91_gpio_chip   *at91_gpio = h->host_data;
> +
> +       irq_set_lockdep_class(virq, &gpio_lock_class);
> +
> +       /*
> +        * Can use the "simple" and not "edge" handler since it's
> +        * shorter, and the AIC handles interrupts sanely.
> +        */
> +       irq_set_chip_and_handler(virq, &gpio_irqchip,
> +                                handle_simple_irq);
> +       set_irq_flags(virq, IRQF_VALID);
> +       irq_set_chip_data(virq, at91_gpio);
> +
> +       return 0;
> +}
> +
> +static struct irq_domain_ops at91_gpio_ops = {
> +       .map    = at91_gpio_irq_map,
> +       .xlate  = irq_domain_xlate_twocell,
> +};
> +
> +static int at91_gpio_of_irq_setup(struct device_node *node,
> +                                 struct at91_gpio_chip *at91_gpio)
> +{
> +       struct at91_gpio_chip   *prev = NULL;
> +       struct irq_data         *d = irq_get_irq_data(at91_gpio->pioc_virq);
> +
> +       at91_gpio->pioc_hwirq = irqd_to_hwirq(d);
> +
> +       /* Setup proper .irq_set_type function */
> +       if (has_pio3())
> +               gpio_irqchip.irq_set_type = alt_gpio_irq_type;
> +       else
> +               gpio_irqchip.irq_set_type = gpio_irq_type;
> +
> +       /* Disable irqs of this PIO controller */
> +       __raw_writel(~0, at91_gpio->regbase + PIO_IDR);
> +
> +       /* Setup irq domain */
> +       at91_gpio->domain = irq_domain_add_linear(node, at91_gpio->chip.ngpio,
> +                                               &at91_gpio_ops, at91_gpio);
> +       if (!at91_gpio->domain)
> +               panic("at91_gpio.%d: couldn't allocate irq domain (DT).\n",
> +                       at91_gpio->pioc_idx);
> +
> +       /* Setup chained handler */
> +       if (at91_gpio->pioc_idx)
> +               prev = gpio_chip[at91_gpio->pioc_idx - 1];
> +
> +       /* The toplevel handler handles one bank of GPIOs, except
> +        * on some SoC it can handles up to three...
> +        * We only set up the handler for the first of the list.
> +        */
> +       if (prev && prev->next == at91_gpio)
> +               return 0;
> +
> +       irq_set_chip_data(at91_gpio->pioc_virq, at91_gpio);
> +       irq_set_chained_handler(at91_gpio->pioc_virq, gpio_irq_handler);
> +
> +       return 0;
> +}
> +
> +/* This structure is replicated for each GPIO block allocated at probe time */
> +static struct gpio_chip at91_gpio_template = {
> +       .request                = at91_gpio_request,
> +       .free                   = at91_gpio_free,
> +       .direction_input        = at91_gpio_direction_input,
> +       .get                    = at91_gpio_get,
> +       .direction_output       = at91_gpio_direction_output,
> +       .set                    = at91_gpio_set,
> +       .to_irq                 = at91_gpio_to_irq,
> +       .dbg_show               = at91_gpio_dbg_show,
> +       .can_sleep              = 0,
> +       .ngpio                  = 32,
> +};
> +
> +static void __devinit at91_gpio_probe_fixup(void)
> +{
> +       unsigned i;
> +       struct at91_gpio_chip *at91_gpio, *last = NULL;
> +
> +       for (i = 0; i < gpio_banks; i++) {
> +               at91_gpio = gpio_chip[i];
> +
> +               /*
> +                * GPIO controller are grouped on some SoC:
> +                * PIOC, PIOD and PIOE can share the same IRQ line
> +                */
> +               if (last && last->pioc_virq == at91_gpio->pioc_virq)
> +                       last->next = at91_gpio;
> +               last = at91_gpio;
> +       }
> +}
> +
> +static int __devinit at91_gpio_probe(struct platform_device *pdev)
> +{
> +       struct device_node *np = pdev->dev.of_node;
> +       struct resource *res;
> +       struct at91_gpio_chip *at91_chip = NULL;
> +       struct gpio_chip *chip;
> +       struct pinctrl_gpio_range *range;
> +       int ret = 0;
> +       int irq;
> +       int alias_idx = of_alias_get_id(np, "gpio");
> +       uint32_t ngpio;
> +
> +       BUG_ON(alias_idx >= ARRAY_SIZE(gpio_chip));
> +       if (gpio_chip[alias_idx]) {
> +               ret = -EBUSY;
> +               goto err;
> +       }
> +
> +       at91_gpio_caps = (unsigned long)np->data;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!res) {
> +               ret = -ENOENT;
> +               goto err;
> +       }
> +
> +       irq = platform_get_irq(pdev, 0);
> +       if (irq < 0) {
> +               ret = irq;
> +               goto err;
> +       }
> +
> +       at91_chip = devm_kzalloc(&pdev->dev, sizeof(*at91_chip), GFP_KERNEL);
> +       if (!at91_chip) {
> +               ret = -ENOMEM;
> +               goto err;
> +       }
> +
> +       at91_chip->regbase = devm_request_and_ioremap(&pdev->dev, res);
> +       if (!at91_chip->regbase) {
> +               dev_err(&pdev->dev, "failed to map registers, ignoring.\n");
> +               ret = -EBUSY;
> +               goto err;
> +       }
> +
> +       at91_chip->pioc_virq = irq;
> +       at91_chip->pioc_idx = alias_idx;
> +
> +       at91_chip->clock = clk_get(&pdev->dev, NULL);
> +       if (IS_ERR(at91_chip->clock)) {
> +               dev_err(&pdev->dev, "failed to get clock, ignoring.\n");
> +               goto err;
> +       }
> +
> +       if (clk_prepare(at91_chip->clock))
> +               goto clk_prep_err;
> +
> +       /* enable PIO controller's clock */
> +       if (clk_enable(at91_chip->clock)) {
> +               dev_err(&pdev->dev, "failed to enable clock, ignoring.\n");
> +               goto clk_err;
> +       }
> +
> +       at91_chip->chip = at91_gpio_template;
> +
> +       chip = &at91_chip->chip;
> +       chip->of_node = np;
> +       chip->label = dev_name(&pdev->dev);
> +       chip->dev = &pdev->dev;
> +       chip->owner = THIS_MODULE;
> +       chip->base = alias_idx * MAX_NB_GPIO_PER_BANK;
> +
> +       if (!of_property_read_u32(np, "gpio-nb", &ngpio)) {
> +               if (ngpio >= MAX_NB_GPIO_PER_BANK)
> +                       pr_err("at91_gpio.%d, gpio-nb >= %d failback to %d\n",
> +                              alias_idx, MAX_NB_GPIO_PER_BANK, MAX_NB_GPIO_PER_BANK);
> +               else
> +                       chip->ngpio = ngpio;
> +       }
> +
> +       range = &at91_chip->range;
> +       range->name = chip->label;
> +       range->id = alias_idx;
> +       range->pin_base = range->base = range->id * MAX_NB_GPIO_PER_BANK;
> +
> +       range->npins = chip->ngpio;
> +       range->gc = chip;
> +
> +       ret = gpiochip_add(chip);
> +       if (ret)
> +               goto clk_err;
> +
> +       gpio_chip[alias_idx] = at91_chip;
> +       gpio_banks = max(gpio_banks, alias_idx + 1);
> +
> +       at91_gpio_probe_fixup();
> +
> +       at91_gpio_of_irq_setup(np, at91_chip);
> +
> +       dev_info(&pdev->dev, "at address %p\n", at91_chip->regbase);
> +
> +       return 0;
> +
> +clk_err:
> +       clk_unprepare(at91_chip->clock);
> +clk_prep_err:
> +       clk_put(at91_chip->clock);
> +err:
> +       dev_err(&pdev->dev, "Failure %i for GPIO %i\n", ret, alias_idx);
> +
> +       return ret;
> +}
> +
> +static struct of_device_id at91_gpio_of_match[] __devinitdata = {
> +       { .compatible = "atmel,at91rm9200-gpio", },
> +       { .compatible = "atmel,at91sam9x5-gpio", .data = (void*)AT91_GPIO_CAP_PIO3, },
> +       { /* sentinel */ }
> +};
> +
> +static struct platform_driver at91_gpio_driver = {
> +       .driver = {
> +               .name = "gpio-at91",
> +               .owner = THIS_MODULE,
> +               .of_match_table = of_match_ptr(at91_gpio_of_match),
> +       },
> +       .probe = at91_gpio_probe,
> +};
> +
> +static struct of_device_id at91_pinctrl_of_match[] __devinitdata = {
> +       { .compatible = "atmel,at91rm9200-pinctrl", },
> +       { /* sentinel */ }
> +};
> +
> +static struct platform_driver at91_pinctrl_driver = {
> +       .driver = {
> +               .name = "pinctrl-at91",
> +               .owner = THIS_MODULE,
> +               .of_match_table = of_match_ptr(at91_pinctrl_of_match),
> +       },
> +       .probe = at91_pinctrl_probe,
> +       .remove = __devexit_p(at91_pinctrl_remove),
> +};
> +
> +static int __init at91_pinctrl_init(void)
> +{
> +       int ret;
> +
> +       ret = platform_driver_register(&at91_gpio_driver);
> +       if (ret)
> +               return ret;
> +       return platform_driver_register(&at91_pinctrl_driver);
> +}
> +arch_initcall(at91_pinctrl_init);
> +
> +static void __exit at91_pinctrl_exit(void)
> +{
> +       platform_driver_unregister(&at91_pinctrl_driver);
> +}
> +
> +module_exit(at91_pinctrl_exit);
> +MODULE_AUTHOR("Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>");
> +MODULE_DESCRIPTION("Atmel AT91 pinctrl driver");
> +MODULE_LICENSE("GPL v2");
> --
> 1.7.10.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


I tested it with a at91sam9g35-ek board, and it hangs before the dbgu
line is configured.
( with at91_dt_defconfig and at91sam9g35ek.dts )
Jean-Christophe PLAGNIOL-VILLARD Aug. 14, 2012, 2:37 a.m. UTC | #2
> > +
> > +dbgu: serial@fffff200 {
> > +       compatible = "atmel,at91sam9260-usart";
> > +       reg = <0xfffff200 0x200>;
> > +       interrupts = <1 4 7>;
> > +       pinctrl-names = "default";
> > +       pinctrl-0 = <&pinctrl_dbgu>;
> > +       status = "disabled";
> > +};
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index e91c7cd..178a619 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -352,6 +352,7 @@ config ARCH_AT91
> >         select CLKDEV_LOOKUP
> >         select IRQ_DOMAIN
> >         select NEED_MACH_IO_H if PCCARD
> > +       select PINCTRL
> >         help
> >           This enables support for systems based on Atmel
> >           AT91RM9200 and AT91SAM9* processors.
> There's a problem with this: if CONFIG_PINCRTL is forced on ARCH_AT91
> and PINCTRL_AT91 is not selected,
> all calls to devm_pinctrl_get_select_default() will fail. => no
> serial, no nand etc..
> IMHO, CONFIG_PINCTRL should not be forced, otherwise it will break
> configs that don't want pinctrl
> Or, if pinctrl is the new and only way, PINCTRL_AT91 should also be
> forced on ARCH_AT91
no pinctrl MUST be forced as we provide pinctrl dummies
which will provide dummy config so not break

and if you want the gpio you need to enable the pinctrl on at91 otherwise you
not have it we can force PINCRTL_AT91 on DT but now the pinctrl need to always
be enabled
> > --
> > 1.7.10.4
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> 
> I tested it with a at91sam9g35-ek board, and it hangs before the dbgu
> line is configured.
> ( with at91_dt_defconfig and at91sam9g35ek.dts )
did you update you dtb before booting?

Best Regards,
J.
Linus Walleij Aug. 15, 2012, 2:38 p.m. UTC | #3
(Hm maybe I should've read this patch first, well whatever.)

On Fri, Aug 10, 2012 at 3:02 PM, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com> wrote:

> This is also include the gpio controller as the IP share both.
> Each soc will have to describe the SoC limitation and pin configuration via
> DT.

This is very good.

> This will allow to do not need to touch the C code when adding new SoC if the
> IP version is supported.

Which is what we want -> less churn.

> +++ b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt

This needs to be CC to devicetree-discuss@lists.ozlabs.org where bindings
are discussed, the people there sometimes make a good job to make sure
the bindings are OS-neutral and stuff (doesn't seem like a problem in this
very case, but anyway).

> +Required properties for iomux controller:
> +- compatible: "atmel,at91rm9200-pinctrl"
> +- atmel,mux-mask: array of mask (periph per bank) to describe if a pin can be
> +  configured in this periph mode. All the periph and bank need to be describe.

Can you please be more elaborate on this mux-mask, like what each bit
means and why the bits are arranged like that and what it means on the
AT91 platform.... I was first reading the .dts and was like ?woot? so
I go to the bindings doc and I read this and I'm still like ?woot?..

> +Required properties for pin configuration node:
> +- atmel,pins: 4 integers array, represents a group of pins mux and config

This also need to be detailed so you can just read this and then
look at the numbers in the device tree and, "aha I get it!".

> +  setting. The format is atmel,pins = <PIN_BANK PIN_BANK_NUM PERIPH CONFIG>.
> +  The PERIPH 0 means gpio.

So please elaborate a bit on the three first members, so it's easy to read
and understand the device tree. I "sort of" understand it but better
be explicit.

(...)
> +Bits used for CONFIG:
> +PULL_UP(1 << 0): indicate this pin need a pull up.
> +MULTIDRIVE(1 << 1): indicate this pin need to be configured as multidrive.

Hm if we just add multidrive to <linux/pinctrl/pinconf-generic.h> we
could probably get this driver (and bindings) to use the generic
pinconf library in drivers/pinctrl/pinonf-generic.c as the coh901
currently does.

Actually multidrive makes me think that this is just shunting in
several MOSFET driver stages, which we *used* to have in
pinconf-generic, just by documenting that the argument passed
with parameter PIN_CONFIG_DRIVE_PUSH_PULL
would indicate the number of drive stages if != 0.

Like this (old doc):

+ * @PIN_CONFIG_DRIVE_PUSH_PULL: the pin will be driven actively high and
+ *     low, this is the most typical case and is typically achieved with two
+ *     active transistors on the output. If the pin can support different
+ *     drive strengths for push/pull, the strength is given in the argument
+ *     as the number of driving stages vs nominal load impedance, so say
+ *     quadruple driving stages (usually 8 transistors rather than two) will
+ *     be configured with the 8 passed as argument.

We're not mandate generic pin config because driver authors told me
repeatedly that their chips are very special (like everyone else, hehe)
but please keep it in mind as it could be hard to change this later.
I'd be happy to put back this piece of doc...

> +2. The pin configuration node intends to work on a specific function should
> +   to be defined under that specific function node.
> +   The function node's name should represent well about what function
> +   this group of pins in this pin configuration node are working on.

I cannot parse this, sorry :-) could you detail...

> +3. The driver can use the function node's name and pin configuration node's
> +   name describe the pin function and group hierarchy.
> +   For example, Linux Iat91 pinctrl driver takes the function node's name
> +   as the function name and pin configuration node's name as group name to
> +   create the map table.
> +4. Each pin configuration node should have a phandle, devices can set pins
> +   configurations by referring to the phandle of that pin configuration node.
> +5. The gpio controller must be describe in the pinctrl simple-bus.

I think I understand this part...

> +pinctrl@fffff400 {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +       ranges;
> +       compatible = "atmel,at91rm9200-pinctrl", "simple-bus";
> +       reg = <0xfffff400 0x600>;
> +
> +       atmel,mux-mask = <
> +             /*    A         B     */
> +              0xffffffff 0xffc00c3b  /* pioA */
> +              0xffffffff 0x7fff3ccf  /* pioB */
> +              0xffffffff 0x007fffff  /* pioC */
> +             >;

I still have a real hard time understanding what is happening here,
but maybe I'll understand as I go on reading the driver.

> +       /* shared pinctrl settings */
> +       dbgu {
> +               pinctrl_dbgu: dbgu-0 {
> +                       atmel,pins =
> +                               <1 14 0x1 0x0   /* PB14 periph A */
> +                                1 15 0x1 0x1>; /* PB15 periph with pullup */

I understand that Pin 14 and 15 in pin bank 1 is connected to some
peripheral numbered 1 and pin 15 is pulled up.

I guess perioheral 1 is identical to debugf uart 0 or something
like that, since the node has this name? ut shouldn't this
peripheral number come from the fact that it's dbgu so there
is somewhere a table cross referencing ... or the device itself
has some node which is separate and has some name like
"pinctrl-periphid" or so?

Just trying to decipher this a bit so I see if I could maintain
it...

Would it be possible to have the periphid as a separate node
entry if it is anyway going to be the same value for all
pins, or is this number also unique per bank or something
like that, so debug uart would be say peripheral ID 5 on another
pin bank? (In that case, keep it like this.)

> +dbgu: serial@fffff200 {
> +       compatible = "atmel,at91sam9260-usart";
> +       reg = <0xfffff200 0x200>;
> +       interrupts = <1 4 7>;
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&pinctrl_dbgu>;
> +       status = "disabled";
> +};

I really like the looks of this.

(...)
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index e91c7cd..178a619 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -352,6 +352,7 @@ config ARCH_AT91
>         select CLKDEV_LOOKUP
>         select IRQ_DOMAIN
>         select NEED_MACH_IO_H if PCCARD
> +       select PINCTRL

Richard said something about also selectong the AT91 driver which
seemed lika a good idea.

(There follow some changes moving a lot of IRQ domain stuff etc from
the GPIO driver which is hard to understand, but I guess you're doing
the right thing.)

(...)
> --- /dev/null
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -0,0 +1,1448 @@
> +/*
> + * at91 pinctrl driver based on at91 pinmux core
> + *
> + * Copyright (C) 2011-2012 Jean-Christophe PLAGNIOL-VILLARD
> + * <plagnioj@jcrosoft.com>
> + *
> + * Under GPLv2 only
> + */
> +
> +#define DEBUG

As noted rely on switching on DEBUG_PINCTRL where needed.
It tags on -DDEBUG in the Makefile.

> +#include <asm/mach/irq.h>

Hm, it seems this platform is not using SPARSE_IRQ ...

(...)
> +static struct at91_gpio_chip *gpio_chip[MAX_GPIO_BANKS];

Nitpick should it not be named *gpio_chips[] (plural) since
there are several chips in that array?

> +static int gpio_banks;
> +static unsigned long at91_gpio_caps;

I usually protect this kind of variables with a mutex but it
may be over-cautious.

(...)
> +#define PULL_UP                (0 << 1)
> +#define MULTI_DRIVE    (1 << 1)

So this I would have attempted to use pinconf-generic.[c|h] for
and use PIN_CONFIG_BIAS_PULL_UP and
PIN_CONFIG_DRIVER_PUSH_PULL with a specific
argument for the multidrive.

Beware that you may want to patch pinconf-generic.c
to get the debugfs output you want etc.

But as I said this is optional, just a good idea in general.

(...)
> +struct at91_pmx_pin {
> +       uint32_t bank;
> +       uint32_t pin;
> +       uint32_t mux;
> +       uint32_t pullup;
> +       unsigned long conf;
> +};

Are they all u32 really? I understand that they are u32 in the
devicetree but pullup seems like a candidate for a bool.

Reading below it seems the "mux" member should be some
kind of enum. Doing that an enum and documenting it seems
like it would solve other readability issues.

And this could actually use some kerneldoc too, if possible.

> +/**
> + * struct at91_pin_group - describes an At91 pin group
> + * @name: the name of this specific pin group

@pins_conf is not documented.

> + * @pins: an array of discrete physical pins used in this group, taken
> + *     from the driver-local pin enumeration space
> + * @npins: the number of pins in this group array, i.e. the number of
> + *     elements in .pins so we can iterate over that array
> + */
> +struct at91_pin_group {
> +       const char *name;
> +       struct at91_pmx_pin *pins_conf;
> +       unsigned int *pins;
> +       unsigned npins;
> +};


> +struct at91_pinctrl {
> +       struct device *dev;
> +       struct pinctrl_dev *pctl;
> +
> +       int nbanks;
> +
> +       int nmux;
> +       uint32_t *mux_mask;
> +
> +       struct at91_pmx_func *functions;
> +       int nfunctions;
> +
> +       struct at91_pin_group *groups;
> +       int ngroups;
> +
> +       void (*set_A_periph)(void __iomem *pio, unsigned mask);
> +       void (*set_B_periph)(void __iomem *pio, unsigned mask);
> +};

kerneldoc. Especually the two last functions are mysterious when just
looking at the struct.

(Then follows a block of really nice code...)

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

This print should actually output something interesting (if it helps)
maybe you could skip it?

> +static int at91_dt_node_to_map(struct pinctrl_dev *pctldev,
> +                       struct device_node *np,
> +                       struct pinctrl_map **map, unsigned *num_maps)

DT parse code, looks nice but would request Stephen to have a look
at it.

(...)
> +       new_map = kmalloc(sizeof(*new_map) * map_num, GFP_KERNEL);

Maybe devm_kzalloc()?

Because:

(...)
> +       if (!parent) {
> +               kfree(new_map);
> +               return -EINVAL;

You could just return here. And:

(...)
> +static void at91_dt_free_map(struct pinctrl_dev *pctldev,
> +                               struct pinctrl_map *map, unsigned num_maps)
> +{
> +       kfree(map);
> +}

Then this wouldn't be needed at all.

(...)
> +static void at91_mux_disable_interrupt(void __iomem *pio, unsigned mask)
> +{
> +       __raw_writel(mask, pio + PIO_IDR);
> +}

I think most people use writel_relaxed() for this purpose (non-timeconsuming
register write) these days. So conside replacing all __raw_* with *_relaxed().

(...)
> +static void at91_mux_pio3_set_A_periph(void __iomem *pio, unsigned mask)
> +{
> +
> +       __raw_writel(__raw_readl(pio + PIO_ABCDSR1) & ~mask,
> +                                               pio + PIO_ABCDSR1);
> +       __raw_writel(__raw_readl(pio + PIO_ABCDSR2) & ~mask,
> +                                               pio + PIO_ABCDSR2);
> +}
> +
> +static void at91_mux_pio3_set_B_periph(void __iomem *pio, unsigned mask)
> +{
> +       __raw_writel(__raw_readl(pio + PIO_ABCDSR1) | mask,
> +                                               pio + PIO_ABCDSR1);
> +       __raw_writel(__raw_readl(pio + PIO_ABCDSR2) & ~mask,
> +                                               pio + PIO_ABCDSR2);
> +}
> +
> +static void at91_mux_set_A_periph(void __iomem *pio, unsigned mask)
> +{
> +       __raw_writel(mask, pio + PIO_ASR);
> +}
> +
> +static void at91_mux_set_B_periph(void __iomem *pio, unsigned mask)
> +{
> +       __raw_writel(mask, pio + PIO_BSR);
> +}
> +
> +static void at91_mux_set_C_periph(void __iomem *pio, unsigned mask)
> +{
> +       __raw_writel(__raw_readl(pio + PIO_ABCDSR1) & ~mask, pio + PIO_ABCDSR1);
> +       __raw_writel(__raw_readl(pio + PIO_ABCDSR2) | mask, pio + PIO_ABCDSR2);
> +}
> +
> +static void at91_mux_set_D_periph(void __iomem *pio, unsigned mask)
> +{
> +       __raw_writel(__raw_readl(pio + PIO_ABCDSR1) | mask, pio + PIO_ABCDSR1);
> +       __raw_writel(__raw_readl(pio + PIO_ABCDSR2) | mask, pio + PIO_ABCDSR2);
> +}

I have no clue whatsoever what is going on here, A? B? C? D?
Some small comment or something giving a hint about this grouping
system and how it works would be really nice.

> +static void at91_pin_dbg(const struct device *dev, const struct at91_pmx_pin *pin)
> +{
> +       if (pin->mux) {
> +               dev_dbg(dev, "pio%c%d configured as periph%c with conf = 0x%lu\n",
> +                       pin->bank + 'A', pin->pin, pin->mux - 1 + 'A', pin->conf);
> +       } else {
> +               dev_dbg(dev, "pio%c%d configured as gpio with conf = 0x%lu\n",
> +                       pin->bank + 'A', pin->pin, pin->conf);
> +       }
> +}

This is nice, and one of the things we try to centralize and standardize in
pinconf-generic. However as I said that would be optional.

(...)

> +static void at91_mux_gpio_enable(void __iomem *pio, unsigned mask, unsigned input)
> +{
> +       __raw_writel(mask, pio + PIO_PER);
> +       __raw_writel(mask, pio + (input ? PIO_ODR : PIO_OER));
> +}
> +

The last argument, "input" seems to be a bool.

Also not that this should maybe be named at91_mux_gpio_endisable()
since it is called from the disable() function below.

(...)
> +       for (i = 0; i < npins; i++) {
> +               pin = &pins_conf[i];
> +               at91_pin_dbg(info->dev, pin);
> +               pio = pin_to_controller(info, pin->bank);
> +               mask = pin_to_mask(pin->pin);
> +               at91_mux_disable_interrupt(pio, mask);
> +               switch(pin->mux) {
> +               case 0:
> +                       at91_mux_gpio_enable(pio, mask, 1);
> +                       break;
> +               case 1:
> +                       info->set_A_periph(pio, mask);
> +                       break;
> +               case 2:
> +                       info->set_B_periph(pio, mask);
> +                       break;
> +               case 3:
> +                       at91_mux_set_C_periph(pio, mask);
> +                       break;
> +               case 4:
> +                       at91_mux_set_D_periph(pio, mask);
> +                       break;
> +               }
> +               if (pin->mux)
> +                       at91_mux_gpio_disable(pio, mask);

Looking at this it seems that the pin->mux variable should be an
enum so we get rid of these magic values 0,1,2,3,4. Documenting
these enums with kerneldoc would probably solve a lot of
questionmarks.

(Noted in the struct review above as well.)

(...)
> +static void at91_pmx_disable(struct pinctrl_dev *pctldev, unsigned selector,
> +                          unsigned group)
> +{
> +       struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
> +       const struct at91_pmx_pin *pins_conf = info->groups[group].pins_conf;
> +       const struct at91_pmx_pin *pin;
> +       uint32_t npins = info->groups[group].npins;
> +       int i;
> +       unsigned mask;
> +       void __iomem *pio;
> +
> +       for (i = 0; i < npins; i++) {
> +               pin = &pins_conf[i];
> +               at91_pin_dbg(info->dev, pin);
> +               pio = pin_to_controller(info, pin->bank);
> +               mask = pin_to_mask(pin->pin);
> +               at91_mux_gpio_enable(pio, mask, 1);

Actually calling a function named "*enable" to disable something looks
quite unintuitive. Maybe rename that function to
"at91_mux_gpio_endisable()" or something?

(...)
> +static void at91_pinconf_dbg_show(struct pinctrl_dev *pctldev,
> +                                  struct seq_file *s, unsigned pin_id)
> +{
> +
> +}

Don't you want some nice debug output for each pin here?
Like outputting the pull/drive conf...

> +static void at91_pinconf_group_dbg_show(struct pinctrl_dev *pctldev,
> +                                        struct seq_file *s, unsigned group)
> +{
> +}

I think this function is optional now, or it should be. So you shouldn't
need to define it.

(...)
> +       pin = grp->pins_conf = devm_kzalloc(info->dev, grp->npins * sizeof(struct at91_pmx_pin),
> +                               GFP_KERNEL);
> +       grp->pins = devm_kzalloc(info->dev, grp->npins * sizeof(unsigned int),
> +                               GFP_KERNEL);

Oh managed resources here, use them everywhere.

(...)
> +       at91_pinctrl_desc.name = dev_name(&pdev->dev);
> +       at91_pinctrl_desc.npins = info->nbanks * MAX_NB_GPIO_PER_BANK;
> +       at91_pinctrl_desc.pins = pdesc =
> +               devm_kzalloc(&pdev->dev, sizeof(*pdesc) * at91_pinctrl_desc.npins, GFP_KERNEL);
> +
> +       if (!at91_pinctrl_desc.pins)
> +               return -ENOMEM;

Nitpick: checking the local variable pdesc for NULL is shorter.

(...)
> +static char peripheral_function(void __iomem *pio, unsigned mask)
> +{
> +       char    ret = 'X';
> +       u8      select;

This looks like a bool.

> +
> +       if (!pio)
> +               return ret;
> +
> +       if (has_pio3()) {
> +               select = !!(__raw_readl(pio + PIO_ABCDSR1) & mask);
> +               select |= (!!(__raw_readl(pio + PIO_ABCDSR2) & mask) << 1);

Especially if you use it like that...

(...)
> +#ifdef CONFIG_PM
> +static u32 wakeups[MAX_GPIO_BANKS];
> +
> +static int gpio_irq_set_wake(struct irq_data *d, unsigned state)

"state" looks like a bool.

> +{
> +       struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
> +       unsigned        mask = 1 << d->hwirq;
> +       unsigned        bank = at91_gpio->pioc_idx;
> +
> +       if (unlikely(bank >= MAX_GPIO_BANKS))
> +               return -EINVAL;
> +
> +       if (state)
> +               wakeups[bank] |= mask;
> +       else
> +               wakeups[bank] &= ~mask;
> +
> +       irq_set_irq_wake(at91_gpio->pioc_virq, state);
> +
> +       return 0;
> +}

So I can see that this sets or clears bits in a 32-bit word per
bank. But I don't see these words being used for anything at all?

So what's the point of all this?

In  ux500 we have a register bit that enables wakeup on a
certain pin, then it makes all kind of sense to have this,
but what is this stuff, really? It seems the function can be
stripped down to two lines just calling irq_set_irq_wake().

(...)
> +       range = &at91_chip->range;
> +       range->name = chip->label;
> +       range->id = alias_idx;
> +       range->pin_base = range->base = range->id * MAX_NB_GPIO_PER_BANK;
> +
> +       range->npins = chip->ngpio;
> +       range->gc = chip;

This way of handling ranges looks very smooth, great that this works
for you!

The rest looks very good.

Yours,
Linus Walleij
Jean-Christophe PLAGNIOL-VILLARD Aug. 16, 2012, 12:18 p.m. UTC | #4
On 16:38 Wed 15 Aug     , Linus Walleij wrote:
> (Hm maybe I should've read this patch first, well whatever.)
> 
> On Fri, Aug 10, 2012 at 3:02 PM, Jean-Christophe PLAGNIOL-VILLARD
> <plagnioj@jcrosoft.com> wrote:
> 
> > This is also include the gpio controller as the IP share both.
> > Each soc will have to describe the SoC limitation and pin configuration via
> > DT.
> 
> This is very good.
> 
> > This will allow to do not need to touch the C code when adding new SoC if the
> > IP version is supported.
> 
> Which is what we want -> less churn.
> 
> > +++ b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
> 
> This needs to be CC to devicetree-discuss@lists.ozlabs.org where bindings
> are discussed, the people there sometimes make a good job to make sure
> the bindings are OS-neutral and stuff (doesn't seem like a problem in this
> very case, but anyway).
> 
> > +Required properties for iomux controller:
> > +- compatible: "atmel,at91rm9200-pinctrl"
> > +- atmel,mux-mask: array of mask (periph per bank) to describe if a pin can be
> > +  configured in this periph mode. All the periph and bank need to be describe.
> 
> Can you please be more elaborate on this mux-mask, like what each bit
> means and why the bits are arranged like that and what it means on the
> AT91 platform.... I was first reading the .dts and was like ?woot? so
> I go to the bindings doc and I read this and I'm still like ?woot?..
> 
> > +Required properties for pin configuration node:
> > +- atmel,pins: 4 integers array, represents a group of pins mux and config
> 
> This also need to be detailed so you can just read this and then
> look at the numbers in the device tree and, "aha I get it!".
> 
> > +  setting. The format is atmel,pins = <PIN_BANK PIN_BANK_NUM PERIPH CONFIG>.
> > +  The PERIPH 0 means gpio.
> 
> So please elaborate a bit on the three first members, so it's easy to read
> and understand the device tree. I "sort of" understand it but better
> be explicit.
> 
> (...)
> > +Bits used for CONFIG:
> > +PULL_UP(1 << 0): indicate this pin need a pull up.
> > +MULTIDRIVE(1 << 1): indicate this pin need to be configured as multidrive.
> 
> Hm if we just add multidrive to <linux/pinctrl/pinconf-generic.h> we
> could probably get this driver (and bindings) to use the generic
> pinconf library in drivers/pinctrl/pinonf-generic.c as the coh901
> currently does.
> 
> Actually multidrive makes me think that this is just shunting in
> several MOSFET driver stages, which we *used* to have in
> pinconf-generic, just by documenting that the argument passed
> with parameter PIN_CONFIG_DRIVE_PUSH_PULL
> would indicate the number of drive stages if != 0.
> 
> Like this (old doc):
> 
> + * @PIN_CONFIG_DRIVE_PUSH_PULL: the pin will be driven actively high and
> + *     low, this is the most typical case and is typically achieved with two
> + *     active transistors on the output. If the pin can support different
> + *     drive strengths for push/pull, the strength is given in the argument
> + *     as the number of driving stages vs nominal load impedance, so say
> + *     quadruple driving stages (usually 8 transistors rather than two) will
> + *     be configured with the 8 passed as argument.
> 
> We're not mandate generic pin config because driver authors told me
> repeatedly that their chips are very special (like everyone else, hehe)
> but please keep it in mind as it could be hard to change this later.
> I'd be happy to put back this piece of doc...
> 
> > +2. The pin configuration node intends to work on a specific function should
> > +   to be defined under that specific function node.
> > +   The function node's name should represent well about what function
> > +   this group of pins in this pin configuration node are working on.
> 
> I cannot parse this, sorry :-) could you detail...
> 
> > +3. The driver can use the function node's name and pin configuration node's
> > +   name describe the pin function and group hierarchy.
> > +   For example, Linux Iat91 pinctrl driver takes the function node's name
> > +   as the function name and pin configuration node's name as group name to
> > +   create the map table.
> > +4. Each pin configuration node should have a phandle, devices can set pins
> > +   configurations by referring to the phandle of that pin configuration node.
> > +5. The gpio controller must be describe in the pinctrl simple-bus.
> 
> I think I understand this part...
> 
> > +pinctrl@fffff400 {
> > +       #address-cells = <1>;
> > +       #size-cells = <1>;
> > +       ranges;
> > +       compatible = "atmel,at91rm9200-pinctrl", "simple-bus";
> > +       reg = <0xfffff400 0x600>;
> > +
> > +       atmel,mux-mask = <
> > +             /*    A         B     */
> > +              0xffffffff 0xffc00c3b  /* pioA */
> > +              0xffffffff 0x7fff3ccf  /* pioB */
> > +              0xffffffff 0x007fffff  /* pioC */
> > +             >;
> 
> I still have a real hard time understanding what is happening here,
> but maybe I'll understand as I go on reading the driver.
> 
> > +       /* shared pinctrl settings */
> > +       dbgu {
> > +               pinctrl_dbgu: dbgu-0 {
> > +                       atmel,pins =
> > +                               <1 14 0x1 0x0   /* PB14 periph A */
> > +                                1 15 0x1 0x1>; /* PB15 periph with pullup */
> 
> I understand that Pin 14 and 15 in pin bank 1 is connected to some
> peripheral numbered 1 and pin 15 is pulled up.
> 
> I guess perioheral 1 is identical to debugf uart 0 or something
> like that, since the node has this name? ut shouldn't this
> peripheral number come from the fact that it's dbgu so there
> is somewhere a table cross referencing ... or the device itself
> has some node which is separate and has some name like
> "pinctrl-periphid" or so?
> 
> Just trying to decipher this a bit so I see if I could maintain
> it...
> 
> Would it be possible to have the periphid as a separate node
> entry if it is anyway going to be the same value for all
> pins, or is this number also unique per bank or something
> like that, so debug uart would be say peripheral ID 5 on another
> pin bank? (In that case, keep it like this.)
the pin can be anywhere on the different bank and configured in different mux
(periph) so can not do as you suggest
> 
> > +dbgu: serial@fffff200 {
> > +       compatible = "atmel,at91sam9260-usart";
> > +       reg = <0xfffff200 0x200>;
> > +       interrupts = <1 4 7>;
> > +       pinctrl-names = "default";
> > +       pinctrl-0 = <&pinctrl_dbgu>;
> > +       status = "disabled";
> > +};
> 
> I really like the looks of this.
> 
> (...)
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index e91c7cd..178a619 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -352,6 +352,7 @@ config ARCH_AT91
> >         select CLKDEV_LOOKUP
> >         select IRQ_DOMAIN
> >         select NEED_MACH_IO_H if PCCARD
> > +       select PINCTRL
> 
> Richard said something about also selectong the AT91 driver which
> seemed lika a good idea.
as I reply Richard we need to force the at91 driver only when DT enabled
> 
> (There follow some changes moving a lot of IRQ domain stuff etc from
> the GPIO driver which is hard to understand, but I guess you're doing
> the right thing.)
> 
> (...)
> > --- /dev/null
> > +++ b/drivers/pinctrl/pinctrl-at91.c
> > @@ -0,0 +1,1448 @@
> > +/*
> > + * at91 pinctrl driver based on at91 pinmux core
> > + *
> > + * Copyright (C) 2011-2012 Jean-Christophe PLAGNIOL-VILLARD
> > + * <plagnioj@jcrosoft.com>
> > + *
> > + * Under GPLv2 only
> > + */
> > +
> > +#define DEBUG
> 
> As noted rely on switching on DEBUG_PINCTRL where needed.
> It tags on -DDEBUG in the Makefile.
> 
> > +#include <asm/mach/irq.h>
> 
> Hm, it seems this platform is not using SPARSE_IRQ ...
it does

see chained_irq_enter/chained_irq_exit
> 
> (...)
> > +static struct at91_gpio_chip *gpio_chip[MAX_GPIO_BANKS];
> 
> Nitpick should it not be named *gpio_chips[] (plural) since
> there are several chips in that array?
ok
> 
> > +static int gpio_banks;
> > +static unsigned long at91_gpio_caps;
> 
> I usually protect this kind of variables with a mutex but it
> may be over-cautious.
> 
> (...)
> > +#define PULL_UP                (0 << 1)
> > +#define MULTI_DRIVE    (1 << 1)
> 
> So this I would have attempted to use pinconf-generic.[c|h] for
> and use PIN_CONFIG_BIAS_PULL_UP and
> PIN_CONFIG_DRIVER_PUSH_PULL with a specific
> argument for the multidrive.
> 
> Beware that you may want to patch pinconf-generic.c
> to get the debugfs output you want etc.
> 
> But as I said this is optional, just a good idea in general.
I'll take a look
> 
> (...)
> > +struct at91_pmx_pin {
> > +       uint32_t bank;
> > +       uint32_t pin;
> > +       uint32_t mux;
> > +       uint32_t pullup;
> > +       unsigned long conf;
> > +};
> 
> Are they all u32 really? I understand that they are u32 in the
> devicetree but pullup seems like a candidate for a bool.
> 
> Reading below it seems the "mux" member should be some
> kind of enum. Doing that an enum and documenting it seems
> like it would solve other readability issues.
> 
> And this could actually use some kerneldoc too, if possible.
ok
> 
> > +/**
> > + * struct at91_pin_group - describes an At91 pin group
> > + * @name: the name of this specific pin group
> 
> @pins_conf is not documented.
> 
> > + * @pins: an array of discrete physical pins used in this group, taken
> > + *     from the driver-local pin enumeration space
> > + * @npins: the number of pins in this group array, i.e. the number of
> > + *     elements in .pins so we can iterate over that array
> > + */
> > +struct at91_pin_group {
> > +       const char *name;
> > +       struct at91_pmx_pin *pins_conf;
> > +       unsigned int *pins;
> > +       unsigned npins;
> > +};
> 
> 
> > +struct at91_pinctrl {
> > +       struct device *dev;
> > +       struct pinctrl_dev *pctl;
> > +
> > +       int nbanks;
> > +
> > +       int nmux;
> > +       uint32_t *mux_mask;
> > +
> > +       struct at91_pmx_func *functions;
> > +       int nfunctions;
> > +
> > +       struct at91_pin_group *groups;
> > +       int ngroups;
> > +
> > +       void (*set_A_periph)(void __iomem *pio, unsigned mask);
> > +       void (*set_B_periph)(void __iomem *pio, unsigned mask);
> > +};
> 
> kerneldoc. Especually the two last functions are mysterious when just
> looking at the struct.
> 
> (Then follows a block of really nice code...)
> 
> (...)
> > +static void at91_pin_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
> > +                  unsigned offset)
> > +{
> > +       seq_printf(s, "%s", dev_name(pctldev->dev));
> > +}
> 
> This print should actually output something interesting (if it helps)
> maybe you could skip it?
I've plan at a second step
> 
> > +static int at91_dt_node_to_map(struct pinctrl_dev *pctldev,
> > +                       struct device_node *np,
> > +                       struct pinctrl_map **map, unsigned *num_maps)
> 
> DT parse code, looks nice but would request Stephen to have a look
> at it.
> 
> (...)
> > +       new_map = kmalloc(sizeof(*new_map) * map_num, GFP_KERNEL);
> 
> Maybe devm_kzalloc()?
> 
> Because:
> 
> (...)
> > +       if (!parent) {
> > +               kfree(new_map);
> > +               return -EINVAL;
> 
> You could just return here. And:
> 
> (...)
> > +static void at91_dt_free_map(struct pinctrl_dev *pctldev,
> > +                               struct pinctrl_map *map, unsigned num_maps)
> > +{
> > +       kfree(map);
> > +}
> 
> Then this wouldn't be needed at all.
> 
> (...)
> > +static void at91_mux_disable_interrupt(void __iomem *pio, unsigned mask)
> > +{
> > +       __raw_writel(mask, pio + PIO_IDR);
> > +}
> 
> I think most people use writel_relaxed() for this purpose (non-timeconsuming
> register write) these days. So conside replacing all __raw_* with *_relaxed().
I known this one I've in mind to do it as second patch
> 
> (...)
> > +static void at91_mux_pio3_set_A_periph(void __iomem *pio, unsigned mask)
> > +{
> > +
> > +       __raw_writel(__raw_readl(pio + PIO_ABCDSR1) & ~mask,
> > +                                               pio + PIO_ABCDSR1);
> > +       __raw_writel(__raw_readl(pio + PIO_ABCDSR2) & ~mask,
> > +                                               pio + PIO_ABCDSR2);
> > +}
> > +
> > +static void at91_mux_pio3_set_B_periph(void __iomem *pio, unsigned mask)
> > +{
> > +       __raw_writel(__raw_readl(pio + PIO_ABCDSR1) | mask,
> > +                                               pio + PIO_ABCDSR1);
> > +       __raw_writel(__raw_readl(pio + PIO_ABCDSR2) & ~mask,
> > +                                               pio + PIO_ABCDSR2);
> > +}
> > +
> > +static void at91_mux_set_A_periph(void __iomem *pio, unsigned mask)
> > +{
> > +       __raw_writel(mask, pio + PIO_ASR);
> > +}
> > +
> > +static void at91_mux_set_B_periph(void __iomem *pio, unsigned mask)
> > +{
> > +       __raw_writel(mask, pio + PIO_BSR);
> > +}
> > +
> > +static void at91_mux_set_C_periph(void __iomem *pio, unsigned mask)
> > +{
> > +       __raw_writel(__raw_readl(pio + PIO_ABCDSR1) & ~mask, pio + PIO_ABCDSR1);
> > +       __raw_writel(__raw_readl(pio + PIO_ABCDSR2) | mask, pio + PIO_ABCDSR2);
> > +}
> > +
> > +static void at91_mux_set_D_periph(void __iomem *pio, unsigned mask)
> > +{
> > +       __raw_writel(__raw_readl(pio + PIO_ABCDSR1) | mask, pio + PIO_ABCDSR1);
> > +       __raw_writel(__raw_readl(pio + PIO_ABCDSR2) | mask, pio + PIO_ABCDSR2);
> > +}
> 
> I have no clue whatsoever what is going on here, A? B? C? D?
> Some small comment or something giving a hint about this grouping
> system and how it works would be really nice.
periph (mux)
> 
> > +static void at91_pin_dbg(const struct device *dev, const struct at91_pmx_pin *pin)
> > +{
> > +       if (pin->mux) {
> > +               dev_dbg(dev, "pio%c%d configured as periph%c with conf = 0x%lu\n",
> > +                       pin->bank + 'A', pin->pin, pin->mux - 1 + 'A', pin->conf);
> > +       } else {
> > +               dev_dbg(dev, "pio%c%d configured as gpio with conf = 0x%lu\n",
> > +                       pin->bank + 'A', pin->pin, pin->conf);
> > +       }
> > +}
> 
> This is nice, and one of the things we try to centralize and standardize in
> pinconf-generic. However as I said that would be optional.
> 
> (...)
> 
> > +static void at91_mux_gpio_enable(void __iomem *pio, unsigned mask, unsigned input)
> > +{
> > +       __raw_writel(mask, pio + PIO_PER);
> > +       __raw_writel(mask, pio + (input ? PIO_ODR : PIO_OER));
> > +}
> > +
> 
> The last argument, "input" seems to be a bool.
> 
> Also not that this should maybe be named at91_mux_gpio_endisable()
> since it is called from the disable() function below.
ok
> 
> (...)
> > +       for (i = 0; i < npins; i++) {
> > +               pin = &pins_conf[i];
> > +               at91_pin_dbg(info->dev, pin);
> > +               pio = pin_to_controller(info, pin->bank);
> > +               mask = pin_to_mask(pin->pin);
> > +               at91_mux_disable_interrupt(pio, mask);
> > +               switch(pin->mux) {
> > +               case 0:
> > +                       at91_mux_gpio_enable(pio, mask, 1);
> > +                       break;
> > +               case 1:
> > +                       info->set_A_periph(pio, mask);
> > +                       break;
> > +               case 2:
> > +                       info->set_B_periph(pio, mask);
> > +                       break;
> > +               case 3:
> > +                       at91_mux_set_C_periph(pio, mask);
> > +                       break;
> > +               case 4:
> > +                       at91_mux_set_D_periph(pio, mask);
> > +                       break;
> > +               }
> > +               if (pin->mux)
> > +                       at91_mux_gpio_disable(pio, mask);
> 
> Looking at this it seems that the pin->mux variable should be an
> enum so we get rid of these magic values 0,1,2,3,4. Documenting
> these enums with kerneldoc would probably solve a lot of
> questionmarks.
> 
> (Noted in the struct review above as well.)
ok
> 
> (...)
> > +static void at91_pmx_disable(struct pinctrl_dev *pctldev, unsigned selector,
> > +                          unsigned group)
> > +{
> > +       struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
> > +       const struct at91_pmx_pin *pins_conf = info->groups[group].pins_conf;
> > +       const struct at91_pmx_pin *pin;
> > +       uint32_t npins = info->groups[group].npins;
> > +       int i;
> > +       unsigned mask;
> > +       void __iomem *pio;
> > +
> > +       for (i = 0; i < npins; i++) {
> > +               pin = &pins_conf[i];
> > +               at91_pin_dbg(info->dev, pin);
> > +               pio = pin_to_controller(info, pin->bank);
> > +               mask = pin_to_mask(pin->pin);
> > +               at91_mux_gpio_enable(pio, mask, 1);
> 
> Actually calling a function named "*enable" to disable something looks
> quite unintuitive. Maybe rename that function to
> "at91_mux_gpio_endisable()" or something?
> 
> (...)
...
> > +{
> > +       struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
> > +       unsigned        mask = 1 << d->hwirq;
> > +       unsigned        bank = at91_gpio->pioc_idx;
> > +
> > +       if (unlikely(bank >= MAX_GPIO_BANKS))
> > +               return -EINVAL;
> > +
> > +       if (state)
> > +               wakeups[bank] |= mask;
> > +       else
> > +               wakeups[bank] &= ~mask;
> > +
> > +       irq_set_irq_wake(at91_gpio->pioc_virq, state);
> > +
> > +       return 0;
> > +}
> 
> So I can see that this sets or clears bits in a 32-bit word per
> bank. But I don't see these words being used for anything at all?
> 
> So what's the point of all this?
> 
> In  ux500 we have a register bit that enables wakeup on a
> certain pin, then it makes all kind of sense to have this,
> but what is this stuff, really? It seems the function can be
> stripped down to two lines just calling irq_set_irq_wake().
> 
will take a look on this
> (...)
> > +       range = &at91_chip->range;
> > +       range->name = chip->label;
> > +       range->id = alias_idx;
> > +       range->pin_base = range->base = range->id * MAX_NB_GPIO_PER_BANK;
> > +
> > +       range->npins = chip->ngpio;
> > +       range->gc = chip;
> 
> This way of handling ranges looks very smooth, great that this works
> for you!

should work for everyone with pinctrl that combine both

Best Regards,
J.
Warner Losh Aug. 16, 2012, 3:15 p.m. UTC | #5
On Aug 16, 2012, at 6:18 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:

> On 16:38 Wed 15 Aug     , Linus Walleij wrote:
>>> +static void at91_mux_pio3_set_A_periph(void __iomem *pio, unsigned mask)
>>> +{
>>> +
>>> +       __raw_writel(__raw_readl(pio + PIO_ABCDSR1) & ~mask,
>>> +                                               pio + PIO_ABCDSR1);
>>> +       __raw_writel(__raw_readl(pio + PIO_ABCDSR2) & ~mask,
>>> +                                               pio + PIO_ABCDSR2);
>>> +}
>>> +
>>> +static void at91_mux_pio3_set_B_periph(void __iomem *pio, unsigned mask)
>>> +{
>>> +       __raw_writel(__raw_readl(pio + PIO_ABCDSR1) | mask,
>>> +                                               pio + PIO_ABCDSR1);
>>> +       __raw_writel(__raw_readl(pio + PIO_ABCDSR2) & ~mask,
>>> +                                               pio + PIO_ABCDSR2);
>>> +}
>>> +
>>> +static void at91_mux_set_A_periph(void __iomem *pio, unsigned mask)
>>> +{
>>> +       __raw_writel(mask, pio + PIO_ASR);
>>> +}
>>> +
>>> +static void at91_mux_set_B_periph(void __iomem *pio, unsigned mask)
>>> +{
>>> +       __raw_writel(mask, pio + PIO_BSR);
>>> +}
>>> +
>>> +static void at91_mux_set_C_periph(void __iomem *pio, unsigned mask)
>>> +{
>>> +       __raw_writel(__raw_readl(pio + PIO_ABCDSR1) & ~mask, pio + PIO_ABCDSR1);
>>> +       __raw_writel(__raw_readl(pio + PIO_ABCDSR2) | mask, pio + PIO_ABCDSR2);
>>> +}
>>> +
>>> +static void at91_mux_set_D_periph(void __iomem *pio, unsigned mask)
>>> +{
>>> +       __raw_writel(__raw_readl(pio + PIO_ABCDSR1) | mask, pio + PIO_ABCDSR1);
>>> +       __raw_writel(__raw_readl(pio + PIO_ABCDSR2) | mask, pio + PIO_ABCDSR2);
>>> +}
>> 
>> I have no clue whatsoever what is going on here, A? B? C? D?
>> Some small comment or something giving a hint about this grouping
>> system and how it works would be really nice.
> periph (mux)
>> 

You should read it in the context that's there.  'A_periph', 'B_periph', etc.  These functions select what device is connected to a given pin.  These are designated A, B, C, or D in the data sheets.  The overloading of PIO names with letters and Peripheral names with letters is mildly confusion in the Atmel datasheets, but is something one quickly gets used to.  Adding a one line comment wouldn't hurt here.  Maybe '/* Select proper peripheral for pin mux. */'

No comments on the rest of the patch, since I've only seen the replies to the original and can't find it any of the archives.  From what was quoted, however, this looks like a giant leap forward.  DT in 3.5 looked to be very limited, and this is one of the last pieces needed to avoid any board specific files in the general case.  I'm glad to see it coming along.

Warner
Stephen Warren Aug. 22, 2012, 3:34 p.m. UTC | #6
On 08/15/2012 08:38 AM, Linus Walleij wrote:
> (Hm maybe I should've read this patch first, well whatever.)
> 
> On Fri, Aug 10, 2012 at 3:02 PM, Jean-Christophe PLAGNIOL-VILLARD
> <plagnioj@jcrosoft.com> wrote:
> 
>> This is also include the gpio controller as the IP share both.
>> Each soc will have to describe the SoC limitation and pin configuration via
>> DT.

>> +++ b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt

>> +Required properties for iomux controller:
>> +- compatible: "atmel,at91rm9200-pinctrl"
>> +- atmel,mux-mask: array of mask (periph per bank) to describe if a pin can be
>> +  configured in this periph mode. All the periph and bank need to be describe.
> 
> Can you please be more elaborate on this mux-mask, like what each bit
> means and why the bits are arranged like that and what it means on the
> AT91 platform.... I was first reading the .dts and was like ?woot? so
> I go to the bindings doc and I read this and I'm still like ?woot?..

Yes, I'm a little confused what this is, and wouldn't have a clue how to
fill it in.

>> +static int at91_dt_node_to_map(struct pinctrl_dev *pctldev,
>> +                       struct device_node *np,
>> +                       struct pinctrl_map **map, unsigned *num_maps)
> 
> DT parse code, looks nice but would request Stephen to have a look
> at it.

I think it looks reasonable; I don't see any obvious issues at a quick
glance.
Richard Genoud Aug. 23, 2012, 7:06 a.m. UTC | #7
2012/8/22 Stephen Warren <swarren@wwwdotorg.org>:
>>> +Required properties for iomux controller:
>>> +- compatible: "atmel,at91rm9200-pinctrl"
>>> +- atmel,mux-mask: array of mask (periph per bank) to describe if a pin can be
>>> +  configured in this periph mode. All the periph and bank need to be describe.
>>
>> Can you please be more elaborate on this mux-mask, like what each bit
>> means and why the bits are arranged like that and what it means on the
>> AT91 platform.... I was first reading the .dts and was like ?woot? so
>> I go to the bindings doc and I read this and I'm still like ?woot?..
>
> Yes, I'm a little confused what this is, and wouldn't have a clue how to
> fill it in.
With a practical example it's easier to understand.
Take a SAM9X5 release manual (here is sam9g35):
http://www.atmel.com/Images/doc11053.pdf page 11 (§4.3 package pinout)
in the file arch/arm/boot/dts/at91sam9x5.dtsi you've got the the
atmel,mux-mask like that:
/* periphA  periphB    periphC     */
0xffffffff 0xffe0399f 0xc000001c  /* pioA */
0x0007ffff 0x8000fe3f 0x00000000  /* pioB */
0x80000000 0x07c0ffff 0xb83fffff  /* pioC */
0x003fffff 0x003f8000 0x00000000  /* pioD */

Let's take the PioA - peripheral B: 0xffe0399f
From the documentation table 4-3, we can extract the column PIO
Periperal B for all signals PA0-PA31:
     PIO Peripheral B
PA0  SPI1_NPCS1
PA1  SPI1_NPCS2
PA2  MCI1_DA1
PA3  MCI1_DA2
PA4  MCI1_DA3
PA5  --------
PA6  --------
PA7  SPI0_NPCS1
PA8  SPI1_NPCS0
PA9  --------
PA10 --------
PA11 MCI1_DA0
PA12 MCI1_CDA
PA13 MCI1_CK
PA14 --------
PA15 --------
etc...
Each time it's possible to mux a pin to the peripheral B function (ie
when there's no "-----") the corresponding bit is set:
     PIO Peripheral B
PA0  SPI1_NPCS1   1
PA1  SPI1_NPCS2   1
PA2  MCI1_DA1     1
PA3  MCI1_DA2     1

PA4  MCI1_DA3     1
PA5  --------     0
PA6  --------     0
PA7  SPI0_NPCS1   1

PA8  SPI1_NPCS0   1
PA9  --------     0
PA10 --------     0
PA11 MCI1_DA0     1

PA12 MCI1_CDA     1
PA13 MCI1_CK      1
PA14 --------     0
PA15 --------     0

=> this gives 0x399f



Best regards,
Richard
Stephen Warren Aug. 23, 2012, 5:49 p.m. UTC | #8
On 08/23/2012 01:06 AM, Richard Genoud wrote:
> 2012/8/22 Stephen Warren <swarren@wwwdotorg.org>:
>>>> +Required properties for iomux controller:
>>>> +- compatible: "atmel,at91rm9200-pinctrl"
>>>> +- atmel,mux-mask: array of mask (periph per bank) to describe if a pin can be
>>>> +  configured in this periph mode. All the periph and bank need to be describe.
>>>
>>> Can you please be more elaborate on this mux-mask, like what each bit
>>> means and why the bits are arranged like that and what it means on the
>>> AT91 platform.... I was first reading the .dts and was like ?woot? so
>>> I go to the bindings doc and I read this and I'm still like ?woot?..
>>
>> Yes, I'm a little confused what this is, and wouldn't have a clue how to
>> fill it in.
>
> With a practical example it's easier to understand.
> Take a SAM9X5 release manual (here is sam9g35):
> http://www.atmel.com/Images/doc11053.pdf page 11 (§4.3 package pinout)
> in the file arch/arm/boot/dts/at91sam9x5.dtsi you've got the the
> atmel,mux-mask like that:
>
> /* periphA  periphB    periphC     */
> 0xffffffff 0xffe0399f 0xc000001c  /* pioA */
> 0x0007ffff 0x8000fe3f 0x00000000  /* pioB */
> 0x80000000 0x07c0ffff 0xb83fffff  /* pioC */
> 0x003fffff 0x003f8000 0x00000000  /* pioD */
> 
> Let's take the PioA - peripheral B: 0xffe0399f
> From the documentation table 4-3, we can extract the column PIO
> Periperal B for all signals PA0-PA31:
...
> Each time it's possible to mux a pin to the peripheral B function (ie
> when there's no "-----") the corresponding bit is set:
...
> => this gives 0x399f

Ah OK. It would be a good idea to briefly describe this process in the
binding document I think. I assume this property exists to avoid the
kernel driver having to contain tables for each Atmel device; the driver
is generic.
Jean-Christophe PLAGNIOL-VILLARD Aug. 25, 2012, 8:38 a.m. UTC | #9
On 11:49 Thu 23 Aug     , Stephen Warren wrote:
> On 08/23/2012 01:06 AM, Richard Genoud wrote:
> > 2012/8/22 Stephen Warren <swarren@wwwdotorg.org>:
> >>>> +Required properties for iomux controller:
> >>>> +- compatible: "atmel,at91rm9200-pinctrl"
> >>>> +- atmel,mux-mask: array of mask (periph per bank) to describe if a pin can be
> >>>> +  configured in this periph mode. All the periph and bank need to be describe.
> >>>
> >>> Can you please be more elaborate on this mux-mask, like what each bit
> >>> means and why the bits are arranged like that and what it means on the
> >>> AT91 platform.... I was first reading the .dts and was like ?woot? so
> >>> I go to the bindings doc and I read this and I'm still like ?woot?..
> >>
> >> Yes, I'm a little confused what this is, and wouldn't have a clue how to
> >> fill it in.
> >
> > With a practical example it's easier to understand.
> > Take a SAM9X5 release manual (here is sam9g35):
> > http://www.atmel.com/Images/doc11053.pdf page 11 (§4.3 package pinout)
> > in the file arch/arm/boot/dts/at91sam9x5.dtsi you've got the the
> > atmel,mux-mask like that:
> >
> > /* periphA  periphB    periphC     */
> > 0xffffffff 0xffe0399f 0xc000001c  /* pioA */
> > 0x0007ffff 0x8000fe3f 0x00000000  /* pioB */
> > 0x80000000 0x07c0ffff 0xb83fffff  /* pioC */
> > 0x003fffff 0x003f8000 0x00000000  /* pioD */
> > 
> > Let's take the PioA - peripheral B: 0xffe0399f
> > From the documentation table 4-3, we can extract the column PIO
> > Periperal B for all signals PA0-PA31:
> ...
> > Each time it's possible to mux a pin to the peripheral B function (ie
> > when there's no "-----") the corresponding bit is set:
> ...
> > => this gives 0x399f
> 
> Ah OK. It would be a good idea to briefly describe this process in the
> binding document I think. I assume this property exists to avoid the
> kernel driver having to contain tables for each Atmel device; the driver
> is generic.
Yeap I did this exactly for this

I do not want to maintain such code for the whole bunch of at91
and this way I add other SoC with just via dtsi without touching c code

I think other driver should do so too

Best Regards,
J.
Linus Walleij Aug. 27, 2012, 11:46 p.m. UTC | #10
On Sat, Aug 25, 2012 at 1:38 AM, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com> wrote:
> On 11:49 Thu 23 Aug     , Stephen Warren wrote:
>> On 08/23/2012 01:06 AM, Richard Genoud wrote:

>> > Let's take the PioA - peripheral B: 0xffe0399f
>> > From the documentation table 4-3, we can extract the column PIO
>> > Periperal B for all signals PA0-PA31:
>> ...
>> > Each time it's possible to mux a pin to the peripheral B function (ie
>> > when there's no "-----") the corresponding bit is set:
>> ...
>> > => this gives 0x399f
>>
>> Ah OK. It would be a good idea to briefly describe this process in the
>> binding document I think. I assume this property exists to avoid the
>> kernel driver having to contain tables for each Atmel device; the driver
>> is generic.
> Yeap I did this exactly for this
>
> I do not want to maintain such code for the whole bunch of at91
> and this way I add other SoC with just via dtsi without touching c code
>
> I think other driver should do so too

I'm following it now I think, after some discussion here at the kernel summit
I have come to the conclusion that the real problem with readability comes
from a shortcoming in the device tree compiler, that it cannot handle
macros and defines (no preprocessor) so it's currently hard to make
it more readable.

But that's not the implementers fault so I'm OK with this looking like
this for the moment.

Yours,
Linus Walleij
Richard Genoud Aug. 28, 2012, 6:51 a.m. UTC | #11
2012/8/28 Linus Walleij <linus.walleij@linaro.org>:
> I'm following it now I think, after some discussion here at the kernel summit
> I have come to the conclusion that the real problem with readability comes
> from a shortcoming in the device tree compiler, that it cannot handle
> macros and defines (no preprocessor) so it's currently hard to make
> it more readable.
Definitively, I've seen quite some hex (or dec) voodoo with comment
next to them like that:
fsl,ssi-dma-events = <25 24 23 22>; /* TX0 RX0 TX1 RX1 */
linux,code = <102>; /* KEY_HOME */
linux,code = <158>; /* KEY_BACK */
fsl,pinmux-ids = <
		0x0180 /* MX28_PAD_GPMI_RDN__GPMI_RDN */
		0x0190 /* MX28_PAD_GPMI_WRN__GPMI_WRN */
		0x01c0 /* MX28_PAD_GPMI_RESETN__GPMI_RESETN */
		>;
Those bits of code are crying out "We want defines !"
(And even bitwise OR would be usefull, instead of having to choose
between hex voodoo and one bool for each bit in a register)

> But that's not the implementers fault so I'm OK with this looking like
> this for the moment.
>
> Yours,
> Linus Walleij

Best regards,
Richard.
Nicolas Ferre Sept. 11, 2012, 10:27 a.m. UTC | #12
On 08/10/2012 03:02 PM, Jean-Christophe PLAGNIOL-VILLARD :
> This is also include the gpio controller as the IP share both.
> Each soc will have to describe the SoC limitation and pin configuration via
> DT.
> 
> This will allow to do not need to touch the C code when adding new SoC if the
> IP version is supported.
> 
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>

I know that a v2 is being cooked on your side, (with some chunks of
comments from Linus, Richard and me inside) so, I wait for this new
revision with delight ;-)

> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

[..]

Bye,
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
new file mode 100644
index 0000000..0296ef4
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
@@ -0,0 +1,84 @@ 
+* Atmel AT91 Pinmux Controller
+
+The AT91 Pinmux Controler, enables the IC
+to share one PAD to several functional blocks. The sharing is done by
+multiplexing the PAD input/output signals. For each PAD there are up to
+8 muxing options (called periph modes). Since different modules require
+different PAD settings (like pull up, keeper, etc) the contoller controls
+also the PAD settings parameters.
+
+Please refer to pinctrl-bindings.txt in this directory for details of the
+common pinctrl bindings used by client devices, including the meaning of the
+phrase "pin configuration node".
+
+Atmel AT91 pin configuration node is a node of a group of pins which can be
+used for a specific device or function. This node represents both mux and config
+of the pins in that group. The 'pins' selects the function mode(also named pin
+mode) this pin can work on and the 'config' configures various pad settings
+such as pull-up, multi drive, etc.
+
+Required properties for iomux controller:
+- compatible: "atmel,at91rm9200-pinctrl"
+- atmel,mux-mask: array of mask (periph per bank) to describe if a pin can be
+  configured in this periph mode. All the periph and bank need to be describe.
+
+Required properties for pin configuration node:
+- atmel,pins: 4 integers array, represents a group of pins mux and config
+  setting. The format is atmel,pins = <PIN_BANK PIN_BANK_NUM PERIPH CONFIG>.
+  The PERIPH 0 means gpio.
+
+Bits used for CONFIG:
+PULL_UP(1 << 0): indicate this pin need a pull up.
+MULTIDRIVE(1 << 1): indicate this pin need to be configured as multidrive.
+
+NOTE:
+Some requirements for using atmel,at91rm9200-pinctrl binding:
+1. We have pin function node defined under at91 controller node to represent
+   what pinmux functions this SoC supports.
+2. The pin configuration node intends to work on a specific function should
+   to be defined under that specific function node.
+   The function node's name should represent well about what function
+   this group of pins in this pin configuration node are working on.
+3. The driver can use the function node's name and pin configuration node's
+   name describe the pin function and group hierarchy.
+   For example, Linux Iat91 pinctrl driver takes the function node's name
+   as the function name and pin configuration node's name as group name to
+   create the map table.
+4. Each pin configuration node should have a phandle, devices can set pins
+   configurations by referring to the phandle of that pin configuration node.
+5. The gpio controller must be describe in the pinctrl simple-bus.
+
+Examples:
+
+pinctrl@fffff400 {
+	#address-cells = <1>;
+	#size-cells = <1>;
+	ranges;
+	compatible = "atmel,at91rm9200-pinctrl", "simple-bus";
+	reg = <0xfffff400 0x600>;
+
+	atmel,mux-mask = <
+	      /*    A         B     */
+	       0xffffffff 0xffc00c3b  /* pioA */
+	       0xffffffff 0x7fff3ccf  /* pioB */
+	       0xffffffff 0x007fffff  /* pioC */
+	      >;
+
+	/* shared pinctrl settings */
+	dbgu {
+		pinctrl_dbgu: dbgu-0 {
+			atmel,pins =
+				<1 14 0x1 0x0	/* PB14 periph A */
+				 1 15 0x1 0x1>;	/* PB15 periph with pullup */
+		};
+	};
+};
+
+dbgu: serial@fffff200 {
+	compatible = "atmel,at91sam9260-usart";
+	reg = <0xfffff200 0x200>;
+	interrupts = <1 4 7>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_dbgu>;
+	status = "disabled";
+};
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index e91c7cd..178a619 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -352,6 +352,7 @@  config ARCH_AT91
 	select CLKDEV_LOOKUP
 	select IRQ_DOMAIN
 	select NEED_MACH_IO_H if PCCARD
+	select PINCTRL
 	help
 	  This enables support for systems based on Atmel
 	  AT91RM9200 and AT91SAM9* processors.
diff --git a/arch/arm/mach-at91/board-dt.c b/arch/arm/mach-at91/board-dt.c
index e8f45c4..3b6a948 100644
--- a/arch/arm/mach-at91/board-dt.c
+++ b/arch/arm/mach-at91/board-dt.c
@@ -30,8 +30,6 @@ 
 static const struct of_device_id irq_of_match[] __initconst = {
 
 	{ .compatible = "atmel,at91rm9200-aic", .data = at91_aic_of_init },
-	{ .compatible = "atmel,at91rm9200-gpio", .data = at91_gpio_of_irq_setup },
-	{ .compatible = "atmel,at91sam9x5-gpio", .data = at91_gpio_of_irq_setup },
 	{ /*sentinel*/ }
 };
 
diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index 8b476fd..67d7fab 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -23,8 +23,6 @@ 
 #include <linux/io.h>
 #include <linux/irqdomain.h>
 #include <linux/of_address.h>
-#include <linux/of_irq.h>
-#include <linux/of_gpio.h>
 
 #include <asm/mach/irq.h>
 
@@ -719,80 +717,6 @@  postcore_initcall(at91_gpio_debugfs_init);
  */
 static struct lock_class_key gpio_lock_class;
 
-#if defined(CONFIG_OF)
-static int at91_gpio_irq_map(struct irq_domain *h, unsigned int virq,
-							irq_hw_number_t hw)
-{
-	struct at91_gpio_chip	*at91_gpio = h->host_data;
-
-	irq_set_lockdep_class(virq, &gpio_lock_class);
-
-	/*
-	 * Can use the "simple" and not "edge" handler since it's
-	 * shorter, and the AIC handles interrupts sanely.
-	 */
-	irq_set_chip_and_handler(virq, &gpio_irqchip,
-				 handle_simple_irq);
-	set_irq_flags(virq, IRQF_VALID);
-	irq_set_chip_data(virq, at91_gpio);
-
-	return 0;
-}
-
-static struct irq_domain_ops at91_gpio_ops = {
-	.map	= at91_gpio_irq_map,
-	.xlate	= irq_domain_xlate_twocell,
-};
-
-int __init at91_gpio_of_irq_setup(struct device_node *node,
-				     struct device_node *parent)
-{
-	struct at91_gpio_chip	*prev = NULL;
-	int			alias_idx = of_alias_get_id(node, "gpio");
-	struct at91_gpio_chip	*at91_gpio = &gpio_chip[alias_idx];
-
-	/* Setup proper .irq_set_type function */
-	if (has_pio3())
-		gpio_irqchip.irq_set_type = alt_gpio_irq_type;
-	else
-		gpio_irqchip.irq_set_type = gpio_irq_type;
-
-	/* Disable irqs of this PIO controller */
-	__raw_writel(~0, at91_gpio->regbase + PIO_IDR);
-
-	/* Setup irq domain */
-	at91_gpio->domain = irq_domain_add_linear(node, at91_gpio->chip.ngpio,
-						&at91_gpio_ops, at91_gpio);
-	if (!at91_gpio->domain)
-		panic("at91_gpio.%d: couldn't allocate irq domain (DT).\n",
-			at91_gpio->pioc_idx);
-
-	/* Setup chained handler */
-	if (at91_gpio->pioc_idx)
-		prev = &gpio_chip[at91_gpio->pioc_idx - 1];
-
-	/* The toplevel handler handles one bank of GPIOs, except
-	 * on some SoC it can handles up to three...
-	 * We only set up the handler for the first of the list.
-	 */
-	if (prev && prev->next == at91_gpio)
-		return 0;
-
-	at91_gpio->pioc_virq = irq_create_mapping(irq_find_host(parent),
-							at91_gpio->pioc_hwirq);
-	irq_set_chip_data(at91_gpio->pioc_virq, at91_gpio);
-	irq_set_chained_handler(at91_gpio->pioc_virq, gpio_irq_handler);
-
-	return 0;
-}
-#else
-int __init at91_gpio_of_irq_setup(struct device_node *node,
-				     struct device_node *parent)
-{
-	return -EINVAL;
-}
-#endif
-
 /*
  * irqdomain initialization: pile up irqdomains on top of AIC range
  */
@@ -996,85 +920,6 @@  err:
 	return -EINVAL;
 }
 
-#ifdef CONFIG_OF_GPIO
-static void __init of_at91_gpio_init_one(struct device_node *np)
-{
-	int alias_idx;
-	struct at91_gpio_chip *at91_gpio;
-	uint32_t ngpio;
-
-	if (!np)
-		return;
-
-	alias_idx = of_alias_get_id(np, "gpio");
-	if (alias_idx >= MAX_GPIO_BANKS) {
-		pr_err("at91_gpio, failed alias idx(%d) > MAX_GPIO_BANKS(%d), ignoring.\n",
-						alias_idx, MAX_GPIO_BANKS);
-		return;
-	}
-
-	at91_gpio = &gpio_chip[alias_idx];
-	at91_gpio->chip.base = alias_idx * MAX_NB_GPIO_PER_BANK;
-
-	at91_gpio->regbase = of_iomap(np, 0);
-	if (!at91_gpio->regbase) {
-		pr_err("at91_gpio.%d, failed to map registers, ignoring.\n",
-								alias_idx);
-		return;
-	}
-
-	/* Get the interrupts property */
-	if (of_property_read_u32(np, "interrupts", &at91_gpio->pioc_hwirq)) {
-		pr_err("at91_gpio.%d, failed to get interrupts property, ignoring.\n",
-								alias_idx);
-		goto ioremap_err;
-	}
-
-	/* Get capabilities from compatibility property */
-	if (of_device_is_compatible(np, "atmel,at91sam9x5-gpio"))
-		at91_gpio_caps |= AT91_GPIO_CAP_PIO3;
-
-	if (!of_property_read_u32(np, "gpio-nb", &ngpio)) {
-		if (ngpio >= MAX_NB_GPIO_PER_BANK)
-			pr_err("at91_gpio.%d, gpio-nb >= %d failback to %d\n",
-			       alias_idx, MAX_NB_GPIO_PER_BANK, MAX_NB_GPIO_PER_BANK);
-		else
-			at91_gpio->chip.ngpio = ngpio;
-	}
-
-	/* Setup clock */
-	if (at91_gpio_setup_clk(alias_idx))
-		goto ioremap_err;
-
-	at91_gpio->chip.of_node = np;
-	gpio_banks = max(gpio_banks, alias_idx + 1);
-	at91_gpio->pioc_idx = alias_idx;
-	return;
-
-ioremap_err:
-	iounmap(at91_gpio->regbase);
-}
-
-static int __init of_at91_gpio_init(void)
-{
-	struct device_node *np = NULL;
-
-	/*
-	 * This isn't ideal, but it gets things hooked up until this
-	 * driver is converted into a platform_device
-	 */
-	for_each_compatible_node(np, NULL, "atmel,at91rm9200-gpio")
-		of_at91_gpio_init_one(np);
-
-	return gpio_banks > 0 ? 0 : -EINVAL;
-}
-#else
-static int __init of_at91_gpio_init(void)
-{
-	return -EINVAL;
-}
-#endif
-
 static void __init at91_gpio_init_one(int idx, u32 regbase, int pioc_hwirq)
 {
 	struct at91_gpio_chip *at91_gpio = &gpio_chip[idx];
@@ -1109,11 +954,11 @@  void __init at91_gpio_init(struct at91_gpio_bank *data, int nr_banks)
 
 	BUG_ON(nr_banks > MAX_GPIO_BANKS);
 
-	if (of_at91_gpio_init() < 0) {
-		/* No GPIO controller found in device tree */
-		for (i = 0; i < nr_banks; i++)
-			at91_gpio_init_one(i, data[i].regbase, data[i].id);
-	}
+	if (of_have_populated_dt())
+		return;
+
+	for (i = 0; i < nr_banks; i++)
+		at91_gpio_init_one(i, data[i].regbase, data[i].id);
 
 	for (i = 0; i < gpio_banks; i++) {
 		at91_gpio = &gpio_chip[i];
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 54e3588..953932a 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -26,6 +26,15 @@  config DEBUG_PINCTRL
 	help
 	  Say Y here to add some extra checks and diagnostics to PINCTRL calls.
 
+config PINCTRL_AT91
+	bool "AT91 pinctrl driver"
+	depends on OF
+	depends on ARCH_AT91
+	select PINMUX
+	select PINCONF
+	help
+	  Say Y here to enable the at91 pinctrl driver
+
 config PINCTRL_IMX
 	bool
 	select PINMUX
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index f40b1f8..d3fda00 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -9,6 +9,7 @@  ifeq ($(CONFIG_OF),y)
 obj-$(CONFIG_PINCTRL)		+= devicetree.o
 endif
 obj-$(CONFIG_GENERIC_PINCONF)	+= pinconf-generic.o
+obj-$(CONFIG_PINCTRL_AT91)	+= pinctrl-at91.o
 obj-$(CONFIG_PINCTRL_IMX)	+= pinctrl-imx.o
 obj-$(CONFIG_PINCTRL_IMX51)	+= pinctrl-imx51.o
 obj-$(CONFIG_PINCTRL_IMX53)	+= pinctrl-imx53.o
diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
new file mode 100644
index 0000000..540943b
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -0,0 +1,1448 @@ 
+/*
+ * at91 pinctrl driver based on at91 pinmux core
+ *
+ * Copyright (C) 2011-2012 Jean-Christophe PLAGNIOL-VILLARD
+ * <plagnioj@jcrosoft.com>
+ *
+ * Under GPLv2 only
+ */
+
+#define DEBUG
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/slab.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/io.h>
+#include <linux/gpio.h>
+#include <linux/irqdomain.h>
+#include <linux/pinctrl/machine.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+/* Since we request GPIOs from ourself */
+#include <linux/pinctrl/consumer.h>
+
+#include <asm/mach/irq.h>
+
+#include <mach/hardware.h>
+#include <mach/at91_pio.h>
+
+#include "core.h"
+
+#define MAX_NB_GPIO_PER_BANK	32
+
+struct at91_gpio_chip {
+	struct gpio_chip	chip;
+	struct pinctrl_gpio_range range;
+	struct at91_gpio_chip	*next;		/* Bank sharing same clock */
+	int			pioc_hwirq;	/* PIO bank interrupt identifier on AIC */
+	int			pioc_virq;	/* PIO bank Linux virtual interrupt */
+	int			pioc_idx;	/* PIO bank index */
+	void __iomem		*regbase;	/* PIO bank virtual address */
+	struct clk		*clock;		/* associated clock */
+	struct irq_domain	*domain;	/* associated irq domain */
+};
+
+#define to_at91_gpio_chip(c) container_of(c, struct at91_gpio_chip, chip)
+
+static struct at91_gpio_chip *gpio_chip[MAX_GPIO_BANKS];
+
+static int gpio_banks;
+static unsigned long at91_gpio_caps;
+
+/* All PIO controllers support PIO3 features */
+#define AT91_GPIO_CAP_PIO3	(1 <<  0)
+
+#define has_pio3()	(at91_gpio_caps & AT91_GPIO_CAP_PIO3)
+
+
+#define PULL_UP		(0 << 1)
+#define MULTI_DRIVE	(1 << 1)
+
+/**
+ * struct at91_pmx_func - describes AT91 pinmux functions
+ * @name: the name of this specific function
+ * @groups: corresponding pin groups
+ * @ngroups: the number of groups
+ */
+struct at91_pmx_func {
+	const char *name;
+	const char **groups;
+	unsigned ngroups;
+};
+
+struct at91_pmx_pin {
+	uint32_t bank;
+	uint32_t pin;
+	uint32_t mux;
+	uint32_t pullup;
+	unsigned long conf;
+};
+
+/**
+ * struct at91_pin_group - describes an At91 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
+ * @npins: the number of pins in this group array, i.e. the number of
+ *	elements in .pins so we can iterate over that array
+ */
+struct at91_pin_group {
+	const char *name;
+	struct at91_pmx_pin *pins_conf;
+	unsigned int *pins;
+	unsigned npins;
+};
+
+struct at91_pinctrl {
+	struct device *dev;
+	struct pinctrl_dev *pctl;
+
+	int nbanks;
+
+	int nmux;
+	uint32_t *mux_mask;
+
+	struct at91_pmx_func *functions;
+	int nfunctions;
+
+	struct at91_pin_group *groups;
+	int ngroups;
+
+	void (*set_A_periph)(void __iomem *pio, unsigned mask);
+	void (*set_B_periph)(void __iomem *pio, unsigned mask);
+};
+
+static const inline struct at91_pin_group *at91_pinctrl_find_group_by_name(
+				const struct at91_pinctrl *info,
+				const char *name)
+{
+	const struct at91_pin_group *grp = NULL;
+	int i;
+
+	for (i = 0; i < info->ngroups; i++) {
+		if (strcmp(info->groups[i].name, name))
+			continue;
+
+		grp = &info->groups[i];
+		dev_dbg(info->dev, "%s: %d 0:%d\n", name, grp->npins, grp->pins[0]);
+		break;
+	}
+
+	return grp;
+}
+
+static int at91_get_groups_count(struct pinctrl_dev *pctldev)
+{
+	struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+
+	return info->ngroups;
+}
+
+static const char *at91_get_group_name(struct pinctrl_dev *pctldev,
+				       unsigned selector)
+{
+	struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+
+	return info->groups[selector].name;
+}
+
+static int at91_get_group_pins(struct pinctrl_dev *pctldev, unsigned selector,
+			       const unsigned **pins,
+			       unsigned *npins)
+{
+	struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+
+	if (selector >= info->ngroups)
+		return -EINVAL;
+
+	*pins = info->groups[selector].pins;
+	*npins = info->groups[selector].npins;
+
+	return 0;
+}
+
+static void at91_pin_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
+		   unsigned offset)
+{
+	seq_printf(s, "%s", dev_name(pctldev->dev));
+}
+
+static int at91_dt_node_to_map(struct pinctrl_dev *pctldev,
+			struct device_node *np,
+			struct pinctrl_map **map, unsigned *num_maps)
+{
+	struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+	const struct at91_pin_group *grp;
+	struct pinctrl_map *new_map;
+	struct device_node *parent;
+	int map_num = 1;
+	int i;
+	struct at91_pmx_pin *pin;
+
+	/*
+	 * first find the group of this node and check if we need create
+	 * config maps for pins
+	 */
+	grp = at91_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;
+	}
+
+	map_num += grp->npins;
+	new_map = kmalloc(sizeof(*new_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 = 0; i < grp->npins; i++) {
+		pin = &grp->pins_conf[i];
+
+		new_map[i].type = PIN_MAP_TYPE_CONFIGS_PIN;
+		new_map[i].data.configs.group_or_pin =
+				pin_get_name(pctldev, grp->pins[i]);
+		new_map[i].data.configs.configs = &grp->pins_conf[i].conf;
+		new_map[i].data.configs.num_configs = 1;
+	}
+
+	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 at91_dt_free_map(struct pinctrl_dev *pctldev,
+				struct pinctrl_map *map, unsigned num_maps)
+{
+	kfree(map);
+}
+
+static struct pinctrl_ops at91_pctrl_ops = {
+	.get_groups_count = at91_get_groups_count,
+	.get_group_name = at91_get_group_name,
+	.get_group_pins = at91_get_group_pins,
+	.pin_dbg_show = at91_pin_dbg_show,
+	.dt_node_to_map = at91_dt_node_to_map,
+	.dt_free_map = at91_dt_free_map,
+
+};
+
+static void __iomem * pin_to_controller(struct at91_pinctrl *info,
+				 unsigned int bank)
+{
+	return gpio_chip[bank]->regbase;
+}
+
+static inline int pin_to_bank(unsigned pin)
+{
+	return pin /= MAX_NB_GPIO_PER_BANK;
+}
+
+static unsigned pin_to_mask(unsigned int pin)
+{
+	return 1 << pin;
+}
+
+static void at91_mux_disable_interrupt(void __iomem *pio, unsigned mask)
+{
+	__raw_writel(mask, pio + PIO_IDR);
+}
+
+static unsigned at91_mux_get_pullup(void __iomem *pio, unsigned pin)
+{
+	return (__raw_readl(pio + PIO_PUSR) >> pin) & 0x1;
+}
+
+static void at91_mux_set_pullup(void __iomem *pio, unsigned mask, unsigned on)
+{
+	__raw_writel(mask, pio + (on ? PIO_PUER : PIO_PUDR));
+}
+
+static unsigned at91_mux_get_multidrive(void __iomem *pio, unsigned pin)
+{
+	return (__raw_readl(pio + PIO_MDSR) >> pin) & 0x1;
+}
+
+static void at91_mux_set_multidrive(void __iomem *pio, unsigned mask, unsigned on)
+{
+	__raw_writel(mask, pio + (on ? PIO_MDER : PIO_MDDR));
+}
+
+static void at91_mux_pio3_set_A_periph(void __iomem *pio, unsigned mask)
+{
+
+	__raw_writel(__raw_readl(pio + PIO_ABCDSR1) & ~mask,
+						pio + PIO_ABCDSR1);
+	__raw_writel(__raw_readl(pio + PIO_ABCDSR2) & ~mask,
+						pio + PIO_ABCDSR2);
+}
+
+static void at91_mux_pio3_set_B_periph(void __iomem *pio, unsigned mask)
+{
+	__raw_writel(__raw_readl(pio + PIO_ABCDSR1) | mask,
+						pio + PIO_ABCDSR1);
+	__raw_writel(__raw_readl(pio + PIO_ABCDSR2) & ~mask,
+						pio + PIO_ABCDSR2);
+}
+
+static void at91_mux_set_A_periph(void __iomem *pio, unsigned mask)
+{
+	__raw_writel(mask, pio + PIO_ASR);
+}
+
+static void at91_mux_set_B_periph(void __iomem *pio, unsigned mask)
+{
+	__raw_writel(mask, pio + PIO_BSR);
+}
+
+static void at91_mux_set_C_periph(void __iomem *pio, unsigned mask)
+{
+	__raw_writel(__raw_readl(pio + PIO_ABCDSR1) & ~mask, pio + PIO_ABCDSR1);
+	__raw_writel(__raw_readl(pio + PIO_ABCDSR2) | mask, pio + PIO_ABCDSR2);
+}
+
+static void at91_mux_set_D_periph(void __iomem *pio, unsigned mask)
+{
+	__raw_writel(__raw_readl(pio + PIO_ABCDSR1) | mask, pio + PIO_ABCDSR1);
+	__raw_writel(__raw_readl(pio + PIO_ABCDSR2) | mask, pio + PIO_ABCDSR2);
+}
+
+static void at91_pin_dbg(const struct device *dev, const struct at91_pmx_pin *pin)
+{
+	if (pin->mux) {
+		dev_dbg(dev, "pio%c%d configured as periph%c with conf = 0x%lu\n",
+			pin->bank + 'A', pin->pin, pin->mux - 1 + 'A', pin->conf);
+	} else {
+		dev_dbg(dev, "pio%c%d configured as gpio with conf = 0x%lu\n",
+			pin->bank + 'A', pin->pin, pin->conf);
+	}
+}
+
+static int pin_check_config(struct at91_pinctrl *info, const char* name,
+			    int index, const struct at91_pmx_pin *pin)
+{
+	int mux;
+
+	/* check if it's a valid config */
+	if (pin->bank >= info->nbanks) {
+		dev_err(info->dev, "%s: pin conf %d bank_id %d >= nbanks %d\n",
+			name, index, pin->bank, info->nbanks);
+		return -EINVAL;
+	}
+
+	if (pin->pin >= 32) {
+		dev_err(info->dev, "%s: pin conf %d pin_bank_id %d >= 32\n",
+			name, index, pin->pin);
+		return -EINVAL;
+	}
+
+	if (!pin->mux)
+		return 0;
+
+	mux = pin->mux - 1;
+
+	if (mux >= info->nmux) {
+		dev_err(info->dev, "%s: pin conf %d mux_id %d >= nmux %d\n",
+			name, index, mux, info->nmux);
+		return -EINVAL;
+	}
+
+	if (!(info->mux_mask[pin->bank * info->nmux + mux] & 1 << pin->pin)) {
+		dev_err(info->dev, "%s: pin conf %d mux_id %d not supported for pio%c%d\n",
+			name, index, mux, pin->bank + 'A', pin->pin);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void at91_mux_gpio_disable(void __iomem *pio, unsigned mask)
+{
+	__raw_writel(mask, pio + PIO_PDR);
+}
+
+static void at91_mux_gpio_enable(void __iomem *pio, unsigned mask, unsigned input)
+{
+	__raw_writel(mask, pio + PIO_PER);
+	__raw_writel(mask, pio + (input ? PIO_ODR : PIO_OER));
+}
+
+static int at91_pmx_enable(struct pinctrl_dev *pctldev, unsigned selector,
+			   unsigned group)
+{
+	struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+	const struct at91_pmx_pin *pins_conf = info->groups[group].pins_conf;
+	const struct at91_pmx_pin *pin;
+	uint32_t npins = info->groups[group].npins;
+	int i, ret;
+	unsigned mask;
+	void __iomem *pio;
+
+	dev_dbg(info->dev, "enable function %s group %s\n",
+		info->functions[selector].name, info->groups[group].name);
+
+	/* first check that all the pins of the group are valid with a valid
+	 * paramter */
+	for (i = 0; i < npins; i++) {
+		pin = &pins_conf[i];
+		ret = pin_check_config(info, info->groups[group].name, i, pin);
+		if (ret)
+			return ret;
+	}
+
+	for (i = 0; i < npins; i++) {
+		pin = &pins_conf[i];
+		at91_pin_dbg(info->dev, pin);
+		pio = pin_to_controller(info, pin->bank);
+		mask = pin_to_mask(pin->pin);
+		at91_mux_disable_interrupt(pio, mask);
+		switch(pin->mux) {
+		case 0:
+			at91_mux_gpio_enable(pio, mask, 1);
+			break;
+		case 1:
+			info->set_A_periph(pio, mask);
+			break;
+		case 2:
+			info->set_B_periph(pio, mask);
+			break;
+		case 3:
+			at91_mux_set_C_periph(pio, mask);
+			break;
+		case 4:
+			at91_mux_set_D_periph(pio, mask);
+			break;
+		}
+		if (pin->mux)
+			at91_mux_gpio_disable(pio, mask);
+	}
+
+	return 0;
+}
+
+static void at91_pmx_disable(struct pinctrl_dev *pctldev, unsigned selector,
+			   unsigned group)
+{
+	struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+	const struct at91_pmx_pin *pins_conf = info->groups[group].pins_conf;
+	const struct at91_pmx_pin *pin;
+	uint32_t npins = info->groups[group].npins;
+	int i;
+	unsigned mask;
+	void __iomem *pio;
+
+	for (i = 0; i < npins; i++) {
+		pin = &pins_conf[i];
+		at91_pin_dbg(info->dev, pin);
+		pio = pin_to_controller(info, pin->bank);
+		mask = pin_to_mask(pin->pin);
+		at91_mux_gpio_enable(pio, mask, 1);
+	}
+}
+
+static int at91_pmx_get_funcs_count(struct pinctrl_dev *pctldev)
+{
+	struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+
+	return info->nfunctions;
+}
+
+static const char *at91_pmx_get_func_name(struct pinctrl_dev *pctldev,
+					  unsigned selector)
+{
+	struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+
+	return info->functions[selector].name;
+}
+
+static int at91_pmx_get_groups(struct pinctrl_dev *pctldev, unsigned selector,
+			       const char * const **groups,
+			       unsigned * const num_groups)
+{
+	struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+
+	*groups = info->functions[selector].groups;
+	*num_groups = info->functions[selector].ngroups;
+
+	return 0;
+}
+
+int at91_gpio_request_enable(struct pinctrl_dev *pctldev,
+			    struct pinctrl_gpio_range *range,
+			    unsigned offset)
+{
+	struct at91_pinctrl *npct = pinctrl_dev_get_drvdata(pctldev);
+	struct at91_gpio_chip *at91_chip;
+	struct gpio_chip *chip;
+	unsigned mask;
+
+	if (!range) {
+		dev_err(npct->dev, "invalid range\n");
+		return -EINVAL;
+	}
+	if (!range->gc) {
+		dev_err(npct->dev, "missing GPIO chip in range\n");
+		return -EINVAL;
+	}
+	chip = range->gc;
+	at91_chip = container_of(chip, struct at91_gpio_chip, chip);
+
+	dev_dbg(npct->dev, "enable pin %u as GPIO\n", offset);
+
+	mask = 1 << (offset - chip->base);
+
+	dev_dbg(npct->dev, "enable pin %u as PIO%c%d 0x%x\n",
+		offset, 'A' + range->id, offset - chip->base, mask);
+
+	__raw_writel(mask, at91_chip->regbase + PIO_PER);
+
+	return 0;
+}
+
+void at91_gpio_disable_free(struct pinctrl_dev *pctldev,
+			   struct pinctrl_gpio_range *range,
+			   unsigned offset)
+{
+	struct at91_pinctrl *npct = pinctrl_dev_get_drvdata(pctldev);
+
+	dev_dbg(npct->dev, "disable pin %u as GPIO\n", offset);
+	/* Set the pin to some default state, GPIO is usually default */
+}
+
+static struct pinmux_ops at91_pmx_ops = {
+	.get_functions_count = at91_pmx_get_funcs_count,
+	.get_function_name = at91_pmx_get_func_name,
+	.get_function_groups = at91_pmx_get_groups,
+	.enable = at91_pmx_enable,
+	.disable = at91_pmx_disable,
+	.gpio_request_enable = at91_gpio_request_enable,
+	.gpio_disable_free = at91_gpio_disable_free,
+};
+
+static int at91_pinconf_get(struct pinctrl_dev *pctldev,
+			     unsigned pin_id, unsigned long *config)
+{
+	struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+	void __iomem *pio;
+	unsigned pin;
+
+	dev_dbg(info->dev, "%s:%d, pin_id=%d, config=0x%lx", __func__, __LINE__, pin_id, *config);
+	pio = pin_to_controller(info, pin_to_bank(pin_id));
+	pin = pin_id % MAX_NB_GPIO_PER_BANK;
+
+	if (at91_mux_get_multidrive(pio, pin))
+		*config |= MULTI_DRIVE;
+
+	if (at91_mux_get_pullup(pio, pin))
+		*config |= PULL_UP;
+
+	return 0;
+}
+
+static int at91_pinconf_set(struct pinctrl_dev *pctldev,
+			     unsigned pin_id, unsigned long config)
+{
+	struct at91_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+	unsigned mask;
+	void __iomem *pio;
+
+	dev_dbg(info->dev, "%s:%d, pin_id=%d, config=0x%lx", __func__, __LINE__, pin_id, config);
+	pio = pin_to_controller(info, pin_to_bank(pin_id));
+	mask = pin_to_mask(pin_id % MAX_NB_GPIO_PER_BANK);
+
+	at91_mux_set_pullup(pio, mask, config & PULL_UP);
+	at91_mux_set_multidrive(pio, mask, config & MULTI_DRIVE);
+	return 0;
+}
+
+static void at91_pinconf_dbg_show(struct pinctrl_dev *pctldev,
+				   struct seq_file *s, unsigned pin_id)
+{
+
+}
+
+static void at91_pinconf_group_dbg_show(struct pinctrl_dev *pctldev,
+					 struct seq_file *s, unsigned group)
+{
+}
+
+struct pinconf_ops at91_pinconf_ops = {
+	.pin_config_get = at91_pinconf_get,
+	.pin_config_set = at91_pinconf_set,
+	.pin_config_dbg_show = at91_pinconf_dbg_show,
+	.pin_config_group_dbg_show = at91_pinconf_group_dbg_show,
+};
+
+static struct pinctrl_desc at91_pinctrl_desc = {
+	.pctlops = &at91_pctrl_ops,
+	.pmxops = &at91_pmx_ops,
+	.confops = &at91_pinconf_ops,
+	.owner = THIS_MODULE,
+};
+
+static const char *gpio_compat = "atmel,at91rm9200-gpio";
+
+static void __devinit at91_pinctrl_child_count(struct at91_pinctrl *info,
+					      struct device_node *np)
+{
+	struct device_node *child;
+
+	for_each_child_of_node(np, child) {
+		if (of_device_is_compatible(child, gpio_compat)) {
+			info->nbanks++;
+		} else {
+			info->nfunctions++;
+			info->ngroups += of_get_child_count(child);
+		}
+	}
+}
+
+static int __devinit at91_pinctrl_mux_mask(struct at91_pinctrl *info,
+					  struct device_node *np)
+{
+	int ret = 0;
+	int size;
+	const const __be32 *list;
+
+	list = of_get_property(np, "atmel,mux-mask", &size);
+	if (!list) {
+		dev_err(info->dev, "can not read the mux-mask of %d\n", size);
+		return -EINVAL;
+	}
+
+	size /= sizeof(*list);
+	if (!size || size % info->nbanks) {
+		dev_err(info->dev, "wrong mux mask array should be by %d\n", info->nbanks);
+		return -EINVAL;
+	}
+	info->nmux = size / info->nbanks;
+
+	info->mux_mask = devm_kzalloc(info->dev, sizeof(u32) * size, GFP_KERNEL);
+	if (!info->mux_mask) {
+		dev_err(info->dev, "could not alloc mux_mask\n");
+		return -ENOMEM;
+	}
+
+	ret = of_property_read_u32_array(np, "atmel,mux-mask",
+					  info->mux_mask, size);
+	if (ret)
+		dev_err(info->dev, "can not read the mux-mask of %d\n", size);
+	return ret;
+}
+
+static int __devinit at91_pinctrl_parse_groups(struct device_node *np,
+				struct at91_pin_group *grp,
+				struct at91_pinctrl *info,
+				u32 index)
+{
+	struct at91_pmx_pin *pin;
+	int size;
+	const const __be32 *list;
+	int i, j;
+
+	dev_dbg(info->dev, "group(%d): %s\n", index, np->name);
+
+	/* Initialise group */
+	grp->name = np->name;
+
+	/*
+	 * the binding format is fsl,pins = <bank pin mux CONFIG ...>,
+	 * do sanity check and calculate pins number
+	 */
+	list = of_get_property(np, "atmel,pins", &size);
+	/* we do not check return since it's safe node passed down */
+	size /= sizeof(*list);
+	if (!size || size % 4) {
+		dev_err(info->dev, "wrong pins number or pins and configs should be by 4\n");
+		return -EINVAL;
+	}
+
+	grp->npins = size / 4;
+	pin = grp->pins_conf = devm_kzalloc(info->dev, grp->npins * sizeof(struct at91_pmx_pin),
+				GFP_KERNEL);
+	grp->pins = devm_kzalloc(info->dev, grp->npins * sizeof(unsigned int),
+				GFP_KERNEL);
+	if (!grp->pins_conf || !grp->pins)
+		return -ENOMEM;
+
+	for (i = 0, j = 0; i < size; i += 4, j++) {
+		pin->bank = be32_to_cpu(*list++);
+		pin->pin = be32_to_cpu(*list++);
+		grp->pins[j] = pin->bank * MAX_NB_GPIO_PER_BANK + pin->pin;
+		pin->mux = be32_to_cpu(*list++);
+		pin->conf = be32_to_cpu(*list++);
+
+		at91_pin_dbg(info->dev, pin);
+		pin++;
+	}
+
+	return 0;
+}
+
+static int __devinit at91_pinctrl_parse_functions(struct device_node *np,
+			struct at91_pinctrl *info, u32 index)
+{
+	struct device_node *child;
+	struct at91_pmx_func *func;
+	struct at91_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->ngroups = of_get_child_count(np);
+	if (func->ngroups <= 0) {
+		dev_err(info->dev, "no groups defined\n");
+		return -EINVAL;
+	}
+	func->groups = devm_kzalloc(info->dev,
+			func->ngroups * sizeof(char *), GFP_KERNEL);
+	if (!func->groups)
+		return -ENOMEM;
+
+	for_each_child_of_node(np, child) {
+		func->groups[i] = child->name;
+		grp = &info->groups[grp_index++];
+		ret = at91_pinctrl_parse_groups(child, grp, info, i++);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int __devinit at91_pinctrl_probe_dt(struct platform_device *pdev,
+					   struct at91_pinctrl *info)
+{
+	int ret = 0;
+	int i, j;
+	uint32_t *tmp;
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *child;
+
+	if (!np)
+		return -ENODEV;
+
+	info->dev = &pdev->dev;
+	at91_pinctrl_child_count(info, np);
+
+	if (info->nbanks < 1) {
+		dev_err(&pdev->dev, "you need to specify atleast one gpio-controller\n");
+		return -EINVAL;
+	}
+
+	ret = at91_pinctrl_mux_mask(info, np);
+	if (ret)
+		return ret;
+
+	dev_dbg(&pdev->dev, "nmux = %d\n", info->nmux);
+
+	dev_dbg(&pdev->dev, "mux-mask\n");
+	tmp = info->mux_mask;
+	for (i = 0; i < info->nbanks; i++) {
+		for (j = 0; j < info->nmux; j++, tmp++) {
+			dev_dbg(&pdev->dev, "%d:%d\t0x%x\n", i, j, tmp[0]);
+		}
+	}
+
+	dev_dbg(&pdev->dev, "nfunctions = %d\n", info->nfunctions);
+	dev_dbg(&pdev->dev, "ngroups = %d\n", info->ngroups);
+	info->functions = devm_kzalloc(&pdev->dev, info->nfunctions * sizeof(struct at91_pmx_func),
+					GFP_KERNEL);
+	if (!info->functions)
+		return -ENOMEM;
+
+	info->groups = devm_kzalloc(&pdev->dev, info->ngroups * sizeof(struct at91_pin_group),
+					GFP_KERNEL);
+	if (!info->groups)
+		return -ENOMEM;
+
+	dev_dbg(&pdev->dev, "nbanks = %d\n", info->nbanks);
+	dev_dbg(&pdev->dev, "nfunctions = %d\n", info->nfunctions);
+	dev_dbg(&pdev->dev, "ngroups = %d\n", info->ngroups);
+
+	i = 0;
+
+	for_each_child_of_node(np, child) {
+		if (of_device_is_compatible(child, gpio_compat))
+			continue;
+		ret = at91_pinctrl_parse_functions(child, info, i++);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to parse function\n");
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int __devinit at91_pinctrl_probe(struct platform_device *pdev)
+{
+	struct at91_pinctrl *info;
+	struct pinctrl_pin_desc *pdesc;
+	int ret, i, j ,k;
+
+	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	ret = at91_pinctrl_probe_dt(pdev, info);
+	if (ret)
+		return ret;
+
+	/*
+	 * We need all the GPIO drivers to probe FIRST, or we will not be able
+	 * to obtain references to the struct gpio_chip * for them, and we
+	 * need this to proceed.
+	 */
+	for (i = 0; i < info->nbanks; i++) {
+		if (!gpio_chip[i]) {
+			dev_warn(&pdev->dev, "GPIO chip %d not registered yet\n", i);
+			devm_kfree(&pdev->dev, info);
+			return -EPROBE_DEFER;
+		}
+	}
+
+	at91_pinctrl_desc.name = dev_name(&pdev->dev);
+	at91_pinctrl_desc.npins = info->nbanks * MAX_NB_GPIO_PER_BANK;
+	at91_pinctrl_desc.pins = pdesc =
+		devm_kzalloc(&pdev->dev, sizeof(*pdesc) * at91_pinctrl_desc.npins, GFP_KERNEL);
+
+	if (!at91_pinctrl_desc.pins)
+		return -ENOMEM;
+
+	for (i = 0 , k = 0; i < info->nbanks; i++) {
+		for (j = 0; j < 32; j++, k++) {
+			pdesc->number = k;
+			pdesc->name = kasprintf(GFP_KERNEL, "pio%c%d", i + 'A', j);
+			pdesc++;
+		}
+	}
+
+	info->set_A_periph = at91_mux_pio3_set_A_periph;
+	info->set_B_periph = at91_mux_pio3_set_B_periph;
+
+	if (has_pio3()) {
+		info->set_A_periph = at91_mux_set_A_periph;
+		info->set_B_periph = at91_mux_set_B_periph;
+	}
+
+	platform_set_drvdata(pdev, info);
+	info->pctl = pinctrl_register(&at91_pinctrl_desc, &pdev->dev, info);
+
+	if (!info->pctl) {
+		dev_err(&pdev->dev, "could not register AT91 pinctrl driver\n");
+		ret = -EINVAL;
+		goto err;
+	}
+
+	/* We will handle a range of GPIO pins */
+	for (i = 0; i < info->nbanks; i++)
+		pinctrl_add_gpio_range(info->pctl, &gpio_chip[i]->range);
+
+	dev_info(&pdev->dev, "initialized AT91 pinctrl driver\n");
+
+	return 0;
+
+err:
+	return ret;
+}
+
+int __devexit at91_pinctrl_remove(struct platform_device *pdev)
+{
+	struct at91_pinctrl *info = platform_get_drvdata(pdev);
+
+	pinctrl_unregister(info->pctl);
+
+	return 0;
+}
+
+static int at91_gpio_request(struct gpio_chip *chip, unsigned offset)
+{
+	/*
+	 * Map back to global GPIO space and request muxing, the direction
+	 * parameter does not matter for this controller.
+	 */
+	int gpio = chip->base + offset;
+	int bank = chip->base / chip->ngpio;
+
+	dev_dbg(chip->dev, "%s:%d pio%c%d(%d)\n", __func__, __LINE__,
+		 'A' + bank, offset, gpio);
+
+	return pinctrl_request_gpio(gpio);
+}
+
+static void at91_gpio_free(struct gpio_chip *chip, unsigned offset)
+{
+	int gpio = chip->base + offset;
+
+	pinctrl_free_gpio(gpio);
+}
+
+static int at91_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
+{
+	struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+	void __iomem *pio = at91_gpio->regbase;
+	unsigned mask = 1 << offset;
+
+	__raw_writel(mask, pio + PIO_ODR);
+	return 0;
+}
+
+static int at91_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+	struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+	void __iomem *pio = at91_gpio->regbase;
+	unsigned mask = 1 << offset;
+	u32 pdsr;
+
+	pdsr = __raw_readl(pio + PIO_PDSR);
+	return (pdsr & mask) != 0;
+}
+
+static void at91_gpio_set(struct gpio_chip *chip, unsigned offset,
+				int val)
+{
+	struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+	void __iomem *pio = at91_gpio->regbase;
+	unsigned mask = 1 << offset;
+
+	__raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
+}
+
+static int at91_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
+				int val)
+{
+	struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+	void __iomem *pio = at91_gpio->regbase;
+	unsigned mask = 1 << offset;
+
+	__raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
+	__raw_writel(mask, pio + PIO_OER);
+
+	return 0;
+}
+
+static int at91_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
+{
+	struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+	int virq;
+
+	if (offset < chip->ngpio)
+		virq = irq_create_mapping(at91_gpio->domain, offset);
+	else
+		virq = -ENXIO;
+
+	dev_dbg(chip->dev, "%s: request IRQ for GPIO %d, return %d\n",
+				chip->label, offset + chip->base, virq);
+	return virq;
+}
+
+static char peripheral_function(void __iomem *pio, unsigned mask)
+{
+	char	ret = 'X';
+	u8	select;
+
+	if (!pio)
+		return ret;
+
+	if (has_pio3()) {
+		select = !!(__raw_readl(pio + PIO_ABCDSR1) & mask);
+		select |= (!!(__raw_readl(pio + PIO_ABCDSR2) & mask) << 1);
+		ret = 'A' + select;
+	} else {
+		ret = __raw_readl(pio + PIO_ABSR) & mask ?
+						'B' : 'A';
+	}
+
+	return ret;
+}
+
+#ifdef CONFIG_DEBUG_FS
+static void at91_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
+{
+	int i;
+	struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+	void __iomem *pio = at91_gpio->regbase;
+
+	for (i = 0; i < chip->ngpio; i++) {
+		unsigned pin = chip->base + i;
+		unsigned mask = pin_to_mask(pin);
+		const char *gpio_label;
+		u32 pdsr;
+
+		gpio_label = gpiochip_is_requested(chip, i);
+		if (!gpio_label)
+			continue;
+
+		seq_printf(s, "[%s] GPIO%s%d: ",
+			   gpio_label, chip->label, i);
+		if (__raw_readl(pio + PIO_PSR) & mask) {
+			pdsr = __raw_readl(pio + PIO_PDSR);
+
+			seq_printf(s, "[gpio] %s\n",
+				   pdsr & mask ?
+				   "set" : "clear");
+		} else {
+			seq_printf(s, "[periph %c]\n",
+				   peripheral_function(pio, mask));
+		}
+	}
+}
+#else
+#define at91_gpio_dbg_show	NULL
+#endif
+
+/* Several AIC controller irqs are dispatched through this GPIO handler.
+ * To use any AT91_PIN_* as an externally triggered IRQ, first call
+ * at91_set_gpio_input() then maybe enable its glitch filter.
+ * Then just request_irq() with the pin ID; it works like any ARM IRQ
+ * handler.
+ * First implementation always triggers on rising and falling edges
+ * whereas the newer PIO3 can be additionally configured to trigger on
+ * level, edge with any polarity.
+ *
+ * Alternatively, certain pins may be used directly as IRQ0..IRQ6 after
+ * configuring them with at91_set_a_periph() or at91_set_b_periph().
+ * IRQ0..IRQ6 should be configurable, e.g. level vs edge triggering.
+ */
+
+static void gpio_irq_mask(struct irq_data *d)
+{
+	struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
+	void __iomem	*pio = at91_gpio->regbase;
+	unsigned	mask = 1 << d->hwirq;
+
+	if (pio)
+		__raw_writel(mask, pio + PIO_IDR);
+}
+
+static void gpio_irq_unmask(struct irq_data *d)
+{
+	struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
+	void __iomem	*pio = at91_gpio->regbase;
+	unsigned	mask = 1 << d->hwirq;
+
+	if (pio)
+		__raw_writel(mask, pio + PIO_IER);
+}
+
+static int gpio_irq_type(struct irq_data *d, unsigned type)
+{
+	switch (type) {
+	case IRQ_TYPE_NONE:
+	case IRQ_TYPE_EDGE_BOTH:
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+/* Alternate irq type for PIO3 support */
+static int alt_gpio_irq_type(struct irq_data *d, unsigned type)
+{
+	struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
+	void __iomem	*pio = at91_gpio->regbase;
+	unsigned	mask = 1 << d->hwirq;
+
+	switch (type) {
+	case IRQ_TYPE_EDGE_RISING:
+		__raw_writel(mask, pio + PIO_ESR);
+		__raw_writel(mask, pio + PIO_REHLSR);
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		__raw_writel(mask, pio + PIO_ESR);
+		__raw_writel(mask, pio + PIO_FELLSR);
+		break;
+	case IRQ_TYPE_LEVEL_LOW:
+		__raw_writel(mask, pio + PIO_LSR);
+		__raw_writel(mask, pio + PIO_FELLSR);
+		break;
+	case IRQ_TYPE_LEVEL_HIGH:
+		__raw_writel(mask, pio + PIO_LSR);
+		__raw_writel(mask, pio + PIO_REHLSR);
+		break;
+	case IRQ_TYPE_EDGE_BOTH:
+		/*
+		 * disable additional interrupt modes:
+		 * fall back to default behavior
+		 */
+		__raw_writel(mask, pio + PIO_AIMDR);
+		return 0;
+	case IRQ_TYPE_NONE:
+	default:
+		pr_warn("AT91: No type for irq %d\n", gpio_to_irq(d->irq));
+		return -EINVAL;
+	}
+
+	/* enable additional interrupt modes */
+	__raw_writel(mask, pio + PIO_AIMER);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static u32 wakeups[MAX_GPIO_BANKS];
+
+static int gpio_irq_set_wake(struct irq_data *d, unsigned state)
+{
+	struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
+	unsigned	mask = 1 << d->hwirq;
+	unsigned	bank = at91_gpio->pioc_idx;
+
+	if (unlikely(bank >= MAX_GPIO_BANKS))
+		return -EINVAL;
+
+	if (state)
+		wakeups[bank] |= mask;
+	else
+		wakeups[bank] &= ~mask;
+
+	irq_set_irq_wake(at91_gpio->pioc_virq, state);
+
+	return 0;
+}
+#else
+#define gpio_irq_set_wake	NULL
+#endif
+
+static struct irq_chip gpio_irqchip = {
+	.name		= "GPIO",
+	.irq_disable	= gpio_irq_mask,
+	.irq_mask	= gpio_irq_mask,
+	.irq_unmask	= gpio_irq_unmask,
+	/* .irq_set_type is set dynamically */
+	.irq_set_wake	= gpio_irq_set_wake,
+};
+
+static void gpio_irq_handler(unsigned irq, struct irq_desc *desc)
+{
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct irq_data *idata = irq_desc_get_irq_data(desc);
+	struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(idata);
+	void __iomem	*pio = at91_gpio->regbase;
+	unsigned long	isr;
+	int		n;
+
+	chained_irq_enter(chip, desc);
+	for (;;) {
+		/* Reading ISR acks pending (edge triggered) GPIO interrupts.
+		 * When there none are pending, we're finished unless we need
+		 * to process multiple banks (like ID_PIOCDE on sam9263).
+		 */
+		isr = __raw_readl(pio + PIO_ISR) & __raw_readl(pio + PIO_IMR);
+		if (!isr) {
+			if (!at91_gpio->next)
+				break;
+			at91_gpio = at91_gpio->next;
+			pio = at91_gpio->regbase;
+			continue;
+		}
+
+		n = find_first_bit(&isr, BITS_PER_LONG);
+		while (n < BITS_PER_LONG) {
+			generic_handle_irq(irq_find_mapping(at91_gpio->domain, n));
+			n = find_next_bit(&isr, BITS_PER_LONG, n + 1);
+		}
+	}
+	chained_irq_exit(chip, desc);
+	/* now it may re-trigger */
+}
+
+/*
+ * This lock class tells lockdep that GPIO irqs are in a different
+ * category than their parents, so it won't report false recursion.
+ */
+static struct lock_class_key gpio_lock_class;
+
+static int at91_gpio_irq_map(struct irq_domain *h, unsigned int virq,
+							irq_hw_number_t hw)
+{
+	struct at91_gpio_chip	*at91_gpio = h->host_data;
+
+	irq_set_lockdep_class(virq, &gpio_lock_class);
+
+	/*
+	 * Can use the "simple" and not "edge" handler since it's
+	 * shorter, and the AIC handles interrupts sanely.
+	 */
+	irq_set_chip_and_handler(virq, &gpio_irqchip,
+				 handle_simple_irq);
+	set_irq_flags(virq, IRQF_VALID);
+	irq_set_chip_data(virq, at91_gpio);
+
+	return 0;
+}
+
+static struct irq_domain_ops at91_gpio_ops = {
+	.map	= at91_gpio_irq_map,
+	.xlate	= irq_domain_xlate_twocell,
+};
+
+static int at91_gpio_of_irq_setup(struct device_node *node,
+				  struct at91_gpio_chip *at91_gpio)
+{
+	struct at91_gpio_chip	*prev = NULL;
+	struct irq_data		*d = irq_get_irq_data(at91_gpio->pioc_virq);
+
+	at91_gpio->pioc_hwirq = irqd_to_hwirq(d);
+
+	/* Setup proper .irq_set_type function */
+	if (has_pio3())
+		gpio_irqchip.irq_set_type = alt_gpio_irq_type;
+	else
+		gpio_irqchip.irq_set_type = gpio_irq_type;
+
+	/* Disable irqs of this PIO controller */
+	__raw_writel(~0, at91_gpio->regbase + PIO_IDR);
+
+	/* Setup irq domain */
+	at91_gpio->domain = irq_domain_add_linear(node, at91_gpio->chip.ngpio,
+						&at91_gpio_ops, at91_gpio);
+	if (!at91_gpio->domain)
+		panic("at91_gpio.%d: couldn't allocate irq domain (DT).\n",
+			at91_gpio->pioc_idx);
+
+	/* Setup chained handler */
+	if (at91_gpio->pioc_idx)
+		prev = gpio_chip[at91_gpio->pioc_idx - 1];
+
+	/* The toplevel handler handles one bank of GPIOs, except
+	 * on some SoC it can handles up to three...
+	 * We only set up the handler for the first of the list.
+	 */
+	if (prev && prev->next == at91_gpio)
+		return 0;
+
+	irq_set_chip_data(at91_gpio->pioc_virq, at91_gpio);
+	irq_set_chained_handler(at91_gpio->pioc_virq, gpio_irq_handler);
+
+	return 0;
+}
+
+/* This structure is replicated for each GPIO block allocated at probe time */
+static struct gpio_chip at91_gpio_template = {
+	.request		= at91_gpio_request,
+	.free			= at91_gpio_free,
+	.direction_input	= at91_gpio_direction_input,
+	.get			= at91_gpio_get,
+	.direction_output	= at91_gpio_direction_output,
+	.set			= at91_gpio_set,
+	.to_irq			= at91_gpio_to_irq,
+	.dbg_show		= at91_gpio_dbg_show,
+	.can_sleep		= 0,
+	.ngpio			= 32,
+};
+
+static void __devinit at91_gpio_probe_fixup(void)
+{
+	unsigned i;
+	struct at91_gpio_chip *at91_gpio, *last = NULL;
+
+	for (i = 0; i < gpio_banks; i++) {
+		at91_gpio = gpio_chip[i];
+
+		/*
+		 * GPIO controller are grouped on some SoC:
+		 * PIOC, PIOD and PIOE can share the same IRQ line
+		 */
+		if (last && last->pioc_virq == at91_gpio->pioc_virq)
+			last->next = at91_gpio;
+		last = at91_gpio;
+	}
+}
+
+static int __devinit at91_gpio_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct resource *res;
+	struct at91_gpio_chip *at91_chip = NULL;
+	struct gpio_chip *chip;
+	struct pinctrl_gpio_range *range;
+	int ret = 0;
+	int irq;
+	int alias_idx = of_alias_get_id(np, "gpio");
+	uint32_t ngpio;
+
+	BUG_ON(alias_idx >= ARRAY_SIZE(gpio_chip));
+	if (gpio_chip[alias_idx]) {
+		ret = -EBUSY;
+		goto err;
+	}
+
+	at91_gpio_caps = (unsigned long)np->data;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		ret = -ENOENT;
+		goto err;
+	}
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		ret = irq;
+		goto err;
+	}
+
+	at91_chip = devm_kzalloc(&pdev->dev, sizeof(*at91_chip), GFP_KERNEL);
+	if (!at91_chip) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	at91_chip->regbase = devm_request_and_ioremap(&pdev->dev, res);
+	if (!at91_chip->regbase) {
+		dev_err(&pdev->dev, "failed to map registers, ignoring.\n");
+		ret = -EBUSY;
+		goto err;
+	}
+
+	at91_chip->pioc_virq = irq;
+	at91_chip->pioc_idx = alias_idx;
+
+	at91_chip->clock = clk_get(&pdev->dev, NULL);
+	if (IS_ERR(at91_chip->clock)) {
+		dev_err(&pdev->dev, "failed to get clock, ignoring.\n");
+		goto err;
+	}
+
+	if (clk_prepare(at91_chip->clock))
+		goto clk_prep_err;
+
+	/* enable PIO controller's clock */
+	if (clk_enable(at91_chip->clock)) {
+		dev_err(&pdev->dev, "failed to enable clock, ignoring.\n");
+		goto clk_err;
+	}
+
+	at91_chip->chip = at91_gpio_template;
+
+	chip = &at91_chip->chip;
+	chip->of_node = np;
+	chip->label = dev_name(&pdev->dev);
+	chip->dev = &pdev->dev;
+	chip->owner = THIS_MODULE;
+	chip->base = alias_idx * MAX_NB_GPIO_PER_BANK;
+
+	if (!of_property_read_u32(np, "gpio-nb", &ngpio)) {
+		if (ngpio >= MAX_NB_GPIO_PER_BANK)
+			pr_err("at91_gpio.%d, gpio-nb >= %d failback to %d\n",
+			       alias_idx, MAX_NB_GPIO_PER_BANK, MAX_NB_GPIO_PER_BANK);
+		else
+			chip->ngpio = ngpio;
+	}
+
+	range = &at91_chip->range;
+	range->name = chip->label;
+	range->id = alias_idx;
+	range->pin_base = range->base = range->id * MAX_NB_GPIO_PER_BANK;
+
+	range->npins = chip->ngpio;
+	range->gc = chip;
+
+	ret = gpiochip_add(chip);
+	if (ret)
+		goto clk_err;
+
+	gpio_chip[alias_idx] = at91_chip;
+	gpio_banks = max(gpio_banks, alias_idx + 1);
+
+	at91_gpio_probe_fixup();
+
+	at91_gpio_of_irq_setup(np, at91_chip);
+
+	dev_info(&pdev->dev, "at address %p\n", at91_chip->regbase);
+
+	return 0;
+
+clk_err:
+	clk_unprepare(at91_chip->clock);
+clk_prep_err:
+	clk_put(at91_chip->clock);
+err:
+	dev_err(&pdev->dev, "Failure %i for GPIO %i\n", ret, alias_idx);
+
+	return ret;
+}
+
+static struct of_device_id at91_gpio_of_match[] __devinitdata = {
+	{ .compatible = "atmel,at91rm9200-gpio", },
+	{ .compatible = "atmel,at91sam9x5-gpio", .data = (void*)AT91_GPIO_CAP_PIO3, },
+	{ /* sentinel */ }
+};
+
+static struct platform_driver at91_gpio_driver = {
+	.driver = {
+		.name = "gpio-at91",
+		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(at91_gpio_of_match),
+	},
+	.probe = at91_gpio_probe,
+};
+
+static struct of_device_id at91_pinctrl_of_match[] __devinitdata = {
+	{ .compatible = "atmel,at91rm9200-pinctrl", },
+	{ /* sentinel */ }
+};
+
+static struct platform_driver at91_pinctrl_driver = {
+	.driver = {
+		.name = "pinctrl-at91",
+		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(at91_pinctrl_of_match),
+	},
+	.probe = at91_pinctrl_probe,
+	.remove = __devexit_p(at91_pinctrl_remove),
+};
+
+static int __init at91_pinctrl_init(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&at91_gpio_driver);
+	if (ret)
+		return ret;
+	return platform_driver_register(&at91_pinctrl_driver);
+}
+arch_initcall(at91_pinctrl_init);
+
+static void __exit at91_pinctrl_exit(void)
+{
+	platform_driver_unregister(&at91_pinctrl_driver);
+}
+
+module_exit(at91_pinctrl_exit);
+MODULE_AUTHOR("Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>");
+MODULE_DESCRIPTION("Atmel AT91 pinctrl driver");
+MODULE_LICENSE("GPL v2");