[OpenWrt-Devel] [PATCH] kernel: drop downstream support for mtd unaligned writes

Rafał Miłecki zajec5 at gmail.com
Mon Mar 9 07:43:02 EDT 2020


From: Rafał Miłecki <rafal at milecki.pl>

This code was developed 10+ years ago to support few specific devices.
It doesn't match current mtd architecture and is probably/hopefully not
needed anymore.

Problems of this code with recent kernels:
1. It assumes that parent mtd boundaries always allow accessing whole
   block data. It was true only for master MTD device and non-tree
   partitions.
2. MTD_ERASE_PARTIAL affects all partition blocks - even those fully
   accessible in a standard way. This code was probably designed for
   tiny partitions only.
3. It seems broken with subpartitions. Unintended usage of this code
   triggered by run_parsers_by_type() rewrite caused in erasing content
   of some unrelated partitions. It may be caused by incorrect offsets
   calculation.

It seems this code was originally needed by some Gateworks devices from
Avila and Cambria families using RedBoot. Those devices were supported
by ixp4xx target which was dropped recently. It's also likely that new
ixp4xx devices didn't require that code at all. Some recent examples:

1. Gateworks Avila GW2342:
0x000000000000-0x000000040000 : "RedBoot"
0x000000040000-0x000000160000 : "linux"
0x000000160000-0x000000fc0000 : "rootfs"
0x000000360000-0x000000fc0000 : "rootfs_data"
0x000000fc0000-0x000000fc1000 : "RedBoot config"
0x000000fe0000-0x000001000000 : "FIS directory"

2. Gateworks Cambria GW2350:
0x000000000000-0x000000080000 : "RedBoot"
0x000000080000-0x000000180000 : "linux"
0x000000180000-0x000001fe0000 : "rootfs"
0x000000320000-0x000001fe0000 : "rootfs_data"
0x000001fe0000-0x000001fff000 : "FIS directory"
0x000001fff000-0x000002000000 : "RedBoot config"

All above partitions seem to be aligned properly.

Signed-off-by: Rafał Miłecki <rafal at milecki.pl>
---
 .../411-mtd-partial_eraseblock_write.patch    | 131 ------------------
 .../412-mtd-partial_eraseblock_unlock.patch   |  40 ------
 .../411-mtd-partial_eraseblock_write.patch    | 131 ------------------
 .../412-mtd-partial_eraseblock_unlock.patch   |  40 ------
 4 files changed, 342 deletions(-)
 delete mode 100644 target/linux/generic/pending-4.19/411-mtd-partial_eraseblock_write.patch
 delete mode 100644 target/linux/generic/pending-4.19/412-mtd-partial_eraseblock_unlock.patch
 delete mode 100644 target/linux/generic/pending-5.4/411-mtd-partial_eraseblock_write.patch
 delete mode 100644 target/linux/generic/pending-5.4/412-mtd-partial_eraseblock_unlock.patch

diff --git a/target/linux/generic/pending-4.19/411-mtd-partial_eraseblock_write.patch b/target/linux/generic/pending-4.19/411-mtd-partial_eraseblock_write.patch
deleted file mode 100644
index f3a314ae02..0000000000
--- a/target/linux/generic/pending-4.19/411-mtd-partial_eraseblock_write.patch
+++ /dev/null
@@ -1,131 +0,0 @@
-From: Felix Fietkau <nbd at nbd.name>
-Subject: mtd: implement write support for partitions covering only a part of an eraseblock (buffer data that would otherwise be erased)
-
-lede-commit: 87a8e8ac1067f58ba831c4aae443f3655c31cd80
-Signed-off-by: Felix Fietkau <nbd at nbd.name>
----
- drivers/mtd/mtdpart.c   | 90 ++++++++++++++++++++++++++++++++++++++++++++-----
- include/linux/mtd/mtd.h |  4 +++
- 2 files changed, 85 insertions(+), 9 deletions(-)
-
---- a/drivers/mtd/mtdpart.c
-+++ b/drivers/mtd/mtdpart.c
-@@ -36,6 +36,8 @@
- #include "mtdcore.h"
- #include "mtdsplit/mtdsplit.h"
- 
-+#define MTD_ERASE_PARTIAL	0x8000 /* partition only covers parts of an erase block */
-+
- /* Our partition linked list */
- static LIST_HEAD(mtd_partitions);
- static DEFINE_MUTEX(mtd_partitions_mutex);
-@@ -220,6 +222,53 @@ static int part_erase(struct mtd_info *m
- {
- 	struct mtd_part *part = mtd_to_part(mtd);
- 	int ret;
-+	size_t wrlen = 0;
-+	u8 *erase_buf = NULL;
-+	u32 erase_buf_ofs = 0;
-+	bool partial_start = false;
-+
-+	if (mtd->flags & MTD_ERASE_PARTIAL) {
-+		size_t readlen = 0;
-+		u64 mtd_ofs;
-+
-+		erase_buf = kmalloc(part->parent->erasesize, GFP_ATOMIC);
-+		if (!erase_buf)
-+			return -ENOMEM;
-+
-+		mtd_ofs = part->offset + instr->addr;
-+		erase_buf_ofs = do_div(mtd_ofs, part->parent->erasesize);
-+
-+		if (erase_buf_ofs > 0) {
-+			instr->addr -= erase_buf_ofs;
-+			ret = mtd_read(part->parent,
-+				instr->addr + part->offset,
-+				part->parent->erasesize,
-+				&readlen, erase_buf);
-+
-+			instr->len += erase_buf_ofs;
-+			partial_start = true;
-+		} else {
-+			mtd_ofs = part->offset + part->mtd.size;
-+			erase_buf_ofs = part->parent->erasesize -
-+				do_div(mtd_ofs, part->parent->erasesize);
-+
-+			if (erase_buf_ofs > 0) {
-+				instr->len += erase_buf_ofs;
-+				ret = mtd_read(part->parent,
-+					part->offset + instr->addr +
-+					instr->len - part->parent->erasesize,
-+					part->parent->erasesize, &readlen,
-+					erase_buf);
-+			} else {
-+				ret = 0;
-+			}
-+		}
-+		if (ret < 0) {
-+			kfree(erase_buf);
-+			return ret;
-+		}
-+
-+	}
- 
- 	instr->addr += part->offset;
- 	ret = part->parent->_erase(part->parent, instr);
-@@ -227,6 +276,24 @@ static int part_erase(struct mtd_info *m
- 		instr->fail_addr -= part->offset;
- 	instr->addr -= part->offset;
- 
-+	if (mtd->flags & MTD_ERASE_PARTIAL) {
-+		if (partial_start) {
-+			part->parent->_write(part->parent,
-+				instr->addr, erase_buf_ofs,
-+				&wrlen, erase_buf);
-+			instr->addr += erase_buf_ofs;
-+		} else {
-+			instr->len -= erase_buf_ofs;
-+			part->parent->_write(part->parent,
-+				instr->addr + instr->len,
-+				erase_buf_ofs, &wrlen,
-+				erase_buf +
-+				part->parent->erasesize -
-+				erase_buf_ofs);
-+		}
-+		kfree(erase_buf);
-+	}
-+
- 	return ret;
- }
- 
-@@ -539,19 +606,22 @@ static struct mtd_part *allocate_partiti
- 	remainder = do_div(tmp, wr_alignment);
- 	if ((slave->mtd.flags & MTD_WRITEABLE) && remainder) {
- 		/* Doesn't start on a boundary of major erase size */
--		/* FIXME: Let it be writable if it is on a boundary of
--		 * _minor_ erase size though */
--		slave->mtd.flags &= ~MTD_WRITEABLE;
--		printk(KERN_WARNING"mtd: partition \"%s\" doesn't start on an erase/write block boundary -- force read-only\n",
--			part->name);
-+		slave->mtd.flags |= MTD_ERASE_PARTIAL;
-+		if (((u32)slave->mtd.size) > parent->erasesize)
-+			slave->mtd.flags &= ~MTD_WRITEABLE;
-+		else
-+			slave->mtd.erasesize = slave->mtd.size;
- 	}
- 
- 	tmp = part_absolute_offset(parent) + slave->offset + slave->mtd.size;
- 	remainder = do_div(tmp, wr_alignment);
- 	if ((slave->mtd.flags & MTD_WRITEABLE) && remainder) {
--		slave->mtd.flags &= ~MTD_WRITEABLE;
--		printk(KERN_WARNING"mtd: partition \"%s\" doesn't end on an erase/write block -- force read-only\n",
--			part->name);
-+		slave->mtd.flags |= MTD_ERASE_PARTIAL;
-+
-+		if ((u32)slave->mtd.size > parent->erasesize)
-+			slave->mtd.flags &= ~MTD_WRITEABLE;
-+		else
-+			slave->mtd.erasesize = slave->mtd.size;
- 	}
- 
- 	mtd_set_ooblayout(&slave->mtd, &part_ooblayout_ops);
diff --git a/target/linux/generic/pending-4.19/412-mtd-partial_eraseblock_unlock.patch b/target/linux/generic/pending-4.19/412-mtd-partial_eraseblock_unlock.patch
deleted file mode 100644
index a54603a0f8..0000000000
--- a/target/linux/generic/pending-4.19/412-mtd-partial_eraseblock_unlock.patch
+++ /dev/null
@@ -1,40 +0,0 @@
-From: Tim Harvey <tharvey at gateworks.com>
-Subject: mtd: allow partial block unlock
-
-This allows sysupgrade for devices such as the Gateworks Avila/Cambria
-product families based on the ixp4xx using the redboot bootloader with
-combined FIS directory and RedBoot config partitions on larger FLASH
-devices with larger eraseblocks.
-
-This second iteration of this patch addresses previous issues:
-- whitespace breakage fixed
-- unlock in all scenarios
-- simplification and fix logic bug
-
-[john at phrozen.org: this should be moved to the ixp4xx folder]
-
-Signed-off-by: Tim Harvey <tharvey at gateworks.com>
----
- drivers/mtd/mtdpart.c | 11 ++++++++++-
- 1 file changed, 10 insertions(+), 1 deletion(-)
-
---- a/drivers/mtd/mtdpart.c
-+++ b/drivers/mtd/mtdpart.c
-@@ -306,7 +306,16 @@ static int part_lock(struct mtd_info *mt
- static int part_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
- {
- 	struct mtd_part *part = mtd_to_part(mtd);
--	return part->parent->_unlock(part->parent, ofs + part->offset, len);
-+
-+	ofs += part->offset;
-+
-+	if (mtd->flags & MTD_ERASE_PARTIAL) {
-+		/* round up len to next erasesize and round down offset to prev block */
-+		len = (mtd_div_by_eb(len, part->parent) + 1) * part->parent->erasesize;
-+		ofs &= ~(part->parent->erasesize - 1);
-+	}
-+
-+	return part->parent->_unlock(part->parent, ofs, len);
- }
- 
- static int part_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
diff --git a/target/linux/generic/pending-5.4/411-mtd-partial_eraseblock_write.patch b/target/linux/generic/pending-5.4/411-mtd-partial_eraseblock_write.patch
deleted file mode 100644
index b46c3f5ed4..0000000000
--- a/target/linux/generic/pending-5.4/411-mtd-partial_eraseblock_write.patch
+++ /dev/null
@@ -1,131 +0,0 @@
-From: Felix Fietkau <nbd at nbd.name>
-Subject: mtd: implement write support for partitions covering only a part of an eraseblock (buffer data that would otherwise be erased)
-
-lede-commit: 87a8e8ac1067f58ba831c4aae443f3655c31cd80
-Signed-off-by: Felix Fietkau <nbd at nbd.name>
----
- drivers/mtd/mtdpart.c   | 90 ++++++++++++++++++++++++++++++++++++++++++++-----
- include/linux/mtd/mtd.h |  4 +++
- 2 files changed, 85 insertions(+), 9 deletions(-)
-
---- a/drivers/mtd/mtdpart.c
-+++ b/drivers/mtd/mtdpart.c
-@@ -22,6 +22,8 @@
- #include "mtdcore.h"
- #include "mtdsplit/mtdsplit.h"
- 
-+#define MTD_ERASE_PARTIAL	0x8000 /* partition only covers parts of an erase block */
-+
- /* Our partition linked list */
- static LIST_HEAD(mtd_partitions);
- static DEFINE_MUTEX(mtd_partitions_mutex);
-@@ -206,6 +208,53 @@ static int part_erase(struct mtd_info *m
- {
- 	struct mtd_part *part = mtd_to_part(mtd);
- 	int ret;
-+	size_t wrlen = 0;
-+	u8 *erase_buf = NULL;
-+	u32 erase_buf_ofs = 0;
-+	bool partial_start = false;
-+
-+	if (mtd->flags & MTD_ERASE_PARTIAL) {
-+		size_t readlen = 0;
-+		u64 mtd_ofs;
-+
-+		erase_buf = kmalloc(part->parent->erasesize, GFP_ATOMIC);
-+		if (!erase_buf)
-+			return -ENOMEM;
-+
-+		mtd_ofs = part->offset + instr->addr;
-+		erase_buf_ofs = do_div(mtd_ofs, part->parent->erasesize);
-+
-+		if (erase_buf_ofs > 0) {
-+			instr->addr -= erase_buf_ofs;
-+			ret = mtd_read(part->parent,
-+				instr->addr + part->offset,
-+				part->parent->erasesize,
-+				&readlen, erase_buf);
-+
-+			instr->len += erase_buf_ofs;
-+			partial_start = true;
-+		} else {
-+			mtd_ofs = part->offset + part->mtd.size;
-+			erase_buf_ofs = part->parent->erasesize -
-+				do_div(mtd_ofs, part->parent->erasesize);
-+
-+			if (erase_buf_ofs > 0) {
-+				instr->len += erase_buf_ofs;
-+				ret = mtd_read(part->parent,
-+					part->offset + instr->addr +
-+					instr->len - part->parent->erasesize,
-+					part->parent->erasesize, &readlen,
-+					erase_buf);
-+			} else {
-+				ret = 0;
-+			}
-+		}
-+		if (ret < 0) {
-+			kfree(erase_buf);
-+			return ret;
-+		}
-+
-+	}
- 
- 	instr->addr += part->offset;
- 	ret = part->parent->_erase(part->parent, instr);
-@@ -213,6 +262,24 @@ static int part_erase(struct mtd_info *m
- 		instr->fail_addr -= part->offset;
- 	instr->addr -= part->offset;
- 
-+	if (mtd->flags & MTD_ERASE_PARTIAL) {
-+		if (partial_start) {
-+			part->parent->_write(part->parent,
-+				instr->addr, erase_buf_ofs,
-+				&wrlen, erase_buf);
-+			instr->addr += erase_buf_ofs;
-+		} else {
-+			instr->len -= erase_buf_ofs;
-+			part->parent->_write(part->parent,
-+				instr->addr + instr->len,
-+				erase_buf_ofs, &wrlen,
-+				erase_buf +
-+				part->parent->erasesize -
-+				erase_buf_ofs);
-+		}
-+		kfree(erase_buf);
-+	}
-+
- 	return ret;
- }
- 
-@@ -525,19 +592,22 @@ static struct mtd_part *allocate_partiti
- 	remainder = do_div(tmp, wr_alignment);
- 	if ((slave->mtd.flags & MTD_WRITEABLE) && remainder) {
- 		/* Doesn't start on a boundary of major erase size */
--		/* FIXME: Let it be writable if it is on a boundary of
--		 * _minor_ erase size though */
--		slave->mtd.flags &= ~MTD_WRITEABLE;
--		printk(KERN_WARNING"mtd: partition \"%s\" doesn't start on an erase/write block boundary -- force read-only\n",
--			part->name);
-+		slave->mtd.flags |= MTD_ERASE_PARTIAL;
-+		if (((u32)slave->mtd.size) > parent->erasesize)
-+			slave->mtd.flags &= ~MTD_WRITEABLE;
-+		else
-+			slave->mtd.erasesize = slave->mtd.size;
- 	}
- 
- 	tmp = part_absolute_offset(parent) + slave->offset + slave->mtd.size;
- 	remainder = do_div(tmp, wr_alignment);
- 	if ((slave->mtd.flags & MTD_WRITEABLE) && remainder) {
--		slave->mtd.flags &= ~MTD_WRITEABLE;
--		printk(KERN_WARNING"mtd: partition \"%s\" doesn't end on an erase/write block -- force read-only\n",
--			part->name);
-+		slave->mtd.flags |= MTD_ERASE_PARTIAL;
-+
-+		if ((u32)slave->mtd.size > parent->erasesize)
-+			slave->mtd.flags &= ~MTD_WRITEABLE;
-+		else
-+			slave->mtd.erasesize = slave->mtd.size;
- 	}
- 
- 	mtd_set_ooblayout(&slave->mtd, &part_ooblayout_ops);
diff --git a/target/linux/generic/pending-5.4/412-mtd-partial_eraseblock_unlock.patch b/target/linux/generic/pending-5.4/412-mtd-partial_eraseblock_unlock.patch
deleted file mode 100644
index 348fb9a842..0000000000
--- a/target/linux/generic/pending-5.4/412-mtd-partial_eraseblock_unlock.patch
+++ /dev/null
@@ -1,40 +0,0 @@
-From: Tim Harvey <tharvey at gateworks.com>
-Subject: mtd: allow partial block unlock
-
-This allows sysupgrade for devices such as the Gateworks Avila/Cambria
-product families based on the ixp4xx using the redboot bootloader with
-combined FIS directory and RedBoot config partitions on larger FLASH
-devices with larger eraseblocks.
-
-This second iteration of this patch addresses previous issues:
-- whitespace breakage fixed
-- unlock in all scenarios
-- simplification and fix logic bug
-
-[john at phrozen.org: this should be moved to the ixp4xx folder]
-
-Signed-off-by: Tim Harvey <tharvey at gateworks.com>
----
- drivers/mtd/mtdpart.c | 11 ++++++++++-
- 1 file changed, 10 insertions(+), 1 deletion(-)
-
---- a/drivers/mtd/mtdpart.c
-+++ b/drivers/mtd/mtdpart.c
-@@ -292,7 +292,16 @@ static int part_lock(struct mtd_info *mt
- static int part_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
- {
- 	struct mtd_part *part = mtd_to_part(mtd);
--	return part->parent->_unlock(part->parent, ofs + part->offset, len);
-+
-+	ofs += part->offset;
-+
-+	if (mtd->flags & MTD_ERASE_PARTIAL) {
-+		/* round up len to next erasesize and round down offset to prev block */
-+		len = (mtd_div_by_eb(len, part->parent) + 1) * part->parent->erasesize;
-+		ofs &= ~(part->parent->erasesize - 1);
-+	}
-+
-+	return part->parent->_unlock(part->parent, ofs, len);
- }
- 
- static int part_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
-- 
2.25.0


_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


More information about the openwrt-devel mailing list