[PATCH v3 3/4] mtd: Stop directly calling master ->_xxx() hooks from mtdpart code

Boris Brezillon boris.brezillon at free-electrons.com
Fri Dec 15 04:39:53 PST 2017


The MTD layer provides several wrappers around mtd->_xxx() hooks. Call
these wrappers instead of directly dereferencing the associated ->_xxx()
pointer.

This change has been motivated by another rework letting the core
handle the case where ->_read/write_oob() are implemented but not
->_read/write(). In this case, we want mtd_read/write() to fall back to
->_read/write_oob() when ->_read/write() are NULL. The problem is,
mtdpart is directly calling the ->_xxx() instead of using the wrappers,
thus leading to a NULL pointer exception.

Even though we only need to do the change for part_read/write(), going
through those wrappers for all kind of part -> master operation
propagation is a good thing, because other wrappers might become
smarter over time, and the duplicated check overhead (parameters will
be checked at the partition and master level instead of only at the
partition level) should be negligible.

Signed-off-by: Boris Brezillon <boris.brezillon at free-electrons.com>
---
Changes in v3:
- unconditionally assign part wrappers as suggested by Brian

Changes in v2:
- new patch needed to fix a NULL pointer dereference BUG
---
 drivers/mtd/mtdpart.c | 141 +++++++++++++++++++-------------------------------
 1 file changed, 53 insertions(+), 88 deletions(-)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index be088bccd593..e83c9d870b11 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -74,8 +74,7 @@ static int part_read(struct mtd_info *mtd, loff_t from, size_t len,
 	int res;
 
 	stats = part->parent->ecc_stats;
-	res = part->parent->_read(part->parent, from + part->offset, len,
-				  retlen, buf);
+	res = mtd_read(part->parent, from + part->offset, len, retlen, buf);
 	if (unlikely(mtd_is_eccerr(res)))
 		mtd->ecc_stats.failed +=
 			part->parent->ecc_stats.failed - stats.failed;
@@ -90,15 +89,15 @@ static int part_point(struct mtd_info *mtd, loff_t from, size_t len,
 {
 	struct mtd_part *part = mtd_to_part(mtd);
 
-	return part->parent->_point(part->parent, from + part->offset, len,
-				    retlen, virt, phys);
+	return mtd_point(part->parent, from + part->offset, len, retlen, virt,
+			 phys);
 }
 
 static int part_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
 {
 	struct mtd_part *part = mtd_to_part(mtd);
 
-	return part->parent->_unpoint(part->parent, from + part->offset, len);
+	return mtd_unpoint(part->parent, from + part->offset, len);
 }
 
 static int part_read_oob(struct mtd_info *mtd, loff_t from,
@@ -126,7 +125,7 @@ static int part_read_oob(struct mtd_info *mtd, loff_t from,
 			return -EINVAL;
 	}
 
-	res = part->parent->_read_oob(part->parent, from + part->offset, ops);
+	res = mtd_read_oob(part->parent, from + part->offset, ops);
 	if (unlikely(res)) {
 		if (mtd_is_bitflip(res))
 			mtd->ecc_stats.corrected++;
@@ -140,48 +139,43 @@ static int part_read_user_prot_reg(struct mtd_info *mtd, loff_t from,
 		size_t len, size_t *retlen, u_char *buf)
 {
 	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_read_user_prot_reg(part->parent, from, len,
-						 retlen, buf);
+	return mtd_read_user_prot_reg(part->parent, from, len, retlen, buf);
 }
 
 static int part_get_user_prot_info(struct mtd_info *mtd, size_t len,
 				   size_t *retlen, struct otp_info *buf)
 {
 	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_get_user_prot_info(part->parent, len, retlen,
-						 buf);
+	return mtd_get_user_prot_info(part->parent, len, retlen, buf);
 }
 
 static int part_read_fact_prot_reg(struct mtd_info *mtd, loff_t from,
 		size_t len, size_t *retlen, u_char *buf)
 {
 	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_read_fact_prot_reg(part->parent, from, len,
-						 retlen, buf);
+	return mtd_read_fact_prot_reg(part->parent, from, len, retlen, buf);
 }
 
 static int part_get_fact_prot_info(struct mtd_info *mtd, size_t len,
 				   size_t *retlen, struct otp_info *buf)
 {
 	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_get_fact_prot_info(part->parent, len, retlen,
-						 buf);
+	return mtd_get_fact_prot_info(part->parent, len, retlen, buf);
 }
 
 static int part_write(struct mtd_info *mtd, loff_t to, size_t len,
 		size_t *retlen, const u_char *buf)
 {
 	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_write(part->parent, to + part->offset, len,
-				    retlen, buf);
+	return mtd_write(part->parent, to + part->offset, len, retlen, buf);
 }
 
 static int part_panic_write(struct mtd_info *mtd, loff_t to, size_t len,
 		size_t *retlen, const u_char *buf)
 {
 	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_panic_write(part->parent, to + part->offset, len,
-					  retlen, buf);
+	return mtd_panic_write(part->parent, to + part->offset, len, retlen,
+			       buf);
 }
 
 static int part_write_oob(struct mtd_info *mtd, loff_t to,
@@ -193,30 +187,29 @@ static int part_write_oob(struct mtd_info *mtd, loff_t to,
 		return -EINVAL;
 	if (ops->datbuf && to + ops->len > mtd->size)
 		return -EINVAL;
-	return part->parent->_write_oob(part->parent, to + part->offset, ops);
+	return mtd_write_oob(part->parent, to + part->offset, ops);
 }
 
 static int part_write_user_prot_reg(struct mtd_info *mtd, loff_t from,
 		size_t len, size_t *retlen, u_char *buf)
 {
 	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_write_user_prot_reg(part->parent, from, len,
-						  retlen, buf);
+	return mtd_write_user_prot_reg(part->parent, from, len, retlen, buf);
 }
 
 static int part_lock_user_prot_reg(struct mtd_info *mtd, loff_t from,
 		size_t len)
 {
 	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_lock_user_prot_reg(part->parent, from, len);
+	return mtd_lock_user_prot_reg(part->parent, from, len);
 }
 
 static int part_writev(struct mtd_info *mtd, const struct kvec *vecs,
 		unsigned long count, loff_t to, size_t *retlen)
 {
 	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_writev(part->parent, vecs, count,
-				     to + part->offset, retlen);
+	return mtd_writev(part->parent, vecs, count, to + part->offset,
+			  retlen);
 }
 
 static int part_erase(struct mtd_info *mtd, struct erase_info *instr)
@@ -225,7 +218,7 @@ static int part_erase(struct mtd_info *mtd, struct erase_info *instr)
 	int ret;
 
 	instr->addr += part->offset;
-	ret = part->parent->_erase(part->parent, instr);
+	ret = mtd_erase(part->parent, instr);
 	if (ret) {
 		if (instr->fail_addr != MTD_FAIL_ADDR_UNKNOWN)
 			instr->fail_addr -= part->offset;
@@ -251,51 +244,51 @@ EXPORT_SYMBOL_GPL(mtd_erase_callback);
 static int part_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
 {
 	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_lock(part->parent, ofs + part->offset, len);
+	return mtd_lock(part->parent, ofs + part->offset, len);
 }
 
 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);
+	return mtd_unlock(part->parent, ofs + part->offset, len);
 }
 
 static int part_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
 {
 	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_is_locked(part->parent, ofs + part->offset, len);
+	return mtd_is_locked(part->parent, ofs + part->offset, len);
 }
 
 static void part_sync(struct mtd_info *mtd)
 {
 	struct mtd_part *part = mtd_to_part(mtd);
-	part->parent->_sync(part->parent);
+	mtd_sync(part->parent);
 }
 
 static int part_suspend(struct mtd_info *mtd)
 {
 	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_suspend(part->parent);
+	return mtd_suspend(part->parent);
 }
 
 static void part_resume(struct mtd_info *mtd)
 {
 	struct mtd_part *part = mtd_to_part(mtd);
-	part->parent->_resume(part->parent);
+	mtd_resume(part->parent);
 }
 
 static int part_block_isreserved(struct mtd_info *mtd, loff_t ofs)
 {
 	struct mtd_part *part = mtd_to_part(mtd);
 	ofs += part->offset;
-	return part->parent->_block_isreserved(part->parent, ofs);
+	return mtd_block_isreserved(part->parent, ofs);
 }
 
 static int part_block_isbad(struct mtd_info *mtd, loff_t ofs)
 {
 	struct mtd_part *part = mtd_to_part(mtd);
 	ofs += part->offset;
-	return part->parent->_block_isbad(part->parent, ofs);
+	return mtd_block_isbad(part->parent, ofs);
 }
 
 static int part_block_markbad(struct mtd_info *mtd, loff_t ofs)
@@ -304,7 +297,7 @@ static int part_block_markbad(struct mtd_info *mtd, loff_t ofs)
 	int res;
 
 	ofs += part->offset;
-	res = part->parent->_block_markbad(part->parent, ofs);
+	res = mtd_block_markbad(part->parent, ofs);
 	if (!res)
 		mtd->ecc_stats.badblocks++;
 	return res;
@@ -313,13 +306,13 @@ static int part_block_markbad(struct mtd_info *mtd, loff_t ofs)
 static int part_get_device(struct mtd_info *mtd)
 {
 	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_get_device(part->parent);
+	return __get_mtd_device(part->parent);
 }
 
 static void part_put_device(struct mtd_info *mtd)
 {
 	struct mtd_part *part = mtd_to_part(mtd);
-	part->parent->_put_device(part->parent);
+	__put_mtd_device(part->parent);
 }
 
 static int part_ooblayout_ecc(struct mtd_info *mtd, int section,
@@ -347,8 +340,7 @@ static int part_max_bad_blocks(struct mtd_info *mtd, loff_t ofs, size_t len)
 {
 	struct mtd_part *part = mtd_to_part(mtd);
 
-	return part->parent->_max_bad_blocks(part->parent,
-					     ofs + part->offset, len);
+	return mtd_max_bad_blocks(part->parent, ofs + part->offset, len);
 }
 
 static inline void free_partition(struct mtd_part *p)
@@ -437,59 +429,32 @@ static struct mtd_part *allocate_partition(struct mtd_info *parent,
 
 	slave->mtd._read = part_read;
 	slave->mtd._write = part_write;
-
-	if (parent->_panic_write)
-		slave->mtd._panic_write = part_panic_write;
-
-	if (parent->_point && parent->_unpoint) {
-		slave->mtd._point = part_point;
-		slave->mtd._unpoint = part_unpoint;
-	}
-
-	if (parent->_read_oob)
-		slave->mtd._read_oob = part_read_oob;
-	if (parent->_write_oob)
-		slave->mtd._write_oob = part_write_oob;
-	if (parent->_read_user_prot_reg)
-		slave->mtd._read_user_prot_reg = part_read_user_prot_reg;
-	if (parent->_read_fact_prot_reg)
-		slave->mtd._read_fact_prot_reg = part_read_fact_prot_reg;
-	if (parent->_write_user_prot_reg)
-		slave->mtd._write_user_prot_reg = part_write_user_prot_reg;
-	if (parent->_lock_user_prot_reg)
-		slave->mtd._lock_user_prot_reg = part_lock_user_prot_reg;
-	if (parent->_get_user_prot_info)
-		slave->mtd._get_user_prot_info = part_get_user_prot_info;
-	if (parent->_get_fact_prot_info)
-		slave->mtd._get_fact_prot_info = part_get_fact_prot_info;
-	if (parent->_sync)
-		slave->mtd._sync = part_sync;
-	if (!partno && !parent->dev.class && parent->_suspend &&
-	    parent->_resume) {
+	slave->mtd._panic_write = part_panic_write;
+	slave->mtd._point = part_point;
+	slave->mtd._unpoint = part_unpoint;
+	slave->mtd._read_oob = part_read_oob;
+	slave->mtd._write_oob = part_write_oob;
+	slave->mtd._read_user_prot_reg = part_read_user_prot_reg;
+	slave->mtd._read_fact_prot_reg = part_read_fact_prot_reg;
+	slave->mtd._write_user_prot_reg = part_write_user_prot_reg;
+	slave->mtd._lock_user_prot_reg = part_lock_user_prot_reg;
+	slave->mtd._get_user_prot_info = part_get_user_prot_info;
+	slave->mtd._get_fact_prot_info = part_get_fact_prot_info;
+	slave->mtd._sync = part_sync;
+	if (!partno && !parent->dev.class) {
 		slave->mtd._suspend = part_suspend;
 		slave->mtd._resume = part_resume;
 	}
-	if (parent->_writev)
-		slave->mtd._writev = part_writev;
-	if (parent->_lock)
-		slave->mtd._lock = part_lock;
-	if (parent->_unlock)
-		slave->mtd._unlock = part_unlock;
-	if (parent->_is_locked)
-		slave->mtd._is_locked = part_is_locked;
-	if (parent->_block_isreserved)
-		slave->mtd._block_isreserved = part_block_isreserved;
-	if (parent->_block_isbad)
-		slave->mtd._block_isbad = part_block_isbad;
-	if (parent->_block_markbad)
-		slave->mtd._block_markbad = part_block_markbad;
-	if (parent->_max_bad_blocks)
-		slave->mtd._max_bad_blocks = part_max_bad_blocks;
-
-	if (parent->_get_device)
-		slave->mtd._get_device = part_get_device;
-	if (parent->_put_device)
-		slave->mtd._put_device = part_put_device;
+	slave->mtd._writev = part_writev;
+	slave->mtd._lock = part_lock;
+	slave->mtd._unlock = part_unlock;
+	slave->mtd._is_locked = part_is_locked;
+	slave->mtd._block_isreserved = part_block_isreserved;
+	slave->mtd._block_isbad = part_block_isbad;
+	slave->mtd._block_markbad = part_block_markbad;
+	slave->mtd._max_bad_blocks = part_max_bad_blocks;
+	slave->mtd._get_device = part_get_device;
+	slave->mtd._put_device = part_put_device;
 
 	slave->mtd._erase = part_erase;
 	slave->parent = parent;
-- 
2.11.0




More information about the linux-mtd mailing list