diff options
author | Vladimir Zapolskiy <vz@mleia.com> | 2019-01-22 16:18:22 -0500 |
---|---|---|
committer | Linus Walleij <linus.walleij@linaro.org> | 2019-01-28 08:39:52 -0500 |
commit | e73339037f6b6d65e84f5fd42e56dd3cdf0d9e9c (patch) | |
tree | 3a1fa2ab93a9378e79d36b175df795021e5d4e01 | |
parent | 87eff9af7efb154cc4a940ed12efc803a0bf3fba (diff) |
pinctrl: remove unused 'pinconf-config' debugfs interface
The main goal of the change is to remove .pin_config_dbg_parse_modify
callback before a driver with its support appears. So far the in-kernel
interface did not attract any users since its introduction 5 years ago.
Originally .pin_config_dbg_parse_modify callback and the associated
'pinconf-config' debugfs file were introduced in commit f07512e615dd
("pinctrl/pinconfig: add debug interface"), a short description of
'pinconf-config' usage for debugging can be expressed this way:
Write to 'pinconf-config' (see pinconf_dbg_config_write() function):
% echo -n modify $map_type $device_name $state_name $pin_name $config > \
/sys/kernel/debug/pinctrl/$pinctrl/pinconf-config
It supposes to update a global (therefore single!) 'pinconf_dbg_conf'
variable with an alternative setting, the arguments should match
an existing pinconf device and some registered pinctrl mapping 'map':
* $map_type is either 'config_pin' or 'config_group', it should match
'map->type' value of PIN_MAP_TYPE_CONFIGS_PIN or
PIN_MAP_TYPE_CONFIGS_GROUP accordingly,
* $device_name should match 'map->dev_name' string value,
* $state_name should match 'map->name' string value,
* $pin_name should match 'map->data.configs.group_or_pin' string value,
If all above has matched, then $config is a new value to be set by calling
pinconfops->pin_config_dbg_parse_modify(pctldev, config, matched_config).
After a successful write into 'pinconf-config' a user can read the file
to get information about that single modified pin configuration.
The fact is .pin_config_dbg_parse_modify callback has never been defined
in 'struct pinconf_ops' of any pinconf driver, thus an actual modification
of a pin or group state on any present pinconf controller does not happen,
and it declares that all related code is no more than dead code.
I discovered the issue while attempting to add .pin_config_dbg_parse_modify
support in some drivers and found that too short 'MAX_NAME_LEN' set by
drivers/pinctrl/pinconf.c:372:#define MAX_NAME_LEN 15
is practically insufficient to store a regular pinctrl device name,
which are like 'e6060000.pin-controller-sh-pfc' or pin names like
'MX6QDL_PAD_ENET_REF_CLK', thus it is another indicator that the code
is barely usable, insufficiently tested and unprepossessing.
Of course it might be possible to increase MAX_NAME_LEN, and then add
.pin_config_dbg_parse_modify callbacks to the drivers, but the whole
idea of such a limited debug option looks inviable. A more flexible
way to functionally substitute the original approach is to implicitly
or explicitly use pinctrl_select_state() function whenever needed.
Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
Cc: Laurent Meunier <laurent.meunier@st.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
-rw-r--r-- | drivers/pinctrl/pinconf.c | 222 | ||||
-rw-r--r-- | include/linux/pinctrl/pinconf.h | 4 |
2 files changed, 0 insertions, 226 deletions
diff --git a/drivers/pinctrl/pinconf.c b/drivers/pinctrl/pinconf.c index 2c7229380f08..2678603df14b 100644 --- a/drivers/pinctrl/pinconf.c +++ b/drivers/pinctrl/pinconf.c | |||
@@ -17,7 +17,6 @@ | |||
17 | #include <linux/slab.h> | 17 | #include <linux/slab.h> |
18 | #include <linux/debugfs.h> | 18 | #include <linux/debugfs.h> |
19 | #include <linux/seq_file.h> | 19 | #include <linux/seq_file.h> |
20 | #include <linux/uaccess.h> | ||
21 | #include <linux/pinctrl/machine.h> | 20 | #include <linux/pinctrl/machine.h> |
22 | #include <linux/pinctrl/pinctrl.h> | 21 | #include <linux/pinctrl/pinctrl.h> |
23 | #include <linux/pinctrl/pinconf.h> | 22 | #include <linux/pinctrl/pinconf.h> |
@@ -369,225 +368,6 @@ static int pinconf_groups_show(struct seq_file *s, void *what) | |||
369 | DEFINE_SHOW_ATTRIBUTE(pinconf_pins); | 368 | DEFINE_SHOW_ATTRIBUTE(pinconf_pins); |
370 | DEFINE_SHOW_ATTRIBUTE(pinconf_groups); | 369 | DEFINE_SHOW_ATTRIBUTE(pinconf_groups); |
371 | 370 | ||
372 | #define MAX_NAME_LEN 15 | ||
373 | |||
374 | struct dbg_cfg { | ||
375 | enum pinctrl_map_type map_type; | ||
376 | char dev_name[MAX_NAME_LEN + 1]; | ||
377 | char state_name[MAX_NAME_LEN + 1]; | ||
378 | char pin_name[MAX_NAME_LEN + 1]; | ||
379 | }; | ||
380 | |||
381 | /* | ||
382 | * Goal is to keep this structure as global in order to simply read the | ||
383 | * pinconf-config file after a write to check config is as expected | ||
384 | */ | ||
385 | static struct dbg_cfg pinconf_dbg_conf; | ||
386 | |||
387 | /** | ||
388 | * pinconf_dbg_config_print() - display the pinctrl config from the pinctrl | ||
389 | * map, of the dev/pin/state that was last written to pinconf-config file. | ||
390 | * @s: string filled in with config description | ||
391 | * @d: not used | ||
392 | */ | ||
393 | static int pinconf_dbg_config_print(struct seq_file *s, void *d) | ||
394 | { | ||
395 | struct pinctrl_maps *maps_node; | ||
396 | const struct pinctrl_map *map; | ||
397 | const struct pinctrl_map *found = NULL; | ||
398 | struct pinctrl_dev *pctldev; | ||
399 | struct dbg_cfg *dbg = &pinconf_dbg_conf; | ||
400 | int i; | ||
401 | |||
402 | mutex_lock(&pinctrl_maps_mutex); | ||
403 | |||
404 | /* Parse the pinctrl map and look for the elected pin/state */ | ||
405 | for_each_maps(maps_node, i, map) { | ||
406 | if (map->type != dbg->map_type) | ||
407 | continue; | ||
408 | if (strcmp(map->dev_name, dbg->dev_name)) | ||
409 | continue; | ||
410 | if (strcmp(map->name, dbg->state_name)) | ||
411 | continue; | ||
412 | |||
413 | if (!strcmp(map->data.configs.group_or_pin, dbg->pin_name)) { | ||
414 | /* We found the right pin */ | ||
415 | found = map; | ||
416 | break; | ||
417 | } | ||
418 | } | ||
419 | |||
420 | if (!found) { | ||
421 | seq_printf(s, "No config found for dev/state/pin, expected:\n"); | ||
422 | seq_printf(s, "Searched dev:%s\n", dbg->dev_name); | ||
423 | seq_printf(s, "Searched state:%s\n", dbg->state_name); | ||
424 | seq_printf(s, "Searched pin:%s\n", dbg->pin_name); | ||
425 | seq_printf(s, "Use: modify config_pin <devname> "\ | ||
426 | "<state> <pinname> <value>\n"); | ||
427 | goto exit; | ||
428 | } | ||
429 | |||
430 | pctldev = get_pinctrl_dev_from_devname(found->ctrl_dev_name); | ||
431 | seq_printf(s, "Dev %s has config of %s in state %s:\n", | ||
432 | dbg->dev_name, dbg->pin_name, dbg->state_name); | ||
433 | pinconf_show_config(s, pctldev, found->data.configs.configs, | ||
434 | found->data.configs.num_configs); | ||
435 | |||
436 | exit: | ||
437 | mutex_unlock(&pinctrl_maps_mutex); | ||
438 | |||
439 | return 0; | ||
440 | } | ||
441 | |||
442 | /** | ||
443 | * pinconf_dbg_config_write() - modify the pinctrl config in the pinctrl | ||
444 | * map, of a dev/pin/state entry based on user entries to pinconf-config | ||
445 | * @user_buf: contains the modification request with expected format: | ||
446 | * modify <config> <devicename> <state> <name> <newvalue> | ||
447 | * modify is literal string, alternatives like add/delete not supported yet | ||
448 | * <config> is the configuration to be changed. Supported configs are | ||
449 | * "config_pin" or "config_group", alternatives like config_mux are not | ||
450 | * supported yet. | ||
451 | * <devicename> <state> <name> are values that should match the pinctrl-maps | ||
452 | * <newvalue> reflects the new config and is driver dependent | ||
453 | */ | ||
454 | static ssize_t pinconf_dbg_config_write(struct file *file, | ||
455 | const char __user *user_buf, size_t count, loff_t *ppos) | ||
456 | { | ||
457 | struct pinctrl_maps *maps_node; | ||
458 | const struct pinctrl_map *map; | ||
459 | const struct pinctrl_map *found = NULL; | ||
460 | struct pinctrl_dev *pctldev; | ||
461 | const struct pinconf_ops *confops = NULL; | ||
462 | struct dbg_cfg *dbg = &pinconf_dbg_conf; | ||
463 | const struct pinctrl_map_configs *configs; | ||
464 | char config[MAX_NAME_LEN + 1]; | ||
465 | char buf[128]; | ||
466 | char *b = &buf[0]; | ||
467 | int buf_size; | ||
468 | char *token; | ||
469 | int i; | ||
470 | |||
471 | /* Get userspace string and assure termination */ | ||
472 | buf_size = min(count, sizeof(buf) - 1); | ||
473 | if (copy_from_user(buf, user_buf, buf_size)) | ||
474 | return -EFAULT; | ||
475 | buf[buf_size] = 0; | ||
476 | |||
477 | /* | ||
478 | * need to parse entry and extract parameters: | ||
479 | * modify configs_pin devicename state pinname newvalue | ||
480 | */ | ||
481 | |||
482 | /* Get arg: 'modify' */ | ||
483 | token = strsep(&b, " "); | ||
484 | if (!token) | ||
485 | return -EINVAL; | ||
486 | if (strcmp(token, "modify")) | ||
487 | return -EINVAL; | ||
488 | |||
489 | /* | ||
490 | * Get arg type: "config_pin" and "config_group" | ||
491 | * types are supported so far | ||
492 | */ | ||
493 | token = strsep(&b, " "); | ||
494 | if (!token) | ||
495 | return -EINVAL; | ||
496 | if (!strcmp(token, "config_pin")) | ||
497 | dbg->map_type = PIN_MAP_TYPE_CONFIGS_PIN; | ||
498 | else if (!strcmp(token, "config_group")) | ||
499 | dbg->map_type = PIN_MAP_TYPE_CONFIGS_GROUP; | ||
500 | else | ||
501 | return -EINVAL; | ||
502 | |||
503 | /* get arg 'device_name' */ | ||
504 | token = strsep(&b, " "); | ||
505 | if (!token) | ||
506 | return -EINVAL; | ||
507 | if (strlen(token) >= MAX_NAME_LEN) | ||
508 | return -EINVAL; | ||
509 | strncpy(dbg->dev_name, token, MAX_NAME_LEN); | ||
510 | |||
511 | /* get arg 'state_name' */ | ||
512 | token = strsep(&b, " "); | ||
513 | if (!token) | ||
514 | return -EINVAL; | ||
515 | if (strlen(token) >= MAX_NAME_LEN) | ||
516 | return -EINVAL; | ||
517 | strncpy(dbg->state_name, token, MAX_NAME_LEN); | ||
518 | |||
519 | /* get arg 'pin_name' */ | ||
520 | token = strsep(&b, " "); | ||
521 | if (!token) | ||
522 | return -EINVAL; | ||
523 | if (strlen(token) >= MAX_NAME_LEN) | ||
524 | return -EINVAL; | ||
525 | strncpy(dbg->pin_name, token, MAX_NAME_LEN); | ||
526 | |||
527 | /* get new_value of config' */ | ||
528 | token = strsep(&b, " "); | ||
529 | if (!token) | ||
530 | return -EINVAL; | ||
531 | if (strlen(token) >= MAX_NAME_LEN) | ||
532 | return -EINVAL; | ||
533 | strncpy(config, token, MAX_NAME_LEN); | ||
534 | |||
535 | mutex_lock(&pinctrl_maps_mutex); | ||
536 | |||
537 | /* Parse the pinctrl map and look for the selected dev/state/pin */ | ||
538 | for_each_maps(maps_node, i, map) { | ||
539 | if (strcmp(map->dev_name, dbg->dev_name)) | ||
540 | continue; | ||
541 | if (map->type != dbg->map_type) | ||
542 | continue; | ||
543 | if (strcmp(map->name, dbg->state_name)) | ||
544 | continue; | ||
545 | |||
546 | /* we found the right pin / state, so overwrite config */ | ||
547 | if (!strcmp(map->data.configs.group_or_pin, dbg->pin_name)) { | ||
548 | found = map; | ||
549 | break; | ||
550 | } | ||
551 | } | ||
552 | |||
553 | if (!found) { | ||
554 | count = -EINVAL; | ||
555 | goto exit; | ||
556 | } | ||
557 | |||
558 | pctldev = get_pinctrl_dev_from_devname(found->ctrl_dev_name); | ||
559 | if (pctldev) | ||
560 | confops = pctldev->desc->confops; | ||
561 | |||
562 | if (confops && confops->pin_config_dbg_parse_modify) { | ||
563 | configs = &found->data.configs; | ||
564 | for (i = 0; i < configs->num_configs; i++) { | ||
565 | confops->pin_config_dbg_parse_modify(pctldev, | ||
566 | config, | ||
567 | &configs->configs[i]); | ||
568 | } | ||
569 | } | ||
570 | |||
571 | exit: | ||
572 | mutex_unlock(&pinctrl_maps_mutex); | ||
573 | |||
574 | return count; | ||
575 | } | ||
576 | |||
577 | static int pinconf_dbg_config_open(struct inode *inode, struct file *file) | ||
578 | { | ||
579 | return single_open(file, pinconf_dbg_config_print, inode->i_private); | ||
580 | } | ||
581 | |||
582 | static const struct file_operations pinconf_dbg_pinconfig_fops = { | ||
583 | .open = pinconf_dbg_config_open, | ||
584 | .write = pinconf_dbg_config_write, | ||
585 | .read = seq_read, | ||
586 | .llseek = seq_lseek, | ||
587 | .release = single_release, | ||
588 | .owner = THIS_MODULE, | ||
589 | }; | ||
590 | |||
591 | void pinconf_init_device_debugfs(struct dentry *devroot, | 371 | void pinconf_init_device_debugfs(struct dentry *devroot, |
592 | struct pinctrl_dev *pctldev) | 372 | struct pinctrl_dev *pctldev) |
593 | { | 373 | { |
@@ -595,8 +375,6 @@ void pinconf_init_device_debugfs(struct dentry *devroot, | |||
595 | devroot, pctldev, &pinconf_pins_fops); | 375 | devroot, pctldev, &pinconf_pins_fops); |
596 | debugfs_create_file("pinconf-groups", S_IFREG | S_IRUGO, | 376 | debugfs_create_file("pinconf-groups", S_IFREG | S_IRUGO, |
597 | devroot, pctldev, &pinconf_groups_fops); | 377 | devroot, pctldev, &pinconf_groups_fops); |
598 | debugfs_create_file("pinconf-config", (S_IRUGO | S_IWUSR | S_IWGRP), | ||
599 | devroot, pctldev, &pinconf_dbg_pinconfig_fops); | ||
600 | } | 378 | } |
601 | 379 | ||
602 | #endif | 380 | #endif |
diff --git a/include/linux/pinctrl/pinconf.h b/include/linux/pinctrl/pinconf.h index 109468d9d849..93c9dd133e9d 100644 --- a/include/linux/pinctrl/pinconf.h +++ b/include/linux/pinctrl/pinconf.h | |||
@@ -29,7 +29,6 @@ struct seq_file; | |||
29 | * @pin_config_group_get: get configurations for an entire pin group; should | 29 | * @pin_config_group_get: get configurations for an entire pin group; should |
30 | * return -ENOTSUPP and -EINVAL using the same rules as pin_config_get. | 30 | * return -ENOTSUPP and -EINVAL using the same rules as pin_config_get. |
31 | * @pin_config_group_set: configure all pins in a group | 31 | * @pin_config_group_set: configure all pins in a group |
32 | * @pin_config_dbg_parse_modify: optional debugfs to modify a pin configuration | ||
33 | * @pin_config_dbg_show: optional debugfs display hook that will provide | 32 | * @pin_config_dbg_show: optional debugfs display hook that will provide |
34 | * per-device info for a certain pin in debugfs | 33 | * per-device info for a certain pin in debugfs |
35 | * @pin_config_group_dbg_show: optional debugfs display hook that will provide | 34 | * @pin_config_group_dbg_show: optional debugfs display hook that will provide |
@@ -55,9 +54,6 @@ struct pinconf_ops { | |||
55 | unsigned selector, | 54 | unsigned selector, |
56 | unsigned long *configs, | 55 | unsigned long *configs, |
57 | unsigned num_configs); | 56 | unsigned num_configs); |
58 | int (*pin_config_dbg_parse_modify) (struct pinctrl_dev *pctldev, | ||
59 | const char *arg, | ||
60 | unsigned long *config); | ||
61 | void (*pin_config_dbg_show) (struct pinctrl_dev *pctldev, | 57 | void (*pin_config_dbg_show) (struct pinctrl_dev *pctldev, |
62 | struct seq_file *s, | 58 | struct seq_file *s, |
63 | unsigned offset); | 59 | unsigned offset); |