On Sat, 17 Jun 2006 12:42:28 -0600
Jim Cromie <[email protected]> wrote:
> Ok,
>
> heres the brand-spanking-new proto-sysfs-gpio interface,
> preceded by some pseudo/proto-Documentation.
>
Well I stuck it in -mm anyway. Let's see what happens.
We don't have a Signed-off-by: for this patch.
Fixup patches agains next -mm would be suitable. Please keep them
super-short: basically one-patch-per-review-comment. That way I can easily
instertion-sort the patches into place and we retain a nice patch series.
> Ive
Apostrophe aversion?
>
> +/* the pin-mode-change 'commands' of the legacy device-file-interface,
> + now refactored for reuse in a sysfs-interface. Includes some
> + updates which arent exposed via sysfs attributes.
> +*/
> +static int common_write(struct nsc_gpio_ops *amp, char c, unsigned m)
> +{
> + struct device *dev = amp->dev;
> + int err = 0;
> + switch (c) {
> + case '0':
> + amp->gpio_set(m, 0);
> + break;
> + case '1':
> + amp->gpio_set(m, 1);
> + break;
> + case 'O':
> + dev_dbg(dev, "GPIO%d output enabled\n", m);
> + amp->gpio_config(m, ~1, 1);
> + break;
> + case 'o':
> + dev_dbg(dev, "GPIO%d output disabled\n", m);
> + amp->gpio_config(m, ~1, 0);
> + break;
> + case 'T':
> + dev_dbg(dev, "GPIO%d output is push pull\n", m);
> + amp->gpio_config(m, ~2, 2);
> + break;
> + case 't':
> + dev_dbg(dev, "GPIO%d output is open drain\n", m);
> + amp->gpio_config(m, ~2, 0);
> + break;
> + case 'P':
> + dev_dbg(dev, "GPIO%d pull up enabled\n", m);
> + amp->gpio_config(m, ~4, 4);
> + break;
> + case 'p':
> + dev_dbg(dev, "GPIO%d pull up disabled\n", m);
> + amp->gpio_config(m, ~4, 0);
> + break;
> + case 'L':
> + dev_dbg(dev, "GPIO%d lock pin\n", m);
> + amp->gpio_config(m, ~8, 8);
> + break;
> + case 'l':
> + dev_warn(dev, "GPIO%d cant unlock locked? pin\n", m);
> + amp->gpio_config(m, ~8, 0);
> + break;
> +
> + case 'D':
> + dev_dbg(dev, "GPIO%d turn on debounce\n", m);
> + amp->gpio_config(m, ~PF_DEBOUNCE, PF_DEBOUNCE);
> + break;
> + case 'd':
> + dev_warn(dev, "GPIO%d cant unlock locked? pin\n", m);
> + amp->gpio_config(m, ~PF_DEBOUNCE, 0);
> + break;
> +
> + case 'v':
> + /* View Current pin settings */
> + amp->gpio_dump(amp, m);
> + break;
> + case 'c':
> + /* view pin's current values: driven and read */
> + dev_info(dev, "io%02d: driven %d, input %d\n",
> + m, amp->gpio_current(m), amp->gpio_get(m));
> + break;
> + case '\n':
> + /* end of settings string, do nothing */
> + break;
> + default:
> + dev_err(dev, "GPIO-%2d bad setting: chr<0x%2x>\n", m,
> + (int)c);
> + err++;
> + }
> + return err;
> +}
> +
> ssize_t nsc_gpio_write(struct file *file, const char __user *data,
> size_t len, loff_t *ppos)
> {
> + int i, err = 0;
> unsigned m = iminor(file->f_dentry->d_inode);
> struct nsc_gpio_ops *amp = file->private_data;
> - struct device *dev = amp->dev;
> - size_t i;
> - int err = 0;
>
> for (i = 0; i < len; ++i) {
> char c;
> if (get_user(c, data + i))
> return -EFAULT;
> - switch (c) {
> - case '0':
> - amp->gpio_set(m, 0);
> - break;
> - case '1':
> - amp->gpio_set(m, 1);
> - break;
> - case 'O':
> - dev_dbg(dev, "GPIO%d output enabled\n", m);
> - amp->gpio_config(m, ~1, 1);
> - break;
> - case 'o':
> - dev_dbg(dev, "GPIO%d output disabled\n", m);
> - amp->gpio_config(m, ~1, 0);
> - break;
> - case 'T':
> - dev_dbg(dev, "GPIO%d output is push pull\n", m);
> - amp->gpio_config(m, ~2, 2);
> - break;
> - case 't':
> - dev_dbg(dev, "GPIO%d output is open drain\n", m);
> - amp->gpio_config(m, ~2, 0);
> - break;
> - case 'P':
> - dev_dbg(dev, "GPIO%d pull up enabled\n", m);
> - amp->gpio_config(m, ~4, 4);
> - break;
> - case 'p':
> - dev_dbg(dev, "GPIO%d pull up disabled\n", m);
> - amp->gpio_config(m, ~4, 0);
> - break;
> -
> - case 'v':
> - /* View Current pin settings */
> - amp->gpio_dump(amp, m);
> - break;
> - case '\n':
> - /* end of settings string, do nothing */
> - break;
> - default:
> - dev_err(dev, "GPIO-%2d bad setting: chr<0x%2x>\n", m,
> - (int)c);
> - err++;
> - }
> +
> + err += common_write(amp, c, m);
> }
> if (err)
> return -EINVAL; /* full string handled, report error */
This all spat a whopping great reject for reasons unknown, which I fixed by
hand. Please check that it all still works.
> +ssize_t nsc_gpio_sysfs_set(struct device *dev,
> + struct device_attribute *devattr, const char *buf,
> + size_t count)
> +{
> + struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
> + int idx = attr->index;
> + int func = attr->nr;
> + struct nsc_gpio_ops *amp = dev->driver_data;
> + int err, xor;
> +
> + /* invert cmd if setting low */
> + xor = simple_strtol(buf, NULL, 10) ? 0 : 'T'^'t';
whoa. How does this work?
> + dev_info(dev, "set func:%d Func:%d, flp%d\n", func, func^xor, xor);
> +
> + err = common_write(amp, func^xor, idx);
The tricksies in here aren't very understandable. Can it be simplified?
> + if (err)
> + return -EINVAL; // full string handled, report error
> +
> + return strlen(buf);
> +}
>
> ...
>
> static void __init pc8736x_init_shadow(void)
> {
> int port;
> @@ -320,6 +368,12 @@ static int __init pc8736x_gpio_init(void
> dev_dbg(&pdev->dev, ": got dynamic major %d\n", major);
> }
>
> + pc8736x_sysfs_init(&pdev->dev);
> +
> + /* provide info wheresysfs callbacks can get them */
> + pc8736x_access.dev->driver_data = &pc8736x_access;
> +
> +;
> return 0;
> }
stray semicolon.
> +/* GPIO pins have 5 attributes we care about: group them */
> +struct gpio_pin_attributes {
> + struct sensor_device_attribute_2
> + output_enabled,
> + totem_pole,
> + pullup_enabled,
> + debounced,
> + locked;
> +};
A bit hard to read. Normally we'd do
struct gpio_pin_attributes {
struct sensor_device_attribute_2 output_enabled;
struct sensor_device_attribute_2 totem_pole;
struct sensor_device_attribute_2 pullup_enabled;
struct sensor_device_attribute_2 debounced;
struct sensor_device_attribute_2 locked;
};
because kernel-style is super-simple-style.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
