[U-Boot-Users] miiphy_speed() broken after latest patch...

I just noticed that Cogent CSB272 is now incorrectly reporting 1000Mbps speed after the following patch is applied to common/miiphyutil.c
* Patches by Travis Sawyer, 12 Mar 2004: - Fix Gigabit Ethernet support for 440GX - Add Gigabit Ethernet Support to MII PHY utilities
Cogent CSB272 board uses Intel LXT971A PHY on board and current code is being fooled by this patch.
According to datasheet of Intel LXT971A, PHY, PHY_1000BTSR register is listed among registers as 1000BASE-T/100BASE-T2 Status Register but it is marked as not implemented and no further data is available. So, the check against 0xFFFF is not working properly.
I have been monitoring the linuxppc embedded list and there appears to be incompatible register sets for 1000Mbps PHYs as well.
Perhaps we need to make the detection of 1000Mbps PHY included optionally based on a CONFIG_ macro and 1000MBps PHY boards define that variable. I have no use for extra code.
Best regards, Tolunay

In message 17331.216.110.51.8.1079651857.squirrel@www.orkun.us you wrote:
Perhaps we need to make the detection of 1000Mbps PHY included optionally based on a CONFIG_ macro and 1000MBps PHY boards define that variable. I have no use for extra code.
OK - can you please submit a patch, then?
Best regards,
Wolfgang Denk

In message 17331.216.110.51.8.1079651857.squirrel@www.orkun.us you wrote:
Perhaps we need to make the detection of 1000Mbps PHY included optionally based on a CONFIG_ macro and 1000MBps PHY boards define that variable. I have no use for extra code.
OK - can you please submit a patch, then?
I can but I would like to hear from Travis first.
I guess I can define CONFIG_PHY_1000 and make the new sections added to be included when depend on it. But, I am not sure which board(s) need this GigE PHY support because I will have to add the macro to their config header files.
Another thing to consider is incompatible GigE PHY register sets. So, perhaps we should have someting have CONFIG_PHY_1000_TYPE as well and use it like:
#if defined(CONFIG_PHY_1000_TYPE) #if CONFIG_PHY_1000_TYPE == CONFIG_PHY_MARVELL_88E1XXX ... marvell specific code ... #elif CONFIG_PHY_1000_TYPE == CONFIG_PHY_1000_GENERIC... ... assumed generic gigE PHY code ... #endif #endif
Best regards, Tolunay

Tolunay, Wolfgang:
See below:
On Thu, 2004-03-18 at 19:32, Tolunay Orkun wrote:
Perhaps we need to make the detection of 1000Mbps PHY included optionally based on a CONFIG_ macro and 1000MBps PHY boards define that variable. I have no use for extra code.
Ouch!
OK - can you please submit a patch, then?
I can but I would like to hear from Travis first.
PING!
Another thing to consider is incompatible GigE PHY register sets. So, perhaps we should have someting have CONFIG_PHY_1000_TYPE as well and use it like:
Incompatible!?! Then they do not follow the IEEE 802.3 standard. The only reason I felt comfortable putting the code in was I read the .3 standard wrt unsupported registers.
According to datasheet of Intel LXT971A, PHY, PHY_1000BTSR register is listed among registers as 1000BASE-T/100BASE-T2 Status Register but it is marked as not implemented and no further data is available. So, the check against 0xFFFF is not working properly.
So the LXT971A doesn't follow the 802.3 standard. Odd in that the intel lxt9762 does...
#if defined(CONFIG_PHY_1000_TYPE) #if CONFIG_PHY_1000_TYPE == CONFIG_PHY_MARVELL_88E1XXX ... marvell specific code ... #elif CONFIG_PHY_1000_TYPE == CONFIG_PHY_1000_GENERIC... ... assumed generic gigE PHY code ... #endif #endif
I deeply regret having broken the code!
By all means patch away! I do like the CONFIG_PHY_1000_TYPE suggestion.
Darn. And I was feeling like a hero too ;)
AFAIK, the only two boards that _*I*_ use GbE on is the XPedite1K and Ocotea boards, both having phys that behave (Xpedite has BCom phys, and they behave :)
Thanx,
Travis (I wonder what other phys don't return 0xFFFF, I only have 4 different types on hand ;)

Tolunay, Wolfgang:
See below:
On Thu, 2004-03-18 at 19:32, Tolunay Orkun wrote:
Another thing to consider is incompatible GigE PHY register sets. So, perhaps we should have someting have CONFIG_PHY_1000_TYPE as well and use it like:
Incompatible!?! Then they do not follow the IEEE 802.3 standard. The only reason I felt comfortable putting the code in was I read the .3 standard wrt unsupported registers.
Well, technically Intel LXT971A is a 10/100 PHY. It does not support GigE but strangely it refers to 1000 mode in status and register 0x9 and 0xA but then again puts a note and says not implemented. It looks like either half implemented GigE design sold as 10/100 PHY.
For this particular case LXT971A is returning 0x1fff for register 0xA.
(I wonder what other phys don't return 0xFFFF, I only have 4 different types on hand ;)
I am afraid I cannot say that. Perhaps folks on the list try "mii read" command to test if register 0xA is returning other than 0xffff.
Regards, Tolunay

According to datasheet of Intel LXT971A, PHY, PHY_1000BTSR register is listed among registers as 1000BASE-T/100BASE-T2 Status Register but it is marked as not implemented and no further data is available. So, the check against 0xFFFF is not working properly.
As a follow up Intel LXT971A is returning 0x1fff when PHY_1000BTSR is read.
participants (4)
-
Tolunay Orkun
-
Tolunay Orkun
-
Travis Sawyer
-
Wolfgang Denk