On 01/18/2017 04:02 PM, Timur Tabi wrote: > On 01/18/2017 07:53 AM, Zefir Kurtisi wrote: >> No, not necessarily. The SGMII link default configuration is set such that >> you do >> not have to bother at all. > > Does the SGMII link need to make the speed and duplex of the external link? > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c#n50 > > > Here is where I can configure the SGMII link to match whatever phydev says the > external link is. This is code that was handed down to me. I've never really > understood what the purpose was. Shouldn't I just be able to configure the > SGMII > link to 1Gbs full-duplex, regardless of what the external link is set to? >
This is because the SGMII link is a transparent interface to the upper layer with no means to inform the other side of the link type in the lower layer. It always operates at 675MHz, which with two lines gives 1.25Gbps, which at 10/8 coding gives exactly 1Gbps net data rate. If the at803x's copper side autonegotiates to 1Gbps, the bits traversing over the SGMII match the copper side 1:1. In case the copper side autonegotiates to e.g. 100Mbps, each bit at the copper side on the SGMII bus is replicated and sent 10x times - or 100x times in case of 10Mbps. The MAC side of the ETH needs to be aware of how the SGMII data has to be interpreted, and this is why you have to set the bits you are referring to. >> That is, if in your case you see the warning popping up but the link always >> regains connection, then it is an ignorable false positive. > > Unfortunately, I can't really ignore it because genphy does this: > > $ ifup eth1 > [ 1034.754276] qcom-emac QCOM8070:00 eth1: Link is Up - 1Gbps/Full - flow > control > rx/tx > > But the at803x driver does this: > > $ ifup eth1 > [ 1020.507728] 803x_aneg_done: SGMII link is not ok > [ 1020.513517] qcom-emac QCOM8070:00 eth1: Link is Up - 1Gbps/Full - flow > control > rx/tx > [ 1020.522839] qcom-emac QCOM8070:00 eth1: Link is Up - 1Gbps/Full - flow > control > rx/tx > > Customers are going to complain. > Yes, the double link-status message is annoying - I was referring to the other message. To track down who is causing the additional message, I would proceed with following technique that helped me dig down a similar problem: since you control the events in question and there is no risk of flooding the kernel log, in the top of phy.c::phy_print_status() add a dump_stack() call. In the debug log ensure that all of the traces end up in the same phydev->adjust_link() callback handler (in your case emac_adjust_link()). In the gianfar's handler there is an additional check whether the link really changed before phy_print_status() is called, in emac_adjust_link() this happens unconditionally - maybe that is the problem you are facing. @Florian: is it safe to assume that when phydev->adjust_link() is called there was in fact a link change (link, duplex, pause), or does the handler have to double check for a change? I see some ETH drivers maintain a private old state instance for that, while others don't. Cheers, Zefir
