[U-Boot-Users] [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.
Signed-off-by: York Sun yorksun@freescale.com Signed-off-by: Timur Tabi timur@freescale.com --- include/configs/MPC8313ERDB.h | 14 ++++++-------- 1 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/include/configs/MPC8313ERDB.h b/include/configs/MPC8313ERDB.h index 6eec240..f9fa535 100644 --- a/include/configs/MPC8313ERDB.h +++ b/include/configs/MPC8313ERDB.h @@ -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
Hi York,
York Sun wrote:
Change LCRR clock ratio from 2 to 4 to commodate VSC7385. Correct TSEC1 vs TSEC2 assignment. Define ETHADDR and ETH1ADDR always.
Signed-off-by: York Sun yorksun@freescale.com Signed-off-by: Timur Tabi timur@freescale.com
include/configs/MPC8313ERDB.h | 14 ++++++-------- 1 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/include/configs/MPC8313ERDB.h b/include/configs/MPC8313ERDB.h index 6eec240..f9fa535 100644 --- a/include/configs/MPC8313ERDB.h +++ b/include/configs/MPC8313ERDB.h @@ -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
Please also remove the default MAC address assignments.
#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
Ben,
On Thu, 2008-05-15 at 13:50 -0700, Ben Warren wrote:
-#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?
#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
York Sun wrote:
Ben,
On Thu, 2008-05-15 at 13:50 -0700, Ben Warren wrote:
-#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?
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.
#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
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
In message 1210886469.880.45.camel@laptop you wrote:
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.
#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
Wolfgang Denk wrote:
In message 1210886469.880.45.camel@laptop you wrote:
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.
In message 482CB880.6050300@freescale.com 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
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
Wolfgang Denk wrote:
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.
In message 482CBCA4.2050009@freescale.com 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
Wolfgang Denk wrote:
In message 482CBCA4.2050009@freescale.com 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.
Timur Tabi wrote:
Wolfgang Denk wrote:
In message 482CBCA4.2050009@freescale.com 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.
That seems reasonable. Wolfgang, if you agree, please pull York's patch directly.
regards, Ben
In message 482CD47E.9010003@gmail.com 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.
...
That seems reasonable. Wolfgang, if you agree, please pull York's patch directly.
Sorry, I missed this one.
Best regards,
Wolfgang Denk
In message 12108831871831-git-send-email-yorksun@freescale.com you wrote:
Change LCRR clock ratio from 2 to 4 to commodate VSC7385. Correct TSEC1 vs TSEC2 assignment. Define ETHADDR and ETH1ADDR always.
Signed-off-by: York Sun yorksun@freescale.com Signed-off-by: Timur Tabi timur@freescale.com
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
participants (4)
-
Ben Warren -
Timur Tabi -
Wolfgang Denk -
York Sun