Skip to contentSkip to conversation

Fix 8313ERDB board configuration

PATCH New York Sun 4 participants 13 messages May 15, 2008 – May 19, 2008

lore
Share & tools
lore

York Sun

author
#0 lore raw

PATCH 1/1 Fix 8313ERDB board configuration

Change LCRR clock ratio from 2 to 4 to commodate VSC7385. Correct TSEC1 vs TSEC2 assignment.
Define ETHADDR and ETH1ADDR always.

---
 include/configs/MPC8313ERDB.h |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)
include/configs/MPC8313ERDB.h +6 -8
index 6eec240..f9fa535 100644
@@ -42,9 +42,12 @@
 
 /*
  * On-board devices
+ *
+ * TSEC1 is VSC switch
+ * TSEC2 is SoC TSEC
  */
 #define CONFIG_VSC7385_ENET
-
+#define CONFIG_TSEC2
 
 #ifdef CFG_66MHZ
 #define CONFIG_83XX_CLKIN	66666667	/* in Hz */
@@ -80,7 +83,7 @@
 
 #ifdef CONFIG_VSC7385_ENET
 
-#define CONFIG_TSEC2
+#define CONFIG_TSEC1
 
 /* The flash address and size of the VSC7385 firmware image */
 #define CONFIG_VSC7385_IMAGE		0xFE7FE000
@@ -209,7 +212,7 @@
 /*
  * Local Bus LCRR and LBCR regs
  */
-#define CFG_LCRR	LCRR_EADC_1 | LCRR_CLKDIV_2	/* 0x00010002 */
+#define CFG_LCRR	LCRR_EADC_1 | LCRR_CLKDIV_4
 #define CFG_LBC_LBCR	( 0x00040000 /* TODO */ \
 			| (0xFF << LBCR_BMT_SHIFT) \
 			| 0xF )	/* 0x0004ff0f */
@@ -523,13 +526,8 @@
  */
 #define CONFIG_ENV_OVERWRITE
 
-#ifdef CONFIG_HAS_ETH0
 #define CONFIG_ETHADDR		00:E0:0C:00:95:01
-#endif
-
-#ifdef CONFIG_HAS_ETH1
 #define CONFIG_ETH1ADDR		00:E0:0C:00:95:02
-#endif
 
 #define CONFIG_IPADDR		10.0.0.2
 #define CONFIG_SERVERIP		10.0.0.1
Signature · 1 line
1.5.2.2
#1 lore raw

PATCH 1/1 Fix 8313ERDB board configuration

Hi York,

Quoted 48 lines · York Sun

Change LCRR clock ratio from 2 to 4 to commodate VSC7385. Correct TSEC1 vs TSEC2 assignment.
Define ETHADDR and ETH1ADDR always.

---
 include/configs/MPC8313ERDB.h |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)
include/configs/MPC8313ERDB.h +6 -3
index 6eec240..f9fa535 100644
@@ -42,9 +42,12 @@
 
 /*
  * On-board devices
+ *
+ * TSEC1 is VSC switch
+ * TSEC2 is SoC TSEC
  */
 #define CONFIG_VSC7385_ENET
-
+#define CONFIG_TSEC2
 
 #ifdef CFG_66MHZ
 #define CONFIG_83XX_CLKIN	66666667	/* in Hz */
@@ -80,7 +83,7 @@
 
 #ifdef CONFIG_VSC7385_ENET
 
-#define CONFIG_TSEC2
+#define CONFIG_TSEC1
 
 /* The flash address and size of the VSC7385 firmware image */
 #define CONFIG_VSC7385_IMAGE		0xFE7FE000
@@ -209,7 +212,7 @@
 /*
  * Local Bus LCRR and LBCR regs
  */
-#define CFG_LCRR	LCRR_EADC_1 | LCRR_CLKDIV_2	/* 0x00010002 */
+#define CFG_LCRR	LCRR_EADC_1 | LCRR_CLKDIV_4
 #define CFG_LBC_LBCR	( 0x00040000 /* TODO */ \
 			| (0xFF << LBCR_BMT_SHIFT) \
 			| 0xF )	/* 0x0004ff0f */
@@ -523,13 +526,8 @@
  */

include/configs/MPC8313ERDB.h line 531

Quoted 10 lines
include/configs/MPC8313ERDB.h +0 -5
 #define CONFIG_ENV_OVERWRITE
 
-#ifdef CONFIG_HAS_ETH0
 #define CONFIG_ETHADDR		00:E0:0C:00:95:01
-#endif
-
-#ifdef CONFIG_HAS_ETH1
 #define CONFIG_ETH1ADDR		00:E0:0C:00:95:02
-#endif
 

Please also remove the default MAC address assignments.

include/configs/MPC8313ERDB.h line 533

Quoted 4 lines
include/configs/MPC8313ERDB.h +0 -0
 #define CONFIG_IPADDR		10.0.0.2
 #define CONFIG_SERVERIP		10.0.0.1

While you're at it, putting default IP addresses is also pointless.

regards,
Ben

author
#2 lore raw

PATCH 1/1 Fix 8313ERDB board configuration

Ben,

Quoted 9 lines · Ben Warren

include/configs/MPC8313ERDB.h line 532

Quoted 8 lines
include/configs/MPC8313ERDB.h +0 -5
-#ifdef CONFIG_HAS_ETH0
 #define CONFIG_ETHADDR		00:E0:0C:00:95:01
-#endif
-
-#ifdef CONFIG_HAS_ETH1
 #define CONFIG_ETH1ADDR		00:E0:0C:00:95:02
-#endif

Please also remove the default MAC address assignments.

I think we need them, don't we?

Quoted 5 lines
Quoted 4 lines

#define CONFIG_IPADDR 10.0.0.2
#define CONFIG_SERVERIP 10.0.0.1

While you're at it, putting default IP addresses is also pointless.

At least it tells you that you can put your address here. It doesn't hurt.
And BTW, I want to make my patch as small as possible. I'll leave the rest to the board maintainer.

York

#3 lore raw

PATCH 1/1 Fix 8313ERDB board configuration

Quoted 16 lines · York Sun

Ben,

Quoted 11 lines · Ben Warren

include/configs/MPC8313ERDB.h line 532

Quoted 9 lines
include/configs/MPC8313ERDB.h +0 -5
-#ifdef CONFIG_HAS_ETH0
 #define CONFIG_ETHADDR		00:E0:0C:00:95:01
-#endif
-
-#ifdef CONFIG_HAS_ETH1
 #define CONFIG_ETH1ADDR		00:E0:0C:00:95:02
-#endif

Please also remove the default MAC address assignments.

Quoted 3 lines

I think we need them, don't we?

No, there have been many discussions about this, and the consensus is that there shouldn't be default addresses assigned. One of many reasons is that the numbers you've chosen actually belong to some kind of corporate entity (I would guess Freescale) and not the end user. It's better for the end user to either use private addresses or ones that they have purchased from IEEE.

Quoted 8 lines
Quoted 7 lines
Quoted 5 lines

#define CONFIG_IPADDR 10.0.0.2
#define CONFIG_SERVERIP 10.0.0.1

While you're at it, putting default IP addresses is also pointless.

Quoted 9 lines

At least it tells you that you can put your address here. It doesn't hurt.
And BTW, I want to make my patch as small as possible. I'll leave the rest to the board maintainer.

York

I'm less picky about this one because as you say it doesn't really hurt. All it will do is initialize the network to an unreachable state on the vast majority of purchasers of your eval boards. Let's leave it up to the board maintainer.

regards,
Ben

#4 lore raw

PATCH 1/1 Fix 8313ERDB board configuration

In reply to York Sun
Quoted 4 lines · In message <1210886469.880.45.camel@laptop> you wrote:
Quoted 2 lines

Please also remove the default MAC address assignments.

I think we need them, don't we?

No, or you can install this U-Boot image exactly on one specific, single board.

Quoted 7 lines
Quoted 4 lines
Quoted 3 lines

#define CONFIG_IPADDR 10.0.0.2
#define CONFIG_SERVERIP 10.0.0.1

While you're at it, putting default IP addresses is also pointless.

At least it tells you that you can put your address here. It doesn't hurt.

Yes, it does hurt. It's a PITA. Please get rid of such "standard" network settings that are almost always and everywhere wrong.

Best regards,

Wolfgang Denk

Signature · 5 lines
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
Those who do not  understand  Unix  are  condemned  to  reinvent  it,
poorly.              - Henry Spencer, University of Toronto Unix hack
#5 lore raw

PATCH 1/1 Fix 8313ERDB board configuration

Quoted 6 lines · Wolfgang Denk
Quoted 2 lines · In message <1210886469.880.45.camel@laptop> you wrote:
Quoted 1 line

Please also remove the default MAC address assignments.

I think we need them, don't we?

No, or you can install this U-Boot image exactly on one specific, single board.

I understand your argument, but it's rather disingenuous when you allow this code to exist:

#ifdef	CONFIG_ETHADDR
	"ethaddr="	MK_STR(CONFIG_ETHADDR)		"\0"
#endif
#ifdef	CONFIG_ETH1ADDR
	"eth1addr="	MK_STR(CONFIG_ETH1ADDR)		"\0"
#endif
#ifdef	CONFIG_ETH2ADDR
	"eth2addr="	MK_STR(CONFIG_ETH2ADDR)		"\0"
#endif
#ifdef	CONFIG_ETH3ADDR
	"eth3addr="	MK_STR(CONFIG_ETH3ADDR)		"\0"
#endif
#ifdef	CONFIG_IPADDR
	"ipaddr="	MK_STR(CONFIG_IPADDR)		"\0"
#endif
#ifdef	CONFIG_SERVERIP
	"serverip="	MK_STR(CONFIG_SERVERIP)		"\0"

#endif

If you get rid of this block, you'll solve the problem for all boards, instead of nagging people who post patches for unrelated bugs.

Signature · 2 lines
Timur Tabi
Linux kernel developer at Freescale
#6 lore raw

PATCH 1/1 Fix 8313ERDB board configuration

Quoted 3 lines · In message <[email protected]> you wrote:

I understand your argument, but it's rather disingenuous when you allow this code to exist:

"UNIX was not designed to stop you from doing stupid things, because
that would also stop you from doing clever things." - Doug Gwyn

Quoted 2 lines

If you get rid of this block, you'll solve the problem for all boards, instead of nagging people who post patches for unrelated bugs.

There are a few cases where exactly this is needed. The fact that some feature exists does not mean that you should use it without careful consideration of the effects and side-effects.

Best regards,

Wolfgang Denk

Signature · 4 lines
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
It's a small world, but I wouldn't want to paint it.
#7 lore raw

PATCH 1/1 Fix 8313ERDB board configuration

Quoted 3 lines · Wolfgang Denk

There are a few cases where exactly this is needed. The fact that some feature exists does not mean that you should use it without careful consideration of the effects and side-effects.

Fair enough, but the changes that are in York's patch have been tested. Considering that the fix window for 1.3.3 is closing fast, we don't have the bandwidth for additional testing of the additional and unrelated changes that are being requested.

Signature · 2 lines
Timur Tabi
Linux kernel developer at Freescale
#8 lore raw

PATCH 1/1 Fix 8313ERDB board configuration

Quoted 5 lines · In message <[email protected]> you wrote:

Fair enough, but the changes that are in York's patch have been tested. Considering that the fix window for 1.3.3 is closing fast, we don't have the bandwidth for additional testing of the additional and unrelated changes that are being requested.

What exactly is the risk?

Best regards,

Wolfgang Denk

Signature · 4 lines
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
Use C++ to confuse your enemies; use C to produce stable code.
#9 lore raw

PATCH 1/1 Fix 8313ERDB board configuration

Quoted 7 lines · Wolfgang Denk
Quoted 4 lines · In message <[email protected]> you wrote:

Fair enough, but the changes that are in York's patch have been tested. Considering that the fix window for 1.3.3 is closing fast, we don't have the bandwidth for additional testing of the additional and unrelated changes that are being requested.

What exactly is the risk?

Probably not much, but that isn't my point. I find it really annoying when someone posts a patch to fix a specific bug, and the reply is "why don't you fix this other bug too, while you're at it?"

Right now, the 8313ERDB will generate a machine check if you boot it with the top-of-tree kernel. This patch fixes that. For 1.3.4, if you like, we can submit a patch that removes all MAC and IP address macros from all 83xx boards.

Signature · 2 lines
Timur Tabi
Linux kernel developer at Freescale
#10 lore raw

PATCH 1/1 Fix 8313ERDB board configuration

Quoted 12 lines · Timur Tabi
Quoted 9 lines · Wolfgang Denk
Quoted 5 lines · In message <[email protected]> you wrote:

Fair enough, but the changes that are in York's patch have been tested. Considering that the fix window for 1.3.3 is closing fast, we don't have the bandwidth for additional testing of the additional and unrelated changes that are being requested.

What exactly is the risk?

Quoted 9 lines

Probably not much, but that isn't my point. I find it really annoying when someone posts a patch to fix a specific bug, and the reply is "why don't you fix this other bug too, while you're at it?"

Right now, the 8313ERDB will generate a machine check if you boot it with the top-of-tree kernel. This patch fixes that. For 1.3.4, if you like, we can submit a patch that removes all MAC and IP address macros from all 83xx boards.

That seems reasonable. Wolfgang, if you agree, please pull York's patch directly.

regards,
Ben

#11 lore raw

PATCH 1/1 Fix 8313ERDB board configuration

Quoted 5 lines · In message <[email protected]> you wrote:
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

Fair enough, but the changes that are in York's patch have been tested. Considering that the fix window for 1.3.3 is closing fast, we don't have the bandwidth for additional testing of the additional and unrelated changes that are being requested.

...

Quoted 2 lines

That seems reasonable. Wolfgang, if you agree, please pull York's patch directly.

Sorry, I missed this one.

Best regards,

Wolfgang Denk

Signature · 4 lines
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
f u cn rd ths, itn tyg h myxbl cd.
#12 lore raw

PATCH 1/1 Fix 8313ERDB board configuration

In reply to York Sun
Quoted 9 lines · In message <[email protected]> you wrote:

Change LCRR clock ratio from 2 to 4 to commodate VSC7385. Correct TSEC1 vs TSEC2 assignment.
Define ETHADDR and ETH1ADDR always.

---
 include/configs/MPC8313ERDB.h |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)

Applied.

Please keep the review comments in mind, and send a follow-up patch ASAP.

Best regards,

Wolfgang Denk

Signature · 8 lines
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
The universe contains any amount of horrible ways  to  be  woken  up,
such as the noise of the mob breaking down the front door, the scream
of fire engines, or the realization that today is the Monday which on
Friday night was a comfortably long way off.
                                 - Terry Pratchett, _Moving Pictures_