[v2,1/2] mtd: Stop directly calling master ->_xxx() hooks from mtdpart code

Message ID 20170625160113.11860-1-boris.brezillon@free-electrons.com
State Changes Requested
Headers show

Commit Message

Boris Brezillon June 25, 2017, 4:01 p.m.
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@free-electrons.com>
---
Changes since v1:
- new patch needed to fix a NULL pointer dereference BUG
---
 drivers/mtd/mtdpart.c | 71 ++++++++++++++++++++++-----------------------------
 1 file changed, 31 insertions(+), 40 deletions(-)

Comments

Boris Brezillon June 25, 2017, 4:36 p.m. | #1
Le Sun, 25 Jun 2017 18:01:12 +0200,
Boris Brezillon <boris.brezillon@free-electrons.com> a écrit :

> 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.

Hm, this one is conflicting with Rafal's work on part parsers, but
before I send a v3 rebased on l2-mtd/master I'd like to have feedback
on the changes done here.

> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
> Changes since v1:
> - new patch needed to fix a NULL pointer dereference BUG
> ---
>  drivers/mtd/mtdpart.c | 71 ++++++++++++++++++++++-----------------------------
>  1 file changed, 31 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> index ea5e5307f667..11486ec6cab0 100644
> --- a/drivers/mtd/mtdpart.c
> +++ b/drivers/mtd/mtdpart.c
> @@ -68,8 +68,7 @@ static int part_read(struct mtd_info *mtd, loff_t from, size_t len,
>  	int res;
>  
>  	stats = part->master->ecc_stats;
> -	res = part->master->_read(part->master, from + part->offset, len,
> -				  retlen, buf);
> +	res = mtd_read(part->master, from + part->offset, len, retlen, buf);
>  	if (unlikely(mtd_is_eccerr(res)))
>  		mtd->ecc_stats.failed +=
>  			part->master->ecc_stats.failed - stats.failed;
> @@ -84,15 +83,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->master->_point(part->master, from + part->offset, len,
> -				    retlen, virt, phys);
> +	return mtd_point(part->master, 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->master->_unpoint(part->master, from + part->offset, len);
> +	return mtd_unpoint(part->master, from + part->offset, len);
>  }
>  
>  static unsigned long part_get_unmapped_area(struct mtd_info *mtd,
> @@ -103,8 +102,7 @@ static unsigned long part_get_unmapped_area(struct mtd_info *mtd,
>  	struct mtd_part *part = mtd_to_part(mtd);
>  
>  	offset += part->offset;
> -	return part->master->_get_unmapped_area(part->master, len, offset,
> -						flags);
> +	return mtd_get_unmapped_area(part->master, len, offset, flags);
>  }
>  
>  static int part_read_oob(struct mtd_info *mtd, loff_t from,
> @@ -132,7 +130,7 @@ static int part_read_oob(struct mtd_info *mtd, loff_t from,
>  			return -EINVAL;
>  	}
>  
> -	res = part->master->_read_oob(part->master, from + part->offset, ops);
> +	res = mtd_read_oob(part->master, from + part->offset, ops);
>  	if (unlikely(res)) {
>  		if (mtd_is_bitflip(res))
>  			mtd->ecc_stats.corrected++;
> @@ -146,48 +144,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->master->_read_user_prot_reg(part->master, from, len,
> -						 retlen, buf);
> +	return mtd_read_user_prot_reg(part->master, 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->master->_get_user_prot_info(part->master, len, retlen,
> -						 buf);
> +	return mtd_get_user_prot_info(part->master, 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->master->_read_fact_prot_reg(part->master, from, len,
> -						 retlen, buf);
> +	return mtd_read_fact_prot_reg(part->master, 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->master->_get_fact_prot_info(part->master, len, retlen,
> -						 buf);
> +	return mtd_get_fact_prot_info(part->master, 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->master->_write(part->master, to + part->offset, len,
> -				    retlen, buf);
> +	return mtd_write(part->master, 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->master->_panic_write(part->master, to + part->offset, len,
> -					  retlen, buf);
> +	return mtd_panic_write(part->master, to + part->offset, len, retlen,
> +			       buf);
>  }
>  
>  static int part_write_oob(struct mtd_info *mtd, loff_t to,
> @@ -199,30 +192,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->master->_write_oob(part->master, to + part->offset, ops);
> +	return mtd_write_oob(part->master, 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->master->_write_user_prot_reg(part->master, from, len,
> -						  retlen, buf);
> +	return mtd_write_user_prot_reg(part->master, 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->master->_lock_user_prot_reg(part->master, from, len);
> +	return mtd_lock_user_prot_reg(part->master, 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->master->_writev(part->master, vecs, count,
> -				     to + part->offset, retlen);
> +	return mtd_writev(part->master, vecs, count, to + part->offset,
> +			  retlen);
>  }
>  
>  static int part_erase(struct mtd_info *mtd, struct erase_info *instr)
> @@ -231,7 +223,7 @@ static int part_erase(struct mtd_info *mtd, struct erase_info *instr)
>  	int ret;
>  
>  	instr->addr += part->offset;
> -	ret = part->master->_erase(part->master, instr);
> +	ret = mtd_erase(part->master, instr);
>  	if (ret) {
>  		if (instr->fail_addr != MTD_FAIL_ADDR_UNKNOWN)
>  			instr->fail_addr -= part->offset;
> @@ -257,51 +249,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->master->_lock(part->master, ofs + part->offset, len);
> +	return mtd_lock(part->master, 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->master->_unlock(part->master, ofs + part->offset, len);
> +	return mtd_unlock(part->master, 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->master->_is_locked(part->master, ofs + part->offset, len);
> +	return mtd_is_locked(part->master, ofs + part->offset, len);
>  }
>  
>  static void part_sync(struct mtd_info *mtd)
>  {
>  	struct mtd_part *part = mtd_to_part(mtd);
> -	part->master->_sync(part->master);
> +	mtd_sync(part->master);
>  }
>  
>  static int part_suspend(struct mtd_info *mtd)
>  {
>  	struct mtd_part *part = mtd_to_part(mtd);
> -	return part->master->_suspend(part->master);
> +	return mtd_suspend(part->master);
>  }
>  
>  static void part_resume(struct mtd_info *mtd)
>  {
>  	struct mtd_part *part = mtd_to_part(mtd);
> -	part->master->_resume(part->master);
> +	mtd_resume(part->master);
>  }
>  
>  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->master->_block_isreserved(part->master, ofs);
> +	return mtd_block_isreserved(part->master, 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->master->_block_isbad(part->master, ofs);
> +	return mtd_block_isbad(part->master, ofs);
>  }
>  
>  static int part_block_markbad(struct mtd_info *mtd, loff_t ofs)
> @@ -310,7 +302,7 @@ static int part_block_markbad(struct mtd_info *mtd, loff_t ofs)
>  	int res;
>  
>  	ofs += part->offset;
> -	res = part->master->_block_markbad(part->master, ofs);
> +	res = mtd_block_markbad(part->master, ofs);
>  	if (!res)
>  		mtd->ecc_stats.badblocks++;
>  	return res;
> @@ -319,13 +311,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->master->_get_device(part->master);
> +	return __get_mtd_device(part->master);
>  }
>  
>  static void part_put_device(struct mtd_info *mtd)
>  {
>  	struct mtd_part *part = mtd_to_part(mtd);
> -	part->master->_put_device(part->master);
> +	__put_mtd_device(part->master);
>  }
>  
>  static int part_ooblayout_ecc(struct mtd_info *mtd, int section,
> @@ -353,8 +345,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->master->_max_bad_blocks(part->master,
> -					     ofs + part->offset, len);
> +	return mtd_max_bad_blocks(part->master, ofs + part->offset, len);
>  }
>  
>  static inline void free_partition(struct mtd_part *p)
Brian Norris June 27, 2017, 6:46 p.m. | #2
On Sun, Jun 25, 2017 at 06:01:12PM +0200, Boris Brezillon wrote:
> 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@free-electrons.com>
> ---
> Changes since v1:
> - new patch needed to fix a NULL pointer dereference BUG

I think I like this patch on the whole. A few notes below.

> ---
>  drivers/mtd/mtdpart.c | 71 ++++++++++++++++++++++-----------------------------
>  1 file changed, 31 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> index ea5e5307f667..11486ec6cab0 100644
> --- a/drivers/mtd/mtdpart.c
> +++ b/drivers/mtd/mtdpart.c
> @@ -68,8 +68,7 @@ static int part_read(struct mtd_info *mtd, loff_t from, size_t len,
>  	int res;
>  
>  	stats = part->master->ecc_stats;
> -	res = part->master->_read(part->master, from + part->offset, len,
> -				  retlen, buf);
> +	res = mtd_read(part->master, from + part->offset, len, retlen, buf);
>  	if (unlikely(mtd_is_eccerr(res)))
>  		mtd->ecc_stats.failed +=
>  			part->master->ecc_stats.failed - stats.failed;

The parent-to-child ECC stat handling is different here than it is
below, in part_read_oob().

...

> @@ -132,7 +130,7 @@ static int part_read_oob(struct mtd_info *mtd, loff_t from,
>  			return -EINVAL;
>  	}
>  
> -	res = part->master->_read_oob(part->master, from + part->offset, ops);
> +	res = mtd_read_oob(part->master, from + part->offset, ops);
>  	if (unlikely(res)) {
>  		if (mtd_is_bitflip(res))
>  			mtd->ecc_stats.corrected++;

Notice how we only count a single increment here.

That seems wrong, doesn't it? But then, do we guarantee that
->_read_oob() implementations follow the same stat recording guarantees
that ->_read() implementations do (or should)? Might double check that
before trying to fiddle here any more than you are.

I'm not sure if that affects your next patch at all. It's not a
criticism of this patch either, but I just noticed it when reviewing.

[...]

I was also wondering whether this patch couldn't go a step further, and
remove conditions like this:

	if (parent->_panic_write)
		slave->mtd._panic_write = part_panic_write; 

Since part_panic_write() should call mtd_panic_write() on the parent
(master), which would do its own -EOPNOTSUPP check. But then I suppose
that might invert the order of the checks, causing (for example) -EINVAL
for out-of-bounds panic write instead of -EOPNOTSUPP. So maybe that's
better left alone.

Brian
Boris Brezillon June 27, 2017, 7:19 p.m. | #3
Le Tue, 27 Jun 2017 11:46:14 -0700,
Brian Norris <computersforpeace@gmail.com> a écrit :

> On Sun, Jun 25, 2017 at 06:01:12PM +0200, Boris Brezillon wrote:
> > 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@free-electrons.com>
> > ---
> > Changes since v1:
> > - new patch needed to fix a NULL pointer dereference BUG  
> 
> I think I like this patch on the whole. A few notes below.

Ok, cool. I was afraid someone would complain about the induced
overhead.

> 
> > ---
> >  drivers/mtd/mtdpart.c | 71 ++++++++++++++++++++++-----------------------------
> >  1 file changed, 31 insertions(+), 40 deletions(-)
> > 
> > diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> > index ea5e5307f667..11486ec6cab0 100644
> > --- a/drivers/mtd/mtdpart.c
> > +++ b/drivers/mtd/mtdpart.c
> > @@ -68,8 +68,7 @@ static int part_read(struct mtd_info *mtd, loff_t from, size_t len,
> >  	int res;
> >  
> >  	stats = part->master->ecc_stats;
> > -	res = part->master->_read(part->master, from + part->offset, len,
> > -				  retlen, buf);
> > +	res = mtd_read(part->master, from + part->offset, len, retlen, buf);
> >  	if (unlikely(mtd_is_eccerr(res)))
> >  		mtd->ecc_stats.failed +=
> >  			part->master->ecc_stats.failed - stats.failed;  
> 
> The parent-to-child ECC stat handling is different here than it is
> below, in part_read_oob().
> 
> ...
> 
> > @@ -132,7 +130,7 @@ static int part_read_oob(struct mtd_info *mtd, loff_t from,
> >  			return -EINVAL;
> >  	}
> >  
> > -	res = part->master->_read_oob(part->master, from + part->offset, ops);
> > +	res = mtd_read_oob(part->master, from + part->offset, ops);
> >  	if (unlikely(res)) {
> >  		if (mtd_is_bitflip(res))
> >  			mtd->ecc_stats.corrected++;  
> 
> Notice how we only count a single increment here.
> 
> That seems wrong, doesn't it?

Indeed, it does, but this patch does not change the current
behavior ;-).

> But then, do we guarantee that
> ->_read_oob() implementations follow the same stat recording guarantees  
> that ->_read() implementations do (or should)? Might double check that
> before trying to fiddle here any more than you are.

Well, if you want my opinion, all this ecc_stats dance on MTD parts is
already fragile, because, AFAICT, nothing guarantees that someone else
is not reading another partition of the same device while we are
extracting the stats, which means that those numbers might already be
inconsistent when we do the calculation to extract the number of
bitflips and failures related to the partition. So, I'm not sure we
really care about these numbers.

If we want things to be perfectly reliable, we should return the stats
attached to a specific read request and then atomically update the
global stats (see the discussion I had we Sacha a while ago [1]).

Anyway, I'm fine making the logic consistent between part_read() and
part_read_oob() if you like, but that should be done in a separate
patch IMO.

> 
> I'm not sure if that affects your next patch at all. It's not a
> criticism of this patch either, but I just noticed it when reviewing.

Nope, because part_read() will still be used for the standard read path,
and the core will only fall back to ->_read_oob() when doing the read
on the master device.

> 
> [...]
> 
> I was also wondering whether this patch couldn't go a step further, and
> remove conditions like this:
> 
> 	if (parent->_panic_write)
> 		slave->mtd._panic_write = part_panic_write; 
> 
> Since part_panic_write() should call mtd_panic_write() on the parent
> (master), which would do its own -EOPNOTSUPP check. But then I suppose
> that might invert the order of the checks, causing (for example) -EINVAL
> for out-of-bounds panic write instead of -EOPNOTSUPP. So maybe that's
> better left alone.

I considered doing that but was too lazy to check if all helpers were
properly checking the pointer value before dereferencing it :).

> 
> Brian

[1]https://patchwork.ozlabs.org/patch/614493/
Boris Brezillon June 27, 2017, 7:52 p.m. | #4
Le Tue, 27 Jun 2017 21:19:38 +0200,
Boris Brezillon <boris.brezillon@free-electrons.com> a écrit :

> > 
> > [...]
> > 
> > I was also wondering whether this patch couldn't go a step further, and
> > remove conditions like this:
> > 
> > 	if (parent->_panic_write)
> > 		slave->mtd._panic_write = part_panic_write; 
> > 
> > Since part_panic_write() should call mtd_panic_write() on the parent
> > (master), which would do its own -EOPNOTSUPP check. But then I suppose
> > that might invert the order of the checks, causing (for example) -EINVAL
> > for out-of-bounds panic write instead of -EOPNOTSUPP. So maybe that's
> > better left alone.  
> 
> I considered doing that but was too lazy to check if all helpers were
> properly checking the pointer value before dereferencing it :).

I just checked, and it seems we can unconditionally set part hooks and
rely on default mtd_xxx() helpers to detect when the feature is not
supported by the master.

Patch

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index ea5e5307f667..11486ec6cab0 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -68,8 +68,7 @@  static int part_read(struct mtd_info *mtd, loff_t from, size_t len,
 	int res;
 
 	stats = part->master->ecc_stats;
-	res = part->master->_read(part->master, from + part->offset, len,
-				  retlen, buf);
+	res = mtd_read(part->master, from + part->offset, len, retlen, buf);
 	if (unlikely(mtd_is_eccerr(res)))
 		mtd->ecc_stats.failed +=
 			part->master->ecc_stats.failed - stats.failed;
@@ -84,15 +83,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->master->_point(part->master, from + part->offset, len,
-				    retlen, virt, phys);
+	return mtd_point(part->master, 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->master->_unpoint(part->master, from + part->offset, len);
+	return mtd_unpoint(part->master, from + part->offset, len);
 }
 
 static unsigned long part_get_unmapped_area(struct mtd_info *mtd,
@@ -103,8 +102,7 @@  static unsigned long part_get_unmapped_area(struct mtd_info *mtd,
 	struct mtd_part *part = mtd_to_part(mtd);
 
 	offset += part->offset;
-	return part->master->_get_unmapped_area(part->master, len, offset,
-						flags);
+	return mtd_get_unmapped_area(part->master, len, offset, flags);
 }
 
 static int part_read_oob(struct mtd_info *mtd, loff_t from,
@@ -132,7 +130,7 @@  static int part_read_oob(struct mtd_info *mtd, loff_t from,
 			return -EINVAL;
 	}
 
-	res = part->master->_read_oob(part->master, from + part->offset, ops);
+	res = mtd_read_oob(part->master, from + part->offset, ops);
 	if (unlikely(res)) {
 		if (mtd_is_bitflip(res))
 			mtd->ecc_stats.corrected++;
@@ -146,48 +144,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->master->_read_user_prot_reg(part->master, from, len,
-						 retlen, buf);
+	return mtd_read_user_prot_reg(part->master, 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->master->_get_user_prot_info(part->master, len, retlen,
-						 buf);
+	return mtd_get_user_prot_info(part->master, 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->master->_read_fact_prot_reg(part->master, from, len,
-						 retlen, buf);
+	return mtd_read_fact_prot_reg(part->master, 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->master->_get_fact_prot_info(part->master, len, retlen,
-						 buf);
+	return mtd_get_fact_prot_info(part->master, 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->master->_write(part->master, to + part->offset, len,
-				    retlen, buf);
+	return mtd_write(part->master, 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->master->_panic_write(part->master, to + part->offset, len,
-					  retlen, buf);
+	return mtd_panic_write(part->master, to + part->offset, len, retlen,
+			       buf);
 }
 
 static int part_write_oob(struct mtd_info *mtd, loff_t to,
@@ -199,30 +192,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->master->_write_oob(part->master, to + part->offset, ops);
+	return mtd_write_oob(part->master, 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->master->_write_user_prot_reg(part->master, from, len,
-						  retlen, buf);
+	return mtd_write_user_prot_reg(part->master, 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->master->_lock_user_prot_reg(part->master, from, len);
+	return mtd_lock_user_prot_reg(part->master, 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->master->_writev(part->master, vecs, count,
-				     to + part->offset, retlen);
+	return mtd_writev(part->master, vecs, count, to + part->offset,
+			  retlen);
 }
 
 static int part_erase(struct mtd_info *mtd, struct erase_info *instr)
@@ -231,7 +223,7 @@  static int part_erase(struct mtd_info *mtd, struct erase_info *instr)
 	int ret;
 
 	instr->addr += part->offset;
-	ret = part->master->_erase(part->master, instr);
+	ret = mtd_erase(part->master, instr);
 	if (ret) {
 		if (instr->fail_addr != MTD_FAIL_ADDR_UNKNOWN)
 			instr->fail_addr -= part->offset;
@@ -257,51 +249,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->master->_lock(part->master, ofs + part->offset, len);
+	return mtd_lock(part->master, 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->master->_unlock(part->master, ofs + part->offset, len);
+	return mtd_unlock(part->master, 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->master->_is_locked(part->master, ofs + part->offset, len);
+	return mtd_is_locked(part->master, ofs + part->offset, len);
 }
 
 static void part_sync(struct mtd_info *mtd)
 {
 	struct mtd_part *part = mtd_to_part(mtd);
-	part->master->_sync(part->master);
+	mtd_sync(part->master);
 }
 
 static int part_suspend(struct mtd_info *mtd)
 {
 	struct mtd_part *part = mtd_to_part(mtd);
-	return part->master->_suspend(part->master);
+	return mtd_suspend(part->master);
 }
 
 static void part_resume(struct mtd_info *mtd)
 {
 	struct mtd_part *part = mtd_to_part(mtd);
-	part->master->_resume(part->master);
+	mtd_resume(part->master);
 }
 
 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->master->_block_isreserved(part->master, ofs);
+	return mtd_block_isreserved(part->master, 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->master->_block_isbad(part->master, ofs);
+	return mtd_block_isbad(part->master, ofs);
 }
 
 static int part_block_markbad(struct mtd_info *mtd, loff_t ofs)
@@ -310,7 +302,7 @@  static int part_block_markbad(struct mtd_info *mtd, loff_t ofs)
 	int res;
 
 	ofs += part->offset;
-	res = part->master->_block_markbad(part->master, ofs);
+	res = mtd_block_markbad(part->master, ofs);
 	if (!res)
 		mtd->ecc_stats.badblocks++;
 	return res;
@@ -319,13 +311,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->master->_get_device(part->master);
+	return __get_mtd_device(part->master);
 }
 
 static void part_put_device(struct mtd_info *mtd)
 {
 	struct mtd_part *part = mtd_to_part(mtd);
-	part->master->_put_device(part->master);
+	__put_mtd_device(part->master);
 }
 
 static int part_ooblayout_ecc(struct mtd_info *mtd, int section,
@@ -353,8 +345,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->master->_max_bad_blocks(part->master,
-					     ofs + part->offset, len);
+	return mtd_max_bad_blocks(part->master, ofs + part->offset, len);
 }
 
 static inline void free_partition(struct mtd_part *p)