Question about settings array in phy.c
Subsystems (2)
In drivers/net/phy/phy.c there is an array of struct phy_setting named settings that is used when auto negotiation is disabled to sanitize the phy settings. This array uses only *baseT* features for validating the various speed/duplex combinations. This may result in not finding the correct array entry in regards to speed and duplex if the *baseT* feature is not supported.
For example, a 10GbE phy that supports KR mode would end up not matching an entry and defaulting to the last entry of the array (SPEED_10 and DUPLEX_HALF) which is not what is desired.
Is it appropriate to be able to extend this array? Either by adding additional array entries or extending the "setting" to include other features? Or is there a specific reason that only the *baseT* values are used?
Examples of changing the array:
Add a new entry
{
.speed = SPEED_10000,
.duplex = DUPLEX_FULL,
.setting = SUPPORTED_10000baseKR_Full,
},or extend current entry
{
.speed = SPEED_10000,
.duplex = DUPLEX_FULL,
.setting = SUPPORTED_10000baseT_Full |
SUPPORTED_10000baseKR_Full,
},Thanks,
Tom
Hi Tom,
2014-07-11 8:50 GMT-07:00 Tom Lendacky <[email protected]>:
Quoted 6 lines
In
drivers/net/phy/phy.cthere is an array of structphy_settingnamed settings that is used when auto negotiation is disabled to sanitize the phy settings. This array uses only *baseT* features for validating the various speed/duplex combinations. This may result in not finding the correct array entry in regards to speed and duplex if the *baseT* feature is not supported.
Part of the reason is that libphy was designed around 2006-2007 and initially only supported 10/100/1000 PHY devices, and until recently it did not support 10GbE PHYs at all.
Quoted 9 lines
For example, a 10GbE phy that supports KR mode would end up not matching an entry and defaulting to the last entry of the array (
SPEED_10andDUPLEX_HALF) which is not what is desired.Is it appropriate to be able to extend this array? Either by adding additional array entries or extending the "setting" to include other features? Or is there a specific reason that only the *baseT* values are used?
It seems to me like it will be easier to refine/extend the matching logic by adding a new entries which contain a single 'setting' to be matched against, such as the one you show below:
Quoted 20 lines
Examples of changing the array:
Add a new entry { .speed = SPEED_10000, .duplex = DUPLEX_FULL, .setting = SUPPORTED_10000baseKR_Full, }, or extend current entry { .speed = SPEED_10000, .duplex = DUPLEX_FULL, .setting = SUPPORTED_10000baseT_Full | SUPPORTED_10000baseKR_Full, },Thanks,
Tom
Signature · 1 line
Florian