| Message ID | 20161123132749.11666-3-quentin.schulz@free-electrons.com |
|---|---|
| State | Superseded |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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
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");
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(-) -- 2.9.3 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html