diff mbox

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

Message ID 71c9da94df2a5938cb8c092e40f8e36eec0b01c3.1512135804.git-series.quentin.schulz@free-electrons.com (mailing list archive)
State New, archived
Headers show

Commit Message

Quentin Schulz Dec. 1, 2017, 1:44 p.m. UTC
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 <quentin.schulz@free-electrons.com>
---
 drivers/pinctrl/pinctrl-axp209.c | 306 +++++++++++++++++++++++++++++---
 1 file changed, 286 insertions(+), 20 deletions(-)

Comments

Maxime Ripard Dec. 1, 2017, 3:57 p.m. UTC | #1
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
Quentin Schulz Dec. 4, 2017, 8:07 a.m. UTC | #2
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
Maxime Ripard Dec. 5, 2017, 8:53 a.m. UTC | #3
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
kernel test robot Dec. 5, 2017, 11:21 p.m. UTC | #4
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 infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox

Patch

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 <maxime.ripard@free-electrons.com>
+ * Copyright (C) 2017 Quentin Schulz <quentin.schulz@free-electrons.com>
  *
  * 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 <linux/mfd/axp20x.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
@@ -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_IN		1
+#define AXP20X_FUNC_LDO			2
+#define AXP20X_FUNC_ADC			3
+#define AXP20X_FUNCS_NB			4
+
+#define AXP20X_MUX_GPIO_OUT		0
+#define AXP20X_MUX_GPIO_IN		BIT(1)
+#define AXP20X_MUX_ADC			BIT(2)
+
+struct axp20x_pctrl_desc {
+	const struct pinctrl_pin_desc	*pins;
+	unsigned int			npins;
+	/* 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 int	muxval;
+	const char	**groups;
+	unsigned int	ngroups;
+};
+
 struct axp20x_gpio {
 	struct gpio_chip	chip;
 	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,
-			    int value)
+static int axp20x_pmx_func_cnt(struct pinctrl_dev *pctldev)
+{
+	struct axp20x_gpio *gpio = pinctrl_dev_get_drvdata(pctldev);
+
+	return ARRAY_SIZE(gpio->funcs);
+}
+
+static const char *axp20x_pmx_func_name(struct pinctrl_dev *pctldev,
+					unsigned int selector)
+{
+	struct axp20x_gpio *gpio = pinctrl_dev_get_drvdata(pctldev);
+
+	return gpio->funcs[selector].name;
+}
+
+static int axp20x_pmx_func_groups(struct pinctrl_dev *pctldev,
+				  unsigned int selector,
+				  const char * const **groups,
+				  unsigned int *num_groups)
+{
+	struct axp20x_gpio *gpio = pinctrl_dev_get_drvdata(pctldev);
+
+	*groups = gpio->funcs[selector].groups;
+	*num_groups = gpio->funcs[selector].ngroups;
+
+	return 0;
+}
+
+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;
+
+	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;
+
+	return axp20x_pmx_set(pctldev, group, gpio->funcs[function].muxval);
+}
+
+static int axp20x_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
+					 struct pinctrl_gpio_range *range,
+					 unsigned int offset, bool input)
+{
+	struct axp20x_gpio *gpio = pinctrl_dev_get_drvdata(pctldev);
+
+	if (input)
+		return axp20x_pmx_set(pctldev, offset,
+				      gpio->funcs[AXP20X_FUNC_GPIO_IN].muxval);
+
+	return axp20x_pmx_set(pctldev, offset,
+			      gpio->funcs[AXP20X_FUNC_GPIO_OUT].muxval);
+}
+
+static const struct pinmux_ops axp20x_pmx_ops = {
+	.get_functions_count	= axp20x_pmx_func_cnt,
+	.get_function_name	= axp20x_pmx_func_name,
+	.get_function_groups	= axp20x_pmx_func_groups,
+	.set_mux		= axp20x_pmx_set_mux,
+	.gpio_set_direction	= axp20x_pmx_gpio_set_direction,
+	.strict			= true,
+};
+
+static int axp20x_groups_cnt(struct pinctrl_dev *pctldev)
+{
+	struct axp20x_gpio *gpio = pinctrl_dev_get_drvdata(pctldev);
+
+	return gpio->desc->npins;
+}
+
+static int axp20x_group_pins(struct pinctrl_dev *pctldev, unsigned int selector,
+			     const unsigned int **pins, unsigned int *num_pins)
+{
+	struct axp20x_gpio *gpio = pinctrl_dev_get_drvdata(pctldev);
+
+	*pins = (unsigned int *)&gpio->desc->pins[selector];
+	*num_pins = 1;
+
+	return 0;
+}
+
+static const char *axp20x_group_name(struct pinctrl_dev *pctldev,
+				     unsigned int selector)
 {
-	axp20x_gpio_output(chip, offset, value);
+	struct axp20x_gpio *gpio = pinctrl_dev_get_drvdata(pctldev);
+
+	return gpio->desc->pins[selector].name;
+}
+
+static const struct pinctrl_ops axp20x_pctrl_ops = {
+	.dt_node_to_map		= pinconf_generic_dt_node_to_map_group,
+	.dt_free_map		= pinconf_generic_dt_free_map,
+	.get_groups_count	= axp20x_groups_cnt,
+	.get_group_name		= axp20x_group_name,
+	.get_group_pins		= axp20x_group_pins,
+};
+
+static void axp20x_funcs_groups_from_mask(struct device *dev, unsigned int mask,
+					  unsigned int mask_len,
+					  struct axp20x_pinctrl_function *func,
+					  const struct pinctrl_pin_desc *pins)
+{
+	unsigned long int mask_cpy = mask;
+	const char **group;
+	unsigned int ngroups = hweight8(mask);
+	int bit;
+
+	func->ngroups = ngroups;
+	if (func->ngroups > 0) {
+		func->groups = devm_kzalloc(dev, ngroups * sizeof(const char *),
+					    GFP_KERNEL);
+		group = func->groups;
+		for_each_set_bit(bit, &mask_cpy, mask_len) {
+			*group = pins[bit].name;
+			group++;
+		}
+	}
+}
+
+static void axp20x_build_funcs_groups(struct platform_device *pdev)
+{
+	struct axp20x_gpio *gpio = platform_get_drvdata(pdev);
+	int i, pin;
+
+	gpio->funcs[AXP20X_FUNC_GPIO_OUT].name = "gpio_out";
+	gpio->funcs[AXP20X_FUNC_GPIO_OUT].muxval = AXP20X_MUX_GPIO_OUT;
+	gpio->funcs[AXP20X_FUNC_GPIO_IN].name = "gpio_in";
+	gpio->funcs[AXP20X_FUNC_GPIO_IN].muxval = AXP20X_MUX_GPIO_IN;
+	gpio->funcs[AXP20X_FUNC_LDO].name = "ldo";
+	/*
+	 * Muxval for LDO is useless as we won't use it.
+	 * See comment in axp20x_pmx_set_mux.
+	 */
+	gpio->funcs[AXP20X_FUNC_ADC].name = "adc";
+	gpio->funcs[AXP20X_FUNC_ADC].muxval = AXP20X_MUX_ADC;
+
+	/* Every pin supports GPIO_OUT and GPIO_IN functions */
+	for (i = 0; i <= AXP20X_FUNC_GPIO_IN; i++) {
+		gpio->funcs[i].ngroups = gpio->desc->npins;
+		gpio->funcs[i].groups = devm_kzalloc(&pdev->dev,
+						     gpio->desc->npins * sizeof(const char *),
+						     GFP_KERNEL);
+		for (pin = 0; pin < gpio->desc->npins; pin++)
+			gpio->funcs[i].groups[pin] = gpio->desc->pins[pin].name;
+	}
+
+	axp20x_funcs_groups_from_mask(&pdev->dev, gpio->desc->ldo_mask,
+				      gpio->desc->npins,
+				      &gpio->funcs[AXP20X_FUNC_LDO],
+				      gpio->desc->pins);
+
+	axp20x_funcs_groups_from_mask(&pdev->dev, gpio->desc->adc_mask,
+				      gpio->desc->npins,
+				      &gpio->funcs[AXP20X_FUNC_ADC],
+				      gpio->desc->pins);
 }
 
 static int axp20x_gpio_probe(struct platform_device *pdev)
 {
 	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
 	struct axp20x_gpio *gpio;
+	struct pinctrl_desc *pctrl_desc;
 	int ret;
 
 	if (!of_device_is_available(pdev->dev.of_node))
@@ -144,6 +375,8 @@  static int axp20x_gpio_probe(struct platform_device *pdev)
 
 	gpio->chip.base			= -1;
 	gpio->chip.can_sleep		= true;
+	gpio->chip.request		= gpiochip_generic_request;
+	gpio->chip.free			= gpiochip_generic_free;
 	gpio->chip.parent		= &pdev->dev;
 	gpio->chip.label		= dev_name(&pdev->dev);
 	gpio->chip.owner		= THIS_MODULE;
@@ -154,7 +387,30 @@  static int axp20x_gpio_probe(struct platform_device *pdev)
 	gpio->chip.direction_output	= axp20x_gpio_output;
 	gpio->chip.ngpio		= 3;
 
+	gpio->desc = &axp20x_data;
 	gpio->regmap = axp20x->regmap;
+	gpio->dev = &pdev->dev;
+
+	platform_set_drvdata(pdev, gpio);
+
+	axp20x_build_funcs_groups(pdev);
+
+	pctrl_desc = devm_kzalloc(&pdev->dev, sizeof(*pctrl_desc), GFP_KERNEL);
+	if (!pctrl_desc)
+		return -ENOMEM;
+
+	pctrl_desc->name = dev_name(&pdev->dev);
+	pctrl_desc->owner = THIS_MODULE;
+	pctrl_desc->pins = gpio->desc->pins;
+	pctrl_desc->npins = gpio->desc->npins;
+	pctrl_desc->pctlops = &axp20x_pctrl_ops;
+	pctrl_desc->pmxops = &axp20x_pmx_ops;
+
+	gpio->pctl_dev = devm_pinctrl_register(&pdev->dev, pctrl_desc, gpio);
+	if (IS_ERR(gpio->pctl_dev)) {
+		dev_err(&pdev->dev, "couldn't register pinctrl driver\n");
+		return PTR_ERR(gpio->pctl_dev);
+	}
 
 	ret = devm_gpiochip_add_data(&pdev->dev, &gpio->chip, gpio);
 	if (ret) {
@@ -162,7 +418,16 @@  static int axp20x_gpio_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	dev_info(&pdev->dev, "AXP209 GPIO driver loaded\n");
+	ret = gpiochip_add_pin_range(&gpio->chip, dev_name(&pdev->dev),
+				     gpio->desc->pins->number,
+				     gpio->desc->pins->number,
+				     gpio->desc->npins);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to add pin range\n");
+		return ret;
+	}
+
+	dev_info(&pdev->dev, "AXP209 pinctrl and GPIO driver loaded\n");
 
 	return 0;
 }
@@ -184,5 +449,6 @@  static struct platform_driver axp20x_gpio_driver = {
 module_platform_driver(axp20x_gpio_driver);
 
 MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
-MODULE_DESCRIPTION("AXP20x PMIC GPIO driver");
+MODULE_AUTHOR("Quentin Schulz <quentin.schulz@free-electrons.com>");
+MODULE_DESCRIPTION("AXP20x PMIC pinctrl and GPIO driver");
 MODULE_LICENSE("GPL");