* [U-Boot] [RFC PATCH v1] dfu: introduce dfu_mtd support
@ 2016-02-01 9:15 Heiko Schocher
2016-02-02 10:06 ` Lukasz Majewski
0 siblings, 1 reply; 3+ messages in thread
From: Heiko Schocher @ 2016-02-01 9:15 UTC (permalink / raw)
To: u-boot
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.
+ *
+ * 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@%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
--
2.5.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [U-Boot] [RFC PATCH v1] dfu: introduce dfu_mtd support
2016-02-01 9:15 [U-Boot] [RFC PATCH v1] dfu: introduce dfu_mtd support Heiko Schocher
@ 2016-02-02 10:06 ` Lukasz Majewski
2016-02-02 10:36 ` Heiko Schocher
0 siblings, 1 reply; 3+ messages in thread
From: Lukasz Majewski @ 2016-02-02 10:06 UTC (permalink / raw)
To: u-boot
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
--
Best regards,
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
^ permalink raw reply [flat|nested] 3+ messages in thread
* [U-Boot] [RFC PATCH v1] dfu: introduce dfu_mtd support
2016-02-02 10:06 ` Lukasz Majewski
@ 2016-02-02 10:36 ` Heiko Schocher
0 siblings, 0 replies; 3+ messages in thread
From: Heiko Schocher @ 2016-02-02 10:36 UTC (permalink / raw)
To: u-boot
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
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-02-02 10:36 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-01 9:15 [U-Boot] [RFC PATCH v1] dfu: introduce dfu_mtd support Heiko Schocher
2016-02-02 10:06 ` Lukasz Majewski
2016-02-02 10:36 ` Heiko Schocher
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.