summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorVladimir Zapolskiy <vz@mleia.com>2019-01-22 16:18:22 -0500
committerLinus Walleij <linus.walleij@linaro.org>2019-01-28 08:39:52 -0500
commite73339037f6b6d65e84f5fd42e56dd3cdf0d9e9c (patch)
tree3a1fa2ab93a9378e79d36b175df795021e5d4e01
parent87eff9af7efb154cc4a940ed12efc803a0bf3fba (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.c222
-rw-r--r--include/linux/pinctrl/pinconf.h4
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)
369DEFINE_SHOW_ATTRIBUTE(pinconf_pins); 368DEFINE_SHOW_ATTRIBUTE(pinconf_pins);
370DEFINE_SHOW_ATTRIBUTE(pinconf_groups); 369DEFINE_SHOW_ATTRIBUTE(pinconf_groups);
371 370
372#define MAX_NAME_LEN 15
373
374struct 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 */
385static 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 */
393static 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
436exit:
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 */
454static 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
571exit:
572 mutex_unlock(&pinctrl_maps_mutex);
573
574 return count;
575}
576
577static 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
582static 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
591void pinconf_init_device_debugfs(struct dentry *devroot, 371void 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);