netdev - Re: [PATCH 2/2] at803x: double check SGMII side autoneg

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e2fbb91e-95ef-49a0-dfd9-0e6cc23925fa@gmail.com>
Date:   Wed, 24 May 2017 14:19:58 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Timur Tabi <timur@....qualcomm.com>, Andrew Lunn <andrew@...n.ch>
Cc:     Matthias May <matthias.may@...atec.com>,
        Zefir Kurtisi <zefir.kurtisi@...atec.com>,
        netdev@...r.kernel.org, David Miller <davem@...emloft.net>,
        Manoj Iyer <manoj.iyer@...onical.com>, jhugo@...eaurora.org
Subject: Re: [PATCH 2/2] at803x: double check SGMII side autoneg

On 05/24/2017 01:57 PM, Timur Tabi wrote:
> On 05/24/2017 02:34 PM, Andrew Lunn wrote:
>>> Ok, I'm going to debug this some more.  It turns out that the MAC
>>> side of
>>> the SGMII link can send an interrupt when it thinks that
>>> auto-negotiation is
>>> done.  I might be able to use this.
>>
>> You can use this for your board. But it still leaves the phy driver
>> broken for everybody else.
> 
> Wait, I thought you said the at803x driver was not broken, since it
> returns 0 when the SGMII side of the link hasn't finished auto-negotiating?
> 
>>> What function should my MAC driver call when it wants the phy core to
>>> call
>>> at803x_aneg_done again to see if autonegotiation is done?
>>
>> You want to trigger the PHY state machine. There is only one exported
>> API call to do this, phy_mac_interrupt(). But you are supposed to pass
>> the new link state. And your MAC driver has no idea of that, it does
>> not know if the copper side of the PHY is up.
> 
> My NIC has a feature called autopolling where it takes over the MDIO bus
> and regularly polls the link state.  When it detects that the link state
> has changed, it generates a MAC interrupt.  This is when I call
> phy_mac_interrupt() normally.
> 
> I think I can use the SGMII interrupt to also call phy_mac_interrupt().
> The problem is the from the copper side, the link is already up, so if I
> call phy_mac_interrupt() again and say the link is up, the phy core is
> going to ignore that.

The question is what the HW is auto-polling on? Most HW implementations
that I have seen either auto-poll and assume that BMSR.LINKSTATUS will
reflect what they want to, or they even let you specify which register
and bit(s) to monitor for link status. So which one is it here?

As Andrew already responded, this clashes with any reads/writes that the
PHY library could do, unless your HW is smart enough to serialize MDIO
reads/writes?

> 
>> So it might be better if you export phy_trigger_machine().
> 
> I'll test that, but that does seem a bit hackish.
> 
>>> Also, is there a way for the MAC driver to know that at803x_aneg_done()
>>> previously returned 0, and that it needs to tell the phy core to
>>> check again?
>>
>> Not that i know of. The MAC layer is not supposed to be messing around
>> in the PHY layer. However, just triggering the PHY state machine
>> should be enough.
> 
> Can you tell my how PHY_HAS_INTERRUPT is supposed to work?  How does the
> PHY send an interrupt?

PHY_HAS_INTERRUPT indicates that you have a dedicated interrupt line for
your PHY device and that you have the ability to implement a custom
interrupt handling callback in the PHY driver (ack_interrupt) that lets
you deal with all possible sorts of the interrupts that the PHY could
generate.

> 
> I'm starting to think that my NIC's autopolling feature is not
> compatible with phylib, and that I should use polling mode.

That's pretty much what Andrew told you about 5 emails ago...
-- 
Florian