2021-02-10 21:36:16

by Drew Fustini

[permalink] [raw]
Subject: [PATCH v3 0/2] pinctrl: pinmux: Add pinmux-select debugfs file

This series first converts the debugfs files in the pinctrl subsystem to
octal permissions and then adds a new debug file "pinmux-select". Add
function name and group name can be written to "pinmux-select" which
will cause the function and group to be activated on the pin controller.

Notes for PATCH v3:
- add Suggested-by: Andy Shevchenko to the "pinctrl: use to octal
permissions for debugfs files" patch
- change the octal permissions from 0400 to 0444 to correctly match the
symbolic permissions (thanks to Joe Perches and Geert Uytterhoeven)
- note that S_IFREG flag is added to the mode in __debugfs_create_file()
(thanks to Andy for highlighting this and Joe for suggesting I should
add a note to the commit message)
- fix order of the goto labels so that the buffers are freed correctly
as suggested by Dan Carpenter
- move from devm_kzalloc() to kzalloc() as the buffers are only used
inside the pinmux_select() function and not related to the lifetime
of the pin controller device (thanks to Andy for pointing this out)
- correct the pinmux-select example in commit message to use the
function and group name instead of selector (thanks to Geert)

Notes for PATCH v2:
- create patch series that includes patch to switch all the debugfs
files in pinctrl subsystem over to octal permission
- write function name and group name, instead of error-prone selector
numbers, to the 'pinmux-select' file
- switch from static to dynamic allocation for the kernel buffer filled
by strncpy_from_user()
- look up function selector from function name using
pinmux_func_name_to_selector()
- validate group name with get_function_groups() and match_string()
- look up selector for group name with pinctrl_get_group_selector()

Notes for PATCH v1:
- posted seperate patch to switch all the debugfs files in pinctrl
subsystem over to octal permission [1]
- there is no existing documentation for any of the debugfs enteries for
pinctrl, so it seemed to have a bigger scope than just this patch. I
also noticed that rst documentation is confusingly named "pinctl" (no
'r') and started thread about that [2]. Linus suggested chaning that
to 'pin-control'. Thus I am planning a seperate documentation patch
series where the file is renamed, references changed and a section on
the pinctrl debugfs files is added.

Notes for RFC v2 [3]:
- rename debugfs file "pinmux-set" to "pinmux-select"
- renmae pinmux_set_write() to pinmux_select()
- switch from memdup_user_nul() to strncpy_from_user()
- switch from pr_warn() to dev_err()

[1] https://lore.kernel.org/linux-gpio/[email protected]/
[2] https://lore.kernel.org/linux-gpio/20210126050817.GA187797@x1/
[3] https://lore.kernel.org/linux-gpio/[email protected]/

Drew Fustini (2):
pinctrl: use to octal permissions for debugfs files
pinctrl: pinmux: Add pinmux-select debugfs file

drivers/pinctrl/core.c | 6 +--
drivers/pinctrl/pinconf.c | 4 +-
drivers/pinctrl/pinmux.c | 111 +++++++++++++++++++++++++++++++++++++-
3 files changed, 114 insertions(+), 7 deletions(-)

--
2.25.1


2021-02-10 21:36:39

by Drew Fustini

[permalink] [raw]
Subject: [PATCH v3 1/2] pinctrl: use to octal permissions for debugfs files

Switch over pinctrl debugfs files to use octal permissions as they are
preferred over symbolic permissions. Refer to commit f90774e1fd27
("checkpatch: look for symbolic permissions and suggest octal instead").

Note: S_IFREG flag is added to the mode by __debugfs_create_file()
in fs/debugfs/inode.c

Thank you,
Drew

Suggested-by: Andy Shevchenko <[email protected]>
Signed-off-by: Drew Fustini <[email protected]>
---
drivers/pinctrl/core.c | 6 +++---
drivers/pinctrl/pinconf.c | 4 ++--
drivers/pinctrl/pinmux.c | 4 ++--
3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 3663d87f51a0..02f8710afb9c 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -1914,11 +1914,11 @@ static void pinctrl_init_debugfs(void)
return;
}

- debugfs_create_file("pinctrl-devices", S_IFREG | S_IRUGO,
+ debugfs_create_file("pinctrl-devices", 0444,
debugfs_root, NULL, &pinctrl_devices_fops);
- debugfs_create_file("pinctrl-maps", S_IFREG | S_IRUGO,
+ debugfs_create_file("pinctrl-maps", 0444,
debugfs_root, NULL, &pinctrl_maps_fops);
- debugfs_create_file("pinctrl-handles", S_IFREG | S_IRUGO,
+ debugfs_create_file("pinctrl-handles", 0444,
debugfs_root, NULL, &pinctrl_fops);
}

diff --git a/drivers/pinctrl/pinconf.c b/drivers/pinctrl/pinconf.c
index 02c075cc010b..d9d54065472e 100644
--- a/drivers/pinctrl/pinconf.c
+++ b/drivers/pinctrl/pinconf.c
@@ -370,9 +370,9 @@ DEFINE_SHOW_ATTRIBUTE(pinconf_groups);
void pinconf_init_device_debugfs(struct dentry *devroot,
struct pinctrl_dev *pctldev)
{
- debugfs_create_file("pinconf-pins", S_IFREG | S_IRUGO,
+ debugfs_create_file("pinconf-pins", 0444,
devroot, pctldev, &pinconf_pins_fops);
- debugfs_create_file("pinconf-groups", S_IFREG | S_IRUGO,
+ debugfs_create_file("pinconf-groups", 0444,
devroot, pctldev, &pinconf_groups_fops);
}

diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
index bab888fe3f8e..c651b2db0925 100644
--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c
@@ -676,9 +676,9 @@ DEFINE_SHOW_ATTRIBUTE(pinmux_pins);
void pinmux_init_device_debugfs(struct dentry *devroot,
struct pinctrl_dev *pctldev)
{
- debugfs_create_file("pinmux-functions", S_IFREG | S_IRUGO,
+ debugfs_create_file("pinmux-functions", 0444,
devroot, pctldev, &pinmux_functions_fops);
- debugfs_create_file("pinmux-pins", S_IFREG | S_IRUGO,
+ debugfs_create_file("pinmux-pins", 0444,
devroot, pctldev, &pinmux_pins_fops);
}

--
2.25.1

2021-02-10 21:36:49

by Drew Fustini

[permalink] [raw]
Subject: [PATCH v3 2/2] pinctrl: pinmux: Add pinmux-select debugfs file

Add "pinmux-select" to debugfs which will activate a function and group
when 2 integers "<function-selector> <group-selector>" are written to
the file. The write operation pinmux_select() handles this by checking
if fsel and gsel are valid selectors and then calling ops->set_mux().

The existing "pinmux-functions" debugfs file lists the pin functions
registered for the pin controller. For example:

function: pinmux-uart0, groups = [ pinmux-uart0-pins ]
function: pinmux-mmc0, groups = [ pinmux-mmc0-pins ]
function: pinmux-mmc1, groups = [ pinmux-mmc1-pins ]
function: pinmux-i2c0, groups = [ pinmux-i2c0-pins ]
function: pinmux-i2c1, groups = [ pinmux-i2c1-pins ]
function: pinmux-spi1, groups = [ pinmux-spi1-pins ]

To activate function pinmux-i2c1 and group pinmux-i2c1-pins:

echo pinmux-i2c1 pinmux-i2c1-pins > mux-select

Signed-off-by: Drew Fustini <[email protected]>
---
drivers/pinctrl/pinmux.c | 107 +++++++++++++++++++++++++++++++++++++++
1 file changed, 107 insertions(+)

diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
index c651b2db0925..23fa32f0a067 100644
--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c
@@ -673,6 +673,111 @@ void pinmux_show_setting(struct seq_file *s,
DEFINE_SHOW_ATTRIBUTE(pinmux_functions);
DEFINE_SHOW_ATTRIBUTE(pinmux_pins);

+#define PINMUX_MAX_NAME 64
+static ssize_t pinmux_select(struct file *file, const char __user *user_buf,
+ size_t len, loff_t *ppos)
+{
+ struct seq_file *sfile = file->private_data;
+ struct pinctrl_dev *pctldev = sfile->private;
+ const struct pinmux_ops *pmxops = pctldev->desc->pmxops;
+ const char *const *groups;
+ char *buf, *fname, *gname;
+ unsigned int num_groups;
+ int fsel, gsel, ret;
+
+ if (len > (PINMUX_MAX_NAME * 2)) {
+ dev_err(pctldev->dev, "write too big for buffer");
+ return -EINVAL;
+ }
+
+ buf = kzalloc(PINMUX_MAX_NAME * 2, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ fname = kzalloc(PINMUX_MAX_NAME, GFP_KERNEL);
+ if (!fname) {
+ ret = -ENOMEM;
+ goto free_buf;
+ }
+
+ gname = kzalloc(PINMUX_MAX_NAME, GFP_KERNEL);
+ if (!buf) {
+ ret = -ENOMEM;
+ goto free_fname;
+ }
+
+ ret = strncpy_from_user(buf, user_buf, PINMUX_MAX_NAME * 2);
+ if (ret < 0) {
+ dev_err(pctldev->dev, "failed to copy buffer from userspace");
+ goto free_gname;
+ }
+ buf[len-1] = '\0';
+
+ ret = sscanf(buf, "%s %s", fname, gname);
+ if (ret != 2) {
+ dev_err(pctldev->dev, "expected format: <function-name> <group-name>");
+ goto free_gname;
+ }
+
+ fsel = pinmux_func_name_to_selector(pctldev, fname);
+ if (fsel < 0) {
+ dev_err(pctldev->dev, "invalid function %s in map table\n", fname);
+ ret = -EINVAL;
+ goto free_gname;
+ }
+
+ ret = pmxops->get_function_groups(pctldev, fsel, &groups, &num_groups);
+ if (ret) {
+ dev_err(pctldev->dev, "no groups for function %d (%s)", fsel, fname);
+ goto free_gname;
+
+ }
+
+ ret = match_string(groups, num_groups, gname);
+ if (ret < 0) {
+ dev_err(pctldev->dev, "invalid group %s", gname);
+ goto free_gname;
+ }
+
+ ret = pinctrl_get_group_selector(pctldev, gname);
+ if (ret < 0) {
+ dev_err(pctldev->dev, "failed to get group selectorL %s", gname);
+ goto free_gname;
+ }
+ gsel = ret;
+
+ ret = pmxops->set_mux(pctldev, fsel, gsel);
+ if (ret) {
+ dev_err(pctldev->dev, "set_mux() failed: %d", ret);
+ goto free_gname;
+ }
+
+ return len;
+
+free_gname:
+ devm_kfree(pctldev->dev, gname);
+free_fname:
+ devm_kfree(pctldev->dev, fname);
+free_buf:
+ devm_kfree(pctldev->dev, buf);
+
+ return ret;
+}
+
+static int pinmux_select_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, NULL, inode->i_private);
+}
+
+static const struct file_operations pinmux_select_ops = {
+ .owner = THIS_MODULE,
+ .open = pinmux_select_open,
+ .read = seq_read,
+ .write = pinmux_select,
+ .llseek = no_llseek,
+ .release = single_release,
+};
+
void pinmux_init_device_debugfs(struct dentry *devroot,
struct pinctrl_dev *pctldev)
{
@@ -680,6 +785,8 @@ void pinmux_init_device_debugfs(struct dentry *devroot,
devroot, pctldev, &pinmux_functions_fops);
debugfs_create_file("pinmux-pins", 0444,
devroot, pctldev, &pinmux_pins_fops);
+ debugfs_create_file("pinmux-select", 0200,
+ devroot, pctldev, &pinmux_select_ops);
}

#endif /* CONFIG_DEBUG_FS */
--
2.25.1