[v2,1/4] mtd: cfi_cmdset_0002: Use right chip in do_ppb_xxlock()

Message ID 20180606101330.11071-1-joakim.tjernlund@infinera.com
State New
Delegated to: Boris Brezillon
Headers show
Series
  • [v2,1/4] mtd: cfi_cmdset_0002: Use right chip in do_ppb_xxlock()
Related show

Commit Message

Joakim Tjernlund June 6, 2018, 10:13 a.m.
do_ppb_xxlock() fails to add chip->start when
quering for lock status(and chip_ready test),
which caused false status reports.
Fix by adding adr += chip->start and adjust call sites accordingly.

Fixes: 1648eaaa1575 ("mtd: cfi_cmdset_0002: Support Persistent Protection Bits (PPB) locking")
Cc: stable@vger.kernel.org
Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
---

 v2 - Spilt into several patches
 
 drivers/mtd/chips/cfi_cmdset_0002.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Boris Brezillon June 20, 2018, 9:03 a.m. | #1
On Wed,  6 Jun 2018 12:13:27 +0200
Joakim Tjernlund <joakim.tjernlund@infinera.com> wrote:

> do_ppb_xxlock() fails to add chip->start when
> quering for lock status(and chip_ready test),

  ^ querying?

> which caused false status reports.

The 3 above lines are wrapped at less than 50 chars, is this normal?

> Fix by adding adr += chip->start and adjust call sites accordingly.
> 
> Fixes: 1648eaaa1575 ("mtd: cfi_cmdset_0002: Support Persistent Protection Bits (PPB) locking")
> Cc: stable@vger.kernel.org
> Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> ---
> 
>  v2 - Spilt into several patches
>  
>  drivers/mtd/chips/cfi_cmdset_0002.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
> index 53a976a8e614..8648b1adccd5 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -2554,8 +2554,9 @@ static int __maybe_unused do_ppb_xxlock(struct map_info *map,
>  	unsigned long timeo;
>  	int ret;
>  
> +	adr += chip->start;
>  	mutex_lock(&chip->mutex);
> -	ret = get_chip(map, chip, adr + chip->start, FL_LOCKING);
> +	ret = get_chip(map, chip, adr, FL_LOCKING);
>  	if (ret) {
>  		mutex_unlock(&chip->mutex);
>  		return ret;
> @@ -2573,8 +2574,8 @@ static int __maybe_unused do_ppb_xxlock(struct map_info *map,
>  
>  	if (thunk == DO_XXLOCK_ONEBLOCK_LOCK) {
>  		chip->state = FL_LOCKING;
> -		map_write(map, CMD(0xA0), chip->start + adr);
> -		map_write(map, CMD(0x00), chip->start + adr);
> +		map_write(map, CMD(0xA0), adr);
> +		map_write(map, CMD(0x00), adr);
>  	} else if (thunk == DO_XXLOCK_ONEBLOCK_UNLOCK) {
>  		/*
>  		 * Unlocking of one specific sector is not supported, so we
> @@ -2612,7 +2613,7 @@ static int __maybe_unused do_ppb_xxlock(struct map_info *map,
>  	map_write(map, CMD(0x00), chip->start);
>  
>  	chip->state = FL_READY;
> -	put_chip(map, chip, adr + chip->start);
> +	put_chip(map, chip, adr);
>  	mutex_unlock(&chip->mutex);
>  
>  	return ret;
Joakim Tjernlund June 20, 2018, 11:10 a.m. | #2
On Wed, 2018-06-20 at 11:03 +0200, Boris Brezillon wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> 
> On Wed,  6 Jun 2018 12:13:27 +0200
> Joakim Tjernlund <joakim.tjernlund@infinera.com> wrote:
> 
> > do_ppb_xxlock() fails to add chip->start when
> > quering for lock status(and chip_ready test),
> 
>   ^ querying?
> 
> > which caused false status reports.
> 
> The 3 above lines are wrapped at less than 50 chars, is this normal?

I actually hit return every now and then when I type, sometimes the
lines might become a bit short, but yes, this is normal for me.

> 
> > Fix by adding adr += chip->start and adjust call sites accordingly.
> > 
> > Fixes: 1648eaaa1575 ("mtd: cfi_cmdset_0002: Support Persistent Protection Bits (PPB) locking")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> > ---
> > 
> >  v2 - Spilt into several patches
> > 
> >  drivers/mtd/chips/cfi_cmdset_0002.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
> > index 53a976a8e614..8648b1adccd5 100644
> > --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> > +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> > @@ -2554,8 +2554,9 @@ static int __maybe_unused do_ppb_xxlock(struct map_info *map,
> >       unsigned long timeo;
> >       int ret;
> > 
> > +     adr += chip->start;
> >       mutex_lock(&chip->mutex);
> > -     ret = get_chip(map, chip, adr + chip->start, FL_LOCKING);
> > +     ret = get_chip(map, chip, adr, FL_LOCKING);
> >       if (ret) {
> >               mutex_unlock(&chip->mutex);
> >               return ret;
> > @@ -2573,8 +2574,8 @@ static int __maybe_unused do_ppb_xxlock(struct map_info *map,
> > 
> >       if (thunk == DO_XXLOCK_ONEBLOCK_LOCK) {
> >               chip->state = FL_LOCKING;
> > -             map_write(map, CMD(0xA0), chip->start + adr);
> > -             map_write(map, CMD(0x00), chip->start + adr);
> > +             map_write(map, CMD(0xA0), adr);
> > +             map_write(map, CMD(0x00), adr);
> >       } else if (thunk == DO_XXLOCK_ONEBLOCK_UNLOCK) {
> >               /*
> >                * Unlocking of one specific sector is not supported, so we
> > @@ -2612,7 +2613,7 @@ static int __maybe_unused do_ppb_xxlock(struct map_info *map,
> >       map_write(map, CMD(0x00), chip->start);
> > 
> >       chip->state = FL_READY;
> > -     put_chip(map, chip, adr + chip->start);
> > +     put_chip(map, chip, adr);
> >       mutex_unlock(&chip->mutex);
> > 
> >       return ret;
> 
>

Patch

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index 53a976a8e614..8648b1adccd5 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -2554,8 +2554,9 @@  static int __maybe_unused do_ppb_xxlock(struct map_info *map,
 	unsigned long timeo;
 	int ret;
 
+	adr += chip->start;
 	mutex_lock(&chip->mutex);
-	ret = get_chip(map, chip, adr + chip->start, FL_LOCKING);
+	ret = get_chip(map, chip, adr, FL_LOCKING);
 	if (ret) {
 		mutex_unlock(&chip->mutex);
 		return ret;
@@ -2573,8 +2574,8 @@  static int __maybe_unused do_ppb_xxlock(struct map_info *map,
 
 	if (thunk == DO_XXLOCK_ONEBLOCK_LOCK) {
 		chip->state = FL_LOCKING;
-		map_write(map, CMD(0xA0), chip->start + adr);
-		map_write(map, CMD(0x00), chip->start + adr);
+		map_write(map, CMD(0xA0), adr);
+		map_write(map, CMD(0x00), adr);
 	} else if (thunk == DO_XXLOCK_ONEBLOCK_UNLOCK) {
 		/*
 		 * Unlocking of one specific sector is not supported, so we
@@ -2612,7 +2613,7 @@  static int __maybe_unused do_ppb_xxlock(struct map_info *map,
 	map_write(map, CMD(0x00), chip->start);
 
 	chip->state = FL_READY;
-	put_chip(map, chip, adr + chip->start);
+	put_chip(map, chip, adr);
 	mutex_unlock(&chip->mutex);
 
 	return ret;