Patchwork mtd: dc21285.c: remove double check of CONFIG_ARCH_NETWINDER

login
register
mail settings
Submitter Christian Dietrich
Date May 29, 2012, 10:06 a.m.
Message ID <20120529100646.GA23383@faui49q.informatik.uni-erlangen.de>
Download mbox | patch
Permalink /patch/161728/
State New
Headers show

Comments

Christian Dietrich - May 29, 2012, 10:06 a.m.
When CONFIG_ARCH_NETWINDER is unset nw_en_write is a NOP. But
machine_is_netwinder() also checks for
CONFIG_ARCH_NETWINDER. Therefore in the !netwinder case the
preprocessed code is:

if (0)
   do {} while(0);

Signed-off-by: Christian Dietrich <christian.dietrich@informatik.uni-erlangen.de>
---
 drivers/mtd/maps/dc21285.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)
David Woodhouse - May 29, 2012, 10:19 a.m.
On Tue, 2012-05-29 at 12:06 +0200, Christian Dietrich wrote:
> When CONFIG_ARCH_NETWINDER is unset nw_en_write is a NOP. But
> machine_is_netwinder() also checks for
> CONFIG_ARCH_NETWINDER. Therefore in the !netwinder case the
> preprocessed code is:
> 
> if (0)
>    do {} while(0); 

It's not a double check. It's a compile time check for "do we even need
to build this code at all?", and a separate run time check for "do we
need to run this code now?".

Think about the case where CONFIG_ARCH_NETWINDER is set, but you aren't
actually running this kernel binary on a netwinder *today*.

Yes, we might not support multi-platform kernels on ARM yet, but we are
slowly getting there. This kind of change just makes that harder.

And even if the machine_is_netwinder() "function" is *currently* a macro
which is hard-coded to return one or zero, that just means that your
change achieves nothing in the compiler output. It'll be silently
optimised away, or not, as appropriate.

Patch

diff --git a/drivers/mtd/maps/dc21285.c b/drivers/mtd/maps/dc21285.c
index 86598a1..38fbf23 100644
--- a/drivers/mtd/maps/dc21285.c
+++ b/drivers/mtd/maps/dc21285.c
@@ -79,8 +79,7 @@  static void dc21285_copy_from(struct map_info *map, void *to, unsigned long from
 
 static void dc21285_write8(struct map_info *map, const map_word d, unsigned long adr)
 {
-	if (machine_is_netwinder())
-		nw_en_write();
+	nw_en_write();
 	*CSR_ROMWRITEREG = adr & 3;
 	adr &= ~3;
 	*(uint8_t*)(map->virt + adr) = d.x[0];
@@ -88,8 +87,7 @@  static void dc21285_write8(struct map_info *map, const map_word d, unsigned long
 
 static void dc21285_write16(struct map_info *map, const map_word d, unsigned long adr)
 {
-	if (machine_is_netwinder())
-		nw_en_write();
+	nw_en_write();
 	*CSR_ROMWRITEREG = adr & 3;
 	adr &= ~3;
 	*(uint16_t*)(map->virt + adr) = d.x[0];
@@ -97,8 +95,7 @@  static void dc21285_write16(struct map_info *map, const map_word d, unsigned lon
 
 static void dc21285_write32(struct map_info *map, const map_word d, unsigned long adr)
 {
-	if (machine_is_netwinder())
-		nw_en_write();
+	nw_en_write();
 	*(uint32_t*)(map->virt + adr) = d.x[0];
 }