| Message ID | 1320857454-15207-1-git-send-email-hs@denx.de |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, Nov 9, 2011 at 9:50 AM, Heiko Schocher <hs@denx.de> wrote: > since commits: > davinci: emac: add support for more than 1 PHYs > 062fe7d332c28ede25626f448681e43d76bb312e > > davinci: remove obsolete macro CONFIG_EMAC_MDIO_PHY_NUM > fb1d6332b5430b90a8fa8ebab709f33a60e9f816 [snip] > - MAX_PHY from 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!
Hello Tom, Tom Rini wrote: > On Wed, Nov 9, 2011 at 9:50 AM, Heiko Schocher <hs@denx.de> wrote: >> since commits: >> davinci: emac: add support for more than 1 PHYs >> 062fe7d332c28ede25626f448681e43d76bb312e >> >> davinci: remove obsolete macro CONFIG_EMAC_MDIO_PHY_NUM >> fb1d6332b5430b90a8fa8ebab709f33a60e9f816 > [snip] >> - MAX_PHY from 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 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
On Wed, Nov 9, 2011 at 9:43 PM, Heiko Schocher <hs@denx.de> wrote: > Hello Tom, > > Tom Rini wrote: >> On Wed, Nov 9, 2011 at 9:50 AM, Heiko Schocher <hs@denx.de> wrote: >>> since commits: >>> davinci: emac: add support for more than 1 PHYs >>> 062fe7d332c28ede25626f448681e43d76bb312e >>> >>> davinci: remove obsolete macro CONFIG_EMAC_MDIO_PHY_NUM >>> fb1d6332b5430b90a8fa8ebab709f33a60e9f816 >> [snip] >>> - MAX_PHY from 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 > > 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? Yes, sounds like a good name to me, thanks.
diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c index fa31159..1648e24 100644 --- a/drivers/net/davinci_emac.c +++ b/drivers/net/davinci_emac.c @@ -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++) {
since commits: davinci: emac: add support for more than 1 PHYs 062fe7d332c28ede25626f448681e43d76bb312e 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() Signed-off-by: Heiko Schocher <hs@denx.de> Cc: Sandeep Paulraj <s-paulraj@ti.com> Cc: Albert ARIBAUD <albert.u.boot@aribaud.net> Cc: Wolfgang Denk <wd@denx.de> Cc: Manjunath Hadli <manjunath.hadli@ti.com> Cc: Prabhakar Lad <prabhakar.csengg@gmail.com> Cc: Mike Frysinger <vapier@gentoo.org> --- - 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(-)