linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] add support for AXP209 GPIOs functions
@ 2016-11-23 13:27 Quentin Schulz
  2016-11-23 13:27 ` [PATCH 1/2] gpio: axp209: use correct register for GPIO input status Quentin Schulz
  2016-11-23 13:27 ` [PATCH 2/2] gpio: axp209: add pinctrl support Quentin Schulz
  0 siblings, 2 replies; 8+ messages in thread
From: Quentin Schulz @ 2016-11-23 13:27 UTC (permalink / raw)
  To: linus.walleij, gnurou, robh+dt, mark.rutland, wens
  Cc: Quentin Schulz, linux-gpio, devicetree, linux-kernel, thomas.petazzoni

The AXP209 PMIC has three GPIOs. Two of them can be muxed in other modes
(namely adc or regulator)[1] which cannot be used while the pin is in one
of GPIO modes.

This adds the possibility to use all functions of the GPIOs present in
the AXP209 PMIC thanks to the pinctrl subsystem.

An upcoming ADC driver for the AXP209 PMIC will make use of this pinctrl to
read ADC values of GPIO0 and GPIO1. At the moment, no driver is pinctrling
these GPIOs.

This patch also corrects the register used to read GPIO input status.

[1] see registers 90H, 92H and 93H at
    http://dl.linux-sunxi.org/AXP/AXP209_Datasheet_v1.0en.pdf

Quentin Schulz (2):
  gpio: axp209: use correct register for GPIO input status
  gpio: axp209: add pinctrl support

 .../devicetree/bindings/gpio/gpio-axp209.txt       |  28 +-
 drivers/gpio/gpio-axp209.c                         | 557 ++++++++++++++++++---
 2 files changed, 504 insertions(+), 81 deletions(-)

-- 
2.9.3

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2] gpio: axp209: use correct register for GPIO input status
  2016-11-23 13:27 [PATCH 0/2] add support for AXP209 GPIOs functions Quentin Schulz
@ 2016-11-23 13:27 ` Quentin Schulz
  2016-11-23 13:45   ` Thomas Petazzoni
  2016-11-23 13:27 ` [PATCH 2/2] gpio: axp209: add pinctrl support Quentin Schulz
  1 sibling, 1 reply; 8+ messages in thread
From: Quentin Schulz @ 2016-11-23 13:27 UTC (permalink / raw)
  To: linus.walleij, gnurou, robh+dt, mark.rutland, wens
  Cc: Quentin Schulz, linux-gpio, devicetree, linux-kernel, thomas.petazzoni

The GPIO input status was read from control register
(AXP20X_GPIO[210]_CTRL) instead of status register (AXP20X_GPIO20_SS).

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
 drivers/gpio/gpio-axp209.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/gpio/gpio-axp209.c b/drivers/gpio/gpio-axp209.c
index d9c2a51..4a346b7 100644
--- a/drivers/gpio/gpio-axp209.c
+++ b/drivers/gpio/gpio-axp209.c
@@ -64,13 +64,9 @@ static int axp20x_gpio_get(struct gpio_chip *chip, unsigned offset)
 {
 	struct axp20x_gpio *gpio = gpiochip_get_data(chip);
 	unsigned int val;
-	int reg, ret;
-
-	reg = axp20x_gpio_get_reg(offset);
-	if (reg < 0)
-		return reg;
+	int ret;
 
-	ret = regmap_read(gpio->regmap, reg, &val);
+	ret = regmap_read(gpio->regmap, AXP20X_GPIO20_SS, &val);
 	if (ret)
 		return ret;
 
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] gpio: axp209: add pinctrl support
  2016-11-23 13:27 [PATCH 0/2] add support for AXP209 GPIOs functions Quentin Schulz
  2016-11-23 13:27 ` [PATCH 1/2] gpio: axp209: use correct register for GPIO input status Quentin Schulz
@ 2016-11-23 13:27 ` Quentin Schulz
  2016-11-23 13:45   ` Thomas Petazzoni
                     ` (2 more replies)
  1 sibling, 3 replies; 8+ messages in thread
From: Quentin Schulz @ 2016-11-23 13:27 UTC (permalink / raw)
  To: linus.walleij, gnurou, robh+dt, mark.rutland, wens
  Cc: Quentin Schulz, linux-gpio, devicetree, linux-kernel, thomas.petazzoni

The GPIOs present in the AXP209 PMIC have multiple functions. They
typically allow a pin to be used as GPIO input or output and can also be
used as ADC or regulator for example.[1]

This adds the possibility to use all functions of the GPIOs present in
the AXP209 PMIC thanks to pinctrl subsystem.

[1] see registers 90H, 92H and 93H at
    http://dl.linux-sunxi.org/AXP/AXP209_Datasheet_v1.0en.pdf

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
 .../devicetree/bindings/gpio/gpio-axp209.txt       |  28 +-
 drivers/gpio/gpio-axp209.c                         | 551 ++++++++++++++++++---
 2 files changed, 503 insertions(+), 76 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpio/gpio-axp209.txt b/Documentation/devicetree/bindings/gpio/gpio-axp209.txt
index a661130..a5bfe87 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-axp209.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-axp209.txt
@@ -1,4 +1,4 @@
-AXP209 GPIO controller
+AXP209 GPIO & pinctrl controller
 
 This driver follows the usual GPIO bindings found in
 Documentation/devicetree/bindings/gpio/gpio.txt
@@ -28,3 +28,29 @@ axp209: pmic@34 {
 		#gpio-cells = <2>;
 	};
 };
+
+The GPIOs can be muxed to other functions and therefore, must be a subnode of
+axp_gpio.
+
+Example:
+
+&axp_gpio {
+	gpio0_adc: gpio0_adc {
+		pin = "GPIO0";
+		function = "adc";
+	};
+};
+
+&example_node {
+	pinctrl-names = "default";
+	pinctrl-0 = <&gpio0_adc>;
+};
+
+GPIOs and their functions
+-------------------------
+
+GPIO	|	Functions
+------------------------
+GPIO0	|	gpio_in, gpio_out, ldo, adc
+GPIO1	|	gpio_in, gpio_out, ldo, adc
+GPIO2	|	gpio_in, gpio_out
diff --git a/drivers/gpio/gpio-axp209.c b/drivers/gpio/gpio-axp209.c
index 4a346b7..0a64cfc 100644
--- a/drivers/gpio/gpio-axp209.c
+++ b/drivers/gpio/gpio-axp209.c
@@ -1,7 +1,8 @@
 /*
- * AXP20x GPIO driver
+ * AXP20x Pin control driver
  *
  * Copyright (C) 2016 Maxime Ripard <maxime.ripard@free-electrons.com>
+ * Copyright (C) 2016 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
@@ -21,52 +22,103 @@
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/pinctrl/pinconf-generic.h>
 
 #define AXP20X_GPIO_FUNCTIONS		0x7
 #define AXP20X_GPIO_FUNCTION_OUT_LOW	0
 #define AXP20X_GPIO_FUNCTION_OUT_HIGH	1
 #define AXP20X_GPIO_FUNCTION_INPUT	2
 
-struct axp20x_gpio {
-	struct gpio_chip	chip;
-	struct regmap		*regmap;
-};
+#define AXP20X_PINCTRL_PIN(_pin_num, _pin, _regs)		\
+	{							\
+		.number = _pin_num,				\
+		.name = _pin,					\
+		.drv_data = _regs,				\
+	}
 
-static int axp20x_gpio_get_reg(unsigned offset)
-{
-	switch (offset) {
-	case 0:
-		return AXP20X_GPIO0_CTRL;
-	case 1:
-		return AXP20X_GPIO1_CTRL;
-	case 2:
-		return AXP20X_GPIO2_CTRL;
+#define AXP20X_PIN(_pin, ...)					\
+	{							\
+		.pin = _pin,					\
+		.functions = (struct axp20x_desc_function[]) {	\
+			      __VA_ARGS__, { } },		\
 	}
 
-	return -EINVAL;
-}
+#define AXP20X_FUNCTION(_val, _name)				\
+	{							\
+		.name = _name,					\
+		.muxval = _val,					\
+	}
 
-static int axp20x_gpio_input(struct gpio_chip *chip, unsigned offset)
-{
-	struct axp20x_gpio *gpio = gpiochip_get_data(chip);
-	int reg;
+struct axp20x_desc_function {
+	const char	*name;
+	u8		muxval;
+};
 
-	reg = axp20x_gpio_get_reg(offset);
-	if (reg < 0)
-		return reg;
+struct axp20x_desc_pin {
+	struct pinctrl_pin_desc		pin;
+	struct axp20x_desc_function	*functions;
+};
 
-	return regmap_update_bits(gpio->regmap, reg,
-				  AXP20X_GPIO_FUNCTIONS,
-				  AXP20X_GPIO_FUNCTION_INPUT);
-}
+struct axp20x_pinctrl_desc {
+	const struct axp20x_desc_pin	*pins;
+	int				npins;
+	unsigned int			pin_base;
+};
+
+struct axp20x_pinctrl_function {
+	const char	*name;
+	const char	**groups;
+	unsigned int	ngroups;
+};
+
+struct axp20x_pinctrl_group {
+	const char	*name;
+	unsigned long	config;
+	unsigned int	pin;
+};
+
+struct axp20x_pctl {
+	struct pinctrl_dev			*pctl_dev;
+	struct device				*dev;
+	struct gpio_chip			chip;
+	struct regmap				*regmap;
+	const struct axp20x_pinctrl_desc	*desc;
+	struct axp20x_pinctrl_group		*groups;
+	unsigned int				ngroups;
+	struct axp20x_pinctrl_function		*functions;
+	unsigned int				nfunctions;
+};
+
+static const struct axp20x_desc_pin axp209_pins[] = {
+	AXP20X_PIN(AXP20X_PINCTRL_PIN(0, "GPIO0", (void *)AXP20X_GPIO0_CTRL),
+		   AXP20X_FUNCTION(0x0, "gpio_out"),
+		   AXP20X_FUNCTION(0x2, "gpio_in"),
+		   AXP20X_FUNCTION(0x3, "ldo"),
+		   AXP20X_FUNCTION(0x4, "adc")),
+	AXP20X_PIN(AXP20X_PINCTRL_PIN(1, "GPIO1", (void *)AXP20X_GPIO1_CTRL),
+		   AXP20X_FUNCTION(0x0, "gpio_out"),
+		   AXP20X_FUNCTION(0x2, "gpio_in"),
+		   AXP20X_FUNCTION(0x3, "ldo"),
+		   AXP20X_FUNCTION(0x4, "adc")),
+	AXP20X_PIN(AXP20X_PINCTRL_PIN(2, "GPIO2", (void *)AXP20X_GPIO2_CTRL),
+		   AXP20X_FUNCTION(0x0, "gpio_out"),
+		   AXP20X_FUNCTION(0x2, "gpio_in")),
+};
+
+static const struct axp20x_pinctrl_desc axp20x_pinctrl_data = {
+	.pins	= axp209_pins,
+	.npins	= ARRAY_SIZE(axp209_pins),
+};
 
 static int axp20x_gpio_get(struct gpio_chip *chip, unsigned offset)
 {
-	struct axp20x_gpio *gpio = gpiochip_get_data(chip);
+	struct axp20x_pctl *pctl = gpiochip_get_data(chip);
 	unsigned int val;
 	int ret;
 
-	ret = regmap_read(gpio->regmap, AXP20X_GPIO20_SS, &val);
+	ret = regmap_read(pctl->regmap, AXP20X_GPIO20_SS, &val);
 	if (ret)
 		return ret;
 
@@ -75,15 +127,12 @@ static int axp20x_gpio_get(struct gpio_chip *chip, unsigned offset)
 
 static int axp20x_gpio_get_direction(struct gpio_chip *chip, unsigned offset)
 {
-	struct axp20x_gpio *gpio = gpiochip_get_data(chip);
+	struct axp20x_pctl *pctl = gpiochip_get_data(chip);
+	int pin_reg = (int)pctl->desc->pins[offset].pin.drv_data;
 	unsigned int val;
-	int reg, ret;
-
-	reg = axp20x_gpio_get_reg(offset);
-	if (reg < 0)
-		return reg;
+	int ret;
 
-	ret = regmap_read(gpio->regmap, reg, &val);
+	ret = regmap_read(pctl->regmap, pin_reg, &val);
 	if (ret)
 		return ret;
 
@@ -102,33 +151,335 @@ static int axp20x_gpio_get_direction(struct gpio_chip *chip, unsigned offset)
 	return val & 2;
 }
 
-static int axp20x_gpio_output(struct gpio_chip *chip, unsigned offset,
+static void axp20x_gpio_set(struct gpio_chip *chip, unsigned int offset,
+			    int value)
+{
+	struct axp20x_pctl *pctl = gpiochip_get_data(chip);
+	int pin_reg = (int)pctl->desc->pins[offset].pin.drv_data;
+
+	regmap_update_bits(pctl->regmap, pin_reg,
+			   AXP20X_GPIO_FUNCTIONS,
+			   value ? AXP20X_GPIO_FUNCTION_OUT_HIGH
+				 : AXP20X_GPIO_FUNCTION_OUT_LOW);
+}
+
+static int axp20x_gpio_input(struct gpio_chip *chip, unsigned int offset)
+{
+	return pinctrl_gpio_direction_input(chip->base + offset);
+}
+
+static int axp20x_gpio_output(struct gpio_chip *chip, unsigned int offset,
 			      int value)
 {
-	struct axp20x_gpio *gpio = gpiochip_get_data(chip);
-	int reg;
+	chip->set(chip, offset, value);
 
-	reg = axp20x_gpio_get_reg(offset);
-	if (reg < 0)
-		return reg;
+	return 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_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+	int pin_reg = (int)pctl->desc->pins[offset].pin.drv_data;
+
+	return regmap_update_bits(pctl->regmap, pin_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_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+
+	return pctl->nfunctions;
+}
+
+static const char *axp20x_pmx_func_name(struct pinctrl_dev *pctldev,
+					unsigned int selector)
+{
+	struct axp20x_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+
+	return pctl->functions[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_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+
+	*groups = pctl->functions[selector].groups;
+	*num_groups = pctl->functions[selector].ngroups;
+
+	return 0;
+}
+
+static struct axp20x_desc_function *
+axp20x_pinctrl_desc_find_func_by_name(struct axp20x_pctl *pctl,
+				      const char *group, const char *func)
+{
+	const struct axp20x_desc_pin *pin;
+	struct axp20x_desc_function *desc_func;
+	int i;
+
+	for (i = 0; i < pctl->desc->npins; i++) {
+		pin = &pctl->desc->pins[i];
+
+		if (!strcmp(pin->pin.name, group)) {
+			desc_func = pin->functions;
+
+			while (desc_func->name) {
+				if (!strcmp(desc_func->name, func))
+					return desc_func;
+				desc_func++;
+			}
+
+			/*
+			 * Pins are uniquely named. Groups are named after one
+			 * pin name. If one pin matches group name but its
+			 * function cannot be found, no other pin will match
+			 * group name.
+			 */
+			return NULL;
+		}
+	}
+
+	return NULL;
+}
+
+static int axp20x_pmx_set_mux(struct pinctrl_dev *pctldev,
+			      unsigned int function, unsigned int group)
+{
+	struct axp20x_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+	struct axp20x_pinctrl_group *g = pctl->groups + group;
+	struct axp20x_pinctrl_function *func = pctl->functions + function;
+	struct axp20x_desc_function *desc_func =
+		axp20x_pinctrl_desc_find_func_by_name(pctl, g->name,
+						      func->name);
+	if (!desc_func)
+		return -EINVAL;
+
+	return axp20x_pmx_set(pctldev, g->pin, desc_func->muxval);
+}
+
+static struct axp20x_desc_function *
+axp20x_pctl_desc_find_func_by_pin(struct axp20x_pctl *pctl, unsigned int offset,
+				  const char *func)
+{
+	const struct axp20x_desc_pin *pin;
+	struct axp20x_desc_function *desc_func;
+	int i;
+
+	for (i = 0; i < pctl->desc->npins; i++) {
+		pin = &pctl->desc->pins[i];
+
+		if (pin->pin.number == offset) {
+			desc_func = pin->functions;
+
+			while (desc_func->name) {
+				if (!strcmp(desc_func->name, func))
+					return desc_func;
+
+				desc_func++;
+			}
+		}
+	}
+
+	return NULL;
+}
+
+static int axp20x_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
+					 struct pinctrl_gpio_range *range,
+					 unsigned int offset, bool input)
+{
+	struct axp20x_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+	struct axp20x_desc_function *desc_func;
+	const char *func;
+
+	if (input)
+		func = "gpio_in";
+	else
+		func = "gpio_out";
+
+	desc_func = axp20x_pctl_desc_find_func_by_pin(pctl, offset, func);
+	if (!desc_func)
+		return -EINVAL;
+
+	return axp20x_pmx_set(pctldev, offset, desc_func->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_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+
+	return pctl->ngroups;
+}
+
+static int axp20x_group_pins(struct pinctrl_dev *pctldev, unsigned int selector,
+			     const unsigned int **pins, unsigned int *num_pins)
+{
+	struct axp20x_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+	struct axp20x_pinctrl_group *g = pctl->groups + selector;
+
+	*pins = (unsigned int *)&g->pin;
+	*num_pins = 1;
+
+	return 0;
+}
+
+static const char *axp20x_group_name(struct pinctrl_dev *pctldev,
+				     unsigned int selector)
+{
+	struct axp20x_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+
+	return pctl->groups[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 struct axp20x_pinctrl_function *
+axp20x_pinctrl_function_by_name(struct axp20x_pctl *pctl, const char *name)
+{
+	struct axp20x_pinctrl_function *func = pctl->functions;
+
+	while (func->name) {
+		if (!strcmp(func->name, name))
+			return func;
+		func++;
+	}
+
+	return NULL;
+}
+
+static int axp20x_pinctrl_add_function(struct axp20x_pctl *pctl,
+				       const char *name)
 {
-	axp20x_gpio_output(chip, offset, value);
+	struct axp20x_pinctrl_function *func = pctl->functions;
+
+	while (func->name) {
+		if (!strcmp(func->name, name)) {
+			func->ngroups++;
+			return -EEXIST;
+		}
+
+		func++;
+	}
+
+	func->name = name;
+	func->ngroups = 1;
+
+	pctl->nfunctions++;
+
+	return 0;
 }
 
-static int axp20x_gpio_probe(struct platform_device *pdev)
+static int axp20x_attach_group_function(struct platform_device *pdev,
+					const struct axp20x_desc_pin *pin)
+{
+	struct axp20x_pctl *pctl = platform_get_drvdata(pdev);
+	struct axp20x_desc_function *desc_func = pin->functions;
+	struct axp20x_pinctrl_function *func;
+	const char **func_grp;
+
+	while (desc_func->name) {
+		func = axp20x_pinctrl_function_by_name(pctl, desc_func->name);
+		if (!func)
+			return -EINVAL;
+
+		if (!func->groups) {
+			func->groups = devm_kzalloc(&pdev->dev,
+						    func->ngroups * sizeof(const char *),
+						    GFP_KERNEL);
+			if (!func->groups)
+				return -ENOMEM;
+		}
+
+		func_grp = func->groups;
+		while (*func_grp)
+			func_grp++;
+
+		*func_grp = pin->pin.name;
+		desc_func++;
+	}
+
+	return 0;
+}
+
+static int axp20x_build_state(struct platform_device *pdev)
+{
+	struct axp20x_pctl *pctl = platform_get_drvdata(pdev);
+	unsigned int npins = pctl->desc->npins;
+	const struct axp20x_desc_pin *pin;
+	struct axp20x_desc_function *func;
+	int i, ret;
+
+	pctl->ngroups = npins;
+	pctl->groups = devm_kzalloc(&pdev->dev,
+				    pctl->ngroups * sizeof(*pctl->groups),
+				    GFP_KERNEL);
+	if (!pctl->groups)
+		return -ENOMEM;
+
+	for (i = 0; i < npins; i++) {
+		pctl->groups[i].name = pctl->desc->pins[i].pin.name;
+		pctl->groups[i].pin = pctl->desc->pins[i].pin.number;
+	}
+
+	/* We assume 4 functions per pin should be enough as a default max */
+	pctl->functions = devm_kzalloc(&pdev->dev,
+				       npins * 4 * sizeof(*pctl->functions),
+				       GFP_KERNEL);
+	if (!pctl->functions)
+		return -ENOMEM;
+
+	/* Create a list of uniquely named functions */
+	for (i = 0; i < npins; i++) {
+		pin = &pctl->desc->pins[i];
+		func = pin->functions;
+
+		while (func->name) {
+			axp20x_pinctrl_add_function(pctl, func->name);
+			func++;
+		}
+	}
+
+	pctl->functions = krealloc(pctl->functions,
+				   pctl->nfunctions * sizeof(*pctl->functions),
+				   GFP_KERNEL);
+
+	for (i = 0; i < npins; i++) {
+		pin = &pctl->desc->pins[i];
+		ret = axp20x_attach_group_function(pdev, pin);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int axp20x_pctl_probe(struct platform_device *pdev)
 {
 	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
-	struct axp20x_gpio *gpio;
-	int ret;
+	const struct axp20x_desc_pin *pin;
+	struct axp20x_pctl *pctl;
+	struct pinctrl_desc *pctrl_desc;
+	struct pinctrl_pin_desc *pins;
+	int ret, i;
 
 	if (!of_device_is_available(pdev->dev.of_node))
 		return -ENODEV;
@@ -138,51 +489,101 @@ static int axp20x_gpio_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL);
-	if (!gpio)
+	pctl = devm_kzalloc(&pdev->dev, sizeof(*pctl), GFP_KERNEL);
+	if (!pctl)
+		return -ENOMEM;
+
+	pctl->chip.base			= -1;
+	pctl->chip.can_sleep		= true;
+	pctl->chip.request		= gpiochip_generic_request;
+	pctl->chip.free			= gpiochip_generic_free;
+	pctl->chip.parent		= &pdev->dev;
+	pctl->chip.label		= dev_name(&pdev->dev);
+	pctl->chip.owner		= THIS_MODULE;
+	pctl->chip.get			= axp20x_gpio_get;
+	pctl->chip.get_direction	= axp20x_gpio_get_direction;
+	pctl->chip.set			= axp20x_gpio_set;
+	pctl->chip.direction_input	= axp20x_gpio_input;
+	pctl->chip.direction_output	= axp20x_gpio_output;
+	pctl->chip.ngpio		= 3;
+	pctl->chip.can_sleep		= true;
+
+	pctl->regmap = axp20x->regmap;
+
+	pctl->desc = &axp20x_pinctrl_data;
+	pctl->dev = &pdev->dev;
+
+	platform_set_drvdata(pdev, pctl);
+
+	ret = axp20x_build_state(pdev);
+	if (ret)
+		return ret;
+
+	pins = devm_kzalloc(&pdev->dev, pctl->desc->npins * sizeof(*pins),
+			    GFP_KERNEL);
+	if (!pins)
 		return -ENOMEM;
 
-	gpio->chip.base			= -1;
-	gpio->chip.can_sleep		= true;
-	gpio->chip.parent		= &pdev->dev;
-	gpio->chip.label		= dev_name(&pdev->dev);
-	gpio->chip.owner		= THIS_MODULE;
-	gpio->chip.get			= axp20x_gpio_get;
-	gpio->chip.get_direction	= axp20x_gpio_get_direction;
-	gpio->chip.set			= axp20x_gpio_set;
-	gpio->chip.direction_input	= axp20x_gpio_input;
-	gpio->chip.direction_output	= axp20x_gpio_output;
-	gpio->chip.ngpio		= 3;
-
-	gpio->regmap = axp20x->regmap;
-
-	ret = devm_gpiochip_add_data(&pdev->dev, &gpio->chip, gpio);
+	for (i = 0; i < pctl->desc->npins; i++)
+		pins[i] = pctl->desc->pins[i].pin;
+
+	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 = pins;
+	pctrl_desc->npins = pctl->desc->npins;
+	pctrl_desc->pctlops = &axp20x_pctrl_ops;
+	pctrl_desc->pmxops = &axp20x_pmx_ops;
+
+	pctl->pctl_dev = devm_pinctrl_register(&pdev->dev, pctrl_desc, pctl);
+	if (IS_ERR(pctl->pctl_dev)) {
+		dev_err(&pdev->dev, "couldn't register pinctrl driver\n");
+		return PTR_ERR(pctl->pctl_dev);
+	}
+
+	ret = devm_gpiochip_add_data(&pdev->dev, &pctl->chip, pctl);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to register GPIO chip\n");
 		return ret;
 	}
 
+	for (i = 0; i < pctl->desc->npins; i++) {
+		pin = pctl->desc->pins + i;
+
+		ret = gpiochip_add_pin_range(&pctl->chip, dev_name(&pdev->dev),
+					     pin->pin.number, pin->pin.number,
+					     1);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to add pin range\n");
+			return ret;
+		}
+	}
+
 	dev_info(&pdev->dev, "AXP209 GPIO driver loaded\n");
 
 	return 0;
 }
 
-static const struct of_device_id axp20x_gpio_match[] = {
+static const struct of_device_id axp20x_pctl_match[] = {
 	{ .compatible = "x-powers,axp209-gpio" },
 	{ }
 };
-MODULE_DEVICE_TABLE(of, axp20x_gpio_match);
+MODULE_DEVICE_TABLE(of, axp20x_pctl_match);
 
-static struct platform_driver axp20x_gpio_driver = {
-	.probe		= axp20x_gpio_probe,
+static struct platform_driver axp20x_pctl_driver = {
+	.probe		= axp20x_pctl_probe,
 	.driver = {
 		.name		= "axp20x-gpio",
-		.of_match_table	= axp20x_gpio_match,
+		.of_match_table	= axp20x_pctl_match,
 	},
 };
 
-module_platform_driver(axp20x_gpio_driver);
+module_platform_driver(axp20x_pctl_driver);
 
 MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
+MODULE_AUTHOR("Quentin Schulz <quentin.schulz@free-electrons.com>");
 MODULE_DESCRIPTION("AXP20x PMIC GPIO driver");
 MODULE_LICENSE("GPL");
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] gpio: axp209: add pinctrl support
  2016-11-23 13:27 ` [PATCH 2/2] gpio: axp209: add pinctrl support Quentin Schulz
@ 2016-11-23 13:45   ` Thomas Petazzoni
  2016-11-23 17:13   ` kbuild test robot
  2016-11-23 17:21   ` kbuild test robot
  2 siblings, 0 replies; 8+ messages in thread
From: Thomas Petazzoni @ 2016-11-23 13:45 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: linus.walleij, gnurou, robh+dt, mark.rutland, wens, linux-gpio,
	devicetree, linux-kernel

Hello,

By far not a full review, just a few things that I saw while scrolling
through the code.

On Wed, 23 Nov 2016 14:27:49 +0100, Quentin Schulz wrote:

> +static struct axp20x_desc_function *
> +axp20x_pinctrl_desc_find_func_by_name(struct axp20x_pctl *pctl,
> +				      const char *group, const char *func)
> +{
> +	const struct axp20x_desc_pin *pin;
> +	struct axp20x_desc_function *desc_func;

These variables should go inside the for loop. There is this problem in
other functions in your code: you should keep local variables in the
scope where they are used.

> +	int i;
> +
> +	for (i = 0; i < pctl->desc->npins; i++) {
> +		pin = &pctl->desc->pins[i];
> +
> +		if (!strcmp(pin->pin.name, group)) {

Change this to:

		if (strcmp(pin->pin.name, group))
			continue;

This way, the rest of the code in the function is intended with one
less tab.

Or alternatively, if only one element in the array will match, you can
also break out from the loop when you have found the matching element,
and handle whatever needs to be done on this element outside of the
loop.

> +static struct axp20x_desc_function *
> +axp20x_pctl_desc_find_func_by_pin(struct axp20x_pctl *pctl, unsigned int offset,
> +				  const char *func)
> +{
> +	const struct axp20x_desc_pin *pin;
> +	struct axp20x_desc_function *desc_func;
> +	int i;
> +
> +	for (i = 0; i < pctl->desc->npins; i++) {
> +		pin = &pctl->desc->pins[i];
> +
> +		if (pin->pin.number == offset) {

Same comment here.

> +static int axp20x_build_state(struct platform_device *pdev)
> +{
> +	struct axp20x_pctl *pctl = platform_get_drvdata(pdev);
> +	unsigned int npins = pctl->desc->npins;
> +	const struct axp20x_desc_pin *pin;
> +	struct axp20x_desc_function *func;
> +	int i, ret;
> +
> +	pctl->ngroups = npins;
> +	pctl->groups = devm_kzalloc(&pdev->dev,
> +				    pctl->ngroups * sizeof(*pctl->groups),
> +				    GFP_KERNEL);
> +	if (!pctl->groups)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < npins; i++) {
> +		pctl->groups[i].name = pctl->desc->pins[i].pin.name;
> +		pctl->groups[i].pin = pctl->desc->pins[i].pin.number;
> +	}
> +
> +	/* We assume 4 functions per pin should be enough as a default max */
> +	pctl->functions = devm_kzalloc(&pdev->dev,
> +				       npins * 4 * sizeof(*pctl->functions),
> +				       GFP_KERNEL);
> +	if (!pctl->functions)
> +		return -ENOMEM;
> +
> +	/* Create a list of uniquely named functions */
> +	for (i = 0; i < npins; i++) {
> +		pin = &pctl->desc->pins[i];
> +		func = pin->functions;
> +
> +		while (func->name) {
> +			axp20x_pinctrl_add_function(pctl, func->name);
> +			func++;
> +		}
> +	}
> +
> +	pctl->functions = krealloc(pctl->functions,
> +				   pctl->nfunctions * sizeof(*pctl->functions),
> +				   GFP_KERNEL);

Not sure why you need to first allocation for 4 functions, and then
reallocate a potentially larger (or smaller?) array here.

Will devm_kzalloc() followed by krealloc() really have the expected
behavior?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] gpio: axp209: use correct register for GPIO input status
  2016-11-23 13:27 ` [PATCH 1/2] gpio: axp209: use correct register for GPIO input status Quentin Schulz
@ 2016-11-23 13:45   ` Thomas Petazzoni
  2016-11-24  5:52     ` Chen-Yu Tsai
  0 siblings, 1 reply; 8+ messages in thread
From: Thomas Petazzoni @ 2016-11-23 13:45 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: linus.walleij, gnurou, robh+dt, mark.rutland, wens, linux-gpio,
	devicetree, linux-kernel

Hello,

On Wed, 23 Nov 2016 14:27:48 +0100, Quentin Schulz wrote:
> The GPIO input status was read from control register
> (AXP20X_GPIO[210]_CTRL) instead of status register (AXP20X_GPIO20_SS).
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>

This smells like a bug fix, so perhaps Cc: stable?

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] gpio: axp209: add pinctrl support
  2016-11-23 13:27 ` [PATCH 2/2] gpio: axp209: add pinctrl support Quentin Schulz
  2016-11-23 13:45   ` Thomas Petazzoni
@ 2016-11-23 17:13   ` kbuild test robot
  2016-11-23 17:21   ` kbuild test robot
  2 siblings, 0 replies; 8+ messages in thread
From: kbuild test robot @ 2016-11-23 17:13 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: kbuild-all, linus.walleij, gnurou, robh+dt, mark.rutland, wens,
	Quentin Schulz, linux-gpio, devicetree, linux-kernel,
	thomas.petazzoni

[-- Attachment #1: Type: text/plain, Size: 17845 bytes --]

Hi Quentin,

[auto build test ERROR on gpio/for-next]
[also build test ERROR on v4.9-rc6 next-20161123]
[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/add-support-for-AXP209-GPIOs-functions/20161124-003102
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: i386-randconfig-i0-201647 (attached as .config)
compiler: gcc-4.8 (Debian 4.8.4-1) 4.8.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All error/warnings (new ones prefixed by >>):

>> drivers/gpio/gpio-axp209.c:60:27: error: field 'pin' has incomplete type
     struct pinctrl_pin_desc  pin;
                              ^
>> drivers/gpio/gpio-axp209.c:95:2: error: field name not in record or union initializer
     AXP20X_PIN(AXP20X_PINCTRL_PIN(0, "GPIO0", (void *)AXP20X_GPIO0_CTRL),
     ^
   drivers/gpio/gpio-axp209.c:95:2: error: (near initialization for 'axp209_pins[0].pin')
>> drivers/gpio/gpio-axp209.c:95:2: error: field name not in record or union initializer
   drivers/gpio/gpio-axp209.c:95:2: error: (near initialization for 'axp209_pins[0].pin')
>> drivers/gpio/gpio-axp209.c:95:2: error: field name not in record or union initializer
   drivers/gpio/gpio-axp209.c:95:2: error: (near initialization for 'axp209_pins[0].pin')
   drivers/gpio/gpio-axp209.c:100:2: error: field name not in record or union initializer
     AXP20X_PIN(AXP20X_PINCTRL_PIN(1, "GPIO1", (void *)AXP20X_GPIO1_CTRL),
     ^
   drivers/gpio/gpio-axp209.c:100:2: error: (near initialization for 'axp209_pins[1].pin')
   drivers/gpio/gpio-axp209.c:100:2: error: field name not in record or union initializer
   drivers/gpio/gpio-axp209.c:100:2: error: (near initialization for 'axp209_pins[1].pin')
   drivers/gpio/gpio-axp209.c:100:2: error: field name not in record or union initializer
   drivers/gpio/gpio-axp209.c:100:2: error: (near initialization for 'axp209_pins[1].pin')
   drivers/gpio/gpio-axp209.c:105:2: error: field name not in record or union initializer
     AXP20X_PIN(AXP20X_PINCTRL_PIN(2, "GPIO2", (void *)AXP20X_GPIO2_CTRL),
     ^
   drivers/gpio/gpio-axp209.c:105:2: error: (near initialization for 'axp209_pins[2].pin')
   drivers/gpio/gpio-axp209.c:105:2: error: field name not in record or union initializer
   drivers/gpio/gpio-axp209.c:105:2: error: (near initialization for 'axp209_pins[2].pin')
   drivers/gpio/gpio-axp209.c:105:2: error: field name not in record or union initializer
   drivers/gpio/gpio-axp209.c:105:2: error: (near initialization for 'axp209_pins[2].pin')
   drivers/gpio/gpio-axp209.c: In function 'axp20x_gpio_get_direction':
>> drivers/gpio/gpio-axp209.c:131:49: error: request for member 'drv_data' in something not a structure or union
     int pin_reg = (int)pctl->desc->pins[offset].pin.drv_data;
                                                    ^
   drivers/gpio/gpio-axp209.c: In function 'axp20x_gpio_set':
   drivers/gpio/gpio-axp209.c:158:49: error: request for member 'drv_data' in something not a structure or union
     int pin_reg = (int)pctl->desc->pins[offset].pin.drv_data;
                                                    ^
   drivers/gpio/gpio-axp209.c: In function 'axp20x_gpio_input':
>> drivers/gpio/gpio-axp209.c:168:2: error: implicit declaration of function 'pinctrl_gpio_direction_input' [-Werror=implicit-function-declaration]
     return pinctrl_gpio_direction_input(chip->base + offset);
     ^
   drivers/gpio/gpio-axp209.c: In function 'axp20x_pmx_set':
>> drivers/gpio/gpio-axp209.c:182:9: error: implicit declaration of function 'pinctrl_dev_get_drvdata' [-Werror=implicit-function-declaration]
     struct axp20x_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
            ^
>> drivers/gpio/gpio-axp209.c:182:29: warning: initialization makes pointer from integer without a cast [enabled by default]
     struct axp20x_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
                                ^
   drivers/gpio/gpio-axp209.c:183:49: error: request for member 'drv_data' in something not a structure or union
     int pin_reg = (int)pctl->desc->pins[offset].pin.drv_data;
                                                    ^
   drivers/gpio/gpio-axp209.c: In function 'axp20x_pmx_func_cnt':
   drivers/gpio/gpio-axp209.c:191:29: warning: initialization makes pointer from integer without a cast [enabled by default]
     struct axp20x_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
                                ^
   drivers/gpio/gpio-axp209.c: In function 'axp20x_pmx_func_name':
   drivers/gpio/gpio-axp209.c:199:29: warning: initialization makes pointer from integer without a cast [enabled by default]
     struct axp20x_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
                                ^
   drivers/gpio/gpio-axp209.c: In function 'axp20x_pmx_func_groups':
   drivers/gpio/gpio-axp209.c:209:29: warning: initialization makes pointer from integer without a cast [enabled by default]
     struct axp20x_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
                                ^
   drivers/gpio/gpio-axp209.c: In function 'axp20x_pinctrl_desc_find_func_by_name':
>> drivers/gpio/gpio-axp209.c:228:23: error: request for member 'name' in something not a structure or union
      if (!strcmp(pin->pin.name, group)) {
                          ^
>> drivers/gpio/gpio-axp209.c:228:3: warning: passing argument 1 of 'strcmp' from incompatible pointer type [enabled by default]
      if (!strcmp(pin->pin.name, group)) {
      ^
   In file included from arch/x86/include/asm/string.h:2:0,
                    from include/linux/string.h:18,
                    from arch/x86/include/asm/page_32.h:34,
                    from arch/x86/include/asm/page.h:13,
                    from arch/x86/include/asm/processor.h:17,
                    from include/linux/mutex.h:19,
                    from include/linux/kernfs.h:13,
                    from include/linux/sysfs.h:15,
                    from include/linux/kobject.h:21,
                    from include/linux/device.h:17,
                    from drivers/gpio/gpio-axp209.c:14:
   arch/x86/include/asm/string_32.h:21:12: note: expected 'const char *' but argument is of type 'const struct axp20x_desc_pin *'
    extern int strcmp(const char *cs, const char *ct);
               ^
   drivers/gpio/gpio-axp209.c: In function 'axp20x_pmx_set_mux':
   drivers/gpio/gpio-axp209.c:253:29: warning: initialization makes pointer from integer without a cast [enabled by default]
     struct axp20x_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
                                ^
   drivers/gpio/gpio-axp209.c: In function 'axp20x_pctl_desc_find_func_by_pin':
>> drivers/gpio/gpio-axp209.c:276:15: error: request for member 'number' in something not a structure or union
      if (pin->pin.number == offset) {
                  ^
>> drivers/gpio/gpio-axp209.c:276:23: warning: comparison between pointer and integer [enabled by default]
      if (pin->pin.number == offset) {
                          ^
   drivers/gpio/gpio-axp209.c: At top level:
>> drivers/gpio/gpio-axp209.c:293:7: warning: 'struct pinctrl_gpio_range' declared inside parameter list [enabled by default]
          unsigned int offset, bool input)
          ^
>> drivers/gpio/gpio-axp209.c:293:7: warning: its scope is only this definition or declaration, which is probably not what you want [enabled by default]
   drivers/gpio/gpio-axp209.c: In function 'axp20x_pmx_gpio_set_direction':
   drivers/gpio/gpio-axp209.c:295:29: warning: initialization makes pointer from integer without a cast [enabled by default]
     struct axp20x_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
                                ^
   drivers/gpio/gpio-axp209.c: At top level:
>> drivers/gpio/gpio-axp209.c:311:21: error: variable 'axp20x_pmx_ops' has initializer but incomplete type
    static const struct pinmux_ops axp20x_pmx_ops = {
                        ^
>> drivers/gpio/gpio-axp209.c:312:2: error: unknown field 'get_functions_count' specified in initializer
     .get_functions_count = axp20x_pmx_func_cnt,
     ^
>> drivers/gpio/gpio-axp209.c:312:2: warning: excess elements in struct initializer [enabled by default]
   drivers/gpio/gpio-axp209.c:312:2: warning: (near initialization for 'axp20x_pmx_ops') [enabled by default]
>> drivers/gpio/gpio-axp209.c:313:2: error: unknown field 'get_function_name' specified in initializer
     .get_function_name = axp20x_pmx_func_name,
     ^
   drivers/gpio/gpio-axp209.c:313:2: warning: excess elements in struct initializer [enabled by default]
   drivers/gpio/gpio-axp209.c:313:2: warning: (near initialization for 'axp20x_pmx_ops') [enabled by default]
>> drivers/gpio/gpio-axp209.c:314:2: error: unknown field 'get_function_groups' specified in initializer
     .get_function_groups = axp20x_pmx_func_groups,
     ^
   drivers/gpio/gpio-axp209.c:314:2: warning: excess elements in struct initializer [enabled by default]
   drivers/gpio/gpio-axp209.c:314:2: warning: (near initialization for 'axp20x_pmx_ops') [enabled by default]
>> drivers/gpio/gpio-axp209.c:315:2: error: unknown field 'set_mux' specified in initializer
     .set_mux  = axp20x_pmx_set_mux,
     ^
   drivers/gpio/gpio-axp209.c:315:2: warning: excess elements in struct initializer [enabled by default]
   drivers/gpio/gpio-axp209.c:315:2: warning: (near initialization for 'axp20x_pmx_ops') [enabled by default]

vim +/pin +60 drivers/gpio/gpio-axp209.c

     8	 * under  the terms of the GNU General  Public License as published by the
     9	 * Free Software Foundation;  either version 2 of the License, or (at your
    10	 * option) any later version.
    11	 */
    12	
    13	#include <linux/bitops.h>
  > 14	#include <linux/device.h>
    15	#include <linux/gpio/driver.h>
    16	#include <linux/init.h>
    17	#include <linux/interrupt.h>
    18	#include <linux/kernel.h>
    19	#include <linux/mfd/axp20x.h>
    20	#include <linux/module.h>
    21	#include <linux/of.h>
    22	#include <linux/platform_device.h>
    23	#include <linux/regmap.h>
    24	#include <linux/slab.h>
    25	#include <linux/pinctrl/pinctrl.h>
    26	#include <linux/pinctrl/pinmux.h>
    27	#include <linux/pinctrl/pinconf-generic.h>
    28	
    29	#define AXP20X_GPIO_FUNCTIONS		0x7
    30	#define AXP20X_GPIO_FUNCTION_OUT_LOW	0
    31	#define AXP20X_GPIO_FUNCTION_OUT_HIGH	1
    32	#define AXP20X_GPIO_FUNCTION_INPUT	2
    33	
    34	#define AXP20X_PINCTRL_PIN(_pin_num, _pin, _regs)		\
    35		{							\
    36			.number = _pin_num,				\
    37			.name = _pin,					\
    38			.drv_data = _regs,				\
    39		}
    40	
    41	#define AXP20X_PIN(_pin, ...)					\
    42		{							\
    43			.pin = _pin,					\
    44			.functions = (struct axp20x_desc_function[]) {	\
    45				      __VA_ARGS__, { } },		\
    46		}
    47	
    48	#define AXP20X_FUNCTION(_val, _name)				\
    49		{							\
    50			.name = _name,					\
    51			.muxval = _val,					\
    52		}
    53	
    54	struct axp20x_desc_function {
    55		const char	*name;
    56		u8		muxval;
    57	};
    58	
    59	struct axp20x_desc_pin {
  > 60		struct pinctrl_pin_desc		pin;
    61		struct axp20x_desc_function	*functions;
    62	};
    63	
    64	struct axp20x_pinctrl_desc {
    65		const struct axp20x_desc_pin	*pins;
    66		int				npins;
    67		unsigned int			pin_base;
    68	};
    69	
    70	struct axp20x_pinctrl_function {
    71		const char	*name;
    72		const char	**groups;
    73		unsigned int	ngroups;
    74	};
    75	
    76	struct axp20x_pinctrl_group {
    77		const char	*name;
    78		unsigned long	config;
    79		unsigned int	pin;
    80	};
    81	
    82	struct axp20x_pctl {
    83		struct pinctrl_dev			*pctl_dev;
    84		struct device				*dev;
    85		struct gpio_chip			chip;
    86		struct regmap				*regmap;
    87		const struct axp20x_pinctrl_desc	*desc;
    88		struct axp20x_pinctrl_group		*groups;
    89		unsigned int				ngroups;
    90		struct axp20x_pinctrl_function		*functions;
    91		unsigned int				nfunctions;
    92	};
    93	
    94	static const struct axp20x_desc_pin axp209_pins[] = {
  > 95		AXP20X_PIN(AXP20X_PINCTRL_PIN(0, "GPIO0", (void *)AXP20X_GPIO0_CTRL),
    96			   AXP20X_FUNCTION(0x0, "gpio_out"),
    97			   AXP20X_FUNCTION(0x2, "gpio_in"),
    98			   AXP20X_FUNCTION(0x3, "ldo"),
    99			   AXP20X_FUNCTION(0x4, "adc")),
   100		AXP20X_PIN(AXP20X_PINCTRL_PIN(1, "GPIO1", (void *)AXP20X_GPIO1_CTRL),
   101			   AXP20X_FUNCTION(0x0, "gpio_out"),
   102			   AXP20X_FUNCTION(0x2, "gpio_in"),
   103			   AXP20X_FUNCTION(0x3, "ldo"),
   104			   AXP20X_FUNCTION(0x4, "adc")),
 > 105		AXP20X_PIN(AXP20X_PINCTRL_PIN(2, "GPIO2", (void *)AXP20X_GPIO2_CTRL),
   106			   AXP20X_FUNCTION(0x0, "gpio_out"),
   107			   AXP20X_FUNCTION(0x2, "gpio_in")),
   108	};
   109	
   110	static const struct axp20x_pinctrl_desc axp20x_pinctrl_data = {
   111		.pins	= axp209_pins,
   112		.npins	= ARRAY_SIZE(axp209_pins),
   113	};
   114	
   115	static int axp20x_gpio_get(struct gpio_chip *chip, unsigned offset)
   116	{
   117		struct axp20x_pctl *pctl = gpiochip_get_data(chip);
   118		unsigned int val;
   119		int ret;
   120	
   121		ret = regmap_read(pctl->regmap, AXP20X_GPIO20_SS, &val);
   122		if (ret)
   123			return ret;
   124	
   125		return !!(val & BIT(offset + 4));
   126	}
   127	
   128	static int axp20x_gpio_get_direction(struct gpio_chip *chip, unsigned offset)
   129	{
   130		struct axp20x_pctl *pctl = gpiochip_get_data(chip);
 > 131		int pin_reg = (int)pctl->desc->pins[offset].pin.drv_data;
   132		unsigned int val;
   133		int ret;
   134	
   135		ret = regmap_read(pctl->regmap, pin_reg, &val);
   136		if (ret)
   137			return ret;
   138	
   139		/*
   140		 * This shouldn't really happen if the pin is in use already,
   141		 * or if it's not in use yet, it doesn't matter since we're
   142		 * going to change the value soon anyway. Default to output.
   143		 */
   144		if ((val & AXP20X_GPIO_FUNCTIONS) > 2)
   145			return 0;
   146	
   147		/*
   148		 * The GPIO directions are the three lowest values.
   149		 * 2 is input, 0 and 1 are output
   150		 */
   151		return val & 2;
   152	}
   153	
   154	static void axp20x_gpio_set(struct gpio_chip *chip, unsigned int offset,
   155				    int value)
   156	{
   157		struct axp20x_pctl *pctl = gpiochip_get_data(chip);
 > 158		int pin_reg = (int)pctl->desc->pins[offset].pin.drv_data;
   159	
   160		regmap_update_bits(pctl->regmap, pin_reg,
   161				   AXP20X_GPIO_FUNCTIONS,
   162				   value ? AXP20X_GPIO_FUNCTION_OUT_HIGH
   163					 : AXP20X_GPIO_FUNCTION_OUT_LOW);
   164	}
   165	
   166	static int axp20x_gpio_input(struct gpio_chip *chip, unsigned int offset)
   167	{
 > 168		return pinctrl_gpio_direction_input(chip->base + offset);
   169	}
   170	
   171	static int axp20x_gpio_output(struct gpio_chip *chip, unsigned int offset,
   172				      int value)
   173	{
   174		chip->set(chip, offset, value);
   175	
   176		return 0;
   177	}
   178	
   179	static int axp20x_pmx_set(struct pinctrl_dev *pctldev, unsigned int offset,
   180				  u8 config)
   181	{
 > 182		struct axp20x_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
   183		int pin_reg = (int)pctl->desc->pins[offset].pin.drv_data;
   184	
   185		return regmap_update_bits(pctl->regmap, pin_reg, AXP20X_GPIO_FUNCTIONS,
   186					  config);
   187	}
   188	
   189	static int axp20x_pmx_func_cnt(struct pinctrl_dev *pctldev)
   190	{
   191		struct axp20x_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
   192	
   193		return pctl->nfunctions;
   194	}
   195	
   196	static const char *axp20x_pmx_func_name(struct pinctrl_dev *pctldev,
   197						unsigned int selector)
   198	{
 > 199		struct axp20x_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
   200	
   201		return pctl->functions[selector].name;
   202	}
   203	
   204	static int axp20x_pmx_func_groups(struct pinctrl_dev *pctldev,
   205					  unsigned int selector,
   206					  const char * const **groups,
   207					  unsigned int *num_groups)
   208	{
 > 209		struct axp20x_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
   210	
   211		*groups = pctl->functions[selector].groups;
   212		*num_groups = pctl->functions[selector].ngroups;
   213	
   214		return 0;
   215	}
   216	
   217	static struct axp20x_desc_function *
   218	axp20x_pinctrl_desc_find_func_by_name(struct axp20x_pctl *pctl,
   219					      const char *group, const char *func)
   220	{
   221		const struct axp20x_desc_pin *pin;
   222		struct axp20x_desc_function *desc_func;
   223		int i;
   224	
   225		for (i = 0; i < pctl->desc->npins; i++) {
   226			pin = &pctl->desc->pins[i];
   227	
 > 228			if (!strcmp(pin->pin.name, group)) {
   229				desc_func = pin->functions;
   230	
   231				while (desc_func->name) {
   232					if (!strcmp(desc_func->name, func))
   233						return desc_func;
   234					desc_func++;
   235				}
   236	
   237				/*
   238				 * Pins are uniquely named. Groups are named after one
   239				 * pin name. If one pin matches group name but its
   240				 * function cannot be found, no other pin will match
   241				 * group name.
   242				 */
   243				return NULL;
   244			}
   245		}
   246	
   247		return NULL;
   248	}
   249	
   250	static int axp20x_pmx_set_mux(struct pinctrl_dev *pctldev,
   251				      unsigned int function, unsigned int group)
   252	{
 > 253		struct axp20x_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
   254		struct axp20x_pinctrl_group *g = pctl->groups + group;
   255		struct axp20x_pinctrl_function *func = pctl->functions + function;
   256		struct axp20x_desc_function *desc_func =

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 20700 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] gpio: axp209: add pinctrl support
  2016-11-23 13:27 ` [PATCH 2/2] gpio: axp209: add pinctrl support Quentin Schulz
  2016-11-23 13:45   ` Thomas Petazzoni
  2016-11-23 17:13   ` kbuild test robot
@ 2016-11-23 17:21   ` kbuild test robot
  2 siblings, 0 replies; 8+ messages in thread
From: kbuild test robot @ 2016-11-23 17:21 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: kbuild-all, linus.walleij, gnurou, robh+dt, mark.rutland, wens,
	Quentin Schulz, linux-gpio, devicetree, linux-kernel,
	thomas.petazzoni

[-- Attachment #1: Type: text/plain, Size: 2834 bytes --]

Hi Quentin,

[auto build test WARNING on gpio/for-next]
[also build test WARNING on v4.9-rc6 next-20161123]
[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/add-support-for-AXP209-GPIOs-functions/20161124-003102
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: x86_64-randconfig-i0-201647 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/gpio/gpio-axp209.c: In function 'axp20x_gpio_get_direction':
>> drivers/gpio/gpio-axp209.c:131:16: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
     int pin_reg = (int)pctl->desc->pins[offset].pin.drv_data;
                   ^
   drivers/gpio/gpio-axp209.c: In function 'axp20x_gpio_set':
   drivers/gpio/gpio-axp209.c:158:16: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
     int pin_reg = (int)pctl->desc->pins[offset].pin.drv_data;
                   ^
   drivers/gpio/gpio-axp209.c: In function 'axp20x_pmx_set':
   drivers/gpio/gpio-axp209.c:183:16: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
     int pin_reg = (int)pctl->desc->pins[offset].pin.drv_data;
                   ^
   drivers/gpio/gpio-axp209.c: At top level:
   drivers/gpio/gpio-axp209.c:348: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/gpio/gpio-axp209.c:349:18: error: 'pinconf_generic_dt_free_map' undeclared here (not in a function)
     .dt_free_map  = pinconf_generic_dt_free_map,
                     ^

vim +131 drivers/gpio/gpio-axp209.c

   115	static int axp20x_gpio_get(struct gpio_chip *chip, unsigned offset)
   116	{
   117		struct axp20x_pctl *pctl = gpiochip_get_data(chip);
   118		unsigned int val;
   119		int ret;
   120	
   121		ret = regmap_read(pctl->regmap, AXP20X_GPIO20_SS, &val);
   122		if (ret)
   123			return ret;
   124	
   125		return !!(val & BIT(offset + 4));
   126	}
   127	
   128	static int axp20x_gpio_get_direction(struct gpio_chip *chip, unsigned offset)
   129	{
   130		struct axp20x_pctl *pctl = gpiochip_get_data(chip);
 > 131		int pin_reg = (int)pctl->desc->pins[offset].pin.drv_data;
   132		unsigned int val;
   133		int ret;
   134	
   135		ret = regmap_read(pctl->regmap, pin_reg, &val);
   136		if (ret)
   137			return ret;
   138	
   139		/*

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33990 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] gpio: axp209: use correct register for GPIO input status
  2016-11-23 13:45   ` Thomas Petazzoni
@ 2016-11-24  5:52     ` Chen-Yu Tsai
  0 siblings, 0 replies; 8+ messages in thread
From: Chen-Yu Tsai @ 2016-11-24  5:52 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: Quentin Schulz, Linus Walleij, Alexandre Courbot, Rob Herring,
	Mark Rutland, Chen-Yu Tsai, linux-gpio, devicetree, linux-kernel

On Wed, Nov 23, 2016 at 9:45 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> On Wed, 23 Nov 2016 14:27:48 +0100, Quentin Schulz wrote:
>> The GPIO input status was read from control register
>> (AXP20X_GPIO[210]_CTRL) instead of status register (AXP20X_GPIO20_SS).
>>
>> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
>
> This smells like a bug fix, so perhaps Cc: stable?

Presently there are no in-tree boards that use the GPIOs
as input. And the only user I see is the CHIP, for the headphone
jack detection. Again, not supported in mainline yet.

Not sure if there is value in sending it for stable.

ChenYu

>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-11-24  5:53 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-23 13:27 [PATCH 0/2] add support for AXP209 GPIOs functions Quentin Schulz
2016-11-23 13:27 ` [PATCH 1/2] gpio: axp209: use correct register for GPIO input status Quentin Schulz
2016-11-23 13:45   ` Thomas Petazzoni
2016-11-24  5:52     ` Chen-Yu Tsai
2016-11-23 13:27 ` [PATCH 2/2] gpio: axp209: add pinctrl support Quentin Schulz
2016-11-23 13:45   ` Thomas Petazzoni
2016-11-23 17:13   ` kbuild test robot
2016-11-23 17:21   ` kbuild test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).