Message ID | 20170127144738.21107-3-sre@kernel.org |
---|---|
State | New |
Headers | show |
Hi Sebastian, [auto build test ERROR on gpio/for-next] [also build test ERROR on v4.10-rc5 next-20170125] [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/Sebastian-Reichel/mcp23s08-pinconf-support/20170127-230243 base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next config: x86_64-randconfig-x003-201704 (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All error/warnings (new ones prefixed by >>): >> drivers/gpio/gpio-mcp23s08.c:86:22: error: field 'pinctrl_desc' has incomplete type struct pinctrl_desc pinctrl_desc; ^~~~~~~~~~~~ >> drivers/gpio/gpio-mcp23s08.c:139:38: error: array type has incomplete element type 'struct pinctrl_pin_desc' static const struct pinctrl_pin_desc mcp23x08_pins[] = { ^~~~~~~~~~~~~ >> drivers/gpio/gpio-mcp23s08.c:140:2: error: implicit declaration of function 'PINCTRL_PIN' [-Werror=implicit-function-declaration] PINCTRL_PIN(0, "gpio0"), ^~~~~~~~~~~ drivers/gpio/gpio-mcp23s08.c:150:38: error: array type has incomplete element type 'struct pinctrl_pin_desc' static const struct pinctrl_pin_desc mcp23x17_pins[] = { ^~~~~~~~~~~~~ >> drivers/gpio/gpio-mcp23s08.c:188:21: error: variable 'mcp_pinctrl_ops' has initializer but incomplete type static const struct pinctrl_ops mcp_pinctrl_ops = { ^~~~~~~~~~~ >> drivers/gpio/gpio-mcp23s08.c:189:2: error: unknown field 'get_groups_count' specified in initializer .get_groups_count = mcp_pinctrl_get_groups_count, ^ >> drivers/gpio/gpio-mcp23s08.c:189:22: warning: excess elements in struct initializer .get_groups_count = mcp_pinctrl_get_groups_count, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpio/gpio-mcp23s08.c:189:22: note: (near initialization for 'mcp_pinctrl_ops') >> drivers/gpio/gpio-mcp23s08.c:190:2: error: unknown field 'get_group_name' specified in initializer .get_group_name = mcp_pinctrl_get_group_name, ^ drivers/gpio/gpio-mcp23s08.c:190:20: warning: excess elements in struct initializer .get_group_name = mcp_pinctrl_get_group_name, ^~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpio/gpio-mcp23s08.c:190:20: note: (near initialization for 'mcp_pinctrl_ops') >> drivers/gpio/gpio-mcp23s08.c:191:2: error: unknown field 'get_group_pins' specified in initializer .get_group_pins = mcp_pinctrl_get_group_pins, ^ drivers/gpio/gpio-mcp23s08.c:191:20: warning: excess elements in struct initializer .get_group_pins = mcp_pinctrl_get_group_pins, ^~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpio/gpio-mcp23s08.c:191:20: note: (near initialization for 'mcp_pinctrl_ops') >> drivers/gpio/gpio-mcp23s08.c:193:2: error: unknown field 'dt_node_to_map' specified in initializer .dt_node_to_map = pinconf_generic_dt_node_to_map_pin, ^ >> drivers/gpio/gpio-mcp23s08.c:193:20: error: 'pinconf_generic_dt_node_to_map_pin' undeclared here (not in a function) .dt_node_to_map = pinconf_generic_dt_node_to_map_pin, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpio/gpio-mcp23s08.c:193:20: warning: excess elements in struct initializer drivers/gpio/gpio-mcp23s08.c:193:20: note: (near initialization for 'mcp_pinctrl_ops') >> drivers/gpio/gpio-mcp23s08.c:194:2: error: unknown field 'dt_free_map' specified in initializer .dt_free_map = pinconf_generic_dt_free_map, ^ >> drivers/gpio/gpio-mcp23s08.c:194:17: error: 'pinconf_generic_dt_free_map' undeclared here (not in a function) .dt_free_map = pinconf_generic_dt_free_map, ^~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpio/gpio-mcp23s08.c:194:17: warning: excess elements in struct initializer drivers/gpio/gpio-mcp23s08.c:194:17: note: (near initialization for 'mcp_pinctrl_ops') drivers/gpio/gpio-mcp23s08.c: In function 'mcp_pinconf_get': >> drivers/gpio/gpio-mcp23s08.c:201:25: error: implicit declaration of function 'pinctrl_dev_get_drvdata' [-Werror=implicit-function-declaration] struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev); ^~~~~~~~~~~~~~~~~~~~~~~ >> drivers/gpio/gpio-mcp23s08.c:201:25: warning: initialization makes pointer from integer without a cast [-Wint-conversion] drivers/gpio/gpio-mcp23s08.c: In function 'mcp_pinconf_set': drivers/gpio/gpio-mcp23s08.c:226:25: warning: initialization makes pointer from integer without a cast [-Wint-conversion] struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev); ^~~~~~~~~~~~~~~~~~~~~~~ drivers/gpio/gpio-mcp23s08.c: At top level: >> drivers/gpio/gpio-mcp23s08.c:252:21: error: variable 'mcp_pinconf_ops' has initializer but incomplete type static const struct pinconf_ops mcp_pinconf_ops = { ^~~~~~~~~~~ >> drivers/gpio/gpio-mcp23s08.c:253:2: error: unknown field 'pin_config_get' specified in initializer .pin_config_get = mcp_pinconf_get, ^ drivers/gpio/gpio-mcp23s08.c:253:20: warning: excess elements in struct initializer .pin_config_get = mcp_pinconf_get, ^~~~~~~~~~~~~~~ drivers/gpio/gpio-mcp23s08.c:253:20: note: (near initialization for 'mcp_pinconf_ops') >> drivers/gpio/gpio-mcp23s08.c:254:2: error: unknown field 'pin_config_set' specified in initializer .pin_config_set = mcp_pinconf_set, ^ drivers/gpio/gpio-mcp23s08.c:254:20: warning: excess elements in struct initializer .pin_config_set = mcp_pinconf_set, ^~~~~~~~~~~~~~~ drivers/gpio/gpio-mcp23s08.c:254:20: note: (near initialization for 'mcp_pinconf_ops') >> drivers/gpio/gpio-mcp23s08.c:255:2: error: unknown field 'is_generic' specified in initializer .is_generic = true, ^ drivers/gpio/gpio-mcp23s08.c:255:16: warning: excess elements in struct initializer .is_generic = true, ^~~~ drivers/gpio/gpio-mcp23s08.c:255:16: note: (near initialization for 'mcp_pinconf_ops') drivers/gpio/gpio-mcp23s08.c: In function 'mcp23s08_probe_one': >> drivers/gpio/gpio-mcp23s08.c:776:17: error: implicit declaration of function 'pinctrl_register' [-Werror=implicit-function-declaration] mcp->pctldev = pinctrl_register(&mcp->pinctrl_desc, dev, mcp); ^~~~~~~~~~~~~~~~ drivers/gpio/gpio-mcp23s08.c: At top level: >> drivers/gpio/gpio-mcp23s08.c:188:33: error: storage size of 'mcp_pinctrl_ops' isn't known static const struct pinctrl_ops mcp_pinctrl_ops = { ^~~~~~~~~~~~~~~ vim +/pinctrl_desc +86 drivers/gpio/gpio-mcp23s08.c 80 struct gpio_chip chip; 81 82 struct regmap *regmap; 83 struct device *dev; 84 85 struct pinctrl_dev *pctldev; > 86 struct pinctrl_desc pinctrl_desc; 87 }; 88 89 static const struct regmap_config mcp23x08_regmap = { 90 .reg_bits = 8, 91 .val_bits = 8, 92 93 .reg_stride = 1, 94 .max_register = MCP_OLAT, 95 }; 96 97 static const struct regmap_config mcp23x17_regmap = { 98 .reg_bits = 8, 99 .val_bits = 16, 100 101 .reg_stride = 2, 102 .max_register = MCP_OLAT << 1, 103 .val_format_endian = REGMAP_ENDIAN_LITTLE, 104 }; 105 106 static int mcp_read(struct mcp23s08 *mcp, unsigned int reg, unsigned int *val) 107 { 108 return regmap_read(mcp->regmap, reg << mcp->reg_shift, val); 109 } 110 111 static int mcp_write(struct mcp23s08 *mcp, unsigned int reg, unsigned int val) 112 { 113 return regmap_write(mcp->regmap, reg << mcp->reg_shift, val); 114 } 115 116 static int mcp_set_bit(struct mcp23s08 *mcp, unsigned int reg, 117 unsigned int pin, bool enabled) 118 { 119 u16 val = enabled ? 0xffff : 0x0000; 120 u16 mask = BIT(pin); 121 return regmap_update_bits(mcp->regmap, reg << mcp->reg_shift, 122 mask, val); 123 } 124 125 static int mcp_update_cache(struct mcp23s08 *mcp) 126 { 127 int ret, reg, i; 128 129 for (i = 0; i < ARRAY_SIZE(mcp->cache); i++) { 130 ret = mcp_read(mcp, i, ®); 131 if (ret < 0) 132 return ret; 133 mcp->cache[i] = reg; 134 } 135 136 return 0; 137 } 138 > 139 static const struct pinctrl_pin_desc mcp23x08_pins[] = { > 140 PINCTRL_PIN(0, "gpio0"), 141 PINCTRL_PIN(1, "gpio1"), 142 PINCTRL_PIN(2, "gpio2"), 143 PINCTRL_PIN(3, "gpio3"), 144 PINCTRL_PIN(4, "gpio4"), 145 PINCTRL_PIN(5, "gpio5"), 146 PINCTRL_PIN(6, "gpio6"), 147 PINCTRL_PIN(7, "gpio7"), 148 }; 149 > 150 static const struct pinctrl_pin_desc mcp23x17_pins[] = { 151 PINCTRL_PIN(0, "gpio0"), 152 PINCTRL_PIN(1, "gpio1"), 153 PINCTRL_PIN(2, "gpio2"), 154 PINCTRL_PIN(3, "gpio3"), 155 PINCTRL_PIN(4, "gpio4"), 156 PINCTRL_PIN(5, "gpio5"), 157 PINCTRL_PIN(6, "gpio6"), 158 PINCTRL_PIN(7, "gpio7"), 159 PINCTRL_PIN(8, "gpio8"), 160 PINCTRL_PIN(9, "gpio9"), 161 PINCTRL_PIN(10, "gpio10"), 162 PINCTRL_PIN(11, "gpio11"), 163 PINCTRL_PIN(12, "gpio12"), 164 PINCTRL_PIN(13, "gpio13"), 165 PINCTRL_PIN(14, "gpio14"), 166 PINCTRL_PIN(15, "gpio15"), 167 }; 168 169 static int mcp_pinctrl_get_groups_count(struct pinctrl_dev *pctldev) 170 { 171 return 0; 172 } 173 174 static const char *mcp_pinctrl_get_group_name(struct pinctrl_dev *pctldev, 175 unsigned int group) 176 { 177 return NULL; 178 } 179 180 static int mcp_pinctrl_get_group_pins(struct pinctrl_dev *pctldev, 181 unsigned int group, 182 const unsigned int **pins, 183 unsigned int *num_pins) 184 { 185 return -ENOTSUPP; 186 } 187 > 188 static const struct pinctrl_ops mcp_pinctrl_ops = { > 189 .get_groups_count = mcp_pinctrl_get_groups_count, > 190 .get_group_name = mcp_pinctrl_get_group_name, > 191 .get_group_pins = mcp_pinctrl_get_group_pins, 192 #ifdef CONFIG_OF > 193 .dt_node_to_map = pinconf_generic_dt_node_to_map_pin, > 194 .dt_free_map = pinconf_generic_dt_free_map, 195 #endif 196 }; 197 198 static int mcp_pinconf_get(struct pinctrl_dev *pctldev, unsigned int pin, 199 unsigned long *config) 200 { > 201 struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev); 202 enum pin_config_param param = pinconf_to_config_param(*config); 203 unsigned int data, status; 204 int ret; 205 206 switch (param) { 207 case PIN_CONFIG_BIAS_PULL_UP: 208 ret = mcp_read(mcp, MCP_GPPU, &data); 209 if (ret < 0) 210 return ret; 211 status = (data & BIT(pin)) ? 1 : 0; 212 break; 213 default: 214 dev_err(mcp->dev, "Invalid config param %04x\n", param); 215 return -ENOTSUPP; 216 } 217 218 *config = 0; 219 220 return status ? 0 : -EINVAL; 221 } 222 223 static int mcp_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin, 224 unsigned long *configs, unsigned int num_configs) 225 { 226 struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev); 227 enum pin_config_param param; 228 u32 arg, mask; 229 u16 val; 230 int ret = 0; 231 int i; 232 233 for (i = 0; i < num_configs; i++) { 234 param = pinconf_to_config_param(configs[i]); 235 arg = pinconf_to_config_argument(configs[i]); 236 237 switch (param) { 238 case PIN_CONFIG_BIAS_PULL_UP: 239 val = arg ? 0xFFFF : 0x0000; 240 mask = BIT(pin); 241 ret = mcp_set_bit(mcp, MCP_GPPU, pin, arg); 242 break; 243 default: 244 dev_err(mcp->dev, "Invalid config param %04x\n", param); 245 return -ENOTSUPP; 246 } 247 } 248 249 return ret; 250 } 251 > 252 static const struct pinconf_ops mcp_pinconf_ops = { > 253 .pin_config_get = mcp_pinconf_get, > 254 .pin_config_set = mcp_pinconf_set, > 255 .is_generic = true, 256 }; 257 258 /*----------------------------------------------------------------------*/ --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Sebastian, [auto build test ERROR on gpio/for-next] [also build test ERROR on v4.10-rc5 next-20170125] [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/Sebastian-Reichel/mcp23s08-pinconf-support/20170127-230243 base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next config: i386-randconfig-c0-01280104 (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=i386 All errors (new ones prefixed by >>): drivers/gpio/gpio-mcp23s08.c:86:22: error: field 'pinctrl_desc' has incomplete type struct pinctrl_desc pinctrl_desc; ^ >> drivers/gpio/gpio-mcp23s08.c:139:38: error: array type has incomplete element type static const struct pinctrl_pin_desc mcp23x08_pins[] = { ^ drivers/gpio/gpio-mcp23s08.c:140:2: error: implicit declaration of function 'PINCTRL_PIN' [-Werror=implicit-function-declaration] PINCTRL_PIN(0, "gpio0"), ^ drivers/gpio/gpio-mcp23s08.c:150:38: error: array type has incomplete element type static const struct pinctrl_pin_desc mcp23x17_pins[] = { ^ drivers/gpio/gpio-mcp23s08.c:188:21: error: variable 'mcp_pinctrl_ops' has initializer but incomplete type static const struct pinctrl_ops mcp_pinctrl_ops = { ^ drivers/gpio/gpio-mcp23s08.c:189:2: error: unknown field 'get_groups_count' specified in initializer .get_groups_count = mcp_pinctrl_get_groups_count, ^ drivers/gpio/gpio-mcp23s08.c:189:2: warning: excess elements in struct initializer drivers/gpio/gpio-mcp23s08.c:189:2: warning: (near initialization for 'mcp_pinctrl_ops') drivers/gpio/gpio-mcp23s08.c:190:2: error: unknown field 'get_group_name' specified in initializer .get_group_name = mcp_pinctrl_get_group_name, ^ drivers/gpio/gpio-mcp23s08.c:190:2: warning: excess elements in struct initializer drivers/gpio/gpio-mcp23s08.c:190:2: warning: (near initialization for 'mcp_pinctrl_ops') drivers/gpio/gpio-mcp23s08.c:191:2: error: unknown field 'get_group_pins' specified in initializer .get_group_pins = mcp_pinctrl_get_group_pins, ^ drivers/gpio/gpio-mcp23s08.c:191:2: warning: excess elements in struct initializer drivers/gpio/gpio-mcp23s08.c:191:2: warning: (near initialization for 'mcp_pinctrl_ops') drivers/gpio/gpio-mcp23s08.c:193:2: error: unknown field 'dt_node_to_map' specified in initializer .dt_node_to_map = pinconf_generic_dt_node_to_map_pin, ^ drivers/gpio/gpio-mcp23s08.c:193:20: error: 'pinconf_generic_dt_node_to_map_pin' undeclared here (not in a function) .dt_node_to_map = pinconf_generic_dt_node_to_map_pin, ^ drivers/gpio/gpio-mcp23s08.c:193:2: warning: excess elements in struct initializer .dt_node_to_map = pinconf_generic_dt_node_to_map_pin, ^ drivers/gpio/gpio-mcp23s08.c:193:2: warning: (near initialization for 'mcp_pinctrl_ops') drivers/gpio/gpio-mcp23s08.c:194:2: error: unknown field 'dt_free_map' specified in initializer .dt_free_map = pinconf_generic_dt_free_map, ^ drivers/gpio/gpio-mcp23s08.c:194:17: error: 'pinconf_generic_dt_free_map' undeclared here (not in a function) .dt_free_map = pinconf_generic_dt_free_map, ^ drivers/gpio/gpio-mcp23s08.c:194:2: warning: excess elements in struct initializer .dt_free_map = pinconf_generic_dt_free_map, ^ drivers/gpio/gpio-mcp23s08.c:194:2: warning: (near initialization for 'mcp_pinctrl_ops') drivers/gpio/gpio-mcp23s08.c: In function 'mcp_pinconf_get': drivers/gpio/gpio-mcp23s08.c:201:9: error: implicit declaration of function 'pinctrl_dev_get_drvdata' [-Werror=implicit-function-declaration] struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev); ^ drivers/gpio/gpio-mcp23s08.c:201:25: warning: initialization makes pointer from integer without a cast struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev); ^ drivers/gpio/gpio-mcp23s08.c: In function 'mcp_pinconf_set': drivers/gpio/gpio-mcp23s08.c:226:25: warning: initialization makes pointer from integer without a cast struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev); ^ drivers/gpio/gpio-mcp23s08.c: At top level: drivers/gpio/gpio-mcp23s08.c:252:21: error: variable 'mcp_pinconf_ops' has initializer but incomplete type static const struct pinconf_ops mcp_pinconf_ops = { ^ drivers/gpio/gpio-mcp23s08.c:253:2: error: unknown field 'pin_config_get' specified in initializer .pin_config_get = mcp_pinconf_get, ^ drivers/gpio/gpio-mcp23s08.c:253:2: warning: excess elements in struct initializer drivers/gpio/gpio-mcp23s08.c:253:2: warning: (near initialization for 'mcp_pinconf_ops') drivers/gpio/gpio-mcp23s08.c:254:2: error: unknown field 'pin_config_set' specified in initializer .pin_config_set = mcp_pinconf_set, ^ drivers/gpio/gpio-mcp23s08.c:254:2: warning: excess elements in struct initializer drivers/gpio/gpio-mcp23s08.c:254:2: warning: (near initialization for 'mcp_pinconf_ops') drivers/gpio/gpio-mcp23s08.c:255:2: error: unknown field 'is_generic' specified in initializer .is_generic = true, ^ drivers/gpio/gpio-mcp23s08.c:255:2: warning: excess elements in struct initializer drivers/gpio/gpio-mcp23s08.c:255:2: warning: (near initialization for 'mcp_pinconf_ops') drivers/gpio/gpio-mcp23s08.c: In function 'mcp23s08_probe_one': drivers/gpio/gpio-mcp23s08.c:776:2: error: implicit declaration of function 'pinctrl_register' [-Werror=implicit-function-declaration] mcp->pctldev = pinctrl_register(&mcp->pinctrl_desc, dev, mcp); ^ drivers/gpio/gpio-mcp23s08.c: At top level: drivers/gpio/gpio-mcp23s08.c:139:38: warning: 'mcp23x08_pins' defined but not used [-Wunused-variable] static const struct pinctrl_pin_desc mcp23x08_pins[] = { ^ drivers/gpio/gpio-mcp23s08.c:150:38: warning: 'mcp23x17_pins' defined but not used [-Wunused-variable] static const struct pinctrl_pin_desc mcp23x17_pins[] = { ^ cc1: some warnings being treated as errors vim +139 drivers/gpio/gpio-mcp23s08.c 80 struct gpio_chip chip; 81 82 struct regmap *regmap; 83 struct device *dev; 84 85 struct pinctrl_dev *pctldev; > 86 struct pinctrl_desc pinctrl_desc; 87 }; 88 89 static const struct regmap_config mcp23x08_regmap = { 90 .reg_bits = 8, 91 .val_bits = 8, 92 93 .reg_stride = 1, 94 .max_register = MCP_OLAT, 95 }; 96 97 static const struct regmap_config mcp23x17_regmap = { 98 .reg_bits = 8, 99 .val_bits = 16, 100 101 .reg_stride = 2, 102 .max_register = MCP_OLAT << 1, 103 .val_format_endian = REGMAP_ENDIAN_LITTLE, 104 }; 105 106 static int mcp_read(struct mcp23s08 *mcp, unsigned int reg, unsigned int *val) 107 { 108 return regmap_read(mcp->regmap, reg << mcp->reg_shift, val); 109 } 110 111 static int mcp_write(struct mcp23s08 *mcp, unsigned int reg, unsigned int val) 112 { 113 return regmap_write(mcp->regmap, reg << mcp->reg_shift, val); 114 } 115 116 static int mcp_set_bit(struct mcp23s08 *mcp, unsigned int reg, 117 unsigned int pin, bool enabled) 118 { 119 u16 val = enabled ? 0xffff : 0x0000; 120 u16 mask = BIT(pin); 121 return regmap_update_bits(mcp->regmap, reg << mcp->reg_shift, 122 mask, val); 123 } 124 125 static int mcp_update_cache(struct mcp23s08 *mcp) 126 { 127 int ret, reg, i; 128 129 for (i = 0; i < ARRAY_SIZE(mcp->cache); i++) { 130 ret = mcp_read(mcp, i, ®); 131 if (ret < 0) 132 return ret; 133 mcp->cache[i] = reg; 134 } 135 136 return 0; 137 } 138 > 139 static const struct pinctrl_pin_desc mcp23x08_pins[] = { 140 PINCTRL_PIN(0, "gpio0"), 141 PINCTRL_PIN(1, "gpio1"), 142 PINCTRL_PIN(2, "gpio2"), --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Fri, Jan 27, 2017 at 3:47 PM, Sebastian Reichel <sre@kernel.org> wrote: > mcp23xxx device have configurable 100k pullup resistors. This adds > support for enabling them using pinctrl's pinconf interface. > > Signed-off-by: Sebastian Reichel <sre@kernel.org> That's the right approach! > diff --git a/drivers/gpio/gpio-mcp23s08.c b/drivers/gpio/gpio-mcp23s08.c Can we move the patch to drivers/pinctrl/* like all other mixed drivers doing combined pinctrl and GPIO? Also: no Kconfig changes? Surely you must select the right pinctrl things, I guess you're just lucky that some other pin controller on your system selects your infrastructure. I think that is why the build robot is complaining. > +static int mcp_update_cache(struct mcp23s08 *mcp) > +{ > + int ret, reg, i; > + > + for (i = 0; i < ARRAY_SIZE(mcp->cache); i++) { > + ret = mcp_read(mcp, i, ®); > + if (ret < 0) > + return ret; > + mcp->cache[i] = reg; > + } > + > + return 0; > +} Why do you need a cache of register values when regmap already supports this? Instead I suggest: exploit the .volatile_reg() callback from struct regmap_config and use regmap to maintain the cache. Apart from this is is nice! With the recent changes from Mika Westerberg scheduled for v4.11 the road is open to expose pullups all the way to userspace from GPIOs chardev if we want to (some patches would be needed). Yours, Linus Walleij -- 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 Linus, On Mon, Jan 30, 2017 at 04:08:01PM +0100, Linus Walleij wrote: > On Fri, Jan 27, 2017 at 3:47 PM, Sebastian Reichel <sre@kernel.org> wrote: > > > mcp23xxx device have configurable 100k pullup resistors. This adds > > support for enabling them using pinctrl's pinconf interface. > > > > Signed-off-by: Sebastian Reichel <sre@kernel.org> > > That's the right approach! > > > diff --git a/drivers/gpio/gpio-mcp23s08.c b/drivers/gpio/gpio-mcp23s08.c > > Can we move the patch to drivers/pinctrl/* like all other mixed drivers > doing combined pinctrl and GPIO? Sure. Two questions: * Should the config name change to PINCTRL_MCP23s08 (or PINCTRL_MCP23XXX)? This will mean people will have to fix their .config * Should there be a "SPI or I2C GPIO expanders" sub-menu for it as in the gpio Kconfig? > Also: no Kconfig changes? Surely you must select the right pinctrl > things, I guess you're just lucky that some other pin controller on your > system selects your infrastructure. I think that is why the build robot > is complaining. Yes, it should select GENERIC_PINCONF as far as I can see. > > +static int mcp_update_cache(struct mcp23s08 *mcp) > > +{ > > + int ret, reg, i; > > + > > + for (i = 0; i < ARRAY_SIZE(mcp->cache); i++) { > > + ret = mcp_read(mcp, i, ®); > > + if (ret < 0) > > + return ret; > > + mcp->cache[i] = reg; > > + } > > + > > + return 0; > > +} > > Why do you need a cache of register values when regmap already > supports this? > > Instead I suggest: exploit the .volatile_reg() callback from > struct regmap_config and use regmap to maintain the cache. The mcp_update_cache method is just moved in this patch. I do not need it, but the existing code uses it. Actually I only moved it, so that it sticks together with the mcp_read and mcp_write functions, which must be placed a bit earlier to be usable by the pinctrl stuff. I did not convert the custom caching in the regmap patch, since it should be done in its own cleanup patch. Introducing basic regmap was much more straight forward than removing the open-coded caching. > Apart from this is is nice! I guess the custom platform data based pullup config could also be removed. I just checked and there are not many users of the platform data: linux/arch $ git grep -l mcp23 | grep -v "/dts/" blackfin/mach-bf527/boards/tll6527m.c blackfin/mach-bf609/boards/ezkit.c None of them seems to use the pullups variable. > With the recent changes from Mika Westerberg scheduled for v4.11 > the road is open to expose pullups all the way to userspace from > GPIOs chardev if we want to (some patches would be needed). I just added the pinctrl config to DT, but that sounds useful. -- Sebastian
On Mon, Jan 30, 2017 at 5:40 PM, Sebastian Reichel <sre@kernel.org> wrote: >> Can we move the patch to drivers/pinctrl/* like all other mixed drivers >> doing combined pinctrl and GPIO? > > Sure. Two questions: > > * Should the config name change to PINCTRL_MCP23s08 (or PINCTRL_MCP23XXX)? > This will mean people will have to fix their .config I never understood what the right way is to deal with this. What happens actually when you run "make oldconfig"? Is there a way to supply a replacement Kconfig symbol in a good way? Else I guess we can keep a dangling GPIO_MCP23S08 bool that just selects the new PINCTRL_MCP23XXX for now? With a comment that it got moved? (Please verify that it works though...) > * Should there be a "SPI or I2C GPIO expanders" sub-menu for it as in the > gpio Kconfig? Maybe, that would be a separate refactoring thing for later. I think it is this one and SX150X that use I2C right now. > I did not convert the custom caching in the regmap patch, since it > should be done in its own cleanup patch. Introducing basic regmap > was much more straight forward than removing the open-coded caching. OK I take it that it will be fixed later then, that's fine. > I guess the custom platform data based pullup config could also > be removed. I just checked and there are not many users of the > platform data: > > linux/arch $ git grep -l mcp23 | grep -v "/dts/" > blackfin/mach-bf527/boards/tll6527m.c > blackfin/mach-bf609/boards/ezkit.c > > None of them seems to use the pullups variable. Just kill it off, good riddance! As separate patch before/after moving it I guess. Yours, Linus Walleij -- 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
diff --git a/drivers/gpio/gpio-mcp23s08.c b/drivers/gpio/gpio-mcp23s08.c index bdb692345428..96abecaccc12 100644 --- a/drivers/gpio/gpio-mcp23s08.c +++ b/drivers/gpio/gpio-mcp23s08.c @@ -24,6 +24,10 @@ #include <linux/of_irq.h> #include <linux/of_device.h> #include <linux/regmap.h> +#include <linux/pinctrl/pinconf.h> +#include <linux/pinctrl/pinctrl.h> +#include <linux/pinctrl/pinmux.h> +#include <linux/pinctrl/pinconf-generic.h> /** * MCP types supported by driver @@ -77,6 +81,9 @@ struct mcp23s08 { struct regmap *regmap; struct device *dev; + + struct pinctrl_dev *pctldev; + struct pinctrl_desc pinctrl_desc; }; static const struct regmap_config mcp23x08_regmap = { @@ -96,6 +103,158 @@ static const struct regmap_config mcp23x17_regmap = { .val_format_endian = REGMAP_ENDIAN_LITTLE, }; +static int mcp_read(struct mcp23s08 *mcp, unsigned int reg, unsigned int *val) +{ + return regmap_read(mcp->regmap, reg << mcp->reg_shift, val); +} + +static int mcp_write(struct mcp23s08 *mcp, unsigned int reg, unsigned int val) +{ + return regmap_write(mcp->regmap, reg << mcp->reg_shift, val); +} + +static int mcp_set_bit(struct mcp23s08 *mcp, unsigned int reg, + unsigned int pin, bool enabled) +{ + u16 val = enabled ? 0xffff : 0x0000; + u16 mask = BIT(pin); + return regmap_update_bits(mcp->regmap, reg << mcp->reg_shift, + mask, val); +} + +static int mcp_update_cache(struct mcp23s08 *mcp) +{ + int ret, reg, i; + + for (i = 0; i < ARRAY_SIZE(mcp->cache); i++) { + ret = mcp_read(mcp, i, ®); + if (ret < 0) + return ret; + mcp->cache[i] = reg; + } + + return 0; +} + +static const struct pinctrl_pin_desc mcp23x08_pins[] = { + PINCTRL_PIN(0, "gpio0"), + PINCTRL_PIN(1, "gpio1"), + PINCTRL_PIN(2, "gpio2"), + PINCTRL_PIN(3, "gpio3"), + PINCTRL_PIN(4, "gpio4"), + PINCTRL_PIN(5, "gpio5"), + PINCTRL_PIN(6, "gpio6"), + PINCTRL_PIN(7, "gpio7"), +}; + +static const struct pinctrl_pin_desc mcp23x17_pins[] = { + PINCTRL_PIN(0, "gpio0"), + PINCTRL_PIN(1, "gpio1"), + PINCTRL_PIN(2, "gpio2"), + PINCTRL_PIN(3, "gpio3"), + PINCTRL_PIN(4, "gpio4"), + PINCTRL_PIN(5, "gpio5"), + PINCTRL_PIN(6, "gpio6"), + PINCTRL_PIN(7, "gpio7"), + PINCTRL_PIN(8, "gpio8"), + PINCTRL_PIN(9, "gpio9"), + PINCTRL_PIN(10, "gpio10"), + PINCTRL_PIN(11, "gpio11"), + PINCTRL_PIN(12, "gpio12"), + PINCTRL_PIN(13, "gpio13"), + PINCTRL_PIN(14, "gpio14"), + PINCTRL_PIN(15, "gpio15"), +}; + +static int mcp_pinctrl_get_groups_count(struct pinctrl_dev *pctldev) +{ + return 0; +} + +static const char *mcp_pinctrl_get_group_name(struct pinctrl_dev *pctldev, + unsigned int group) +{ + return NULL; +} + +static int mcp_pinctrl_get_group_pins(struct pinctrl_dev *pctldev, + unsigned int group, + const unsigned int **pins, + unsigned int *num_pins) +{ + return -ENOTSUPP; +} + +static const struct pinctrl_ops mcp_pinctrl_ops = { + .get_groups_count = mcp_pinctrl_get_groups_count, + .get_group_name = mcp_pinctrl_get_group_name, + .get_group_pins = mcp_pinctrl_get_group_pins, +#ifdef CONFIG_OF + .dt_node_to_map = pinconf_generic_dt_node_to_map_pin, + .dt_free_map = pinconf_generic_dt_free_map, +#endif +}; + +static int mcp_pinconf_get(struct pinctrl_dev *pctldev, unsigned int pin, + unsigned long *config) +{ + struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev); + enum pin_config_param param = pinconf_to_config_param(*config); + unsigned int data, status; + int ret; + + switch (param) { + case PIN_CONFIG_BIAS_PULL_UP: + ret = mcp_read(mcp, MCP_GPPU, &data); + if (ret < 0) + return ret; + status = (data & BIT(pin)) ? 1 : 0; + break; + default: + dev_err(mcp->dev, "Invalid config param %04x\n", param); + return -ENOTSUPP; + } + + *config = 0; + + return status ? 0 : -EINVAL; +} + +static int mcp_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin, + unsigned long *configs, unsigned int num_configs) +{ + struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev); + enum pin_config_param param; + u32 arg, mask; + u16 val; + int ret = 0; + int i; + + for (i = 0; i < num_configs; i++) { + param = pinconf_to_config_param(configs[i]); + arg = pinconf_to_config_argument(configs[i]); + + switch (param) { + case PIN_CONFIG_BIAS_PULL_UP: + val = arg ? 0xFFFF : 0x0000; + mask = BIT(pin); + ret = mcp_set_bit(mcp, MCP_GPPU, pin, arg); + break; + default: + dev_err(mcp->dev, "Invalid config param %04x\n", param); + return -ENOTSUPP; + } + } + + return ret; +} + +static const struct pinconf_ops mcp_pinconf_ops = { + .pin_config_get = mcp_pinconf_get, + .pin_config_set = mcp_pinconf_set, + .is_generic = true, +}; + /*----------------------------------------------------------------------*/ #ifdef CONFIG_SPI_MASTER @@ -158,30 +317,6 @@ static const struct regmap_bus mcp23sxx_spi_regmap = { #endif /* CONFIG_SPI_MASTER */ -static int mcp_read(struct mcp23s08 *mcp, unsigned int reg, unsigned int *val) -{ - return regmap_read(mcp->regmap, reg << mcp->reg_shift, val); -} - -static int mcp_write(struct mcp23s08 *mcp, unsigned int reg, unsigned int val) -{ - return regmap_write(mcp->regmap, reg << mcp->reg_shift, val); -} - -static int mcp_update_cache(struct mcp23s08 *mcp) -{ - int ret, reg, i; - - for (i = 0; i < ARRAY_SIZE(mcp->cache); i++) { - ret = mcp_read(mcp, i, ®); - if (ret < 0) - return ret; - mcp->cache[i] = reg; - } - - return 0; -} - /*----------------------------------------------------------------------*/ /* A given spi_device can represent up to eight mcp23sxx chips @@ -627,6 +762,23 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev, if (ret) goto fail; } + + mcp->pinctrl_desc.name = "mcp23xxx-pinctrl"; + mcp->pinctrl_desc.pctlops = &mcp_pinctrl_ops; + mcp->pinctrl_desc.confops = &mcp_pinconf_ops; + mcp->pinctrl_desc.npins = mcp->chip.ngpio; + if (mcp->pinctrl_desc.npins == 8) + mcp->pinctrl_desc.pins = mcp23x08_pins; + else if (mcp->pinctrl_desc.npins == 16) + mcp->pinctrl_desc.pins = mcp23x17_pins; + mcp->pinctrl_desc.owner = THIS_MODULE; + + mcp->pctldev = pinctrl_register(&mcp->pinctrl_desc, dev, mcp); + if (IS_ERR(mcp->pctldev)) { + ret = PTR_ERR(mcp->pctldev); + goto fail; + } + fail: if (ret < 0) dev_dbg(dev, "can't setup chip %d, --> %d\n", addr, ret);
mcp23xxx device have configurable 100k pullup resistors. This adds support for enabling them using pinctrl's pinconf interface. Signed-off-by: Sebastian Reichel <sre@kernel.org> --- drivers/gpio/gpio-mcp23s08.c | 200 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 176 insertions(+), 24 deletions(-)