All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] mtd: cfi: reduce stack size
@ 2015-03-10 16:48 ` Arnd Bergmann
  0 siblings, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2015-03-10 16:48 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris
  Cc: Jingoo Han, linux-mtd, Ezequiel Garcia, linux-arm-kernel, linux-kernel

The cfi_staa_write_buffers function uses a large amount of kernel stack
whenever CONFIG_MTD_MAP_BANK_WIDTH_32 is set, and that results in a
warning on ARM allmodconfig builds:

drivers/mtd/chips/cfi_cmdset_0020.c: In function 'cfi_staa_write_buffers':
drivers/mtd/chips/cfi_cmdset_0020.c:651:1: warning: the frame size of 1208 bytes is larger than 1024 bytes [-Wframe-larger-than=]

It turns out that this is largely a result of a suboptimal implementation
of map_word_andequal(). Replacing this function with a straightforward
one reduces the stack size in this function by exactly 200 bytes,
shrinks the .text segment for this file from 27648 bytes to 26608 bytes,
and makes the warning go away.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v3: more whitespace changes
v2: whitespace changes

diff --git a/include/linux/mtd/map.h b/include/linux/mtd/map.h
index 47c59991491b..29975c73a953 100644
--- a/include/linux/mtd/map.h
+++ b/include/linux/mtd/map.h
@@ -316,7 +316,17 @@ static inline map_word map_word_or(struct map_info *map, map_word val1, map_word
 	return r;
 }
 
-#define map_word_andequal(m, a, b, z) map_word_equal(m, z, map_word_and(m, a, b))
+static inline int map_word_andequal(struct map_info *map, map_word val1, map_word val2, map_word val3)
+{
+	int i;
+
+	for (i = 0; i < map_words(map); i++) {
+		if ((val1.x[i] & val2.x[i]) != val3.x[i])
+			return 0;
+	}
+
+	return 1;
+}
 
 static inline int map_word_bitsset(struct map_info *map, map_word val1, map_word val2)
 {


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v3] mtd: cfi: reduce stack size
@ 2015-03-10 16:48 ` Arnd Bergmann
  0 siblings, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2015-03-10 16:48 UTC (permalink / raw)
  To: linux-arm-kernel

The cfi_staa_write_buffers function uses a large amount of kernel stack
whenever CONFIG_MTD_MAP_BANK_WIDTH_32 is set, and that results in a
warning on ARM allmodconfig builds:

drivers/mtd/chips/cfi_cmdset_0020.c: In function 'cfi_staa_write_buffers':
drivers/mtd/chips/cfi_cmdset_0020.c:651:1: warning: the frame size of 1208 bytes is larger than 1024 bytes [-Wframe-larger-than=]

It turns out that this is largely a result of a suboptimal implementation
of map_word_andequal(). Replacing this function with a straightforward
one reduces the stack size in this function by exactly 200 bytes,
shrinks the .text segment for this file from 27648 bytes to 26608 bytes,
and makes the warning go away.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v3: more whitespace changes
v2: whitespace changes

diff --git a/include/linux/mtd/map.h b/include/linux/mtd/map.h
index 47c59991491b..29975c73a953 100644
--- a/include/linux/mtd/map.h
+++ b/include/linux/mtd/map.h
@@ -316,7 +316,17 @@ static inline map_word map_word_or(struct map_info *map, map_word val1, map_word
 	return r;
 }
 
-#define map_word_andequal(m, a, b, z) map_word_equal(m, z, map_word_and(m, a, b))
+static inline int map_word_andequal(struct map_info *map, map_word val1, map_word val2, map_word val3)
+{
+	int i;
+
+	for (i = 0; i < map_words(map); i++) {
+		if ((val1.x[i] & val2.x[i]) != val3.x[i])
+			return 0;
+	}
+
+	return 1;
+}
 
 static inline int map_word_bitsset(struct map_info *map, map_word val1, map_word val2)
 {

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH] mtd: clean up whitespace in linux/mtd/map.h
  2015-03-10 16:48 ` Arnd Bergmann
@ 2015-03-10 16:51   ` Arnd Bergmann
  -1 siblings, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2015-03-10 16:51 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris
  Cc: Jingoo Han, linux-mtd, Ezequiel Garcia, linux-arm-kernel, linux-kernel

As the only comments I got for the "mtd: cfi: reduce stack size"
patch were about whitespace changes, it appears necessary to fix
up the rest of the file as well, which contains the exact same
mistakes.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
The two patches can be applied in either order, or this one dropped
entirely, I don't care as long as the other patch can get merged,
or get some constructive feedback.

diff --git a/include/linux/mtd/map.h b/include/linux/mtd/map.h
index 5f487d776411..47c59991491b 100644
--- a/include/linux/mtd/map.h
+++ b/include/linux/mtd/map.h
@@ -77,7 +77,7 @@
 /* ensure we never evaluate anything shorted than an unsigned long
  * to zero, and ensure we'll never miss the end of an comparison (bjd) */
 
-#define map_calc_words(map) ((map_bankwidth(map) + (sizeof(unsigned long)-1))/ sizeof(unsigned long))
+#define map_calc_words(map) ((map_bankwidth(map) + (sizeof(unsigned long)-1)) / sizeof(unsigned long))
 
 #ifdef CONFIG_MTD_MAP_BANK_WIDTH_8
 # ifdef map_bankwidth
@@ -181,7 +181,7 @@ static inline int map_bankwidth_supported(int w)
 	}
 }
 
-#define MAX_MAP_LONGS ( ((MAX_MAP_BANKWIDTH*8) + BITS_PER_LONG - 1) / BITS_PER_LONG )
+#define MAX_MAP_LONGS (((MAX_MAP_BANKWIDTH * 8) + BITS_PER_LONG - 1) / BITS_PER_LONG)
 
 typedef union {
 	unsigned long x[MAX_MAP_LONGS];
@@ -264,20 +264,22 @@ void unregister_mtd_chip_driver(struct mtd_chip_driver *);
 struct mtd_info *do_map_probe(const char *name, struct map_info *map);
 void map_destroy(struct mtd_info *mtd);
 
-#define ENABLE_VPP(map) do { if(map->set_vpp) map->set_vpp(map, 1); } while(0)
-#define DISABLE_VPP(map) do { if(map->set_vpp) map->set_vpp(map, 0); } while(0)
+#define ENABLE_VPP(map) do { if (map->set_vpp) map->set_vpp(map, 1); } while (0)
+#define DISABLE_VPP(map) do { if (map->set_vpp) map->set_vpp(map, 0); } while (0)
 
 #define INVALIDATE_CACHED_RANGE(map, from, size) \
-	do { if(map->inval_cache) map->inval_cache(map, from, size); } while(0)
+	do { if (map->inval_cache) map->inval_cache(map, from, size); } while (0)
 
 
 static inline int map_word_equal(struct map_info *map, map_word val1, map_word val2)
 {
 	int i;
-	for (i=0; i<map_words(map); i++) {
+
+	for (i = 0; i < map_words(map); i++) {
 		if (val1.x[i] != val2.x[i])
 			return 0;
 	}
+
 	return 1;
 }
 
@@ -286,9 +288,9 @@ static inline map_word map_word_and(struct map_info *map, map_word val1, map_wor
 	map_word r;
 	int i;
 
-	for (i=0; i<map_words(map); i++) {
+	for (i = 0; i < map_words(map); i++)
 		r.x[i] = val1.x[i] & val2.x[i];
-	}
+
 	return r;
 }
 
@@ -297,9 +299,9 @@ static inline map_word map_word_clr(struct map_info *map, map_word val1, map_wor
 	map_word r;
 	int i;
 
-	for (i=0; i<map_words(map); i++) {
+	for (i = 0; i < map_words(map); i++)
 		r.x[i] = val1.x[i] & ~val2.x[i];
-	}
+
 	return r;
 }
 
@@ -308,9 +310,9 @@ static inline map_word map_word_or(struct map_info *map, map_word val1, map_word
 	map_word r;
 	int i;
 
-	for (i=0; i<map_words(map); i++) {
+	for (i = 0; i < map_words(map); i++)
 		r.x[i] = val1.x[i] | val2.x[i];
-	}
+
 	return r;
 }
 
@@ -320,10 +322,11 @@ static inline int map_word_bitsset(struct map_info *map, map_word val1, map_word
 {
 	int i;
 
-	for (i=0; i<map_words(map); i++) {
+	for (i = 0; i < map_words(map); i++) {
 		if (val1.x[i] & val2.x[i])
 			return 1;
 	}
+
 	return 0;
 }
 
@@ -355,14 +358,16 @@ static inline map_word map_word_load_partial(struct map_info *map, map_word orig
 
 	if (map_bankwidth_is_large(map)) {
 		char *dest = (char *)&orig;
+
 		memcpy(dest+start, buf, len);
 	} else {
-		for (i=start; i < start+len; i++) {
+		for (i = start; i < start+len; i++) {
 			int bitpos;
+
 #ifdef __LITTLE_ENDIAN
-			bitpos = i*8;
+			bitpos = i * 8;
 #else /* __BIG_ENDIAN */
-			bitpos = (map_bankwidth(map)-1-i)*8;
+			bitpos = (map_bankwidth(map) - 1 - i) * 8;
 #endif
 			orig.x[0] &= ~(0xff << bitpos);
 			orig.x[0] |= (unsigned long)buf[i-start] << bitpos;
@@ -384,9 +389,10 @@ static inline map_word map_word_ff(struct map_info *map)
 
 	if (map_bankwidth(map) < MAP_FF_LIMIT) {
 		int bw = 8 * map_bankwidth(map);
+
 		r.x[0] = (1UL << bw) - 1;
 	} else {
-		for (i=0; i<map_words(map); i++)
+		for (i = 0; i < map_words(map); i++)
 			r.x[i] = ~0UL;
 	}
 	return r;
@@ -407,7 +413,7 @@ static inline map_word inline_map_read(struct map_info *map, unsigned long ofs)
 		r.x[0] = __raw_readq(map->virt + ofs);
 #endif
 	else if (map_bankwidth_is_large(map))
-		memcpy_fromio(r.x, map->virt+ofs, map->bankwidth);
+		memcpy_fromio(r.x, map->virt + ofs, map->bankwidth);
 	else
 		BUG();
 


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH] mtd: clean up whitespace in linux/mtd/map.h
@ 2015-03-10 16:51   ` Arnd Bergmann
  0 siblings, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2015-03-10 16:51 UTC (permalink / raw)
  To: linux-arm-kernel

As the only comments I got for the "mtd: cfi: reduce stack size"
patch were about whitespace changes, it appears necessary to fix
up the rest of the file as well, which contains the exact same
mistakes.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
The two patches can be applied in either order, or this one dropped
entirely, I don't care as long as the other patch can get merged,
or get some constructive feedback.

diff --git a/include/linux/mtd/map.h b/include/linux/mtd/map.h
index 5f487d776411..47c59991491b 100644
--- a/include/linux/mtd/map.h
+++ b/include/linux/mtd/map.h
@@ -77,7 +77,7 @@
 /* ensure we never evaluate anything shorted than an unsigned long
  * to zero, and ensure we'll never miss the end of an comparison (bjd) */
 
-#define map_calc_words(map) ((map_bankwidth(map) + (sizeof(unsigned long)-1))/ sizeof(unsigned long))
+#define map_calc_words(map) ((map_bankwidth(map) + (sizeof(unsigned long)-1)) / sizeof(unsigned long))
 
 #ifdef CONFIG_MTD_MAP_BANK_WIDTH_8
 # ifdef map_bankwidth
@@ -181,7 +181,7 @@ static inline int map_bankwidth_supported(int w)
 	}
 }
 
-#define MAX_MAP_LONGS ( ((MAX_MAP_BANKWIDTH*8) + BITS_PER_LONG - 1) / BITS_PER_LONG )
+#define MAX_MAP_LONGS (((MAX_MAP_BANKWIDTH * 8) + BITS_PER_LONG - 1) / BITS_PER_LONG)
 
 typedef union {
 	unsigned long x[MAX_MAP_LONGS];
@@ -264,20 +264,22 @@ void unregister_mtd_chip_driver(struct mtd_chip_driver *);
 struct mtd_info *do_map_probe(const char *name, struct map_info *map);
 void map_destroy(struct mtd_info *mtd);
 
-#define ENABLE_VPP(map) do { if(map->set_vpp) map->set_vpp(map, 1); } while(0)
-#define DISABLE_VPP(map) do { if(map->set_vpp) map->set_vpp(map, 0); } while(0)
+#define ENABLE_VPP(map) do { if (map->set_vpp) map->set_vpp(map, 1); } while (0)
+#define DISABLE_VPP(map) do { if (map->set_vpp) map->set_vpp(map, 0); } while (0)
 
 #define INVALIDATE_CACHED_RANGE(map, from, size) \
-	do { if(map->inval_cache) map->inval_cache(map, from, size); } while(0)
+	do { if (map->inval_cache) map->inval_cache(map, from, size); } while (0)
 
 
 static inline int map_word_equal(struct map_info *map, map_word val1, map_word val2)
 {
 	int i;
-	for (i=0; i<map_words(map); i++) {
+
+	for (i = 0; i < map_words(map); i++) {
 		if (val1.x[i] != val2.x[i])
 			return 0;
 	}
+
 	return 1;
 }
 
@@ -286,9 +288,9 @@ static inline map_word map_word_and(struct map_info *map, map_word val1, map_wor
 	map_word r;
 	int i;
 
-	for (i=0; i<map_words(map); i++) {
+	for (i = 0; i < map_words(map); i++)
 		r.x[i] = val1.x[i] & val2.x[i];
-	}
+
 	return r;
 }
 
@@ -297,9 +299,9 @@ static inline map_word map_word_clr(struct map_info *map, map_word val1, map_wor
 	map_word r;
 	int i;
 
-	for (i=0; i<map_words(map); i++) {
+	for (i = 0; i < map_words(map); i++)
 		r.x[i] = val1.x[i] & ~val2.x[i];
-	}
+
 	return r;
 }
 
@@ -308,9 +310,9 @@ static inline map_word map_word_or(struct map_info *map, map_word val1, map_word
 	map_word r;
 	int i;
 
-	for (i=0; i<map_words(map); i++) {
+	for (i = 0; i < map_words(map); i++)
 		r.x[i] = val1.x[i] | val2.x[i];
-	}
+
 	return r;
 }
 
@@ -320,10 +322,11 @@ static inline int map_word_bitsset(struct map_info *map, map_word val1, map_word
 {
 	int i;
 
-	for (i=0; i<map_words(map); i++) {
+	for (i = 0; i < map_words(map); i++) {
 		if (val1.x[i] & val2.x[i])
 			return 1;
 	}
+
 	return 0;
 }
 
@@ -355,14 +358,16 @@ static inline map_word map_word_load_partial(struct map_info *map, map_word orig
 
 	if (map_bankwidth_is_large(map)) {
 		char *dest = (char *)&orig;
+
 		memcpy(dest+start, buf, len);
 	} else {
-		for (i=start; i < start+len; i++) {
+		for (i = start; i < start+len; i++) {
 			int bitpos;
+
 #ifdef __LITTLE_ENDIAN
-			bitpos = i*8;
+			bitpos = i * 8;
 #else /* __BIG_ENDIAN */
-			bitpos = (map_bankwidth(map)-1-i)*8;
+			bitpos = (map_bankwidth(map) - 1 - i) * 8;
 #endif
 			orig.x[0] &= ~(0xff << bitpos);
 			orig.x[0] |= (unsigned long)buf[i-start] << bitpos;
@@ -384,9 +389,10 @@ static inline map_word map_word_ff(struct map_info *map)
 
 	if (map_bankwidth(map) < MAP_FF_LIMIT) {
 		int bw = 8 * map_bankwidth(map);
+
 		r.x[0] = (1UL << bw) - 1;
 	} else {
-		for (i=0; i<map_words(map); i++)
+		for (i = 0; i < map_words(map); i++)
 			r.x[i] = ~0UL;
 	}
 	return r;
@@ -407,7 +413,7 @@ static inline map_word inline_map_read(struct map_info *map, unsigned long ofs)
 		r.x[0] = __raw_readq(map->virt + ofs);
 #endif
 	else if (map_bankwidth_is_large(map))
-		memcpy_fromio(r.x, map->virt+ofs, map->bankwidth);
+		memcpy_fromio(r.x, map->virt + ofs, map->bankwidth);
 	else
 		BUG();
 

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH] mtd: clean up whitespace in linux/mtd/map.h
  2015-03-10 16:51   ` Arnd Bergmann
  (?)
@ 2015-03-10 18:33     ` Joe Perches
  -1 siblings, 0 replies; 25+ messages in thread
From: Joe Perches @ 2015-03-10 18:33 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: David Woodhouse, Brian Norris, Jingoo Han, linux-mtd,
	Ezequiel Garcia, linux-arm-kernel, linux-kernel

On Tue, 2015-03-10 at 17:51 +0100, Arnd Bergmann wrote:
> As the only comments I got for the "mtd: cfi: reduce stack size"
> patch were about whitespace changes, it appears necessary to fix
> up the rest of the file as well, which contains the exact same
> mistakes.

trivia:

> diff --git a/include/linux/mtd/map.h b/include/linux/mtd/map.h
[]
> @@ -77,7 +77,7 @@
>  /* ensure we never evaluate anything shorted than an unsigned long
>   * to zero, and ensure we'll never miss the end of an comparison (bjd) */
>  
> -#define map_calc_words(map) ((map_bankwidth(map) + (sizeof(unsigned long)-1))/ sizeof(unsigned long))
> +#define map_calc_words(map) ((map_bankwidth(map) + (sizeof(unsigned long)-1)) / sizeof(unsigned long))

DIV_ROUND_UP?
 
>  #ifdef CONFIG_MTD_MAP_BANK_WIDTH_8
>  # ifdef map_bankwidth
> @@ -181,7 +181,7 @@ static inline int map_bankwidth_supported(int w)
>  	}
>  }
>  
> -#define MAX_MAP_LONGS ( ((MAX_MAP_BANKWIDTH*8) + BITS_PER_LONG - 1) / BITS_PER_LONG )
> +#define MAX_MAP_LONGS (((MAX_MAP_BANKWIDTH * 8) + BITS_PER_LONG - 1) / BITS_PER_LONG)

BITS_TO_LONGS?



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] mtd: clean up whitespace in linux/mtd/map.h
@ 2015-03-10 18:33     ` Joe Perches
  0 siblings, 0 replies; 25+ messages in thread
From: Joe Perches @ 2015-03-10 18:33 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jingoo Han, linux-kernel, linux-mtd, Ezequiel Garcia,
	Brian Norris, David Woodhouse, linux-arm-kernel

On Tue, 2015-03-10 at 17:51 +0100, Arnd Bergmann wrote:
> As the only comments I got for the "mtd: cfi: reduce stack size"
> patch were about whitespace changes, it appears necessary to fix
> up the rest of the file as well, which contains the exact same
> mistakes.

trivia:

> diff --git a/include/linux/mtd/map.h b/include/linux/mtd/map.h
[]
> @@ -77,7 +77,7 @@
>  /* ensure we never evaluate anything shorted than an unsigned long
>   * to zero, and ensure we'll never miss the end of an comparison (bjd) */
>  
> -#define map_calc_words(map) ((map_bankwidth(map) + (sizeof(unsigned long)-1))/ sizeof(unsigned long))
> +#define map_calc_words(map) ((map_bankwidth(map) + (sizeof(unsigned long)-1)) / sizeof(unsigned long))

DIV_ROUND_UP?
 
>  #ifdef CONFIG_MTD_MAP_BANK_WIDTH_8
>  # ifdef map_bankwidth
> @@ -181,7 +181,7 @@ static inline int map_bankwidth_supported(int w)
>  	}
>  }
>  
> -#define MAX_MAP_LONGS ( ((MAX_MAP_BANKWIDTH*8) + BITS_PER_LONG - 1) / BITS_PER_LONG )
> +#define MAX_MAP_LONGS (((MAX_MAP_BANKWIDTH * 8) + BITS_PER_LONG - 1) / BITS_PER_LONG)

BITS_TO_LONGS?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] mtd: clean up whitespace in linux/mtd/map.h
@ 2015-03-10 18:33     ` Joe Perches
  0 siblings, 0 replies; 25+ messages in thread
From: Joe Perches @ 2015-03-10 18:33 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2015-03-10 at 17:51 +0100, Arnd Bergmann wrote:
> As the only comments I got for the "mtd: cfi: reduce stack size"
> patch were about whitespace changes, it appears necessary to fix
> up the rest of the file as well, which contains the exact same
> mistakes.

trivia:

> diff --git a/include/linux/mtd/map.h b/include/linux/mtd/map.h
[]
> @@ -77,7 +77,7 @@
>  /* ensure we never evaluate anything shorted than an unsigned long
>   * to zero, and ensure we'll never miss the end of an comparison (bjd) */
>  
> -#define map_calc_words(map) ((map_bankwidth(map) + (sizeof(unsigned long)-1))/ sizeof(unsigned long))
> +#define map_calc_words(map) ((map_bankwidth(map) + (sizeof(unsigned long)-1)) / sizeof(unsigned long))

DIV_ROUND_UP?
 
>  #ifdef CONFIG_MTD_MAP_BANK_WIDTH_8
>  # ifdef map_bankwidth
> @@ -181,7 +181,7 @@ static inline int map_bankwidth_supported(int w)
>  	}
>  }
>  
> -#define MAX_MAP_LONGS ( ((MAX_MAP_BANKWIDTH*8) + BITS_PER_LONG - 1) / BITS_PER_LONG )
> +#define MAX_MAP_LONGS (((MAX_MAP_BANKWIDTH * 8) + BITS_PER_LONG - 1) / BITS_PER_LONG)

BITS_TO_LONGS?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3] mtd: cfi: reduce stack size
  2015-03-10 16:48 ` Arnd Bergmann
  (?)
@ 2015-03-10 19:41   ` Brian Norris
  -1 siblings, 0 replies; 25+ messages in thread
From: Brian Norris @ 2015-03-10 19:41 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: David Woodhouse, Jingoo Han, linux-mtd, Ezequiel Garcia,
	linux-arm-kernel, linux-kernel

On Tue, Mar 10, 2015 at 05:48:37PM +0100, Arnd Bergmann wrote:
> The cfi_staa_write_buffers function uses a large amount of kernel stack
> whenever CONFIG_MTD_MAP_BANK_WIDTH_32 is set, and that results in a
> warning on ARM allmodconfig builds:
> 
> drivers/mtd/chips/cfi_cmdset_0020.c: In function 'cfi_staa_write_buffers':
> drivers/mtd/chips/cfi_cmdset_0020.c:651:1: warning: the frame size of 1208 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> 
> It turns out that this is largely a result of a suboptimal implementation
> of map_word_andequal(). Replacing this function with a straightforward
> one reduces the stack size in this function by exactly 200 bytes,
> shrinks the .text segment for this file from 27648 bytes to 26608 bytes,
> and makes the warning go away.

Patch looks OK to me, and those results are nice.

Did you (or anyone else) do any testing?

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> v3: more whitespace changes
> v2: whitespace changes
> 
> diff --git a/include/linux/mtd/map.h b/include/linux/mtd/map.h
> index 47c59991491b..29975c73a953 100644
> --- a/include/linux/mtd/map.h
> +++ b/include/linux/mtd/map.h
> @@ -316,7 +316,17 @@ static inline map_word map_word_or(struct map_info *map, map_word val1, map_word
>  	return r;
>  }
>  
> -#define map_word_andequal(m, a, b, z) map_word_equal(m, z, map_word_and(m, a, b))
> +static inline int map_word_andequal(struct map_info *map, map_word val1, map_word val2, map_word val3)
> +{
> +	int i;
> +
> +	for (i = 0; i < map_words(map); i++) {
> +		if ((val1.x[i] & val2.x[i]) != val3.x[i])
> +			return 0;
> +	}
> +
> +	return 1;
> +}
>  
>  static inline int map_word_bitsset(struct map_info *map, map_word val1, map_word val2)
>  {
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3] mtd: cfi: reduce stack size
@ 2015-03-10 19:41   ` Brian Norris
  0 siblings, 0 replies; 25+ messages in thread
From: Brian Norris @ 2015-03-10 19:41 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jingoo Han, linux-kernel, linux-mtd, Ezequiel Garcia,
	David Woodhouse, linux-arm-kernel

On Tue, Mar 10, 2015 at 05:48:37PM +0100, Arnd Bergmann wrote:
> The cfi_staa_write_buffers function uses a large amount of kernel stack
> whenever CONFIG_MTD_MAP_BANK_WIDTH_32 is set, and that results in a
> warning on ARM allmodconfig builds:
> 
> drivers/mtd/chips/cfi_cmdset_0020.c: In function 'cfi_staa_write_buffers':
> drivers/mtd/chips/cfi_cmdset_0020.c:651:1: warning: the frame size of 1208 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> 
> It turns out that this is largely a result of a suboptimal implementation
> of map_word_andequal(). Replacing this function with a straightforward
> one reduces the stack size in this function by exactly 200 bytes,
> shrinks the .text segment for this file from 27648 bytes to 26608 bytes,
> and makes the warning go away.

Patch looks OK to me, and those results are nice.

Did you (or anyone else) do any testing?

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> v3: more whitespace changes
> v2: whitespace changes
> 
> diff --git a/include/linux/mtd/map.h b/include/linux/mtd/map.h
> index 47c59991491b..29975c73a953 100644
> --- a/include/linux/mtd/map.h
> +++ b/include/linux/mtd/map.h
> @@ -316,7 +316,17 @@ static inline map_word map_word_or(struct map_info *map, map_word val1, map_word
>  	return r;
>  }
>  
> -#define map_word_andequal(m, a, b, z) map_word_equal(m, z, map_word_and(m, a, b))
> +static inline int map_word_andequal(struct map_info *map, map_word val1, map_word val2, map_word val3)
> +{
> +	int i;
> +
> +	for (i = 0; i < map_words(map); i++) {
> +		if ((val1.x[i] & val2.x[i]) != val3.x[i])
> +			return 0;
> +	}
> +
> +	return 1;
> +}
>  
>  static inline int map_word_bitsset(struct map_info *map, map_word val1, map_word val2)
>  {
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3] mtd: cfi: reduce stack size
@ 2015-03-10 19:41   ` Brian Norris
  0 siblings, 0 replies; 25+ messages in thread
From: Brian Norris @ 2015-03-10 19:41 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Mar 10, 2015 at 05:48:37PM +0100, Arnd Bergmann wrote:
> The cfi_staa_write_buffers function uses a large amount of kernel stack
> whenever CONFIG_MTD_MAP_BANK_WIDTH_32 is set, and that results in a
> warning on ARM allmodconfig builds:
> 
> drivers/mtd/chips/cfi_cmdset_0020.c: In function 'cfi_staa_write_buffers':
> drivers/mtd/chips/cfi_cmdset_0020.c:651:1: warning: the frame size of 1208 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> 
> It turns out that this is largely a result of a suboptimal implementation
> of map_word_andequal(). Replacing this function with a straightforward
> one reduces the stack size in this function by exactly 200 bytes,
> shrinks the .text segment for this file from 27648 bytes to 26608 bytes,
> and makes the warning go away.

Patch looks OK to me, and those results are nice.

Did you (or anyone else) do any testing?

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> v3: more whitespace changes
> v2: whitespace changes
> 
> diff --git a/include/linux/mtd/map.h b/include/linux/mtd/map.h
> index 47c59991491b..29975c73a953 100644
> --- a/include/linux/mtd/map.h
> +++ b/include/linux/mtd/map.h
> @@ -316,7 +316,17 @@ static inline map_word map_word_or(struct map_info *map, map_word val1, map_word
>  	return r;
>  }
>  
> -#define map_word_andequal(m, a, b, z) map_word_equal(m, z, map_word_and(m, a, b))
> +static inline int map_word_andequal(struct map_info *map, map_word val1, map_word val2, map_word val3)
> +{
> +	int i;
> +
> +	for (i = 0; i < map_words(map); i++) {
> +		if ((val1.x[i] & val2.x[i]) != val3.x[i])
> +			return 0;
> +	}
> +
> +	return 1;
> +}
>  
>  static inline int map_word_bitsset(struct map_info *map, map_word val1, map_word val2)
>  {
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] mtd: clean up whitespace in linux/mtd/map.h
  2015-03-10 18:33     ` Joe Perches
  (?)
@ 2015-03-10 19:58       ` Brian Norris
  -1 siblings, 0 replies; 25+ messages in thread
From: Brian Norris @ 2015-03-10 19:58 UTC (permalink / raw)
  To: Joe Perches
  Cc: Arnd Bergmann, David Woodhouse, Jingoo Han, linux-mtd,
	Ezequiel Garcia, linux-arm-kernel, linux-kernel

On Tue, Mar 10, 2015 at 11:33:36AM -0700, Joe Perches wrote:
> On Tue, 2015-03-10 at 17:51 +0100, Arnd Bergmann wrote:
> > As the only comments I got for the "mtd: cfi: reduce stack size"
> > patch were about whitespace changes, it appears necessary to fix
> > up the rest of the file as well, which contains the exact same
> > mistakes.
> 
> trivia:
> 
> > diff --git a/include/linux/mtd/map.h b/include/linux/mtd/map.h
> []
> > @@ -77,7 +77,7 @@
> >  /* ensure we never evaluate anything shorted than an unsigned long
> >   * to zero, and ensure we'll never miss the end of an comparison (bjd) */
> >  
> > -#define map_calc_words(map) ((map_bankwidth(map) + (sizeof(unsigned long)-1))/ sizeof(unsigned long))
> > +#define map_calc_words(map) ((map_bankwidth(map) + (sizeof(unsigned long)-1)) / sizeof(unsigned long))
> 
> DIV_ROUND_UP?
>  
> >  #ifdef CONFIG_MTD_MAP_BANK_WIDTH_8
> >  # ifdef map_bankwidth
> > @@ -181,7 +181,7 @@ static inline int map_bankwidth_supported(int w)
> >  	}
> >  }
> >  
> > -#define MAX_MAP_LONGS ( ((MAX_MAP_BANKWIDTH*8) + BITS_PER_LONG - 1) / BITS_PER_LONG )
> > +#define MAX_MAP_LONGS (((MAX_MAP_BANKWIDTH * 8) + BITS_PER_LONG - 1) / BITS_PER_LONG)
> 
> BITS_TO_LONGS?

It seems the $subject patch is really not that necessary, as it was just
inspired by similarly trivial comments. But I thought CodingStyle
was supposed to mostly be a guide for new code, not a charter to "fix
up" old code like drivers/mtd/{chips,maps}.

So I would have been happy with ignoring the whitespace comments on the
v1 stack usage patch (esp. since it *did* match the existing style), and
avoiding the ensuing comments about helper macros. IMO, it's pretty
silly when a simple patch to fix a real issue turns into an extended
search for other trivial issues.

I'll probably take both of Arnd's patches as they stand, but any more
trivial requests to stable code like this should come in the form of
real patches, not respins of Arnd's patch.

Disclaimer: I'm probably just as guilty of adding trivial tangential
comments/requests that distract from the original issue. So I'm partly
speaking to myself here.

Happy coding,
Brian

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] mtd: clean up whitespace in linux/mtd/map.h
@ 2015-03-10 19:58       ` Brian Norris
  0 siblings, 0 replies; 25+ messages in thread
From: Brian Norris @ 2015-03-10 19:58 UTC (permalink / raw)
  To: Joe Perches
  Cc: Arnd Bergmann, Jingoo Han, linux-kernel, linux-mtd,
	Ezequiel Garcia, David Woodhouse, linux-arm-kernel

On Tue, Mar 10, 2015 at 11:33:36AM -0700, Joe Perches wrote:
> On Tue, 2015-03-10 at 17:51 +0100, Arnd Bergmann wrote:
> > As the only comments I got for the "mtd: cfi: reduce stack size"
> > patch were about whitespace changes, it appears necessary to fix
> > up the rest of the file as well, which contains the exact same
> > mistakes.
> 
> trivia:
> 
> > diff --git a/include/linux/mtd/map.h b/include/linux/mtd/map.h
> []
> > @@ -77,7 +77,7 @@
> >  /* ensure we never evaluate anything shorted than an unsigned long
> >   * to zero, and ensure we'll never miss the end of an comparison (bjd) */
> >  
> > -#define map_calc_words(map) ((map_bankwidth(map) + (sizeof(unsigned long)-1))/ sizeof(unsigned long))
> > +#define map_calc_words(map) ((map_bankwidth(map) + (sizeof(unsigned long)-1)) / sizeof(unsigned long))
> 
> DIV_ROUND_UP?
>  
> >  #ifdef CONFIG_MTD_MAP_BANK_WIDTH_8
> >  # ifdef map_bankwidth
> > @@ -181,7 +181,7 @@ static inline int map_bankwidth_supported(int w)
> >  	}
> >  }
> >  
> > -#define MAX_MAP_LONGS ( ((MAX_MAP_BANKWIDTH*8) + BITS_PER_LONG - 1) / BITS_PER_LONG )
> > +#define MAX_MAP_LONGS (((MAX_MAP_BANKWIDTH * 8) + BITS_PER_LONG - 1) / BITS_PER_LONG)
> 
> BITS_TO_LONGS?

It seems the $subject patch is really not that necessary, as it was just
inspired by similarly trivial comments. But I thought CodingStyle
was supposed to mostly be a guide for new code, not a charter to "fix
up" old code like drivers/mtd/{chips,maps}.

So I would have been happy with ignoring the whitespace comments on the
v1 stack usage patch (esp. since it *did* match the existing style), and
avoiding the ensuing comments about helper macros. IMO, it's pretty
silly when a simple patch to fix a real issue turns into an extended
search for other trivial issues.

I'll probably take both of Arnd's patches as they stand, but any more
trivial requests to stable code like this should come in the form of
real patches, not respins of Arnd's patch.

Disclaimer: I'm probably just as guilty of adding trivial tangential
comments/requests that distract from the original issue. So I'm partly
speaking to myself here.

Happy coding,
Brian

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] mtd: clean up whitespace in linux/mtd/map.h
@ 2015-03-10 19:58       ` Brian Norris
  0 siblings, 0 replies; 25+ messages in thread
From: Brian Norris @ 2015-03-10 19:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Mar 10, 2015 at 11:33:36AM -0700, Joe Perches wrote:
> On Tue, 2015-03-10 at 17:51 +0100, Arnd Bergmann wrote:
> > As the only comments I got for the "mtd: cfi: reduce stack size"
> > patch were about whitespace changes, it appears necessary to fix
> > up the rest of the file as well, which contains the exact same
> > mistakes.
> 
> trivia:
> 
> > diff --git a/include/linux/mtd/map.h b/include/linux/mtd/map.h
> []
> > @@ -77,7 +77,7 @@
> >  /* ensure we never evaluate anything shorted than an unsigned long
> >   * to zero, and ensure we'll never miss the end of an comparison (bjd) */
> >  
> > -#define map_calc_words(map) ((map_bankwidth(map) + (sizeof(unsigned long)-1))/ sizeof(unsigned long))
> > +#define map_calc_words(map) ((map_bankwidth(map) + (sizeof(unsigned long)-1)) / sizeof(unsigned long))
> 
> DIV_ROUND_UP?
>  
> >  #ifdef CONFIG_MTD_MAP_BANK_WIDTH_8
> >  # ifdef map_bankwidth
> > @@ -181,7 +181,7 @@ static inline int map_bankwidth_supported(int w)
> >  	}
> >  }
> >  
> > -#define MAX_MAP_LONGS ( ((MAX_MAP_BANKWIDTH*8) + BITS_PER_LONG - 1) / BITS_PER_LONG )
> > +#define MAX_MAP_LONGS (((MAX_MAP_BANKWIDTH * 8) + BITS_PER_LONG - 1) / BITS_PER_LONG)
> 
> BITS_TO_LONGS?

It seems the $subject patch is really not that necessary, as it was just
inspired by similarly trivial comments. But I thought CodingStyle
was supposed to mostly be a guide for new code, not a charter to "fix
up" old code like drivers/mtd/{chips,maps}.

So I would have been happy with ignoring the whitespace comments on the
v1 stack usage patch (esp. since it *did* match the existing style), and
avoiding the ensuing comments about helper macros. IMO, it's pretty
silly when a simple patch to fix a real issue turns into an extended
search for other trivial issues.

I'll probably take both of Arnd's patches as they stand, but any more
trivial requests to stable code like this should come in the form of
real patches, not respins of Arnd's patch.

Disclaimer: I'm probably just as guilty of adding trivial tangential
comments/requests that distract from the original issue. So I'm partly
speaking to myself here.

Happy coding,
Brian

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] mtd: clean up whitespace in linux/mtd/map.h
  2015-03-10 19:58       ` Brian Norris
  (?)
@ 2015-03-10 20:28         ` Joe Perches
  -1 siblings, 0 replies; 25+ messages in thread
From: Joe Perches @ 2015-03-10 20:28 UTC (permalink / raw)
  To: Brian Norris
  Cc: Arnd Bergmann, David Woodhouse, Jingoo Han, linux-mtd,
	Ezequiel Garcia, linux-arm-kernel, linux-kernel

On Tue, 2015-03-10 at 12:58 -0700, Brian Norris wrote:
> On Tue, Mar 10, 2015 at 11:33:36AM -0700, Joe Perches wrote:
> > On Tue, 2015-03-10 at 17:51 +0100, Arnd Bergmann wrote:
> > > As the only comments I got for the "mtd: cfi: reduce stack size"
> > > patch were about whitespace changes, it appears necessary to fix
> > > up the rest of the file as well, which contains the exact same
> > > mistakes.
> > 
> > trivia:
> > 
> > > diff --git a/include/linux/mtd/map.h b/include/linux/mtd/map.h
> > []
> > > @@ -77,7 +77,7 @@
> > >  /* ensure we never evaluate anything shorted than an unsigned long
> > >   * to zero, and ensure we'll never miss the end of an comparison (bjd) */
> > >  
> > > -#define map_calc_words(map) ((map_bankwidth(map) + (sizeof(unsigned long)-1))/ sizeof(unsigned long))
> > > +#define map_calc_words(map) ((map_bankwidth(map) + (sizeof(unsigned long)-1)) / sizeof(unsigned long))
> > 
> > DIV_ROUND_UP?
> >  
> > >  #ifdef CONFIG_MTD_MAP_BANK_WIDTH_8
> > >  # ifdef map_bankwidth
> > > @@ -181,7 +181,7 @@ static inline int map_bankwidth_supported(int w)
> > >  	}
> > >  }
> > >  
> > > -#define MAX_MAP_LONGS ( ((MAX_MAP_BANKWIDTH*8) + BITS_PER_LONG - 1) / BITS_PER_LONG )
> > > +#define MAX_MAP_LONGS (((MAX_MAP_BANKWIDTH * 8) + BITS_PER_LONG - 1) / BITS_PER_LONG)
> > 
> > BITS_TO_LONGS?
> 
> It seems the $subject patch is really not that necessary,

Coding style patches rarely are.

> as it was just
> inspired by similarly trivial comments. But I thought CodingStyle
> was supposed to mostly be a guide for new code, not a charter to "fix
> up" old code like drivers/mtd/{chips,maps}.

'tisn't but consistency has its own virtue.

> So I would have been happy with ignoring the whitespace comments on the
> v1 stack usage patch (esp. since it *did* match the existing style), and
> avoiding the ensuing comments about helper macros. IMO, it's pretty
> silly when a simple patch to fix a real issue turns into an extended
> search for other trivial issues.
> 
> I'll probably take both of Arnd's patches as they stand,

No worries.   The comments weren't meant to derail
the original patches.

>  but any more
> trivial requests to stable code like this should come in the form of
> real patches, not respins of Arnd's patch.

I'm not respinning the patches.
If Arnd wants to do more work, that's up to him.


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] mtd: clean up whitespace in linux/mtd/map.h
@ 2015-03-10 20:28         ` Joe Perches
  0 siblings, 0 replies; 25+ messages in thread
From: Joe Perches @ 2015-03-10 20:28 UTC (permalink / raw)
  To: Brian Norris
  Cc: Arnd Bergmann, Jingoo Han, linux-kernel, linux-mtd,
	Ezequiel Garcia, David Woodhouse, linux-arm-kernel

On Tue, 2015-03-10 at 12:58 -0700, Brian Norris wrote:
> On Tue, Mar 10, 2015 at 11:33:36AM -0700, Joe Perches wrote:
> > On Tue, 2015-03-10 at 17:51 +0100, Arnd Bergmann wrote:
> > > As the only comments I got for the "mtd: cfi: reduce stack size"
> > > patch were about whitespace changes, it appears necessary to fix
> > > up the rest of the file as well, which contains the exact same
> > > mistakes.
> > 
> > trivia:
> > 
> > > diff --git a/include/linux/mtd/map.h b/include/linux/mtd/map.h
> > []
> > > @@ -77,7 +77,7 @@
> > >  /* ensure we never evaluate anything shorted than an unsigned long
> > >   * to zero, and ensure we'll never miss the end of an comparison (bjd) */
> > >  
> > > -#define map_calc_words(map) ((map_bankwidth(map) + (sizeof(unsigned long)-1))/ sizeof(unsigned long))
> > > +#define map_calc_words(map) ((map_bankwidth(map) + (sizeof(unsigned long)-1)) / sizeof(unsigned long))
> > 
> > DIV_ROUND_UP?
> >  
> > >  #ifdef CONFIG_MTD_MAP_BANK_WIDTH_8
> > >  # ifdef map_bankwidth
> > > @@ -181,7 +181,7 @@ static inline int map_bankwidth_supported(int w)
> > >  	}
> > >  }
> > >  
> > > -#define MAX_MAP_LONGS ( ((MAX_MAP_BANKWIDTH*8) + BITS_PER_LONG - 1) / BITS_PER_LONG )
> > > +#define MAX_MAP_LONGS (((MAX_MAP_BANKWIDTH * 8) + BITS_PER_LONG - 1) / BITS_PER_LONG)
> > 
> > BITS_TO_LONGS?
> 
> It seems the $subject patch is really not that necessary,

Coding style patches rarely are.

> as it was just
> inspired by similarly trivial comments. But I thought CodingStyle
> was supposed to mostly be a guide for new code, not a charter to "fix
> up" old code like drivers/mtd/{chips,maps}.

'tisn't but consistency has its own virtue.

> So I would have been happy with ignoring the whitespace comments on the
> v1 stack usage patch (esp. since it *did* match the existing style), and
> avoiding the ensuing comments about helper macros. IMO, it's pretty
> silly when a simple patch to fix a real issue turns into an extended
> search for other trivial issues.
> 
> I'll probably take both of Arnd's patches as they stand,

No worries.   The comments weren't meant to derail
the original patches.

>  but any more
> trivial requests to stable code like this should come in the form of
> real patches, not respins of Arnd's patch.

I'm not respinning the patches.
If Arnd wants to do more work, that's up to him.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] mtd: clean up whitespace in linux/mtd/map.h
@ 2015-03-10 20:28         ` Joe Perches
  0 siblings, 0 replies; 25+ messages in thread
From: Joe Perches @ 2015-03-10 20:28 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2015-03-10 at 12:58 -0700, Brian Norris wrote:
> On Tue, Mar 10, 2015 at 11:33:36AM -0700, Joe Perches wrote:
> > On Tue, 2015-03-10 at 17:51 +0100, Arnd Bergmann wrote:
> > > As the only comments I got for the "mtd: cfi: reduce stack size"
> > > patch were about whitespace changes, it appears necessary to fix
> > > up the rest of the file as well, which contains the exact same
> > > mistakes.
> > 
> > trivia:
> > 
> > > diff --git a/include/linux/mtd/map.h b/include/linux/mtd/map.h
> > []
> > > @@ -77,7 +77,7 @@
> > >  /* ensure we never evaluate anything shorted than an unsigned long
> > >   * to zero, and ensure we'll never miss the end of an comparison (bjd) */
> > >  
> > > -#define map_calc_words(map) ((map_bankwidth(map) + (sizeof(unsigned long)-1))/ sizeof(unsigned long))
> > > +#define map_calc_words(map) ((map_bankwidth(map) + (sizeof(unsigned long)-1)) / sizeof(unsigned long))
> > 
> > DIV_ROUND_UP?
> >  
> > >  #ifdef CONFIG_MTD_MAP_BANK_WIDTH_8
> > >  # ifdef map_bankwidth
> > > @@ -181,7 +181,7 @@ static inline int map_bankwidth_supported(int w)
> > >  	}
> > >  }
> > >  
> > > -#define MAX_MAP_LONGS ( ((MAX_MAP_BANKWIDTH*8) + BITS_PER_LONG - 1) / BITS_PER_LONG )
> > > +#define MAX_MAP_LONGS (((MAX_MAP_BANKWIDTH * 8) + BITS_PER_LONG - 1) / BITS_PER_LONG)
> > 
> > BITS_TO_LONGS?
> 
> It seems the $subject patch is really not that necessary,

Coding style patches rarely are.

> as it was just
> inspired by similarly trivial comments. But I thought CodingStyle
> was supposed to mostly be a guide for new code, not a charter to "fix
> up" old code like drivers/mtd/{chips,maps}.

'tisn't but consistency has its own virtue.

> So I would have been happy with ignoring the whitespace comments on the
> v1 stack usage patch (esp. since it *did* match the existing style), and
> avoiding the ensuing comments about helper macros. IMO, it's pretty
> silly when a simple patch to fix a real issue turns into an extended
> search for other trivial issues.
> 
> I'll probably take both of Arnd's patches as they stand,

No worries.   The comments weren't meant to derail
the original patches.

>  but any more
> trivial requests to stable code like this should come in the form of
> real patches, not respins of Arnd's patch.

I'm not respinning the patches.
If Arnd wants to do more work, that's up to him.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3] mtd: cfi: reduce stack size
  2015-03-10 19:41   ` Brian Norris
  (?)
@ 2015-03-10 21:00     ` Arnd Bergmann
  -1 siblings, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2015-03-10 21:00 UTC (permalink / raw)
  To: Brian Norris
  Cc: David Woodhouse, Jingoo Han, linux-mtd, Ezequiel Garcia,
	linux-arm-kernel, linux-kernel

On Tuesday 10 March 2015, Brian Norris wrote:
> On Tue, Mar 10, 2015 at 05:48:37PM +0100, Arnd Bergmann wrote:
> > The cfi_staa_write_buffers function uses a large amount of kernel stack
> > whenever CONFIG_MTD_MAP_BANK_WIDTH_32 is set, and that results in a
> > warning on ARM allmodconfig builds:
> > 
> > drivers/mtd/chips/cfi_cmdset_0020.c: In function 'cfi_staa_write_buffers':
> > drivers/mtd/chips/cfi_cmdset_0020.c:651:1: warning: the frame size of 1208 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> > 
> > It turns out that this is largely a result of a suboptimal implementation
> > of map_word_andequal(). Replacing this function with a straightforward
> > one reduces the stack size in this function by exactly 200 bytes,
> > shrinks the .text segment for this file from 27648 bytes to 26608 bytes,
> > and makes the warning go away.
> 
> Patch looks OK to me, and those results are nice.
> 
> Did you (or anyone else) do any testing?

No runtime tests, I only built them, sorry.

	Arnd

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3] mtd: cfi: reduce stack size
@ 2015-03-10 21:00     ` Arnd Bergmann
  0 siblings, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2015-03-10 21:00 UTC (permalink / raw)
  To: Brian Norris
  Cc: Jingoo Han, linux-kernel, linux-mtd, Ezequiel Garcia,
	David Woodhouse, linux-arm-kernel

On Tuesday 10 March 2015, Brian Norris wrote:
> On Tue, Mar 10, 2015 at 05:48:37PM +0100, Arnd Bergmann wrote:
> > The cfi_staa_write_buffers function uses a large amount of kernel stack
> > whenever CONFIG_MTD_MAP_BANK_WIDTH_32 is set, and that results in a
> > warning on ARM allmodconfig builds:
> > 
> > drivers/mtd/chips/cfi_cmdset_0020.c: In function 'cfi_staa_write_buffers':
> > drivers/mtd/chips/cfi_cmdset_0020.c:651:1: warning: the frame size of 1208 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> > 
> > It turns out that this is largely a result of a suboptimal implementation
> > of map_word_andequal(). Replacing this function with a straightforward
> > one reduces the stack size in this function by exactly 200 bytes,
> > shrinks the .text segment for this file from 27648 bytes to 26608 bytes,
> > and makes the warning go away.
> 
> Patch looks OK to me, and those results are nice.
> 
> Did you (or anyone else) do any testing?

No runtime tests, I only built them, sorry.

	Arnd

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3] mtd: cfi: reduce stack size
@ 2015-03-10 21:00     ` Arnd Bergmann
  0 siblings, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2015-03-10 21:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Tuesday 10 March 2015, Brian Norris wrote:
> On Tue, Mar 10, 2015 at 05:48:37PM +0100, Arnd Bergmann wrote:
> > The cfi_staa_write_buffers function uses a large amount of kernel stack
> > whenever CONFIG_MTD_MAP_BANK_WIDTH_32 is set, and that results in a
> > warning on ARM allmodconfig builds:
> > 
> > drivers/mtd/chips/cfi_cmdset_0020.c: In function 'cfi_staa_write_buffers':
> > drivers/mtd/chips/cfi_cmdset_0020.c:651:1: warning: the frame size of 1208 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> > 
> > It turns out that this is largely a result of a suboptimal implementation
> > of map_word_andequal(). Replacing this function with a straightforward
> > one reduces the stack size in this function by exactly 200 bytes,
> > shrinks the .text segment for this file from 27648 bytes to 26608 bytes,
> > and makes the warning go away.
> 
> Patch looks OK to me, and those results are nice.
> 
> Did you (or anyone else) do any testing?

No runtime tests, I only built them, sorry.

	Arnd

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3] mtd: cfi: reduce stack size
  2015-03-10 16:48 ` Arnd Bergmann
  (?)
@ 2015-03-11 23:49   ` Brian Norris
  -1 siblings, 0 replies; 25+ messages in thread
From: Brian Norris @ 2015-03-11 23:49 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: David Woodhouse, Jingoo Han, linux-mtd, Ezequiel Garcia,
	linux-arm-kernel, linux-kernel

On Tue, Mar 10, 2015 at 05:48:37PM +0100, Arnd Bergmann wrote:
> The cfi_staa_write_buffers function uses a large amount of kernel stack
> whenever CONFIG_MTD_MAP_BANK_WIDTH_32 is set, and that results in a
> warning on ARM allmodconfig builds:
> 
> drivers/mtd/chips/cfi_cmdset_0020.c: In function 'cfi_staa_write_buffers':
> drivers/mtd/chips/cfi_cmdset_0020.c:651:1: warning: the frame size of 1208 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> 
> It turns out that this is largely a result of a suboptimal implementation
> of map_word_andequal(). Replacing this function with a straightforward
> one reduces the stack size in this function by exactly 200 bytes,
> shrinks the .text segment for this file from 27648 bytes to 26608 bytes,
> and makes the warning go away.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> v3: more whitespace changes
> v2: whitespace changes

Applied to l2-mtd.git. Thanks!

Brian

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3] mtd: cfi: reduce stack size
@ 2015-03-11 23:49   ` Brian Norris
  0 siblings, 0 replies; 25+ messages in thread
From: Brian Norris @ 2015-03-11 23:49 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jingoo Han, linux-kernel, linux-mtd, Ezequiel Garcia,
	David Woodhouse, linux-arm-kernel

On Tue, Mar 10, 2015 at 05:48:37PM +0100, Arnd Bergmann wrote:
> The cfi_staa_write_buffers function uses a large amount of kernel stack
> whenever CONFIG_MTD_MAP_BANK_WIDTH_32 is set, and that results in a
> warning on ARM allmodconfig builds:
> 
> drivers/mtd/chips/cfi_cmdset_0020.c: In function 'cfi_staa_write_buffers':
> drivers/mtd/chips/cfi_cmdset_0020.c:651:1: warning: the frame size of 1208 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> 
> It turns out that this is largely a result of a suboptimal implementation
> of map_word_andequal(). Replacing this function with a straightforward
> one reduces the stack size in this function by exactly 200 bytes,
> shrinks the .text segment for this file from 27648 bytes to 26608 bytes,
> and makes the warning go away.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> v3: more whitespace changes
> v2: whitespace changes

Applied to l2-mtd.git. Thanks!

Brian

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3] mtd: cfi: reduce stack size
@ 2015-03-11 23:49   ` Brian Norris
  0 siblings, 0 replies; 25+ messages in thread
From: Brian Norris @ 2015-03-11 23:49 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Mar 10, 2015 at 05:48:37PM +0100, Arnd Bergmann wrote:
> The cfi_staa_write_buffers function uses a large amount of kernel stack
> whenever CONFIG_MTD_MAP_BANK_WIDTH_32 is set, and that results in a
> warning on ARM allmodconfig builds:
> 
> drivers/mtd/chips/cfi_cmdset_0020.c: In function 'cfi_staa_write_buffers':
> drivers/mtd/chips/cfi_cmdset_0020.c:651:1: warning: the frame size of 1208 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> 
> It turns out that this is largely a result of a suboptimal implementation
> of map_word_andequal(). Replacing this function with a straightforward
> one reduces the stack size in this function by exactly 200 bytes,
> shrinks the .text segment for this file from 27648 bytes to 26608 bytes,
> and makes the warning go away.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> v3: more whitespace changes
> v2: whitespace changes

Applied to l2-mtd.git. Thanks!

Brian

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] mtd: clean up whitespace in linux/mtd/map.h
  2015-03-10 16:51   ` Arnd Bergmann
  (?)
@ 2015-03-11 23:50     ` Brian Norris
  -1 siblings, 0 replies; 25+ messages in thread
From: Brian Norris @ 2015-03-11 23:50 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: David Woodhouse, Jingoo Han, linux-mtd, Ezequiel Garcia,
	linux-arm-kernel, linux-kernel

On Tue, Mar 10, 2015 at 05:51:55PM +0100, Arnd Bergmann wrote:
> As the only comments I got for the "mtd: cfi: reduce stack size"
> patch were about whitespace changes, it appears necessary to fix
> up the rest of the file as well, which contains the exact same
> mistakes.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> The two patches can be applied in either order, or this one dropped
> entirely, I don't care as long as the other patch can get merged,
> or get some constructive feedback.

Also applied to l2-mtd.git.

Brian

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] mtd: clean up whitespace in linux/mtd/map.h
@ 2015-03-11 23:50     ` Brian Norris
  0 siblings, 0 replies; 25+ messages in thread
From: Brian Norris @ 2015-03-11 23:50 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jingoo Han, linux-kernel, linux-mtd, Ezequiel Garcia,
	David Woodhouse, linux-arm-kernel

On Tue, Mar 10, 2015 at 05:51:55PM +0100, Arnd Bergmann wrote:
> As the only comments I got for the "mtd: cfi: reduce stack size"
> patch were about whitespace changes, it appears necessary to fix
> up the rest of the file as well, which contains the exact same
> mistakes.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> The two patches can be applied in either order, or this one dropped
> entirely, I don't care as long as the other patch can get merged,
> or get some constructive feedback.

Also applied to l2-mtd.git.

Brian

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] mtd: clean up whitespace in linux/mtd/map.h
@ 2015-03-11 23:50     ` Brian Norris
  0 siblings, 0 replies; 25+ messages in thread
From: Brian Norris @ 2015-03-11 23:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Mar 10, 2015 at 05:51:55PM +0100, Arnd Bergmann wrote:
> As the only comments I got for the "mtd: cfi: reduce stack size"
> patch were about whitespace changes, it appears necessary to fix
> up the rest of the file as well, which contains the exact same
> mistakes.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> The two patches can be applied in either order, or this one dropped
> entirely, I don't care as long as the other patch can get merged,
> or get some constructive feedback.

Also applied to l2-mtd.git.

Brian

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2015-03-11 23:50 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-10 16:48 [PATCH v3] mtd: cfi: reduce stack size Arnd Bergmann
2015-03-10 16:48 ` Arnd Bergmann
2015-03-10 16:51 ` [PATCH] mtd: clean up whitespace in linux/mtd/map.h Arnd Bergmann
2015-03-10 16:51   ` Arnd Bergmann
2015-03-10 18:33   ` Joe Perches
2015-03-10 18:33     ` Joe Perches
2015-03-10 18:33     ` Joe Perches
2015-03-10 19:58     ` Brian Norris
2015-03-10 19:58       ` Brian Norris
2015-03-10 19:58       ` Brian Norris
2015-03-10 20:28       ` Joe Perches
2015-03-10 20:28         ` Joe Perches
2015-03-10 20:28         ` Joe Perches
2015-03-11 23:50   ` Brian Norris
2015-03-11 23:50     ` Brian Norris
2015-03-11 23:50     ` Brian Norris
2015-03-10 19:41 ` [PATCH v3] mtd: cfi: reduce stack size Brian Norris
2015-03-10 19:41   ` Brian Norris
2015-03-10 19:41   ` Brian Norris
2015-03-10 21:00   ` Arnd Bergmann
2015-03-10 21:00     ` Arnd Bergmann
2015-03-10 21:00     ` Arnd Bergmann
2015-03-11 23:49 ` Brian Norris
2015-03-11 23:49   ` Brian Norris
2015-03-11 23:49   ` Brian Norris

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.