Re: [PATCH][MIPS][2/7] AR7: mtd partition map

Re: [PATCH][MIPS][2/7] AR7: mtd partition map

[Matteo Croce]

Partition map support

Signed-off-by: Matteo Croce <technoboy85@gmail.com>
Signed-off-by: Felix Fietkau <nbd@openwrt.org>
Signed-off-by: Eugene Konev <ejka@imfi.kspu.ru>

diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index fbec8cd..c1b2508 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -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
diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
index 6d958a4..8451c64 100644
--- a/drivers/mtd/Makefile
+++ b/drivers/mtd/Makefile
@@ -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
diff --git a/drivers/mtd/ar7part.c b/drivers/mtd/ar7part.c
new file mode 100644
index 0000000..775041d
--- /dev/null
+++ b/drivers/mtd/ar7part.c
@@ -0,0 +1,142 @@
+/*
+ * Copyright (C) 2007 Eugene Konev <ejka@openwrt.org>
+ *
+ * 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 <nbd@openwrt.org>
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+#include <linux/bootmem.h>
+#include <linux/squashfs_fs.h>
+
+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;
+
+	ar7_parts[1].name = "config";
+	ar7_parts[1].offset = 0;
+	ar7_parts[1].size = master->erasesize;
+	ar7_parts[1].mask_flags = 0;
+
+	do { /* Try 10 blocks starting from master->erasesize */
+		offset = pre_size;
+		master->read(master, offset,
+			sizeof(header), &len, (u_char *)&header);
+		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)
+{
+	return register_mtd_parser(&ar7_parser);
+}
+
+module_init(ar7_parser_init);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR(	"Felix Fietkau <nbd@openwrt.org>, "
+		"Eugene Konev <ejka@openwrt.org>");
+MODULE_DESCRIPTION("MTD partitioning for TI AR7");

Re: [PATCH][MIPS][2/7] AR7: mtd partition map

[Ralf Baechle]

On Thu, Sep 20, 2007 at 05:55:06PM +0200, Matteo Croce wrote:

> Signed-off-by: Matteo Croce <technoboy85@gmail.com>
> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
> Signed-off-by: Eugene Konev <ejka@imfi.kspu.ru>
> 
> diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
> index fbec8cd..c1b2508 100644
> --- a/drivers/mtd/Kconfig
> +++ b/drivers/mtd/Kconfig
> @@ -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
> diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
> index 6d958a4..8451c64 100644
> --- a/drivers/mtd/Makefile
> +++ b/drivers/mtd/Makefile
> @@ -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
> diff --git a/drivers/mtd/ar7part.c b/drivers/mtd/ar7part.c
> new file mode 100644
> index 0000000..775041d
> --- /dev/null
> +++ b/drivers/mtd/ar7part.c
> @@ -0,0 +1,142 @@
> +/*
> + * Copyright (C) 2007 Eugene Konev <ejka@openwrt.org>
> + *
> + * 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 <nbd@openwrt.org>
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/partitions.h>
> +#include <linux/bootmem.h>
> +#include <linux/squashfs_fs.h>

There is no such header file so this won't build ...

> +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;
> +
> +	ar7_parts[1].name = "config";
> +	ar7_parts[1].offset = 0;
> +	ar7_parts[1].size = master->erasesize;
> +	ar7_parts[1].mask_flags = 0;
> +
> +	do { /* Try 10 blocks starting from master->erasesize */
> +		offset = pre_size;
> +		master->read(master, offset,
> +			sizeof(header), &len, (u_char *)&header);

The u_char type is deprecated.  Use u8 from <linux/types.h> or unsigned char
instead.

> +		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)
> +{
> +	return register_mtd_parser(&ar7_parser);
> +}
> +
> +module_init(ar7_parser_init);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR(	"Felix Fietkau <nbd@openwrt.org>, "
> +		"Eugene Konev <ejka@openwrt.org>");
> +MODULE_DESCRIPTION("MTD partitioning for TI AR7");

  Ralf

Re: [PATCH][MIPS][2/7] AR7: mtd partition map

[Christoph Hellwig]

> +#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.

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

> +	unsigned int root_offset = 0xe0000;

Please provide a symbolic name for this one.

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

Do we really need this printk?

> +		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,
};

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

> +	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.. */

Re: [PATCH][MIPS][2/7] AR7: mtd partition map

[Matteo Croce]

Il Thursday 20 September 2007 19:52:05 hai scritto:
> > +#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.

You're right, i'll move the magic into linux/magic.h

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

we use 4, and bootloader  is right, it returns 4
The fourth partition is empty, the kernel formats it as jffs2
when mounting

> > +	unsigned int root_offset = 0xe0000;
> 
> Please provide a symbolic name for this one.

Ok

> > +	printk(KERN_INFO "Parsing AR7 partition map...\n");
> 
> Do we really need this printk?

No?

> > +		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,
> };
> 
> > +	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.

Right, will do


> > +	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.. */

No. There are MIPS instructions. We look for the loader
start offset since it is in the first partition

Re: [PATCH][MIPS][2/7] AR7: mtd partition map

[Matteo Croce]

Partition map support, fixed

Signed-off-by: Matteo Croce <technoboy85@gmail.com>
Signed-off-by: Felix Fietkau <nbd@openwrt.org>
Signed-off-by: Eugene Konev <ejka@imfi.kspu.ru>

diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index fbec8cd..c1b2508 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -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
diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
index 6d958a4..8451c64 100644
--- a/drivers/mtd/Makefile
+++ b/drivers/mtd/Makefile
@@ -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
diff --git a/drivers/mtd/ar7part.c b/drivers/mtd/ar7part.c
new file mode 100644
index 0000000..8bfd571
--- /dev/null
+++ b/drivers/mtd/ar7part.c
@@ -0,0 +1,151 @@
+/*
+ * Copyright (C) 2007 Eugene Konev <ejka@openwrt.org>
+ *
+ * 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 <nbd@openwrt.org>
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+#include <linux/bootmem.h>
+#include <linux/magic.h>
+
+#define AR7_PARTS	4
+#define ROOT_OFFSET	0xe0000
+
+#ifdef CONFIG_CPU_LITTLE_ENDIAN
+#define LOADER_MAGIC1	0xfeedfa42
+#define LOADER_MAGIC2	0xfeed1281
+#else
+#define LOADER_MAGIC1	0x42faedfe
+#define LOADER_MAGIC2	0x8112edfe
+#endif
+
+struct ar7_bin_rec {
+	unsigned int checksum;
+	unsigned int length;
+	unsigned int address;
+};
+
+static struct mtd_partition ar7_parts[AR7_PARTS];
+
+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 = ROOT_OFFSET;
+
+	int retries = 10;
+
+	ar7_parts[0].name = "loader";
+	ar7_parts[0].offset = 0;
+	ar7_parts[0].size = master->erasesize;
+	ar7_parts[0].mask_flags = MTD_WRITEABLE;
+
+	ar7_parts[1].name = "config";
+	ar7_parts[1].offset = 0;
+	ar7_parts[1].size = master->erasesize;
+	ar7_parts[1].mask_flags = 0;
+
+	do { /* Try 10 blocks starting from master->erasesize */
+		offset = pre_size;
+		master->read(master, offset,
+			sizeof(header), &len, (u_char *)&header);
+		if (!strncmp((char *)&header, "TIENV0.8", 8))
+			ar7_parts[1].offset = pre_size;
+		if (header.checksum == LOADER_MAGIC1)
+			break;
+		if (header.checksum == LOADER_MAGIC2)
+			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 LOADER_MAGIC1:
+		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 LOADER_MAGIC2:
+		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 AR7_PARTS;
+}
+
+static struct mtd_part_parser ar7_parser = {
+	.owner = THIS_MODULE,
+	.parse_fn = create_mtd_partitions,
+	.name = "ar7part",
+};
+
+static int __init ar7_parser_init(void)
+{
+	return register_mtd_parser(&ar7_parser);
+}
+
+module_init(ar7_parser_init);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR(	"Felix Fietkau <nbd@openwrt.org>, "
+		"Eugene Konev <ejka@openwrt.org>");
+MODULE_DESCRIPTION("MTD partitioning for TI AR7");

Re: [PATCH][MIPS][2/7] AR7: mtd partition map

[Christoph Hellwig]

On Thu, Sep 20, 2007 at 09:29:11PM +0200, Matteo Croce wrote:
> +#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.

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

Re: [PATCH][MIPS][2/7] AR7: mtd partition map

[Jörn Engel]

On Thu, 20 September 2007 21:35:49 +0200, Christoph Hellwig wrote:
> On Thu, Sep 20, 2007 at 09:29:11PM +0200, Matteo Croce wrote:
> > +#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.
> 
> > +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.

Jörn

-- 
Joern's library part 13:
http://www.chip-architect.com/

Re: [PATCH][MIPS][2/7] AR7: mtd partition map

[Matteo Croce]

Il Thursday 20 September 2007 22:00:59 Jörn Engel ha scritto:
> On Thu, 20 September 2007 21:35:49 +0200, Christoph Hellwig wrote:
> > On Thu, Sep 20, 2007 at 09:29:11PM +0200, Matteo Croce wrote:
> > > +#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.
> > 
> > > +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?

Re: [PATCH][MIPS][2/7] AR7: mtd partition map

[Jörn Engel]

On Fri, 21 September 2007 04:09:22 +0200, Matteo Croce wrote:
> 
> I use little endian since 99% of AR7s are little endian. Dunno if
> le/be32_to_cpu does some runtime calculations. Do they?

Only if they have to convert endianness.  And even in that case I doubt
that you will notice it in any benchmarks.

Jörn

-- 
The story so far:
In the beginning the Universe was created.  This has made a lot
of people very angry and been widely regarded as a bad move.
-- Douglas Adams

Re: [PATCH][MIPS][2/7] AR7: mtd partition map

[Christoph Hellwig]

On Fri, Sep 21, 2007 at 04:09:22AM +0200, Matteo Croce wrote:
> I use little endian since 99% of AR7s are little endian. Dunno if
> le/be32_to_cpu does some runtime calculations. Do they?

They do runtime unless the argument is const.  If you say that you cant
change endianess of a given system during it's lifetime it's probably
fine to ignore the endianess issue and always use host endian without
any conversions.  In that case you'll need to remove the ifdef aswell,
though.

Re: [PATCH][MIPS][2/7] AR7: mtd partition map

[Geert Uytterhoeven]

On Thu, 20 Sep 2007, Matteo Croce wrote:
> +static int create_mtd_partitions(struct mtd_info *master,
> +				 struct mtd_partition **pparts,
> +				 unsigned long origin)
> +{

    [...]

> +		master->read(master, offset,
> +			sizeof(header), &len, (u_char *)&header);
                                              ^^^^^^^^^^^
Probably we should teach mtd to use `void *' for read and write buffers,
so all these ugly casts can go away...

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH][MIPS][2/7] AR7: mtd partition map

[Matteo Croce]

Partition map support

Signed-off-by: Matteo Croce <technoboy85@gmail.com>
Signed-off-by: Felix Fietkau <nbd@openwrt.org>
Signed-off-by: Eugene Konev <ejka@imfi.kspu.ru>

diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index fbec8cd..c1b2508 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -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
diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
index 6d958a4..8451c64 100644
--- a/drivers/mtd/Makefile
+++ b/drivers/mtd/Makefile
@@ -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
diff --git a/drivers/mtd/ar7part.c b/drivers/mtd/ar7part.c
new file mode 100644
index 0000000..72dd07a
--- /dev/null
+++ b/drivers/mtd/ar7part.c
@@ -0,0 +1,140 @@
+/*
+ * Copyright (C) 2007 Felix Fietkau, Eugene Konev
+ *
+ * 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.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+#include <linux/bootmem.h>
+#include <linux/squashfs_fs.h>
+
+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;
+	unsigned int post_size = 0, 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;
+
+	ar7_parts[1].name = "config";
+	ar7_parts[1].offset = 0;
+	ar7_parts[1].size = master->erasesize;
+	ar7_parts[1].mask_flags = 0;
+
+	do {
+		offset = pre_size;
+		master->read(master, offset,
+			sizeof(header), &len, (u_char *)&header);
+		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_ERR "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)
+{
+	return register_mtd_parser(&ar7_parser);
+}
+
+module_init(ar7_parser_init);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Felix Fietkau, Eugene Konev");
+MODULE_DESCRIPTION("MTD partitioning for TI AR7");