diff mbox series

[1/2] pinctrl: core: configure pinmux from pins debug file

Message ID 20210516135531.2203-2-dariobin@libero.it
State Superseded
Headers show
Series am335x: set pinmux registers from pins debug file | expand

Commit Message

Dario Binacchi May 16, 2021, 1:55 p.m. UTC 
The MPUs of some architectures (e.g AM335x) must be in privileged
operating mode to write on the pinmux registers. In such cases, where
writes will not work from user space, now it can be done from the pins
debug file if the platform driver exports the pin_dbg_set() helper among
the registered operations.

Signed-off-by: Dario Binacchi <dariobin@libero.it>
---

 drivers/pinctrl/core.c          | 56 +++++++++++++++++++++++++++++++--
 include/linux/pinctrl/pinctrl.h |  2 ++
 2 files changed, 56 insertions(+), 2 deletions(-)

Comments

Andy Shevchenko May 17, 2021, 8:02 p.m. UTC | #1 
On Sun, May 16, 2021 at 7:43 PM Dario Binacchi <dariobin@libero.it> wrote:
>
> The MPUs of some architectures (e.g AM335x) must be in privileged
> operating mode to write on the pinmux

pinmux is not pin configuration. You need to rethink the approach.

> registers. In such cases, where
> writes will not work from user space, now it can be done from the pins
> debug file if the platform driver exports the pin_dbg_set() helper among
> the registered operations.

Drew, is it similar to what you are trying to achieve?

...

> +static ssize_t pinctrl_pins_write(struct file *file,
> +                                 const char __user *user_buf, size_t count,
> +                                 loff_t *ppos)
> +{
> +       struct seq_file *s = file->private_data;
> +       struct pinctrl_dev *pctldev = s->private;
> +       const struct pinctrl_ops *ops = pctldev->desc->pctlops;
> +       char buf[32];
> +       char *c = &buf[0];
> +       char *token;
> +       int ret, buf_size;
> +       unsigned int i, pin;
> +
> +       if (!ops->pin_dbg_set)
> +               return -EFAULT;
> +
> +       /* Get userspace string and assure termination */
> +       buf_size = min(count, sizeof(buf) - 1);
> +       if (copy_from_user(buf, user_buf, buf_size))
> +               return -EFAULT;
> +
> +       buf[buf_size] = 0;

Can't you use strncpy_from_user() ?


> +       token = strsep(&c, " ");

> +       if (kstrtouint(token, 0, &pin))
> +               return -EINVAL;

Don't shadow an error code.

> +       for (i = 0; i < pctldev->desc->npins; i++) {
> +               if (pin != pctldev->desc->pins[i].number)
> +                       continue;

Hmm... I don't get this. Why is it needed?

> +               ret = ops->pin_dbg_set(pctldev, pin, c);
> +               if (ret)
> +                       return ret;
> +
> +               return count;
> +       }
> +
> +       return -EINVAL;
> +}

...

> -       debugfs_create_file("pins", 0444,
> +       debugfs_create_file("pins", 0644,
>                             device_root, pctldev, &pinctrl_pins_fops);

Why is it in this file?
diff mbox series

Patch

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index a4ac87c8b4f8..f5c9a7d44039 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -1620,6 +1620,46 @@  EXPORT_SYMBOL_GPL(pinctrl_pm_select_idle_state);
 
 #ifdef CONFIG_DEBUG_FS
 
+static ssize_t pinctrl_pins_write(struct file *file,
+				  const char __user *user_buf, size_t count,
+				  loff_t *ppos)
+{
+	struct seq_file	*s = file->private_data;
+	struct pinctrl_dev *pctldev = s->private;
+	const struct pinctrl_ops *ops = pctldev->desc->pctlops;
+	char buf[32];
+	char *c = &buf[0];
+	char *token;
+	int ret, buf_size;
+	unsigned int i, pin;
+
+	if (!ops->pin_dbg_set)
+		return -EFAULT;
+
+	/* Get userspace string and assure termination */
+	buf_size = min(count, sizeof(buf) - 1);
+	if (copy_from_user(buf, user_buf, buf_size))
+		return -EFAULT;
+
+	buf[buf_size] = 0;
+	token = strsep(&c, " ");
+	if (kstrtouint(token, 0, &pin))
+		return -EINVAL;
+
+	for (i = 0; i < pctldev->desc->npins; i++) {
+		if (pin != pctldev->desc->pins[i].number)
+			continue;
+
+		ret = ops->pin_dbg_set(pctldev, pin, c);
+		if (ret)
+			return ret;
+
+		return count;
+	}
+
+	return -EINVAL;
+}
+
 static int pinctrl_pins_show(struct seq_file *s, void *what)
 {
 	struct pinctrl_dev *pctldev = s->private;
@@ -1677,7 +1717,11 @@  static int pinctrl_pins_show(struct seq_file *s, void *what)
 
 	return 0;
 }
-DEFINE_SHOW_ATTRIBUTE(pinctrl_pins);
+
+static int pinctrl_pins_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, pinctrl_pins_show, inode->i_private);
+}
 
 static int pinctrl_groups_show(struct seq_file *s, void *what)
 {
@@ -1886,6 +1930,14 @@  static int pinctrl_show(struct seq_file *s, void *what)
 }
 DEFINE_SHOW_ATTRIBUTE(pinctrl);
 
+static const struct file_operations pinctrl_pins_fops = {
+	.open		= pinctrl_pins_open,
+	.read		= seq_read,
+	.write		= pinctrl_pins_write,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
 static struct dentry *debugfs_root;
 
 static void pinctrl_init_device_debugfs(struct pinctrl_dev *pctldev)
@@ -1915,7 +1967,7 @@  static void pinctrl_init_device_debugfs(struct pinctrl_dev *pctldev)
 			dev_name(pctldev->dev));
 		return;
 	}
-	debugfs_create_file("pins", 0444,
+	debugfs_create_file("pins", 0644,
 			    device_root, pctldev, &pinctrl_pins_fops);
 	debugfs_create_file("pingroups", 0444,
 			    device_root, pctldev, &pinctrl_groups_fops);
diff --git a/include/linux/pinctrl/pinctrl.h b/include/linux/pinctrl/pinctrl.h
index 70b45d28e7a9..6db4a775f549 100644
--- a/include/linux/pinctrl/pinctrl.h
+++ b/include/linux/pinctrl/pinctrl.h
@@ -95,6 +95,8 @@  struct pinctrl_ops {
 			       unsigned *num_pins);
 	void (*pin_dbg_show) (struct pinctrl_dev *pctldev, struct seq_file *s,
 			  unsigned offset);
+	int (*pin_dbg_set) (struct pinctrl_dev *pctldev, unsigned int offset,
+			    char *buf);
 	int (*dt_node_to_map) (struct pinctrl_dev *pctldev,
 			       struct device_node *np_config,
 			       struct pinctrl_map **map, unsigned *num_maps);