Message ID | 1345920652-11728-1-git-send-email-zajec5@gmail.com |
---|---|
State | Superseded |
Headers | show |
On Sat, Aug 25, 2012 at 11:50 AM, Rafał Miłecki <zajec5@gmail.com> wrote: > +config MTD_BCM47_PARTS > + tristate "BCM47XX partitioning support" > + depends on BCM47XX You may want to change "bcm47" to "bcm47xx", for consistency with the other bcm47xx/bcm63xx references found throughout the tree. > + u8 *buf; [...] > + buf = kzalloc(sizeof(*buf) * BCM47PART_BYTES_TO_READ, GFP_KERNEL); sizeof(u8) probably isn't needed. > + /* CFE has small NVRAM at 0x400 */ > + fourcc = (u32 *)&buf[0x400]; > + if (*fourcc == NVRAM_HEADER) { > + parts[curr_part].name = "boot"; > + parts[curr_part].mask_flags = MTD_WRITEABLE; > + parts[curr_part].offset = offset; > + curr_part++; > + } > + > + /* Standard NVRAM */ > + fourcc = (u32 *)&buf[0x000]; > + if (*fourcc == NVRAM_HEADER) { > + parts[curr_part].name = "nvram"; > + parts[curr_part].mask_flags = MTD_WRITEABLE; > + parts[curr_part].offset = offset; > + curr_part++; > + } > + > + /* board_data starts with board_id which differs across boards, > + * but we can use 'MPFR' (hopefully) magic at 0x100 */ > + fourcc = (u32 *)&buf[0x100]; > + if (*fourcc == 0x5246504D) { /* MPFR */ > + parts[curr_part].name = "board_data"; > + parts[curr_part].mask_flags = MTD_WRITEABLE; > + parts[curr_part].offset = offset; > + curr_part++; > + } > + > + /* POT(TOP) */ > + fourcc = (u32 *)&buf[0x000]; > + fourcc2 = (u32 *)&buf[0x004]; > + if (*fourcc == 0x54544f50 && /* POTT */ > + (*fourcc2 & 0xFFFF) == 0x504f) { /* OP */ > + parts[curr_part].name = "POT"; > + parts[curr_part].mask_flags = MTD_WRITEABLE; > + parts[curr_part].offset = offset; > + curr_part++; > + } > + > + /* ML */ > + fourcc = (u32 *)&buf[0x010]; > + fourcc2 = (u32 *)&buf[0x014]; > + if (*fourcc == 0x39685a42 && *fourcc2 == 0x26594131) { I would suggest using symbolic names for the constants, even if it's just something like ML_MAGIC1. > + parts[curr_part].name = "ML"; > + parts[curr_part].mask_flags = MTD_WRITEABLE; > + parts[curr_part].offset = offset; > + curr_part++; > + } Are all of these cases mutually exclusive, given that they use the same offset? It would be nice to reduce the amount of duplicated code if possible. Maybe something as simple as: char *name = NULL; if (some_condition) name = "POT"; else if (some_other_condition) name = "ML"; [...] if (name) { parts[curr_part].name = name; parts[curr_part].mask_flags = MTD_WRITEABLE; parts[curr_part].offset = offset; curr_part++; } Can the contents of buf[] be represented in a struct?
Thanks for comments! 2012/8/26 Kevin Cernekee <cernekee@gmail.com>: > On Sat, Aug 25, 2012 at 11:50 AM, Rafał Miłecki <zajec5@gmail.com> wrote: >> +config MTD_BCM47_PARTS >> + tristate "BCM47XX partitioning support" >> + depends on BCM47XX > > You may want to change "bcm47" to "bcm47xx", for consistency with the > other bcm47xx/bcm63xx references found throughout the tree. Personally I prefer that "bcm47" over "bcm47xx". 2 minor advantages for me: 1) It's shorted but still unique 2) We have chars after digits in it's name, it's easier to read than "xxpart" or "xxnflash" David: AFAIK you're the maintainer of mtd? Could you share your opinion? I won't argue if you guys prefer "bcm47xx" :) >> + u8 *buf; > [...] >> + buf = kzalloc(sizeof(*buf) * BCM47PART_BYTES_TO_READ, GFP_KERNEL); > sizeof(u8) probably isn't needed. I hoped it will make reading the code easier. Multiplying should be optimized by compiler. But can drop this if you think it's not worth it. >> + /* CFE has small NVRAM at 0x400 */ >> + fourcc = (u32 *)&buf[0x400]; >> + if (*fourcc == NVRAM_HEADER) { >> + parts[curr_part].name = "boot"; >> + parts[curr_part].mask_flags = MTD_WRITEABLE; >> + parts[curr_part].offset = offset; >> + curr_part++; >> + } >> + >> + /* Standard NVRAM */ >> + fourcc = (u32 *)&buf[0x000]; >> + if (*fourcc == NVRAM_HEADER) { >> + parts[curr_part].name = "nvram"; >> + parts[curr_part].mask_flags = MTD_WRITEABLE; >> + parts[curr_part].offset = offset; >> + curr_part++; >> + } >> + >> + /* board_data starts with board_id which differs across boards, >> + * but we can use 'MPFR' (hopefully) magic at 0x100 */ >> + fourcc = (u32 *)&buf[0x100]; >> + if (*fourcc == 0x5246504D) { /* MPFR */ >> + parts[curr_part].name = "board_data"; >> + parts[curr_part].mask_flags = MTD_WRITEABLE; >> + parts[curr_part].offset = offset; >> + curr_part++; >> + } >> + >> + /* POT(TOP) */ >> + fourcc = (u32 *)&buf[0x000]; >> + fourcc2 = (u32 *)&buf[0x004]; >> + if (*fourcc == 0x54544f50 && /* POTT */ >> + (*fourcc2 & 0xFFFF) == 0x504f) { /* OP */ >> + parts[curr_part].name = "POT"; >> + parts[curr_part].mask_flags = MTD_WRITEABLE; >> + parts[curr_part].offset = offset; >> + curr_part++; >> + } >> + >> + /* ML */ >> + fourcc = (u32 *)&buf[0x010]; >> + fourcc2 = (u32 *)&buf[0x014]; >> + if (*fourcc == 0x39685a42 && *fourcc2 == 0x26594131) { > > I would suggest using symbolic names for the constants, even if it's > just something like ML_MAGIC1. Agree. >> + parts[curr_part].name = "ML"; >> + parts[curr_part].mask_flags = MTD_WRITEABLE; >> + parts[curr_part].offset = offset; >> + curr_part++; >> + } > > Are all of these cases mutually exclusive, given that they use the same offset? Yes. > It would be nice to reduce the amount of duplicated code if possible. > Maybe something as simple as: > > char *name = NULL; > if (some_condition) > name = "POT"; > else if (some_other_condition) > name = "ML"; > [...] > > if (name) { > parts[curr_part].name = name; > parts[curr_part].mask_flags = MTD_WRITEABLE; > parts[curr_part].offset = offset; > curr_part++; > } Ouch, you just reminded me I've forgotten to drop mask_flags in few places. Masking MTD_WRITEABLE is needed for only ~half of our cases. So if we follow your way now, we have to pass two arguments to the generic partition handler: "name" and "mask_flags". But maybe I can use some additional function for that? It should de-duplicate some code and should work even for code inside "trx" parser. > Can the contents of buf[] be represented in a struct? Theoretically yes, but I think it doesn't make much sense. It's for detecting type of partition, so it wouldn't have nice struct. It usually would have only one valid field per type. So it would be mostly like struct partitions_ids { union { u32 possible_nvram_fourcc; u32 possible_pot_fourcc; u32 possible_trx_fourcc; }, u32 possible_pot_fourcc2; u32 pad[2] u32 possible_ml_fourcc; u32 possible_ml_fourcc2; } I really don't like idea of such a struct.
diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig index 27143e0..0cd4096 100644 --- a/drivers/mtd/Kconfig +++ b/drivers/mtd/Kconfig @@ -148,6 +148,10 @@ config MTD_BCM63XX_PARTS This provides partions parsing for BCM63xx devices with CFE bootloaders. +config MTD_BCM47_PARTS + tristate "BCM47XX partitioning support" + depends on BCM47XX + comment "User Modules And Translation Layers" config MTD_CHAR diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile index f901354..dac90e6 100644 --- a/drivers/mtd/Makefile +++ b/drivers/mtd/Makefile @@ -12,6 +12,7 @@ obj-$(CONFIG_MTD_CMDLINE_PARTS) += cmdlinepart.o obj-$(CONFIG_MTD_AFS_PARTS) += afs.o obj-$(CONFIG_MTD_AR7_PARTS) += ar7part.o obj-$(CONFIG_MTD_BCM63XX_PARTS) += bcm63xxpart.o +obj-$(CONFIG_MTD_BCM47_PARTS) += bcm47part.o # 'Users' - code which presents functionality to userspace. obj-$(CONFIG_MTD_CHAR) += mtdchar.o diff --git a/drivers/mtd/bcm47part.c b/drivers/mtd/bcm47part.c new file mode 100644 index 0000000..29c6828 --- /dev/null +++ b/drivers/mtd/bcm47part.c @@ -0,0 +1,193 @@ +/* + * BCM47XX MTD partitioning + * + * Copyright (C) 2012 Rafał Miłecki <zajec5@gmail.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + */ + +#include <linux/module.h> +#include <linux/kernel.h> +#include <linux/slab.h> +#include <linux/mtd/mtd.h> +#include <linux/mtd/partitions.h> +#include <asm/mach-bcm47xx/nvram.h> + +/* 10 parts were found on sflash on Netgear WNDR4500 */ +#define BCM47PART_MAX_PARTS 12 + +/* Amount of bytes we read when analyzing each block of flash memory. + * Set it big enough to allow detecting partition and reading important data. */ +#define BCM47PART_BYTES_TO_READ 0x401 + +#define TRX_MAGIC 0x30524448 + +struct trx_header { + u32 magic; + u32 length; + u32 crc32; + u16 flags; + u16 version; + u32 offset[3]; +} __packed; + +static int bcm47part_parse(struct mtd_info *master, + struct mtd_partition **pparts, + struct mtd_part_parser_data *data) +{ + struct mtd_partition *parts; + u8 i, curr_part = 0; + u8 *buf; + u32 *fourcc, *fourcc2; + size_t bytes_read; + u32 offset; + u32 blocksize = 0x10000; + struct trx_header *trx; + + /* Alloc */ + parts = kzalloc(sizeof(struct mtd_partition) * BCM47PART_MAX_PARTS, + GFP_KERNEL); + buf = kzalloc(sizeof(*buf) * BCM47PART_BYTES_TO_READ, GFP_KERNEL); + + /* Parse block by block looking for magics */ + for (offset = 0; offset <= master->size - blocksize; + offset += blocksize) { + /* Nothing more in higher memory */ + if (offset >= 0x2000000) + break; + + /* Read beginning of the block */ + if (mtd_read(master, offset, BCM47PART_BYTES_TO_READ, + &bytes_read, buf) < 0) { + pr_err("mtd_read error while parsing (offset: 0x%X)!\n", + offset); + continue; + } + + /* CFE has small NVRAM at 0x400 */ + fourcc = (u32 *)&buf[0x400]; + if (*fourcc == NVRAM_HEADER) { + parts[curr_part].name = "boot"; + parts[curr_part].mask_flags = MTD_WRITEABLE; + parts[curr_part].offset = offset; + curr_part++; + } + + /* Standard NVRAM */ + fourcc = (u32 *)&buf[0x000]; + if (*fourcc == NVRAM_HEADER) { + parts[curr_part].name = "nvram"; + parts[curr_part].mask_flags = MTD_WRITEABLE; + parts[curr_part].offset = offset; + curr_part++; + } + + /* board_data starts with board_id which differs across boards, + * but we can use 'MPFR' (hopefully) magic at 0x100 */ + fourcc = (u32 *)&buf[0x100]; + if (*fourcc == 0x5246504D) { /* MPFR */ + parts[curr_part].name = "board_data"; + parts[curr_part].mask_flags = MTD_WRITEABLE; + parts[curr_part].offset = offset; + curr_part++; + } + + /* POT(TOP) */ + fourcc = (u32 *)&buf[0x000]; + fourcc2 = (u32 *)&buf[0x004]; + if (*fourcc == 0x54544f50 && /* POTT */ + (*fourcc2 & 0xFFFF) == 0x504f) { /* OP */ + parts[curr_part].name = "POT"; + parts[curr_part].mask_flags = MTD_WRITEABLE; + parts[curr_part].offset = offset; + curr_part++; + } + + /* ML */ + fourcc = (u32 *)&buf[0x010]; + fourcc2 = (u32 *)&buf[0x014]; + if (*fourcc == 0x39685a42 && *fourcc2 == 0x26594131) { + parts[curr_part].name = "ML"; + parts[curr_part].mask_flags = MTD_WRITEABLE; + parts[curr_part].offset = offset; + curr_part++; + } + + /* TRX */ + fourcc = (u32 *)&buf[0x000]; + if (*fourcc == TRX_MAGIC) { + trx = (struct trx_header *)buf; + + i = 0; + /* We have LZMA loader if offset[2] points to sth */ + if (trx->offset[2]) { + /* TODO: should we add LZMA loader partition? */ + i++; + } + + parts[curr_part].name = "linux"; + parts[curr_part].mask_flags = MTD_WRITEABLE; + parts[curr_part].offset = offset + trx->offset[i]; + curr_part++; + i++; + + parts[curr_part].name = "rootfs"; + parts[curr_part].mask_flags = MTD_WRITEABLE; + parts[curr_part].offset = offset + trx->offset[i]; + /* Pure rootfs size is known and can be calculated as: + * trx->length - trx->offset[i]. We don't fill it as + * we want to have jffs2 (overlay) in the same mtd. */ + curr_part++; + i++; + + /* We have whole TRX scanned, skip to the next part. Use + * roundown (not roundup), as the loop will increase + * offset in next step. */ + offset = rounddown(offset + trx->length, blocksize); + } + + if (curr_part == BCM47PART_MAX_PARTS) { + pr_warn("Reached maximum number of partitions, scanning stopped!\n"); + break; + } + } + + /* We can't read sizes of some (most of) partitions. Assume that + * these partitions end at the beginning of the one they are + * followed by. */ + for (i = 0; i < curr_part - 1; i++) { + if (parts[i].size == 0) + parts[i].size = parts[i + 1].offset - parts[i].offset; + } + if (curr_part > 0 && parts[curr_part - 1].size == 0) + parts[curr_part - 1].size = + master->size - parts[curr_part - 1].offset; + + *pparts = parts; + return curr_part; +}; + +static struct mtd_part_parser bcm47part_mtd_parser = { + .owner = THIS_MODULE, + .parse_fn = bcm47part_parse, + .name = "bcm47part", +}; + +static int __init bcm47part_init(void) +{ + return register_mtd_parser(&bcm47part_mtd_parser); +} + +static void __exit bcm47part_exit(void) +{ + deregister_mtd_parser(&bcm47part_mtd_parser); +} + +module_init(bcm47part_init); +module_exit(bcm47part_exit); + +MODULE_LICENSE("GPL"); +MODULE_DESCRIPTION("MTD partitioning for BCM47XX flash memories");
This driver provides parser detecting partitions on BCM47XX flash memories. It has many differences in comparision to older BCM63XX, like: 1) Different CFE with no more trivial MAGICs 2) More partitions types (board_data, ML, POT) 3) Supporting more than 1 flash on a device which resulted in decision of writing new parser. It uses generic mtd interface and was successfully tested with Netgear WNDR4500 router which has 2 flash memories: serial one and NAND one. Signed-off-by: Rafał Miłecki <zajec5@gmail.com> --- V2: 1) Add support for more partitinos (ML and POT) 2) Optimize: don't scan whole flash (like up to 128 MiB) 3) Optimize: don't scan TRX partitions after detecting header --- drivers/mtd/Kconfig | 4 + drivers/mtd/Makefile | 1 + drivers/mtd/bcm47part.c | 193 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 198 insertions(+), 0 deletions(-) create mode 100644 drivers/mtd/bcm47part.c