Hi,

Il 17/05/2021 22:02 Andy Shevchenko [off-list ref] ha scritto:

 
On Sun, May 16, 2021 at 7:43 PM Dario Binacchi [off-list ref] wrote:

quoted

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.

quoted

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?

...

quoted

+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() ?

Ok, I'll use strncpy_from_user() in the next version of the patch

quoted

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

quoted

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

Don't shadow an error code.

You are right

quoted

+       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?

I want to make sure the pin is managed

Thanks and regards,
Dario

quoted

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

...

quoted

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

Why is it in this file?



-- 
With Best Regards,
Andy Shevchenko