Hi Florian/Andrew, Thank you for review comments.
On Thu, Oct 06, 2016 at 04:09:56AM -0700, Florian Fainelli wrote: > EXTERNAL EMAIL > > > 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. > Andrew, Do you want me to add new flag (mdix_ctrl) or keep it as it is? These changes are more relevant for mdix get status function. Do you want to me implement along with mdix get status function? > Raju, Andrew, does that work for you? > -- > Florian --- Thanks, Raju.