Message ID | 1454318108-5033-1-git-send-email-hs@denx.de |
---|---|
State | RFC |
Delegated to: | Przemyslaw Marczak |
Headers | show |
Hi Heiko, Please find below comments. > With the new dfu_mtd layer, now dfu supports reading/writing > to mtd partitions, found on mtd devices. With this approach > it is also possible to read/write to concatenated mtd > devices. > > Signed-off-by: Heiko Schocher <hs@denx.de> > > --- > This patch is based on patch: > dfu: allow get_medium_size() to return bigger medium sizes than 2GiB > > Tested this driver on etamin module on the dxr2 board > with a DDP nand with a size of 4GiB (2 CS -> 2 nand > devices, concatenated with mtdconcat to a new mtd device) > > drivers/dfu/Makefile | 1 + > drivers/dfu/dfu.c | 3 + > drivers/dfu/dfu_mtd.c | 274 > ++++++++++++++++++++++++++++++++++++++++++++++++++ > include/dfu.h | 23 +++++ 4 files changed, 301 insertions(+) > create mode 100644 drivers/dfu/dfu_mtd.c > > diff --git a/drivers/dfu/Makefile b/drivers/dfu/Makefile > index 61f2b71..c769d8c 100644 > --- a/drivers/dfu/Makefile > +++ b/drivers/dfu/Makefile > @@ -7,6 +7,7 @@ > > obj-$(CONFIG_USB_FUNCTION_DFU) += dfu.o > obj-$(CONFIG_DFU_MMC) += dfu_mmc.o > +obj-$(CONFIG_DFU_MTD) += dfu_mtd.o > obj-$(CONFIG_DFU_NAND) += dfu_nand.o > obj-$(CONFIG_DFU_RAM) += dfu_ram.o > obj-$(CONFIG_DFU_SF) += dfu_sf.o > diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c > index 873dad5..bce619c 100644 > --- a/drivers/dfu/dfu.c > +++ b/drivers/dfu/dfu.c > @@ -406,6 +406,9 @@ static int dfu_fill_entity(struct dfu_entity > *dfu, char *s, int alt, if (strcmp(interface, "mmc") == 0) { > if (dfu_fill_entity_mmc(dfu, devstr, s)) > return -1; > + } else if (strcmp(interface, "mtd") == 0) { > + if (dfu_fill_entity_mtd(dfu, devstr, s)) > + return -1; > } else if (strcmp(interface, "nand") == 0) { > if (dfu_fill_entity_nand(dfu, devstr, s)) > return -1; > diff --git a/drivers/dfu/dfu_mtd.c b/drivers/dfu/dfu_mtd.c > new file mode 100644 > index 0000000..de24f94 > --- /dev/null > +++ b/drivers/dfu/dfu_mtd.c > @@ -0,0 +1,274 @@ > +/* > + * dfu_mtd.c -- DFU for MTD routines. MTD devices? > + * > + * Copyright (C) 2016 DENX Software Engineering GmbH <hs@denx.de> > + * > + * based on: > + * Copyright (C) 2012-2013 Texas Instruments, Inc. Based on: > + * > + * Based on dfu_mmc.c which is: > + * Copyright (C) 2012 Samsung Electronics > + * author: Lukasz Majewski <l.majewski@samsung.com> > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#include <common.h> > +#include <malloc.h> > +#include <errno.h> > +#include <div64.h> > +#include <dfu.h> > +#include <linux/mtd/mtd.h> > +#include <linux/mtd/partitions.h> > +#include <jffs2/load_kernel.h> > + > +static int mtd_block_op(enum dfu_op op, struct dfu_entity *dfu, > + u64 offset, void *buf, long long *len) > +{ > + loff_t start; > + size_t retlen; > + int length = *len; > + int ret = 0; > + struct mtd_info *mtd = dfu->data.mtd.mtd; > + int bsize = mtd->erasesize; > + int loops = length / bsize; > + int rest = length % bsize; > + char *curbuf; > + int i = 0; > + struct erase_info ei; Try to clean it up to avoid camel case definitions. (If in doubt please refer to automatic definitions at dfu.c). Please check this globally. > + > + /* if buf == NULL return total size of the area */ > + if (buf == NULL) { > + *len = dfu->data.mtd.part->size; > + return 0; > + } Do we need this if (buf == NULL) { } construct to get the size of the area? A few lines down you have defined the dfu_get_medium_size_mtd() function. I think that we can remove the above code. > + > + start = dfu->data.mtd.part->offset + dfu->bad_skip + offset; > + if (start > dfu->data.mtd.part->offset + > dfu->data.mtd.part->size) > + return -EIO; > + > + if (offset % bsize) > + return -EFAULT; > + > + if (rest) > + loops++; > + > + curbuf = buf; > + while (i < loops) { > + ret = mtd_block_isbad(mtd, start); > + if (ret == -EINVAL) > + return ret; > + > + if (ret) { > + /* we have a bad block */ > + start += bsize; > + dfu->bad_skip += bsize; > + continue; > + } > + > + if (op == DFU_OP_READ) { > + ret = mtd_read(mtd, start, bsize, &retlen, > + (u_char *)curbuf); > + } else { > + memset(&ei, 0, sizeof(struct erase_info)); > + ei.mtd = mtd; > + ei.addr = start; > + ei.len = bsize; > + ret = mtd_erase(mtd, &ei); > + if (ret != 0) { > + if (ret == -EIO) { > + ret = mtd_block_isbad(mtd, > start); > + if (ret == -EINVAL) > + return ret; > + > + if (ret) { > + /* This is now a bad > block */ > + start += bsize; > + dfu->bad_skip += > bsize; > + continue; > + } > + return -EIO; > + } else { > + /* else we have an error */ > + return ret; > + } > + } > + > + /* now we are sure, we can write to the > block */ > + ret = mtd_write(mtd, start, bsize, &retlen, > + (const u_char *)curbuf); > + if (ret < 0) > + return ret; > + > + if (retlen != bsize) > + return -EIO; > + } > + curbuf += bsize; > + start += bsize; > + i++; > + } > + if (ret != 0) { > + printf("%s: mtd %s call failed at %llx!\n", > + __func__, op == DFU_OP_READ ? "read" : > "write", > + start); > + return ret; > + } > + > + dfu->data.mtd.start_erase = start; > + return ret; > +} > + > +static inline int mtd_block_write(struct dfu_entity *dfu, > + u64 offset, void *buf, long long *len) > +{ > + return mtd_block_op(DFU_OP_WRITE, dfu, offset, buf, len); > +} > + > +static inline int mtd_block_read(struct dfu_entity *dfu, > + u64 offset, void *buf, long long *len) > +{ > + return mtd_block_op(DFU_OP_READ, dfu, offset, buf, len); > +} > + > +static int dfu_write_medium_mtd(struct dfu_entity *dfu, > + u64 offset, void *buf, long long *len) > +{ > + int ret = -1; > + > + switch (dfu->layout) { > + case DFU_RAW_ADDR: > + ret = mtd_block_write(dfu, offset, buf, len); > + break; > + default: > + printf("%s: Layout (%s) not (yet) supported!\n", > __func__, > + dfu_get_layout(dfu->layout)); > + } > + > + return ret; > +} > + > +static int dfu_get_medium_size_mtd(struct dfu_entity *dfu, long long > *size) +{ > + if (!size) > + return -EFAULT; > + > + *size = dfu->data.mtd.part->size; > + return 0; > +} > + > +static int dfu_read_medium_mtd(struct dfu_entity *dfu, u64 offset, > void *buf, > + long long *len) > +{ > + int ret = -1; > + > + switch (dfu->layout) { > + case DFU_RAW_ADDR: > + ret = mtd_block_read(dfu, offset, buf, len); > + break; > + default: > + printf("%s: Layout (%s) not (yet) supported!\n", > __func__, > + dfu_get_layout(dfu->layout)); > + } > + > + return ret; > +} > + > +static int dfu_flush_medium_mtd(struct dfu_entity *dfu) > +{ > + int ret = 0; > + > + /* in case of ubi partition, erase rest of the partition */ > + if (dfu->data.mtd.ubi) { > + int ret; > + struct erase_info ei; > + int bsize = dfu->data.mtd.mtd->erasesize; > + int loops; > + int i = 0; > + int len; > + > + memset(&ei, 0, sizeof(struct erase_info)); > + ei.mtd = dfu->data.mtd.mtd; > + ei.addr = dfu->data.mtd.start_erase; > + ei.len = bsize; > + len = dfu->data.mtd.part->size - > + (ei.addr - dfu->data.mtd.part->offset); > + loops = len / bsize; > + > + while (i < loops) { > + ret = mtd_erase(dfu->data.mtd.mtd, &ei); > + if (ret != 0) if (ret) would be enough > + printf("Failure erase: %d\n", ret); printf("%s: Failure erase: %d\n", __func__, ret) or error(). Please check this globally. > + i++; > + ei.addr += bsize; > + } > + } > + > + return ret; > +} > + > +static unsigned int dfu_polltimeout_mtd(struct dfu_entity *dfu) > +{ > + /* > + * Currently, Poll Timeout != 0 is only needed on MTD > + * ubi partition, as the not used sectors need an erase > + */ > + if (dfu->data.mtd.ubi) > + return DFU_MANIFEST_POLL_TIMEOUT; > + > + return DFU_DEFAULT_POLL_TIMEOUT; > +} > + > +int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, char > *s) +{ > + char *st; > + struct mtd_device *dev; > + struct part_info *part; > + > + dfu->data.mtd.ubi = 0; > + dfu->dev_type = DFU_DEV_MTD; > + st = strsep(&s, " "); > + if ((!strcmp(st, "mtddev")) || (!strcmp(st, "mtddevubi"))) { > + char mtd_dev[16]; > + u8 pnum; > + > + if (!strcmp(st, "mtddevubi")) > + dfu->data.mtd.ubi = 1; > + dfu->layout = DFU_RAW_ADDR; > + /* > + * Search the mtd device number where this partition > + * is located > + */ > + if (mtdparts_init() != 0) { > + printf("Error initializing mtdparts!\n"); Please make this printf more verbose (as pointed above) or simply use error() For reference, please look into dfu_fill_entity_mmc() at dfu_mmc.c > + return -EINVAL; > + } > + > + if (find_dev_and_part(dfu->name, &dev, &pnum, > &part)) { > + printf("Partition %s not found!\n", > dfu->name); The same as above. Please check globally. > + return -ENODEV; > + } > + sprintf(mtd_dev, "%s%d", MTD_DEV_TYPE(dev->id->type), > + dev->id->num); > + dfu->data.mtd.mtd = get_mtd_device_nm(mtd_dev); > + dfu->data.mtd.part = part; > + if (IS_ERR(dfu->data.mtd.mtd)) { > + printf("Partition %s not found on device > %s!\n", > + dfu->name, mtd_dev); > + return -ENODEV; > + } > + } else { > + printf("%s: Memory layout (%s) not supported!\n", > __func__, st); > + return -ENXIO; > + } > + > + dfu->get_medium_size = dfu_get_medium_size_mtd; > + dfu->read_medium = dfu_read_medium_mtd; > + dfu->write_medium = dfu_write_medium_mtd; > + dfu->flush_medium = dfu_flush_medium_mtd; > + dfu->poll_timeout = dfu_polltimeout_mtd; > + > + /* initial state */ > + dfu->inited = 0; > + > + return 0; > +} > diff --git a/include/dfu.h b/include/dfu.h > index c327bb5..a0b111c 100644 > --- a/include/dfu.h > +++ b/include/dfu.h > @@ -23,6 +23,7 @@ enum dfu_device_type { > DFU_DEV_NAND, > DFU_DEV_RAM, > DFU_DEV_SF, > + DFU_DEV_MTD, > }; > > enum dfu_layout { > @@ -67,6 +68,16 @@ struct nand_internal_data { > unsigned int ubi; > }; > > +struct mtd_internal_data { > + struct mtd_info *mtd; > + struct part_info *part; > + > + size_t len; It seems that size_t is defined at posix_types.h file as unsigned int. This means that the MTD partition (entity) cannot be larger than 4 GiB. Is this assumption correct? Shouldn't we be prepared for larger ones? > + /* for MTD/ubi use */ > + unsigned int ubi; > + loff_t start_erase; > +}; > + > struct ram_internal_data { > void *start; > unsigned int size; > @@ -108,6 +119,7 @@ struct dfu_entity { > struct nand_internal_data nand; > struct ram_internal_data ram; > struct sf_internal_data sf; > + struct mtd_internal_data mtd; > } data; > > int (*get_medium_size)(struct dfu_entity *dfu, long long > *size); @@ -189,6 +201,17 @@ static inline int > dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, } > #endif > > +#ifdef CONFIG_DFU_MTD > +extern int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, > char *s); +#else > +static inline int dfu_fill_entity_mtd(struct dfu_entity *dfu, char > *devstr, > + char *s) > +{ > + puts("MTD support not available!\n"); > + return -1; > +} > +#endif > + > #ifdef CONFIG_DFU_NAND > extern int dfu_fill_entity_nand(struct dfu_entity *dfu, char > *devstr, char *s); #else
Hello Lukasz, Am 02.02.2016 um 11:06 schrieb Lukasz Majewski: > Hi Heiko, > > Please find below comments. > >> With the new dfu_mtd layer, now dfu supports reading/writing >> to mtd partitions, found on mtd devices. With this approach >> it is also possible to read/write to concatenated mtd >> devices. >> >> Signed-off-by: Heiko Schocher <hs@denx.de> >> >> --- >> This patch is based on patch: >> dfu: allow get_medium_size() to return bigger medium sizes than 2GiB >> >> Tested this driver on etamin module on the dxr2 board >> with a DDP nand with a size of 4GiB (2 CS -> 2 nand >> devices, concatenated with mtdconcat to a new mtd device) >> >> drivers/dfu/Makefile | 1 + >> drivers/dfu/dfu.c | 3 + >> drivers/dfu/dfu_mtd.c | 274 >> ++++++++++++++++++++++++++++++++++++++++++++++++++ >> include/dfu.h | 23 +++++ 4 files changed, 301 insertions(+) >> create mode 100644 drivers/dfu/dfu_mtd.c >> >> diff --git a/drivers/dfu/Makefile b/drivers/dfu/Makefile >> index 61f2b71..c769d8c 100644 >> --- a/drivers/dfu/Makefile >> +++ b/drivers/dfu/Makefile >> @@ -7,6 +7,7 @@ >> >> obj-$(CONFIG_USB_FUNCTION_DFU) += dfu.o >> obj-$(CONFIG_DFU_MMC) += dfu_mmc.o >> +obj-$(CONFIG_DFU_MTD) += dfu_mtd.o >> obj-$(CONFIG_DFU_NAND) += dfu_nand.o >> obj-$(CONFIG_DFU_RAM) += dfu_ram.o >> obj-$(CONFIG_DFU_SF) += dfu_sf.o >> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c >> index 873dad5..bce619c 100644 >> --- a/drivers/dfu/dfu.c >> +++ b/drivers/dfu/dfu.c >> @@ -406,6 +406,9 @@ static int dfu_fill_entity(struct dfu_entity >> *dfu, char *s, int alt, if (strcmp(interface, "mmc") == 0) { >> if (dfu_fill_entity_mmc(dfu, devstr, s)) >> return -1; >> + } else if (strcmp(interface, "mtd") == 0) { >> + if (dfu_fill_entity_mtd(dfu, devstr, s)) >> + return -1; >> } else if (strcmp(interface, "nand") == 0) { >> if (dfu_fill_entity_nand(dfu, devstr, s)) >> return -1; >> diff --git a/drivers/dfu/dfu_mtd.c b/drivers/dfu/dfu_mtd.c >> new file mode 100644 >> index 0000000..de24f94 >> --- /dev/null >> +++ b/drivers/dfu/dfu_mtd.c >> @@ -0,0 +1,274 @@ >> +/* >> + * dfu_mtd.c -- DFU for MTD routines. > > MTD devices? Fixed. >> + * >> + * Copyright (C) 2016 DENX Software Engineering GmbH <hs@denx.de> >> + * >> + * based on: >> + * Copyright (C) 2012-2013 Texas Instruments, Inc. > > Based on: Fixed. >> + * >> + * Based on dfu_mmc.c which is: >> + * Copyright (C) 2012 Samsung Electronics >> + * author: Lukasz Majewski <l.majewski@samsung.com> >> + * >> + * SPDX-License-Identifier: GPL-2.0+ >> + */ >> + >> +#include <common.h> >> +#include <malloc.h> >> +#include <errno.h> >> +#include <div64.h> >> +#include <dfu.h> >> +#include <linux/mtd/mtd.h> >> +#include <linux/mtd/partitions.h> >> +#include <jffs2/load_kernel.h> >> + >> +static int mtd_block_op(enum dfu_op op, struct dfu_entity *dfu, >> + u64 offset, void *buf, long long *len) >> +{ >> + loff_t start; >> + size_t retlen; >> + int length = *len; >> + int ret = 0; >> + struct mtd_info *mtd = dfu->data.mtd.mtd; >> + int bsize = mtd->erasesize; >> + int loops = length / bsize; >> + int rest = length % bsize; >> + char *curbuf; >> + int i = 0; >> + struct erase_info ei; > > Try to clean it up to avoid camel case definitions. (If in doubt please > refer to automatic definitions at dfu.c). Please check this globally. Do I used CamelCase here? Maybe I do not understand you here ... >> + >> + /* if buf == NULL return total size of the area */ >> + if (buf == NULL) { >> + *len = dfu->data.mtd.part->size; >> + return 0; >> + } > > Do we need this if (buf == NULL) { > } > construct to get the size of the area? > > A few lines down you have defined the dfu_get_medium_size_mtd() > function. i> I think that we can remove the above code. Yes, removed. >> + >> + start = dfu->data.mtd.part->offset + dfu->bad_skip + offset; >> + if (start > dfu->data.mtd.part->offset + >> dfu->data.mtd.part->size) >> + return -EIO; >> + >> + if (offset % bsize) >> + return -EFAULT; >> + >> + if (rest) >> + loops++; >> + >> + curbuf = buf; >> + while (i < loops) { >> + ret = mtd_block_isbad(mtd, start); >> + if (ret == -EINVAL) >> + return ret; >> + >> + if (ret) { >> + /* we have a bad block */ >> + start += bsize; >> + dfu->bad_skip += bsize; >> + continue; >> + } >> + >> + if (op == DFU_OP_READ) { >> + ret = mtd_read(mtd, start, bsize, &retlen, >> + (u_char *)curbuf); >> + } else { >> + memset(&ei, 0, sizeof(struct erase_info)); >> + ei.mtd = mtd; >> + ei.addr = start; >> + ei.len = bsize; >> + ret = mtd_erase(mtd, &ei); >> + if (ret != 0) { >> + if (ret == -EIO) { >> + ret = mtd_block_isbad(mtd, >> start); >> + if (ret == -EINVAL) >> + return ret; >> + >> + if (ret) { >> + /* This is now a bad >> block */ >> + start += bsize; >> + dfu->bad_skip += >> bsize; >> + continue; >> + } >> + return -EIO; >> + } else { >> + /* else we have an error */ >> + return ret; >> + } >> + } >> + >> + /* now we are sure, we can write to the >> block */ >> + ret = mtd_write(mtd, start, bsize, &retlen, >> + (const u_char *)curbuf); >> + if (ret < 0) >> + return ret; >> + >> + if (retlen != bsize) >> + return -EIO; >> + } >> + curbuf += bsize; >> + start += bsize; >> + i++; >> + } >> + if (ret != 0) { >> + printf("%s: mtd %s call failed at %llx!\n", >> + __func__, op == DFU_OP_READ ? "read" : >> "write", >> + start); >> + return ret; >> + } >> + >> + dfu->data.mtd.start_erase = start; >> + return ret; >> +} >> + >> +static inline int mtd_block_write(struct dfu_entity *dfu, >> + u64 offset, void *buf, long long *len) >> +{ >> + return mtd_block_op(DFU_OP_WRITE, dfu, offset, buf, len); >> +} >> + >> +static inline int mtd_block_read(struct dfu_entity *dfu, >> + u64 offset, void *buf, long long *len) >> +{ >> + return mtd_block_op(DFU_OP_READ, dfu, offset, buf, len); >> +} >> + >> +static int dfu_write_medium_mtd(struct dfu_entity *dfu, >> + u64 offset, void *buf, long long *len) >> +{ >> + int ret = -1; >> + >> + switch (dfu->layout) { >> + case DFU_RAW_ADDR: >> + ret = mtd_block_write(dfu, offset, buf, len); >> + break; >> + default: >> + printf("%s: Layout (%s) not (yet) supported!\n", >> __func__, >> + dfu_get_layout(dfu->layout)); >> + } >> + >> + return ret; >> +} >> + >> +static int dfu_get_medium_size_mtd(struct dfu_entity *dfu, long long >> *size) +{ >> + if (!size) >> + return -EFAULT; >> + >> + *size = dfu->data.mtd.part->size; >> + return 0; >> +} >> + >> +static int dfu_read_medium_mtd(struct dfu_entity *dfu, u64 offset, >> void *buf, >> + long long *len) >> +{ >> + int ret = -1; >> + >> + switch (dfu->layout) { >> + case DFU_RAW_ADDR: >> + ret = mtd_block_read(dfu, offset, buf, len); >> + break; >> + default: >> + printf("%s: Layout (%s) not (yet) supported!\n", >> __func__, >> + dfu_get_layout(dfu->layout)); >> + } >> + >> + return ret; >> +} >> + >> +static int dfu_flush_medium_mtd(struct dfu_entity *dfu) >> +{ >> + int ret = 0; >> + >> + /* in case of ubi partition, erase rest of the partition */ >> + if (dfu->data.mtd.ubi) { >> + int ret; >> + struct erase_info ei; >> + int bsize = dfu->data.mtd.mtd->erasesize; >> + int loops; >> + int i = 0; >> + int len; >> + >> + memset(&ei, 0, sizeof(struct erase_info)); >> + ei.mtd = dfu->data.mtd.mtd; >> + ei.addr = dfu->data.mtd.start_erase; >> + ei.len = bsize; >> + len = dfu->data.mtd.part->size - >> + (ei.addr - dfu->data.mtd.part->offset); >> + loops = len / bsize; >> + >> + while (i < loops) { >> + ret = mtd_erase(dfu->data.mtd.mtd, &ei); >> + if (ret != 0) > > if (ret) would be enough > >> + printf("Failure erase: %d\n", ret); > > printf("%s: Failure erase: %d\n", > __func__, > ret) or error(). > Please check > this globally. Changed to error() (And all similiar) >> + i++; >> + ei.addr += bsize; >> + } >> + } >> + >> + return ret; >> +} >> + >> +static unsigned int dfu_polltimeout_mtd(struct dfu_entity *dfu) >> +{ >> + /* >> + * Currently, Poll Timeout != 0 is only needed on MTD >> + * ubi partition, as the not used sectors need an erase >> + */ >> + if (dfu->data.mtd.ubi) >> + return DFU_MANIFEST_POLL_TIMEOUT; >> + >> + return DFU_DEFAULT_POLL_TIMEOUT; >> +} >> + >> +int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, char >> *s) +{ >> + char *st; >> + struct mtd_device *dev; >> + struct part_info *part; >> + >> + dfu->data.mtd.ubi = 0; >> + dfu->dev_type = DFU_DEV_MTD; >> + st = strsep(&s, " "); >> + if ((!strcmp(st, "mtddev")) || (!strcmp(st, "mtddevubi"))) { >> + char mtd_dev[16]; >> + u8 pnum; >> + >> + if (!strcmp(st, "mtddevubi")) >> + dfu->data.mtd.ubi = 1; >> + dfu->layout = DFU_RAW_ADDR; >> + /* >> + * Search the mtd device number where this partition >> + * is located >> + */ >> + if (mtdparts_init() != 0) { >> + printf("Error initializing mtdparts!\n"); > > Please make this printf more verbose (as > pointed above) or simply use error() > > For reference, please look into > dfu_fill_entity_mmc() at dfu_mmc.c > >> + return -EINVAL; >> + } >> + >> + if (find_dev_and_part(dfu->name, &dev, &pnum, >> &part)) { >> + printf("Partition %s not found!\n", >> dfu->name); > > The same as above. Please check globally. > >> + return -ENODEV; >> + } >> + sprintf(mtd_dev, "%s%d", MTD_DEV_TYPE(dev->id->type), >> + dev->id->num); >> + dfu->data.mtd.mtd = get_mtd_device_nm(mtd_dev); >> + dfu->data.mtd.part = part; >> + if (IS_ERR(dfu->data.mtd.mtd)) { >> + printf("Partition %s not found on device >> %s!\n", >> + dfu->name, mtd_dev); >> + return -ENODEV; >> + } >> + } else { >> + printf("%s: Memory layout (%s) not supported!\n", >> __func__, st); >> + return -ENXIO; >> + } >> + >> + dfu->get_medium_size = dfu_get_medium_size_mtd; >> + dfu->read_medium = dfu_read_medium_mtd; >> + dfu->write_medium = dfu_write_medium_mtd; >> + dfu->flush_medium = dfu_flush_medium_mtd; >> + dfu->poll_timeout = dfu_polltimeout_mtd; >> + >> + /* initial state */ >> + dfu->inited = 0; >> + >> + return 0; >> +} >> diff --git a/include/dfu.h b/include/dfu.h >> index c327bb5..a0b111c 100644 >> --- a/include/dfu.h >> +++ b/include/dfu.h >> @@ -23,6 +23,7 @@ enum dfu_device_type { >> DFU_DEV_NAND, >> DFU_DEV_RAM, >> DFU_DEV_SF, >> + DFU_DEV_MTD, >> }; >> >> enum dfu_layout { >> @@ -67,6 +68,16 @@ struct nand_internal_data { >> unsigned int ubi; >> }; >> >> +struct mtd_internal_data { >> + struct mtd_info *mtd; >> + struct part_info *part; >> + >> + size_t len; > > It seems that size_t is defined at posix_types.h file as > unsigned int. This means that the MTD partition (entity) cannot > be larger than 4 GiB. Is this assumption correct? Shouldn't we > be prepared for larger ones? Yes, correct. Good catch! This var is not needed longer, as I use from "struct part_info" the "u64 size", so deleted this "size_t len;" >> + /* for MTD/ubi use */ >> + unsigned int ubi; >> + loff_t start_erase; >> +}; >> + >> struct ram_internal_data { >> void *start; >> unsigned int size; >> @@ -108,6 +119,7 @@ struct dfu_entity { >> struct nand_internal_data nand; >> struct ram_internal_data ram; >> struct sf_internal_data sf; >> + struct mtd_internal_data mtd; >> } data; >> >> int (*get_medium_size)(struct dfu_entity *dfu, long long >> *size); @@ -189,6 +201,17 @@ static inline int >> dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, } >> #endif >> >> +#ifdef CONFIG_DFU_MTD >> +extern int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, >> char *s); +#else >> +static inline int dfu_fill_entity_mtd(struct dfu_entity *dfu, char >> *devstr, >> + char *s) >> +{ >> + puts("MTD support not available!\n"); >> + return -1; >> +} >> +#endif >> + >> #ifdef CONFIG_DFU_NAND >> extern int dfu_fill_entity_nand(struct dfu_entity *dfu, char >> *devstr, char *s); #else Thanks for the review. bye, Heiko
diff --git a/drivers/dfu/Makefile b/drivers/dfu/Makefile index 61f2b71..c769d8c 100644 --- a/drivers/dfu/Makefile +++ b/drivers/dfu/Makefile @@ -7,6 +7,7 @@ obj-$(CONFIG_USB_FUNCTION_DFU) += dfu.o obj-$(CONFIG_DFU_MMC) += dfu_mmc.o +obj-$(CONFIG_DFU_MTD) += dfu_mtd.o obj-$(CONFIG_DFU_NAND) += dfu_nand.o obj-$(CONFIG_DFU_RAM) += dfu_ram.o obj-$(CONFIG_DFU_SF) += dfu_sf.o diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c index 873dad5..bce619c 100644 --- a/drivers/dfu/dfu.c +++ b/drivers/dfu/dfu.c @@ -406,6 +406,9 @@ static int dfu_fill_entity(struct dfu_entity *dfu, char *s, int alt, if (strcmp(interface, "mmc") == 0) { if (dfu_fill_entity_mmc(dfu, devstr, s)) return -1; + } else if (strcmp(interface, "mtd") == 0) { + if (dfu_fill_entity_mtd(dfu, devstr, s)) + return -1; } else if (strcmp(interface, "nand") == 0) { if (dfu_fill_entity_nand(dfu, devstr, s)) return -1; diff --git a/drivers/dfu/dfu_mtd.c b/drivers/dfu/dfu_mtd.c new file mode 100644 index 0000000..de24f94 --- /dev/null +++ b/drivers/dfu/dfu_mtd.c @@ -0,0 +1,274 @@ +/* + * dfu_mtd.c -- DFU for MTD routines. + * + * Copyright (C) 2016 DENX Software Engineering GmbH <hs@denx.de> + * + * based on: + * Copyright (C) 2012-2013 Texas Instruments, Inc. + * + * Based on dfu_mmc.c which is: + * Copyright (C) 2012 Samsung Electronics + * author: Lukasz Majewski <l.majewski@samsung.com> + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <malloc.h> +#include <errno.h> +#include <div64.h> +#include <dfu.h> +#include <linux/mtd/mtd.h> +#include <linux/mtd/partitions.h> +#include <jffs2/load_kernel.h> + +static int mtd_block_op(enum dfu_op op, struct dfu_entity *dfu, + u64 offset, void *buf, long long *len) +{ + loff_t start; + size_t retlen; + int length = *len; + int ret = 0; + struct mtd_info *mtd = dfu->data.mtd.mtd; + int bsize = mtd->erasesize; + int loops = length / bsize; + int rest = length % bsize; + char *curbuf; + int i = 0; + struct erase_info ei; + + /* if buf == NULL return total size of the area */ + if (buf == NULL) { + *len = dfu->data.mtd.part->size; + return 0; + } + + start = dfu->data.mtd.part->offset + dfu->bad_skip + offset; + if (start > dfu->data.mtd.part->offset + dfu->data.mtd.part->size) + return -EIO; + + if (offset % bsize) + return -EFAULT; + + if (rest) + loops++; + + curbuf = buf; + while (i < loops) { + ret = mtd_block_isbad(mtd, start); + if (ret == -EINVAL) + return ret; + + if (ret) { + /* we have a bad block */ + start += bsize; + dfu->bad_skip += bsize; + continue; + } + + if (op == DFU_OP_READ) { + ret = mtd_read(mtd, start, bsize, &retlen, + (u_char *)curbuf); + } else { + memset(&ei, 0, sizeof(struct erase_info)); + ei.mtd = mtd; + ei.addr = start; + ei.len = bsize; + ret = mtd_erase(mtd, &ei); + if (ret != 0) { + if (ret == -EIO) { + ret = mtd_block_isbad(mtd, start); + if (ret == -EINVAL) + return ret; + + if (ret) { + /* This is now a bad block */ + start += bsize; + dfu->bad_skip += bsize; + continue; + } + return -EIO; + } else { + /* else we have an error */ + return ret; + } + } + + /* now we are sure, we can write to the block */ + ret = mtd_write(mtd, start, bsize, &retlen, + (const u_char *)curbuf); + if (ret < 0) + return ret; + + if (retlen != bsize) + return -EIO; + } + curbuf += bsize; + start += bsize; + i++; + } + if (ret != 0) { + printf("%s: mtd %s call failed at %llx!\n", + __func__, op == DFU_OP_READ ? "read" : "write", + start); + return ret; + } + + dfu->data.mtd.start_erase = start; + return ret; +} + +static inline int mtd_block_write(struct dfu_entity *dfu, + u64 offset, void *buf, long long *len) +{ + return mtd_block_op(DFU_OP_WRITE, dfu, offset, buf, len); +} + +static inline int mtd_block_read(struct dfu_entity *dfu, + u64 offset, void *buf, long long *len) +{ + return mtd_block_op(DFU_OP_READ, dfu, offset, buf, len); +} + +static int dfu_write_medium_mtd(struct dfu_entity *dfu, + u64 offset, void *buf, long long *len) +{ + int ret = -1; + + switch (dfu->layout) { + case DFU_RAW_ADDR: + ret = mtd_block_write(dfu, offset, buf, len); + break; + default: + printf("%s: Layout (%s) not (yet) supported!\n", __func__, + dfu_get_layout(dfu->layout)); + } + + return ret; +} + +static int dfu_get_medium_size_mtd(struct dfu_entity *dfu, long long *size) +{ + if (!size) + return -EFAULT; + + *size = dfu->data.mtd.part->size; + return 0; +} + +static int dfu_read_medium_mtd(struct dfu_entity *dfu, u64 offset, void *buf, + long long *len) +{ + int ret = -1; + + switch (dfu->layout) { + case DFU_RAW_ADDR: + ret = mtd_block_read(dfu, offset, buf, len); + break; + default: + printf("%s: Layout (%s) not (yet) supported!\n", __func__, + dfu_get_layout(dfu->layout)); + } + + return ret; +} + +static int dfu_flush_medium_mtd(struct dfu_entity *dfu) +{ + int ret = 0; + + /* in case of ubi partition, erase rest of the partition */ + if (dfu->data.mtd.ubi) { + int ret; + struct erase_info ei; + int bsize = dfu->data.mtd.mtd->erasesize; + int loops; + int i = 0; + int len; + + memset(&ei, 0, sizeof(struct erase_info)); + ei.mtd = dfu->data.mtd.mtd; + ei.addr = dfu->data.mtd.start_erase; + ei.len = bsize; + len = dfu->data.mtd.part->size - + (ei.addr - dfu->data.mtd.part->offset); + loops = len / bsize; + + while (i < loops) { + ret = mtd_erase(dfu->data.mtd.mtd, &ei); + if (ret != 0) + printf("Failure erase: %d\n", ret); + i++; + ei.addr += bsize; + } + } + + return ret; +} + +static unsigned int dfu_polltimeout_mtd(struct dfu_entity *dfu) +{ + /* + * Currently, Poll Timeout != 0 is only needed on MTD + * ubi partition, as the not used sectors need an erase + */ + if (dfu->data.mtd.ubi) + return DFU_MANIFEST_POLL_TIMEOUT; + + return DFU_DEFAULT_POLL_TIMEOUT; +} + +int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, char *s) +{ + char *st; + struct mtd_device *dev; + struct part_info *part; + + dfu->data.mtd.ubi = 0; + dfu->dev_type = DFU_DEV_MTD; + st = strsep(&s, " "); + if ((!strcmp(st, "mtddev")) || (!strcmp(st, "mtddevubi"))) { + char mtd_dev[16]; + u8 pnum; + + if (!strcmp(st, "mtddevubi")) + dfu->data.mtd.ubi = 1; + dfu->layout = DFU_RAW_ADDR; + /* + * Search the mtd device number where this partition + * is located + */ + if (mtdparts_init() != 0) { + printf("Error initializing mtdparts!\n"); + return -EINVAL; + } + + if (find_dev_and_part(dfu->name, &dev, &pnum, &part)) { + printf("Partition %s not found!\n", dfu->name); + return -ENODEV; + } + sprintf(mtd_dev, "%s%d", MTD_DEV_TYPE(dev->id->type), + dev->id->num); + dfu->data.mtd.mtd = get_mtd_device_nm(mtd_dev); + dfu->data.mtd.part = part; + if (IS_ERR(dfu->data.mtd.mtd)) { + printf("Partition %s not found on device %s!\n", + dfu->name, mtd_dev); + return -ENODEV; + } + } else { + printf("%s: Memory layout (%s) not supported!\n", __func__, st); + return -ENXIO; + } + + dfu->get_medium_size = dfu_get_medium_size_mtd; + dfu->read_medium = dfu_read_medium_mtd; + dfu->write_medium = dfu_write_medium_mtd; + dfu->flush_medium = dfu_flush_medium_mtd; + dfu->poll_timeout = dfu_polltimeout_mtd; + + /* initial state */ + dfu->inited = 0; + + return 0; +} diff --git a/include/dfu.h b/include/dfu.h index c327bb5..a0b111c 100644 --- a/include/dfu.h +++ b/include/dfu.h @@ -23,6 +23,7 @@ enum dfu_device_type { DFU_DEV_NAND, DFU_DEV_RAM, DFU_DEV_SF, + DFU_DEV_MTD, }; enum dfu_layout { @@ -67,6 +68,16 @@ struct nand_internal_data { unsigned int ubi; }; +struct mtd_internal_data { + struct mtd_info *mtd; + struct part_info *part; + + size_t len; + /* for MTD/ubi use */ + unsigned int ubi; + loff_t start_erase; +}; + struct ram_internal_data { void *start; unsigned int size; @@ -108,6 +119,7 @@ struct dfu_entity { struct nand_internal_data nand; struct ram_internal_data ram; struct sf_internal_data sf; + struct mtd_internal_data mtd; } data; int (*get_medium_size)(struct dfu_entity *dfu, long long *size); @@ -189,6 +201,17 @@ static inline int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, } #endif +#ifdef CONFIG_DFU_MTD +extern int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, char *s); +#else +static inline int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, + char *s) +{ + puts("MTD support not available!\n"); + return -1; +} +#endif + #ifdef CONFIG_DFU_NAND extern int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr, char *s); #else
With the new dfu_mtd layer, now dfu supports reading/writing to mtd partitions, found on mtd devices. With this approach it is also possible to read/write to concatenated mtd devices. Signed-off-by: Heiko Schocher <hs@denx.de> --- This patch is based on patch: dfu: allow get_medium_size() to return bigger medium sizes than 2GiB Tested this driver on etamin module on the dxr2 board with a DDP nand with a size of 4GiB (2 CS -> 2 nand devices, concatenated with mtdconcat to a new mtd device) drivers/dfu/Makefile | 1 + drivers/dfu/dfu.c | 3 + drivers/dfu/dfu_mtd.c | 274 ++++++++++++++++++++++++++++++++++++++++++++++++++ include/dfu.h | 23 +++++ 4 files changed, 301 insertions(+) create mode 100644 drivers/dfu/dfu_mtd.c