| Message ID | 1520974749-5372-2-git-send-email-eajames@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Tue, Mar 13, 2018 at 10:59 PM, Eddie James <eajames@linux.vnet.ibm.com> wrote: > From: Christopher Bostic <cbostic@linux.vnet.ibm.com> > > Add a struct gpio_chip and define some methods so that this device's > I/O can be accessed via /sys/class/gpio. > + /* > + * Note: > + * > + * Pinmux support has not been added to the new gpio_chip. > + * This support should be added when possible given the mux > + * behavior of these IO devices. > + */ > + data->gpio.label = (const char *)&client->name; Hmm... Why do you need this casting? > + data->gpio.get_direction = ucd9000_gpio_get_direction; > + data->gpio.direction_input = ucd9000_gpio_direction_input; > + data->gpio.direction_output = ucd9000_gpio_direction_output; > + data->gpio.get = ucd9000_gpio_get; > + data->gpio.set = ucd9000_gpio_set; > + data->gpio.can_sleep = 1; Isn't it type of boolean? > + data->gpio.base = -1; > + data->gpio.parent = &client->dev; > + data->gpio.owner = THIS_MODULE; > + data->gpio.of_node = client->dev.of_node; I think GPIO core does this for you. > + if (data->gpio.ngpio) { Hmm... I would rather reorganize the above part to a separate helper, like static int ..._probe_gpio() { ... switch () { default: return 0; /* GPIO part is optional */ } return 0; } ret = _probe_gpio(); if (ret) dev_warn(); > + ret = devm_gpiochip_add_data(&client->dev, &data->gpio, > + client); > + if (ret) > + dev_warn(&client->dev, "Could not add gpiochip: %d\n", > + ret); > + } > + > return pmbus_do_probe(client, mid, info); > } > > -- > 1.8.3.1 >
On Tue, Mar 13, 2018 at 11:13:04PM +0200, Andy Shevchenko wrote: > On Tue, Mar 13, 2018 at 10:59 PM, Eddie James > <eajames@linux.vnet.ibm.com> wrote: > > From: Christopher Bostic <cbostic@linux.vnet.ibm.com> > > > > Add a struct gpio_chip and define some methods so that this device's > > I/O can be accessed via /sys/class/gpio. > > > + /* > > + * Note: > > + * > > + * Pinmux support has not been added to the new gpio_chip. > > + * This support should be added when possible given the mux > > + * behavior of these IO devices. > > + */ > > > + data->gpio.label = (const char *)&client->name; > > Hmm... Why do you need this casting? > > > + data->gpio.get_direction = ucd9000_gpio_get_direction; > > + data->gpio.direction_input = ucd9000_gpio_direction_input; > > + data->gpio.direction_output = ucd9000_gpio_direction_output; > > + data->gpio.get = ucd9000_gpio_get; > > + data->gpio.set = ucd9000_gpio_set; > > > + data->gpio.can_sleep = 1; > > Isn't it type of boolean? > You are right. > > + data->gpio.base = -1; > > > + data->gpio.parent = &client->dev; > > + data->gpio.owner = THIS_MODULE; > > > + data->gpio.of_node = client->dev.of_node; > > I think GPIO core does this for you. > Same here. I checked and found that it also sets the owner, so maybe this can be dropped as well. > > + if (data->gpio.ngpio) { > > Hmm... > > I would rather reorganize the above part to a separate helper, like > > static int ..._probe_gpio() > { > ... > switch () { > default: > return 0; /* GPIO part is optional */ > } > return 0; > } > > ret = _probe_gpio(); > if (ret) > dev_warn(); > I am neutral to positiva on that. Might as well add the call to devm_gpiochip_add_data() into that function as well. > > + ret = devm_gpiochip_add_data(&client->dev, &data->gpio, > > + client); > > + if (ret) > > + dev_warn(&client->dev, "Could not add gpiochip: %d\n", > > + ret); > > + } > > + > > return pmbus_do_probe(client, mid, info); > > } > > > > -- > > 1.8.3.1 > > > > > > -- > With Best Regards, > Andy Shevchenko -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/14/2018 01:55 PM, Guenter Roeck wrote: > On Tue, Mar 13, 2018 at 11:13:04PM +0200, Andy Shevchenko wrote: >> On Tue, Mar 13, 2018 at 10:59 PM, Eddie James >> <eajames@linux.vnet.ibm.com> wrote: >>> From: Christopher Bostic <cbostic@linux.vnet.ibm.com> >>> >>> Add a struct gpio_chip and define some methods so that this device's >>> I/O can be accessed via /sys/class/gpio. >>> + /* >>> + * Note: >>> + * >>> + * Pinmux support has not been added to the new gpio_chip. >>> + * This support should be added when possible given the mux >>> + * behavior of these IO devices. >>> + */ >>> + data->gpio.label = (const char *)&client->name; >> Hmm... Why do you need this casting? >> >>> + data->gpio.get_direction = ucd9000_gpio_get_direction; >>> + data->gpio.direction_input = ucd9000_gpio_direction_input; >>> + data->gpio.direction_output = ucd9000_gpio_direction_output; >>> + data->gpio.get = ucd9000_gpio_get; >>> + data->gpio.set = ucd9000_gpio_set; >>> + data->gpio.can_sleep = 1; >> Isn't it type of boolean? >> > You are right. > >>> + data->gpio.base = -1; >>> + data->gpio.parent = &client->dev; >>> + data->gpio.owner = THIS_MODULE; >>> + data->gpio.of_node = client->dev.of_node; >> I think GPIO core does this for you. >> > Same here. I checked and found that it also sets the owner, so maybe this can be > dropped as well. Ah true. Especially since there is a TODO: remove chip->owner in the core. > >>> + if (data->gpio.ngpio) { >> Hmm... >> >> I would rather reorganize the above part to a separate helper, like >> >> static int ..._probe_gpio() >> { >> ... >> switch () { >> default: >> return 0; /* GPIO part is optional */ >> } >> return 0; >> } >> >> ret = _probe_gpio(); >> if (ret) >> dev_warn(); >> > I am neutral to positiva on that. > Might as well add the call to devm_gpiochip_add_data() > into that function as well. Sure. I can do a v4. Thanks, Eddie > >>> + ret = devm_gpiochip_add_data(&client->dev, &data->gpio, >>> + client); >>> + if (ret) >>> + dev_warn(&client->dev, "Could not add gpiochip: %d\n", >>> + ret); >>> + } >>> + >>> return pmbus_do_probe(client, mid, info); >>> } >>> >>> -- >>> 1.8.3.1 >>> >> >> >> -- >> With Best Regards, >> Andy Shevchenko -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Christopher,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.16-rc5 next-20180315]
[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/Eddie-James/hwmon-ucd9000-Add-gpio-and-debugfs-interfaces/20180316-125048
config: x86_64-randconfig-x019-201810 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All error/warnings (new ones prefixed by >>):
>> drivers/hwmon/pmbus/ucd9000.c:69:19: error: field 'gpio' has incomplete type
struct gpio_chip gpio;
^~~~
drivers/hwmon/pmbus/ucd9000.c: In function 'ucd9000_gpio_get':
>> drivers/hwmon/pmbus/ucd9000.c:184:31: error: implicit declaration of function 'gpiochip_get_data'; did you mean 'gpio_get_value'? [-Werror=implicit-function-declaration]
struct i2c_client *client = gpiochip_get_data(gc);
^~~~~~~~~~~~~~~~~
gpio_get_value
>> drivers/hwmon/pmbus/ucd9000.c:184:31: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
drivers/hwmon/pmbus/ucd9000.c: In function 'ucd9000_gpio_set':
drivers/hwmon/pmbus/ucd9000.c:197:30: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
struct i2c_client *client = gpiochip_get_data(gc);
^~~~~~~~~~~~~~~~~
drivers/hwmon/pmbus/ucd9000.c: In function 'ucd9000_gpio_get_direction':
drivers/hwmon/pmbus/ucd9000.c:240:30: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
struct i2c_client *client = gpiochip_get_data(gc);
^~~~~~~~~~~~~~~~~
drivers/hwmon/pmbus/ucd9000.c: In function 'ucd9000_gpio_set_direction':
drivers/hwmon/pmbus/ucd9000.c:254:30: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
struct i2c_client *client = gpiochip_get_data(gc);
^~~~~~~~~~~~~~~~~
drivers/hwmon/pmbus/ucd9000.c: In function 'ucd9000_probe':
>> drivers/hwmon/pmbus/ucd9000.c:460:9: error: implicit declaration of function 'devm_gpiochip_add_data'; did you mean 'devm_gpio_request'? [-Werror=implicit-function-declaration]
ret = devm_gpiochip_add_data(&client->dev, &data->gpio,
^~~~~~~~~~~~~~~~~~~~~~
devm_gpio_request
cc1: some warnings being treated as errors
vim +/gpio +69 drivers/hwmon/pmbus/ucd9000.c
65
66 struct ucd9000_data {
67 u8 fan_data[UCD9000_NUM_FAN][I2C_SMBUS_BLOCK_MAX];
68 struct pmbus_driver_info info;
> 69 struct gpio_chip gpio;
70 };
71 #define to_ucd9000_data(_info) container_of(_info, struct ucd9000_data, info)
72
73 static int ucd9000_get_fan_config(struct i2c_client *client, int fan)
74 {
75 int fan_config = 0;
76 struct ucd9000_data *data
77 = to_ucd9000_data(pmbus_get_driver_info(client));
78
79 if (data->fan_data[fan][3] & 1)
80 fan_config |= PB_FAN_2_INSTALLED; /* Use lower bit position */
81
82 /* Pulses/revolution */
83 fan_config |= (data->fan_data[fan][3] & 0x06) >> 1;
84
85 return fan_config;
86 }
87
88 static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg)
89 {
90 int ret = 0;
91 int fan_config;
92
93 switch (reg) {
94 case PMBUS_FAN_CONFIG_12:
95 if (page > 0)
96 return -ENXIO;
97
98 ret = ucd9000_get_fan_config(client, 0);
99 if (ret < 0)
100 return ret;
101 fan_config = ret << 4;
102 ret = ucd9000_get_fan_config(client, 1);
103 if (ret < 0)
104 return ret;
105 fan_config |= ret;
106 ret = fan_config;
107 break;
108 case PMBUS_FAN_CONFIG_34:
109 if (page > 0)
110 return -ENXIO;
111
112 ret = ucd9000_get_fan_config(client, 2);
113 if (ret < 0)
114 return ret;
115 fan_config = ret << 4;
116 ret = ucd9000_get_fan_config(client, 3);
117 if (ret < 0)
118 return ret;
119 fan_config |= ret;
120 ret = fan_config;
121 break;
122 default:
123 ret = -ENODATA;
124 break;
125 }
126 return ret;
127 }
128
129 static const struct i2c_device_id ucd9000_id[] = {
130 {"ucd9000", ucd9000},
131 {"ucd90120", ucd90120},
132 {"ucd90124", ucd90124},
133 {"ucd90160", ucd90160},
134 {"ucd9090", ucd9090},
135 {"ucd90910", ucd90910},
136 {}
137 };
138 MODULE_DEVICE_TABLE(i2c, ucd9000_id);
139
140 static const struct of_device_id ucd9000_of_match[] = {
141 {
142 .compatible = "ti,ucd9000",
143 .data = (void *)ucd9000
144 },
145 {
146 .compatible = "ti,ucd90120",
147 .data = (void *)ucd90120
148 },
149 {
150 .compatible = "ti,ucd90124",
151 .data = (void *)ucd90124
152 },
153 {
154 .compatible = "ti,ucd90160",
155 .data = (void *)ucd90160
156 },
157 {
158 .compatible = "ti,ucd9090",
159 .data = (void *)ucd9090
160 },
161 {
162 .compatible = "ti,ucd90910",
163 .data = (void *)ucd90910
164 },
165 { },
166 };
167 MODULE_DEVICE_TABLE(of, ucd9000_of_match);
168
169 static int ucd9000_gpio_read_config(struct i2c_client *client,
170 unsigned int offset)
171 {
172 int ret;
173
174 /* No page set required */
175 ret = i2c_smbus_write_byte_data(client, UCD9000_GPIO_SELECT, offset);
176 if (ret < 0)
177 return ret;
178
179 return i2c_smbus_read_byte_data(client, UCD9000_GPIO_CONFIG);
180 }
181
182 static int ucd9000_gpio_get(struct gpio_chip *gc, unsigned int offset)
183 {
> 184 struct i2c_client *client = gpiochip_get_data(gc);
185 int ret;
186
187 ret = ucd9000_gpio_read_config(client, offset);
188 if (ret < 0)
189 return ret;
190
191 return !!(ret & UCD9000_GPIO_CONFIG_STATUS);
192 }
193
194 static void ucd9000_gpio_set(struct gpio_chip *gc, unsigned int offset,
195 int value)
196 {
197 struct i2c_client *client = gpiochip_get_data(gc);
198 int ret;
199
200 ret = ucd9000_gpio_read_config(client, offset);
201 if (ret < 0) {
202 dev_dbg(&client->dev, "failed to read GPIO %d config: %d\n",
203 offset, ret);
204 return;
205 }
206
207 if (value) {
208 if (ret & UCD9000_GPIO_CONFIG_STATUS)
209 return;
210
211 ret |= UCD9000_GPIO_CONFIG_STATUS;
212 } else {
213 if (!(ret & UCD9000_GPIO_CONFIG_STATUS))
214 return;
215
216 ret &= ~UCD9000_GPIO_CONFIG_STATUS;
217 }
218
219 ret |= UCD9000_GPIO_CONFIG_ENABLE;
220
221 /* Page set not required */
222 ret = i2c_smbus_write_byte_data(client, UCD9000_GPIO_CONFIG, ret);
223 if (ret < 0) {
224 dev_dbg(&client->dev, "Failed to write GPIO %d config: %d\n",
225 offset, ret);
226 return;
227 }
228
229 ret &= ~UCD9000_GPIO_CONFIG_ENABLE;
230
231 ret = i2c_smbus_write_byte_data(client, UCD9000_GPIO_CONFIG, ret);
232 if (ret < 0)
233 dev_dbg(&client->dev, "Failed to write GPIO %d config: %d\n",
234 offset, ret);
235 }
236
237 static int ucd9000_gpio_get_direction(struct gpio_chip *gc,
238 unsigned int offset)
239 {
> 240 struct i2c_client *client = gpiochip_get_data(gc);
241 int ret;
242
243 ret = ucd9000_gpio_read_config(client, offset);
244 if (ret < 0)
245 return ret;
246
247 return !(ret & UCD9000_GPIO_CONFIG_OUT_ENABLE);
248 }
249
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Christopher,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.16-rc5 next-20180315]
[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/Eddie-James/hwmon-ucd9000-Add-gpio-and-debugfs-interfaces/20180316-125048
config: x86_64-randconfig-x017-201810 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers/hwmon/pmbus/ucd9000.c: In function 'ucd9000_probe':
>> drivers/hwmon/pmbus/ucd9000.c:457:12: error: 'struct gpio_chip' has no member named 'of_node'
data->gpio.of_node = client->dev.of_node;
^
vim +457 drivers/hwmon/pmbus/ucd9000.c
308
309 static int ucd9000_probe(struct i2c_client *client,
310 const struct i2c_device_id *id)
311 {
312 u8 block_buffer[I2C_SMBUS_BLOCK_MAX + 1];
313 struct ucd9000_data *data;
314 struct pmbus_driver_info *info;
315 const struct i2c_device_id *mid;
316 enum chips chip;
317 int i, ret;
318
319 if (!i2c_check_functionality(client->adapter,
320 I2C_FUNC_SMBUS_BYTE_DATA |
321 I2C_FUNC_SMBUS_BLOCK_DATA))
322 return -ENODEV;
323
324 ret = i2c_smbus_read_block_data(client, UCD9000_DEVICE_ID,
325 block_buffer);
326 if (ret < 0) {
327 dev_err(&client->dev, "Failed to read device ID\n");
328 return ret;
329 }
330 block_buffer[ret] = '\0';
331 dev_info(&client->dev, "Device ID %s\n", block_buffer);
332
333 for (mid = ucd9000_id; mid->name[0]; mid++) {
334 if (!strncasecmp(mid->name, block_buffer, strlen(mid->name)))
335 break;
336 }
337 if (!mid->name[0]) {
338 dev_err(&client->dev, "Unsupported device\n");
339 return -ENODEV;
340 }
341
342 if (client->dev.of_node)
343 chip = (enum chips)of_device_get_match_data(&client->dev);
344 else
345 chip = id->driver_data;
346
347 if (chip != ucd9000 && chip != mid->driver_data)
348 dev_notice(&client->dev,
349 "Device mismatch: Configured %s, detected %s\n",
350 id->name, mid->name);
351
352 data = devm_kzalloc(&client->dev, sizeof(struct ucd9000_data),
353 GFP_KERNEL);
354 if (!data)
355 return -ENOMEM;
356 info = &data->info;
357
358 ret = i2c_smbus_read_byte_data(client, UCD9000_NUM_PAGES);
359 if (ret < 0) {
360 dev_err(&client->dev,
361 "Failed to read number of active pages\n");
362 return ret;
363 }
364 info->pages = ret;
365 if (!info->pages) {
366 dev_err(&client->dev, "No pages configured\n");
367 return -ENODEV;
368 }
369
370 /* The internal temperature sensor is always active */
371 info->func[0] = PMBUS_HAVE_TEMP;
372
373 /* Everything else is configurable */
374 ret = i2c_smbus_read_block_data(client, UCD9000_MONITOR_CONFIG,
375 block_buffer);
376 if (ret <= 0) {
377 dev_err(&client->dev, "Failed to read configuration data\n");
378 return -ENODEV;
379 }
380 for (i = 0; i < ret; i++) {
381 int page = UCD9000_MON_PAGE(block_buffer[i]);
382
383 if (page >= info->pages)
384 continue;
385
386 switch (UCD9000_MON_TYPE(block_buffer[i])) {
387 case UCD9000_MON_VOLTAGE:
388 case UCD9000_MON_VOLTAGE_HW:
389 info->func[page] |= PMBUS_HAVE_VOUT
390 | PMBUS_HAVE_STATUS_VOUT;
391 break;
392 case UCD9000_MON_TEMPERATURE:
393 info->func[page] |= PMBUS_HAVE_TEMP2
394 | PMBUS_HAVE_STATUS_TEMP;
395 break;
396 case UCD9000_MON_CURRENT:
397 info->func[page] |= PMBUS_HAVE_IOUT
398 | PMBUS_HAVE_STATUS_IOUT;
399 break;
400 default:
401 break;
402 }
403 }
404
405 /* Fan configuration */
406 if (mid->driver_data == ucd90124) {
407 for (i = 0; i < UCD9000_NUM_FAN; i++) {
408 i2c_smbus_write_byte_data(client,
409 UCD9000_FAN_CONFIG_INDEX, i);
410 ret = i2c_smbus_read_block_data(client,
411 UCD9000_FAN_CONFIG,
412 data->fan_data[i]);
413 if (ret < 0)
414 return ret;
415 }
416 i2c_smbus_write_byte_data(client, UCD9000_FAN_CONFIG_INDEX, 0);
417
418 info->read_byte_data = ucd9000_read_byte_data;
419 info->func[0] |= PMBUS_HAVE_FAN12 | PMBUS_HAVE_STATUS_FAN12
420 | PMBUS_HAVE_FAN34 | PMBUS_HAVE_STATUS_FAN34;
421 }
422
423 /*
424 * Note:
425 *
426 * Pinmux support has not been added to the new gpio_chip.
427 * This support should be added when possible given the mux
428 * behavior of these IO devices.
429 */
430 data->gpio.label = (const char *)&client->name;
431 data->gpio.get_direction = ucd9000_gpio_get_direction;
432 data->gpio.direction_input = ucd9000_gpio_direction_input;
433 data->gpio.direction_output = ucd9000_gpio_direction_output;
434 data->gpio.get = ucd9000_gpio_get;
435 data->gpio.set = ucd9000_gpio_set;
436 data->gpio.can_sleep = 1;
437 data->gpio.base = -1;
438
439 switch (mid->driver_data) {
440 case ucd9090:
441 data->gpio.ngpio = UCD9090_NUM_GPIOS;
442 break;
443 case ucd90120:
444 case ucd90124:
445 case ucd90160:
446 data->gpio.ngpio = UCD901XX_NUM_GPIOS;
447 break;
448 case ucd90910:
449 data->gpio.ngpio = UCD90910_NUM_GPIOS;
450 break;
451 default:
452 break;
453 }
454
455 data->gpio.parent = &client->dev;
456 data->gpio.owner = THIS_MODULE;
> 457 data->gpio.of_node = client->dev.of_node;
458
459 if (data->gpio.ngpio) {
460 ret = devm_gpiochip_add_data(&client->dev, &data->gpio,
461 client);
462 if (ret)
463 dev_warn(&client->dev, "Could not add gpiochip: %d\n",
464 ret);
465 }
466
467 return pmbus_do_probe(client, mid, info);
468 }
469
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/drivers/hwmon/pmbus/ucd9000.c b/drivers/hwmon/pmbus/ucd9000.c index b74dbec..023fb9e 100644 --- a/drivers/hwmon/pmbus/ucd9000.c +++ b/drivers/hwmon/pmbus/ucd9000.c @@ -27,6 +27,7 @@ #include <linux/slab.h> #include <linux/i2c.h> #include <linux/pmbus.h> +#include <linux/gpio.h> #include "pmbus.h" enum chips { ucd9000, ucd90120, ucd90124, ucd90160, ucd9090, ucd90910 }; @@ -35,8 +36,18 @@ #define UCD9000_NUM_PAGES 0xd6 #define UCD9000_FAN_CONFIG_INDEX 0xe7 #define UCD9000_FAN_CONFIG 0xe8 +#define UCD9000_GPIO_SELECT 0xfa +#define UCD9000_GPIO_CONFIG 0xfb #define UCD9000_DEVICE_ID 0xfd +/* GPIO CONFIG bits */ +#define UCD9000_GPIO_CONFIG_ENABLE BIT(0) +#define UCD9000_GPIO_CONFIG_OUT_ENABLE BIT(1) +#define UCD9000_GPIO_CONFIG_OUT_VALUE BIT(2) +#define UCD9000_GPIO_CONFIG_STATUS BIT(3) +#define UCD9000_GPIO_INPUT 0 +#define UCD9000_GPIO_OUTPUT 1 + #define UCD9000_MON_TYPE(x) (((x) >> 5) & 0x07) #define UCD9000_MON_PAGE(x) ((x) & 0x0f) @@ -47,9 +58,15 @@ #define UCD9000_NUM_FAN 4 +#define UCD9000_GPIO_NAME_LEN 16 +#define UCD9090_NUM_GPIOS 23 +#define UCD901XX_NUM_GPIOS 26 +#define UCD90910_NUM_GPIOS 26 + struct ucd9000_data { u8 fan_data[UCD9000_NUM_FAN][I2C_SMBUS_BLOCK_MAX]; struct pmbus_driver_info info; + struct gpio_chip gpio; }; #define to_ucd9000_data(_info) container_of(_info, struct ucd9000_data, info) @@ -149,6 +166,146 @@ static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) }; MODULE_DEVICE_TABLE(of, ucd9000_of_match); +static int ucd9000_gpio_read_config(struct i2c_client *client, + unsigned int offset) +{ + int ret; + + /* No page set required */ + ret = i2c_smbus_write_byte_data(client, UCD9000_GPIO_SELECT, offset); + if (ret < 0) + return ret; + + return i2c_smbus_read_byte_data(client, UCD9000_GPIO_CONFIG); +} + +static int ucd9000_gpio_get(struct gpio_chip *gc, unsigned int offset) +{ + struct i2c_client *client = gpiochip_get_data(gc); + int ret; + + ret = ucd9000_gpio_read_config(client, offset); + if (ret < 0) + return ret; + + return !!(ret & UCD9000_GPIO_CONFIG_STATUS); +} + +static void ucd9000_gpio_set(struct gpio_chip *gc, unsigned int offset, + int value) +{ + struct i2c_client *client = gpiochip_get_data(gc); + int ret; + + ret = ucd9000_gpio_read_config(client, offset); + if (ret < 0) { + dev_dbg(&client->dev, "failed to read GPIO %d config: %d\n", + offset, ret); + return; + } + + if (value) { + if (ret & UCD9000_GPIO_CONFIG_STATUS) + return; + + ret |= UCD9000_GPIO_CONFIG_STATUS; + } else { + if (!(ret & UCD9000_GPIO_CONFIG_STATUS)) + return; + + ret &= ~UCD9000_GPIO_CONFIG_STATUS; + } + + ret |= UCD9000_GPIO_CONFIG_ENABLE; + + /* Page set not required */ + ret = i2c_smbus_write_byte_data(client, UCD9000_GPIO_CONFIG, ret); + if (ret < 0) { + dev_dbg(&client->dev, "Failed to write GPIO %d config: %d\n", + offset, ret); + return; + } + + ret &= ~UCD9000_GPIO_CONFIG_ENABLE; + + ret = i2c_smbus_write_byte_data(client, UCD9000_GPIO_CONFIG, ret); + if (ret < 0) + dev_dbg(&client->dev, "Failed to write GPIO %d config: %d\n", + offset, ret); +} + +static int ucd9000_gpio_get_direction(struct gpio_chip *gc, + unsigned int offset) +{ + struct i2c_client *client = gpiochip_get_data(gc); + int ret; + + ret = ucd9000_gpio_read_config(client, offset); + if (ret < 0) + return ret; + + return !(ret & UCD9000_GPIO_CONFIG_OUT_ENABLE); +} + +static int ucd9000_gpio_set_direction(struct gpio_chip *gc, + unsigned int offset, bool direction_out, + int requested_out) +{ + struct i2c_client *client = gpiochip_get_data(gc); + int ret, config, out_val; + + ret = ucd9000_gpio_read_config(client, offset); + if (ret < 0) + return ret; + + if (direction_out) { + out_val = requested_out ? UCD9000_GPIO_CONFIG_OUT_VALUE : 0; + + if (ret & UCD9000_GPIO_CONFIG_OUT_ENABLE) { + if ((ret & UCD9000_GPIO_CONFIG_OUT_VALUE) == out_val) + return 0; + } else { + ret |= UCD9000_GPIO_CONFIG_OUT_ENABLE; + } + + if (out_val) + ret |= UCD9000_GPIO_CONFIG_OUT_VALUE; + else + ret &= ~UCD9000_GPIO_CONFIG_OUT_VALUE; + + } else { + if (!(ret & UCD9000_GPIO_CONFIG_OUT_ENABLE)) + return 0; + + ret &= ~UCD9000_GPIO_CONFIG_OUT_ENABLE; + } + + ret |= UCD9000_GPIO_CONFIG_ENABLE; + config = ret; + + /* Page set not required */ + ret = i2c_smbus_write_byte_data(client, UCD9000_GPIO_CONFIG, config); + if (ret < 0) + return ret; + + config &= ~UCD9000_GPIO_CONFIG_ENABLE; + + return i2c_smbus_write_byte_data(client, UCD9000_GPIO_CONFIG, config); +} + +static int ucd9000_gpio_direction_input(struct gpio_chip *gc, + unsigned int offset) +{ + return ucd9000_gpio_set_direction(gc, offset, UCD9000_GPIO_INPUT, 0); +} + +static int ucd9000_gpio_direction_output(struct gpio_chip *gc, + unsigned int offset, int val) +{ + return ucd9000_gpio_set_direction(gc, offset, UCD9000_GPIO_OUTPUT, + val); +} + static int ucd9000_probe(struct i2c_client *client, const struct i2c_device_id *id) { @@ -263,6 +420,50 @@ static int ucd9000_probe(struct i2c_client *client, | PMBUS_HAVE_FAN34 | PMBUS_HAVE_STATUS_FAN34; } + /* + * Note: + * + * Pinmux support has not been added to the new gpio_chip. + * This support should be added when possible given the mux + * behavior of these IO devices. + */ + data->gpio.label = (const char *)&client->name; + data->gpio.get_direction = ucd9000_gpio_get_direction; + data->gpio.direction_input = ucd9000_gpio_direction_input; + data->gpio.direction_output = ucd9000_gpio_direction_output; + data->gpio.get = ucd9000_gpio_get; + data->gpio.set = ucd9000_gpio_set; + data->gpio.can_sleep = 1; + data->gpio.base = -1; + + switch (mid->driver_data) { + case ucd9090: + data->gpio.ngpio = UCD9090_NUM_GPIOS; + break; + case ucd90120: + case ucd90124: + case ucd90160: + data->gpio.ngpio = UCD901XX_NUM_GPIOS; + break; + case ucd90910: + data->gpio.ngpio = UCD90910_NUM_GPIOS; + break; + default: + break; + } + + data->gpio.parent = &client->dev; + data->gpio.owner = THIS_MODULE; + data->gpio.of_node = client->dev.of_node; + + if (data->gpio.ngpio) { + ret = devm_gpiochip_add_data(&client->dev, &data->gpio, + client); + if (ret) + dev_warn(&client->dev, "Could not add gpiochip: %d\n", + ret); + } + return pmbus_do_probe(client, mid, info); }