AR7: 4th effort

Signed-off-by Matteo Croce <[email protected]>

Signed-off-by Felix Fietkau <[email protected]>

Signed-off-by Eugene Konev <[email protected]>

drivers/mtd/Kconfig +6 -0

index fbec8cd..c1b2508 100644
@@ -150,6 +150,12 @@ config MTD_AFS_PARTS
 	  for your particular device. It won't happen automatically. The
 	  'armflash' map driver (CONFIG_MTD_ARMFLASH) does this, for example.
 
+config MTD_AR7_PARTS
+	tristate "TI AR7 partitioning support"
+	depends on MTD_PARTITIONS
+	---help---
+	  TI AR7 partitioning support
+
 comment "User Modules And Translation Layers"
 
 config MTD_CHAR

drivers/mtd/Makefile +1 -0

index 6d958a4..8451c64 100644
@@ -11,6 +11,7 @@ obj-$(CONFIG_MTD_CONCAT)	+= mtdconcat.o
 obj-$(CONFIG_MTD_REDBOOT_PARTS) += redboot.o
 obj-$(CONFIG_MTD_CMDLINE_PARTS) += cmdlinepart.o
 obj-$(CONFIG_MTD_AFS_PARTS)	+= afs.o
+obj-$(CONFIG_MTD_AR7_PARTS)	+= ar7part.o
 
 # 'Users' - code which presents functionality to userspace.
 obj-$(CONFIG_MTD_CHAR)		+= mtdchar.o

drivers/mtd/ar7part.c new file +19 -0

new file mode 100644
index 0000000..775041d
@@ -0,0 +1,142 @@
+/*
+ * Copyright (C) 2007 Eugene Konev <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ * TI AR7 flash partition table.
+ * Based on ar7 map by Felix Fietkau <[email protected]>

drivers/mtd/ar7part.c new file +25 -0

+struct ar7_bin_rec {
+	unsigned int checksum;
+	unsigned int length;
+	unsigned int address;
+};
+
+static struct mtd_partition ar7_parts[5];
+
+static int create_mtd_partitions(struct mtd_info *master,
+				 struct mtd_partition **pparts,
+				 unsigned long origin)
+{
+	struct ar7_bin_rec header;
+	unsigned int offset, len;
+	unsigned int pre_size = master->erasesize, post_size = 0;
+	unsigned int root_offset = 0xe0000;
+
+	int retries = 10;
+
+	printk(KERN_INFO "Parsing AR7 partition map...\n");
+
+	ar7_parts[0].name = "loader";
+	ar7_parts[0].offset = 0;
+	ar7_parts[0].size = master->erasesize;
+	ar7_parts[0].mask_flags = MTD_WRITEABLE;

drivers/mtd/ar7part.c new file +67 -0

+		if (!strncmp((char *)&header, "TIENV0.8", 8))
+			ar7_parts[1].offset = pre_size;
+		if (header.checksum == 0xfeedfa42)
+			break;
+		if (header.checksum == 0xfeed1281)
+			break;
+		pre_size += master->erasesize;
+	} while (retries--);
+
+	pre_size = offset;
+
+	if (!ar7_parts[1].offset) {
+		ar7_parts[1].offset = master->size - master->erasesize;
+		post_size = master->erasesize;
+	}
+
+	switch (header.checksum) {
+	case 0xfeedfa42:
+		while (header.length) {
+			offset += sizeof(header) + header.length;
+			master->read(master, offset, sizeof(header),
+				     &len, (u_char *)&header);
+		}
+		root_offset = offset + sizeof(header) + 4;
+		break;
+	case 0xfeed1281:
+		while (header.length) {
+			offset += sizeof(header) + header.length;
+			master->read(master, offset, sizeof(header),
+				     &len, (u_char *)&header);
+		}
+		root_offset = offset + sizeof(header) + 4 + 0xff;
+		root_offset &= ~(u32)0xff;
+		break;
+	default:
+		printk(KERN_WARNING "Unknown magic: %08x\n", header.checksum);
+		break;
+	}
+
+	master->read(master, root_offset,
+		sizeof(header), &len, (u_char *)&header);
+	if (header.checksum != SQUASHFS_MAGIC) {
+		root_offset += master->erasesize - 1;
+		root_offset &= ~(master->erasesize - 1);
+	}
+
+	ar7_parts[2].name = "linux";
+	ar7_parts[2].offset = pre_size;
+	ar7_parts[2].size = master->size - pre_size - post_size;
+	ar7_parts[2].mask_flags = 0;
+
+	ar7_parts[3].name = "rootfs";
+	ar7_parts[3].offset = root_offset;
+	ar7_parts[3].size = master->size - root_offset - post_size;
+	ar7_parts[3].mask_flags = 0;
+
+	*pparts = ar7_parts;
+	return 4;
+}
+
+static struct mtd_part_parser ar7_parser = {
+	.owner = THIS_MODULE,
+	.parse_fn = create_mtd_partitions,
+	.name = "ar7part",
+};
+
+static int __init ar7_parser_init(void)

+#include <linux/squashfs_fs.h>

+static struct mtd_partition ar7_parts[5];

Quoted 1 line

+#include <linux/squashfs_fs.h>

As Ralf mentioned this is not in mainline, so you can't include it. Fortunately for you including it seems to be a rather dumb idea anyway, see below for further comments.

Quoted 1 line

+static struct mtd_partition ar7_parts[5];

Can we please get a symbolic name for that 5? Then again we only seems to use 3 anyway, and create_mtd_partitions returns 4..

drivers/mtd/ar7part.c line 46

Quoted 1 line

drivers/mtd/ar7part.c new file +1 -0

+	unsigned int root_offset = 0xe0000;

Please provide a symbolic name for this one.

drivers/mtd/ar7part.c line 50

Quoted 1 line

drivers/mtd/ar7part.c new file +1 -0

+	printk(KERN_INFO "Parsing AR7 partition map...\n");

Do we really need this printk?

drivers/mtd/ar7part.c line 71

Quoted 4 lines

drivers/mtd/ar7part.c new file +4 -0

+		if (header.checksum == 0xfeedfa42)
+			break;
+		if (header.checksum == 0xfeed1281)
+			break;

These two want symbolic names, please:

enum {
	/* some comment */
	FOO_PART_MAGIC = 0xfeedfa42,	

	/* more comment */
	BAR_PART_MAGIC = 0xfeed1281,
};

drivers/mtd/ar7part.c line 91

Quoted 10 lines

drivers/mtd/ar7part.c new file +10 -0

+	switch (header.checksum) {
+	case 0xfeedfa42:
+		while (header.length) {
+			offset += sizeof(header) + header.length;
+			master->read(master, offset, sizeof(header),
+				     &len, (u_char *)&header);
+		}
+		root_offset = offset + sizeof(header) + 4;
+		break;
+	case 0xfeed1281:

Especially as you use them here again.

drivers/mtd/ar7part.c line 110

Quoted 11 lines

drivers/mtd/ar7part.c new file +11 -0

+	default:
+		printk(KERN_WARNING "Unknown magic: %08x\n", header.checksum);
+		break;
+	}
+
+	master->read(master, root_offset,
+		sizeof(header), &len, (u_char *)&header);
+	if (header.checksum != SQUASHFS_MAGIC) {
+		root_offset += master->erasesize - 1;
+		root_offset &= ~(master->erasesize - 1);
+	}

And after this I'm pretty sure either of the two hex constants above is SQUASHFS_MAGIC. But not actually the magic in the squashfs superblock, but someone decided to reuse it for the partitions magic. So one of the comments for *_PART_MAGIC becomes:

	/* same as SQUASHFS_MAGIC for some odd reason.. */

drivers/mtd/ar7part.c line 40

Quoted 7 lines · Matteo Croce

drivers/mtd/ar7part.c new file +7 -0

+#ifdef CONFIG_CPU_LITTLE_ENDIAN
+#define LOADER_MAGIC1	0xfeedfa42
+#define LOADER_MAGIC2	0xfeed1281
+#else
+#define LOADER_MAGIC1	0x42faedfe
+#define LOADER_MAGIC2	0x8112edfe
+#endif

Please keep only one defintion and use le/be32_to_cpu on it.

drivers/mtd/ar7part.c line 46

Quoted 5 lines

drivers/mtd/ar7part.c new file +5 -0

+struct ar7_bin_rec {
+	unsigned int checksum;
+	unsigned int length;
+	unsigned int address;
+};

Wich means you'd need an endianess annotation here. What about the length and address fields, are they always native-endian unlike the checksum field or will the need to be byte-swapped aswell?

Support for the leds in front of the board usually used to show power status, network traffic, connected eth devices etc.
Will convert it to use leds-gpio when 2.6.23 will out.

Quoted 11 lines · Matteo Croce

drivers/mtd/ar7part.c line 40

Quoted 7 lines

drivers/mtd/ar7part.c new file +7 -0

+#ifdef CONFIG_CPU_LITTLE_ENDIAN
+#define LOADER_MAGIC1	0xfeedfa42
+#define LOADER_MAGIC2	0xfeed1281
+#else
+#define LOADER_MAGIC1	0x42faedfe
+#define LOADER_MAGIC2	0x8112edfe
+#endif

Please keep only one defintion and use le/be32_to_cpu on it.

Quoted 9 lines

drivers/mtd/ar7part.c line 46

Quoted 5 lines

drivers/mtd/ar7part.c new file +5 -0

+struct ar7_bin_rec {
+	unsigned int checksum;
+	unsigned int length;
+	unsigned int address;
+};

Wich means you'd need an endianess annotation here. What about the length and address fields, are they always native-endian unlike the checksum field or will the need to be byte-swapped aswell?

<slightly off-topic, feel free to skip>
If this is indeed the squashfs magic, le/be32_to_cpu won't help. Squashfs can have either endianness, tries to detect the one actually used by checking either magic and sets a flag in the superblock. Afterwards every single access checks the flag and conditionally swaps fields around or not.

If squashfs had a fixed endianness, quite a lot of this logic could get removed and both source and object size would shrink. Some two years after requesting this for the first time, I'm thinking about just doing it myself. If I find a sponsor who pays me for it, I might even do it soon.
</offtopic>

I don't really understand why the squashfs magic number should be used in this code at all. It may have set a bad example, though. In general you should decide on a fixed endianness (1) and use the beXX_to_cpu macros when accessing data unless you have a very good reason to do otherwise.

1) Big endian is my preferred choice because it is easy to read in a hexdump and the opposite of my notebook. Being forced to do endian conversions during development/testing helps to find problems early.

I use little endian since 99% of AR7s are little endian. Dunno if le/be32_to_cpu does some runtime calculations. Do they?

I use little endian since 99% of AR7s are little endian. Dunno if le/be32_to_cpu does some runtime calculations. Do they?

Char device to access GPIO pins

Signed-off-by Matteo Croce <[email protected]>

Signed-off-by Nicolas Thill <[email protected]>

drivers/char/Kconfig +9 -0

index b391776..b98aedf 100644
@@ -928,6 +928,15 @@ config MWAVE
 	  To compile this driver as a module, choose M here: the
 	  module will be called mwave.
 
+config AR7_GPIO
+	tristate "TI AR7 GPIO Support"
+	depends on AR7
+	help
+	  Give userspace access to the GPIO pins on the Texas Instruments AR7 
+	  processors.
+
+	  If compiled as a module, it will be called ar7_gpio.
+
 config SCx200_GPIO
 	tristate "NatSemi SCx200 GPIO Support"
 	depends on SCx200

drivers/char/Makefile +1 -0

index d68ddbe..804319e 100644
@@ -89,6 +89,7 @@ obj-$(CONFIG_COBALT_LCD)	+= lcd.o
 obj-$(CONFIG_PPDEV)		+= ppdev.o
 obj-$(CONFIG_NWBUTTON)		+= nwbutton.o
 obj-$(CONFIG_NWFLASH)		+= nwflash.o
+obj-$(CONFIG_AR7_GPIO)		+= ar7_gpio.o
 obj-$(CONFIG_SCx200_GPIO)	+= scx200_gpio.o
 obj-$(CONFIG_PC8736x_GPIO)	+= pc8736x_gpio.o
 obj-$(CONFIG_NSC_GPIO)		+= nsc_gpio.o

drivers/char/ar7_gpio.c new file +20 -0

new file mode 100644
index 0000000..d57a23e
@@ -0,0 +1,158 @@
+/*
+ * Copyright (C) 2007 Nicolas Thill <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#include <linux/device.h>
+#include <linux/fs.h>

drivers/char/ar7_gpio.c new file +118 -0

+
+#define DRVNAME "ar7_gpio"
+#define LONGNAME "TI AR7 GPIOs Driver"
+
+MODULE_AUTHOR("Nicolas Thill <[email protected]>");
+MODULE_DESCRIPTION(LONGNAME);
+MODULE_LICENSE("GPL");
+
+static int ar7_gpio_major;
+
+static ssize_t ar7_gpio_write(struct file *file, const char __user *buf,
+	size_t len, loff_t *ppos)
+{
+	int pin = iminor(file->f_dentry->d_inode);
+	size_t i;
+
+	for (i = 0; i < len; ++i) {
+		char c;
+		if (get_user(c, buf + i))
+			return -EFAULT;
+		switch (c) {
+		case '0':
+			gpio_set_value(pin, 0);
+			break;
+		case '1':
+			gpio_set_value(pin, 1);
+			break;
+		case 'd':
+		case 'D':
+			ar7_gpio_disable(pin);
+			break;
+		case 'e':
+		case 'E':
+			ar7_gpio_enable(pin);
+			break;
+		case 'i':
+		case 'I':
+		case '<':
+			gpio_direction_input(pin);
+			break;
+		case 'o':
+		case 'O':
+		case '>':
+			gpio_direction_output(pin);
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	return len;
+}
+
+static ssize_t ar7_gpio_read(struct file *file, char __user *buf,
+	size_t len, loff_t *ppos)
+{
+	int pin = iminor(file->f_dentry->d_inode);
+	int value;
+
+	value = gpio_get_value(pin);
+	if (put_user(value ? '1' : '0', buf))
+		return -EFAULT;
+
+	return 1;
+}
+
+static int ar7_gpio_open(struct inode *inode, struct file *file)
+{
+	int m = iminor(inode);
+
+	if (m >= AR7_GPIO_MAX)
+		return -EINVAL;
+
+	return nonseekable_open(inode, file);
+}
+
+static int ar7_gpio_release(struct inode *inode, struct file *file)
+{
+	return 0;
+}
+
+static const struct file_operations ar7_gpio_fops = {
+	.owner   = THIS_MODULE,
+	.write   = ar7_gpio_write,
+	.read    = ar7_gpio_read,
+	.open    = ar7_gpio_open,
+	.release = ar7_gpio_release,
+	.llseek  = no_llseek,
+};
+
+static struct platform_device *ar7_gpio_device;
+
+static int __init ar7_gpio_init(void)
+{
+	int rc;
+
+	ar7_gpio_device = platform_device_alloc(DRVNAME, -1);
+	if (!ar7_gpio_device)
+		return -ENOMEM;
+
+	rc = platform_device_add(ar7_gpio_device);
+	if (rc < 0)
+		goto out_put;
+
+	rc = register_chrdev(ar7_gpio_major, DRVNAME, &ar7_gpio_fops);
+	if (rc < 0)
+		goto out_put;
+
+	ar7_gpio_major = rc;
+
+	rc = 0;
+
+	goto out;
+
+out_put:
+	platform_device_put(ar7_gpio_device);
+out:
+	return rc;

Quoted 4 lines · Matteo Croce

Char device to access GPIO pins

Signed-off-by Matteo Croce <[email protected]>

Signed-off-by Nicolas Thill <[email protected]>

This driver is almost platform independent. The only platform-specific part is its name and AR7_GPIO_MAX. It would be great if this driver was really "generic" and could be used with any GPIO API providers.

I think there were some discussions about userspace API for GPIO on LKML, but cannot remember the detail.

David, give us a comment please?

drivers/char/Kconfig config MWAVE · line 933

Quoted 11 lines

drivers/char/Kconfig +3 -0

index b391776..b98aedf 100644
@@ -928,6 +928,15 @@ config MWAVE
 	  To compile this driver as a module, choose M here: the
 	  module will be called mwave.
 
+config AR7_GPIO
+	tristate "TI AR7 GPIO Support"
+	depends on AR7

Quoted 51 lines

drivers/char/Kconfig +6 -0

+	help
+	  Give userspace access to the GPIO pins on the Texas Instruments AR7 
+	  processors.
+
+	  If compiled as a module, it will be called ar7_gpio.
+
 config SCx200_GPIO
 	tristate "NatSemi SCx200 GPIO Support"
 	depends on SCx200

drivers/char/Makefile +1 -0

index d68ddbe..804319e 100644
@@ -89,6 +89,7 @@ obj-$(CONFIG_COBALT_LCD)	+= lcd.o
 obj-$(CONFIG_PPDEV)		+= ppdev.o
 obj-$(CONFIG_NWBUTTON)		+= nwbutton.o
 obj-$(CONFIG_NWFLASH)		+= nwflash.o
+obj-$(CONFIG_AR7_GPIO)		+= ar7_gpio.o
 obj-$(CONFIG_SCx200_GPIO)	+= scx200_gpio.o
 obj-$(CONFIG_PC8736x_GPIO)	+= pc8736x_gpio.o
 obj-$(CONFIG_NSC_GPIO)		+= nsc_gpio.o

drivers/char/ar7_gpio.c new file +24 -0

new file mode 100644
index 0000000..d57a23e
@@ -0,0 +1,158 @@
+/*
+ * Copyright (C) 2007 Nicolas Thill <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/module.h>
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/init.h>

drivers/char/ar7_gpio.c line 30

Quoted 6 lines

drivers/char/ar7_gpio.c new file +6 -0

+#include <linux/platform_device.h>
+#include <linux/uaccess.h>
+#include <linux/io.h>
+#include <linux/types.h>
+#include <linux/cdev.h>
+#include <gpio.h>

This should be "#include <asm/gpio.h>".

Quoted 6 lines

drivers/char/ar7_gpio.c new file +6 -0

+
+#define DRVNAME "ar7_gpio"
+#define LONGNAME "TI AR7 GPIOs Driver"
+
+MODULE_AUTHOR("Nicolas Thill <[email protected]>");
+MODULE_DESCRIPTION(LONGNAME);

drivers/char/ar7_gpio.c line 44

Quoted 8 lines

drivers/char/ar7_gpio.c new file +8 -0

+MODULE_LICENSE("GPL");
+
+static int ar7_gpio_major;
+
+static ssize_t ar7_gpio_write(struct file *file, const char __user *buf,
+	size_t len, loff_t *ppos)
+{
+	int pin = iminor(file->f_dentry->d_inode);

Quoted 5 lines

drivers/char/ar7_gpio.c new file +5 -0

+	size_t i;
+
+	for (i = 0; i < len; ++i) {
+		char c;
+		if (get_user(c, buf + i))

drivers/char/ar7_gpio.c line 57

Quoted 8 lines

drivers/char/ar7_gpio.c new file +8 -0

+			return -EFAULT;
+		switch (c) {
+		case '0':
+			gpio_set_value(pin, 0);
+			break;
+		case '1':
+			gpio_set_value(pin, 1);
+			break;

drivers/char/ar7_gpio.c line 65

Quoted 8 lines

drivers/char/ar7_gpio.c new file +8 -0

+		case 'd':
+		case 'D':
+			ar7_gpio_disable(pin);
+			break;
+		case 'e':
+		case 'E':
+			ar7_gpio_enable(pin);
+			break;

drivers/char/ar7_gpio.c line 74

Quoted 10 lines

drivers/char/ar7_gpio.c new file +9 -0

+		case 'i':
+		case 'I':
+		case '<':
+			gpio_direction_input(pin);
 
+			break;
+		case 'o':
+		case 'O':
+		case '>':
+			gpio_direction_output(pin);

Quoted 13 lines

drivers/char/ar7_gpio.c new file +13 -0

+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	return len;
+}
+
+static ssize_t ar7_gpio_read(struct file *file, char __user *buf,
+	size_t len, loff_t *ppos)
+{
+	int pin = iminor(file->f_dentry->d_inode);

drivers/char/ar7_gpio.c line 95

Quoted 8 lines

drivers/char/ar7_gpio.c new file +8 -0

+	int value;
+
+	value = gpio_get_value(pin);
+	if (put_user(value ? '1' : '0', buf))
+		return -EFAULT;
+
+	return 1;
+}

drivers/char/ar7_gpio.c line 102

Quoted 7 lines

drivers/char/ar7_gpio.c new file +7 -0

+
+static int ar7_gpio_open(struct inode *inode, struct file *file)
+{
+	int m = iminor(inode);
+
+	if (m >= AR7_GPIO_MAX)
+		return -EINVAL;

drivers/char/ar7_gpio.c line 109

Quoted 7 lines

drivers/char/ar7_gpio.c new file +7 -0

+
+	return nonseekable_open(inode, file);
+}
+
+static int ar7_gpio_release(struct inode *inode, struct file *file)
+{
+	return 0;

Quoted 12 lines

drivers/char/ar7_gpio.c new file +12 -0

+}
+
+static const struct file_operations ar7_gpio_fops = {
+	.owner   = THIS_MODULE,
+	.write   = ar7_gpio_write,
+	.read    = ar7_gpio_read,
+	.open    = ar7_gpio_open,
+	.release = ar7_gpio_release,
+	.llseek  = no_llseek,
+};
+
+static struct platform_device *ar7_gpio_device;

drivers/char/ar7_gpio.c line 129

Quoted 8 lines

drivers/char/ar7_gpio.c new file +8 -0

+
+static int __init ar7_gpio_init(void)
+{
+	int rc;
+
+	ar7_gpio_device = platform_device_alloc(DRVNAME, -1);
+	if (!ar7_gpio_device)
+		return -ENOMEM;

drivers/char/ar7_gpio.c line 139

Quoted 10 lines

drivers/char/ar7_gpio.c new file +10 -0

+
+	rc = platform_device_add(ar7_gpio_device);
+	if (rc < 0)
+		goto out_put;
+
+	rc = register_chrdev(ar7_gpio_major, DRVNAME, &ar7_gpio_fops);
+	if (rc < 0)
+		goto out_put;
+
+	ar7_gpio_major = rc;

Quoted 9 lines

drivers/char/ar7_gpio.c new file +9 -0

+
+	rc = 0;
+
+	goto out;
+
+out_put:
+	platform_device_put(ar7_gpio_device);
+out:
+	return rc;

drivers/char/ar7_gpio.c line 158

Quoted 10 lines

drivers/char/ar7_gpio.c new file +10 -0

+}
+
+static void __exit ar7_gpio_exit(void)
+{
+	unregister_chrdev(ar7_gpio_major, DRVNAME);
+	platform_device_unregister(ar7_gpio_device);
+}
+
+module_init(ar7_gpio_init);
+module_exit(ar7_gpio_exit);

---
Atsushi Nemoto

Quoted 4 lines

I think there were some discussions about userspace API for GPIO on LKML, but cannot remember the detail.

David, give us a comment please?

It's not yet platform-neutral even given those issues; see below. And it's insufficient by itself, which is the main technical point I'd raise: without even udev/mdev hooks, it needs manual setup.

I don't think anyone has yet *proposed* a platform-neutral userspace interface to GPIOs yet. They all seem to include at least platform specific pinmux setup ... which is probably inevitable, but that would seem to need abstracting into platform-specific hooks.

There have been a few folk expressing interest in a userspace GPIO interface, and a few system-specific examples. The most flexible ones that come to mind are on Gumstix PXA2xx boards. One enables GPIO IRQs through a gpio-events module; and a /proc/gpio/GPIOnn interface monitors all the pins and their configurations (which may mean they aren't used for GPIOs at all). On some AVR32 boards, Atmel had a (less capable) configfs interface, mostly used for LED access.

More detailed comments are embedded below.

(quoted diff) +2 -0

+static int __init ar7_wdt_init(void)
+{

drivers/char/watchdog/ar7_wdt.c new file +11 -0

+	rc = misc_register(&ar7_wdt_miscdev);
+	if (rc) {
+		printk(KERN_ERR DRVNAME ": unable to register misc device\n");
+		goto out_alloc;
+	}
+
+	rc = register_reboot_notifier(&ar7_wdt_notifier);
+	if (rc) {
+		printk(KERN_ERR DRVNAME
+			": unable to register reboot notifier\n");
+		goto out_register;

If you want I'll add it to the linux-2.6-watchdog-mm tree with the above mentioned changes.

Quoted 2 lines

If you want I'll add it to the linux-2.6-watchdog-mm tree with the above mentioned changes.

Yes, please