All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk (csb337)
@ 2009-06-09 18:14 David Brownell
  2009-06-12 21:30 ` Jean-Christophe PLAGNIOL-VILLARD
  2009-06-18  5:14 ` Ben Warren
  0 siblings, 2 replies; 16+ messages in thread
From: David Brownell @ 2009-06-09 18:14 UTC (permalink / raw)
  To: u-boot

CSB337 boards originally shipped with MicroMonitor, not U-Boot;
and with a version using a different convention for recording
Ethernet addresses than anyone else.  To avoid breaking Linux
when it uses U-Boot, have it use the same convention on that
hardware.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
 cpu/arm920t/at91rm9200/ether.c |   22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

--- a/cpu/arm920t/at91rm9200/ether.c
+++ b/cpu/arm920t/at91rm9200/ether.c
@@ -24,6 +24,7 @@
 #include <at91rm9200_net.h>
 #include <net.h>
 #include <miiphy.h>
+#include <asm/mach-types.h>
 
 /* ----- Ethernet Buffer definitions ----- */
 
@@ -184,7 +185,7 @@ int eth_init (bd_t * bd)
 
 	p_mac->EMAC_CFG |= AT91C_EMAC_CSR;	/* Clear statistics */
 
-	/* Init Ehternet buffers */
+	/* Init Ethernet buffers */
 	for (i = 0; i < RBF_FRAMEMAX; i++) {
 		rbfdt[i].addr = (unsigned long)rbf_framebuf[i];
 		rbfdt[i].size = 0;
@@ -193,9 +194,22 @@ int eth_init (bd_t * bd)
 	rbfp = &rbfdt[0];
 
 	eth_getenv_enetaddr("ethaddr", enetaddr);
-	p_mac->EMAC_SA2L = (enetaddr[3] << 24) | (enetaddr[2] << 16)
-			 | (enetaddr[1] <<  8) | (enetaddr[0]);
-	p_mac->EMAC_SA2H = (enetaddr[5] <<  8) | (enetaddr[4]);
+
+	/* The CSB337 originally used a version of the MicroMonitor bootloader
+	 * which saved Ethernet addresses in the "wrong" order.  Operating
+	 * systems (like Linux) know this, and apply a workaround.  Replicate
+	 * that MicroMonitor behavior so we avoid needing to make such OS code
+	 * care about which bootloader was used.
+	 */
+	if (machine_is_csb337()) {
+		p_mac->EMAC_SA2H = (enetaddr[0] <<  8) | (enetaddr[1]);
+		p_mac->EMAC_SA2L = (enetaddr[2] << 24) | (enetaddr[3] << 16)
+				 | (enetaddr[4] <<  8) | (enetaddr[5]);
+	} else {
+		p_mac->EMAC_SA2L = (enetaddr[3] << 24) | (enetaddr[2] << 16)
+				 | (enetaddr[1] <<  8) | (enetaddr[0]);
+		p_mac->EMAC_SA2H = (enetaddr[5] <<  8) | (enetaddr[4]);
+	}
 
 	p_mac->EMAC_RBQP = (long) (&rbfdt[0]);
 	p_mac->EMAC_RSR &= ~(AT91C_EMAC_RSR_OVR | AT91C_EMAC_REC | AT91C_EMAC_BNA);

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

* [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk (csb337)
  2009-06-09 18:14 [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk (csb337) David Brownell
@ 2009-06-12 21:30 ` Jean-Christophe PLAGNIOL-VILLARD
  2009-06-12 22:02   ` David Brownell
  2009-06-12 23:02   ` [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk (csb337) Ben Warren
  2009-06-18  5:14 ` Ben Warren
  1 sibling, 2 replies; 16+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2009-06-12 21:30 UTC (permalink / raw)
  To: u-boot

On 11:14 Tue 09 Jun     , David Brownell wrote:
> CSB337 boards originally shipped with MicroMonitor, not U-Boot;
> and with a version using a different convention for recording
> Ethernet addresses than anyone else.  To avoid breaking Linux
> when it uses U-Boot, have it use the same convention on that
> hardware.
> 
> Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
> ---
>  cpu/arm920t/at91rm9200/ether.c |   22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
I'm not really a fan of this
but ok if I see the csn337 board patch

Ben what do you think?

> --- a/cpu/arm920t/at91rm9200/ether.c
> +++ b/cpu/arm920t/at91rm9200/ether.c
> @@ -24,6 +24,7 @@
>  #include <at91rm9200_net.h>
>  #include <net.h>
>  #include <miiphy.h>
> +#include <asm/mach-types.h>
>  
>  /* ----- Ethernet Buffer definitions ----- */
>  
> @@ -184,7 +185,7 @@ int eth_init (bd_t * bd)
>  
>  	p_mac->EMAC_CFG |= AT91C_EMAC_CSR;	/* Clear statistics */
>  
> -	/* Init Ehternet buffers */
> +	/* Init Ethernet buffers */
>  	for (i = 0; i < RBF_FRAMEMAX; i++) {
>  		rbfdt[i].addr = (unsigned long)rbf_framebuf[i];
>  		rbfdt[i].size = 0;
> @@ -193,9 +194,22 @@ int eth_init (bd_t * bd)
>  	rbfp = &rbfdt[0];
>  
>  	eth_getenv_enetaddr("ethaddr", enetaddr);
> -	p_mac->EMAC_SA2L = (enetaddr[3] << 24) | (enetaddr[2] << 16)
> -			 | (enetaddr[1] <<  8) | (enetaddr[0]);
> -	p_mac->EMAC_SA2H = (enetaddr[5] <<  8) | (enetaddr[4]);
> +
> +	/* The CSB337 originally used a version of the MicroMonitor bootloader
> +	 * which saved Ethernet addresses in the "wrong" order.  Operating
> +	 * systems (like Linux) know this, and apply a workaround.  Replicate
> +	 * that MicroMonitor behavior so we avoid needing to make such OS code
> +	 * care about which bootloader was used.
> +	 */
> +	if (machine_is_csb337()) {
please use ifdef to avoid other people size impact due this
> +		p_mac->EMAC_SA2H = (enetaddr[0] <<  8) | (enetaddr[1]);
> +		p_mac->EMAC_SA2L = (enetaddr[2] << 24) | (enetaddr[3] << 16)
> +				 | (enetaddr[4] <<  8) | (enetaddr[5]);
> +	} else {
> +		p_mac->EMAC_SA2L = (enetaddr[3] << 24) | (enetaddr[2] << 16)
> +				 | (enetaddr[1] <<  8) | (enetaddr[0]);
> +		p_mac->EMAC_SA2H = (enetaddr[5] <<  8) | (enetaddr[4]);
> +	}
>  
Best Regards,
J.

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

* [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk (csb337)
  2009-06-12 21:30 ` Jean-Christophe PLAGNIOL-VILLARD
@ 2009-06-12 22:02   ` David Brownell
  2009-06-13 10:26     ` Jean-Christophe PLAGNIOL-VILLARD
  2009-06-12 23:02   ` [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk (csb337) Ben Warren
  1 sibling, 1 reply; 16+ messages in thread
From: David Brownell @ 2009-06-12 22:02 UTC (permalink / raw)
  To: u-boot

On Friday 12 June 2009, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 11:14 Tue 09 Jun     , David Brownell wrote:

> I'm not really a fan of this
> but ok if I see the csn337 board patch

I meant to send that in before, thanks for the reminder.
The email I just sent has that.

If you don't like it, what would you propose instead?
Remember that *EVERY* Linux kernel (for example) is
expecting these boards to work this way.  I don't
think it's realistic, or desirable, to change Linux
in this way.  It's not supposed to matter what boot
loader gets used.


> > +	/* The CSB337 originally used a version of the MicroMonitor bootloader
> > +	 * which saved Ethernet addresses in the "wrong" order.  Operating
> > +	 * systems (like Linux) know this, and apply a workaround.  Replicate
> > +	 * that MicroMonitor behavior so we avoid needing to make such OS code
> > +	 * care about which bootloader was used.
> > +	 */
> > +	if (machine_is_csb337()) {
>
> please use ifdef to avoid other people size impact due this

The machine_is_X() macros are automatically #ifdeffed in
the header; no size impact.  Read <asm/mach-types.h> ...


> > +		p_mac->EMAC_SA2H = (enetaddr[0] <<  8) | (enetaddr[1]);
> > +		p_mac->EMAC_SA2L = (enetaddr[2] << 24) | (enetaddr[3] << 16)
> > +				 | (enetaddr[4] <<  8) | (enetaddr[5]);
> > +	} else {
> > +		p_mac->EMAC_SA2L = (enetaddr[3] << 24) | (enetaddr[2] << 16)
> > +				 | (enetaddr[1] <<  8) | (enetaddr[0]);
> > +		p_mac->EMAC_SA2H = (enetaddr[5] <<  8) | (enetaddr[4]);
> > +	}
> >  
> Best Regards,
> J.
> 
> 

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

* [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk (csb337)
  2009-06-12 21:30 ` Jean-Christophe PLAGNIOL-VILLARD
  2009-06-12 22:02   ` David Brownell
@ 2009-06-12 23:02   ` Ben Warren
  1 sibling, 0 replies; 16+ messages in thread
From: Ben Warren @ 2009-06-12 23:02 UTC (permalink / raw)
  To: u-boot

Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 11:14 Tue 09 Jun     , David Brownell wrote:
>   
>> CSB337 boards originally shipped with MicroMonitor, not U-Boot;
>> and with a version using a different convention for recording
>> Ethernet addresses than anyone else.  To avoid breaking Linux
>> when it uses U-Boot, have it use the same convention on that
>> hardware.
>>
>> Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
>> ---
>>  cpu/arm920t/at91rm9200/ether.c |   22 ++++++++++++++++++----
>>  1 file changed, 18 insertions(+), 4 deletions(-)
>>
>>     
> I'm not really a fan of this
> but ok if I see the csn337 board patch
>
> Ben what do you think?
>
>   
Not a big fan either, but if we include this board I guess there's not 
much of an alternative.

regards,
Ben

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

* [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk (csb337)
  2009-06-12 22:02   ` David Brownell
@ 2009-06-13 10:26     ` Jean-Christophe PLAGNIOL-VILLARD
  2009-06-13 11:13       ` David Brownell
  0 siblings, 1 reply; 16+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2009-06-13 10:26 UTC (permalink / raw)
  To: u-boot

On 15:02 Fri 12 Jun     , David Brownell wrote:
> On Friday 12 June 2009, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 11:14 Tue 09 Jun     , David Brownell wrote:
> 
> > I'm not really a fan of this
> > but ok if I see the csn337 board patch
> 
> I meant to send that in before, thanks for the reminder.
> The email I just sent has that.
> 
> If you don't like it, what would you propose instead?
> Remember that *EVERY* Linux kernel (for example) is
> expecting these boards to work this way.  I don't
> think it's realistic, or desirable, to change Linux
> in this way.  It's not supposed to matter what boot
> loader gets used.
Actually I've no better idea and as Ben If we have the csb337 mainline
we will accept it
> 
> 
> > > +	/* The CSB337 originally used a version of the MicroMonitor bootloader
> > > +	 * which saved Ethernet addresses in the "wrong" order.  Operating
> > > +	 * systems (like Linux) know this, and apply a workaround.  Replicate
> > > +	 * that MicroMonitor behavior so we avoid needing to make such OS code
> > > +	 * care about which bootloader was used.
> > > +	 */
> > > +	if (machine_is_csb337()) {
> >
> > please use ifdef to avoid other people size impact due this
> 
> The machine_is_X() macros are automatically #ifdeffed in
> the header; no size impact.  Read <asm/mach-types.h> ...
If I use this pacth on the rm9200ek the u-boot.bin size will increase
for nothing
If I was able to detect dynamicly on which board I will run ok
but it's not the case here
so please use
#ifdef CONFIG_MACH_xxx
#else
#endif

Best Regards,
J.

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

* [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk (csb337)
  2009-06-13 10:26     ` Jean-Christophe PLAGNIOL-VILLARD
@ 2009-06-13 11:13       ` David Brownell
  2009-06-13 12:28         ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 1 reply; 16+ messages in thread
From: David Brownell @ 2009-06-13 11:13 UTC (permalink / raw)
  To: u-boot

On Saturday 13 June 2009, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > 
> > The machine_is_X() macros are automatically #ifdeffed in
> > the header; no size impact. ?Read <asm/mach-types.h> ...
>
> If I use this pacth on the rm9200ek the u-boot.bin size will increase
> for nothing

I'm not following you.  The lines are:


#ifdef CONFIG_MACH_CSB337
# ifdef machine_arch_type
#  undef machine_arch_type
#  define machine_arch_type     __machine_arch_type
# else
#  define machine_arch_type     MACH_TYPE_CSB337
# endif
# define machine_is_csb337()    (machine_arch_type == MACH_TYPE_CSB337)
#else
# define machine_is_csb337()    (0)
#endif

... and similar for EK.  The csb337 config file sets CONFIG_MACH_CSB337,
and nothing else does.  Result:  on rm9200ek, that test becomes if(0),
while on csb337 it becomes if (X == X), where X == MACH_TYPE_CSB337.


> If I was able to detect dynamicly on which board I will run ok
> but it's not the case here
> so please use
> #ifdef CONFIG_MACH_xxx
> #else
> #endif

See above.  There already *IS* such an #ifdef, but it's just not
cluttering up the guts of that driver.

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

* [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk (csb337)
  2009-06-13 11:13       ` David Brownell
@ 2009-06-13 12:28         ` Jean-Christophe PLAGNIOL-VILLARD
  2009-06-13 14:50           ` Mike Frysinger
  0 siblings, 1 reply; 16+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2009-06-13 12:28 UTC (permalink / raw)
  To: u-boot

On 04:13 Sat 13 Jun     , David Brownell wrote:
> On Saturday 13 June 2009, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > 
> > > The machine_is_X() macros are automatically #ifdeffed in
> > > the header; no size impact. ?Read <asm/mach-types.h> ...
> >
> > If I use this pacth on the rm9200ek the u-boot.bin size will increase
> > for nothing
> 
> I'm not following you.  The lines are:
> 
> 
> #ifdef CONFIG_MACH_CSB337
> # ifdef machine_arch_type
> #  undef machine_arch_type
> #  define machine_arch_type     __machine_arch_type
> # else
> #  define machine_arch_type     MACH_TYPE_CSB337
> # endif
> # define machine_is_csb337()    (machine_arch_type == MACH_TYPE_CSB337)
> #else
> # define machine_is_csb337()    (0)
> #endif
> 
> ... and similar for EK.  The csb337 config file sets CONFIG_MACH_CSB337,
> and nothing else does.  Result:  on rm9200ek, that test becomes if(0),
> while on csb337 it becomes if (X == X), where X == MACH_TYPE_CSB337.
> 
> 
> > If I was able to detect dynamicly on which board I will run ok
> > but it's not the case here
> > so please use
> > #ifdef CONFIG_MACH_xxx
> > #else
> > #endif
> 
> See above.  There already *IS* such an #ifdef, but it's just not
> cluttering up the guts of that driver.
you adding MUST have NO size impact on other board

if we apply you code as it we will increase the size of u-boot for every
rm9200 board that use the ethernet

so please do the ifdef on the ether driver

Best Regards,
J.

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

* [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk (csb337)
  2009-06-13 12:28         ` Jean-Christophe PLAGNIOL-VILLARD
@ 2009-06-13 14:50           ` Mike Frysinger
  2009-06-13 14:54             ` [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk?(csb337) Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 1 reply; 16+ messages in thread
From: Mike Frysinger @ 2009-06-13 14:50 UTC (permalink / raw)
  To: u-boot

On Saturday 13 June 2009 08:28:23 Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 04:13 Sat 13 Jun     , David Brownell wrote:
> > On Saturday 13 June 2009, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > > The machine_is_X() macros are automatically #ifdeffed in
> > > > the header; no size impact.  Read <asm/mach-types.h> ...
> > >
> > > If I use this pacth on the rm9200ek the u-boot.bin size will increase
> > > for nothing
> >
> > I'm not following you.  The lines are:
> >
> >
> > #ifdef CONFIG_MACH_CSB337
> > # ifdef machine_arch_type
> > #  undef machine_arch_type
> > #  define machine_arch_type     __machine_arch_type
> > # else
> > #  define machine_arch_type     MACH_TYPE_CSB337
> > # endif
> > # define machine_is_csb337()    (machine_arch_type == MACH_TYPE_CSB337)
> > #else
> > # define machine_is_csb337()    (0)
> > #endif
> >
> > ... and similar for EK.  The csb337 config file sets CONFIG_MACH_CSB337,
> > and nothing else does.  Result:  on rm9200ek, that test becomes if(0),
> > while on csb337 it becomes if (X == X), where X == MACH_TYPE_CSB337.
> >
> > > If I was able to detect dynamicly on which board I will run ok
> > > but it's not the case here
> > > so please use
> > > #ifdef CONFIG_MACH_xxx
> > > #else
> > > #endif
> >
> > See above.  There already *IS* such an #ifdef, but it's just not
> > cluttering up the guts of that driver.
>
> you adding MUST have NO size impact on other board
>
> if we apply you code as it we will increase the size of u-boot for every
> rm9200 board that use the ethernet

i dont get it.  the CPP expands into if(0) and unless you have a completely 
shitty compiler, gcc will do dead code elimination on it resulting in the same 
binary size.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090613/da9320c6/attachment-0001.pgp 

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

* [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk?(csb337)
  2009-06-13 14:50           ` Mike Frysinger
@ 2009-06-13 14:54             ` Jean-Christophe PLAGNIOL-VILLARD
  2009-06-13 15:13               ` Mike Frysinger
  2009-07-10 20:39               ` [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk?(csb337) Wolfgang Denk
  0 siblings, 2 replies; 16+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2009-06-13 14:54 UTC (permalink / raw)
  To: u-boot

On 10:50 Sat 13 Jun     , Mike Frysinger wrote:
> On Saturday 13 June 2009 08:28:23 Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 04:13 Sat 13 Jun     , David Brownell wrote:
> > > On Saturday 13 June 2009, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > > > The machine_is_X() macros are automatically #ifdeffed in
> > > > > the header; no size impact.  Read <asm/mach-types.h> ...
> > > >
> > > > If I use this pacth on the rm9200ek the u-boot.bin size will increase
> > > > for nothing
> > >
> > > I'm not following you.  The lines are:
> > >
> > >
> > > #ifdef CONFIG_MACH_CSB337
> > > # ifdef machine_arch_type
> > > #  undef machine_arch_type
> > > #  define machine_arch_type     __machine_arch_type
> > > # else
> > > #  define machine_arch_type     MACH_TYPE_CSB337
> > > # endif
> > > # define machine_is_csb337()    (machine_arch_type == MACH_TYPE_CSB337)
> > > #else
> > > # define machine_is_csb337()    (0)
> > > #endif
> > >
> > > ... and similar for EK.  The csb337 config file sets CONFIG_MACH_CSB337,
> > > and nothing else does.  Result:  on rm9200ek, that test becomes if(0),
> > > while on csb337 it becomes if (X == X), where X == MACH_TYPE_CSB337.
> > >
> > > > If I was able to detect dynamicly on which board I will run ok
> > > > but it's not the case here
> > > > so please use
> > > > #ifdef CONFIG_MACH_xxx
> > > > #else
> > > > #endif
> > >
> > > See above.  There already *IS* such an #ifdef, but it's just not
> > > cluttering up the guts of that driver.
> >
> > you adding MUST have NO size impact on other board
> >
> > if we apply you code as it we will increase the size of u-boot for every
> > rm9200 board that use the ethernet
> 
> i dont get it.  the CPP expands into if(0) and unless you have a completely 
> shitty compiler, gcc will do dead code elimination on it resulting in the same 
> binary size.
I've seen to much "shitty version" that does not do it correctly
so I prefer ot avoid the problem

Best Regards,
J.

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

* [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk?(csb337)
  2009-06-13 14:54             ` [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk?(csb337) Jean-Christophe PLAGNIOL-VILLARD
@ 2009-06-13 15:13               ` Mike Frysinger
  2009-06-13 17:19                 ` [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk (csb337) David Brownell
  2009-07-10 20:39               ` [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk?(csb337) Wolfgang Denk
  1 sibling, 1 reply; 16+ messages in thread
From: Mike Frysinger @ 2009-06-13 15:13 UTC (permalink / raw)
  To: u-boot

On Saturday 13 June 2009 10:54:39 Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 10:50 Sat 13 Jun     , Mike Frysinger wrote:
> > On Saturday 13 June 2009 08:28:23 Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > On 04:13 Sat 13 Jun     , David Brownell wrote:
> > > > On Saturday 13 June 2009, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > > > > The machine_is_X() macros are automatically #ifdeffed in
> > > > > > the header; no size impact.  Read <asm/mach-types.h> ...
> > > > >
> > > > > If I use this pacth on the rm9200ek the u-boot.bin size will
> > > > > increase for nothing
> > > >
> > > > I'm not following you.  The lines are:
> > > >
> > > >
> > > > #ifdef CONFIG_MACH_CSB337
> > > > # ifdef machine_arch_type
> > > > #  undef machine_arch_type
> > > > #  define machine_arch_type     __machine_arch_type
> > > > # else
> > > > #  define machine_arch_type     MACH_TYPE_CSB337
> > > > # endif
> > > > # define machine_is_csb337()    (machine_arch_type ==
> > > > MACH_TYPE_CSB337) #else
> > > > # define machine_is_csb337()    (0)
> > > > #endif
> > > >
> > > > ... and similar for EK.  The csb337 config file sets
> > > > CONFIG_MACH_CSB337, and nothing else does.  Result:  on rm9200ek,
> > > > that test becomes if(0), while on csb337 it becomes if (X == X),
> > > > where X == MACH_TYPE_CSB337.
> > > >
> > > > > If I was able to detect dynamicly on which board I will run ok
> > > > > but it's not the case here
> > > > > so please use
> > > > > #ifdef CONFIG_MACH_xxx
> > > > > #else
> > > > > #endif
> > > >
> > > > See above.  There already *IS* such an #ifdef, but it's just not
> > > > cluttering up the guts of that driver.
> > >
> > > you adding MUST have NO size impact on other board
> > >
> > > if we apply you code as it we will increase the size of u-boot for
> > > every rm9200 board that use the ethernet
> >
> > i dont get it.  the CPP expands into if(0) and unless you have a
> > completely shitty compiler, gcc will do dead code elimination on it
> > resulting in the same binary size.
>
> I've seen to much "shitty version" that does not do it correctly
> so I prefer ot avoid the problem

i'm pretty sure we're already relying on this behavior.  so you've got a 
bigger problem that needs addressing (i.e. shit can that compiler) than what 
David is proposing.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090613/79242da8/attachment.pgp 

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

* [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk (csb337)
  2009-06-13 15:13               ` Mike Frysinger
@ 2009-06-13 17:19                 ` David Brownell
  0 siblings, 0 replies; 16+ messages in thread
From: David Brownell @ 2009-06-13 17:19 UTC (permalink / raw)
  To: u-boot

On Saturday 13 June 2009, Mike Frysinger wrote:
> > > > > See above. ?There already *IS* such an #ifdef, but it's just not
> > > > > cluttering up the guts of that driver.
> > > >
> > > > you adding MUST have NO size impact on other board
> > > >
> > > > if we apply you code as it we will increase the size of u-boot for
> > > > every rm9200 board that use the ethernet

Not on any compiler I've used for at least the past dozen years or so.


> > > i dont get it. ?the CPP expands into if(0) and unless you have a
> > > completely shitty compiler, gcc will do dead code elimination on it
> > > resulting in the same binary size.
> >
> > I've seen to much "shitty version" that does not do it correctly
> > so I prefer ot avoid the problem

Which compiler versions are that broken?  And are used regularly
to build rm9200 U-Boot code?  At normal optimization levels?
(We know that disabling optimization puts out all kinds of crap.)

This is really basic stuff:  "if (0) { dead } else { live }" and
the converse.  


> i'm pretty sure we're already relying on this behavior. ?so you've got a 
> bigger problem that needs addressing (i.e. shit can that compiler) than what 
> David is proposing.

I know that when Linux started relying on that type of dead-code
elimination quite a few years back, nobody thought this was a
real issue.  And Linux is, on the whole, far more demanding of
compilers than U-Boot.

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

* [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk (csb337)
  2009-06-09 18:14 [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk (csb337) David Brownell
  2009-06-12 21:30 ` Jean-Christophe PLAGNIOL-VILLARD
@ 2009-06-18  5:14 ` Ben Warren
  2009-06-19  6:59   ` David Brownell
  1 sibling, 1 reply; 16+ messages in thread
From: Ben Warren @ 2009-06-18  5:14 UTC (permalink / raw)
  To: u-boot

David,

David Brownell wrote:
> CSB337 boards originally shipped with MicroMonitor, not U-Boot;
> and with a version using a different convention for recording
> Ethernet addresses than anyone else.  To avoid breaking Linux
> when it uses U-Boot, have it use the same convention on that
> hardware.
>
> Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
> ---
Applied to net repo.

thanks,
Ben

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

* [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk (csb337)
  2009-06-18  5:14 ` Ben Warren
@ 2009-06-19  6:59   ` David Brownell
  2009-06-19  7:01     ` Ben Warren
  0 siblings, 1 reply; 16+ messages in thread
From: David Brownell @ 2009-06-19  6:59 UTC (permalink / raw)
  To: u-boot

On Wednesday 17 June 2009, Ben Warren wrote:
> Applied to net repo.

Thanks.  Random question:  is that driver going to move
into drivers/net or will it stay in cpu/arm920t?  I thought
I saw you moving things out of cpu/* a while back.

- Dave

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

* [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk (csb337)
  2009-06-19  6:59   ` David Brownell
@ 2009-06-19  7:01     ` Ben Warren
  2009-06-19  7:56       ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 1 reply; 16+ messages in thread
From: Ben Warren @ 2009-06-19  7:01 UTC (permalink / raw)
  To: u-boot

David Brownell wrote:
> On Wednesday 17 June 2009, Ben Warren wrote:
>   
>> Applied to net repo.
>>     
>
> Thanks.  Random question:  is that driver going to move
> into drivers/net or will it stay in cpu/arm920t?  I thought
> I saw you moving things out of cpu/* a while back.
>
> - Dave
>
>   
Good question.  I'd like to move it to drivers/net, as well as modify it 
to use the newer API.  Time willing... :)

regards,
Ben

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

* [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk (csb337)
  2009-06-19  7:01     ` Ben Warren
@ 2009-06-19  7:56       ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 0 replies; 16+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2009-06-19  7:56 UTC (permalink / raw)
  To: u-boot

On 00:01 Fri 19 Jun     , Ben Warren wrote:
> David Brownell wrote:
> > On Wednesday 17 June 2009, Ben Warren wrote:
> >   
> >> Applied to net repo.
> >>     
> >
> > Thanks.  Random question:  is that driver going to move
> > into drivers/net or will it stay in cpu/arm920t?  I thought
> > I saw you moving things out of cpu/* a while back.
> >
> > - Dave
> >
> >   
> Good question.  I'd like to move it to drivers/net, as well as modify it 
> to use the newer API.  Time willing... :)
I'll prefer you do not move it
as it's plan to drop it soon and swithc the rm9200 to the at91 api and now
stop to double implement the driver as serial, spi as example

Best Regards,
J.

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

* [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk?(csb337)
  2009-06-13 14:54             ` [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk?(csb337) Jean-Christophe PLAGNIOL-VILLARD
  2009-06-13 15:13               ` Mike Frysinger
@ 2009-07-10 20:39               ` Wolfgang Denk
  1 sibling, 0 replies; 16+ messages in thread
From: Wolfgang Denk @ 2009-07-10 20:39 UTC (permalink / raw)
  To: u-boot

Dear Jean-Christophe PLAGNIOL-VILLARD,

In message <20090613145439.GG25406@game.jcrosoft.org> you wrote:
>
> > i dont get it.  the CPP expands into if(0) and unless you have a completely 
> > shitty compiler, gcc will do dead code elimination on it resulting in the same 
> > binary size.
> I've seen to much "shitty version" that does not do it correctly
> so I prefer ot avoid the problem

There are zillions of places that rely on the compiler optimizing away
"if (0) ..." stuff. It is a valid and even recommended C programming
technique.

This is definitely no reason to reject a patch.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Space is big. You just won't believe how vastly, hugely, mind-
bogglingly big it is. I mean, you may think it's a long way down the
road to the drug store, but that's just peanuts to space.
                              -- The Hitchhiker's Guide to the Galaxy

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

end of thread, other threads:[~2009-07-10 20:39 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-06-09 18:14 [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk (csb337) David Brownell
2009-06-12 21:30 ` Jean-Christophe PLAGNIOL-VILLARD
2009-06-12 22:02   ` David Brownell
2009-06-13 10:26     ` Jean-Christophe PLAGNIOL-VILLARD
2009-06-13 11:13       ` David Brownell
2009-06-13 12:28         ` Jean-Christophe PLAGNIOL-VILLARD
2009-06-13 14:50           ` Mike Frysinger
2009-06-13 14:54             ` [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk?(csb337) Jean-Christophe PLAGNIOL-VILLARD
2009-06-13 15:13               ` Mike Frysinger
2009-06-13 17:19                 ` [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk (csb337) David Brownell
2009-07-10 20:39               ` [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk?(csb337) Wolfgang Denk
2009-06-12 23:02   ` [U-Boot] [patch] rm9200 ethernet driver: board-specific quirk (csb337) Ben Warren
2009-06-18  5:14 ` Ben Warren
2009-06-19  6:59   ` David Brownell
2009-06-19  7:01     ` Ben Warren
2009-06-19  7:56       ` Jean-Christophe PLAGNIOL-VILLARD

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.