arm, davinci_emac: fixup
PATCH 1/1 arm, davinci_emac: fixup
since commits:
davinci: emac: add support for more than 1 PHYs062fe7d332c28ede25626f448681e43d76bb312e
davinci: remove obsolete macro CONFIG_EMAC_MDIO_PHY_NUM fb1d6332b5430b90a8fa8ebab709f33a60e9f816
I get following warning on the enbw_cmc board:
Err: serial Net: 5 ETH PHY detected miiphy_register: non unique device name 'KSZ8873 @ 0x01'
DaVinci-EMAC
Hit any key to stop autoboot: 0
Also I see some debug printfs:
=> run load
+ emac_close
+ emac_ch_teardown
- emac_ch_teardown
+ emac_ch_teardown
- emac_ch_teardown
- emac_close
+ emac_open
- emac_open
Using DaVinci-EMAC device
reason is 062fe7d332c28ede25626f448681e43d76bb312e new define MAX_PHY. This is set to 3! I get on this board 5 active phys, so this leads in wrong memory writes ...
so I changed:
- MAX_PHY from 3 to 7
- print an error message if more then MAX_PHYs are detected.
- fill the active_phy_addr array in a for loop with 0xff
- changed printf() in debug_emac()
--- drivers/net/davinci_emac.c | 17 +++++++++++------ 1 files changed, 11 insertions(+), 6 deletions(-)
drivers/net/davinci_emac.c +11 -6
@@ -85,7 +85,7 @@ static int emac_rx_queue_active = 0;/* Receive packet buffers */ static unsigned char emac_rx_buffers[EMAC_MAX_RX_BUFFERS * (EMAC_MAX_ETHERNET_PKT_SIZE +EMAC_PKT_ALIGN )]; -#define MAX_PHY 3 +#define MAX_PHY 7/* PHY address for a discovered PHY (0xff - not found) */ static u_int8_t active_phy_addr[MAX_PHY ] = {0xff ,0xff ,0xff };
@@ -160,9 +160,8 @@ static int davinci_eth_phy_detect(void)int j;unsigned int count =0 ; - active_phy_addr[0 ] =0xff ; - active_phy_addr[1 ] =0xff ; - active_phy_addr[2 ] =0xff ; +for (i =0 ; i <MAX_PHY ; i++) + active_phy_addr[i] =0xff ;udelay (1000 ); phy_act_state =readl (&adap_mdio->ALIVE);
@@ -175,7 +174,13 @@ static int davinci_eth_phy_detect(void)for (i =0 , j =0 ; i <32 ; i++)if (phy_act_state & (1 << i)) { count++; - active_phy_addr[j++] = i; +if (count <MAX_PHY ) + active_phy_addr[j++] = i; +else { +printf ("%s: to much PHYs detected.\n" , + __func__); +break ; + } } num_phy = count;
@@ -752,7 +757,7 @@ int davinci_emac_initialize(void)if (!ret)return (0 );else -printf (" %d ETH PHY detected\n" , ret); +debug_emac (" %d ETH PHY detected\n" , ret);/* Get PHY ID and initialize phy_ops for a detected PHY */ for (i =0 ; i < num_phy; i++) {
Signature · 1 line
1.7.6.4
PATCH 1/1 arm, davinci_emac: fixup
Dear Heiko Schocher,
In message <[email protected]> you wrote:
...
drivers/net/davinci_emac.c static int davinci_eth_phy_detect(void) · line 183
Quoted 7 lines
drivers/net/davinci_emac.c +7
-0+if (count <MAX_PHY ) + active_phy_addr[j++] = i; +else { +printf ("%s: to much PHYs detected.\n" , + __func__); +break ; + }
Coding Style: if the "else" branch uses braces, the "if" branch shall use braces, too.
Also, please s/too much/too many/ in the printf() above.
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 Be kind to unkind people - they need it the most.
PATCH 1/1 arm, davinci_emac: fixup
Hello Wolfgang,
Quoted 4 lines · Wolfgang Denk
Dear Heiko Schocher,
In message <[email protected]> you wrote:
...
Quoted 10 lines
drivers/net/davinci_emac.c static int davinci_eth_phy_detect(void) · line 183
Quoted 7 lines
drivers/net/davinci_emac.c +7
-0+if (count <MAX_PHY ) + active_phy_addr[j++] = i; +else { +printf ("%s: to much PHYs detected.\n" , + __func__); +break ; + }Coding Style: if the "else" branch uses braces, the "if" branch shall use braces, too.
Hups, you are right, just checked only with checkpatch ... sorry. Changed!
Quoted 1 line
Also, please s/too much/too many/ in the
printf()above.
Done.
Thanks!
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
PATCH 1/1 arm, davinci_emac: fixup
In reply to Heiko SchocherHi Heiko,
Quoted 77 lines · Heiko Schocher
since commits:
davinci: emac: add support for more than 1 PHYs062fe7d332c28ede25626f448681e43d76bb312edavinci: remove obsolete macro
CONFIG_EMAC_MDIO_PHY_NUMfb1d6332b5430b90a8fa8ebab709f33a60e9f816I get following warning on the
enbw_cmcboard:Err: serial Net: 5 ETH PHY detected miiphy_register: non unique device name 'KSZ8873 @ 0x01'DaVinci-EMAC
Hit any key to stop autoboot: 0Also I see some debug printfs:
=> run load
+emac_close
+emac_ch_teardown
-emac_ch_teardown
+emac_ch_teardown
-emac_ch_teardown
-emac_close
+emac_open
-emac_open
Using DaVinci-EMAC devicereason is
062fe7d332c28ede25626f448681e43d76bb312enew defineMAX_PHY. This is set to 3! I get on this board 5 active phys, so this leads in wrong memory writes ...so I changed:
-
MAX_PHYfrom 3 to 7
- print an error message if more thenMAX_PHYsare detected.
- fill theactive_phy_addrarray in a for loop with0xff
- changedprintf()indebug_emac()Signed-off-by Heiko Schocher <[email protected]>Cc Sandeep Paulraj <[email protected]>Cc Albert ARIBAUD <[email protected]>Cc Wolfgang Denk <[email protected]>Cc Manjunath Hadli <[email protected]>--- drivers/net/davinci_emac.c | 17 +++++++++++------ 1 files changed, 11 insertions(+), 6 deletions(-)drivers/net/davinci_emac.c +1
-1@@ -85,7 +85,7 @@ static int emac_rx_queue_active = 0;/* Receive packet buffers */ static unsigned char emac_rx_buffers[EMAC_MAX_RX_BUFFERS * (EMAC_MAX_ETHERNET_PKT_SIZE +EMAC_PKT_ALIGN )]; -#define MAX_PHY 3 +#define MAX_PHY 7/* PHY address for a discovered PHY (0xff - not found) */ static
u_int8_tactive_phy_addr[MAX_PHY] = {0xff,0xff,0xff};drivers/net/davinci_emac.c +2
-3@@ -160,9 +160,8 @@ static int davinci_eth_phy_detect(void)int j;unsigned int count =0 ; - active_phy_addr[0 ] =0xff ; - active_phy_addr[1 ] =0xff ; - active_phy_addr[2 ] =0xff ; +for (i =0 ; i <MAX_PHY ; i++) + active_phy_addr[i] =0xff ;udelay (1000 ); phy_act_state =readl (&adap_mdio->ALIVE);
drivers/net/davinci_emac.c static int davinci_eth_phy_detect(void) · line 182
Quoted 10 lines
drivers/net/davinci_emac.c +5
-1@@ -175,7 +174,13 @@ static int davinci_eth_phy_detect(void)for (i =0 , j =0 ; i <32 ; i++)if (phy_act_state & (1 << i)) { count++; - active_phy_addr[j++] = i; +if (count <MAX_PHY ) + active_phy_addr[j++] = i; +else { +printf ("%s: to much PHYs detected.\n" , + __func__);
why not make here count = 0 and then break, so that later in davinci_emac_initialize() it wont initializes the phy's
Regards,
--Prabhakar Lad
Quoted 22 lines
drivers/net/davinci_emac.c +2
-0+break ; + } }num_phy = count;drivers/net/davinci_emac.c +1
-2@@ -752,7 +757,7 @@ int davinci_emac_initialize(void)if (!ret)return (0 );else -printf (" %d ETH PHY detected\n" , ret); +debug_emac (" %d ETH PHY detected\n" , ret);/* Get PHY ID and initialize phy_ops for a detected PHY */ for (i =0 ; i < num_phy; i++) { --1.7.6.4
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
PATCH 1/1 arm, davinci_emac: fixup
Hello Prabhakar Lad,
Quoted 3 lines · Prabhakar Lad
Hi Heiko,
On Wed, Nov 9, 2011 at 11:56 AM, Heiko Schocher <[email protected]> wrote:
[...]
Quoted 13 lines
Quoted 13 lines
Signed-off-by Heiko Schocher <[email protected]>Cc Sandeep Paulraj <[email protected]>Cc Albert ARIBAUD <[email protected]>Cc Wolfgang Denk <[email protected]>Cc Manjunath Hadli <[email protected]>--- drivers/net/davinci_emac.c | 17 +++++++++++------ 1 files changed, 11 insertions(+), 6 deletions(-)drivers/net/davinci_emac.c +0
-0
[...]
Quoted 5 lines
Quoted 5 lines
drivers/net/davinci_emac.c +0
-1@@ -175,7 +174,13 @@ static int davinci_eth_phy_detect(void)for (i =0 , j =0 ; i <32 ; i++)if (phy_act_state & (1 << i)) { count++; - active_phy_addr[j++] = i;
Quoted 8 lines
drivers/net/davinci_emac.c static int davinci_eth_phy_detect(void) · line 185
Quoted 6 lines
drivers/net/davinci_emac.c +5
-0+if (count <MAX_PHY ) + active_phy_addr[j++] = i; +else { +printf ("%s: to much PHYs detected.\n" , + __func__);why not make here count = 0 and then break, so that later in
davinci_emac_initialize()it wont initializes the phy's
I prefer here the error printf, because you see immediately what is wrong...
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
PATCH 1/1 arm, davinci_emac: fixup
Hi Heiko,
Quoted 35 lines · Heiko Schocher
Hello Prabhakar Lad,
Quoted 3 lines · Prabhakar Lad
Hi Heiko,
On Wed, Nov 9, 2011 at 11:56 AM, Heiko Schocher <[email protected]> wrote:
[...]
Quoted 13 lines
Quoted 13 lines
Signed-off-by Heiko Schocher <[email protected]>Cc Sandeep Paulraj <[email protected]>Cc Albert ARIBAUD <[email protected]>Cc Wolfgang Denk <[email protected]>Cc Manjunath Hadli <[email protected]>--- drivers/net/davinci_emac.c | 17 +++++++++++------ 1 files changed, 11 insertions(+), 6 deletions(-)drivers/net/davinci_emac.c +0
-0[...]
Quoted 5 lines
Quoted 5 lines
drivers/net/davinci_emac.c +0
-1@@ -175,7 +174,13 @@ static int davinci_eth_phy_detect(void)for (i =0 , j =0 ; i <32 ; i++)if (phy_act_state & (1 << i)) { count++; - active_phy_addr[j++] = i;Quoted 8 lines
drivers/net/davinci_emac.c static int davinci_eth_phy_detect(void) · line 185
Quoted 6 lines
drivers/net/davinci_emac.c +5
-0+if (count <MAX_PHY ) + active_phy_addr[j++] = i; +else { +printf ("%s: to much PHYs detected.\n" , + __func__);why not make here count = 0 and then break, so that later in
davinci_emac_initialize()it wont initializes the phy's
Quoted 4 lines
I prefer here the error printf, because you see immediately what is wrong...
Agreed to have a printf, I was suggesting to even have a statement
count = 0;
in that block, if you don't make count zero laterdavinci_emac_initialize() function
it will proceed further in initializing the phys , which i believe is
not correct.
Regards,
--Prabhakar Lad
Quoted 6 lines
bye,
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
PATCH 1/1 arm, davinci_emac: fixup
Hello Prabhakar Lad,
Quoted 41 lines · Prabhakar Lad
Hi Heiko,
Quoted 37 lines · Heiko Schocher
Hello Prabhakar Lad,
Quoted 3 lines · Prabhakar Lad
Hi Heiko,
On Wed, Nov 9, 2011 at 11:56 AM, Heiko Schocher <[email protected]> wrote:
[...]
Quoted 13 lines
Quoted 13 lines
Signed-off-by Heiko Schocher <[email protected]>Cc Sandeep Paulraj <[email protected]>Cc Albert ARIBAUD <[email protected]>Cc Wolfgang Denk <[email protected]>Cc Manjunath Hadli <[email protected]>--- drivers/net/davinci_emac.c | 17 +++++++++++------ 1 files changed, 11 insertions(+), 6 deletions(-)drivers/net/davinci_emac.c +0
-0[...]
Quoted 5 lines
Quoted 5 lines
drivers/net/davinci_emac.c +0
-1@@ -175,7 +174,13 @@ static int davinci_eth_phy_detect(void)for (i =0 , j =0 ; i <32 ; i++)if (phy_act_state & (1 << i)) { count++; - active_phy_addr[j++] = i;Quoted 8 lines
drivers/net/davinci_emac.c static int davinci_eth_phy_detect(void) · line 185
Quoted 6 lines
drivers/net/davinci_emac.c +5
-0+if (count <MAX_PHY ) + active_phy_addr[j++] = i; +else { +printf ("%s: to much PHYs detected.\n" , + __func__);why not make here count = 0 and then break, so that later in
davinci_emac_initialize()it wont initializes the phy'sI prefer here the error printf, because you see immediately what is wrong...
Quoted 6 lines
Quoted 1 line
Agreed to have a printf, I was suggesting to even have a statement
count = 0;
in that block, if you don't make count zero laterdavinci_emac_initialize()function
it will proceed further in initializing the phys , which i believe is
not correct.
Ah, Ok, yes, I change this, thanks!
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
PATCH v2 1/1 arm, davinci_emac: fixup
In reply to Heiko Schochersince commits:
davinci: emac: add support for more than 1 PHYs062fe7d332c28ede25626f448681e43d76bb312e
davinci: remove obsolete macro CONFIG_EMAC_MDIO_PHY_NUM fb1d6332b5430b90a8fa8ebab709f33a60e9f816
I get following warning on the enbw_cmc board:
Err: serial Net: 5 ETH PHY detected miiphy_register: non unique device name 'KSZ8873 @ 0x01'
DaVinci-EMAC
Hit any key to stop autoboot: 0
Also I see some debug printfs:
=> run load
+ emac_close
+ emac_ch_teardown
- emac_ch_teardown
+ emac_ch_teardown
- emac_ch_teardown
- emac_close
+ emac_open
- emac_open
Using DaVinci-EMAC device
reason is 062fe7d332c28ede25626f448681e43d76bb312e new define MAX_PHY. This is set to 3! I get on this board 5 active phys, so this leads in wrong memory writes ...
so I changed:
- MAX_PHY from 3 to 7
- print an error message if more then MAX_PHYs are detected.
- fill the active_phy_addr array in a for loop with 0xff
- changed printf() in debug_emac()
---
- changes for v2:
- add comments from Wolfgang Denk
- Codingstyle cleanup
if the "else" branch uses braces, the "if" branch shall use braces, too.
- s/too much/too many/ in the added printf()
- add comment from Prabhakar Lad:
- add count = 0 in error case, so that no phys are initialized later.
--- drivers/net/davinci_emac.c | 18 ++++++++++++------ 1 files changed, 12 insertions(+), 6 deletions(-)
drivers/net/davinci_emac.c +12 -6
@@ -85,7 +85,7 @@ static int emac_rx_queue_active = 0;/* Receive packet buffers */ static unsigned char emac_rx_buffers[EMAC_MAX_RX_BUFFERS * (EMAC_MAX_ETHERNET_PKT_SIZE +EMAC_PKT_ALIGN )]; -#define MAX_PHY 3 +#define MAX_PHY 7/* PHY address for a discovered PHY (0xff - not found) */ static u_int8_t active_phy_addr[MAX_PHY ] = {0xff ,0xff ,0xff };
@@ -160,9 +160,8 @@ static int davinci_eth_phy_detect(void)int j;unsigned int count =0 ; - active_phy_addr[0 ] =0xff ; - active_phy_addr[1 ] =0xff ; - active_phy_addr[2 ] =0xff ; +for (i =0 ; i <MAX_PHY ; i++) + active_phy_addr[i] =0xff ;udelay (1000 ); phy_act_state =readl (&adap_mdio->ALIVE);
@@ -175,7 +174,14 @@ static int davinci_eth_phy_detect(void)for (i =0 , j =0 ; i <32 ; i++)if (phy_act_state & (1 << i)) { count++; - active_phy_addr[j++] = i; +if (count <MAX_PHY ) { + active_phy_addr[j++] = i; + }else { +printf ("%s: to many PHYs detected.\n" , + __func__); + count =0 ; +break ; + } } num_phy = count;
@@ -752,7 +758,7 @@ int davinci_emac_initialize(void)if (!ret)return (0 );else -printf (" %d ETH PHY detected\n" , ret); +debug_emac (" %d ETH PHY detected\n" , ret);/* Get PHY ID and initialize phy_ops for a detected PHY */ for (i =0 ; i < num_phy; i++) {
Signature · 1 line
1.7.6.4
PATCH v2 1/1 arm, davinci_emac: fixup
the summary "fixup" tells us nothing as to what you're fixing. could you use a little more descriptive summary ?
-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/20111109/e78bff57/attachment.pgp
PATCH v3 1/1 arm, davinci_emac: fix driver bug if more then 3 PHYs are detected
In reply to Heiko Schochersince commits:
davinci: emac: add support for more than 1 PHYs062fe7d332c28ede25626f448681e43d76bb312e
davinci: remove obsolete macro CONFIG_EMAC_MDIO_PHY_NUM fb1d6332b5430b90a8fa8ebab709f33a60e9f816
I get following warning on the enbw_cmc board:
Err: serial Net: 5 ETH PHY detected miiphy_register: non unique device name 'KSZ8873 @ 0x01'
DaVinci-EMAC
Hit any key to stop autoboot: 0
Also I see some debug printfs:
=> run load
+ emac_close
+ emac_ch_teardown
- emac_ch_teardown
+ emac_ch_teardown
- emac_ch_teardown
- emac_close
+ emac_open
- emac_open
Using DaVinci-EMAC device
reason is 062fe7d332c28ede25626f448681e43d76bb312e new define MAX_PHY. This is set to 3! I get on this board 5 active phys, so this leads in wrong memory writes ...
so I changed:
- MAX_PHY from 3 to 7
- print an error message if more then MAX_PHYs are detected.
- fill the active_phy_addr array in a for loop with 0xff
- changed printf() in debug_emac()
---
- changes for v2:
- add comments from Wolfgang Denk
- Codingstyle cleanup
if the "else" branch uses braces, the "if" branch shall use braces, too.
- s/too much/too many/ in the added printf()
- add comment from Prabhakar Lad:
- add count = 0 in error case, so that no phys are initialized later.
- changes for v3:
- add comment from Mike Frysinger
change subject line to a hopefully more descriptive summary
drivers/net/davinci_emac.c | 18 ++++++++++++------ 1 files changed, 12 insertions(+), 6 deletions(-)
drivers/net/davinci_emac.c +12 -6
@@ -85,7 +85,7 @@ static int emac_rx_queue_active = 0;/* Receive packet buffers */ static unsigned char emac_rx_buffers[EMAC_MAX_RX_BUFFERS * (EMAC_MAX_ETHERNET_PKT_SIZE +EMAC_PKT_ALIGN )]; -#define MAX_PHY 3 +#define MAX_PHY 7/* PHY address for a discovered PHY (0xff - not found) */ static u_int8_t active_phy_addr[MAX_PHY ] = {0xff ,0xff ,0xff };
@@ -160,9 +160,8 @@ static int davinci_eth_phy_detect(void)int j;unsigned int count =0 ; - active_phy_addr[0 ] =0xff ; - active_phy_addr[1 ] =0xff ; - active_phy_addr[2 ] =0xff ; +for (i =0 ; i <MAX_PHY ; i++) + active_phy_addr[i] =0xff ;udelay (1000 ); phy_act_state =readl (&adap_mdio->ALIVE);
@@ -175,7 +174,14 @@ static int davinci_eth_phy_detect(void)for (i =0 , j =0 ; i <32 ; i++)if (phy_act_state & (1 << i)) { count++; - active_phy_addr[j++] = i; +if (count <MAX_PHY ) { + active_phy_addr[j++] = i; + }else { +printf ("%s: to many PHYs detected.\n" , + __func__); + count =0 ; +break ; + } } num_phy = count;
@@ -752,7 +758,7 @@ int davinci_emac_initialize(void)if (!ret)return (0 );else -printf (" %d ETH PHY detected\n" , ret); +debug_emac (" %d ETH PHY detected\n" , ret);/* Get PHY ID and initialize phy_ops for a detected PHY */ for (i =0 ; i < num_phy; i++) {
Signature · 1 line
1.7.6.4
PATCH v3 1/1 arm, davinci_emac: fix driver bug if more then 3 PHYs are detected
Quoted 6 lines · Heiko Schocher
since commits:
davinci: emac: add support for more than 1 PHYs062fe7d332c28ede25626f448681e43d76bb312edavinci: remove obsolete macro
CONFIG_EMAC_MDIO_PHY_NUMfb1d6332b5430b90a8fa8ebab709f33a60e9f816
[snip]
Quoted 1 line
-
MAX_PHYfrom 3 to 7
Why don't we add a CONFIG here and default to 3, ie #ifndef CONFIG_SYS_something
#define CONFIG_SYS_something 3
#endif
Thanks!
Signature · 1 line
Tom
PATCH v3 1/1 arm, davinci_emac: fix driver bug if more then 3 PHYs are detected
Hello Tom,
Quoted 7 lines · Tom Rini
Quoted 6 lines · Heiko Schocher
since commits:
davinci: emac: add support for more than 1 PHYs062fe7d332c28ede25626f448681e43d76bb312edavinci: remove obsolete macro
CONFIG_EMAC_MDIO_PHY_NUMfb1d6332b5430b90a8fa8ebab709f33a60e9f816
Quoted 7 lines
[snip]
Quoted 1 line
-
MAX_PHYfrom 3 to 7Why don't we add a CONFIG here and default to 3, ie #ifndef
CONFIG_SYS_something
#defineCONFIG_SYS_something3
#endif
Do we really need a config option for this? Why 3 or 7 as I did? Shouldn't we set this define to 32, as this is the max possible PHYs? Ok, we loose some RAM with this option ...
Would CONFIG_SYS_DAVINCI_EMAC_PHY_COUNT a good name?
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
PATCH v3 1/1 arm, davinci_emac: fix driver bug if more then 3 PHYs are detected
Quoted 18 lines · Heiko Schocher
Hello Tom,
Quoted 7 lines · Tom Rini
Quoted 6 lines · Heiko Schocher
since commits:
davinci: emac: add support for more than 1 PHYs062fe7d332c28ede25626f448681e43d76bb312edavinci: remove obsolete macro
CONFIG_EMAC_MDIO_PHY_NUMfb1d6332b5430b90a8fa8ebab709f33a60e9f816Quoted 7 lines
[snip]
Quoted 1 line
-
MAX_PHYfrom 3 to 7Why don't we add a CONFIG here and default to 3, ie #ifndef
CONFIG_SYS_something
#defineCONFIG_SYS_something3
#endif
Quoted 5 lines
Do we really need a config option for this? Why 3 or 7 as I did? Shouldn't we set this define to 32, as this is the max possible PHYs? Ok, we loose some RAM with this option ...
Would
CONFIG_SYS_DAVINCI_EMAC_PHY_COUNTa good name?
Yes, sounds like a good name to me, thanks.
Signature · 1 line
Tom
PATCH v4 1/1 arm, davinci_emac: fix driver bug if more then 3 PHYs are detected
In reply to Heiko Schochersince commits:
davinci: emac: add support for more than 1 PHYs062fe7d332c28ede25626f448681e43d76bb312e
davinci: remove obsolete macro CONFIG_EMAC_MDIO_PHY_NUM fb1d6332b5430b90a8fa8ebab709f33a60e9f816
I get following warning on the enbw_cmc board:
Err: serial Net: 5 ETH PHY detected miiphy_register: non unique device name 'KSZ8873 @ 0x01'
DaVinci-EMAC
Hit any key to stop autoboot: 0
Also I see some debug printfs:
=> run load
+ emac_close
+ emac_ch_teardown
- emac_ch_teardown
+ emac_ch_teardown
- emac_ch_teardown
- emac_close
+ emac_open
- emac_open
Using DaVinci-EMAC device
reason is 062fe7d332c28ede25626f448681e43d76bb312e new define MAX_PHY. This is set to 3! I get on this board 5 active phys, so this leads in wrong memory writes ...
so I changed:
- define CONFIG_SYS_DAVINCI_EMAC_PHY_COUNT to set the MAX_PHY value, add a description in README for the new CONFIG_SYS option.
- print an error message if more then MAX_PHYs are detected.
- fill the active_phy_addr array in a for loop with 0xff
- changed printf() in debug_emac()
---
- changes for v2:
- add comments from Wolfgang Denk
- Codingstyle cleanup
if the "else" branch uses braces, the "if" branch shall use braces, too.
- s/too much/too many/ in the added printf()
- add comment from Prabhakar Lad:
- add count = 0 in error case, so that no phys are initialized later.
- changes for v3:
- add comment from Mike Frysinger
change subject line to a hopefully more descriptive summary
- changes for v4:
- add comment from Tom Rini:
introduce CONFIG_SYS_DAVINCI_EMAC_PHY_COUNT, so boards can define this, if they need a value != 3. Add this config option to the readme file
README | 6 ++++++ drivers/net/davinci_emac.c | 24 ++++++++++++++++-------- 2 files changed, 22 insertions(+), 8 deletions(-)
README +6 -0
@@ -1027,6 +1027,12 @@ The following options need to be configured: Define this to use i/o functions instead of macros (some hardware wont work with macros) + CONFIG_DRIVER_TI_EMAC + Support for davinci emac + + CONFIG_SYS_DAVINCI_EMAC_PHY_COUNT + Define this if you have more then 3 PHYs. + CONFIG_FTGMAC100 Support for Faraday's FTGMAC100 Gigabit SoC Ethernet
drivers/net/davinci_emac.c +16 -8
@@ -85,15 +85,17 @@ static int emac_rx_queue_active = 0;/* Receive packet buffers */ static unsigned char emac_rx_buffers[EMAC_MAX_RX_BUFFERS * (EMAC_MAX_ETHERNET_PKT_SIZE +EMAC_PKT_ALIGN )]; -#define MAX_PHY 3 +#ifndef CONFIG_SYS_DAVINCI_EMAC_PHY_COUNT +#define CONFIG_SYS_DAVINCI_EMAC_PHY_COUNT 3 +#endif /* PHY address for a discovered PHY (0xff - not found) */ -static u_int8_t active_phy_addr[MAX_PHY ] = {0xff ,0xff ,0xff }; +static u_int8_t active_phy_addr[CONFIG_SYS_DAVINCI_EMAC_PHY_COUNT ];/* number of PHY found active */ static u_int8_t num_phy; -phy_t phy[MAX_PHY ]; +phy_t phy[CONFIG_SYS_DAVINCI_EMAC_PHY_COUNT ];static int davinci_eth_set_mac_addr (struct eth_device *dev) {
@@ -160,9 +162,8 @@ static int davinci_eth_phy_detect(void)int j;unsigned int count =0 ; - active_phy_addr[0 ] =0xff ; - active_phy_addr[1 ] =0xff ; - active_phy_addr[2 ] =0xff ; +for (i =0 ; i <CONFIG_SYS_DAVINCI_EMAC_PHY_COUNT ; i++) + active_phy_addr[i] =0xff ;udelay (1000 ); phy_act_state =readl (&adap_mdio->ALIVE);
@@ -175,7 +176,14 @@ static int davinci_eth_phy_detect(void)for (i =0 , j =0 ; i <32 ; i++)if (phy_act_state & (1 << i)) { count++; - active_phy_addr[j++] = i; +if (count <CONFIG_SYS_DAVINCI_EMAC_PHY_COUNT ) { + active_phy_addr[j++] = i; + }else { +printf ("%s: to many PHYs detected.\n" , + __func__); + count =0 ; +break ; + } } num_phy = count;
@@ -752,7 +760,7 @@ int davinci_emac_initialize(void)if (!ret)return (0 );else -printf (" %d ETH PHY detected\n" , ret); +debug_emac (" %d ETH PHY detected\n" , ret);/* Get PHY ID and initialize phy_ops for a detected PHY */ for (i =0 ; i < num_phy; i++) {
Signature · 1 line
1.7.6.4
PATCH v4 1/1 arm, davinci_emac: fix driver bug if more then 3 PHYs are detected
Quoted 43 lines · Heiko Schocher
since commits:
davinci: emac: add support for more than 1 PHYs062fe7d332c28ede25626f448681e43d76bb312edavinci: remove obsolete macro
CONFIG_EMAC_MDIO_PHY_NUMfb1d6332b5430b90a8fa8ebab709f33a60e9f816I get following warning on the
enbw_cmcboard:Err: ? serial
Net: ? ?5 ETH PHY detectedmiiphy_register: non unique device name 'KSZ8873 @0x01' DaVinci-EMAC
Hit any key to stop autoboot: ?0Also I see some debug printfs:
=> run load
+emac_close
+emac_ch_teardown
-emac_ch_teardown
+emac_ch_teardown
-emac_ch_teardown
-emac_close
+emac_open
-emac_open
Using DaVinci-EMAC devicereason is
062fe7d332c28ede25626f448681e43d76bb312enew defineMAX_PHY. This is set to 3! I get on this board 5 active phys, so this leads in wrong memory writes ...so I changed:
- define
CONFIG_SYS_DAVINCI_EMAC_PHY_COUNTto set ?theMAX_PHYvalue, add a description in README ?for the newCONFIG_SYSoption.
- print an error message if more thenMAX_PHYsare ?detected.
- fill theactive_phy_addrarray in a for loop with ?0xff
- changedprintf()indebug_emac()
Quoted 8 lines
Signed-off-by Heiko Schocher <[email protected]>Cc Sandeep Paulraj <[email protected]>Cc Albert ARIBAUD <[email protected]>Cc Wolfgang Denk <[email protected]>Cc Manjunath Hadli <[email protected]>Cc Prabhakar Lad <[email protected]>Cc Mike Frysinger <[email protected]>Cc Tom Rini <[email protected]>
Thanks.
Signature · 1 line
Tom