On 10/05/2016 12:18 AM, Andrew Lunn wrote: >>>> + phydev->mdix = ETH_TP_MDI_AUTO; >>> >>> Humm, interesting. The only other driver supporting mdix is the >>> Marvell one. It does not do this, it leaves it to its default value of >>> ETH_TP_MDI_INVALID. It does however interpret ETH_TP_MDI_INVALID as >>> meaning as ETH_TP_MDI_AUTO. >>> >>> There needs to be consistency here. You either need to do the same as >>> the Marvell driver, or you need to modify the Marvell driver to also >>> set phydev->mdix to ETH_TP_MDI_AUTO. >>> >> In Ethtool two variable i.e. eth_tp_mdix_ctrl, eth_tp_mdix use to update >> the status. But, driver header is having one variable i.e. mdix. >> Driver header should also have another variabl like mdix_ctrl. >> Then, Ethtool can get/set the Auto MDIX/MDI. >> In case, mdix is not configure with ETH_TP_MDI_AUTO, Ethtool shows error as >> "setting MDI not supported"
Agreed, we currently report eth_tp_mdi and eth_tp_mdi_ctrl using phydev->mdix, but this is too limiting. >> >> Please suggest me if you have any better method to fix this issue. > > Maybe we should add a new flag for the .flags member of the > phy_driver. If PHY_HAS_MDIX is set, the phy core will set phydev->mdix > to ETH_TP_MDI_AUTO? I agree with Raju here, like most other Ethernet drivers, we should allow PHY drivers to have an eth_tp_mdix_ctrl to indicate what is the configured MDI setting, and read eth_tp_mdi to indicate what is the current status, then ethtool can properly differentiate what is going on. Raju, Andrew, does that work for you? -- Florian
