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 <h...@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 <h...@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.majew...@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
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
