Re: [PATCH v2 3/7] pinctrl: armada-37xx: Add pin controller support for Armada 37xx

[Gregory CLEMENT]

Hi Linus,
 
 On lun., mars 27 2017, Linus Walleij  wrote:

> On Tue, Mar 21, 2017 at 7:28 PM, Gregory CLEMENT
>  wrote:
>
>> The Armada 37xx SoC come with 2 pin controllers: one on the south
>> bridge (managing 28 pins) and one on the north bridge (managing 36 pins).
>>
>> At the hardware level the controller configure the pins by group and not
>> pin by pin. This constraint is reflected in the design of the driver:
>> only the group related functions are implemented.
>
> Interesting!
>
>> +static int armada_37xx_pmx_direction_input(struct armada_37xx_pinctrl *info,
>> +  unsigned int offset)
>> +{
>> +   unsigned int reg = OUTPUT_EN;
>> +   unsigned int mask;
>> +
>> +   if (offset >= GPIO_PER_REG) {
>> +   offset -= GPIO_PER_REG;
>> +   reg += sizeof(u32);
>> +   }
>> +   mask = BIT(offset);
>> +
>> +   return regmap_update_bits(info->regmap, reg, mask, 0);
>> +}
>> +
>> +
>> +
>
> A bit of excess whitespace, OK nitpicking.
>

As I need to send a v4, I will fix it in the same time.

> Then this stuff:
>
>> +static int armada_37xx_add_function(struct armada_37xx_pmx_func *funcs,
>> +   int *funcsize, const char *name)
>> +{
>> +   int i = 0;
>> +
>> +   if (*funcsize <= 0)
>> +   return -EOVERFLOW;
>> +
>> +   while (funcs->ngroups) {
>> +   /* function already there */
>> +   if (strcmp(funcs->name, name) == 0) {
>> +   funcs->ngroups++;
>> +
>> +   return -EEXIST;
>> +   }
>> +   funcs++;
>> +   i++;
>> +   }
>> +
>> +   /* append new unique function */
>> +   funcs->name = name;
>> +   funcs->ngroups = 1;
>> +   (*funcsize)--;
>> +
>> +   return 0;
>> +}
>> +
>> +static int armada_37xx_fill_group(struct armada_37xx_pinctrl *info, int 
>> base)
>> +{
>> +   int n, num = 0, funcsize = info->data->nr_pins;
>> +
>> +   for (n = 0; n < info->ngroups; n++) {
>> +   struct armada_37xx_pin_group *grp = >groups[n];
>> +   int i, j, f;
>> +
>> +   grp->pins = devm_kzalloc(info->dev,
>> +(grp->npins + grp->extra_npins) *
>> +sizeof(*grp->pins), GFP_KERNEL);
>> +   if (!grp->pins)
>> +   return -ENOMEM;
>> +
>> +   for (i = 0; i < grp->npins; i++)
>> +   grp->pins[i] = grp->start_pin + base + i;
>> +
>> +   for (j = 0; j < grp->extra_npins; j++)
>> +   grp->pins[i+j] = grp->extra_pin + base + j;
>> +
>> +   for (f = 0; f < NB_FUNCS; f++) {
>> +   int ret;
>> +   /* check for unique functions and count groups */
>> +   ret = armada_37xx_add_function(info->funcs, 
>> ,
>> +   grp->funcs[f]);
>> +   if (ret == -EOVERFLOW)
>> +   dev_err(info->dev,
>> +   "More functions than pins(%d)\n",
>> +   info->data->nr_pins);
>> +   if (ret < 0)
>> +   continue;
>> +   num++;
>> +   }
>> +   }
>> +
>> +   info->nfuncs = num;
>> +
>> +   return 0;
>> +}
>> +
>> +static int armada_37xx_fill_func(struct armada_37xx_pinctrl *info)
>> +{
>> +   struct armada_37xx_pmx_func *funcs = info->funcs;
>> +   int n;
>> +
>> +   for (n = 0; n < info->nfuncs; n++) {
>> +   const char *name = funcs[n].name;
>> +   const char **groups;
>> +   int g;
>> +
>> +   funcs[n].groups = devm_kzalloc(info->dev, funcs[n].ngroups *
>> +  sizeof(*(funcs[n].groups)),
>> +  GFP_KERNEL);
>> +   if (!funcs[n].groups)
>> +   return -ENOMEM;
>> +
>> +   groups = funcs[n].groups;
>> +
>> +   for (g = 0; g < info->ngroups; g++) {
>> +   struct armada_37xx_pin_group *gp = >groups[g];
>> +   int f;
>> +
>> +   for (f = 0; f < NB_FUNCS; f++) {
>> +   if (strcmp(gp->funcs[f], name) == 0) {
>> +   *groups = gp->name;
>> +   groups++;
>> +   }
>> +   }
>> +   }
>> +   }
>> +   return 0;
>> +}
>
> I would be happy if you add kerneldoc to these functions and explain
> what they do. Because I don't get it. I guess they are filling in the data
> structures but yeah. Hard to follow.

OK

>
>> 

Re: [PATCH v2 3/7] pinctrl: armada-37xx: Add pin controller support for Armada 37xx

[Gregory CLEMENT]

Hi Linus,
 
 On lun., mars 27 2017, Linus Walleij  wrote:

> On Tue, Mar 21, 2017 at 7:28 PM, Gregory CLEMENT
>  wrote:
>
>> The Armada 37xx SoC come with 2 pin controllers: one on the south
>> bridge (managing 28 pins) and one on the north bridge (managing 36 pins).
>>
>> At the hardware level the controller configure the pins by group and not
>> pin by pin. This constraint is reflected in the design of the driver:
>> only the group related functions are implemented.
>
> Interesting!
>
>> +static int armada_37xx_pmx_direction_input(struct armada_37xx_pinctrl *info,
>> +  unsigned int offset)
>> +{
>> +   unsigned int reg = OUTPUT_EN;
>> +   unsigned int mask;
>> +
>> +   if (offset >= GPIO_PER_REG) {
>> +   offset -= GPIO_PER_REG;
>> +   reg += sizeof(u32);
>> +   }
>> +   mask = BIT(offset);
>> +
>> +   return regmap_update_bits(info->regmap, reg, mask, 0);
>> +}
>> +
>> +
>> +
>
> A bit of excess whitespace, OK nitpicking.
>

As I need to send a v4, I will fix it in the same time.

> Then this stuff:
>
>> +static int armada_37xx_add_function(struct armada_37xx_pmx_func *funcs,
>> +   int *funcsize, const char *name)
>> +{
>> +   int i = 0;
>> +
>> +   if (*funcsize <= 0)
>> +   return -EOVERFLOW;
>> +
>> +   while (funcs->ngroups) {
>> +   /* function already there */
>> +   if (strcmp(funcs->name, name) == 0) {
>> +   funcs->ngroups++;
>> +
>> +   return -EEXIST;
>> +   }
>> +   funcs++;
>> +   i++;
>> +   }
>> +
>> +   /* append new unique function */
>> +   funcs->name = name;
>> +   funcs->ngroups = 1;
>> +   (*funcsize)--;
>> +
>> +   return 0;
>> +}
>> +
>> +static int armada_37xx_fill_group(struct armada_37xx_pinctrl *info, int 
>> base)
>> +{
>> +   int n, num = 0, funcsize = info->data->nr_pins;
>> +
>> +   for (n = 0; n < info->ngroups; n++) {
>> +   struct armada_37xx_pin_group *grp = >groups[n];
>> +   int i, j, f;
>> +
>> +   grp->pins = devm_kzalloc(info->dev,
>> +(grp->npins + grp->extra_npins) *
>> +sizeof(*grp->pins), GFP_KERNEL);
>> +   if (!grp->pins)
>> +   return -ENOMEM;
>> +
>> +   for (i = 0; i < grp->npins; i++)
>> +   grp->pins[i] = grp->start_pin + base + i;
>> +
>> +   for (j = 0; j < grp->extra_npins; j++)
>> +   grp->pins[i+j] = grp->extra_pin + base + j;
>> +
>> +   for (f = 0; f < NB_FUNCS; f++) {
>> +   int ret;
>> +   /* check for unique functions and count groups */
>> +   ret = armada_37xx_add_function(info->funcs, 
>> ,
>> +   grp->funcs[f]);
>> +   if (ret == -EOVERFLOW)
>> +   dev_err(info->dev,
>> +   "More functions than pins(%d)\n",
>> +   info->data->nr_pins);
>> +   if (ret < 0)
>> +   continue;
>> +   num++;
>> +   }
>> +   }
>> +
>> +   info->nfuncs = num;
>> +
>> +   return 0;
>> +}
>> +
>> +static int armada_37xx_fill_func(struct armada_37xx_pinctrl *info)
>> +{
>> +   struct armada_37xx_pmx_func *funcs = info->funcs;
>> +   int n;
>> +
>> +   for (n = 0; n < info->nfuncs; n++) {
>> +   const char *name = funcs[n].name;
>> +   const char **groups;
>> +   int g;
>> +
>> +   funcs[n].groups = devm_kzalloc(info->dev, funcs[n].ngroups *
>> +  sizeof(*(funcs[n].groups)),
>> +  GFP_KERNEL);
>> +   if (!funcs[n].groups)
>> +   return -ENOMEM;
>> +
>> +   groups = funcs[n].groups;
>> +
>> +   for (g = 0; g < info->ngroups; g++) {
>> +   struct armada_37xx_pin_group *gp = >groups[g];
>> +   int f;
>> +
>> +   for (f = 0; f < NB_FUNCS; f++) {
>> +   if (strcmp(gp->funcs[f], name) == 0) {
>> +   *groups = gp->name;
>> +   groups++;
>> +   }
>> +   }
>> +   }
>> +   }
>> +   return 0;
>> +}
>
> I would be happy if you add kerneldoc to these functions and explain
> what they do. Because I don't get it. I guess they are filling in the data
> structures but yeah. Hard to follow.

OK

>
>> +   match = of_match_node(armada_37xx_pinctrl_of_match, 

Re: [PATCH v2 3/7] pinctrl: armada-37xx: Add pin controller support for Armada 37xx

[Linus Walleij]

On Tue, Mar 21, 2017 at 7:28 PM, Gregory CLEMENT
 wrote:

> The Armada 37xx SoC come with 2 pin controllers: one on the south
> bridge (managing 28 pins) and one on the north bridge (managing 36 pins).
>
> At the hardware level the controller configure the pins by group and not
> pin by pin. This constraint is reflected in the design of the driver:
> only the group related functions are implemented.

Interesting!

> +static int armada_37xx_pmx_direction_input(struct armada_37xx_pinctrl *info,
> +  unsigned int offset)
> +{
> +   unsigned int reg = OUTPUT_EN;
> +   unsigned int mask;
> +
> +   if (offset >= GPIO_PER_REG) {
> +   offset -= GPIO_PER_REG;
> +   reg += sizeof(u32);
> +   }
> +   mask = BIT(offset);
> +
> +   return regmap_update_bits(info->regmap, reg, mask, 0);
> +}
> +
> +
> +

A bit of excess whitespace, OK nitpicking.

Then this stuff:

> +static int armada_37xx_add_function(struct armada_37xx_pmx_func *funcs,
> +   int *funcsize, const char *name)
> +{
> +   int i = 0;
> +
> +   if (*funcsize <= 0)
> +   return -EOVERFLOW;
> +
> +   while (funcs->ngroups) {
> +   /* function already there */
> +   if (strcmp(funcs->name, name) == 0) {
> +   funcs->ngroups++;
> +
> +   return -EEXIST;
> +   }
> +   funcs++;
> +   i++;
> +   }
> +
> +   /* append new unique function */
> +   funcs->name = name;
> +   funcs->ngroups = 1;
> +   (*funcsize)--;
> +
> +   return 0;
> +}
> +
> +static int armada_37xx_fill_group(struct armada_37xx_pinctrl *info, int base)
> +{
> +   int n, num = 0, funcsize = info->data->nr_pins;
> +
> +   for (n = 0; n < info->ngroups; n++) {
> +   struct armada_37xx_pin_group *grp = >groups[n];
> +   int i, j, f;
> +
> +   grp->pins = devm_kzalloc(info->dev,
> +(grp->npins + grp->extra_npins) *
> +sizeof(*grp->pins), GFP_KERNEL);
> +   if (!grp->pins)
> +   return -ENOMEM;
> +
> +   for (i = 0; i < grp->npins; i++)
> +   grp->pins[i] = grp->start_pin + base + i;
> +
> +   for (j = 0; j < grp->extra_npins; j++)
> +   grp->pins[i+j] = grp->extra_pin + base + j;
> +
> +   for (f = 0; f < NB_FUNCS; f++) {
> +   int ret;
> +   /* check for unique functions and count groups */
> +   ret = armada_37xx_add_function(info->funcs, ,
> +   grp->funcs[f]);
> +   if (ret == -EOVERFLOW)
> +   dev_err(info->dev,
> +   "More functions than pins(%d)\n",
> +   info->data->nr_pins);
> +   if (ret < 0)
> +   continue;
> +   num++;
> +   }
> +   }
> +
> +   info->nfuncs = num;
> +
> +   return 0;
> +}
> +
> +static int armada_37xx_fill_func(struct armada_37xx_pinctrl *info)
> +{
> +   struct armada_37xx_pmx_func *funcs = info->funcs;
> +   int n;
> +
> +   for (n = 0; n < info->nfuncs; n++) {
> +   const char *name = funcs[n].name;
> +   const char **groups;
> +   int g;
> +
> +   funcs[n].groups = devm_kzalloc(info->dev, funcs[n].ngroups *
> +  sizeof(*(funcs[n].groups)),
> +  GFP_KERNEL);
> +   if (!funcs[n].groups)
> +   return -ENOMEM;
> +
> +   groups = funcs[n].groups;
> +
> +   for (g = 0; g < info->ngroups; g++) {
> +   struct armada_37xx_pin_group *gp = >groups[g];
> +   int f;
> +
> +   for (f = 0; f < NB_FUNCS; f++) {
> +   if (strcmp(gp->funcs[f], name) == 0) {
> +   *groups = gp->name;
> +   groups++;
> +   }
> +   }
> +   }
> +   }
> +   return 0;
> +}

I would be happy if you add kerneldoc to these functions and explain
what they do. Because I don't get it. I guess they are filling in the data
structures but yeah. Hard to follow.

> +   match = of_match_node(armada_37xx_pinctrl_of_match, np);
> +   info->data = (struct armada_37xx_pin_data *)match->data;

Use of_device_get_match_data()


> +static struct platform_driver armada_37xx_pinctrl_driver = {
> +   .driver = {
> +   .name = "armada-37xx-pinctrl",
> 

Re: [PATCH v2 3/7] pinctrl: armada-37xx: Add pin controller support for Armada 37xx

[Linus Walleij]

On Tue, Mar 21, 2017 at 7:28 PM, Gregory CLEMENT
 wrote:

> The Armada 37xx SoC come with 2 pin controllers: one on the south
> bridge (managing 28 pins) and one on the north bridge (managing 36 pins).
>
> At the hardware level the controller configure the pins by group and not
> pin by pin. This constraint is reflected in the design of the driver:
> only the group related functions are implemented.

Interesting!

> +static int armada_37xx_pmx_direction_input(struct armada_37xx_pinctrl *info,
> +  unsigned int offset)
> +{
> +   unsigned int reg = OUTPUT_EN;
> +   unsigned int mask;
> +
> +   if (offset >= GPIO_PER_REG) {
> +   offset -= GPIO_PER_REG;
> +   reg += sizeof(u32);
> +   }
> +   mask = BIT(offset);
> +
> +   return regmap_update_bits(info->regmap, reg, mask, 0);
> +}
> +
> +
> +

A bit of excess whitespace, OK nitpicking.

Then this stuff:

> +static int armada_37xx_add_function(struct armada_37xx_pmx_func *funcs,
> +   int *funcsize, const char *name)
> +{
> +   int i = 0;
> +
> +   if (*funcsize <= 0)
> +   return -EOVERFLOW;
> +
> +   while (funcs->ngroups) {
> +   /* function already there */
> +   if (strcmp(funcs->name, name) == 0) {
> +   funcs->ngroups++;
> +
> +   return -EEXIST;
> +   }
> +   funcs++;
> +   i++;
> +   }
> +
> +   /* append new unique function */
> +   funcs->name = name;
> +   funcs->ngroups = 1;
> +   (*funcsize)--;
> +
> +   return 0;
> +}
> +
> +static int armada_37xx_fill_group(struct armada_37xx_pinctrl *info, int base)
> +{
> +   int n, num = 0, funcsize = info->data->nr_pins;
> +
> +   for (n = 0; n < info->ngroups; n++) {
> +   struct armada_37xx_pin_group *grp = >groups[n];
> +   int i, j, f;
> +
> +   grp->pins = devm_kzalloc(info->dev,
> +(grp->npins + grp->extra_npins) *
> +sizeof(*grp->pins), GFP_KERNEL);
> +   if (!grp->pins)
> +   return -ENOMEM;
> +
> +   for (i = 0; i < grp->npins; i++)
> +   grp->pins[i] = grp->start_pin + base + i;
> +
> +   for (j = 0; j < grp->extra_npins; j++)
> +   grp->pins[i+j] = grp->extra_pin + base + j;
> +
> +   for (f = 0; f < NB_FUNCS; f++) {
> +   int ret;
> +   /* check for unique functions and count groups */
> +   ret = armada_37xx_add_function(info->funcs, ,
> +   grp->funcs[f]);
> +   if (ret == -EOVERFLOW)
> +   dev_err(info->dev,
> +   "More functions than pins(%d)\n",
> +   info->data->nr_pins);
> +   if (ret < 0)
> +   continue;
> +   num++;
> +   }
> +   }
> +
> +   info->nfuncs = num;
> +
> +   return 0;
> +}
> +
> +static int armada_37xx_fill_func(struct armada_37xx_pinctrl *info)
> +{
> +   struct armada_37xx_pmx_func *funcs = info->funcs;
> +   int n;
> +
> +   for (n = 0; n < info->nfuncs; n++) {
> +   const char *name = funcs[n].name;
> +   const char **groups;
> +   int g;
> +
> +   funcs[n].groups = devm_kzalloc(info->dev, funcs[n].ngroups *
> +  sizeof(*(funcs[n].groups)),
> +  GFP_KERNEL);
> +   if (!funcs[n].groups)
> +   return -ENOMEM;
> +
> +   groups = funcs[n].groups;
> +
> +   for (g = 0; g < info->ngroups; g++) {
> +   struct armada_37xx_pin_group *gp = >groups[g];
> +   int f;
> +
> +   for (f = 0; f < NB_FUNCS; f++) {
> +   if (strcmp(gp->funcs[f], name) == 0) {
> +   *groups = gp->name;
> +   groups++;
> +   }
> +   }
> +   }
> +   }
> +   return 0;
> +}

I would be happy if you add kerneldoc to these functions and explain
what they do. Because I don't get it. I guess they are filling in the data
structures but yeah. Hard to follow.

> +   match = of_match_node(armada_37xx_pinctrl_of_match, np);
> +   info->data = (struct armada_37xx_pin_data *)match->data;

Use of_device_get_match_data()


> +static struct platform_driver armada_37xx_pinctrl_driver = {
> +   .driver = {
> +   .name = "armada-37xx-pinctrl",
> +   .of_match_table = 

[PATCH v2 3/7] pinctrl: armada-37xx: Add pin controller support for Armada 37xx

[Gregory CLEMENT]

The Armada 37xx SoC come with 2 pin controllers: one on the south
bridge (managing 28 pins) and one on the north bridge (managing 36 pins).

At the hardware level the controller configure the pins by group and not
pin by pin. This constraint is reflected in the design of the driver:
only the group related functions are implemented.

Signed-off-by: Gregory CLEMENT 
---
 drivers/pinctrl/Makefile|   2 +-
 drivers/pinctrl/mvebu/Kconfig   |   7 +-
 drivers/pinctrl/mvebu/Makefile  |   3 +-
 drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 605 +-
 4 files changed, 615 insertions(+), 2 deletions(-)
 create mode 100644 drivers/pinctrl/mvebu/pinctrl-armada-37xx.c

diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index a251f439626f..95080811f36f 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -44,7 +44,7 @@ obj-y += bcm/
 obj-$(CONFIG_PINCTRL_BERLIN)   += berlin/
 obj-y  += freescale/
 obj-$(CONFIG_X86)  += intel/
-obj-$(CONFIG_PINCTRL_MVEBU)+= mvebu/
+obj-y  += mvebu/
 obj-y  += nomadik/
 obj-$(CONFIG_PINCTRL_PXA)  += pxa/
 obj-$(CONFIG_ARCH_QCOM)+= qcom/
diff --git a/drivers/pinctrl/mvebu/Kconfig b/drivers/pinctrl/mvebu/Kconfig
index 170602407c0d..5bade32d3089 100644
--- a/drivers/pinctrl/mvebu/Kconfig
+++ b/drivers/pinctrl/mvebu/Kconfig
@@ -39,3 +39,10 @@ config PINCTRL_ORION
select PINCTRL_MVEBU
 
 endif
+
+config PINCTRL_ARMADA_37XX
+   bool
+   select GENERIC_PINCONF
+   select MFD_SYSCON
+   select PINCONF
+   select PINMUX
diff --git a/drivers/pinctrl/mvebu/Makefile b/drivers/pinctrl/mvebu/Makefile
index 18270cd5ea43..60c245a60f39 100644
--- a/drivers/pinctrl/mvebu/Makefile
+++ b/drivers/pinctrl/mvebu/Makefile
@@ -1,4 +1,4 @@
-obj-y  += pinctrl-mvebu.o
+obj-$(CONFIG_PINCTRL_MVEBU)+= pinctrl-mvebu.o
 obj-$(CONFIG_PINCTRL_DOVE) += pinctrl-dove.o
 obj-$(CONFIG_PINCTRL_KIRKWOOD) += pinctrl-kirkwood.o
 obj-$(CONFIG_PINCTRL_ARMADA_370) += pinctrl-armada-370.o
@@ -6,4 +6,5 @@ obj-$(CONFIG_PINCTRL_ARMADA_375) += pinctrl-armada-375.o
 obj-$(CONFIG_PINCTRL_ARMADA_38X) += pinctrl-armada-38x.o
 obj-$(CONFIG_PINCTRL_ARMADA_39X) += pinctrl-armada-39x.o
 obj-$(CONFIG_PINCTRL_ARMADA_XP)  += pinctrl-armada-xp.o
+obj-$(CONFIG_PINCTRL_ARMADA_37XX)  += pinctrl-armada-37xx.o
 obj-$(CONFIG_PINCTRL_ORION)  += pinctrl-orion.o
diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c 
b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
new file mode 100644
index ..98cde04e07e1
--- /dev/null
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
@@ -0,0 +1,605 @@
+/*
+ * Marvell 37xx SoC pinctrl driver
+ *
+ * Copyright (C) 2017 Marvell
+ *
+ * Gregory CLEMENT 
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2 or later. This program is licensed "as is"
+ * without any warranty of any kind, whether express or implied.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "../pinctrl-utils.h"
+
+#define OUTPUT_EN  0x0
+#define OUTPUT_CTL 0x20
+#define SELECTION  0x30
+
+static int global_pin;
+
+#define NB_FUNCS 2
+#define GPIO_PER_REG   32
+
+/**
+ * struct armada_37xx_pin_group: represents group of pins of a pinmux function.
+ * The pins of a pinmux groups are composed of one or two groups of contiguous
+ * pins.
+ * @name:  Name of the pin group, used to lookup the group.
+ * @start_pins:Index of the first pin of the main range of pins 
belonging to
+ * the group
+ * @npins: Number of pins included in the first range
+ * @reg_mask:  Bit mask matching the group in the selection register
+ * @extra_pins:Index of the first pin of the optional second range of 
pins
+ * belonging to the group
+ * @npins: Number of pins included in the second optional range
+ * @funcs: A list of pinmux functions that can be selected for this group.
+ * @pins:  Total number pins included in the group
+ */
+struct armada_37xx_pin_group {
+   const char  *name;
+   unsigned intstart_pin;
+   unsigned intnpins;
+   u32 reg_mask;
+   unsigned intextra_pin;
+   unsigned intextra_npins;
+   const char  *funcs[NB_FUNCS];
+   unsigned int*pins;
+};
+
+struct armada_37xx_pin_data {
+   u8  nr_pins;
+   char*name;
+   struct armada_37xx_pin_group*groups;
+   int ngroups;
+};
+
+struct armada_37xx_pmx_func {
+   const char  *name;
+   const char  **groups;
+   unsigned intngroups;
+};
+
+struct 

[PATCH v2 3/7] pinctrl: armada-37xx: Add pin controller support for Armada 37xx

[Gregory CLEMENT]

The Armada 37xx SoC come with 2 pin controllers: one on the south
bridge (managing 28 pins) and one on the north bridge (managing 36 pins).

At the hardware level the controller configure the pins by group and not
pin by pin. This constraint is reflected in the design of the driver:
only the group related functions are implemented.

Signed-off-by: Gregory CLEMENT 
---
 drivers/pinctrl/Makefile|   2 +-
 drivers/pinctrl/mvebu/Kconfig   |   7 +-
 drivers/pinctrl/mvebu/Makefile  |   3 +-
 drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 605 +-
 4 files changed, 615 insertions(+), 2 deletions(-)
 create mode 100644 drivers/pinctrl/mvebu/pinctrl-armada-37xx.c

diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index a251f439626f..95080811f36f 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -44,7 +44,7 @@ obj-y += bcm/
 obj-$(CONFIG_PINCTRL_BERLIN)   += berlin/
 obj-y  += freescale/
 obj-$(CONFIG_X86)  += intel/
-obj-$(CONFIG_PINCTRL_MVEBU)+= mvebu/
+obj-y  += mvebu/
 obj-y  += nomadik/
 obj-$(CONFIG_PINCTRL_PXA)  += pxa/
 obj-$(CONFIG_ARCH_QCOM)+= qcom/
diff --git a/drivers/pinctrl/mvebu/Kconfig b/drivers/pinctrl/mvebu/Kconfig
index 170602407c0d..5bade32d3089 100644
--- a/drivers/pinctrl/mvebu/Kconfig
+++ b/drivers/pinctrl/mvebu/Kconfig
@@ -39,3 +39,10 @@ config PINCTRL_ORION
select PINCTRL_MVEBU
 
 endif
+
+config PINCTRL_ARMADA_37XX
+   bool
+   select GENERIC_PINCONF
+   select MFD_SYSCON
+   select PINCONF
+   select PINMUX
diff --git a/drivers/pinctrl/mvebu/Makefile b/drivers/pinctrl/mvebu/Makefile
index 18270cd5ea43..60c245a60f39 100644
--- a/drivers/pinctrl/mvebu/Makefile
+++ b/drivers/pinctrl/mvebu/Makefile
@@ -1,4 +1,4 @@
-obj-y  += pinctrl-mvebu.o
+obj-$(CONFIG_PINCTRL_MVEBU)+= pinctrl-mvebu.o
 obj-$(CONFIG_PINCTRL_DOVE) += pinctrl-dove.o
 obj-$(CONFIG_PINCTRL_KIRKWOOD) += pinctrl-kirkwood.o
 obj-$(CONFIG_PINCTRL_ARMADA_370) += pinctrl-armada-370.o
@@ -6,4 +6,5 @@ obj-$(CONFIG_PINCTRL_ARMADA_375) += pinctrl-armada-375.o
 obj-$(CONFIG_PINCTRL_ARMADA_38X) += pinctrl-armada-38x.o
 obj-$(CONFIG_PINCTRL_ARMADA_39X) += pinctrl-armada-39x.o
 obj-$(CONFIG_PINCTRL_ARMADA_XP)  += pinctrl-armada-xp.o
+obj-$(CONFIG_PINCTRL_ARMADA_37XX)  += pinctrl-armada-37xx.o
 obj-$(CONFIG_PINCTRL_ORION)  += pinctrl-orion.o
diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c 
b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
new file mode 100644
index ..98cde04e07e1
--- /dev/null
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
@@ -0,0 +1,605 @@
+/*
+ * Marvell 37xx SoC pinctrl driver
+ *
+ * Copyright (C) 2017 Marvell
+ *
+ * Gregory CLEMENT 
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2 or later. This program is licensed "as is"
+ * without any warranty of any kind, whether express or implied.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "../pinctrl-utils.h"
+
+#define OUTPUT_EN  0x0
+#define OUTPUT_CTL 0x20
+#define SELECTION  0x30
+
+static int global_pin;
+
+#define NB_FUNCS 2
+#define GPIO_PER_REG   32
+
+/**
+ * struct armada_37xx_pin_group: represents group of pins of a pinmux function.
+ * The pins of a pinmux groups are composed of one or two groups of contiguous
+ * pins.
+ * @name:  Name of the pin group, used to lookup the group.
+ * @start_pins:Index of the first pin of the main range of pins 
belonging to
+ * the group
+ * @npins: Number of pins included in the first range
+ * @reg_mask:  Bit mask matching the group in the selection register
+ * @extra_pins:Index of the first pin of the optional second range of 
pins
+ * belonging to the group
+ * @npins: Number of pins included in the second optional range
+ * @funcs: A list of pinmux functions that can be selected for this group.
+ * @pins:  Total number pins included in the group
+ */
+struct armada_37xx_pin_group {
+   const char  *name;
+   unsigned intstart_pin;
+   unsigned intnpins;
+   u32 reg_mask;
+   unsigned intextra_pin;
+   unsigned intextra_npins;
+   const char  *funcs[NB_FUNCS];
+   unsigned int*pins;
+};
+
+struct armada_37xx_pin_data {
+   u8  nr_pins;
+   char*name;
+   struct armada_37xx_pin_group*groups;
+   int ngroups;
+};
+
+struct armada_37xx_pmx_func {
+   const char  *name;
+   const char  **groups;
+   unsigned intngroups;
+};
+
+struct armada_37xx_pinctrl {
+   struct regmap   *regmap;
+