Re: [PATCH v4 02/10] pinctrl: axp209: add pinctrl features

[kbuild test robot]

Hi Quentin,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on pinctrl/devel]
[also build test ERROR on v4.15-rc2 next-20171205]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Quentin-Schulz/pinctrl-move-gpio-axp209-to-pinctrl/20171204-050707
base:   
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
config: i386-randconfig-h1-12060258 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

>> drivers//pinctrl/pinctrl-axp209.c:290:21: error: 
>> 'pinconf_generic_dt_node_to_map_group' undeclared here (not in a function)
 .dt_node_to_map  = pinconf_generic_dt_node_to_map_group,
^~~~
>> drivers//pinctrl/pinctrl-axp209.c:291:18: error: 
>> 'pinconf_generic_dt_free_map' undeclared here (not in a function)
 .dt_free_map  = pinconf_generic_dt_free_map,
 ^~~

vim +/pinconf_generic_dt_node_to_map_group +290 
drivers//pinctrl/pinctrl-axp209.c

   288  
   289  static const struct pinctrl_ops axp20x_pctrl_ops = {
 > 290  .dt_node_to_map = pinconf_generic_dt_node_to_map_group,
 > 291  .dt_free_map= pinconf_generic_dt_free_map,
   292  .get_groups_count   = axp20x_groups_cnt,
   293  .get_group_name = axp20x_group_name,
   294  .get_group_pins = axp20x_group_pins,
   295  };
   296  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v4 02/10] pinctrl: axp209: add pinctrl features

[kbuild test robot]

Hi Quentin,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on pinctrl/devel]
[also build test ERROR on v4.15-rc2 next-20171205]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Quentin-Schulz/pinctrl-move-gpio-axp209-to-pinctrl/20171204-050707
base:   
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
config: i386-randconfig-h1-12060258 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

>> drivers//pinctrl/pinctrl-axp209.c:290:21: error: 
>> 'pinconf_generic_dt_node_to_map_group' undeclared here (not in a function)
 .dt_node_to_map  = pinconf_generic_dt_node_to_map_group,
^~~~
>> drivers//pinctrl/pinctrl-axp209.c:291:18: error: 
>> 'pinconf_generic_dt_free_map' undeclared here (not in a function)
 .dt_free_map  = pinconf_generic_dt_free_map,
 ^~~

vim +/pinconf_generic_dt_node_to_map_group +290 
drivers//pinctrl/pinctrl-axp209.c

   288  
   289  static const struct pinctrl_ops axp20x_pctrl_ops = {
 > 290  .dt_node_to_map = pinconf_generic_dt_node_to_map_group,
 > 291  .dt_free_map= pinconf_generic_dt_free_map,
   292  .get_groups_count   = axp20x_groups_cnt,
   293  .get_group_name = axp20x_group_name,
   294  .get_group_pins = axp20x_group_pins,
   295  };
   296  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v4 02/10] pinctrl: axp209: add pinctrl features

[Maxime Ripard]

Hi,

On Mon, Dec 04, 2017 at 09:07:52AM +0100, Quentin Schulz wrote:
> >> +static int axp20x_pmx_set_mux(struct pinctrl_dev *pctldev,
> >> +unsigned int function, unsigned int group)
> >> +{
> >> +  struct axp20x_gpio *gpio = pinctrl_dev_get_drvdata(pctldev);
> >> +  unsigned int mask;
> >> +
> >> +  /* Every pin supports GPIO_OUT and GPIO_IN functions */
> >> +  if (function <= AXP20X_FUNC_GPIO_IN)
> >> +  return axp20x_pmx_set(pctldev, group,
> >> +gpio->funcs[function].muxval);
> >> +
> >> +  if (function == AXP20X_FUNC_LDO)
> >> +  mask = gpio->desc->ldo_mask;
> >> +  else
> >> +  mask = gpio->desc->adc_mask;
> > 
> > What is the point of this test...
> > 
> >> +  if (!(BIT(group) & mask))
> >> +  return -EINVAL;
> >> +
> >> +  /*
> >> +   * We let the regulator framework handle the LDO muxing as muxing bits
> >> +   * are basically also regulators on/off bits. It's better not to enforce
> >> +   * any state of the regulator when selecting LDO mux so that we don't
> >> +   * interfere with the regulator driver.
> >> +   */
> >> +  if (function == AXP20X_FUNC_LDO)
> >> +  return 0;
> > 
> > ... if you know that you're not going to do anything with one of the
> > outcomes. It would be better to just move that part above, instead of
> > doing the same test twice.
> > 
> 
> Return value is different. In one case, it is an error to request "ldo"
> for a pin that does not support it. In the other case, the ldo request
> is valid but nothing's done on driver side.
> 
> Both cases are handled differently by the core:
> http://elixir.free-electrons.com/linux/latest/source/drivers/pinctrl/pinmux.c#L439
> 
> I think that's the behavior we're expecting from this driver.

Ah, right.

> Or maybe you're asking to do:
> 
> + if (function == AXP20X_FUNC_LDO) {
> + if (!(BIT(group) & gpio->desc->ldo_mask))
> + return -EINVAL;
> + return 0;
> + } else if (!(BIT(group) & gpio->desc->adc_mask)) {
> + return -EINVAL;
> + }
> 
> ?

No, it's definitely better the way you did it.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [PATCH v4 02/10] pinctrl: axp209: add pinctrl features

[Maxime Ripard]

Hi,

On Mon, Dec 04, 2017 at 09:07:52AM +0100, Quentin Schulz wrote:
> >> +static int axp20x_pmx_set_mux(struct pinctrl_dev *pctldev,
> >> +unsigned int function, unsigned int group)
> >> +{
> >> +  struct axp20x_gpio *gpio = pinctrl_dev_get_drvdata(pctldev);
> >> +  unsigned int mask;
> >> +
> >> +  /* Every pin supports GPIO_OUT and GPIO_IN functions */
> >> +  if (function <= AXP20X_FUNC_GPIO_IN)
> >> +  return axp20x_pmx_set(pctldev, group,
> >> +gpio->funcs[function].muxval);
> >> +
> >> +  if (function == AXP20X_FUNC_LDO)
> >> +  mask = gpio->desc->ldo_mask;
> >> +  else
> >> +  mask = gpio->desc->adc_mask;
> > 
> > What is the point of this test...
> > 
> >> +  if (!(BIT(group) & mask))
> >> +  return -EINVAL;
> >> +
> >> +  /*
> >> +   * We let the regulator framework handle the LDO muxing as muxing bits
> >> +   * are basically also regulators on/off bits. It's better not to enforce
> >> +   * any state of the regulator when selecting LDO mux so that we don't
> >> +   * interfere with the regulator driver.
> >> +   */
> >> +  if (function == AXP20X_FUNC_LDO)
> >> +  return 0;
> > 
> > ... if you know that you're not going to do anything with one of the
> > outcomes. It would be better to just move that part above, instead of
> > doing the same test twice.
> > 
> 
> Return value is different. In one case, it is an error to request "ldo"
> for a pin that does not support it. In the other case, the ldo request
> is valid but nothing's done on driver side.
> 
> Both cases are handled differently by the core:
> http://elixir.free-electrons.com/linux/latest/source/drivers/pinctrl/pinmux.c#L439
> 
> I think that's the behavior we're expecting from this driver.

Ah, right.

> Or maybe you're asking to do:
> 
> + if (function == AXP20X_FUNC_LDO) {
> + if (!(BIT(group) & gpio->desc->ldo_mask))
> + return -EINVAL;
> + return 0;
> + } else if (!(BIT(group) & gpio->desc->adc_mask)) {
> + return -EINVAL;
> + }
> 
> ?

No, it's definitely better the way you did it.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [PATCH v4 02/10] pinctrl: axp209: add pinctrl features

[Quentin Schulz]

Hi Maxime,

On 01/12/2017 16:57, Maxime Ripard wrote:
> On Fri, Dec 01, 2017 at 02:44:43PM +0100, Quentin Schulz wrote:
>> +static void axp20x_gpio_set(struct gpio_chip *chip, unsigned offset,
>> +int value)
>> +{
> 
> checkpatch output:
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> 
>> +static int axp20x_pmx_set_mux(struct pinctrl_dev *pctldev,
>> +  unsigned int function, unsigned int group)
>> +{
>> +struct axp20x_gpio *gpio = pinctrl_dev_get_drvdata(pctldev);
>> +unsigned int mask;
>> +
>> +/* Every pin supports GPIO_OUT and GPIO_IN functions */
>> +if (function <= AXP20X_FUNC_GPIO_IN)
>> +return axp20x_pmx_set(pctldev, group,
>> +  gpio->funcs[function].muxval);
>> +
>> +if (function == AXP20X_FUNC_LDO)
>> +mask = gpio->desc->ldo_mask;
>> +else
>> +mask = gpio->desc->adc_mask;
> 
> What is the point of this test...
> 
>> +if (!(BIT(group) & mask))
>> +return -EINVAL;
>> +
>> +/*
>> + * We let the regulator framework handle the LDO muxing as muxing bits
>> + * are basically also regulators on/off bits. It's better not to enforce
>> + * any state of the regulator when selecting LDO mux so that we don't
>> + * interfere with the regulator driver.
>> + */
>> +if (function == AXP20X_FUNC_LDO)
>> +return 0;
> 
> ... if you know that you're not going to do anything with one of the
> outcomes. It would be better to just move that part above, instead of
> doing the same test twice.
> 

Return value is different. In one case, it is an error to request "ldo"
for a pin that does not support it. In the other case, the ldo request
is valid but nothing's done on driver side.

Both cases are handled differently by the core:
http://elixir.free-electrons.com/linux/latest/source/drivers/pinctrl/pinmux.c#L439

I think that's the behavior we're expecting from this driver.

Or maybe you're asking to do:

+   if (function == AXP20X_FUNC_LDO) {
+   if (!(BIT(group) & gpio->desc->ldo_mask))
+   return -EINVAL;
+   return 0;
+   } else if (!(BIT(group) & gpio->desc->adc_mask)) {
+   return -EINVAL;
+   }

?

Thanks,
Quentin
-- 
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v4 02/10] pinctrl: axp209: add pinctrl features

[Quentin Schulz]

Hi Maxime,

On 01/12/2017 16:57, Maxime Ripard wrote:
> On Fri, Dec 01, 2017 at 02:44:43PM +0100, Quentin Schulz wrote:
>> +static void axp20x_gpio_set(struct gpio_chip *chip, unsigned offset,
>> +int value)
>> +{
> 
> checkpatch output:
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> 
>> +static int axp20x_pmx_set_mux(struct pinctrl_dev *pctldev,
>> +  unsigned int function, unsigned int group)
>> +{
>> +struct axp20x_gpio *gpio = pinctrl_dev_get_drvdata(pctldev);
>> +unsigned int mask;
>> +
>> +/* Every pin supports GPIO_OUT and GPIO_IN functions */
>> +if (function <= AXP20X_FUNC_GPIO_IN)
>> +return axp20x_pmx_set(pctldev, group,
>> +  gpio->funcs[function].muxval);
>> +
>> +if (function == AXP20X_FUNC_LDO)
>> +mask = gpio->desc->ldo_mask;
>> +else
>> +mask = gpio->desc->adc_mask;
> 
> What is the point of this test...
> 
>> +if (!(BIT(group) & mask))
>> +return -EINVAL;
>> +
>> +/*
>> + * We let the regulator framework handle the LDO muxing as muxing bits
>> + * are basically also regulators on/off bits. It's better not to enforce
>> + * any state of the regulator when selecting LDO mux so that we don't
>> + * interfere with the regulator driver.
>> + */
>> +if (function == AXP20X_FUNC_LDO)
>> +return 0;
> 
> ... if you know that you're not going to do anything with one of the
> outcomes. It would be better to just move that part above, instead of
> doing the same test twice.
> 

Return value is different. In one case, it is an error to request "ldo"
for a pin that does not support it. In the other case, the ldo request
is valid but nothing's done on driver side.

Both cases are handled differently by the core:
http://elixir.free-electrons.com/linux/latest/source/drivers/pinctrl/pinmux.c#L439

I think that's the behavior we're expecting from this driver.

Or maybe you're asking to do:

+   if (function == AXP20X_FUNC_LDO) {
+   if (!(BIT(group) & gpio->desc->ldo_mask))
+   return -EINVAL;
+   return 0;
+   } else if (!(BIT(group) & gpio->desc->adc_mask)) {
+   return -EINVAL;
+   }

?

Thanks,
Quentin
-- 
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v4 02/10] pinctrl: axp209: add pinctrl features

[Maxime Ripard]

On Fri, Dec 01, 2017 at 02:44:43PM +0100, Quentin Schulz wrote:
> +static void axp20x_gpio_set(struct gpio_chip *chip, unsigned offset,
> + int value)
> +{

checkpatch output:
WARNING: Prefer 'unsigned int' to bare use of 'unsigned'

> +static int axp20x_pmx_set_mux(struct pinctrl_dev *pctldev,
> +   unsigned int function, unsigned int group)
> +{
> + struct axp20x_gpio *gpio = pinctrl_dev_get_drvdata(pctldev);
> + unsigned int mask;
> +
> + /* Every pin supports GPIO_OUT and GPIO_IN functions */
> + if (function <= AXP20X_FUNC_GPIO_IN)
> + return axp20x_pmx_set(pctldev, group,
> +   gpio->funcs[function].muxval);
> +
> + if (function == AXP20X_FUNC_LDO)
> + mask = gpio->desc->ldo_mask;
> + else
> + mask = gpio->desc->adc_mask;

What is the point of this test...

> + if (!(BIT(group) & mask))
> + return -EINVAL;
> +
> + /*
> +  * We let the regulator framework handle the LDO muxing as muxing bits
> +  * are basically also regulators on/off bits. It's better not to enforce
> +  * any state of the regulator when selecting LDO mux so that we don't
> +  * interfere with the regulator driver.
> +  */
> + if (function == AXP20X_FUNC_LDO)
> + return 0;

... if you know that you're not going to do anything with one of the
outcomes. It would be better to just move that part above, instead of
doing the same test twice.

It looks good otherwise, thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [PATCH v4 02/10] pinctrl: axp209: add pinctrl features

[Maxime Ripard]

On Fri, Dec 01, 2017 at 02:44:43PM +0100, Quentin Schulz wrote:
> +static void axp20x_gpio_set(struct gpio_chip *chip, unsigned offset,
> + int value)
> +{

checkpatch output:
WARNING: Prefer 'unsigned int' to bare use of 'unsigned'

> +static int axp20x_pmx_set_mux(struct pinctrl_dev *pctldev,
> +   unsigned int function, unsigned int group)
> +{
> + struct axp20x_gpio *gpio = pinctrl_dev_get_drvdata(pctldev);
> + unsigned int mask;
> +
> + /* Every pin supports GPIO_OUT and GPIO_IN functions */
> + if (function <= AXP20X_FUNC_GPIO_IN)
> + return axp20x_pmx_set(pctldev, group,
> +   gpio->funcs[function].muxval);
> +
> + if (function == AXP20X_FUNC_LDO)
> + mask = gpio->desc->ldo_mask;
> + else
> + mask = gpio->desc->adc_mask;

What is the point of this test...

> + if (!(BIT(group) & mask))
> + return -EINVAL;
> +
> + /*
> +  * We let the regulator framework handle the LDO muxing as muxing bits
> +  * are basically also regulators on/off bits. It's better not to enforce
> +  * any state of the regulator when selecting LDO mux so that we don't
> +  * interfere with the regulator driver.
> +  */
> + if (function == AXP20X_FUNC_LDO)
> + return 0;

... if you know that you're not going to do anything with one of the
outcomes. It would be better to just move that part above, instead of
doing the same test twice.

It looks good otherwise, thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

[PATCH v4 02/10] pinctrl: axp209: add pinctrl features

[Quentin Schulz]

The X-Powers AXP209 has 3 GPIOs. GPIO0/1 can each act either as a GPIO,
an ADC or a LDO regulator. GPIO2 can only act as a GPIO.

This adds the pinctrl features to the driver so GPIO0/1 can be used as
ADC or LDO regulator.

Signed-off-by: Quentin Schulz 
---
 drivers/pinctrl/pinctrl-axp209.c | 306 +---
 1 file changed, 286 insertions(+), 20 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-axp209.c b/drivers/pinctrl/pinctrl-axp209.c
index 4a346b7..2dc286f 100644
--- a/drivers/pinctrl/pinctrl-axp209.c
+++ b/drivers/pinctrl/pinctrl-axp209.c
@@ -1,7 +1,8 @@
 /*
- * AXP20x GPIO driver
+ * AXP20x pinctrl and GPIO driver
  *
  * Copyright (C) 2016 Maxime Ripard 
+ * Copyright (C) 2017 Quentin Schulz 
  *
  * This program is free software; you can redistribute it and/or modify it
  * under  the terms of the GNU General  Public License as published by the
@@ -18,6 +19,9 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -27,9 +31,52 @@
 #define AXP20X_GPIO_FUNCTION_OUT_HIGH  1
 #define AXP20X_GPIO_FUNCTION_INPUT 2
 
+#define AXP20X_FUNC_GPIO_OUT   0
+#define AXP20X_FUNC_GPIO_IN1
+#define AXP20X_FUNC_LDO2
+#define AXP20X_FUNC_ADC3
+#define AXP20X_FUNCS_NB4
+
+#define AXP20X_MUX_GPIO_OUT0
+#define AXP20X_MUX_GPIO_IN BIT(1)
+#define AXP20X_MUX_ADC BIT(2)
+
+struct axp20x_pctrl_desc {
+   const struct pinctrl_pin_desc   *pins;
+   unsigned intnpins;
+   /* Stores the pins supporting LDO function. Bit offset is pin number. */
+   u8  ldo_mask;
+   /* Stores the pins supporting ADC function. Bit offset is pin number. */
+   u8  adc_mask;
+};
+
+struct axp20x_pinctrl_function {
+   const char  *name;
+   unsigned intmuxval;
+   const char  **groups;
+   unsigned intngroups;
+};
+
 struct axp20x_gpio {
struct gpio_chipchip;
struct regmap   *regmap;
+   struct pinctrl_dev  *pctl_dev;
+   struct device   *dev;
+   const struct axp20x_pctrl_desc  *desc;
+   struct axp20x_pinctrl_function  funcs[AXP20X_FUNCS_NB];
+};
+
+static const struct pinctrl_pin_desc axp209_pins[] = {
+   PINCTRL_PIN(0, "GPIO0"),
+   PINCTRL_PIN(1, "GPIO1"),
+   PINCTRL_PIN(2, "GPIO2"),
+};
+
+static const struct axp20x_pctrl_desc axp20x_data = {
+   .pins   = axp209_pins,
+   .npins  = ARRAY_SIZE(axp209_pins),
+   .ldo_mask = BIT(0) | BIT(1),
+   .adc_mask = BIT(0) | BIT(1),
 };
 
 static int axp20x_gpio_get_reg(unsigned offset)
@@ -48,16 +95,7 @@ static int axp20x_gpio_get_reg(unsigned offset)
 
 static int axp20x_gpio_input(struct gpio_chip *chip, unsigned offset)
 {
-   struct axp20x_gpio *gpio = gpiochip_get_data(chip);
-   int reg;
-
-   reg = axp20x_gpio_get_reg(offset);
-   if (reg < 0)
-   return reg;
-
-   return regmap_update_bits(gpio->regmap, reg,
- AXP20X_GPIO_FUNCTIONS,
- AXP20X_GPIO_FUNCTION_INPUT);
+   return pinctrl_gpio_direction_input(chip->base + offset);
 }
 
 static int axp20x_gpio_get(struct gpio_chip *chip, unsigned offset)
@@ -105,29 +143,222 @@ static int axp20x_gpio_get_direction(struct gpio_chip 
*chip, unsigned offset)
 static int axp20x_gpio_output(struct gpio_chip *chip, unsigned offset,
  int value)
 {
+   chip->set(chip, offset, value);
+
+   return 0;
+}
+
+static void axp20x_gpio_set(struct gpio_chip *chip, unsigned offset,
+   int value)
+{
struct axp20x_gpio *gpio = gpiochip_get_data(chip);
int reg;
 
reg = axp20x_gpio_get_reg(offset);
if (reg < 0)
+   return;
+
+   regmap_update_bits(gpio->regmap, reg,
+  AXP20X_GPIO_FUNCTIONS,
+  value ? AXP20X_GPIO_FUNCTION_OUT_HIGH :
+  AXP20X_GPIO_FUNCTION_OUT_LOW);
+}
+
+static int axp20x_pmx_set(struct pinctrl_dev *pctldev, unsigned int offset,
+ u8 config)
+{
+   struct axp20x_gpio *gpio = pinctrl_dev_get_drvdata(pctldev);
+   int reg;
+
+   reg = axp20x_gpio_get_reg(offset);
+   if (reg < 0)
return reg;
 
-   return regmap_update_bits(gpio->regmap, reg,
- AXP20X_GPIO_FUNCTIONS,
- value ? AXP20X_GPIO_FUNCTION_OUT_HIGH
- : AXP20X_GPIO_FUNCTION_OUT_LOW);
+   return regmap_update_bits(gpio->regmap, reg, AXP20X_GPIO_FUNCTIONS,
+ 

[PATCH v4 02/10] pinctrl: axp209: add pinctrl features

[Quentin Schulz]

The X-Powers AXP209 has 3 GPIOs. GPIO0/1 can each act either as a GPIO,
an ADC or a LDO regulator. GPIO2 can only act as a GPIO.

This adds the pinctrl features to the driver so GPIO0/1 can be used as
ADC or LDO regulator.

Signed-off-by: Quentin Schulz 
---
 drivers/pinctrl/pinctrl-axp209.c | 306 +---
 1 file changed, 286 insertions(+), 20 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-axp209.c b/drivers/pinctrl/pinctrl-axp209.c
index 4a346b7..2dc286f 100644
--- a/drivers/pinctrl/pinctrl-axp209.c
+++ b/drivers/pinctrl/pinctrl-axp209.c
@@ -1,7 +1,8 @@
 /*
- * AXP20x GPIO driver
+ * AXP20x pinctrl and GPIO driver
  *
  * Copyright (C) 2016 Maxime Ripard 
+ * Copyright (C) 2017 Quentin Schulz 
  *
  * This program is free software; you can redistribute it and/or modify it
  * under  the terms of the GNU General  Public License as published by the
@@ -18,6 +19,9 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -27,9 +31,52 @@
 #define AXP20X_GPIO_FUNCTION_OUT_HIGH  1
 #define AXP20X_GPIO_FUNCTION_INPUT 2
 
+#define AXP20X_FUNC_GPIO_OUT   0
+#define AXP20X_FUNC_GPIO_IN1
+#define AXP20X_FUNC_LDO2
+#define AXP20X_FUNC_ADC3
+#define AXP20X_FUNCS_NB4
+
+#define AXP20X_MUX_GPIO_OUT0
+#define AXP20X_MUX_GPIO_IN BIT(1)
+#define AXP20X_MUX_ADC BIT(2)
+
+struct axp20x_pctrl_desc {
+   const struct pinctrl_pin_desc   *pins;
+   unsigned intnpins;
+   /* Stores the pins supporting LDO function. Bit offset is pin number. */
+   u8  ldo_mask;
+   /* Stores the pins supporting ADC function. Bit offset is pin number. */
+   u8  adc_mask;
+};
+
+struct axp20x_pinctrl_function {
+   const char  *name;
+   unsigned intmuxval;
+   const char  **groups;
+   unsigned intngroups;
+};
+
 struct axp20x_gpio {
struct gpio_chipchip;
struct regmap   *regmap;
+   struct pinctrl_dev  *pctl_dev;
+   struct device   *dev;
+   const struct axp20x_pctrl_desc  *desc;
+   struct axp20x_pinctrl_function  funcs[AXP20X_FUNCS_NB];
+};
+
+static const struct pinctrl_pin_desc axp209_pins[] = {
+   PINCTRL_PIN(0, "GPIO0"),
+   PINCTRL_PIN(1, "GPIO1"),
+   PINCTRL_PIN(2, "GPIO2"),
+};
+
+static const struct axp20x_pctrl_desc axp20x_data = {
+   .pins   = axp209_pins,
+   .npins  = ARRAY_SIZE(axp209_pins),
+   .ldo_mask = BIT(0) | BIT(1),
+   .adc_mask = BIT(0) | BIT(1),
 };
 
 static int axp20x_gpio_get_reg(unsigned offset)
@@ -48,16 +95,7 @@ static int axp20x_gpio_get_reg(unsigned offset)
 
 static int axp20x_gpio_input(struct gpio_chip *chip, unsigned offset)
 {
-   struct axp20x_gpio *gpio = gpiochip_get_data(chip);
-   int reg;
-
-   reg = axp20x_gpio_get_reg(offset);
-   if (reg < 0)
-   return reg;
-
-   return regmap_update_bits(gpio->regmap, reg,
- AXP20X_GPIO_FUNCTIONS,
- AXP20X_GPIO_FUNCTION_INPUT);
+   return pinctrl_gpio_direction_input(chip->base + offset);
 }
 
 static int axp20x_gpio_get(struct gpio_chip *chip, unsigned offset)
@@ -105,29 +143,222 @@ static int axp20x_gpio_get_direction(struct gpio_chip 
*chip, unsigned offset)
 static int axp20x_gpio_output(struct gpio_chip *chip, unsigned offset,
  int value)
 {
+   chip->set(chip, offset, value);
+
+   return 0;
+}
+
+static void axp20x_gpio_set(struct gpio_chip *chip, unsigned offset,
+   int value)
+{
struct axp20x_gpio *gpio = gpiochip_get_data(chip);
int reg;
 
reg = axp20x_gpio_get_reg(offset);
if (reg < 0)
+   return;
+
+   regmap_update_bits(gpio->regmap, reg,
+  AXP20X_GPIO_FUNCTIONS,
+  value ? AXP20X_GPIO_FUNCTION_OUT_HIGH :
+  AXP20X_GPIO_FUNCTION_OUT_LOW);
+}
+
+static int axp20x_pmx_set(struct pinctrl_dev *pctldev, unsigned int offset,
+ u8 config)
+{
+   struct axp20x_gpio *gpio = pinctrl_dev_get_drvdata(pctldev);
+   int reg;
+
+   reg = axp20x_gpio_get_reg(offset);
+   if (reg < 0)
return reg;
 
-   return regmap_update_bits(gpio->regmap, reg,
- AXP20X_GPIO_FUNCTIONS,
- value ? AXP20X_GPIO_FUNCTION_OUT_HIGH
- : AXP20X_GPIO_FUNCTION_OUT_LOW);
+   return regmap_update_bits(gpio->regmap, reg, AXP20X_GPIO_FUNCTIONS,
+ config);
 }
 
-static void axp20x_gpio_set(struct gpio_chip *chip, unsigned offset,
-