[PATCH 00/18] MTD-CFI: Fine-tuning for several function implementations

[PATCH 00/18] MTD-CFI: Fine-tuning for several function implementations

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 11 Jan 2017 21:21:12 +0100

Several update suggestions were taken into account
from static source code analysis.

Markus Elfring (18):
  Use kmalloc_array() in cfi_intelext_partition_fixup()
  Improve another size determination in cfi_intelext_partition_fixup()
  cfi_cmdset_0001: Add some spaces for better code readability
  cfi_cmdset_0001: Delete an unnecessary variable initialisation in do_write_oneword()
  cfi_cmdset_0001: Use common error handling code in do_write_oneword()
  Use kcalloc() in cfi_intelext_setup()
  One function call and an unnecessary check less in cfi_intelext_setup()
  Rename a jump label in cfi_intelext_setup()
  Use kmalloc_array() in cfi_amdstd_setup()
  One function call less in cfi_amdstd_setup() after error detection
  Rename a jump label in cfi_amdstd_setup()
  cfi_cmdset_0002: Add some spaces for better code readability
  cfi_cmdset_0002: Delete an unnecessary variable initialisation in do_write_oneword()
  cfi_cmdset_0002: Use common error handling code in do_write_oneword()
  Use kcalloc() in cfi_ppb_unlock()
  Use kmalloc_array() in cfi_staa_setup()
  Use common error handling code in cfi_staa_setup()
  cfi_cmdset_0020: Add some spaces for better code readability

 drivers/mtd/chips/cfi_cmdset_0001.c | 165 +++++++++++++++++++-----------------
 drivers/mtd/chips/cfi_cmdset_0002.c | 110 +++++++++++++-----------
 drivers/mtd/chips/cfi_cmdset_0020.c | 127 ++++++++++++++-------------
 3 files changed, 214 insertions(+), 188 deletions(-)

-- 
2.11.0

[PATCH 01/18] mtd-cfi_cmdset_0001: Use kmalloc_array() in cfi_intelext_partition_fixup()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 10 Jan 2017 18:48:34 +0100

* A multiplication for the size determination of a memory allocation
  indicated that an array data structure should be processed.
  Thus use the corresponding function "kmalloc_array".

  This issue was detected by using the Coccinelle software.

* Replace the specification of a data structure by a pointer dereference
  to make the corresponding size determination a bit safer according to
  the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mtd/chips/cfi_cmdset_0001.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
index 5e1b68cbcd0a..68227aa74d22 100644
--- a/drivers/mtd/chips/cfi_cmdset_0001.c
+++ b/drivers/mtd/chips/cfi_cmdset_0001.c
@@ -746,7 +746,9 @@ static int cfi_intelext_partition_fixup(struct mtd_info *mtd,
 		newcfi = kmalloc(sizeof(struct cfi_private) + numvirtchips * sizeof(struct flchip), GFP_KERNEL);
 		if (!newcfi)
 			return -ENOMEM;
-		shared = kmalloc(sizeof(struct flchip_shared) * cfi->numchips, GFP_KERNEL);
+		shared = kmalloc_array(cfi->numchips,
+				       sizeof(*shared),
+				       GFP_KERNEL);
 		if (!shared) {
 			kfree(newcfi);
 			return -ENOMEM;
-- 
2.11.0

Re: [PATCH 01/18] mtd-cfi_cmdset_0001: Use kmalloc_array() in cfi_intelext_partition_fixup()

[Marek Vasut]

On 01/11/2017 09:35 PM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 10 Jan 2017 18:48:34 +0100
> 
> * A multiplication for the size determination of a memory allocation
>   indicated that an array data structure should be processed.
>   Thus use the corresponding function "kmalloc_array".
> 
>   This issue was detected by using the Coccinelle software.
> 
> * Replace the specification of a data structure by a pointer dereference
>   to make the corresponding size determination a bit safer according to
>   the Linux coding style convention.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/mtd/chips/cfi_cmdset_0001.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
> index 5e1b68cbcd0a..68227aa74d22 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0001.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0001.c
> @@ -746,7 +746,9 @@ static int cfi_intelext_partition_fixup(struct mtd_info *mtd,
>  		newcfi = kmalloc(sizeof(struct cfi_private) + numvirtchips * sizeof(struct flchip), GFP_KERNEL);
>  		if (!newcfi)
>  			return -ENOMEM;
> -		shared = kmalloc(sizeof(struct flchip_shared) * cfi->numchips, GFP_KERNEL);
> +		shared = kmalloc_array(cfi->numchips,
> +				       sizeof(*shared),
> +				       GFP_KERNEL);

You can keep this on a single line, no need for the linebreak. It
doesn't violate the 80-chars per line rule IMO.

>  		if (!shared) {
>  			kfree(newcfi);
>  			return -ENOMEM;
> 


-- 
Best regards,
Marek Vasut

[PATCH 02/18] mtd-cfi_cmdset_0001: Improve another size determination in cfi_intelext_partition_fixup()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 10 Jan 2017 19:09:58 +0100

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mtd/chips/cfi_cmdset_0001.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
index 68227aa74d22..0d7c2ef533d1 100644
--- a/drivers/mtd/chips/cfi_cmdset_0001.c
+++ b/drivers/mtd/chips/cfi_cmdset_0001.c
@@ -753,7 +753,7 @@ static int cfi_intelext_partition_fixup(struct mtd_info *mtd,
 			kfree(newcfi);
 			return -ENOMEM;
 		}
-		memcpy(newcfi, cfi, sizeof(struct cfi_private));
+		memcpy(newcfi, cfi, sizeof(*cfi));
 		newcfi->numchips = numvirtchips;
 		newcfi->chipshift = partshift;
 
-- 
2.11.0

Re: [PATCH 02/18] mtd-cfi_cmdset_0001: Improve another size determination in cfi_intelext_partition_fixup()

[Marek Vasut]

On 01/11/2017 09:37 PM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 10 Jan 2017 19:09:58 +0100

Please drop this stuff ^^^^^ git send-email should do that for you.

> Replace the specification of a data structure by a pointer dereference
> as the parameter for the operator "sizeof" to make the corresponding size
> determination a bit safer according to the Linux coding style convention.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/mtd/chips/cfi_cmdset_0001.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
> index 68227aa74d22..0d7c2ef533d1 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0001.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0001.c
> @@ -753,7 +753,7 @@ static int cfi_intelext_partition_fixup(struct mtd_info *mtd,
>  			kfree(newcfi);
>  			return -ENOMEM;
>  		}
> -		memcpy(newcfi, cfi, sizeof(struct cfi_private));
> +		memcpy(newcfi, cfi, sizeof(*cfi));
>  		newcfi->numchips = numvirtchips;
>  		newcfi->chipshift = partshift;
>  
> 


-- 
Best regards,
Marek Vasut

[PATCH 03/18] mtd-cfi_cmdset_0001: Add some spaces for better code readability

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 11 Jan 2017 16:34:12 +0100

Use space characters at some source code places according to
the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mtd/chips/cfi_cmdset_0001.c | 132 +++++++++++++++++++-----------------
 1 file changed, 70 insertions(+), 62 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
index 0d7c2ef533d1..0bcb785305d6 100644
--- a/drivers/mtd/chips/cfi_cmdset_0001.c
+++ b/drivers/mtd/chips/cfi_cmdset_0001.c
@@ -131,30 +131,30 @@ static void cfi_tell_features(struct cfi_pri_intelext *extp)
 	printk("     - Synchronous read:        %s\n", extp->FeatureSupport&256?"supported":"unsupported");
 	printk("     - Simultaneous operations: %s\n", extp->FeatureSupport&512?"supported":"unsupported");
 	printk("     - Extended Flash Array:    %s\n", extp->FeatureSupport&1024?"supported":"unsupported");
-	for (i=11; i<32; i++) {
-		if (extp->FeatureSupport & (1<<i))
+	for (i = 11; i < 32; i++) {
+		if (extp->FeatureSupport & (1 << i))
 			printk("     - Unknown Bit %X:      supported\n", i);
 	}
 
 	printk("  Supported functions after Suspend: %2.2X\n", extp->SuspendCmdSupport);
 	printk("     - Program after Erase Suspend: %s\n", extp->SuspendCmdSupport&1?"supported":"unsupported");
-	for (i=1; i<8; i++) {
-		if (extp->SuspendCmdSupport & (1<<i))
+	for (i = 1; i < 8; i++) {
+		if (extp->SuspendCmdSupport & (1 << i))
 			printk("     - Unknown Bit %X:               supported\n", i);
 	}
 
 	printk("  Block Status Register Mask: %4.4X\n", extp->BlkStatusRegMask);
 	printk("     - Lock Bit Active:      %s\n", extp->BlkStatusRegMask&1?"yes":"no");
 	printk("     - Lock-Down Bit Active: %s\n", extp->BlkStatusRegMask&2?"yes":"no");
-	for (i=2; i<3; i++) {
-		if (extp->BlkStatusRegMask & (1<<i))
-			printk("     - Unknown Bit %X Active: yes\n",i);
+	for (i = 2; i < 3; i++) {
+		if (extp->BlkStatusRegMask & (1 << i))
+			printk("     - Unknown Bit %X Active: yes\n", i);
 	}
 	printk("     - EFA Lock Bit:         %s\n", extp->BlkStatusRegMask&16?"yes":"no");
 	printk("     - EFA Lock-Down Bit:    %s\n", extp->BlkStatusRegMask&32?"yes":"no");
-	for (i=6; i<16; i++) {
-		if (extp->BlkStatusRegMask & (1<<i))
-			printk("     - Unknown Bit %X Active: yes\n",i);
+	for (i = 6; i < 16; i++) {
+		if (extp->BlkStatusRegMask & (1 << i))
+			printk("     - Unknown Bit %X Active: yes\n", i);
 	}
 
 	printk("  Vcc Logic Supply Optimum Program/Erase Voltage: %d.%d V\n",
@@ -185,19 +185,19 @@ static void fixup_convert_atmel_pri(struct mtd_info *mtd)
 	printk(KERN_ERR "atmel Features: %02x\n", atmel_pri.Features);
 
 	if (atmel_pri.Features & 0x01) /* chip erase supported */
-		features |= (1<<0);
+		features |= (1 << 0);
 	if (atmel_pri.Features & 0x02) /* erase suspend supported */
-		features |= (1<<1);
+		features |= (1 << 1);
 	if (atmel_pri.Features & 0x04) /* program suspend supported */
-		features |= (1<<2);
+		features |= (1 << 2);
 	if (atmel_pri.Features & 0x08) /* simultaneous operations supported */
-		features |= (1<<9);
+		features |= (1 << 9);
 	if (atmel_pri.Features & 0x20) /* page mode read supported */
-		features |= (1<<7);
+		features |= (1 << 7);
 	if (atmel_pri.Features & 0x40) /* queued erase supported */
-		features |= (1<<4);
+		features |= (1 << 4);
 	if (atmel_pri.Features & 0x80) /* Protection bits supported */
-		features |= (1<<6);
+		features |= (1 << 6);
 
 	extp->FeatureSupport = features;
 
@@ -439,7 +439,8 @@ read_pri_intelext(struct map_info *map, __u16 adr)
 			extra_size += sizeof(*rinfo);
 			if (extp_size < sizeof(*extp) + extra_size)
 				goto need_more;
-			rinfo->NumIdentPartitions=le16_to_cpu(rinfo->NumIdentPartitions);
+			rinfo->NumIdentPartitions
+				= le16_to_cpu(rinfo->NumIdentPartitions);
 			extra_size += (rinfo->NumBlockTypes - 1)
 				      * sizeof(struct cfi_intelext_blockinfo);
 		}
@@ -499,7 +500,7 @@ struct mtd_info *cfi_cmdset_0001(struct map_info *map, int primary)
 		 * routine faked a CFI structure. So we read the feature
 		 * table from it.
 		 */
-		__u16 adr = primary?cfi->cfiq->P_ADR:cfi->cfiq->A_ADR;
+		__u16 adr = primary ? cfi->cfiq->P_ADR : cfi->cfiq->A_ADR;
 		struct cfi_pri_intelext *extp;
 
 		extp = read_pri_intelext(map, adr);
@@ -518,7 +519,7 @@ struct mtd_info *cfi_cmdset_0001(struct map_info *map, int primary)
 		cfi_tell_features(extp);
 #endif
 
-		if(extp->SuspendCmdSupport & 1) {
+		if (extp->SuspendCmdSupport & 1) {
 			printk(KERN_NOTICE "cfi_cmdset_0001: Erase suspend on write enabled\n");
 		}
 	}
@@ -529,43 +530,43 @@ struct mtd_info *cfi_cmdset_0001(struct map_info *map, int primary)
 	/* Apply generic fixups */
 	cfi_fixup(mtd, fixup_table);
 
-	for (i=0; i< cfi->numchips; i++) {
+	for (i = 0; i < cfi->numchips; i++) {
 		if (cfi->cfiq->WordWriteTimeoutTyp)
 			cfi->chips[i].word_write_time =
-				1<<cfi->cfiq->WordWriteTimeoutTyp;
+				1 << cfi->cfiq->WordWriteTimeoutTyp;
 		else
 			cfi->chips[i].word_write_time = 50000;
 
 		if (cfi->cfiq->BufWriteTimeoutTyp)
 			cfi->chips[i].buffer_write_time =
-				1<<cfi->cfiq->BufWriteTimeoutTyp;
+				1 << cfi->cfiq->BufWriteTimeoutTyp;
 		/* No default; if it isn't specified, we won't use it */
 
 		if (cfi->cfiq->BlockEraseTimeoutTyp)
 			cfi->chips[i].erase_time =
-				1000<<cfi->cfiq->BlockEraseTimeoutTyp;
+				1000 << cfi->cfiq->BlockEraseTimeoutTyp;
 		else
 			cfi->chips[i].erase_time = 2000000;
 
 		if (cfi->cfiq->WordWriteTimeoutTyp &&
 		    cfi->cfiq->WordWriteTimeoutMax)
 			cfi->chips[i].word_write_time_max =
-				1<<(cfi->cfiq->WordWriteTimeoutTyp +
-				    cfi->cfiq->WordWriteTimeoutMax);
+				1 << (cfi->cfiq->WordWriteTimeoutTyp
+				     + cfi->cfiq->WordWriteTimeoutMax);
 		else
 			cfi->chips[i].word_write_time_max = 50000 * 8;
 
 		if (cfi->cfiq->BufWriteTimeoutTyp &&
 		    cfi->cfiq->BufWriteTimeoutMax)
 			cfi->chips[i].buffer_write_time_max =
-				1<<(cfi->cfiq->BufWriteTimeoutTyp +
-				    cfi->cfiq->BufWriteTimeoutMax);
+				1 << (cfi->cfiq->BufWriteTimeoutTyp
+				     + cfi->cfiq->BufWriteTimeoutMax);
 
 		if (cfi->cfiq->BlockEraseTimeoutTyp &&
 		    cfi->cfiq->BlockEraseTimeoutMax)
 			cfi->chips[i].erase_time_max =
-				1000<<(cfi->cfiq->BlockEraseTimeoutTyp +
-				       cfi->cfiq->BlockEraseTimeoutMax);
+				1000 << (cfi->cfiq->BlockEraseTimeoutTyp
+					+ cfi->cfiq->BlockEraseTimeoutMax);
 		else
 			cfi->chips[i].erase_time_max = 2000000 * 8;
 
@@ -588,8 +589,8 @@ static struct mtd_info *cfi_intelext_setup(struct mtd_info *mtd)
 	struct map_info *map = mtd->priv;
 	struct cfi_private *cfi = map->fldrv_priv;
 	unsigned long offset = 0;
-	int i,j;
-	unsigned long devsize = (1<<cfi->cfiq->DevSize) * cfi->interleave;
+	int i, j;
+	unsigned long devsize = (1 << cfi->cfiq->DevSize) * cfi->interleave;
 
 	//printk(KERN_DEBUG "number of CFI chips: %d\n", cfi->numchips);
 
@@ -601,7 +602,7 @@ static struct mtd_info *cfi_intelext_setup(struct mtd_info *mtd)
 	if (!mtd->eraseregions)
 		goto setup_err;
 
-	for (i=0; i<cfi->cfiq->NumEraseRegions; i++) {
+	for (i = 0; i < cfi->cfiq->NumEraseRegions; i++) {
 		unsigned long ernum, ersize;
 		ersize = ((cfi->cfiq->EraseRegionInfo[i] >> 8) & ~0xff) * cfi->interleave;
 		ernum = (cfi->cfiq->EraseRegionInfo[i] & 0xffff) + 1;
@@ -609,12 +610,15 @@ static struct mtd_info *cfi_intelext_setup(struct mtd_info *mtd)
 		if (mtd->erasesize < ersize) {
 			mtd->erasesize = ersize;
 		}
-		for (j=0; j<cfi->numchips; j++) {
-			mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].offset = (j*devsize)+offset;
-			mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].erasesize = ersize;
-			mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].numblocks = ernum;
-			mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].lockmap = kmalloc(ernum / 8 + 1, GFP_KERNEL);
-			if (!mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].lockmap)
+		for (j = 0; j < cfi->numchips; j++) {
+			unsigned int const x = j * cfi->cfiq->NumEraseRegions
+					       + i;
+
+			mtd->eraseregions[x].offset = j * devsize + offset;
+			mtd->eraseregions[x].erasesize = ersize;
+			mtd->eraseregions[x].numblocks = ernum;
+			mtd->eraseregions[x].lockmap = kmalloc(ernum / 8 + 1, GFP_KERNEL);
+			if (!mtd->eraseregions[x].lockmap)
 				goto setup_err;
 		}
 		offset += (ersize * ernum);
@@ -626,9 +630,9 @@ static struct mtd_info *cfi_intelext_setup(struct mtd_info *mtd)
 		goto setup_err;
 	}
 
-	for (i=0; i<mtd->numeraseregions;i++){
+	for (i = 0; i < mtd->numeraseregions; i++) {
 		printk(KERN_DEBUG "erase region %d: offset=0x%llx,size=0x%x,blocks=%d\n",
-		       i,(unsigned long long)mtd->eraseregions[i].offset,
+		       i, (unsigned long long)mtd->eraseregions[i].offset,
 		       mtd->eraseregions[i].erasesize,
 		       mtd->eraseregions[i].numblocks);
 	}
@@ -653,9 +657,12 @@ static struct mtd_info *cfi_intelext_setup(struct mtd_info *mtd)
 
  setup_err:
 	if (mtd->eraseregions)
-		for (i=0; i<cfi->cfiq->NumEraseRegions; i++)
-			for (j=0; j<cfi->numchips; j++)
-				kfree(mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].lockmap);
+		for (i = 0; i < cfi->cfiq->NumEraseRegions; i++)
+			for (j = 0; j < cfi->numchips; j++)
+				kfree(mtd->eraseregions[j
+							* cfi->cfiq
+							  ->NumEraseRegions
+							+ i].lockmap);
 	kfree(mtd->eraseregions);
 	kfree(mtd);
 	kfree(cfi->cmdset_priv);
@@ -775,8 +782,9 @@ static int cfi_intelext_partition_fixup(struct mtd_info *mtd,
 
 		printk(KERN_DEBUG "%s: %d set(s) of %d interleaved chips "
 				  "--> %d partitions of %d KiB\n",
-				  map->name, cfi->numchips, cfi->interleave,
-				  newcfi->numchips, 1<<(newcfi->chipshift-10));
+		       map->name, cfi->numchips, cfi->interleave,
+		       newcfi->numchips,
+		       1 << (newcfi->chipshift - 10));
 
 		map->fldrv_priv = newcfi;
 		*pcfi = newcfi;
@@ -1032,7 +1040,7 @@ static void put_chip(struct map_info *map, struct flchip *chip, unsigned long ad
 		mutex_unlock(&shared->lock);
 	}
 
-	switch(chip->oldstate) {
+	switch (chip->oldstate) {
 	case FL_ERASING:
 		/* What if one interleaved chip has finished and the
 		   other hasn't? The old code would leave the finished
@@ -1387,8 +1395,8 @@ static int cfi_intelext_point(struct mtd_info *mtd, loff_t from, size_t len,
 		else if (cfi->chips[chipnum].start != last_end)
 			break;
 
-		if ((len + ofs -1) >> cfi->chipshift)
-			thislen = (1<<cfi->chipshift) - ofs;
+		if ((len + ofs - 1) >> cfi->chipshift)
+			thislen = (1 << cfi->chipshift) - ofs;
 		else
 			thislen = len;
 
@@ -1427,15 +1435,15 @@ static int cfi_intelext_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
 		if (chipnum >= cfi->numchips)
 			break;
 
-		if ((len + ofs -1) >> cfi->chipshift)
-			thislen = (1<<cfi->chipshift) - ofs;
+		if ((len + ofs - 1) >> cfi->chipshift)
+			thislen = (1 << cfi->chipshift) - ofs;
 		else
 			thislen = len;
 
 		mutex_lock(&chip->mutex);
 		if (chip->state == FL_POINT) {
 			chip->ref_point_counter--;
-			if(chip->ref_point_counter == 0)
+			if (chip->ref_point_counter == 0)
 				chip->state = FL_READY;
 		} else {
 			printk(KERN_ERR "%s: Error: unpoint called on non pointed region\n", map->name);
@@ -1503,8 +1511,8 @@ static int cfi_intelext_read (struct mtd_info *mtd, loff_t from, size_t len, siz
 		if (chipnum >= cfi->numchips)
 			break;
 
-		if ((len + ofs -1) >> cfi->chipshift)
-			thislen = (1<<cfi->chipshift) - ofs;
+		if ((len + ofs - 1) >> cfi->chipshift)
+			thislen = (1 << cfi->chipshift) - ofs;
 		else
 			thislen = len;
 
@@ -1637,7 +1645,7 @@ static int cfi_intelext_write_words (struct mtd_info *mtd, loff_t to , size_t le
 		}
 	}
 
-	while(len >= map_bankwidth(map)) {
+	while (len >= map_bankwidth(map)) {
 		map_word datum = map_word_load(map, buf);
 
 		ret = do_write_oneword(map, &cfi->chips[chipnum],
@@ -2019,7 +2027,7 @@ static void cfi_intelext_sync (struct mtd_info *mtd)
 	struct flchip *chip;
 	int ret = 0;
 
-	for (i=0; !ret && i<cfi->numchips; i++) {
+	for (i = 0; !ret && i < cfi->numchips; i++) {
 		chip = &cfi->chips[i];
 
 		mutex_lock(&chip->mutex);
@@ -2038,7 +2046,7 @@ static void cfi_intelext_sync (struct mtd_info *mtd)
 
 	/* Unlock the chips again */
 
-	for (i--; i >=0; i--) {
+	for (i--; i >= 0; i--) {
 		chip = &cfi->chips[i];
 
 		mutex_lock(&chip->mutex);
@@ -2474,7 +2482,7 @@ static void cfi_intelext_save_locks(struct mtd_info *mtd)
 		if (!region->lockmap)
 			continue;
 
-		for (block = 0; block < region->numblocks; block++){
+		for (block = 0; block < region->numblocks; block++) {
 			len = region->erasesize;
 			adr = region->offset + block * len;
 
@@ -2501,7 +2509,7 @@ static int cfi_intelext_suspend(struct mtd_info *mtd)
 	    && extp && (extp->FeatureSupport & (1 << 5)))
 		cfi_intelext_save_locks(mtd);
 
-	for (i=0; !ret && i<cfi->numchips; i++) {
+	for (i = 0; !ret && i < cfi->numchips; i++) {
 		chip = &cfi->chips[i];
 
 		mutex_lock(&chip->mutex);
@@ -2542,7 +2550,7 @@ static int cfi_intelext_suspend(struct mtd_info *mtd)
 	/* Unlock the chips again */
 
 	if (ret) {
-		for (i--; i >=0; i--) {
+		for (i--; i >= 0; i--) {
 			chip = &cfi->chips[i];
 
 			mutex_lock(&chip->mutex);
@@ -2590,7 +2598,7 @@ static void cfi_intelext_resume(struct mtd_info *mtd)
 	int i;
 	struct flchip *chip;
 
-	for (i=0; i<cfi->numchips; i++) {
+	for (i = 0; i < cfi->numchips; i++) {
 
 		chip = &cfi->chips[i];
 
@@ -2619,7 +2627,7 @@ static int cfi_intelext_reset(struct mtd_info *mtd)
 	struct cfi_private *cfi = map->fldrv_priv;
 	int i, ret;
 
-	for (i=0; i < cfi->numchips; i++) {
+	for (i = 0; i < cfi->numchips; i++) {
 		struct flchip *chip = &cfi->chips[i];
 
 		/* force the completion of any ongoing operation
-- 
2.11.0

Re: [PATCH 03/18] mtd-cfi_cmdset_0001: Add some spaces for better code readability

[Dan Carpenter]

On Wed, Jan 11, 2017 at 09:38:18PM +0100, SF Markus Elfring wrote:
> -		for (i=0; i<cfi->cfiq->NumEraseRegions; i++)
> -			for (j=0; j<cfi->numchips; j++)
> -				kfree(mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].lockmap);
> +		for (i = 0; i < cfi->cfiq->NumEraseRegions; i++)
> +			for (j = 0; j < cfi->numchips; j++)
> +				kfree(mtd->eraseregions[j
> +							* cfi->cfiq
> +							  ->NumEraseRegions
> +							+ i].lockmap);

No.

regards,
dan carpenter

[PATCH 04/18] mtd-cfi_cmdset_0001: Delete an unnecessary variable initialisation in do_write_oneword()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 11 Jan 2017 16:36:18 +0100

The local variable "ret" will be set to an appropriate value a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mtd/chips/cfi_cmdset_0001.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
index 0bcb785305d6..0c0fa918c9ff 100644
--- a/drivers/mtd/chips/cfi_cmdset_0001.c
+++ b/drivers/mtd/chips/cfi_cmdset_0001.c
@@ -1535,7 +1535,7 @@ static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
 {
 	struct cfi_private *cfi = map->fldrv_priv;
 	map_word status, write_cmd;
-	int ret=0;
+	int ret;
 
 	adr += chip->start;
 
-- 
2.11.0

[PATCH 05/18] mtd-cfi_cmdset_0001: Use common error handling code in do_write_oneword()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 11 Jan 2017 16:40:47 +0100

Add a jump target so that a bit of exception handling can be better reused
at the end of this function.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mtd/chips/cfi_cmdset_0001.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
index 0c0fa918c9ff..4b134e8beb93 100644
--- a/drivers/mtd/chips/cfi_cmdset_0001.c
+++ b/drivers/mtd/chips/cfi_cmdset_0001.c
@@ -1552,10 +1552,8 @@ static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
 
 	mutex_lock(&chip->mutex);
 	ret = get_chip(map, chip, adr, mode);
-	if (ret) {
-		mutex_unlock(&chip->mutex);
-		return ret;
-	}
+	if (ret)
+		goto unlock;
 
 	XIP_INVAL_CACHED_RANGE(map, adr, map_bankwidth(map));
 	ENABLE_VPP(map);
@@ -1600,6 +1598,7 @@ static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
 	xip_enable(map, chip, adr);
  out:	DISABLE_VPP(map);
 	put_chip(map, chip, adr);
+unlock:
 	mutex_unlock(&chip->mutex);
 	return ret;
 }
-- 
2.11.0

[PATCH 06/18] mtd-cfi_cmdset_0001: Use kcalloc() in cfi_intelext_setup()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 11 Jan 2017 16:57:38 +0100

* A multiplication for the size determination of a memory allocation
  indicated that an array data structure should be processed.
  Thus use the corresponding function "kcalloc".

  This issue was detected by using the Coccinelle software.

* Replace the specification of a data structure by a pointer dereference
  to make the corresponding size determination a bit safer according to
  the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mtd/chips/cfi_cmdset_0001.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
index 4b134e8beb93..d7d3d398e4d4 100644
--- a/drivers/mtd/chips/cfi_cmdset_0001.c
+++ b/drivers/mtd/chips/cfi_cmdset_0001.c
@@ -597,8 +597,9 @@ static struct mtd_info *cfi_intelext_setup(struct mtd_info *mtd)
 	mtd->size = devsize * cfi->numchips;
 
 	mtd->numeraseregions = cfi->cfiq->NumEraseRegions * cfi->numchips;
-	mtd->eraseregions = kzalloc(sizeof(struct mtd_erase_region_info)
-			* mtd->numeraseregions, GFP_KERNEL);
+	mtd->eraseregions = kcalloc(mtd->numeraseregions,
+				    sizeof(*mtd->eraseregions),
+				    GFP_KERNEL);
 	if (!mtd->eraseregions)
 		goto setup_err;
 
-- 
2.11.0

[PATCH 07/18] mtd-cfi_cmdset_0001: One function call and an unnecessary check less in cfi_intelext_setup()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 11 Jan 2017 17:27:53 +0100

The kfree() function was called in one case by the cfi_intelext_setup()
function during error handling even if the passed data structure member
contained a null pointer.

* Adjust a jump target according to the Linux coding style convention
  so that memory will be also released for members of a data structure
  before the container "mtd" in the error handling case.

* Delete a check which became unnecessary with this refactoring.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mtd/chips/cfi_cmdset_0001.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
index d7d3d398e4d4..7f6ed7293e40 100644
--- a/drivers/mtd/chips/cfi_cmdset_0001.c
+++ b/drivers/mtd/chips/cfi_cmdset_0001.c
@@ -601,7 +601,7 @@ static struct mtd_info *cfi_intelext_setup(struct mtd_info *mtd)
 				    sizeof(*mtd->eraseregions),
 				    GFP_KERNEL);
 	if (!mtd->eraseregions)
-		goto setup_err;
+		goto free_priv;
 
 	for (i = 0; i < cfi->cfiq->NumEraseRegions; i++) {
 		unsigned long ernum, ersize;
@@ -657,16 +657,14 @@ static struct mtd_info *cfi_intelext_setup(struct mtd_info *mtd)
 	return mtd;
 
  setup_err:
-	if (mtd->eraseregions)
-		for (i = 0; i < cfi->cfiq->NumEraseRegions; i++)
-			for (j = 0; j < cfi->numchips; j++)
-				kfree(mtd->eraseregions[j
-							* cfi->cfiq
-							  ->NumEraseRegions
-							+ i].lockmap);
+	for (i = 0; i < cfi->cfiq->NumEraseRegions; i++)
+		for (j = 0; j < cfi->numchips; j++)
+			kfree(mtd->eraseregions[j * cfi->cfiq->NumEraseRegions
+						+ i].lockmap);
 	kfree(mtd->eraseregions);
-	kfree(mtd);
+free_priv:
 	kfree(cfi->cmdset_priv);
+	kfree(mtd);
 	return NULL;
 }
 
-- 
2.11.0

[PATCH 08/18] mtd-cfi_cmdset_0001: Rename a jump label in cfi_intelext_setup()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 11 Jan 2017 17:49:05 +0100

Adjust a jump label according to the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mtd/chips/cfi_cmdset_0001.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
index 7f6ed7293e40..97329484555d 100644
--- a/drivers/mtd/chips/cfi_cmdset_0001.c
+++ b/drivers/mtd/chips/cfi_cmdset_0001.c
@@ -620,7 +620,7 @@ static struct mtd_info *cfi_intelext_setup(struct mtd_info *mtd)
 			mtd->eraseregions[x].numblocks = ernum;
 			mtd->eraseregions[x].lockmap = kmalloc(ernum / 8 + 1, GFP_KERNEL);
 			if (!mtd->eraseregions[x].lockmap)
-				goto setup_err;
+				goto free_regions;
 		}
 		offset += (ersize * ernum);
 	}
@@ -628,7 +628,7 @@ static struct mtd_info *cfi_intelext_setup(struct mtd_info *mtd)
 	if (offset != devsize) {
 		/* Argh */
 		printk(KERN_WARNING "Sum of regions (%lx) != total size of set of interleaved chips (%lx)\n", offset, devsize);
-		goto setup_err;
+		goto free_regions;
 	}
 
 	for (i = 0; i < mtd->numeraseregions; i++) {
@@ -650,13 +650,12 @@ static struct mtd_info *cfi_intelext_setup(struct mtd_info *mtd)
 	/* This function has the potential to distort the reality
 	   a bit and therefore should be called last. */
 	if (cfi_intelext_partition_fixup(mtd, &cfi) != 0)
-		goto setup_err;
+		goto free_regions;
 
 	__module_get(THIS_MODULE);
 	register_reboot_notifier(&mtd->reboot_notifier);
 	return mtd;
-
- setup_err:
+free_regions:
 	for (i = 0; i < cfi->cfiq->NumEraseRegions; i++)
 		for (j = 0; j < cfi->numchips; j++)
 			kfree(mtd->eraseregions[j * cfi->cfiq->NumEraseRegions
-- 
2.11.0

[PATCH 09/18] mtd-cfi_cmdset_0002: Use kmalloc_array() in cfi_amdstd_setup()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 11 Jan 2017 17:56:42 +0100

* A multiplication for the size determination of a memory allocation
  indicated that an array data structure should be processed.
  Thus use the corresponding function "kmalloc_array".

  This issue was detected by using the Coccinelle software.

* Replace the specification of a data structure by a pointer dereference
  to make the corresponding size determination a bit safer according to
  the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mtd/chips/cfi_cmdset_0002.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index 9dca881bb378..3bed67e35692 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -688,8 +688,9 @@ static struct mtd_info *cfi_amdstd_setup(struct mtd_info *mtd)
 	mtd->size = devsize * cfi->numchips;
 
 	mtd->numeraseregions = cfi->cfiq->NumEraseRegions * cfi->numchips;
-	mtd->eraseregions = kmalloc(sizeof(struct mtd_erase_region_info)
-				    * mtd->numeraseregions, GFP_KERNEL);
+	mtd->eraseregions = kmalloc_array(mtd->numeraseregions,
+					  sizeof(*mtd->eraseregions),
+					  GFP_KERNEL);
 	if (!mtd->eraseregions)
 		goto setup_err;
 
-- 
2.11.0

[PATCH 10/18] mtd-cfi_cmdset_0002: One function call less in cfi_amdstd_setup() after error detection

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 11 Jan 2017 18:32:06 +0100

The kfree() function was called in one case by the cfi_amdstd_setup()
function during error handling even if the passed data structure member
contained a null pointer.

Adjust a jump target according to the Linux coding style convention
so that memory will be also released for members of a data structure
before the container "mtd" in the error handling case.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mtd/chips/cfi_cmdset_0002.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index 3bed67e35692..25d485f6d2ff 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -692,7 +692,7 @@ static struct mtd_info *cfi_amdstd_setup(struct mtd_info *mtd)
 					  sizeof(*mtd->eraseregions),
 					  GFP_KERNEL);
 	if (!mtd->eraseregions)
-		goto setup_err;
+		goto free_priv;
 
 	for (i=0; i<cfi->cfiq->NumEraseRegions; i++) {
 		unsigned long ernum, ersize;
@@ -721,9 +721,10 @@ static struct mtd_info *cfi_amdstd_setup(struct mtd_info *mtd)
 
  setup_err:
 	kfree(mtd->eraseregions);
-	kfree(mtd);
+free_priv:
 	kfree(cfi->cmdset_priv);
 	kfree(cfi->cfiq);
+	kfree(mtd);
 	return NULL;
 }
 
-- 
2.11.0

[PATCH 11/18] mtd-cfi_cmdset_0002: Rename a jump label in cfi_amdstd_setup()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 11 Jan 2017 18:34:03 +0100

Adjust a jump label according to the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mtd/chips/cfi_cmdset_0002.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index 25d485f6d2ff..dc02d99bd7dc 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -712,14 +712,13 @@ static struct mtd_info *cfi_amdstd_setup(struct mtd_info *mtd)
 	if (offset != devsize) {
 		/* Argh */
 		printk(KERN_WARNING "Sum of regions (%lx) != total size of set of interleaved chips (%lx)\n", offset, devsize);
-		goto setup_err;
+		goto free_regions;
 	}
 
 	__module_get(THIS_MODULE);
 	register_reboot_notifier(&mtd->reboot_notifier);
 	return mtd;
-
- setup_err:
+free_regions:
 	kfree(mtd->eraseregions);
 free_priv:
 	kfree(cfi->cmdset_priv);
-- 
2.11.0

[PATCH 12/18] mtd-cfi_cmdset_0002: Add some spaces for better code readability

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 11 Jan 2017 18:45:20 +0100

Use space characters at some source code places according to
the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mtd/chips/cfi_cmdset_0002.c | 84 +++++++++++++++++++++----------------
 1 file changed, 47 insertions(+), 37 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index dc02d99bd7dc..7d4d4a144631 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -546,9 +546,9 @@ struct mtd_info *cfi_cmdset_0002(struct map_info *map, int primary)
 	mtd->_panic_write = cfi_amdstd_panic_write;
 	mtd->reboot_notifier.notifier_call = cfi_amdstd_reboot;
 
-	if (cfi->cfi_mode==CFI_MODE_CFI){
+	if (cfi->cfi_mode == CFI_MODE_CFI) {
 		unsigned char bootloc;
-		__u16 adr = primary?cfi->cfiq->P_ADR:cfi->cfiq->A_ADR;
+		__u16 adr = primary ? cfi->cfiq->P_ADR : cfi->cfiq->A_ADR;
 		struct cfi_pri_amdstd *extp;
 
 		extp = (struct cfi_pri_amdstd*)cfi_read_pri(map, adr, sizeof(*extp), "Amd/Fujitsu");
@@ -613,8 +613,11 @@ struct mtd_info *cfi_cmdset_0002(struct map_info *map, int primary)
 			if (bootloc == 3 && cfi->cfiq->NumEraseRegions > 1) {
 				printk(KERN_WARNING "%s: Swapping erase regions for top-boot CFI table.\n", map->name);
 
-				for (i=0; i<cfi->cfiq->NumEraseRegions / 2; i++) {
-					int j = (cfi->cfiq->NumEraseRegions-1)-i;
+				for (i = 0;
+				     i < cfi->cfiq->NumEraseRegions / 2;
+				     i++) {
+					int j = cfi->cfiq->NumEraseRegions - 1
+						- i;
 
 					swap(cfi->cfiq->EraseRegionInfo[i],
 					     cfi->cfiq->EraseRegionInfo[j]);
@@ -639,10 +642,12 @@ struct mtd_info *cfi_cmdset_0002(struct map_info *map, int primary)
 	/* Apply generic fixups */
 	cfi_fixup(mtd, fixup_table);
 
-	for (i=0; i< cfi->numchips; i++) {
-		cfi->chips[i].word_write_time = 1<<cfi->cfiq->WordWriteTimeoutTyp;
-		cfi->chips[i].buffer_write_time = 1<<cfi->cfiq->BufWriteTimeoutTyp;
-		cfi->chips[i].erase_time = 1<<cfi->cfiq->BlockEraseTimeoutTyp;
+	for (i = 0; i < cfi->numchips; i++) {
+		cfi->chips[i].word_write_time
+			= 1 << cfi->cfiq->WordWriteTimeoutTyp;
+		cfi->chips[i].buffer_write_time
+			= 1 << cfi->cfiq->BufWriteTimeoutTyp;
+		cfi->chips[i].erase_time = 1 << cfi->cfiq->BlockEraseTimeoutTyp;
 		/*
 		 * First calculate the timeout max according to timeout field
 		 * of struct cfi_ident that probed from chip's CFI aera, if
@@ -678,12 +683,13 @@ static struct mtd_info *cfi_amdstd_setup(struct mtd_info *mtd)
 {
 	struct map_info *map = mtd->priv;
 	struct cfi_private *cfi = map->fldrv_priv;
-	unsigned long devsize = (1<<cfi->cfiq->DevSize) * cfi->interleave;
+	unsigned long devsize = (1 << cfi->cfiq->DevSize) * cfi->interleave;
 	unsigned long offset = 0;
-	int i,j;
+	int i, j;
 
 	printk(KERN_NOTICE "number of %s chips: %d\n",
-	       (cfi->cfi_mode == CFI_MODE_CFI)?"CFI":"JEDEC",cfi->numchips);
+	       (cfi->cfi_mode == CFI_MODE_CFI) ? "CFI" : "JEDEC",
+	       cfi->numchips);
 	/* Select the correct geometry setup */
 	mtd->size = devsize * cfi->numchips;
 
@@ -694,7 +700,7 @@ static struct mtd_info *cfi_amdstd_setup(struct mtd_info *mtd)
 	if (!mtd->eraseregions)
 		goto free_priv;
 
-	for (i=0; i<cfi->cfiq->NumEraseRegions; i++) {
+	for (i = 0; i < cfi->cfiq->NumEraseRegions; i++) {
 		unsigned long ernum, ersize;
 		ersize = ((cfi->cfiq->EraseRegionInfo[i] >> 8) & ~0xff) * cfi->interleave;
 		ernum = (cfi->cfiq->EraseRegionInfo[i] & 0xffff) + 1;
@@ -702,10 +708,13 @@ static struct mtd_info *cfi_amdstd_setup(struct mtd_info *mtd)
 		if (mtd->erasesize < ersize) {
 			mtd->erasesize = ersize;
 		}
-		for (j=0; j<cfi->numchips; j++) {
-			mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].offset = (j*devsize)+offset;
-			mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].erasesize = ersize;
-			mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].numblocks = ernum;
+		for (j = 0; j < cfi->numchips; j++) {
+			unsigned int const x = j * cfi->cfiq->NumEraseRegions
+					       + i;
+
+			mtd->eraseregions[x].offset = j * devsize + offset;
+			mtd->eraseregions[x].erasesize = ersize;
+			mtd->eraseregions[x].numblocks = ernum;
 		}
 		offset += (ersize * ernum);
 	}
@@ -882,7 +891,7 @@ static void put_chip(struct map_info *map, struct flchip *chip, unsigned long ad
 {
 	struct cfi_private *cfi = map->fldrv_priv;
 
-	switch(chip->oldstate) {
+	switch (chip->oldstate) {
 	case FL_ERASING:
 		cfi_fixup_m29ew_erase_suspend(map,
 			chip->in_progress_block_addr);
@@ -1151,8 +1160,8 @@ static int cfi_amdstd_read (struct mtd_info *mtd, loff_t from, size_t len, size_
 		if (chipnum >= cfi->numchips)
 			break;
 
-		if ((len + ofs -1) >> cfi->chipshift)
-			thislen = (1<<cfi->chipshift) - ofs;
+		if ((len + ofs - 1) >> cfi->chipshift)
+			thislen = (1 << cfi->chipshift) - ofs;
 		else
 			thislen = len;
 
@@ -1216,7 +1225,7 @@ static inline int do_read_secsi_onechip(struct map_info *map,
  retry:
 	mutex_lock(&chip->mutex);
 
-	if (chip->state != FL_READY){
+	if (chip->state != FL_READY) {
 		set_current_state(TASK_UNINTERRUPTIBLE);
 		add_wait_queue(&chip->wq, &wait);
 
@@ -1253,8 +1262,8 @@ static int cfi_amdstd_secsi_read (struct mtd_info *mtd, loff_t from, size_t len,
 
 	/* ofs: offset within the first chip that the first read should start */
 	/* 8 secsi bytes per chip */
-	chipnum=from>>3;
-	ofs=from & 7;
+	chipnum = from >> 3;
+	ofs = from & 7;
 
 	while (len) {
 		unsigned long thislen;
@@ -1262,8 +1271,8 @@ static int cfi_amdstd_secsi_read (struct mtd_info *mtd, loff_t from, size_t len,
 		if (chipnum >= cfi->numchips)
 			break;
 
-		if ((len + ofs -1) >> 3)
-			thislen = (1<<3) - ofs;
+		if ((len + ofs - 1) >> 3)
+			thislen = (1 << 3) - ofs;
 		else
 			thislen = len;
 
@@ -1624,7 +1633,7 @@ static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
 			continue;
 		}
 
-		if (time_after(jiffies, timeo) && !chip_ready(map, adr)){
+		if (time_after(jiffies, timeo) && !chip_ready(map, adr)) {
 			xip_enable(map, chip, adr);
 			printk(KERN_WARNING "MTD %s(): software timeout\n", __func__);
 			xip_disable(map, chip, adr);
@@ -1725,7 +1734,7 @@ static int cfi_amdstd_write_words(struct mtd_info *mtd, loff_t to, size_t len,
 	}
 
 	/* We are now aligned, write as much as possible */
-	while(len >= map_bankwidth(map)) {
+	while (len >= map_bankwidth(map)) {
 		map_word datum;
 
 		datum = map_word_load(map, buf);
@@ -1837,7 +1846,7 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
 	map_write(map, CMD(words - 1), cmd_adr);
 	/* Write data */
 	z = 0;
-	while(z < words * map_bankwidth(map)) {
+	while (z < words * map_bankwidth(map)) {
 		datum = map_word_load(map, buf);
 		map_write(map, datum, adr + z);
 
@@ -1934,7 +1943,8 @@ static int cfi_amdstd_write_buffers(struct mtd_info *mtd, loff_t to, size_t len,
 		size_t local_len = (-ofs)&(map_bankwidth(map)-1);
 		if (local_len > len)
 			local_len = len;
-		ret = cfi_amdstd_write_words(mtd, ofs + (chipnum<<cfi->chipshift),
+		ret = cfi_amdstd_write_words(mtd,
+					     ofs + (chipnum << cfi->chipshift),
 					     local_len, retlen, buf);
 		if (ret)
 			return ret;
@@ -1981,7 +1991,8 @@ static int cfi_amdstd_write_buffers(struct mtd_info *mtd, loff_t to, size_t len,
 	if (len) {
 		size_t retlen_dregs = 0;
 
-		ret = cfi_amdstd_write_words(mtd, ofs + (chipnum<<cfi->chipshift),
+		ret = cfi_amdstd_write_words(mtd,
+					     ofs + (chipnum << cfi->chipshift),
 					     len, &retlen_dregs, buf);
 
 		*retlen += retlen_dregs;
@@ -2732,13 +2743,13 @@ static void cfi_amdstd_sync (struct mtd_info *mtd)
 	int ret = 0;
 	DECLARE_WAITQUEUE(wait, current);
 
-	for (i=0; !ret && i<cfi->numchips; i++) {
+	for (i = 0; !ret && i < cfi->numchips; i++) {
 		chip = &cfi->chips[i];
 
 	retry:
 		mutex_lock(&chip->mutex);
 
-		switch(chip->state) {
+		switch (chip->state) {
 		case FL_READY:
 		case FL_STATUS:
 		case FL_CFI_QUERY:
@@ -2770,7 +2781,7 @@ static void cfi_amdstd_sync (struct mtd_info *mtd)
 
 	/* Unlock the chips again */
 
-	for (i--; i >=0; i--) {
+	for (i--; i >= 0; i--) {
 		chip = &cfi->chips[i];
 
 		mutex_lock(&chip->mutex);
@@ -2792,12 +2803,12 @@ static int cfi_amdstd_suspend(struct mtd_info *mtd)
 	struct flchip *chip;
 	int ret = 0;
 
-	for (i=0; !ret && i<cfi->numchips; i++) {
+	for (i = 0; !ret && i < cfi->numchips; i++) {
 		chip = &cfi->chips[i];
 
 		mutex_lock(&chip->mutex);
 
-		switch(chip->state) {
+		switch (chip->state) {
 		case FL_READY:
 		case FL_STATUS:
 		case FL_CFI_QUERY:
@@ -2821,7 +2832,7 @@ static int cfi_amdstd_suspend(struct mtd_info *mtd)
 	/* Unlock the chips again */
 
 	if (ret) {
-		for (i--; i >=0; i--) {
+		for (i--; i >= 0; i--) {
 			chip = &cfi->chips[i];
 
 			mutex_lock(&chip->mutex);
@@ -2845,8 +2856,7 @@ static void cfi_amdstd_resume(struct mtd_info *mtd)
 	int i;
 	struct flchip *chip;
 
-	for (i=0; i<cfi->numchips; i++) {
-
+	for (i = 0; i < cfi->numchips; i++) {
 		chip = &cfi->chips[i];
 
 		mutex_lock(&chip->mutex);
-- 
2.11.0

[PATCH 13/18] mtd-cfi_cmdset_0002: Delete an unnecessary variable initialisation in do_write_oneword()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 11 Jan 2017 19:00:30 +0100

The local variable "ret" will be set to an appropriate value a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mtd/chips/cfi_cmdset_0002.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index 7d4d4a144631..05d740e8eb51 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -1569,7 +1569,7 @@ static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
 	 * timeout of 0 jiffies if HZ is smaller than 1000.
 	 */
 	unsigned long uWriteTimeout = ( HZ / 1000 ) + 1;
-	int ret = 0;
+	int ret;
 	map_word oldd;
 	int retry_cnt = 0;
 
-- 
2.11.0

[PATCH 14/18] mtd-cfi_cmdset_0002: Use common error handling code in do_write_oneword()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 11 Jan 2017 19:08:01 +0100

Add a jump target so that a bit of exception handling can be better reused
at the end of this function.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mtd/chips/cfi_cmdset_0002.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index 05d740e8eb51..0c4751eb6efe 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -1577,10 +1577,8 @@ static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
 
 	mutex_lock(&chip->mutex);
 	ret = get_chip(map, chip, adr, mode);
-	if (ret) {
-		mutex_unlock(&chip->mutex);
-		return ret;
-	}
+	if (ret)
+		goto unlock;
 
 	pr_debug("MTD %s(): WRITE 0x%.8lx(0x%.8lx)\n",
 	       __func__, adr, datum.x[0] );
@@ -1664,6 +1662,7 @@ static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
 	chip->state = FL_READY;
 	DISABLE_VPP(map);
 	put_chip(map, chip, adr);
+unlock:
 	mutex_unlock(&chip->mutex);
 
 	return ret;
-- 
2.11.0

[PATCH 15/18] mtd-cfi_cmdset_0002: Use kcalloc() in cfi_ppb_unlock()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 11 Jan 2017 19:15:52 +0100

* A multiplication for the size determination of a memory allocation
  indicated that an array data structure should be processed.
  Thus use the corresponding function "kcalloc".

  This issue was detected by using the Coccinelle software.

* Replace the specification of a data structure by a pointer dereference
  to make the corresponding size determination a bit safer according to
  the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mtd/chips/cfi_cmdset_0002.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index 0c4751eb6efe..81b6fd35be91 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -2649,7 +2649,7 @@ static int __maybe_unused cfi_ppb_unlock(struct mtd_info *mtd, loff_t ofs,
 	 * first check the locking status of all sectors and save
 	 * it for future use.
 	 */
-	sect = kzalloc(MAX_SECTORS * sizeof(struct ppb_lock), GFP_KERNEL);
+	sect = kcalloc(MAX_SECTORS, sizeof(*sect), GFP_KERNEL);
 	if (!sect)
 		return -ENOMEM;
 
-- 
2.11.0

[PATCH 16/18] mtd-cfi_cmdset_0020: Use kmalloc_array() in cfi_staa_setup()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 11 Jan 2017 19:33:48 +0100

* A multiplication for the size determination of a memory allocation
  indicated that an array data structure should be processed.
  Thus use the corresponding function "kmalloc_array".

  This issue was detected by using the Coccinelle software.

* Replace the specification of a data structure by a pointer dereference
  to make the corresponding size determination a bit safer according to
  the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mtd/chips/cfi_cmdset_0020.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0020.c b/drivers/mtd/chips/cfi_cmdset_0020.c
index 94d3eb42c4d5..9cd7b1c6faba 100644
--- a/drivers/mtd/chips/cfi_cmdset_0020.c
+++ b/drivers/mtd/chips/cfi_cmdset_0020.c
@@ -184,8 +184,9 @@ static struct mtd_info *cfi_staa_setup(struct map_info *map)
 	mtd->size = devsize * cfi->numchips;
 
 	mtd->numeraseregions = cfi->cfiq->NumEraseRegions * cfi->numchips;
-	mtd->eraseregions = kmalloc(sizeof(struct mtd_erase_region_info)
-			* mtd->numeraseregions, GFP_KERNEL);
+	mtd->eraseregions = kmalloc_array(mtd->numeraseregions,
+					  sizeof(*mtd->eraseregions),
+					  GFP_KERNEL);
 	if (!mtd->eraseregions) {
 		kfree(cfi->cmdset_priv);
 		kfree(mtd);
-- 
2.11.0

[PATCH 17/18] mtd-cfi_cmdset_0020: Use common error handling code in cfi_staa_setup()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 11 Jan 2017 20:02:40 +0100

Add jump targets so that a bit of exception handling can be better reused
at the end of this function.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mtd/chips/cfi_cmdset_0020.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0020.c b/drivers/mtd/chips/cfi_cmdset_0020.c
index 9cd7b1c6faba..60de9d58dcc6 100644
--- a/drivers/mtd/chips/cfi_cmdset_0020.c
+++ b/drivers/mtd/chips/cfi_cmdset_0020.c
@@ -173,11 +173,8 @@ static struct mtd_info *cfi_staa_setup(struct map_info *map)
 
 	mtd = kzalloc(sizeof(*mtd), GFP_KERNEL);
 	//printk(KERN_DEBUG "number of CFI chips: %d\n", cfi->numchips);
-
-	if (!mtd) {
-		kfree(cfi->cmdset_priv);
-		return NULL;
-	}
+	if (!mtd)
+		goto free_priv;
 
 	mtd->priv = map;
 	mtd->type = MTD_NORFLASH;
@@ -187,11 +184,8 @@ static struct mtd_info *cfi_staa_setup(struct map_info *map)
 	mtd->eraseregions = kmalloc_array(mtd->numeraseregions,
 					  sizeof(*mtd->eraseregions),
 					  GFP_KERNEL);
-	if (!mtd->eraseregions) {
-		kfree(cfi->cmdset_priv);
-		kfree(mtd);
-		return NULL;
-	}
+	if (!mtd->eraseregions)
+		goto free_mtd;
 
 	for (i=0; i<cfi->cfiq->NumEraseRegions; i++) {
 		unsigned long ernum, ersize;
@@ -213,9 +207,7 @@ static struct mtd_info *cfi_staa_setup(struct map_info *map)
 		/* Argh */
 		printk(KERN_WARNING "Sum of regions (%lx) != total size of set of interleaved chips (%lx)\n", offset, devsize);
 		kfree(mtd->eraseregions);
-		kfree(cfi->cmdset_priv);
-		kfree(mtd);
-		return NULL;
+		goto free_mtd;
 	}
 
 	for (i=0; i<mtd->numeraseregions;i++){
@@ -242,6 +234,11 @@ static struct mtd_info *cfi_staa_setup(struct map_info *map)
 	__module_get(THIS_MODULE);
 	mtd->name = map->name;
 	return mtd;
+free_mtd:
+	kfree(mtd);
+free_priv:
+	kfree(cfi->cmdset_priv);
+	return NULL;
 }
 
 
-- 
2.11.0

[PATCH 18/18] mtd-cfi_cmdset_0020: Add some spaces for better code readability

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 11 Jan 2017 20:55:40 +0100

Use space characters at some source code places according to
the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mtd/chips/cfi_cmdset_0020.c | 99 ++++++++++++++++++++-----------------
 1 file changed, 55 insertions(+), 44 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0020.c b/drivers/mtd/chips/cfi_cmdset_0020.c
index 60de9d58dcc6..f6b2bf056849 100644
--- a/drivers/mtd/chips/cfi_cmdset_0020.c
+++ b/drivers/mtd/chips/cfi_cmdset_0020.c
@@ -75,24 +75,24 @@ static void cfi_tell_features(struct cfi_pri_intelext *extp)
 	printk("     - Protection Bits:    %s\n", extp->FeatureSupport&64?"supported":"unsupported");
 	printk("     - Page-mode read:     %s\n", extp->FeatureSupport&128?"supported":"unsupported");
 	printk("     - Synchronous read:   %s\n", extp->FeatureSupport&256?"supported":"unsupported");
-	for (i=9; i<32; i++) {
-		if (extp->FeatureSupport & (1<<i))
+	for (i = 9; i < 32; i++) {
+		if (extp->FeatureSupport & (1 << i))
 			printk("     - Unknown Bit %X:      supported\n", i);
 	}
 
 	printk("  Supported functions after Suspend: %2.2X\n", extp->SuspendCmdSupport);
 	printk("     - Program after Erase Suspend: %s\n", extp->SuspendCmdSupport&1?"supported":"unsupported");
-	for (i=1; i<8; i++) {
-		if (extp->SuspendCmdSupport & (1<<i))
+	for (i = 1; i < 8; i++) {
+		if (extp->SuspendCmdSupport & (1 << i))
 			printk("     - Unknown Bit %X:               supported\n", i);
 	}
 
 	printk("  Block Status Register Mask: %4.4X\n", extp->BlkStatusRegMask);
 	printk("     - Lock Bit Active:      %s\n", extp->BlkStatusRegMask&1?"yes":"no");
 	printk("     - Valid Bit Active:     %s\n", extp->BlkStatusRegMask&2?"yes":"no");
-	for (i=2; i<16; i++) {
-		if (extp->BlkStatusRegMask & (1<<i))
-			printk("     - Unknown Bit %X Active: yes\n",i);
+	for (i = 2; i < 16; i++) {
+		if (extp->BlkStatusRegMask & (1 << i))
+			printk("     - Unknown Bit %X Active: yes\n", i);
 	}
 
 	printk("  Vcc Logic Supply Optimum Program/Erase Voltage: %d.%d V\n",
@@ -121,7 +121,7 @@ struct mtd_info *cfi_cmdset_0020(struct map_info *map, int primary)
 		 * routine faked a CFI structure. So we read the feature
 		 * table from it.
 		 */
-		__u16 adr = primary?cfi->cfiq->P_ADR:cfi->cfiq->A_ADR;
+		__u16 adr = primary ? cfi->cfiq->P_ADR : cfi->cfiq->A_ADR;
 		struct cfi_pri_intelext *extp;
 
 		extp = (struct cfi_pri_intelext*)cfi_read_pri(map, adr, sizeof(*extp), "ST Microelectronics");
@@ -151,7 +151,7 @@ struct mtd_info *cfi_cmdset_0020(struct map_info *map, int primary)
 		cfi->cmdset_priv = extp;
 	}
 
-	for (i=0; i< cfi->numchips; i++) {
+	for (i = 0; i < cfi->numchips; i++) {
 		cfi->chips[i].word_write_time = 128;
 		cfi->chips[i].buffer_write_time = 128;
 		cfi->chips[i].erase_time = 1024;
@@ -168,8 +168,8 @@ static struct mtd_info *cfi_staa_setup(struct map_info *map)
 	struct cfi_private *cfi = map->fldrv_priv;
 	struct mtd_info *mtd;
 	unsigned long offset = 0;
-	int i,j;
-	unsigned long devsize = (1<<cfi->cfiq->DevSize) * cfi->interleave;
+	int i, j;
+	unsigned long devsize = (1 << cfi->cfiq->DevSize) * cfi->interleave;
 
 	mtd = kzalloc(sizeof(*mtd), GFP_KERNEL);
 	//printk(KERN_DEBUG "number of CFI chips: %d\n", cfi->numchips);
@@ -187,7 +187,7 @@ static struct mtd_info *cfi_staa_setup(struct map_info *map)
 	if (!mtd->eraseregions)
 		goto free_mtd;
 
-	for (i=0; i<cfi->cfiq->NumEraseRegions; i++) {
+	for (i = 0; i < cfi->cfiq->NumEraseRegions; i++) {
 		unsigned long ernum, ersize;
 		ersize = ((cfi->cfiq->EraseRegionInfo[i] >> 8) & ~0xff) * cfi->interleave;
 		ernum = (cfi->cfiq->EraseRegionInfo[i] & 0xffff) + 1;
@@ -195,10 +195,13 @@ static struct mtd_info *cfi_staa_setup(struct map_info *map)
 		if (mtd->erasesize < ersize) {
 			mtd->erasesize = ersize;
 		}
-		for (j=0; j<cfi->numchips; j++) {
-			mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].offset = (j*devsize)+offset;
-			mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].erasesize = ersize;
-			mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].numblocks = ernum;
+		for (j = 0; j < cfi->numchips; j++) {
+			unsigned int const x = j * cfi->cfiq->NumEraseRegions
+					       + i;
+
+			mtd->eraseregions[x].offset = j * devsize + offset;
+			mtd->eraseregions[x].erasesize = ersize;
+			mtd->eraseregions[x].numblocks = ernum;
 		}
 		offset += (ersize * ernum);
 	}
@@ -210,7 +213,7 @@ static struct mtd_info *cfi_staa_setup(struct map_info *map)
 		goto free_mtd;
 	}
 
-	for (i=0; i<mtd->numeraseregions;i++){
+	for (i = 0; i < mtd->numeraseregions; i++) {
 		printk(KERN_DEBUG "%d: offset=0x%llx,size=0x%x,blocks=%d\n",
 		       i, (unsigned long long)mtd->eraseregions[i].offset,
 		       mtd->eraseregions[i].erasesize,
@@ -395,8 +398,8 @@ static int cfi_staa_read (struct mtd_info *mtd, loff_t from, size_t len, size_t
 		if (chipnum >= cfi->numchips)
 			break;
 
-		if ((len + ofs -1) >> cfi->chipshift)
-			thislen = (1<<cfi->chipshift) - ofs;
+		if ((len + ofs - 1) >> cfi->chipshift)
+			thislen = (1 << cfi->chipshift) - ofs;
 		else
 			thislen = len;
 
@@ -676,7 +679,7 @@ cfi_staa_writev(struct mtd_info *mtd, const struct kvec *vecs,
 	if (!buffer)
 		return -ENOMEM;
 
-	for (i=0; i<count; i++) {
+	for (i = 0; i < count; i++) {
 		size_t elem_len = vecs[i].iov_len;
 		void *elem_base = vecs[i].iov_base;
 		if (!elem_len) /* FIXME: Might be unnecessary. Check that */
@@ -844,8 +847,9 @@ static inline int do_erase_oneblock(struct map_info *map, struct flchip *chip, u
 		unsigned char chipstatus = status.x[0];
 		if (!map_word_equal(map, status, CMD(chipstatus))) {
 			int i, w;
-			for (w=0; w<map_words(map); w++) {
-				for (i = 0; i<cfi_interleave(cfi); i++) {
+
+			for (w = 0; w < map_words(map); w++) {
+				for (i = 0; i < cfi_interleave(cfi); i++) {
 					chipstatus |= status.x[w] >> (cfi->device_type * 8);
 				}
 			}
@@ -925,7 +929,8 @@ static int cfi_staa_erase_varsize(struct mtd_info *mtd,
 	 * with the erase region at that address.
 	 */
 
-	while (i<mtd->numeraseregions && (instr->addr + instr->len) >= regions[i].offset)
+	while (i < mtd->numeraseregions
+	      && (instr->addr + instr->len) >= regions[i].offset)
 		i++;
 
 	/* As before, drop back one to point at the region in which
@@ -933,16 +938,15 @@ static int cfi_staa_erase_varsize(struct mtd_info *mtd,
 	*/
 	i--;
 
-	if ((instr->addr + instr->len) & (regions[i].erasesize-1))
+	if ((instr->addr + instr->len) & (regions[i].erasesize - 1))
 		return -EINVAL;
 
 	chipnum = instr->addr >> cfi->chipshift;
 	adr = instr->addr - (chipnum << cfi->chipshift);
 	len = instr->len;
+	i = first;
 
-	i=first;
-
-	while(len) {
+	while (len) {
 		ret = do_erase_oneblock(map, &cfi->chips[chipnum], adr);
 
 		if (ret)
@@ -951,7 +955,10 @@ static int cfi_staa_erase_varsize(struct mtd_info *mtd,
 		adr += regions[i].erasesize;
 		len -= regions[i].erasesize;
 
-		if (adr % (1<< cfi->chipshift) == (((unsigned long)regions[i].offset + (regions[i].erasesize * regions[i].numblocks)) %( 1<< cfi->chipshift)))
+		if (adr % (1 << cfi->chipshift)
+		   == (((unsigned long)regions[i].offset
+		      + (regions[i].erasesize * regions[i].numblocks))
+			% (1 << cfi->chipshift)))
 			i++;
 
 		if (adr >> cfi->chipshift) {
@@ -978,13 +985,13 @@ static void cfi_staa_sync (struct mtd_info *mtd)
 	int ret = 0;
 	DECLARE_WAITQUEUE(wait, current);
 
-	for (i=0; !ret && i<cfi->numchips; i++) {
+	for (i = 0; !ret && i < cfi->numchips; i++) {
 		chip = &cfi->chips[i];
 
 	retry:
 		mutex_lock(&chip->mutex);
 
-		switch(chip->state) {
+		switch (chip->state) {
 		case FL_READY:
 		case FL_STATUS:
 		case FL_CFI_QUERY:
@@ -1014,7 +1021,7 @@ static void cfi_staa_sync (struct mtd_info *mtd)
 
 	/* Unlock the chips again */
 
-	for (i--; i >=0; i--) {
+	for (i--; i >= 0; i--) {
 		chip = &cfi->chips[i];
 
 		mutex_lock(&chip->mutex);
@@ -1092,7 +1099,7 @@ static inline int do_lock_oneblock(struct map_info *map, struct flchip *chip, un
 	/* FIXME. Use a timer to check this, and return immediately. */
 	/* Once the state machine's known to be working I'll do that */
 
-	timeo = jiffies + (HZ*2);
+	timeo = jiffies + HZ * 2;
 	for (;;) {
 
 		status = map_read(map, adr);
@@ -1135,17 +1142,18 @@ static int cfi_staa_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
 	if (ofs & (mtd->erasesize - 1))
 		return -EINVAL;
 
-	if (len & (mtd->erasesize -1))
+	if (len & (mtd->erasesize - 1))
 		return -EINVAL;
 
 	chipnum = ofs >> cfi->chipshift;
 	adr = ofs - (chipnum << cfi->chipshift);
 
-	while(len) {
+	while (len) {
 
 #ifdef DEBUG_LOCK_BITS
 		cfi_send_gen_cmd(0x90, 0x55, 0, map, cfi, cfi->device_type, NULL);
-		printk("before lock: block status register is %x\n",cfi_read_query(map, adr+(2*ofs_factor)));
+		printk("before lock: block status register is %x\n",
+		       cfi_read_query(map, adr + 2 * ofs_factor));
 		cfi_send_gen_cmd(0xff, 0x55, 0, map, cfi, cfi->device_type, NULL);
 #endif
 
@@ -1153,7 +1161,8 @@ static int cfi_staa_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
 
 #ifdef DEBUG_LOCK_BITS
 		cfi_send_gen_cmd(0x90, 0x55, 0, map, cfi, cfi->device_type, NULL);
-		printk("after lock: block status register is %x\n",cfi_read_query(map, adr+(2*ofs_factor)));
+		printk("after lock: block status register is %x\n",
+		       cfi_read_query(map, adr + 2 * ofs_factor));
 		cfi_send_gen_cmd(0xff, 0x55, 0, map, cfi, cfi->device_type, NULL);
 #endif
 
@@ -1238,7 +1247,7 @@ static inline int do_unlock_oneblock(struct map_info *map, struct flchip *chip,
 	/* FIXME. Use a timer to check this, and return immediately. */
 	/* Once the state machine's known to be working I'll do that */
 
-	timeo = jiffies + (HZ*2);
+	timeo = jiffies + HZ * 2;
 	for (;;) {
 
 		status = map_read(map, adr);
@@ -1288,7 +1297,9 @@ static int cfi_staa_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
 
 		cfi_send_gen_cmd(0x90, 0x55, 0, map, cfi, cfi->device_type, NULL);
                 while (temp_len) {
-			printk("before unlock %x: block status register is %x\n",temp_adr,cfi_read_query(map, temp_adr+(2*ofs_factor)));
+			printk("before unlock %x: block status register is %x\n",
+			       temp_adr,
+			       cfi_read_query(map, temp_adr + 2 * ofs_factor));
 			temp_adr += mtd->erasesize;
 			temp_len -= mtd->erasesize;
 		}
@@ -1300,7 +1311,8 @@ static int cfi_staa_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
 
 #ifdef DEBUG_LOCK_BITS
 	cfi_send_gen_cmd(0x90, 0x55, 0, map, cfi, cfi->device_type, NULL);
-	printk("after unlock: block status register is %x\n",cfi_read_query(map, adr+(2*ofs_factor)));
+	printk("after unlock: block status register is %x\n",
+	       cfi_read_query(map, adr + 2 * ofs_factor));
 	cfi_send_gen_cmd(0xff, 0x55, 0, map, cfi, cfi->device_type, NULL);
 #endif
 
@@ -1315,12 +1327,12 @@ static int cfi_staa_suspend(struct mtd_info *mtd)
 	struct flchip *chip;
 	int ret = 0;
 
-	for (i=0; !ret && i<cfi->numchips; i++) {
+	for (i = 0; !ret && i < cfi->numchips; i++) {
 		chip = &cfi->chips[i];
 
 		mutex_lock(&chip->mutex);
 
-		switch(chip->state) {
+		switch (chip->state) {
 		case FL_READY:
 		case FL_STATUS:
 		case FL_CFI_QUERY:
@@ -1344,7 +1356,7 @@ static int cfi_staa_suspend(struct mtd_info *mtd)
 	/* Unlock the chips again */
 
 	if (ret) {
-		for (i--; i >=0; i--) {
+		for (i--; i >= 0; i--) {
 			chip = &cfi->chips[i];
 
 			mutex_lock(&chip->mutex);
@@ -1370,8 +1382,7 @@ static void cfi_staa_resume(struct mtd_info *mtd)
 	int i;
 	struct flchip *chip;
 
-	for (i=0; i<cfi->numchips; i++) {
-
+	for (i = 0; i < cfi->numchips; i++) {
 		chip = &cfi->chips[i];
 
 		mutex_lock(&chip->mutex);
-- 
2.11.0

Re: [PATCH 18/18] mtd-cfi_cmdset_0020: Add some spaces for better code readability

[Dan Carpenter]

On Wed, Jan 11, 2017 at 09:55:20PM +0100, SF Markus Elfring wrote:
> -	timeo = jiffies + (HZ*2);
> +	timeo = jiffies + HZ * 2;

You've done this a couple times already, but these parenthesis are there
to make the code more readable don't delete them.

regards,
dan carpenter

Re: [PATCH 00/18] MTD-CFI: Fine-tuning for several function implementations

[Boris Brezillon]

Hi Markus,

On Wed, 11 Jan 2017 21:34:13 +0100
SF Markus Elfring <elfring@users.sourceforge.net> wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 11 Jan 2017 21:21:12 +0100
> 
> Several update suggestions were taken into account
> from static source code analysis.
> 
> Markus Elfring (18):
>   Use kmalloc_array() in cfi_intelext_partition_fixup()
>   Improve another size determination in cfi_intelext_partition_fixup()
>   cfi_cmdset_0001: Add some spaces for better code readability
>   cfi_cmdset_0001: Delete an unnecessary variable initialisation in do_write_oneword()
>   cfi_cmdset_0001: Use common error handling code in do_write_oneword()
>   Use kcalloc() in cfi_intelext_setup()
>   One function call and an unnecessary check less in cfi_intelext_setup()
>   Rename a jump label in cfi_intelext_setup()
>   Use kmalloc_array() in cfi_amdstd_setup()
>   One function call less in cfi_amdstd_setup() after error detection
>   Rename a jump label in cfi_amdstd_setup()
>   cfi_cmdset_0002: Add some spaces for better code readability
>   cfi_cmdset_0002: Delete an unnecessary variable initialisation in do_write_oneword()
>   cfi_cmdset_0002: Use common error handling code in do_write_oneword()
>   Use kcalloc() in cfi_ppb_unlock()
>   Use kmalloc_array() in cfi_staa_setup()
>   Use common error handling code in cfi_staa_setup()
>   cfi_cmdset_0020: Add some spaces for better code readability

You know what, I almost considered reviewing this series, but I had a
look at the commit titles (and the sort of things you're
'fixing/improving') and decided to google your name instead.

It seems that you've already sent quite a lot of so-called
'cleanup/improvement' patches, and most of the time you don't understand
what you're doing.
Maybe it's time for you to stop using static analysis tools and start
spending time on real stuff.

Note that I'm usually not against cleanup/coding-style patches, but it
really seems that you're only interested in getting as much patches as
you can in mainline, no matter what they do.

Sorry, that's a NACK on my side. Let's see of other MTD maintainers are
willing to take the time to review your patches. And BTW, when you
submit something, you could at least have a look at previous commits
touching the same files/directories and try to match the subsystem
prefix ("mtd: cfi_cmdset: ", "mtd: cfi: " or "mtd: chips: cfi: ").

Regards,

Boris

> 
>  drivers/mtd/chips/cfi_cmdset_0001.c | 165 +++++++++++++++++++-----------------
>  drivers/mtd/chips/cfi_cmdset_0002.c | 110 +++++++++++++-----------
>  drivers/mtd/chips/cfi_cmdset_0020.c | 127 ++++++++++++++-------------
>  3 files changed, 214 insertions(+), 188 deletions(-)
> 

Re: [PATCH 00/18] MTD-CFI: Fine-tuning for several function implementations

[Marek Vasut]

On 01/11/2017 09:34 PM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 11 Jan 2017 21:21:12 +0100
> 
> Several update suggestions were taken into account
> from static source code analysis.

The patch listing doesn't seem to match the content of the patchset ?
The $subject are different ...

> Markus Elfring (18):
>   Use kmalloc_array() in cfi_intelext_partition_fixup()
>   Improve another size determination in cfi_intelext_partition_fixup()
>   cfi_cmdset_0001: Add some spaces for better code readability
>   cfi_cmdset_0001: Delete an unnecessary variable initialisation in do_write_oneword()
>   cfi_cmdset_0001: Use common error handling code in do_write_oneword()
>   Use kcalloc() in cfi_intelext_setup()
>   One function call and an unnecessary check less in cfi_intelext_setup()
>   Rename a jump label in cfi_intelext_setup()
>   Use kmalloc_array() in cfi_amdstd_setup()
>   One function call less in cfi_amdstd_setup() after error detection
>   Rename a jump label in cfi_amdstd_setup()
>   cfi_cmdset_0002: Add some spaces for better code readability
>   cfi_cmdset_0002: Delete an unnecessary variable initialisation in do_write_oneword()
>   cfi_cmdset_0002: Use common error handling code in do_write_oneword()
>   Use kcalloc() in cfi_ppb_unlock()
>   Use kmalloc_array() in cfi_staa_setup()
>   Use common error handling code in cfi_staa_setup()
>   cfi_cmdset_0020: Add some spaces for better code readability
> 
>  drivers/mtd/chips/cfi_cmdset_0001.c | 165 +++++++++++++++++++-----------------
>  drivers/mtd/chips/cfi_cmdset_0002.c | 110 +++++++++++++-----------
>  drivers/mtd/chips/cfi_cmdset_0020.c | 127 ++++++++++++++-------------
>  3 files changed, 214 insertions(+), 188 deletions(-)
> 


-- 
Best regards,
Marek Vasut